[U-Boot] [PATCH 1/4] mmc: atmel: Silence debug output

[U-Boot] [PATCH 1/4] mmc: atmel: Silence debug output

[Marek Vasut]

This driver generates clearly debugging prints when changing clock
speed, so silence those. Furthermore, the driver generates further
prints in case a command fails to complete. The later case woud be
useful, but for eMMC, command 8 can fail and it's not an error but
a part of the specification. Thus, make this debug() as well.

Signed-off-by: Marek Vasut <marex@denx.de>
---
 drivers/mmc/gen_atmel_mci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
index 45bcffb..2815e57 100644
--- a/drivers/mmc/gen_atmel_mci.c
+++ b/drivers/mmc/gen_atmel_mci.c
@@ -48,8 +48,8 @@ static unsigned int atmel_mci_get_version(struct atmel_mci *mci)
  */
 static void dump_cmd(u32 cmdr, u32 arg, u32 status, const char* msg)
 {
-	printf("gen_atmel_mci: CMDR %08x (%2u) ARGR %08x (SR: %08x) %s\n",
-		cmdr, cmdr&0x3F, arg, status, msg);
+	debug("gen_atmel_mci: CMDR %08x (%2u) ARGR %08x (SR: %08x) %s\n",
+	      cmdr, cmdr & 0x3F, arg, status, msg);
 }
 
 /* Setup for MCI Clock and Block Size */
@@ -73,7 +73,7 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 			clkodd = clkdiv & 1;
 			clkdiv >>= 1;
 
-			printf("mci: setting clock %u Hz, block size %u\n",
+			debug("mci: setting clock %u Hz, block size %u\n",
 			       bus_hz / (clkdiv * 2 + clkodd + 2), blklen);
 		} else {
 			/* find clkdiv yielding a rate <= than requested */
@@ -81,7 +81,7 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 				if ((bus_hz / (clkdiv + 1) / 2) <= hz)
 					break;
 			}
-			printf("mci: setting clock %u Hz, block size %u\n",
+			debug("mci: setting clock %u Hz, block size %u\n",
 			       (bus_hz / (clkdiv + 1)) / 2, blklen);
 
 		}
-- 
2.1.4

[U-Boot] [PATCH 2/4] mmc: atmel: Fix clock configuration

[Marek Vasut]

After silencing the prints which were generated when reconfiguring the
clock of the SD/MMC bus, surprisingly, the driver stopped working such
that every attempt to use the SD/MMC bus caused the CPU to get totally
stuck hard. It turns out that the prints generated a short delay, which
was necessary for the CPU to reconfigure the clock without getting stuck.
Thus, this patch adds a short delay after the clock configuration instead.

Signed-off-by: Marek Vasut <marex@denx.de>
---
 drivers/mmc/gen_atmel_mci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
index 2815e57..8b05fcd 100644
--- a/drivers/mmc/gen_atmel_mci.c
+++ b/drivers/mmc/gen_atmel_mci.c
@@ -113,6 +113,8 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 	if (mmc->card_caps & mmc->cfg->host_caps & MMC_MODE_HS)
 		writel(MMCI_BIT(HSMODE), &mci->cfg);
 
+	udelay(50);
+
 	initialized = 1;
 }
 
-- 
2.1.4

[U-Boot] [PATCH 3/4] mmc: atmel: Implement proper private data

[Marek Vasut]

Instead of passing just the register area as a private data, introduce
a proper struct atmel_mci_priv structure instead. This will become useful
in the subsequent patch, where we eliminate the global variable from this
driver.

Signed-off-by: Marek Vasut <marex@denx.de>
---
 drivers/mmc/gen_atmel_mci.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
index 8b05fcd..abc77cc 100644
--- a/drivers/mmc/gen_atmel_mci.c
+++ b/drivers/mmc/gen_atmel_mci.c
@@ -32,6 +32,11 @@
 # define MCI_BUS 0
 #endif
 
+struct atmel_mci_priv {
+	struct mmc_config	cfg;
+	struct atmel_mci	*mci;
+};
+
 static int initialized = 0;
 
 /* Read Atmel MCI IP version */
@@ -55,7 +60,8 @@ static void dump_cmd(u32 cmdr, u32 arg, u32 status, const char* msg)
 /* Setup for MCI Clock and Block Size */
 static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 {
-	atmel_mci_t *mci = mmc->priv;
+	struct atmel_mci_priv *priv = mmc->priv;
+	atmel_mci_t *mci = priv->mci;
 	u32 bus_hz = get_mci_clk_rate();
 	u32 clkdiv = 255;
 	unsigned int version = atmel_mci_get_version(mci);
@@ -198,7 +204,8 @@ io_fail:
 static int
 mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 {
-	atmel_mci_t *mci = mmc->priv;
+	struct atmel_mci_priv *priv = mmc->priv;
+	atmel_mci_t *mci = priv->mci;
 	u32 cmdr;
 	u32 error_flags = 0;
 	u32 status;
@@ -323,7 +330,8 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 /* Entered into mmc structure during driver init */
 static void mci_set_ios(struct mmc *mmc)
 {
-	atmel_mci_t *mci = mmc->priv;
+	struct atmel_mci_priv *priv = mmc->priv;
+	atmel_mci_t *mci = priv->mci;
 	int bus_width = mmc->bus_width;
 	unsigned int version = atmel_mci_get_version(mci);
 	int busw;
@@ -359,7 +367,8 @@ static void mci_set_ios(struct mmc *mmc)
 /* Entered into mmc structure during driver init */
 static int mci_init(struct mmc *mmc)
 {
-	atmel_mci_t *mci = mmc->priv;
+	struct atmel_mci_priv *priv = mmc->priv;
+	atmel_mci_t *mci = priv->mci;
 
 	/* Initialize controller */
 	writel(MMCI_BIT(SWRST), &mci->cr);	/* soft reset */
@@ -393,22 +402,23 @@ int atmel_mci_init(void *regs)
 {
 	struct mmc *mmc;
 	struct mmc_config *cfg;
-	struct atmel_mci *mci;
+	struct atmel_mci_priv *priv;
 	unsigned int version;
 
-	cfg = malloc(sizeof(*cfg));
-	if (cfg == NULL)
-		return -1;
-	memset(cfg, 0, sizeof(*cfg));
+	priv = calloc(1, sizeof(*priv));
+	if (!priv)
+		return -ENOMEM;
 
-	mci = (struct atmel_mci *)regs;
+	cfg = &priv->cfg;
 
 	cfg->name = "mci";
 	cfg->ops = &atmel_mci_ops;
 
+	priv->mci = (struct atmel_mci *)regs;
+
 	/* need to be able to pass these in on a board by board basis */
 	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
-	version = atmel_mci_get_version(mci);
+	version = atmel_mci_get_version(priv->mci);
 	if ((version & 0xf00) >= 0x300) {
 		cfg->host_caps = MMC_MODE_8BIT;
 		cfg->host_caps |= MMC_MODE_HS | MMC_MODE_HS_52MHz;
@@ -425,7 +435,7 @@ int atmel_mci_init(void *regs)
 
 	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
 
-	mmc = mmc_create(cfg, regs);
+	mmc = mmc_create(cfg, priv);
 
 	if (mmc == NULL) {
 		free(cfg);
-- 
2.1.4

[U-Boot] [PATCH 4/4] mmc: atmel: Zap global 'initialized' variable

[Marek Vasut]

Global variables are bad. Get rid of this particular one, so we can
correctly instantiate multiple atmel mci interfaces, without having
them interfere with one another.

Signed-off-by: Marek Vasut <marex@denx.de>
---
 drivers/mmc/gen_atmel_mci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
index abc77cc..9a0332f 100644
--- a/drivers/mmc/gen_atmel_mci.c
+++ b/drivers/mmc/gen_atmel_mci.c
@@ -35,10 +35,9 @@
 struct atmel_mci_priv {
 	struct mmc_config	cfg;
 	struct atmel_mci	*mci;
+	unsigned int		initialized:1;
 };
 
-static int initialized = 0;
-
 /* Read Atmel MCI IP version */
 static unsigned int atmel_mci_get_version(struct atmel_mci *mci)
 {
@@ -121,7 +120,7 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 
 	udelay(50);
 
-	initialized = 1;
+	priv->initialized = 1;
 }
 
 /* Return the CMDR with flags for a given command and data packet */
@@ -210,7 +209,7 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	u32 error_flags = 0;
 	u32 status;
 
-	if (!initialized) {
+	if (!priv->initialized) {
 		puts ("MCI not initialized!\n");
 		return COMM_ERR;
 	}
@@ -415,6 +414,7 @@ int atmel_mci_init(void *regs)
 	cfg->ops = &atmel_mci_ops;
 
 	priv->mci = (struct atmel_mci *)regs;
+	priv->initialized = 0;
 
 	/* need to be able to pass these in on a board by board basis */
 	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
-- 
2.1.4

[U-Boot] [PATCH 1/4] mmc: atmel: Silence debug output

[Andreas Bießmann]

On 23.10.15 20:46, Marek Vasut wrote:
> This driver generates clearly debugging prints when changing clock
> speed, so silence those. Furthermore, the driver generates further
> prints in case a command fails to complete. The later case woud be
> useful, but for eMMC, command 8 can fail and it's not an error but
> a part of the specification. Thus, make this debug() as well.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>

> ---
>  drivers/mmc/gen_atmel_mci.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
> index 45bcffb..2815e57 100644
> --- a/drivers/mmc/gen_atmel_mci.c
> +++ b/drivers/mmc/gen_atmel_mci.c
> @@ -48,8 +48,8 @@ static unsigned int atmel_mci_get_version(struct atmel_mci *mci)
>   */
>  static void dump_cmd(u32 cmdr, u32 arg, u32 status, const char* msg)
>  {
> -	printf("gen_atmel_mci: CMDR %08x (%2u) ARGR %08x (SR: %08x) %s\n",
> -		cmdr, cmdr&0x3F, arg, status, msg);
> +	debug("gen_atmel_mci: CMDR %08x (%2u) ARGR %08x (SR: %08x) %s\n",
> +	      cmdr, cmdr & 0x3F, arg, status, msg);
>  }
>  
>  /* Setup for MCI Clock and Block Size */
> @@ -73,7 +73,7 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
>  			clkodd = clkdiv & 1;
>  			clkdiv >>= 1;
>  
> -			printf("mci: setting clock %u Hz, block size %u\n",
> +			debug("mci: setting clock %u Hz, block size %u\n",
>  			       bus_hz / (clkdiv * 2 + clkodd + 2), blklen);
>  		} else {
>  			/* find clkdiv yielding a rate <= than requested */
> @@ -81,7 +81,7 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
>  				if ((bus_hz / (clkdiv + 1) / 2) <= hz)
>  					break;
>  			}
> -			printf("mci: setting clock %u Hz, block size %u\n",
> +			debug("mci: setting clock %u Hz, block size %u\n",
>  			       (bus_hz / (clkdiv + 1)) / 2, blklen);
>  
>  		}
> 

[U-Boot] [PATCH 2/4] mmc: atmel: Fix clock configuration

[Andreas Bießmann]

On 23.10.15 20:46, Marek Vasut wrote:
> After silencing the prints which were generated when reconfiguring the
> clock of the SD/MMC bus, surprisingly, the driver stopped working such
> that every attempt to use the SD/MMC bus caused the CPU to get totally
> stuck hard. It turns out that the prints generated a short delay, which
> was necessary for the CPU to reconfigure the clock without getting stuck.
> Thus, this patch adds a short delay after the clock configuration instead.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>

> ---
>  drivers/mmc/gen_atmel_mci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
> index 2815e57..8b05fcd 100644
> --- a/drivers/mmc/gen_atmel_mci.c
> +++ b/drivers/mmc/gen_atmel_mci.c
> @@ -113,6 +113,8 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
>  	if (mmc->card_caps & mmc->cfg->host_caps & MMC_MODE_HS)
>  		writel(MMCI_BIT(HSMODE), &mci->cfg);
>  
> +	udelay(50);
> +
>  	initialized = 1;
>  }
>  
> 

[U-Boot] [PATCH 3/4] mmc: atmel: Implement proper private data

[Andreas Bießmann]

On 23.10.15 20:46, Marek Vasut wrote:
> Instead of passing just the register area as a private data, introduce
> a proper struct atmel_mci_priv structure instead. This will become useful
> in the subsequent patch, where we eliminate the global variable from this
> driver.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reveiwed-by: Andreas Bie?mann <andreas.devel@googlemail.com>

> ---
>  drivers/mmc/gen_atmel_mci.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
> index 8b05fcd..abc77cc 100644
> --- a/drivers/mmc/gen_atmel_mci.c
> +++ b/drivers/mmc/gen_atmel_mci.c
> @@ -32,6 +32,11 @@
>  # define MCI_BUS 0
>  #endif
>  
> +struct atmel_mci_priv {
> +	struct mmc_config	cfg;
> +	struct atmel_mci	*mci;
> +};
> +
>  static int initialized = 0;
>  
>  /* Read Atmel MCI IP version */
> @@ -55,7 +60,8 @@ static void dump_cmd(u32 cmdr, u32 arg, u32 status, const char* msg)
>  /* Setup for MCI Clock and Block Size */
>  static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
>  {
> -	atmel_mci_t *mci = mmc->priv;
> +	struct atmel_mci_priv *priv = mmc->priv;
> +	atmel_mci_t *mci = priv->mci;
>  	u32 bus_hz = get_mci_clk_rate();
>  	u32 clkdiv = 255;
>  	unsigned int version = atmel_mci_get_version(mci);
> @@ -198,7 +204,8 @@ io_fail:
>  static int
>  mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  {
> -	atmel_mci_t *mci = mmc->priv;
> +	struct atmel_mci_priv *priv = mmc->priv;
> +	atmel_mci_t *mci = priv->mci;
>  	u32 cmdr;
>  	u32 error_flags = 0;
>  	u32 status;
> @@ -323,7 +330,8 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  /* Entered into mmc structure during driver init */
>  static void mci_set_ios(struct mmc *mmc)
>  {
> -	atmel_mci_t *mci = mmc->priv;
> +	struct atmel_mci_priv *priv = mmc->priv;
> +	atmel_mci_t *mci = priv->mci;
>  	int bus_width = mmc->bus_width;
>  	unsigned int version = atmel_mci_get_version(mci);
>  	int busw;
> @@ -359,7 +367,8 @@ static void mci_set_ios(struct mmc *mmc)
>  /* Entered into mmc structure during driver init */
>  static int mci_init(struct mmc *mmc)
>  {
> -	atmel_mci_t *mci = mmc->priv;
> +	struct atmel_mci_priv *priv = mmc->priv;
> +	atmel_mci_t *mci = priv->mci;
>  
>  	/* Initialize controller */
>  	writel(MMCI_BIT(SWRST), &mci->cr);	/* soft reset */
> @@ -393,22 +402,23 @@ int atmel_mci_init(void *regs)
>  {
>  	struct mmc *mmc;
>  	struct mmc_config *cfg;
> -	struct atmel_mci *mci;
> +	struct atmel_mci_priv *priv;
>  	unsigned int version;
>  
> -	cfg = malloc(sizeof(*cfg));
> -	if (cfg == NULL)
> -		return -1;
> -	memset(cfg, 0, sizeof(*cfg));
> +	priv = calloc(1, sizeof(*priv));
> +	if (!priv)
> +		return -ENOMEM;
>  
> -	mci = (struct atmel_mci *)regs;
> +	cfg = &priv->cfg;
>  
>  	cfg->name = "mci";
>  	cfg->ops = &atmel_mci_ops;
>  
> +	priv->mci = (struct atmel_mci *)regs;
> +
>  	/* need to be able to pass these in on a board by board basis */
>  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
> -	version = atmel_mci_get_version(mci);
> +	version = atmel_mci_get_version(priv->mci);
>  	if ((version & 0xf00) >= 0x300) {
>  		cfg->host_caps = MMC_MODE_8BIT;
>  		cfg->host_caps |= MMC_MODE_HS | MMC_MODE_HS_52MHz;
> @@ -425,7 +435,7 @@ int atmel_mci_init(void *regs)
>  
>  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>  
> -	mmc = mmc_create(cfg, regs);
> +	mmc = mmc_create(cfg, priv);
>  
>  	if (mmc == NULL) {
>  		free(cfg);

We shouldn't free cfg here but priv, the rest looks sane to me. Eventual
return -ENODEV on !mmc and adopt the following comment, we may leak priv
now as there is no de-init.

Andreas

> 

[U-Boot] [PATCH 4/4] mmc: atmel: Zap global 'initialized' variable

[Andreas Bießmann]

On 23.10.15 20:46, Marek Vasut wrote:
> Global variables are bad. Get rid of this particular one, so we can
> correctly instantiate multiple atmel mci interfaces, without having
> them interfere with one another.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>

> ---
>  drivers/mmc/gen_atmel_mci.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
> index abc77cc..9a0332f 100644
> --- a/drivers/mmc/gen_atmel_mci.c
> +++ b/drivers/mmc/gen_atmel_mci.c
> @@ -35,10 +35,9 @@
>  struct atmel_mci_priv {
>  	struct mmc_config	cfg;
>  	struct atmel_mci	*mci;
> +	unsigned int		initialized:1;
>  };
>  
> -static int initialized = 0;
> -
>  /* Read Atmel MCI IP version */
>  static unsigned int atmel_mci_get_version(struct atmel_mci *mci)
>  {
> @@ -121,7 +120,7 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
>  
>  	udelay(50);
>  
> -	initialized = 1;
> +	priv->initialized = 1;
>  }
>  
>  /* Return the CMDR with flags for a given command and data packet */
> @@ -210,7 +209,7 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  	u32 error_flags = 0;
>  	u32 status;
>  
> -	if (!initialized) {
> +	if (!priv->initialized) {
>  		puts ("MCI not initialized!\n");
>  		return COMM_ERR;
>  	}
> @@ -415,6 +414,7 @@ int atmel_mci_init(void *regs)
>  	cfg->ops = &atmel_mci_ops;
>  
>  	priv->mci = (struct atmel_mci *)regs;
> +	priv->initialized = 0;
>  
>  	/* need to be able to pass these in on a board by board basis */
>  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
> 

[U-Boot] [PATCH 3/4] mmc: atmel: Implement proper private data

[Marek Vasut]

On Saturday, October 24, 2015 at 12:59:14 AM, Andreas Bie?mann wrote:
> On 23.10.15 20:46, Marek Vasut wrote:
> > Instead of passing just the register area as a private data, introduce
> > a proper struct atmel_mci_priv structure instead. This will become useful
> > in the subsequent patch, where we eliminate the global variable from this
> > driver.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> 
> Reveiwed-by: Andreas Bie?mann <andreas.devel@googlemail.com>
> 
> > ---
> > 
> >  drivers/mmc/gen_atmel_mci.c | 34 ++++++++++++++++++++++------------
> >  1 file changed, 22 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
> > index 8b05fcd..abc77cc 100644
> > --- a/drivers/mmc/gen_atmel_mci.c
> > +++ b/drivers/mmc/gen_atmel_mci.c
> > @@ -32,6 +32,11 @@
> > 
> >  # define MCI_BUS 0
> >  #endif
> > 
> > +struct atmel_mci_priv {
> > +	struct mmc_config	cfg;
> > +	struct atmel_mci	*mci;
> > +};
> > +
> > 
> >  static int initialized = 0;
> >  
> >  /* Read Atmel MCI IP version */
> > 
> > @@ -55,7 +60,8 @@ static void dump_cmd(u32 cmdr, u32 arg, u32 status,
> > const char* msg)
> > 
> >  /* Setup for MCI Clock and Block Size */
> >  static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
> >  {
> > 
> > -	atmel_mci_t *mci = mmc->priv;
> > +	struct atmel_mci_priv *priv = mmc->priv;
> > +	atmel_mci_t *mci = priv->mci;
> > 
> >  	u32 bus_hz = get_mci_clk_rate();
> >  	u32 clkdiv = 255;
> >  	unsigned int version = atmel_mci_get_version(mci);
> > 
> > @@ -198,7 +204,8 @@ io_fail:
> >  static int
> >  mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data
> >  *data) {
> > 
> > -	atmel_mci_t *mci = mmc->priv;
> > +	struct atmel_mci_priv *priv = mmc->priv;
> > +	atmel_mci_t *mci = priv->mci;
> > 
> >  	u32 cmdr;
> >  	u32 error_flags = 0;
> >  	u32 status;
> > 
> > @@ -323,7 +330,8 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> > struct mmc_data *data)
> > 
> >  /* Entered into mmc structure during driver init */
> >  static void mci_set_ios(struct mmc *mmc)
> >  {
> > 
> > -	atmel_mci_t *mci = mmc->priv;
> > +	struct atmel_mci_priv *priv = mmc->priv;
> > +	atmel_mci_t *mci = priv->mci;
> > 
> >  	int bus_width = mmc->bus_width;
> >  	unsigned int version = atmel_mci_get_version(mci);
> >  	int busw;
> > 
> > @@ -359,7 +367,8 @@ static void mci_set_ios(struct mmc *mmc)
> > 
> >  /* Entered into mmc structure during driver init */
> >  static int mci_init(struct mmc *mmc)
> >  {
> > 
> > -	atmel_mci_t *mci = mmc->priv;
> > +	struct atmel_mci_priv *priv = mmc->priv;
> > +	atmel_mci_t *mci = priv->mci;
> > 
> >  	/* Initialize controller */
> >  	writel(MMCI_BIT(SWRST), &mci->cr);	/* soft reset */
> > 
> > @@ -393,22 +402,23 @@ int atmel_mci_init(void *regs)
> > 
> >  {
> >  
> >  	struct mmc *mmc;
> >  	struct mmc_config *cfg;
> > 
> > -	struct atmel_mci *mci;
> > +	struct atmel_mci_priv *priv;
> > 
> >  	unsigned int version;
> > 
> > -	cfg = malloc(sizeof(*cfg));
> > -	if (cfg == NULL)
> > -		return -1;
> > -	memset(cfg, 0, sizeof(*cfg));
> > +	priv = calloc(1, sizeof(*priv));
> > +	if (!priv)
> > +		return -ENOMEM;
> > 
> > -	mci = (struct atmel_mci *)regs;
> > +	cfg = &priv->cfg;
> > 
> >  	cfg->name = "mci";
> >  	cfg->ops = &atmel_mci_ops;
> > 
> > +	priv->mci = (struct atmel_mci *)regs;
> > +
> > 
> >  	/* need to be able to pass these in on a board by board basis */
> >  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
> > 
> > -	version = atmel_mci_get_version(mci);
> > +	version = atmel_mci_get_version(priv->mci);
> > 
> >  	if ((version & 0xf00) >= 0x300) {
> >  	
> >  		cfg->host_caps = MMC_MODE_8BIT;
> >  		cfg->host_caps |= MMC_MODE_HS | MMC_MODE_HS_52MHz;
> > 
> > @@ -425,7 +435,7 @@ int atmel_mci_init(void *regs)
> > 
> >  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
> > 
> > -	mmc = mmc_create(cfg, regs);
> > +	mmc = mmc_create(cfg, priv);
> > 
> >  	if (mmc == NULL) {
> >  	
> >  		free(cfg);
> 
> We shouldn't free cfg here but priv, the rest looks sane to me. Eventual
> return -ENODEV on !mmc and adopt the following comment, we may leak priv
> now as there is no de-init.

Uh right, can you fix that while applying the patchset or do you want me to 
repost ?

[U-Boot] [PATCH 3/4] mmc: atmel: Implement proper private data

[Andreas Bießmann]

Hi Marek,

On 24.10.15 01:29, Marek Vasut wrote:
> On Saturday, October 24, 2015 at 12:59:14 AM, Andreas Bie?mann wrote:
>> On 23.10.15 20:46, Marek Vasut wrote:
>>> Instead of passing just the register area as a private data, introduce
>>> a proper struct atmel_mci_priv structure instead. This will become useful
>>> in the subsequent patch, where we eliminate the global variable from this
>>> driver.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>
>> Reveiwed-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>>
>>> ---
>>>
>>>  drivers/mmc/gen_atmel_mci.c | 34 ++++++++++++++++++++++------------
>>>  1 file changed, 22 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
>>> index 8b05fcd..abc77cc 100644
>>> --- a/drivers/mmc/gen_atmel_mci.c
>>> +++ b/drivers/mmc/gen_atmel_mci.c

>>> @@ -425,7 +435,7 @@ int atmel_mci_init(void *regs)
>>>
>>>  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>>>
>>> -	mmc = mmc_create(cfg, regs);
>>> +	mmc = mmc_create(cfg, priv);
>>>
>>>  	if (mmc == NULL) {
>>>  	
>>>  		free(cfg);
>>
>> We shouldn't free cfg here but priv, the rest looks sane to me. Eventual
>> return -ENODEV on !mmc and adopt the following comment, we may leak priv
>> now as there is no de-init.
> 
> Uh right, can you fix that while applying the patchset or do you want me to 
> repost ?

I'll fix it.

Andreas

[U-Boot] [PATCH 3/4] mmc: atmel: Implement proper private data

[Marek Vasut]

On Saturday, October 24, 2015 at 10:35:45 AM, Andreas Bie?mann wrote:
> Hi Marek,

Hi!

> On 24.10.15 01:29, Marek Vasut wrote:
> > On Saturday, October 24, 2015 at 12:59:14 AM, Andreas Bie?mann wrote:
> >> On 23.10.15 20:46, Marek Vasut wrote:
> >>> Instead of passing just the register area as a private data, introduce
> >>> a proper struct atmel_mci_priv structure instead. This will become
> >>> useful in the subsequent patch, where we eliminate the global variable
> >>> from this driver.
> >>> 
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >> 
> >> Reveiwed-by: Andreas Bie?mann <andreas.devel@googlemail.com>
> >> 
> >>> ---
> >>> 
> >>>  drivers/mmc/gen_atmel_mci.c | 34 ++++++++++++++++++++++------------
> >>>  1 file changed, 22 insertions(+), 12 deletions(-)
> >>> 
> >>> diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
> >>> index 8b05fcd..abc77cc 100644
> >>> --- a/drivers/mmc/gen_atmel_mci.c
> >>> +++ b/drivers/mmc/gen_atmel_mci.c
> >>> 
> >>> @@ -425,7 +435,7 @@ int atmel_mci_init(void *regs)
> >>> 
> >>>  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
> >>> 
> >>> -	mmc = mmc_create(cfg, regs);
> >>> +	mmc = mmc_create(cfg, priv);
> >>> 
> >>>  	if (mmc == NULL) {
> >>>  	
> >>>  		free(cfg);
> >> 
> >> We shouldn't free cfg here but priv, the rest looks sane to me. Eventual
> >> return -ENODEV on !mmc and adopt the following comment, we may leak priv
> >> now as there is no de-init.
> > 
> > Uh right, can you fix that while applying the patchset or do you want me
> > to repost ?
> 
> I'll fix it.

Thanks!

Best regards,
Marek Vasut

[U-Boot] [U-Boot,1/4] mmc: atmel: Silence debug output

[Andreas Bießmann]

Dear Marek Vasut,

Marek Vasut <marex@denx.de> writes:
>This driver generates clearly debugging prints when changing clock
>speed, so silence those. Furthermore, the driver generates further
>prints in case a command fails to complete. The later case woud be
>useful, but for eMMC, command 8 can fail and it's not an error but
>a part of the specification. Thus, make this debug() as well.
>
>Signed-off-by: Marek Vasut <marex@denx.de>
>Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>[fix checkpatch warnings]
>Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>---
> drivers/mmc/gen_atmel_mci.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)

applied to u-boot-atmel/master, thanks!

Also applied some chackpatch fixes.

Best regards,
Andreas Bie?mann

[U-Boot] [U-Boot,2/4] mmc: atmel: Fix clock configuration

[Andreas Bießmann]

Dear Marek Vasut,

Marek Vasut <marex@denx.de> writes:
>After silencing the prints which were generated when reconfiguring the
>clock of the SD/MMC bus, surprisingly, the driver stopped working such
>that every attempt to use the SD/MMC bus caused the CPU to get totally
>stuck hard. It turns out that the prints generated a short delay, which
>was necessary for the CPU to reconfigure the clock without getting stuck.
>Thus, this patch adds a short delay after the clock configuration instead.
>
>Signed-off-by: Marek Vasut <marex@denx.de>
>Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>---
> drivers/mmc/gen_atmel_mci.c | 2 ++
> 1 file changed, 2 insertions(+)

applied to u-boot-atmel/master, thanks!

Best regards,
Andreas Bie?mann

[U-Boot] [U-Boot, 3/4] mmc: atmel: Implement proper private data

[Andreas Bießmann]

Dear Marek Vasut,

Marek Vasut <marex@denx.de> writes:
>Instead of passing just the register area as a private data, introduce
>a proper struct atmel_mci_priv structure instead. This will become useful
>in the subsequent patch, where we eliminate the global variable from this
>driver.
>
>Signed-off-by: Marek Vasut <marex@denx.de>
>Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>[fix free()]
>Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>---
> drivers/mmc/gen_atmel_mci.c | 40 +++++++++++++++++++++++++---------------
> 1 file changed, 25 insertions(+), 15 deletions(-)

applied to u-boot-atmel/master, thanks!

Also fix the missed free().

Best regards,
Andreas Bie?mann

[U-Boot] [U-Boot, 4/4] mmc: atmel: Zap global 'initialized' variable

[Andreas Bießmann]

Dear Marek Vasut,

Marek Vasut <marex@denx.de> writes:
>Global variables are bad. Get rid of this particular one, so we can
>correctly instantiate multiple atmel mci interfaces, without having
>them interfere with one another.
>
>Signed-off-by: Marek Vasut <marex@denx.de>
>Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>---
> drivers/mmc/gen_atmel_mci.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)

applied to u-boot-atmel/master, thanks!

Best regards,
Andreas Bie?mann