On Thu, Dec 07, 2023 at 09:17:32PM +0530, Krishna Kurapati PSSNV wrote: > On 12/7/2023 8:58 PM, Johan Hovold wrote: > > Here too you should say something about why this won't break any systems > > booting using an older devicetree. Specifically, the QUSB2 PHY interrupt > > has never been armed on any system running mainline as those bits never > > made it upstream. > > > > So an alternative to this could also be to just drop the QUSB2 PHY > > interrupt handling from this driver for now. > > So, are you suggesting that we drop the whole patch ? No, I meant that an alternative could be to drop the current hs_phy_irq handling from the driver. > I assume if the older kernels are using old DT, they would be using an > old driver version too right ? No, and this is part of the devicetree ABI as we discussed the other week. You should generally be able to continue booting with an older devicetree on a newer kernel (even if newer functionality may not be enabled then). > Is there a case where DT is not updated but driver is ? Because if we > drop this patch from series, targets with updated DT's would break. Actually they would not due to the fact that the QUSB2 PHY interrupt is currently never armed in the PHY (and the interrupts are looked up by name and are considered optional by the driver). But simply dropping this patch is not an option here. I'm fine with this patch as it is, but the reason we can merge it is that those interrupts are currently not actually used. Otherwise, this would break older devicetrees. But this also means, we could consider dropping the current hs_phy_irq handling altogether. Hmm. Looking at the qusb2_phy_runtime_suspend() again now I see that the interrupt is actually armed on runtime suspend, it's just that it is configured incorrectly and would wakeup immediately if someone ever exercised this path. Specifically, the bits that would set those PHY_MODE_USB_HOST_HS modes (that should never have been merged) never made it upstream so this code is just dead code currently. I said before I'll look into ripping this out, but yeah, I'm swamped with work as usual (and it has been sitting there dead for years so there's no rush). So to summarise, the QUSB2 wakeup handling is incomplete and broken, so we won't actually make things worse by renaming the interrupts. If this was working, we would need to continue supporting the old names, though. Johan
