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 <ma...@denx.de>

Reveiwed-by: Andreas Bießmann <andreas.de...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to