2023-09-29 16:43:19

by Nitin Rawat

[permalink] [raw]
Subject: [PATCH V4 0/4] Add UFS host controller and Phy nodes for sc7280

This patch adds UFS host controller and Phy nodes for Qualcomm sc7280 SOC
and sc7280 Board.

Changes from v3:
- Addresses Mani comment to include interconnect entry.

Changes from v2:
- Addressed Konrad comment to update binding qcom,ufs.yaml
- Addresses mani/konrad comment to align ufs clock entry in devicetree.

Changes from v1:
- Addressed mani comment to separate soc and board change.
- Addressed mani comment to sort ufs node in ascending order.

Nitin Rawat (4):
scsi: ufs: qcom: dt-bindings: Add SC7280 compatible string
arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc
arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 IDP board
dt-bindings: ufs: qcom: Align clk binding property for Qualcomm UFS

Nitin Rawat (4):
scsi: ufs: qcom: dt-bindings: Add SC7280 compatible string
arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc
arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 IDP board
dt-bindings: ufs: qcom: Align clk binding property for Qualcomm UFS

.../devicetree/bindings/ufs/qcom,ufs.yaml | 18 ++---
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 19 ++++++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 66 +++++++++++++++++++
3 files changed, 95 insertions(+), 8 deletions(-)

--
2.17.1


2023-09-29 18:45:39

by Nitin Rawat

[permalink] [raw]
Subject: [PATCH V4 2/4] arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc

Add UFS host controller and PHY nodes for sc7280 soc.

Signed-off-by: Nitin Rawat <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 66 ++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 66f1eb83cca7..163e3df60966 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3353,6 +3353,72 @@
};
};

+ ufs_mem_hc: ufs@1d84000 {
+ compatible = "qcom,sc7280-ufshc", "qcom,ufshc",
+ "jedec,ufs-2.0";
+ reg = <0x0 0x01d84000 0x0 0x3000>;
+ interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
+ phys = <&ufs_mem_phy>;
+ phy-names = "ufsphy";
+ lanes-per-direction = <2>;
+ #reset-cells = <1>;
+ resets = <&gcc GCC_UFS_PHY_BCR>;
+ reset-names = "rst";
+
+ power-domains = <&gcc GCC_UFS_PHY_GDSC>;
+ required-opps = <&rpmhpd_opp_nom>;
+
+ iommus = <&apps_smmu 0x80 0x0>;
+ dma-coherent;
+
+ interconnects = <&aggre1_noc MASTER_UFS_MEM 0 &mc_virt SLAVE_EBI1 0>,
+ <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_UFS_MEM_CFG 0>;
+
+ clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
+ <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
+ <&gcc GCC_UFS_PHY_AHB_CLK>,
+ <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
+ <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
+ <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
+ <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
+ clock-names = "core_clk",
+ "bus_aggr_clk",
+ "iface_clk",
+ "core_clk_unipro",
+ "ref_clk",
+ "tx_lane0_sync_clk",
+ "rx_lane0_sync_clk",
+ "rx_lane1_sync_clk";
+ freq-table-hz =
+ <75000000 300000000>,
+ <0 0>,
+ <0 0>,
+ <75000000 300000000>,
+ <0 0>,
+ <0 0>,
+ <0 0>,
+ <0 0>;
+ status = "disabled";
+ };
+
+ ufs_mem_phy: phy@1d87000 {
+ compatible = "qcom,sc7280-qmp-ufs-phy";
+ reg = <0x0 0x01d87000 0x0 0xe00>;
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_1_CLKREF_EN>;
+ clock-names = "ref", "ref_aux", "qref";
+
+ resets = <&ufs_mem_hc 0>;
+ reset-names = "ufsphy";
+
+ #clock-cells = <1>;
+ #phy-cells = <0>;
+
+ status = "disabled";
+ };
+
usb_1_hsphy: phy@88e3000 {
compatible = "qcom,sc7280-usb-hs-phy",
"qcom,usb-snps-hs-7nm-phy";
--
2.17.1

2023-09-29 18:59:01

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH V4 2/4] arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc



On 9/29/23 15:19, Nitin Rawat wrote:
> Add UFS host controller and PHY nodes for sc7280 soc.
>
> Signed-off-by: Nitin Rawat <[email protected]>
> ---
Not sure if intentionally, but you didn't include my review tags from v3.

[...]

> + interconnects = <&aggre1_noc MASTER_UFS_MEM 0 &mc_virt SLAVE_EBI1 0>,
> + <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_UFS_MEM_CFG 0>;
include dt-bindings/interconnect/qcom,icc.h

interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
&cnoc2 SLAVE_UFS_MEM_CFG QCOM_ICC_TAG_ALWAYS>;

Konrad

2023-09-29 19:49:28

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH V4 0/4] Add UFS host controller and Phy nodes for sc7280



On 9/29/23 15:19, Nitin Rawat wrote:
> This patch adds UFS host controller and Phy nodes for Qualcomm sc7280 SOC
> and sc7280 Board.
>
> Changes from v3:
> - Addresses Mani comment to include interconnect entry.
These are not the only changes you've made.

>
> Changes from v2:
> - Addressed Konrad comment to update binding qcom,ufs.yaml
> - Addresses mani/konrad comment to align ufs clock entry in devicetree.
>
> Changes from v1:
> - Addressed mani comment to separate soc and board change.
> - Addressed mani comment to sort ufs node in ascending order.
>
> Nitin Rawat (4):
> scsi: ufs: qcom: dt-bindings: Add SC7280 compatible string
> arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc
> arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 IDP board
> dt-bindings: ufs: qcom: Align clk binding property for Qualcomm UFS
>
> Nitin Rawat (4):
> scsi: ufs: qcom: dt-bindings: Add SC7280 compatible string
> arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc
> arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 IDP board
> dt-bindings: ufs: qcom: Align clk binding property for Qualcomm UFS
And this is bad.

Please use the b4 tool [1], half of the issues will be solved..

Konrad

[1] https://b4.docs.kernel.org/

2023-09-29 20:05:47

by Nitin Rawat

[permalink] [raw]
Subject: [PATCH V4 3/4] arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 IDP board

Add UFS host controller and PHY nodes for sc7280 IDP board.

Signed-off-by: Nitin Rawat <[email protected]>
Acked-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
Tested-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 2ff549f4dc7a..a0059527d9e4 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -499,6 +499,25 @@
status = "okay";
};

+&ufs_mem_hc {
+ reset-gpios = <&tlmm 175 GPIO_ACTIVE_LOW>;
+ vcc-supply = <&vreg_l7b_2p9>;
+ vcc-max-microamp = <800000>;
+ vccq-supply = <&vreg_l9b_1p2>;
+ vccq-max-microamp = <900000>;
+ vccq2-supply = <&vreg_l9b_1p2>;
+ vccq2-max-microamp = <900000>;
+
+ status = "okay";
+};
+
+&ufs_mem_phy {
+ vdda-phy-supply = <&vreg_l10c_0p8>;
+ vdda-pll-supply = <&vreg_l6b_1p2>;
+
+ status = "okay";
+};
+
&usb_1 {
status = "okay";
};
--
2.17.1

2023-09-29 23:09:24

by Nitin Rawat

[permalink] [raw]
Subject: [PATCH V4 4/4] dt-bindings: ufs: qcom: Align clk binding property for Qualcomm UFS

Align the binding property for clock such that "clocks" property
comes first followed by "clock-names" property.

Signed-off-by: Nitin Rawat <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
.../devicetree/bindings/ufs/qcom,ufs.yaml | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index 802640efa956..d17bdc4e934f 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -295,14 +295,6 @@ examples:
<&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_UFS_MEM_CFG>;
interconnect-names = "ufs-ddr", "cpu-ufs";

- clock-names = "core_clk",
- "bus_aggr_clk",
- "iface_clk",
- "core_clk_unipro",
- "ref_clk",
- "tx_lane0_sync_clk",
- "rx_lane0_sync_clk",
- "rx_lane1_sync_clk";
clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
<&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
<&gcc GCC_UFS_PHY_AHB_CLK>,
@@ -311,6 +303,14 @@ examples:
<&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
<&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
<&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
+ clock-names = "core_clk",
+ "bus_aggr_clk",
+ "iface_clk",
+ "core_clk_unipro",
+ "ref_clk",
+ "tx_lane0_sync_clk",
+ "rx_lane0_sync_clk",
+ "rx_lane1_sync_clk";
freq-table-hz = <75000000 300000000>,
<0 0>,
<0 0>,
--
2.17.1

2023-09-30 17:05:51

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH V4 4/4] dt-bindings: ufs: qcom: Align clk binding property for Qualcomm UFS

On 29/09/2023 15:19, Nitin Rawat wrote:
> Align the binding property for clock such that "clocks" property
> comes first followed by "clock-names" property.

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

Since you also ignored tags or added them wrong, let's be clear here:
NAK till you solve all the issues.

Best regards,
Krzysztof

2023-10-05 14:15:24

by Luca Weiss

[permalink] [raw]
Subject: Re: [PATCH V4 2/4] arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc

On Fri Sep 29, 2023 at 3:19 PM CEST, Nitin Rawat wrote:
> Add UFS host controller and PHY nodes for sc7280 soc.

Hi Nitin,

I left some comments for v3 that maybe you missed before sending v4:
https://lore.kernel.org/linux-arm-msm/CVVA1OVF4W9E.380D6QC1K9GD6@otso/

At least the clock part should definitely be fixed, ICE we can do later.

Regards
Luca

>
> Signed-off-by: Nitin Rawat <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 66 ++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 66f1eb83cca7..163e3df60966 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3353,6 +3353,72 @@
> };
> };
>
> + ufs_mem_hc: ufs@1d84000 {
> + compatible = "qcom,sc7280-ufshc", "qcom,ufshc",
> + "jedec,ufs-2.0";
> + reg = <0x0 0x01d84000 0x0 0x3000>;
> + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&ufs_mem_phy>;
> + phy-names = "ufsphy";
> + lanes-per-direction = <2>;
> + #reset-cells = <1>;
> + resets = <&gcc GCC_UFS_PHY_BCR>;
> + reset-names = "rst";
> +
> + power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> + required-opps = <&rpmhpd_opp_nom>;
> +
> + iommus = <&apps_smmu 0x80 0x0>;
> + dma-coherent;
> +
> + interconnects = <&aggre1_noc MASTER_UFS_MEM 0 &mc_virt SLAVE_EBI1 0>,
> + <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_UFS_MEM_CFG 0>;
> +
> + clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
> + <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> + <&gcc GCC_UFS_PHY_AHB_CLK>,
> + <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
> + <&rpmhcc RPMH_CXO_CLK>,
> + <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
> + <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
> + <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
> + clock-names = "core_clk",
> + "bus_aggr_clk",
> + "iface_clk",
> + "core_clk_unipro",
> + "ref_clk",
> + "tx_lane0_sync_clk",
> + "rx_lane0_sync_clk",
> + "rx_lane1_sync_clk";
> + freq-table-hz =
> + <75000000 300000000>,
> + <0 0>,
> + <0 0>,
> + <75000000 300000000>,
> + <0 0>,
> + <0 0>,
> + <0 0>,
> + <0 0>;
> + status = "disabled";
> + };
> +
> + ufs_mem_phy: phy@1d87000 {
> + compatible = "qcom,sc7280-qmp-ufs-phy";
> + reg = <0x0 0x01d87000 0x0 0xe00>;
> + clocks = <&rpmhcc RPMH_CXO_CLK>,
> + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
> + <&gcc GCC_UFS_1_CLKREF_EN>;
> + clock-names = "ref", "ref_aux", "qref";
> +
> + resets = <&ufs_mem_hc 0>;
> + reset-names = "ufsphy";
> +
> + #clock-cells = <1>;
> + #phy-cells = <0>;
> +
> + status = "disabled";
> + };
> +
> usb_1_hsphy: phy@88e3000 {
> compatible = "qcom,sc7280-usb-hs-phy",
> "qcom,usb-snps-hs-7nm-phy";
> --
> 2.17.1