Received-SPF: pass (google.com: domain of linux-kernel+bounces-213971-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 13 Jun 2024 13:13:26 -0700 (PDT)
From: matthew.gerlach@linux.intel.com
To: Rob Herring <robh@kernel.org>
cc: Bjorn Helgaas <helgaas@kernel.org>, lpieralisi@kernel.org, kw@linux.com, 
    bhelgaas@google.com, krzysztof.kozlowski+dt@linaro.org, 
    conor+dt@kernel.org, joyce.ooi@intel.com, linux-pci@vger.kernel.org, 
    devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/2] PCI: altera: support dt binding update
In-Reply-To: <20240613175131.GA2070202-robh@kernel.org>
Message-ID: <alpine.DEB.2.22.394.2406131301550.662691@sj-4150-psse-sw-opae-dev2>
References: <20240611170410.GA989554@bhelgaas> <alpine.DEB.2.22.394.2406120744350.662691@sj-4150-psse-sw-opae-dev2> <20240613175131.GA2070202-robh@kernel.org>
User-Agent: Alpine 2.22 (DEB 394 2020-01-19)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII; format=flowed


On Thu, 13 Jun 2024, Rob Herring wrote:

> On Wed, Jun 12, 2024 at 08:12:05AM -0700, matthew.gerlach@linux.intel.com wrote:
>>
>>
>> On Tue, 11 Jun 2024, Bjorn Helgaas wrote:
>>
>>> On Tue, Jun 11, 2024 at 11:35:25AM -0500, matthew.gerlach@linux.intel.com wrote:
>>>> From: Matthew Gerlach <matthew.gerlach@linux.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.
>
> The policy for conversions is changes to match reality are fine, just
> need to be noted in the commit message. That generally implies no driver
> or dts changes which is not the case here.
>
>> 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.
>
> I wasn't suggesting you fix it. Just something I noticed looking at
> the other issue. If no one noticed or cared, why bother? It should work
> fine for recent kernels.

Thanks for the feedback. I can resubmit a single conversion commit that 
passes the schema check by adding 3 address fields as you suggested. I 
will also mention this slight modification in the commit message.

Matthew

>
> Rob
>