[PATCH v5 06/12] arm: mvebu: clearfog: Add SATA mode flags

[Joel Johnson]

On 2020-01-27 23:06, Baruch Siach wrote:
> Hi Joel,
> 
> On Mon, Jan 27, 2020 at 01:01:50PM -0700, Joel Johnson wrote:
>> --- a/board/solidrun/clearfog/Kconfig
>> +++ b/board/solidrun/clearfog/Kconfig
>> +
>> +config CLEARFOG_CON2_SATA
>> +	bool "Use CON2 slot in SATA mode"
>> +	depends on !TARGET_CLEARFOG_BASE
>> +	help
>> +	  Use the CON2 port with SATA protocol instead of the default PCIe.
>> +	  The ClearFog port allows usage of either mSATA or miniPCIe
>> +	  modules, but the desired protocol must be configured at build
>> +	  time since it affects the SerDes topology layout.
>> +
>> --- a/board/solidrun/clearfog/clearfog.c
>> +++ b/board/solidrun/clearfog/clearfog.c
>> +	if (IS_ENABLED(CONFIG_CLEARFOG_CON2_SATA) &&
>> +	    !IS_ENABLED(CONFIG_TARGET_CLEARFOG_BASE)) {
> 
> This second condition looks redundant. CONFIG_CLEARFOG_CON2_SATA 
> already
> depends on !CONFIG_TARGET_CLEARFOG_BASE at the Kconfig level above.

It is redundant between the config and code sides - I viewed the check 
in code and the final check and the config entry a convenience to 
indicate to a configuring user that an option isn't available. If 
there's a prevailing standard in U-Boot to treat the configuration as 
pristine or otherwise not want the redundancy, I wouldn't have an issue 
removing the double-check in code.

> Looks good to me otherwise.
> 
> Have you had a chance to test mSATA?

Yes, I added a brief overview to the cover letter - I tested on a 
ClearFog Base and verified that the CON3 setting does in fact enable 
SATA, detect the drive, and both U-Boot and Linux have access to the 
block device. Strictly speaking I only tested CON3 and not CON2, but 
both run identical parallel paths.

Joel