All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2] dm/mmc: gen_atmel_mci: Add driver model support for mci
@ 2016-09-20  3:05 Wenyou Yang
  2016-09-20 21:14 ` Stephen Warren
  0 siblings, 1 reply; 3+ messages in thread
From: Wenyou Yang @ 2016-09-20  3:05 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v2:
 - Change the return type of atmel_mci_setup_cfg() from int to void.
 - Add comments on the features depends on the IP version.
 - Add the error handle path of clock.
 - Fix the missing use priv->bus_clk_rate.
 - Return from mmc_bind() directly, instead of checking its return.

 drivers/mmc/Kconfig         |   9 +++
 drivers/mmc/gen_atmel_mci.c | 164 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index a71afa5..e56eb94 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -44,6 +44,15 @@ config ATMEL_SDHCI
 	  It is compliant with the SD Host Controller Standard V3.0
 	  specification.
 
+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 specification
+	  and CE-ATA V1.1.
+
 config ROCKCHIP_DWMMC
 	bool "Rockchip SD/MMC controller support"
 	depends on DM_MMC && OF_CONTROL
diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
index 69770df..b2d6d86 100644
--- a/drivers/mmc/gen_atmel_mci.c
+++ b/drivers/mmc/gen_atmel_mci.c
@@ -10,6 +10,7 @@
  */
 
 #include <common.h>
+#include <clk.h>
 #include <mmc.h>
 #include <part.h>
 #include <malloc.h>
@@ -18,8 +19,11 @@
 #include <asm/byteorder.h>
 #include <asm/arch/clk.h>
 #include <asm/arch/hardware.h>
+#include <dm/device.h>
 #include "atmel_mci.h"
 
+DECLARE_GLOBAL_DATA_PTR;
+
 #ifndef CONFIG_SYS_MMC_CLK_OD
 # define CONFIG_SYS_MMC_CLK_OD	150000
 #endif
@@ -37,6 +41,10 @@ struct atmel_mci_priv {
 	struct atmel_mci	*mci;
 	unsigned int		initialized:1;
 	unsigned int		curr_clk;
+#ifdef CONFIG_DM_MMC
+	struct mmc	mmc;
+	ulong		bus_clk_rate;
+#endif
 };
 
 /* Read Atmel MCI IP version */
@@ -57,12 +65,20 @@ static void dump_cmd(u32 cmdr, u32 arg, u32 status, const char* msg)
 	      cmdr, cmdr & 0x3F, arg, status, msg);
 }
 
+#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
+
+	atmel_mci_t *mci = priv->mci;
 	u32 clkdiv = 255;
 	unsigned int version = atmel_mci_get_version(mci);
 	u32 clkodd = 0;
@@ -202,10 +218,18 @@ io_fail:
  * Sends a command out on the bus and deals with the block data.
  * Takes the mmc pointer, a command pointer, and an optional data pointer.
  */
+#ifndef CONFIG_DM_MMC
 static int
 mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 {
 	struct atmel_mci_priv *priv = mmc->priv;
+#else
+static int atmel_mci_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
+			      struct mmc_data *data)
+{
+	struct atmel_mci_priv *priv = dev_get_priv(dev);
+	struct mmc *mmc = mmc_get_mmc_dev(dev);
+#endif
 	atmel_mci_t *mci = priv->mci;
 	u32 cmdr;
 	u32 error_flags = 0;
@@ -335,17 +359,29 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	return 0;
 }
 
+#ifndef CONFIG_DM_MMC
 /* Entered into mmc structure during driver init */
 static void mci_set_ios(struct mmc *mmc)
 {
 	struct atmel_mci_priv *priv = mmc->priv;
+#else
+static int atmel_mci_set_ios(struct udevice *dev)
+{
+	struct atmel_mci_priv *priv = dev_get_priv(dev);
+	struct mmc *mmc = mmc_get_mmc_dev(dev);
+#endif
+
 	atmel_mci_t *mci = priv->mci;
 	int bus_width = mmc->bus_width;
 	unsigned int version = atmel_mci_get_version(mci);
 	int busw;
 
 	/* Set the clock speed */
+#ifndef CONFIG_DM_MMC
 	mci_set_mode(mmc, mmc->clock, MMC_DEFAULT_BLKLEN);
+#else
+	mci_set_mode(priv, mmc->clock, MMC_DEFAULT_BLKLEN);
+#endif
 
 	/*
 	 * set the bus width and select slot for this interface
@@ -370,12 +406,22 @@ static void mci_set_ios(struct mmc *mmc)
 
 		writel(busw << 7 | MMCI_BF(SCDSEL, MCI_BUS), &mci->sdcr);
 	}
+
+#ifdef CONFIG_DM_MMC
+	return 0;
+#endif
 }
 
+
+#ifndef CONFIG_DM_MMC
 /* Entered into mmc structure during driver init */
 static int mci_init(struct mmc *mmc)
 {
 	struct atmel_mci_priv *priv = mmc->priv;
+#else
+static int atmel_mci_hw_init(struct atmel_mci_priv *priv)
+{
+#endif
 	atmel_mci_t *mci = priv->mci;
 
 	/* Initialize controller */
@@ -390,11 +436,16 @@ static int mci_init(struct mmc *mmc)
 	writel(~0UL, &mci->idr);
 
 	/* Set default clocks and blocklen */
+#ifndef CONFIG_DM_MMC
 	mci_set_mode(mmc, CONFIG_SYS_MMC_CLK_OD, MMC_DEFAULT_BLKLEN);
+#else
+	mci_set_mode(priv, CONFIG_SYS_MMC_CLK_OD, MMC_DEFAULT_BLKLEN);
+#endif
 
 	return 0;
 }
 
+#ifndef CONFIG_DM_MMC
 static const struct mmc_ops atmel_mci_ops = {
 	.send_cmd	= mci_send_cmd,
 	.set_ios	= mci_set_ios,
@@ -454,3 +505,114 @@ int atmel_mci_init(void *regs)
 
 	return 0;
 }
+#endif
+
+#ifdef CONFIG_DM_MMC
+static const struct dm_mmc_ops atmel_mci_mmc_ops = {
+	.send_cmd = atmel_mci_send_cmd,
+	.set_ios = atmel_mci_set_ios,
+};
+
+static void atmel_mci_setup_cfg(struct atmel_mci_priv *priv)
+{
+	struct mmc_config *cfg;
+	u32 version;
+
+	cfg = &priv->cfg;
+	cfg->name = "Atmel mci";
+	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
+
+	/*
+	 * If the version is above 3.0, the capabilities of the 8-bit
+	 * bus width and high speed are supported.
+	 */
+	version = atmel_mci_get_version(priv->mci);
+	if ((version & 0xf00) >= 0x300) {
+		cfg->host_caps = MMC_MODE_8BIT |
+				 MMC_MODE_HS | MMC_MODE_HS_52MHz;
+	}
+
+	cfg->host_caps |= MMC_MODE_4BIT;
+	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
+	cfg->f_min = priv->bus_clk_rate / (2 * 256);
+	cfg->f_max = priv->bus_clk_rate / 2;
+}
+
+static int atmel_mci_enable_clk(struct udevice *dev)
+{
+	struct atmel_mci_priv *priv = dev_get_priv(dev);
+	struct clk clk;
+	ulong clk_rate;
+	int ret = 0;
+
+	ret = clk_get_by_index(dev, 0, &clk);
+	if (ret) {
+		ret = -EINVAL;
+		goto failed;
+	}
+
+	ret = clk_enable(&clk);
+	if (ret)
+		goto failed;
+
+	clk_rate = clk_get_rate(&clk);
+	if (!clk_rate) {
+		ret = -EINVAL;
+		goto failed;
+	}
+
+	priv->bus_clk_rate = clk_rate;
+
+failed:
+	clk_free(&clk);
+
+	return ret;
+}
+
+static int atmel_mci_probe(struct udevice *dev)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct atmel_mci_priv *priv = dev_get_priv(dev);
+	struct mmc *mmc;
+	int ret;
+
+	ret = atmel_mci_enable_clk(dev);
+	if (ret)
+		return ret;
+
+	priv->mci = (struct atmel_mci *)dev_get_addr_ptr(dev);
+
+	atmel_mci_setup_cfg(priv);
+
+	mmc = &priv->mmc;
+	mmc->cfg = &priv->cfg;
+	mmc->dev = dev;
+	upriv->mmc = mmc;
+
+	atmel_mci_hw_init(priv);
+
+	return 0;
+}
+
+static int atmel_mci_bind(struct udevice *dev)
+{
+	struct atmel_mci_priv *priv = dev_get_priv(dev);
+
+	return mmc_bind(dev, &priv->mmc, &priv->cfg);
+}
+
+static const struct udevice_id atmel_mci_ids[] = {
+	{ .compatible = "atmel,hsmci" },
+	{ }
+};
+
+U_BOOT_DRIVER(atmel_mci) = {
+	.name = "atmel-mci",
+	.id = UCLASS_MMC,
+	.of_match = atmel_mci_ids,
+	.bind = atmel_mci_bind,
+	.probe = atmel_mci_probe,
+	.priv_auto_alloc_size = sizeof(struct atmel_mci_priv),
+	.ops = &atmel_mci_mmc_ops,
+};
+#endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [U-Boot] [PATCH v2] dm/mmc: gen_atmel_mci: Add driver model support for mci
  2016-09-20  3:05 [U-Boot] [PATCH v2] dm/mmc: gen_atmel_mci: Add driver model support for mci Wenyou Yang
@ 2016-09-20 21:14 ` Stephen Warren
  2016-09-21  5:13   ` Wenyou.Yang at microchip.com
  0 siblings, 1 reply; 3+ messages in thread
From: Stephen Warren @ 2016-09-20 21:14 UTC (permalink / raw)
  To: u-boot

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 specification
> +	  and 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?

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [U-Boot] [PATCH v2] dm/mmc: gen_atmel_mci: Add driver model support for mci
  2016-09-20 21:14 ` Stephen Warren
@ 2016-09-21  5:13   ` Wenyou.Yang at microchip.com
  0 siblings, 0 replies; 3+ messages in thread
From: Wenyou.Yang at microchip.com @ 2016-09-21  5:13 UTC (permalink / raw)
  To: u-boot

Hi Stephen,

> -----Original Message-----
> From: Stephen Warren [mailto:swarren at 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 specification
> > +	  and 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-09-21  5:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-20  3:05 [U-Boot] [PATCH v2] dm/mmc: gen_atmel_mci: Add driver model support for mci Wenyou Yang
2016-09-20 21:14 ` Stephen Warren
2016-09-21  5:13   ` Wenyou.Yang at microchip.com

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.