Re: [RFC PATCH 01/10] mmc: core: improve API to make clear mmc_hw_reset is for cards

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

 



On Wed, 6 Apr 2022 at 09:58, Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Hi Ulf,
>
> > > To make it unambiguous that mmc_hw_reset() is for cards and not for
> > > controllers, we a) add 'card' to the function name and b) make the
> > > function argument mmc_card instead of mmc_host. A fallback is provided
> > > until all users are converted.
> >
> > In my opinion, I think b) is sufficient and would be a nice improvement.
>
> If you say so... but I do wonder why we can't be super clear with the
> function name alone without the function argument as an additional
> source of information? Kernel hacking is complicated enough.
>
> > In this regard, I suggest we make one big cross-subsystem patch (the
> > smallest change as possible) then I can pick it up and send it for the
> > v5.18-rc2.
>
> Ok, I can prepare this.

Great!

>
> > > -static void mmc_hw_reset_for_init(struct mmc_host *host)
> > > +/* we can't use mmc_card as a parameter, it is not populated yet */
> >
> > Please drop this. The function is internal/static and at least to me,
> > rather self-explanatory.
>
> All other ?w_reset() functions have a card as a parameter. For people
> trying to get into the MMC core, this comment might be helpful to
> understand the anomaly? I know that you as the maintainer do know this
> by heart, this comment is meant for people learning the stuff.

I understand your point, however I don't think it makes much sense to
try to clarify the names on mmc_hw|sw_reset() alone. There are simply
lots of other functions that then would need to be changed too.
Otherwise we would just end up with having even more in-consistency in
function namings. To me, that's even worse.

>
> All the best,
>
>    Wolfram

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