lkml.org 
[lkml]   [2023]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access
From


On 08/03/2023 15:42, Rafał Miłecki wrote:
> On 8.03.2023 14:31, Srinivas Kandagatla wrote:
>> Thanks for doing this,
>
> Thank you for reviewing. Sadly it seems it still isn't clear if we can
> have this generic driver.

I don't mean to be rude, but TBH, I don't see any value for this ATM, it
is going to add something that we need to keep updating for every user.

Unless anyone thinks otherwise.

>
> I guess I missed some important questions or comments. In previous
> series we were discussing implementation details so I thought it's OK to
> have this driver after all. Not sure if I didn't waste time working on
> V4. I'll see if I can I address your concerns (see below).
Lets not waste your time for now, we can revist this once we have more
users.

thanks,
srini
>
>
>> On 28/02/2023 07:29, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Some NVMEM devices can be accessed by simply mapping memory and reading
>>> from / writing to it. This driver adds support for a generic
>>> "mmio-nvmem" DT binding used by such devices.
>>>
>>> One of such devices is Broadcom's NVRAM. It's already supported (see
>>> NVMEM_BRCM_NVRAM) but existing driver covers both:
>>
>> What will happen to the old "brcm,nvram" compatible and the dt
>> firmware that already have this node?
>
> I treat backward compatibility with previouly used bindings very
> seriously. I'm going to keep it. I may make an attempt to drop it in
> few years if it's very unlikely to break any setups.
>
>
>> If there is only one user for this then one would object that why do
>> we need this DT level of abstraction to start with?
>> If this is not the case please consider adding those patches to this
>> series.
>
> Existing Linux drivers prove that there is more hardware with MMIO based
> read access: brcm_nvram, mtk-efuse, uniphier-efuse. Migration of other
> drivers (mtk, unipher) is on hold as apparently there may be support for
> writing support soon. In any case this MMIO solution isn't completely
> unique to Broadcom.
> I don't have other patches to add to it right now.
>
>
>>> 1. NVMEM device access
>>> 2. NVMEM content parsing
>>>
>>> Once we get support for NVMEM layouts then existing NVRAM driver will
>>> get converted into a layout and generic driver will take over
>>> responsibility for data access.
>>>
>>
>> Even though this series is simple, but it is really confusing for two
>> reasons.
>>
>> 1> Generic mmio nvmem bindings are incomplete and potentially
>> change/evolve on every new user. Ex clks, regulators, endianess ... So
>> it looks really fragile and incomplete to me as a generic bindings.
>> Is this want you are expecting?
>
> All 3 existing hardware support MMIO reads without extra clocks or
> regulators. I'm not sure if endianess belongs to this layer. Isn't that
> NVMEM content thing?
>
> I'm not claiming this driver is in its final and perfect state. For
> simple hardware that needs minor fixups we can add those later to this
> generic driver. Adding clocks should be possible, fine and easy.
>
> I'm sure there will be more complex hardware that we will not be able
> to support with this driver. It's require another driver and I'm fine
> with that.
>
>
>> 2> As you mentioned that this will replace broadcom NVMRAM, but this
>> patch does nothing in relation to updating that driver, so the code is
>> dead as it is. If you are considering to use it for Broadcom NVMRAM,
>> please add those patches to this series so that we could see the real
>> user for this code.
>
> Of course it does nothing because there are no layouts yet. I could
> migrate brcm_nvram into layout once there is layouts support.
>
> I don't agree this code is dead. It support new binding. It works.
> Every new binding and its driver are "dead" until you add first DT
> users.
>
> Here is real use:
>
> nvmem@1eff0000 {
>     compatible = "mmio-nvmem";
>     reg = <0x1eff0000 0x10000>;
> };
>

\
 
 \ /
  Last update: 2023-03-27 00:51    [W:0.053 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site