| Message ID | 20211216160541.544974-2-alexander.stein@ew.tq-group.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | i.MX8MP: more USB3 glue layer feature support | expand |
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 > >
Hello, thanks for the review. Am Dienstag, 21. Dezember 2021, 17:59:52 CET schrieb Rob Herring: > 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 '_'. Alright, got it. > > .../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. You mean describing the downstream hub in device tree? I guess you are thinking about Documentation/devicetree/bindings/usb/usb-device.yaml, no? I'll try using this. This flag (and the others below) are used to set some specific flag in the host controller (not the PHY, see Li Jun's response). But I have to admit I do not know what they actually do. The description is pretty much everything written in the reference manual. > > + > > + 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. Thanks for the hints on how to write bindings, still fiddling with it. Best regards, Alexander
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. + + 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 + +else: + properties: + reg: + maxItems: 1 + items: + - description: PHY register base address + fsl,permanently-attached: false + fsl,disable-port-power-control: false + fsl,over-current-active-low: false + fsl,power-active-low: false + additionalProperties: false examples:
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? .../bindings/phy/fsl,imx8mq-usb-phy.yaml | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-)