On 12/7/2023 10:04 PM, Johan Hovold wrote:
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.
Thanks for the review. Since renaming the interrupts won't be an issue,
I will keep this patch as is in that case in v3.
Regards,
Krishna,
