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

From: Krishna Kurapati PSSNV
Date: Fri Nov 24 2023 - 07:03:00 EST



Thanks for sorting this out.

It seems like we have a few combinations of these interrupts and we
should probably try to define the order for these once and for all and
update the current devicetrees to match (even if it means adding new
interrupts in the middle).

Instead of adding separate compatibles for the controllers without SS
support, I suggest keeping that interrupt last as an optional one.

But IIUC we essentially have something like:

qusb2-:

- const: qusb2_phy
- const: pwr_event
- const: ss_phy_irq (optional)

qusb2:

- const: hs_phy_irq
- const: qusb2_phy
- const: pwr_event
- const: ss_phy_irq (optional)

qusb2+:

- const: hs_phy_irq
- const: qusb2_phy
- const: dp_hs_phy_irq
- const: dm_hs_phy_irq
- const: pwr_event
- const: ss_phy_irq (optional)


This combination doesn't exist. So we can skip this one.

femto-:
- const: dp_hs_phy_irq
- const: dm_hs_phy_irq
- const: pwr_event
- const: ss_phy_irq (optional)

femto:
- const: hs_phy_irq
- const: dp_hs_phy_irq
- const: dm_hs_phy_irq
- const: pwr_event
- const: ss_phy_irq (optional)

Does this look like it would cover all of our currents SoCs?

Do all of them have the pwr_event interrupt?

Yes. From whatever targets I was able to find, only one of them didn't have the power_event irq. Rest all of them had. I will recheck that particular one again.

Note that DP comes before DM above as that seems like the natural order
of these (plus before minus).

Now if the HS interrupt is truly unusable, I guess we can consider
dropping it throughout and the above becomes just three permutations
instead, which can even be expressed along the lines of:


Infact, I wanted to do this but since you mentioned before that if HW has it, we must describe it, I kept it in. But since this functionality is confirmed to be mutually exclusive of qusb2/{dp/dm}, I am aligned to skip it in bindings and drop it in DT.

- anyOf:
- items:
- const: qusb2_phy
- items:
- const: dp_hs_phy_irq
- const: dm_hs_phy_irq
- const: pwr_event
- const: ss_phy_irq (optional)


This must cover all cases AFAIK. How about we keep pwr_event also optional for time being. The ones I am not able to find also would come up under still binding block.

Regards,
Krishna,