Re: [PATCH v5 00/18] power: sequencing: implement the subsystem and add first users

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: neil.armstrong@linaro.org, Marcel Holtmann <marcel@holtmann.org>,
	 Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	 Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	 Conor Dooley <conor+dt@kernel.org>,
	Kalle Valo <kvalo@kernel.org>,
	 Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	 Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	 Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	 Bjorn Helgaas <bhelgaas@google.com>,
	Saravana Kannan <saravanak@google.com>,
	 Geert Uytterhoeven <geert+renesas@glider.be>,
	Arnd Bergmann <arnd@arndb.de>,
	 Marek Szyprowski <m.szyprowski@samsung.com>,
	Alex Elder <elder@linaro.org>,
	 Srini Kandagatla <srinivas.kandagatla@linaro.org>,
	 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Abel Vesa <abel.vesa@linaro.org>,
	 Manivannan Sadhasivam <mani@kernel.org>,
	Lukas Wunner <lukas@wunner.de>,
	linux-bluetooth@vger.kernel.org,  netdev@vger.kernel.org,
	devicetree@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-wireless@vger.kernel.org,  linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,  linux-pci@vger.kernel.org,
	linux-pm@vger.kernel.org,
	 Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Subject: Re: [PATCH v5 00/18] power: sequencing: implement the subsystem and add first users
Date: Thu, 22 Feb 2024 13:26:59 +0200	[thread overview]
Message-ID: <CAA8EJprUM6=ZqTwWLB8rW8WRDqwncafa-szSsTvPQCOOSXUn_w@mail.gmail.com> (raw)
In-Reply-To: <CAMRc=Md6SoXukoGb4bW-CSYgjpO4RL+0Uu3tYrZzgSgVtFH6Sw@mail.gmail.com>

On Thu, 22 Feb 2024 at 13:00, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote:
> > >
> > > On 19/02/2024 13:33, Dmitry Baryshkov wrote:
> > > > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >>
> > > >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov
> > > >> <dmitry.baryshkov@linaro.org> wrote:
> > > >>>
> > > >>
> > > >> [snip]
> > > >>
> > > >>>>>>>>
> > > >>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not
> > > >>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as
> > > >>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU
> > > >>>>>>>> directly.
> > > >>>>>>>
> > > >>>>>>> We are describing the hardware. From the hardware point of view, there
> > > >>>>>>> is a PMU. I think at some point we would really like to describe all
> > > >>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the
> > > >>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174).
> > > >>>>>>
> > > >>>>>> While I agree with older WiFi+BT units, I don't think it's needed for
> > > >>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is
> > > >>>>>> transparent.
> > > >>>>>
> > > >>>>> I don't see any significant difference between WCN6750/WCN6855 and
> > > >>>>> WCN7850 from the PMU / power up point of view. Could you please point
> > > >>>>> me to the difference?
> > > >>>>>
> > > >>>>
> > > >>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN
> > > >>>> and BT_EN ordering and the only requirement is to have all input regulators
> > > >>>> up before pulling up WLAN_EN and/or BT_EN.
> > > >>>>
> > > >>>> This makes the PMU transparent and BT and WLAN can be described as independent.
> > > >>>
> > > >>>  From the hardware perspective, there is a PMU. It has several LDOs. So
> > > >>> the device tree should have the same style as the previous
> > > >>> generations.
> > > >>>
> > > >>
> > > >> My thinking was this: yes, there is a PMU but describing it has no
> > > >> benefit (unlike QCA6x90). If we do describe, then we'll end up having
> > > >> to use pwrseq here despite it not being needed because now we won't be
> > > >> able to just get regulators from WLAN/BT drivers directly.
> > > >>
> > > >> So I also vote for keeping it this way. Let's go into the package
> > > >> detail only if it's required.
> > > >
> > > > The WiFi / BT parts are not powered up by the board regulators. They
> > > > are powered up by the PSU. So we are not describing it in the accurate
> > > > way.
> > >
> > > I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in
> > > this situation the PCIe part is powered with the M.2 slot and the BT side
> > > is powered separately as we currently do it now.
> >
> > QCA6390 can also be used as a discrete M.2 card.
> >
> > > So yes there's a PMU, but it's not an always visible hardware part, from the
> > > SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable.
> >
> > From the hardware point:
> > - There is a PMU
> > - The PMU is connected to the board supplies
> > - Both WiFi and BT parts are connected to the PMU
> > - The BT_EN / WLAN_EN pins are not connected to the PMU
> >
> > So, not representing the PMU in the device tree is a simplification.
> >
>
> What about the existing WLAN and BT users of similar packages? We
> would have to deprecate a lot of existing bindings. I don't think it's
> worth it.

We have bindings that are not reflecting the hardware. So yes, we
should gradually update them once the powerseq is merged.

> The WCN7850 is already described in bindings as consuming what is PMUs
> inputs and not its outputs.

So do WCN6855 and QCA6391 BlueTooth parts.

>
> Bart
>
> > >
> > > Neil
> > >
> > > >
> > > > Moreover, I think we definitely want to move BT driver to use only the
> > > > pwrseq power up method. Doing it in the other way results in the code
> > > > duplication and possible issues because of the regulator / pwrseq
> > > > taking different code paths.
> >
> > --
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry