Re: [PATCH] mmc: core: document mmc_hw_reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 18 Sep 2020 at 23:48, Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Sep 16, 2020 at 11:47:10AM +0200, Ulf Hansson wrote:
> > On Wed, 16 Sep 2020 at 11:01, Wolfram Sang
> > <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > Add documentation for mmc_hw_reset to make sure the intended use case is
> > > clear.
> > >
> > > Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> > > ---
> > >
> > > While working on this, I get the feeling this function should be renamed
> > > to 'mmc_card_reset' or something similar. 'hw' is still confusing
> > > because it could easily be the host controller, too. I volunteer to
> > > prepare a patch if we can agree on a better name.
> >
> > You have a point. Although we also have mmc_sw_reset().
>
> Ah, I didn't know that. Though, mmc_card_{hw|sw}_reset() sounds still
> way better to me.

I fully agree.

>
> > Another thing that I would like to change is to make both of these
> > functions take a struct mmc_card* as in-parameter, rather than the
> > current struct mmc_host*.
> >
> > Not sure that it would completely solve the confusion, but at least it
> > would be a little more clear.
>
> Maybe if we do both, it will be really clear? :)

Makes perfect sense to me.

>
> > > +/**
> > > + * mmc_hw_reset - reset the card
> > > + * @host: MMC host to which the card is attached
> > > + *
> > > + * Reset the remote card. This function is only for upper layers, like the
> >
> > Perhaps make it clear that it's a full (or hw) reset, not just a reset
> > (as it could also be a soft reset). Moreover, I think you can skip
> > "remote".
>
> OK, will send V2 in a minute.
>

I have applied that one. Let's deal with potential additional
improvements on top.

Kind regards
Uffe



[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux