From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Maxim Kiselev <bigunclemax@gmail.com>
Cc: "Maxim Kochetkov" <fido_max@inbox.ru>,
"Thorsten Leemhuis" <regressions@leemhuis.info>,
linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
"Vignesh Raghavendra" <vigneshr@ti.com>,
"Richard Weinberger" <richard@nod.at>,
"Rafał Miłecki" <rafal@milecki.pl>,
"regressions@lists.linux.dev" <regressions@lists.linux.dev>,
"Rob Herring" <robh+dt@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
"Saravana Kannan" <saravanak@google.com>
Subject: Re: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions'
Date: Tue, 13 Dec 2022 17:54:24 +0100 [thread overview]
Message-ID: <20221213175424.79895b63@xps-13> (raw)
In-Reply-To: <CALHCpMhOku2b+1y5Z=N5RJ6SvCvNakD2rSyRAqp6Gz3JWVvXGg@mail.gmail.com>
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
next prev parent reply other threads:[~2022-12-13 16:55 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CALHCpMgSZOZdOGpLwTYf0sFD5EMNL7CuqHuFJV_6w5VPSWZnUw@mail.gmail.com>
2022-12-10 9:52 ` Fwd: nvmem-cells regression after adding 'call of_platform_populate() for MTD partitions' Maxim Kiselev
2022-12-10 12:35 ` Thorsten Leemhuis
2022-12-11 8:26 ` Maxim Kiselev
2022-12-12 9:14 ` Miquel Raynal
2022-12-12 13:06 ` Maxim Kiselev
2022-12-12 16:37 ` Miquel Raynal
2022-12-12 17:57 ` Maxim Kochetkov
2022-12-13 9:46 ` Miquel Raynal
2022-12-13 11:02 ` Maxim Kiselev
2022-12-13 16:54 ` Miquel Raynal [this message]
2022-12-14 21:53 ` Saravana Kannan
2022-12-16 11:04 ` Miquel Raynal
2022-12-16 11:33 ` Maxim Kiselev
2022-12-16 13:13 ` Maxim Kiselev
2022-12-17 1:44 ` Saravana Kannan
2022-12-17 1:45 ` Saravana Kannan
2023-02-24 16:15 ` Fwd: " Linux regression tracking #update (Thorsten Leemhuis)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221213175424.79895b63@xps-13 \
--to=miquel.raynal@bootlin.com \
--cc=bigunclemax@gmail.com \
--cc=fido_max@inbox.ru \
--cc=gregkh@linuxfoundation.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=rafael@kernel.org \
--cc=rafal@milecki.pl \
--cc=regressions@leemhuis.info \
--cc=regressions@lists.linux.dev \
--cc=richard@nod.at \
--cc=robh+dt@kernel.org \
--cc=saravanak@google.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).