Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Thu, 7 Dec 2023 17:34:00 +0100
From:   Johan Hovold <johan@kernel.org>
To:     Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
Cc:     Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Wesley Cheng <quic_wcheng@quicinc.com>,
        Conor Dooley <conor+dt@kernel.org>,
        cros-qcom-dts-watchers@chromium.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, quic_ppratap@quicinc.com,
        quic_jackp@quicinc.com
Subject: Re: [PATCH v2 2/6] usb: dwc3: qcom: Rename hs_phy_irq to
 qusb2_phy_irq
Message-ID: <ZXHz-HYgVLbgFp2k@hovoldconsulting.com>
References: <20231204100950.28712-1-quic_kriskura@quicinc.com>
 <20231204100950.28712-3-quic_kriskura@quicinc.com>
 <ZXHkpwji_AzXMjfm@hovoldconsulting.com>
 <9d52fa8c-41d1-46a7-be89-5c1c11ca09b4@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9d52fa8c-41d1-46a7-be89-5c1c11ca09b4@quicinc.com>
Precedence: bulk

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