> > On 28/02/2024 12:29, Xu Yang wrote: > > As more and more NXP i.MX chips come out, it becomes harder to maintain > > ci-hdrc-usb2.yaml if more stuffs like property restrictions are added to > > this file. This will separate i.MX parts out of ci-hdrc-usb2.yaml and add > > a new schema for NXP ChipIdea USB2 Controller, also add a common schema. > > > > 1. Copy common ci-hdrc-usb2.yaml properties to a new shared > > ci-hdrc-usb2-common.yaml schema. > > 2. Move fsl,* compatible devices and imx spefific properties > > to dedicated binding file ci-hdrc-usb2-imx.yaml. > > > > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx> > > > > --- > > Changes in v6: > > - new patch > > Changes in v7: > > - not remove ci-hdrc-usb2.yaml and move imx parts to ci-hdrc-usb2-imx.yaml > > --- > > .../bindings/usb/ci-hdrc-usb2-common.yaml | 197 ++++++++++++++++++ > > .../bindings/usb/ci-hdrc-usb2-imx.yaml | 197 ++++++++++++++++++ > > .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 186 ----------------- > > 3 files changed, 394 insertions(+), 186 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2-common.yaml > > create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-common.yaml > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-common.yaml > > new file mode 100644 > > index 000000000000..9f8f2f343dd3 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-common.yaml > > Use compatible style for filenames. chipidea,hdrc-usb2-common.yaml > (assuming that "ci" was a shortcut of chipidea) Okay. > > > > @@ -0,0 +1,197 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/usb/ci- > hdrc-usb2- > common.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C2c370c9411664be2924608dc3938b8ac%7C686ea1d3bc2b4 > c6fa92cd99c5c301635%7C0%7C0%7C638448162982019025%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=Aez1Kqo7NsYagBsrTfjys51HUQit0wbHjYg5WigzfzI > %3D&reserved=0 > > +$schema: http://devicetree.org/meta- > schemas%2Fcore.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C2c370c9411664be2924608dc3938b8ac%7C686ea1 > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638448162982031177%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=oXRXyhK7NTRb79hXry9qRP14plBJfdApUh > Z8OHdLB%2FI%3D&reserved=0 > > + > > +title: USB2 ChipIdea USB controller Common Properties > > + > > +maintainers: > > + - Xu Yang <xu.yang_2@xxxxxxx> > > + > > +properties: > > + reg: > > + minItems: 1 > > + maxItems: 2 > > + > > + interrupts: > > + minItems: 1 > > + maxItems: 2 > > + > > + clocks: > > + minItems: 1 > > + maxItems: 3 > > + > > + clock-names: > > + minItems: 1 > > + maxItems: 3 > > + > > + dr_mode: true > > + > > + power-domains: > > + maxItems: 1 > > + > > + resets: > > + maxItems: 1 > > + > > + reset-names: > > + maxItems: 1 > > + > > + "#reset-cells": > > + const: 1 > > + > > + 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 > > + minimum: 0x0 > > + maximum: 0x20 > > + > > + 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 > > + minimum: 0x0 > > + maximum: 0x20 > > + > > + 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. > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + minItems: 1 > > + items: > > + - description: vbus extcon > > + - description: id extcon > > + > > + 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 previous 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. > > + type: boolean > > + > > + 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. > > + oneOf: > > + - items: > > + - const: idle > > + - const: active > > + - items: > > + - const: default > > + - enum: > > + - host > > + - device > > + - items: > > + - const: default > > + > > + pinctrl-0: > > + maxItems: 1 > > + > > + pinctrl-1: > > + maxItems: 1 > > + > > + phys: > > + maxItems: 1 > > + > > + phy-names: > > + const: usb-phy > > + > > + vbus-supply: > > + description: reference to the VBUS regulator. > > + > > + usb-phy: > > + description: phandle for the PHY device. Use "phys" instead. > > + maxItems: 1 > > + deprecated: true > > + > > + port: > > + description: > > + Any connector to the data bus of this controller should be modelled > > + using the OF graph bindings specified, if the "usb-role-switch" > > + property is used. > > + $ref: /schemas/graph.yaml#/properties/port > > + > > + reset-gpios: > > + maxItems: 1 > > + > > +dependencies: > > + port: [ usb-role-switch ] > > + mux-controls: [ mux-control-names ] > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > I don't see where you remove the properties from the existing schema, so > you now just duplicated a lot. > > So again let me describe the task: > 1. Create common schema by MOVING the common pieces from the existing > schema. The common schema MUST be referenced by other existing schemas. I will remove common parts from old ci-hdrc-usb2.yaml and let ci-hdrc-usb2.yaml refer to the common yaml too. > > 2. Create IMX specific schema by moving IMX specific bits. New schema > references common schema. > > 3. The remaining old ci-hdrc-usb2.yaml does not have as a result common > properties and IMX bits. Okay. > > > > + > > +allOf: > > + - $ref: usb-hcd.yaml# > > + - $ref: usb-drd.yaml# > > + > > +additionalProperties: true > > \ No newline at end of file > > You have patch errors, fix them. Okay. > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > > new file mode 100644 > > index 000000000000..50494ce06d07 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > > @@ -0,0 +1,197 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/usb/ci- > hdrc-usb2- > imx.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C2c370c9411664be2924608dc3938b8ac%7C686ea1d3bc2b4c6fa9 > 2cd99c5c301635%7C0%7C0%7C638448162982040409%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l > uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=1MesqU7ed0dAXtuar%2BNIyV9tOQFahTaDNLxvhgL02 > bw%3D&reserved=0 > > Filename like compatible. Okay. > > > +$schema: http://devicetree.org/meta- > schemas%2Fcore.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C2c370c9411664be2924608dc3938b8ac%7C686ea1 > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638448162982048575%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=4YG0rLj%2B%2FwsNf7koSMOeiy23Bz7Z7S > f7B9Z3KcNVvUc%3D&reserved=0 > > + > > +title: NXP USB2 ChipIdea USB controller > > + > > +maintainers: > > + - Xu Yang <xu.yang_2@xxxxxxx> > > + > > +properties: > > + compatible: > > + oneOf: > > + - enum: > > + - fsl,imx27-usb > > + - items: > > + - enum: > > + - fsl,imx23-usb > > + - fsl,imx25-usb > > + - fsl,imx28-usb > > + - fsl,imx35-usb > > + - fsl,imx50-usb > > + - fsl,imx51-usb > > + - fsl,imx53-usb > > + - fsl,imx6q-usb > > + - fsl,imx6sl-usb > > + - fsl,imx6sx-usb > > + - fsl,imx6ul-usb > > + - fsl,imx7d-usb > > + - fsl,vf610-usb > > + - const: fsl,imx27-usb > > + - items: > > + - enum: > > + - fsl,imx8dxl-usb > > + - fsl,imx8ulp-usb > > + - const: fsl,imx7ulp-usb > > + - const: fsl,imx6ul-usb > > + - items: > > + - enum: > > + - fsl,imx8mm-usb > > + - fsl,imx8mn-usb > > + - const: fsl,imx7d-usb > > + - const: fsl,imx27-usb > > + - items: > > + - enum: > > + - fsl,imx6sll-usb > > + - fsl,imx7ulp-usb > > + - const: fsl,imx6ul-usb > > + - const: fsl,imx27-usb > > + > > + clocks: > > + minItems: 1 > > + maxItems: 3 > > > No need to, it's already in common. Drop clocks. No. I cannot remove clocks property in this yaml since the usb node may use assigned-clocks and assigned-clock-parents. Then dtschema will have warning like below patch: https://lore.kernel.org/all/20230508071837.68552-1-krzysztof.kozlowski@xxxxxxxxxx/ Thanks, Xu Yang > > > + > > + clock-names: > > + minItems: 1 > > + maxItems: 3 > > Drop property. > > > + > > + 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 > > + > > + fsl,anatop: > > + description: phandle for the anatop node. > > + $ref: /schemas/types.yaml#/definitions/phandle > > + > > + disable-over-current: > > + type: boolean > > + description: disable over current detect > > + > > + over-current-active-low: > > + type: boolean > > + description: over current signal polarity is active low > > + > > + over-current-active-high: > > + type: boolean > > + description: > > + Over current signal polarity is active high. It's recommended to > > + specify the over current polarity. > > + > > + power-active-high: > > + type: boolean > > + description: power signal polarity is active high > > + > > + external-vbus-divider: > > + type: boolean > > + 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 > > + > > + fsl,picophy-rise-fall-time-adjust: > > + description: > > + HS Transmitter Rise/Fall Time Adjustment. Adjust the rise/fall times > > + of the high-speed transmitter waveform. It has no unit. The rise/fall > > + time will be increased or decreased by a certain percentage relative > > + to design default time. (0:-10%; 1:design default; 2:+15%; 3:+20%) > > + Details can refer to TXRISETUNE0 bit of USBNC_n_PHY_CFG1. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 0 > > + maximum: 3 > > + default: 1 > > + > > + fsl,usbphy: > > + description: phandle of usb phy that connects to the port. Use "phys" instead. > > + $ref: /schemas/types.yaml#/definitions/phandle > > + deprecated: true > > Missing required: block here. > > > + > > +allOf: > > + - $ref: ci-hdrc-usb2-common.yaml# > > + - if: > > + properties: > > + phy_type: > > + const: hsic > > + required: > > + - phy_type > > + then: > > + properties: > > + pinctrl-names: > > + items: > > + - const: idle > > + - const: active > > + > > +required: > > + - compatible > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/clock/imx7d-clock.h> > > + > > + usb@30b10000 { > > + compatible = "fsl,imx7d-usb", "fsl,imx27-usb"; > > + reg = <0x30b10000 0x200>; > > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clks IMX7D_USB_CTRL_CLK>; > > + fsl,usbphy = <&usbphynop1>; > > + fsl,usbmisc = <&usbmisc1 0>; > > + phy-clkgate-delay-us = <400>; > > + }; > > + > > + # 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>; > > + > > + ethernet@1 { > > + compatible = "usb424,9730"; > > + reg = <1>; > > + }; > > + }; > > + > > +... > > > Best regards, > Krzysztof
