[PATCH v2 02/10] mmc: Add init() API

[Simon Goldschmidt]

Forgot to ask: why isn't the subsystem maintainer on CC?
If you would use patman to send patches or at least the get_maintainer script,
he would have been...

Regards,
Simon

On Wed, Jan 29, 2020 at 9:03 AM Simon Goldschmidt
<simon.k.r.goldschmidt at gmail.com> wrote:
>
> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas at ti.com> wrote:
> >
> > Add an init() API for platform specific init() operations.
>
> Could you describe why this cannot be done in the probe callback? It's not
> easily visible as the function you changed (mmc_get_op_cond) doesn't even have
> a comment to describe what it does...
>
> In general, I think commit messages could be more detailed than one line. If
> only to make it easier in the future to recap why things have been done.
>
> >
> > Signed-off-by: Faiz Abbas <faiz_abbas at ti.com>
> > Signed-off-by: Lokesh Vutla <lokeshvutla at ti.com>
> > ---
> >  drivers/mmc/mmc.c | 13 ++++++-------
> >  include/mmc.h     |  7 +++++++
> >  2 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > index d43983d4a6..50df8c8626 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -2787,14 +2787,13 @@ int mmc_get_op_cond(struct mmc *mmc)
> >         }
> >         if (err)
> >                 return err;
> > -
> >  #if CONFIG_IS_ENABLED(DM_MMC)
> > -       /* The device has already been probed ready for use */
> > -#else
> > -       /* made sure it's not NULL earlier */
> > -       err = mmc->cfg->ops->init(mmc);
> > -       if (err)
> > -               return err;
>
> You're removing the init code for non-DM MMC here and did not describe it in
> the commit message. Is this change intended at all?
>
> Regards,
> Simon
>
> > +       struct dm_mmc_ops *ops = mmc_get_ops(mmc->dev);
> > +       if (ops->init) {
> > +               err = ops->init(mmc->dev);
> > +               if (err)
> > +                       return err;
> > +       }
> >  #endif
> >         mmc->ddr_mode = 0;
> >
> > diff --git a/include/mmc.h b/include/mmc.h
> > index 2f21dbf1b7..6aef125f25 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -406,6 +406,13 @@ struct mmc;
> >
> >  #if CONFIG_IS_ENABLED(DM_MMC)
> >  struct dm_mmc_ops {
> > +       /**
> > +        * init() - platform specific initialization for the host controller
> > +        *
> > +        * @dev:        Device to init
> > +        * @return 0 if Ok, -ve if error
> > +        */
> > +       int (*init)(struct udevice *dev);
> >         /**
> >          * send_cmd() - Send a command to the MMC device
> >          *
> > --
> > 2.19.2
> >