Re: [PATCH] nvmem: layouts: Add fixed-layout driver

[Miquel Raynal]

Hello,

>>> Current implementation isn't working well when device tree nodes have a
>>> phandle on a fixed-layout nvmem node. As the fixed layout is handled in
>>> nvmem core, no driver is ever associated with the layout, and the device
>>> consumer driver probe is deferred indefinitely.
>>>
>>> Remove the specific handling of fixed-layout and add a layout driver.
>>> This makes the fixed-layout similar to all other layouts, fixing the
>>> whole issue.
>>
>> I guess this deserves a Fixes tag as well as a stable backport.
>>
>
> That's right. I will add:
> Fixes: fc29fd821d9a (nvmem: core: Rework layouts to become regular devices)
>
> Arguably, we could reference a commit even before, but I believe this is
> the commit that introduced a different behaviour between fixed-layout
> and other layouts.

Agreed.

Please also don't forget to add there:

Cc: stable@xxxxxxxxxxxxxxx

>>> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@xxxxxxxxxxx>
>>> ---
>>>  MAINTAINERS                          |  5 ++++
>>>  drivers/nvmem/core.c                 | 23 +---------------
>>>  drivers/nvmem/layouts.c              | 11 --------
>>>  drivers/nvmem/layouts/Makefile       |  1 +
>>>  drivers/nvmem/layouts/fixed-layout.c | 52 ++++++++++++++++++++++++++++++++++++
>>>  include/linux/nvmem-provider.h       |  7 +++++
>>>  6 files changed, 66 insertions(+), 33 deletions(-)
>>>
>>
>> [...]
>>
>>> diff --git a/drivers/nvmem/layouts/Makefile b/drivers/nvmem/layouts/Makefile
>>> index 4940c9db0665..fbc195aa382c 100644
>>> --- a/drivers/nvmem/layouts/Makefile
>>> +++ b/drivers/nvmem/layouts/Makefile
>>> @@ -3,6 +3,7 @@
>>>  # Makefile for nvmem layouts.
>>>  #
>>>  
>>> +obj-y += fixed-layout.o
>>
>> This implies that you cannot remove that driver. I'm fine with it, but
>> maybe since this is now almost costless, we could have a Kconfig entry
>> that defaults to =y? This is not a blocker, I have no strong
>> opinion. Srinivas?
>>
>
> Right. My first idea was fixed-layout is still a bit special, and should
> always be present, just as before. But thinking again about it, there is
> no real reason to make it different.
>
> I will wait for Srinivas opinion, but I'm open to make it optional.
> Should this be a separate commit, so it can be excluded from a potential
> backport?

Definitely.

>>>  obj-$(CONFIG_NVMEM_LAYOUT_SL28_VPD) += sl28vpd.o
>>>  obj-$(CONFIG_NVMEM_LAYOUT_ONIE_TLV) += onie-tlv.o
>>>  obj-$(CONFIG_NVMEM_LAYOUT_U_BOOT_ENV) += u-boot-env.o
>>> diff --git a/drivers/nvmem/layouts/fixed-layout.c b/drivers/nvmem/layouts/fixed-layout.c
>>> new file mode 100644
>>> index 000000000000..e5078d72a6fc
>>> --- /dev/null
>>> +++ b/drivers/nvmem/layouts/fixed-layout.c
>>> @@ -0,0 +1,52 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright 2025 Bootlin
>>
>> Can probably be bumped :-)
>>
>
> Thanks :)
>
>>> + *
>>> + * Authors: Mathieu Dubois-Briand <mathieu.dubois-briand@xxxxxxxxxxx>
>>> + */
>>> +
>>> +#include <linux/nvmem-provider.h>
>>> +#include <linux/of.h>
>>
>> Thanks,
>> Miquèl
>
> Thanks for your review,
> Mathieu