| Message ID | 20210908100257.17833-2-qiangqing.zhang@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | nvmem: add "cell-type" property to support mac-address | expand |
Hi, On 08.09.21 12:02, Joakim Zhang wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > 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? > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > --- > Documentation/devicetree/bindings/nvmem/nvmem.yaml | 11 +++++++++++ > include/dt-bindings/nvmem/nvmem.h | 8 ++++++++ > 2 files changed, 19 insertions(+) > create mode 100644 include/dt-bindings/nvmem/nvmem.h > > diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml > index b8dc3d2b6e92..8cf6c7e72b0a 100644 > --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml > +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml > @@ -60,6 +60,11 @@ patternProperties: > - minimum: 1 > description: > Size in bit within the address range specified by reg. > + cell-type: > + $ref: /schemas/types.yaml#/definitions/uint32 > + maxItems: 1 > + description: > + Type of nvmem, Use defines in dt-bindings/nvmem/nvmem.h. > > required: > - reg > @@ -69,6 +74,7 @@ additionalProperties: true > examples: > - | > #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/nvmem/nvmem.h> > > qfprom: eeprom@700000 { > #address-cells = <1>; > @@ -98,6 +104,11 @@ examples: > reg = <0xc 0x1>; > bits = <2 3>; > }; > + > + mac_addr: mac-addr@90{ > + reg = <0x90 0x6>; > + cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>; > + }; > }; > > ... > diff --git a/include/dt-bindings/nvmem/nvmem.h b/include/dt-bindings/nvmem/nvmem.h > new file mode 100644 > index 000000000000..eed0478f6bfd > --- /dev/null > +++ b/include/dt-bindings/nvmem/nvmem.h > @@ -0,0 +1,8 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __DT_NVMMEM_H > +#define __DT_NVMMEM_H > + > +#define NVMEM_CELL_TYPE_UNKNOWN 0 > +#define NVMEM_CELL_TYPE_MAC_ADDRESS 1 > + > +#endif /* __DT_NVMMEM_H */ > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 22/09/2021 12:34, Ahmad Fatoum wrote: > Hi, > > On 08.09.21 12:02, Joakim Zhang wrote: >> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> >> 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. > That is okay. > 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>; > No, this is not okay, cell-type is just representing what is the cell type in a generic way, and its not intended to have any encoding information. Encoding information should be derived from the provider specific bindings. If we start adding this info in generic binding we will endup with a mess. And this has been discussed in various instances. > * and then the decoder is placed into some generic location, e.g. > drivers/nvmem/encodings.c for Linux This is fine, we could have a library to do these post-processing, but only if we have enough users. For now its better to keep it within provider drivers till we have more consumers of these functions. --srini > > 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? > >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> >> --- >> Documentation/devicetree/bindings/nvmem/nvmem.yaml | 11 +++++++++++ >> include/dt-bindings/nvmem/nvmem.h | 8 ++++++++ >> 2 files changed, 19 insertions(+) >> create mode 100644 include/dt-bindings/nvmem/nvmem.h >> >> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml >> index b8dc3d2b6e92..8cf6c7e72b0a 100644 >> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml >> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml >> @@ -60,6 +60,11 @@ patternProperties: >> - minimum: 1 >> description: >> Size in bit within the address range specified by reg. >> + cell-type: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + maxItems: 1 >> + description: >> + Type of nvmem, Use defines in dt-bindings/nvmem/nvmem.h. >> >> required: >> - reg >> @@ -69,6 +74,7 @@ additionalProperties: true >> examples: >> - | >> #include <dt-bindings/gpio/gpio.h> >> + #include <dt-bindings/nvmem/nvmem.h> >> >> qfprom: eeprom@700000 { >> #address-cells = <1>; >> @@ -98,6 +104,11 @@ examples: >> reg = <0xc 0x1>; >> bits = <2 3>; >> }; >> + >> + mac_addr: mac-addr@90{ >> + reg = <0x90 0x6>; >> + cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>; >> + }; >> }; >> >> ... >> diff --git a/include/dt-bindings/nvmem/nvmem.h b/include/dt-bindings/nvmem/nvmem.h >> new file mode 100644 >> index 000000000000..eed0478f6bfd >> --- /dev/null >> +++ b/include/dt-bindings/nvmem/nvmem.h >> @@ -0,0 +1,8 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef __DT_NVMMEM_H >> +#define __DT_NVMMEM_H >> + >> +#define NVMEM_CELL_TYPE_UNKNOWN 0 >> +#define NVMEM_CELL_TYPE_MAC_ADDRESS 1 >> + >> +#endif /* __DT_NVMMEM_H */ >> > >
Hello Srini, On 22.09.21 14:23, Srinivas Kandagatla wrote: > > > On 22/09/2021 12:34, Ahmad Fatoum wrote: >> Hi, >> >> On 08.09.21 12:02, Joakim Zhang wrote: >>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> >>> 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. >> > > That is okay. How would you go about using this same format on an EEPROM? >> 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>; >> > > No, this is not okay, cell-type is just representing what is the cell type in a generic way, and its not intended to have any encoding information. > > Encoding information should be derived from the provider specific bindings. If we start adding this info in generic binding we will endup with a mess. > And this has been discussed in various instances. A linux-nvmem list would be nice to collect such discussions. >> * and then the decoder is placed into some generic location, e.g. >> drivers/nvmem/encodings.c for Linux > > This is fine, we could have a library to do these post-processing, but only if we have enough users. For now its better to keep it within provider drivers till we have more consumers of these functions. > > > --srini >> >> 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? >> >>> >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> >>> --- >>> Documentation/devicetree/bindings/nvmem/nvmem.yaml | 11 +++++++++++ >>> include/dt-bindings/nvmem/nvmem.h | 8 ++++++++ >>> 2 files changed, 19 insertions(+) >>> create mode 100644 include/dt-bindings/nvmem/nvmem.h >>> >>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml >>> index b8dc3d2b6e92..8cf6c7e72b0a 100644 >>> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml >>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml >>> @@ -60,6 +60,11 @@ patternProperties: >>> - minimum: 1 >>> description: >>> Size in bit within the address range specified by reg. >>> + cell-type: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + maxItems: 1 >>> + description: >>> + Type of nvmem, Use defines in dt-bindings/nvmem/nvmem.h. >>> required: >>> - reg >>> @@ -69,6 +74,7 @@ additionalProperties: true >>> examples: >>> - | >>> #include <dt-bindings/gpio/gpio.h> >>> + #include <dt-bindings/nvmem/nvmem.h> >>> qfprom: eeprom@700000 { >>> #address-cells = <1>; >>> @@ -98,6 +104,11 @@ examples: >>> reg = <0xc 0x1>; >>> bits = <2 3>; >>> }; >>> + >>> + mac_addr: mac-addr@90{ >>> + reg = <0x90 0x6>; >>> + cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>; >>> + }; >>> }; >>> ... >>> diff --git a/include/dt-bindings/nvmem/nvmem.h b/include/dt-bindings/nvmem/nvmem.h >>> new file mode 100644 >>> index 000000000000..eed0478f6bfd >>> --- /dev/null >>> +++ b/include/dt-bindings/nvmem/nvmem.h >>> @@ -0,0 +1,8 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +#ifndef __DT_NVMMEM_H >>> +#define __DT_NVMMEM_H >>> + >>> +#define NVMEM_CELL_TYPE_UNKNOWN 0 >>> +#define NVMEM_CELL_TYPE_MAC_ADDRESS 1 >>> + >>> +#endif /* __DT_NVMMEM_H */ >>> >> >> > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 22/09/2021 13:31, Ahmad Fatoum wrote: >>> >>> On 08.09.21 12:02, Joakim Zhang wrote: >>>> From: Srinivas Kandagatla<srinivas.kandagatla@linaro.org> >>>> >>>> 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. >>> >> That is okay. > How would you go about using this same format on an EEPROM? Am guessing that by the time there are more users for such formats, those post-processing functions should be converted into some library functions. --srini > >>> I'd thus prefer to not make this specific to the OCOTP as all: >>> >>> * #define NVMEM_CELL_ENCODING_MAC_ADDRESS_IMX /* ... */
Hi Srini, On 22.09.21 14:49, Srinivas Kandagatla wrote: > > > On 22/09/2021 13:31, Ahmad Fatoum wrote: >>>> >>>> On 08.09.21 12:02, Joakim Zhang wrote: >>>>> From: Srinivas Kandagatla<srinivas.kandagatla@linaro.org> >>>>> >>>>> 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. >>>> >>> That is okay. >> How would you go about using this same format on an EEPROM? > > Am guessing that by the time there are more users for such formats, those post-processing functions should be converted into some library functions. User A wants to reverse bytes in MAC address. User B stores it in ASCII. Both use the exact same EEPROM. How could this ever work when the encoding decision is left to the EEPROM driver? > > --srini > >> >>>> I'd thus prefer to not make this specific to the OCOTP as all: >>>> >>>> * #define NVMEM_CELL_ENCODING_MAC_ADDRESS_IMX /* ... */ > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 22/09/2021 13:58, Ahmad Fatoum wrote: > Hi Srini, > > On 22.09.21 14:49, Srinivas Kandagatla wrote: >> >> >> On 22/09/2021 13:31, Ahmad Fatoum wrote: >>>>> >>>>> On 08.09.21 12:02, Joakim Zhang wrote: >>>>>> From: Srinivas Kandagatla<srinivas.kandagatla@linaro.org> >>>>>> >>>>>> 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. >>>>> >>>> That is okay. >>> How would you go about using this same format on an EEPROM? >> >> Am guessing that by the time there are more users for such formats, those post-processing functions should be converted into some library functions. > > User A wants to reverse bytes in MAC address. User B stores it in ASCII. > Both use the exact same EEPROM. How could this ever work when the > encoding decision is left to the EEPROM driver? User A and B should mention about this encoding information in there NVMEM provider bindings. Based on that specific post-processing should be selected. --srini > >> >> --srini >> >>> >>>>> I'd thus prefer to not make this specific to the OCOTP as all: >>>>> >>>>> * #define NVMEM_CELL_ENCODING_MAC_ADDRESS_IMX /* ... */ >> > >
Hi, On 22.09.21 15:03, Srinivas Kandagatla wrote: > > > On 22/09/2021 13:58, Ahmad Fatoum wrote: >> Hi Srini, >> >> On 22.09.21 14:49, Srinivas Kandagatla wrote: >>> >>> >>> On 22/09/2021 13:31, Ahmad Fatoum wrote: >>>>>> >>>>>> On 08.09.21 12:02, Joakim Zhang wrote: >>>>>>> From: Srinivas Kandagatla<srinivas.kandagatla@linaro.org> >>>>>>> >>>>>>> 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. >>>>>> >>>>> That is okay. >>>> How would you go about using this same format on an EEPROM? >>> >>> Am guessing that by the time there are more users for such formats, those post-processing functions should be converted into some library functions. >> >> User A wants to reverse bytes in MAC address. User B stores it in ASCII. >> Both use the exact same EEPROM. How could this ever work when the >> encoding decision is left to the EEPROM driver? > > User A and B should mention about this encoding information in there NVMEM provider bindings. > > Based on that specific post-processing should be selected. So instead of just compatible = "atmel,at24c16"; there will be compatible = "user-A,my-eeprom", "atmel,at24c16"; and compatible = "user-B,my-eeprom", "atmel,at24c16"; and they each need to patch the at24 driver to call one of the common library functions? > > --srini >> > >>> >>> --srini >>> >>>> >>>>>> I'd thus prefer to not make this specific to the OCOTP as all: >>>>>> >>>>>> * #define NVMEM_CELL_ENCODING_MAC_ADDRESS_IMX /* ... */ >>> >> >> > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 22/09/2021 14:08, Ahmad Fatoum wrote: > Hi, > > On 22.09.21 15:03, Srinivas Kandagatla wrote: >> >> >> On 22/09/2021 13:58, Ahmad Fatoum wrote: >>> Hi Srini, >>> >>> On 22.09.21 14:49, Srinivas Kandagatla wrote: >>>> >>>> >>>> On 22/09/2021 13:31, Ahmad Fatoum wrote: >>>>>>> >>>>>>> On 08.09.21 12:02, Joakim Zhang wrote: >>>>>>>> From: Srinivas Kandagatla<srinivas.kandagatla@linaro.org> >>>>>>>> >>>>>>>> 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. >>>>>>> >>>>>> That is okay. >>>>> How would you go about using this same format on an EEPROM? >>>> >>>> Am guessing that by the time there are more users for such formats, those post-processing functions should be converted into some library functions. >>> >>> User A wants to reverse bytes in MAC address. User B stores it in ASCII. >>> Both use the exact same EEPROM. How could this ever work when the >>> encoding decision is left to the EEPROM driver? >> >> User A and B should mention about this encoding information in there NVMEM provider bindings. >> >> Based on that specific post-processing should be selected. > > So instead of just compatible = "atmel,at24c16"; there will be > > compatible = "user-A,my-eeprom", "atmel,at24c16"; > > and > > compatible = "user-B,my-eeprom", "atmel,at24c16"; It will depend how you specify encoding information. The issue with making this encoding information generic is that the combinations are endless and nvmem core bindings is definitely not the right place for this. ex: If I remember correctly we have mac-address stored in various formats: from old thread I can see Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped) Type 2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so on) (Swapped/non-Swapped) Type 3: Is the one which stores mac address in Type1/2 but this has to be incremented to be used on other instances of eth. Type 4: Octets as bytes/u8, swapped/non-swapped This list can be endless and its not just the cell-type property you have to deal with, new properties keep popping up. --srini > > and they each need to patch the at24 driver to call one of the > common library functions? > >> >> --srini >>> >> >>>> >>>> --srini >>>> >>>>> >>>>>>> I'd thus prefer to not make this specific to the OCOTP as all: >>>>>>> >>>>>>> * #define NVMEM_CELL_ENCODING_MAC_ADDRESS_IMX /* ... */ >>>> >>> >>> >> > >
Hi Ahmad, > -----Original Message----- > From: Ahmad Fatoum <a.fatoum@pengutronix.de> > Sent: 2021年9月22日 19:34 > To: Joakim Zhang <qiangqing.zhang@nxp.com>; > srinivas.kandagatla@linaro.org; robh+dt@kernel.org; shawnguo@kernel.org; > Jan Lübbe <jlu@pengutronix.de> > Cc: devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > kernel@pengutronix.de; linux-kernel@vger.kernel.org > 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@linaro.org> > > > > 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
Hello Srini, On 22.09.21 15:23, Srinivas Kandagatla wrote: > On 22/09/2021 14:08, Ahmad Fatoum wrote: >> On 22.09.21 15:03, Srinivas Kandagatla wrote: >>> User A and B should mention about this encoding information in there NVMEM provider bindings. >>> >>> Based on that specific post-processing should be selected. >> >> So instead of just compatible = "atmel,at24c16"; there will be >> >> compatible = "user-A,my-eeprom", "atmel,at24c16"; >> >> and >> >> compatible = "user-B,my-eeprom", "atmel,at24c16"; > > It will depend how you specify encoding information. I would specify them in cell-type, which you disagree with, thus I am asking: Is using a stock EEPROM with a non-canonical format for e.g. a MAC address something that you think should be supported with NVMEM? > The issue with making this encoding information generic is that the combinations are endless and nvmem core bindings is definitely not the right place for this. Add a separate file and include it from the core file? > ex: > If I remember correctly we have mac-address stored in various formats: > from old thread I can see > > Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped) > > Type 2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so on) (Swapped/non-Swapped) > > Type 3: Is the one which stores mac address in Type1/2 but this has to > be incremented to be used on other instances of eth. > > Type 4: Octets as bytes/u8, swapped/non-swapped > > This list can be endless and its not just the cell-type property you have to deal with, new properties keep popping up. Yes, an extendible interface will likely encourage people to extend it. Cheers, Ahmad > --srini > > > >> >> and they each need to patch the at24 driver to call one of the >> common library functions? >> >>> >>> --srini >>>> >>> >>>>> >>>>> --srini >>>>> >>>>>> >>>>>>>> I'd thus prefer to not make this specific to the OCOTP as all: >>>>>>>> >>>>>>>> * #define NVMEM_CELL_ENCODING_MAC_ADDRESS_IMX /* ... */ >>>>> >>>> >>>> >>> >> >> > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml index b8dc3d2b6e92..8cf6c7e72b0a 100644 --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml @@ -60,6 +60,11 @@ patternProperties: - minimum: 1 description: Size in bit within the address range specified by reg. + cell-type: + $ref: /schemas/types.yaml#/definitions/uint32 + maxItems: 1 + description: + Type of nvmem, Use defines in dt-bindings/nvmem/nvmem.h. required: - reg @@ -69,6 +74,7 @@ additionalProperties: true examples: - | #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/nvmem/nvmem.h> qfprom: eeprom@700000 { #address-cells = <1>; @@ -98,6 +104,11 @@ examples: reg = <0xc 0x1>; bits = <2 3>; }; + + mac_addr: mac-addr@90{ + reg = <0x90 0x6>; + cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>; + }; }; ... diff --git a/include/dt-bindings/nvmem/nvmem.h b/include/dt-bindings/nvmem/nvmem.h new file mode 100644 index 000000000000..eed0478f6bfd --- /dev/null +++ b/include/dt-bindings/nvmem/nvmem.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __DT_NVMMEM_H +#define __DT_NVMMEM_H + +#define NVMEM_CELL_TYPE_UNKNOWN 0 +#define NVMEM_CELL_TYPE_MAC_ADDRESS 1 + +#endif /* __DT_NVMMEM_H */