linux-kernel - Re: [PATCH 1/5] arm64: dts: qcom: sc8280xp: Add missing dwc3 quirks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZCKrXZn7Eu/jvdpG@hovoldconsulting.com>
Date:   Tue, 28 Mar 2023 10:54:53 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc:     andersson@...nel.org, Thinh.Nguyen@...opsys.com,
        gregkh@...uxfoundation.org, mathias.nyman@...el.com,
        konrad.dybcio@...aro.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, linux-arm-msm@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/5] arm64: dts: qcom: sc8280xp: Add missing dwc3 quirks

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.

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?

Did you confirm that the below makes any sense or has this just been
copied verbatim from the vendor devicetree (it looks like that)?

The fact that almost none of the qcom SoCs sets these also indicates
that something is not right here.

> Cc: stable@...r.kernel.org # 5.20
> Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.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@...0000 {
>  				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.

> +				snps,has-lpm-erratum;
> +				tx-fifo-resize;

Same here.

>  				port {
>  					usb_0_role_switch: endpoint {

Johan