Re: [PATCH v2 1/6] dt-bindings: usb: dwc3: Clean up hs_phy_irq in bindings

[Johan Hovold]

On Thu, Dec 07, 2023 at 09:14:55PM +0530, Krishna Kurapati PSSNV wrote:
 
> >> +        - qusb2_phy:: SoCs with QUSB2 PHY do not have separate DP/DM IRQs and
> >> +                      expose only a single IRQ whose behavior can be modified
> >> +                      by the QUSB2PHY_INTR_CTRL register. The required DPSE/
> >> +                      DMSE configuration is done in QUSB2PHY_INTR_CTRL register
> >> +                      of phy address space.
> >> +        - {dp/dm}_hs_phy_irq:: These IRQ's directly reflect changes on the DP/
> >> +                               DM pads of the SoC. These are used for wakeup
> >> +                               only on SoCs with non-QUSBb2 targets with
> > 
> > QUSB2 typo
> > 
> >> +                               exception of SDM670/SDM845/SM6350.
> >> +        - ss_phy_irq:: When in super speed mode of operation, interrupts are
> > 
> > Capitalise 'Super Speed'
> > 
> >> +                       received when a wakeup event is received on ss_phy_irq.
> > 
> > The description as it stands sounds circular. And this one is only used
> > for remote wakeup right?
> > 
> Yes. It is used for remote wakeup. Mentioning it as wakeup event should 
> be changed ?

It would be good to clarify that this one is the IIUC not used for
connect/disconnect events but just for remote wakeup, that is, unlike
the qusb2_phy and dp/dm_hs_phy interrupts.

The old descriptions just vaguely said "wakeup event" and
connect/disconnect events aren't necessarily wakeup events.

> > Also have you set up the tools so that you can verify your bindings
> > before posing them? I assume the above wouldn't pass (e.g. due to the
> > "(optional)" strings).
> > 
> > There's some more details here:
> > 
> > 	https://docs.kernel.org/devicetree/bindings/writing-schema.html
> > 
> > under "Running checks".
> 
> I did do a dt-binding check and got the following line as well:
> 
>    DTC_CHK Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb
> /local/mnt/workspace/sriramd/upstream/torvalds/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: 
> usb@a6f8800: interrupt-names:4: 'ss_phy_irq (optional)' was expected
>          From schema:

Good that you got that set up.

Johan