* [U-Boot] [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support
@ 2016-07-20 10:04 ` Wenyou Yang
2016-07-21 4:16 ` Jaehoon Chung
0 siblings, 1 reply; 5+ messages in thread
From: Wenyou Yang @ 2016-07-20 10:04 UTC (permalink / raw)
To: u-boot
Convert the driver to the driver model 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.
Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v7:
- Add support for using driver model for block devices and MMC operations.
- Change clk_client.h -> clk.h to adapt to clk API conversion.
Changes in v6:
- Remove unnecessary white space.
- Use sdhci_read(), instead of readl().
- Remove the local variables min_clk.
Changes in v5:
- Add Reviewed-by tag.
Changes in v4:
- Update the clk API based on [PATCH] clk: convert API to match
reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
- Remove check on dev_get_parent() return.
- Fixed the return value, such as -ENODEV->-EINVAL.
Changes in v3:
- Remove the redundant log print.
Changes in v2:
- Add clock support, include enabling peripheral clock and
generated clock.
- Retain the existing legacy code to support boards which have not
converted to driver model.
drivers/mmc/Kconfig | 10 ++++
drivers/mmc/atmel_sdhci.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
include/sdhci.h | 2 +
3 files changed, 137 insertions(+)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 79cf18f..49b325e 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -34,6 +34,16 @@ config MSM_SDHCI
SD 3.0 specifications. Both SD and eMMC devices are supported.
Card-detect gpios are not supported.
+config ATMEL_SDHCI
+ bool "Atmel SDHCI controller support"
+ depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
+ help
+ This enables support for the Atmel SDHCI controller, which supports
+ the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
+ Memory Card Specification V3.0, and the SDIO V3.0 specification.
+ It is compliant with the SD Host Controller Standard V3.0
+ specification.
+
config ROCKCHIP_DWMMC
bool "Rockchip SD/MMC controller support"
depends on DM_MMC && OF_CONTROL
diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
index 24b68b6..977f6ef 100644
--- a/drivers/mmc/atmel_sdhci.c
+++ b/drivers/mmc/atmel_sdhci.c
@@ -6,12 +6,15 @@
*/
#include <common.h>
+#include <clk.h>
+#include <dm.h>
#include <malloc.h>
#include <sdhci.h>
#include <asm/arch/clk.h>
#define ATMEL_SDHC_MIN_FREQ 400000
+#ifndef CONFIG_DM_MMC
int atmel_sdhci_init(void *regbase, u32 id)
{
struct sdhci_host *host;
@@ -38,3 +41,125 @@ int atmel_sdhci_init(void *regbase, u32 id)
return 0;
}
+
+#else
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct atmel_sdhci_plat {
+ struct mmc_config cfg;
+ struct mmc mmc;
+};
+
+static int atmel_sdhci_probe(struct udevice *dev)
+{
+ struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+ struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
+ struct sdhci_host *host = dev_get_priv(dev);
+ u32 max_clk;
+ u32 caps, caps_1;
+ u32 clk_base, clk_mul;
+ u32 gck_rate;
+ struct udevice *dev_clk;
+ struct clk clk;
+ int periph, ret;
+
+ ret = clk_get_by_index(dev, 0, &clk);
+ if (ret)
+ return ret;
+
+ periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
+ if (periph < 0)
+ return -EINVAL;
+
+ dev_clk = dev_get_parent(clk.dev);
+ ret = clk_request(dev_clk, &clk);
+ if (ret)
+ return ret;
+
+ clk.id = periph;
+ ret = clk_enable(&clk);
+ if (ret)
+ return ret;
+
+ host->name = (char *)dev->name;
+ host->ioaddr = (void *)dev_get_addr(dev);
+
+ host->quirks = 0;
+ host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
+
+ host->bus_width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+ "bus-width", 4);
+
+ caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+ clk_base = (caps & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
+ caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
+ gck_rate = clk_base * 1000000 * (clk_mul + 1);
+
+ ret = clk_get_by_index(dev, 1, &clk);
+ if (ret)
+ return ret;
+
+ periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
+ if (periph < 0)
+ return -EINVAL;
+
+ dev_clk = dev_get_parent(clk.dev);
+ ret = clk_request(dev_clk, &clk);
+ if (ret)
+ return ret;
+
+ clk.id = periph;
+ ret = clk_set_rate(&clk, gck_rate);
+ if (ret)
+ return ret;
+
+ max_clk = clk_get_rate(&clk);
+ if (!max_clk)
+ return -EINVAL;
+
+ ret = sdhci_setup_cfg(&plat->cfg, dev->name, host->bus_width,
+ caps, max_clk, ATMEL_SDHC_MIN_FREQ,
+ host->version, host->quirks, 0);
+ if (ret)
+ return ret;
+
+ host->mmc = &plat->mmc;
+ host->mmc->dev = dev;
+ host->mmc->priv = host;
+ upriv->mmc = host->mmc;
+
+ clk_free(&clk);
+
+ return sdhci_probe(dev);
+}
+
+static int atmel_sdhci_bind(struct udevice *dev)
+{
+ struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
+ int ret;
+
+ ret = sdhci_bind(dev, &plat->mmc, &plat->cfg);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static const struct udevice_id atmel_sdhci_ids[] = {
+ { .compatible = "atmel,sama5d2-sdhci" },
+ { }
+};
+
+U_BOOT_DRIVER(atmel_sdhci_drv) = {
+ .name = "atmel_sdhci",
+ .id = UCLASS_MMC,
+ .of_match = atmel_sdhci_ids,
+ .ops = &sdhci_ops,
+ .bind = atmel_sdhci_bind,
+ .probe = atmel_sdhci_probe,
+ .priv_auto_alloc_size = sizeof(struct sdhci_host),
+ .platdata_auto_alloc_size = sizeof(struct atmel_sdhci_plat),
+};
+#endif
diff --git a/include/sdhci.h b/include/sdhci.h
index c4d3b55..fdc0032 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -166,6 +166,8 @@
#define SDHCI_CAN_64BIT 0x10000000
#define SDHCI_CAPABILITIES_1 0x44
+#define SDHCI_CLOCK_MUL_MASK 0x00FF0000
+#define SDHCI_CLOCK_MUL_SHIFT 16
#define SDHCI_MAX_CURRENT 0x48
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support
2016-07-20 10:04 ` [U-Boot] [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support Wenyou Yang
@ 2016-07-21 4:16 ` Jaehoon Chung
2016-07-22 3:21 ` Simon Glass
2016-07-25 10:14 ` Wenyou.Yang at microchip.com
0 siblings, 2 replies; 5+ messages in thread
From: Jaehoon Chung @ 2016-07-21 4:16 UTC (permalink / raw)
To: u-boot
Hi Wenyuo,
On 07/20/2016 07:04 PM, Wenyou Yang wrote:
> Convert the driver to the driver model 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.
>
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v7:
> - Add support for using driver model for block devices and MMC operations.
> - Change clk_client.h -> clk.h to adapt to clk API conversion.
>
> Changes in v6:
> - Remove unnecessary white space.
> - Use sdhci_read(), instead of readl().
> - Remove the local variables min_clk.
>
> Changes in v5:
> - Add Reviewed-by tag.
>
> Changes in v4:
> - Update the clk API based on [PATCH] clk: convert API to match
> reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
> - Remove check on dev_get_parent() return.
> - Fixed the return value, such as -ENODEV->-EINVAL.
>
> Changes in v3:
> - Remove the redundant log print.
>
> Changes in v2:
> - Add clock support, include enabling peripheral clock and
> generated clock.
> - Retain the existing legacy code to support boards which have not
> converted to driver model.
>
> drivers/mmc/Kconfig | 10 ++++
> drivers/mmc/atmel_sdhci.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
> include/sdhci.h | 2 +
> 3 files changed, 137 insertions(+)
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 79cf18f..49b325e 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -34,6 +34,16 @@ config MSM_SDHCI
> SD 3.0 specifications. Both SD and eMMC devices are supported.
> Card-detect gpios are not supported.
>
> +config ATMEL_SDHCI
> + bool "Atmel SDHCI controller support"
> + depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
> + help
> + This enables support for the Atmel SDHCI controller, which supports
> + the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
> + Memory Card Specification V3.0, and the SDIO V3.0 specification.
> + It is compliant with the SD Host Controller Standard V3.0
> + specification.
> +
> config ROCKCHIP_DWMMC
> bool "Rockchip SD/MMC controller support"
> depends on DM_MMC && OF_CONTROL
> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
> index 24b68b6..977f6ef 100644
> --- a/drivers/mmc/atmel_sdhci.c
> +++ b/drivers/mmc/atmel_sdhci.c
> @@ -6,12 +6,15 @@
> */
>
> #include <common.h>
> +#include <clk.h>
> +#include <dm.h>
If it doesn't use DM_MMC, then doesn't it need to include dm.h?
> #include <malloc.h>
> #include <sdhci.h>
> #include <asm/arch/clk.h>
>
> #define ATMEL_SDHC_MIN_FREQ 400000
>
> +#ifndef CONFIG_DM_MMC
> int atmel_sdhci_init(void *regbase, u32 id)
> {
> struct sdhci_host *host;
> @@ -38,3 +41,125 @@ int atmel_sdhci_init(void *regbase, u32 id)
>
> return 0;
> }
> +
> +#else
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct atmel_sdhci_plat {
> + struct mmc_config cfg;
> + struct mmc mmc;
> +};
> +
> +static int atmel_sdhci_probe(struct udevice *dev)
> +{
> + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> + struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
> + struct sdhci_host *host = dev_get_priv(dev);
> + u32 max_clk;
> + u32 caps, caps_1;
> + u32 clk_base, clk_mul;
> + u32 gck_rate;
> + struct udevice *dev_clk;
> + struct clk clk;
> + int periph, ret;
> +
> + ret = clk_get_by_index(dev, 0, &clk);
> + if (ret)
> + return ret;
> +
> + periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
> + if (periph < 0)
> + return -EINVAL;
periph is just used to pass to clk.id. it looks meaningless.
As i already mentioned, you can make more cleaner than now.
> +
> + dev_clk = dev_get_parent(clk.dev);
> + ret = clk_request(dev_clk, &clk);
> + if (ret)
> + return ret;
> +
> + clk.id = periph;
> + ret = clk_enable(&clk);
> + if (ret)
> + return ret;
> +
> + host->name = (char *)dev->name;
Needs the type casting?
> + host->ioaddr = (void *)dev_get_addr(dev);
> +
> + host->quirks = 0;
> + host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
> +
> + host->bus_width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> + "bus-width", 4);
> +
> + caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> + clk_base = (caps & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
> + caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
> + clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
> + gck_rate = clk_base * 1000000 * (clk_mul + 1);
gck_rate can be over u32 boundary, doesn't?
> +
> + ret = clk_get_by_index(dev, 1, &clk);
> + if (ret)
> + return ret;
It can reuse the above clock get sequence.
If reuse this sequence, lines can be reduced.
My suggestion is
1. Make atmel_sdhci_get_clk(..., int index), it should be returned "struct clk" type.
2. Then the below sequence should be included in function,
atmel_sdhci_get_clk(..., int index) {
clk_get_by_index(dev, index, &clk);
...
return clk;
}
How about?
Best Regards,
Jaehoon Chung
> +
> + periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
> + if (periph < 0)
> + return -EINVAL;
> +
> + dev_clk = dev_get_parent(clk.dev);
> + ret = clk_request(dev_clk, &clk);
> + if (ret)
> + return ret;
> +
> + clk.id = periph;
> + ret = clk_set_rate(&clk, gck_rate);
> + if (ret)
> + return ret;
> +
> + max_clk = clk_get_rate(&clk);
> + if (!max_clk)
> + return -EINVAL;
> +
> + ret = sdhci_setup_cfg(&plat->cfg, dev->name, host->bus_width,
> + caps, max_clk, ATMEL_SDHC_MIN_FREQ,
> + host->version, host->quirks, 0);
> + if (ret)
> + return ret;
> +
> + host->mmc = &plat->mmc;
> + host->mmc->dev = dev;
> + host->mmc->priv = host;
> + upriv->mmc = host->mmc;
> +
> + clk_free(&clk);
> +
> + return sdhci_probe(dev);
> +}
> +
> +static int atmel_sdhci_bind(struct udevice *dev)
> +{
> + struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
> + int ret;
> +
> + ret = sdhci_bind(dev, &plat->mmc, &plat->cfg);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct udevice_id atmel_sdhci_ids[] = {
> + { .compatible = "atmel,sama5d2-sdhci" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(atmel_sdhci_drv) = {
> + .name = "atmel_sdhci",
> + .id = UCLASS_MMC,
> + .of_match = atmel_sdhci_ids,
> + .ops = &sdhci_ops,
> + .bind = atmel_sdhci_bind,
> + .probe = atmel_sdhci_probe,
> + .priv_auto_alloc_size = sizeof(struct sdhci_host),
> + .platdata_auto_alloc_size = sizeof(struct atmel_sdhci_plat),
> +};
> +#endif
> diff --git a/include/sdhci.h b/include/sdhci.h
> index c4d3b55..fdc0032 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -166,6 +166,8 @@
> #define SDHCI_CAN_64BIT 0x10000000
>
> #define SDHCI_CAPABILITIES_1 0x44
> +#define SDHCI_CLOCK_MUL_MASK 0x00FF0000
> +#define SDHCI_CLOCK_MUL_SHIFT 16
>
> #define SDHCI_MAX_CURRENT 0x48
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support
2016-07-21 4:16 ` Jaehoon Chung
@ 2016-07-22 3:21 ` Simon Glass
2016-07-22 4:00 ` Jaehoon Chung
2016-07-25 10:14 ` Wenyou.Yang at microchip.com
1 sibling, 1 reply; 5+ messages in thread
From: Simon Glass @ 2016-07-22 3:21 UTC (permalink / raw)
To: u-boot
Hi,
On 20 July 2016 at 22:16, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Wenyuo,
>
> On 07/20/2016 07:04 PM, Wenyou Yang wrote:
>> Convert the driver to the driver model 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.
>>
>> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v7:
>> - Add support for using driver model for block devices and MMC operations.
>> - Change clk_client.h -> clk.h to adapt to clk API conversion.
>>
>> Changes in v6:
>> - Remove unnecessary white space.
>> - Use sdhci_read(), instead of readl().
>> - Remove the local variables min_clk.
>>
>> Changes in v5:
>> - Add Reviewed-by tag.
>>
>> Changes in v4:
>> - Update the clk API based on [PATCH] clk: convert API to match
>> reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
>> - Remove check on dev_get_parent() return.
>> - Fixed the return value, such as -ENODEV->-EINVAL.
>>
>> Changes in v3:
>> - Remove the redundant log print.
>>
>> Changes in v2:
>> - Add clock support, include enabling peripheral clock and
>> generated clock.
>> - Retain the existing legacy code to support boards which have not
>> converted to driver model.
>>
>> drivers/mmc/Kconfig | 10 ++++
>> drivers/mmc/atmel_sdhci.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
>> include/sdhci.h | 2 +
>> 3 files changed, 137 insertions(+)
>>
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index 79cf18f..49b325e 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -34,6 +34,16 @@ config MSM_SDHCI
>> SD 3.0 specifications. Both SD and eMMC devices are supported.
>> Card-detect gpios are not supported.
>>
>> +config ATMEL_SDHCI
>> + bool "Atmel SDHCI controller support"
>> + depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
>> + help
>> + This enables support for the Atmel SDHCI controller, which supports
>> + the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
>> + Memory Card Specification V3.0, and the SDIO V3.0 specification.
>> + It is compliant with the SD Host Controller Standard V3.0
>> + specification.
>> +
>> config ROCKCHIP_DWMMC
>> bool "Rockchip SD/MMC controller support"
>> depends on DM_MMC && OF_CONTROL
>> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
>> index 24b68b6..977f6ef 100644
>> --- a/drivers/mmc/atmel_sdhci.c
>> +++ b/drivers/mmc/atmel_sdhci.c
>> @@ -6,12 +6,15 @@
>> */
>>
>> #include <common.h>
>> +#include <clk.h>
>> +#include <dm.h>
>
> If it doesn't use DM_MMC, then doesn't it need to include dm.h?
Please don't put #idefs around header files, particularly this one. It
just clutters the code. We can put logic in header files if needed.
Eventually most files will include dm.h.
>
>> #include <malloc.h>
>> #include <sdhci.h>
>> #include <asm/arch/clk.h>
>>
>> #define ATMEL_SDHC_MIN_FREQ 400000
Regards,
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support
2016-07-22 3:21 ` Simon Glass
@ 2016-07-22 4:00 ` Jaehoon Chung
0 siblings, 0 replies; 5+ messages in thread
From: Jaehoon Chung @ 2016-07-22 4:00 UTC (permalink / raw)
To: u-boot
On 07/22/2016 12:21 PM, Simon Glass wrote:
> Hi,
>
> On 20 July 2016 at 22:16, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi Wenyuo,
>>
>> On 07/20/2016 07:04 PM, Wenyou Yang wrote:
>>> Convert the driver to the driver model 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.
>>>
>>> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> Changes in v7:
>>> - Add support for using driver model for block devices and MMC operations.
>>> - Change clk_client.h -> clk.h to adapt to clk API conversion.
>>>
>>> Changes in v6:
>>> - Remove unnecessary white space.
>>> - Use sdhci_read(), instead of readl().
>>> - Remove the local variables min_clk.
>>>
>>> Changes in v5:
>>> - Add Reviewed-by tag.
>>>
>>> Changes in v4:
>>> - Update the clk API based on [PATCH] clk: convert API to match
>>> reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
>>> - Remove check on dev_get_parent() return.
>>> - Fixed the return value, such as -ENODEV->-EINVAL.
>>>
>>> Changes in v3:
>>> - Remove the redundant log print.
>>>
>>> Changes in v2:
>>> - Add clock support, include enabling peripheral clock and
>>> generated clock.
>>> - Retain the existing legacy code to support boards which have not
>>> converted to driver model.
>>>
>>> drivers/mmc/Kconfig | 10 ++++
>>> drivers/mmc/atmel_sdhci.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
>>> include/sdhci.h | 2 +
>>> 3 files changed, 137 insertions(+)
>>>
>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>> index 79cf18f..49b325e 100644
>>> --- a/drivers/mmc/Kconfig
>>> +++ b/drivers/mmc/Kconfig
>>> @@ -34,6 +34,16 @@ config MSM_SDHCI
>>> SD 3.0 specifications. Both SD and eMMC devices are supported.
>>> Card-detect gpios are not supported.
>>>
>>> +config ATMEL_SDHCI
>>> + bool "Atmel SDHCI controller support"
>>> + depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
>>> + help
>>> + This enables support for the Atmel SDHCI controller, which supports
>>> + the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
>>> + Memory Card Specification V3.0, and the SDIO V3.0 specification.
>>> + It is compliant with the SD Host Controller Standard V3.0
>>> + specification.
>>> +
>>> config ROCKCHIP_DWMMC
>>> bool "Rockchip SD/MMC controller support"
>>> depends on DM_MMC && OF_CONTROL
>>> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
>>> index 24b68b6..977f6ef 100644
>>> --- a/drivers/mmc/atmel_sdhci.c
>>> +++ b/drivers/mmc/atmel_sdhci.c
>>> @@ -6,12 +6,15 @@
>>> */
>>>
>>> #include <common.h>
>>> +#include <clk.h>
>>> +#include <dm.h>
>>
>> If it doesn't use DM_MMC, then doesn't it need to include dm.h?
>
> Please don't put #idefs around header files, particularly this one. It
> just clutters the code. We can put logic in header files if needed.
> Eventually most files will include dm.h.
Agreed! Thanks for explanation.
Best Regards,
Jaehoon Chung
>
>>
>>> #include <malloc.h>
>>> #include <sdhci.h>
>>> #include <asm/arch/clk.h>
>>>
>>> #define ATMEL_SDHC_MIN_FREQ 400000
>
> Regards,
> Simon
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support
2016-07-21 4:16 ` Jaehoon Chung
2016-07-22 3:21 ` Simon Glass
@ 2016-07-25 10:14 ` Wenyou.Yang at microchip.com
1 sibling, 0 replies; 5+ messages in thread
From: Wenyou.Yang at microchip.com @ 2016-07-25 10:14 UTC (permalink / raw)
To: u-boot
Hi Jaehoon,
> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Sent: 2016?7?21? 12:17
> To: Wenyou Yang <wenyou.yang@atmel.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>; Andreas Bie?mann
> <andreas@biessmann.org>; Simon Glass <sjg@chromium.org>
> Subject: Re: [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support
>
> Hi Wenyuo,
>
> On 07/20/2016 07:04 PM, Wenyou Yang wrote:
> > Convert the driver to the driver model 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.
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v7:
> > - Add support for using driver model for block devices and MMC operations.
> > - Change clk_client.h -> clk.h to adapt to clk API conversion.
> >
> > Changes in v6:
> > - Remove unnecessary white space.
> > - Use sdhci_read(), instead of readl().
> > - Remove the local variables min_clk.
> >
> > Changes in v5:
> > - Add Reviewed-by tag.
> >
> > Changes in v4:
> > - Update the clk API based on [PATCH] clk: convert API to match
> > reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
> > - Remove check on dev_get_parent() return.
> > - Fixed the return value, such as -ENODEV->-EINVAL.
> >
> > Changes in v3:
> > - Remove the redundant log print.
> >
> > Changes in v2:
> > - Add clock support, include enabling peripheral clock and
> > generated clock.
> > - Retain the existing legacy code to support boards which have not
> > converted to driver model.
> >
> > drivers/mmc/Kconfig | 10 ++++
> > drivers/mmc/atmel_sdhci.c | 125
> ++++++++++++++++++++++++++++++++++++++++++++++
> > include/sdhci.h | 2 +
> > 3 files changed, 137 insertions(+)
> >
> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index
> > 79cf18f..49b325e 100644
> > --- a/drivers/mmc/Kconfig
> > +++ b/drivers/mmc/Kconfig
> > @@ -34,6 +34,16 @@ config MSM_SDHCI
> > SD 3.0 specifications. Both SD and eMMC devices are supported.
> > Card-detect gpios are not supported.
> >
> > +config ATMEL_SDHCI
> > + bool "Atmel SDHCI controller support"
> > + depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
> > + help
> > + This enables support for the Atmel SDHCI controller, which supports
> > + the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
> > + Memory Card Specification V3.0, and the SDIO V3.0 specification.
> > + It is compliant with the SD Host Controller Standard V3.0
> > + specification.
> > +
> > config ROCKCHIP_DWMMC
> > bool "Rockchip SD/MMC controller support"
> > depends on DM_MMC && OF_CONTROL
> > diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
> > index 24b68b6..977f6ef 100644
> > --- a/drivers/mmc/atmel_sdhci.c
> > +++ b/drivers/mmc/atmel_sdhci.c
> > @@ -6,12 +6,15 @@
> > */
> >
> > #include <common.h>
> > +#include <clk.h>
> > +#include <dm.h>
>
> If it doesn't use DM_MMC, then doesn't it need to include dm.h?
>
> > #include <malloc.h>
> > #include <sdhci.h>
> > #include <asm/arch/clk.h>
> >
> > #define ATMEL_SDHC_MIN_FREQ 400000
> >
> > +#ifndef CONFIG_DM_MMC
> > int atmel_sdhci_init(void *regbase, u32 id) {
> > struct sdhci_host *host;
> > @@ -38,3 +41,125 @@ int atmel_sdhci_init(void *regbase, u32 id)
> >
> > return 0;
> > }
> > +
> > +#else
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +struct atmel_sdhci_plat {
> > + struct mmc_config cfg;
> > + struct mmc mmc;
> > +};
> > +
> > +static int atmel_sdhci_probe(struct udevice *dev) {
> > + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> > + struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
> > + struct sdhci_host *host = dev_get_priv(dev);
> > + u32 max_clk;
> > + u32 caps, caps_1;
> > + u32 clk_base, clk_mul;
> > + u32 gck_rate;
> > + struct udevice *dev_clk;
> > + struct clk clk;
> > + int periph, ret;
> > +
> > + ret = clk_get_by_index(dev, 0, &clk);
> > + if (ret)
> > + return ret;
> > +
> > + periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
> > + if (periph < 0)
> > + return -EINVAL;
>
> periph is just used to pass to clk.id. it looks meaningless.
> As i already mentioned, you can make more cleaner than now.
There is a litter difference from usual clock structure for Atmel clock driver.
The periph value is got from a child node, then is assigned to its parent clock node.
>
> > +
> > + dev_clk = dev_get_parent(clk.dev);
> > + ret = clk_request(dev_clk, &clk);
> > + if (ret)
> > + return ret;
> > +
> > + clk.id = periph;
> > + ret = clk_enable(&clk);
> > + if (ret)
> > + return ret;
> > +
> > + host->name = (char *)dev->name;
>
> Needs the type casting?
Accepted.
>
> > + host->ioaddr = (void *)dev_get_addr(dev);
> > +
> > + host->quirks = 0;
> > + host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
> > +
> > + host->bus_width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> > + "bus-width", 4);
> > +
> > + caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> > + clk_base = (caps & SDHCI_CLOCK_V3_BASE_MASK) >>
> SDHCI_CLOCK_BASE_SHIFT;
> > + caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
> > + clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >>
> SDHCI_CLOCK_MUL_SHIFT;
> > + gck_rate = clk_base * 1000000 * (clk_mul + 1);
>
> gck_rate can be over u32 boundary, doesn't?
Accepted.
>
> > +
> > + ret = clk_get_by_index(dev, 1, &clk);
> > + if (ret)
> > + return ret;
>
> It can reuse the above clock get sequence.
>
> If reuse this sequence, lines can be reduced.
>
> My suggestion is
>
> 1. Make atmel_sdhci_get_clk(..., int index), it should be returned "struct clk" type.
>
> 2. Then the below sequence should be included in function,
>
> atmel_sdhci_get_clk(..., int index) {
>
> clk_get_by_index(dev, index, &clk);
> ...
>
> return clk;
> }
>
> How about?
Good ideas, thanks a lot.
>
> Best Regards,
> Jaehoon Chung
>
> > +
> > + periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
> > + if (periph < 0)
> > + return -EINVAL;
> > +
> > + dev_clk = dev_get_parent(clk.dev);
> > + ret = clk_request(dev_clk, &clk);
> > + if (ret)
> > + return ret;
> > +
> > + clk.id = periph;
> > + ret = clk_set_rate(&clk, gck_rate);
> > + if (ret)
> > + return ret;
> > +
> > + max_clk = clk_get_rate(&clk);
> > + if (!max_clk)
> > + return -EINVAL;
> > +
> > + ret = sdhci_setup_cfg(&plat->cfg, dev->name, host->bus_width,
> > + caps, max_clk, ATMEL_SDHC_MIN_FREQ,
> > + host->version, host->quirks, 0);
> > + if (ret)
> > + return ret;
> > +
> > + host->mmc = &plat->mmc;
> > + host->mmc->dev = dev;
> > + host->mmc->priv = host;
> > + upriv->mmc = host->mmc;
> > +
> > + clk_free(&clk);
> > +
> > + return sdhci_probe(dev);
> > +}
> > +
> > +static int atmel_sdhci_bind(struct udevice *dev) {
> > + struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
> > + int ret;
> > +
> > + ret = sdhci_bind(dev, &plat->mmc, &plat->cfg);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct udevice_id atmel_sdhci_ids[] = {
> > + { .compatible = "atmel,sama5d2-sdhci" },
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(atmel_sdhci_drv) = {
> > + .name = "atmel_sdhci",
> > + .id = UCLASS_MMC,
> > + .of_match = atmel_sdhci_ids,
> > + .ops = &sdhci_ops,
> > + .bind = atmel_sdhci_bind,
> > + .probe = atmel_sdhci_probe,
> > + .priv_auto_alloc_size = sizeof(struct sdhci_host),
> > + .platdata_auto_alloc_size = sizeof(struct atmel_sdhci_plat), };
> > +#endif
> > diff --git a/include/sdhci.h b/include/sdhci.h index c4d3b55..fdc0032
> > 100644
> > --- a/include/sdhci.h
> > +++ b/include/sdhci.h
> > @@ -166,6 +166,8 @@
> > #define SDHCI_CAN_64BIT 0x10000000
> >
> > #define SDHCI_CAPABILITIES_1 0x44
> > +#define SDHCI_CLOCK_MUL_MASK 0x00FF0000
> > +#define SDHCI_CLOCK_MUL_SHIFT 16
> >
> > #define SDHCI_MAX_CURRENT 0x48
> >
> >
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-07-25 10:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20160720101618epcas1p12e6a42e8898dcd45b3250df406127f44@epcas1p1.samsung.com>
2016-07-20 10:04 ` [U-Boot] [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support Wenyou Yang
2016-07-21 4:16 ` Jaehoon Chung
2016-07-22 3:21 ` Simon Glass
2016-07-22 4:00 ` Jaehoon Chung
2016-07-25 10:14 ` Wenyou.Yang at microchip.com
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox