Messages in this thread | | | Date | Tue, 28 Mar 2023 15:08:53 +0530 | From | Manivannan Sadhasivam <> | Subject | Re: [PATCH 1/5] arm64: dts: qcom: sc8280xp: Add missing dwc3 quirks |
| |
On Tue, Mar 28, 2023 at 10:54:53AM +0200, Johan Hovold wrote: > On Sat, Mar 25, 2023 at 10:22:13PM +0530, Manivannan Sadhasivam wrote: > > Add missing quirks for the USB DWC3 IP. > > This is not an acceptable commit message generally and certainly not for > something that you have tagged for stable. > > At a minimum, you need to describe why these are needed and what the > impact is. >
I can certainly improve the commit message. But usually the quirks are copied from the downstream devicetree where qualcomm engineers would've added them based on the platform requirements.
> Also, why are you sending as part of a series purporting to enable > runtime PM when it appears to be all about optimising specific gadget > applications? >
It's not related to this series I agree but just wanted to group it with a series touching usb so that it won't get lost.
I could respin it separately though in v2.
> Did you confirm that the below makes any sense or has this just been > copied verbatim from the vendor devicetree (it looks like that)? >
As you've mentioned, most of the quirks are for gadget mode which is not supported by the upstream supported boards. So I haven't really tested them but for I assumed that Qcom engineers did.
> The fact that almost none of the qcom SoCs sets these also indicates > that something is not right here. > > > Cc: stable@vger.kernel.org # 5.20 > > Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform") > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > index 0d02599d8867..266a94c712aa 100644 > > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > @@ -3040,6 +3040,13 @@ usb_0_dwc3: usb@a600000 { > > iommus = <&apps_smmu 0x820 0x0>; > > phys = <&usb_0_hsphy>, <&usb_0_qmpphy QMP_USB43DP_USB3_PHY>; > > phy-names = "usb2-phy", "usb3-phy"; > > + snps,hird-threshold = /bits/ 8 <0x0>; > > + snps,usb2-gadget-lpm-disable; > > Here you are disabling LPM for gadget mode, which makes most of the > other properties entirely pointless. > > > + snps,is-utmi-l1-suspend; > > + snps,dis-u1-entry-quirk; > > + snps,dis-u2-entry-quirk; > > These appear to be used to optimise certain gadget application and > likely not something that should be set in a dtsi. >
I will cross check these with Qcom and respin accordingly.
- Mani
> > + snps,has-lpm-erratum; > > + tx-fifo-resize; > > Same here. > > > port { > > usb_0_role_switch: endpoint { > > Johan
-- மணிவண்ணன் சதாசிவம்
| |