Hi Stephen,
-----Original Message----- From: Stephen Warren [mailto:swarren@wwwdotorg.org] Sent: 2016年9月21日 5:15 To: Wenyou Yang - A41535 Wenyou.Yang@microchip.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Stephen Warren swarren@nvidia.com Subject: Re: [U-Boot] [PATCH v2] dm/mmc: gen_atmel_mci: Add driver model support for mci
On 09/19/2016 09:05 PM, Wenyou Yang wrote:
Add the driver model support for Atmel mci while retaining the existing legacy code. This allows the driver to support boards that have converted to driver model as well as those that have not.
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
+config GENERIC_ATMEL_MCI
- bool "Atmel Multimedia Card Interface support"
- depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
- help
This enables support for Atmel High Speed Multimedia Card Interface(HSMCI), which supports the MultiMedia Card (MMC) Specification V4.3,the SD Memory Card Specification V2.0, the SDIO V2.0 specificationand CE-ATA V1.1.This seems unrelated to the actual driver conversion. It's more like part of a convert-the-option-to-Kconfig patch. I suspect it should be removed for now?
Yes, it is unrelated. It seems not good on this patch.
But here, only add the option, doesn't enable.
I think it is somewhat acceptable.
I think you need to remove "#define GENERIC_ATMEL_MCI" from include/configs/*.h and add it to configs/*_defconfig as part of the patch that adds this Kconfig option.
+#ifndef CONFIG_DM_MMC /* Setup for MCI Clock and Block Size */ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen) { struct atmel_mci_priv *priv = mmc->priv;
- atmel_mci_t *mci = priv->mci; u32 bus_hz = get_mci_clk_rate();
+#else +static void mci_set_mode(struct atmel_mci_priv *priv, u32 hz, u32 +blklen) {
- struct mmc *mmc = &priv->mmc;
- u32 bus_hz = priv->bus_clk_rate;
+#endif
It's technically CONFIG_DM_MMC_OPS that changes that function signature, I believe. This code could allow someone to enable just CONFIG_DM_MMC but not CONFIG_DM_MMC_OPS and then see a build error. Once everything is converted to Kconfig that won't be possible due to the "depends on" you added above, but while the option is still selected by config.h files, this is possible.
Still, this is a minor issue so if you're going to do the config.h -> defconfig conversion soon, maybe just ignore this.
Yes, I will do the config.h soon.
Best Regards, Wenyou Yang