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

[Krishna Kurapati PSSNV]

On 12/13/2023 12:45 PM, Krzysztof Kozlowski wrote:
> On 11/12/2023 13:11, Krishna Kurapati wrote:
> > The high speed related interrupts present on QC targets are as follows:
> >
> > dp/dm irq's
> > These IRQ's directly reflect changes on the DP/DM pads of the SoC. These
> > are used as wakeup interrupts only on SoCs with non-QUSB2 targets with
> > exception of SDM670/SDM845/SM6350.
> >
> > qusb2_phy irq
> > 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.
> >
> > hs_phy_irq
> > This is completely different from the above two and is present on all
> > targets with exception of a few IPQ ones. The interrupt is not enabled by
> > default and its functionality is mutually exclusive of qusb2_phy on QUSB
> > targets and DP/DM on femto phy targets.
> >
> > The DTs of several QUSB2 PHY based SoCs incorrectly define "hs_phy_irq"
> > when they should have been "qusb2_phy_irq". On Femto phy targets, the
> > "hs_phy_irq" mentioned is either the actual "hs_phy_irq" or "pwr_event",
> > neither of which would never be triggered directly are non-functional
> > currently. The implementation tries to clean up this issue by addressing
> > the discrepencies involved and fixing the hs_phy_irq's in respective DT's.
> >
> > Classiffy SoC's into four groups based on whether qusb2_phy interrupt
> > or {dp/dm}_hs_phy_irq is used for wakeup in high speed and whether the
> > SoCs have hs_phy_irq present in them or not.
> >
> > The ss_phy_irq is optional interrupt because there are mutliple SoC's
> > which either support only High Speed or there are multiple controllers
> > within same Soc and the secondary controller is High Speed only capable.
> >
> > This breaks ABI on targets running older kernels, but since the interrupt
> > definitions are given wrong on many targets and to establish proper rules
> > for usage of DWC3 interrupts on Qualcomm platforms, DT binding update is
> > necessary.
>
> This still does not explain why missing property has to be added as
> first one, causing huge reordering of everything here and in DTS.
>
> If pwr_event is required and we already break the ABI, reduce the impact
> of the change by putting it after all required interrupts. Otherwise
> please explain here and in commit msg why different approach is taken.

Hi Krzysztof. I don't know much about the effect of the ordering on ABI. 
I will try to learn up on it. Would the series be good if we just move 
the pwr_event to the end and keep everything in v3 as it is, and push v4 
for now ?

Regards,
Krishna,