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

[Krzysztof Kozlowski]

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.

Best regards,
Krzysztof