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 8D467C761AF for ; Fri, 17 Mar 2023 07:38:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230084AbjCQHiU (ORCPT ); Fri, 17 Mar 2023 03:38:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37590 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229955AbjCQHiP (ORCPT ); Fri, 17 Mar 2023 03:38:15 -0400 Received: from sin.source.kernel.org (sin.source.kernel.org [IPv6:2604:1380:40e1:4800::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6EC8D1421B; Fri, 17 Mar 2023 00:38:13 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id 19AA0CE1F6F; Fri, 17 Mar 2023 07:38:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 06B2BC4339B; Fri, 17 Mar 2023 07:38:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1679038687; bh=iEJpyZ9jXapfaJWky/MC2x1NLyVZPYHtlZserzfH3pU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=EbYKMO9P3WzyjRWnMaMPaCg+JNnCadnKf1C4HvZZP6GzV20QRjd8sP7ElHpEsNYs1 WEV8x18+0M8dGcw8KcWf938vhz1OrjNLR4uBBI6uPkmhQHsaRZTVzY6o4ydnPxJ4o1 f5q5Ia3JNzNfdJhQp9AJCY+jpmHiexU59/XRaW4P76aRyaehfwZbKETnOsS7uCbMpE 8OTdsG4CywWxTMduLKIc0gUsyh1HgNU3ewzKuYJRFDtBmoSA5oEdPz8YkZTk0trdkW QOnY2zVy31sWEW41w4AE75RrznDRWpmD6ER+nojyFmfgN/Wgc0gJxPWCYPCYmWOp7k gL1UQcFeBEaMg== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from ) id 1pd4gI-0001DB-3N; Fri, 17 Mar 2023 08:39:18 +0100 Date: Fri, 17 Mar 2023 08:39:18 +0100 From: Johan Hovold To: Steev Klimaszewski Cc: Bjorn Andersson , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Andy Gross , Konrad Dybcio , Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz , Sven Peter , netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-bluetooth@vger.kernel.org, Mark Pearson Subject: Re: [PATCH v6 4/4] arm64: dts: qcom: sc8280xp-x13s: Add bluetooth Message-ID: References: <20230316034759.73489-1-steev@kali.org> <20230316034759.73489-5-steev@kali.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 16, 2023 at 02:17:38PM -0500, Steev Klimaszewski wrote: > On Thu, Mar 16, 2023 at 6:05 AM Johan Hovold wrote: > > On Thu, Mar 16, 2023 at 11:26:12AM +0100, Johan Hovold wrote: > > > On Wed, Mar 15, 2023 at 10:47:58PM -0500, Steev Klimaszewski wrote: > > > > + vreg_s1c: smps1 { > > > > + regulator-name = "vreg_s1c"; > > > > + regulator-min-microvolt = <1880000>; > > > > + regulator-max-microvolt = <1900000>; > > > > + regulator-initial-mode = ; > > > > + regulator-allowed-modes = , > > > > + ; > > > > + regulator-allow-set-load; > > > > > > So this does not look quite right still as you're specifying an initial > > > mode which is not listed as allowed. > > > > > > Also there are no other in-tree users of RPMH_REGULATOR_MODE_RET and > > > AUTO is used to switch mode automatically which seems odd to use with > > > allow-set-load. > > > > > > This regulator is in fact also used by the wifi part of the chip and as > > > that driver does not set any loads so we may end up with a regulator in > > > retention mode while wifi is in use. > > > > > > Perhaps Bjorn can enlighten us, but my guess is that this should just be > > > "intial-mode = AUTO" (or even HPM, but I have no idea where this came > > > from originally). > > > > This one probably also needs to be marked as always-on as we don't > > currently describe the fact that the wifi part also uses s1c. > I couldn't remember exactly why I chose HPM, and so I recreated what > I'd done. I looked to see what modes were available by git grepping > the kernel sources and since they are in > include/dt-bindings/regulator/qcom,rpmh-regulator.h with a comment > explaining what each mode is, I picked HPM since it starts it at the > full rated current. As to why I chose the others... it was based on > SMPS being mentioned in that comment block. Since I wasn't sure what > PFM is (and admittedly, did not look it up) I skipped it. > > And you are right, we probably don't want to yank that regulator out > from under the wifi... will add that in v7, so I guess for v7 we want > HPM, LPM, AUTO with AUTO being initial. I guess I was trying to think > that RET would allow as little power usage as possible when bluetooth > isn't in use. No, I think you need to stick with HPM and disallow setting the load since doing so could impact other consumers that are not yet described. Johan