[PATCH 1/2] checks: Allow PCI bridge child nodes without an address

[PATCH 1/2] checks: Allow PCI bridge child nodes without an address

[Rob Herring]

Some PCI bridge nodes have child nodes such as an interrupt controller
which are not PCI devices. Allow these nodes which don't have a
unit-address.

Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 checks.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/checks.c b/checks.c
index b7955dbd71ca..17cb6890d45a 100644
--- a/checks.c
+++ b/checks.c
@@ -891,10 +891,8 @@ static void check_pci_device_reg(struct check *c, struct dt_info *dti, struct no
 		return;
 
 	prop = get_property(node, "reg");
-	if (!prop) {
-		FAIL(c, dti, node, "missing PCI reg property");
+	if (!prop)
 		return;
-	}
 
 	cells = (cell_t *)prop->val.val;
 	if (cells[1] || cells[2])
-- 
2.25.1

[PATCH 2/2] checks: Relax SPI slave checks

[Rob Herring]

SPI slaves only have a single child node and therefore don't need
'#address-cells' nor '#size-cells', so let's skip the check for those when
'spi-slave' is present.

Cc: Kumar Gala <kumar.gala-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 checks.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/checks.c b/checks.c
index 17cb6890d45a..c923295c6e40 100644
--- a/checks.c
+++ b/checks.c
@@ -1102,13 +1102,12 @@ static void check_spi_bus_bridge(struct check *c, struct dt_info *dti, struct no
 	if (node->bus != &spi_bus || !node->children)
 		return;
 
-	if (get_property(node, "spi-slave"))
-		spi_addr_cells = 0;
-	if (node_addr_cells(node) != spi_addr_cells)
-		FAIL(c, dti, node, "incorrect #address-cells for SPI bus");
-	if (node_size_cells(node) != 0)
-		FAIL(c, dti, node, "incorrect #size-cells for SPI bus");
-
+	if (!get_property(node, "spi-slave")) {
+		if (node_addr_cells(node) != spi_addr_cells)
+			FAIL(c, dti, node, "incorrect #address-cells for SPI bus");
+		if (node_size_cells(node) != 0)
+			FAIL(c, dti, node, "incorrect #size-cells for SPI bus");
+	}
 }
 WARNING(spi_bus_bridge, check_spi_bus_bridge, NULL, &addr_size_cells);
 
-- 
2.25.1

Re: [PATCH 2/2] checks: Relax SPI slave checks

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1964 bytes --]

On Mon, Sep 28, 2020 at 03:19:42PM -0500, Rob Herring wrote:
> SPI slaves only have a single child node and therefore don't need
> '#address-cells' nor '#size-cells', so let's skip the check for those when
> 'spi-slave' is present.

I really don't like this idea.  Yes, you can work out what's going on
unambiguously because there's only one child, but that requires at
least some familiarity with SPI.  I think general consistency trumps
minimality here, and we should encourage SPI nodes to have the correct
-cells properties, even if they're not strictly essential.

> 
> Cc: Kumar Gala <kumar.gala-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  checks.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index 17cb6890d45a..c923295c6e40 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1102,13 +1102,12 @@ static void check_spi_bus_bridge(struct check *c, struct dt_info *dti, struct no
>  	if (node->bus != &spi_bus || !node->children)
>  		return;
>  
> -	if (get_property(node, "spi-slave"))
> -		spi_addr_cells = 0;
> -	if (node_addr_cells(node) != spi_addr_cells)
> -		FAIL(c, dti, node, "incorrect #address-cells for SPI bus");
> -	if (node_size_cells(node) != 0)
> -		FAIL(c, dti, node, "incorrect #size-cells for SPI bus");
> -
> +	if (!get_property(node, "spi-slave")) {
> +		if (node_addr_cells(node) != spi_addr_cells)
> +			FAIL(c, dti, node, "incorrect #address-cells for SPI bus");
> +		if (node_size_cells(node) != 0)
> +			FAIL(c, dti, node, "incorrect #size-cells for SPI bus");
> +	}
>  }
>  WARNING(spi_bus_bridge, check_spi_bus_bridge, NULL, &addr_size_cells);
>  

-- 
David Gibson			| 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 --]

Re: [PATCH 1/2] checks: Allow PCI bridge child nodes without an address

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

On Mon, Sep 28, 2020 at 03:19:41PM -0500, Rob Herring wrote:
> Some PCI bridge nodes have child nodes such as an interrupt controller
> which are not PCI devices. Allow these nodes which don't have a
> unit-address.
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

I suspect doing that isn't really IEEE1275-ly correct, but I can see
how that would be convenient.  Applied.

> ---
>  checks.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index b7955dbd71ca..17cb6890d45a 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -891,10 +891,8 @@ static void check_pci_device_reg(struct check *c, struct dt_info *dti, struct no
>  		return;
>  
>  	prop = get_property(node, "reg");
> -	if (!prop) {
> -		FAIL(c, dti, node, "missing PCI reg property");
> +	if (!prop)
>  		return;
> -	}
>  
>  	cells = (cell_t *)prop->val.val;
>  	if (cells[1] || cells[2])

-- 
David Gibson			| 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 --]