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

From: Krishna Kurapati PSSNV
Date: Fri Nov 24 2023 - 12:39:35 EST




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.

Please do. The driver polls the corresponding status register on all
platforms currently, and perhaps this interrupt can one day be used to
get rid of the polling.

Ok, I just rechecked and case is, I am not able to get my hands on the doc. I can't say for sure that the target is missing the pwr_event interrupt. I say we can safely add the target assuming pwr_event is present for ipq9574. Every target so far even on downstream has this IRQ present in hw.

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.

As I mentioned elsewhere, it depends on whether it can be used at all.
Not simply whether there is some other mechanism that can be used in its
stead. Such a decision should be left up to the implementation.

That's why I said "truly unusable" above. It's still not clear to me
whether that is the case or not.


I looked at the code of 4.4, 4.14/ 4.19/ 5.4/ 5.10/ 5.15/ 6.1 and none of them implement the hs_phy_irq.

- 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.

No, we should avoid that if we can as with two many optional things,
these quickly gets messy (one optional interrupt at the end is fine and
can be expressed using min/maxItems).

If the "qusb2+" combination above isn't needed, then we're down to four
permutations, which is few enough to be spelled out explicitly even if
we decide that the hs_phy_irq should be kept in. Without hs_phy_irq, it
seems there's really only two permutations.


My opinion would be to keep the power_event irq as mandatory and not to include the hs_phy_irq.

Regards,
Krishna,