[PATCH 0/4] support secondary ufs for Exynos Auto v9 SoC

[PATCH 0/4] support secondary ufs for Exynos Auto v9 SoC

[Chanho Park]

To support ufs #1 for Exynos Auto v9 SoC, we need to control its own pmu
register. So, we need to specify the offset of the register via an
argument of syscon node.
Regarding UFS HCI driver, we need to specify different sysreg
configuration for UFS #1's io coherency setting.

Chanho Park (4):
  dt-bindings: phy: samsung,ufs-phy: make pmu-syscon as phandle-array
  phy: samsung: ufs: support secondary ufs phy
  arm64: dts: exynosautov9: add secondary ufs devices
  arm64: dts: exynosautov9-sadk: enable secondary ufs devices

 .../bindings/phy/samsung,ufs-phy.yaml         | 13 +++++---
 .../boot/dts/exynos/exynosautov9-sadk.dts     | 18 +++++++++++
 arch/arm64/boot/dts/exynos/exynosautov9.dtsi  | 32 +++++++++++++++++++
 drivers/phy/samsung/phy-samsung-ufs.c         |  7 +++-
 drivers/phy/samsung/phy-samsung-ufs.h         |  2 +-
 5 files changed, 66 insertions(+), 6 deletions(-)

-- 
2.36.1

[PATCH 1/4] dt-bindings: phy: samsung,ufs-phy: make pmu-syscon as phandle-array

[Chanho Park]

To support secondary ufs phy devices, we need to get an offset value
from pmu-syscon.

Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 .../devicetree/bindings/phy/samsung,ufs-phy.yaml    | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
index f6ed1a005e7a..80c3ec14375a 100644
--- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
@@ -40,9 +40,14 @@ properties:
       - const: tx0_symbol_clk
 
   samsung,pmu-syscon:
-    $ref: '/schemas/types.yaml#/definitions/phandle'
-    description: phandle for PMU system controller interface, used to
-                 control pmu registers bits for ufs m-phy
+    $ref: '/schemas/types.yaml#/definitions/phandle-array'
+    items:
+      - items:
+        - description: phandle for PMU system controller interface, used to
+                       control pmu registers bits for ufs m-phy
+        - description: offset of pmu control register for ufs m-phy
+    description:
+      can be phandle/offset pair. The offset can be optional.
 
 required:
   - "#phy-cells"
@@ -63,7 +68,7 @@ examples:
         compatible = "samsung,exynos7-ufs-phy";
         reg = <0x15571800 0x240>;
         reg-names = "phy-pma";
-        samsung,pmu-syscon = <&pmu_system_controller>;
+        samsung,pmu-syscon = <&pmu_system_controller 0x720>;
         #phy-cells = <0>;
         clocks = <&clock_fsys1 SCLK_COMBO_PHY_EMBEDDED_26M>,
                  <&clock_fsys1 PHYCLK_UFS20_RX1_SYMBOL_USER>,
-- 
2.36.1

Re: [PATCH 1/4] dt-bindings: phy: samsung,ufs-phy: make pmu-syscon as phandle-array

[Krzysztof Kozlowski]

On 31/05/2022 14:19, Chanho Park wrote:
> To support secondary ufs phy devices, we need to get an offset value
> from pmu-syscon.
> 
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  .../devicetree/bindings/phy/samsung,ufs-phy.yaml    | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> index f6ed1a005e7a..80c3ec14375a 100644
> --- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> @@ -40,9 +40,14 @@ properties:
>        - const: tx0_symbol_clk
>  
>    samsung,pmu-syscon:
> -    $ref: '/schemas/types.yaml#/definitions/phandle'
> -    description: phandle for PMU system controller interface, used to
> -                 control pmu registers bits for ufs m-phy
> +    $ref: '/schemas/types.yaml#/definitions/phandle-array'
> +    items:
> +      - items:
> +        - description: phandle for PMU system controller interface, used to
> +                       control pmu registers bits for ufs m-phy

Wrong indentation.

> +        - description: offset of pmu control register for ufs m-phy
> +    description:
> +      can be phandle/offset pair. The offset can be optional.

You made offset required, not optional.  You need minItems:1 on the
level of second "items".

>  
>  required:
>    - "#phy-cells"
> @@ -63,7 +68,7 @@ examples:
>          compatible = "samsung,exynos7-ufs-phy";
>          reg = <0x15571800 0x240>;
>          reg-names = "phy-pma";
> -        samsung,pmu-syscon = <&pmu_system_controller>;
> +        samsung,pmu-syscon = <&pmu_system_controller 0x720>;
>          #phy-cells = <0>;
>          clocks = <&clock_fsys1 SCLK_COMBO_PHY_EMBEDDED_26M>,
>                   <&clock_fsys1 PHYCLK_UFS20_RX1_SYMBOL_USER>,


Best regards,
Krzysztof

RE: [PATCH 1/4] dt-bindings: phy: samsung,ufs-phy: make pmu-syscon as phandle-array

[Chanho Park]

> > +    items:
> > +      - items:
> > +        - description: phandle for PMU system controller interface, used
> to
> > +                       control pmu registers bits for ufs m-phy
> 
> Wrong indentation.

This will be fixed on the next patchset.

> 
> > +        - description: offset of pmu control register for ufs m-phy
> > +    description:
> > +      can be phandle/offset pair. The offset can be optional.
> 
> You made offset required, not optional.  You need minItems:1 on the level
> of second "items".

Thanks. I didn't know how I make such that.. I'll apply this to next patchset.

> 
> >
> >  required:
> >    - "#phy-cells"
> > @@ -63,7 +68,7 @@ examples:
> >          compatible = "samsung,exynos7-ufs-phy";
> >          reg = <0x15571800 0x240>;
> >          reg-names = "phy-pma";
> > -        samsung,pmu-syscon = <&pmu_system_controller>;
> > +        samsung,pmu-syscon = <&pmu_system_controller 0x720>;

This will be dropped as well.

Best Regards,
Chanho Park

Re: [PATCH 1/4] dt-bindings: phy: samsung,ufs-phy: make pmu-syscon as phandle-array

[Rob Herring]

On Tue, 31 May 2022 21:19:10 +0900, Chanho Park wrote:
> To support secondary ufs phy devices, we need to get an offset value
> from pmu-syscon.
> 
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  .../devicetree/bindings/phy/samsung,ufs-phy.yaml    | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml:46:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

[PATCH 2/4] phy: samsung: ufs: support secondary ufs phy

[Chanho Park]

To support secondary ufs phy device, we need to get an offset for phy
isolation from the syscon DT node. If the first index argument of the
node is existing, we can read the offset value and set it as isol->offset.
To allow this, we should drop the const qualifier of struct pmu_isol.

Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 drivers/phy/samsung/phy-samsung-ufs.c | 7 ++++++-
 drivers/phy/samsung/phy-samsung-ufs.h | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c
index 602ddef259eb..bb0f8e481a4b 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.c
+++ b/drivers/phy/samsung/phy-samsung-ufs.c
@@ -288,6 +288,7 @@ static int samsung_ufs_phy_probe(struct platform_device *pdev)
 	struct phy *gen_phy;
 	struct phy_provider *phy_provider;
 	const struct samsung_ufs_phy_drvdata *drvdata;
+	u32 isol_offset;
 	int err = 0;
 
 	match = of_match_node(samsung_ufs_phy_match, dev->of_node);
@@ -328,7 +329,11 @@ static int samsung_ufs_phy_probe(struct platform_device *pdev)
 	phy->dev = dev;
 	phy->drvdata = drvdata;
 	phy->cfg = (struct samsung_ufs_phy_cfg **)drvdata->cfg;
-	phy->isol = &drvdata->isol;
+	phy->isol = (struct pmu_isol *)&drvdata->isol;
+	if (!of_property_read_u32_index(dev->of_node, "samsung,pmu-syscon", 1,
+					&isol_offset))
+		phy->isol->offset = isol_offset;
+
 	phy->lane_cnt = PHY_DEF_LANE_CNT;
 
 	phy_set_drvdata(gen_phy, phy);
diff --git a/drivers/phy/samsung/phy-samsung-ufs.h b/drivers/phy/samsung/phy-samsung-ufs.h
index 91a0e9f94f98..75dc533273f9 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.h
+++ b/drivers/phy/samsung/phy-samsung-ufs.h
@@ -122,7 +122,7 @@ struct samsung_ufs_phy {
 	struct clk *rx1_symbol_clk;
 	const struct samsung_ufs_phy_drvdata *drvdata;
 	struct samsung_ufs_phy_cfg **cfg;
-	const struct pmu_isol *isol;
+	struct pmu_isol *isol;
 	u8 lane_cnt;
 	int ufs_phy_state;
 	enum phy_mode mode;
-- 
2.36.1

Re: [PATCH 2/4] phy: samsung: ufs: support secondary ufs phy

[Krzysztof Kozlowski]

On 31/05/2022 14:19, Chanho Park wrote:
> To support secondary ufs phy device, we need to get an offset for phy
> isolation from the syscon DT node. If the first index argument of the
> node is existing, we can read the offset value and set it as isol->offset.
> To allow this, we should drop the const qualifier of struct pmu_isol.
> 
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  drivers/phy/samsung/phy-samsung-ufs.c | 7 ++++++-
>  drivers/phy/samsung/phy-samsung-ufs.h | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c
> index 602ddef259eb..bb0f8e481a4b 100644
> --- a/drivers/phy/samsung/phy-samsung-ufs.c
> +++ b/drivers/phy/samsung/phy-samsung-ufs.c
> @@ -288,6 +288,7 @@ static int samsung_ufs_phy_probe(struct platform_device *pdev)
>  	struct phy *gen_phy;
>  	struct phy_provider *phy_provider;
>  	const struct samsung_ufs_phy_drvdata *drvdata;
> +	u32 isol_offset;
>  	int err = 0;
>  
>  	match = of_match_node(samsung_ufs_phy_match, dev->of_node);
> @@ -328,7 +329,11 @@ static int samsung_ufs_phy_probe(struct platform_device *pdev)
>  	phy->dev = dev;
>  	phy->drvdata = drvdata;
>  	phy->cfg = (struct samsung_ufs_phy_cfg **)drvdata->cfg;
> -	phy->isol = &drvdata->isol;
> +	phy->isol = (struct pmu_isol *)&drvdata->isol;

No casts are needed for proper code. If you need it - code is not proper.

> +	if (!of_property_read_u32_index(dev->of_node, "samsung,pmu-syscon", 1,
> +					&isol_offset))
> +		phy->isol->offset = isol_offset;

drvdata is const.

> +
>  	phy->lane_cnt = PHY_DEF_LANE_CNT;
>  
>  	phy_set_drvdata(gen_phy, phy);
> diff --git a/drivers/phy/samsung/phy-samsung-ufs.h b/drivers/phy/samsung/phy-samsung-ufs.h
> index 91a0e9f94f98..75dc533273f9 100644
> --- a/drivers/phy/samsung/phy-samsung-ufs.h
> +++ b/drivers/phy/samsung/phy-samsung-ufs.h
> @@ -122,7 +122,7 @@ struct samsung_ufs_phy {
>  	struct clk *rx1_symbol_clk;
>  	const struct samsung_ufs_phy_drvdata *drvdata;
>  	struct samsung_ufs_phy_cfg **cfg;
> -	const struct pmu_isol *isol;
> +	struct pmu_isol *isol;
>  	u8 lane_cnt;
>  	int ufs_phy_state;
>  	enum phy_mode mode;


Best regards,
Krzysztof

RE: [PATCH 2/4] phy: samsung: ufs: support secondary ufs phy

[Chanho Park]

> > +	u32 isol_offset;
> >  	int err = 0;
> >
> >  	match = of_match_node(samsung_ufs_phy_match, dev->of_node); @@
> > -328,7 +329,11 @@ static int samsung_ufs_phy_probe(struct
> platform_device *pdev)
> >  	phy->dev = dev;
> >  	phy->drvdata = drvdata;
> >  	phy->cfg = (struct samsung_ufs_phy_cfg **)drvdata->cfg;
> > -	phy->isol = &drvdata->isol;
> > +	phy->isol = (struct pmu_isol *)&drvdata->isol;
> 
> No casts are needed for proper code. If you need it - code is not proper.

To change a value of phy->isol, I need to allocate its own isol data and copying whole data from drvdata instead of using drvdata->isol pointer.

Best Regards,
Chanho Park

[PATCH 3/4] arm64: dts: exynosautov9: add secondary ufs devices

[Chanho Park]

Add ufs_1_phy and ufs_1 for secondary ufs hci controller and phy
device.

Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynosautov9.dtsi | 32 ++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynosautov9.dtsi b/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
index 3e23db8f09d9..c146271af477 100644
--- a/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
@@ -380,6 +380,17 @@ ufs_0_phy: ufs0-phy@17e04000 {
 			status = "disabled";
 		};
 
+		ufs_1_phy: ufs0-phy@17f04000 {
+			compatible = "samsung,exynosautov9-ufs-phy";
+			reg = <0x17f04000 0xc00>;
+			reg-names = "phy-pma";
+			samsung,pmu-syscon = <&pmu_system_controller 0x72c>;
+			#phy-cells = <0>;
+			clocks = <&xtcxo>;
+			clock-names = "ref_clk";
+			status = "disabled";
+		};
+
 		ufs_0: ufs0@17e00000 {
 			compatible ="samsung,exynosautov9-ufs";
 
@@ -400,6 +411,27 @@ ufs_0: ufs0@17e00000 {
 			samsung,sysreg = <&syscon_fsys2 0x710>;
 			status = "disabled";
 		};
+
+		ufs_1: ufs0@17f00000 {
+			compatible ="samsung,exynosautov9-ufs";
+
+			reg = <0x17f00000 0x100>,  /* 0: HCI standard */
+				<0x17f01100 0x410>,  /* 1: Vendor-specific */
+				<0x17f80000 0x8000>,  /* 2: UNIPRO */
+				<0x17de0000 0x2200>;  /* 3: UFS protector */
+			reg-names = "hci", "vs_hci", "unipro", "ufsp";
+			interrupts = <GIC_SPI 235 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cmu_fsys2 CLK_GOUT_FSYS2_UFS_EMBD1_ACLK>,
+				 <&cmu_fsys2 CLK_GOUT_FSYS2_UFS_EMBD1_UNIPRO>;
+			clock-names = "core_clk", "sclk_unipro_main";
+			freq-table-hz = <0 0>, <0 0>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&ufs_rst_n_1 &ufs_refclk_out_1>;
+			phys = <&ufs_1_phy>;
+			phy-names = "ufs-phy";
+			samsung,sysreg = <&syscon_fsys2 0x714>;
+			status = "disabled";
+		};
 	};
 };
 
-- 
2.36.1

Re: [PATCH 3/4] arm64: dts: exynosautov9: add secondary ufs devices

[Krzysztof Kozlowski]

On 31/05/2022 14:19, Chanho Park wrote:
> Add ufs_1_phy and ufs_1 for secondary ufs hci controller and phy
> device.
> 
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynosautov9.dtsi | 32 ++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynosautov9.dtsi b/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
> index 3e23db8f09d9..c146271af477 100644
> --- a/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
> @@ -380,6 +380,17 @@ ufs_0_phy: ufs0-phy@17e04000 {
>  			status = "disabled";
>  		};
>  
> +		ufs_1_phy: ufs0-phy@17f04000 {

Node name: "phy" (or ufs-phy). The previous node also could be corrected.

> +			compatible = "samsung,exynosautov9-ufs-phy";
> +			reg = <0x17f04000 0xc00>;
> +			reg-names = "phy-pma";
> +			samsung,pmu-syscon = <&pmu_system_controller 0x72c>;
> +			#phy-cells = <0>;
> +			clocks = <&xtcxo>;
> +			clock-names = "ref_clk";
> +			status = "disabled";
> +		};
> +
>  		ufs_0: ufs0@17e00000 {
>  			compatible ="samsung,exynosautov9-ufs";
>  
> @@ -400,6 +411,27 @@ ufs_0: ufs0@17e00000 {
>  			samsung,sysreg = <&syscon_fsys2 0x710>;
>  			status = "disabled";
>  		};
> +
> +		ufs_1: ufs0@17f00000 {
> +			compatible ="samsung,exynosautov9-ufs";
> +
> +			reg = <0x17f00000 0x100>,  /* 0: HCI standard */
> +				<0x17f01100 0x410>,  /* 1: Vendor-specific */
> +				<0x17f80000 0x8000>,  /* 2: UNIPRO */
> +				<0x17de0000 0x2200>;  /* 3: UFS protector */

Align these please with first <> entry.

> +			reg-names = "hci", "vs_hci", "unipro", "ufsp";
> +			interrupts = <GIC_SPI 235 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cmu_fsys2 CLK_GOUT_FSYS2_UFS_EMBD1_ACLK>,
> +				 <&cmu_fsys2 CLK_GOUT_FSYS2_UFS_EMBD1_UNIPRO>;
> +			clock-names = "core_clk", "sclk_unipro_main";
> +			freq-table-hz = <0 0>, <0 0>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&ufs_rst_n_1 &ufs_refclk_out_1>;
> +			phys = <&ufs_1_phy>;
> +			phy-names = "ufs-phy";
> +			samsung,sysreg = <&syscon_fsys2 0x714>;
> +			status = "disabled";
> +		};
>  	};
>  };
>  


Best regards,
Krzysztof

RE: [PATCH 3/4] arm64: dts: exynosautov9: add secondary ufs devices

[Chanho Park]

> > +		ufs_1_phy: ufs0-phy@17f04000 {
> 
> Node name: "phy" (or ufs-phy). The previous node also could be corrected.

phy@17f04000 looks better. I'll change this and make an additional patch to correct previous ufs0 nodes.

> 
> > +			compatible = "samsung,exynosautov9-ufs-phy";
> > +			reg = <0x17f04000 0xc00>;
> > +			reg-names = "phy-pma";
> > +			samsung,pmu-syscon = <&pmu_system_controller 0x72c>;
> > +			#phy-cells = <0>;
> > +			clocks = <&xtcxo>;
> > +			clock-names = "ref_clk";
> > +			status = "disabled";
> > +		};
> > +
> >  		ufs_0: ufs0@17e00000 {
> >  			compatible ="samsung,exynosautov9-ufs";
> >
> > @@ -400,6 +411,27 @@ ufs_0: ufs0@17e00000 {
> >  			samsung,sysreg = <&syscon_fsys2 0x710>;
> >  			status = "disabled";
> >  		};
> > +
> > +		ufs_1: ufs0@17f00000 {
> > +			compatible ="samsung,exynosautov9-ufs";
> > +
> > +			reg = <0x17f00000 0x100>,  /* 0: HCI standard */
> > +				<0x17f01100 0x410>,  /* 1: Vendor-specific */
> > +				<0x17f80000 0x8000>,  /* 2: UNIPRO */
> > +				<0x17de0000 0x2200>;  /* 3: UFS protector */
> 
> Align these please with first <> entry.

I'll apply this on the next patchset and fix the same on ufs0 node as well.

Best Regards,
Chanho Park

[PATCH 4/4] arm64: dts: exynosautov9-sadk: enable secondary ufs devices

[Chanho Park]

Enable ufs_1_phy and ufs_1 devices with ufs_1_fixed_vcc_reg regulator.

Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 .../boot/dts/exynos/exynosautov9-sadk.dts      | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynosautov9-sadk.dts b/arch/arm64/boot/dts/exynos/exynosautov9-sadk.dts
index 17e568853eb6..2b30a7458297 100644
--- a/arch/arm64/boot/dts/exynos/exynosautov9-sadk.dts
+++ b/arch/arm64/boot/dts/exynos/exynosautov9-sadk.dts
@@ -39,6 +39,14 @@ ufs_0_fixed_vcc_reg: regulator-0 {
 		regulator-boot-on;
 		enable-active-high;
 	};
+
+	ufs_1_fixed_vcc_reg: regulator-1 {
+		compatible = "regulator-fixed";
+		regulator-name = "ufs-vcc";
+		gpio = <&gpg2 2 GPIO_ACTIVE_HIGH>;
+		regulator-boot-on;
+		enable-active-high;
+	};
 };
 
 &serial_0 {
@@ -49,12 +57,22 @@ &ufs_0_phy {
 	status = "okay";
 };
 
+&ufs_1_phy {
+	status = "okay";
+};
+
 &ufs_0 {
 	status = "okay";
 	vcc-supply = <&ufs_0_fixed_vcc_reg>;
 	vcc-fixed-regulator;
 };
 
+&ufs_1 {
+	status = "okay";
+	vcc-supply = <&ufs_1_fixed_vcc_reg>;
+	vcc-fixed-regulator;
+};
+
 &usi_0 {
 	status = "okay";
 };
-- 
2.36.1