| Message ID | 20240720-a38x-utmi-phy-v3-3-4c16f9abdbdc@solid-run.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | phy: mvebu-cp110-utmi: add support for armada-380 utmi phys | expand |
On 20/07/2024 16:19, Josua Mayer wrote: > Armada 38x USB-2.0 PHYs are similar to Armada 8K (CP110) and can be > supported by the same driver with small differences. > > Add new compatible string for armada-38x variant of utmi phy. > Then add descriptions and names for two additional register definitions > that may be specified instead of a syscon phandle. > > Signed-off-by: Josua Mayer <josua@solid-run.com> > --- > .../phy/marvell,armada-cp110-utmi-phy.yaml | 34 ++++++++++++++++++---- > 1 file changed, 29 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml > index 9ce7b4c6d208..246e48d51755 100644 > --- a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml > @@ -23,12 +23,36 @@ description: > UTMI PHY1 --------\ > 1.H----- USB HOST1 > > + On Armada 380 there is an additional USB-2.0-only controller, > + and an additional UTMI PHY respectively. > + The USB device controller can only be connected to a single UTMI PHY port, > + either UTMI PHY0 or UTMI PHY2. > + > + > + One blank line is enough. > properties: > compatible: > - const: marvell,cp110-utmi-phy > + enum: > + - marvell,a38x-utmi-phy > + - marvell,cp110-utmi-phy > > reg: > - maxItems: 1 > + anyOf: That's oneOf. > + - items: > + - description: UTMI registers > + - items: > + - description: UTMI registers > + - description: USB config register > + - description: UTMI config registers > + > + reg-names: > + anyOf: oneOf > + - items: > + - const: utmi > + - items: > + - const: utmi > + - const: usb-cfg > + - const: utmi-cfg > > "#address-cells": > const: 1 > @@ -38,13 +62,14 @@ properties: > > marvell,system-controller: > description: > - Phandle to the system controller node > + Phandle to the system controller node. > + Optional when usb-cfg and utmi-cfg regs are given. > $ref: /schemas/types.yaml#/definitions/phandle > > # Required child nodes: > > patternProperties: > - "^usb-phy@[0|1]$": > + "^usb-phy@[0|1|2]$": [0-2] > type: object > description: > Each UTMI PHY port must be represented as a sub-node. > @@ -68,7 +93,6 @@ required: > - reg > - "#address-cells" > - "#size-cells" > - - marvell,system-controller you miss here allOf:if:then: narrowing and marvell,system-controller per each variant. Best regards, Krzysztof
Am 21.07.24 um 11:31 schrieb Krzysztof Kozlowski: > On 20/07/2024 16:19, Josua Mayer wrote: >> Armada 38x USB-2.0 PHYs are similar to Armada 8K (CP110) and can be >> supported by the same driver with small differences. >> >> Add new compatible string for armada-38x variant of utmi phy. >> Then add descriptions and names for two additional register definitions >> that may be specified instead of a syscon phandle. >> >> Signed-off-by: Josua Mayer <josua@solid-run.com> >> --- >> .../phy/marvell,armada-cp110-utmi-phy.yaml | 34 ++++++++++++++++++---- >> 1 file changed, 29 insertions(+), 5 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >> index 9ce7b4c6d208..246e48d51755 100644 >> --- a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >> +++ b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >> @@ -23,12 +23,36 @@ description: >> UTMI PHY1 --------\ >> 1.H----- USB HOST1 >> >> + On Armada 380 there is an additional USB-2.0-only controller, >> + and an additional UTMI PHY respectively. >> + The USB device controller can only be connected to a single UTMI PHY port, >> + either UTMI PHY0 or UTMI PHY2. >> + >> + >> + > One blank line is enough. Ack. > >> properties: >> compatible: >> - const: marvell,cp110-utmi-phy >> + enum: >> + - marvell,a38x-utmi-phy >> + - marvell,cp110-utmi-phy >> >> reg: >> - maxItems: 1 >> + anyOf: > That's oneOf. Acknowledged, thanks! Today oneOf seems correct to me, too. > >> + - items: >> + - description: UTMI registers >> + - items: >> + - description: UTMI registers >> + - description: USB config register >> + - description: UTMI config registers >> + >> + reg-names: >> + anyOf: > oneOf Ack. > >> + - items: >> + - const: utmi >> + - items: >> + - const: utmi >> + - const: usb-cfg >> + - const: utmi-cfg >> >> "#address-cells": >> const: 1 >> @@ -38,13 +62,14 @@ properties: >> >> marvell,system-controller: >> description: >> - Phandle to the system controller node >> + Phandle to the system controller node. >> + Optional when usb-cfg and utmi-cfg regs are given. >> $ref: /schemas/types.yaml#/definitions/phandle >> >> # Required child nodes: >> >> patternProperties: >> - "^usb-phy@[0|1]$": >> + "^usb-phy@[0|1|2]$": > [0-2] Ack. > >> type: object >> description: >> Each UTMI PHY port must be represented as a sub-node. >> @@ -68,7 +93,6 @@ required: >> - reg >> - "#address-cells" >> - "#size-cells" >> - - marvell,system-controller > you miss here allOf:if:then: narrowing and marvell,system-controller per > each variant. Correct. I will learn how to do that, and included it a non-rfc version. Thanks! sincerely Josua Mayer
Am 22.07.24 um 17:05 schrieb Josua Mayer: > Am 21.07.24 um 11:31 schrieb Krzysztof Kozlowski: >> On 20/07/2024 16:19, Josua Mayer wrote: >>> Armada 38x USB-2.0 PHYs are similar to Armada 8K (CP110) and can be >>> supported by the same driver with small differences. >>> >>> Add new compatible string for armada-38x variant of utmi phy. >>> Then add descriptions and names for two additional register definitions >>> that may be specified instead of a syscon phandle. >>> >>> Signed-off-by: Josua Mayer <josua@solid-run.com> >>> --- >>> .../phy/marvell,armada-cp110-utmi-phy.yaml | 34 ++++++++++++++++++---- >>> 1 file changed, 29 insertions(+), 5 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >>> index 9ce7b4c6d208..246e48d51755 100644 >>> --- a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >>> +++ b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml cut >>> @@ -68,7 +93,6 @@ required: >>> - reg >>> - "#address-cells" >>> - "#size-cells" >>> - - marvell,system-controller >> you miss here allOf:if:then: narrowing and marvell,system-controller per >> each variant. I am struggling a bit with the options. First attempt says: if not both usb-cfg and utmi-cfg reg-names are specified, then marvell,system-controller is required. allOf: - required: - compatible - reg - "#address-cells" - "#size-cells" - if: not: properties: reg-names: allOf: - contains: const: usb-cfg - contains: const: utmi-cfg then: required: - marvell,system-controller This works okay for any combinations of reg-names. However when device-tree is missing reg-names all together, marvell,system-controller is not marked required. Would it be acceptable to make reg-names required?
On 22/07/2024 17:14, Josua Mayer wrote: > > Am 22.07.24 um 17:05 schrieb Josua Mayer: >> Am 21.07.24 um 11:31 schrieb Krzysztof Kozlowski: >>> On 20/07/2024 16:19, Josua Mayer wrote: >>>> Armada 38x USB-2.0 PHYs are similar to Armada 8K (CP110) and can be >>>> supported by the same driver with small differences. >>>> >>>> Add new compatible string for armada-38x variant of utmi phy. >>>> Then add descriptions and names for two additional register definitions >>>> that may be specified instead of a syscon phandle. >>>> >>>> Signed-off-by: Josua Mayer <josua@solid-run.com> >>>> --- >>>> .../phy/marvell,armada-cp110-utmi-phy.yaml | 34 ++++++++++++++++++---- >>>> 1 file changed, 29 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >>>> index 9ce7b4c6d208..246e48d51755 100644 >>>> --- a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >>>> +++ b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml > cut >>>> @@ -68,7 +93,6 @@ required: >>>> - reg >>>> - "#address-cells" >>>> - "#size-cells" >>>> - - marvell,system-controller >>> you miss here allOf:if:then: narrowing and marvell,system-controller per >>> each variant. > I am struggling a bit with the options. > > First attempt says: if not both usb-cfg and utmi-cfg reg-names are specified, > then marvell,system-controller is required. > > allOf: > - required: That's not part of allOf. > - compatible > - reg > - "#address-cells" > - "#size-cells" > - if: > not: > properties: > reg-names: > allOf: > - contains: > const: usb-cfg > - contains: > const: utmi-cfg > then: > required: > - marvell,system-controller > > This works okay for any combinations of reg-names. ??? I expected this to be per variant. > > However when device-tree is missing reg-names all together, > marvell,system-controller is not marked required. > > Would it be acceptable to make reg-names required? I don't understand what you want to achieve. Best regards, Krzysztof
Am 22.07.24 um 17:17 schrieb Krzysztof Kozlowski: > On 22/07/2024 17:14, Josua Mayer wrote: >> Am 22.07.24 um 17:05 schrieb Josua Mayer: >>> Am 21.07.24 um 11:31 schrieb Krzysztof Kozlowski: >>>> On 20/07/2024 16:19, Josua Mayer wrote: >>>>> Armada 38x USB-2.0 PHYs are similar to Armada 8K (CP110) and can be >>>>> supported by the same driver with small differences. >>>>> >>>>> Add new compatible string for armada-38x variant of utmi phy. >>>>> Then add descriptions and names for two additional register definitions >>>>> that may be specified instead of a syscon phandle. >>>>> >>>>> Signed-off-by: Josua Mayer <josua@solid-run.com> >>>>> --- >>>>> .../phy/marvell,armada-cp110-utmi-phy.yaml | 34 ++++++++++++++++++---- >>>>> 1 file changed, 29 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >>>>> index 9ce7b4c6d208..246e48d51755 100644 >>>>> --- a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >>>>> +++ b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml >> cut >>>>> @@ -68,7 +93,6 @@ required: >>>>> - reg >>>>> - "#address-cells" >>>>> - "#size-cells" >>>>> - - marvell,system-controller >>>> you miss here allOf:if:then: narrowing and marvell,system-controller per >>>> each variant. >> I am struggling a bit with the options. >> >> First attempt says: if not both usb-cfg and utmi-cfg reg-names are specified, >> then marvell,system-controller is required. >> >> allOf: >> - required: > That's not part of allOf. > >> - compatible >> - reg >> - "#address-cells" >> - "#size-cells" >> - if: >> not: >> properties: >> reg-names: >> allOf: >> - contains: >> const: usb-cfg >> - contains: >> const: utmi-cfg >> then: >> required: >> - marvell,system-controller >> >> This works okay for any combinations of reg-names. > ??? I expected this to be per variant. As in by compatible string? > >> However when device-tree is missing reg-names all together, >> marvell,system-controller is not marked required. >> >> Would it be acceptable to make reg-names required? > I don't understand what you want to achieve. When there are both usb-cfg and utmi-cfg regs, then marvell,system-controller is optional, regardless of armada 380 or 8k. sincerely Josua Mayer
On 22/07/2024 17:31, Josua Mayer wrote: >> >>> - compatible >>> - reg >>> - "#address-cells" >>> - "#size-cells" >>> - if: >>> not: >>> properties: >>> reg-names: >>> allOf: >>> - contains: >>> const: usb-cfg >>> - contains: >>> const: utmi-cfg >>> then: >>> required: >>> - marvell,system-controller >>> >>> This works okay for any combinations of reg-names. >> ??? I expected this to be per variant. > As in by compatible string? Yes, each device has fixed properties, at least usually. >> >>> However when device-tree is missing reg-names all together, >>> marvell,system-controller is not marked required. >>> >>> Would it be acceptable to make reg-names required? >> I don't understand what you want to achieve. > When there are both usb-cfg and utmi-cfg regs, > then marvell,system-controller is optional, > > regardless of armada 380 or 8k. Whether the device has additional MMIO address space, depends on type of the device, not on some other properties. IOW, either you have here second reg or not. The hardware has or has not. Best regards, Krzysztof
Am 22.07.24 um 17:45 schrieb Krzysztof Kozlowski: > On 22/07/2024 17:31, Josua Mayer wrote: >>>> - compatible >>>> - reg >>>> - "#address-cells" >>>> - "#size-cells" >>>> - if: >>>> not: >>>> properties: >>>> reg-names: >>>> allOf: >>>> - contains: >>>> const: usb-cfg >>>> - contains: >>>> const: utmi-cfg >>>> then: >>>> required: >>>> - marvell,system-controller >>>> >>>> This works okay for any combinations of reg-names. >>> ??? I expected this to be per variant. >> As in by compatible string? > Yes, each device has fixed properties, at least usually. > >>>> However when device-tree is missing reg-names all together, >>>> marvell,system-controller is not marked required. >>>> >>>> Would it be acceptable to make reg-names required? >>> I don't understand what you want to achieve. >> When there are both usb-cfg and utmi-cfg regs, >> then marvell,system-controller is optional, >> >> regardless of armada 380 or 8k. > Whether the device has additional MMIO address space, depends on type of > the device, not on some other properties. IOW, either you have here > second reg or not. The hardware has or has not. At least Armada 8K and Armada 388 both have them (physically). The difference is one of them uses syscon for access. Consequently, would it be better then to always require all 3? (In driver I could use lack of syscon handle to decide, currently I use presence of regs by name) sincerely Josua Mayer
diff --git a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml index 9ce7b4c6d208..246e48d51755 100644 --- a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml +++ b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml @@ -23,12 +23,36 @@ description: UTMI PHY1 --------\ 1.H----- USB HOST1 + On Armada 380 there is an additional USB-2.0-only controller, + and an additional UTMI PHY respectively. + The USB device controller can only be connected to a single UTMI PHY port, + either UTMI PHY0 or UTMI PHY2. + + + properties: compatible: - const: marvell,cp110-utmi-phy + enum: + - marvell,a38x-utmi-phy + - marvell,cp110-utmi-phy reg: - maxItems: 1 + anyOf: + - items: + - description: UTMI registers + - items: + - description: UTMI registers + - description: USB config register + - description: UTMI config registers + + reg-names: + anyOf: + - items: + - const: utmi + - items: + - const: utmi + - const: usb-cfg + - const: utmi-cfg "#address-cells": const: 1 @@ -38,13 +62,14 @@ properties: marvell,system-controller: description: - Phandle to the system controller node + Phandle to the system controller node. + Optional when usb-cfg and utmi-cfg regs are given. $ref: /schemas/types.yaml#/definitions/phandle # Required child nodes: patternProperties: - "^usb-phy@[0|1]$": + "^usb-phy@[0|1|2]$": type: object description: Each UTMI PHY port must be represented as a sub-node. @@ -68,7 +93,6 @@ required: - reg - "#address-cells" - "#size-cells" - - marvell,system-controller additionalProperties: false
Armada 38x USB-2.0 PHYs are similar to Armada 8K (CP110) and can be supported by the same driver with small differences. Add new compatible string for armada-38x variant of utmi phy. Then add descriptions and names for two additional register definitions that may be specified instead of a syscon phandle. Signed-off-by: Josua Mayer <josua@solid-run.com> --- .../phy/marvell,armada-cp110-utmi-phy.yaml | 34 ++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-)