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 miniPCIemodules, but the desired protocol must be configured at buildtime 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