On Thu, 22 Feb 2024 at 14:27, Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > > On Thu, Feb 22, 2024 at 12:27 PM Dmitry Baryshkov > <dmitry.baryshkov@xxxxxxxxxx> wrote: > > > > On Thu, 22 Feb 2024 at 13:00, Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > > > > > > On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov > > > <dmitry.baryshkov@xxxxxxxxxx> wrote: > > > > > > > > On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@xxxxxxxxxx> wrote: > > > > > > > > > > On 19/02/2024 13:33, Dmitry Baryshkov wrote: > > > > > > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > > > > > >> > > > > > >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov > > > > > >> <dmitry.baryshkov@xxxxxxxxxx> 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. > > > > That is not true for the latter, this series is adding regulators for it. But the bindings exist already, so you still have to extend it, deprecating regulator-less bindings. Bartosz, I really don't understand what is the issue there. There is a PMU. As such it should be represented in the DT and it can be handled by the same driver as you are adding for QCA6390. > > Bart > > > > > > > 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 -- With best wishes Dmitry