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.goldschm...@gmail.com> wrote:
>
> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_ab...@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_ab...@ti.com>
> > Signed-off-by: Lokesh Vutla <lokeshvu...@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
> >
