[PATCH v4 0/8] MediaTek UFS fixes and cleanups - Part 1

[PATCH v4 0/8] MediaTek UFS fixes and cleanups - Part 1

[AngeloGioacchino Del Regno]

Changes in v4:
 - Replaced Stanley Chu with myself in maintainers as his email is gone
 - Fixed commit [5/8]

Changes in v3:
 - Disallowed mt8192 compatible in isolation
 - Added maxItems to clocks

Changes in v2:
 - Rebased over next-20240409 (because of merge issue for patch 1)
 - Added ufs: prefix to patch 1
 - Added forgotten ufs-rx-symbol clock to the binding


This series performs some fixes and cleanups for the MediaTek UFSHCI
controller driver.

In particular, while adding the MT8195 compatible to the mediatek,ufs
binding, I noticed that it was allowing just one clock, completely
ignoring the optional ones, including the crypt-xxx clocks, all of
the optional regulators, and other properties.

Between all the other properties, two are completely useless, as they
are there just to activate features that, on SoCs that don't support
these, won't anyway be activated because of missing clocks or missing
regulators, or missing other properties;
as for the other vendor-specific properties, like ufs-disable-ah8,
ufs-broken-vcc, ufs-pmc-via-fastauto, since the current merge window
is closing, I didn't do extensive research so I've left them in place
but didn't add them to the devicetree binding yet.

The plan is to check those later and eventually give them a removal
treatment, or add them to the bindings in a part two series.

For now, at least, this is already a big improvement.

P.S.: The only SoC having UFSHCI upstream is MT8183, which only has
just one clock, and *nothing else* uses properties, clocks, etc that
were renamed in this cleanup.

Cheers!

AngeloGioacchino Del Regno (8):
  scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-support-va09
    property
  scsi: ufs: ufs-mediatek: Fix property name for crypt boost voltage
  scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt
    property
  scsi: ufs: ufs-mediatek: Avoid underscores in crypt clock names
  dt-bindings: ufs: mediatek,ufs: Document MT8192 compatible with MT8183
  dt-bindings: ufs: mediatek,ufs: Document MT8195 compatible
  dt-bindings: ufs: mediatek,ufs: Document additional clocks
  dt-bindings: ufs: mediatek,ufs: Document optional dvfsrc/va09
    regulators

 .../devicetree/bindings/ufs/mediatek,ufs.yaml | 29 +++++-
 drivers/ufs/host/ufs-mediatek.c               | 91 +++++++++++--------
 2 files changed, 79 insertions(+), 41 deletions(-)

-- 
2.44.0

[PATCH v4 1/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-support-va09 property

[AngeloGioacchino Del Regno]

Remove checking the mediatek,ufs-support-va09 property to decide
whether to try to support the VA09 regulator handling and change
the ufs_mtk_init_va09_pwr_ctrl() function to make it call
devm_regulator_get_optional(): if the regulator is present, then
we set the UFS_MTK_CAP_VA09_PWR_CTRL, effectively enabling the
handling of the VA09 regulator based on that.

Also, make sure to pass the return value of the call to
devm_regulator_get_optional() to the probe function, so that
if it returns a probe deferral, the appropriate action will be
taken.

While at it, remove the error print (disguised as info...) when
the va09 regulator was not found.

Fixes: ac8c2459091c ("scsi: ufs-mediatek: Decouple features from platform bindings")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/ufs/host/ufs-mediatek.c | 34 +++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 0b0c923b1d7b..e4643ac49033 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -622,27 +622,38 @@ static void ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
 	return;
 }
 
-static void ufs_mtk_init_va09_pwr_ctrl(struct ufs_hba *hba)
+static int ufs_mtk_init_va09_pwr_ctrl(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+	int ret;
 
-	host->reg_va09 = regulator_get(hba->dev, "va09");
-	if (IS_ERR(host->reg_va09))
-		dev_info(hba->dev, "failed to get va09");
-	else
-		host->caps |= UFS_MTK_CAP_VA09_PWR_CTRL;
+	host->reg_va09 = devm_regulator_get_optional(hba->dev, "va09");
+	if (IS_ERR(host->reg_va09)) {
+		ret = PTR_ERR(host->reg_va09);
+
+		/* Return an error only if this is a deferral */
+		if (ret == -EPROBE_DEFER)
+			return ret;
+
+		return 0;
+	}
+
+	host->caps |= UFS_MTK_CAP_VA09_PWR_CTRL;
+	return 0;
 }
 
-static void ufs_mtk_init_host_caps(struct ufs_hba *hba)
+static int ufs_mtk_init_host_caps(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
 	struct device_node *np = hba->dev->of_node;
+	int ret;
 
 	if (of_property_read_bool(np, "mediatek,ufs-boost-crypt"))
 		ufs_mtk_init_boost_crypt(hba);
 
-	if (of_property_read_bool(np, "mediatek,ufs-support-va09"))
-		ufs_mtk_init_va09_pwr_ctrl(hba);
+	ret = ufs_mtk_init_va09_pwr_ctrl(hba);
+	if (ret)
+		return ret;
 
 	if (of_property_read_bool(np, "mediatek,ufs-disable-ah8"))
 		host->caps |= UFS_MTK_CAP_DISABLE_AH8;
@@ -663,6 +674,7 @@ static void ufs_mtk_init_host_caps(struct ufs_hba *hba)
 		host->caps |= UFS_MTK_CAP_RTFF_MTCMOS;
 
 	dev_info(hba->dev, "caps: 0x%x", host->caps);
+	return 0;
 }
 
 static void ufs_mtk_scale_perf(struct ufs_hba *hba, bool scale_up)
@@ -985,7 +997,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
 	}
 
 	/* Initialize host capability */
-	ufs_mtk_init_host_caps(hba);
+	err = ufs_mtk_init_host_caps(hba);
+	if (err)
+		goto out;
 
 	ufs_mtk_init_mcq_irq(hba);
 
-- 
2.44.0

[PATCH v4 2/8] scsi: ufs: ufs-mediatek: Fix property name for crypt boost voltage

[AngeloGioacchino Del Regno]

Rename "boost-crypt-vcore-min" to "mediatek,boost-crypt-microvolt":
this is a vendor specific property and needs the "mediatek," prefix,
moreover, this is not defining a minimum voltage per-se;

Even if technically a call to regulator_set_voltage() does indeed
internally set a VMIN for a regulator, the API also supports other
calls to set VMIN-VMAX constraints, so this "vcore-min"->"microvolt"
rename is performed in order to avoid confusion, other than adding
the "microvolt" suffix to it (as this does take microvolts!).

Fixes: 590b0d2372fe ("scsi: ufs-mediatek: Support performance mode for inline encryption engine")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/ufs/host/ufs-mediatek.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index e4643ac49033..688d85909ad6 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -595,9 +595,9 @@ static void ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
 		goto disable_caps;
 	}
 
-	if (of_property_read_u32(dev->of_node, "boost-crypt-vcore-min",
+	if (of_property_read_u32(dev->of_node, "mediatek,boost-crypt-microvolt",
 				 &volt)) {
-		dev_info(dev, "failed to get boost-crypt-vcore-min");
+		dev_info(dev, "failed to get mediatek,boost-crypt-microvolt");
 		goto disable_caps;
 	}
 
-- 
2.44.0

[PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[AngeloGioacchino Del Regno]

There is no need to have a property that activates the inline crypto
boost feature, as this needs many things: a regulator, three clocks,
and the mediatek,boost-crypt-microvolt property to be set.

If any one of these is missing, the feature won't be activated,
hence, it is useless to have yet one more property to enable that.

While at it, also address another two issues:
1. Give back the return value to the caller and make sure to fail
   probing if we get an -EPROBE_DEFER or -ENOMEM; and
2. Free the ufs_mtk_crypt_cfg structure allocated in the crypto
   boost function if said functionality could not be enabled because
   it's not supported, as that'd be only wasted memory.

Last but not least, move the devm_kzalloc() call for ufs_mtk_crypt_cfg
to after getting the dvfsrc-vcore regulator and the boost microvolt
property, as if those fail there's no reason to even allocate that.

Fixes: ac8c2459091c ("scsi: ufs-mediatek: Decouple features from platform bindings")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/ufs/host/ufs-mediatek.c | 55 ++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 688d85909ad6..47f16e6720f4 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -575,51 +575,55 @@ static int ufs_mtk_init_host_clk(struct ufs_hba *hba, const char *name,
 	return ret;
 }
 
-static void ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
+static int ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
 	struct ufs_mtk_crypt_cfg *cfg;
 	struct device *dev = hba->dev;
 	struct regulator *reg;
 	u32 volt;
-
-	host->crypt = devm_kzalloc(dev, sizeof(*(host->crypt)),
-				   GFP_KERNEL);
-	if (!host->crypt)
-		goto disable_caps;
+	int ret;
 
 	reg = devm_regulator_get_optional(dev, "dvfsrc-vcore");
 	if (IS_ERR(reg)) {
-		dev_info(dev, "failed to get dvfsrc-vcore: %ld",
-			 PTR_ERR(reg));
-		goto disable_caps;
+		ret = PTR_ERR(reg);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+
+		return 0;
 	}
 
-	if (of_property_read_u32(dev->of_node, "mediatek,boost-crypt-microvolt",
-				 &volt)) {
+	ret = of_property_read_u32(dev->of_node, "mediatek,boost-crypt-microvolt", &volt);
+	if (ret) {
 		dev_info(dev, "failed to get mediatek,boost-crypt-microvolt");
-		goto disable_caps;
+		return 0;
 	}
 
+	host->crypt = devm_kzalloc(dev, sizeof(*host->crypt), GFP_KERNEL);
+	if (!host->crypt)
+		return -ENOMEM;
+
 	cfg = host->crypt;
-	if (ufs_mtk_init_host_clk(hba, "crypt_mux",
-				  &cfg->clk_crypt_mux))
-		goto disable_caps;
+	ret = ufs_mtk_init_host_clk(hba, "crypt_mux", &cfg->clk_crypt_mux);
+	if (ret)
+		goto out;
 
-	if (ufs_mtk_init_host_clk(hba, "crypt_lp",
-				  &cfg->clk_crypt_lp))
-		goto disable_caps;
+	ret = ufs_mtk_init_host_clk(hba, "crypt_lp", &cfg->clk_crypt_lp);
+	if (ret)
+		goto out;
 
-	if (ufs_mtk_init_host_clk(hba, "crypt_perf",
-				  &cfg->clk_crypt_perf))
-		goto disable_caps;
+	ret = ufs_mtk_init_host_clk(hba, "crypt_perf", &cfg->clk_crypt_perf);
+	if (ret)
+		goto out;
 
 	cfg->reg_vcore = reg;
 	cfg->vcore_volt = volt;
 	host->caps |= UFS_MTK_CAP_BOOST_CRYPT_ENGINE;
 
-disable_caps:
-	return;
+out:
+	if (ret)
+		devm_kfree(dev, host->crypt);
+	return 0;
 }
 
 static int ufs_mtk_init_va09_pwr_ctrl(struct ufs_hba *hba)
@@ -648,8 +652,9 @@ static int ufs_mtk_init_host_caps(struct ufs_hba *hba)
 	struct device_node *np = hba->dev->of_node;
 	int ret;
 
-	if (of_property_read_bool(np, "mediatek,ufs-boost-crypt"))
-		ufs_mtk_init_boost_crypt(hba);
+	ret = ufs_mtk_init_boost_crypt(hba);
+	if (ret)
+		return ret;
 
 	ret = ufs_mtk_init_va09_pwr_ctrl(hba);
 	if (ret)
-- 
2.44.0

[PATCH v4 4/8] scsi: ufs: ufs-mediatek: Avoid underscores in crypt clock names

[AngeloGioacchino Del Regno]

Change all of crypt_{mux,lp,perf} clock names to crypt-{mux,lp-perf}:
retaining compatibility with the old names is ignored as there is no
user of this driver declaring any of those clocks, and the binding
also doesn't allow these ones at all.

Fixes: 590b0d2372fe ("scsi: ufs-mediatek: Support performance mode for inline encryption engine")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/ufs/host/ufs-mediatek.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 47f16e6720f4..5db6d27f75af 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -604,15 +604,15 @@ static int ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
 		return -ENOMEM;
 
 	cfg = host->crypt;
-	ret = ufs_mtk_init_host_clk(hba, "crypt_mux", &cfg->clk_crypt_mux);
+	ret = ufs_mtk_init_host_clk(hba, "crypt-mux", &cfg->clk_crypt_mux);
 	if (ret)
 		goto out;
 
-	ret = ufs_mtk_init_host_clk(hba, "crypt_lp", &cfg->clk_crypt_lp);
+	ret = ufs_mtk_init_host_clk(hba, "crypt-lp", &cfg->clk_crypt_lp);
 	if (ret)
 		goto out;
 
-	ret = ufs_mtk_init_host_clk(hba, "crypt_perf", &cfg->clk_crypt_perf);
+	ret = ufs_mtk_init_host_clk(hba, "crypt-perf", &cfg->clk_crypt_perf);
 	if (ret)
 		goto out;
 
-- 
2.44.0

[PATCH v4 5/8] dt-bindings: ufs: mediatek,ufs: Document MT8192 compatible with MT8183

[AngeloGioacchino Del Regno]

The MT8192 UFS controller is compatible with the MT8183 one:
document this by allowing to assign both compatible strings
"mediatek,mt8192-ufshci", "mediatek,mt8183-ufshci" to the UFSHCI node.

Moreover, since no MT8192 devicetree ever declared any UFSHCI node,
disallow specifying only the MT8192 compatible.

In preparation for adding MT8195 to the mix, the MT8192 compatible
was added as enum instead of const.

Also, while at it, replace Stanley Chu with me in the maintainers
field, as he is unreachable and his email isn't active anymore.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../devicetree/bindings/ufs/mediatek,ufs.yaml         | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
index 32fd535a514a..f14887ea6fdc 100644
--- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
@@ -7,16 +7,19 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: Mediatek Universal Flash Storage (UFS) Controller
 
 maintainers:
-  - Stanley Chu <stanley.chu@mediatek.com>
+  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
 
 allOf:
   - $ref: ufs-common.yaml
 
 properties:
   compatible:
-    enum:
-      - mediatek,mt8183-ufshci
-      - mediatek,mt8192-ufshci
+    oneOf:
+      - const: mediatek,mt8183-ufshci
+      - items:
+          - enum:
+              - mediatek,mt8192-ufshci
+          - const: mediatek,mt8183-ufshci
 
   clocks:
     maxItems: 1
-- 
2.44.0

[PATCH v4 6/8] dt-bindings: ufs: mediatek,ufs: Document MT8195 compatible

[AngeloGioacchino Del Regno]

Add the new mediatek,mt8195-ufshci string.
This SoC's UFSHCI controller is compatible with MT8183.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
index f14887ea6fdc..5728e750761f 100644
--- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
@@ -19,6 +19,7 @@ properties:
       - items:
           - enum:
               - mediatek,mt8192-ufshci
+              - mediatek,mt8195-ufshci
           - const: mediatek,mt8183-ufshci
 
   clocks:
-- 
2.44.0

[PATCH v4 7/8] dt-bindings: ufs: mediatek,ufs: Document additional clocks

[AngeloGioacchino Del Regno]

Add additional clocks, used on all MediaTek SoCs' UFSHCI controllers:
some of these clocks are optional and used only for scaling purposes
to save power, or to improve performance in the case of the crypt
clocks.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../devicetree/bindings/ufs/mediatek,ufs.yaml     | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
index 5728e750761f..1df8779ee902 100644
--- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
@@ -23,11 +23,24 @@ properties:
           - const: mediatek,mt8183-ufshci
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 12
 
   clock-names:
+    minItems: 1
     items:
       - const: ufs
+      - const: ufs-aes
+      - const: ufs-tick
+      - const: unipro-sys
+      - const: unipro-tick
+      - const: ufs-sap
+      - const: ufs-tx-symbol
+      - const: ufs-rx-symbol
+      - const: ufs-mem
+      - const: crypt-mux
+      - const: crypt-lp
+      - const: crypt-perf
 
   phys:
     maxItems: 1
-- 
2.44.0

[PATCH v4 8/8] dt-bindings: ufs: mediatek,ufs: Document optional dvfsrc/va09 regulators

[AngeloGioacchino Del Regno]

Document the optional dvfsrc-vcore and va09 regulators used for,
respectively, crypt boost and internal MPHY power management in
when powering on/off the (external) MediaTek UFS PHY.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
index 1df8779ee902..b74a2464196d 100644
--- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
@@ -48,6 +48,8 @@ properties:
   reg:
     maxItems: 1
 
+  dvfsrc-vcore-supply: true
+  va09-supply: true
   vcc-supply: true
 
 required:
-- 
2.44.0

Re: [PATCH v4 7/8] dt-bindings: ufs: mediatek,ufs: Document additional clocks

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]

On Mon, Apr 15, 2024 at 01:00:11PM +0200, AngeloGioacchino Del Regno wrote:
> Add additional clocks, used on all MediaTek SoCs' UFSHCI controllers:
> some of these clocks are optional and used only for scaling purposes
> to save power, or to improve performance in the case of the crypt
> clocks.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

> ---
>  .../devicetree/bindings/ufs/mediatek,ufs.yaml     | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> index 5728e750761f..1df8779ee902 100644
> --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> @@ -23,11 +23,24 @@ properties:
>            - const: mediatek,mt8183-ufshci
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 12
>  
>    clock-names:
> +    minItems: 1
>      items:
>        - const: ufs
> +      - const: ufs-aes
> +      - const: ufs-tick
> +      - const: unipro-sys
> +      - const: unipro-tick
> +      - const: ufs-sap
> +      - const: ufs-tx-symbol
> +      - const: ufs-rx-symbol
> +      - const: ufs-mem
> +      - const: crypt-mux
> +      - const: crypt-lp
> +      - const: crypt-perf
>  
>    phys:
>      maxItems: 1
> -- 
> 2.44.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 5/8] dt-bindings: ufs: mediatek,ufs: Document MT8192 compatible with MT8183

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

On Mon, Apr 15, 2024 at 01:00:09PM +0200, AngeloGioacchino Del Regno wrote:
> The MT8192 UFS controller is compatible with the MT8183 one:
> document this by allowing to assign both compatible strings
> "mediatek,mt8192-ufshci", "mediatek,mt8183-ufshci" to the UFSHCI node.
> 
> Moreover, since no MT8192 devicetree ever declared any UFSHCI node,
> disallow specifying only the MT8192 compatible.
> 
> In preparation for adding MT8195 to the mix, the MT8192 compatible
> was added as enum instead of const.
> 
> Also, while at it, replace Stanley Chu with me in the maintainers
> field, as he is unreachable and his email isn't active anymore.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[Peter Wang (王信友)]

On Mon, 2024-04-15 at 13:00 +0200, AngeloGioacchino Del Regno wrote:
> There is no need to have a property that activates the inline crypto
> boost feature, as this needs many things: a regulator, three clocks,
> and the mediatek,boost-crypt-microvolt property to be set.
> 
> If any one of these is missing, the feature won't be activated,
> hence, it is useless to have yet one more property to enable that.
> 
> While at it, also address another two issues:
> 1. Give back the return value to the caller and make sure to fail
>    probing if we get an -EPROBE_DEFER or -ENOMEM; and
> 2. Free the ufs_mtk_crypt_cfg structure allocated in the crypto
>    boost function if said functionality could not be enabled because
>    it's not supported, as that'd be only wasted memory.
> 
> Last but not least, move the devm_kzalloc() call for
> ufs_mtk_crypt_cfg
> to after getting the dvfsrc-vcore regulator and the boost microvolt
> property, as if those fail there's no reason to even allocate that.
> 
> Fixes: ac8c2459091c ("scsi: ufs-mediatek: Decouple features from
> platform bindings")
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/ufs/host/ufs-mediatek.c | 55 ++++++++++++++++++-------------
> --
>  1 file changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
> mediatek.c
> index 688d85909ad6..47f16e6720f4 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -575,51 +575,55 @@ static int ufs_mtk_init_host_clk(struct ufs_hba
> *hba, const char *name,
>  	return ret;
>  }
>  
> -static void ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
> +static int ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
>  {
>  	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
>  	struct ufs_mtk_crypt_cfg *cfg;
>  	struct device *dev = hba->dev;
>  	struct regulator *reg;
>  	u32 volt;
> -
> -	host->crypt = devm_kzalloc(dev, sizeof(*(host->crypt)),
> -				   GFP_KERNEL);
> -	if (!host->crypt)
> -		goto disable_caps;
> +	int ret;
>  
>  	reg = devm_regulator_get_optional(dev, "dvfsrc-vcore");
>  	if (IS_ERR(reg)) {
> -		dev_info(dev, "failed to get dvfsrc-vcore: %ld",
> -			 PTR_ERR(reg));
> -		goto disable_caps;
> +		ret = PTR_ERR(reg);
> +		if (ret == -EPROBE_DEFER)
> +			return ret;
> +
> +		return 0;
>  	}
>  
> -	if (of_property_read_u32(dev->of_node, "mediatek,boost-crypt-
> microvolt",
> -				 &volt)) {
> +	ret = of_property_read_u32(dev->of_node, "mediatek,boost-crypt-
> microvolt", &volt);
> +	if (ret) {
>  		dev_info(dev, "failed to get mediatek,boost-crypt-
> microvolt");
> -		goto disable_caps;
> +		return 0;
>  	}
>  
> +	host->crypt = devm_kzalloc(dev, sizeof(*host->crypt),
> GFP_KERNEL);
> +	if (!host->crypt)
> +		return -ENOMEM;
> +
> 

Hi Angelo,

If retrun -ENOMEN, host will init fail.
But previous is skip boost crypt feature only.
It change the driver behavior.




>  	cfg = host->crypt;
> -	if (ufs_mtk_init_host_clk(hba, "crypt_mux",
> -				  &cfg->clk_crypt_mux))
> -		goto disable_caps;
> +	ret = ufs_mtk_init_host_clk(hba, "crypt_mux", &cfg-
> >clk_crypt_mux);
> +	if (ret)
> +		goto out;
>  
> -	if (ufs_mtk_init_host_clk(hba, "crypt_lp",
> -				  &cfg->clk_crypt_lp))
> -		goto disable_caps;
> +	ret = ufs_mtk_init_host_clk(hba, "crypt_lp", &cfg-
> >clk_crypt_lp);
> +	if (ret)
> +		goto out;
>  
> -	if (ufs_mtk_init_host_clk(hba, "crypt_perf",
> -				  &cfg->clk_crypt_perf))
> -		goto disable_caps;
> +	ret = ufs_mtk_init_host_clk(hba, "crypt_perf", &cfg-
> >clk_crypt_perf);
> +	if (ret)
> +		goto out;
>  
>  	cfg->reg_vcore = reg;
>  	cfg->vcore_volt = volt;
>  	host->caps |= UFS_MTK_CAP_BOOST_CRYPT_ENGINE;
>  
> -disable_caps:
> -	return;
> +out:
> +	if (ret)
> +		devm_kfree(dev, host->crypt);
> +	return 0;
>  }
>  
>  static int ufs_mtk_init_va09_pwr_ctrl(struct ufs_hba *hba)
> @@ -648,8 +652,9 @@ static int ufs_mtk_init_host_caps(struct ufs_hba
> *hba)
>  	struct device_node *np = hba->dev->of_node;
>  	int ret;
>  
> -	if (of_property_read_bool(np, "mediatek,ufs-boost-crypt"))
> -		ufs_mtk_init_boost_crypt(hba);
> +	ret = ufs_mtk_init_boost_crypt(hba);
> +	if (ret)
> +		return ret;
>  

Most ufs-mediatek platform dosen't need "mediatek,ufs-boost-crypt"
Remove this property will casue most platform try error and add init
latency.

Thanks.
Peter



>  	ret = ufs_mtk_init_va09_pwr_ctrl(hba);
>  	if (ret)

Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[AngeloGioacchino Del Regno]

Il 16/04/24 09:03, Peter Wang (王信友) ha scritto:
> On Mon, 2024-04-15 at 13:00 +0200, AngeloGioacchino Del Regno wrote:
>> There is no need to have a property that activates the inline crypto
>> boost feature, as this needs many things: a regulator, three clocks,
>> and the mediatek,boost-crypt-microvolt property to be set.
>>
>> If any one of these is missing, the feature won't be activated,
>> hence, it is useless to have yet one more property to enable that.
>>
>> While at it, also address another two issues:
>> 1. Give back the return value to the caller and make sure to fail
>>     probing if we get an -EPROBE_DEFER or -ENOMEM; and
>> 2. Free the ufs_mtk_crypt_cfg structure allocated in the crypto
>>     boost function if said functionality could not be enabled because
>>     it's not supported, as that'd be only wasted memory.
>>
>> Last but not least, move the devm_kzalloc() call for
>> ufs_mtk_crypt_cfg
>> to after getting the dvfsrc-vcore regulator and the boost microvolt
>> property, as if those fail there's no reason to even allocate that.
>>
>> Fixes: ac8c2459091c ("scsi: ufs-mediatek: Decouple features from
>> platform bindings")
>> Signed-off-by: AngeloGioacchino Del Regno <
>> angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/ufs/host/ufs-mediatek.c | 55 ++++++++++++++++++-------------
>> --
>>   1 file changed, 30 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
>> mediatek.c
>> index 688d85909ad6..47f16e6720f4 100644
>> --- a/drivers/ufs/host/ufs-mediatek.c
>> +++ b/drivers/ufs/host/ufs-mediatek.c
>> @@ -575,51 +575,55 @@ static int ufs_mtk_init_host_clk(struct ufs_hba
>> *hba, const char *name,
>>   	return ret;
>>   }
>>   
>> -static void ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
>> +static int ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
>>   {
>>   	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
>>   	struct ufs_mtk_crypt_cfg *cfg;
>>   	struct device *dev = hba->dev;
>>   	struct regulator *reg;
>>   	u32 volt;
>> -
>> -	host->crypt = devm_kzalloc(dev, sizeof(*(host->crypt)),
>> -				   GFP_KERNEL);
>> -	if (!host->crypt)
>> -		goto disable_caps;
>> +	int ret;
>>   
>>   	reg = devm_regulator_get_optional(dev, "dvfsrc-vcore");
>>   	if (IS_ERR(reg)) {
>> -		dev_info(dev, "failed to get dvfsrc-vcore: %ld",
>> -			 PTR_ERR(reg));
>> -		goto disable_caps;
>> +		ret = PTR_ERR(reg);
>> +		if (ret == -EPROBE_DEFER)
>> +			return ret;
>> +
>> +		return 0;
>>   	}
>>   
>> -	if (of_property_read_u32(dev->of_node, "mediatek,boost-crypt-
>> microvolt",
>> -				 &volt)) {
>> +	ret = of_property_read_u32(dev->of_node, "mediatek,boost-crypt-
>> microvolt", &volt);
>> +	if (ret) {
>>   		dev_info(dev, "failed to get mediatek,boost-crypt-
>> microvolt");
>> -		goto disable_caps;
>> +		return 0;
>>   	}
>>   
>> +	host->crypt = devm_kzalloc(dev, sizeof(*host->crypt),
>> GFP_KERNEL);
>> +	if (!host->crypt)
>> +		return -ENOMEM;
>> +
>>
> 
> Hi Angelo,
> 
> If retrun -ENOMEN, host will init fail.
> But previous is skip boost crypt feature only.
> It change the driver behavior.
> 

This is fully intentional: if a platform supports boost-crypt, this means that the
feature *MUST* be enabled, and must *not* be disabled if a memory allocation fails,
as that is relative to available pages at boot, and not to SoC feature support.

Keep in mind that the allocation was moved to *after* checking if such platform
does indeed support the boost-crypt feature, and it is critical to FAIL probing
if there was no memory to allocate the host->crypt structure.

> 
> 
> 
>>   	cfg = host->crypt;
>> -	if (ufs_mtk_init_host_clk(hba, "crypt_mux",
>> -				  &cfg->clk_crypt_mux))
>> -		goto disable_caps;
>> +	ret = ufs_mtk_init_host_clk(hba, "crypt_mux", &cfg-
>>> clk_crypt_mux);
>> +	if (ret)
>> +		goto out;
>>   
>> -	if (ufs_mtk_init_host_clk(hba, "crypt_lp",
>> -				  &cfg->clk_crypt_lp))
>> -		goto disable_caps;
>> +	ret = ufs_mtk_init_host_clk(hba, "crypt_lp", &cfg-
>>> clk_crypt_lp);
>> +	if (ret)
>> +		goto out;
>>   
>> -	if (ufs_mtk_init_host_clk(hba, "crypt_perf",
>> -				  &cfg->clk_crypt_perf))
>> -		goto disable_caps;
>> +	ret = ufs_mtk_init_host_clk(hba, "crypt_perf", &cfg-
>>> clk_crypt_perf);
>> +	if (ret)
>> +		goto out;
>>   
>>   	cfg->reg_vcore = reg;
>>   	cfg->vcore_volt = volt;
>>   	host->caps |= UFS_MTK_CAP_BOOST_CRYPT_ENGINE;
>>   
>> -disable_caps:
>> -	return;
>> +out:
>> +	if (ret)
>> +		devm_kfree(dev, host->crypt);
>> +	return 0;
>>   }
>>   
>>   static int ufs_mtk_init_va09_pwr_ctrl(struct ufs_hba *hba)
>> @@ -648,8 +652,9 @@ static int ufs_mtk_init_host_caps(struct ufs_hba
>> *hba)
>>   	struct device_node *np = hba->dev->of_node;
>>   	int ret;
>>   
>> -	if (of_property_read_bool(np, "mediatek,ufs-boost-crypt"))
>> -		ufs_mtk_init_boost_crypt(hba);
>> +	ret = ufs_mtk_init_boost_crypt(hba);
>> +	if (ret)
>> +		return ret;
>>   
> 
> Most ufs-mediatek platform dosen't need "mediatek,ufs-boost-crypt"
> Remove this property will casue most platform try error and add init
> latency.
> 

Yes this causes -> less than half of a millisecond <- of additional boot time
if the dvfsrc-supply is present but boost-microvolt is not.

I really don't see the problem with that :-)

Regards,
Angelo

> Thanks.
> Peter
> 
> 
> 
>>   	ret = ufs_mtk_init_va09_pwr_ctrl(hba);
>>   	if (ret)

Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[Peter Wang (王信友)]

> Yes this causes -> less than half of a millisecond <- of additional
> boot time
> if the dvfsrc-supply is present but boost-microvolt is not.
> 
> I really don't see the problem with that :-)
> 

Adding a little bit of boot time to one smartphone might not be a
problem, but when you consider a billion smartphones each adding a
little bit, the cumulative effect becomes significant. The power
consumption of these accumulated times will continue to increase,
contributing to the Earth's carbon emissions. Moreover, removing the
master switch for this feature doesn't seem to have any benefits other
than not having to set it in the DTS. Similarly, the master switch for
VA09 seems to have more disadvantage.


> Regards,
> Angelo
> 
> > Thanks.
> > Peter
> > 
> > 
> > 
> > >   	ret = ufs_mtk_init_va09_pwr_ctrl(hba);
> > >   	if (ret)
> 
> 

Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[AngeloGioacchino Del Regno]

Il 16/04/24 12:31, Peter Wang (王信友) ha scritto:
> 
>> Yes this causes -> less than half of a millisecond <- of additional
>> boot time
>> if the dvfsrc-supply is present but boost-microvolt is not.
>>
>> I really don't see the problem with that :-)
>>
> 
> Adding a little bit of boot time to one smartphone might not be a
> problem, but when you consider a billion smartphones each adding a
> little bit, the cumulative effect becomes significant. The power
> consumption of these accumulated times will continue to increase,
> contributing to the Earth's carbon emissions. Moreover, removing the
> master switch for this feature doesn't seem to have any benefits other
> than not having to set it in the DTS. Similarly, the master switch for
> VA09 seems to have more disadvantage.
> 

Sorry, but I still don't see how a few *microseconds* more of boot time can
be significant, even related to power consumption during boot.

If that was a few milliseconds, then I'd agree with you, but that's not the case.

Removing the master switch has a benefit: you *lose* a few microseconds of boot
time (so, boots in *few microseconds LESS*) on platforms that would have this set
in devicetree, as this property is redundant with the other activation checks
for those features.

So, there you go: if the majority of MediaTek platforms are already using this
crypt boost feature, then this commit reduces carbon emissions, as those would
boot in a few less microseconds.

Regards,
Angelo

> 
>> Regards,
>> Angelo
>>
>>> Thanks.
>>> Peter
>>>
>>>
>>>
>>>>    	ret = ufs_mtk_init_va09_pwr_ctrl(hba);
>>>>    	if (ret)
>>
>>

Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[Peter Wang (王信友)]

On Tue, 2024-04-16 at 12:38 +0200, AngeloGioacchino Del Regno wrote:
> Il 16/04/24 12:31, Peter Wang (王信友) ha scritto:
> > 
> > > Yes this causes -> less than half of a millisecond <- of
> > > additional
> > > boot time
> > > if the dvfsrc-supply is present but boost-microvolt is not.
> > > 
> > > I really don't see the problem with that :-)
> > > 
> > 
> > Adding a little bit of boot time to one smartphone might not be a
> > problem, but when you consider a billion smartphones each adding a
> > little bit, the cumulative effect becomes significant. The power
> > consumption of these accumulated times will continue to increase,
> > contributing to the Earth's carbon emissions. Moreover, removing
> > the
> > master switch for this feature doesn't seem to have any benefits
> > other
> > than not having to set it in the DTS. Similarly, the master switch
> > for
> > VA09 seems to have more disadvantage.
> > 
> 
> Sorry, but I still don't see how a few *microseconds* more of boot
> time can
> be significant, even related to power consumption during boot.
> 
> If that was a few milliseconds, then I'd agree with you, but that's
> not the case.
> 
> Removing the master switch has a benefit: you *lose* a few
> microseconds of boot
> time (so, boots in *few microseconds LESS*) on platforms that would
> have this set
> in devicetree, as this property is redundant with the other
> activation checks
> for those features.
> 
> So, there you go: if the majority of MediaTek platforms are already
> using this
> crypt boost feature, then this commit reduces carbon emissions, as
> those would
> boot in a few less microseconds.
> 

But the majority platfomrs dosen't need this feature.
This feature is only for legacy chip which at least 4 years ago.

Thanks
Peter


> Regards,
> Angelo
> 
> > 
> > > Regards,
> > > Angelo
> > > 
> > > > Thanks.
> > > > Peter
> > > > 
> > > > 
> > > > 
> > > > >    	ret = ufs_mtk_init_va09_pwr_ctrl(hba);
> > > > >    	if (ret)
> > > 
> > > 
> 
> 

Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[AngeloGioacchino Del Regno]

Il 16/04/24 15:05, Peter Wang (王信友) ha scritto:
> On Tue, 2024-04-16 at 12:38 +0200, AngeloGioacchino Del Regno wrote:
>> Il 16/04/24 12:31, Peter Wang (王信友) ha scritto:
>>>
>>>> Yes this causes -> less than half of a millisecond <- of
>>>> additional
>>>> boot time
>>>> if the dvfsrc-supply is present but boost-microvolt is not.
>>>>
>>>> I really don't see the problem with that :-)
>>>>
>>>
>>> Adding a little bit of boot time to one smartphone might not be a
>>> problem, but when you consider a billion smartphones each adding a
>>> little bit, the cumulative effect becomes significant. The power
>>> consumption of these accumulated times will continue to increase,
>>> contributing to the Earth's carbon emissions. Moreover, removing
>>> the
>>> master switch for this feature doesn't seem to have any benefits
>>> other
>>> than not having to set it in the DTS. Similarly, the master switch
>>> for
>>> VA09 seems to have more disadvantage.
>>>
>>
>> Sorry, but I still don't see how a few *microseconds* more of boot
>> time can
>> be significant, even related to power consumption during boot.
>>
>> If that was a few milliseconds, then I'd agree with you, but that's
>> not the case.
>>
>> Removing the master switch has a benefit: you *lose* a few
>> microseconds of boot
>> time (so, boots in *few microseconds LESS*) on platforms that would
>> have this set
>> in devicetree, as this property is redundant with the other
>> activation checks
>> for those features.
>>
>> So, there you go: if the majority of MediaTek platforms are already
>> using this
>> crypt boost feature, then this commit reduces carbon emissions, as
>> those would
>> boot in a few less microseconds.
>>
> 
> But the majority platfomrs dosen't need this feature.
> This feature is only for legacy chip which at least 4 years ago.
> 

Upstream supports platforms that do and don't need this feature, and having
redundant device tree properties performing the same checks is not just
suboptimal but plain wrong.

Adding to this, devicetree describes the hardware - and there is no physical
hardware switch that needs this redundant property, this means that the
property that is getting removed in this commit (and the va09 one in another
commit of this series) is a *software switch*, not HW.

Keep in mind, also, that this feature (and again, the va09 one as well) has
a specific requirement to be supported - and this is what the code does even
without the software switch to add it.

In case there's any need to disallow such feature from a specific SoC, the DT
bindings can be modified such that a specific compatible string would disallow
adding the required regulator and/or boost-microvolt properties.

Besides, I want to remind you that there is no reason to drop support, or have
them unreliably working, or use hacks, for SoCs that are "old" - especially
when this is a driver that works on both old and new ones.

Regards,
Angelo

Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[Peter Wang (王信友)]

On Wed, 2024-04-17 at 10:14 +0200, AngeloGioacchino Del Regno wrote:
> 
> 
> Upstream supports platforms that do and don't need this feature, and
> having
> redundant device tree properties performing the same checks is not
> just
> suboptimal but plain wrong.
> 
> Adding to this, devicetree describes the hardware - and there is no
> physical
> hardware switch that needs this redundant property, this means that
> the
> property that is getting removed in this commit (and the va09 one in
> another
> commit of this series) is a *software switch*, not HW.
> 
> Keep in mind, also, that this feature (and again, the va09 one as
> well) has
> a specific requirement to be supported - and this is what the code
> does even
> without the software switch to add it.
> 
> In case there's any need to disallow such feature from a specific
> SoC, the DT
> bindings can be modified such that a specific compatible string would
> disallow
> adding the required regulator and/or boost-microvolt properties.
> 
> Besides, I want to remind you that there is no reason to drop
> support, or have
> them unreliably working, or use hacks, for SoCs that are "old" -
> especially
> when this is a driver that works on both old and new ones.
> 
> Regards,
> Angelo

Hi Angelo,

These two property(boost and va09) is not software switch, they
are hardware switch. Because if platform support crypto boost, we set
boost property in device tree. And if platform support ufs va09, we set
va09 proberty in device tree. These property are main hardware switch.

We don't use sub switch like voltage or clock setting becasue it is
not intiutive. Especially when va09 is not used by ufs (No va09
property but have va09 voltage), The behavior should be wrong if ufs
control va09 which used by other module.

Thanks.
Peter

Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

[AngeloGioacchino Del Regno]

Il 17/04/24 11:29, Peter Wang (王信友) ha scritto:
> On Wed, 2024-04-17 at 10:14 +0200, AngeloGioacchino Del Regno wrote:
>>
>>
>> Upstream supports platforms that do and don't need this feature, and
>> having
>> redundant device tree properties performing the same checks is not
>> just
>> suboptimal but plain wrong.
>>
>> Adding to this, devicetree describes the hardware - and there is no
>> physical
>> hardware switch that needs this redundant property, this means that
>> the
>> property that is getting removed in this commit (and the va09 one in
>> another
>> commit of this series) is a *software switch*, not HW.
>>
>> Keep in mind, also, that this feature (and again, the va09 one as
>> well) has
>> a specific requirement to be supported - and this is what the code
>> does even
>> without the software switch to add it.
>>
>> In case there's any need to disallow such feature from a specific
>> SoC, the DT
>> bindings can be modified such that a specific compatible string would
>> disallow
>> adding the required regulator and/or boost-microvolt properties.
>>
>> Besides, I want to remind you that there is no reason to drop
>> support, or have
>> them unreliably working, or use hacks, for SoCs that are "old" -
>> especially
>> when this is a driver that works on both old and new ones.
>>
>> Regards,
>> Angelo
> 
> Hi Angelo,
> 
> These two property(boost and va09) is not software switch, they
> are hardware switch. Because if platform support crypto boost, we set
> boost property in device tree. And if platform support ufs va09, we set
> va09 proberty in device tree. These property are main hardware switch.

I disagree. If a platform supports crypto boost, it will have the dvfsrc
regulator and the supported voltage for the boost; if a platform supports
ufs va09, it will have the va09 regulator assigned in the ufshci devicetree
node.

> 
> We don't use sub switch like voltage or clock setting becasue it is
> not intiutive. Especially when va09 is not used by ufs (No va09
> property but have va09 voltage), The behavior should be wrong if ufs
> control va09 which used by other module.
> 

As I said, devicetree describes hardware - and this strategy being intuitive
or not boils down to personal opinions and nothing else.
Aside personal opinions, again, properties not describing hardware are wrong.

And again, if VA09 shall not be controlled by the UFSHCI driver on a specific
platform, then the regulator shall not be assigned to the UFSHCI node on that
specific platform.

Regards,
Angelo