| Message ID | 20241017184152.128395-1-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: at24: add ST M24256E Additional Write lockable page support | expand |
On Thu, Oct 17, 2024 at 08:41:25PM +0200, Marek Vasut wrote: > The ST M24256E behaves as a regular M24C256, except for the E variant > which uses up another I2C address for Additional Write lockable page. > This page is 64 Bytes long and can contain additional data. Add entry > for it, so users can describe that page in DT. Note that users still > have to describe the main M24C256 area separately as that is on separate > I2C address from this page. I think this should be modelled as 1 node having 2 addresses, not 2 nodes. > > Unlike M24C32-D and M24C64-D, this part is specifically ST and does not > have any comparable M24* counterparts from other vendors, hence the st, > vendor prefix. Furthermore, the part name is M24256E without C between > the 24 and 256, this is not a typo. Finally, there is M24C256-D part, > which does contain 32 Bytes long Additional Write lockable page, which > is a different part and not supported by this patch. > > Datasheet: https://www.st.com/resource/en/datasheet/m24256e-f.pdf > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Alexander Stein <alexander.stein@ew.tq-group.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Cc: Bartosz Golaszewski <brgl@bgdev.pl> > Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com> > Cc: Conor Dooley <conor+dt@kernel.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org> > Cc: Rob Herring <robh@kernel.org> > Cc: devicetree@vger.kernel.org > Cc: kernel@dh-electronics.com > Cc: linux-i2c@vger.kernel.org > --- > Documentation/devicetree/bindings/eeprom/at24.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml > index b6239ec3512b3..590ba0ef5fa26 100644 > --- a/Documentation/devicetree/bindings/eeprom/at24.yaml > +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml > @@ -141,6 +141,8 @@ properties: > - const: microchip,24aa025e48 > - items: > - const: microchip,24aa025e64 > + - items: > + - const: st,24256e-wl > - pattern: '^atmel,24c(32|64)d-wl$' # Actual vendor is st > > label: > -- > 2.45.2 >
On 10/18/24 3:27 PM, Rob Herring wrote: > On Thu, Oct 17, 2024 at 08:41:25PM +0200, Marek Vasut wrote: >> The ST M24256E behaves as a regular M24C256, except for the E variant >> which uses up another I2C address for Additional Write lockable page. >> This page is 64 Bytes long and can contain additional data. Add entry >> for it, so users can describe that page in DT. Note that users still >> have to describe the main M24C256 area separately as that is on separate >> I2C address from this page. > > I think this should be modelled as 1 node having 2 addresses, not 2 > nodes. We had the exact same discussion regarding M24C32D, see: https://lore.kernel.org/all/CAMRc=MdTu1gagX-L4_cHmN9aUCoKhN-b5i7yEeszKSdr+BuROg@mail.gmail.com/
On Sun, Oct 20, 2024 at 06:29:13AM +0200, Marek Vasut wrote: > On 10/18/24 3:27 PM, Rob Herring wrote: > > On Thu, Oct 17, 2024 at 08:41:25PM +0200, Marek Vasut wrote: > > > The ST M24256E behaves as a regular M24C256, except for the E variant > > > which uses up another I2C address for Additional Write lockable page. > > > This page is 64 Bytes long and can contain additional data. Add entry > > > for it, so users can describe that page in DT. Note that users still > > > have to describe the main M24C256 area separately as that is on separate > > > I2C address from this page. > > > > I think this should be modelled as 1 node having 2 addresses, not 2 > > nodes. > We had the exact same discussion regarding M24C32D, see: > > https://lore.kernel.org/all/CAMRc=MdTu1gagX-L4_cHmN9aUCoKhN-b5i7yEeszKSdr+BuROg@mail.gmail.com/ Seems like kernel implementation details dictating the binding to me. Won't be a problem until there are shared resources on "both" devices. I guess since it is inline with what we already have: Reviewed-by: Rob Herring <robh@kernel.org>
On Mon, Oct 21, 2024 at 8:14 PM Rob Herring <robh@kernel.org> wrote: > > On Sun, Oct 20, 2024 at 06:29:13AM +0200, Marek Vasut wrote: > > On 10/18/24 3:27 PM, Rob Herring wrote: > > > On Thu, Oct 17, 2024 at 08:41:25PM +0200, Marek Vasut wrote: > > > > The ST M24256E behaves as a regular M24C256, except for the E variant > > > > which uses up another I2C address for Additional Write lockable page. > > > > This page is 64 Bytes long and can contain additional data. Add entry > > > > for it, so users can describe that page in DT. Note that users still > > > > have to describe the main M24C256 area separately as that is on separate > > > > I2C address from this page. > > > > > > I think this should be modelled as 1 node having 2 addresses, not 2 > > > nodes. > > We had the exact same discussion regarding M24C32D, see: > > > > https://lore.kernel.org/all/CAMRc=MdTu1gagX-L4_cHmN9aUCoKhN-b5i7yEeszKSdr+BuROg@mail.gmail.com/ > > Seems like kernel implementation details dictating the binding to me. Yeah, that's on me. I would have known better today but 8 years ago the DT situation was much more volatile. > Won't be a problem until there are shared resources on "both" devices. > Fortunately, that could be addressed with the power sequencing subsystem. :) Bart
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> On Thu, 17 Oct 2024 20:41:25 +0200, Marek Vasut wrote: > The ST M24256E behaves as a regular M24C256, except for the E variant > which uses up another I2C address for Additional Write lockable page. > This page is 64 Bytes long and can contain additional data. Add entry > for it, so users can describe that page in DT. Note that users still > have to describe the main M24C256 area separately as that is on separate > I2C address from this page. > > [...] Applied, thanks! [1/2] dt-bindings: at24: add ST M24256E Additional Write lockable page support (no commit info) [2/2] eeprom: at24: add ST M24256E Additional Write lockable page support (no commit info) Best regards,
On Tue, Oct 22, 2024 at 9:13 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > On Thu, 17 Oct 2024 20:41:25 +0200, Marek Vasut wrote: > > The ST M24256E behaves as a regular M24C256, except for the E variant > > which uses up another I2C address for Additional Write lockable page. > > This page is 64 Bytes long and can contain additional data. Add entry > > for it, so users can describe that page in DT. Note that users still > > have to describe the main M24C256 area separately as that is on separate > > I2C address from this page. > > > > [...] > > Applied, thanks! > > [1/2] dt-bindings: at24: add ST M24256E Additional Write lockable page support > (no commit info) > [2/2] eeprom: at24: add ST M24256E Additional Write lockable page support > (no commit info) > No worries about the "no commit info" I was on a different branch when generating thankyou emails. Sorry. Bart > Best regards, > -- > Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
On Tue, 19 Nov 2024 12:01:54 +0100, Geert Uytterhoeven <geert@linux-m68k.org> said: > Hi Bartosz, > > On Mon, Oct 21, 2024 at 8:36 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >> On Mon, Oct 21, 2024 at 8:14 PM Rob Herring <robh@kernel.org> wrote: >> > On Sun, Oct 20, 2024 at 06:29:13AM +0200, Marek Vasut wrote: >> > > On 10/18/24 3:27 PM, Rob Herring wrote: >> > > > On Thu, Oct 17, 2024 at 08:41:25PM +0200, Marek Vasut wrote: >> > > > > The ST M24256E behaves as a regular M24C256, except for the E variant >> > > > > which uses up another I2C address for Additional Write lockable page. >> > > > > This page is 64 Bytes long and can contain additional data. Add entry >> > > > > for it, so users can describe that page in DT. Note that users still >> > > > > have to describe the main M24C256 area separately as that is on separate >> > > > > I2C address from this page. >> > > > >> > > > I think this should be modelled as 1 node having 2 addresses, not 2 >> > > > nodes. >> > > We had the exact same discussion regarding M24C32D, see: >> > > >> > > https://lore.kernel.org/all/CAMRc=MdTu1gagX-L4_cHmN9aUCoKhN-b5i7yEeszKSdr+BuROg@mail.gmail.com/ >> > >> > Seems like kernel implementation details dictating the binding to me. >> >> Yeah, that's on me. I would have known better today but 8 years ago >> the DT situation was much more volatile. > > And there's no way we can fix that for new devices? Perhaps even > for old devices, by counting the number of entries in the "reg" > compatible value? > For sure. We can always deprecate old bindings after upstreaming new ones but we'd need to still carry the model ID in the compatible string and remain backward compatible with existing DTs. So an existing example of: at24c01@57 { compatible = "atmel,24c01"; reg = <0x57>; }; at24cs01@5f { compatible = "atmel,24cs01"; reg = <0x5f>; }; Would become: at24cs01@57 { compatible = "atmel,24cs01"; reg = <0x57>, <0x5f>; }; Now the driver takes the "atmel,24cs01" compatible and needs to count the 'reg' items to determine whether to create one or two nvmem sub-devices. It's nothing impossible but would be quite tedious and make the driver so much more complex. I, for sure, don't have the bandwidth right now to tackle it but I'd happily help with testing and reviewing. Bart
diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml index b6239ec3512b3..590ba0ef5fa26 100644 --- a/Documentation/devicetree/bindings/eeprom/at24.yaml +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml @@ -141,6 +141,8 @@ properties: - const: microchip,24aa025e48 - items: - const: microchip,24aa025e64 + - items: + - const: st,24256e-wl - pattern: '^atmel,24c(32|64)d-wl$' # Actual vendor is st label:
The ST M24256E behaves as a regular M24C256, except for the E variant which uses up another I2C address for Additional Write lockable page. This page is 64 Bytes long and can contain additional data. Add entry for it, so users can describe that page in DT. Note that users still have to describe the main M24C256 area separately as that is on separate I2C address from this page. Unlike M24C32-D and M24C64-D, this part is specifically ST and does not have any comparable M24* counterparts from other vendors, hence the st, vendor prefix. Furthermore, the part name is M24256E without C between the 24 and 256, this is not a typo. Finally, there is M24C256-D part, which does contain 32 Bytes long Additional Write lockable page, which is a different part and not supported by this patch. Datasheet: https://www.st.com/resource/en/datasheet/m24256e-f.pdf Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Alexander Stein <alexander.stein@ew.tq-group.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Cc: Bartosz Golaszewski <brgl@bgdev.pl> Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com> Cc: Conor Dooley <conor+dt@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org> Cc: Rob Herring <robh@kernel.org> Cc: devicetree@vger.kernel.org Cc: kernel@dh-electronics.com Cc: linux-i2c@vger.kernel.org --- Documentation/devicetree/bindings/eeprom/at24.yaml | 2 ++ 1 file changed, 2 insertions(+)