| Message ID | 20211229160245.1338-1-jszhang@kernel.org |
|---|---|
| Headers | show |
| Series | arm/arm64: dts: Remove unused num-viewport from pcie node | expand |
> From: Jisheng Zhang <jszhang@kernel.org> > Date: Thu, 30 Dec 2021 00:02:38 +0800 > > After commit 281f1f99cf3a("PCI: dwc: Detect number of iATU windows"), > the number of iATU windows is detected at runtime, what's more, > the 'num-viewport' property parsing has been removed, so remove the > unused num-viewport from pcie node(s). > > It's too late for linux-5.17-rc1, I will rebase and send out v2 if > necessary when 5.17-rc1 is released. Please no. This only makes the device trees unnecessarily incompatible with older kernels and other OSes that do rely on the "num-viewport" property. It really doesn't hurt to keep this property even if future Linux kernels no longer look at it. Thanks, Mark > Jisheng Zhang (7): > ARM: dts: ls1021a: remove unused num-viewport from pcie nodes > arm64: dts: visconti: Remove unused num-viewport from pcie node > arm64: dts: uniphier: Remove unused num-viewport from pcie node > arm64: tegra: Remove unused num-viewport from pcie node > arm64: dts: marvell: Remove unused num-viewport from pcie node > arm64: dts: freescale: Remove unused num-viewport from pcie node > arm64: dts: exynos: Remove unused num-viewport from pcie node > > arch/arm/boot/dts/ls1021a.dtsi | 2 -- > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1 - > arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 - > arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 2 -- > arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 3 --- > arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 3 --- > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 3 --- > arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ---- > arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi | 1 - > arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts | 1 - > arch/arm64/boot/dts/marvell/cn9130-crb-A.dts | 1 - > arch/arm64/boot/dts/marvell/cn9130-crb-B.dts | 1 - > arch/arm64/boot/dts/marvell/cn9130-db.dtsi | 1 - > arch/arm64/boot/dts/marvell/cn9131-db.dtsi | 1 - > arch/arm64/boot/dts/marvell/cn9132-db.dtsi | 2 -- > arch/arm64/boot/dts/nvidia/tegra194.dtsi | 6 ------ > arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi | 1 - > arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi | 1 - > arch/arm64/boot/dts/toshiba/tmpv7708.dtsi | 1 - > 19 files changed, 36 deletions(-) > > -- > 2.34.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 29/12/2021 17:50, Mark Kettenis wrote: >> From: Jisheng Zhang <jszhang@kernel.org> >> Date: Thu, 30 Dec 2021 00:02:38 +0800 >> >> After commit 281f1f99cf3a("PCI: dwc: Detect number of iATU windows"), >> the number of iATU windows is detected at runtime, what's more, >> the 'num-viewport' property parsing has been removed, so remove the >> unused num-viewport from pcie node(s). >> >> It's too late for linux-5.17-rc1, I will rebase and send out v2 if >> necessary when 5.17-rc1 is released. > > Please no. This only makes the device trees unnecessarily > incompatible with older kernels Anyone who is running a new DTB with older kernel is doomed anyway, not only because of this change but hundreds of other similar cleanups, e.g. making DTS conforming to dtschema. Are you sure there are such use cases of using new DTB with old kernel? I cannot imagine making a stable product with such scenario... > and other OSes that do rely on the > "num-viewport" property. Right. We should have move the DTS out of the kernel when it was still small. :) > It really doesn't hurt to keep this property > even if future Linux kernels no longer look at it. For Exynos PCIe, the property is still required by bindings, so definitely it cannot be removed from DTS. I did not check the other bindings. Best regards, Krzysztof
> Date: Fri, 7 Jan 2022 13:47:03 +0100 > From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > > On 29/12/2021 17:50, Mark Kettenis wrote: > >> From: Jisheng Zhang <jszhang@kernel.org> > >> Date: Thu, 30 Dec 2021 00:02:38 +0800 > >> > >> After commit 281f1f99cf3a("PCI: dwc: Detect number of iATU windows"), > >> the number of iATU windows is detected at runtime, what's more, > >> the 'num-viewport' property parsing has been removed, so remove the > >> unused num-viewport from pcie node(s). > >> > >> It's too late for linux-5.17-rc1, I will rebase and send out v2 if > >> necessary when 5.17-rc1 is released. > > > > Please no. This only makes the device trees unnecessarily > > incompatible with older kernels > > Anyone who is running a new DTB with older kernel is doomed anyway, not > only because of this change but hundreds of other similar cleanups, e.g. > making DTS conforming to dtschema. Are you sure there are such use cases > of using new DTB with old kernel? I cannot imagine making a stable > product with such scenario... Well, many of those changes just affect the node names, which aren't part of the ABI. And adding missing properties or compatibles doesn't break things either. But yes, we keep seeing diffs to "cleanup" bindings and device trees, especially in the context of converting them to dtschema. And that's just wrong. If old device trees don't pass validation, the default assumption should be that the schema is wrong; not the other way around. > > and other OSes that do rely on the > > "num-viewport" property. > > Right. We should have move the DTS out of the kernel when it was still > small. :) I don't think the number/size of DTs really matters. But yes, moving them to a separate repository would certainly make it more obvious that they should not be tied to a particular kernel version and that they are shared with other projects such as U-Boot and other OSes. > > It really doesn't hurt to keep this property > > even if future Linux kernels no longer look at it. > > For Exynos PCIe, the property is still required by bindings, so > definitely it cannot be removed from DTS. I did not check the other > bindings. > > Best regards, > Krzysztof Cheers, Mark
On 07/01/2022 20:39, Mark Kettenis wrote: >> Date: Fri, 7 Jan 2022 13:47:03 +0100 >> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >> >> On 29/12/2021 17:50, Mark Kettenis wrote: >>>> From: Jisheng Zhang <jszhang@kernel.org> >>>> Date: Thu, 30 Dec 2021 00:02:38 +0800 >>>> >>>> After commit 281f1f99cf3a("PCI: dwc: Detect number of iATU windows"), >>>> the number of iATU windows is detected at runtime, what's more, >>>> the 'num-viewport' property parsing has been removed, so remove the >>>> unused num-viewport from pcie node(s). >>>> >>>> It's too late for linux-5.17-rc1, I will rebase and send out v2 if >>>> necessary when 5.17-rc1 is released. >>> >>> Please no. This only makes the device trees unnecessarily >>> incompatible with older kernels >> >> Anyone who is running a new DTB with older kernel is doomed anyway, not >> only because of this change but hundreds of other similar cleanups, e.g. >> making DTS conforming to dtschema. Are you sure there are such use cases >> of using new DTB with old kernel? I cannot imagine making a stable >> product with such scenario... > > Well, many of those changes just affect the node names, which aren't > part of the ABI. And adding missing properties or compatibles doesn't > break things either. But yes, we keep seeing diffs to "cleanup" > bindings and device trees, especially in the context of converting > them to dtschema. And that's just wrong. If old device trees don't > pass validation, the default assumption should be that the schema is > wrong; not the other way around. I cannot get how you reached a conclusion that old device tree could be good, but old bindings would be bad... Both were developed without consistency, sometimes without proper review. Simply both can be wrong and now we fix them - the bindings by converting to stricter schema and DTS files by aligning them with new schema. There was never a contract between us and users that OLD kernel will work with NEW DTB. The only contract we made was the other way around - NEW kernel will work with OLD DTB. I understand that it is useful to have new DTB working with old kernel. I consider it as a "nice to have" feature but: 1. Still there are no real users of such pattern (new DTB with old kernel), around Linux kernel. If they are - I am repaeting - their Linux project is already broken. 2. If Linux drivers or other projects depend on node names and anything not being part of bindings (the ABI), they are broken by design. They should either be fixed or accept that might get broken anytime soon because they do not use bindings but undocumented parts (which are not ABI). 3. "Nice to have" should not stop us in improving out codebase and making it easier to maintain for us. We do not make these "dtschema align" changes for pure fun, but to make everything easier for us in the longterm. The dtschema checks I was running (and converting to dtschema) already found errors in DTS. These are real bugs which are fixed by this stricter dtschema. Best regards, Krzysztof
After commit 281f1f99cf3a("PCI: dwc: Detect number of iATU windows"), the number of iATU windows is detected at runtime, what's more, the 'num-viewport' property parsing has been removed, so remove the unused num-viewport from pcie node(s). It's too late for linux-5.17-rc1, I will rebase and send out v2 if necessary when 5.17-rc1 is released. Jisheng Zhang (7): ARM: dts: ls1021a: remove unused num-viewport from pcie nodes arm64: dts: visconti: Remove unused num-viewport from pcie node arm64: dts: uniphier: Remove unused num-viewport from pcie node arm64: tegra: Remove unused num-viewport from pcie node arm64: dts: marvell: Remove unused num-viewport from pcie node arm64: dts: freescale: Remove unused num-viewport from pcie node arm64: dts: exynos: Remove unused num-viewport from pcie node arch/arm/boot/dts/ls1021a.dtsi | 2 -- arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1 - arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 - arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 2 -- arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 3 --- arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 3 --- arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 3 --- arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ---- arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi | 1 - arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts | 1 - arch/arm64/boot/dts/marvell/cn9130-crb-A.dts | 1 - arch/arm64/boot/dts/marvell/cn9130-crb-B.dts | 1 - arch/arm64/boot/dts/marvell/cn9130-db.dtsi | 1 - arch/arm64/boot/dts/marvell/cn9131-db.dtsi | 1 - arch/arm64/boot/dts/marvell/cn9132-db.dtsi | 2 -- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 6 ------ arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi | 1 - arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi | 1 - arch/arm64/boot/dts/toshiba/tmpv7708.dtsi | 1 - 19 files changed, 36 deletions(-)