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

From: Rob Herring <robh@kernel.org>
To: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
Date: Tue, 21 Dec 2021 12:59:52 -0400	[thread overview]
Message-ID: <YcIICIKjbgEn0I+P@robh.at.kernel.org> (raw)
In-Reply-To: <20211216160541.544974-2-alexander.stein@ew.tq-group.com>

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy