* of_platform_populate() for address-less nodes (OF: Bad cell count for ...)
@ 2022-07-15 11:59 Rafał Miłecki
2022-07-15 15:02 ` Rob Herring
0 siblings, 1 reply; 4+ messages in thread
From: Rafał Miłecki @ 2022-07-15 11:59 UTC (permalink / raw)
To: devicetree@vger.kernel.org; +Cc: Ansuel Smith
I need Linux to support MTD partitions drivers. They should get probed
for MTD partitions, access it, do their stuff. Random example:
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
partition@0 {
compatible = "u-boot,bootloader";
label = "loader";
reg = <0x0 0x100000>;
};
partition@100000 {
compatible = "u-boot,env";
label = "image";
reg = <0x100000 0x100000>;
};
};
(please don't confuse them with parsers which are MTD internals)
To support that I added of_platform_populate() calls, see commit
bcdf0315a61a2 ("mtd: call of_platform_populate() for MTD partitions"):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcdf0315a61a29eb753a607d3a85a4032de72d94
The problem I just noticed is it triggers errors like:
OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0/partitions
It comes from (forward-trace):
of_platform_populate()
of_platform_bus_create()
of_platform_device_create_pdata()
of_device_alloc()
of_address_to_resource()
of_address_to_resource()
__of_address_to_resource()
of_translate_address()
__of_translate_address()
OF_CHECK_COUNTS()
pr_err()
It's caused by "partitions" node having 1 address cell and 0 size cells.
It's a consequence of inheriting sizes from NAND CS:
nand-controller@1800 {
...
#address-cells = <1>;
#size-cells = <0>;
nand@0 {
compatible = "brcm,nandcs";
reg = <0>;
partitions {
...
};
};
};
Is that something that can / should be fixed in OF implementation?
I don't think I should assign sizes to "partitions" node as it doesn't
use "reg" at all. All "reg" in "partitions" subnodes contain flash
relative offsets and they should not be translated.
[ 4.360609] OF: ** translation for device /bus@ff800000/nand-controller@1800/nand@0/partitions/partition@0 **
[ 4.370814] OF: bus is default (na=1, ns=1) on /bus@ff800000/nand-controller@1800/nand@0/partitions
[ 4.380130] OF: translating address: 00000000
[ 4.384609] OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0/partitions
[ 4.392859] OF: ** translation for device /bus@ff800000/nand-controller@1800/nand@0/partitions/partition@0 **
[ 4.403069] OF: bus is default (na=1, ns=1) on /bus@ff800000/nand-controller@1800/nand@0/partitions
[ 4.412384] OF: translating address: 00000000
[ 4.416864] OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0/partitions
[ 4.425110] OF: ** translation for device /bus@ff800000/nand-controller@1800/nand@0 **
[ 4.433263] OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0
[ 4.440520] OF: ** translation for device /bus@ff800000/nand-controller@1800 **
[ 4.448044] OF: bus is default (na=1, ns=1) on /bus@ff800000
[ 4.453868] OF: translating address: 00001800
[ 4.458347] OF: parent bus is default (na=2, ns=2) on
[ 4.463635] OF: walking ranges...
[ 4.467038] OF: default map, cp=0, s=3000, da=1800
[ 4.471967] OF: parent translation for: 00000000 ff800000
[ 4.477521] OF: with offset: 1800
[ 4.480928] OF: one level translation: 00000000 ff801800
[ 4.486390] OF: reached root node
[ 4.489874] OF: ** translation for device /bus@ff800000/nand-controller@1800/nand@0/partitions/partition@100000 **
[ 4.500465] OF: bus is default (na=1, ns=1) on /bus@ff800000/nand-controller@1800/nand@0/partitions
[ 4.509776] OF: translating address: 00100000
[ 4.514262] OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0/partitions
[ 4.522509] OF: ** translation for device /bus@ff800000/nand-controller@1800/nand@0/partitions/partition@100000 **
[ 4.533167] OF: bus is default (na=1, ns=1) on /bus@ff800000/nand-controller@1800/nand@0/partitions
[ 4.542483] OF: translating address: 00100000
[ 4.546963] OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0/partitions
[ 4.555211] OF: ** translation for device /bus@ff800000/nand-controller@1800/nand@0 **
[ 4.563362] OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0
[ 4.570620] OF: ** translation for device /bus@ff800000/nand-controller@1800 **
[ 4.578142] OF: bus is default (na=1, ns=1) on /bus@ff800000
[ 4.583967] OF: translating address: 00001800
[ 4.588446] OF: parent bus is default (na=2, ns=2) on
[ 4.593733] OF: walking ranges...
[ 4.597137] OF: default map, cp=0, s=3000, da=1800
[ 4.602067] OF: parent translation for: 00000000 ff800000
[ 4.607619] OF: with offset: 1800
[ 4.611027] OF: one level translation: 00000000 ff801800
[ 4.616489] OF: reached root node
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: of_platform_populate() for address-less nodes (OF: Bad cell count for ...)
2022-07-15 11:59 of_platform_populate() for address-less nodes (OF: Bad cell count for ...) Rafał Miłecki
@ 2022-07-15 15:02 ` Rob Herring
2022-07-15 15:16 ` Rafał Miłecki
0 siblings, 1 reply; 4+ messages in thread
From: Rob Herring @ 2022-07-15 15:02 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: devicetree@vger.kernel.org, Ansuel Smith
On Fri, Jul 15, 2022 at 5:59 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> I need Linux to support MTD partitions drivers. They should get probed
> for MTD partitions, access it, do their stuff. Random example:
>
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <1>;
> #size-cells = <1>;
>
> partition@0 {
> compatible = "u-boot,bootloader";
> label = "loader";
> reg = <0x0 0x100000>;
> };
>
> partition@100000 {
> compatible = "u-boot,env";
> label = "image";
> reg = <0x100000 0x100000>;
> };
> };
>
> (please don't confuse them with parsers which are MTD internals)
>
>
> To support that I added of_platform_populate() calls, see commit
> bcdf0315a61a2 ("mtd: call of_platform_populate() for MTD partitions"):
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcdf0315a61a29eb753a607d3a85a4032de72d94
>
>
> The problem I just noticed is it triggers errors like:
> OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0/partitions
>
> It comes from (forward-trace):
> of_platform_populate()
> of_platform_bus_create()
> of_platform_device_create_pdata()
> of_device_alloc()
> of_address_to_resource()
> of_address_to_resource()
> __of_address_to_resource()
> of_translate_address()
> __of_translate_address()
> OF_CHECK_COUNTS()
> pr_err()
>
>
> It's caused by "partitions" node having 1 address cell and 0 size cells.
> It's a consequence of inheriting sizes from NAND CS:
>
> nand-controller@1800 {
> ...
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> nand@0 {
> compatible = "brcm,nandcs";
> reg = <0>;
>
> partitions {
> ...
> };
> };
> };
>
>
> Is that something that can / should be fixed in OF implementation?
>
> I don't think I should assign sizes to "partitions" node as it doesn't
> use "reg" at all. All "reg" in "partitions" subnodes contain flash
> relative offsets and they should not be translated.
Yes, you should. The parent node of a node with 'reg' must have
#address-cells and #size-cells. Simple as that.
It is 'ranges' that determines if addresses are translatable or not.
Rob
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: of_platform_populate() for address-less nodes (OF: Bad cell count for ...)
2022-07-15 15:02 ` Rob Herring
@ 2022-07-15 15:16 ` Rafał Miłecki
2022-07-15 19:49 ` Rob Herring
0 siblings, 1 reply; 4+ messages in thread
From: Rafał Miłecki @ 2022-07-15 15:16 UTC (permalink / raw)
To: Rob Herring; +Cc: devicetree@vger.kernel.org, Ansuel Smith
On 15.07.2022 17:02, Rob Herring wrote:
> On Fri, Jul 15, 2022 at 5:59 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>>
>> I need Linux to support MTD partitions drivers. They should get probed
>> for MTD partitions, access it, do their stuff. Random example:
>>
>> partitions {
>> compatible = "fixed-partitions";
>> #address-cells = <1>;
>> #size-cells = <1>;
>>
>> partition@0 {
>> compatible = "u-boot,bootloader";
>> label = "loader";
>> reg = <0x0 0x100000>;
>> };
>>
>> partition@100000 {
>> compatible = "u-boot,env";
>> label = "image";
>> reg = <0x100000 0x100000>;
>> };
>> };
>>
>> (please don't confuse them with parsers which are MTD internals)
>>
>>
>> To support that I added of_platform_populate() calls, see commit
>> bcdf0315a61a2 ("mtd: call of_platform_populate() for MTD partitions"):
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcdf0315a61a29eb753a607d3a85a4032de72d94
>>
>>
>> The problem I just noticed is it triggers errors like:
>> OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0/partitions
>>
>> It comes from (forward-trace):
>> of_platform_populate()
>> of_platform_bus_create()
>> of_platform_device_create_pdata()
>> of_device_alloc()
>> of_address_to_resource()
>> of_address_to_resource()
>> __of_address_to_resource()
>> of_translate_address()
>> __of_translate_address()
>> OF_CHECK_COUNTS()
>> pr_err()
>>
>>
>> It's caused by "partitions" node having 1 address cell and 0 size cells.
>> It's a consequence of inheriting sizes from NAND CS:
>>
>> nand-controller@1800 {
>> ...
>>
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> nand@0 {
>> compatible = "brcm,nandcs";
>> reg = <0>;
>>
>> partitions {
>> ...
>> };
>> };
>> };
>>
>>
>> Is that something that can / should be fixed in OF implementation?
>>
>> I don't think I should assign sizes to "partitions" node as it doesn't
>> use "reg" at all. All "reg" in "partitions" subnodes contain flash
>> relative offsets and they should not be translated.
>
> Yes, you should. The parent node of a node with 'reg' must have
> #address-cells and #size-cells. Simple as that.
For "parent node of a node with 'reg'" it's obvious. We have a different
case here though.
Please take one more look. Node named "partitions" does not have "reg".
That is what I don't have #foo-cells in the nand@0.
A complete example:
nand-controller@1800 {
...
#address-cells = <1>;
#size-cells = <0>;
nand@0 {
compatible = "brcm,nandcs";
reg = <0>;
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
partition@0 {
compatible = "u-boot,bootloader";
label = "loader";
reg = <0x0 0x100000>;
};
partition@100000 {
compatible = "u-boot,env";
label = "image";
reg = <0x100000 0x100000>;
};
};
};
};
> It is 'ranges' that determines if addresses are translatable or not.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: of_platform_populate() for address-less nodes (OF: Bad cell count for ...)
2022-07-15 15:16 ` Rafał Miłecki
@ 2022-07-15 19:49 ` Rob Herring
0 siblings, 0 replies; 4+ messages in thread
From: Rob Herring @ 2022-07-15 19:49 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: devicetree@vger.kernel.org, Ansuel Smith
On Fri, Jul 15, 2022 at 9:16 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> On 15.07.2022 17:02, Rob Herring wrote:
> > On Fri, Jul 15, 2022 at 5:59 AM Rafał Miłecki <zajec5@gmail.com> wrote:
> >>
> >> I need Linux to support MTD partitions drivers. They should get probed
> >> for MTD partitions, access it, do their stuff. Random example:
> >>
> >> partitions {
> >> compatible = "fixed-partitions";
> >> #address-cells = <1>;
> >> #size-cells = <1>;
> >>
> >> partition@0 {
> >> compatible = "u-boot,bootloader";
> >> label = "loader";
> >> reg = <0x0 0x100000>;
> >> };
> >>
> >> partition@100000 {
> >> compatible = "u-boot,env";
> >> label = "image";
> >> reg = <0x100000 0x100000>;
> >> };
> >> };
> >>
> >> (please don't confuse them with parsers which are MTD internals)
> >>
> >>
> >> To support that I added of_platform_populate() calls, see commit
> >> bcdf0315a61a2 ("mtd: call of_platform_populate() for MTD partitions"):
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcdf0315a61a29eb753a607d3a85a4032de72d94
> >>
> >>
> >> The problem I just noticed is it triggers errors like:
> >> OF: Bad cell count for /bus@ff800000/nand-controller@1800/nand@0/partitions
> >>
> >> It comes from (forward-trace):
> >> of_platform_populate()
> >> of_platform_bus_create()
> >> of_platform_device_create_pdata()
> >> of_device_alloc()
> >> of_address_to_resource()
> >> of_address_to_resource()
> >> __of_address_to_resource()
> >> of_translate_address()
> >> __of_translate_address()
> >> OF_CHECK_COUNTS()
> >> pr_err()
> >>
> >>
> >> It's caused by "partitions" node having 1 address cell and 0 size cells.
> >> It's a consequence of inheriting sizes from NAND CS:
> >>
> >> nand-controller@1800 {
> >> ...
> >>
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> nand@0 {
> >> compatible = "brcm,nandcs";
> >> reg = <0>;
> >>
> >> partitions {
> >> ...
> >> };
> >> };
> >> };
> >>
> >>
> >> Is that something that can / should be fixed in OF implementation?
> >>
> >> I don't think I should assign sizes to "partitions" node as it doesn't
> >> use "reg" at all. All "reg" in "partitions" subnodes contain flash
> >> relative offsets and they should not be translated.
> >
> > Yes, you should. The parent node of a node with 'reg' must have
> > #address-cells and #size-cells. Simple as that.
>
> For "parent node of a node with 'reg'" it's obvious. We have a different
> case here though.
>
> Please take one more look. Node named "partitions" does not have "reg".
> That is what I don't have #foo-cells in the nand@0.
>
> A complete example:
Okay, that looks valid.
Back to your original issue, I think the issue is that for (DT)
platform devices, the assumption is that they are MMIO (i.e.
translatable) based devices. Certainly that's the case for devices
created by the DT core if not all devices created. Maybe we could
relax this... But reg could not be a struct resource in this case
because there's not an address space for it (it's not MEM or IO). That
all seems to me a bit of abuse of the platform bus. Perhaps a
partition bus and devices are needed? You'll need to ask a wider
audience that.
Rob
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-15 19:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-15 11:59 of_platform_populate() for address-less nodes (OF: Bad cell count for ...) Rafał Miłecki
2022-07-15 15:02 ` Rob Herring
2022-07-15 15:16 ` Rafał Miłecki
2022-07-15 19:49 ` Rob Herring
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).