Fwd: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

Fwd: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Maxim Kiselev]

Hi, friends.

After applying this commit 'mtd: call of_platform_populate() for MTD
partitions' (bcdf0315),
I faced with a problem that my ethernet device can't be probed because it
wait when 'nvmem-cells' device will be probed first.

But there is no such driver which is compatible with 'nvmem-cells' because
'nvmem-cells' is just a mark used by the 'mtd_nvmem_add' function.

So this leads to appeating of unresolved dependency for the ethernet device.
And that's why the ethernet device can't be added and probed.

Here is a part of kernel log when spi flash probe start:

> device: 'spi0': device_add
> device: 'spi0.0': device_add
> spi-nor spi0.0: mx66l51235f (65536 Kbytes)
> 7 fixed-partitions partitions found on MTD device spi0.0

After 'm25p80' probe 'f1070000.ethernet' linked to 'partition@1' :

> device: 'f1010600.spi:m25p80@0:
partitions:partition@1': device_add
> device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet': device_add
> devices_kset: Moving f1070000.ethernet to end of list
> platform f1070000.ethernet: Linked as a consumer to f1010600.spi:m25p80@0:partitions:partition@1
> ethernet@70000 Dropping the fwnode link to partition@1

And as a result I got `-EPROBE_DEFER` for `f1070000.ethernet`

> platform f1070000.ethernet: error -EPROBE_DEFER: supplier f1010600.spi:m25p80@0:partitions:partition@1 not ready

Here is a part of my device tree:

    enet1: ethernet@70000 {
        status = "okay";
        nvmem-cells = <&macaddr>;
        nvmem-cell-names = "mac-address";
        phy-mode = "rgmii";
        phy = <&phy0>;
    };

    spi@10600 {
        status = "okay";

        m25p80@0 {
            compatible = "mx66l51235l";
            reg = <0>;
            #address-cells = <1>;
            #size-cells = <1>;

            partitions {
                compatible = "fixed-partitions";
                #address-cells = <1>;
                #size-cells = <1>;

                partition@0 {
                    reg = <0x00000000 0x000080000>;
                    label = "SPI.U_BOOT";
                };

                partition@1 {
                    compatible = "nvmem-cells";
                    reg = <0x000A0000 0x00020000>;
                    label = "SPI.INV_INFO";
                    #address-cells = <1>;
                    #size-cells = <1>;
                    ranges = <0 0x000A0000 0x00020000>;

                    macaddr: mac@6 {
                        reg = <0x6 0x6>;
                    };
                };

            };
        };
    };

In the example above 'ethernet@70000' requires 'macaddr: mac@6' which is
located inside mtd 'partition@1' of 'm25p80@0' spi flash.

Re: Fwd: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Thorsten Leemhuis]

[CCing the regression mailing list, as it should be in the loop for all
regressions, as explained in
https://docs.kernel.org/admin-guide/reporting-regressions.html ]

Hi, this is your Linux kernel regression tracker. Thx for the report.

On 10.12.22 10:52, Maxim Kiselev wrote:
> 
> After applying

This makes me wonder: "applying" as in "applying it to some version that
doesn't contain this change normally" or as it "after it was applied to
mainline I have the following problem with vanilla kernel version <???>"?

> this commit 'mtd: call of_platform_populate() for MTD
> partitions' (bcdf0315),

CCing Rafał, who authored bcdf0315.

> I faced with a problem that my ethernet device can't be probed because it
> wait when 'nvmem-cells' device will be probed first.

FWIW, there is a discussion about a problems that at least to my
untrained eyes looks similar:
https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/

Rafał, has some progress been made to resolve this?

To me it sounds like this might warrant a "revert, and reapply later
when the cause for the regression was addressed". Rafał, it seems you
suggested something like that, but it doesn't look like that happened
for one reason or another. Or am I missing something?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

> But there is no such driver which is compatible with 'nvmem-cells' because
> 'nvmem-cells' is just a mark used by the 'mtd_nvmem_add' function.
> 
> So this leads to appeating of unresolved dependency for the ethernet device.
> And that's why the ethernet device can't be added and probed.
> 
> Here is a part of kernel log when spi flash probe start:
> 
>> device: 'spi0': device_add
>> device: 'spi0.0': device_add
>> spi-nor spi0.0: mx66l51235f (65536 Kbytes)
>> 7 fixed-partitions partitions found on MTD device spi0.0
> 
> After 'm25p80' probe 'f1070000.ethernet' linked to 'partition@1' :
> 
>> device: 'f1010600.spi:m25p80@0:
> partitions:partition@1': device_add
>> device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet': device_add
>> devices_kset: Moving f1070000.ethernet to end of list
>> platform f1070000.ethernet: Linked as a consumer to f1010600.spi:m25p80@0:partitions:partition@1
>> ethernet@70000 Dropping the fwnode link to partition@1
> 
> And as a result I got `-EPROBE_DEFER` for `f1070000.ethernet`
> 
>> platform f1070000.ethernet: error -EPROBE_DEFER: supplier f1010600.spi:m25p80@0:partitions:partition@1 not ready
> 
> Here is a part of my device tree:
> 
>     enet1: ethernet@70000 {
>         status = "okay";
>         nvmem-cells = <&macaddr>;
>         nvmem-cell-names = "mac-address";
>         phy-mode = "rgmii";
>         phy = <&phy0>;
>     };
> 
>     spi@10600 {
>         status = "okay";
> 
>         m25p80@0 {
>             compatible = "mx66l51235l";
>             reg = <0>;
>             #address-cells = <1>;
>             #size-cells = <1>;
> 
>             partitions {
>                 compatible = "fixed-partitions";
>                 #address-cells = <1>;
>                 #size-cells = <1>;
> 
>                 partition@0 {
>                     reg = <0x00000000 0x000080000>;
>                     label = "SPI.U_BOOT";
>                 };
> 
>                 partition@1 {
>                     compatible = "nvmem-cells";
>                     reg = <0x000A0000 0x00020000>;
>                     label = "SPI.INV_INFO";
>                     #address-cells = <1>;
>                     #size-cells = <1>;
>                     ranges = <0 0x000A0000 0x00020000>;
> 
>                     macaddr: mac@6 {
>                         reg = <0x6 0x6>;
>                     };
>                 };
> 
>             };
>         };
>     };
> 
> In the example above 'ethernet@70000' requires 'macaddr: mac@6' which is
> located inside mtd 'partition@1' of 'm25p80@0' spi flash.

P.P.S.: let me add this to the regression tracking:

#regzbot ^introduced bcdf0315
#regzbot title mtd: ethernet device can't be probed anymore due to
broken nvmem-cells dep
#regzbot monitor: https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
#regzbot ignore-activity

Re: Fwd: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Maxim Kiselev]

>On 10.12.22 10:52, Maxim Kiselev wrote:
>>
>> After applying
>
>This makes me wonder: "applying" as in "applying it to some version that
>doesn't contain this change normally" or as it "after it was applied to
>mainline I have the following problem with vanilla kernel version <???>"?

Sorry for confusing you, I mean "after it was applied to mainline".
I have this problem with vanilla kernel version 6.0.

>>> I faced with a problem that my ethernet device can't be probed because it
>>> wait when 'nvmem-cells' device will be probed first.
>>
>>FWIW, there is a discussion about a problems that at least to my
>>untrained eyes looks similar:
>>https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/

Yes it looks like the same issue.


I think the root of the problem was the choice of 'compatible'
device tree property to mark the mtd partition node as a nvmem provider.

I'm talking about this part in 'mtd_nvmem_add' function.
> config.no_of_node = !of_device_is_compatible(node, "nvmem-cells");

Maybe we should change the 'compatible' property to something else?

сб, 10 дек. 2022 г. в 15:35, Thorsten Leemhuis <regressions@leemhuis.info>:
>
> [CCing the regression mailing list, as it should be in the loop for all
> regressions, as explained in
> https://docs.kernel.org/admin-guide/reporting-regressions.html ]
>
> Hi, this is your Linux kernel regression tracker. Thx for the report.
>
> On 10.12.22 10:52, Maxim Kiselev wrote:
> >
> > After applying
>
> This makes me wonder: "applying" as in "applying it to some version that
> doesn't contain this change normally" or as it "after it was applied to
> mainline I have the following problem with vanilla kernel version <???>"?
>
> > this commit 'mtd: call of_platform_populate() for MTD
> > partitions' (bcdf0315),
>
> CCing Rafał, who authored bcdf0315.
>
> > I faced with a problem that my ethernet device can't be probed because it
> > wait when 'nvmem-cells' device will be probed first.
>
> FWIW, there is a discussion about a problems that at least to my
> untrained eyes looks similar:
> https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
>
> Rafał, has some progress been made to resolve this?
>
> To me it sounds like this might warrant a "revert, and reapply later
> when the cause for the regression was addressed". Rafał, it seems you
> suggested something like that, but it doesn't look like that happened
> for one reason or another. Or am I missing something?
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>
> P.S.: As the Linux kernel's regression tracker I deal with a lot of
> reports and sometimes miss something important when writing mails like
> this. If that's the case here, don't hesitate to tell me in a public
> reply, it's in everyone's interest to set the public record straight.
>
> > But there is no such driver which is compatible with 'nvmem-cells' because
> > 'nvmem-cells' is just a mark used by the 'mtd_nvmem_add' function.
> >
> > So this leads to appeating of unresolved dependency for the ethernet device.
> > And that's why the ethernet device can't be added and probed.
> >
> > Here is a part of kernel log when spi flash probe start:
> >
> >> device: 'spi0': device_add
> >> device: 'spi0.0': device_add
> >> spi-nor spi0.0: mx66l51235f (65536 Kbytes)
> >> 7 fixed-partitions partitions found on MTD device spi0.0
> >
> > After 'm25p80' probe 'f1070000.ethernet' linked to 'partition@1' :
> >
> >> device: 'f1010600.spi:m25p80@0:
> > partitions:partition@1': device_add
> >> device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet': device_add
> >> devices_kset: Moving f1070000.ethernet to end of list
> >> platform f1070000.ethernet: Linked as a consumer to f1010600.spi:m25p80@0:partitions:partition@1
> >> ethernet@70000 Dropping the fwnode link to partition@1
> >
> > And as a result I got `-EPROBE_DEFER` for `f1070000.ethernet`
> >
> >> platform f1070000.ethernet: error -EPROBE_DEFER: supplier f1010600.spi:m25p80@0:partitions:partition@1 not ready
> >
> > Here is a part of my device tree:
> >
> >     enet1: ethernet@70000 {
> >         status = "okay";
> >         nvmem-cells = <&macaddr>;
> >         nvmem-cell-names = "mac-address";
> >         phy-mode = "rgmii";
> >         phy = <&phy0>;
> >     };
> >
> >     spi@10600 {
> >         status = "okay";
> >
> >         m25p80@0 {
> >             compatible = "mx66l51235l";
> >             reg = <0>;
> >             #address-cells = <1>;
> >             #size-cells = <1>;
> >
> >             partitions {
> >                 compatible = "fixed-partitions";
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> >
> >                 partition@0 {
> >                     reg = <0x00000000 0x000080000>;
> >                     label = "SPI.U_BOOT";
> >                 };
> >
> >                 partition@1 {
> >                     compatible = "nvmem-cells";
> >                     reg = <0x000A0000 0x00020000>;
> >                     label = "SPI.INV_INFO";
> >                     #address-cells = <1>;
> >                     #size-cells = <1>;
> >                     ranges = <0 0x000A0000 0x00020000>;
> >
> >                     macaddr: mac@6 {
> >                         reg = <0x6 0x6>;
> >                     };
> >                 };
> >
> >             };
> >         };
> >     };
> >
> > In the example above 'ethernet@70000' requires 'macaddr: mac@6' which is
> > located inside mtd 'partition@1' of 'm25p80@0' spi flash.
>
> P.P.S.: let me add this to the regression tracking:
>
> #regzbot ^introduced bcdf0315
> #regzbot title mtd: ethernet device can't be probed anymore due to
> broken nvmem-cells dep
> #regzbot monitor: https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> #regzbot ignore-activity

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Miquel Raynal]

Hi Maxim,

bigunclemax@gmail.com wrote on Sun, 11 Dec 2022 11:26:29 +0300:

> >On 10.12.22 10:52, Maxim Kiselev wrote:  
> >>
> >> After applying  
> >
> >This makes me wonder: "applying" as in "applying it to some version that
> >doesn't contain this change normally" or as it "after it was applied to
> >mainline I have the following problem with vanilla kernel version <???>"?  
> 
> Sorry for confusing you, I mean "after it was applied to mainline".
> I have this problem with vanilla kernel version 6.0.
> 
> >>> I faced with a problem that my ethernet device can't be probed because it
> >>> wait when 'nvmem-cells' device will be probed first.  
> >>
> >>FWIW, there is a discussion about a problems that at least to my
> >>untrained eyes looks similar:
> >>https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/  
> 
> Yes it looks like the same issue.
> 
> 
> I think the root of the problem was the choice of 'compatible'
> device tree property to mark the mtd partition node as a nvmem provider.
> 
> I'm talking about this part in 'mtd_nvmem_add' function.
> > config.no_of_node = !of_device_is_compatible(node, "nvmem-cells");  
> 
> Maybe we should change the 'compatible' property to something else?

At a first glance I don't get why the compatible would matter so much
here, can you point to some core DT logic that would have an effect? I
mean besides leading to the creation of a cell. IOW, what would be done
differently if the compatible was different?

Can you also dump the device links (if you can reach a prompt) from
sysfs?

In theory there should be a link between ethernet and spi-nor which is
fulfilled when the spi-nor device probes and leads to the creation of
device links. Maybe there is "something else" that the mtd core should
do, because this just works with eeproms (non-mtd cells), so let's find
out.

> сб, 10 дек. 2022 г. в 15:35, Thorsten Leemhuis <regressions@leemhuis.info>:
> >
> > [CCing the regression mailing list, as it should be in the loop for all
> > regressions, as explained in
> > https://docs.kernel.org/admin-guide/reporting-regressions.html ]
> >
> > Hi, this is your Linux kernel regression tracker. Thx for the report.
> >
> > On 10.12.22 10:52, Maxim Kiselev wrote:  
> > >
> > > After applying  
> >
> > This makes me wonder: "applying" as in "applying it to some version that
> > doesn't contain this change normally" or as it "after it was applied to
> > mainline I have the following problem with vanilla kernel version <???>"?
> >  
> > > this commit 'mtd: call of_platform_populate() for MTD
> > > partitions' (bcdf0315),  
> >
> > CCing Rafał, who authored bcdf0315.
> >  
> > > I faced with a problem that my ethernet device can't be probed because it
> > > wait when 'nvmem-cells' device will be probed first.  
> >
> > FWIW, there is a discussion about a problems that at least to my
> > untrained eyes looks similar:
> > https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> >
> > Rafał, has some progress been made to resolve this?
> >
> > To me it sounds like this might warrant a "revert, and reapply later
> > when the cause for the regression was addressed". Rafał, it seems you
> > suggested something like that, but it doesn't look like that happened
> > for one reason or another. Or am I missing something?
> >
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> >
> > P.S.: As the Linux kernel's regression tracker I deal with a lot of
> > reports and sometimes miss something important when writing mails like
> > this. If that's the case here, don't hesitate to tell me in a public
> > reply, it's in everyone's interest to set the public record straight.
> >  
> > > But there is no such driver which is compatible with 'nvmem-cells' because
> > > 'nvmem-cells' is just a mark used by the 'mtd_nvmem_add' function.
> > >
> > > So this leads to appeating of unresolved dependency for the ethernet device.
> > > And that's why the ethernet device can't be added and probed.
> > >
> > > Here is a part of kernel log when spi flash probe start:
> > >  
> > >> device: 'spi0': device_add
> > >> device: 'spi0.0': device_add
> > >> spi-nor spi0.0: mx66l51235f (65536 Kbytes)
> > >> 7 fixed-partitions partitions found on MTD device spi0.0  
> > >
> > > After 'm25p80' probe 'f1070000.ethernet' linked to 'partition@1' :
> > >  
> > >> device: 'f1010600.spi:m25p80@0:  
> > > partitions:partition@1': device_add  
> > >> device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet': device_add
> > >> devices_kset: Moving f1070000.ethernet to end of list
> > >> platform f1070000.ethernet: Linked as a consumer to f1010600.spi:m25p80@0:partitions:partition@1
> > >> ethernet@70000 Dropping the fwnode link to partition@1  
> > >
> > > And as a result I got `-EPROBE_DEFER` for `f1070000.ethernet`
> > >  
> > >> platform f1070000.ethernet: error -EPROBE_DEFER: supplier f1010600.spi:m25p80@0:partitions:partition@1 not ready  
> > >
> > > Here is a part of my device tree:
> > >
> > >     enet1: ethernet@70000 {
> > >         status = "okay";
> > >         nvmem-cells = <&macaddr>;
> > >         nvmem-cell-names = "mac-address";
> > >         phy-mode = "rgmii";
> > >         phy = <&phy0>;
> > >     };
> > >
> > >     spi@10600 {
> > >         status = "okay";
> > >
> > >         m25p80@0 {
> > >             compatible = "mx66l51235l";
> > >             reg = <0>;
> > >             #address-cells = <1>;
> > >             #size-cells = <1>;
> > >
> > >             partitions {
> > >                 compatible = "fixed-partitions";
> > >                 #address-cells = <1>;
> > >                 #size-cells = <1>;
> > >
> > >                 partition@0 {
> > >                     reg = <0x00000000 0x000080000>;
> > >                     label = "SPI.U_BOOT";
> > >                 };
> > >
> > >                 partition@1 {
> > >                     compatible = "nvmem-cells";
> > >                     reg = <0x000A0000 0x00020000>;
> > >                     label = "SPI.INV_INFO";
> > >                     #address-cells = <1>;
> > >                     #size-cells = <1>;
> > >                     ranges = <0 0x000A0000 0x00020000>;
> > >
> > >                     macaddr: mac@6 {
> > >                         reg = <0x6 0x6>;
> > >                     };
> > >                 };
> > >
> > >             };
> > >         };
> > >     };
> > >
> > > In the example above 'ethernet@70000' requires 'macaddr: mac@6' which is
> > > located inside mtd 'partition@1' of 'm25p80@0' spi flash.  
> >
> > P.P.S.: let me add this to the regression tracking:
> >
> > #regzbot ^introduced bcdf0315
> > #regzbot title mtd: ethernet device can't be probed anymore due to
> > broken nvmem-cells dep
> > #regzbot monitor: https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> > #regzbot ignore-activity  


Thanks,
Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Maxim Kiselev]

> At a first glance I don't get why the compatible would matter so much
> here, can you point to some core DT logic that would have an effect? I
> mean besides leading to the creation of a cell. IOW, what would be done
> differently if the compatible was different?

After adding a call of_platform_populate() for MTD partitions (bcdf0315)
we got a call of 'device_add' for each mtd partition which have
a 'compatible' property in its DT node. As a result any other devices
which have a DT reference to 'nvmem-cell' compatible mtd partition
got dependence from it (see attached log below). Now they will be waiting until
a 'nvmem-cell' device will be probed. But this will never happen because there
is no 'nvmem-cell' driver.


Here is part of kernel log after bcdf0315 commit:
  device: 'spi0.0': device_add
  spi-nor spi0.0: mx66l51235f (65536 Kbytes)
  7 fixed-partitions partitions found on MTD device spi0.0
  device: 'f1010600.spi:m25p80@0:partitions:partition@1': device_add
  device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet':
device_add
  devices_kset: Moving f1070000.ethernet to end of list
  platform f1070000.ethernet: Linked as a consumer to
f1010600.spi:m25p80@0:partitions:partition@1
  ethernet@70000 Dropping the fwnode link to partition@1
  Creating 7 MTD partitions on "spi0.0":
  platform f1070000.ethernet: error -EPROBE_DEFER: supplier
f1010600.spi:m25p80@0:partitions:partition@1 not ready
  devices_kset: Moving f1070000.ethernet to end of list
  platform f1070000.ethernet: error -EPROBE_DEFER: supplier
f1010600.spi:m25p80@0:partitions:partition@1 not ready

Here is part of kernel log with reverted bcdf0315 commit:
  device: 'spi0.0': device_add
  spi-nor spi0.0: mx66l51235f (65536 Kbytes)
  7 fixed-partitions partitions found on MTD device spi0.0
  Creating 7 MTD partitions on "spi0.0":
  ethernet@70000 Dropping the fwnode link to partition@1
  device: 'eth0': device_add
  mvneta f1070000.ethernet eth0: Using device tree mac address de:fa:ce:db:ab:e1

Look at strings below which exists only in the first log (with bcdf0315):
  device: 'f1010600.spi:m25p80@0:partitions:partition@1': device_add
  device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet':
device_add
  platform f1070000.ethernet: error -EPROBE_DEFER: supplier
f1010600.spi:m25p80@0:partitions:partition@1 not ready


> because this just works with eeproms (non-mtd cells), so let's find out.
This works for eeproms and even for whole mtd partitions because there
are compatible drivers and those devices
can be probed, ulike a mtd subnode which is compatible with
non-existent 'nvmem-cell' driver.

пн, 12 дек. 2022 г. в 12:14, Miquel Raynal <miquel.raynal@bootlin.com>:
>
> Hi Maxim,
>
> bigunclemax@gmail.com wrote on Sun, 11 Dec 2022 11:26:29 +0300:
>
> > >On 10.12.22 10:52, Maxim Kiselev wrote:
> > >>
> > >> After applying
> > >
> > >This makes me wonder: "applying" as in "applying it to some version that
> > >doesn't contain this change normally" or as it "after it was applied to
> > >mainline I have the following problem with vanilla kernel version <???>"?
> >
> > Sorry for confusing you, I mean "after it was applied to mainline".
> > I have this problem with vanilla kernel version 6.0.
> >
> > >>> I faced with a problem that my ethernet device can't be probed because it
> > >>> wait when 'nvmem-cells' device will be probed first.
> > >>
> > >>FWIW, there is a discussion about a problems that at least to my
> > >>untrained eyes looks similar:
> > >>https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> >
> > Yes it looks like the same issue.
> >
> >
> > I think the root of the problem was the choice of 'compatible'
> > device tree property to mark the mtd partition node as a nvmem provider.
> >
> > I'm talking about this part in 'mtd_nvmem_add' function.
> > > config.no_of_node = !of_device_is_compatible(node, "nvmem-cells");
> >
> > Maybe we should change the 'compatible' property to something else?
>
> At a first glance I don't get why the compatible would matter so much
> here, can you point to some core DT logic that would have an effect? I
> mean besides leading to the creation of a cell. IOW, what would be done
> differently if the compatible was different?
>
> Can you also dump the device links (if you can reach a prompt) from
> sysfs?
>
> In theory there should be a link between ethernet and spi-nor which is
> fulfilled when the spi-nor device probes and leads to the creation of
> device links. Maybe there is "something else" that the mtd core should
> do, because this just works with eeproms (non-mtd cells), so let's find
> out.
>
> > сб, 10 дек. 2022 г. в 15:35, Thorsten Leemhuis <regressions@leemhuis.info>:
> > >
> > > [CCing the regression mailing list, as it should be in the loop for all
> > > regressions, as explained in
> > > https://docs.kernel.org/admin-guide/reporting-regressions.html ]
> > >
> > > Hi, this is your Linux kernel regression tracker. Thx for the report.
> > >
> > > On 10.12.22 10:52, Maxim Kiselev wrote:
> > > >
> > > > After applying
> > >
> > > This makes me wonder: "applying" as in "applying it to some version that
> > > doesn't contain this change normally" or as it "after it was applied to
> > > mainline I have the following problem with vanilla kernel version <???>"?
> > >
> > > > this commit 'mtd: call of_platform_populate() for MTD
> > > > partitions' (bcdf0315),
> > >
> > > CCing Rafał, who authored bcdf0315.
> > >
> > > > I faced with a problem that my ethernet device can't be probed because it
> > > > wait when 'nvmem-cells' device will be probed first.
> > >
> > > FWIW, there is a discussion about a problems that at least to my
> > > untrained eyes looks similar:
> > > https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> > >
> > > Rafał, has some progress been made to resolve this?
> > >
> > > To me it sounds like this might warrant a "revert, and reapply later
> > > when the cause for the regression was addressed". Rafał, it seems you
> > > suggested something like that, but it doesn't look like that happened
> > > for one reason or another. Or am I missing something?
> > >
> > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> > >
> > > P.S.: As the Linux kernel's regression tracker I deal with a lot of
> > > reports and sometimes miss something important when writing mails like
> > > this. If that's the case here, don't hesitate to tell me in a public
> > > reply, it's in everyone's interest to set the public record straight.
> > >
> > > > But there is no such driver which is compatible with 'nvmem-cells' because
> > > > 'nvmem-cells' is just a mark used by the 'mtd_nvmem_add' function.
> > > >
> > > > So this leads to appeating of unresolved dependency for the ethernet device.
> > > > And that's why the ethernet device can't be added and probed.
> > > >
> > > > Here is a part of kernel log when spi flash probe start:
> > > >
> > > >> device: 'spi0': device_add
> > > >> device: 'spi0.0': device_add
> > > >> spi-nor spi0.0: mx66l51235f (65536 Kbytes)
> > > >> 7 fixed-partitions partitions found on MTD device spi0.0
> > > >
> > > > After 'm25p80' probe 'f1070000.ethernet' linked to 'partition@1' :
> > > >
> > > >> device: 'f1010600.spi:m25p80@0:
> > > > partitions:partition@1': device_add
> > > >> device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet': device_add
> > > >> devices_kset: Moving f1070000.ethernet to end of list
> > > >> platform f1070000.ethernet: Linked as a consumer to f1010600.spi:m25p80@0:partitions:partition@1
> > > >> ethernet@70000 Dropping the fwnode link to partition@1
> > > >
> > > > And as a result I got `-EPROBE_DEFER` for `f1070000.ethernet`
> > > >
> > > >> platform f1070000.ethernet: error -EPROBE_DEFER: supplier f1010600.spi:m25p80@0:partitions:partition@1 not ready
> > > >
> > > > Here is a part of my device tree:
> > > >
> > > >     enet1: ethernet@70000 {
> > > >         status = "okay";
> > > >         nvmem-cells = <&macaddr>;
> > > >         nvmem-cell-names = "mac-address";
> > > >         phy-mode = "rgmii";
> > > >         phy = <&phy0>;
> > > >     };
> > > >
> > > >     spi@10600 {
> > > >         status = "okay";
> > > >
> > > >         m25p80@0 {
> > > >             compatible = "mx66l51235l";
> > > >             reg = <0>;
> > > >             #address-cells = <1>;
> > > >             #size-cells = <1>;
> > > >
> > > >             partitions {
> > > >                 compatible = "fixed-partitions";
> > > >                 #address-cells = <1>;
> > > >                 #size-cells = <1>;
> > > >
> > > >                 partition@0 {
> > > >                     reg = <0x00000000 0x000080000>;
> > > >                     label = "SPI.U_BOOT";
> > > >                 };
> > > >
> > > >                 partition@1 {
> > > >                     compatible = "nvmem-cells";
> > > >                     reg = <0x000A0000 0x00020000>;
> > > >                     label = "SPI.INV_INFO";
> > > >                     #address-cells = <1>;
> > > >                     #size-cells = <1>;
> > > >                     ranges = <0 0x000A0000 0x00020000>;
> > > >
> > > >                     macaddr: mac@6 {
> > > >                         reg = <0x6 0x6>;
> > > >                     };
> > > >                 };
> > > >
> > > >             };
> > > >         };
> > > >     };
> > > >
> > > > In the example above 'ethernet@70000' requires 'macaddr: mac@6' which is
> > > > located inside mtd 'partition@1' of 'm25p80@0' spi flash.
> > >
> > > P.P.S.: let me add this to the regression tracking:
> > >
> > > #regzbot ^introduced bcdf0315
> > > #regzbot title mtd: ethernet device can't be probed anymore due to
> > > broken nvmem-cells dep
> > > #regzbot monitor: https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> > > #regzbot ignore-activity
>
>
> Thanks,
> Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Miquel Raynal]

Hi Maxim,

+ Rob & Krzysztof for the binding side
+ Greg, Rafael & Saravana for the device links side

bigunclemax@gmail.com wrote on Mon, 12 Dec 2022 16:06:35 +0300:

> > At a first glance I don't get why the compatible would matter so much
> > here, can you point to some core DT logic that would have an effect? I
> > mean besides leading to the creation of a cell. IOW, what would be done
> > differently if the compatible was different?  
> 
> After adding a call of_platform_populate() for MTD partitions (bcdf0315)
> we got a call of 'device_add' for each mtd partition which have
> a 'compatible' property in its DT node. As a result any other devices
> which have a DT reference to 'nvmem-cell' compatible mtd partition
> got dependence from it (see attached log below). Now they will be waiting until
> a 'nvmem-cell' device will be probed. But this will never happen because there
> is no 'nvmem-cell' driver.
> 
> 
> Here is part of kernel log after bcdf0315 commit:
>   device: 'spi0.0': device_add
>   spi-nor spi0.0: mx66l51235f (65536 Kbytes)
>   7 fixed-partitions partitions found on MTD device spi0.0
>   device: 'f1010600.spi:m25p80@0:partitions:partition@1': device_add
>   device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet':
> device_add
>   devices_kset: Moving f1070000.ethernet to end of list
>   platform f1070000.ethernet: Linked as a consumer to
> f1010600.spi:m25p80@0:partitions:partition@1
>   ethernet@70000 Dropping the fwnode link to partition@1
>   Creating 7 MTD partitions on "spi0.0":
>   platform f1070000.ethernet: error -EPROBE_DEFER: supplier
> f1010600.spi:m25p80@0:partitions:partition@1 not ready
>   devices_kset: Moving f1070000.ethernet to end of list
>   platform f1070000.ethernet: error -EPROBE_DEFER: supplier
> f1010600.spi:m25p80@0:partitions:partition@1 not ready
> 
> Here is part of kernel log with reverted bcdf0315 commit:
>   device: 'spi0.0': device_add
>   spi-nor spi0.0: mx66l51235f (65536 Kbytes)
>   7 fixed-partitions partitions found on MTD device spi0.0
>   Creating 7 MTD partitions on "spi0.0":
>   ethernet@70000 Dropping the fwnode link to partition@1
>   device: 'eth0': device_add
>   mvneta f1070000.ethernet eth0: Using device tree mac address de:fa:ce:db:ab:e1
> 
> Look at strings below which exists only in the first log (with bcdf0315):
>   device: 'f1010600.spi:m25p80@0:partitions:partition@1': device_add
>   device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet':
> device_add
>   platform f1070000.ethernet: error -EPROBE_DEFER: supplier
> f1010600.spi:m25p80@0:partitions:partition@1 not ready
> 
> 
> > because this just works with eeproms (non-mtd cells), so let's find out.  
> This works for eeproms and even for whole mtd partitions because there
> are compatible drivers and those devices
> can be probed, ulike a mtd subnode which is compatible with
> non-existent 'nvmem-cell' driver.

Let me try to recap the situation for all the people I just involved:

* An Ethernet driver gets its mac address from an nvmem cell. The
  Ethernet controller DT node then has an "nvmem-cells" property
  pointing towards an nvmem cell.
* The nvmem cell comes from an mtd partition.
* The mtd partition is flagged with a particular compatible
  (which is also named "nvmem-cells") to tell the kernel that the node
  produces nvmem cells.
* The mtd partition itself has no driver, but is the child node of a
  "partitions" container which has one (in this case,
  "fixed-partitions", see the snippet below).

Because the "nvmem-cells" property of the Ethernet node points at the
nvmem-cell node, the core create a device link between the Ethernet
controller (consumer) and the mtd partition (producer).

The device link in this case will never be satisfied because no driver
matches the "nvmem-cells" compatible of the partition node.

Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
partitions") would IMHO not make much sense, the problem comes from the
device link side and even there, there is nothing really "wrong",
because I really expect the mtd device to be ready before the
Ethernet controller probe, the device link is legitimate.

So I would like to explore other alternatives. Here are a bunch of
ideas, but I'm open:

1. compatible = "nvmem-cells" was maybe not the right way to say "this
   is an nvmem-cells producer":
     1a. Maybe we could use the (new) nvmem-layout container [1]?
     1b. Or we could force users to use the user-otp container [2]?
     1c. Or replace the compatible by a property?

   1c is ugly, all three solutions would break the current bindings,
   hence I would really prefer to got for #2.

2. I don't understand the device link state/flags deeply enough to
   understand what they all convey exactly, but I guess we
   should be able to tell the driver model that the "device" (ie. the
   partition) was created by a driver which owns this device. In this
   case an mtd device owned by the mtd core was created by a parser,
   and there is no additional probe step that should be expected.

   Maybe there is a way to tell this to the driver core at device
   registration time?

   Or perhaps we should workaround it by calling one of:
	- device_links_driver_bound()
	- device_links_no_driver()
   from the mtd core, after registering all the mtd partition devices?

   I am really interested to understand better how the device links are
   expected to evolve, so any pointers would be appreciated.

[1] https://lore.kernel.org/all/20221114085659.847611-1-miquel.raynal@bootlin.com/T/#m5c6ed1efd675fda0e2cd174502d38c1565f0274b

[2] https://lore.kernel.org/all/20210424110608.15748-4-michael@walle.cc/

Thanks,
Miquèl

> 
> пн, 12 дек. 2022 г. в 12:14, Miquel Raynal <miquel.raynal@bootlin.com>:
> >
> > Hi Maxim,
> >
> > bigunclemax@gmail.com wrote on Sun, 11 Dec 2022 11:26:29 +0300:
> >  
> > > >On 10.12.22 10:52, Maxim Kiselev wrote:  
> > > >>
> > > >> After applying  
> > > >
> > > >This makes me wonder: "applying" as in "applying it to some version that
> > > >doesn't contain this change normally" or as it "after it was applied to
> > > >mainline I have the following problem with vanilla kernel version <???>"?  
> > >
> > > Sorry for confusing you, I mean "after it was applied to mainline".
> > > I have this problem with vanilla kernel version 6.0.
> > >  
> > > >>> I faced with a problem that my ethernet device can't be probed because it
> > > >>> wait when 'nvmem-cells' device will be probed first.  
> > > >>
> > > >>FWIW, there is a discussion about a problems that at least to my
> > > >>untrained eyes looks similar:
> > > >>https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/  
> > >
> > > Yes it looks like the same issue.
> > >
> > >
> > > I think the root of the problem was the choice of 'compatible'
> > > device tree property to mark the mtd partition node as a nvmem provider.
> > >
> > > I'm talking about this part in 'mtd_nvmem_add' function.  
> > > > config.no_of_node = !of_device_is_compatible(node, "nvmem-cells");  
> > >
> > > Maybe we should change the 'compatible' property to something else?  
> >
> > At a first glance I don't get why the compatible would matter so much
> > here, can you point to some core DT logic that would have an effect? I
> > mean besides leading to the creation of a cell. IOW, what would be done
> > differently if the compatible was different?
> >
> > Can you also dump the device links (if you can reach a prompt) from
> > sysfs?
> >
> > In theory there should be a link between ethernet and spi-nor which is
> > fulfilled when the spi-nor device probes and leads to the creation of
> > device links. Maybe there is "something else" that the mtd core should
> > do, because this just works with eeproms (non-mtd cells), so let's find
> > out.
> >  
> > > сб, 10 дек. 2022 г. в 15:35, Thorsten Leemhuis <regressions@leemhuis.info>:  
> > > >
> > > > [CCing the regression mailing list, as it should be in the loop for all
> > > > regressions, as explained in
> > > > https://docs.kernel.org/admin-guide/reporting-regressions.html ]
> > > >
> > > > Hi, this is your Linux kernel regression tracker. Thx for the report.
> > > >
> > > > On 10.12.22 10:52, Maxim Kiselev wrote:  
> > > > >
> > > > > After applying  
> > > >
> > > > This makes me wonder: "applying" as in "applying it to some version that
> > > > doesn't contain this change normally" or as it "after it was applied to
> > > > mainline I have the following problem with vanilla kernel version <???>"?
> > > >  
> > > > > this commit 'mtd: call of_platform_populate() for MTD
> > > > > partitions' (bcdf0315),  
> > > >
> > > > CCing Rafał, who authored bcdf0315.
> > > >  
> > > > > I faced with a problem that my ethernet device can't be probed because it
> > > > > wait when 'nvmem-cells' device will be probed first.  
> > > >
> > > > FWIW, there is a discussion about a problems that at least to my
> > > > untrained eyes looks similar:
> > > > https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> > > >
> > > > Rafał, has some progress been made to resolve this?
> > > >
> > > > To me it sounds like this might warrant a "revert, and reapply later
> > > > when the cause for the regression was addressed". Rafał, it seems you
> > > > suggested something like that, but it doesn't look like that happened
> > > > for one reason or another. Or am I missing something?
> > > >
> > > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> > > >
> > > > P.S.: As the Linux kernel's regression tracker I deal with a lot of
> > > > reports and sometimes miss something important when writing mails like
> > > > this. If that's the case here, don't hesitate to tell me in a public
> > > > reply, it's in everyone's interest to set the public record straight.
> > > >  
> > > > > But there is no such driver which is compatible with 'nvmem-cells' because
> > > > > 'nvmem-cells' is just a mark used by the 'mtd_nvmem_add' function.
> > > > >
> > > > > So this leads to appeating of unresolved dependency for the ethernet device.
> > > > > And that's why the ethernet device can't be added and probed.
> > > > >
> > > > > Here is a part of kernel log when spi flash probe start:
> > > > >  
> > > > >> device: 'spi0': device_add
> > > > >> device: 'spi0.0': device_add
> > > > >> spi-nor spi0.0: mx66l51235f (65536 Kbytes)
> > > > >> 7 fixed-partitions partitions found on MTD device spi0.0  
> > > > >
> > > > > After 'm25p80' probe 'f1070000.ethernet' linked to 'partition@1' :
> > > > >  
> > > > >> device: 'f1010600.spi:m25p80@0:  
> > > > > partitions:partition@1': device_add  
> > > > >> device: 'platform:f1010600.spi:m25p80@0:partitions:partition@1--platform:f1070000.ethernet': device_add
> > > > >> devices_kset: Moving f1070000.ethernet to end of list
> > > > >> platform f1070000.ethernet: Linked as a consumer to f1010600.spi:m25p80@0:partitions:partition@1
> > > > >> ethernet@70000 Dropping the fwnode link to partition@1  
> > > > >
> > > > > And as a result I got `-EPROBE_DEFER` for `f1070000.ethernet`
> > > > >  
> > > > >> platform f1070000.ethernet: error -EPROBE_DEFER: supplier f1010600.spi:m25p80@0:partitions:partition@1 not ready  
> > > > >
> > > > > Here is a part of my device tree:
> > > > >
> > > > >     enet1: ethernet@70000 {
> > > > >         status = "okay";
> > > > >         nvmem-cells = <&macaddr>;
> > > > >         nvmem-cell-names = "mac-address";
> > > > >         phy-mode = "rgmii";
> > > > >         phy = <&phy0>;
> > > > >     };
> > > > >
> > > > >     spi@10600 {
> > > > >         status = "okay";
> > > > >
> > > > >         m25p80@0 {
> > > > >             compatible = "mx66l51235l";
> > > > >             reg = <0>;
> > > > >             #address-cells = <1>;
> > > > >             #size-cells = <1>;
> > > > >
> > > > >             partitions {
> > > > >                 compatible = "fixed-partitions";
> > > > >                 #address-cells = <1>;
> > > > >                 #size-cells = <1>;
> > > > >
> > > > >                 partition@0 {
> > > > >                     reg = <0x00000000 0x000080000>;
> > > > >                     label = "SPI.U_BOOT";
> > > > >                 };
> > > > >
> > > > >                 partition@1 {
> > > > >                     compatible = "nvmem-cells";
> > > > >                     reg = <0x000A0000 0x00020000>;
> > > > >                     label = "SPI.INV_INFO";
> > > > >                     #address-cells = <1>;
> > > > >                     #size-cells = <1>;
> > > > >                     ranges = <0 0x000A0000 0x00020000>;
> > > > >
> > > > >                     macaddr: mac@6 {
> > > > >                         reg = <0x6 0x6>;
> > > > >                     };
> > > > >                 };
> > > > >
> > > > >             };
> > > > >         };
> > > > >     };
> > > > >
> > > > > In the example above 'ethernet@70000' requires 'macaddr: mac@6' which is
> > > > > located inside mtd 'partition@1' of 'm25p80@0' spi flash.  
> > > >
> > > > P.P.S.: let me add this to the regression tracking:
> > > >
> > > > #regzbot ^introduced bcdf0315
> > > > #regzbot title mtd: ethernet device can't be probed anymore due to
> > > > broken nvmem-cells dep
> > > > #regzbot monitor: https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> > > > #regzbot ignore-activity  
> >
> >
> > Thanks,
> > Miquèl  

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Maxim Kochetkov]

Hi, Miquel!

On 12.12.2022 19:37, Miquel Raynal wrote:

> Let me try to recap the situation for all the people I just involved:
> 
> * An Ethernet driver gets its mac address from an nvmem cell. The
>    Ethernet controller DT node then has an "nvmem-cells" property
>    pointing towards an nvmem cell.
> * The nvmem cell comes from an mtd partition.
> * The mtd partition is flagged with a particular compatible
>    (which is also named "nvmem-cells") to tell the kernel that the node
>    produces nvmem cells.
> * The mtd partition itself has no driver, but is the child node of a
>    "partitions" container which has one (in this case,
>    "fixed-partitions", see the snippet below).
> 
> Because the "nvmem-cells" property of the Ethernet node points at the
> nvmem-cell node, the core create a device link between the Ethernet
> controller (consumer) and the mtd partition (producer).
> 
> The device link in this case will never be satisfied because no driver
> matches the "nvmem-cells" compatible of the partition node.
> 
> Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> partitions") would IMHO not make much sense, the problem comes from the
> device link side and even there, there is nothing really "wrong",
> because I really expect the mtd device to be ready before the
> Ethernet controller probe, the device link is legitimate.
> 
> So I would like to explore other alternatives. Here are a bunch of
> ideas, but I'm open:

How about to create simple driver with compatible="nvmem-cell" and to 
move all the suff from main mtd driver which serves nvmem-cell to the 
probe function?

Thanks, Maxim.

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Miquel Raynal]

Hi Maxim,

fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:

> Hi, Miquel!
> 
> On 12.12.2022 19:37, Miquel Raynal wrote:
> 
> > Let me try to recap the situation for all the people I just involved:
> > 
> > * An Ethernet driver gets its mac address from an nvmem cell. The
> >    Ethernet controller DT node then has an "nvmem-cells" property
> >    pointing towards an nvmem cell.
> > * The nvmem cell comes from an mtd partition.
> > * The mtd partition is flagged with a particular compatible
> >    (which is also named "nvmem-cells") to tell the kernel that the node
> >    produces nvmem cells.
> > * The mtd partition itself has no driver, but is the child node of a
> >    "partitions" container which has one (in this case,
> >    "fixed-partitions", see the snippet below).
> > 
> > Because the "nvmem-cells" property of the Ethernet node points at the
> > nvmem-cell node, the core create a device link between the Ethernet
> > controller (consumer) and the mtd partition (producer).
> > 
> > The device link in this case will never be satisfied because no driver
> > matches the "nvmem-cells" compatible of the partition node.
> > 
> > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > partitions") would IMHO not make much sense, the problem comes from the
> > device link side and even there, there is nothing really "wrong",
> > because I really expect the mtd device to be ready before the
> > Ethernet controller probe, the device link is legitimate.
> > 
> > So I would like to explore other alternatives. Here are a bunch of
> > ideas, but I'm open:  
> 
> How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?

This is probably worth the try but I doubt you can make it work without
regressions because IIRC the nvmem registration happens no matter the
compatible (not mentioning the user-otp and factory-otp cases). You can
definitely try this out if you think you can come up with something
though.

But I would like to hear from the device-link gurus :) because even if
we fix mtd with a "trick" like above, I guess we'll very likely find
other corner cases like that and I am interested in understanding the
rationale of what could be a proper fix.

Thanks,
Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Maxim Kiselev]

I looked closer at commit 658c4448bbbf and bcdf0315a61a, 5db1c2dbc04c16 commits.
Looks like we have two different features binded to one property - "compatible".

From one side it is the ability to forward the subnode of the mtd
partition to the nvmem subsystem (658c4448bbbf and ac42c46f983e).
And from another side is the ability to use custom initialization of
the mtd partition (bcdf0315a61a and 5db1c2dbc04c16).

What I mean:
According to ac42c46f983e I can create DT like this:
 - |
    partitions {
        compatible = "fixed-partitions";
        #address-cells = <1>;
        #size-cells = <1>;

        partition@0 {
            compatible = "nvmem-cells";
            reg = <0x40000 0x10000>;
            #address-cells = <1>;
            #size-cells = <1>;
            macaddr_gmac1: macaddr_gmac1@0 {
                reg = <0x0 0x6>;
            };
        };
    };


And according to 5db1c2dbc04c16 I can create DT like this:
 - |
    partitions {
        compatible = "fixed-partitions";
        #address-cells = <1>;
        #size-cells = <1>;

        partition@0 {
            compatible = "u-boot,env";
            reg = <0x40000 0x10000>;
        };
    };

But I can not use them both, because only one "compatible" property allowed.
This will be incorrect:
 - |
    partitions {
        compatible = "fixed-partitions";
        #address-cells = <1>;
        #size-cells = <1>;

        partition@0 {
            compatible = "u-boot,env";  # from ac42c46f983e
            compatible = "nvmem-cells"; # from 5db1c2dbc04c
            reg = <0x40000 0x10000>;
            #address-cells = <1>;
            #size-cells = <1>;
            macaddr_gmac1: macaddr_gmac1@0 {
                reg = <0x0 0x6>;
            };
        };
    };

> compatible: Duplicate property name

вт, 13 дек. 2022 г. в 12:46, Miquel Raynal <miquel.raynal@bootlin.com>:
>
> Hi Maxim,
>
> fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:
>
> > Hi, Miquel!
> >
> > On 12.12.2022 19:37, Miquel Raynal wrote:
> >
> > > Let me try to recap the situation for all the people I just involved:
> > >
> > > * An Ethernet driver gets its mac address from an nvmem cell. The
> > >    Ethernet controller DT node then has an "nvmem-cells" property
> > >    pointing towards an nvmem cell.
> > > * The nvmem cell comes from an mtd partition.
> > > * The mtd partition is flagged with a particular compatible
> > >    (which is also named "nvmem-cells") to tell the kernel that the node
> > >    produces nvmem cells.
> > > * The mtd partition itself has no driver, but is the child node of a
> > >    "partitions" container which has one (in this case,
> > >    "fixed-partitions", see the snippet below).
> > >
> > > Because the "nvmem-cells" property of the Ethernet node points at the
> > > nvmem-cell node, the core create a device link between the Ethernet
> > > controller (consumer) and the mtd partition (producer).
> > >
> > > The device link in this case will never be satisfied because no driver
> > > matches the "nvmem-cells" compatible of the partition node.
> > >
> > > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > > partitions") would IMHO not make much sense, the problem comes from the
> > > device link side and even there, there is nothing really "wrong",
> > > because I really expect the mtd device to be ready before the
> > > Ethernet controller probe, the device link is legitimate.
> > >
> > > So I would like to explore other alternatives. Here are a bunch of
> > > ideas, but I'm open:
> >
> > How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?
>
> This is probably worth the try but I doubt you can make it work without
> regressions because IIRC the nvmem registration happens no matter the
> compatible (not mentioning the user-otp and factory-otp cases). You can
> definitely try this out if you think you can come up with something
> though.
>
> But I would like to hear from the device-link gurus :) because even if
> we fix mtd with a "trick" like above, I guess we'll very likely find
> other corner cases like that and I am interested in understanding the
> rationale of what could be a proper fix.
>
> Thanks,
> Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Miquel Raynal]

Hi Maxim,

bigunclemax@gmail.com wrote on Tue, 13 Dec 2022 14:02:34 +0300:

> I looked closer at commit 658c4448bbbf and bcdf0315a61a, 5db1c2dbc04c16 commits.
> Looks like we have two different features binded to one property - "compatible".
> 
> From one side it is the ability to forward the subnode of the mtd
> partition to the nvmem subsystem (658c4448bbbf and ac42c46f983e).
> And from another side is the ability to use custom initialization of
> the mtd partition (bcdf0315a61a and 5db1c2dbc04c16).
> 
> What I mean:
> According to ac42c46f983e I can create DT like this:
>  - |
>     partitions {
>         compatible = "fixed-partitions";
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         partition@0 {
>             compatible = "nvmem-cells";
>             reg = <0x40000 0x10000>;
>             #address-cells = <1>;
>             #size-cells = <1>;
>             macaddr_gmac1: macaddr_gmac1@0 {
>                 reg = <0x0 0x6>;
>             };
>         };
>     };
> 
> 
> And according to 5db1c2dbc04c16 I can create DT like this:
>  - |
>     partitions {
>         compatible = "fixed-partitions";
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         partition@0 {
>             compatible = "u-boot,env";
>             reg = <0x40000 0x10000>;
>         };
>     };
> 
> But I can not use them both, because only one "compatible" property allowed.
> This will be incorrect:
>  - |
>     partitions {
>         compatible = "fixed-partitions";
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         partition@0 {
>             compatible = "u-boot,env";  # from ac42c46f983e
>             compatible = "nvmem-cells"; # from 5db1c2dbc04c

What about:

	      compatible = "u-boot,env", "nvmem-cells";

instead? that should actually work.

>             reg = <0x40000 0x10000>;
>             #address-cells = <1>;
>             #size-cells = <1>;
>             macaddr_gmac1: macaddr_gmac1@0 {
>                 reg = <0x0 0x6>;
>             };
>         };
>     };
> 
> > compatible: Duplicate property name  
> 
> вт, 13 дек. 2022 г. в 12:46, Miquel Raynal <miquel.raynal@bootlin.com>:
> >
> > Hi Maxim,
> >
> > fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:
> >  
> > > Hi, Miquel!
> > >
> > > On 12.12.2022 19:37, Miquel Raynal wrote:
> > >  
> > > > Let me try to recap the situation for all the people I just involved:
> > > >
> > > > * An Ethernet driver gets its mac address from an nvmem cell. The
> > > >    Ethernet controller DT node then has an "nvmem-cells" property
> > > >    pointing towards an nvmem cell.
> > > > * The nvmem cell comes from an mtd partition.
> > > > * The mtd partition is flagged with a particular compatible
> > > >    (which is also named "nvmem-cells") to tell the kernel that the node
> > > >    produces nvmem cells.
> > > > * The mtd partition itself has no driver, but is the child node of a
> > > >    "partitions" container which has one (in this case,
> > > >    "fixed-partitions", see the snippet below).
> > > >
> > > > Because the "nvmem-cells" property of the Ethernet node points at the
> > > > nvmem-cell node, the core create a device link between the Ethernet
> > > > controller (consumer) and the mtd partition (producer).
> > > >
> > > > The device link in this case will never be satisfied because no driver
> > > > matches the "nvmem-cells" compatible of the partition node.
> > > >
> > > > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > > > partitions") would IMHO not make much sense, the problem comes from the
> > > > device link side and even there, there is nothing really "wrong",
> > > > because I really expect the mtd device to be ready before the
> > > > Ethernet controller probe, the device link is legitimate.
> > > >
> > > > So I would like to explore other alternatives. Here are a bunch of
> > > > ideas, but I'm open:  
> > >
> > > How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?  
> >
> > This is probably worth the try but I doubt you can make it work without
> > regressions because IIRC the nvmem registration happens no matter the
> > compatible (not mentioning the user-otp and factory-otp cases). You can
> > definitely try this out if you think you can come up with something
> > though.
> >
> > But I would like to hear from the device-link gurus :) because even if
> > we fix mtd with a "trick" like above, I guess we'll very likely find
> > other corner cases like that and I am interested in understanding the
> > rationale of what could be a proper fix.
> >
> > Thanks,
> > Miquèl  


Thanks,
Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Saravana Kannan]

On Tue, Dec 13, 2022 at 8:54 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Maxim,
>
> bigunclemax@gmail.com wrote on Tue, 13 Dec 2022 14:02:34 +0300:
>
> > I looked closer at commit 658c4448bbbf and bcdf0315a61a, 5db1c2dbc04c16 commits.
> > Looks like we have two different features binded to one property - "compatible".
> >
> > From one side it is the ability to forward the subnode of the mtd
> > partition to the nvmem subsystem (658c4448bbbf and ac42c46f983e).
> > And from another side is the ability to use custom initialization of
> > the mtd partition (bcdf0315a61a and 5db1c2dbc04c16).
> >
> > What I mean:
> > According to ac42c46f983e I can create DT like this:
> >  - |
> >     partitions {
> >         compatible = "fixed-partitions";
> >         #address-cells = <1>;
> >         #size-cells = <1>;
> >
> >         partition@0 {
> >             compatible = "nvmem-cells";
> >             reg = <0x40000 0x10000>;
> >             #address-cells = <1>;
> >             #size-cells = <1>;
> >             macaddr_gmac1: macaddr_gmac1@0 {
> >                 reg = <0x0 0x6>;
> >             };
> >         };
> >     };
> >
> >
> > And according to 5db1c2dbc04c16 I can create DT like this:
> >  - |
> >     partitions {
> >         compatible = "fixed-partitions";
> >         #address-cells = <1>;
> >         #size-cells = <1>;
> >
> >         partition@0 {
> >             compatible = "u-boot,env";
> >             reg = <0x40000 0x10000>;
> >         };
> >     };
> >
> > But I can not use them both, because only one "compatible" property allowed.
> > This will be incorrect:
> >  - |
> >     partitions {
> >         compatible = "fixed-partitions";
> >         #address-cells = <1>;
> >         #size-cells = <1>;
> >
> >         partition@0 {
> >             compatible = "u-boot,env";  # from ac42c46f983e
> >             compatible = "nvmem-cells"; # from 5db1c2dbc04c
>
> What about:
>
>               compatible = "u-boot,env", "nvmem-cells";
>
> instead? that should actually work.
>
> >             reg = <0x40000 0x10000>;
> >             #address-cells = <1>;
> >             #size-cells = <1>;
> >             macaddr_gmac1: macaddr_gmac1@0 {
> >                 reg = <0x0 0x6>;
> >             };
> >         };
> >     };
> >
> > > compatible: Duplicate property name
> >
> > вт, 13 дек. 2022 г. в 12:46, Miquel Raynal <miquel.raynal@bootlin.com>:
> > >
> > > Hi Maxim,
> > >
> > > fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:
> > >
> > > > Hi, Miquel!
> > > >
> > > > On 12.12.2022 19:37, Miquel Raynal wrote:
> > > >
> > > > > Let me try to recap the situation for all the people I just involved:
> > > > >
> > > > > * An Ethernet driver gets its mac address from an nvmem cell. The
> > > > >    Ethernet controller DT node then has an "nvmem-cells" property
> > > > >    pointing towards an nvmem cell.
> > > > > * The nvmem cell comes from an mtd partition.
> > > > > * The mtd partition is flagged with a particular compatible
> > > > >    (which is also named "nvmem-cells") to tell the kernel that the node
> > > > >    produces nvmem cells.
> > > > > * The mtd partition itself has no driver, but is the child node of a
> > > > >    "partitions" container which has one (in this case,
> > > > >    "fixed-partitions", see the snippet below).
> > > > >
> > > > > Because the "nvmem-cells" property of the Ethernet node points at the
> > > > > nvmem-cell node, the core create a device link between the Ethernet
> > > > > controller (consumer) and the mtd partition (producer).
> > > > >
> > > > > The device link in this case will never be satisfied because no driver
> > > > > matches the "nvmem-cells" compatible of the partition node.
> > > > >
> > > > > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > > > > partitions") would IMHO not make much sense, the problem comes from the
> > > > > device link side and even there, there is nothing really "wrong",
> > > > > because I really expect the mtd device to be ready before the
> > > > > Ethernet controller probe, the device link is legitimate.
> > > > >
> > > > > So I would like to explore other alternatives. Here are a bunch of
> > > > > ideas, but I'm open:
> > > >
> > > > How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?
> > >
> > > This is probably worth the try but I doubt you can make it work without
> > > regressions because IIRC the nvmem registration happens no matter the
> > > compatible (not mentioning the user-otp and factory-otp cases). You can
> > > definitely try this out if you think you can come up with something
> > > though.
> > >
> > > But I would like to hear from the device-link gurus :) because even if
> > > we fix mtd with a "trick" like above, I guess we'll very likely find
> > > other corner cases like that and I am interested in understanding the
> > > rationale of what could be a proper fix.
> > >

Responding to the whole thread.

I'm going by Miquel's first email in which he cc'ed me and haven't
actually looked at the mtd code. Couple of comments:

Independent of mtd/nvmem-cell, I generally frown on having a
compatible string for a child node that you don't treat as a device.
Even more so if you actually create a struct device for it and then
don't do anything else with it. That's just a waste of memory. So, in
general try to avoid that in the future if you can.

Also, there are flags the parent device's driver can set that'll tell
fw_devlink not to treat a specific DT node as a real device. So, if we
really need that I'll dig up and suggest a fix.

Lastly and more importantly, I've a series[1] that stops depending on
the compatible property for fw_devlink to work. So it should be
smarter than it is today. But that series has known bugs for which I
gave test fixes in that thread. I plan to make a v2 of that series
with that fix and I'm expecting it'll fix a bunch of fw_devlink
issues.

Feel free to give v1 + squashing the fixes a shot if you are excited
to try it. Otherwise, I'll try my best to get around to it this week
(kinda swamped though + holidays coming up, so no promises).

[1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/

Thanks,
Saravana

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Miquel Raynal]

Hi Saravana, Maxim, Maxim,

saravanak@google.com wrote on Wed, 14 Dec 2022 13:53:54 -0800:

> On Tue, Dec 13, 2022 at 8:54 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Maxim,
> >
> > bigunclemax@gmail.com wrote on Tue, 13 Dec 2022 14:02:34 +0300:
> >  
> > > I looked closer at commit 658c4448bbbf and bcdf0315a61a, 5db1c2dbc04c16 commits.
> > > Looks like we have two different features binded to one property - "compatible".
> > >
> > > From one side it is the ability to forward the subnode of the mtd
> > > partition to the nvmem subsystem (658c4448bbbf and ac42c46f983e).
> > > And from another side is the ability to use custom initialization of
> > > the mtd partition (bcdf0315a61a and 5db1c2dbc04c16).
> > >
> > > What I mean:
> > > According to ac42c46f983e I can create DT like this:
> > >  - |
> > >     partitions {
> > >         compatible = "fixed-partitions";
> > >         #address-cells = <1>;
> > >         #size-cells = <1>;
> > >
> > >         partition@0 {
> > >             compatible = "nvmem-cells";
> > >             reg = <0x40000 0x10000>;
> > >             #address-cells = <1>;
> > >             #size-cells = <1>;
> > >             macaddr_gmac1: macaddr_gmac1@0 {
> > >                 reg = <0x0 0x6>;
> > >             };
> > >         };
> > >     };
> > >
> > >
> > > And according to 5db1c2dbc04c16 I can create DT like this:
> > >  - |
> > >     partitions {
> > >         compatible = "fixed-partitions";
> > >         #address-cells = <1>;
> > >         #size-cells = <1>;
> > >
> > >         partition@0 {
> > >             compatible = "u-boot,env";
> > >             reg = <0x40000 0x10000>;
> > >         };
> > >     };
> > >
> > > But I can not use them both, because only one "compatible" property allowed.
> > > This will be incorrect:
> > >  - |
> > >     partitions {
> > >         compatible = "fixed-partitions";
> > >         #address-cells = <1>;
> > >         #size-cells = <1>;
> > >
> > >         partition@0 {
> > >             compatible = "u-boot,env";  # from ac42c46f983e
> > >             compatible = "nvmem-cells"; # from 5db1c2dbc04c  
> >
> > What about:
> >
> >               compatible = "u-boot,env", "nvmem-cells";
> >
> > instead? that should actually work.
> >  
> > >             reg = <0x40000 0x10000>;
> > >             #address-cells = <1>;
> > >             #size-cells = <1>;
> > >             macaddr_gmac1: macaddr_gmac1@0 {
> > >                 reg = <0x0 0x6>;
> > >             };
> > >         };
> > >     };
> > >  
> > > > compatible: Duplicate property name  
> > >
> > > вт, 13 дек. 2022 г. в 12:46, Miquel Raynal <miquel.raynal@bootlin.com>:  
> > > >
> > > > Hi Maxim,
> > > >
> > > > fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:
> > > >  
> > > > > Hi, Miquel!
> > > > >
> > > > > On 12.12.2022 19:37, Miquel Raynal wrote:
> > > > >  
> > > > > > Let me try to recap the situation for all the people I just involved:
> > > > > >
> > > > > > * An Ethernet driver gets its mac address from an nvmem cell. The
> > > > > >    Ethernet controller DT node then has an "nvmem-cells" property
> > > > > >    pointing towards an nvmem cell.
> > > > > > * The nvmem cell comes from an mtd partition.
> > > > > > * The mtd partition is flagged with a particular compatible
> > > > > >    (which is also named "nvmem-cells") to tell the kernel that the node
> > > > > >    produces nvmem cells.
> > > > > > * The mtd partition itself has no driver, but is the child node of a
> > > > > >    "partitions" container which has one (in this case,
> > > > > >    "fixed-partitions", see the snippet below).
> > > > > >
> > > > > > Because the "nvmem-cells" property of the Ethernet node points at the
> > > > > > nvmem-cell node, the core create a device link between the Ethernet
> > > > > > controller (consumer) and the mtd partition (producer).
> > > > > >
> > > > > > The device link in this case will never be satisfied because no driver
> > > > > > matches the "nvmem-cells" compatible of the partition node.
> > > > > >
> > > > > > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > > > > > partitions") would IMHO not make much sense, the problem comes from the
> > > > > > device link side and even there, there is nothing really "wrong",
> > > > > > because I really expect the mtd device to be ready before the
> > > > > > Ethernet controller probe, the device link is legitimate.
> > > > > >
> > > > > > So I would like to explore other alternatives. Here are a bunch of
> > > > > > ideas, but I'm open:  
> > > > >
> > > > > How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?  
> > > >
> > > > This is probably worth the try but I doubt you can make it work without
> > > > regressions because IIRC the nvmem registration happens no matter the
> > > > compatible (not mentioning the user-otp and factory-otp cases). You can
> > > > definitely try this out if you think you can come up with something
> > > > though.
> > > >
> > > > But I would like to hear from the device-link gurus :) because even if
> > > > we fix mtd with a "trick" like above, I guess we'll very likely find
> > > > other corner cases like that and I am interested in understanding the
> > > > rationale of what could be a proper fix.
> > > >  
> 
> Responding to the whole thread.
> 
> I'm going by Miquel's first email in which he cc'ed me and haven't
> actually looked at the mtd code. Couple of comments:
> 
> Independent of mtd/nvmem-cell, I generally frown on having a
> compatible string for a child node that you don't treat as a device.
> Even more so if you actually create a struct device for it and then
> don't do anything else with it. That's just a waste of memory. So, in
> general try to avoid that in the future if you can.

Agreed, it didn't triggered any warnings in my head in the first place,
sorry about that.

> Also, there are flags the parent device's driver can set that'll tell
> fw_devlink not to treat a specific DT node as a real device. So, if we
> really need that I'll dig up and suggest a fix.

Interesting, that would indeed very likely fix it.

> Lastly and more importantly, I've a series[1] that stops depending on
> the compatible property for fw_devlink to work. So it should be
> smarter than it is today. But that series has known bugs for which I
> gave test fixes in that thread. I plan to make a v2 of that series
> with that fix and I'm expecting it'll fix a bunch of fw_devlink
> issues.
> 
> Feel free to give v1 + squashing the fixes a shot if you are excited
> to try it. Otherwise, I'll try my best to get around to it this week
> (kinda swamped though + holidays coming up, so no promises).

Can you please include us in your next submission?
* Maxim Kiselev <bigunclemax@gmail.com>
* Maxim Kochetkov <fido_max@inbox.ru>
* Miquel Raynal <miquel.raynal@bootlin.com>

> [1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/

Maxim, any chance you give this a try?

Thanks,
Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Maxim Kiselev]

> Maxim, any chance you give this a try?
Ok, I'll try it as soon as possible.

пт, 16 дек. 2022 г. в 14:04, Miquel Raynal <miquel.raynal@bootlin.com>:
>
> Hi Saravana, Maxim, Maxim,
>
> saravanak@google.com wrote on Wed, 14 Dec 2022 13:53:54 -0800:
>
> > On Tue, Dec 13, 2022 at 8:54 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Maxim,
> > >
> > > bigunclemax@gmail.com wrote on Tue, 13 Dec 2022 14:02:34 +0300:
> > >
> > > > I looked closer at commit 658c4448bbbf and bcdf0315a61a, 5db1c2dbc04c16 commits.
> > > > Looks like we have two different features binded to one property - "compatible".
> > > >
> > > > From one side it is the ability to forward the subnode of the mtd
> > > > partition to the nvmem subsystem (658c4448bbbf and ac42c46f983e).
> > > > And from another side is the ability to use custom initialization of
> > > > the mtd partition (bcdf0315a61a and 5db1c2dbc04c16).
> > > >
> > > > What I mean:
> > > > According to ac42c46f983e I can create DT like this:
> > > >  - |
> > > >     partitions {
> > > >         compatible = "fixed-partitions";
> > > >         #address-cells = <1>;
> > > >         #size-cells = <1>;
> > > >
> > > >         partition@0 {
> > > >             compatible = "nvmem-cells";
> > > >             reg = <0x40000 0x10000>;
> > > >             #address-cells = <1>;
> > > >             #size-cells = <1>;
> > > >             macaddr_gmac1: macaddr_gmac1@0 {
> > > >                 reg = <0x0 0x6>;
> > > >             };
> > > >         };
> > > >     };
> > > >
> > > >
> > > > And according to 5db1c2dbc04c16 I can create DT like this:
> > > >  - |
> > > >     partitions {
> > > >         compatible = "fixed-partitions";
> > > >         #address-cells = <1>;
> > > >         #size-cells = <1>;
> > > >
> > > >         partition@0 {
> > > >             compatible = "u-boot,env";
> > > >             reg = <0x40000 0x10000>;
> > > >         };
> > > >     };
> > > >
> > > > But I can not use them both, because only one "compatible" property allowed.
> > > > This will be incorrect:
> > > >  - |
> > > >     partitions {
> > > >         compatible = "fixed-partitions";
> > > >         #address-cells = <1>;
> > > >         #size-cells = <1>;
> > > >
> > > >         partition@0 {
> > > >             compatible = "u-boot,env";  # from ac42c46f983e
> > > >             compatible = "nvmem-cells"; # from 5db1c2dbc04c
> > >
> > > What about:
> > >
> > >               compatible = "u-boot,env", "nvmem-cells";
> > >
> > > instead? that should actually work.
> > >
> > > >             reg = <0x40000 0x10000>;
> > > >             #address-cells = <1>;
> > > >             #size-cells = <1>;
> > > >             macaddr_gmac1: macaddr_gmac1@0 {
> > > >                 reg = <0x0 0x6>;
> > > >             };
> > > >         };
> > > >     };
> > > >
> > > > > compatible: Duplicate property name
> > > >
> > > > вт, 13 дек. 2022 г. в 12:46, Miquel Raynal <miquel.raynal@bootlin.com>:
> > > > >
> > > > > Hi Maxim,
> > > > >
> > > > > fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:
> > > > >
> > > > > > Hi, Miquel!
> > > > > >
> > > > > > On 12.12.2022 19:37, Miquel Raynal wrote:
> > > > > >
> > > > > > > Let me try to recap the situation for all the people I just involved:
> > > > > > >
> > > > > > > * An Ethernet driver gets its mac address from an nvmem cell. The
> > > > > > >    Ethernet controller DT node then has an "nvmem-cells" property
> > > > > > >    pointing towards an nvmem cell.
> > > > > > > * The nvmem cell comes from an mtd partition.
> > > > > > > * The mtd partition is flagged with a particular compatible
> > > > > > >    (which is also named "nvmem-cells") to tell the kernel that the node
> > > > > > >    produces nvmem cells.
> > > > > > > * The mtd partition itself has no driver, but is the child node of a
> > > > > > >    "partitions" container which has one (in this case,
> > > > > > >    "fixed-partitions", see the snippet below).
> > > > > > >
> > > > > > > Because the "nvmem-cells" property of the Ethernet node points at the
> > > > > > > nvmem-cell node, the core create a device link between the Ethernet
> > > > > > > controller (consumer) and the mtd partition (producer).
> > > > > > >
> > > > > > > The device link in this case will never be satisfied because no driver
> > > > > > > matches the "nvmem-cells" compatible of the partition node.
> > > > > > >
> > > > > > > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > > > > > > partitions") would IMHO not make much sense, the problem comes from the
> > > > > > > device link side and even there, there is nothing really "wrong",
> > > > > > > because I really expect the mtd device to be ready before the
> > > > > > > Ethernet controller probe, the device link is legitimate.
> > > > > > >
> > > > > > > So I would like to explore other alternatives. Here are a bunch of
> > > > > > > ideas, but I'm open:
> > > > > >
> > > > > > How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?
> > > > >
> > > > > This is probably worth the try but I doubt you can make it work without
> > > > > regressions because IIRC the nvmem registration happens no matter the
> > > > > compatible (not mentioning the user-otp and factory-otp cases). You can
> > > > > definitely try this out if you think you can come up with something
> > > > > though.
> > > > >
> > > > > But I would like to hear from the device-link gurus :) because even if
> > > > > we fix mtd with a "trick" like above, I guess we'll very likely find
> > > > > other corner cases like that and I am interested in understanding the
> > > > > rationale of what could be a proper fix.
> > > > >
> >
> > Responding to the whole thread.
> >
> > I'm going by Miquel's first email in which he cc'ed me and haven't
> > actually looked at the mtd code. Couple of comments:
> >
> > Independent of mtd/nvmem-cell, I generally frown on having a
> > compatible string for a child node that you don't treat as a device.
> > Even more so if you actually create a struct device for it and then
> > don't do anything else with it. That's just a waste of memory. So, in
> > general try to avoid that in the future if you can.
>
> Agreed, it didn't triggered any warnings in my head in the first place,
> sorry about that.
>
> > Also, there are flags the parent device's driver can set that'll tell
> > fw_devlink not to treat a specific DT node as a real device. So, if we
> > really need that I'll dig up and suggest a fix.
>
> Interesting, that would indeed very likely fix it.
>
> > Lastly and more importantly, I've a series[1] that stops depending on
> > the compatible property for fw_devlink to work. So it should be
> > smarter than it is today. But that series has known bugs for which I
> > gave test fixes in that thread. I plan to make a v2 of that series
> > with that fix and I'm expecting it'll fix a bunch of fw_devlink
> > issues.
> >
> > Feel free to give v1 + squashing the fixes a shot if you are excited
> > to try it. Otherwise, I'll try my best to get around to it this week
> > (kinda swamped though + holidays coming up, so no promises).
>
> Can you please include us in your next submission?
> * Maxim Kiselev <bigunclemax@gmail.com>
> * Maxim Kochetkov <fido_max@inbox.ru>
> * Miquel Raynal <miquel.raynal@bootlin.com>
>
> > [1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/
>
> Maxim, any chance you give this a try?
>
> Thanks,
> Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Maxim Kiselev]

> Maxim, any chance you give this a try?
> [1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/

OK, I tried it and it didn't seem to help me.
I'm still getting an unresolved dependence for 'f1070000.ethernet'
from 'mac@6' nvmem-cell.
Here is a related part of kernel log:

[    0.035553] ethernet@70000 Linked as a fwnode consumer to
main-interrupt-ctrl@20200
[    0.035601] ethernet@70000 Linked as a fwnode consumer to
clock-gating-control@1821c
[    0.035713] ethernet@70000 Linked as a fwnode consumer to mac@6

[    0.046423] device: 'f1070000.ethernet': device_add
[    0.046689] platform f1070000.ethernet: Not linking
/ocp@f1000000/clock-gating-control@1821c - dev might never probe
[    0.046747] ethernet@70000 Dropping the fwnode link to
clock-gating-control@1821c
[    0.046788] platform f1070000.ethernet: Not linking
/ocp@f1000000/main-interrupt-ctrl@20200 - might never become dev
[    0.046835] ethernet@70000 Dropping the fwnode link to
main-interrupt-ctrl@20200

[    0.113759] device: 'spi0': device_add
[    0.114523] device: 'spi0.0': device_add
[    0.115349] spi-nor spi0.0: mx66l51235f (65536 Kbytes)
[    0.116833] 7 fixed-partitions partitions found on MTD device spi0.0
[    0.117026] device: 'f1010600.spi:m25p80@0:partitions:partition@1':
device_add
[    0.117416] Creating 7 MTD partitions on "spi0.0":

[    3.537102] devices_kset: Moving f1070000.ethernet to end of list
[    3.537391] platform f1070000.ethernet: error -EPROBE_DEFER: wait
for supplier mac@6

пт, 16 дек. 2022 г. в 14:33, Maxim Kiselev <bigunclemax@gmail.com>:
>
> > Maxim, any chance you give this a try?
> Ok, I'll try it as soon as possible.
>
> пт, 16 дек. 2022 г. в 14:04, Miquel Raynal <miquel.raynal@bootlin.com>:
> >
> > Hi Saravana, Maxim, Maxim,
> >
> > saravanak@google.com wrote on Wed, 14 Dec 2022 13:53:54 -0800:
> >
> > > On Tue, Dec 13, 2022 at 8:54 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > Hi Maxim,
> > > >
> > > > bigunclemax@gmail.com wrote on Tue, 13 Dec 2022 14:02:34 +0300:
> > > >
> > > > > I looked closer at commit 658c4448bbbf and bcdf0315a61a, 5db1c2dbc04c16 commits.
> > > > > Looks like we have two different features binded to one property - "compatible".
> > > > >
> > > > > From one side it is the ability to forward the subnode of the mtd
> > > > > partition to the nvmem subsystem (658c4448bbbf and ac42c46f983e).
> > > > > And from another side is the ability to use custom initialization of
> > > > > the mtd partition (bcdf0315a61a and 5db1c2dbc04c16).
> > > > >
> > > > > What I mean:
> > > > > According to ac42c46f983e I can create DT like this:
> > > > >  - |
> > > > >     partitions {
> > > > >         compatible = "fixed-partitions";
> > > > >         #address-cells = <1>;
> > > > >         #size-cells = <1>;
> > > > >
> > > > >         partition@0 {
> > > > >             compatible = "nvmem-cells";
> > > > >             reg = <0x40000 0x10000>;
> > > > >             #address-cells = <1>;
> > > > >             #size-cells = <1>;
> > > > >             macaddr_gmac1: macaddr_gmac1@0 {
> > > > >                 reg = <0x0 0x6>;
> > > > >             };
> > > > >         };
> > > > >     };
> > > > >
> > > > >
> > > > > And according to 5db1c2dbc04c16 I can create DT like this:
> > > > >  - |
> > > > >     partitions {
> > > > >         compatible = "fixed-partitions";
> > > > >         #address-cells = <1>;
> > > > >         #size-cells = <1>;
> > > > >
> > > > >         partition@0 {
> > > > >             compatible = "u-boot,env";
> > > > >             reg = <0x40000 0x10000>;
> > > > >         };
> > > > >     };
> > > > >
> > > > > But I can not use them both, because only one "compatible" property allowed.
> > > > > This will be incorrect:
> > > > >  - |
> > > > >     partitions {
> > > > >         compatible = "fixed-partitions";
> > > > >         #address-cells = <1>;
> > > > >         #size-cells = <1>;
> > > > >
> > > > >         partition@0 {
> > > > >             compatible = "u-boot,env";  # from ac42c46f983e
> > > > >             compatible = "nvmem-cells"; # from 5db1c2dbc04c
> > > >
> > > > What about:
> > > >
> > > >               compatible = "u-boot,env", "nvmem-cells";
> > > >
> > > > instead? that should actually work.
> > > >
> > > > >             reg = <0x40000 0x10000>;
> > > > >             #address-cells = <1>;
> > > > >             #size-cells = <1>;
> > > > >             macaddr_gmac1: macaddr_gmac1@0 {
> > > > >                 reg = <0x0 0x6>;
> > > > >             };
> > > > >         };
> > > > >     };
> > > > >
> > > > > > compatible: Duplicate property name
> > > > >
> > > > > вт, 13 дек. 2022 г. в 12:46, Miquel Raynal <miquel.raynal@bootlin.com>:
> > > > > >
> > > > > > Hi Maxim,
> > > > > >
> > > > > > fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:
> > > > > >
> > > > > > > Hi, Miquel!
> > > > > > >
> > > > > > > On 12.12.2022 19:37, Miquel Raynal wrote:
> > > > > > >
> > > > > > > > Let me try to recap the situation for all the people I just involved:
> > > > > > > >
> > > > > > > > * An Ethernet driver gets its mac address from an nvmem cell. The
> > > > > > > >    Ethernet controller DT node then has an "nvmem-cells" property
> > > > > > > >    pointing towards an nvmem cell.
> > > > > > > > * The nvmem cell comes from an mtd partition.
> > > > > > > > * The mtd partition is flagged with a particular compatible
> > > > > > > >    (which is also named "nvmem-cells") to tell the kernel that the node
> > > > > > > >    produces nvmem cells.
> > > > > > > > * The mtd partition itself has no driver, but is the child node of a
> > > > > > > >    "partitions" container which has one (in this case,
> > > > > > > >    "fixed-partitions", see the snippet below).
> > > > > > > >
> > > > > > > > Because the "nvmem-cells" property of the Ethernet node points at the
> > > > > > > > nvmem-cell node, the core create a device link between the Ethernet
> > > > > > > > controller (consumer) and the mtd partition (producer).
> > > > > > > >
> > > > > > > > The device link in this case will never be satisfied because no driver
> > > > > > > > matches the "nvmem-cells" compatible of the partition node.
> > > > > > > >
> > > > > > > > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > > > > > > > partitions") would IMHO not make much sense, the problem comes from the
> > > > > > > > device link side and even there, there is nothing really "wrong",
> > > > > > > > because I really expect the mtd device to be ready before the
> > > > > > > > Ethernet controller probe, the device link is legitimate.
> > > > > > > >
> > > > > > > > So I would like to explore other alternatives. Here are a bunch of
> > > > > > > > ideas, but I'm open:
> > > > > > >
> > > > > > > How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?
> > > > > >
> > > > > > This is probably worth the try but I doubt you can make it work without
> > > > > > regressions because IIRC the nvmem registration happens no matter the
> > > > > > compatible (not mentioning the user-otp and factory-otp cases). You can
> > > > > > definitely try this out if you think you can come up with something
> > > > > > though.
> > > > > >
> > > > > > But I would like to hear from the device-link gurus :) because even if
> > > > > > we fix mtd with a "trick" like above, I guess we'll very likely find
> > > > > > other corner cases like that and I am interested in understanding the
> > > > > > rationale of what could be a proper fix.
> > > > > >
> > >
> > > Responding to the whole thread.
> > >
> > > I'm going by Miquel's first email in which he cc'ed me and haven't
> > > actually looked at the mtd code. Couple of comments:
> > >
> > > Independent of mtd/nvmem-cell, I generally frown on having a
> > > compatible string for a child node that you don't treat as a device.
> > > Even more so if you actually create a struct device for it and then
> > > don't do anything else with it. That's just a waste of memory. So, in
> > > general try to avoid that in the future if you can.
> >
> > Agreed, it didn't triggered any warnings in my head in the first place,
> > sorry about that.
> >
> > > Also, there are flags the parent device's driver can set that'll tell
> > > fw_devlink not to treat a specific DT node as a real device. So, if we
> > > really need that I'll dig up and suggest a fix.
> >
> > Interesting, that would indeed very likely fix it.
> >
> > > Lastly and more importantly, I've a series[1] that stops depending on
> > > the compatible property for fw_devlink to work. So it should be
> > > smarter than it is today. But that series has known bugs for which I
> > > gave test fixes in that thread. I plan to make a v2 of that series
> > > with that fix and I'm expecting it'll fix a bunch of fw_devlink
> > > issues.
> > >
> > > Feel free to give v1 + squashing the fixes a shot if you are excited
> > > to try it. Otherwise, I'll try my best to get around to it this week
> > > (kinda swamped though + holidays coming up, so no promises).
> >
> > Can you please include us in your next submission?
> > * Maxim Kiselev <bigunclemax@gmail.com>
> > * Maxim Kochetkov <fido_max@inbox.ru>
> > * Miquel Raynal <miquel.raynal@bootlin.com>
> >
> > > [1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/
> >
> > Maxim, any chance you give this a try?
> >
> > Thanks,
> > Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Saravana Kannan]

On Fri, Dec 16, 2022 at 5:13 AM Maxim Kiselev <bigunclemax@gmail.com> wrote:
>
> > Maxim, any chance you give this a try?
> > [1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/
>
> OK, I tried it and it didn't seem to help me.

Did you include the fixes to the v1 series I gave as replies to some
of the emails in that thread?

-Saravana

> I'm still getting an unresolved dependence for 'f1070000.ethernet'
> from 'mac@6' nvmem-cell.
> Here is a related part of kernel log:
>
> [    0.035553] ethernet@70000 Linked as a fwnode consumer to
> main-interrupt-ctrl@20200
> [    0.035601] ethernet@70000 Linked as a fwnode consumer to
> clock-gating-control@1821c
> [    0.035713] ethernet@70000 Linked as a fwnode consumer to mac@6
>
> [    0.046423] device: 'f1070000.ethernet': device_add
> [    0.046689] platform f1070000.ethernet: Not linking
> /ocp@f1000000/clock-gating-control@1821c - dev might never probe
> [    0.046747] ethernet@70000 Dropping the fwnode link to
> clock-gating-control@1821c
> [    0.046788] platform f1070000.ethernet: Not linking
> /ocp@f1000000/main-interrupt-ctrl@20200 - might never become dev
> [    0.046835] ethernet@70000 Dropping the fwnode link to
> main-interrupt-ctrl@20200
>
> [    0.113759] device: 'spi0': device_add
> [    0.114523] device: 'spi0.0': device_add
> [    0.115349] spi-nor spi0.0: mx66l51235f (65536 Kbytes)
> [    0.116833] 7 fixed-partitions partitions found on MTD device spi0.0
> [    0.117026] device: 'f1010600.spi:m25p80@0:partitions:partition@1':
> device_add
> [    0.117416] Creating 7 MTD partitions on "spi0.0":
>
> [    3.537102] devices_kset: Moving f1070000.ethernet to end of list
> [    3.537391] platform f1070000.ethernet: error -EPROBE_DEFER: wait
> for supplier mac@6
>
> пт, 16 дек. 2022 г. в 14:33, Maxim Kiselev <bigunclemax@gmail.com>:
> >
> > > Maxim, any chance you give this a try?
> > Ok, I'll try it as soon as possible.
> >
> > пт, 16 дек. 2022 г. в 14:04, Miquel Raynal <miquel.raynal@bootlin.com>:
> > >
> > > Hi Saravana, Maxim, Maxim,
> > >
> > > saravanak@google.com wrote on Wed, 14 Dec 2022 13:53:54 -0800:
> > >
> > > > On Tue, Dec 13, 2022 at 8:54 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > >
> > > > > Hi Maxim,
> > > > >
> > > > > bigunclemax@gmail.com wrote on Tue, 13 Dec 2022 14:02:34 +0300:
> > > > >
> > > > > > I looked closer at commit 658c4448bbbf and bcdf0315a61a, 5db1c2dbc04c16 commits.
> > > > > > Looks like we have two different features binded to one property - "compatible".
> > > > > >
> > > > > > From one side it is the ability to forward the subnode of the mtd
> > > > > > partition to the nvmem subsystem (658c4448bbbf and ac42c46f983e).
> > > > > > And from another side is the ability to use custom initialization of
> > > > > > the mtd partition (bcdf0315a61a and 5db1c2dbc04c16).
> > > > > >
> > > > > > What I mean:
> > > > > > According to ac42c46f983e I can create DT like this:
> > > > > >  - |
> > > > > >     partitions {
> > > > > >         compatible = "fixed-partitions";
> > > > > >         #address-cells = <1>;
> > > > > >         #size-cells = <1>;
> > > > > >
> > > > > >         partition@0 {
> > > > > >             compatible = "nvmem-cells";
> > > > > >             reg = <0x40000 0x10000>;
> > > > > >             #address-cells = <1>;
> > > > > >             #size-cells = <1>;
> > > > > >             macaddr_gmac1: macaddr_gmac1@0 {
> > > > > >                 reg = <0x0 0x6>;
> > > > > >             };
> > > > > >         };
> > > > > >     };
> > > > > >
> > > > > >
> > > > > > And according to 5db1c2dbc04c16 I can create DT like this:
> > > > > >  - |
> > > > > >     partitions {
> > > > > >         compatible = "fixed-partitions";
> > > > > >         #address-cells = <1>;
> > > > > >         #size-cells = <1>;
> > > > > >
> > > > > >         partition@0 {
> > > > > >             compatible = "u-boot,env";
> > > > > >             reg = <0x40000 0x10000>;
> > > > > >         };
> > > > > >     };
> > > > > >
> > > > > > But I can not use them both, because only one "compatible" property allowed.
> > > > > > This will be incorrect:
> > > > > >  - |
> > > > > >     partitions {
> > > > > >         compatible = "fixed-partitions";
> > > > > >         #address-cells = <1>;
> > > > > >         #size-cells = <1>;
> > > > > >
> > > > > >         partition@0 {
> > > > > >             compatible = "u-boot,env";  # from ac42c46f983e
> > > > > >             compatible = "nvmem-cells"; # from 5db1c2dbc04c
> > > > >
> > > > > What about:
> > > > >
> > > > >               compatible = "u-boot,env", "nvmem-cells";
> > > > >
> > > > > instead? that should actually work.
> > > > >
> > > > > >             reg = <0x40000 0x10000>;
> > > > > >             #address-cells = <1>;
> > > > > >             #size-cells = <1>;
> > > > > >             macaddr_gmac1: macaddr_gmac1@0 {
> > > > > >                 reg = <0x0 0x6>;
> > > > > >             };
> > > > > >         };
> > > > > >     };
> > > > > >
> > > > > > > compatible: Duplicate property name
> > > > > >
> > > > > > вт, 13 дек. 2022 г. в 12:46, Miquel Raynal <miquel.raynal@bootlin.com>:
> > > > > > >
> > > > > > > Hi Maxim,
> > > > > > >
> > > > > > > fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:
> > > > > > >
> > > > > > > > Hi, Miquel!
> > > > > > > >
> > > > > > > > On 12.12.2022 19:37, Miquel Raynal wrote:
> > > > > > > >
> > > > > > > > > Let me try to recap the situation for all the people I just involved:
> > > > > > > > >
> > > > > > > > > * An Ethernet driver gets its mac address from an nvmem cell. The
> > > > > > > > >    Ethernet controller DT node then has an "nvmem-cells" property
> > > > > > > > >    pointing towards an nvmem cell.
> > > > > > > > > * The nvmem cell comes from an mtd partition.
> > > > > > > > > * The mtd partition is flagged with a particular compatible
> > > > > > > > >    (which is also named "nvmem-cells") to tell the kernel that the node
> > > > > > > > >    produces nvmem cells.
> > > > > > > > > * The mtd partition itself has no driver, but is the child node of a
> > > > > > > > >    "partitions" container which has one (in this case,
> > > > > > > > >    "fixed-partitions", see the snippet below).
> > > > > > > > >
> > > > > > > > > Because the "nvmem-cells" property of the Ethernet node points at the
> > > > > > > > > nvmem-cell node, the core create a device link between the Ethernet
> > > > > > > > > controller (consumer) and the mtd partition (producer).
> > > > > > > > >
> > > > > > > > > The device link in this case will never be satisfied because no driver
> > > > > > > > > matches the "nvmem-cells" compatible of the partition node.
> > > > > > > > >
> > > > > > > > > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > > > > > > > > partitions") would IMHO not make much sense, the problem comes from the
> > > > > > > > > device link side and even there, there is nothing really "wrong",
> > > > > > > > > because I really expect the mtd device to be ready before the
> > > > > > > > > Ethernet controller probe, the device link is legitimate.
> > > > > > > > >
> > > > > > > > > So I would like to explore other alternatives. Here are a bunch of
> > > > > > > > > ideas, but I'm open:
> > > > > > > >
> > > > > > > > How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?
> > > > > > >
> > > > > > > This is probably worth the try but I doubt you can make it work without
> > > > > > > regressions because IIRC the nvmem registration happens no matter the
> > > > > > > compatible (not mentioning the user-otp and factory-otp cases). You can
> > > > > > > definitely try this out if you think you can come up with something
> > > > > > > though.
> > > > > > >
> > > > > > > But I would like to hear from the device-link gurus :) because even if
> > > > > > > we fix mtd with a "trick" like above, I guess we'll very likely find
> > > > > > > other corner cases like that and I am interested in understanding the
> > > > > > > rationale of what could be a proper fix.
> > > > > > >
> > > >
> > > > Responding to the whole thread.
> > > >
> > > > I'm going by Miquel's first email in which he cc'ed me and haven't
> > > > actually looked at the mtd code. Couple of comments:
> > > >
> > > > Independent of mtd/nvmem-cell, I generally frown on having a
> > > > compatible string for a child node that you don't treat as a device.
> > > > Even more so if you actually create a struct device for it and then
> > > > don't do anything else with it. That's just a waste of memory. So, in
> > > > general try to avoid that in the future if you can.
> > >
> > > Agreed, it didn't triggered any warnings in my head in the first place,
> > > sorry about that.
> > >
> > > > Also, there are flags the parent device's driver can set that'll tell
> > > > fw_devlink not to treat a specific DT node as a real device. So, if we
> > > > really need that I'll dig up and suggest a fix.
> > >
> > > Interesting, that would indeed very likely fix it.
> > >
> > > > Lastly and more importantly, I've a series[1] that stops depending on
> > > > the compatible property for fw_devlink to work. So it should be
> > > > smarter than it is today. But that series has known bugs for which I
> > > > gave test fixes in that thread. I plan to make a v2 of that series
> > > > with that fix and I'm expecting it'll fix a bunch of fw_devlink
> > > > issues.
> > > >
> > > > Feel free to give v1 + squashing the fixes a shot if you are excited
> > > > to try it. Otherwise, I'll try my best to get around to it this week
> > > > (kinda swamped though + holidays coming up, so no promises).
> > >
> > > Can you please include us in your next submission?
> > > * Maxim Kiselev <bigunclemax@gmail.com>
> > > * Maxim Kochetkov <fido_max@inbox.ru>
> > > * Miquel Raynal <miquel.raynal@bootlin.com>
> > >
> > > > [1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/
> > >
> > > Maxim, any chance you give this a try?
> > >
> > > Thanks,
> > > Miquèl

Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Saravana Kannan]

On Fri, Dec 16, 2022 at 3:05 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Saravana, Maxim, Maxim,
>
> saravanak@google.com wrote on Wed, 14 Dec 2022 13:53:54 -0800:
>
> > On Tue, Dec 13, 2022 at 8:54 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Maxim,
> > >
> > > bigunclemax@gmail.com wrote on Tue, 13 Dec 2022 14:02:34 +0300:
> > >
> > > > I looked closer at commit 658c4448bbbf and bcdf0315a61a, 5db1c2dbc04c16 commits.
> > > > Looks like we have two different features binded to one property - "compatible".
> > > >
> > > > From one side it is the ability to forward the subnode of the mtd
> > > > partition to the nvmem subsystem (658c4448bbbf and ac42c46f983e).
> > > > And from another side is the ability to use custom initialization of
> > > > the mtd partition (bcdf0315a61a and 5db1c2dbc04c16).
> > > >
> > > > What I mean:
> > > > According to ac42c46f983e I can create DT like this:
> > > >  - |
> > > >     partitions {
> > > >         compatible = "fixed-partitions";
> > > >         #address-cells = <1>;
> > > >         #size-cells = <1>;
> > > >
> > > >         partition@0 {
> > > >             compatible = "nvmem-cells";
> > > >             reg = <0x40000 0x10000>;
> > > >             #address-cells = <1>;
> > > >             #size-cells = <1>;
> > > >             macaddr_gmac1: macaddr_gmac1@0 {
> > > >                 reg = <0x0 0x6>;
> > > >             };
> > > >         };
> > > >     };
> > > >
> > > >
> > > > And according to 5db1c2dbc04c16 I can create DT like this:
> > > >  - |
> > > >     partitions {
> > > >         compatible = "fixed-partitions";
> > > >         #address-cells = <1>;
> > > >         #size-cells = <1>;
> > > >
> > > >         partition@0 {
> > > >             compatible = "u-boot,env";
> > > >             reg = <0x40000 0x10000>;
> > > >         };
> > > >     };
> > > >
> > > > But I can not use them both, because only one "compatible" property allowed.
> > > > This will be incorrect:
> > > >  - |
> > > >     partitions {
> > > >         compatible = "fixed-partitions";
> > > >         #address-cells = <1>;
> > > >         #size-cells = <1>;
> > > >
> > > >         partition@0 {
> > > >             compatible = "u-boot,env";  # from ac42c46f983e
> > > >             compatible = "nvmem-cells"; # from 5db1c2dbc04c
> > >
> > > What about:
> > >
> > >               compatible = "u-boot,env", "nvmem-cells";
> > >
> > > instead? that should actually work.
> > >
> > > >             reg = <0x40000 0x10000>;
> > > >             #address-cells = <1>;
> > > >             #size-cells = <1>;
> > > >             macaddr_gmac1: macaddr_gmac1@0 {
> > > >                 reg = <0x0 0x6>;
> > > >             };
> > > >         };
> > > >     };
> > > >
> > > > > compatible: Duplicate property name
> > > >
> > > > вт, 13 дек. 2022 г. в 12:46, Miquel Raynal <miquel.raynal@bootlin.com>:
> > > > >
> > > > > Hi Maxim,
> > > > >
> > > > > fido_max@inbox.ru wrote on Mon, 12 Dec 2022 20:57:49 +0300:
> > > > >
> > > > > > Hi, Miquel!
> > > > > >
> > > > > > On 12.12.2022 19:37, Miquel Raynal wrote:
> > > > > >
> > > > > > > Let me try to recap the situation for all the people I just involved:
> > > > > > >
> > > > > > > * An Ethernet driver gets its mac address from an nvmem cell. The
> > > > > > >    Ethernet controller DT node then has an "nvmem-cells" property
> > > > > > >    pointing towards an nvmem cell.
> > > > > > > * The nvmem cell comes from an mtd partition.
> > > > > > > * The mtd partition is flagged with a particular compatible
> > > > > > >    (which is also named "nvmem-cells") to tell the kernel that the node
> > > > > > >    produces nvmem cells.
> > > > > > > * The mtd partition itself has no driver, but is the child node of a
> > > > > > >    "partitions" container which has one (in this case,
> > > > > > >    "fixed-partitions", see the snippet below).
> > > > > > >
> > > > > > > Because the "nvmem-cells" property of the Ethernet node points at the
> > > > > > > nvmem-cell node, the core create a device link between the Ethernet
> > > > > > > controller (consumer) and the mtd partition (producer).
> > > > > > >
> > > > > > > The device link in this case will never be satisfied because no driver
> > > > > > > matches the "nvmem-cells" compatible of the partition node.
> > > > > > >
> > > > > > > Reverting commit bcdf0315a61a ("mtd: call of_platform_populate() for MTD
> > > > > > > partitions") would IMHO not make much sense, the problem comes from the
> > > > > > > device link side and even there, there is nothing really "wrong",
> > > > > > > because I really expect the mtd device to be ready before the
> > > > > > > Ethernet controller probe, the device link is legitimate.
> > > > > > >
> > > > > > > So I would like to explore other alternatives. Here are a bunch of
> > > > > > > ideas, but I'm open:
> > > > > >
> > > > > > How about to create simple driver with compatible="nvmem-cell" and to move all the suff from main mtd driver which serves nvmem-cell to the probe function?
> > > > >
> > > > > This is probably worth the try but I doubt you can make it work without
> > > > > regressions because IIRC the nvmem registration happens no matter the
> > > > > compatible (not mentioning the user-otp and factory-otp cases). You can
> > > > > definitely try this out if you think you can come up with something
> > > > > though.
> > > > >
> > > > > But I would like to hear from the device-link gurus :) because even if
> > > > > we fix mtd with a "trick" like above, I guess we'll very likely find
> > > > > other corner cases like that and I am interested in understanding the
> > > > > rationale of what could be a proper fix.
> > > > >
> >
> > Responding to the whole thread.
> >
> > I'm going by Miquel's first email in which he cc'ed me and haven't
> > actually looked at the mtd code. Couple of comments:
> >
> > Independent of mtd/nvmem-cell, I generally frown on having a
> > compatible string for a child node that you don't treat as a device.
> > Even more so if you actually create a struct device for it and then
> > don't do anything else with it. That's just a waste of memory. So, in
> > general try to avoid that in the future if you can.
>
> Agreed, it didn't triggered any warnings in my head in the first place,
> sorry about that.
>
> > Also, there are flags the parent device's driver can set that'll tell
> > fw_devlink not to treat a specific DT node as a real device. So, if we
> > really need that I'll dig up and suggest a fix.
>
> Interesting, that would indeed very likely fix it.
>
> > Lastly and more importantly, I've a series[1] that stops depending on
> > the compatible property for fw_devlink to work. So it should be
> > smarter than it is today. But that series has known bugs for which I
> > gave test fixes in that thread. I plan to make a v2 of that series
> > with that fix and I'm expecting it'll fix a bunch of fw_devlink
> > issues.
> >
> > Feel free to give v1 + squashing the fixes a shot if you are excited
> > to try it. Otherwise, I'll try my best to get around to it this week
> > (kinda swamped though + holidays coming up, so no promises).
>
> Can you please include us in your next submission?
> * Maxim Kiselev <bigunclemax@gmail.com>
> * Maxim Kochetkov <fido_max@inbox.ru>
> * Miquel Raynal <miquel.raynal@bootlin.com>

Will do.

-Saravana

>
> > [1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/
>
> Maxim, any chance you give this a try?
>
> Thanks,
> Miquèl

Re: Fwd: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'

[Linux regression tracking #update (Thorsten Leemhuis)]

[TLDR: This mail in primarily relevant for Linux regression tracking. A
change or fix related to the regression discussed in this thread was
posted or applied, but it did not use a Link: tag to point to the
report, as Linus and the documentation call for. Things happen, no
worries -- but now the regression tracking bot needs to be told manually
about the fix. See link in footer if these mails annoy you.]

On 10.12.22 13:35, Thorsten Leemhuis wrote:
> [...]
> P.P.S.: let me add this to the regression tracking:
> 
> #regzbot ^introduced bcdf0315
> #regzbot title mtd: ethernet device can't be probed anymore due to
> broken nvmem-cells dep
> #regzbot monitor: https://lore.kernel.org/all/Yyj7wJlqJkCwObRn@lx2k/
> #regzbot ignore-activity

#regzbot fix: mtd: mtdpart: Don't create platform device that'll never probe
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.