Re: [PATCH V5 05/13] mmc: core: Add support for handling CQE requests

Linus Walleij
2017-08-31T11:32:58Z

On Mon, Aug 21, 2017 at 11:26 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 20/08/17 14:39, Linus Walleij wrote:
>> On Thu, Aug 10, 2017 at 2:08 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>>> Add core support for handling CQE requests, including starting, completing
>>> and recovering.
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>
>>
>>> +static void __mmc_cqe_request_done(struct mmc_host *host,
>>> + struct mmc_request *mrq)
>>
>> We are naming too much stuff __foo now, it gets really hard to figure
>> out from the function name and the surrounding code what is going on.
>
> You have written several times that you don't like __foo() names, however it
> is a normal kernel paradigm.

Normal doesn't mean "good".

I am negative to it because it has very unclear semantics. What is the
semantic meaning of prefixing a function with __* really?

I have referred to Rusty Russell's API levels:
http://sweng.the-davies.net/Home/rustys-api-design-manifesto

This is on level 3-4 and definately not at 6.

So in my opinion, I have informed, founded in theory and
valid reasons to dislike it, and I don't think it is a matter of taste or
opinion.

>> I guess people are using this like "do parts of what mmc_cqe_request_done()
>> is doing" but it'd be nice if we could be specific.
>>
>> mmc_cqe_request_finalize() could work?
>
> It can be rolled into mmc_cqe_request_done().

OK!

Yours,
Linus Walleij