[PATCH v1 0/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

[PATCH v1 0/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

[Muhammad Husaini Zulkifli]

Hi,

This patch series adds Ultra High Speed(UHS-1) Bus Speed Mode Support for Keem Bay SoC SD Card.
Summary of each patches as per below:

Patch 1: Use of_device_get_match_data() helper to get the match-data.
Patch 2: Convert to use np pointer instead of using pdev->dev.of_node.
Patch 3: Add struct device *dev in probe func(), so that dev pointer can be widely use in probe to make code more readable.
Patch 4: Change from dev_err to dev_err_probe() to avoid spamming logs when probe is deferred.
Patch 5: Export function to be use by device driver to configure i/o voltage rail output which communicate with Trusted Firmware.
Patch 6: Update phy and regulator supply for Keem Bay SoC.
Patch 7: Add DT Binding for Keem Bay SoC SD Regulator.
Patch 8: Add SD Regulator driver to support Keem Bay SoC. This is to model using standard regulator abstraction during voltage operation
as for Keem Bay SoC, i/o voltage rail need to be configure by setting specific bit in the AON_CFG1 Register.
AON_CFG1 Register is a secure register. Direct access to AON_CFG1 register will cause firewall violation in secure system.
Patch 9: Add Ultra High Speed (UHS-1) Support for Keem Bay SOC. For Keem Bay hardware, two regulators are been used to change the I/O bus line voltage which are "vqmmc-supply" and "sdvrail-supply".

All of these patches was tested with Keem Bay evaluation module board.

Kindly help to review this patch set.

Muhammad Husaini Zulkifli (9):
  mmc: sdhci-of-arasan: use of_device_get_match_data()
  mmc: sdhci-of-arasan: Convert to use np instead of pdev->dev.of_node
  mmc: sdhci-of-arasan: Add structure device pointer in probe function
  mmc: sdhci-of-arasan: Use dev_err_probe() to avoid spamming logs
  firmware: keembay: Add support for Trusted Firmware Service call
  dt-bindings: mmc: Update phy and regulator supply for Keem Bay SOC
  dt-bindings: regulator: keembay: Add DT binding documentation
  regulator: keembay: Add regulator for Keem Bay SoC
  mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

 .../devicetree/bindings/mmc/arasan,sdhci.yaml |   7 +-
 .../bindings/regulator/keembay-regulator.yaml |  36 ++
 drivers/mmc/host/sdhci-of-arasan.c            | 313 ++++++++++++++++--
 drivers/regulator/Kconfig                     |  10 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/keembay-sd-regulator.c      | 112 +++++++
 include/linux/firmware/intel/keembay.h        |  82 +++++
 7 files changed, 532 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/keembay-regulator.yaml
 create mode 100644 drivers/regulator/keembay-sd-regulator.c
 create mode 100644 include/linux/firmware/intel/keembay.h

--
2.17.1

[PATCH v1 1/9] mmc: sdhci-of-arasan: use of_device_get_match_data()

[Muhammad Husaini Zulkifli]

Use of_device_get_match_data() helper to get the match-data.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 4f3774bcda94..07479019caf9 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1508,7 +1508,6 @@ static int sdhci_arasan_add_host(struct sdhci_arasan_data *sdhci_arasan)
 static int sdhci_arasan_probe(struct platform_device *pdev)
 {
 	int ret;
-	const struct of_device_id *match;
 	struct device_node *node;
 	struct clk *clk_xin;
 	struct sdhci_host *host;
@@ -1517,8 +1516,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	const struct sdhci_arasan_of_data *data;
 
-	match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node);
-	data = match->data;
+	data = of_device_get_match_data(&pdev->dev);
 	host = sdhci_pltfm_init(pdev, data->pdata, sizeof(*sdhci_arasan));
 
 	if (IS_ERR(host))
-- 
2.17.1

[PATCH v1 2/9] mmc: sdhci-of-arasan: Convert to use np instead of pdev->dev.of_node

[Muhammad Husaini Zulkifli]

Use np pointer to simplify code and improve readability.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 07479019caf9..ecaea4b45367 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1529,7 +1529,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	sdhci_arasan->soc_ctl_map = data->soc_ctl_map;
 	sdhci_arasan->clk_ops = data->clk_ops;
 
-	node = of_parse_phandle(pdev->dev.of_node, "arasan,soc-ctl-syscon", 0);
+	node = of_parse_phandle(np, "arasan,soc-ctl-syscon", 0);
 	if (node) {
 		sdhci_arasan->soc_ctl_base = syscon_node_to_regmap(node);
 		of_node_put(node);
@@ -1578,8 +1578,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 
 	pltfm_host->clk = clk_xin;
 
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "rockchip,rk3399-sdhci-5.1"))
+	if (of_device_is_compatible(np, "rockchip,rk3399-sdhci-5.1"))
 		sdhci_arasan_update_clockmultiplier(host, 0x0);
 
 	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-emmc") ||
@@ -1612,8 +1611,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	}
 
 	sdhci_arasan->phy = ERR_PTR(-ENODEV);
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "arasan,sdhci-5.1")) {
+	if (of_device_is_compatible(np, "arasan,sdhci-5.1")) {
 		sdhci_arasan->phy = devm_phy_get(&pdev->dev,
 						 "phy_arasan");
 		if (IS_ERR(sdhci_arasan->phy)) {
-- 
2.17.1

[PATCH v1 3/9] mmc: sdhci-of-arasan: Add structure device pointer in probe function

[Muhammad Husaini Zulkifli]

Add struct device *dev in probe func() so that it can widely use in
probe to make code more readable.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 34 +++++++++++++++---------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index ecaea4b45367..4e6ee9e69a1b 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1512,11 +1512,12 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	struct clk *clk_xin;
 	struct sdhci_host *host;
 	struct sdhci_pltfm_host *pltfm_host;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct sdhci_arasan_data *sdhci_arasan;
-	struct device_node *np = pdev->dev.of_node;
 	const struct sdhci_arasan_of_data *data;
 
-	data = of_device_get_match_data(&pdev->dev);
+	data = of_device_get_match_data(dev);
 	host = sdhci_pltfm_init(pdev, data->pdata, sizeof(*sdhci_arasan));
 
 	if (IS_ERR(host))
@@ -1535,36 +1536,36 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 		of_node_put(node);
 
 		if (IS_ERR(sdhci_arasan->soc_ctl_base)) {
-			ret = dev_err_probe(&pdev->dev,
+			ret = dev_err_probe(dev,
 					    PTR_ERR(sdhci_arasan->soc_ctl_base),
 					    "Can't get syscon\n");
 			goto err_pltfm_free;
 		}
 	}
 
-	sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
+	sdhci_arasan->clk_ahb = devm_clk_get(dev, "clk_ahb");
 	if (IS_ERR(sdhci_arasan->clk_ahb)) {
-		dev_err(&pdev->dev, "clk_ahb clock not found.\n");
+		dev_err(dev, "clk_ahb clock not found.\n");
 		ret = PTR_ERR(sdhci_arasan->clk_ahb);
 		goto err_pltfm_free;
 	}
 
-	clk_xin = devm_clk_get(&pdev->dev, "clk_xin");
+	clk_xin = devm_clk_get(dev, "clk_xin");
 	if (IS_ERR(clk_xin)) {
-		dev_err(&pdev->dev, "clk_xin clock not found.\n");
+		dev_err(dev, "clk_xin clock not found.\n");
 		ret = PTR_ERR(clk_xin);
 		goto err_pltfm_free;
 	}
 
 	ret = clk_prepare_enable(sdhci_arasan->clk_ahb);
 	if (ret) {
-		dev_err(&pdev->dev, "Unable to enable AHB clock.\n");
+		dev_err(dev, "Unable to enable AHB clock.\n");
 		goto err_pltfm_free;
 	}
 
 	ret = clk_prepare_enable(clk_xin);
 	if (ret) {
-		dev_err(&pdev->dev, "Unable to enable SD clock.\n");
+		dev_err(dev, "Unable to enable SD clock.\n");
 		goto clk_dis_ahb;
 	}
 
@@ -1592,7 +1593,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 
 	sdhci_arasan_update_baseclkfreq(host);
 
-	ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, &pdev->dev);
+	ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, dev);
 	if (ret)
 		goto clk_disable_all;
 
@@ -1601,28 +1602,27 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 			arasan_zynqmp_execute_tuning;
 	}
 
-	arasan_dt_parse_clk_phases(&pdev->dev, &sdhci_arasan->clk_data);
+	arasan_dt_parse_clk_phases(dev, &sdhci_arasan->clk_data);
 
 	ret = mmc_of_parse(host->mmc);
 	if (ret) {
 		if (ret != -EPROBE_DEFER)
-			dev_err(&pdev->dev, "parsing dt failed (%d)\n", ret);
+			dev_err(dev, "parsing dt failed (%d)\n", ret);
 		goto unreg_clk;
 	}
 
 	sdhci_arasan->phy = ERR_PTR(-ENODEV);
 	if (of_device_is_compatible(np, "arasan,sdhci-5.1")) {
-		sdhci_arasan->phy = devm_phy_get(&pdev->dev,
-						 "phy_arasan");
+		sdhci_arasan->phy = devm_phy_get(dev, "phy_arasan");
 		if (IS_ERR(sdhci_arasan->phy)) {
 			ret = PTR_ERR(sdhci_arasan->phy);
-			dev_err(&pdev->dev, "No phy for arasan,sdhci-5.1.\n");
+			dev_err(dev, "No phy for arasan,sdhci-5.1.\n");
 			goto unreg_clk;
 		}
 
 		ret = phy_init(sdhci_arasan->phy);
 		if (ret < 0) {
-			dev_err(&pdev->dev, "phy_init err.\n");
+			dev_err(dev, "phy_init err.\n");
 			goto unreg_clk;
 		}
 
@@ -1647,7 +1647,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	if (!IS_ERR(sdhci_arasan->phy))
 		phy_exit(sdhci_arasan->phy);
 unreg_clk:
-	sdhci_arasan_unregister_sdclk(&pdev->dev);
+	sdhci_arasan_unregister_sdclk(dev);
 clk_disable_all:
 	clk_disable_unprepare(clk_xin);
 clk_dis_ahb:
-- 
2.17.1

[PATCH v1 4/9] mmc: sdhci-of-arasan: Use dev_err_probe() to avoid spamming logs

[Muhammad Husaini Zulkifli]

Using dev_err_probe() can avoid spamming logs when probe is deferred.
This function can also help to reduce code the size, uniform error handling
and simplify the code.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 4e6ee9e69a1b..585ca32ff330 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1545,15 +1545,14 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 
 	sdhci_arasan->clk_ahb = devm_clk_get(dev, "clk_ahb");
 	if (IS_ERR(sdhci_arasan->clk_ahb)) {
-		dev_err(dev, "clk_ahb clock not found.\n");
-		ret = PTR_ERR(sdhci_arasan->clk_ahb);
+		ret = dev_err_probe(dev, PTR_ERR(sdhci_arasan->clk_ahb),
+				    "clk_ahb clock not found.\n");
 		goto err_pltfm_free;
 	}
 
 	clk_xin = devm_clk_get(dev, "clk_xin");
 	if (IS_ERR(clk_xin)) {
-		dev_err(dev, "clk_xin clock not found.\n");
-		ret = PTR_ERR(clk_xin);
+		ret = dev_err_probe(dev, PTR_ERR(clk_xin), "clk_xin clock not found.\n");
 		goto err_pltfm_free;
 	}
 
@@ -1606,8 +1605,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 
 	ret = mmc_of_parse(host->mmc);
 	if (ret) {
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "parsing dt failed (%d)\n", ret);
+		ret = dev_err_probe(dev, ret, "parsing dt failed.\n");
 		goto unreg_clk;
 	}
 
@@ -1615,8 +1613,8 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(np, "arasan,sdhci-5.1")) {
 		sdhci_arasan->phy = devm_phy_get(dev, "phy_arasan");
 		if (IS_ERR(sdhci_arasan->phy)) {
-			ret = PTR_ERR(sdhci_arasan->phy);
-			dev_err(dev, "No phy for arasan,sdhci-5.1.\n");
+			ret = dev_err_probe(dev, PTR_ERR(sdhci_arasan->phy),
+					    "No phy for arasan,sdhci-5.1.\n");
 			goto unreg_clk;
 		}
 
-- 
2.17.1

[PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Muhammad Husaini Zulkifli]

Export inline function to encapsulate AON_CFG1 for controling the I/O Rail
supplied voltage levels which communicate with Trusted Firmware.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 include/linux/firmware/intel/keembay.h | 82 ++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 include/linux/firmware/intel/keembay.h

diff --git a/include/linux/firmware/intel/keembay.h b/include/linux/firmware/intel/keembay.h
new file mode 100644
index 000000000000..f5a8dbfdb63b
--- /dev/null
+++ b/include/linux/firmware/intel/keembay.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Intel Keembay SOC Firmware API Layer
+ *
+ *  Copyright (C) 2020, Intel Corporation
+ *
+ *  Author: Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+ */
+
+#ifndef __FIRMWARE_KEEMBAY_SMC_H__
+#define __FIRMWARE_KEEMBAY_SMC_H__
+
+#include <linux/arm-smccc.h>
+
+/*
+ * This file defines an API function that can be called by a device driver in order to
+ * communicate with Trusted Firmware - A profile(TF-A) or Trusted Firmware - M profile (TF-M).
+ */
+
+#define KEEMBAY_SET_1V8_IO_RAIL	1
+#define KEEMBAY_SET_3V3_IO_RAIL	0
+
+#define KEEMBAY_IOV_1_8V_uV	1800000
+#define KEEMBAY_IOV_3_3V_uV	3300000
+
+#define KEEMBAY_SET_SD_VOLTAGE_ID 0xFF26
+#define KEEMBAY_GET_SD_VOLTAGE_ID 0xFF2A
+
+#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE		\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
+			   ARM_SMCCC_SMC_32,		\
+			   ARM_SMCCC_OWNER_SIP,		\
+			   KEEMBAY_SET_SD_VOLTAGE_ID)
+
+#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE		\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
+			   ARM_SMCCC_SMC_32,		\
+			   ARM_SMCCC_OWNER_SIP,		\
+			   KEEMBAY_GET_SD_VOLTAGE_ID)
+
+#define KEEMBAY_REG_NUM_CONSUMERS 2
+
+struct keembay_reg_supply {
+	struct regulator *consumer;
+};
+
+#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
+/*
+ * Voltage applied on the IO Rail is controlled from the Always On Register using specific
+ * bits in AON_CGF1 register. This is a secure register. Keem Bay SOC cannot exposed this
+ * register address to the outside world.
+ */
+static inline int keembay_set_io_rail_supplied_voltage(int volt)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res);
+
+	return res.a0;
+}
+
+static inline int keembay_get_io_rail_supplied_voltage(void)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE, &res);
+
+	return res.a1;
+}
+#else
+static inline int keembay_set_io_rail_supplied_voltage(int volt)
+{
+	return -ENODEV;
+}
+
+static inline int keembay_get_io_rail_supplied_voltage(void)
+{
+	return -ENODEV;
+}
+#endif
+
+#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */
-- 
2.17.1

Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Mark Brown]

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

On Thu, Jan 14, 2021 at 11:26:56PM +0800, Muhammad Husaini Zulkifli wrote:
> Export inline function to encapsulate AON_CFG1 for controling the I/O Rail
> supplied voltage levels which communicate with Trusted Firmware.

Adding Sudeep for the SMCCC bits, not deleting any context for his
benefit.

> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
>  include/linux/firmware/intel/keembay.h | 82 ++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 include/linux/firmware/intel/keembay.h
> 
> diff --git a/include/linux/firmware/intel/keembay.h b/include/linux/firmware/intel/keembay.h
> new file mode 100644
> index 000000000000..f5a8dbfdb63b
> --- /dev/null
> +++ b/include/linux/firmware/intel/keembay.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *  Intel Keembay SOC Firmware API Layer
> + *
> + *  Copyright (C) 2020, Intel Corporation
> + *
> + *  Author: Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
> + */
> +
> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__
> +#define __FIRMWARE_KEEMBAY_SMC_H__
> +
> +#include <linux/arm-smccc.h>
> +
> +/*
> + * This file defines an API function that can be called by a device driver in order to
> + * communicate with Trusted Firmware - A profile(TF-A) or Trusted Firmware - M profile (TF-M).
> + */
> +
> +#define KEEMBAY_SET_1V8_IO_RAIL	1
> +#define KEEMBAY_SET_3V3_IO_RAIL	0
> +
> +#define KEEMBAY_IOV_1_8V_uV	1800000
> +#define KEEMBAY_IOV_3_3V_uV	3300000
> +
> +#define KEEMBAY_SET_SD_VOLTAGE_ID 0xFF26
> +#define KEEMBAY_GET_SD_VOLTAGE_ID 0xFF2A
> +
> +#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE		\
> +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
> +			   ARM_SMCCC_SMC_32,		\
> +			   ARM_SMCCC_OWNER_SIP,		\
> +			   KEEMBAY_SET_SD_VOLTAGE_ID)
> +
> +#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE		\
> +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
> +			   ARM_SMCCC_SMC_32,		\
> +			   ARM_SMCCC_OWNER_SIP,		\
> +			   KEEMBAY_GET_SD_VOLTAGE_ID)
> +
> +#define KEEMBAY_REG_NUM_CONSUMERS 2
> +
> +struct keembay_reg_supply {
> +	struct regulator *consumer;
> +};
> +
> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
> +/*
> + * Voltage applied on the IO Rail is controlled from the Always On Register using specific
> + * bits in AON_CGF1 register. This is a secure register. Keem Bay SOC cannot exposed this
> + * register address to the outside world.
> + */
> +static inline int keembay_set_io_rail_supplied_voltage(int volt)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res);

There is a SCMI voltage domain protocol intended for just this use case
of controlling regulators managed by the firmware, why are you not using
that for these systems?  See drivers/firmware/arm_scmi/voltage.c.

> +
> +	return res.a0;
> +}
> +
> +static inline int keembay_get_io_rail_supplied_voltage(void)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE, &res);
> +
> +	return res.a1;
> +}
> +#else
> +static inline int keembay_set_io_rail_supplied_voltage(int volt)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int keembay_get_io_rail_supplied_voltage(void)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
> +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */
> -- 
> 2.17.1
> 

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

Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Sudeep Holla]

On Thu, Jan 14, 2021 at 04:48:11PM +0000, Mark Brown wrote:
> On Thu, Jan 14, 2021 at 11:26:56PM +0800, Muhammad Husaini Zulkifli wrote:
> > Export inline function to encapsulate AON_CFG1 for controling the I/O Rail
> > supplied voltage levels which communicate with Trusted Firmware.
>
> Adding Sudeep for the SMCCC bits, not deleting any context for his
> benefit.
>

Thanks Mark for cc-ing me and joining the dots. I completely forgot about
that fact that this platform was using SCMI using SMC as transport. Sorry
for that and it is my fault. I did review the SCMI/SMC support for this
platform sometime in June/July last year and forgot the fact it is same
platform when voltage/regulator support patches for SD/MMC was posted
sometime later last year. I concentrated on SMCCC conventions and other
details.

[...]

> > +#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE		\
> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
> > +			   ARM_SMCCC_SMC_32,		\
> > +			   ARM_SMCCC_OWNER_SIP,		\
> > +			   KEEMBAY_SET_SD_VOLTAGE_ID)
> > +
> > +#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE		\
> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
> > +			   ARM_SMCCC_SMC_32,		\
> > +			   ARM_SMCCC_OWNER_SIP,		\
> > +			   KEEMBAY_GET_SD_VOLTAGE_ID)
> > +
> > +#define KEEMBAY_REG_NUM_CONSUMERS 2
> > +
> > +struct keembay_reg_supply {
> > +	struct regulator *consumer;
> > +};
> > +
> > +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
> > +/*
> > + * Voltage applied on the IO Rail is controlled from the Always On Register using specific
> > + * bits in AON_CGF1 register. This is a secure register. Keem Bay SOC cannot exposed this
> > + * register address to the outside world.
> > + */
> > +static inline int keembay_set_io_rail_supplied_voltage(int volt)
> > +{
> > +	struct arm_smccc_res res;
> > +
> > +	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res);
>
> There is a SCMI voltage domain protocol intended for just this use case
> of controlling regulators managed by the firmware, why are you not using
> that for these systems?  See drivers/firmware/arm_scmi/voltage.c.
>

Indeed. Please switch to using the new voltage protocol added for this without
any extra code. You just need to wire up DT for this.

Just for curiosity, where is SCMI platform firmware implemented ? On Cortex-A,
secure side or external processor. Does this platform run TF-A ?

--
Regards,
Sudeep

RE: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Sudeep and Mark,

Thanks for the review. I replied inline.

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Saturday, January 16, 2021 2:58 AM
>To: Mark Brown <broonie@kernel.org>
>Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
>ulf.hansson@linaro.org; lgirdwood@gmail.com; robh+dt@kernel.org;
>devicetree@vger.kernel.org; Hunter, Adrian <adrian.hunter@intel.com>;
>michal.simek@xilinx.com; linux-mmc@vger.kernel.org; linux-
>kernel@vger.kernel.org; Shevchenko, Andriy
><andriy.shevchenko@intel.com>; A, Rashmi <rashmi.a@intel.com>; Vaidya,
>Mahesh R <mahesh.r.vaidya@intel.com>; Sudeep Holla
><sudeep.holla@arm.com>
>Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted
>Firmware Service call
>
>On Thu, Jan 14, 2021 at 04:48:11PM +0000, Mark Brown wrote:
>> On Thu, Jan 14, 2021 at 11:26:56PM +0800, Muhammad Husaini Zulkifli
>wrote:
>> > Export inline function to encapsulate AON_CFG1 for controling the
>> > I/O Rail supplied voltage levels which communicate with Trusted Firmware.
>>
>> Adding Sudeep for the SMCCC bits, not deleting any context for his
>> benefit.
>>
>
>Thanks Mark for cc-ing me and joining the dots. I completely forgot about that
>fact that this platform was using SCMI using SMC as transport. Sorry for that and
>it is my fault. I did review the SCMI/SMC support for this platform sometime in
>June/July last year and forgot the fact it is same platform when
>voltage/regulator support patches for SD/MMC was posted sometime later last
>year. I concentrated on SMCCC conventions and other details.

Yes Sudeep. I redesigned the way I handle the smccc call. Previously it was handled directly in mmc driver.
After few discussion, we conclude to create an abstraction using regulator framework to encapsulate this smccc call
during set voltage operation. Using standard abstraction will make things easier for the maintainer.

>
>[...]
>
>> > +#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE		\
>> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
>> > +			   ARM_SMCCC_SMC_32,		\
>> > +			   ARM_SMCCC_OWNER_SIP,		\
>> > +			   KEEMBAY_SET_SD_VOLTAGE_ID)
>> > +
>> > +#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE		\
>> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
>> > +			   ARM_SMCCC_SMC_32,		\
>> > +			   ARM_SMCCC_OWNER_SIP,		\
>> > +			   KEEMBAY_GET_SD_VOLTAGE_ID)
>> > +
>> > +#define KEEMBAY_REG_NUM_CONSUMERS 2
>> > +
>> > +struct keembay_reg_supply {
>> > +	struct regulator *consumer;
>> > +};
>> > +
>> > +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
>> > +/*
>> > + * Voltage applied on the IO Rail is controlled from the Always On
>> > +Register using specific
>> > + * bits in AON_CGF1 register. This is a secure register. Keem Bay
>> > +SOC cannot exposed this
>> > + * register address to the outside world.
>> > + */
>> > +static inline int keembay_set_io_rail_supplied_voltage(int volt) {
>> > +	struct arm_smccc_res res;
>> > +
>> > +
>	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTA
>GE, volt,
>> > +&res);
>>
>> There is a SCMI voltage domain protocol intended for just this use
>> case of controlling regulators managed by the firmware, why are you
>> not using that for these systems?  See drivers/firmware/arm_scmi/voltage.c.

From mmc maintainer's perspective, I should use the common modelling either using 
regulator framework or pinctrl to perform voltage operation. Not just directly invoke 
smccc call in the mmc driver. That is why I came up with this regulator driver to perform 
voltage operation. 

>>
>
>Indeed. Please switch to using the new voltage protocol added for this without
>any extra code. You just need to wire up DT for this.

May I know even if I wire up the DT, how should I call this from the mmc driver
For set/get voltage operation? Any example?

>
>Just for curiosity, where is SCMI platform firmware implemented ? On Cortex-
>A, secure side or external processor. Does this platform run TF-A ?

The KMB SCMI framework is implemented in secure runtime firmware (TF-A BL31). 
Hopefully I am answering your question.

>
>--
>Regards,
>Sudeep

Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Mark Brown]

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

On Mon, Jan 18, 2021 at 10:28:33AM +0000, Zulkifli, Muhammad Husaini wrote:

> >> There is a SCMI voltage domain protocol intended for just this use
> >> case of controlling regulators managed by the firmware, why are you
> >> not using that for these systems?  See drivers/firmware/arm_scmi/voltage.c.

> From mmc maintainer's perspective, I should use the common modelling either using 
> regulator framework or pinctrl to perform voltage operation. Not just directly invoke 
> smccc call in the mmc driver. That is why I came up with this regulator driver to perform 
> voltage operation. 

The above is a standard way of controlling regulators via SMCCC which
already has a regulator driver, you're duplicating this functionality.

> >Indeed. Please switch to using the new voltage protocol added for this without
> >any extra code. You just need to wire up DT for this.

> May I know even if I wire up the DT, how should I call this from the mmc driver
> For set/get voltage operation? Any example?

There's one in the binding document for the driver.

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

Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Sudeep Holla]

On Mon, Jan 18, 2021 at 10:28:33AM +0000, Zulkifli, Muhammad Husaini wrote:
> Hi Sudeep and Mark,
> 
> Thanks for the review. I replied inline.
> 
> >-----Original Message-----
> >From: Sudeep Holla <sudeep.holla@arm.com>
> >Sent: Saturday, January 16, 2021 2:58 AM
> >To: Mark Brown <broonie@kernel.org>
> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
> >ulf.hansson@linaro.org; lgirdwood@gmail.com; robh+dt@kernel.org;
> >devicetree@vger.kernel.org; Hunter, Adrian <adrian.hunter@intel.com>;
> >michal.simek@xilinx.com; linux-mmc@vger.kernel.org; linux-
> >kernel@vger.kernel.org; Shevchenko, Andriy
> ><andriy.shevchenko@intel.com>; A, Rashmi <rashmi.a@intel.com>; Vaidya,
> >Mahesh R <mahesh.r.vaidya@intel.com>; Sudeep Holla
> ><sudeep.holla@arm.com>
> >Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted
> >Firmware Service call
> >
> >On Thu, Jan 14, 2021 at 04:48:11PM +0000, Mark Brown wrote:
> >> On Thu, Jan 14, 2021 at 11:26:56PM +0800, Muhammad Husaini Zulkifli
> >wrote:
> >> > Export inline function to encapsulate AON_CFG1 for controling the
> >> > I/O Rail supplied voltage levels which communicate with Trusted Firmware.
> >>
> >> Adding Sudeep for the SMCCC bits, not deleting any context for his
> >> benefit.
> >>
> >
> >Thanks Mark for cc-ing me and joining the dots. I completely forgot about that
> >fact that this platform was using SCMI using SMC as transport. Sorry for that and
> >it is my fault. I did review the SCMI/SMC support for this platform sometime in
> >June/July last year and forgot the fact it is same platform when
> >voltage/regulator support patches for SD/MMC was posted sometime later last
> >year. I concentrated on SMCCC conventions and other details.
> 
> Yes Sudeep. I redesigned the way I handle the smccc call. Previously it was handled directly in mmc driver.
> After few discussion, we conclude to create an abstraction using regulator framework to encapsulate this smccc call
> during set voltage operation. Using standard abstraction will make things easier for the maintainer.
> 
> >
> >[...]
> >
> >> > +#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE		\
> >> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
> >> > +			   ARM_SMCCC_SMC_32,		\
> >> > +			   ARM_SMCCC_OWNER_SIP,		\
> >> > +			   KEEMBAY_SET_SD_VOLTAGE_ID)
> >> > +
> >> > +#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE		\
> >> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
> >> > +			   ARM_SMCCC_SMC_32,		\
> >> > +			   ARM_SMCCC_OWNER_SIP,		\
> >> > +			   KEEMBAY_GET_SD_VOLTAGE_ID)
> >> > +
> >> > +#define KEEMBAY_REG_NUM_CONSUMERS 2
> >> > +
> >> > +struct keembay_reg_supply {
> >> > +	struct regulator *consumer;
> >> > +};
> >> > +
> >> > +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
> >> > +/*
> >> > + * Voltage applied on the IO Rail is controlled from the Always On
> >> > +Register using specific
> >> > + * bits in AON_CGF1 register. This is a secure register. Keem Bay
> >> > +SOC cannot exposed this
> >> > + * register address to the outside world.
> >> > + */
> >> > +static inline int keembay_set_io_rail_supplied_voltage(int volt) {
> >> > +	struct arm_smccc_res res;
> >> > +
> >> > +
> >	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTA
> >GE, volt,
> >> > +&res);
> >>
> >> There is a SCMI voltage domain protocol intended for just this use
> >> case of controlling regulators managed by the firmware, why are you
> >> not using that for these systems?  See drivers/firmware/arm_scmi/voltage.c.
> 
> From mmc maintainer's perspective, I should use the common modelling either
> using regulator framework or pinctrl to perform voltage operation. Not just
> directly invoke  smccc call in the mmc driver. That is why I came up with
> this regulator driver to perform voltage operation.
>

That's correct. Since the platform uses SCMI and SCMI spec[1] supports Voltage
protocol and there is upstream driver[2] to support it, I see no point in
duplicating the support with another custom/non-standard solution.

> >>
> >
> >Indeed. Please switch to using the new voltage protocol added for this without
> >any extra code. You just need to wire up DT for this.
> 
> May I know even if I wire up the DT, how should I call this from the mmc driver
> For set/get voltage operation? Any example?
>

Mark has already pointed you to the binding document[3]

> >
> >Just for curiosity, where is SCMI platform firmware implemented ? On Cortex-
> >A, secure side or external processor. Does this platform run TF-A ?
> 
> The KMB SCMI framework is implemented in secure runtime firmware (TF-A BL31). 
> Hopefully I am answering your question.
>

Yes, it should be easy to extend the implementation and add support for
voltage protocol.

If you still face any issues, please ask any queries on the list cc-ing
me and Cristian Marussi(cc-ed)

--
Regards,
Sudeep

[1] https://developer.arm.com/documentation/den0056/latest
[2] drivers/firmware/arm_scmi/voltage.c + drivers/regulator/scmi-regulator.c
[3] Documentation/devicetree/bindings/arm/arm,scmi.txt

Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Cristian Marussi]

Hi 

sorry I'm a bit late on this.

On Mon, Jan 18, 2021 at 10:28:33AM +0000, Zulkifli, Muhammad Husaini wrote:
> Hi Sudeep and Mark,
> 
> Thanks for the review. I replied inline.
> 
> >-----Original Message-----
> >From: Sudeep Holla <sudeep.holla@arm.com>
> >Sent: Saturday, January 16, 2021 2:58 AM
> >To: Mark Brown <broonie@kernel.org>
> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
> >ulf.hansson@linaro.org; lgirdwood@gmail.com; robh+dt@kernel.org;
> >devicetree@vger.kernel.org; Hunter, Adrian <adrian.hunter@intel.com>;
> >michal.simek@xilinx.com; linux-mmc@vger.kernel.org; linux-
> >kernel@vger.kernel.org; Shevchenko, Andriy
> ><andriy.shevchenko@intel.com>; A, Rashmi <rashmi.a@intel.com>; Vaidya,
> >Mahesh R <mahesh.r.vaidya@intel.com>; Sudeep Holla
> ><sudeep.holla@arm.com>
> >Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted
> >Firmware Service call
> >
> >On Thu, Jan 14, 2021 at 04:48:11PM +0000, Mark Brown wrote:
> >> On Thu, Jan 14, 2021 at 11:26:56PM +0800, Muhammad Husaini Zulkifli
> >wrote:
> >> > Export inline function to encapsulate AON_CFG1 for controling the
> >> > I/O Rail supplied voltage levels which communicate with Trusted Firmware.
> >>
> >> Adding Sudeep for the SMCCC bits, not deleting any context for his
> >> benefit.
> >>
> >
> >Thanks Mark for cc-ing me and joining the dots. I completely forgot about that
> >fact that this platform was using SCMI using SMC as transport. Sorry for that and
> >it is my fault. I did review the SCMI/SMC support for this platform sometime in
> >June/July last year and forgot the fact it is same platform when
> >voltage/regulator support patches for SD/MMC was posted sometime later last
> >year. I concentrated on SMCCC conventions and other details.
> 
> Yes Sudeep. I redesigned the way I handle the smccc call. Previously it was handled directly in mmc driver.
> After few discussion, we conclude to create an abstraction using regulator framework to encapsulate this smccc call
> during set voltage operation. Using standard abstraction will make things easier for the maintainer.
> 
> >
> >[...]
> >
> >> > +#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE		\
> >> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
> >> > +			   ARM_SMCCC_SMC_32,		\
> >> > +			   ARM_SMCCC_OWNER_SIP,		\
> >> > +			   KEEMBAY_SET_SD_VOLTAGE_ID)
> >> > +
> >> > +#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE		\
> >> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
> >> > +			   ARM_SMCCC_SMC_32,		\
> >> > +			   ARM_SMCCC_OWNER_SIP,		\
> >> > +			   KEEMBAY_GET_SD_VOLTAGE_ID)
> >> > +
> >> > +#define KEEMBAY_REG_NUM_CONSUMERS 2
> >> > +
> >> > +struct keembay_reg_supply {
> >> > +	struct regulator *consumer;
> >> > +};
> >> > +
> >> > +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
> >> > +/*
> >> > + * Voltage applied on the IO Rail is controlled from the Always On
> >> > +Register using specific
> >> > + * bits in AON_CGF1 register. This is a secure register. Keem Bay
> >> > +SOC cannot exposed this
> >> > + * register address to the outside world.
> >> > + */
> >> > +static inline int keembay_set_io_rail_supplied_voltage(int volt) {
> >> > +	struct arm_smccc_res res;
> >> > +
> >> > +
> >	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTA
> >GE, volt,
> >> > +&res);
> >>
> >> There is a SCMI voltage domain protocol intended for just this use
> >> case of controlling regulators managed by the firmware, why are you
> >> not using that for these systems?  See drivers/firmware/arm_scmi/voltage.c.
> 
> From mmc maintainer's perspective, I should use the common modelling either using 
> regulator framework or pinctrl to perform voltage operation. Not just directly invoke 
> smccc call in the mmc driver. That is why I came up with this regulator driver to perform 
> voltage operation. 
> 

There is an SCMI regulator driver built on top of SCMIv3.0 Voltage Domain
Protocol, so as long as your SCMI platform firmware support the new VD
protocol you should be able to wire up a generic SCMI regulator in the DT
(as described in the arm,scmi.txt bindings) and then just use the usual
regulator framework ops with such SCMI regulator without the need to add
a custom regulator using custom SMCCC calls).

Thanks

Cristian

> >>
> >
> >Indeed. Please switch to using the new voltage protocol added for this without
> >any extra code. You just need to wire up DT for this.
> 
> May I know even if I wire up the DT, how should I call this from the mmc driver
> For set/get voltage operation? Any example?
> 
> >
> >Just for curiosity, where is SCMI platform firmware implemented ? On Cortex-
> >A, secure side or external processor. Does this platform run TF-A ?
> 
> The KMB SCMI framework is implemented in secure runtime firmware (TF-A BL31). 
> Hopefully I am answering your question.
> 
> >
> >--
> >Regards,
> >Sudeep

RE: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Cristian, Sudeep and Mark,

>-----Original Message-----
>From: Cristian Marussi <cristian.marussi@arm.com>
>Sent: Monday, January 18, 2021 8:02 PM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Sudeep Holla <sudeep.holla@arm.com>; Mark Brown
><broonie@kernel.org>; ulf.hansson@linaro.org; lgirdwood@gmail.com;
>robh+dt@kernel.org; devicetree@vger.kernel.org; Hunter, Adrian
><adrian.hunter@intel.com>; michal.simek@xilinx.com; linux-
>mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Shevchenko, Andriy
><andriy.shevchenko@intel.com>; A, Rashmi <rashmi.a@intel.com>; Vaidya,
>Mahesh R <mahesh.r.vaidya@intel.com>
>Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted
>Firmware Service call
>
>Hi
>
>sorry I'm a bit late on this.
>
>On Mon, Jan 18, 2021 at 10:28:33AM +0000, Zulkifli, Muhammad Husaini
>wrote:
>> Hi Sudeep and Mark,
>>
>> Thanks for the review. I replied inline.
>>
>> >-----Original Message-----
>> >From: Sudeep Holla <sudeep.holla@arm.com>
>> >Sent: Saturday, January 16, 2021 2:58 AM
>> >To: Mark Brown <broonie@kernel.org>
>> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
>> >ulf.hansson@linaro.org; lgirdwood@gmail.com; robh+dt@kernel.org;
>> >devicetree@vger.kernel.org; Hunter, Adrian <adrian.hunter@intel.com>;
>> >michal.simek@xilinx.com; linux-mmc@vger.kernel.org; linux-
>> >kernel@vger.kernel.org; Shevchenko, Andriy
>> ><andriy.shevchenko@intel.com>; A, Rashmi <rashmi.a@intel.com>;
>> >Vaidya, Mahesh R <mahesh.r.vaidya@intel.com>; Sudeep Holla
>> ><sudeep.holla@arm.com>
>> >Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for
>> >Trusted Firmware Service call
>> >
>> >On Thu, Jan 14, 2021 at 04:48:11PM +0000, Mark Brown wrote:
>> >> On Thu, Jan 14, 2021 at 11:26:56PM +0800, Muhammad Husaini Zulkifli
>> >wrote:
>> >> > Export inline function to encapsulate AON_CFG1 for controling the
>> >> > I/O Rail supplied voltage levels which communicate with Trusted
>Firmware.
>> >>
>> >> Adding Sudeep for the SMCCC bits, not deleting any context for his
>> >> benefit.
>> >>
>> >
>> >Thanks Mark for cc-ing me and joining the dots. I completely forgot
>> >about that fact that this platform was using SCMI using SMC as
>> >transport. Sorry for that and it is my fault. I did review the
>> >SCMI/SMC support for this platform sometime in June/July last year
>> >and forgot the fact it is same platform when voltage/regulator
>> >support patches for SD/MMC was posted sometime later last year. I
>concentrated on SMCCC conventions and other details.
>>
>> Yes Sudeep. I redesigned the way I handle the smccc call. Previously it was
>handled directly in mmc driver.
>> After few discussion, we conclude to create an abstraction using
>> regulator framework to encapsulate this smccc call during set voltage
>operation. Using standard abstraction will make things easier for the
>maintainer.
>>
>> >
>> >[...]
>> >
>> >> > +#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE
>	\
>> >> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
>	\
>> >> > +			   ARM_SMCCC_SMC_32,		\
>> >> > +			   ARM_SMCCC_OWNER_SIP,		\
>> >> > +			   KEEMBAY_SET_SD_VOLTAGE_ID)
>> >> > +
>> >> > +#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE
>	\
>> >> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
>	\
>> >> > +			   ARM_SMCCC_SMC_32,		\
>> >> > +			   ARM_SMCCC_OWNER_SIP,		\
>> >> > +			   KEEMBAY_GET_SD_VOLTAGE_ID)
>> >> > +
>> >> > +#define KEEMBAY_REG_NUM_CONSUMERS 2
>> >> > +
>> >> > +struct keembay_reg_supply {
>> >> > +	struct regulator *consumer;
>> >> > +};
>> >> > +
>> >> > +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
>> >> > +/*
>> >> > + * Voltage applied on the IO Rail is controlled from the Always
>> >> > +On Register using specific
>> >> > + * bits in AON_CGF1 register. This is a secure register. Keem
>> >> > +Bay SOC cannot exposed this
>> >> > + * register address to the outside world.
>> >> > + */
>> >> > +static inline int keembay_set_io_rail_supplied_voltage(int volt) {
>> >> > +	struct arm_smccc_res res;
>> >> > +
>> >> > +
>> >	arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTA
>> >GE, volt,
>> >> > +&res);
>> >>
>> >> There is a SCMI voltage domain protocol intended for just this use
>> >> case of controlling regulators managed by the firmware, why are you
>> >> not using that for these systems?  See
>drivers/firmware/arm_scmi/voltage.c.
>>
>> From mmc maintainer's perspective, I should use the common modelling
>> either using regulator framework or pinctrl to perform voltage
>> operation. Not just directly invoke smccc call in the mmc driver. That
>> is why I came up with this regulator driver to perform voltage operation.
>>
>
>There is an SCMI regulator driver built on top of SCMIv3.0 Voltage Domain
>Protocol, so as long as your SCMI platform firmware support the new VD
>protocol you should be able to wire up a generic SCMI regulator in the DT
>(as described in the arm,scmi.txt bindings) and then just use the usual
>regulator framework ops with such SCMI regulator without the need to add
>a custom regulator using custom SMCCC calls).

I try to hook up the DT last night. Seems like the SCMI Protocol 17 is not implemented at ATF side.
Double check with ATF Team, currently we don't have SCMI voltage domain control in ARM Trusted Firmware yet
as of now, that is why even if I map the function to scmi, my call will be fail.

[    2.648989] arm-scmi firmware:scmi: SCMI Notifications - Core Enabled.
[    2.656157] arm-scmi firmware:scmi: SCMI Protocol v1.0 'INTEL:KMB' Firmware version 0x1
[    2.664513] arm-scmi firmware:scmi: SCMI protocol 23 not implemented
[    2.675898] arm-scmi firmware:scmi: SCMI protocol 17 not implemented

Any possibilities that for UHS patch we go with my current regulator driver implementation?

>
>Thanks
>
>Cristian
>
>> >>
>> >
>> >Indeed. Please switch to using the new voltage protocol added for this
>without
>> >any extra code. You just need to wire up DT for this.
>>
>> May I know even if I wire up the DT, how should I call this from the mmc
>driver
>> For set/get voltage operation? Any example?
>>
>> >
>> >Just for curiosity, where is SCMI platform firmware implemented ? On
>Cortex-
>> >A, secure side or external processor. Does this platform run TF-A ?
>>
>> The KMB SCMI framework is implemented in secure runtime firmware (TF-A
>BL31).
>> Hopefully I am answering your question.
>>
>> >
>> >--
>> >Regards,
>> >Sudeep

Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

[Sudeep Holla]

On Tue, Jan 19, 2021 at 02:38:32AM +0000, Zulkifli, Muhammad Husaini wrote:

[...]

> 
> I try to hook up the DT last night. Seems like the SCMI Protocol 17 is not
> implemented at ATF side.

I had guessed that.

> Double check with ATF Team, currently we don't have SCMI voltage domain
> control in ARM Trusted Firmware yet as of now, that is why even if I map the
> function to scmi, my call will be fail.

Correct, but if you already have this custom SMCCC for voltage already
implemented in TF-A, I don't see it is a big deal to support voltage
protocol there.

> 
> [    2.648989] arm-scmi firmware:scmi: SCMI Notifications - Core Enabled.
> [    2.656157] arm-scmi firmware:scmi: SCMI Protocol v1.0 'INTEL:KMB' Firmware version 0x1
> [    2.664513] arm-scmi firmware:scmi: SCMI protocol 23 not implemented
> [    2.675898] arm-scmi firmware:scmi: SCMI protocol 17 not implemented
> 
> Any possibilities that for UHS patch we go with my current regulator driver
> implementation?

Sorry absolutely no. If this platform was not using SCMI, I wouldn't have
pushed back hard on this custom SMCCC. Please update TF-A to add this support.
There is no point in having custom interface just for this when everything
else is already using SCMI on this platform.

-- 
Regards,
Sudeep

[PATCH v1 6/9] dt-bindings: mmc: Update phy and regulator supply for Keem Bay SOC

[Muhammad Husaini Zulkifli]

Add DT bindings of vmmc, vqmmc and sdvrail supplies of regulator
and phys for the phandle of sd0_phy which contain additional property
for otap delay and sel_clk_buffer.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
index 37a5fe7b26dc..b77a1ff37afa 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
@@ -83,7 +83,7 @@ properties:
       - const: intel,keembay-sdhci-5.1-sd       # Intel Keem Bay SD controller
         description:
           For this device it is strongly suggested to include
-          arasan,soc-ctl-syscon.
+          arasan,soc-ctl-syscon, phys, vmmc-supply, vqmmc-supply and sdvrail-supply.
       - const: intel,keembay-sdhci-5.1-sdio     # Intel Keem Bay SDIO controller
         description:
           For this device it is strongly suggested to include
@@ -299,5 +299,10 @@ examples:
           clock-names = "clk_xin", "clk_ahb";
           clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD0>,
                    <&scmi_clk KEEM_BAY_PSS_SD0>;
+          phys = <&sd0_phy>;
+          phy-names = "phy_arasan";
           arasan,soc-ctl-syscon = <&sd0_phy_syscon>;
+          vmmc-supply = <&reg_sd0_vcc>;
+          vqmmc-supply = <&reg_sd0_vqcc>;
+          sdvrail-supply = <&regulator_rail>;
     };
-- 
2.17.1

[PATCH v1 7/9] dt-bindings: regulator: keembay: Add DT binding documentation

[Muhammad Husaini Zulkifli]

Add DT Binding Documentation for Keem Bay SD Regulator.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 .../bindings/regulator/keembay-regulator.yaml | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/keembay-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/keembay-regulator.yaml b/Documentation/devicetree/bindings/regulator/keembay-regulator.yaml
new file mode 100644
index 000000000000..a32e87c9eeed
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/keembay-regulator.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/keembay-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Keem Bay SD regulator
+
+maintainers:
+  - Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+
+allOf:
+  - $ref: "regulator.yaml#"
+
+properties:
+  compatible:
+    const: regulator-keembay-sd
+
+  regulator-name: true
+
+required:
+  - compatible
+  - regulator-name
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    regulator_rail {
+      compatible = "regulator-keembay-sd";
+
+      regulator-name = "sd-volt-rail";
+      regulator-min-microvolt = <1800000>;
+      regulator-max-microvolt = <3300000>;
+    };
+...
-- 
2.17.1

Re: [PATCH v1 7/9] dt-bindings: regulator: keembay: Add DT binding documentation

[Mark Brown]

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

On Thu, Jan 14, 2021 at 11:26:58PM +0800, Muhammad Husaini Zulkifli wrote:
> Add DT Binding Documentation for Keem Bay SD Regulator.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> +properties:
> +  compatible:
> +    const: regulator-keembay-sd

Device tree compatible strings should be in the format vendor,device.

> +required:
> +  - compatible
> +  - regulator-name

Why is regulator-name required here?  This is a standard regulator
binding property which is covered by the generic regulator bindings and
should be optional since it is for use by the system integrator to
provide more user friendly diagnostic information about which supply
this is in the system.  If you are attempting to use this for device
identification then you should be using different compatible strings for
the different regulators - even if you were using regulator-name you'd
need to document the values.

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

[PATCH v1 8/9] regulator: keembay: Add regulator for Keem Bay SoC

[Muhammad Husaini Zulkifli]

Keem Bay SD regulator driver module is added to encapsulate ARM
Secure Monitor Call Calling Convention (SMCCC) during set voltage
operations to control I/O Rail supplied voltage levels which communicate
with Trusted Firmware.

I/O Rail voltage need to be configure through AON_CFG1 Register by
setting specific bit in the AON_CFG1 Register. AON_CFG1 Register is
a secure register. Direct access to AON_CFG1 register bit will cause
firewall violation in secure system.

Modelling using standard regulator abstraction during voltage operation
make things easier for device driver to consume it.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/regulator/Kconfig                |  10 ++
 drivers/regulator/Makefile               |   1 +
 drivers/regulator/keembay-sd-regulator.c | 112 +++++++++++++++++++++++
 3 files changed, 123 insertions(+)
 create mode 100644 drivers/regulator/keembay-sd-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 5abdd29fb9f3..72cfd0d14066 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -436,6 +436,16 @@ config REGULATOR_ISL6271A
 	help
 	  This driver supports ISL6271A voltage regulator chip.
 
+config REGULATOR_KEEMBAY_SD
+	tristate "Intel Keem Bay SD Regulator"
+	depends on HAVE_ARM_SMCCC && (OF || COMPILE_TEST)
+	help
+	  This driver provides support for the voltage regulators of the
+	  Keem Bay SOC to encapsulate ARM Secure Monitor Call Calling Convention
+	  to change the voltage rail output.
+	  This is specific for Keem Bay hardware design only.
+	  The module name will be called keembay-sd-regulator.
+
 config REGULATOR_LM363X
 	tristate "TI LM363X voltage regulators"
 	depends on MFD_TI_LMU
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 680e539f6579..0d2392441b66 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_REGULATOR_HI6421V530) += hi6421v530-regulator.o
 obj-$(CONFIG_REGULATOR_HI655X) += hi655x-regulator.o
 obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
 obj-$(CONFIG_REGULATOR_ISL9305) += isl9305.o
+obj-$(CONFIG_REGULATOR_KEEMBAY_SD) += keembay-sd-regulator.o
 obj-$(CONFIG_REGULATOR_LM363X) += lm363x-regulator.o
 obj-$(CONFIG_REGULATOR_LOCHNAGAR) += lochnagar-regulator.o
 obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
diff --git a/drivers/regulator/keembay-sd-regulator.c b/drivers/regulator/keembay-sd-regulator.c
new file mode 100644
index 000000000000..7f90c08de283
--- /dev/null
+++ b/drivers/regulator/keembay-sd-regulator.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel Keem Bay SD Regulator
+ *
+ * Copyright (C) 2020, Intel Corporation
+ * Author: Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+ */
+
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <linux/firmware/intel/keembay.h>
+
+static int keembay_regulator_set_voltage(struct regulator_dev *dev,
+					int min_uV, int max_uV,
+					unsigned *selector)
+{
+	int tmp_volt;
+
+	if (min_uV == KEEMBAY_IOV_1_8V_uV && max_uV == KEEMBAY_IOV_1_8V_uV)
+		tmp_volt = KEEMBAY_SET_1V8_IO_RAIL;
+	else
+		tmp_volt = KEEMBAY_SET_3V3_IO_RAIL;
+
+	return keembay_set_io_rail_supplied_voltage(tmp_volt);
+}
+
+static int keembay_regulator_get_voltage(struct regulator_dev *dev)
+{
+	int ret;
+
+	ret = keembay_get_io_rail_supplied_voltage();
+
+	return ret ? KEEMBAY_IOV_1_8V_uV : KEEMBAY_IOV_3_3V_uV;
+}
+
+static const struct regulator_ops keembay_regulator_voltage_ops = {
+	.get_voltage = keembay_regulator_get_voltage,
+	.set_voltage = keembay_regulator_set_voltage,
+};
+
+static int keembay_regulator_probe(struct platform_device *pdev)
+{
+	struct regulator_desc *desc;
+	struct regulator_init_data *init_data;
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+	struct device *dev = &pdev->dev;
+
+	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+
+	init_data = of_get_regulator_init_data(dev, dev->of_node, desc);
+	if (!init_data)
+		return -EINVAL;
+
+	desc->name = dev_name(dev);
+	desc->type = REGULATOR_VOLTAGE;
+	desc->owner = THIS_MODULE;
+	desc->ops = &keembay_regulator_voltage_ops;
+
+	config.dev = dev;
+	config.init_data = init_data;
+	config.of_node = dev->of_node;
+
+	rdev = devm_regulator_register(dev, desc, &config);
+	if (IS_ERR(rdev))
+		return dev_err_probe(dev, PTR_ERR(rdev),
+				     "Failed to register Keem Bay SD regulator.\n");
+
+	return 0;
+}
+
+static const struct of_device_id regulator_keembay_of_match[] = {
+	{ .compatible = "regulator-keembay-sd" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, regulator_keembay_of_match);
+
+static struct platform_driver keembay_regulator_driver = {
+	.probe		= keembay_regulator_probe,
+	.driver		= {
+		.name		= "keembay-sd-regulator",
+		.of_match_table = regulator_keembay_of_match,
+	},
+};
+
+static int __init keembay_regulator_init(void)
+{
+	return platform_driver_register(&keembay_regulator_driver);
+}
+
+/*
+ * Using subsys_initcall to ensure that Keem Bay regulator platform driver
+ * is initialized before device driver try to utilize it.
+ */
+subsys_initcall(keembay_regulator_init);
+
+static void __exit keembay_regulator_exit(void)
+{
+	platform_driver_unregister(&keembay_regulator_driver);
+}
+module_exit(keembay_regulator_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Intel Keem Bay SD Regulator");
+MODULE_AUTHOR("Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>");
-- 
2.17.1

Re: [PATCH v1 8/9] regulator: keembay: Add regulator for Keem Bay SoC

[Mark Brown]

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

On Thu, Jan 14, 2021 at 11:26:59PM +0800, Muhammad Husaini Zulkifli wrote:

> Keem Bay SD regulator driver module is added to encapsulate ARM
> Secure Monitor Call Calling Convention (SMCCC) during set voltage
> operations to control I/O Rail supplied voltage levels which communicate
> with Trusted Firmware.

Adding in Sudeep again for the SMCCC bits.  I just checked and am I
right in thinking this might already be shipping in production?

> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Intel Keem Bay SD Regulator
> + *
> + * Copyright (C) 2020, Intel Corporation
> + * Author: Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
> + */

Please make the entire comment a C++ comment to improve legibility.

> +static int keembay_regulator_set_voltage(struct regulator_dev *dev,
> +					int min_uV, int max_uV,
> +					unsigned *selector)
> +{
> +	int tmp_volt;
> +
> +	if (min_uV == KEEMBAY_IOV_1_8V_uV && max_uV == KEEMBAY_IOV_1_8V_uV)
> +		tmp_volt = KEEMBAY_SET_1V8_IO_RAIL;
> +	else
> +		tmp_volt = KEEMBAY_SET_3V3_IO_RAIL;
> +
> +	return keembay_set_io_rail_supplied_voltage(tmp_volt);
> +}

This has serious problems with input validation and is broken for most
valid input.  A set_voltage() function should set the voltage to the
lowest valid voltage within the range specified in the arguments and
return an error if it is not possible to set a voltage within the range
specified by the arguments.  This function will set 3.3V for any input
range other than exactly 1.8V so for example if the caller validly sets
a range of 1.7V-1.9V then 3.3V will be selected instead of 1.8V, or if
the user sets 1.0-1.1V then it will set 3.3V instead of returning an
error.

> +static int keembay_regulator_get_voltage(struct regulator_dev *dev)
> +{
> +	int ret;
> +
> +	ret = keembay_get_io_rail_supplied_voltage();
> +
> +	return ret ? KEEMBAY_IOV_1_8V_uV : KEEMBAY_IOV_3_3V_uV;
> +}

This ignores any errors or out of bounds values returned by the called
function, and please write normal conditional statements rather than
using the ternery operator to improve legibility.

> +/*
> + * Using subsys_initcall to ensure that Keem Bay regulator platform driver
> + * is initialized before device driver try to utilize it.
> + */
> +subsys_initcall(keembay_regulator_init);

There is no need to do this, probe deferral will ensure that
dependencies will be resolved.

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

Re: [PATCH v1 8/9] regulator: keembay: Add regulator for Keem Bay SoC

[Sudeep Holla]

On Thu, Jan 14, 2021 at 05:14:34PM +0000, Mark Brown wrote:
> On Thu, Jan 14, 2021 at 11:26:59PM +0800, Muhammad Husaini Zulkifli wrote:
>
> > Keem Bay SD regulator driver module is added to encapsulate ARM
> > Secure Monitor Call Calling Convention (SMCCC) during set voltage
> > operations to control I/O Rail supplied voltage levels which communicate
> > with Trusted Firmware.
>
> Adding in Sudeep again for the SMCCC bits.  I just checked and am I
> right in thinking this might already be shipping in production?
>

OK you seem to have asked the most important question here directly.
I have asked for the details of platform firmware implementation in
the other email basically for the same reason(to check how feasible is
it to move to new SCMI voltage protocol). Some work in the firmware, but
if the implement is on the A-profile itself in TF-A or any other equivalent,
it should not be too difficult.

--
Regards,
Sudeep

[PATCH v1 9/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

[Muhammad Husaini Zulkifli]

Keem Bay SOC can support dual voltage operations for GPIO SD pins to
either 1.8V or 3.3V for bus IO line power. In order to operate the GPIOs
line for Clk, Cmd and Data on Keem Bay hardware, it is important to
configure the supplied voltage applied to their I/O Rail and the output
of the I²C expander pin. Final Voltage applied on the GPIOs line are
dependent by both supplied voltage rail and expander pin output as it is
been set after passing through the voltage sense resistor.

Keem Bay hardware is somewhat unique in the way of how IO bus line
voltage are been controlled.

Expander pin output is controlled by gpio-regulator. Voltage rail output
is controlled by Keem Bay SD regulator. Keem Bay SD regulator encapsulated
the Secure Monitor Calling Convention (SMCCC) to communicate with Trusted
Firmware during set voltage operation.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 263 +++++++++++++++++++++++++++++
 1 file changed, 263 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 585ca32ff330..94253e3b8e52 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -22,6 +22,7 @@
 #include <linux/phy/phy.h>
 #include <linux/regmap.h>
 #include <linux/of.h>
+#include <linux/firmware/intel/keembay.h>
 #include <linux/firmware/xlnx-zynqmp.h>
 
 #include "cqhci.h"
@@ -79,6 +80,8 @@ struct sdhci_arasan_soc_ctl_field {
  * @baseclkfreq:	Where to find corecfg_baseclkfreq
  * @clockmultiplier:	Where to find corecfg_clockmultiplier
  * @support64b:		Where to find SUPPORT64B bit
+ * @otap_delay:		Where to find otap_delay
+ * @sel_clk_buffer:	Where to find clock buffer delay
  * @hiword_update:	If true, use HIWORD_UPDATE to access the syscon
  *
  * It's up to the licensee of the Arsan IP block to make these available
@@ -89,6 +92,8 @@ struct sdhci_arasan_soc_ctl_map {
 	struct sdhci_arasan_soc_ctl_field	baseclkfreq;
 	struct sdhci_arasan_soc_ctl_field	clockmultiplier;
 	struct sdhci_arasan_soc_ctl_field	support64b;
+	struct sdhci_arasan_soc_ctl_field	otap_delay;
+	struct sdhci_arasan_soc_ctl_field	sel_clk_buffer;
 	bool					hiword_update;
 };
 
@@ -139,6 +144,8 @@ struct sdhci_arasan_clk_data {
  * @soc_ctl_base:	Pointer to regmap for syscon for soc_ctl registers.
  * @soc_ctl_map:	Map to get offsets into soc_ctl registers.
  * @quirks:		Arasan deviations from spec.
+ * @aux_regulator:	Struct for regulator.
+ * @aux_regulator_en:	True if regulator is on; false if not.
  */
 struct sdhci_arasan_data {
 	struct sdhci_host *host;
@@ -153,6 +160,8 @@ struct sdhci_arasan_data {
 	struct regmap	*soc_ctl_base;
 	const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
 	unsigned int	quirks;
+	struct regulator *aux_regulator;
+	bool aux_regulator_en;
 
 /* Controller does not have CD wired and will not function normally without */
 #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST	BIT(0)
@@ -189,6 +198,8 @@ static const struct sdhci_arasan_soc_ctl_map intel_keembay_soc_ctl_map = {
 	.baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 },
 	.clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 },
 	.support64b = { .reg = 0x4, .width = 1, .shift = 24 },
+	.otap_delay = { .reg = 0x24, .width = 5, .shift = 23 },
+	.sel_clk_buffer = { .reg = 0x2c, .width = 3, .shift = 25 },
 	.hiword_update = false,
 };
 
@@ -364,6 +375,132 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
 	return -EINVAL;
 }
 
+/**
+ * sdhci_arasan_keembay_voltage_switch - Voltage switch operation
+ * @mmc:	Pointer to mmc_host
+ * @ios:	Pointer to IO bus setting
+ *
+ * For Keem Bay hardware, in order to operate the GPIOs line for Clk, Cmd and Data,
+ * it is important to configure the supplied voltage applied to their I/O Rail
+ * and the output of the I²C expander Pin.
+ *
+ * Note that to configure the voltage rail output setting, specific bits in AON_CFG
+ * register must be set. This AON_CFG register is a secure register. Keem Bay regulator
+ * was introduced to encapsulate the Secure Monitor Call Calling Convention (SMCCC) for
+ * voltage rail output change. While to configure the I²C expander pin output,
+ * GPIO regulator modelling is been used to control the pin state.
+ *
+ * Regulator configuration:
+ * mmc->supply.vqmmc		- expander pin output voltage
+ * sdhci_arasan->aux_regulator	- voltage rail output voltage
+ *
+ * Final Voltage applied on the GPIOs Line are dependent by both supplied voltage
+ * I/O Rail and expander pin output as it is been set after passing through the
+ * voltage sense resistor.
+ *
+ * Return: 0 on success and error value on error
+ */
+static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc,
+					       struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+	struct keembay_reg_supply supplies[KEEMBAY_REG_NUM_CONSUMERS];
+	u16 ctrl_2, clk;
+	int i, ret;
+
+	/* If no vqmmc supply then we can't change the expander pin output voltage */
+	if (IS_ERR(mmc->supply.vqmmc))
+		return PTR_ERR(mmc->supply.vqmmc);
+
+	/* If no sdvrail supply then we can't change the voltage rail output voltage */
+	if (IS_ERR(sdhci_arasan->aux_regulator))
+		return PTR_ERR(sdhci_arasan->aux_regulator);
+
+	supplies[0].consumer = mmc->supply.vqmmc;
+	supplies[1].consumer = sdhci_arasan->aux_regulator;
+
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_180:
+		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+		clk &= ~SDHCI_CLOCK_CARD_EN;
+		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+		if (clk & SDHCI_CLOCK_CARD_EN)
+			return -EAGAIN;
+
+		sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180,
+				   SDHCI_POWER_CONTROL);
+
+		for (i = 0; i < KEEMBAY_REG_NUM_CONSUMERS; i++) {
+			ret = regulator_set_voltage(supplies[i].consumer,
+						    KEEMBAY_IOV_1_8V_uV,
+						    KEEMBAY_IOV_1_8V_uV);
+			if (ret)
+				return ret;
+		}
+
+		ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		ctrl_2 |= SDHCI_CTRL_VDD_180;
+		sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+
+		/* Sleep for 5ms to stabilize 1.8V regulator */
+		usleep_range(5000, 5500);
+
+		/* 1.8V regulator output should be stable within 5 ms */
+		ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		if (!(ctrl_2 & SDHCI_CTRL_VDD_180))
+			return -EAGAIN;
+
+		clk  = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+		clk |= SDHCI_CLOCK_CARD_EN;
+		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+		return 0;
+	case MMC_SIGNAL_VOLTAGE_330:
+		for (i = 0; i < KEEMBAY_REG_NUM_CONSUMERS; i++) {
+			ret = regulator_set_voltage(supplies[i].consumer,
+						    KEEMBAY_IOV_3_3V_uV,
+						    KEEMBAY_IOV_3_3V_uV);
+			if (ret)
+				return ret;
+		}
+
+		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
+		ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		ctrl_2 &= ~SDHCI_CTRL_VDD_180;
+		sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+
+		/* Sleep for 5ms to stabilize 3.3V regulator */
+		usleep_range(5000, 5500);
+
+		/* 3.3V regulator output should be stable within 5 ms */
+		ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		if (ctrl_2 & SDHCI_CTRL_VDD_180)
+			return -EAGAIN;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int sdhci_arasan_keembay_select_drive_strength(struct mmc_card *card,
+					unsigned int max_dtr, int host_drv,
+					int card_drv, int *drv_type)
+{
+	switch (card->host->ios.signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_180:
+		*drv_type = MMC_SET_DRIVER_TYPE_C;
+		return 0;
+	case MMC_SIGNAL_VOLTAGE_330:
+		*drv_type = MMC_SET_DRIVER_TYPE_B;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
 static const struct sdhci_ops sdhci_arasan_ops = {
 	.set_clock = sdhci_arasan_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
@@ -968,6 +1105,72 @@ static void sdhci_arasan_update_baseclkfreq(struct sdhci_host *host)
 	sdhci_arasan_syscon_write(host, &soc_ctl_map->baseclkfreq, mhz);
 }
 
+/**
+ * sdhci_arasan_update_otap_delay - Set otap delay
+ * @host:		The sdhci_host
+ * @value:		The value to write
+ *
+ * Otap delay can be use to control the txclk tap delay for flopping the final stage flops.
+ *
+ * NOTE:
+ * Many existing devices don't seem to do this and work fine. To keep compatibility for
+ * old hardware where the device tree doesn't provide a register map, this function is a
+ * noop if a soc_ctl_map hasn't been provided for this platform.
+ */
+static void sdhci_arasan_update_otap_delay(struct sdhci_host *host, u32 value)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
+	struct device *dev = host->mmc->parent;
+
+	/* Having a map is optional */
+	soc_ctl_map = sdhci_arasan->soc_ctl_map;
+	if (!soc_ctl_map)
+		return;
+
+	/* If we have a map, we expect to have a syscon */
+	if (!sdhci_arasan->soc_ctl_base) {
+		dev_warn(dev, "Have regmap, but no soc-ctl-syscon.\n");
+		return;
+	}
+
+	sdhci_arasan_syscon_write(host, &soc_ctl_map->otap_delay, value);
+}
+
+/**
+ * sdhci_arasan_update_sel_clkbuf - Clock buffer select
+ * @host:		The sdhci_host
+ * @value:		The value to write
+ *
+ * Clock buffer select is use to delay the clock buffer.
+ *
+ * NOTE:
+ * Many existing devices don't seem to do this and work fine. To keep compatibility for
+ * old hardware where the device tree doesn't provide a register map, this function is a
+ * noop if a soc_ctl_map hasn't been provided for this platform.
+ */
+static void sdhci_arasan_update_sel_clkbuf(struct sdhci_host *host, u32 value)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
+	struct device *dev = host->mmc->parent;
+
+	/* Having a map is optional */
+	soc_ctl_map = sdhci_arasan->soc_ctl_map;
+	if (!soc_ctl_map)
+		return;
+
+	/* If we have a map, we expect to have a syscon */
+	if (!sdhci_arasan->soc_ctl_base) {
+		dev_warn(dev, "Have regmap, but no soc-ctl-syscon.\n");
+		return;
+	}
+
+	sdhci_arasan_syscon_write(host, &soc_ctl_map->sel_clk_buffer, value);
+}
+
 static void sdhci_arasan_set_clk_delays(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1256,6 +1459,42 @@ static const struct of_device_id sdhci_arasan_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, sdhci_arasan_of_match);
 
+static int sdhci_arasan_keembay_regulator_setup(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+	struct device *dev = host->mmc->parent;
+
+	sdhci_arasan->aux_regulator = devm_regulator_get_optional(dev, "sdvrail");
+	if (IS_ERR(sdhci_arasan->aux_regulator))
+		return PTR_ERR(sdhci_arasan->aux_regulator);
+
+	return regulator_enable(sdhci_arasan->aux_regulator);
+}
+
+static int sdhci_arasan_keembay_phy_configuration(struct sdhci_host *host)
+{
+	struct device *dev = host->mmc->parent;
+	struct device_node *phys;
+	u32 otap_delay, sel_clk_buffer;
+
+	phys = of_parse_phandle(dev->of_node, "phys", 0);
+	if (!phys) {
+		dev_err(dev, "Can't get phys for sd0\n");
+		return -ENODEV;
+	}
+
+	of_property_read_u32(phys, "intel,keembay-emmc-phy-otap-dly", &otap_delay);
+	of_property_read_u32(phys, "intel,keembay-emmc-phy-sel-clkbuf", &sel_clk_buffer);
+
+	of_node_put(phys);
+
+	sdhci_arasan_update_otap_delay(host, otap_delay);
+	sdhci_arasan_update_sel_clkbuf(host, sel_clk_buffer);
+
+	return 0;
+}
+
 /**
  * sdhci_arasan_register_sdcardclk - Register the sdcardclk for a PHY to use
  *
@@ -1590,6 +1829,24 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 		host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
 	}
 
+	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
+		ret = sdhci_arasan_keembay_regulator_setup(host);
+		if (ret)
+			goto clk_disable_all;
+
+		sdhci_arasan->aux_regulator_en = true;
+
+		ret = sdhci_arasan_keembay_phy_configuration(host);
+		if (ret)
+			goto clk_disable_all;
+
+		host->mmc_host_ops.start_signal_voltage_switch =
+			sdhci_arasan_keembay_voltage_switch;
+
+		host->mmc_host_ops.select_drive_strength =
+			sdhci_arasan_keembay_select_drive_strength;
+	}
+
 	sdhci_arasan_update_baseclkfreq(host);
 
 	ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, dev);
@@ -1651,6 +1908,9 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 clk_dis_ahb:
 	clk_disable_unprepare(sdhci_arasan->clk_ahb);
 err_pltfm_free:
+	if (sdhci_arasan->aux_regulator_en)
+		regulator_disable(sdhci_arasan->aux_regulator);
+
 	sdhci_pltfm_free(pdev);
 	return ret;
 }
@@ -1669,6 +1929,9 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
 		phy_exit(sdhci_arasan->phy);
 	}
 
+	if (sdhci_arasan->aux_regulator_en)
+		regulator_disable(sdhci_arasan->aux_regulator);
+
 	sdhci_arasan_unregister_sdclk(&pdev->dev);
 
 	ret = sdhci_pltfm_unregister(pdev);
-- 
2.17.1

Re: [PATCH v1 9/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

[Rob Herring]

On Thu, Jan 14, 2021 at 11:27:00PM +0800, Muhammad Husaini Zulkifli wrote:
> Keem Bay SOC can support dual voltage operations for GPIO SD pins to
> either 1.8V or 3.3V for bus IO line power. In order to operate the GPIOs
> line for Clk, Cmd and Data on Keem Bay hardware, it is important to
> configure the supplied voltage applied to their I/O Rail and the output
> of the I²C expander pin. Final Voltage applied on the GPIOs line are
> dependent by both supplied voltage rail and expander pin output as it is
> been set after passing through the voltage sense resistor.
> 
> Keem Bay hardware is somewhat unique in the way of how IO bus line
> voltage are been controlled.
> 
> Expander pin output is controlled by gpio-regulator. Voltage rail output
> is controlled by Keem Bay SD regulator. Keem Bay SD regulator encapsulated
> the Secure Monitor Calling Convention (SMCCC) to communicate with Trusted
> Firmware during set voltage operation.
> 
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 263 +++++++++++++++++++++++++++++
>  1 file changed, 263 insertions(+)

> +	u32 otap_delay, sel_clk_buffer;
> +
> +	phys = of_parse_phandle(dev->of_node, "phys", 0);

Normally, you'd use the phy API here. Though not required from a DT 
perspective.

> +	if (!phys) {
> +		dev_err(dev, "Can't get phys for sd0\n");
> +		return -ENODEV;
> +	}
> +
> +	of_property_read_u32(phys, "intel,keembay-emmc-phy-otap-dly", &otap_delay);
> +	of_property_read_u32(phys, "intel,keembay-emmc-phy-sel-clkbuf", &sel_clk_buffer);

Not doucmented?

For property names, I'd leave out the SoC name. Might want to use it in 
the next chip.

Rob

Re: [PATCH v1 0/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

[Ulf Hansson]

On Thu, 14 Jan 2021 at 16:28, Muhammad Husaini Zulkifli
<muhammad.husaini.zulkifli@intel.com> wrote:
>
> Hi,
>
> This patch series adds Ultra High Speed(UHS-1) Bus Speed Mode Support for Keem Bay SoC SD Card.
> Summary of each patches as per below:
>
> Patch 1: Use of_device_get_match_data() helper to get the match-data.
> Patch 2: Convert to use np pointer instead of using pdev->dev.of_node.
> Patch 3: Add struct device *dev in probe func(), so that dev pointer can be widely use in probe to make code more readable.
> Patch 4: Change from dev_err to dev_err_probe() to avoid spamming logs when probe is deferred.
> Patch 5: Export function to be use by device driver to configure i/o voltage rail output which communicate with Trusted Firmware.
> Patch 6: Update phy and regulator supply for Keem Bay SoC.
> Patch 7: Add DT Binding for Keem Bay SoC SD Regulator.
> Patch 8: Add SD Regulator driver to support Keem Bay SoC. This is to model using standard regulator abstraction during voltage operation
> as for Keem Bay SoC, i/o voltage rail need to be configure by setting specific bit in the AON_CFG1 Register.
> AON_CFG1 Register is a secure register. Direct access to AON_CFG1 register will cause firewall violation in secure system.
> Patch 9: Add Ultra High Speed (UHS-1) Support for Keem Bay SOC. For Keem Bay hardware, two regulators are been used to change the I/O bus line voltage which are "vqmmc-supply" and "sdvrail-supply".
>
> All of these patches was tested with Keem Bay evaluation module board.
>
> Kindly help to review this patch set.
>
> Muhammad Husaini Zulkifli (9):
>   mmc: sdhci-of-arasan: use of_device_get_match_data()
>   mmc: sdhci-of-arasan: Convert to use np instead of pdev->dev.of_node
>   mmc: sdhci-of-arasan: Add structure device pointer in probe function
>   mmc: sdhci-of-arasan: Use dev_err_probe() to avoid spamming logs
>   firmware: keembay: Add support for Trusted Firmware Service call
>   dt-bindings: mmc: Update phy and regulator supply for Keem Bay SOC
>   dt-bindings: regulator: keembay: Add DT binding documentation
>   regulator: keembay: Add regulator for Keem Bay SoC
>   mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC
>
>  .../devicetree/bindings/mmc/arasan,sdhci.yaml |   7 +-
>  .../bindings/regulator/keembay-regulator.yaml |  36 ++
>  drivers/mmc/host/sdhci-of-arasan.c            | 313 ++++++++++++++++--
>  drivers/regulator/Kconfig                     |  10 +
>  drivers/regulator/Makefile                    |   1 +
>  drivers/regulator/keembay-sd-regulator.c      | 112 +++++++
>  include/linux/firmware/intel/keembay.h        |  82 +++++
>  7 files changed, 532 insertions(+), 29 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regulator/keembay-regulator.yaml
>  create mode 100644 drivers/regulator/keembay-sd-regulator.c
>  create mode 100644 include/linux/firmware/intel/keembay.h
>
> --
> 2.17.1
>

Applied patch 1 to patch 4. I assume you will be respinning the rest?

Thanks and kind regards
Uffe

RE: [PATCH v1 0/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

[Zulkifli, Muhammad Husaini]

Hi Ulf,

>-----Original Message-----
>From: Ulf Hansson <ulf.hansson@linaro.org>
>Sent: Tuesday, January 19, 2021 9:43 PM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Mark Brown <broonie@kernel.org>; Liam Girdwood
><lgirdwood@gmail.com>; Rob Herring <robh+dt@kernel.org>; DTML
><devicetree@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>;
>Michal Simek <michal.simek@xilinx.com>; linux-mmc@vger.kernel.org; Linux
>Kernel Mailing List <linux-kernel@vger.kernel.org>; Shevchenko, Andriy
><andriy.shevchenko@intel.com>; A, Rashmi <rashmi.a@intel.com>; Vaidya,
>Mahesh R <mahesh.r.vaidya@intel.com>
>Subject: Re: [PATCH v1 0/9] mmc: sdhci-of-arasan: Add UHS-1 support for
>Keem Bay SOC
>
>On Thu, 14 Jan 2021 at 16:28, Muhammad Husaini Zulkifli
><muhammad.husaini.zulkifli@intel.com> wrote:
>>
>> Hi,
>>
>> This patch series adds Ultra High Speed(UHS-1) Bus Speed Mode Support
>for Keem Bay SoC SD Card.
>> Summary of each patches as per below:
>>
>> Patch 1: Use of_device_get_match_data() helper to get the match-data.
>> Patch 2: Convert to use np pointer instead of using pdev->dev.of_node.
>> Patch 3: Add struct device *dev in probe func(), so that dev pointer can be
>widely use in probe to make code more readable.
>> Patch 4: Change from dev_err to dev_err_probe() to avoid spamming logs
>when probe is deferred.
>> Patch 5: Export function to be use by device driver to configure i/o voltage
>rail output which communicate with Trusted Firmware.
>> Patch 6: Update phy and regulator supply for Keem Bay SoC.
>> Patch 7: Add DT Binding for Keem Bay SoC SD Regulator.
>> Patch 8: Add SD Regulator driver to support Keem Bay SoC. This is to
>> model using standard regulator abstraction during voltage operation as for
>Keem Bay SoC, i/o voltage rail need to be configure by setting specific bit in
>the AON_CFG1 Register.
>> AON_CFG1 Register is a secure register. Direct access to AON_CFG1 register
>will cause firewall violation in secure system.
>> Patch 9: Add Ultra High Speed (UHS-1) Support for Keem Bay SOC. For
>Keem Bay hardware, two regulators are been used to change the I/O bus line
>voltage which are "vqmmc-supply" and "sdvrail-supply".
>>
>> All of these patches was tested with Keem Bay evaluation module board.
>>
>> Kindly help to review this patch set.
>>
>> Muhammad Husaini Zulkifli (9):
>>   mmc: sdhci-of-arasan: use of_device_get_match_data()
>>   mmc: sdhci-of-arasan: Convert to use np instead of pdev->dev.of_node
>>   mmc: sdhci-of-arasan: Add structure device pointer in probe function
>>   mmc: sdhci-of-arasan: Use dev_err_probe() to avoid spamming logs
>>   firmware: keembay: Add support for Trusted Firmware Service call
>>   dt-bindings: mmc: Update phy and regulator supply for Keem Bay SOC
>>   dt-bindings: regulator: keembay: Add DT binding documentation
>>   regulator: keembay: Add regulator for Keem Bay SoC
>>   mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC
>>
>>  .../devicetree/bindings/mmc/arasan,sdhci.yaml |   7 +-
>>  .../bindings/regulator/keembay-regulator.yaml |  36 ++
>>  drivers/mmc/host/sdhci-of-arasan.c            | 313 ++++++++++++++++--
>>  drivers/regulator/Kconfig                     |  10 +
>>  drivers/regulator/Makefile                    |   1 +
>>  drivers/regulator/keembay-sd-regulator.c      | 112 +++++++
>>  include/linux/firmware/intel/keembay.h        |  82 +++++
>>  7 files changed, 532 insertions(+), 29 deletions(-)  create mode
>> 100644
>> Documentation/devicetree/bindings/regulator/keembay-regulator.yaml
>>  create mode 100644 drivers/regulator/keembay-sd-regulator.c
>>  create mode 100644 include/linux/firmware/intel/keembay.h
>>
>> --
>> 2.17.1
>>
>
>Applied patch 1 to patch 4. I assume you will be respinning the rest?

Thanks!!  Yeah. Seems like I need to request to ATF Team to implement SCMI voltage domain 
control first in ARM trusted firmware.

>
>Thanks and kind regards
>Uffe