Patchwork [21/30] PCI: tegra: Add "pci" type check before parsing child device tree node

login
register
mail settings
Submitter Manikanta Maddireddy
Date April 11, 2019, 5:03 p.m.
Message ID <20190411170355.6882-22-mmaddireddy@nvidia.com>
Download mbox | patch
Permalink /patch/770853/
State New
Headers show

Comments

Manikanta Maddireddy - April 11, 2019, 5:03 p.m.
Each root port is added as a child device tree node of PCIe controller
node. These child nodes are parsed using open firmware PCI bus accessor
functions. If the child node is not of "pci" type then device tree
parsing fails. Add "pci" type check before parsing child device tree node.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
---
 drivers/pci/controller/pci-tegra.c | 3 +++
 1 file changed, 3 insertions(+)
Thierry Reding - April 15, 2019, 1:37 p.m.
On Thu, Apr 11, 2019 at 10:33:46PM +0530, Manikanta Maddireddy wrote:
> Each root port is added as a child device tree node of PCIe controller
> node. These child nodes are parsed using open firmware PCI bus accessor
> functions. If the child node is not of "pci" type then device tree
> parsing fails. Add "pci" type check before parsing child device tree node.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> ---
>  drivers/pci/controller/pci-tegra.c | 3 +++
>  1 file changed, 3 insertions(+)

Erm... what is the use-case that you're trying to support? Why would we
ever have children nodes that are not of type "pci"?

Thierry

> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 8fdc7934d4c9..d08a63132c77 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2197,6 +2197,9 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>  		unsigned int index;
>  		u32 value;
>  
> +		if (!of_node_is_type(port, "pci"))
> +			continue;
> +
>  		err = of_pci_get_devfn(port);
>  		if (err < 0) {
>  			dev_err(dev, "failed to parse address: %d\n", err);
> -- 
> 2.17.1
>
Manikanta Maddireddy - April 15, 2019, 3:30 p.m.
On 15-Apr-19 7:07 PM, Thierry Reding wrote:
> On Thu, Apr 11, 2019 at 10:33:46PM +0530, Manikanta Maddireddy wrote:
>> Each root port is added as a child device tree node of PCIe controller
>> node. These child nodes are parsed using open firmware PCI bus accessor
>> functions. If the child node is not of "pci" type then device tree
>> parsing fails. Add "pci" type check before parsing child device tree node.
>>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>> ---
>>  drivers/pci/controller/pci-tegra.c | 3 +++
>>  1 file changed, 3 insertions(+)
> Erm... what is the use-case that you're trying to support? Why would we
> ever have children nodes that are not of type "pci"?
>
> Thierry
In downstream kernel we have "prod-settings" node as one of the child nodes.
Even though we are not supporting this in upstream kernel, I believe this
check is good to have.

Manikanta
>> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
>> index 8fdc7934d4c9..d08a63132c77 100644
>> --- a/drivers/pci/controller/pci-tegra.c
>> +++ b/drivers/pci/controller/pci-tegra.c
>> @@ -2197,6 +2197,9 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>>  		unsigned int index;
>>  		u32 value;
>>  
>> +		if (!of_node_is_type(port, "pci"))
>> +			continue;
>> +
>>  		err = of_pci_get_devfn(port);
>>  		if (err < 0) {
>>  			dev_err(dev, "failed to parse address: %d\n", err);
>> -- 
>> 2.17.1
>>
Thierry Reding - April 15, 2019, 3:42 p.m.
On Mon, Apr 15, 2019 at 09:00:58PM +0530, Manikanta Maddireddy wrote:
> 
> 
> On 15-Apr-19 7:07 PM, Thierry Reding wrote:
> > On Thu, Apr 11, 2019 at 10:33:46PM +0530, Manikanta Maddireddy wrote:
> >> Each root port is added as a child device tree node of PCIe controller
> >> node. These child nodes are parsed using open firmware PCI bus accessor
> >> functions. If the child node is not of "pci" type then device tree
> >> parsing fails. Add "pci" type check before parsing child device tree node.
> >>
> >> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> >> ---
> >>  drivers/pci/controller/pci-tegra.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> > Erm... what is the use-case that you're trying to support? Why would we
> > ever have children nodes that are not of type "pci"?
> >
> > Thierry
> In downstream kernel we have "prod-settings" node as one of the child nodes.
> Even though we are not supporting this in upstream kernel, I believe this
> check is good to have.

We don't have prod-settings upstream and for good reason. There's no
need to add this check in upstream. We can carry the change downstream
until we've moved away from prod-settings downstream as well.

Thierry

Patch

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 8fdc7934d4c9..d08a63132c77 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2197,6 +2197,9 @@  static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 		unsigned int index;
 		u32 value;
 
+		if (!of_node_is_type(port, "pci"))
+			continue;
+
 		err = of_pci_get_devfn(port);
 		if (err < 0) {
 			dev_err(dev, "failed to parse address: %d\n", err);