* [PATCH] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties
@ 2024-10-25 16:13 Philipp Zabel
2024-10-28 2:42 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Zabel @ 2024-10-25 16:13 UTC (permalink / raw)
To: devicetree-compiler; +Cc: Rob Herring, Philipp Zabel
Do not fail the unnecessary #address-cells/#size-cells check if any
children of the node have a "ranges" property.
Suggested-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/all/CAL_JsqKebRL454poAYZ9i=sCsHqGzmocLy0psQcng-79UWJB-A@mail.gmail.com/
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
checks.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/checks.c b/checks.c
index 6e06aeab5503..76fdee2ed030 100644
--- a/checks.c
+++ b/checks.c
@@ -1217,9 +1217,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *dti,
struct node *node)
{
- struct property *prop;
struct node *child;
- bool has_reg = false;
if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
return;
@@ -1228,13 +1226,11 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
return;
for_each_child(node, child) {
- prop = get_property(child, "reg");
- if (prop)
- has_reg = true;
+ if (get_property(child, "reg") || get_property(child, "ranges"))
+ return;
}
- if (!has_reg)
- FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
+ FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
}
WARNING(avoid_unnecessary_addr_size, check_avoid_unnecessary_addr_size, NULL, &avoid_default_addr_size);
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties
2024-10-25 16:13 [PATCH] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties Philipp Zabel
@ 2024-10-28 2:42 ` David Gibson
2024-11-04 13:32 ` Philipp Zabel
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2024-10-28 2:42 UTC (permalink / raw)
To: Philipp Zabel; +Cc: devicetree-compiler, Rob Herring
[-- Attachment #1: Type: text/plain, Size: 2746 bytes --]
On Fri, Oct 25, 2024 at 06:13:07PM +0200, Philipp Zabel wrote:
> Do not fail the unnecessary #address-cells/#size-cells check if any
> children of the node have a "ranges" property.
I think this is correct, but I had to think abuot it for a while,
because it's subtler than it looks.
If there is no 'ranges' in the node itself, then the child devices'
address space is not mapped into the parent bus. Of course, you can
still establish a local address space for them that (e.g.) could be
accessed indirectly via registers in this bridge device.
Having a child device which acts as a bridge from this local address
to another subordinate address space, but no children with any
registers directly on the local bus seems odd... but it is logically
possible.
Given the subtlety, it would be pretty nice to add an explanatory
comment about what this is check for and what some of the edge cases
are.
> Suggested-by: Rob Herring <robh@kernel.org>
> Link: https://lore.kernel.org/all/CAL_JsqKebRL454poAYZ9i=sCsHqGzmocLy0psQcng-79UWJB-A@mail.gmail.com/
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> checks.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/checks.c b/checks.c
> index 6e06aeab5503..76fdee2ed030 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1217,9 +1217,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
> static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *dti,
> struct node *node)
> {
> - struct property *prop;
> struct node *child;
> - bool has_reg = false;
>
> if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
> return;
> @@ -1228,13 +1226,11 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
> return;
>
> for_each_child(node, child) {
> - prop = get_property(child, "reg");
> - if (prop)
> - has_reg = true;
> + if (get_property(child, "reg") || get_property(child, "ranges"))
> + return;
> }
>
> - if (!has_reg)
> - FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> + FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
..also this message needs updating to reference child "ranges" as well.
> }
> WARNING(avoid_unnecessary_addr_size, check_avoid_unnecessary_addr_size, NULL, &avoid_default_addr_size);
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties
2024-10-28 2:42 ` David Gibson
@ 2024-11-04 13:32 ` Philipp Zabel
2024-11-05 23:55 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Zabel @ 2024-11-04 13:32 UTC (permalink / raw)
To: David Gibson; +Cc: devicetree-compiler, Rob Herring
On Mo, 2024-10-28 at 13:42 +1100, David Gibson wrote:
> On Fri, Oct 25, 2024 at 06:13:07PM +0200, Philipp Zabel wrote:
> > Do not fail the unnecessary #address-cells/#size-cells check if any
> > children of the node have a "ranges" property.
>
> I think this is correct, but I had to think abuot it for a while,
> because it's subtler than it looks.
>
> If there is no 'ranges' in the node itself, then the child devices'
> address space is not mapped into the parent bus. Of course, you can
> still establish a local address space for them that (e.g.) could be
> accessed indirectly via registers in this bridge device.
>
> Having a child device which acts as a bridge from this local address
> to another subordinate address space, but no children with any
> registers directly on the local bus seems odd... but it is logically
> possible.
>
> Given the subtlety, it would be pretty nice to add an explanatory
> comment about what this is check for and what some of the edge cases
> are.
Thank you. Can I steal your wording and add the following comment
inside the for_each_child() loop:
/*
* Even if the child devices' address space is not mapped into
* the parent bus (no 'ranges' property on node), children can
* still have registers on a local bus, or map local addresses
* to another subordinate address space. The properties on the
* child nodes then make #address-cells/#size-cells necessary:
*/
The specific reason for this patch is a PCI device tree overlay [1]
where the '#address-cells'/'#size-cells' properties are on the
__overlay__ node and a child simple-bus node contains the 'ranges'
property mapping between BAR space and the local bus.
[1] https://lore.kernel.org/all/20241014124636.24221-2-herve.codina@bootlin.com/
> > Suggested-by: Rob Herring <robh@kernel.org>
> > Link: https://lore.kernel.org/all/CAL_JsqKebRL454poAYZ9i=sCsHqGzmocLy0psQcng-79UWJB-A@mail.gmail.com/
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> > checks.c | 10 +++-------
> > 1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/checks.c b/checks.c
> > index 6e06aeab5503..76fdee2ed030 100644
> > --- a/checks.c
> > +++ b/checks.c
> > @@ -1217,9 +1217,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
> > static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *dti,
> > struct node *node)
> > {
> > - struct property *prop;
> > struct node *child;
> > - bool has_reg = false;
> >
> > if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
> > return;
> > @@ -1228,13 +1226,11 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
> > return;
> >
> > for_each_child(node, child) {
> > - prop = get_property(child, "reg");
> > - if (prop)
> > - has_reg = true;
> > + if (get_property(child, "reg") || get_property(child, "ranges"))
> > + return;
> > }
> >
> > - if (!has_reg)
> > - FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> > + FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
>
> ..also this message needs updating to reference child "ranges" as well.
I'll change it to:
"unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" or \"ranges\" property"
regards
Philipp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties
2024-11-04 13:32 ` Philipp Zabel
@ 2024-11-05 23:55 ` David Gibson
2024-11-06 13:23 ` Philipp Zabel
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2024-11-05 23:55 UTC (permalink / raw)
To: Philipp Zabel; +Cc: devicetree-compiler, Rob Herring
[-- Attachment #1: Type: text/plain, Size: 4394 bytes --]
On Mon, Nov 04, 2024 at 02:32:54PM +0100, Philipp Zabel wrote:
> On Mo, 2024-10-28 at 13:42 +1100, David Gibson wrote:
> > On Fri, Oct 25, 2024 at 06:13:07PM +0200, Philipp Zabel wrote:
> > > Do not fail the unnecessary #address-cells/#size-cells check if any
> > > children of the node have a "ranges" property.
> >
> > I think this is correct, but I had to think abuot it for a while,
> > because it's subtler than it looks.
> >
> > If there is no 'ranges' in the node itself, then the child devices'
> > address space is not mapped into the parent bus. Of course, you can
> > still establish a local address space for them that (e.g.) could be
> > accessed indirectly via registers in this bridge device.
> >
> > Having a child device which acts as a bridge from this local address
> > to another subordinate address space, but no children with any
> > registers directly on the local bus seems odd... but it is logically
> > possible.
> >
> > Given the subtlety, it would be pretty nice to add an explanatory
> > comment about what this is check for and what some of the edge cases
> > are.
>
> Thank you. Can I steal your wording and add the following comment
> inside the for_each_child() loop:
>
> /*
> * Even if the child devices' address space is not mapped into
> * the parent bus (no 'ranges' property on node), children can
> * still have registers on a local bus, or map local addresses
> * to another subordinate address space. The properties on the
> * child nodes then make #address-cells/#size-cells necessary:
> */
Sure!
> The specific reason for this patch is a PCI device tree overlay [1]
> where the '#address-cells'/'#size-cells' properties are on the
> __overlay__ node and a child simple-bus node contains the 'ranges'
> property mapping between BAR space and the local bus.
>
> [1]
> https://lore.kernel.org/all/20241014124636.24221-2-herve.codina@bootlin.com/
Hrm... two things strike me as suspicious about that example.
1) The __overlay__ node is the PCI bus root... which should already
have #address-cells and #size-cells in the base tree; it's PCI so
those values are known and standard.
2) The PCI<->simple-bus bridge must surely have at least the normal
configuration space registers, and so should have a 'reg' property as
well as 'ranges'.
>
> > > Suggested-by: Rob Herring <robh@kernel.org>
> > > Link: https://lore.kernel.org/all/CAL_JsqKebRL454poAYZ9i=sCsHqGzmocLy0psQcng-79UWJB-A@mail.gmail.com/
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > ---
> > > checks.c | 10 +++-------
> > > 1 file changed, 3 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/checks.c b/checks.c
> > > index 6e06aeab5503..76fdee2ed030 100644
> > > --- a/checks.c
> > > +++ b/checks.c
> > > @@ -1217,9 +1217,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
> > > static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *dti,
> > > struct node *node)
> > > {
> > > - struct property *prop;
> > > struct node *child;
> > > - bool has_reg = false;
> > >
> > > if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
> > > return;
> > > @@ -1228,13 +1226,11 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
> > > return;
> > >
> > > for_each_child(node, child) {
> > > - prop = get_property(child, "reg");
> > > - if (prop)
> > > - has_reg = true;
> > > + if (get_property(child, "reg") || get_property(child, "ranges"))
> > > + return;
> > > }
> > >
> > > - if (!has_reg)
> > > - FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> > > + FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> >
> > ..also this message needs updating to reference child "ranges" as well.
>
> I'll change it to:
>
> "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" or \"ranges\" property"
>
>
> regards
> Philipp
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties
2024-11-05 23:55 ` David Gibson
@ 2024-11-06 13:23 ` Philipp Zabel
2024-11-06 21:16 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Zabel @ 2024-11-06 13:23 UTC (permalink / raw)
To: David Gibson; +Cc: devicetree-compiler, Rob Herring
On Mi, 2024-11-06 at 10:55 +1100, David Gibson wrote:
> On Mon, Nov 04, 2024 at 02:32:54PM +0100, Philipp Zabel wrote:
> > On Mo, 2024-10-28 at 13:42 +1100, David Gibson wrote:
> > > On Fri, Oct 25, 2024 at 06:13:07PM +0200, Philipp Zabel wrote:
> > > > Do not fail the unnecessary #address-cells/#size-cells check if any
> > > > children of the node have a "ranges" property.
> > >
> > > I think this is correct, but I had to think abuot it for a while,
> > > because it's subtler than it looks.
> > >
> > > If there is no 'ranges' in the node itself, then the child devices'
> > > address space is not mapped into the parent bus. Of course, you can
> > > still establish a local address space for them that (e.g.) could be
> > > accessed indirectly via registers in this bridge device.
> > >
> > > Having a child device which acts as a bridge from this local address
> > > to another subordinate address space, but no children with any
> > > registers directly on the local bus seems odd... but it is logically
> > > possible.
> > >
> > > Given the subtlety, it would be pretty nice to add an explanatory
> > > comment about what this is check for and what some of the edge cases
> > > are.
> >
> > Thank you. Can I steal your wording and add the following comment
> > inside the for_each_child() loop:
> >
> > /*
> > * Even if the child devices' address space is not mapped into
> > * the parent bus (no 'ranges' property on node), children can
> > * still have registers on a local bus, or map local addresses
> > * to another subordinate address space. The properties on the
> > * child nodes then make #address-cells/#size-cells necessary:
> > */
>
> Sure!
Done in v2.
> > The specific reason for this patch is a PCI device tree overlay [1]
> > where the '#address-cells'/'#size-cells' properties are on the
> > __overlay__ node and a child simple-bus node contains the 'ranges'
> > property mapping between BAR space and the local bus.
> >
> > [1]
> > https://lore.kernel.org/all/20241014124636.24221-2-herve.codina@bootlin.com/
>
> Hrm... two things strike me as suspicious about that example.
>
> 1) The __overlay__ node is the PCI bus root... which should already
> have #address-cells and #size-cells in the base tree; it's PCI so
> those values are known and standard.
dtc doesn't know about this when compiling the overlay.
Removing the #address-cells/#size-cells properties from the __overlay__
node would cause these compiler warnings:
DTC drivers/misc/lan966x_pci.dtbo
../drivers/misc/lan966x_pci.dtso:59.5-60.52: Warning (ranges_format): /fragment@0/__overlay__/pci-ep-bus@0:ranges: "ranges" property has invalid length (40 bytes) (parent #address-cells == 2, child #address-cells == 1, #size-cells == 1)
../drivers/misc/lan966x_pci.dtso:50.17-174.6: Warning (avoid_default_addr_size): /fragment@0/__overlay__/pci-ep-bus@0: Relying on default #address-cells value
../drivers/misc/lan966x_pci.dtso:50.17-174.6: Warning (avoid_default_addr_size): /fragment@0/__overlay__/pci-ep-bus@0: Relying on default #size-cells value
> 2) The PCI<->simple-bus bridge must surely have at least the normal
> configuration space registers, and so should have a 'reg' property as
> well as 'ranges'.
This I don't know about.
> > > > Suggested-by: Rob Herring <robh@kernel.org>
> > > > Link: https://lore.kernel.org/all/CAL_JsqKebRL454poAYZ9i=sCsHqGzmocLy0psQcng-79UWJB-A@mail.gmail.com/
> > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > > ---
> > > > checks.c | 10 +++-------
> > > > 1 file changed, 3 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/checks.c b/checks.c
> > > > index 6e06aeab5503..76fdee2ed030 100644
> > > > --- a/checks.c
> > > > +++ b/checks.c
> > > > @@ -1217,9 +1217,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
> > > > static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *dti,
> > > > struct node *node)
> > > > {
> > > > - struct property *prop;
> > > > struct node *child;
> > > > - bool has_reg = false;
> > > >
> > > > if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
> > > > return;
> > > > @@ -1228,13 +1226,11 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
> > > > return;
> > > >
> > > > for_each_child(node, child) {
> > > > - prop = get_property(child, "reg");
> > > > - if (prop)
> > > > - has_reg = true;
> > > > + if (get_property(child, "reg") || get_property(child, "ranges"))
> > > > + return;
> > > > }
> > > >
> > > > - if (!has_reg)
> > > > - FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> > > > + FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> > >
> > > ..also this message needs updating to reference child "ranges" as well.
> >
> > I'll change it to:
> >
> > "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" or \"ranges\" property"
Done in v2.
regards
Philipp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties
2024-11-06 13:23 ` Philipp Zabel
@ 2024-11-06 21:16 ` David Gibson
0 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2024-11-06 21:16 UTC (permalink / raw)
To: Philipp Zabel; +Cc: devicetree-compiler, Rob Herring
[-- Attachment #1: Type: text/plain, Size: 6153 bytes --]
On Wed, Nov 06, 2024 at 02:23:33PM +0100, Philipp Zabel wrote:
> On Mi, 2024-11-06 at 10:55 +1100, David Gibson wrote:
> > On Mon, Nov 04, 2024 at 02:32:54PM +0100, Philipp Zabel wrote:
> > > On Mo, 2024-10-28 at 13:42 +1100, David Gibson wrote:
> > > > On Fri, Oct 25, 2024 at 06:13:07PM +0200, Philipp Zabel wrote:
> > > > > Do not fail the unnecessary #address-cells/#size-cells check if any
> > > > > children of the node have a "ranges" property.
> > > >
> > > > I think this is correct, but I had to think abuot it for a while,
> > > > because it's subtler than it looks.
> > > >
> > > > If there is no 'ranges' in the node itself, then the child devices'
> > > > address space is not mapped into the parent bus. Of course, you can
> > > > still establish a local address space for them that (e.g.) could be
> > > > accessed indirectly via registers in this bridge device.
> > > >
> > > > Having a child device which acts as a bridge from this local address
> > > > to another subordinate address space, but no children with any
> > > > registers directly on the local bus seems odd... but it is logically
> > > > possible.
> > > >
> > > > Given the subtlety, it would be pretty nice to add an explanatory
> > > > comment about what this is check for and what some of the edge cases
> > > > are.
> > >
> > > Thank you. Can I steal your wording and add the following comment
> > > inside the for_each_child() loop:
> > >
> > > /*
> > > * Even if the child devices' address space is not mapped into
> > > * the parent bus (no 'ranges' property on node), children can
> > > * still have registers on a local bus, or map local addresses
> > > * to another subordinate address space. The properties on the
> > > * child nodes then make #address-cells/#size-cells necessary:
> > > */
> >
> > Sure!
>
> Done in v2.
>
> > > The specific reason for this patch is a PCI device tree overlay [1]
> > > where the '#address-cells'/'#size-cells' properties are on the
> > > __overlay__ node and a child simple-bus node contains the 'ranges'
> > > property mapping between BAR space and the local bus.
> > >
> > > [1]
> > > https://lore.kernel.org/all/20241014124636.24221-2-herve.codina@bootlin.com/
> >
> > Hrm... two things strike me as suspicious about that example.
> >
> > 1) The __overlay__ node is the PCI bus root... which should already
> > have #address-cells and #size-cells in the base tree; it's PCI so
> > those values are known and standard.
>
> dtc doesn't know about this when compiling the overlay.
> Removing the #address-cells/#size-cells properties from the __overlay__
> node would cause these compiler warnings:
>
> DTC drivers/misc/lan966x_pci.dtbo
> ../drivers/misc/lan966x_pci.dtso:59.5-60.52: Warning (ranges_format): /fragment@0/__overlay__/pci-ep-bus@0:ranges: "ranges" property has invalid length (40 bytes) (parent #address-cells == 2, child #address-cells == 1, #size-cells == 1)
> ../drivers/misc/lan966x_pci.dtso:50.17-174.6: Warning (avoid_default_addr_size): /fragment@0/__overlay__/pci-ep-bus@0: Relying on default #address-cells value
> ../drivers/misc/lan966x_pci.dtso:50.17-174.6: Warning (avoid_default_addr_size): /fragment@0/__overlay__/pci-ep-bus@0: Relying on default #size-cells value
Ah, yeah, I realised that after I sent the mail. That's the hackiness
of the overlay design showing itself again. There's a limit to how
well the checks can work on an overlay alone - and even when it can
theoretically work, not all the checks have been written with that in
mind. Duplicating the #address-cells in the overlay is a reasonable
workaround under the circumstances, but it's pretty ugly :(.
> > 2) The PCI<->simple-bus bridge must surely have at least the normal
> > configuration space registers, and so should have a 'reg' property as
> > well as 'ranges'.
>
> This I don't know about.
>
> > > > > Suggested-by: Rob Herring <robh@kernel.org>
> > > > > Link: https://lore.kernel.org/all/CAL_JsqKebRL454poAYZ9i=sCsHqGzmocLy0psQcng-79UWJB-A@mail.gmail.com/
> > > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > > > ---
> > > > > checks.c | 10 +++-------
> > > > > 1 file changed, 3 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/checks.c b/checks.c
> > > > > index 6e06aeab5503..76fdee2ed030 100644
> > > > > --- a/checks.c
> > > > > +++ b/checks.c
> > > > > @@ -1217,9 +1217,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
> > > > > static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *dti,
> > > > > struct node *node)
> > > > > {
> > > > > - struct property *prop;
> > > > > struct node *child;
> > > > > - bool has_reg = false;
> > > > >
> > > > > if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
> > > > > return;
> > > > > @@ -1228,13 +1226,11 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
> > > > > return;
> > > > >
> > > > > for_each_child(node, child) {
> > > > > - prop = get_property(child, "reg");
> > > > > - if (prop)
> > > > > - has_reg = true;
> > > > > + if (get_property(child, "reg") || get_property(child, "ranges"))
> > > > > + return;
> > > > > }
> > > > >
> > > > > - if (!has_reg)
> > > > > - FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> > > > > + FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> > > >
> > > > ..also this message needs updating to reference child "ranges" as well.
> > >
> > > I'll change it to:
> > >
> > > "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" or \"ranges\" property"
>
> Done in v2.
>
> regards
> Philipp
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-06 21:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-25 16:13 [PATCH] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties Philipp Zabel
2024-10-28 2:42 ` David Gibson
2024-11-04 13:32 ` Philipp Zabel
2024-11-05 23:55 ` David Gibson
2024-11-06 13:23 ` Philipp Zabel
2024-11-06 21:16 ` David Gibson
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).