On Fri, Oct 22, 2021 at 11:13:39AM +0200, Sergio Paracuellos wrote:

On Fri, Oct 22, 2021 at 10:35 AM Lorenzo Pieralisi
[off-list ref] wrote:

quoted

On Thu, Oct 21, 2021 at 09:23:35PM +0200, Sergio Paracuellos wrote:

quoted

On Thu, Oct 21, 2021 at 8:11 PM Bjorn Helgaas [off-list ref] wrote:

quoted

On Thu, Oct 21, 2021 at 07:27:21PM +0200, Sergio Paracuellos wrote:

quoted

On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas [off-list ref] wrote:

quoted

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.

quoted

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.

The bindings txt file removed in staging with this patchset was also
added by me three years ago[0], and has been changing until the YAML
bindings are reviewed by Rob and driver updated accordly in this
patchset.

OpenWRT maintains its own file[1] which I don't know is updated or
not according to the one in staging which I am pretending to
properly mainline for 5.17. But yes, I agree there might be firmware
out there using current compatible string.

[0]: Commit 5451e22618b8 ("staging: mt7621-pci: dt-bindings: add dt
bindings for mt7621 pcie controller")
[1]: https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/dts/mt7621.dtsi

OK, for now I left my rework as-is:

  - changed CONFIG_PCI_MT7621 to CONFIG_PCIE_MT7621
  - renamed mediatek,mt7621-pci.yaml to mediatek,mt7621-pcie.yaml
  - renamed pci-mt7621.c to pcie-mt7621.c
  - kept DT compatible string "mediatek,mt7621-pci" in .yaml and .c

I reason that the Kconfig and filename changes only affect people
building kernels or DTs, but a compatible string change would force a
DT update to be synchronized with a kernel update.

Happy to change this if necessary.

Bjorn