| Message ID | 20231202232217.89652-2-wahrenst@gmx.net |
|---|---|
| State | New |
| Headers | show |
| Series | ARM: dts: bcm2711: Add BCM2711 xHCI support | expand |
On Sun, Dec 03, 2023 at 12:22:15AM +0100, Stefan Wahren wrote: > The xHCI IP on the BCM2711 SoC is compatible to "brcm,xhci-brcm-v2", > but also requires a power domain. So introduce a new compatible > and the specific constraints. Since the key allOf can only occur > once, merge the reference below. > > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Cheers, Conor. > --- > .../devicetree/bindings/usb/generic-xhci.yaml | 21 ++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml > index 594ebb3ee432..b6e10b0a3c24 100644 > --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml > +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml > @@ -9,9 +9,6 @@ title: USB xHCI Controller > maintainers: > - Mathias Nyman <mathias.nyman@intel.com> > > -allOf: > - - $ref: usb-xhci.yaml# > - > properties: > compatible: > oneOf: > @@ -28,6 +25,7 @@ properties: > - description: Broadcom STB SoCs with xHCI > enum: > - brcm,xhci-brcm-v2 > + - brcm,bcm2711-xhci > - brcm,bcm7445-xhci > - description: Generic xHCI device > const: xhci-platform > @@ -49,6 +47,9 @@ properties: > - const: core > - const: reg > > + power-domains: > + maxItems: 1 > + > unevaluatedProperties: false > > required: > @@ -56,6 +57,20 @@ required: > - reg > - interrupts > > +allOf: > + - $ref: usb-xhci.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: brcm,bcm2711-xhci > + then: > + required: > + - power-domains > + else: > + properties: > + power-domains: false > + > examples: > - | > usb@f0931000 { > -- > 2.34.1 >
On Sun, Dec 03, 2023 at 11:06:43AM +0000, Conor Dooley wrote: > On Sun, Dec 03, 2023 at 12:22:15AM +0100, Stefan Wahren wrote: > > The xHCI IP on the BCM2711 SoC is compatible to "brcm,xhci-brcm-v2", > > but also requires a power domain. Hmm This & the driver change makes it look like your compatible setup should be `compatible = "brcm,bcm2711-xhci", "brcm,xhci-brcm-v2";. If the pattern in this patch was repeated, we'd have to modify the driver like your 2nd patch does for each and new broadcom system that needs the power domain. > > So introduce a new compatible > > and the specific constraints. Since the key allOf can only occur > > once, merge the reference below. > > > > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > Cheers, > Conor. > > > --- > > .../devicetree/bindings/usb/generic-xhci.yaml | 21 ++++++++++++++++--- > > 1 file changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml > > index 594ebb3ee432..b6e10b0a3c24 100644 > > --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml > > +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml > > @@ -9,9 +9,6 @@ title: USB xHCI Controller > > maintainers: > > - Mathias Nyman <mathias.nyman@intel.com> > > > > -allOf: > > - - $ref: usb-xhci.yaml# > > - > > properties: > > compatible: > > oneOf: > > @@ -28,6 +25,7 @@ properties: > > - description: Broadcom STB SoCs with xHCI > > enum: > > - brcm,xhci-brcm-v2 > > + - brcm,bcm2711-xhci > > - brcm,bcm7445-xhci > > - description: Generic xHCI device > > const: xhci-platform > > @@ -49,6 +47,9 @@ properties: > > - const: core > > - const: reg > > > > + power-domains: > > + maxItems: 1 > > + > > unevaluatedProperties: false > > > > required: > > @@ -56,6 +57,20 @@ required: > > - reg > > - interrupts > > > > +allOf: > > + - $ref: usb-xhci.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: brcm,bcm2711-xhci > > + then: > > + required: > > + - power-domains > > + else: > > + properties: > > + power-domains: false > > + > > examples: > > - | > > usb@f0931000 { > > -- > > 2.34.1 > >
Hi, Am 03.12.23 um 12:11 schrieb Conor Dooley: > On Sun, Dec 03, 2023 at 11:06:43AM +0000, Conor Dooley wrote: >> On Sun, Dec 03, 2023 at 12:22:15AM +0100, Stefan Wahren wrote: >>> The xHCI IP on the BCM2711 SoC is compatible to "brcm,xhci-brcm-v2", >>> but also requires a power domain. > Hmm > This & the driver change makes it look like your compatible setup should > be `compatible = "brcm,bcm2711-xhci", "brcm,xhci-brcm-v2";. i don't have insight into the hardware, but the fact that the other Broadcom SoC didn't require a power domain before let me think we shouldn't do this. Otherwise this binding was broken before. But Justin and Florian could clarify this. > If the pattern in this patch was repeated, we'd have to modify the > driver like your 2nd patch does for each and new broadcom system that > needs the power domain. From my understanding the DT compatible should be specific as possible. This is what i did, especially because the Raspberry Pi boards tends to needs some quirks. Best regards > > >>> So introduce a new compatible >>> and the specific constraints. Since the key allOf can only occur >>> once, merge the reference below. >>> >>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net> >> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >> >> Cheers, >> Conor. >> >>> --- >>> .../devicetree/bindings/usb/generic-xhci.yaml | 21 ++++++++++++++++--- >>> 1 file changed, 18 insertions(+), 3 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml >>> index 594ebb3ee432..b6e10b0a3c24 100644 >>> --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml >>> +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml >>> @@ -9,9 +9,6 @@ title: USB xHCI Controller >>> maintainers: >>> - Mathias Nyman <mathias.nyman@intel.com> >>> >>> -allOf: >>> - - $ref: usb-xhci.yaml# >>> - >>> properties: >>> compatible: >>> oneOf: >>> @@ -28,6 +25,7 @@ properties: >>> - description: Broadcom STB SoCs with xHCI >>> enum: >>> - brcm,xhci-brcm-v2 >>> + - brcm,bcm2711-xhci >>> - brcm,bcm7445-xhci >>> - description: Generic xHCI device >>> const: xhci-platform >>> @@ -49,6 +47,9 @@ properties: >>> - const: core >>> - const: reg >>> >>> + power-domains: >>> + maxItems: 1 >>> + >>> unevaluatedProperties: false >>> >>> required: >>> @@ -56,6 +57,20 @@ required: >>> - reg >>> - interrupts >>> >>> +allOf: >>> + - $ref: usb-xhci.yaml# >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: brcm,bcm2711-xhci >>> + then: >>> + required: >>> + - power-domains >>> + else: >>> + properties: >>> + power-domains: false >>> + >>> examples: >>> - | >>> usb@f0931000 { >>> -- >>> 2.34.1 >>> > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Sun, Dec 03, 2023 at 05:56:24PM +0100, Stefan Wahren wrote: > Hi, > > Am 03.12.23 um 12:11 schrieb Conor Dooley: > > On Sun, Dec 03, 2023 at 11:06:43AM +0000, Conor Dooley wrote: > > > On Sun, Dec 03, 2023 at 12:22:15AM +0100, Stefan Wahren wrote: > > > > The xHCI IP on the BCM2711 SoC is compatible to "brcm,xhci-brcm-v2", > > > > but also requires a power domain. > > Hmm > > This & the driver change makes it look like your compatible setup should > > be `compatible = "brcm,bcm2711-xhci", "brcm,xhci-brcm-v2";. > i don't have insight into the hardware, but the fact that the other > Broadcom SoC didn't require a power domain before let me think we > shouldn't do this. Otherwise this binding was broken before. But Justin > and Florian could clarify this. > > If the pattern in this patch was repeated, we'd have to modify the > > driver like your 2nd patch does for each and new broadcom system that > > needs the power domain. > From my understanding the DT compatible should be specific as possible. Note that I am suggesting have 2 compatibles. One specific, falling back to the existing generic one.
On 12/4/23 09:04, Conor Dooley wrote: > On Sun, Dec 03, 2023 at 05:56:24PM +0100, Stefan Wahren wrote: >> Hi, >> >> Am 03.12.23 um 12:11 schrieb Conor Dooley: >>> On Sun, Dec 03, 2023 at 11:06:43AM +0000, Conor Dooley wrote: >>>> On Sun, Dec 03, 2023 at 12:22:15AM +0100, Stefan Wahren wrote: >>>>> The xHCI IP on the BCM2711 SoC is compatible to "brcm,xhci-brcm-v2", >>>>> but also requires a power domain. >>> Hmm >>> This & the driver change makes it look like your compatible setup should >>> be `compatible = "brcm,bcm2711-xhci", "brcm,xhci-brcm-v2";. >> i don't have insight into the hardware, but the fact that the other >> Broadcom SoC didn't require a power domain before let me think we >> shouldn't do this. Otherwise this binding was broken before. But Justin >> and Florian could clarify this. That seems to me like the right approach, the XHCI controller in 2711 is tied to a power domain, however that is not the case for other Broadcom STB SoCs. >>> If the pattern in this patch was repeated, we'd have to modify the >>> driver like your 2nd patch does for each and new broadcom system that >>> needs the power domain. >> From my understanding the DT compatible should be specific as possible. > > Note that I am suggesting have 2 compatibles. One specific, falling back > to the existing generic one. We could do that, yes.
Hi, Am 04.12.23 um 18:49 schrieb Florian Fainelli: > On 12/4/23 09:04, Conor Dooley wrote: >> On Sun, Dec 03, 2023 at 05:56:24PM +0100, Stefan Wahren wrote: >>> Hi, >>> >>> Am 03.12.23 um 12:11 schrieb Conor Dooley: >>>> On Sun, Dec 03, 2023 at 11:06:43AM +0000, Conor Dooley wrote: >>>>> On Sun, Dec 03, 2023 at 12:22:15AM +0100, Stefan Wahren wrote: >>>>>> The xHCI IP on the BCM2711 SoC is compatible to "brcm,xhci-brcm-v2", >>>>>> but also requires a power domain. >>>> Hmm >>>> This & the driver change makes it look like your compatible setup >>>> should >>>> be `compatible = "brcm,bcm2711-xhci", "brcm,xhci-brcm-v2";. >>> i don't have insight into the hardware, but the fact that the other >>> Broadcom SoC didn't require a power domain before let me think we >>> shouldn't do this. Otherwise this binding was broken before. But Justin >>> and Florian could clarify this. > > That seems to me like the right approach, the XHCI controller in 2711 > is tied to a power domain, however that is not the case for other > Broadcom STB SoCs. before i send just another trial and error version of this series, this what i understand: diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml index 594ebb3ee432..6f8744d1ace7 100644 --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml @@ -9,9 +9,6 @@ title: USB xHCI Controller maintainers: - Mathias Nyman <mathias.nyman@intel.com> -allOf: - - $ref: usb-xhci.yaml# - properties: compatible: oneOf: @@ -25,6 +22,11 @@ properties: - marvell,armada-380-xhci - marvell,armada-8k-xhci - const: generic-xhci + - description: Broadcom BCM2711 SoC + items: + - enum: + - brcm,bcm2711-xhci + - const: brcm,xhci-brcm-v2 - description: Broadcom STB SoCs with xHCI enum: - brcm,xhci-brcm-v2 @@ -49,6 +51,9 @@ properties: - const: core - const: reg + power-domains: + maxItems: 1 + unevaluatedProperties: false required: @@ -56,6 +61,20 @@ required: - reg - interrupts +allOf: + - $ref: usb-xhci.yaml# + - if: + properties: + compatible: + contains: + const: brcm,bcm2711-xhci + then: + required: + - power-domains + else: + properties: + power-domains: false + examples: - | usb@f0931000 {
On Mon, Dec 04, 2023 at 09:27:54PM +0100, Stefan Wahren wrote: > Hi, > > Am 04.12.23 um 18:49 schrieb Florian Fainelli: > > On 12/4/23 09:04, Conor Dooley wrote: > > > On Sun, Dec 03, 2023 at 05:56:24PM +0100, Stefan Wahren wrote: > > > > Hi, > > > > > > > > Am 03.12.23 um 12:11 schrieb Conor Dooley: > > > > > On Sun, Dec 03, 2023 at 11:06:43AM +0000, Conor Dooley wrote: > > > > > > On Sun, Dec 03, 2023 at 12:22:15AM +0100, Stefan Wahren wrote: > > > > > > > The xHCI IP on the BCM2711 SoC is compatible to "brcm,xhci-brcm-v2", > > > > > > > but also requires a power domain. > > > > > Hmm > > > > > This & the driver change makes it look like your compatible setup > > > > > should > > > > > be `compatible = "brcm,bcm2711-xhci", "brcm,xhci-brcm-v2";. > > > > i don't have insight into the hardware, but the fact that the other > > > > Broadcom SoC didn't require a power domain before let me think we > > > > shouldn't do this. Otherwise this binding was broken before. But Justin > > > > and Florian could clarify this. > > > > That seems to me like the right approach, the XHCI controller in 2711 > > is tied to a power domain, however that is not the case for other > > Broadcom STB SoCs. > before i send just another trial and error version of this series, this > what i understand: > > diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml > b/Documentation/devicetree/bindings/usb/generic-xhci.yaml > index 594ebb3ee432..6f8744d1ace7 100644 > --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml > +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml > @@ -9,9 +9,6 @@ title: USB xHCI Controller > maintainers: > - Mathias Nyman <mathias.nyman@intel.com> > > -allOf: > - - $ref: usb-xhci.yaml# > - > properties: > compatible: > oneOf: > @@ -25,6 +22,11 @@ properties: > - marvell,armada-380-xhci > - marvell,armada-8k-xhci > - const: generic-xhci > + - description: Broadcom BCM2711 SoC > + items: The comment and enum here kinda contradict eachother. I'd probably say "Broadcom SoCs with power domains" or just leave the comment out altogether. But in general, yes. You can keep the tag. Thanks, Conor. > + - enum: > + - brcm,bcm2711-xhci > + - const: brcm,xhci-brcm-v2 > - description: Broadcom STB SoCs with xHCI > enum: > - brcm,xhci-brcm-v2 > @@ -49,6 +51,9 @@ properties: > - const: core > - const: reg > > + power-domains: > + maxItems: 1 > + > unevaluatedProperties: false > > required: > @@ -56,6 +61,20 @@ required: > - reg > - interrupts > > +allOf: > + - $ref: usb-xhci.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: brcm,bcm2711-xhci > + then: > + required: > + - power-domains > + else: > + properties: > + power-domains: false > + > examples: > - | > usb@f0931000 { > >
diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml index 594ebb3ee432..b6e10b0a3c24 100644 --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml @@ -9,9 +9,6 @@ title: USB xHCI Controller maintainers: - Mathias Nyman <mathias.nyman@intel.com> -allOf: - - $ref: usb-xhci.yaml# - properties: compatible: oneOf: @@ -28,6 +25,7 @@ properties: - description: Broadcom STB SoCs with xHCI enum: - brcm,xhci-brcm-v2 + - brcm,bcm2711-xhci - brcm,bcm7445-xhci - description: Generic xHCI device const: xhci-platform @@ -49,6 +47,9 @@ properties: - const: core - const: reg + power-domains: + maxItems: 1 + unevaluatedProperties: false required: @@ -56,6 +57,20 @@ required: - reg - interrupts +allOf: + - $ref: usb-xhci.yaml# + - if: + properties: + compatible: + contains: + const: brcm,bcm2711-xhci + then: + required: + - power-domains + else: + properties: + power-domains: false + examples: - | usb@f0931000 {
The xHCI IP on the BCM2711 SoC is compatible to "brcm,xhci-brcm-v2", but also requires a power domain. So introduce a new compatible and the specific constraints. Since the key allOf can only occur once, merge the reference below. Signed-off-by: Stefan Wahren <wahrenst@gmx.net> --- .../devicetree/bindings/usb/generic-xhci.yaml | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) -- 2.34.1