Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

Rob Herring
2021-12-21T16:59:58Z

On Thu, Dec 16, 2021 at 05:05:39PM +0100, Alexander Stein wrote:
> This adds bindings for features only available on imx8mp. They allow
> setting polarity of PWR and OC as well as disabling port power control.
> Also permanently atteched can be annotated as well.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Adding properties specific to one compatible globally and disabling them on
> other compatibles is the way to go?
>
> Are there any best practices on the usage of '-' and/or '_' in property names?

Yes, don't use '_'.

>
> .../bindings/phy/fsl,imx8mq-usb-phy.yaml | 52 ++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> index 2936f3510a6a..1d28b7d1c413 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> @@ -16,7 +16,8 @@ properties:
> - fsl,imx8mp-usb-phy
>
> reg:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
>
> "#phy-cells":
> const: 0
> @@ -32,6 +33,28 @@ properties:
> description:
> A phandle to the regulator for USB VBUS.
>
> + fsl,permanently-attached:
> + type: boolean
> + description:
> + Indicates if the device atached to a downstream port is
> + permanently attached.

Wouldn't just describing the downstream device be enough to indicate
this? Though that is in the host controller rather than the phy.

> +
> + fsl,disable-port-power-control:
> + type: boolean
> + description:
> + Indicates whether the host controller implementation includes port
> + power control. Defines Bit 3 in capability register (HCCPARAMS).
> +
> + fsl,over-current-active-low:
> + type: boolean
> + description:
> + Over current signal polarity is active low.
> +
> + fsl,power-active-low:
> + type: boolean
> + description:
> + Power pad (PWR) polarity is active low.
> +
> required:
> - compatible
> - reg
> @@ -39,6 +62,33 @@ required:
> - clocks
> - clock-names
>
> +if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - fsl,imx8mp-usb-phy
> +
> +then:
> + properties:
> + reg:
> + minItems: 2
> + maxItems: 2
> + items:
> + - description: PHY register base address
> + - description: Glue layer base address

Move 'items' to the top level and then here you only need 'minItems: 2'.

> +
> +else:
> + properties:
> + reg:
> + maxItems: 1
> + items:
> + - description: PHY register base address

And just 'maxItems' here.

> + fsl,permanently-attached: false
> + fsl,disable-port-power-control: false
> + fsl,over-current-active-low: false
> + fsl,power-active-low: false
> +
> additionalProperties: false
>
> examples:
> --
> 2.25.1