2022-10-14 10:09:34

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V2 4/6] dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format

From: Peng Fan <[email protected]>

Convert the binding to DT schema format

Signed-off-by: Peng Fan <[email protected]>
---
.../devicetree/bindings/usb/ci-hdrc-usb2.txt | 158 --------
.../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 341 ++++++++++++++++++
2 files changed, 341 insertions(+), 158 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
deleted file mode 100644
index ba51fb1252b9..000000000000
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ /dev/null
@@ -1,158 +0,0 @@
-* USB2 ChipIdea USB controller for ci13xxx
-
-Required properties:
-- compatible: should be one of:
- "fsl,imx23-usb"
- "fsl,imx27-usb"
- "fsl,imx28-usb"
- "fsl,imx6q-usb"
- "fsl,imx6sl-usb"
- "fsl,imx6sx-usb"
- "fsl,imx6ul-usb"
- "fsl,imx7d-usb"
- "fsl,imx7ulp-usb"
- "lsi,zevio-usb"
- "qcom,ci-hdrc"
- "chipidea,usb2"
- "xlnx,zynq-usb-2.20a"
- "nvidia,tegra20-udc"
- "nvidia,tegra30-udc"
- "nvidia,tegra114-udc"
- "nvidia,tegra124-udc"
-- reg: base address and length of the registers
-- interrupts: interrupt for the USB controller
-
-Recommended properies:
-- phy_type: the type of the phy connected to the core. Should be one
- of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
- property the PORTSC register won't be touched.
-- dr_mode: One of "host", "peripheral" or "otg". Defaults to "otg"
-
-Deprecated properties:
-- usb-phy: phandle for the PHY device. Use "phys" instead.
-- fsl,usbphy: phandle of usb phy that connects to the port. Use "phys" instead.
-
-Optional properties:
-- clocks: reference to the USB clock
-- phys: reference to the USB PHY
-- phy-names: should be "usb-phy"
-- vbus-supply: reference to the VBUS regulator
-- maximum-speed: limit the maximum connection speed to "full-speed".
-- tpl-support: TPL (Targeted Peripheral List) feature for targeted hosts
-- itc-setting: interrupt threshold control register control, the setting
- should be aligned with ITC bits at register USBCMD.
-- ahb-burst-config: it is vendor dependent, the required value should be
- aligned with AHBBRST at SBUSCFG, the range is from 0x0 to 0x7. This
- property is used to change AHB burst configuration, check the chipidea
- spec for meaning of each value. If this property is not existed, it
- will use the reset value.
-- tx-burst-size-dword: it is vendor dependent, the tx burst size in dword
- (4 bytes), This register represents the maximum length of a the burst
- in 32-bit words while moving data from system memory to the USB
- bus, the value of this property will only take effect if property
- "ahb-burst-config" is set to 0, if this property is missing the reset
- default of the hardware implementation will be used.
-- rx-burst-size-dword: it is vendor dependent, the rx burst size in dword
- (4 bytes), This register represents the maximum length of a the burst
- in 32-bit words while moving data from the USB bus to system memory,
- the value of this property will only take effect if property
- "ahb-burst-config" is set to 0, if this property is missing the reset
- default of the hardware implementation will be used.
-- extcon: phandles to external connector devices. First phandle should point to
- external connector, which provide "USB" cable events, the second should point
- to external connector device, which provide "USB-HOST" cable events. If one
- of the external connector devices is not required, empty <0> phandle should
- be specified.
-- phy-clkgate-delay-us: the delay time (us) between putting the PHY into
- low power mode and gating the PHY clock.
-- non-zero-ttctrl-ttha: after setting this property, the value of register
- ttctrl.ttha will be 0x7f; if not, the value will be 0x0, this is the default
- value. It needs to be very carefully for setting this property, it is
- recommended that consult with your IC engineer before setting this value.
- On the most of chipidea platforms, the "usage_tt" flag at RTL is 0, so this
- property only affects siTD.
- If this property is not set, the max packet size is 1023 bytes, and if
- the total of packet size for pervious transactions are more than 256 bytes,
- it can't accept any transactions within this frame. The use case is single
- transaction, but higher frame rate.
- If this property is set, the max packet size is 188 bytes, it can handle
- more transactions than above case, it can accept transactions until it
- considers the left room size within frame is less than 188 bytes, software
- needs to make sure it does not send more than 90%
- maximum_periodic_data_per_frame. The use case is multiple transactions, but
- less frame rate.
-- mux-controls: The mux control for toggling host/device output of this
- controller. It's expected that a mux state of 0 indicates device mode and a
- mux state of 1 indicates host mode.
-- mux-control-names: Shall be "usb_switch" if mux-controls is specified.
-- pinctrl-names: Names for optional pin modes in "default", "host", "device".
- In case of HSIC-mode, "idle" and "active" pin modes are mandatory. In this
- case, the "idle" state needs to pull down the data and strobe pin
- and the "active" state needs to pull up the strobe pin.
-- pinctrl-n: alternate pin modes
-
-i.mx specific properties
-- fsl,usbmisc: phandler of non-core register device, with one
- argument that indicate usb controller index
-- disable-over-current: disable over current detect
-- over-current-active-low: over current signal polarity is active low.
-- over-current-active-high: over current signal polarity is active high.
- It's recommended to specify the over current polarity.
-- power-active-high: power signal polarity is active high
-- external-vbus-divider: enables off-chip resistor divider for Vbus
-- samsung,picophy-pre-emp-curr-control: HS Transmitter Pre-Emphasis Current
- Control. This signal controls the amount of current sourced to the
- USB_OTG*_DP and USB_OTG*_DN pins after a J-to-K or K-to-J transition.
- The range is from 0x0 to 0x3, the default value is 0x1.
- Details can refer to TXPREEMPAMPTUNE0 bits of USBNC_n_PHY_CFG1.
-- samsung,picophy-dc-vol-level-adjust: HS DC Voltage Level Adjustment.
- Adjust the high-speed transmitter DC level voltage.
- The range is from 0x0 to 0xf, the default value is 0x3.
- Details can refer to TXVREFTUNE0 bits of USBNC_n_PHY_CFG1.
-
-Example:
-
- usb@f7ed0000 {
- compatible = "chipidea,usb2";
- reg = <0xf7ed0000 0x10000>;
- interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&chip CLKID_USB0>;
- phys = <&usb_phy0>;
- phy-names = "usb-phy";
- vbus-supply = <&reg_usb0_vbus>;
- itc-setting = <0x4>; /* 4 micro-frames */
- /* Incremental burst of unspecified length */
- ahb-burst-config = <0x0>;
- tx-burst-size-dword = <0x10>; /* 64 bytes */
- rx-burst-size-dword = <0x10>;
- extcon = <0>, <&usb_id>;
- phy-clkgate-delay-us = <400>;
- mux-controls = <&usb_switch>;
- mux-control-names = "usb_switch";
- };
-
-Example for HSIC:
-
- usb@2184400 {
- compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
- reg = <0x02184400 0x200>;
- interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clks IMX6QDL_CLK_USBOH3>;
- fsl,usbphy = <&usbphynop1>;
- fsl,usbmisc = <&usbmisc 2>;
- phy_type = "hsic";
- dr_mode = "host";
- ahb-burst-config = <0x0>;
- tx-burst-size-dword = <0x10>;
- rx-burst-size-dword = <0x10>;
- pinctrl-names = "idle", "active";
- pinctrl-0 = <&pinctrl_usbh2_idle>;
- pinctrl-1 = <&pinctrl_usbh2_active>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- usbnet: ethernet@1 {
- compatible = "usb424,9730";
- reg = <1>;
- };
- };
diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
new file mode 100644
index 000000000000..ed03649f17a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -0,0 +1,341 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/ci-hdrc-usb2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: USB2 ChipIdea USB controller for ci13xxx Binding
+
+maintainers:
+ - Xu Yang <[email protected]>
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - chipidea,usb2
+ - lsi,zevio-usb
+ - nvidia,tegra20-udc
+ - nvidia,tegra30-udc
+ - nvidia,tegra114-udc
+ - nvidia,tegra124-udc
+ - qcom,ci-hdrc
+ - items:
+ - enum:
+ - fsl,imx23-usb
+ - fsl,imx25-usb
+ - fsl,imx28-usb
+ - fsl,imx6q-usb
+ - fsl,imx6sl-usb
+ - fsl,imx6sx-usb
+ - fsl,imx6ul-usb
+ - fsl,imx7d-usb
+ - const: fsl,imx27-usb
+ - items:
+ - const: fsl,imx7ulp-usb
+ - const: fsl,imx6ul-usb
+ - items:
+ - const: xlnx,zynq-usb-2.20a
+ - const: chipidea,usb2
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+
+ dr_mode: true
+
+ phy_type: true
+
+ itc-setting:
+ description:
+ interrupt threshold control register control, the setting should be
+ aligned with ITC bits at register USBCMD.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ ahb-burst-config:
+ description:
+ it is vendor dependent, the required value should be aligned with
+ AHBBRST at SBUSCFG, the range is from 0x0 to 0x7. This property is
+ used to change AHB burst configuration, check the chipidea spec for
+ meaning of each value. If this property is not existed, it will use
+ the reset value.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0x0
+ maximum: 0x7
+
+ tx-burst-size-dword:
+ description:
+ it is vendor dependent, the tx burst size in dword (4 bytes), This
+ register represents the maximum length of a the burst in 32-bit
+ words while moving data from system memory to the USB bus, the value
+ of this property will only take effect if property "ahb-burst-config"
+ is set to 0, if this property is missing the reset default of the
+ hardware implementation will be used.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ rx-burst-size-dword:
+ description:
+ it is vendor dependent, the rx burst size in dword (4 bytes), This
+ register represents the maximum length of a the burst in 32-bit words
+ while moving data from the USB bus to system memory, the value of
+ this property will only take effect if property "ahb-burst-config"
+ is set to 0, if this property is missing the reset default of the
+ hardware implementation will be used.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ extcon:
+ description: |
+ Phandles to external connector devices. First phandle should point
+ to external connector, which provide "USB" cable events, the second
+ should point to external connector device, which provide "USB-HOST"
+ cable events. If one of the external connector devices is not
+ required, empty <0> phandle should be specified.
+
+ phy-clkgate-delay-us:
+ description: |
+ The delay time (us) between putting the PHY into low power mode and
+ gating the PHY clock.
+
+ non-zero-ttctrl-ttha:
+ description: |
+ After setting this property, the value of register ttctrl.ttha
+ will be 0x7f; if not, the value will be 0x0, this is the default
+ value. It needs to be very carefully for setting this property, it
+ is recommended that consult with your IC engineer before setting
+ this value. On the most of chipidea platforms, the "usage_tt" flag
+ at RTL is 0, so this property only affects siTD.
+
+ If this property is not set, the max packet size is 1023 bytes, and
+ if the total of packet size for pervious transactions are more than
+ 256 bytes, it can't accept any transactions within this frame. The
+ use case is single transaction, but higher frame rate.
+
+ If this property is set, the max packet size is 188 bytes, it can
+ handle more transactions than above case, it can accept transactions
+ until it considers the left room size within frame is less than 188
+ bytes, software needs to make sure it does not send more than 90%
+ maximum_periodic_data_per_frame. The use case is multiple
+ transactions, but less frame rate.
+
+ mux-controls:
+ description: |
+ The mux control for toggling host/device output of this controller.
+ It's expected that a mux state of 0 indicates device mode and a mux
+ state of 1 indicates host mode.
+
+ mux-control-names:
+ const: usb_switch
+
+ pinctrl-names:
+ description: |
+ Names for optional pin modes in "default", "host", "device".
+ In case of HSIC-mode, "idle" and "active" pin modes are mandatory.
+ In this case, the "idle" state needs to pull down the data and
+ strobe pin and the "active" state needs to pull up the strobe pin.
+
+ pinctrl-0:
+ maxItems: 1
+
+ pinctrl-1:
+ maxItems: 1
+
+ phys:
+ maxItems: 1
+
+ phy-names:
+ const: usb-phy
+
+ vbus-supply:
+ description: reference to the VBUS regulator.
+
+ fsl,usbmisc:
+ description:
+ Phandler of non-core register device, with one argument that
+ indicate usb controller index
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+
+ disable-over-current:
+ description: disable over current detect
+
+ over-current-active-low:
+ description: over current signal polarity is active low
+
+ over-current-active-high:
+ description: |
+ Over current signal polarity is active high. It's recommended to
+ specify the over current polarity.
+
+ power-active-high:
+ description: power signal polarity is active high
+
+ external-vbus-divider:
+ description: enables off-chip resistor divider for Vbus
+
+ samsung,picophy-pre-emp-curr-control:
+ description: |
+ HS Transmitter Pre-Emphasis Current Control. This signal controls
+ the amount of current sourced to the USB_OTG*_DP and USB_OTG*_DN
+ pins after a J-to-K or K-to-J transition. The range is from 0x0 to
+ 0x3, the default value is 0x1. Details can refer to TXPREEMPAMPTUNE0
+ bits of USBNC_n_PHY_CFG1.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0x0
+ maximum: 0x3
+
+ samsung,picophy-dc-vol-level-adjust:
+ description: |
+ HS DC Voltage Level Adjustment. Adjust the high-speed transmitter DC
+ level voltage. The range is from 0x0 to 0xf, the default value is
+ 0x3. Details can refer to TXVREFTUNE0 bits of USBNC_n_PHY_CFG1.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0x0
+ maximum: 0xf
+
+ usb-phy:
+ description: phandle for the PHY device. Use "phys" instead.
+ $ref: /schemas/types.yaml#/definitions/phandle
+ deprecated: true
+
+ fsl,usbphy:
+ description: phandle of usb phy that connects to the port. Use "phys" instead.
+ $ref: /schemas/types.yaml#/definitions/phandle
+ deprecated: true
+
+patternProperties:
+ "ethernet@[0-9a-f]$":
+ description: The hard wired USB devices
+ type: object
+ $ref: /schemas/net/microchip,lan95xx.yaml
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+allOf:
+ - $ref: usb-hcd.yaml#
+ - $ref: usb-drd.yaml#
+ - if:
+ properties:
+ mux-controls:
+ true
+ then:
+ properties:
+ mux-control-names:
+ const: usb_switch
+ - if:
+ properties:
+ phy_type:
+ const: hsic
+
+ required:
+ - phy_type
+ then:
+ properties:
+ pinctrl-names:
+ items:
+ - const: idle
+ - const: active
+ else:
+ properties:
+ pinctrl-names:
+ oneOf:
+ - items:
+ - const: default
+ - enum:
+ - host
+ - device
+ - items:
+ - const: default
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - chipidea,usb2
+ - lsi,zevio-usb
+ - nvidia,tegra20-udc
+ - nvidia,tegra30-udc
+ - nvidia,tegra114-udc
+ - nvidia,tegra124-udc
+ - qcom,ci-hdrc
+ - xlnx,zynq-usb-2.20a
+ then:
+ properties:
+ fsl,usbmisc: false
+ disable-over-current: false
+ over-current-active-low: false
+ over-current-active-high: false
+ power-active-high: false
+ external-vbus-divider: false
+ samsung,picophy-pre-emp-curr-control: false
+ samsung,picophy-dc-vol-level-adjust: false
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/berlin2.h>
+
+ usb@f7ed0000 {
+ compatible = "chipidea,usb2";
+ reg = <0xf7ed0000 0x10000>;
+ interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_USB0>;
+ phys = <&usb_phy0>;
+ phy-names = "usb-phy";
+ vbus-supply = <&reg_usb0_vbus>;
+ itc-setting = <0x4>; /* 4 micro-frames */
+ /* Incremental burst of unspecified length */
+ ahb-burst-config = <0x0>;
+ tx-burst-size-dword = <0x10>; /* 64 bytes */
+ rx-burst-size-dword = <0x10>;
+ extcon = <0>, <&usb_id>;
+ phy-clkgate-delay-us = <400>;
+ mux-controls = <&usb_switch>;
+ mux-control-names = "usb_switch";
+ };
+
+ # Example for HSIC:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/imx6qdl-clock.h>
+
+ usb@2184400 {
+ compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+ reg = <0x02184400 0x200>;
+ interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6QDL_CLK_USBOH3>;
+ fsl,usbphy = <&usbphynop1>;
+ fsl,usbmisc = <&usbmisc 2>;
+ phy_type = "hsic";
+ dr_mode = "host";
+ ahb-burst-config = <0x0>;
+ tx-burst-size-dword = <0x10>;
+ rx-burst-size-dword = <0x10>;
+ pinctrl-names = "idle", "active";
+ pinctrl-0 = <&pinctrl_usbh2_idle>;
+ pinctrl-1 = <&pinctrl_usbh2_active>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usbnet: ethernet@1 {
+ compatible = "usb424,9730";
+ reg = <1>;
+ };
+ };
+
+...
--
2.37.1


2022-10-14 15:33:32

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 4/6] dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format

On Fri, Oct 14, 2022 at 05:51:46PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Convert the binding to DT schema format
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> .../devicetree/bindings/usb/ci-hdrc-usb2.txt | 158 --------
> .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 341 ++++++++++++++++++
> 2 files changed, 341 insertions(+), 158 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml

> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> new file mode 100644
> index 000000000000..ed03649f17a6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> @@ -0,0 +1,341 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/ci-hdrc-usb2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: USB2 ChipIdea USB controller for ci13xxx Binding
> +
> +maintainers:
> + - Xu Yang <[email protected]>
> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - chipidea,usb2
> + - lsi,zevio-usb
> + - nvidia,tegra20-udc
> + - nvidia,tegra30-udc
> + - nvidia,tegra114-udc
> + - nvidia,tegra124-udc
> + - qcom,ci-hdrc
> + - items:
> + - enum:
> + - fsl,imx23-usb
> + - fsl,imx25-usb
> + - fsl,imx28-usb
> + - fsl,imx6q-usb
> + - fsl,imx6sl-usb
> + - fsl,imx6sx-usb
> + - fsl,imx6ul-usb
> + - fsl,imx7d-usb
> + - const: fsl,imx27-usb
> + - items:
> + - const: fsl,imx7ulp-usb
> + - const: fsl,imx6ul-usb
> + - items:
> + - const: xlnx,zynq-usb-2.20a
> + - const: chipidea,usb2
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + minItems: 1
> + maxItems: 2
> +
> + dr_mode: true
> +
> + phy_type: true
> +
> + itc-setting:
> + description:
> + interrupt threshold control register control, the setting should be
> + aligned with ITC bits at register USBCMD.
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + ahb-burst-config:
> + description:
> + it is vendor dependent, the required value should be aligned with
> + AHBBRST at SBUSCFG, the range is from 0x0 to 0x7. This property is
> + used to change AHB burst configuration, check the chipidea spec for
> + meaning of each value. If this property is not existed, it will use
> + the reset value.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0x0
> + maximum: 0x7
> +
> + tx-burst-size-dword:
> + description:
> + it is vendor dependent, the tx burst size in dword (4 bytes), This
> + register represents the maximum length of a the burst in 32-bit
> + words while moving data from system memory to the USB bus, the value
> + of this property will only take effect if property "ahb-burst-config"
> + is set to 0, if this property is missing the reset default of the
> + hardware implementation will be used.
> + $ref: /schemas/types.yaml#/definitions/uint32

Constraints? I assume 2^32 is not valid...

> +
> + rx-burst-size-dword:
> + description:
> + it is vendor dependent, the rx burst size in dword (4 bytes), This
> + register represents the maximum length of a the burst in 32-bit words
> + while moving data from the USB bus to system memory, the value of
> + this property will only take effect if property "ahb-burst-config"
> + is set to 0, if this property is missing the reset default of the
> + hardware implementation will be used.
> + $ref: /schemas/types.yaml#/definitions/uint32

Constraints?

> +
> + extcon:
> + description: |
> + Phandles to external connector devices. First phandle should point
> + to external connector, which provide "USB" cable events, the second
> + should point to external connector device, which provide "USB-HOST"
> + cable events. If one of the external connector devices is not
> + required, empty <0> phandle should be specified.
> +
> + phy-clkgate-delay-us:
> + description: |
> + The delay time (us) between putting the PHY into low power mode and
> + gating the PHY clock.
> +
> + non-zero-ttctrl-ttha:
> + description: |
> + After setting this property, the value of register ttctrl.ttha
> + will be 0x7f; if not, the value will be 0x0, this is the default
> + value. It needs to be very carefully for setting this property, it
> + is recommended that consult with your IC engineer before setting
> + this value. On the most of chipidea platforms, the "usage_tt" flag
> + at RTL is 0, so this property only affects siTD.
> +
> + If this property is not set, the max packet size is 1023 bytes, and
> + if the total of packet size for pervious transactions are more than
> + 256 bytes, it can't accept any transactions within this frame. The
> + use case is single transaction, but higher frame rate.
> +
> + If this property is set, the max packet size is 188 bytes, it can
> + handle more transactions than above case, it can accept transactions
> + until it considers the left room size within frame is less than 188
> + bytes, software needs to make sure it does not send more than 90%
> + maximum_periodic_data_per_frame. The use case is multiple
> + transactions, but less frame rate.
> +
> + mux-controls:
> + description: |
> + The mux control for toggling host/device output of this controller.
> + It's expected that a mux state of 0 indicates device mode and a mux
> + state of 1 indicates host mode.

maxItems: 1

> +
> + mux-control-names:
> + const: usb_switch
> +
> + pinctrl-names:
> + description: |
> + Names for optional pin modes in "default", "host", "device".
> + In case of HSIC-mode, "idle" and "active" pin modes are mandatory.
> + In this case, the "idle" state needs to pull down the data and
> + strobe pin and the "active" state needs to pull up the strobe pin.

The names need to be constraints, not freeform text.

> +
> + pinctrl-0:
> + maxItems: 1
> +
> + pinctrl-1:
> + maxItems: 1
> +
> + phys:
> + maxItems: 1
> +
> + phy-names:
> + const: usb-phy
> +
> + vbus-supply:
> + description: reference to the VBUS regulator.
> +
> + fsl,usbmisc:
> + description:
> + Phandler of non-core register device, with one argument that
> + indicate usb controller index
> + $ref: /schemas/types.yaml#/definitions/phandle-array

items:
- items:
- description: phandle to usbmisc node
- description: index of usb controller

> +
> + disable-over-current:
> + description: disable over current detect

type?

> +
> + over-current-active-low:
> + description: over current signal polarity is active low

type?

> +
> + over-current-active-high:
> + description: |
> + Over current signal polarity is active high. It's recommended to
> + specify the over current polarity.

type?

> +
> + power-active-high:
> + description: power signal polarity is active high

type?

> +
> + external-vbus-divider:
> + description: enables off-chip resistor divider for Vbus

type?

> +
> + samsung,picophy-pre-emp-curr-control:
> + description: |
> + HS Transmitter Pre-Emphasis Current Control. This signal controls
> + the amount of current sourced to the USB_OTG*_DP and USB_OTG*_DN
> + pins after a J-to-K or K-to-J transition. The range is from 0x0 to
> + 0x3, the default value is 0x1. Details can refer to TXPREEMPAMPTUNE0
> + bits of USBNC_n_PHY_CFG1.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0x0
> + maximum: 0x3
> +
> + samsung,picophy-dc-vol-level-adjust:
> + description: |
> + HS DC Voltage Level Adjustment. Adjust the high-speed transmitter DC
> + level voltage. The range is from 0x0 to 0xf, the default value is
> + 0x3. Details can refer to TXVREFTUNE0 bits of USBNC_n_PHY_CFG1.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0x0
> + maximum: 0xf
> +
> + usb-phy:
> + description: phandle for the PHY device. Use "phys" instead.
> + $ref: /schemas/types.yaml#/definitions/phandle
> + deprecated: true
> +
> + fsl,usbphy:
> + description: phandle of usb phy that connects to the port. Use "phys" instead.
> + $ref: /schemas/types.yaml#/definitions/phandle
> + deprecated: true
> +
> +patternProperties:
> + "ethernet@[0-9a-f]$":

This is board specific and shouldn't be part of the schema. But you do
need to define child nodes with a reference to common usb schemas.

> + description: The hard wired USB devices
> + type: object
> + $ref: /schemas/net/microchip,lan95xx.yaml
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +allOf:
> + - $ref: usb-hcd.yaml#
> + - $ref: usb-drd.yaml#
> + - if:
> + properties:
> + mux-controls:
> + true
> + then:
> + properties:
> + mux-control-names:
> + const: usb_switch
> + - if:
> + properties:
> + phy_type:
> + const: hsic
> +
> + required:
> + - phy_type
> + then:
> + properties:
> + pinctrl-names:
> + items:
> + - const: idle
> + - const: active
> + else:
> + properties:
> + pinctrl-names:
> + oneOf:
> + - items:
> + - const: default
> + - enum:
> + - host
> + - device
> + - items:
> + - const: default

'minItems: 1' in the 1st (oneOf) entry and drop the 2nd entry.

> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - chipidea,usb2
> + - lsi,zevio-usb
> + - nvidia,tegra20-udc
> + - nvidia,tegra30-udc
> + - nvidia,tegra114-udc
> + - nvidia,tegra124-udc
> + - qcom,ci-hdrc
> + - xlnx,zynq-usb-2.20a
> + then:
> + properties:
> + fsl,usbmisc: false
> + disable-over-current: false
> + over-current-active-low: false
> + over-current-active-high: false
> + power-active-high: false
> + external-vbus-divider: false
> + samsung,picophy-pre-emp-curr-control: false
> + samsung,picophy-dc-vol-level-adjust: false
> +
> +additionalProperties: false

unevaluatedProperties: false

> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/berlin2.h>
> +
> + usb@f7ed0000 {
> + compatible = "chipidea,usb2";
> + reg = <0xf7ed0000 0x10000>;
> + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&chip CLKID_USB0>;
> + phys = <&usb_phy0>;
> + phy-names = "usb-phy";
> + vbus-supply = <&reg_usb0_vbus>;
> + itc-setting = <0x4>; /* 4 micro-frames */
> + /* Incremental burst of unspecified length */
> + ahb-burst-config = <0x0>;
> + tx-burst-size-dword = <0x10>; /* 64 bytes */
> + rx-burst-size-dword = <0x10>;
> + extcon = <0>, <&usb_id>;
> + phy-clkgate-delay-us = <400>;
> + mux-controls = <&usb_switch>;
> + mux-control-names = "usb_switch";
> + };
> +
> + # Example for HSIC:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/imx6qdl-clock.h>
> +
> + usb@2184400 {
> + compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> + reg = <0x02184400 0x200>;
> + interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6QDL_CLK_USBOH3>;
> + fsl,usbphy = <&usbphynop1>;
> + fsl,usbmisc = <&usbmisc 2>;
> + phy_type = "hsic";
> + dr_mode = "host";
> + ahb-burst-config = <0x0>;
> + tx-burst-size-dword = <0x10>;
> + rx-burst-size-dword = <0x10>;
> + pinctrl-names = "idle", "active";
> + pinctrl-0 = <&pinctrl_usbh2_idle>;
> + pinctrl-1 = <&pinctrl_usbh2_active>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + usbnet: ethernet@1 {
> + compatible = "usb424,9730";
> + reg = <1>;
> + };
> + };
> +
> +...
> --
> 2.37.1
>
>

2022-10-15 15:05:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH V2 4/6] dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format

On 14/10/2022 05:51, Peng Fan (OSS) wrote:


> +
> + vbus-supply:
> + description: reference to the VBUS regulator.
> +
> + fsl,usbmisc:
> + description:
> + Phandler of non-core register device, with one argument that
> + indicate usb controller index
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> +
> + disable-over-current:
> + description: disable over current detect
> +
> + over-current-active-low:
> + description: over current signal polarity is active low

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

> +
> + over-current-active-high:
> + description: |
> + Over current signal polarity is active high. It's recommended to
> + specify the over current polarity.
> +
> + power-active-high:
> + description: power signal polarity is active high
> +
> + external-vbus-divider:
> + description: enables off-chip resistor divider for Vbus
> +
> + samsung,picophy-pre-emp-curr-control:
> + description: |
> + HS Transmitter Pre-Emphasis Current Control. This signal controls
> + the amount of current sourced to the USB_OTG*_DP and USB_OTG*_DN
> + pins after a J-to-K or K-to-J transition. The range is from 0x0 to
> + 0x3, the default value is 0x1. Details can refer to TXPREEMPAMPTUNE0
> + bits of USBNC_n_PHY_CFG1.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0x0
> + maximum: 0x3
> +
> + samsung,picophy-dc-vol-level-adjust:
> + description: |
> + HS DC Voltage Level Adjustment. Adjust the high-speed transmitter DC
> + level voltage. The range is from 0x0 to 0xf, the default value is
> + 0x3. Details can refer to TXVREFTUNE0 bits of USBNC_n_PHY_CFG1.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0x0
> + maximum: 0xf
> +
> + usb-phy:
> + description: phandle for the PHY device. Use "phys" instead.
> + $ref: /schemas/types.yaml#/definitions/phandle
> + deprecated: true
> +
> + fsl,usbphy:
> + description: phandle of usb phy that connects to the port. Use "phys" instead.
> + $ref: /schemas/types.yaml#/definitions/phandle
> + deprecated: true
> +
> +patternProperties:
> + "ethernet@[0-9a-f]$":
> + description: The hard wired USB devices
> + type: object
> + $ref: /schemas/net/microchip,lan95xx.yaml
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +allOf:
> + - $ref: usb-hcd.yaml#
> + - $ref: usb-drd.yaml#
> + - if:
> + properties:
> + mux-controls:
> + true
> + then:
> + properties:
> + mux-control-names:
> + const: usb_switch

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.


Additionally: Blank line

> + - if:
> + properties:
> + phy_type:
> + const: hsic
> +
> + required:
> + - phy_type> + then:
> + properties:
> + pinctrl-names:
> + items:
> + - const: idle
> + - const: active
> + else:
> + properties:
> + pinctrl-names:
> + oneOf:
> + - items:
> + - const: default
> + - enum:
> + - host
> + - device
> + - items:
> + - const: default

No, just minItems: 1

Additionally: blank line

> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - chipidea,usb2
> + - lsi,zevio-usb
> + - nvidia,tegra20-udc
> + - nvidia,tegra30-udc
> + - nvidia,tegra114-udc
> + - nvidia,tegra124-udc
> + - qcom,ci-hdrc
> + - xlnx,zynq-usb-2.20a
> + then:
> + properties:
> + fsl,usbmisc: false
> + disable-over-current: false
> + over-current-active-low: false
> + over-current-active-high: false
> + power-active-high: false
> + external-vbus-divider: false
> + samsung,picophy-pre-emp-curr-control: false
> + samsung,picophy-dc-vol-level-adjust: false
> +


Best regards,
Krzysztof

2022-10-17 05:58:47

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH V2 4/6] dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format

Hi Rob,

> Subject: Re: [PATCH V2 4/6] dt-bindings: usb: ci-hdrc-usb2: convert to DT
> schema format
>
> On Fri, Oct 14, 2022 at 05:51:46PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <[email protected]>
> >
> > Convert the binding to DT schema format
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > .../devicetree/bindings/usb/ci-hdrc-usb2.txt | 158 --------
> > .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 341 ++++++++++++++++++
> > 2 files changed, 341 insertions(+), 158 deletions(-) delete mode
> > 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > create mode 100644
> > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > new file mode 100644
> > index 000000000000..ed03649f17a6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > @@ -0,0 +1,341 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> >
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fschemas%2Fusb%2Fci-hdrc-
> usb2.yaml%23&amp;data=05%7C01%7C
> >
> +peng.fan%40nxp.com%7Cc82aff1e7786408bdfb908daadf05fba%7C686ea1
> d3bc2b4
> >
> +c6fa92cd99c5c301635%7C0%7C0%7C638013545157576176%7CUnknown%
> 7CTWFpbGZs
> >
> +b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn0%3
> >
> +D%7C3000%7C%7C%7C&amp;sdata=WCoRU5%2FlPpdKF984bqAmU7WPU8
> XbDvBZoVyJ6K3
> > +nggY%3D&amp;reserved=0
> > +$schema:
> >
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-
> schemas%2Fcore.yaml%23&amp;data=05%7C01%7Cpeng.fan%
> >
> +40nxp.com%7Cc82aff1e7786408bdfb908daadf05fba%7C686ea1d3bc2b4c6
> fa92cd9
> >
> +9c5c301635%7C0%7C0%7C638013545157576176%7CUnknown%7CTWFpb
> GZsb3d8eyJWI
> >
> +joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000%
> >
> +7C%7C%7C&amp;sdata=l6g5TrxpwRjZqUgRmqpAbhCIO4%2BLRp0y1IFV5SS
> nkx0%3D&a
> > +mp;reserved=0
> > +
> > +title: USB2 ChipIdea USB controller for ci13xxx Binding
> > +
> > +maintainers:
> > + - Xu Yang <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - enum:
> > + - chipidea,usb2
> > + - lsi,zevio-usb
> > + - nvidia,tegra20-udc
> > + - nvidia,tegra30-udc
> > + - nvidia,tegra114-udc
> > + - nvidia,tegra124-udc
> > + - qcom,ci-hdrc
> > + - items:
> > + - enum:
> > + - fsl,imx23-usb
> > + - fsl,imx25-usb
> > + - fsl,imx28-usb
> > + - fsl,imx6q-usb
> > + - fsl,imx6sl-usb
> > + - fsl,imx6sx-usb
> > + - fsl,imx6ul-usb
> > + - fsl,imx7d-usb
> > + - const: fsl,imx27-usb
> > + - items:
> > + - const: fsl,imx7ulp-usb
> > + - const: fsl,imx6ul-usb
> > + - items:
> > + - const: xlnx,zynq-usb-2.20a
> > + - const: chipidea,usb2
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + minItems: 1
> > + maxItems: 2
> > +
> > + dr_mode: true
> > +
> > + phy_type: true
> > +
> > + itc-setting:
> > + description:
> > + interrupt threshold control register control, the setting should be
> > + aligned with ITC bits at register USBCMD.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > + ahb-burst-config:
> > + description:
> > + it is vendor dependent, the required value should be aligned with
> > + AHBBRST at SBUSCFG, the range is from 0x0 to 0x7. This property is
> > + used to change AHB burst configuration, check the chipidea spec for
> > + meaning of each value. If this property is not existed, it will use
> > + the reset value.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0x0
> > + maximum: 0x7
> > +
> > + tx-burst-size-dword:
> > + description:
> > + it is vendor dependent, the tx burst size in dword (4 bytes), This
> > + register represents the maximum length of a the burst in 32-bit
> > + words while moving data from system memory to the USB bus, the
> value
> > + of this property will only take effect if property "ahb-burst-config"
> > + is set to 0, if this property is missing the reset default of the
> > + hardware implementation will be used.
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> Constraints? I assume 2^32 is not valid...

Actually I am not sure, per txt binding, it is vendor dependent.
i.MX has value 0x8 or 0x10, but I am not other vendors.

>
> > +
> > + rx-burst-size-dword:
> > + description:
> > + it is vendor dependent, the rx burst size in dword (4 bytes), This
> > + register represents the maximum length of a the burst in 32-bit words
> > + while moving data from the USB bus to system memory, the value of
> > + this property will only take effect if property "ahb-burst-config"
> > + is set to 0, if this property is missing the reset default of the
> > + hardware implementation will be used.
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> Constraints?

Vendor dependent, I am not sure.

>
> > +
> > + extcon:
> > + description: |
> > + Phandles to external connector devices. First phandle should point
> > + to external connector, which provide "USB" cable events, the second
> > + should point to external connector device, which provide "USB-HOST"
> > + cable events. If one of the external connector devices is not
> > + required, empty <0> phandle should be specified.
> > +
> > + phy-clkgate-delay-us:
> > + description: |
> > + The delay time (us) between putting the PHY into low power mode
> and
> > + gating the PHY clock.
> > +
> > + non-zero-ttctrl-ttha:
> > + description: |
> > + After setting this property, the value of register ttctrl.ttha
> > + will be 0x7f; if not, the value will be 0x0, this is the default
> > + value. It needs to be very carefully for setting this property, it
> > + is recommended that consult with your IC engineer before setting
> > + this value. On the most of chipidea platforms, the "usage_tt" flag
> > + at RTL is 0, so this property only affects siTD.
> > +
> > + If this property is not set, the max packet size is 1023 bytes, and
> > + if the total of packet size for pervious transactions are more than
> > + 256 bytes, it can't accept any transactions within this frame. The
> > + use case is single transaction, but higher frame rate.
> > +
> > + If this property is set, the max packet size is 188 bytes, it can
> > + handle more transactions than above case, it can accept transactions
> > + until it considers the left room size within frame is less than 188
> > + bytes, software needs to make sure it does not send more than 90%
> > + maximum_periodic_data_per_frame. The use case is multiple
> > + transactions, but less frame rate.
> > +
> > + mux-controls:
> > + description: |
> > + The mux control for toggling host/device output of this controller.
> > + It's expected that a mux state of 0 indicates device mode and a mux
> > + state of 1 indicates host mode.
>
> maxItems: 1

Fix in v3.
>
> > +
> > + mux-control-names:
> > + const: usb_switch
> > +
> > + pinctrl-names:
> > + description: |
> > + Names for optional pin modes in "default", "host", "device".
> > + In case of HSIC-mode, "idle" and "active" pin modes are mandatory.
> > + In this case, the "idle" state needs to pull down the data and
> > + strobe pin and the "active" state needs to pull up the strobe pin.
>
> The names need to be constraints, not freeform text.

I am not sure, I have added constraints in allOf section.

>
> > +
> > + pinctrl-0:
> > + maxItems: 1
> > +
> > + pinctrl-1:
> > + maxItems: 1
> > +
> > + phys:
> > + maxItems: 1
> > +
> > + phy-names:
> > + const: usb-phy
> > +
> > + vbus-supply:
> > + description: reference to the VBUS regulator.
> > +
> > + fsl,usbmisc:
> > + description:
> > + Phandler of non-core register device, with one argument that
> > + indicate usb controller index
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
>
> items:
> - items:
> - description: phandle to usbmisc node
> - description: index of usb controller
>
> > +
> > + disable-over-current:
> > + description: disable over current detect
>
> type?

It is a Boolean type. I should add "type: boolean"
>
> > +
> > + over-current-active-low:
> > + description: over current signal polarity is active low
>
> type?

type: boolean
>
> > +
> > + over-current-active-high:
> > + description: |
> > + Over current signal polarity is active high. It's recommended to
> > + specify the over current polarity.
>
> type?

type: boolean
>
> > +
> > + power-active-high:
> > + description: power signal polarity is active high
>
> type?

type: boolean
>
> > +
> > + external-vbus-divider:
> > + description: enables off-chip resistor divider for Vbus
>
> type?

type: boolean
>
> > +
> > + samsung,picophy-pre-emp-curr-control:
> > + description: |
> > + HS Transmitter Pre-Emphasis Current Control. This signal controls
> > + the amount of current sourced to the USB_OTG*_DP and
> USB_OTG*_DN
> > + pins after a J-to-K or K-to-J transition. The range is from 0x0 to
> > + 0x3, the default value is 0x1. Details can refer to
> TXPREEMPAMPTUNE0
> > + bits of USBNC_n_PHY_CFG1.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0x0
> > + maximum: 0x3
> > +
> > + samsung,picophy-dc-vol-level-adjust:
> > + description: |
> > + HS DC Voltage Level Adjustment. Adjust the high-speed transmitter
> DC
> > + level voltage. The range is from 0x0 to 0xf, the default value is
> > + 0x3. Details can refer to TXVREFTUNE0 bits of USBNC_n_PHY_CFG1.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0x0
> > + maximum: 0xf
> > +
> > + usb-phy:
> > + description: phandle for the PHY device. Use "phys" instead.
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + deprecated: true
> > +
> > + fsl,usbphy:
> > + description: phandle of usb phy that connects to the port. Use "phys"
> instead.
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + deprecated: true
> > +
> > +patternProperties:
> > + "ethernet@[0-9a-f]$":
>
> This is board specific and shouldn't be part of the schema. But you do need
> to define child nodes with a reference to common usb schemas.

The txt binding has the child node
usbnet: ethernet@1 {
compatible = "usb424,9730";
reg = <1>;
};

I am not sure how to support it. If I not add such patterProperties, there will
be dt binding check error. The child node is a usb ethernet, I think common
usb schemas not has that.

If you have a simple example, I could follow that.
>
> > + description: The hard wired USB devices
> > + type: object
> > + $ref: /schemas/net/microchip,lan95xx.yaml
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > +
> > +allOf:
> > + - $ref: usb-hcd.yaml#
> > + - $ref: usb-drd.yaml#
> > + - if:
> > + properties:
> > + mux-controls:
> > + true
> > + then:
> > + properties:
> > + mux-control-names:
> > + const: usb_switch
> > + - if:
> > + properties:
> > + phy_type:
> > + const: hsic
> > +
> > + required:
> > + - phy_type
> > + then:
> > + properties:
> > + pinctrl-names:
> > + items:
> > + - const: idle
> > + - const: active
> > + else:
> > + properties:
> > + pinctrl-names:
> > + oneOf:
> > + - items:
> > + - const: default
> > + - enum:
> > + - host
> > + - device
> > + - items:
> > + - const: default
>
> 'minItems: 1' in the 1st (oneOf) entry and drop the 2nd entry.

ok

>
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - chipidea,usb2
> > + - lsi,zevio-usb
> > + - nvidia,tegra20-udc
> > + - nvidia,tegra30-udc
> > + - nvidia,tegra114-udc
> > + - nvidia,tegra124-udc
> > + - qcom,ci-hdrc
> > + - xlnx,zynq-usb-2.20a
> > + then:
> > + properties:
> > + fsl,usbmisc: false
> > + disable-over-current: false
> > + over-current-active-low: false
> > + over-current-active-high: false
> > + power-active-high: false
> > + external-vbus-divider: false
> > + samsung,picophy-pre-emp-curr-control: false
> > + samsung,picophy-dc-vol-level-adjust: false
> > +
> > +additionalProperties: false
>
> unevaluatedProperties: false

Will add this.

Thanks,
Peng.
>
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/berlin2.h>
> > +
> > + usb@f7ed0000 {
> > + compatible = "chipidea,usb2";
> > + reg = <0xf7ed0000 0x10000>;
> > + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&chip CLKID_USB0>;
> > + phys = <&usb_phy0>;
> > + phy-names = "usb-phy";
> > + vbus-supply = <&reg_usb0_vbus>;
> > + itc-setting = <0x4>; /* 4 micro-frames */
> > + /* Incremental burst of unspecified length */
> > + ahb-burst-config = <0x0>;
> > + tx-burst-size-dword = <0x10>; /* 64 bytes */
> > + rx-burst-size-dword = <0x10>;
> > + extcon = <0>, <&usb_id>;
> > + phy-clkgate-delay-us = <400>;
> > + mux-controls = <&usb_switch>;
> > + mux-control-names = "usb_switch";
> > + };
> > +
> > + # Example for HSIC:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/imx6qdl-clock.h>
> > +
> > + usb@2184400 {
> > + compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> > + reg = <0x02184400 0x200>;
> > + interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clks IMX6QDL_CLK_USBOH3>;
> > + fsl,usbphy = <&usbphynop1>;
> > + fsl,usbmisc = <&usbmisc 2>;
> > + phy_type = "hsic";
> > + dr_mode = "host";
> > + ahb-burst-config = <0x0>;
> > + tx-burst-size-dword = <0x10>;
> > + rx-burst-size-dword = <0x10>;
> > + pinctrl-names = "idle", "active";
> > + pinctrl-0 = <&pinctrl_usbh2_idle>;
> > + pinctrl-1 = <&pinctrl_usbh2_active>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + usbnet: ethernet@1 {
> > + compatible = "usb424,9730";
> > + reg = <1>;
> > + };
> > + };
> > +
> > +...
> > --
> > 2.37.1
> >
> >

2022-10-17 06:10:07

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH V2 4/6] dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format

> Subject: Re: [PATCH V2 4/6] dt-bindings: usb: ci-hdrc-usb2: convert to DT
> schema format
>
> On 14/10/2022 05:51, Peng Fan (OSS) wrote:
>
>
> > +
> > + vbus-supply:
> > + description: reference to the VBUS regulator.
> > +
> > + fsl,usbmisc:
> > + description:
> > + Phandler of non-core register device, with one argument that
> > + indicate usb controller index
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > +
> > + disable-over-current:
> > + description: disable over current detect
> > +
> > + over-current-active-low:
> > + description: over current signal polarity is active low
>
> This is a friendly reminder during the review process.
>
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.

I thought no need to add type for boolean property. Compared V1, V2 has
more changes. As I wrote in cover-letter, I may miss some comments, sorry
for that. I will go back and re-check the V1 comments.

Thanks,
Peng.
>
> Thank you.
>
> > +
> > + over-current-active-high:
> > + description: |
> > + Over current signal polarity is active high. It's recommended to
> > + specify the over current polarity.
> > +
> > + power-active-high:
> > + description: power signal polarity is active high
> > +
> > + external-vbus-divider:
> > + description: enables off-chip resistor divider for Vbus
> > +
> > + samsung,picophy-pre-emp-curr-control:
> > + description: |
> > + HS Transmitter Pre-Emphasis Current Control. This signal controls
> > + the amount of current sourced to the USB_OTG*_DP and
> USB_OTG*_DN
> > + pins after a J-to-K or K-to-J transition. The range is from 0x0 to
> > + 0x3, the default value is 0x1. Details can refer to
> TXPREEMPAMPTUNE0
> > + bits of USBNC_n_PHY_CFG1.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0x0
> > + maximum: 0x3
> > +
> > + samsung,picophy-dc-vol-level-adjust:
> > + description: |
> > + HS DC Voltage Level Adjustment. Adjust the high-speed transmitter
> DC
> > + level voltage. The range is from 0x0 to 0xf, the default value is
> > + 0x3. Details can refer to TXVREFTUNE0 bits of USBNC_n_PHY_CFG1.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0x0
> > + maximum: 0xf
> > +
> > + usb-phy:
> > + description: phandle for the PHY device. Use "phys" instead.
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + deprecated: true
> > +
> > + fsl,usbphy:
> > + description: phandle of usb phy that connects to the port. Use "phys"
> instead.
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + deprecated: true
> > +
> > +patternProperties:
> > + "ethernet@[0-9a-f]$":
> > + description: The hard wired USB devices
> > + type: object
> > + $ref: /schemas/net/microchip,lan95xx.yaml
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > +
> > +allOf:
> > + - $ref: usb-hcd.yaml#
> > + - $ref: usb-drd.yaml#
> > + - if:
> > + properties:
> > + mux-controls:
> > + true
> > + then:
> > + properties:
> > + mux-control-names:
> > + const: usb_switch
>
> This is a friendly reminder during the review process.
>
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.
>
> Thank you.
>
>
> Additionally: Blank line
>
> > + - if:
> > + properties:
> > + phy_type:
> > + const: hsic
> > +
> > + required:
> > + - phy_type> + then:
> > + properties:
> > + pinctrl-names:
> > + items:
> > + - const: idle
> > + - const: active
> > + else:
> > + properties:
> > + pinctrl-names:
> > + oneOf:
> > + - items:
> > + - const: default
> > + - enum:
> > + - host
> > + - device
> > + - items:
> > + - const: default
>
> No, just minItems: 1
>
> Additionally: blank line
>
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - chipidea,usb2
> > + - lsi,zevio-usb
> > + - nvidia,tegra20-udc
> > + - nvidia,tegra30-udc
> > + - nvidia,tegra114-udc
> > + - nvidia,tegra124-udc
> > + - qcom,ci-hdrc
> > + - xlnx,zynq-usb-2.20a
> > + then:
> > + properties:
> > + fsl,usbmisc: false
> > + disable-over-current: false
> > + over-current-active-low: false
> > + over-current-active-high: false
> > + power-active-high: false
> > + external-vbus-divider: false
> > + samsung,picophy-pre-emp-curr-control: false
> > + samsung,picophy-dc-vol-level-adjust: false
> > +
>
>
> Best regards,
> Krzysztof

2022-12-22 16:13:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH V2 4/6] dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format

On 14/10/2022 11:51, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Convert the binding to DT schema format
>
> Signed-off-by: Peng Fan <[email protected]>
> ---

(...)

> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> new file mode 100644
> index 000000000000..ed03649f17a6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> @@ -0,0 +1,341 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/ci-hdrc-usb2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: USB2 ChipIdea USB controller for ci13xxx Binding
> +
> +maintainers:
> + - Xu Yang <[email protected]>
> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - chipidea,usb2
> + - lsi,zevio-usb
> + - nvidia,tegra20-udc
> + - nvidia,tegra30-udc
> + - nvidia,tegra114-udc
> + - nvidia,tegra124-udc
> + - qcom,ci-hdrc

Except other comments, this conversion does not match existing Qualcomm
driver and Qualcomm DTS. You miss the "ulpi" node.

Best regards,
Krzysztof