On Fri, 2 Feb 2024 at 06:34, Bjorn Andersson <andersson@xxxxxxxxxx> wrote:
>
> On Thu, Feb 01, 2024 at 04:55:25PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> >
> > Add a node for the PMU module of the QCA6391 present on the RB5 board.
> > Assign its LDO power outputs to the existing Bluetooth module. Add a
> > node for the PCIe port to sm8250.dtsi and define the WLAN node on it in
> > the board's .dts and also make it consume the power outputs of the PMU.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> > ---
> > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 128 +++++++++++++++++++++--
> > arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 ++
> > 2 files changed, 127 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > index cd0db4f31d4a..fab5bebafbad 100644
> > --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > @@ -108,6 +108,87 @@ lt9611_3v3: lt9611-3v3 {
> > regulator-always-on;
> > };
> >
> > + qca6390_pmu: pmu@0 {
> > + compatible = "qcom,qca6390-pmu";
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&bt_en_state>, <&wlan_en_state>;
> > +
> > + vddaon-supply = <&vreg_s6a_0p95>;
> > + vddpmu-supply = <&vreg_s2f_0p95>;
> > + vddrfa1-supply = <&vreg_s2f_0p95>;
> > + vddrfa2-supply = <&vreg_s8c_1p3>;
> > + vddrfa3-supply = <&vreg_s5a_1p9>;
> > + vddpcie1-supply = <&vreg_s8c_1p3>;
> > + vddpcie2-supply = <&vreg_s5a_1p9>;
> > + vddio-supply = <&vreg_s4a_1p8>;
> > +
> > + 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".
Please take a look at the description of VDD08_PMU_RFA_CMN and
VDD_PMU_AON_I pins in the spec (80-WL522-1, page 25). I'm not sure if
I'm allowed to quote it, so I won't. But the spec clearly describes
VDD_PMU_AON_I as 0.95V LDO input and VDD08_PMU_RFA_CMN as 0.8 LDO
output generated using that input. I think this proves that the
on-chip PMU has actual LDOs.
I must admit, I find this representation very verbose, but on the
other hand Bartosz is right, it represents actual hardware. Maybe we
can drop some of the properties of corresponding regulator blocks, as
we don't actually need them and they are internal properties of the
hardware.
>
> [..]
> >
> > &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?
Power sequencing in the same spec describes that PCIe voltages should
be up even if only BT is being brought up. PMU itself handles
distributing voltages according to the actual load needs.
>
> Regards,
> Bjorn
>
> > };
> > };
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > index 4d849e98bf9b..7cd21d4e7278 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > dma-coherent;
> >
> > status = "disabled";
> > +
> > + pcieport0: pcie@0 {
> > + device_type = "pci";
> > + reg = <0x0 0x0 0x0 0x0 0x0>;
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + bus-range = <0x01 0xff>;
> > + };
> > };
> >
> > pcie0_phy: phy@1c06000 {
> > --
> > 2.40.1
> >
>
--
With best wishes
Dmitry
