mbox series

[v3,0/7] Add PCIe support for SM8150 SoC

Message ID 20220302203045.184500-1-bhupesh.sharma@linaro.org
Headers show
Series Add PCIe support for SM8150 SoC | expand

Message

Bhupesh Sharma March 2, 2022, 8:30 p.m. UTC
Changes since v2:
-----------------
- v2 can be found here: https://lore.kernel.org/linux-arm-msm/20220301072511.117818-1-bhupesh.sharma@linaro.org/T/
- Fixed review comments from Dmitry and Bjorn.
- Modified [PATCH 3/7] from v1 to include gdsc driver structs and
  support code for PCIe0 and PCIe1 (in addition to defines for the
  same).

Changes since v1:
-----------------
- v1 can be found here: https://lore.kernel.org/linux-arm-msm/20220223192946.473172-1-bhupesh.sharma@linaro.org/T/
- Collected ACKs on [PATCH 1/7], [PATCH 2/7] and [PATCH 4/7] from Rob
  and Dmitry.
- Broke down another separately sent out PATCH (see [1]), into a 3 patches (one each for emac, pci
  and ufs gdsc defines) - one of which is carried as [PATCH 3/7]
  in this series, which fixes a compilation error.
  The rest of the gdsc defines have been sent out as separate patch(es).
[1]. https://patchwork.kernel.org/project/netdevbpf/patch/20220126221725.710167-4-bhupesh.sharma@linaro.org/
- Rob's bot reported a number of 'dtbs_check' errors with the v1 series,
  which are been fixed with a separate series now (see [2]), to ease the
  review of this series.
[2]. https://lore.kernel.org/linux-arm-msm/20220228123019.382037-1-bhupesh.sharma@linaro.org/T/


This series adds PCIe support for Qualcomm SM8150 SoC with relevant PHYs.
There are 2 PCIe instances on this SoC each with different PHYs. The PCIe
controller and PHYs are mostly compatible with the ones found on SM8250
SoC, hence the old drivers are modified to add the support.

This series has been tested on SA8155p ADP board with QCA6696 chipset connected
onboard.

Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>

Bhupesh Sharma (7):
  dt-bindings: pci: qcom: Document PCIe bindings for SM8150 SoC
  dt-bindings: phy: qcom,qmp: Add SM8150 PCIe PHY bindings
  clk: qcom: gcc: Add PCIe0 and PCIe1 GDSC for SM8150
  phy: qcom-qmp: Add SM8150 PCIe QMP PHYs
  PCI: qcom: Add SM8150 SoC support
  arm64: dts: qcom: sm8150: Add PCIe nodes
  arm64: dts: qcom: sa8155: Enable PCIe nodes

 .../devicetree/bindings/pci/qcom,pcie.txt     |   5 +-
 .../devicetree/bindings/phy/qcom,qmp-phy.yaml |   4 +
 arch/arm64/boot/dts/qcom/sa8155p-adp.dts      |  42 +++
 arch/arm64/boot/dts/qcom/sm8150.dtsi          | 243 ++++++++++++++++++
 drivers/clk/qcom/gcc-sm8150.c                 |  20 ++
 drivers/pci/controller/dwc/pcie-qcom.c        |   8 +
 drivers/phy/qualcomm/phy-qcom-qmp.c           |  90 +++++++
 include/dt-bindings/clock/qcom,gcc-sm8150.h   |   2 +
 8 files changed, 412 insertions(+), 2 deletions(-)

Comments

Dmitry Baryshkov March 2, 2022, 8:59 p.m. UTC | #1
On Wed, 2 Mar 2022 at 23:31, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote:
>
> SA8155p ADP board supports the PCIe0 controller in the RC
> mode (only). So add the support for the same.
>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 42 ++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> index 8756c2b25c7e..3f6b3ee404f5 100644
> --- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> @@ -387,9 +387,51 @@ &usb_2_qmpphy {
>         vdda-pll-supply = <&vdda_usb_ss_dp_core_1>;
>  };
>
> +&pcie0 {
> +       status = "okay";
> +};
> +
> +&pcie0_phy {
> +       status = "okay";
> +       vdda-phy-supply = <&vreg_l18c_0p88>;
> +       vdda-pll-supply = <&vreg_l8c_1p2>;
> +};
> +
> +&pcie1_phy {
> +       vdda-phy-supply = <&vreg_l18c_0p88>;
> +       vdda-pll-supply = <&vreg_l8c_1p2>;
> +};
> +
>  &tlmm {
>         gpio-reserved-ranges = <0 4>;
>
> +       bt_en_default: bt_en_default {
> +               mux {
> +                       pins = "gpio172";
> +                       function = "gpio";
> +               };
> +
> +               config {
> +                       pins = "gpio172";
> +                       drive-strength = <2>;
> +                       bias-pull-down;
> +               };
> +       };
> +
> +       wlan_en_default: wlan_en_default {
> +               mux {
> +                       pins = "gpio169";
> +                       function = "gpio";
> +               };
> +
> +               config {
> +                       pins = "gpio169";
> +                       drive-strength = <16>;
> +                       output-high;
> +                       bias-pull-up;
> +               };
> +       };
> +

Not related to PCIe

>         usb2phy_ac_en1_default: usb2phy_ac_en1_default {
>                 mux {
>                         pins = "gpio113";
> --
> 2.35.1
>
Bhupesh Sharma March 3, 2022, 6:09 a.m. UTC | #2
Hi Dmitry,

On Thu, 3 Mar 2022 at 02:29, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, 2 Mar 2022 at 23:31, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote:
> >
> > SA8155p ADP board supports the PCIe0 controller in the RC
> > mode (only). So add the support for the same.
> >
> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Cc: Vinod Koul <vkoul@kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 42 ++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> > index 8756c2b25c7e..3f6b3ee404f5 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> > @@ -387,9 +387,51 @@ &usb_2_qmpphy {
> >         vdda-pll-supply = <&vdda_usb_ss_dp_core_1>;
> >  };
> >
> > +&pcie0 {
> > +       status = "okay";
> > +};
> > +
> > +&pcie0_phy {
> > +       status = "okay";
> > +       vdda-phy-supply = <&vreg_l18c_0p88>;
> > +       vdda-pll-supply = <&vreg_l8c_1p2>;
> > +};
> > +
> > +&pcie1_phy {
> > +       vdda-phy-supply = <&vreg_l18c_0p88>;
> > +       vdda-pll-supply = <&vreg_l8c_1p2>;
> > +};
> > +
> >  &tlmm {
> >         gpio-reserved-ranges = <0 4>;
> >
> > +       bt_en_default: bt_en_default {
> > +               mux {
> > +                       pins = "gpio172";
> > +                       function = "gpio";
> > +               };
> > +
> > +               config {
> > +                       pins = "gpio172";
> > +                       drive-strength = <2>;
> > +                       bias-pull-down;
> > +               };
> > +       };
> > +
> > +       wlan_en_default: wlan_en_default {
> > +               mux {
> > +                       pins = "gpio169";
> > +                       function = "gpio";
> > +               };
> > +
> > +               config {
> > +                       pins = "gpio169";
> > +                       drive-strength = <16>;
> > +                       output-high;
> > +                       bias-pull-up;
> > +               };
> > +       };
> > +
>
> Not related to PCIe

Hmm.. I have no strong personal opinion on this, so let's see what
Bjorn thinks about the same.
My reasoning for keeping it here was to just capture that we have
'bt_en' and 'wlan_en' related tlmm details here, so that when you send
out the reworked QCAxxxx mfd series (see [1]) later, I can easily plug
it in for SA8155p ADP dts as well with the 'bt' and 'wlan' constructs.

[1]. https://lore.kernel.org/lkml/20210621223141.1638189-2-dmitry.baryshkov@linaro.org/T/

Regards.
Bhupesh

> >         usb2phy_ac_en1_default: usb2phy_ac_en1_default {
> >                 mux {
> >                         pins = "gpio113";
> > --
> > 2.35.1
> >
>
>
> --
> With best wishes
> Dmitry
Bjorn Andersson March 8, 2022, 10:31 p.m. UTC | #3
On Thu 03 Mar 00:09 CST 2022, Bhupesh Sharma wrote:

> Hi Dmitry,
> 
> On Thu, 3 Mar 2022 at 02:29, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Wed, 2 Mar 2022 at 23:31, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote:
> > >
> > > SA8155p ADP board supports the PCIe0 controller in the RC
> > > mode (only). So add the support for the same.
> > >
> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > Cc: Vinod Koul <vkoul@kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 42 ++++++++++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> > > index 8756c2b25c7e..3f6b3ee404f5 100644
> > > --- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> > > +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
> > > @@ -387,9 +387,51 @@ &usb_2_qmpphy {
> > >         vdda-pll-supply = <&vdda_usb_ss_dp_core_1>;
> > >  };
> > >
> > > +&pcie0 {
> > > +       status = "okay";
> > > +};
> > > +
> > > +&pcie0_phy {
> > > +       status = "okay";
> > > +       vdda-phy-supply = <&vreg_l18c_0p88>;
> > > +       vdda-pll-supply = <&vreg_l8c_1p2>;
> > > +};
> > > +
> > > +&pcie1_phy {
> > > +       vdda-phy-supply = <&vreg_l18c_0p88>;
> > > +       vdda-pll-supply = <&vreg_l8c_1p2>;
> > > +};
> > > +
> > >  &tlmm {
> > >         gpio-reserved-ranges = <0 4>;
> > >
> > > +       bt_en_default: bt_en_default {

'_' is not a valid character in the node name (it is in the label).

> > > +               mux {

Please flatten this, you can omit the mux and config subnodes and put
the properties directly in the state node.

> > > +                       pins = "gpio172";
> > > +                       function = "gpio";
> > > +               };
> > > +
> > > +               config {
> > > +                       pins = "gpio172";
> > > +                       drive-strength = <2>;
> > > +                       bias-pull-down;
> > > +               };
> > > +       };
> > > +
> > > +       wlan_en_default: wlan_en_default {
> > > +               mux {
> > > +                       pins = "gpio169";
> > > +                       function = "gpio";
> > > +               };
> > > +
> > > +               config {
> > > +                       pins = "gpio169";
> > > +                       drive-strength = <16>;
> > > +                       output-high;
> > > +                       bias-pull-up;
> > > +               };
> > > +       };
> > > +
> >
> > Not related to PCIe
> 
> Hmm.. I have no strong personal opinion on this, so let's see what
> Bjorn thinks about the same.
> My reasoning for keeping it here was to just capture that we have
> 'bt_en' and 'wlan_en' related tlmm details here, so that when you send
> out the reworked QCAxxxx mfd series (see [1]) later, I can easily plug
> it in for SA8155p ADP dts as well with the 'bt' and 'wlan' constructs.
> 

The BT_EN is unrelated to PCIe, and I'm not able to see where you select
the wlan_en_default state, so this would be dangling.

So the bt_en should come in a patch together with a bluetooth node and
the wlan_en_default should come with something that ensures that the
WiFi portion of the chip is powered and the gpio enabled.

Regards,
Bjorn

> [1]. https://lore.kernel.org/lkml/20210621223141.1638189-2-dmitry.baryshkov@linaro.org/T/
> 
> Regards.
> Bhupesh
> 
> > >         usb2phy_ac_en1_default: usb2phy_ac_en1_default {
> > >                 mux {
> > >                         pins = "gpio113";
> > > --
> > > 2.35.1
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry
Rob Herring (Arm) March 24, 2022, 8:52 p.m. UTC | #4
On Thu, Mar 03, 2022 at 02:00:43AM +0530, Bhupesh Sharma wrote:
> The PCIe IP (rev 1.5.0) on SM8150 SoC is similar to the one used on
> SM8250. Hence the support is added reusing the members of ops_1_9_0.
> 
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>