| Message ID | 20230125175943.675823-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: usb: samsung,exynos-dwc3: allow unit address in DTS | expand |
On Wed, Jan 25, 2023 at 06:59:43PM +0100, Krzysztof Kozlowski wrote: > The Samsung Exynos SoC USB 3.0 DWC3 Controller is a simple wrapper of > actual DWC3 Controller device node. It handles necessary Samsung > Exynos-specific resources (regulators, clocks), but does not have its > own MMIO address space. > > However neither simple-bus bindings nor dtc W=1 accept device nodes in > soc@ node which do not have unit address. Therefore allow using > the address space of child device (actual DWC3 Controller) as the > wrapper's address. The correct fix is 'ranges' should have a value. Though the whole wrapper thing when there are no registers I dislike... > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > DTS fixes are here: > https://lore.kernel.org/linux-samsung-soc/20230125175751.675090-1-krzysztof.kozlowski@linaro.org/T/#t > --- > .../devicetree/bindings/usb/samsung,exynos-dwc3.yaml | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml b/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml > index 6b9a3bcb3926..a94b1926dda0 100644 > --- a/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml > +++ b/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml > @@ -29,6 +29,9 @@ properties: > > ranges: true > > + reg: > + maxItems: 1 > + > '#size-cells': > const: 1 > > @@ -108,8 +111,9 @@ examples: > #include <dt-bindings/clock/exynos5420.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > > - usb { > + usb-wrapper@12000000 { > compatible = "samsung,exynos5250-dwusb3"; > + reg = <0x12000000 0x10000>; > #address-cells = <1>; > #size-cells = <1>; > ranges; > -- > 2.34.1 >
On 25/01/2023 22:13, Rob Herring wrote: > On Wed, Jan 25, 2023 at 06:59:43PM +0100, Krzysztof Kozlowski wrote: >> The Samsung Exynos SoC USB 3.0 DWC3 Controller is a simple wrapper of >> actual DWC3 Controller device node. It handles necessary Samsung >> Exynos-specific resources (regulators, clocks), but does not have its >> own MMIO address space. >> >> However neither simple-bus bindings nor dtc W=1 accept device nodes in >> soc@ node which do not have unit address. Therefore allow using >> the address space of child device (actual DWC3 Controller) as the >> wrapper's address. > > The correct fix is 'ranges' should have a value. Though the whole > wrapper thing when there are no registers I dislike... You mean something like this (diff against this patchset): ---------- diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi index 08786fd9c6ea..75b6f9678672 100644 --- a/arch/arm/boot/dts/exynos54xx.dtsi +++ b/arch/arm/boot/dts/exynos54xx.dtsi @@ -142,16 +142,15 @@ hsi2c_7: i2c@12cd0000 { status = "disabled"; }; - usbdrd3_0: usb-wrapper@12000000 { + usbdrd3_0: usb-wrapper { compatible = "samsung,exynos5250-dwusb3"; - reg = <0x12000000 0x10000>; #address-cells = <1>; #size-cells = <1>; - ranges; + ranges = <0x0 0x12000000 0x10000>; - usbdrd_dwc3_0: usb@12000000 { + usbdrd_dwc3_0: usb@0 { compatible = "snps,dwc3"; - reg = <0x12000000 0x10000>; + reg = <0x0 0x10000>; --------- Unfortunately dtc W=1 is still not happy: exynos54xx.dtsi:145.26-159.5: Warning (unit_address_vs_reg): /soc/usb-wrapper: node has a reg or ranges property, but no unit name neither dtbs_check is: exynos5410-smdk5410.dtb: soc: usb-wrapper: {'compatible': ['samsung,exynos5250-dwusb3'], '#address-cells': [[1]], '#size-cells': [[1]], 'ranges': [[0, 301989888, 65536]], 'clocks': [[5, 366]], 'clock-names': ['usbdrd30'], 'pinctrl-names': ['default'], 'pinctrl-0': [[21, 22]], 'vdd10-supply': [[23]], 'vdd33-supply': [[23]], 'usb@0': {'compatible': ['snps,dwc3'], 'reg': [[0, 65536]], 'interrupts': [[0, 72, 4]], 'phys': [[24, 0], [24, 1]], 'phy-names': ['usb2-phy', 'usb3-phy'], 'snps,dis_u3_susphy_quirk': True}} should not be valid under {'type': 'object'} From schema: /home/krzk/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml Best regards, Krzysztof
On Thu, Jan 26, 2023 at 4:48 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 25/01/2023 22:13, Rob Herring wrote: > > On Wed, Jan 25, 2023 at 06:59:43PM +0100, Krzysztof Kozlowski wrote: > >> The Samsung Exynos SoC USB 3.0 DWC3 Controller is a simple wrapper of > >> actual DWC3 Controller device node. It handles necessary Samsung > >> Exynos-specific resources (regulators, clocks), but does not have its > >> own MMIO address space. > >> > >> However neither simple-bus bindings nor dtc W=1 accept device nodes in > >> soc@ node which do not have unit address. Therefore allow using > >> the address space of child device (actual DWC3 Controller) as the > >> wrapper's address. > > > > The correct fix is 'ranges' should have a value. Though the whole > > wrapper thing when there are no registers I dislike... > > You mean something like this (diff against this patchset): > ---------- > diff --git a/arch/arm/boot/dts/exynos54xx.dtsi > b/arch/arm/boot/dts/exynos54xx.dtsi > index 08786fd9c6ea..75b6f9678672 100644 > --- a/arch/arm/boot/dts/exynos54xx.dtsi > +++ b/arch/arm/boot/dts/exynos54xx.dtsi > @@ -142,16 +142,15 @@ hsi2c_7: i2c@12cd0000 { > status = "disabled"; > }; > > - usbdrd3_0: usb-wrapper@12000000 { > + usbdrd3_0: usb-wrapper { Why did you drop the unit-address? Unit-address is valid with 'reg' or 'ranges'. > compatible = "samsung,exynos5250-dwusb3"; > - reg = <0x12000000 0x10000>; > #address-cells = <1>; > #size-cells = <1>; > - ranges; > + ranges = <0x0 0x12000000 0x10000>; > > - usbdrd_dwc3_0: usb@12000000 { > + usbdrd_dwc3_0: usb@0 { > compatible = "snps,dwc3"; > - reg = <0x12000000 0x10000>; > + reg = <0x0 0x10000>; > > --------- > > Unfortunately dtc W=1 is still not happy: > > exynos54xx.dtsi:145.26-159.5: Warning (unit_address_vs_reg): > /soc/usb-wrapper: node has a reg or ranges property, but no unit name > > neither dtbs_check is: > > exynos5410-smdk5410.dtb: soc: usb-wrapper: {'compatible': > ['samsung,exynos5250-dwusb3'], '#address-cells': [[1]], '#size-cells': > [[1]], 'ranges': [[0, 301989888, 65536]], 'clocks': [[5, 366]], > 'clock-names': ['usbdrd30'], 'pinctrl-names': ['default'], 'pinctrl-0': > [[21, 22]], 'vdd10-supply': [[23]], 'vdd33-supply': [[23]], 'usb@0': > {'compatible': ['snps,dwc3'], 'reg': [[0, 65536]], 'interrupts': [[0, > 72, 4]], 'phys': [[24, 0], [24, 1]], 'phy-names': ['usb2-phy', > 'usb3-phy'], 'snps,dis_u3_susphy_quirk': True}} should not be valid > under {'type': 'object'} > From schema: > /home/krzk/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml > > > Best regards, > Krzysztof >
On 26/01/2023 14:41, Rob Herring wrote: > On Thu, Jan 26, 2023 at 4:48 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 25/01/2023 22:13, Rob Herring wrote: >>> On Wed, Jan 25, 2023 at 06:59:43PM +0100, Krzysztof Kozlowski wrote: >>>> The Samsung Exynos SoC USB 3.0 DWC3 Controller is a simple wrapper of >>>> actual DWC3 Controller device node. It handles necessary Samsung >>>> Exynos-specific resources (regulators, clocks), but does not have its >>>> own MMIO address space. >>>> >>>> However neither simple-bus bindings nor dtc W=1 accept device nodes in >>>> soc@ node which do not have unit address. Therefore allow using >>>> the address space of child device (actual DWC3 Controller) as the >>>> wrapper's address. >>> >>> The correct fix is 'ranges' should have a value. Though the whole >>> wrapper thing when there are no registers I dislike... >> >> You mean something like this (diff against this patchset): >> ---------- >> diff --git a/arch/arm/boot/dts/exynos54xx.dtsi >> b/arch/arm/boot/dts/exynos54xx.dtsi >> index 08786fd9c6ea..75b6f9678672 100644 >> --- a/arch/arm/boot/dts/exynos54xx.dtsi >> +++ b/arch/arm/boot/dts/exynos54xx.dtsi >> @@ -142,16 +142,15 @@ hsi2c_7: i2c@12cd0000 { >> status = "disabled"; >> }; >> >> - usbdrd3_0: usb-wrapper@12000000 { >> + usbdrd3_0: usb-wrapper { > > Why did you drop the unit-address? Unit-address is valid with 'reg' or 'ranges'. I misunderstood your comment then. To which problem did you refer with "The correct fix is ranges ...."? To my understanding this only changes the unit address, so I won't have to change the node name usb->usb-wrapper. Except this, my patches having empty ranges are equivalent. > >> compatible = "samsung,exynos5250-dwusb3"; >> - reg = <0x12000000 0x10000>; >> #address-cells = <1>; >> #size-cells = <1>; >> - ranges; >> + ranges = <0x0 0x12000000 0x10000>; >> Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml b/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml index 6b9a3bcb3926..a94b1926dda0 100644 --- a/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml @@ -29,6 +29,9 @@ properties: ranges: true + reg: + maxItems: 1 + '#size-cells': const: 1 @@ -108,8 +111,9 @@ examples: #include <dt-bindings/clock/exynos5420.h> #include <dt-bindings/interrupt-controller/arm-gic.h> - usb { + usb-wrapper@12000000 { compatible = "samsung,exynos5250-dwusb3"; + reg = <0x12000000 0x10000>; #address-cells = <1>; #size-cells = <1>; ranges;
The Samsung Exynos SoC USB 3.0 DWC3 Controller is a simple wrapper of actual DWC3 Controller device node. It handles necessary Samsung Exynos-specific resources (regulators, clocks), but does not have its own MMIO address space. However neither simple-bus bindings nor dtc W=1 accept device nodes in soc@ node which do not have unit address. Therefore allow using the address space of child device (actual DWC3 Controller) as the wrapper's address. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- DTS fixes are here: https://lore.kernel.org/linux-samsung-soc/20230125175751.675090-1-krzysztof.kozlowski@linaro.org/T/#t --- .../devicetree/bindings/usb/samsung,exynos-dwc3.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)