linux-kernel - Re: [PATCH v6 2/2] PCI: altera: support dt binding update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.22.394.2406120744350.662691@sj-4150-psse-sw-opae-dev2>
Date: Wed, 12 Jun 2024 08:12:05 -0700 (PDT)
From: matthew.gerlach@...ux.intel.com
To: Bjorn Helgaas <helgaas@...nel.org>
cc: lpieralisi@...nel.org, kw@...ux.com, robh@...nel.org, bhelgaas@...gle.com, 
    krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, 
    joyce.ooi@...el.com, linux-pci@...r.kernel.org, devicetree@...r.kernel.org, 
    linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 2/2] PCI: altera: support dt binding update



On Tue, 11 Jun 2024, Bjorn Helgaas wrote:

> On Tue, Jun 11, 2024 at 11:35:25AM -0500, matthew.gerlach@...ux.intel.com wrote:
>> From: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
>>
>> Add support for the device tree binding update. As part of
>> converting the binding document from text to yaml, with schema
>> validation, a device tree subnode was added to properly map
>> legacy interrupts. Maintain backward compatibility with previous binding.
>
> If something was *added* to the binding, I think it would be helpful
> to split that into two patches: (1) convert to YAML with zero
> functional changes, (2) add the new stuff.  Adding something at the
> same time as changing the format makes it hard to review.

Thanks for feedback. It was during the conversion to YAML that a problem 
with the original binding was discovered. As Rob Herring pointed out in
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240513205913.313592-1-matthew.gerlach@linux.intel.com/

"Making the PCI host the interrupt parent didn't even work in the kernel
  until somewhat recently (maybe a few years now). That's why a bunch of PCI
  hosts have an interrupt-controller child node."

This was an attempt to fix the problem. I can resubmit a conversion to YAML 
with zero functional changes.

Matthew Gerlach


>
> Then we could have a more specific subject and commit log for *this*
> patch.
>
>> Signed-off-by: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
>> ---
>>  drivers/pci/controller/pcie-altera.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
>> index a9536dc4bf96..88511fa2f078 100644
>> --- a/drivers/pci/controller/pcie-altera.c
>> +++ b/drivers/pci/controller/pcie-altera.c
>> @@ -667,11 +667,20 @@ static void altera_pcie_isr(struct irq_desc *desc)
>>  static int altera_pcie_init_irq_domain(struct altera_pcie *pcie)
>>  {
>>  	struct device *dev = &pcie->pdev->dev;
>> -	struct device_node *node = dev->of_node;
>> +	struct device_node *node, *child;
>>
>>  	/* Setup INTx */
>> +	child = of_get_next_child(dev->of_node, NULL);
>> +	if (child)
>> +		node = child;
>> +	else
>> +		node = dev->of_node;
>> +
>>  	pcie->irq_domain = irq_domain_add_linear(node, PCI_NUM_INTX,
>> -					&intx_domain_ops, pcie);
>> +						 &intx_domain_ops, pcie);
>> +	if (child)
>> +		of_node_put(child);
>> +
>>  	if (!pcie->irq_domain) {
>>  		dev_err(dev, "Failed to get a INTx IRQ domain\n");
>>  		return -ENOMEM;
>> --
>> 2.34.1
>>
>