RE: [PATCH 1/6] dt-bindings: nvmem: add cell-type to nvmem cells

[Joakim Zhang]

Hi Ahmad,

> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> Sent: 2021年9月22日 19:34
> To: Joakim Zhang <qiangqing.zhang@xxxxxxx>;
> srinivas.kandagatla@xxxxxxxxxx; robh+dt@xxxxxxxxxx; shawnguo@xxxxxxxxxx;
> Jan Lübbe <jlu@xxxxxxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>;
> kernel@xxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/6] dt-bindings: nvmem: add cell-type to nvmem cells
> 
> Hi,
> 
> On 08.09.21 12:02, Joakim Zhang wrote:
> > From: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> >
> > Some of the nvmem providers encode data for certain type of nvmem
> > cell, example mac-address is stored in ascii or with delimiter or in reverse
> order.
> >
> > This is much specific to vendor, so having a cell-type would allow
> > nvmem provider drivers to post-process this before using it.
> 
> I don't agree with this assessment. Users of the OCOTP so far used this specific
> encoding. Bootloaders decode the OCOTP this way, but this encoding isn't
> really an inherent attribute of the OCOTP. A new NXP SoC with a different OTP
> IP will likely use the same format. Users may even use the same format on an
> EEPROM to populate a second off-SoC interface, .. etc.
> 
> I'd thus prefer to not make this specific to the OCOTP as all:
> 
>   * #define NVMEM_CELL_ENCODING_MAC_ADDRESS_IMX	/* ... */
> 
>   * cell-type = <NVMEM_CELL_ENCODING_MAC_ADDRESS_IMX>;
> 
>   * and then the decoder is placed into some generic location, e.g.
>    drivers/nvmem/encodings.c for Linux
> 
> That way, we can reuse this and future encodings across nvmem providers.
> It's also more extendable: e.g. big endian fields on EEPROMs. Just stick the
> cell-type in, document it in the binding and drivers supporting it will interpret
> bytes appropriately.
> 
> It's still a good idea to record the type as well as the encoding, e.g. split the 32
> bit encoding constant into two 16-bit values.
> One is an enum of possible types (unknown, mac_address, IP address ... etc.)
> and one is an enum of the available encodings.
> 
> What do you think?

Go through the thread you discussed with Srinivas, as we discussed before, we prefer to offload this decoding to
specific nvmem provider driver, instead of nvmem core. 

Best Regards,
Joakim Zhang