On Fri, Feb 2, 2024 at 5:34 AM Bjorn Andersson <andersson@xxxxxxxxxx> wrote:
>
[snip]
> > +
> > + wlan-enable-gpios = <&tlmm 20 GPIO_ACTIVE_HIGH>;
> > + bt-enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>;
> > +
> > + regulators {
> > + vreg_pmu_rfa_cmn: ldo0 {
> > + regulator-name = "vreg_pmu_rfa_cmn";
> > + regulator-min-microvolt = <760000>;
> > + regulator-max-microvolt = <840000>;
>
> I'm still not convinced that the PMU has a set of LDOs, and looking at
> your implementation you neither register these with the regulator
> framework, nor provide any means of controlling the state or voltage of
> these "regulators".
>
Why are you so fixated on the driver implementation matching the
device-tree 1:1? I asked that question before - what does it matter if
we use the regulator subsystem or not? This is just what HW there is.
What we do with that knowledge in C is irrelevant. Yes, I don't use
the regulator subsystem because it's unnecessary and would actually
get in the way of the power sequencing. But it doesn't change the fact
that the regulators *are* there so let's show them.
What isn't there is a "power sequencer device". This was the main
concern about Dmitry's implementation before. We must not have
"bt-pwrseq = <&...>;" -like properties in device-tree because there is
no device that this would represent. But there *are* LDO outputs of
the PMU which can be modelled and then used in C to retrieve the power
sequencer and this is what I'm proposing.
Bartosz
> [..]
> >
> > &uart6 {
> > @@ -1311,17 +1418,16 @@ &uart6 {
> > bluetooth {
> > compatible = "qcom,qca6390-bt";
> >
> > - pinctrl-names = "default";
> > - pinctrl-0 = <&bt_en_state>;
> > -
> > - enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>;
> > -
> > - vddio-supply = <&vreg_s4a_1p8>;
> > - vddpmu-supply = <&vreg_s2f_0p95>;
> > - vddaon-supply = <&vreg_s6a_0p95>;
> > - vddrfa0p9-supply = <&vreg_s2f_0p95>;
> > - vddrfa1p3-supply = <&vreg_s8c_1p3>;
> > - vddrfa1p9-supply = <&vreg_s5a_1p9>;
> > + vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
> > + vddaon-supply = <&vreg_pmu_aon_0p59>;
> > + vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
> > + vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
> > + vddbtcmx-supply = <&vreg_pmu_btcmx_0p85>;
> > + vddrfa0-supply = <&vreg_pmu_rfa_0p8>;
> > + vddrfa1-supply = <&vreg_pmu_rfa_1p2>;
> > + vddrfa2-supply = <&vreg_pmu_rfa_1p7>;
> > + vddpcie0-supply = <&vreg_pmu_pcie_0p9>;
> > + vddpcie1-supply = <&vreg_pmu_pcie_1p8>;
>
> As I asked before, why does bluetooth suddenly care about PCIe supplies?
>
Yes, I forgot to remove it, I'll do it next time.
Bartosz
[snip]
