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

[Peng Fan]

> 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