nvmem: Defining cells on mtd created by mtdparts

nvmem: Defining cells on mtd created by mtdparts

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]

Hi,

OpenWrt switched [1] their MAC address (from flash) retrieval code from their 
mtd-mac-address based solution [2] to nvmem-cells. The mtd-mac-address based 
solution had the benefit that it could find the correct partition by using 
get_mtd_device_nm - which was label based. So a lookup for a partition which 
was defined via mtdparts was absolutely no problem. This doesn't seem to be
the case for nvmem - at least not how it is integrated at the moment in
OpenWrt.

This means that the performed switch broke the vendor defined MAC addresses 
when the u-boot must dynamically define the partitions via mtdpart - and where 
fixed-partitions are not possible in the DT. The bootloaders used by the ath79 
usually have no devicetree support and cannot modify the device tree (beside 
the problem to get the sources for these bootloaders). These devices will now 
only have random mac addresses.

Since there are most likely more devices out there which use mtdparts, I would 
guess that there might already be a strategy out there which can be used to 
define the nvmem-provider for mtdparts defined partitions. At least I saw that 
Bartosz Golaszewski added all the mtd devices automatically as nvmem provider 
in c4dfa25ab307 ("mtd: add support for reading MTD devices via the nvmem 
API"). So there might also be something for nvmem-cells to find the correct 
mtd instead of relying on the fixed-partitions registration + of_node (which 
doesn't exist because it comes from mtdparts and not devicetree).

Right now nvmem_cell_get (actually __nvmem_device_get) in 
nvmem_get_mac_address just return -517 (EPROBE_DEFER). So the nvmem_device is 
not yet registered - which absolutely makes sense when mtdparts is used. 
of_nvmem_find will just not be able to find the of_node for this partition
via bus_find_device_by_of_node because there is no such of_node for mtdparts 
partitions.

Kind regards,
	Sven

[1] https://github.com/openwrt/openwrt/pull/4041
[2] https://github.com/openwrt/openwrt/commit/5ae2e786395c7f9db0167ebe875be5df9502d8d8

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: nvmem: Defining cells on mtd created by mtdparts

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

On Sunday, 10 October 2021 14:53:13 CEST Sven Eckelmann wrote:
[...]
> Since there are most likely more devices out there which use mtdparts, I would 
> guess that there might already be a strategy out there which can be used to 
> define the nvmem-provider for mtdparts defined partitions. At least I saw that 
> Bartosz Golaszewski added all the mtd devices automatically as nvmem provider 
> in c4dfa25ab307 ("mtd: add support for reading MTD devices via the nvmem 
> API"). So there might also be something for nvmem-cells to find the correct 
> mtd instead of relying on the fixed-partitions registration + of_node (which 
> doesn't exist because it comes from mtdparts and not devicetree).

Ansuel Smith just proposed in OpenWrt [1] a workaround. It basically adds a 
minimal fixed-partitions parser to the mtd cmdlinepart parser (responsible for 
the mtdparts=) that tries to find the matching (size + offset) fixed-partition 
from the devicetree. The code in mtd_device_parse_register
(add_mtd_partitions -> add_mtd_device -> mtd_nvmem_add) will then 
automatically take care of the rest.

Kind regards,
	Sven

[1] https://github.com/openwrt/openwrt/pull/4664#issuecomment-939567963

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: nvmem: Defining cells on mtd created by mtdparts

[Pratyush Yadav]

Hi Sven,

On 11/10/21 09:06AM, Sven Eckelmann wrote:
> On Sunday, 10 October 2021 14:53:13 CEST Sven Eckelmann wrote:
> [...]
> > Since there are most likely more devices out there which use mtdparts, I would 
> > guess that there might already be a strategy out there which can be used to 
> > define the nvmem-provider for mtdparts defined partitions. At least I saw that 
> > Bartosz Golaszewski added all the mtd devices automatically as nvmem provider 
> > in c4dfa25ab307 ("mtd: add support for reading MTD devices via the nvmem 
> > API"). So there might also be something for nvmem-cells to find the correct 
> > mtd instead of relying on the fixed-partitions registration + of_node (which 
> > doesn't exist because it comes from mtdparts and not devicetree).

I have been wanting to fix this problem for a while but just never got 
around to it. I was thinking about either extending the mtdparts syntax 
to maybe add nvmem cell information in there or adding a separate 
cmdline argument that complements mtdparts with nvmem cell info. Dunno 
if either of these would work well though...

> 
> Ansuel Smith just proposed in OpenWrt [1] a workaround. It basically adds a 
> minimal fixed-partitions parser to the mtd cmdlinepart parser (responsible for 
> the mtdparts=) that tries to find the matching (size + offset) fixed-partition 
> from the devicetree. The code in mtd_device_parse_register
> (add_mtd_partitions -> add_mtd_device -> mtd_nvmem_add) will then 
> automatically take care of the rest.

I don't quite see how this helps. You say that some devices don't have a 
device tree at all so how would you even match the fixed partition? And 
this of course doesn't solve the problem where you might want nvmem 
cells with a partition layout that is different from the one in device 
tree.

I unfortunately don't really have any answers for this at this point, 
but maybe I can figure something out in the future...

> 
> Kind regards,
> 	Sven
> 
> [1] https://github.com/openwrt/openwrt/pull/4664#issuecomment-939567963



> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/


-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: nvmem: Defining cells on mtd created by mtdparts

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 2050 bytes --]

On Tuesday, 12 October 2021 20:24:46 CEST Pratyush Yadav wrote:
[...]
> I have been wanting to fix this problem for a while but just never got 
> around to it. I was thinking about either extending the mtdparts syntax 
> to maybe add nvmem cell information in there or adding a separate 
> cmdline argument that complements mtdparts with nvmem cell info. Dunno 
> if either of these would work well though...

At least for the devices I have in mind, this doesn't seem to be a good 
solution. The devices are shipped since years and the bootloader isn't updated 
with firmware updates. So changing to a different mtdparts might sometimes not 
be easily possible.

But this might help in situations which don't have this limitation.

> > Ansuel Smith just proposed in OpenWrt [1] a workaround. It basically adds a 
> > minimal fixed-partitions parser to the mtd cmdlinepart parser (responsible for 
> > the mtdparts=) that tries to find the matching (size + offset) fixed-partition 
> > from the devicetree. The code in mtd_device_parse_register
> > (add_mtd_partitions -> add_mtd_device -> mtd_nvmem_add) will then 
> > automatically take care of the rest.
> 
> I don't quite see how this helps. You say that some devices don't have a 
> device tree at all so how would you even match the fixed partition? And 
> this of course doesn't solve the problem where you might want nvmem 
> cells with a partition layout that is different from the one in device 
> tree.

I personally had a different thing in mind. The situation for OpenWrt's ath79 
target (and Linux's ath79) is that they use DTs and some of these devices are 
still using some information which are not part of the device tree. But the
DT partitions for the nvmem-cells MUST match the ones in mtdparts

But you are right, this will definitely not solve situations when there is no 
DT used. And didn't say that I needed a solution for this - I was (hopefully) 
always referring to devices which use mtdparts (which can also be used with 
devices which are using DTs)

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: nvmem: Defining cells on mtd created by mtdparts

[Rafał Miłecki]

On 11.10.2021 09:06, Sven Eckelmann wrote:
> On Sunday, 10 October 2021 14:53:13 CEST Sven Eckelmann wrote:
> [...]
>> Since there are most likely more devices out there which use mtdparts, I would
>> guess that there might already be a strategy out there which can be used to
>> define the nvmem-provider for mtdparts defined partitions. At least I saw that
>> Bartosz Golaszewski added all the mtd devices automatically as nvmem provider
>> in c4dfa25ab307 ("mtd: add support for reading MTD devices via the nvmem
>> API"). So there might also be something for nvmem-cells to find the correct
>> mtd instead of relying on the fixed-partitions registration + of_node (which
>> doesn't exist because it comes from mtdparts and not devicetree).
> 
> Ansuel Smith just proposed in OpenWrt [1] a workaround. It basically adds a
> minimal fixed-partitions parser to the mtd cmdlinepart parser (responsible for
> the mtdparts=) that tries to find the matching (size + offset) fixed-partition
> from the devicetree. The code in mtd_device_parse_register
> (add_mtd_partitions -> add_mtd_device -> mtd_nvmem_add) will then
> automatically take care of the rest.

I like the idea of connecting cmdline specified partitions (mtdparts=)
with DT. That will help devices that use DT at least.

One thing I'm not sure about is criteria. Ansuel in his patch
[PATCH] drivers: mtd: parsers: add nvmem support to cmdlinepart
https://gist.github.com/Ansuel/35abba1f5663ea3d9bd8eded01e8d95b
requires a matching name, offset & size.

I guess above may work for partitions that stay static like a
bootloader. What if someone wants to describe more dynamic partition
(firmware?). Should we reduce criteria to allow matching just by a
name?