From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA6F8C6FD18 for ; Wed, 29 Mar 2023 11:24:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229788AbjC2LYe (ORCPT ); Wed, 29 Mar 2023 07:24:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55056 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229718AbjC2LYc (ORCPT ); Wed, 29 Mar 2023 07:24:32 -0400 Received: from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com [IPv6:2a00:1450:4864:20::12a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 714E43C3A for ; Wed, 29 Mar 2023 04:24:30 -0700 (PDT) Received: by mail-lf1-x12a.google.com with SMTP id c29so19678616lfv.3 for ; Wed, 29 Mar 2023 04:24:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1680089069; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=bqRvDrf+LQjYdWpmaqzz2Gtf9PBY/+z/KX3/Flqh8zo=; b=J15yimuSbI/QHHkde0jQbkLiVnMFJ0pwJ3jsJ8A+gTcf10SptIjTp8duoBMmP64HOE Re4DDS3AYZnNGELgZjvkQUZMsmXxbbaM+fWzGPZy7ChVuvDEfeVcBsPZte9rhXzCF5Yv /49CrCfCE7Sc1igVj+4i71jQdLBt+ULPTz7adSUo/tEr2JEE19fwi+g5TOH/u7J4m7tW k348HcGAaoa5JnY6BiT1yTVEV+9A7FxfjXadtcqyFjlCAG7omZ7kEt+ROUJQv4UGaHxk BPYzCGkuIxc8EAoCN6WkeKcYeNRrSlzSU9g4bIFJ4wqwXetX81gEoWUOyKTuKjkJhyJX e0yA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680089069; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bqRvDrf+LQjYdWpmaqzz2Gtf9PBY/+z/KX3/Flqh8zo=; b=qKq7Zy2sGKnIwMHGwqJ9QisdMh9EYpdf9PCOoqioG9O8FPO6L9CHS4lLptlQ2QpBwv NWx9BDPnKyRgTR1FegJpjGNMWQhwby+ENPWdBk0dh09mUHePzwjYe4uLz4Z18MTfUiah 5Y+y9eyT3C2F0+1ukPYos9cjsiA+uV9VRyKqSqNfBK1nAefdR0GgVUOiqwm2fQsJkZ7t GQa4uD+AFxqg/V+lPbKbNA4MvEchyfmaAm8Z5jNbAQ8+9H/k7RIhewGiHz3iibEo7B8N nnjTBc7R2ig3YDge5U7snO3+8Qh3LQrGBHGSEdYUwawQyDB2h+zf3MLb7FOBbBR9FxUF go9g== X-Gm-Message-State: AAQBX9d9W/TdJTfMdmA9pKcQTlOnEgJHIUMCjkwZUMLIYW3Jd7vcHfcb a4GyiNuXsVtWccwLL8QAbSxGIQ== X-Google-Smtp-Source: AKy350bYzEOoUSw6cqkGqFLbQhDHA1c4SLLhwVXtXFCwCzlrfCjvVgQhuWXvohm9l2QUOfhgtgfNNw== X-Received: by 2002:a19:550b:0:b0:4e9:c627:195d with SMTP id n11-20020a19550b000000b004e9c627195dmr5339617lfe.57.1680089068735; Wed, 29 Mar 2023 04:24:28 -0700 (PDT) Received: from [192.168.1.101] (abxj225.neoplus.adsl.tpnet.pl. [83.9.3.225]) by smtp.gmail.com with ESMTPSA id m22-20020a195216000000b004db4936c866sm5407496lfb.38.2023.03.29.04.24.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Mar 2023 04:24:28 -0700 (PDT) Message-ID: Date: Wed, 29 Mar 2023 13:24:27 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [PATCH 1/5] arm64: dts: qcom: sc8280xp: Add missing dwc3 quirks Content-Language: en-US To: Johan Hovold , Manivannan Sadhasivam Cc: andersson@kernel.org, Thinh.Nguyen@synopsys.com, gregkh@linuxfoundation.org, mathias.nyman@intel.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, stable@vger.kernel.org References: <20230325165217.31069-1-manivannan.sadhasivam@linaro.org> <20230325165217.31069-2-manivannan.sadhasivam@linaro.org> <20230328093853.GA5695@thinkpad> <20230329052600.GA5575@thinkpad> From: Konrad Dybcio In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 29.03.2023 10:34, Johan Hovold wrote: > On Wed, Mar 29, 2023 at 10:56:00AM +0530, Manivannan Sadhasivam wrote: >> On Tue, Mar 28, 2023 at 03:09:03PM +0530, Manivannan Sadhasivam wrote: >>> 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. > > That's also generally best for USB patches as Greg expects series to be > merged through a single tree. > >>>> 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 >>>>> --- >>>>> 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. >> >> Checked with Qcom on these quirks. So this one is just disabling lpm for USB2 >> and rest of the quirks below are for SS/SSP modes. > > No, snps,hird-threshold is for USB2 LPM and so is > snps,is-utmi-l1-suspend and snps,has-lpm-erratum as you'll see if you > look at the implementation. > >>>>> + 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. >>> >> >> These quirks are needed as per the DWC IP integration with this SoC it seems. >> But I got the point that these don't add any values for host only >> configurations. At the same time, these quirks still hold true for the SoC even >> if not exercised. >> >> So I think we should keep these in the dtsi itself. > > Please take a closer look at the quirks you're enabling first. Commit > 729dcffd1ed3 ("usb: dwc3: gadget: Add support for disabling U1 and U2 > entries") which added > >>>>> + snps,dis-u1-entry-quirk; >>>>> + snps,dis-u2-entry-quirk; > > explicitly mentions > > Gadget applications may have a requirement to disable the U1 and U2 > entry based on the usecase. > > which sounds like something that needs to be done in a per board dts at > least. > > Perhaps keeping all of these in in the dtsi is correct, but that's going > to need some more motivation than simply that some vendor does so (as > they often do all sorts of things they should not). I'm looking at the DWC3 code and admittedly I don't understand much, but is there any harm to keeping them? What if somebody decides to plug in a laptop as a gadget device? Konrad > > Johan