Re: [PATCH V3 1/3] dt-bindings: usb: xhci: add support for BCM2711

Conor Dooley
2023-12-04T20:42:52Z

[-- Attachment #1: Type: text/plain, Size: 3089 bytes --]

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 {
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]