[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 at denx.de>
> 
> Reveiwed-by: Andreas Bießmann <andreas.devel at 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 ?