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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 22/02/2024 13:50, Bartosz Golaszewski wrote:
> On Thu, Feb 22, 2024 at 1:47 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> On Thu, 22 Feb 2024 at 14:27, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>
>>> On Thu, Feb 22, 2024 at 12:27 PM Dmitry Baryshkov
>>> <dmitry.baryshkov@linaro.org> wrote:
>>>>
>>>> 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.
>>>>
>>>
>>> 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.
>>
> 
> The issue is that we'll pull in the pwrseq subsystem for WCN7850 which
> clearly does not require it in practice.
> 
> I'd like to hear Krzysztof, Conor or Rob chime in here and make the
> decision on how to proceed.

There's like 12 emails here, so please don't just point "MR X, please
read everything to find the question I want to ask", but just ask the
question with short intro. We all (and I bet you as well) are way too
busy to read long threads...

If I got it correctly, you ask if some other, existing QCA/WCN chips
should be changed to this PMU approach?

If yes, then:
1. It depends whether they have the PMU, so some sort of analysis of
datasheet should be done.
2. You could but you don't have to. Bindings were done, they represent
the hardware more-or-less, maybe less, but still good enough.
3. It does not have to impact actual behavior of Linux. You don't have
to bind entire pwrseq driver to that QCA/WCN compatible. Anyway Linux
behavior is here a bit separate question - it can change, it can stay
the same, up to you.

Best regards,
Krzysztof