| Message ID | 20201014101402.18271-1-Sergey.Semin@baikalelectronics.ru |
|---|---|
| Headers | show |
| Series | dt-bindings: usb: Add generic USB HCD, xHCI, DWC USB3 DT schema | expand |
On Wed, 14 Oct 2020 at 12:23, Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote: > > In accordance with the DWC USB3 bindings the corresponding node name is > suppose to comply with Generic USB HCD DT schema, which requires the USB > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause > the dtbs_check procedure failure. Let's fix the nodes naming to be > compatible with the DWC USB3 DT schema to make dtbs_check happy. > > Note we don't change the DWC USB3-compatible nodes names of > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the > in-source comment says that the nodes name need to be preserved as > "^dwusb@.*" for some backward compatibility. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Please, test the patch out to make sure it doesn't brake the dependent DTS > files. I did only a manual grepping of the possible nodes dependencies. 1. It is you who should compare the decompiled DTS, not us. For example: $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64 scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64 fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt dts-new/${i#dts-old/}.fdt ; done 2. Split it per arm architectures (and proper subject prefix - not "arch") and subarchitectures so maintainers can pick it up. 3. The subject title could be more accurate - there is no fix here because there was no errors in the first place. Requirement of DWC node names comes recently, so it is more alignment with dtschema. Otherwise automatic-pickup-stable-bot might want to pick up... and it should not go to stable. Best regards, Krzysztof > arch/arm/boot/dts/armada-375.dtsi | 2 +- > arch/arm/boot/dts/exynos5250.dtsi | 2 +- > arch/arm/boot/dts/exynos54xx.dtsi | 4 ++-- > arch/arm/boot/dts/keystone-k2e.dtsi | 4 ++-- > arch/arm/boot/dts/keystone.dtsi | 2 +- > arch/arm/boot/dts/ls1021a.dtsi | 2 +- > arch/arm/boot/dts/omap5-l4.dtsi | 2 +- > arch/arm/boot/dts/stih407-family.dtsi | 2 +- > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +- > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 ++-- > arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +- > arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 ++-- > arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++--- > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++-- > arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++-- > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +- > arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 ++-- > arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++-- > arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++-- > arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +- > arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +- > arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++-- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++-- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +- > 25 files changed, 38 insertions(+), 38 deletions(-) >
Hi Serge, Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > In accordance with the DWC USB3 bindings the corresponding node name is > suppose to comply with Generic USB HCD DT schema, which requires the USB DWC3 is not a simple HDC, though. > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause > the dtbs_check procedure failure. Let's fix the nodes naming to be > compatible with the DWC USB3 DT schema to make dtbs_check happy. > > Note we don't change the DWC USB3-compatible nodes names of > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the > in-source comment says that the nodes name need to be preserved as > "^dwusb@.*" for some backward compatibility. interesting, compatibility with what? Some debugfs files, perhaps? :-) In any case, I don't have any problems with this, so I'll let other folks comment.
On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: > > Hi Serge, > > Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > > In accordance with the DWC USB3 bindings the corresponding node name is > > suppose to comply with Generic USB HCD DT schema, which requires the USB > > DWC3 is not a simple HDC, though. Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, which are tuned by the DWC USB3 driver in the kernel. But after that the controller is registered as xhci-hcd device so it's serviced by the xHCI driver, which then registers the HCD device so the corresponding DT node is supposed to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted compatibility. > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause > > the dtbs_check procedure failure. Let's fix the nodes naming to be > > compatible with the DWC USB3 DT schema to make dtbs_check happy. > > > > Note we don't change the DWC USB3-compatible nodes names of > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the > > in-source comment says that the nodes name need to be preserved as > > "^dwusb@.*" for some backward compatibility. > > interesting, compatibility with what? Some debugfs files, perhaps? :-) Don't really know.) In my experience the worst type of such compatibility is connected with some bootloader magic, which may add/remove/modify properties to nodes with pre-defined names. -Sergey > > In any case, I don't have any problems with this, so I'll let other > folks comment. > > -- > balbi
On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote: > On Wed, 14 Oct 2020 at 12:23, Serge Semin > <Sergey.Semin@baikalelectronics.ru> wrote: > > > > In accordance with the DWC USB3 bindings the corresponding node name is > > suppose to comply with Generic USB HCD DT schema, which requires the USB > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause > > the dtbs_check procedure failure. Let's fix the nodes naming to be > > compatible with the DWC USB3 DT schema to make dtbs_check happy. > > > > Note we don't change the DWC USB3-compatible nodes names of > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the > > in-source comment says that the nodes name need to be preserved as > > "^dwusb@.*" for some backward compatibility. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Please, test the patch out to make sure it doesn't brake the dependent DTS > > files. I did only a manual grepping of the possible nodes dependencies. > > 1. It is you who should compare the decompiled DTS, not us. For example: > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64 > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64 > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} > > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt > dts-new/${i#dts-old/}.fdt ; done So basically you suggest first to compile the old and new dts files, then to de-compile them, then diff old and new fdt's, and visually compare the results. Personally it isn't that much better than what I did, since each old and new dtbs will for sure differ due to the node names change suggested in this patch. So it will lead to the visual debugging too, which isn't that effective. But your approach is still more demonstrative to make sure that I didn't loose any nodes redefinition, since in the occasion the old and new de-compiled nodes will differ not only by the node names but with an additional old named node. So to speak thanks for suggesting it. I'll try it to validate the proposed changes. Two questions: 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I can get all the updated dtsi'es, then find out all the dts'es which include them, then directly use dtc to compile the found dts'es... On the other hand I can just compile all dts'es, then compare old and new ones. The diff of the non-modified dtb'es will be just empty... 2) What crosc64 is? > > 2. Split it per arm architectures (and proper subject prefix - not > "arch") and subarchitectures so maintainers can pick it up. Why? The changes are simple and can be formatted as a single patch. I've seen tons of patches submitted like that, accepted and then merged. What you suggest is just much more work, which I don't see quite required. > > 3. The subject title could be more accurate - there is no fix here > because there was no errors in the first place. Requirement of DWC > node names comes recently, so it is more alignment with dtschema. > Otherwise automatic-pickup-stable-bot might want to pick up... and it > should not go to stable. Actually it is a fix, because the USB DT nodes should have been named with "usb" prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was wrong in the first place. Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the emails, but most likely the stable@vger.kernel.org list only or the emails having the "Fixes:" tag. If I am wrong please give me a link to the bot sources or refer to a doc where I can read about the way it works, to take it into account in future commits. Just to note I submitted patches which did some fixes, had the word "fix" in the subject but weren't selected to be backported to the stable kernel. Anyway I don't really care that much about the subject text using the word "fix" or some else. So if you suggest some better alternative, I'd be glad to consider it. -Sergey > > Best regards, > Krzysztof > > > arch/arm/boot/dts/armada-375.dtsi | 2 +- > > arch/arm/boot/dts/exynos5250.dtsi | 2 +- > > arch/arm/boot/dts/exynos54xx.dtsi | 4 ++-- > > arch/arm/boot/dts/keystone-k2e.dtsi | 4 ++-- > > arch/arm/boot/dts/keystone.dtsi | 2 +- > > arch/arm/boot/dts/ls1021a.dtsi | 2 +- > > arch/arm/boot/dts/omap5-l4.dtsi | 2 +- > > arch/arm/boot/dts/stih407-family.dtsi | 2 +- > > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +- > > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 ++-- > > arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +- > > arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 ++-- > > arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++--- > > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++-- > > arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++-- > > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +- > > arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 ++-- > > arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++-- > > arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++-- > > arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +- > > arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +- > > arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++-- > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++-- > > arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +- > > 25 files changed, 38 insertions(+), 38 deletions(-) > >
Ah, forgot to mark the series as v2. Sorry about that. The next one will be v3 then... -Sergey On Wed, Oct 14, 2020 at 01:13:42PM +0300, Serge Semin wrote: > We've performed some work on the Generic USB HCD, xHCI and DWC USB3 DT > bindings in the framework of the Baikal-T1 SoC support integration into > the kernel. This patchset is a result of that work. > > First of all we moved the generic USB properties from the legacy text > bindings into the USB HCD DT schema. So now the generic USB HCD-compatible > DT nodes are validated taking into account the optional properties like: > maximum-speed, dr_mode, otg-rev, usb-role-switch, etc. We've fixed these > properties a bit so they would correspond to what functionality kernel > currently supports. > > Secondly we converted generic USB xHCI text bindings file into the DT > schema. It had to be split up into two bindings: DT schema with generic > xHCI properties and a generic xHCI device DT schema. The later will be > used to validate the pure xHCI-based nodes, while the former can be > utilized by some vendor-specific versions of xHCI. > > Thirdly, what was primarily intended to be done for Baikal-T1 SoC USB we > converted the legacy text-based DWC USB3 bindings to DT schema and altered > the result a bit so it would be more coherent with what actually > controller and its driver support. Since we've now got the DWC USB3 DT > schema, we made it used to validate the sub-nodes of the Qualcom, TI and > Amlogic DWC3 DT nodes. > > Finally we've also fixed all the OHCI/EHCI, xHCI and DW USB3 compatible DT > nodes so they would comply with the nodes naming scema declared in the USB > HCD DT bindings file. > > Link: https://lore.kernel.org/linux-usb/20201010224121.12672-1-Sergey.Semin@baikalelectronics.ru/ > Changelog v2: > - Thanks to Sergei Shtylyov for suggesting the commit logs grammar fixes: > [PATCH 04/18] dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types > [PATCH 05/18] dt-bindings: usb: usb-hcd: Add "tpl-support" property > [PATCH 11/18] dt-bindings: usb: dwc3: Add interrupt-names property support > [PATCH 13/18] dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions > [PATCH 17/18] dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node > - Set FL-adj of the amlogiv,meson-g12a-usb controller with value 0x20 instead > of completely removing the property. > - Drop the patch: > [PATCH 02/18] dt-bindings: usb: usb-hcd: Add "wireless" maximum-speed > property value > since "wireless" speed type is depracated due to lack of the device > supporting it. > - Drop quotes from around the compat string constant. > - Discard '|' from the property descriptions, since we don't need to preserve > the text formatting. > - Convert abbreviated form of the "maximum-speed" enum constraint into > the multi-lined version of the list. > - Fix the DW USB3 "clock-names" prop description to be refererring to the > enumerated clock-names instead of the ones from the Databook. > - Add explicit "additionalProperties: true" to the usb-xhci.yaml schema, > since additionalProperties/unevaluatedProperties are going to be mandary > for each binding. > - Use "oneOf: [dwc2.yaml#, snps,dwc3.yaml#]" instead of the bulky "if: > properties: compatibe: ..." statement. > - Discard the "^dwc3@[0-9a-f]+$" nodes from being acceptable as sub-nodes > of the Qualcomm DWC3 DT nodes. > - Add new patches: > [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name > [PATCH 19/20] arch: dts: Fix xHCI DT nodes name > [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru> > Cc: Andy Gross <agross@kernel.org> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Manu Gautam <mgautam@codeaurora.org> > Cc: Roger Quadros <rogerq@ti.com> > Cc: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > Cc: Neil Armstrong <narmstrong@baylibre.com> > Cc: Kevin Hilman <khilman@baylibre.com> > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-snps-arc@lists.infradead.org > Cc: linux-mips@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-usb@vger.kernel.org > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > > Serge Semin (20): > dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema > dt-bindings: usb: usb-hcd: Add "otg-rev" property restriction > dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types > dt-bindings: usb: usb-hcd: Add "tpl-support" property > dt-bindings: usb: usb-hcd: Add generic "usb-phy" property > dt-bindings: usb: Convert xHCI bindings to DT schema > dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device > dt-bindings: usb: renesas-xhci: Refer to the usb-xhci.yaml file > dt-bindings: usb: Convert DWC USB3 bindings to DT schema > dt-bindings: usb: dwc3: Add interrupt-names property support > dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string > dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions > dt-bindings: usb: dwc3: Add Frame Length Adj restrictions > dt-bindings: usb: meson-g12a-usb: Fix FL-adj property value > dt-bindings: usb: meson-g12a-usb: Validate DWC2/DWC3 sub-nodes > dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node > dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node > arch: dts: Fix EHCI/OHCI DT nodes name > arch: dts: Fix xHCI DT nodes name > arch: dts: Fix DWC USB3 DT nodes name > > .../usb/amlogic,meson-g12a-usb-ctrl.yaml | 6 +- > .../devicetree/bindings/usb/dwc3.txt | 125 ------- > .../devicetree/bindings/usb/generic-xhci.yaml | 65 ++++ > .../devicetree/bindings/usb/generic.txt | 57 ---- > .../devicetree/bindings/usb/qcom,dwc3.yaml | 9 +- > .../bindings/usb/renesas,usb-xhci.yaml | 4 +- > .../devicetree/bindings/usb/snps,dwc3.yaml | 315 ++++++++++++++++++ > .../bindings/usb/ti,keystone-dwc3.yaml | 4 +- > .../devicetree/bindings/usb/usb-hcd.yaml | 104 ++++++ > .../devicetree/bindings/usb/usb-xhci.txt | 41 --- > .../devicetree/bindings/usb/usb-xhci.yaml | 42 +++ > arch/arc/boot/dts/axs10x_mb.dtsi | 4 +- > arch/arc/boot/dts/hsdk.dts | 4 +- > arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +- > arch/arm/boot/dts/armada-375.dtsi | 2 +- > arch/arm/boot/dts/bcm5301x.dtsi | 6 +- > arch/arm/boot/dts/bcm53573.dtsi | 4 +- > arch/arm/boot/dts/exynos5250.dtsi | 2 +- > arch/arm/boot/dts/exynos54xx.dtsi | 4 +- > arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 +- > arch/arm/boot/dts/keystone-k2e.dtsi | 4 +- > arch/arm/boot/dts/keystone.dtsi | 2 +- > arch/arm/boot/dts/lpc18xx.dtsi | 4 +- > arch/arm/boot/dts/ls1021a.dtsi | 2 +- > arch/arm/boot/dts/omap5-l4.dtsi | 2 +- > arch/arm/boot/dts/stih407-family.dtsi | 2 +- > arch/arm/boot/dts/stm32mp151.dtsi | 4 +- > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +- > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 +- > arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +- > .../arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 +- > .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +- > .../arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 +- > .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 +- > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +- > .../arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 4 +- > arch/arm64/boot/dts/hisilicon/hip06.dtsi | 4 +- > arch/arm64/boot/dts/hisilicon/hip07.dtsi | 4 +- > arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 4 +- > arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 +- > arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 +- > arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 +- > arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +- > arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +- > arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 +- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 +- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +- > arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +- > arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +- > arch/mips/boot/dts/mti/sead3.dts | 2 +- > arch/mips/boot/dts/ralink/mt7628a.dtsi | 2 +- > arch/powerpc/boot/dts/akebono.dts | 6 +- > 53 files changed, 605 insertions(+), 305 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt > create mode 100644 Documentation/devicetree/bindings/usb/generic-xhci.yaml > delete mode 100644 Documentation/devicetree/bindings/usb/generic.txt > create mode 100644 Documentation/devicetree/bindings/usb/snps,dwc3.yaml > delete mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.txt > create mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.yaml > > -- > 2.27.0 >
On 10/14/20 3:14 AM, Serge Semin wrote: > In accordance with the Generic EHCI/OHCI bindings the corresponding node > name is suppose to comply with the Generic USB HCD DT schema, which > requires the USB nodes to have the name acceptable by the regexp: > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > incompatible names. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Please, test the patch out to make sure it doesn't brake the dependent DTS > files. I did only a manual grepping of the possible nodes dependencies. Not sure how you envisioned these change to be picked up, but you may need to split these changes between ARM/ARM64, MIPS and PowerPC at least. And within ARM/ARM64 you will most likely have to split according to the various SoC maintainers.
On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote: > On 10/14/20 3:14 AM, Serge Semin wrote: > > In accordance with the Generic EHCI/OHCI bindings the corresponding node > > name is suppose to comply with the Generic USB HCD DT schema, which > > requires the USB nodes to have the name acceptable by the regexp: > > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > > incompatible names. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Please, test the patch out to make sure it doesn't brake the dependent DTS > > files. I did only a manual grepping of the possible nodes dependencies. > > Not sure how you envisioned these change to be picked up, but you may > need to split these changes between ARM/ARM64, MIPS and PowerPC at > least. And within ARM/ARM64 you will most likely have to split according > to the various SoC maintainers. Hmm, I don't really know how it's going to be done in this case, but there must be a way to get the cross-platform patches picked up in general. For instance, see the patches like: 714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread() 140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS They touched the files from different files, but still have been merged in. Maybe I should have copied these three patches to the "linux-arch@vger.kernel.org" list or some other mailing list... -Sergey > -- > Florian
On Wed, Oct 14, 2020 at 9:37 AM Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote: > > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: > > > > Hi Serge, > > > > Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > > > In accordance with the DWC USB3 bindings the corresponding node name is > > > suppose to comply with Generic USB HCD DT schema, which requires the USB > > > > > DWC3 is not a simple HDC, though. > > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, > which are tuned by the DWC USB3 driver in the kernel. But after that the > controller is registered as xhci-hcd device so it's serviced by the xHCI driver, > which then registers the HCD device so the corresponding DT node is supposed > to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml > and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted > compatibility. > > > > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause > > > the dtbs_check procedure failure. Let's fix the nodes naming to be > > > compatible with the DWC USB3 DT schema to make dtbs_check happy. > > > > > > Note we don't change the DWC USB3-compatible nodes names of > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the > > > in-source comment says that the nodes name need to be preserved as > > > "^dwusb@.*" for some backward compatibility. > > > > > interesting, compatibility with what? Some debugfs files, perhaps? :-) > > Don't really know.) In my experience the worst type of such compatibility is > connected with some bootloader magic, which may add/remove/modify properties > to nodes with pre-defined names. I seriously doubt anyone is using the APM machines with DT (even ACPI is somewhat doubtful). I say change them. Or remove the dts files and see what happens. Either way it can always be reverted. Rob
On 10/14/20 11:11 AM, Serge Semin wrote: > On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote: >> On 10/14/20 3:14 AM, Serge Semin wrote: >>> In accordance with the Generic EHCI/OHCI bindings the corresponding node >>> name is suppose to comply with the Generic USB HCD DT schema, which >>> requires the USB nodes to have the name acceptable by the regexp: >>> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with >>> incompatible names. >>> >>> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> >>> >>> --- >>> >>> Please, test the patch out to make sure it doesn't brake the dependent DTS >>> files. I did only a manual grepping of the possible nodes dependencies. >> > >> Not sure how you envisioned these change to be picked up, but you may >> need to split these changes between ARM/ARM64, MIPS and PowerPC at >> least. And within ARM/ARM64 you will most likely have to split according >> to the various SoC maintainers. > > Hmm, I don't really know how it's going to be done in this case, but there must > be a way to get the cross-platform patches picked up in general. For > instance, see the patches like: > 714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread() > 140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS > They touched the files from different files, but still have been merged in. That situation is different, when a new facility is added and someone has gone through the work of adding support for all architectures (or nearly all of them), you want them to be merged in a way that limits merge conflicts with other architecture changes. Here you are fixing warnings, and each file you touch can clearly be independently change from other files in the series without causing merge conflicts. You are however creating the potential for merge conflicts with other changes that the various SoC maintainers have queued up. > Maybe I should have copied these three patches to the "linux-arch@vger.kernel.org" > list or some other mailing list... Maybe Rob can queue them through his device tree repository, with the ack of the various SoC maintainers...
Le mer. 14 oct. 2020 à 13:14, Serge Semin <Sergey.Semin@baikalelectronics.ru> a écrit : > In accordance with the Generic EHCI/OHCI bindings the corresponding > node > name is suppose to comply with the Generic USB HCD DT schema, which > requires the USB nodes to have the name acceptable by the regexp: > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined > with > incompatible names. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Please, test the patch out to make sure it doesn't brake the > dependent DTS > files. I did only a manual grepping of the possible nodes > dependencies. > --- > arch/arc/boot/dts/axs10x_mb.dtsi | 4 ++-- > arch/arc/boot/dts/hsdk.dts | 4 ++-- > arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +- > arch/arm/boot/dts/bcm5301x.dtsi | 4 ++-- > arch/arm/boot/dts/bcm53573.dtsi | 4 ++-- > arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 ++-- > arch/arm/boot/dts/lpc18xx.dtsi | 4 ++-- > arch/arm/boot/dts/stm32mp151.dtsi | 4 ++-- > arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 4 ++-- > arch/arm64/boot/dts/hisilicon/hip06.dtsi | 4 ++-- > arch/arm64/boot/dts/hisilicon/hip07.dtsi | 4 ++-- > arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +- > arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +- For jz4740.dtsi and jz4770.dtsi: Acked-by: Paul Cercueil <paul@crapouillou.net> Cheers, -Paul > arch/mips/boot/dts/mti/sead3.dts | 2 +- > arch/mips/boot/dts/ralink/mt7628a.dtsi | 2 +- > arch/powerpc/boot/dts/akebono.dts | 6 +++--- > 16 files changed, 28 insertions(+), 28 deletions(-) > > diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi > b/arch/arc/boot/dts/axs10x_mb.dtsi > index 99d3e7175bf7..b64435385304 100644 > --- a/arch/arc/boot/dts/axs10x_mb.dtsi > +++ b/arch/arc/boot/dts/axs10x_mb.dtsi > @@ -87,13 +87,13 @@ gmac: ethernet@18000 { > mac-address = [00 00 00 00 00 00]; /* Filled in by U-Boot */ > }; > > - ehci@40000 { > + usb@40000 { > compatible = "generic-ehci"; > reg = < 0x40000 0x100 >; > interrupts = < 8 >; > }; > > - ohci@60000 { > + usb@60000 { > compatible = "generic-ohci"; > reg = < 0x60000 0x100 >; > interrupts = < 8 >; > diff --git a/arch/arc/boot/dts/hsdk.dts b/arch/arc/boot/dts/hsdk.dts > index dcaa44e408ac..fdd4f7f635d3 100644 > --- a/arch/arc/boot/dts/hsdk.dts > +++ b/arch/arc/boot/dts/hsdk.dts > @@ -234,7 +234,7 @@ phy0: ethernet-phy@0 { /* Micrel KSZ9031 */ > }; > }; > > - ohci@60000 { > + usb@60000 { > compatible = "snps,hsdk-v1.0-ohci", "generic-ohci"; > reg = <0x60000 0x100>; > interrupts = <15>; > @@ -242,7 +242,7 @@ ohci@60000 { > dma-coherent; > }; > > - ehci@40000 { > + usb@40000 { > compatible = "snps,hsdk-v1.0-ehci", "generic-ehci"; > reg = <0x40000 0x100>; > interrupts = <15>; > diff --git a/arch/arc/boot/dts/vdk_axs10x_mb.dtsi > b/arch/arc/boot/dts/vdk_axs10x_mb.dtsi > index cbb179770293..90a412026e64 100644 > --- a/arch/arc/boot/dts/vdk_axs10x_mb.dtsi > +++ b/arch/arc/boot/dts/vdk_axs10x_mb.dtsi > @@ -46,7 +46,7 @@ ethernet@18000 { > clock-names = "stmmaceth"; > }; > > - ehci@40000 { > + usb@40000 { > compatible = "generic-ehci"; > reg = < 0x40000 0x100 >; > interrupts = < 8 >; > diff --git a/arch/arm/boot/dts/bcm5301x.dtsi > b/arch/arm/boot/dts/bcm5301x.dtsi > index 0016720ce530..bf5656d79a55 100644 > --- a/arch/arm/boot/dts/bcm5301x.dtsi > +++ b/arch/arm/boot/dts/bcm5301x.dtsi > @@ -261,7 +261,7 @@ usb2: usb2@21000 { > > interrupt-parent = <&gic>; > > - ehci: ehci@21000 { > + ehci: usb@21000 { > #usb-cells = <0>; > > compatible = "generic-ehci"; > @@ -283,7 +283,7 @@ ehci_port2: port@2 { > }; > }; > > - ohci: ohci@22000 { > + ohci: usb@22000 { > #usb-cells = <0>; > > compatible = "generic-ohci"; > diff --git a/arch/arm/boot/dts/bcm53573.dtsi > b/arch/arm/boot/dts/bcm53573.dtsi > index 4af8e3293cff..51546fccc616 100644 > --- a/arch/arm/boot/dts/bcm53573.dtsi > +++ b/arch/arm/boot/dts/bcm53573.dtsi > @@ -135,7 +135,7 @@ usb2: usb2@4000 { > #address-cells = <1>; > #size-cells = <1>; > > - ehci: ehci@4000 { > + ehci: usb@4000 { > compatible = "generic-ehci"; > reg = <0x4000 0x1000>; > interrupt-parent = <&gic>; > @@ -155,7 +155,7 @@ ehci_port2: port@2 { > }; > }; > > - ohci: ohci@d000 { > + ohci: usb@d000 { > #usb-cells = <0>; > > compatible = "generic-ohci"; > diff --git a/arch/arm/boot/dts/hisi-x5hd2.dtsi > b/arch/arm/boot/dts/hisi-x5hd2.dtsi > index 3ee7967c202d..693b85b2cc7d 100644 > --- a/arch/arm/boot/dts/hisi-x5hd2.dtsi > +++ b/arch/arm/boot/dts/hisi-x5hd2.dtsi > @@ -452,14 +452,14 @@ gmac1: ethernet@1841000 { > status = "disabled"; > }; > > - usb0: ehci@1890000 { > + usb0: usb@1890000 { > compatible = "generic-ehci"; > reg = <0x1890000 0x1000>; > interrupts = <0 66 4>; > clocks = <&clock HIX5HD2_USB_CLK>; > }; > > - usb1: ohci@1880000 { > + usb1: usb@1880000 { > compatible = "generic-ohci"; > reg = <0x1880000 0x1000>; > interrupts = <0 67 4>; > diff --git a/arch/arm/boot/dts/lpc18xx.dtsi > b/arch/arm/boot/dts/lpc18xx.dtsi > index 10b8249b8ab6..82ffd7b0ad8a 100644 > --- a/arch/arm/boot/dts/lpc18xx.dtsi > +++ b/arch/arm/boot/dts/lpc18xx.dtsi > @@ -121,7 +121,7 @@ mmcsd: mmcsd@40004000 { > status = "disabled"; > }; > > - usb0: ehci@40006100 { > + usb0: usb@40006100 { > compatible = "nxp,lpc1850-ehci", "generic-ehci"; > reg = <0x40006100 0x100>; > interrupts = <8>; > @@ -133,7 +133,7 @@ usb0: ehci@40006100 { > status = "disabled"; > }; > > - usb1: ehci@40007100 { > + usb1: usb@40007100 { > compatible = "nxp,lpc1850-ehci", "generic-ehci"; > reg = <0x40007100 0x100>; > interrupts = <9>; > diff --git a/arch/arm/boot/dts/stm32mp151.dtsi > b/arch/arm/boot/dts/stm32mp151.dtsi > index bfe29023fbd5..576f7da564c5 100644 > --- a/arch/arm/boot/dts/stm32mp151.dtsi > +++ b/arch/arm/boot/dts/stm32mp151.dtsi > @@ -1404,7 +1404,7 @@ ethernet0: ethernet@5800a000 { > status = "disabled"; > }; > > - usbh_ohci: usbh-ohci@5800c000 { > + usbh_ohci: usb@5800c000 { > compatible = "generic-ohci"; > reg = <0x5800c000 0x1000>; > clocks = <&rcc USBH>; > @@ -1413,7 +1413,7 @@ usbh_ohci: usbh-ohci@5800c000 { > status = "disabled"; > }; > > - usbh_ehci: usbh-ehci@5800d000 { > + usbh_ehci: usb@5800d000 { > compatible = "generic-ehci"; > reg = <0x5800d000 0x1000>; > clocks = <&rcc USBH>; > diff --git a/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > index 12bc1d3ed424..a4acecb75c89 100644 > --- a/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > @@ -585,7 +585,7 @@ pcie: pcie@9860000 { > status = "disabled"; > }; > > - ohci: ohci@9880000 { > + ohci: usb@9880000 { > compatible = "generic-ohci"; > reg = <0x9880000 0x10000>; > interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>; > @@ -600,7 +600,7 @@ ohci: ohci@9880000 { > status = "disabled"; > }; > > - ehci: ehci@9890000 { > + ehci: usb@9890000 { > compatible = "generic-ehci"; > reg = <0x9890000 0x10000>; > interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>; > diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi > b/arch/arm64/boot/dts/hisilicon/hip06.dtsi > index 50ceaa959bdc..1226440d54ad 100644 > --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi > @@ -373,7 +373,7 @@ refclk: refclk { > #clock-cells = <0>; > }; > > - usb_ohci: ohci@a7030000 { > + usb_ohci: usb@a7030000 { > compatible = "generic-ohci"; > reg = <0x0 0xa7030000 0x0 0x10000>; > interrupt-parent = <&mbigen_usb>; > @@ -382,7 +382,7 @@ usb_ohci: ohci@a7030000 { > status = "disabled"; > }; > > - usb_ehci: ehci@a7020000 { > + usb_ehci: usb@a7020000 { > compatible = "generic-ehci"; > reg = <0x0 0xa7020000 0x0 0x10000>; > interrupt-parent = <&mbigen_usb>; > diff --git a/arch/arm64/boot/dts/hisilicon/hip07.dtsi > b/arch/arm64/boot/dts/hisilicon/hip07.dtsi > index 4773a533fce5..93f99a5255ac 100644 > --- a/arch/arm64/boot/dts/hisilicon/hip07.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hip07.dtsi > @@ -1253,7 +1253,7 @@ uart0: uart@602b0000 { > status = "disabled"; > }; > > - usb_ohci: ohci@a7030000 { > + usb_ohci: usb@a7030000 { > compatible = "generic-ohci"; > reg = <0x0 0xa7030000 0x0 0x10000>; > interrupt-parent = <&mbigen_usb>; > @@ -1262,7 +1262,7 @@ usb_ohci: ohci@a7030000 { > status = "disabled"; > }; > > - usb_ehci: ehci@a7020000 { > + usb_ehci: usb@a7020000 { > compatible = "generic-ehci"; > reg = <0x0 0xa7020000 0x0 0x10000>; > interrupt-parent = <&mbigen_usb>; > diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi > b/arch/mips/boot/dts/ingenic/jz4740.dtsi > index 1520585c235c..b989ff62ffbc 100644 > --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi > +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi > @@ -281,7 +281,7 @@ dmac: dma-controller@13020000 { > clocks = <&cgu JZ4740_CLK_DMA>; > }; > > - uhc: uhc@13030000 { > + uhc: usb@13030000 { > compatible = "ingenic,jz4740-ohci", "generic-ohci"; > reg = <0x13030000 0x1000>; > > diff --git a/arch/mips/boot/dts/ingenic/jz4770.dtsi > b/arch/mips/boot/dts/ingenic/jz4770.dtsi > index fa11ac950499..e45c03038826 100644 > --- a/arch/mips/boot/dts/ingenic/jz4770.dtsi > +++ b/arch/mips/boot/dts/ingenic/jz4770.dtsi > @@ -417,7 +417,7 @@ dmac1: dma-controller@13420100 { > interrupts = <23>; > }; > > - uhc: uhc@13430000 { > + uhc: usb@13430000 { > compatible = "generic-ohci"; > reg = <0x13430000 0x1000>; > > diff --git a/arch/mips/boot/dts/mti/sead3.dts > b/arch/mips/boot/dts/mti/sead3.dts > index 192c26ff1d3d..1cf6728af8fe 100644 > --- a/arch/mips/boot/dts/mti/sead3.dts > +++ b/arch/mips/boot/dts/mti/sead3.dts > @@ -56,7 +56,7 @@ gic: interrupt-controller@1b1c0000 { > interrupt-parent = <&cpu_intc>; > }; > > - ehci@1b200000 { > + usb@1b200000 { > compatible = "generic-ehci"; > reg = <0x1b200000 0x1000>; > > diff --git a/arch/mips/boot/dts/ralink/mt7628a.dtsi > b/arch/mips/boot/dts/ralink/mt7628a.dtsi > index 892e8ab863c5..45bf96a3d17a 100644 > --- a/arch/mips/boot/dts/ralink/mt7628a.dtsi > +++ b/arch/mips/boot/dts/ralink/mt7628a.dtsi > @@ -275,7 +275,7 @@ usb_phy: usb-phy@10120000 { > reset-names = "host", "device"; > }; > > - ehci@101c0000 { > + usb@101c0000 { > compatible = "generic-ehci"; > reg = <0x101c0000 0x1000>; > > diff --git a/arch/powerpc/boot/dts/akebono.dts > b/arch/powerpc/boot/dts/akebono.dts > index df18f8dc4642..343326c30380 100644 > --- a/arch/powerpc/boot/dts/akebono.dts > +++ b/arch/powerpc/boot/dts/akebono.dts > @@ -126,7 +126,7 @@ SATA0: sata@30000010000 { > interrupts = <93 2>; > }; > > - EHCI0: ehci@30010000000 { > + EHCI0: usb@30010000000 { > compatible = "ibm,476gtr-ehci", "generic-ehci"; > reg = <0x300 0x10000000 0x0 0x10000>; > interrupt-parent = <&MPIC>; > @@ -140,14 +140,14 @@ SD0: sd@30000000000 { > interrupt-parent = <&MPIC>; > }; > > - OHCI0: ohci@30010010000 { > + OHCI0: usb@30010010000 { > compatible = "ibm,476gtr-ohci", "generic-ohci"; > reg = <0x300 0x10010000 0x0 0x10000>; > interrupt-parent = <&MPIC>; > interrupts = <89 1>; > }; > > - OHCI1: ohci@30010020000 { > + OHCI1: usb@30010020000 { > compatible = "ibm,476gtr-ohci", "generic-ohci"; > reg = <0x300 0x10020000 0x0 0x10000>; > interrupt-parent = <&MPIC>; > -- > 2.27.0 >
Hi Sergey, > arch/arc/boot/dts/axs10x_mb.dtsi | 4 ++-- > arch/arc/boot/dts/hsdk.dts | 4 ++-- > arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +- For ARC boards Acked-by: Alexey Brodkin <abrodkin@synopsys.com>
On Wed, 14 Oct 2020 at 19:16, Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote: > > On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote: > > On Wed, 14 Oct 2020 at 12:23, Serge Semin > > <Sergey.Semin@baikalelectronics.ru> wrote: > > > > > > In accordance with the DWC USB3 bindings the corresponding node name is > > > suppose to comply with Generic USB HCD DT schema, which requires the USB > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause > > > the dtbs_check procedure failure. Let's fix the nodes naming to be > > > compatible with the DWC USB3 DT schema to make dtbs_check happy. > > > > > > Note we don't change the DWC USB3-compatible nodes names of > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the > > > in-source comment says that the nodes name need to be preserved as > > > "^dwusb@.*" for some backward compatibility. > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > > > --- > > > > > > Please, test the patch out to make sure it doesn't brake the dependent DTS > > > files. I did only a manual grepping of the possible nodes dependencies. > > > > > 1. It is you who should compare the decompiled DTS, not us. For example: > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64 > > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done > > > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64 > > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} > > > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt > > dts-new/${i#dts-old/}.fdt ; done > > So basically you suggest first to compile the old and new dts files, then to > de-compile them, then diff old and new fdt's, and visually compare the results. > Personally it isn't that much better than what I did, since each old and new > dtbs will for sure differ due to the node names change suggested in this patch. > So it will lead to the visual debugging too, which isn't that effective. But > your approach is still more demonstrative to make sure that I didn't loose any > nodes redefinition, since in the occasion the old and new de-compiled nodes will > differ not only by the node names but with an additional old named node. My suggestion is to compare the entire, effective DTS after all inclusions. Maybe you did it already, I don't know. The point is that when you change node names in DTSI but you miss one in DTS, you end up with two nodes. This is much easier to spot with dtxdiff or with fdtdump (which behaves better for node moves). Indeed it is still a visual comparison - if you have any ideas how to automate it (e.g. ignore phandle changes), please share. It would solve my testings as well. But asking others to test because you do not want to check it is not the best way to handle such changes. > > So to speak thanks for suggesting it. I'll try it to validate the proposed > changes. > > Two questions: > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I > can get all the updated dtsi'es, then find out all the dts'es which include > them, then directly use dtc to compile the found dts'es... On the other hand I > can just compile all dts'es, then compare old and new ones. The diff of the > non-modified dtb'es will be just empty... make dtbs touch your dts or git stash pop make dtbs compare diff for all unchanged will be simply empty, so easy to spot > 2) What crosc64 is? Ah, just an alias for cross compiling + ccache + kbuild out. I just copied you my helpers, so you need to tweak them. > > > > > 2. Split it per arm architectures (and proper subject prefix - not > > "arch") and subarchitectures so maintainers can pick it up. > > Why? The changes are simple and can be formatted as a single patch. I've seen > tons of patches submitted like that, accepted and then merged. What you suggest > is just much more work, which I don't see quite required. DTS changes go separate between arm64 and arm. There is nothing unusual here - all changes are submitted like this. Second topic is to split by subarchitectures which is necessary if you want it to be picked up by maintainers. It also makes it easier to review. Sure, without split ber subarchitectures this could be picked up by SoC folks but you did not even CC them. So if you do not want to split it per subarchitectures for maintainers and you do not CC SoC, then how do you believe this should be picked up? Out of the regular patch submission way? That's not how the changes are handled. > > > > > 3. The subject title could be more accurate - there is no fix here > > because there was no errors in the first place. Requirement of DWC > > node names comes recently, so it is more alignment with dtschema. > > Otherwise automatic-pickup-stable-bot might want to pick up... and it > > should not go to stable. > > Actually it is a fix, because the USB DT nodes should have been named with "usb" > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was > wrong in the first place. Not following the naming convention of DT spec which was loosely enforced is not an error which should be "fixed". Simply wrong title. This is an alignment with dtschema or correcting naming convention. Not fixing errors. > > Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the > emails, but most likely the stable@vger.kernel.org list only or the emails > having the "Fixes:" tag. If I am wrong please give me a link to the bot sources > or refer to a doc where I can read about the way it works, to take it into > account in future commits. Just to note I submitted patches which did some fixes, > had the word "fix" in the subject but weren't selected to be backported to the > stable kernel. You mixed up bots. The regular stable bot picks commits with cc-stable or with "Fixes". The auto-pickup bot picks all commits (not emails... why would it look at emails?) looking like a fix. Wording could be one of the hints used in the heuristic. Anyway, this is not a fix, regardless of autosel, so the wording is not correct. Just Google for AUTOSEL. You can then ask Sasha for sources... > Anyway I don't really care that much about the subject text using the word "fix" > or some else. So if you suggest some better alternative, I'd be glad to consider > it. I already did. One example is: alignment with dtschema. Best regards, Krzysztof
On Wed, Oct 14, 2020 at 11:41:17AM -0700, Florian Fainelli wrote: > On 10/14/20 11:11 AM, Serge Semin wrote: > > On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote: > >> On 10/14/20 3:14 AM, Serge Semin wrote: > >>> In accordance with the Generic EHCI/OHCI bindings the corresponding node > >>> name is suppose to comply with the Generic USB HCD DT schema, which > >>> requires the USB nodes to have the name acceptable by the regexp: > >>> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > >>> incompatible names. > >>> > >>> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > >>> > >>> --- > >>> > >>> Please, test the patch out to make sure it doesn't brake the dependent DTS > >>> files. I did only a manual grepping of the possible nodes dependencies. > >> > > > >> Not sure how you envisioned these change to be picked up, but you may > >> need to split these changes between ARM/ARM64, MIPS and PowerPC at > >> least. And within ARM/ARM64 you will most likely have to split according > >> to the various SoC maintainers. > > > > Hmm, I don't really know how it's going to be done in this case, but there must > > be a way to get the cross-platform patches picked up in general. For > > instance, see the patches like: > > 714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread() > > 140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS > > They touched the files from different files, but still have been merged in. > > That situation is different, when a new facility is added and someone > has gone through the work of adding support for all architectures (or > nearly all of them), you want them to be merged in a way that limits > merge conflicts with other architecture changes. > > Here you are fixing warnings, and each file you touch can clearly be > independently change from other files in the series without causing > merge conflicts. You are however creating the potential for merge > conflicts with other changes that the various SoC maintainers have > queued up. > > > Maybe I should have copied these three patches to the "linux-arch@vger.kernel.org" > > list or some other mailing list... > > Maybe Rob can queue them through his device tree repository, with the > ack of the various SoC maintainers... That's what I hoped for in the first place. But AFAICS now Rob does the splitting of his patches himself, while the repo is used either for the Documentation/devicetree/ patches or for the Rob'es own work. So it seems to me I'll have to split the series up and resubmit... ( Hope I am wrong. So, @Rob, will you be able to merge this and the next two patches in via your repo or you'd rather suggest for me to split it up and resubmit? -Sergey > -- > Florian
On Wed, Oct 14, 2020 at 01:35:16PM -0500, Rob Herring wrote: > On Wed, Oct 14, 2020 at 9:37 AM Serge Semin > <Sergey.Semin@baikalelectronics.ru> wrote: > > > > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: > > > > > > Hi Serge, > > > > > > Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > > > > In accordance with the DWC USB3 bindings the corresponding node name is > > > > suppose to comply with Generic USB HCD DT schema, which requires the USB > > > > > > > > DWC3 is not a simple HDC, though. > > > > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, > > which are tuned by the DWC USB3 driver in the kernel. But after that the > > controller is registered as xhci-hcd device so it's serviced by the xHCI driver, > > which then registers the HCD device so the corresponding DT node is supposed > > to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml > > and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted > > compatibility. > > > > > > > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot > > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have > > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause > > > > the dtbs_check procedure failure. Let's fix the nodes naming to be > > > > compatible with the DWC USB3 DT schema to make dtbs_check happy. > > > > > > > > Note we don't change the DWC USB3-compatible nodes names of > > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the > > > > in-source comment says that the nodes name need to be preserved as > > > > "^dwusb@.*" for some backward compatibility. > > > > > > > > interesting, compatibility with what? Some debugfs files, perhaps? :-) > > > > Don't really know.) In my experience the worst type of such compatibility is > > connected with some bootloader magic, which may add/remove/modify properties > > to nodes with pre-defined names. > > I seriously doubt anyone is using the APM machines with DT (even ACPI > is somewhat doubtful). I say change them. Or remove the dts files and > see what happens. Either way it can always be reverted. Ok. I'll change them in v3. -Sergey > > Rob
On Wed, Oct 14, 2020 at 10:04:32PM +0200, Krzysztof Kozlowski wrote: > On Wed, 14 Oct 2020 at 19:16, Serge Semin > <Sergey.Semin@baikalelectronics.ru> wrote: > > > > On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote: > > > On Wed, 14 Oct 2020 at 12:23, Serge Semin > > > <Sergey.Semin@baikalelectronics.ru> wrote: > > > > > > > > In accordance with the DWC USB3 bindings the corresponding node name is > > > > suppose to comply with Generic USB HCD DT schema, which requires the USB > > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot > > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have > > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause > > > > the dtbs_check procedure failure. Let's fix the nodes naming to be > > > > compatible with the DWC USB3 DT schema to make dtbs_check happy. > > > > > > > > Note we don't change the DWC USB3-compatible nodes names of > > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the > > > > in-source comment says that the nodes name need to be preserved as > > > > "^dwusb@.*" for some backward compatibility. > > > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > > > > > --- > > > > > > > > Please, test the patch out to make sure it doesn't brake the dependent DTS > > > > files. I did only a manual grepping of the possible nodes dependencies. > > > > > > > > 1. It is you who should compare the decompiled DTS, not us. For example: > > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64 > > > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done > > > > > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64 > > > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} > > > > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt > > > dts-new/${i#dts-old/}.fdt ; done > > > > So basically you suggest first to compile the old and new dts files, then to > > de-compile them, then diff old and new fdt's, and visually compare the results. > > Personally it isn't that much better than what I did, since each old and new > > dtbs will for sure differ due to the node names change suggested in this patch. > > So it will lead to the visual debugging too, which isn't that effective. But > > your approach is still more demonstrative to make sure that I didn't loose any > > nodes redefinition, since in the occasion the old and new de-compiled nodes will > > differ not only by the node names but with an additional old named node. > > My suggestion is to compare the entire, effective DTS after all > inclusions. Maybe you did it already, I don't know. Only by grepping the dts'es, which include the dtsi'es modified in this patch. So your suggestion of compiling and de-compiling has been indeed relevant. > The point is that > when you change node names in DTSI but you miss one in DTS, you end up > with two nodes. Yep, that's exactly what I meant when I said that your approach was more demonstrative, etc. > This is much easier to spot with dtxdiff or with > fdtdump (which behaves better for node moves). > > Indeed it is still a visual comparison - if you have any ideas how to > automate it (e.g. ignore phandle changes), please share. It would > solve my testings as well. Alas I don't. That's why to save my time of coming up with an automated solution I did a very thorough modification making sure that each affected node isn't updated in the corresponding dts'es and asked to test the patches out. Anyway the approach suggested by you will indeed give us a better understanding of my patches correctness. So I'll use it before sending v3. Thanks. > But asking others to test because you do > not want to check it is not the best way to handle such changes. > > > > > So to speak thanks for suggesting it. I'll try it to validate the proposed > > changes. > > > > Two questions: > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I > > can get all the updated dtsi'es, then find out all the dts'es which include > > them, then directly use dtc to compile the found dts'es... On the other hand I > > can just compile all dts'es, then compare old and new ones. The diff of the > > non-modified dtb'es will be just empty... > > make dtbs It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which first need to be enabled in the kernel config. So I'll need to have a config with all the affected dts. The later is the same as if I just found all the affected dts and built them one-by-one by directly calling dtc. > touch your dts or git stash pop > make dtbs > compare > diff for all unchanged will be simply empty, so easy to spot > > > 2) What crosc64 is? > > Ah, just an alias for cross compiling + ccache + kbuild out. I just > copied you my helpers, so you need to tweak them. > > > > > > > > > 2. Split it per arm architectures (and proper subject prefix - not > > > "arch") and subarchitectures so maintainers can pick it up. > > > > Why? The changes are simple and can be formatted as a single patch. I've seen > > tons of patches submitted like that, accepted and then merged. What you suggest > > is just much more work, which I don't see quite required. > > DTS changes go separate between arm64 and arm. There is nothing > unusual here - all changes are submitted like this. > Second topic is to split by subarchitectures which is necessary if you > want it to be picked up by maintainers. It also makes it easier to > review. The current patches are easy enough for review. The last three patches of the series is a collection of the one-type changes concerning the same type of nodes. So reviewing them won't cause any difficulty. But I assume that's not the main point in this discussion. > Sure, without split ber subarchitectures this could be picked > up by SoC folks but you did not even CC them. So if you do not want to > split it per subarchitectures for maintainers and you do not CC SoC, > then how do you believe this should be picked up? Out of the regular > patch submission way? That's not how the changes are handled. AFAIU there are another ways of merging comprehensive patches. If they get to collect all the Acked-by tags, they could be merged in, for instance, through Greg' or Rob' (for dts) repos, if of course they get to agree with doing that. Am I wrong? My hope was to ask Rob or Greg to get the patches merged in when they get to collect all the ackes, since I thought it was an option in such cases. So if they refuse to do so I'll have no choice but to split the series up into a smaller patches as you say. > > > > > > > > > 3. The subject title could be more accurate - there is no fix here > > > because there was no errors in the first place. Requirement of DWC > > > node names comes recently, so it is more alignment with dtschema. > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it > > > should not go to stable. > > > > Actually it is a fix, because the USB DT nodes should have been named with "usb" > > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes > > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm > > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was > > wrong in the first place. > > Not following the naming convention of DT spec which was loosely > enforced is not an error which should be "fixed". Simply wrong title. > This is an alignment with dtschema or correcting naming convention. > Not fixing errors. From your perspective it wasn't an error, from mine and most likely Rob' it was.) Anyway as I said I don't care that much about preserving the subject wording, so what about the next one: <arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema ? -Sergey > > > > > Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the > > emails, but most likely the stable@vger.kernel.org list only or the emails > > having the "Fixes:" tag. If I am wrong please give me a link to the bot sources > > or refer to a doc where I can read about the way it works, to take it into > > account in future commits. Just to note I submitted patches which did some fixes, > > had the word "fix" in the subject but weren't selected to be backported to the > > stable kernel. > > You mixed up bots. The regular stable bot picks commits with cc-stable > or with "Fixes". The auto-pickup bot picks all commits (not emails... > why would it look at emails?) looking like a fix. Wording could be one > of the hints used in the heuristic. Anyway, this is not a fix, > regardless of autosel, so the wording is not correct. > > Just Google for AUTOSEL. You can then ask Sasha for sources... > > > Anyway I don't really care that much about the subject text using the word "fix" > > or some else. So if you suggest some better alternative, I'd be glad to consider > > it. > > I already did. One example is: alignment with dtschema. > > Best regards, > Krzysztof
On Thu, Oct 15, 2020 at 02:51:05AM +0300, Serge Semin wrote: > > > > > So to speak thanks for suggesting it. I'll try it to validate the proposed > > > changes. > > > > > > Two questions: > > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I > > > can get all the updated dtsi'es, then find out all the dts'es which include > > > them, then directly use dtc to compile the found dts'es... On the other hand I > > > can just compile all dts'es, then compare old and new ones. The diff of the > > > non-modified dtb'es will be just empty... > > > > > make dtbs > > It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which > first need to be enabled in the kernel config. So I'll need to have a config > with all the affected dts. The later is the same as if I just found all the > affected dts and built them one-by-one by directly calling dtc. True. Sometimes allyesconfig for given arch might be helpful but not always (e.g. for ARM it does not select all of ARMv4 and ARMv5 boards). Most likely your approach is actually faster/more reliable. > > > touch your dts or git stash pop > > make dtbs > > compare > > diff for all unchanged will be simply empty, so easy to spot > > > > > 2) What crosc64 is? > > > > Ah, just an alias for cross compiling + ccache + kbuild out. I just > > copied you my helpers, so you need to tweak them. > > > > > > > > > > > > > 2. Split it per arm architectures (and proper subject prefix - not > > > > "arch") and subarchitectures so maintainers can pick it up. > > > > > > Why? The changes are simple and can be formatted as a single patch. I've seen > > > tons of patches submitted like that, accepted and then merged. What you suggest > > > is just much more work, which I don't see quite required. > > > > > DTS changes go separate between arm64 and arm. There is nothing > > unusual here - all changes are submitted like this. > > Second topic is to split by subarchitectures which is necessary if you > > want it to be picked up by maintainers. It also makes it easier to > > review. > > The current patches are easy enough for review. The last three patches of the > series is a collection of the one-type changes concerning the same type of > nodes. So reviewing them won't cause any difficulty. But I assume that's not > the main point in this discussion. > > > Sure, without split ber subarchitectures this could be picked > > up by SoC folks but you did not even CC them. So if you do not want to > > split it per subarchitectures for maintainers and you do not CC SoC, > > then how do you believe this should be picked up? Out of the regular > > patch submission way? That's not how the changes are handled. > > AFAIU there are another ways of merging comprehensive patches. If they get to collect > all the Acked-by tags, they could be merged in, for instance, through Greg' or Rob' > (for dts) repos, if of course they get to agree with doing that. Am I wrong? > > My hope was to ask Rob or Greg to get the patches merged in when they get > to collect all the ackes, since I thought it was an option in such cases. So if > they refuse to do so I'll have no choice but to split the series up into a > smaller patches as you say. This is neither Rob's nor Greg's patch to pick up, but ARM SoC (which was not CCed here). And most likely they won't pick it up because judging by contents it is obvious it should go via ARM SoC. Sure, if there are dependencies between some patches they can go with acks through unrelated trees, but this not the usual way. This is an exception in the process to solve particular dependency problem. It has drawbacks - increases the chances of annoying conflicts. The case here does not fall into this criteria - there is no dependency of this patch on the others Therefore there is no reason to use the unusual/exceptional way of handling patches. There is no reason why this shouldn't go via either specific ARM subarchitecture maintainers or via ARM SoC. > > > > 3. The subject title could be more accurate - there is no fix here > > > > because there was no errors in the first place. Requirement of DWC > > > > node names comes recently, so it is more alignment with dtschema. > > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it > > > > should not go to stable. > > > > > > Actually it is a fix, because the USB DT nodes should have been named with "usb" > > > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes > > > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm > > > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was > > > wrong in the first place. > > > > > Not following the naming convention of DT spec which was loosely > > enforced is not an error which should be "fixed". Simply wrong title. > > This is an alignment with dtschema or correcting naming convention. > > Not fixing errors. > > From your perspective it wasn't an error, from mine and most likely Rob' it > was.) Anyway as I said I don't care that much about preserving the subject > wording, so what about the next one: > <arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema > ? Looks good. Best regards, Krzysztof
Hi Serge, On 10/14/20 12:14 PM, Serge Semin wrote: > In accordance with the Generic EHCI/OHCI bindings the corresponding node > name is suppose to comply with the Generic USB HCD DT schema, which > requires the USB nodes to have the name acceptable by the regexp: > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > incompatible names. > > Signed-off-by: Serge Semin<Sergey.Semin@baikalelectronics.ru> > > diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi > index bfe29023fbd5..576f7da564c5 100644 > --- a/arch/arm/boot/dts/stm32mp151.dtsi > +++ b/arch/arm/boot/dts/stm32mp151.dtsi > @@ -1404,7 +1404,7 @@ ethernet0: ethernet@5800a000 { > status = "disabled"; > }; > > - usbh_ohci: usbh-ohci@5800c000 { > + usbh_ohci: usb@5800c000 { > compatible = "generic-ohci"; > reg = <0x5800c000 0x1000>; > clocks = <&rcc USBH>; > @@ -1413,7 +1413,7 @@ usbh_ohci: usbh-ohci@5800c000 { > status = "disabled"; > }; > > - usbh_ehci: usbh-ehci@5800d000 { > + usbh_ehci: usb@5800d000 { > compatible = "generic-ehci"; > reg = <0x5800d000 0x1000>; > clocks = <&rcc USBH>; For STM32MP151: Acked-by: Amelie Delaunay <amelie.delaunay@st.com> Thanks, Amelie
Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: >> >> Hi Serge, >> >> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: >> > In accordance with the DWC USB3 bindings the corresponding node name is >> > suppose to comply with Generic USB HCD DT schema, which requires the USB >> > >> DWC3 is not a simple HDC, though. > > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, > which are tuned by the DWC USB3 driver in the kernel. But after that the > controller is registered as xhci-hcd device so it's serviced by the xHCI driver, in Dual-role or host-only builds, that's correct. We can also have peripheral-only builds (both SW or HW versions) which means xhci isn't even in the picture.
On Thu, Oct 15, 2020 at 01:15:37PM +0300, Felipe Balbi wrote: > Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > > > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: > >> > >> Hi Serge, > >> > >> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > >> > In accordance with the DWC USB3 bindings the corresponding node name is > >> > suppose to comply with Generic USB HCD DT schema, which requires the USB > >> > > > >> DWC3 is not a simple HDC, though. > > > > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, > > which are tuned by the DWC USB3 driver in the kernel. But after that the > > controller is registered as xhci-hcd device so it's serviced by the xHCI driver, > > in Dual-role or host-only builds, that's correct. We can also have > peripheral-only builds (both SW or HW versions) which means xhci isn't > even in the picture. Hm, good point. In that case perhaps we'll need to apply the xHCI DT schema conditionally. Like this: - allOf: - - $ref: usb-xhci.yaml# + allOf: + - if: + properties: + dr_mode: + const: peripheral + then: + $ref: usb-hcd.yaml# + else: + $ref: usb-xhci.yaml# Note, we need to have the peripheral device being compatible with the usb-hcd.yaml schema to support the maximum-speed, dr_mode properties and to comply with the USB node naming constraint. -Sergey > > -- > balbi
On Thu, Oct 15, 2020 at 01:15:37PM +0300, Felipe Balbi wrote: > Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > > > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: > >> > >> Hi Serge, > >> > >> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes: > >> > In accordance with the DWC USB3 bindings the corresponding node name is > >> > suppose to comply with Generic USB HCD DT schema, which requires the USB > >> > > > >> DWC3 is not a simple HDC, though. > > > > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, > > which are tuned by the DWC USB3 driver in the kernel. But after that the > > controller is registered as xhci-hcd device so it's serviced by the xHCI driver, > > in Dual-role or host-only builds, that's correct. We can also have > peripheral-only builds (both SW or HW versions) which means xhci isn't > even in the picture. It doesn't really matter though, or at least it does for what the new name might be, but the old one currently used is still pretty bad. The DT spec says that the node name is the class of the device. "usb" as the HCD binding mandates is one, but the current nodes currently have completely different names from one DT to another - which is already an issue - and most of them have dwc3 or some variant of it, which doesn't really qualify for a class name. Maxime
On Thu, Oct 15, 2020 at 08:14:39AM +0200, Krzysztof Kozlowski wrote: > On Thu, Oct 15, 2020 at 02:51:05AM +0300, Serge Semin wrote: > > > > > > > So to speak thanks for suggesting it. I'll try it to validate the proposed > > > > changes. > > > > > > > > Two questions: > > > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I > > > > can get all the updated dtsi'es, then find out all the dts'es which include > > > > them, then directly use dtc to compile the found dts'es... On the other hand I > > > > can just compile all dts'es, then compare old and new ones. The diff of the > > > > non-modified dtb'es will be just empty... > > > > > > > > make dtbs > > > > It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which > > first need to be enabled in the kernel config. So I'll need to have a config > > with all the affected dts. The later is the same as if I just found all the > > affected dts and built them one-by-one by directly calling dtc. > > True. Sometimes allyesconfig for given arch might be helpful but not > always (e.g. for ARM it does not select all of ARMv4 and ARMv5 boards). > Most likely your approach is actually faster/more reliable. > > > > > > touch your dts or git stash pop > > > make dtbs > > > compare > > > diff for all unchanged will be simply empty, so easy to spot > > > > > > > 2) What crosc64 is? > > > > > > Ah, just an alias for cross compiling + ccache + kbuild out. I just > > > copied you my helpers, so you need to tweak them. > > > > > > > > > > > > > > > > > 2. Split it per arm architectures (and proper subject prefix - not > > > > > "arch") and subarchitectures so maintainers can pick it up. > > > > > > > > Why? The changes are simple and can be formatted as a single patch. I've seen > > > > tons of patches submitted like that, accepted and then merged. What you suggest > > > > is just much more work, which I don't see quite required. > > > > > > > > DTS changes go separate between arm64 and arm. There is nothing > > > unusual here - all changes are submitted like this. > > > Second topic is to split by subarchitectures which is necessary if you > > > want it to be picked up by maintainers. It also makes it easier to > > > review. > > > > The current patches are easy enough for review. The last three patches of the > > series is a collection of the one-type changes concerning the same type of > > nodes. So reviewing them won't cause any difficulty. But I assume that's not > > the main point in this discussion. > > > > > Sure, without split ber subarchitectures this could be picked > > > up by SoC folks but you did not even CC them. So if you do not want to > > > split it per subarchitectures for maintainers and you do not CC SoC, > > > then how do you believe this should be picked up? Out of the regular > > > patch submission way? That's not how the changes are handled. > > > > AFAIU there are another ways of merging comprehensive patches. If they get to collect > > all the Acked-by tags, they could be merged in, for instance, through Greg' or Rob' > > (for dts) repos, if of course they get to agree with doing that. Am I wrong? > > > > My hope was to ask Rob or Greg to get the patches merged in when they get > > to collect all the ackes, since I thought it was an option in such cases. So if > > they refuse to do so I'll have no choice but to split the series up into a > > smaller patches as you say. > > This is neither Rob's nor Greg's patch to pick up, but ARM SoC (which was > not CCed here). And most likely they won't pick it up because judging by > contents it is obvious it should go via ARM SoC. > > Sure, if there are dependencies between some patches they can go with > acks through unrelated trees, but this not the usual way. This is an > exception in the process to solve particular dependency problem. It has > drawbacks - increases the chances of annoying conflicts. > > The case here does not fall into this criteria - there is no dependency > of this patch on the others Therefore there is no reason to use the > unusual/exceptional way of handling patches. There is no reason why > this shouldn't go via either specific ARM subarchitecture maintainers or > via ARM SoC. Ok. I see your point. To sum it up I've studied the git log arch/ commit messages and it turns out even Rob has to split the cleanup changes like this ones. So thanks for your patience with stating your point. I'll split the last three patches up to be merged in via the corresponding archs/subarch'es repos. -Sergey > > > > > > 3. The subject title could be more accurate - there is no fix here > > > > > because there was no errors in the first place. Requirement of DWC > > > > > node names comes recently, so it is more alignment with dtschema. > > > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it > > > > > should not go to stable. > > > > > > > > Actually it is a fix, because the USB DT nodes should have been named with "usb" > > > > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes > > > > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm > > > > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was > > > > wrong in the first place. > > > > > > > > Not following the naming convention of DT spec which was loosely > > > enforced is not an error which should be "fixed". Simply wrong title. > > > This is an alignment with dtschema or correcting naming convention. > > > Not fixing errors. > > > > From your perspective it wasn't an error, from mine and most likely Rob' it > > was.) Anyway as I said I don't care that much about preserving the subject > > wording, so what about the next one: > > <arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema > > ? > > Looks good. > > Best regards, > Krzysztof >
On 10/14/20 3:14 AM, Serge Semin wrote: > In accordance with the Generic EHCI/OHCI bindings the corresponding node > name is suppose to comply with the Generic USB HCD DT schema, which > requires the USB nodes to have the name acceptable by the regexp: > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > incompatible names. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Please, test the patch out to make sure it doesn't brake the dependent DTS > files. I did only a manual grepping of the possible nodes dependencies. > --- > arch/arm/boot/dts/bcm5301x.dtsi | 4 ++-- > arch/arm/boot/dts/bcm53573.dtsi | 4 ++-- Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Hi Serge, On 10/14/20 12:14 PM, Serge Semin wrote: > In accordance with the Generic EHCI/OHCI bindings the corresponding node > name is suppose to comply with the Generic USB HCD DT schema, which > requires the USB nodes to have the name acceptable by the regexp: > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > incompatible names. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Please, test the patch out to make sure it doesn't brake the dependent DTS > files. I did only a manual grepping of the possible nodes dependencies. > --- > arch/arc/boot/dts/axs10x_mb.dtsi | 4 ++-- > arch/arc/boot/dts/hsdk.dts | 4 ++-- > arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +- > arch/arm/boot/dts/bcm5301x.dtsi | 4 ++-- > arch/arm/boot/dts/bcm53573.dtsi | 4 ++-- > arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 ++-- > arch/arm/boot/dts/lpc18xx.dtsi | 4 ++-- > arch/arm/boot/dts/stm32mp151.dtsi | 4 ++-- > arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 4 ++-- > arch/arm64/boot/dts/hisilicon/hip06.dtsi | 4 ++-- > arch/arm64/boot/dts/hisilicon/hip07.dtsi | 4 ++-- > arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +- > arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +- > arch/mips/boot/dts/mti/sead3.dts | 2 +- > arch/mips/boot/dts/ralink/mt7628a.dtsi | 2 +- > arch/powerpc/boot/dts/akebono.dts | 6 +++--- > 16 files changed, 28 insertions(+), 28 deletions(-) > I surely missed something, but we have here in the same patch modifications for different architectures and different vendors. Do you plan to split this patch after getting some Acked-by / Tested-by ? regards Alex > diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi > index 99d3e7175bf7..b64435385304 100644 > --- a/arch/arc/boot/dts/axs10x_mb.dtsi > +++ b/arch/arc/boot/dts/axs10x_mb.dtsi > @@ -87,13 +87,13 @@ gmac: ethernet@18000 { > mac-address = [00 00 00 00 00 00]; /* Filled in by U-Boot */ > }; > > - ehci@40000 { > + usb@40000 { > compatible = "generic-ehci"; > reg = < 0x40000 0x100 >; > interrupts = < 8 >; > }; > > - ohci@60000 { > + usb@60000 { > compatible = "generic-ohci"; > reg = < 0x60000 0x100 >; > interrupts = < 8 >; > diff --git a/arch/arc/boot/dts/hsdk.dts b/arch/arc/boot/dts/hsdk.dts > index dcaa44e408ac..fdd4f7f635d3 100644 > --- a/arch/arc/boot/dts/hsdk.dts > +++ b/arch/arc/boot/dts/hsdk.dts > @@ -234,7 +234,7 @@ phy0: ethernet-phy@0 { /* Micrel KSZ9031 */ > }; > }; > > - ohci@60000 { > + usb@60000 { > compatible = "snps,hsdk-v1.0-ohci", "generic-ohci"; > reg = <0x60000 0x100>; > interrupts = <15>; > @@ -242,7 +242,7 @@ ohci@60000 { > dma-coherent; > }; > > - ehci@40000 { > + usb@40000 { > compatible = "snps,hsdk-v1.0-ehci", "generic-ehci"; > reg = <0x40000 0x100>; > interrupts = <15>; > diff --git a/arch/arc/boot/dts/vdk_axs10x_mb.dtsi b/arch/arc/boot/dts/vdk_axs10x_mb.dtsi > index cbb179770293..90a412026e64 100644 > --- a/arch/arc/boot/dts/vdk_axs10x_mb.dtsi > +++ b/arch/arc/boot/dts/vdk_axs10x_mb.dtsi > @@ -46,7 +46,7 @@ ethernet@18000 { > clock-names = "stmmaceth"; > }; > > - ehci@40000 { > + usb@40000 { > compatible = "generic-ehci"; > reg = < 0x40000 0x100 >; > interrupts = < 8 >; > diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi > index 0016720ce530..bf5656d79a55 100644 > --- a/arch/arm/boot/dts/bcm5301x.dtsi > +++ b/arch/arm/boot/dts/bcm5301x.dtsi > @@ -261,7 +261,7 @@ usb2: usb2@21000 { > > interrupt-parent = <&gic>; > > - ehci: ehci@21000 { > + ehci: usb@21000 { > #usb-cells = <0>; > > compatible = "generic-ehci"; > @@ -283,7 +283,7 @@ ehci_port2: port@2 { > }; > }; > > - ohci: ohci@22000 { > + ohci: usb@22000 { > #usb-cells = <0>; > > compatible = "generic-ohci"; > diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi > index 4af8e3293cff..51546fccc616 100644 > --- a/arch/arm/boot/dts/bcm53573.dtsi > +++ b/arch/arm/boot/dts/bcm53573.dtsi > @@ -135,7 +135,7 @@ usb2: usb2@4000 { > #address-cells = <1>; > #size-cells = <1>; > > - ehci: ehci@4000 { > + ehci: usb@4000 { > compatible = "generic-ehci"; > reg = <0x4000 0x1000>; > interrupt-parent = <&gic>; > @@ -155,7 +155,7 @@ ehci_port2: port@2 { > }; > }; > > - ohci: ohci@d000 { > + ohci: usb@d000 { > #usb-cells = <0>; > > compatible = "generic-ohci"; > diff --git a/arch/arm/boot/dts/hisi-x5hd2.dtsi b/arch/arm/boot/dts/hisi-x5hd2.dtsi > index 3ee7967c202d..693b85b2cc7d 100644 > --- a/arch/arm/boot/dts/hisi-x5hd2.dtsi > +++ b/arch/arm/boot/dts/hisi-x5hd2.dtsi > @@ -452,14 +452,14 @@ gmac1: ethernet@1841000 { > status = "disabled"; > }; > > - usb0: ehci@1890000 { > + usb0: usb@1890000 { > compatible = "generic-ehci"; > reg = <0x1890000 0x1000>; > interrupts = <0 66 4>; > clocks = <&clock HIX5HD2_USB_CLK>; > }; > > - usb1: ohci@1880000 { > + usb1: usb@1880000 { > compatible = "generic-ohci"; > reg = <0x1880000 0x1000>; > interrupts = <0 67 4>; > diff --git a/arch/arm/boot/dts/lpc18xx.dtsi b/arch/arm/boot/dts/lpc18xx.dtsi > index 10b8249b8ab6..82ffd7b0ad8a 100644 > --- a/arch/arm/boot/dts/lpc18xx.dtsi > +++ b/arch/arm/boot/dts/lpc18xx.dtsi > @@ -121,7 +121,7 @@ mmcsd: mmcsd@40004000 { > status = "disabled"; > }; > > - usb0: ehci@40006100 { > + usb0: usb@40006100 { > compatible = "nxp,lpc1850-ehci", "generic-ehci"; > reg = <0x40006100 0x100>; > interrupts = <8>; > @@ -133,7 +133,7 @@ usb0: ehci@40006100 { > status = "disabled"; > }; > > - usb1: ehci@40007100 { > + usb1: usb@40007100 { > compatible = "nxp,lpc1850-ehci", "generic-ehci"; > reg = <0x40007100 0x100>; > interrupts = <9>; > diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi > index bfe29023fbd5..576f7da564c5 100644 > --- a/arch/arm/boot/dts/stm32mp151.dtsi > +++ b/arch/arm/boot/dts/stm32mp151.dtsi > @@ -1404,7 +1404,7 @@ ethernet0: ethernet@5800a000 { > status = "disabled"; > }; > > - usbh_ohci: usbh-ohci@5800c000 { > + usbh_ohci: usb@5800c000 { > compatible = "generic-ohci"; > reg = <0x5800c000 0x1000>; > clocks = <&rcc USBH>; > @@ -1413,7 +1413,7 @@ usbh_ohci: usbh-ohci@5800c000 { > status = "disabled"; > }; > > - usbh_ehci: usbh-ehci@5800d000 { > + usbh_ehci: usb@5800d000 { > compatible = "generic-ehci"; > reg = <0x5800d000 0x1000>; > clocks = <&rcc USBH>; > diff --git a/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > index 12bc1d3ed424..a4acecb75c89 100644 > --- a/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > @@ -585,7 +585,7 @@ pcie: pcie@9860000 { > status = "disabled"; > }; > > - ohci: ohci@9880000 { > + ohci: usb@9880000 { > compatible = "generic-ohci"; > reg = <0x9880000 0x10000>; > interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>; > @@ -600,7 +600,7 @@ ohci: ohci@9880000 { > status = "disabled"; > }; > > - ehci: ehci@9890000 { > + ehci: usb@9890000 { > compatible = "generic-ehci"; > reg = <0x9890000 0x10000>; > interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>; > diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi > index 50ceaa959bdc..1226440d54ad 100644 > --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi > @@ -373,7 +373,7 @@ refclk: refclk { > #clock-cells = <0>; > }; > > - usb_ohci: ohci@a7030000 { > + usb_ohci: usb@a7030000 { > compatible = "generic-ohci"; > reg = <0x0 0xa7030000 0x0 0x10000>; > interrupt-parent = <&mbigen_usb>; > @@ -382,7 +382,7 @@ usb_ohci: ohci@a7030000 { > status = "disabled"; > }; > > - usb_ehci: ehci@a7020000 { > + usb_ehci: usb@a7020000 { > compatible = "generic-ehci"; > reg = <0x0 0xa7020000 0x0 0x10000>; > interrupt-parent = <&mbigen_usb>; > diff --git a/arch/arm64/boot/dts/hisilicon/hip07.dtsi b/arch/arm64/boot/dts/hisilicon/hip07.dtsi > index 4773a533fce5..93f99a5255ac 100644 > --- a/arch/arm64/boot/dts/hisilicon/hip07.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hip07.dtsi > @@ -1253,7 +1253,7 @@ uart0: uart@602b0000 { > status = "disabled"; > }; > > - usb_ohci: ohci@a7030000 { > + usb_ohci: usb@a7030000 { > compatible = "generic-ohci"; > reg = <0x0 0xa7030000 0x0 0x10000>; > interrupt-parent = <&mbigen_usb>; > @@ -1262,7 +1262,7 @@ usb_ohci: ohci@a7030000 { > status = "disabled"; > }; > > - usb_ehci: ehci@a7020000 { > + usb_ehci: usb@a7020000 { > compatible = "generic-ehci"; > reg = <0x0 0xa7020000 0x0 0x10000>; > interrupt-parent = <&mbigen_usb>; > diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi > index 1520585c235c..b989ff62ffbc 100644 > --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi > +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi > @@ -281,7 +281,7 @@ dmac: dma-controller@13020000 { > clocks = <&cgu JZ4740_CLK_DMA>; > }; > > - uhc: uhc@13030000 { > + uhc: usb@13030000 { > compatible = "ingenic,jz4740-ohci", "generic-ohci"; > reg = <0x13030000 0x1000>; > > diff --git a/arch/mips/boot/dts/ingenic/jz4770.dtsi b/arch/mips/boot/dts/ingenic/jz4770.dtsi > index fa11ac950499..e45c03038826 100644 > --- a/arch/mips/boot/dts/ingenic/jz4770.dtsi > +++ b/arch/mips/boot/dts/ingenic/jz4770.dtsi > @@ -417,7 +417,7 @@ dmac1: dma-controller@13420100 { > interrupts = <23>; > }; > > - uhc: uhc@13430000 { > + uhc: usb@13430000 { > compatible = "generic-ohci"; > reg = <0x13430000 0x1000>; > > diff --git a/arch/mips/boot/dts/mti/sead3.dts b/arch/mips/boot/dts/mti/sead3.dts > index 192c26ff1d3d..1cf6728af8fe 100644 > --- a/arch/mips/boot/dts/mti/sead3.dts > +++ b/arch/mips/boot/dts/mti/sead3.dts > @@ -56,7 +56,7 @@ gic: interrupt-controller@1b1c0000 { > interrupt-parent = <&cpu_intc>; > }; > > - ehci@1b200000 { > + usb@1b200000 { > compatible = "generic-ehci"; > reg = <0x1b200000 0x1000>; > > diff --git a/arch/mips/boot/dts/ralink/mt7628a.dtsi b/arch/mips/boot/dts/ralink/mt7628a.dtsi > index 892e8ab863c5..45bf96a3d17a 100644 > --- a/arch/mips/boot/dts/ralink/mt7628a.dtsi > +++ b/arch/mips/boot/dts/ralink/mt7628a.dtsi > @@ -275,7 +275,7 @@ usb_phy: usb-phy@10120000 { > reset-names = "host", "device"; > }; > > - ehci@101c0000 { > + usb@101c0000 { > compatible = "generic-ehci"; > reg = <0x101c0000 0x1000>; > > diff --git a/arch/powerpc/boot/dts/akebono.dts b/arch/powerpc/boot/dts/akebono.dts > index df18f8dc4642..343326c30380 100644 > --- a/arch/powerpc/boot/dts/akebono.dts > +++ b/arch/powerpc/boot/dts/akebono.dts > @@ -126,7 +126,7 @@ SATA0: sata@30000010000 { > interrupts = <93 2>; > }; > > - EHCI0: ehci@30010000000 { > + EHCI0: usb@30010000000 { > compatible = "ibm,476gtr-ehci", "generic-ehci"; > reg = <0x300 0x10000000 0x0 0x10000>; > interrupt-parent = <&MPIC>; > @@ -140,14 +140,14 @@ SD0: sd@30000000000 { > interrupt-parent = <&MPIC>; > }; > > - OHCI0: ohci@30010010000 { > + OHCI0: usb@30010010000 { > compatible = "ibm,476gtr-ohci", "generic-ohci"; > reg = <0x300 0x10010000 0x0 0x10000>; > interrupt-parent = <&MPIC>; > interrupts = <89 1>; > }; > > - OHCI1: ohci@30010020000 { > + OHCI1: usb@30010020000 { > compatible = "ibm,476gtr-ohci", "generic-ohci"; > reg = <0x300 0x10020000 0x0 0x10000>; > interrupt-parent = <&MPIC>; >
Hello Alexandre, On Fri, Oct 16, 2020 at 09:08:23AM +0200, Alexandre Torgue wrote: > Hi Serge, > > On 10/14/20 12:14 PM, Serge Semin wrote: > > In accordance with the Generic EHCI/OHCI bindings the corresponding node > > name is suppose to comply with the Generic USB HCD DT schema, which > > requires the USB nodes to have the name acceptable by the regexp: > > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > > incompatible names. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Please, test the patch out to make sure it doesn't brake the dependent DTS > > files. I did only a manual grepping of the possible nodes dependencies. > > --- > > arch/arc/boot/dts/axs10x_mb.dtsi | 4 ++-- > > arch/arc/boot/dts/hsdk.dts | 4 ++-- > > arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +- > > arch/arm/boot/dts/bcm5301x.dtsi | 4 ++-- > > arch/arm/boot/dts/bcm53573.dtsi | 4 ++-- > > arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 ++-- > > arch/arm/boot/dts/lpc18xx.dtsi | 4 ++-- > > arch/arm/boot/dts/stm32mp151.dtsi | 4 ++-- > > arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 4 ++-- > > arch/arm64/boot/dts/hisilicon/hip06.dtsi | 4 ++-- > > arch/arm64/boot/dts/hisilicon/hip07.dtsi | 4 ++-- > > arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +- > > arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +- > > arch/mips/boot/dts/mti/sead3.dts | 2 +- > > arch/mips/boot/dts/ralink/mt7628a.dtsi | 2 +- > > arch/powerpc/boot/dts/akebono.dts | 6 +++--- > > 16 files changed, 28 insertions(+), 28 deletions(-) > > > > I surely missed something, but we have here in the same patch modifications > for different architectures and different vendors. > > Do you plan to split this patch after getting some Acked-by / Tested-by ? Yeah, I'll split this patch and two next ones up in v3. -Sergey > > regards > Alex > >
We've performed some work on the Generic USB HCD, xHCI and DWC USB3 DT bindings in the framework of the Baikal-T1 SoC support integration into the kernel. This patchset is a result of that work. First of all we moved the generic USB properties from the legacy text bindings into the USB HCD DT schema. So now the generic USB HCD-compatible DT nodes are validated taking into account the optional properties like: maximum-speed, dr_mode, otg-rev, usb-role-switch, etc. We've fixed these properties a bit so they would correspond to what functionality kernel currently supports. Secondly we converted generic USB xHCI text bindings file into the DT schema. It had to be split up into two bindings: DT schema with generic xHCI properties and a generic xHCI device DT schema. The later will be used to validate the pure xHCI-based nodes, while the former can be utilized by some vendor-specific versions of xHCI. Thirdly, what was primarily intended to be done for Baikal-T1 SoC USB we converted the legacy text-based DWC USB3 bindings to DT schema and altered the result a bit so it would be more coherent with what actually controller and its driver support. Since we've now got the DWC USB3 DT schema, we made it used to validate the sub-nodes of the Qualcom, TI and Amlogic DWC3 DT nodes. Finally we've also fixed all the OHCI/EHCI, xHCI and DW USB3 compatible DT nodes so they would comply with the nodes naming scema declared in the USB HCD DT bindings file. Link: https://lore.kernel.org/linux-usb/20201010224121.12672-1-Sergey.Semin@baikalelectronics.ru/ Changelog v2: - Thanks to Sergei Shtylyov for suggesting the commit logs grammar fixes: [PATCH 04/18] dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types [PATCH 05/18] dt-bindings: usb: usb-hcd: Add "tpl-support" property [PATCH 11/18] dt-bindings: usb: dwc3: Add interrupt-names property support [PATCH 13/18] dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions [PATCH 17/18] dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node - Set FL-adj of the amlogiv,meson-g12a-usb controller with value 0x20 instead of completely removing the property. - Drop the patch: [PATCH 02/18] dt-bindings: usb: usb-hcd: Add "wireless" maximum-speed property value since "wireless" speed type is depracated due to lack of the device supporting it. - Drop quotes from around the compat string constant. - Discard '|' from the property descriptions, since we don't need to preserve the text formatting. - Convert abbreviated form of the "maximum-speed" enum constraint into the multi-lined version of the list. - Fix the DW USB3 "clock-names" prop description to be refererring to the enumerated clock-names instead of the ones from the Databook. - Add explicit "additionalProperties: true" to the usb-xhci.yaml schema, since additionalProperties/unevaluatedProperties are going to be mandary for each binding. - Use "oneOf: [dwc2.yaml#, snps,dwc3.yaml#]" instead of the bulky "if: properties: compatibe: ..." statement. - Discard the "^dwc3@[0-9a-f]+$" nodes from being acceptable as sub-nodes of the Qualcomm DWC3 DT nodes. - Add new patches: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name [PATCH 19/20] arch: dts: Fix xHCI DT nodes name [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru> Cc: Andy Gross <agross@kernel.org> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Manu Gautam <mgautam@codeaurora.org> Cc: Roger Quadros <rogerq@ti.com> Cc: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Kevin Hilman <khilman@baylibre.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-snps-arc@lists.infradead.org Cc: linux-mips@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-usb@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Serge Semin (20): dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema dt-bindings: usb: usb-hcd: Add "otg-rev" property restriction dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types dt-bindings: usb: usb-hcd: Add "tpl-support" property dt-bindings: usb: usb-hcd: Add generic "usb-phy" property dt-bindings: usb: Convert xHCI bindings to DT schema dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device dt-bindings: usb: renesas-xhci: Refer to the usb-xhci.yaml file dt-bindings: usb: Convert DWC USB3 bindings to DT schema dt-bindings: usb: dwc3: Add interrupt-names property support dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions dt-bindings: usb: dwc3: Add Frame Length Adj restrictions dt-bindings: usb: meson-g12a-usb: Fix FL-adj property value dt-bindings: usb: meson-g12a-usb: Validate DWC2/DWC3 sub-nodes dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node arch: dts: Fix EHCI/OHCI DT nodes name arch: dts: Fix xHCI DT nodes name arch: dts: Fix DWC USB3 DT nodes name .../usb/amlogic,meson-g12a-usb-ctrl.yaml | 6 +- .../devicetree/bindings/usb/dwc3.txt | 125 ------- .../devicetree/bindings/usb/generic-xhci.yaml | 65 ++++ .../devicetree/bindings/usb/generic.txt | 57 ---- .../devicetree/bindings/usb/qcom,dwc3.yaml | 9 +- .../bindings/usb/renesas,usb-xhci.yaml | 4 +- .../devicetree/bindings/usb/snps,dwc3.yaml | 315 ++++++++++++++++++ .../bindings/usb/ti,keystone-dwc3.yaml | 4 +- .../devicetree/bindings/usb/usb-hcd.yaml | 104 ++++++ .../devicetree/bindings/usb/usb-xhci.txt | 41 --- .../devicetree/bindings/usb/usb-xhci.yaml | 42 +++ arch/arc/boot/dts/axs10x_mb.dtsi | 4 +- arch/arc/boot/dts/hsdk.dts | 4 +- arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +- arch/arm/boot/dts/armada-375.dtsi | 2 +- arch/arm/boot/dts/bcm5301x.dtsi | 6 +- arch/arm/boot/dts/bcm53573.dtsi | 4 +- arch/arm/boot/dts/exynos5250.dtsi | 2 +- arch/arm/boot/dts/exynos54xx.dtsi | 4 +- arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 +- arch/arm/boot/dts/keystone-k2e.dtsi | 4 +- arch/arm/boot/dts/keystone.dtsi | 2 +- arch/arm/boot/dts/lpc18xx.dtsi | 4 +- arch/arm/boot/dts/ls1021a.dtsi | 2 +- arch/arm/boot/dts/omap5-l4.dtsi | 2 +- arch/arm/boot/dts/stih407-family.dtsi | 2 +- arch/arm/boot/dts/stm32mp151.dtsi | 4 +- arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +- arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 +- arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +- .../arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 +- .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +- .../arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 +- .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 +- arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +- .../arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 4 +- arch/arm64/boot/dts/hisilicon/hip06.dtsi | 4 +- arch/arm64/boot/dts/hisilicon/hip07.dtsi | 4 +- arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 4 +- arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 +- arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 +- arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 +- arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +- arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +- arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 +- arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 +- arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +- arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +- arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +- arch/mips/boot/dts/mti/sead3.dts | 2 +- arch/mips/boot/dts/ralink/mt7628a.dtsi | 2 +- arch/powerpc/boot/dts/akebono.dts | 6 +- 53 files changed, 605 insertions(+), 305 deletions(-) delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt create mode 100644 Documentation/devicetree/bindings/usb/generic-xhci.yaml delete mode 100644 Documentation/devicetree/bindings/usb/generic.txt create mode 100644 Documentation/devicetree/bindings/usb/snps,dwc3.yaml delete mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.txt create mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.yaml