From: Rob Herring <robh@kernel.org>
To: matthew.gerlach@linux.intel.com
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
Date: Thu, 13 Jun 2024 11:51:31 -0600 [thread overview]
Message-ID: <20240613175131.GA2070202-robh@kernel.org> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2406120744350.662691@sj-4150-psse-sw-opae-dev2>
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.
Rob
next prev parent reply other threads:[~2024-06-13 17:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-11 16:35 [PATCH v6 1/2] dt-bindings: PCI: altera: Convert to YAML matthew.gerlach
2024-06-11 16:35 ` [PATCH v6 2/2] PCI: altera: support dt binding update matthew.gerlach
2024-06-11 17:04 ` Bjorn Helgaas
2024-06-12 15:12 ` matthew.gerlach
2024-06-13 17:51 ` Rob Herring [this message]
2024-06-13 20:13 ` matthew.gerlach
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240613175131.GA2070202-robh@kernel.org \
--to=robh@kernel.org \
--cc=bhelgaas@google.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=helgaas@kernel.org \
--cc=joyce.ooi@intel.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=kw@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=matthew.gerlach@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).