Re: [PATCH v2 01/16] nvmem: remove unused APIs

From: Bartosz Golaszewski
Date: Mon Sep 10 2018 - 08:22:29 EST


2018-09-10 14:18 GMT+02:00 Boris Brezillon <boris.brezillon@xxxxxxxxxxx>:
> Hi Srinivas,
>
> On Mon, 10 Sep 2018 12:47:19 +0100
> Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> wrote:
>
>> On 10/09/18 12:31, Bartosz Golaszewski wrote:
>> > About that: there are no users of nvmem_device_cell_read/write()
>> > currently and with the new API I'm not sure this is still needed. Are
>> > you alright with removing those two?
>>
>> Why do you want to remove them? Other than reason of no users.
>
> I'm just sharing my (and probably other maintainers/developers) PoV
> here, so please don't take this as an attempt to force you to change
> your mind, but rather an attempt at explaining why APIs usually stay
> private when they have no users.
>
> Kernel maintainers tend to reject APIs that have no users because
> adding something that nobody needs yet is hard to get right. I mean,
> you can design an API on what you think will be needed/appropriate, and
> then, when people actually start needing this feature, they realize it
> does not quite match what they need, and they have to adjust/rework
> this API.
>
>>
>> All it boils down to if we support device based apis using cell info or
>> not?
>
> But it looks like nobody is actually using it, and the first potential
> user (Bart) proposed a different approach with the nvmem cell table
> declaration.
>
>> IMO, I see no harm in leaving these apis as it is, unless there is a
>> strong reason to do so.
>
> It's harmless, but it's also unused. If people start using it and you
> realize the API is not working for some use cases, then you'll have to
> patch all existing users.
>
> All this being said, it's your call to make, and if you think it makes
> sense to keep these functions around for any reason, then be it.
>

Also: the general notion in most subsystems is that drivers should be
generalized enough to not have to care about the provider's internals,
which is the case if we allow users to specify cell info. They should
just query the subsystem for a given resource, use it, then put it.
Fortunately this is what all current users do in this case - we don't
even have any users of nvmem_device_get() which is good since it
should be reserved for very special cases.

While one can argue that nvmem_device_cell_read/write() may be needed
in some strange corner case now, once we add the cell lookup API, we
can safely remove it and force users to do the right thing.

Best regards,
Bartosz Golaszewski