Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver

From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Sergio Paracuellos <sergio.paracuellos@gmail.com>, robh+dt@kernel.org
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	linux-pci <linux-pci@vger.kernel.org>,
	"open list:MIPS" <linux-mips@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	John Crispin <john@phrozen.org>, NeilBrown <neil@brown.name>,
	Greg KH <gregkh@linuxfoundation.org>,
	linux-staging@lists.linux.dev,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>, Bjorn Helgaas <bhelgaas@google.com>
Subject: Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
Date: Fri, 22 Oct 2021 09:34:55 +0100	[thread overview]
Message-ID: <20211022083455.GA20345@lpieralisi> (raw)
In-Reply-To: <CAMhs-H8pTmbG0idbPWjnW4faFj0F4TKwSSK6wzwepbqWSEtx4w@mail.gmail.com>

On Thu, Oct 21, 2021 at 09:23:35PM +0200, Sergio Paracuellos wrote:
> On Thu, Oct 21, 2021 at 8:11 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Thu, Oct 21, 2021 at 07:27:21PM +0200, Sergio Paracuellos wrote:
> > > On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > Since this is a PCIe (not conventional PCI) controller, I vote for
> > > > renaming these from:
> > > >
> > > >   PCI_MT7621
> > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
> > > >   drivers/pci/controller/pci-mt7621.c
> > > >
> > > > to:
> > > >
> > > >   PCIE_MT7621
> > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> > > >   drivers/pci/controller/pcie-mt7621.c
> > > >
> > > > We have a mix of these, with many of the early PCIe drivers being
> > > > named "pci", but I think that was my mistake and there's no reason to
> > > > continue it.
> > >
> > > I see.
> > >
> > > >
> > > > I can do this locally unless somebody objects.
> > >
> > > I have no problem at all. Only one question. Do you mean to change
> > > compatible string also, or only the name of the file? Let me know if I
> > > have to do anything.
> >
> > I didn't change the compatible string, to avoid a DT incompatibility.
> > But I *did* change the Kconfig symbol to PCIE_MT7621, which could
> > require changes to out-of-tree .configs.  I'm open to suggestions
> > either way for both things.
> 
> IMHO, I do think we should not worry about out-of-tree stuff at all.

For Kconfig I tend to agree. For DT I see some "bindings" in the staging
tree are being deleted and published as official DT bindings with this
patchset but I believe we still have to keep the compatible string
backward compatibility regardless because there may be firmware out
there using it.

Rob, what's the standard policy that should be used in this case ?

Thanks,
Lorenzo

> If the correct way to define the Kconfig symbol or the compatible
> string is to change them, just do that. MT7621 SoC is extensively used
> by openWRT community. As far as I have seen until now, the way of
> doing things there is to take the latest long term kernel (now they
> are using 5.4 as stable and 5.10 as testing kernel), apply a bunch of
> patches they have and do a complete build of both kernel, device tree
> and rootfs. So I guess it is not a big problem if we also change
> compatible string since when an update is performed for a device all
> of the stuff is just replaced. Maybe I am wrong and John has a
> different opinion... John, any comments on this?
> 
> Best regards,
>     Sergio Paracuellos