| Message ID | 1488875164-30440-2-git-send-email-albeu@free.fr |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
On 03/07/2017 09:26 AM, Alban wrote: > Config data for drivers, like MAC addresses, is often stored in MTD. > Add a binding that define how such data storage can be represented in > device tree. > > Signed-off-by: Alban <albeu@free.fr> > --- > Changelog: > v2: * Added a "Required properties" section with the nvmem-provider > property > --- > .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > > diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > new file mode 100644 > index 0000000..8ed25e6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > @@ -0,0 +1,33 @@ > += NVMEM in MTD = > + > +Config data for drivers, like MAC addresses, is often stored in MTD. > +This binding define how such data storage can be represented in device tree. > + > +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` > +property to their node. Data cells can then be defined as child nodes > +of the partition as defined in nvmem.txt. Why don't we just read the data from MTD and be done with it ? What's the benefit of complicating things by using nvmem ? > +Required properties: > +nvmem-provider: Indicate that the device should be registered as > + NVMEM provider > + > +Example: > + > + flash@0 { > + ... > + > + partition@2 { > + label = "art"; > + reg = <0x7F0000 0x010000>; > + read-only; > + > + nvmem-provider; > + #address-cells = <1>; > + #size-cells = <1>; > + > + eeprom@1000 { > + label = "wmac-eeprom"; > + reg = <0x1000 0x1000>; > + }; > + }; > + }; >
On Thu, Mar 9, 2017 at 7:17 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > On 03/07/2017 09:26 AM, Alban wrote: >> Config data for drivers, like MAC addresses, is often stored in MTD. >> Add a binding that define how such data storage can be represented in >> device tree. >> >> Signed-off-by: Alban <albeu@free.fr> >> --- >> Changelog: >> v2: * Added a "Required properties" section with the nvmem-provider >> property >> --- >> .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >> >> diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >> new file mode 100644 >> index 0000000..8ed25e6 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >> @@ -0,0 +1,33 @@ >> += NVMEM in MTD = >> + >> +Config data for drivers, like MAC addresses, is often stored in MTD. >> +This binding define how such data storage can be represented in device tree. >> + >> +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` >> +property to their node. Data cells can then be defined as child nodes >> +of the partition as defined in nvmem.txt. > > Why don't we just read the data from MTD and be done with it ? What's > the benefit of complicating things by using nvmem ? Well because usually stuff like MAC addresses etc are stored in eeproms. This gives a nice abstraction with making them both look like nvmem (that was my reasoning back then when I submitted a patch to support the OTP part in a SPI NOR part. >> +Required properties: >> +nvmem-provider: Indicate that the device should be registered as >> + NVMEM provider >> + >> +Example: >> + >> + flash@0 { >> + ... >> + >> + partition@2 { >> + label = "art"; >> + reg = <0x7F0000 0x010000>; >> + read-only; >> + >> + nvmem-provider; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + >> + eeprom@1000 { >> + label = "wmac-eeprom"; >> + reg = <0x1000 0x1000>; >> + }; >> + }; >> + }; >> > > > -- > Best regards, > Marek Vasut Cheers, Moritz
On 03/10/2017 05:06 AM, Moritz Fischer wrote: > On Thu, Mar 9, 2017 at 7:17 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >> On 03/07/2017 09:26 AM, Alban wrote: >>> Config data for drivers, like MAC addresses, is often stored in MTD. >>> Add a binding that define how such data storage can be represented in >>> device tree. >>> >>> Signed-off-by: Alban <albeu@free.fr> >>> --- >>> Changelog: >>> v2: * Added a "Required properties" section with the nvmem-provider >>> property >>> --- >>> .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ >>> 1 file changed, 33 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >>> >>> diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >>> new file mode 100644 >>> index 0000000..8ed25e6 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >>> @@ -0,0 +1,33 @@ >>> += NVMEM in MTD = >>> + >>> +Config data for drivers, like MAC addresses, is often stored in MTD. >>> +This binding define how such data storage can be represented in device tree. >>> + >>> +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` >>> +property to their node. Data cells can then be defined as child nodes >>> +of the partition as defined in nvmem.txt. >> >> Why don't we just read the data from MTD and be done with it ? What's >> the benefit of complicating things by using nvmem ? > > Well because usually stuff like MAC addresses etc are stored in eeproms. But eeproms are already supported, see drivers/misc/ . > This gives a nice abstraction with making them both look like nvmem (that was my > reasoning back then when I submitted a patch to support the OTP part in a > SPI NOR part. Hm, I am confused here, we're mixing SPI NOR, EEPROMs and OTP devices here.
Hi Marek, On Fri, Mar 10, 2017 at 05:52:36AM +0100, Marek Vasut wrote: > On 03/10/2017 05:06 AM, Moritz Fischer wrote: > > On Thu, Mar 9, 2017 at 7:17 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > >> On 03/07/2017 09:26 AM, Alban wrote: > >>> Config data for drivers, like MAC addresses, is often stored in MTD. > >>> Add a binding that define how such data storage can be represented in > >>> device tree. > >>> > >>> Signed-off-by: Alban <albeu@free.fr> > >>> --- > >>> Changelog: > >>> v2: * Added a "Required properties" section with the nvmem-provider > >>> property > >>> --- > >>> .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ > >>> 1 file changed, 33 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > >>> new file mode 100644 > >>> index 0000000..8ed25e6 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > >>> @@ -0,0 +1,33 @@ > >>> += NVMEM in MTD = > >>> + > >>> +Config data for drivers, like MAC addresses, is often stored in MTD. > >>> +This binding define how such data storage can be represented in device tree. > >>> + > >>> +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` > >>> +property to their node. Data cells can then be defined as child nodes > >>> +of the partition as defined in nvmem.txt. > >> > >> Why don't we just read the data from MTD and be done with it ? What's > >> the benefit of complicating things by using nvmem ? > > > > Well because usually stuff like MAC addresses etc are stored in eeproms. > > But eeproms are already supported, see drivers/misc/ . This the old, free for all, way to support eeproms. We have a proper framework for them now, and it's called nvmem. Maxime
On 03/10/2017 07:38 AM, Maxime Ripard wrote: > Hi Marek, > > On Fri, Mar 10, 2017 at 05:52:36AM +0100, Marek Vasut wrote: >> On 03/10/2017 05:06 AM, Moritz Fischer wrote: >>> On Thu, Mar 9, 2017 at 7:17 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >>>> On 03/07/2017 09:26 AM, Alban wrote: >>>>> Config data for drivers, like MAC addresses, is often stored in MTD. >>>>> Add a binding that define how such data storage can be represented in >>>>> device tree. >>>>> >>>>> Signed-off-by: Alban <albeu@free.fr> >>>>> --- >>>>> Changelog: >>>>> v2: * Added a "Required properties" section with the nvmem-provider >>>>> property >>>>> --- >>>>> .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ >>>>> 1 file changed, 33 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >>>>> new file mode 100644 >>>>> index 0000000..8ed25e6 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >>>>> @@ -0,0 +1,33 @@ >>>>> += NVMEM in MTD = >>>>> + >>>>> +Config data for drivers, like MAC addresses, is often stored in MTD. >>>>> +This binding define how such data storage can be represented in device tree. >>>>> + >>>>> +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` >>>>> +property to their node. Data cells can then be defined as child nodes >>>>> +of the partition as defined in nvmem.txt. >>>> >>>> Why don't we just read the data from MTD and be done with it ? What's >>>> the benefit of complicating things by using nvmem ? >>> >>> Well because usually stuff like MAC addresses etc are stored in eeproms. >> >> But eeproms are already supported, see drivers/misc/ . > > This the old, free for all, way to support eeproms. We have a proper > framework for them now, and it's called nvmem. Ha, so that's why this patchset, I see. Thanks for clarifying.
On Tue, Mar 07, 2017 at 09:26:03AM +0100, Alban wrote: > Config data for drivers, like MAC addresses, is often stored in MTD. > Add a binding that define how such data storage can be represented in > device tree. > > Signed-off-by: Alban <albeu@free.fr> > --- > Changelog: > v2: * Added a "Required properties" section with the nvmem-provider > property > --- > .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > > diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > new file mode 100644 > index 0000000..8ed25e6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > @@ -0,0 +1,33 @@ > += NVMEM in MTD = > + > +Config data for drivers, like MAC addresses, is often stored in MTD. > +This binding define how such data storage can be represented in device tree. > + > +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` > +property to their node. Data cells can then be defined as child nodes > +of the partition as defined in nvmem.txt. > + > +Required properties: > +nvmem-provider: Indicate that the device should be registered as > + NVMEM provider I think we should use a compatible string here (perhaps with a generic fallback), and that can imply it is an nvmem provider. The reason is then the compatible can also imply other information that isn't defined in DT. Rob
On Wed, 15 Mar 2017 12:24:01 -0500 Rob Herring <robh@kernel.org> wrote: > On Tue, Mar 07, 2017 at 09:26:03AM +0100, Alban wrote: > > Config data for drivers, like MAC addresses, is often stored in MTD. > > Add a binding that define how such data storage can be represented in > > device tree. > > > > Signed-off-by: Alban <albeu@free.fr> > > --- > > Changelog: > > v2: * Added a "Required properties" section with the nvmem-provider > > property > > --- > > .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > > > > diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > > new file mode 100644 > > index 0000000..8ed25e6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > > @@ -0,0 +1,33 @@ > > += NVMEM in MTD = > > + > > +Config data for drivers, like MAC addresses, is often stored in MTD. > > +This binding define how such data storage can be represented in device tree. > > + > > +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` > > +property to their node. Data cells can then be defined as child nodes > > +of the partition as defined in nvmem.txt. > > + > > +Required properties: > > +nvmem-provider: Indicate that the device should be registered as > > + NVMEM provider > > I think we should use a compatible string here (perhaps with a > generic fallback), and that can imply it is an nvmem provider. The > reason is then the compatible can also imply other information that > isn't defined in DT. That would work for partitions but not for unpartitioned MTD as these will already have a compatible string for the MTD hardware. I was also under the impression that capabilities/services provided by devices were represented with such properties, like interrupt-controller or gpio-controller, and not with compatible strings. There is also another problem with unpartitioned MTD, earlier MTD partitions binding allowed to have partitions as direct child nodes without any compatible strings. The current nvmem binding do the same for the nvmem cells, so it wouldn't be clear if a child node of the MTD is a partition using the old binding or an nvmem cell. As I think this problem could happen with some other device types I suggested to re-work the nvmem binding to be more like the current MTD partitions. See these threads[1][2], but a short example would look like this: flash { compatible = "vendor,flash-device-model"; ... nvmem-provider; nvmem-cells { compatible = "nvmem-cells"; #address-cells = <1>; #size-cells = <1>; cell@100 { label = "mac-address"; reg = <0x100 0x6>; }; }; }; or like this if the device is partitioned: flash { compatible = "vendor,flash-device-model"; ... partitions { compatible = "fixed-partitions" ... partition@1000 { ... nvmem-provider; nvmem-cells { compatible = "nvmem-cells"; #address-cells = <1>; #size-cells = <1>; cell@100 { label = "mac-address"; reg = <0x100 0x6>; }; }; }; }; }; Alban [1]:https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1344899.html [2]:https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1348295.html
On Wed, Mar 15, 2017 at 2:41 PM, Alban <albeu@free.fr> wrote: > On Wed, 15 Mar 2017 12:24:01 -0500 > Rob Herring <robh@kernel.org> wrote: > >> On Tue, Mar 07, 2017 at 09:26:03AM +0100, Alban wrote: >> > Config data for drivers, like MAC addresses, is often stored in MTD. >> > Add a binding that define how such data storage can be represented in >> > device tree. >> > >> > Signed-off-by: Alban <albeu@free.fr> >> > --- >> > Changelog: >> > v2: * Added a "Required properties" section with the nvmem-provider >> > property >> > --- >> > .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ >> > 1 file changed, 33 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >> > >> > diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >> > new file mode 100644 >> > index 0000000..8ed25e6 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt >> > @@ -0,0 +1,33 @@ >> > += NVMEM in MTD = >> > + >> > +Config data for drivers, like MAC addresses, is often stored in MTD. >> > +This binding define how such data storage can be represented in device tree. >> > + >> > +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` >> > +property to their node. Data cells can then be defined as child nodes >> > +of the partition as defined in nvmem.txt. >> > + >> > +Required properties: >> > +nvmem-provider: Indicate that the device should be registered as >> > + NVMEM provider >> >> I think we should use a compatible string here (perhaps with a >> generic fallback), and that can imply it is an nvmem provider. The >> reason is then the compatible can also imply other information that >> isn't defined in DT. > > That would work for partitions but not for unpartitioned MTD as these > will already have a compatible string for the MTD hardware. I was also > under the impression that capabilities/services provided by devices > were represented with such properties, like interrupt-controller or > gpio-controller, and not with compatible strings. > > There is also another problem with unpartitioned MTD, earlier MTD > partitions binding allowed to have partitions as direct child nodes > without any compatible strings. The current nvmem binding do the same > for the nvmem cells, so it wouldn't be clear if a child node of the MTD > is a partition using the old binding or an nvmem cell. Perhaps a sign we should not repeat that. > As I think this problem could happen with some other device types I > suggested to re-work the nvmem binding to be more like the current MTD > partitions. See these threads[1][2], but a short example would look like > this: > > flash { > compatible = "vendor,flash-device-model"; > ... > nvmem-provider; > nvmem-cells { > compatible = "nvmem-cells"; Isn't the node name or compatible here enough to imply this is a nvmem provider? > #address-cells = <1>; > #size-cells = <1>; > > cell@100 { > label = "mac-address"; > reg = <0x100 0x6>; > }; > }; > };
On Sat, 18 Mar 2017 15:58:11 -0500 Rob Herring <robh@kernel.org> wrote: > On Wed, Mar 15, 2017 at 2:41 PM, Alban <albeu@free.fr> wrote: > > On Wed, 15 Mar 2017 12:24:01 -0500 > > Rob Herring <robh@kernel.org> wrote: > > > >> On Tue, Mar 07, 2017 at 09:26:03AM +0100, Alban wrote: > >> > Config data for drivers, like MAC addresses, is often stored in MTD. > >> > Add a binding that define how such data storage can be represented in > >> > device tree. > >> > > >> > Signed-off-by: Alban <albeu@free.fr> > >> > --- > >> > Changelog: > >> > v2: * Added a "Required properties" section with the nvmem-provider > >> > property > >> > --- > >> > .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ > >> > 1 file changed, 33 insertions(+) > >> > create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > >> > > >> > diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > >> > new file mode 100644 > >> > index 0000000..8ed25e6 > >> > --- /dev/null > >> > +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > >> > @@ -0,0 +1,33 @@ > >> > += NVMEM in MTD = > >> > + > >> > +Config data for drivers, like MAC addresses, is often stored in MTD. > >> > +This binding define how such data storage can be represented in device tree. > >> > + > >> > +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` > >> > +property to their node. Data cells can then be defined as child nodes > >> > +of the partition as defined in nvmem.txt. > >> > + > >> > +Required properties: > >> > +nvmem-provider: Indicate that the device should be registered as > >> > + NVMEM provider > >> > >> I think we should use a compatible string here (perhaps with a > >> generic fallback), and that can imply it is an nvmem provider. The > >> reason is then the compatible can also imply other information that > >> isn't defined in DT. > > > > That would work for partitions but not for unpartitioned MTD as these > > will already have a compatible string for the MTD hardware. I was also > > under the impression that capabilities/services provided by devices > > were represented with such properties, like interrupt-controller or > > gpio-controller, and not with compatible strings. > > > > There is also another problem with unpartitioned MTD, earlier MTD > > partitions binding allowed to have partitions as direct child nodes > > without any compatible strings. The current nvmem binding do the same > > for the nvmem cells, so it wouldn't be clear if a child node of the MTD > > is a partition using the old binding or an nvmem cell. > > Perhaps a sign we should not repeat that. Yes, that's why I think we should "upgrade" the nvmem binding as a whole and not just limit this to the MTD case. > > As I think this problem could happen with some other device types I > > suggested to re-work the nvmem binding to be more like the current MTD > > partitions. See these threads[1][2], but a short example would look like > > this: > > > > flash { > > compatible = "vendor,flash-device-model"; > > ... > > nvmem-provider; > > nvmem-cells { > > compatible = "nvmem-cells"; > > Isn't the node name or compatible here enough to imply this is a nvmem provider? If there are cells defined yes, but nvmem also allow referencing the provider as a whole, so cells are optional. But yes it could be removed, in the case of MTD it is only used to filter the relevant devices to avoid having too many "useless" nvmem device registered. > > #address-cells = <1>; > > #size-cells = <1>; > > > > cell@100 { > > label = "mac-address"; > > reg = <0x100 0x6>; > > }; > > }; > > };
diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt new file mode 100644 index 0000000..8ed25e6 --- /dev/null +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt @@ -0,0 +1,33 @@ += NVMEM in MTD = + +Config data for drivers, like MAC addresses, is often stored in MTD. +This binding define how such data storage can be represented in device tree. + +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` +property to their node. Data cells can then be defined as child nodes +of the partition as defined in nvmem.txt. + +Required properties: +nvmem-provider: Indicate that the device should be registered as + NVMEM provider + +Example: + + flash@0 { + ... + + partition@2 { + label = "art"; + reg = <0x7F0000 0x010000>; + read-only; + + nvmem-provider; + #address-cells = <1>; + #size-cells = <1>; + + eeprom@1000 { + label = "wmac-eeprom"; + reg = <0x1000 0x1000>; + }; + }; + };
Config data for drivers, like MAC addresses, is often stored in MTD. Add a binding that define how such data storage can be represented in device tree. Signed-off-by: Alban <albeu@free.fr> --- Changelog: v2: * Added a "Required properties" section with the nvmem-provider property --- .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt