[PATCH v2 0/3] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC

[PATCH v2 0/3] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC

[muhammad.husaini.zulkifli]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Hi.

The first patch is to document DT bindings for Keem Bay Firmware Driver and update
the Maintainers list.

The second patch is the firmware driver file.

The third patch is to enable UHS-1 Support for Keem Bay EVM.

The patch was tested with Keem Bay evaluation module board.

Kindly help to review this patch set.

Thank you.

Changes since v1:
- Add Document DT Bindings for Keembay Firmware.
- Created Firmware Driver to handle ATF Service call
- Provide API for arasan driver for sd card voltage changes

Muhammad Husaini Zulkifli (3):
  dt-bindings: arm: firmware: Add binding for Keem Bay Firmware Support
  firmware: Keem Bay: Add support for Arm Trusted Firmware Service call
  mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC

 .../arm/firmware/keembay,firmware.yaml        |  36 +++++
 MAINTAINERS                                   |   7 +
 drivers/firmware/Kconfig                      |   1 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/intel/Kconfig                |  14 ++
 drivers/firmware/intel/Makefile               |   4 +
 drivers/firmware/intel/keembay_smc.c          | 119 ++++++++++++++++
 drivers/mmc/host/sdhci-of-arasan.c            | 127 ++++++++++++++++++
 include/linux/firmware/intel/keembay_smc.h    |  27 ++++
 9 files changed, 336 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml
 create mode 100644 drivers/firmware/intel/Kconfig
 create mode 100644 drivers/firmware/intel/Makefile
 create mode 100644 drivers/firmware/intel/keembay_smc.c
 create mode 100644 include/linux/firmware/intel/keembay_smc.h

--
2.17.1

[PATCH v2 0/3] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC

[muhammad.husaini.zulkifli]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Hi.

The first patch is to document DT bindings for Keem Bay Firmware Driver and update
the Maintainers list.

The second patch is the firmware driver file.

The third patch is to enable UHS-1 Support for Keem Bay EVM.

The patch was tested with Keem Bay evaluation module board.

Kindly help to review this patch set.

Thank you.

Changes since v1:
- Add Document DT Bindings for Keembay Firmware.
- Created Firmware Driver to handle ATF Service call
- Provide API for arasan driver for sd card voltage changes

Muhammad Husaini Zulkifli (3):
  dt-bindings: arm: firmware: Add binding for Keem Bay Firmware Support
  firmware: Keem Bay: Add support for Arm Trusted Firmware Service call
  mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC

 .../arm/firmware/keembay,firmware.yaml        |  36 +++++
 MAINTAINERS                                   |   7 +
 drivers/firmware/Kconfig                      |   1 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/intel/Kconfig                |  14 ++
 drivers/firmware/intel/Makefile               |   4 +
 drivers/firmware/intel/keembay_smc.c          | 119 ++++++++++++++++
 drivers/mmc/host/sdhci-of-arasan.c            | 127 ++++++++++++++++++
 include/linux/firmware/intel/keembay_smc.h    |  27 ++++
 9 files changed, 336 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml
 create mode 100644 drivers/firmware/intel/Kconfig
 create mode 100644 drivers/firmware/intel/Makefile
 create mode 100644 drivers/firmware/intel/keembay_smc.c
 create mode 100644 include/linux/firmware/intel/keembay_smc.h

--
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/3] dt-bindings: arm: firmware: Add binding for Keem Bay Firmware Support

[muhammad.husaini.zulkifli]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Add bindings for Keem Bay implementation of Arm Trusted Firmware
Services call.

Update the MAINTAINERS list.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 .../arm/firmware/keembay,firmware.yaml        | 36 +++++++++++++++++++
 MAINTAINERS                                   |  7 ++++
 2 files changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml

diff --git a/Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml b/Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml
new file mode 100644
index 000000000000..9bb5a15f1e3c
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/firmware/keembay,firmware.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Keem Bay Device Tree Bindings
+
+maintainers:
+  - Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+
+properties:
+  $nodename:
+    const: keembay_firmware
+
+  compatible:
+    enum:
+      - keembay,firmware
+
+  method:
+   $ref: '/schemas/types.yaml#/definitions/string'
+   oneOf:
+      - enum:
+          - smc
+
+required:
+  - compatible
+  - method
+
+examples:
+  - |
+    keembay_firmware {
+        compatible = "keembay,firmware";
+        method = "smc";
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 190c7fa2ea01..33b8ded820f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19278,6 +19278,13 @@ L:	linux-mm@kvack.org
 S:	Maintained
 F:	mm/zswap.c
 
+KEEMBAY FIRMWARE
+M:	Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml
+F:	drivers/firmware/intel/keembay_smc.c
+F:	linux/include/linux/firmware/intel/keembay_smc.h
+
 THE REST
 M:	Linus Torvalds <torvalds@linux-foundation.org>
 L:	linux-kernel@vger.kernel.org
-- 
2.17.1

[PATCH v2 1/3] dt-bindings: arm: firmware: Add binding for Keem Bay Firmware Support

[muhammad.husaini.zulkifli]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Add bindings for Keem Bay implementation of Arm Trusted Firmware
Services call.

Update the MAINTAINERS list.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 .../arm/firmware/keembay,firmware.yaml        | 36 +++++++++++++++++++
 MAINTAINERS                                   |  7 ++++
 2 files changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml

diff --git a/Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml b/Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml
new file mode 100644
index 000000000000..9bb5a15f1e3c
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/firmware/keembay,firmware.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Keem Bay Device Tree Bindings
+
+maintainers:
+  - Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+
+properties:
+  $nodename:
+    const: keembay_firmware
+
+  compatible:
+    enum:
+      - keembay,firmware
+
+  method:
+   $ref: '/schemas/types.yaml#/definitions/string'
+   oneOf:
+      - enum:
+          - smc
+
+required:
+  - compatible
+  - method
+
+examples:
+  - |
+    keembay_firmware {
+        compatible = "keembay,firmware";
+        method = "smc";
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 190c7fa2ea01..33b8ded820f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19278,6 +19278,13 @@ L:	linux-mm@kvack.org
 S:	Maintained
 F:	mm/zswap.c
 
+KEEMBAY FIRMWARE
+M:	Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/arm/firmware/keembay,firmware.yaml
+F:	drivers/firmware/intel/keembay_smc.c
+F:	linux/include/linux/firmware/intel/keembay_smc.h
+
 THE REST
 M:	Linus Torvalds <torvalds@linux-foundation.org>
 L:	linux-kernel@vger.kernel.org
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[muhammad.husaini.zulkifli]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Add generic firmware driver for Keem Bay SOC to support
Arm Trusted Firmware Services call.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 drivers/firmware/Kconfig                   |   1 +
 drivers/firmware/Makefile                  |   1 +
 drivers/firmware/intel/Kconfig             |  14 +++
 drivers/firmware/intel/Makefile            |   4 +
 drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
 include/linux/firmware/intel/keembay_smc.h |  27 +++++
 6 files changed, 166 insertions(+)
 create mode 100644 drivers/firmware/intel/Kconfig
 create mode 100644 drivers/firmware/intel/Makefile
 create mode 100644 drivers/firmware/intel/keembay_smc.c
 create mode 100644 include/linux/firmware/intel/keembay_smc.h

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index fbd785dd0513..41de77d2720e 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
 source "drivers/firmware/smccc/Kconfig"
 source "drivers/firmware/tegra/Kconfig"
 source "drivers/firmware/xilinx/Kconfig"
+source "drivers/firmware/intel/Kconfig"
 
 endmenu
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 99510be9f5ed..00f295ab9860 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -33,3 +33,4 @@ obj-y				+= psci/
 obj-y				+= smccc/
 obj-y				+= tegra/
 obj-y				+= xilinx/
+obj-y				+= intel/
diff --git a/drivers/firmware/intel/Kconfig b/drivers/firmware/intel/Kconfig
new file mode 100644
index 000000000000..b2b7a4e5410b
--- /dev/null
+++ b/drivers/firmware/intel/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "Intel Firmware Drivers"
+
+config KEEMBAY_FIRMWARE
+	bool "Enable Keem Bay firmware interface support"
+	depends on HAVE_ARM_SMCCC
+	default n
+	help
+	  Firmware interface driver is used by device drivers
+	  to communicate with the arm-trusted-firmware
+	  for platform management services.
+	  If in doubt, say "N".
+
+endmenu
diff --git a/drivers/firmware/intel/Makefile b/drivers/firmware/intel/Makefile
new file mode 100644
index 000000000000..e6d2e1ea69a7
--- /dev/null
+++ b/drivers/firmware/intel/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for Intel firmwares
+
+obj-$(CONFIG_KEEMBAY_FIRMWARE) = keembay_smc.o
diff --git a/drivers/firmware/intel/keembay_smc.c b/drivers/firmware/intel/keembay_smc.c
new file mode 100644
index 000000000000..24013cd1f5da
--- /dev/null
+++ b/drivers/firmware/intel/keembay_smc.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2020-2021, Intel Corporation
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+
+#include <linux/firmware/intel/keembay_smc.h>
+
+static noinline int do_fw_call_fail(u64 arg0, u64 arg1)
+{
+	return -ENODEV;
+}
+
+/**
+ * Simple wrapper functions to be able to use a function pointer
+ * Invoke do_fw_call_smc or others in future, depending on the configuration
+ */
+static int (*do_fw_call)(u64, u64) = do_fw_call_fail;
+
+/**
+ * do_fw_call_smc() - Call system-level platform management layer (SMC)
+ * @arg0:		Argument 0 to SMC call
+ * @arg1:		Argument 1 to SMC call
+ *
+ * Invoke platform management function via SMC call.
+ *
+ * Return: Returns status, either success or error
+ */
+static noinline int do_fw_call_smc(u64 arg0, u64 arg1)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(arg0, arg1, 0, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+/**
+ * keembay_sd_voltage_selection() - Set the IO Pad voltage
+ * @volt: voltage selection either 1.8V or 3.3V
+ *
+ * This function is used to set the IO Line Voltage
+ *
+ * Return: 0 for success, Invalid for failure
+ */
+int keembay_sd_voltage_selection(int volt)
+{
+	return do_fw_call(KEEMBAY_SIP_FUNC_ID, volt);
+}
+EXPORT_SYMBOL_GPL(keembay_sd_voltage_selection);
+
+/**
+ * keembay_get_invoke_func() - method of call
+ * @np:	Pointer to the device_node structure
+ *
+ * Return: Returns 0 on success or error code
+ */
+static int keembay_get_invoke_func(struct device_node *np)
+{
+	const char *method;
+
+	if (of_property_read_string(np, "method", &method)) {
+		pr_warn("%s missing \"method\" property\n", __func__);
+		return -ENXIO;
+	}
+
+	if (!strcmp("smc", method)) {
+		do_fw_call = do_fw_call_smc;
+	} else {
+		pr_warn("%s Invalid \"method\" property: %s\n",
+			__func__, method);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int keembay_firmware_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = keembay_get_invoke_func(dev->of_node);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to get method of call\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int keembay_firmware_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id keembay_firmware_of_match[] = {
+	{.compatible = "keembay,firmware"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, keembay_firmware_of_match);
+
+static struct platform_driver keembay_firmware_driver = {
+	.driver = {
+		.name = "keembay_firmware",
+		.of_match_table = keembay_firmware_of_match,
+	},
+	.probe = keembay_firmware_probe,
+	.remove = keembay_firmware_remove,
+};
+module_platform_driver(keembay_firmware_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Intel Keembay SOC Firmware Layer");
+MODULE_AUTHOR("Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>");
diff --git a/include/linux/firmware/intel/keembay_smc.h b/include/linux/firmware/intel/keembay_smc.h
new file mode 100644
index 000000000000..87b1417f7e4e
--- /dev/null
+++ b/include/linux/firmware/intel/keembay_smc.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Intel Keembay SOC Firmware Layer
+ *
+ *  Copyright (C) 2020-2021, Intel Corporation
+ *
+ *  Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+ */
+
+#ifndef __FIRMWARE_KEEMBAY_SMC_H__
+#define __FIRMWARE_KEEMBAY_SMC_H__
+
+/* Setting for Keem Bay IO Pad Line Voltage Selection */
+#define KEEMBAY_SIP_FUNC_ID	0x8200ff26
+#define KEEMBAY_SET_1V8_VOLT	0x01
+#define KEEMBAY_SET_3V3_VOLT	0x00
+
+#if IS_ENABLED(CONFIG_KEEMBAY_FIRMWARE)
+int keembay_sd_voltage_selection(int volt);
+#else
+static inline int keembay_sd_voltage_selection(int volt)
+{
+	return -ENODEV;
+}
+#endif
+
+#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */
-- 
2.17.1

[PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[muhammad.husaini.zulkifli]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Add generic firmware driver for Keem Bay SOC to support
Arm Trusted Firmware Services call.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 drivers/firmware/Kconfig                   |   1 +
 drivers/firmware/Makefile                  |   1 +
 drivers/firmware/intel/Kconfig             |  14 +++
 drivers/firmware/intel/Makefile            |   4 +
 drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
 include/linux/firmware/intel/keembay_smc.h |  27 +++++
 6 files changed, 166 insertions(+)
 create mode 100644 drivers/firmware/intel/Kconfig
 create mode 100644 drivers/firmware/intel/Makefile
 create mode 100644 drivers/firmware/intel/keembay_smc.c
 create mode 100644 include/linux/firmware/intel/keembay_smc.h

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index fbd785dd0513..41de77d2720e 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
 source "drivers/firmware/smccc/Kconfig"
 source "drivers/firmware/tegra/Kconfig"
 source "drivers/firmware/xilinx/Kconfig"
+source "drivers/firmware/intel/Kconfig"
 
 endmenu
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 99510be9f5ed..00f295ab9860 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -33,3 +33,4 @@ obj-y				+= psci/
 obj-y				+= smccc/
 obj-y				+= tegra/
 obj-y				+= xilinx/
+obj-y				+= intel/
diff --git a/drivers/firmware/intel/Kconfig b/drivers/firmware/intel/Kconfig
new file mode 100644
index 000000000000..b2b7a4e5410b
--- /dev/null
+++ b/drivers/firmware/intel/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "Intel Firmware Drivers"
+
+config KEEMBAY_FIRMWARE
+	bool "Enable Keem Bay firmware interface support"
+	depends on HAVE_ARM_SMCCC
+	default n
+	help
+	  Firmware interface driver is used by device drivers
+	  to communicate with the arm-trusted-firmware
+	  for platform management services.
+	  If in doubt, say "N".
+
+endmenu
diff --git a/drivers/firmware/intel/Makefile b/drivers/firmware/intel/Makefile
new file mode 100644
index 000000000000..e6d2e1ea69a7
--- /dev/null
+++ b/drivers/firmware/intel/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for Intel firmwares
+
+obj-$(CONFIG_KEEMBAY_FIRMWARE) = keembay_smc.o
diff --git a/drivers/firmware/intel/keembay_smc.c b/drivers/firmware/intel/keembay_smc.c
new file mode 100644
index 000000000000..24013cd1f5da
--- /dev/null
+++ b/drivers/firmware/intel/keembay_smc.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2020-2021, Intel Corporation
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+
+#include <linux/firmware/intel/keembay_smc.h>
+
+static noinline int do_fw_call_fail(u64 arg0, u64 arg1)
+{
+	return -ENODEV;
+}
+
+/**
+ * Simple wrapper functions to be able to use a function pointer
+ * Invoke do_fw_call_smc or others in future, depending on the configuration
+ */
+static int (*do_fw_call)(u64, u64) = do_fw_call_fail;
+
+/**
+ * do_fw_call_smc() - Call system-level platform management layer (SMC)
+ * @arg0:		Argument 0 to SMC call
+ * @arg1:		Argument 1 to SMC call
+ *
+ * Invoke platform management function via SMC call.
+ *
+ * Return: Returns status, either success or error
+ */
+static noinline int do_fw_call_smc(u64 arg0, u64 arg1)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(arg0, arg1, 0, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+/**
+ * keembay_sd_voltage_selection() - Set the IO Pad voltage
+ * @volt: voltage selection either 1.8V or 3.3V
+ *
+ * This function is used to set the IO Line Voltage
+ *
+ * Return: 0 for success, Invalid for failure
+ */
+int keembay_sd_voltage_selection(int volt)
+{
+	return do_fw_call(KEEMBAY_SIP_FUNC_ID, volt);
+}
+EXPORT_SYMBOL_GPL(keembay_sd_voltage_selection);
+
+/**
+ * keembay_get_invoke_func() - method of call
+ * @np:	Pointer to the device_node structure
+ *
+ * Return: Returns 0 on success or error code
+ */
+static int keembay_get_invoke_func(struct device_node *np)
+{
+	const char *method;
+
+	if (of_property_read_string(np, "method", &method)) {
+		pr_warn("%s missing \"method\" property\n", __func__);
+		return -ENXIO;
+	}
+
+	if (!strcmp("smc", method)) {
+		do_fw_call = do_fw_call_smc;
+	} else {
+		pr_warn("%s Invalid \"method\" property: %s\n",
+			__func__, method);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int keembay_firmware_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = keembay_get_invoke_func(dev->of_node);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to get method of call\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int keembay_firmware_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id keembay_firmware_of_match[] = {
+	{.compatible = "keembay,firmware"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, keembay_firmware_of_match);
+
+static struct platform_driver keembay_firmware_driver = {
+	.driver = {
+		.name = "keembay_firmware",
+		.of_match_table = keembay_firmware_of_match,
+	},
+	.probe = keembay_firmware_probe,
+	.remove = keembay_firmware_remove,
+};
+module_platform_driver(keembay_firmware_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Intel Keembay SOC Firmware Layer");
+MODULE_AUTHOR("Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>");
diff --git a/include/linux/firmware/intel/keembay_smc.h b/include/linux/firmware/intel/keembay_smc.h
new file mode 100644
index 000000000000..87b1417f7e4e
--- /dev/null
+++ b/include/linux/firmware/intel/keembay_smc.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Intel Keembay SOC Firmware Layer
+ *
+ *  Copyright (C) 2020-2021, Intel Corporation
+ *
+ *  Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com>
+ */
+
+#ifndef __FIRMWARE_KEEMBAY_SMC_H__
+#define __FIRMWARE_KEEMBAY_SMC_H__
+
+/* Setting for Keem Bay IO Pad Line Voltage Selection */
+#define KEEMBAY_SIP_FUNC_ID	0x8200ff26
+#define KEEMBAY_SET_1V8_VOLT	0x01
+#define KEEMBAY_SET_3V3_VOLT	0x00
+
+#if IS_ENABLED(CONFIG_KEEMBAY_FIRMWARE)
+int keembay_sd_voltage_selection(int volt);
+#else
+static inline int keembay_sd_voltage_selection(int volt)
+{
+	return -ENODEV;
+}
+#endif
+
+#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/3] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC

[muhammad.husaini.zulkifli]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Voltage switching sequence is needed to support UHS-1 interface.
There are 2 places to control the voltage.
1) By setting the AON register using firmware driver calling
system-level platform management layer (SMC) to set the register.
2) By controlling the GPIO expander value to drive either 1.8V or 3.3V
for power mux input.

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

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index f186fbd016b1..73941418eaa7 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/gpio/consumer.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -23,6 +24,7 @@
 #include <linux/regmap.h>
 #include <linux/of.h>
 #include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/firmware/intel/keembay_smc.h>
 
 #include "cqhci.h"
 #include "sdhci-pltfm.h"
@@ -150,6 +152,7 @@ struct sdhci_arasan_data {
 	struct regmap	*soc_ctl_base;
 	const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
 	unsigned int	quirks;
+	struct gpio_desc *uhs_gpio;
 
 /* Controller does not have CD wired and will not function normally without */
 #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST	BIT(0)
@@ -361,6 +364,113 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
 	return -EINVAL;
 }
 
+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);
+	u16 ctrl_2;
+	u16 clk;
+	int ret;
+
+	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);
+
+		/*
+		 * Set VDDIO_B voltage to Low for 1.8V
+		 * which is controlling by GPIO Expander.
+		 */
+		gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0);
+
+		/*
+		 * This is like final gatekeeper. Need to ensure changed voltage
+		 * is settled before and after turn on this bit.
+		 */
+		usleep_range(1000, 1100);
+
+		ret = keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT);
+		if (ret)
+			return ret;
+
+		usleep_range(1000, 1100);
+
+		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);
+		break;
+	case MMC_SIGNAL_VOLTAGE_330:
+		/*
+		 * Set VDDIO_B voltage to High for 3.3V
+		 * which is controlling by GPIO Expander.
+		 */
+		gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 1);
+
+		/*
+		 * This is like final gatekeeper. Need to ensure changed voltage
+		 * is settled before and after turn on this bit.
+		 */
+		usleep_range(1000, 1100);
+
+		ret = keembay_sd_voltage_selection(KEEMBAY_SET_3V3_VOLT);
+		if (ret)
+			return ret;
+
+		usleep_range(1000, 1100);
+
+		/* 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;
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+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)
+{
+	if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180)
+		*drv_type = MMC_SET_DRIVER_TYPE_C;
+
+	return 0;
+}
+
 static const struct sdhci_ops sdhci_arasan_ops = {
 	.set_clock = sdhci_arasan_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
@@ -1521,6 +1631,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_arasan_data *sdhci_arasan;
 	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
 	const struct sdhci_arasan_of_data *data;
 
 	match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node);
@@ -1600,6 +1711,22 @@ 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")) {
+		struct gpio_desc *uhs;
+
+		uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH);
+		if (IS_ERR(uhs))
+			return dev_err_probe(dev, PTR_ERR(uhs), "can't get uhs gpio\n");
+
+		sdhci_arasan->uhs_gpio = uhs;
+
+		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, &pdev->dev);
-- 
2.17.1

[PATCH v2 3/3] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC

[muhammad.husaini.zulkifli]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Voltage switching sequence is needed to support UHS-1 interface.
There are 2 places to control the voltage.
1) By setting the AON register using firmware driver calling
system-level platform management layer (SMC) to set the register.
2) By controlling the GPIO expander value to drive either 1.8V or 3.3V
for power mux input.

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

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index f186fbd016b1..73941418eaa7 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/gpio/consumer.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -23,6 +24,7 @@
 #include <linux/regmap.h>
 #include <linux/of.h>
 #include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/firmware/intel/keembay_smc.h>
 
 #include "cqhci.h"
 #include "sdhci-pltfm.h"
@@ -150,6 +152,7 @@ struct sdhci_arasan_data {
 	struct regmap	*soc_ctl_base;
 	const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
 	unsigned int	quirks;
+	struct gpio_desc *uhs_gpio;
 
 /* Controller does not have CD wired and will not function normally without */
 #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST	BIT(0)
@@ -361,6 +364,113 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
 	return -EINVAL;
 }
 
+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);
+	u16 ctrl_2;
+	u16 clk;
+	int ret;
+
+	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);
+
+		/*
+		 * Set VDDIO_B voltage to Low for 1.8V
+		 * which is controlling by GPIO Expander.
+		 */
+		gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0);
+
+		/*
+		 * This is like final gatekeeper. Need to ensure changed voltage
+		 * is settled before and after turn on this bit.
+		 */
+		usleep_range(1000, 1100);
+
+		ret = keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT);
+		if (ret)
+			return ret;
+
+		usleep_range(1000, 1100);
+
+		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);
+		break;
+	case MMC_SIGNAL_VOLTAGE_330:
+		/*
+		 * Set VDDIO_B voltage to High for 3.3V
+		 * which is controlling by GPIO Expander.
+		 */
+		gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 1);
+
+		/*
+		 * This is like final gatekeeper. Need to ensure changed voltage
+		 * is settled before and after turn on this bit.
+		 */
+		usleep_range(1000, 1100);
+
+		ret = keembay_sd_voltage_selection(KEEMBAY_SET_3V3_VOLT);
+		if (ret)
+			return ret;
+
+		usleep_range(1000, 1100);
+
+		/* 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;
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+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)
+{
+	if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180)
+		*drv_type = MMC_SET_DRIVER_TYPE_C;
+
+	return 0;
+}
+
 static const struct sdhci_ops sdhci_arasan_ops = {
 	.set_clock = sdhci_arasan_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
@@ -1521,6 +1631,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_arasan_data *sdhci_arasan;
 	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
 	const struct sdhci_arasan_of_data *data;
 
 	match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node);
@@ -1600,6 +1711,22 @@ 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")) {
+		struct gpio_desc *uhs;
+
+		uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH);
+		if (IS_ERR(uhs))
+			return dev_err_probe(dev, PTR_ERR(uhs), "can't get uhs gpio\n");
+
+		sdhci_arasan->uhs_gpio = uhs;
+
+		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, &pdev->dev);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Thu, Oct 01, 2020 at 10:21:48PM +0800, muhammad.husaini.zulkifli@intel.com wrote:
> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> 
> Add generic firmware driver for Keem Bay SOC to support
> Arm Trusted Firmware Services call.
> 
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> ---
>  drivers/firmware/Kconfig                   |   1 +
>  drivers/firmware/Makefile                  |   1 +
>  drivers/firmware/intel/Kconfig             |  14 +++
>  drivers/firmware/intel/Makefile            |   4 +
>  drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
>  include/linux/firmware/intel/keembay_smc.h |  27 +++++
>  6 files changed, 166 insertions(+)
>  create mode 100644 drivers/firmware/intel/Kconfig
>  create mode 100644 drivers/firmware/intel/Makefile
>  create mode 100644 drivers/firmware/intel/keembay_smc.c
>  create mode 100644 include/linux/firmware/intel/keembay_smc.h
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index fbd785dd0513..41de77d2720e 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
>  source "drivers/firmware/smccc/Kconfig"
>  source "drivers/firmware/tegra/Kconfig"
>  source "drivers/firmware/xilinx/Kconfig"
> +source "drivers/firmware/intel/Kconfig"
>  
>  endmenu
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 99510be9f5ed..00f295ab9860 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -33,3 +33,4 @@ obj-y				+= psci/
>  obj-y				+= smccc/
>  obj-y				+= tegra/
>  obj-y				+= xilinx/
> +obj-y				+= intel/
> diff --git a/drivers/firmware/intel/Kconfig b/drivers/firmware/intel/Kconfig
> new file mode 100644
> index 000000000000..b2b7a4e5410b
> --- /dev/null
> +++ b/drivers/firmware/intel/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +menu "Intel Firmware Drivers"
> +
> +config KEEMBAY_FIRMWARE
> +	bool "Enable Keem Bay firmware interface support"
> +	depends on HAVE_ARM_SMCCC

What is the version of SMCCC implemented ?
If SMCCC v1.1+, use HAVE_ARM_SMCCC_DISCOVERY

> +	default n
> +	help
> +	  Firmware interface driver is used by device drivers
> +	  to communicate with the arm-trusted-firmware
> +	  for platform management services.
> +	  If in doubt, say "N".
> +
> +endmenu
> diff --git a/drivers/firmware/intel/Makefile b/drivers/firmware/intel/Makefile
> new file mode 100644
> index 000000000000..e6d2e1ea69a7
> --- /dev/null
> +++ b/drivers/firmware/intel/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Makefile for Intel firmwares
> +
> +obj-$(CONFIG_KEEMBAY_FIRMWARE) = keembay_smc.o
> diff --git a/drivers/firmware/intel/keembay_smc.c b/drivers/firmware/intel/keembay_smc.c
> new file mode 100644
> index 000000000000..24013cd1f5da
> --- /dev/null
> +++ b/drivers/firmware/intel/keembay_smc.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2020-2021, Intel Corporation
> + */
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +
> +#include <linux/firmware/intel/keembay_smc.h>
> +
> +static noinline int do_fw_call_fail(u64 arg0, u64 arg1)
> +{
> +	return -ENODEV;
> +}
> +
> +/**
> + * Simple wrapper functions to be able to use a function pointer
> + * Invoke do_fw_call_smc or others in future, depending on the configuration
> + */
> +static int (*do_fw_call)(u64, u64) = do_fw_call_fail;
> +
> +/**
> + * do_fw_call_smc() - Call system-level platform management layer (SMC)
> + * @arg0:		Argument 0 to SMC call
> + * @arg1:		Argument 1 to SMC call
> + *
> + * Invoke platform management function via SMC call.
> + *
> + * Return: Returns status, either success or error
> + */
> +static noinline int do_fw_call_smc(u64 arg0, u64 arg1)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(arg0, arg1, 0, 0, 0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +/**
> + * keembay_sd_voltage_selection() - Set the IO Pad voltage
> + * @volt: voltage selection either 1.8V or 3.3V
> + *
> + * This function is used to set the IO Line Voltage
> + *
> + * Return: 0 for success, Invalid for failure
> + */
> +int keembay_sd_voltage_selection(int volt)
> +{
> +	return do_fw_call(KEEMBAY_SIP_FUNC_ID, volt);


What are the other uses of this KEEMBAY_SIP_* ?
For now I tend to move this to the driver making use of it using
arm_smccc_1_1_invoke directly if possible. I don't see the need for this
to be separate driver. But do let us know the features implemented in the
firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
for some CPU errata implementation.

-- 
Regards,
Sudeep

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Thu, Oct 01, 2020 at 10:21:48PM +0800, muhammad.husaini.zulkifli@intel.com wrote:
> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> 
> Add generic firmware driver for Keem Bay SOC to support
> Arm Trusted Firmware Services call.
> 
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> ---
>  drivers/firmware/Kconfig                   |   1 +
>  drivers/firmware/Makefile                  |   1 +
>  drivers/firmware/intel/Kconfig             |  14 +++
>  drivers/firmware/intel/Makefile            |   4 +
>  drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
>  include/linux/firmware/intel/keembay_smc.h |  27 +++++
>  6 files changed, 166 insertions(+)
>  create mode 100644 drivers/firmware/intel/Kconfig
>  create mode 100644 drivers/firmware/intel/Makefile
>  create mode 100644 drivers/firmware/intel/keembay_smc.c
>  create mode 100644 include/linux/firmware/intel/keembay_smc.h
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index fbd785dd0513..41de77d2720e 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
>  source "drivers/firmware/smccc/Kconfig"
>  source "drivers/firmware/tegra/Kconfig"
>  source "drivers/firmware/xilinx/Kconfig"
> +source "drivers/firmware/intel/Kconfig"
>  
>  endmenu
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 99510be9f5ed..00f295ab9860 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -33,3 +33,4 @@ obj-y				+= psci/
>  obj-y				+= smccc/
>  obj-y				+= tegra/
>  obj-y				+= xilinx/
> +obj-y				+= intel/
> diff --git a/drivers/firmware/intel/Kconfig b/drivers/firmware/intel/Kconfig
> new file mode 100644
> index 000000000000..b2b7a4e5410b
> --- /dev/null
> +++ b/drivers/firmware/intel/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +menu "Intel Firmware Drivers"
> +
> +config KEEMBAY_FIRMWARE
> +	bool "Enable Keem Bay firmware interface support"
> +	depends on HAVE_ARM_SMCCC

What is the version of SMCCC implemented ?
If SMCCC v1.1+, use HAVE_ARM_SMCCC_DISCOVERY

> +	default n
> +	help
> +	  Firmware interface driver is used by device drivers
> +	  to communicate with the arm-trusted-firmware
> +	  for platform management services.
> +	  If in doubt, say "N".
> +
> +endmenu
> diff --git a/drivers/firmware/intel/Makefile b/drivers/firmware/intel/Makefile
> new file mode 100644
> index 000000000000..e6d2e1ea69a7
> --- /dev/null
> +++ b/drivers/firmware/intel/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Makefile for Intel firmwares
> +
> +obj-$(CONFIG_KEEMBAY_FIRMWARE) = keembay_smc.o
> diff --git a/drivers/firmware/intel/keembay_smc.c b/drivers/firmware/intel/keembay_smc.c
> new file mode 100644
> index 000000000000..24013cd1f5da
> --- /dev/null
> +++ b/drivers/firmware/intel/keembay_smc.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2020-2021, Intel Corporation
> + */
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +
> +#include <linux/firmware/intel/keembay_smc.h>
> +
> +static noinline int do_fw_call_fail(u64 arg0, u64 arg1)
> +{
> +	return -ENODEV;
> +}
> +
> +/**
> + * Simple wrapper functions to be able to use a function pointer
> + * Invoke do_fw_call_smc or others in future, depending on the configuration
> + */
> +static int (*do_fw_call)(u64, u64) = do_fw_call_fail;
> +
> +/**
> + * do_fw_call_smc() - Call system-level platform management layer (SMC)
> + * @arg0:		Argument 0 to SMC call
> + * @arg1:		Argument 1 to SMC call
> + *
> + * Invoke platform management function via SMC call.
> + *
> + * Return: Returns status, either success or error
> + */
> +static noinline int do_fw_call_smc(u64 arg0, u64 arg1)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(arg0, arg1, 0, 0, 0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +/**
> + * keembay_sd_voltage_selection() - Set the IO Pad voltage
> + * @volt: voltage selection either 1.8V or 3.3V
> + *
> + * This function is used to set the IO Line Voltage
> + *
> + * Return: 0 for success, Invalid for failure
> + */
> +int keembay_sd_voltage_selection(int volt)
> +{
> +	return do_fw_call(KEEMBAY_SIP_FUNC_ID, volt);


What are the other uses of this KEEMBAY_SIP_* ?
For now I tend to move this to the driver making use of it using
arm_smccc_1_1_invoke directly if possible. I don't see the need for this
to be separate driver. But do let us know the features implemented in the
firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
for some CPU errata implementation.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Michal Simek]

Hi Sudeep,

On 01. 10. 20 17:35, Sudeep Holla wrote:
> On Thu, Oct 01, 2020 at 10:21:48PM +0800, muhammad.husaini.zulkifli@intel.com wrote:
>> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
>>
>> Add generic firmware driver for Keem Bay SOC to support
>> Arm Trusted Firmware Services call.
>>
>> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
>> ---
>>  drivers/firmware/Kconfig                   |   1 +
>>  drivers/firmware/Makefile                  |   1 +
>>  drivers/firmware/intel/Kconfig             |  14 +++
>>  drivers/firmware/intel/Makefile            |   4 +
>>  drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
>>  include/linux/firmware/intel/keembay_smc.h |  27 +++++
>>  6 files changed, 166 insertions(+)
>>  create mode 100644 drivers/firmware/intel/Kconfig
>>  create mode 100644 drivers/firmware/intel/Makefile
>>  create mode 100644 drivers/firmware/intel/keembay_smc.c
>>  create mode 100644 include/linux/firmware/intel/keembay_smc.h
>>
>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>> index fbd785dd0513..41de77d2720e 100644
>> --- a/drivers/firmware/Kconfig
>> +++ b/drivers/firmware/Kconfig
>> @@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
>>  source "drivers/firmware/smccc/Kconfig"
>>  source "drivers/firmware/tegra/Kconfig"
>>  source "drivers/firmware/xilinx/Kconfig"
>> +source "drivers/firmware/intel/Kconfig"
>>  
>>  endmenu
>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
>> index 99510be9f5ed..00f295ab9860 100644
>> --- a/drivers/firmware/Makefile
>> +++ b/drivers/firmware/Makefile
>> @@ -33,3 +33,4 @@ obj-y				+= psci/
>>  obj-y				+= smccc/
>>  obj-y				+= tegra/
>>  obj-y				+= xilinx/
>> +obj-y				+= intel/
>> diff --git a/drivers/firmware/intel/Kconfig b/drivers/firmware/intel/Kconfig
>> new file mode 100644
>> index 000000000000..b2b7a4e5410b
>> --- /dev/null
>> +++ b/drivers/firmware/intel/Kconfig
>> @@ -0,0 +1,14 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +menu "Intel Firmware Drivers"
>> +
>> +config KEEMBAY_FIRMWARE
>> +	bool "Enable Keem Bay firmware interface support"
>> +	depends on HAVE_ARM_SMCCC
> 
> What is the version of SMCCC implemented ?
> If SMCCC v1.1+, use HAVE_ARM_SMCCC_DISCOVERY
> 
>> +	default n
>> +	help
>> +	  Firmware interface driver is used by device drivers
>> +	  to communicate with the arm-trusted-firmware
>> +	  for platform management services.
>> +	  If in doubt, say "N".
>> +
>> +endmenu
>> diff --git a/drivers/firmware/intel/Makefile b/drivers/firmware/intel/Makefile
>> new file mode 100644
>> index 000000000000..e6d2e1ea69a7
>> --- /dev/null
>> +++ b/drivers/firmware/intel/Makefile
>> @@ -0,0 +1,4 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Makefile for Intel firmwares
>> +
>> +obj-$(CONFIG_KEEMBAY_FIRMWARE) = keembay_smc.o
>> diff --git a/drivers/firmware/intel/keembay_smc.c b/drivers/firmware/intel/keembay_smc.c
>> new file mode 100644
>> index 000000000000..24013cd1f5da
>> --- /dev/null
>> +++ b/drivers/firmware/intel/keembay_smc.c
>> @@ -0,0 +1,119 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *  Copyright (C) 2020-2021, Intel Corporation
>> + */
>> +
>> +#include <linux/arm-smccc.h>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +
>> +#include <linux/firmware/intel/keembay_smc.h>
>> +
>> +static noinline int do_fw_call_fail(u64 arg0, u64 arg1)
>> +{
>> +	return -ENODEV;
>> +}
>> +
>> +/**
>> + * Simple wrapper functions to be able to use a function pointer
>> + * Invoke do_fw_call_smc or others in future, depending on the configuration
>> + */
>> +static int (*do_fw_call)(u64, u64) = do_fw_call_fail;
>> +
>> +/**
>> + * do_fw_call_smc() - Call system-level platform management layer (SMC)
>> + * @arg0:		Argument 0 to SMC call
>> + * @arg1:		Argument 1 to SMC call
>> + *
>> + * Invoke platform management function via SMC call.
>> + *
>> + * Return: Returns status, either success or error
>> + */
>> +static noinline int do_fw_call_smc(u64 arg0, u64 arg1)
>> +{
>> +	struct arm_smccc_res res;
>> +
>> +	arm_smccc_smc(arg0, arg1, 0, 0, 0, 0, 0, 0, &res);
>> +
>> +	return res.a0;
>> +}
>> +
>> +/**
>> + * keembay_sd_voltage_selection() - Set the IO Pad voltage
>> + * @volt: voltage selection either 1.8V or 3.3V
>> + *
>> + * This function is used to set the IO Line Voltage
>> + *
>> + * Return: 0 for success, Invalid for failure
>> + */
>> +int keembay_sd_voltage_selection(int volt)
>> +{
>> +	return do_fw_call(KEEMBAY_SIP_FUNC_ID, volt);
> 
> 
> What are the other uses of this KEEMBAY_SIP_* ?
> For now I tend to move this to the driver making use of it using
> arm_smccc_1_1_invoke directly if possible. I don't see the need for this
> to be separate driver. But do let us know the features implemented in the
> firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
> for some CPU errata implementation.

This driver has been created based on my request to move it out the mmc
driver. It looks quite hacky to have arm_smccc_res and call
arm_smccc_smc() also with some IDs where it is visible that the part of
ID is just based on any spec.
Also in v1 he is just calling SMC. But maybe there is going a need to
call HVC instead which is something what device driver shouldn't decide
that's why IMHO doing step via firmware driver is much better approach.
Of course if there is a better/cleaner way how this should be done I am
happy to get more information about it.

Thanks,
Michal

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Michal Simek]

Hi Sudeep,

On 01. 10. 20 17:35, Sudeep Holla wrote:
> On Thu, Oct 01, 2020 at 10:21:48PM +0800, muhammad.husaini.zulkifli@intel.com wrote:
>> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
>>
>> Add generic firmware driver for Keem Bay SOC to support
>> Arm Trusted Firmware Services call.
>>
>> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
>> ---
>>  drivers/firmware/Kconfig                   |   1 +
>>  drivers/firmware/Makefile                  |   1 +
>>  drivers/firmware/intel/Kconfig             |  14 +++
>>  drivers/firmware/intel/Makefile            |   4 +
>>  drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
>>  include/linux/firmware/intel/keembay_smc.h |  27 +++++
>>  6 files changed, 166 insertions(+)
>>  create mode 100644 drivers/firmware/intel/Kconfig
>>  create mode 100644 drivers/firmware/intel/Makefile
>>  create mode 100644 drivers/firmware/intel/keembay_smc.c
>>  create mode 100644 include/linux/firmware/intel/keembay_smc.h
>>
>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>> index fbd785dd0513..41de77d2720e 100644
>> --- a/drivers/firmware/Kconfig
>> +++ b/drivers/firmware/Kconfig
>> @@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
>>  source "drivers/firmware/smccc/Kconfig"
>>  source "drivers/firmware/tegra/Kconfig"
>>  source "drivers/firmware/xilinx/Kconfig"
>> +source "drivers/firmware/intel/Kconfig"
>>  
>>  endmenu
>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
>> index 99510be9f5ed..00f295ab9860 100644
>> --- a/drivers/firmware/Makefile
>> +++ b/drivers/firmware/Makefile
>> @@ -33,3 +33,4 @@ obj-y				+= psci/
>>  obj-y				+= smccc/
>>  obj-y				+= tegra/
>>  obj-y				+= xilinx/
>> +obj-y				+= intel/
>> diff --git a/drivers/firmware/intel/Kconfig b/drivers/firmware/intel/Kconfig
>> new file mode 100644
>> index 000000000000..b2b7a4e5410b
>> --- /dev/null
>> +++ b/drivers/firmware/intel/Kconfig
>> @@ -0,0 +1,14 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +menu "Intel Firmware Drivers"
>> +
>> +config KEEMBAY_FIRMWARE
>> +	bool "Enable Keem Bay firmware interface support"
>> +	depends on HAVE_ARM_SMCCC
> 
> What is the version of SMCCC implemented ?
> If SMCCC v1.1+, use HAVE_ARM_SMCCC_DISCOVERY
> 
>> +	default n
>> +	help
>> +	  Firmware interface driver is used by device drivers
>> +	  to communicate with the arm-trusted-firmware
>> +	  for platform management services.
>> +	  If in doubt, say "N".
>> +
>> +endmenu
>> diff --git a/drivers/firmware/intel/Makefile b/drivers/firmware/intel/Makefile
>> new file mode 100644
>> index 000000000000..e6d2e1ea69a7
>> --- /dev/null
>> +++ b/drivers/firmware/intel/Makefile
>> @@ -0,0 +1,4 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Makefile for Intel firmwares
>> +
>> +obj-$(CONFIG_KEEMBAY_FIRMWARE) = keembay_smc.o
>> diff --git a/drivers/firmware/intel/keembay_smc.c b/drivers/firmware/intel/keembay_smc.c
>> new file mode 100644
>> index 000000000000..24013cd1f5da
>> --- /dev/null
>> +++ b/drivers/firmware/intel/keembay_smc.c
>> @@ -0,0 +1,119 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *  Copyright (C) 2020-2021, Intel Corporation
>> + */
>> +
>> +#include <linux/arm-smccc.h>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +
>> +#include <linux/firmware/intel/keembay_smc.h>
>> +
>> +static noinline int do_fw_call_fail(u64 arg0, u64 arg1)
>> +{
>> +	return -ENODEV;
>> +}
>> +
>> +/**
>> + * Simple wrapper functions to be able to use a function pointer
>> + * Invoke do_fw_call_smc or others in future, depending on the configuration
>> + */
>> +static int (*do_fw_call)(u64, u64) = do_fw_call_fail;
>> +
>> +/**
>> + * do_fw_call_smc() - Call system-level platform management layer (SMC)
>> + * @arg0:		Argument 0 to SMC call
>> + * @arg1:		Argument 1 to SMC call
>> + *
>> + * Invoke platform management function via SMC call.
>> + *
>> + * Return: Returns status, either success or error
>> + */
>> +static noinline int do_fw_call_smc(u64 arg0, u64 arg1)
>> +{
>> +	struct arm_smccc_res res;
>> +
>> +	arm_smccc_smc(arg0, arg1, 0, 0, 0, 0, 0, 0, &res);
>> +
>> +	return res.a0;
>> +}
>> +
>> +/**
>> + * keembay_sd_voltage_selection() - Set the IO Pad voltage
>> + * @volt: voltage selection either 1.8V or 3.3V
>> + *
>> + * This function is used to set the IO Line Voltage
>> + *
>> + * Return: 0 for success, Invalid for failure
>> + */
>> +int keembay_sd_voltage_selection(int volt)
>> +{
>> +	return do_fw_call(KEEMBAY_SIP_FUNC_ID, volt);
> 
> 
> What are the other uses of this KEEMBAY_SIP_* ?
> For now I tend to move this to the driver making use of it using
> arm_smccc_1_1_invoke directly if possible. I don't see the need for this
> to be separate driver. But do let us know the features implemented in the
> firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
> for some CPU errata implementation.

This driver has been created based on my request to move it out the mmc
driver. It looks quite hacky to have arm_smccc_res and call
arm_smccc_smc() also with some IDs where it is visible that the part of
ID is just based on any spec.
Also in v1 he is just calling SMC. But maybe there is going a need to
call HVC instead which is something what device driver shouldn't decide
that's why IMHO doing step via firmware driver is much better approach.
Of course if there is a better/cleaner way how this should be done I am
happy to get more information about it.

Thanks,
Michal




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Sudeep,

>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: Friday, October 2, 2020 4:23 PM
>To: Sudeep Holla <sudeep.holla@arm.com>; Zulkifli, Muhammad Husaini
><muhammad.husaini.zulkifli@intel.com>
>Cc: Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com;
>ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Raja Subramanian,
>Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan
>Mohamad, Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>Hi Sudeep,
>
>On 01. 10. 20 17:35, Sudeep Holla wrote:
>> On Thu, Oct 01, 2020 at 10:21:48PM +0800,
>muhammad.husaini.zulkifli@intel.com wrote:
>>> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
>>>
>>> Add generic firmware driver for Keem Bay SOC to support Arm Trusted
>>> Firmware Services call.
>>>
>>> Signed-off-by: Muhammad Husaini Zulkifli
>>> <muhammad.husaini.zulkifli@intel.com>
>>> ---
>>>  drivers/firmware/Kconfig                   |   1 +
>>>  drivers/firmware/Makefile                  |   1 +
>>>  drivers/firmware/intel/Kconfig             |  14 +++
>>>  drivers/firmware/intel/Makefile            |   4 +
>>>  drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
>>>  include/linux/firmware/intel/keembay_smc.h |  27 +++++
>>>  6 files changed, 166 insertions(+)
>>>  create mode 100644 drivers/firmware/intel/Kconfig  create mode
>>> 100644 drivers/firmware/intel/Makefile  create mode 100644
>>> drivers/firmware/intel/keembay_smc.c
>>>  create mode 100644 include/linux/firmware/intel/keembay_smc.h
>>>
>>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>>> index fbd785dd0513..41de77d2720e 100644
>>> --- a/drivers/firmware/Kconfig
>>> +++ b/drivers/firmware/Kconfig
>>> @@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
>>>  source "drivers/firmware/smccc/Kconfig"
>>>  source "drivers/firmware/tegra/Kconfig"
>>>  source "drivers/firmware/xilinx/Kconfig"
>>> +source "drivers/firmware/intel/Kconfig"
>>>
>>>  endmenu
>>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
>>> index 99510be9f5ed..00f295ab9860 100644
>>> --- a/drivers/firmware/Makefile
>>> +++ b/drivers/firmware/Makefile
>>> @@ -33,3 +33,4 @@ obj-y				+= psci/
>>>  obj-y				+= smccc/
>>>  obj-y				+= tegra/
>>>  obj-y				+= xilinx/
>>> +obj-y				+= intel/
>>> diff --git a/drivers/firmware/intel/Kconfig
>>> b/drivers/firmware/intel/Kconfig new file mode 100644 index
>>> 000000000000..b2b7a4e5410b
>>> --- /dev/null
>>> +++ b/drivers/firmware/intel/Kconfig
>>> @@ -0,0 +1,14 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only menu "Intel Firmware
>>> +Drivers"
>>> +
>>> +config KEEMBAY_FIRMWARE
>>> +	bool "Enable Keem Bay firmware interface support"
>>> +	depends on HAVE_ARM_SMCCC
>>
>> What is the version of SMCCC implemented ?
Our current Arm Trusted Firmware framework supports v1.1.
Does it mean I should use HAVE_ARM_SMCCC_DISCOVERY?
Not really clear about this. As for HAVE_ARM_SMCCC will include 
support for the SMC and HVC.

>> If SMCCC v1.1+, use HAVE_ARM_SMCCC_DISCOVERY
>>
>>> +	default n
>>> +	help
>>> +	  Firmware interface driver is used by device drivers
>>> +	  to communicate with the arm-trusted-firmware
>>> +	  for platform management services.
>>> +	  If in doubt, say "N".
>>> +
>>> +endmenu
>>> diff --git a/drivers/firmware/intel/Makefile
>>> b/drivers/firmware/intel/Makefile new file mode 100644 index
>>> 000000000000..e6d2e1ea69a7
>>> --- /dev/null
>>> +++ b/drivers/firmware/intel/Makefile
>>> @@ -0,0 +1,4 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# Makefile for Intel firmwares
>>> +
>>> +obj-$(CONFIG_KEEMBAY_FIRMWARE) = keembay_smc.o
>>> diff --git a/drivers/firmware/intel/keembay_smc.c
>>> b/drivers/firmware/intel/keembay_smc.c
>>> new file mode 100644
>>> index 000000000000..24013cd1f5da
>>> --- /dev/null
>>> +++ b/drivers/firmware/intel/keembay_smc.c
>>> @@ -0,0 +1,119 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + *  Copyright (C) 2020-2021, Intel Corporation  */
>>> +
>>> +#include <linux/arm-smccc.h>
>>> +#include <linux/init.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_platform.h>
>>> +
>>> +#include <linux/firmware/intel/keembay_smc.h>
>>> +
>>> +static noinline int do_fw_call_fail(u64 arg0, u64 arg1) {
>>> +	return -ENODEV;
>>> +}
>>> +
>>> +/**
>>> + * Simple wrapper functions to be able to use a function pointer
>>> + * Invoke do_fw_call_smc or others in future, depending on the
>>> +configuration  */ static int (*do_fw_call)(u64, u64) =
>>> +do_fw_call_fail;
>>> +
>>> +/**
>>> + * do_fw_call_smc() - Call system-level platform management layer (SMC)
>>> + * @arg0:		Argument 0 to SMC call
>>> + * @arg1:		Argument 1 to SMC call
>>> + *
>>> + * Invoke platform management function via SMC call.
>>> + *
>>> + * Return: Returns status, either success or error  */ static
>>> +noinline int do_fw_call_smc(u64 arg0, u64 arg1) {
>>> +	struct arm_smccc_res res;
>>> +
>>> +	arm_smccc_smc(arg0, arg1, 0, 0, 0, 0, 0, 0, &res);
>>> +
>>> +	return res.a0;
>>> +}
>>> +
>>> +/**
>>> + * keembay_sd_voltage_selection() - Set the IO Pad voltage
>>> + * @volt: voltage selection either 1.8V or 3.3V
>>> + *
>>> + * This function is used to set the IO Line Voltage
>>> + *
>>> + * Return: 0 for success, Invalid for failure  */ int
>>> +keembay_sd_voltage_selection(int volt) {
>>> +	return do_fw_call(KEEMBAY_SIP_FUNC_ID, volt);
>>
>>
>> What are the other uses of this KEEMBAY_SIP_* ?
>> For now I tend to move this to the driver making use of it using
>> arm_smccc_1_1_invoke directly if possible. I don't see the need for
>> this to be separate driver. But do let us know the features
>> implemented in the firmware. If it is not v1.1+, reasons for not
>> upgrading as you need v1.1 for some CPU errata implementation.
>
>This driver has been created based on my request to move it out the mmc
>driver. It looks quite hacky to have arm_smccc_res and call
>arm_smccc_smc() also with some IDs where it is visible that the part of ID is just
>based on any spec.
>Also in v1 he is just calling SMC. But maybe there is going a need to call HVC
>instead which is something what device driver shouldn't decide that's why
>IMHO doing step via firmware driver is much better approach.
>Of course if there is a better/cleaner way how this should be done I am happy
>to get more information about it.
>
>Thanks,
>Michal
>
>

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Sudeep,

>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: Friday, October 2, 2020 4:23 PM
>To: Sudeep Holla <sudeep.holla@arm.com>; Zulkifli, Muhammad Husaini
><muhammad.husaini.zulkifli@intel.com>
>Cc: Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com;
>ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Raja Subramanian,
>Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan
>Mohamad, Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>Hi Sudeep,
>
>On 01. 10. 20 17:35, Sudeep Holla wrote:
>> On Thu, Oct 01, 2020 at 10:21:48PM +0800,
>muhammad.husaini.zulkifli@intel.com wrote:
>>> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
>>>
>>> Add generic firmware driver for Keem Bay SOC to support Arm Trusted
>>> Firmware Services call.
>>>
>>> Signed-off-by: Muhammad Husaini Zulkifli
>>> <muhammad.husaini.zulkifli@intel.com>
>>> ---
>>>  drivers/firmware/Kconfig                   |   1 +
>>>  drivers/firmware/Makefile                  |   1 +
>>>  drivers/firmware/intel/Kconfig             |  14 +++
>>>  drivers/firmware/intel/Makefile            |   4 +
>>>  drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
>>>  include/linux/firmware/intel/keembay_smc.h |  27 +++++
>>>  6 files changed, 166 insertions(+)
>>>  create mode 100644 drivers/firmware/intel/Kconfig  create mode
>>> 100644 drivers/firmware/intel/Makefile  create mode 100644
>>> drivers/firmware/intel/keembay_smc.c
>>>  create mode 100644 include/linux/firmware/intel/keembay_smc.h
>>>
>>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>>> index fbd785dd0513..41de77d2720e 100644
>>> --- a/drivers/firmware/Kconfig
>>> +++ b/drivers/firmware/Kconfig
>>> @@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
>>>  source "drivers/firmware/smccc/Kconfig"
>>>  source "drivers/firmware/tegra/Kconfig"
>>>  source "drivers/firmware/xilinx/Kconfig"
>>> +source "drivers/firmware/intel/Kconfig"
>>>
>>>  endmenu
>>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
>>> index 99510be9f5ed..00f295ab9860 100644
>>> --- a/drivers/firmware/Makefile
>>> +++ b/drivers/firmware/Makefile
>>> @@ -33,3 +33,4 @@ obj-y				+= psci/
>>>  obj-y				+= smccc/
>>>  obj-y				+= tegra/
>>>  obj-y				+= xilinx/
>>> +obj-y				+= intel/
>>> diff --git a/drivers/firmware/intel/Kconfig
>>> b/drivers/firmware/intel/Kconfig new file mode 100644 index
>>> 000000000000..b2b7a4e5410b
>>> --- /dev/null
>>> +++ b/drivers/firmware/intel/Kconfig
>>> @@ -0,0 +1,14 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only menu "Intel Firmware
>>> +Drivers"
>>> +
>>> +config KEEMBAY_FIRMWARE
>>> +	bool "Enable Keem Bay firmware interface support"
>>> +	depends on HAVE_ARM_SMCCC
>>
>> What is the version of SMCCC implemented ?
Our current Arm Trusted Firmware framework supports v1.1.
Does it mean I should use HAVE_ARM_SMCCC_DISCOVERY?
Not really clear about this. As for HAVE_ARM_SMCCC will include 
support for the SMC and HVC.

>> If SMCCC v1.1+, use HAVE_ARM_SMCCC_DISCOVERY
>>
>>> +	default n
>>> +	help
>>> +	  Firmware interface driver is used by device drivers
>>> +	  to communicate with the arm-trusted-firmware
>>> +	  for platform management services.
>>> +	  If in doubt, say "N".
>>> +
>>> +endmenu
>>> diff --git a/drivers/firmware/intel/Makefile
>>> b/drivers/firmware/intel/Makefile new file mode 100644 index
>>> 000000000000..e6d2e1ea69a7
>>> --- /dev/null
>>> +++ b/drivers/firmware/intel/Makefile
>>> @@ -0,0 +1,4 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# Makefile for Intel firmwares
>>> +
>>> +obj-$(CONFIG_KEEMBAY_FIRMWARE) = keembay_smc.o
>>> diff --git a/drivers/firmware/intel/keembay_smc.c
>>> b/drivers/firmware/intel/keembay_smc.c
>>> new file mode 100644
>>> index 000000000000..24013cd1f5da
>>> --- /dev/null
>>> +++ b/drivers/firmware/intel/keembay_smc.c
>>> @@ -0,0 +1,119 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + *  Copyright (C) 2020-2021, Intel Corporation  */
>>> +
>>> +#include <linux/arm-smccc.h>
>>> +#include <linux/init.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_platform.h>
>>> +
>>> +#include <linux/firmware/intel/keembay_smc.h>
>>> +
>>> +static noinline int do_fw_call_fail(u64 arg0, u64 arg1) {
>>> +	return -ENODEV;
>>> +}
>>> +
>>> +/**
>>> + * Simple wrapper functions to be able to use a function pointer
>>> + * Invoke do_fw_call_smc or others in future, depending on the
>>> +configuration  */ static int (*do_fw_call)(u64, u64) =
>>> +do_fw_call_fail;
>>> +
>>> +/**
>>> + * do_fw_call_smc() - Call system-level platform management layer (SMC)
>>> + * @arg0:		Argument 0 to SMC call
>>> + * @arg1:		Argument 1 to SMC call
>>> + *
>>> + * Invoke platform management function via SMC call.
>>> + *
>>> + * Return: Returns status, either success or error  */ static
>>> +noinline int do_fw_call_smc(u64 arg0, u64 arg1) {
>>> +	struct arm_smccc_res res;
>>> +
>>> +	arm_smccc_smc(arg0, arg1, 0, 0, 0, 0, 0, 0, &res);
>>> +
>>> +	return res.a0;
>>> +}
>>> +
>>> +/**
>>> + * keembay_sd_voltage_selection() - Set the IO Pad voltage
>>> + * @volt: voltage selection either 1.8V or 3.3V
>>> + *
>>> + * This function is used to set the IO Line Voltage
>>> + *
>>> + * Return: 0 for success, Invalid for failure  */ int
>>> +keembay_sd_voltage_selection(int volt) {
>>> +	return do_fw_call(KEEMBAY_SIP_FUNC_ID, volt);
>>
>>
>> What are the other uses of this KEEMBAY_SIP_* ?
>> For now I tend to move this to the driver making use of it using
>> arm_smccc_1_1_invoke directly if possible. I don't see the need for
>> this to be separate driver. But do let us know the features
>> implemented in the firmware. If it is not v1.1+, reasons for not
>> upgrading as you need v1.1 for some CPU errata implementation.
>
>This driver has been created based on my request to move it out the mmc
>driver. It looks quite hacky to have arm_smccc_res and call
>arm_smccc_smc() also with some IDs where it is visible that the part of ID is just
>based on any spec.
>Also in v1 he is just calling SMC. But maybe there is going a need to call HVC
>instead which is something what device driver shouldn't decide that's why
>IMHO doing step via firmware driver is much better approach.
>Of course if there is a better/cleaner way how this should be done I am happy
>to get more information about it.
>
>Thanks,
>Michal
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

Hi Michal,

On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
> Hi Sudeep,
> 
> On 01. 10. 20 17:35, Sudeep Holla wrote:

[...]

> > 
> > What are the other uses of this KEEMBAY_SIP_* ?
> > For now I tend to move this to the driver making use of it using
> > arm_smccc_1_1_invoke directly if possible. I don't see the need for this
> > to be separate driver. But do let us know the features implemented in the
> > firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
> > for some CPU errata implementation.
> 
> This driver has been created based on my request to move it out the mmc
> driver. It looks quite hacky to have arm_smccc_res and call
> arm_smccc_smc() also with some IDs where it is visible that the part of
> ID is just based on any spec.

OK, driver is fine but no dt-bindings as it is discoverable. It can
also be just a wrapper library instead as it needs no explicit
initialisation like drivers to setup.

> Also in v1 he is just calling SMC. But maybe there is going a need to
> call HVC instead which is something what device driver shouldn't decide
> that's why IMHO doing step via firmware driver is much better approach.

Agreed and one must use arm_smccc_get_conduit or something similar. No
additional bindings for each and ever platform and driver that uses SMCCC
please.

> Of course if there is a better/cleaner way how this should be done I am
> happy to get more information about it.
> 

Let me know what you think about my thoughts stated above.

-- 
Regards,
Sudeep

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

Hi Michal,

On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
> Hi Sudeep,
> 
> On 01. 10. 20 17:35, Sudeep Holla wrote:

[...]

> > 
> > What are the other uses of this KEEMBAY_SIP_* ?
> > For now I tend to move this to the driver making use of it using
> > arm_smccc_1_1_invoke directly if possible. I don't see the need for this
> > to be separate driver. But do let us know the features implemented in the
> > firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
> > for some CPU errata implementation.
> 
> This driver has been created based on my request to move it out the mmc
> driver. It looks quite hacky to have arm_smccc_res and call
> arm_smccc_smc() also with some IDs where it is visible that the part of
> ID is just based on any spec.

OK, driver is fine but no dt-bindings as it is discoverable. It can
also be just a wrapper library instead as it needs no explicit
initialisation like drivers to setup.

> Also in v1 he is just calling SMC. But maybe there is going a need to
> call HVC instead which is something what device driver shouldn't decide
> that's why IMHO doing step via firmware driver is much better approach.

Agreed and one must use arm_smccc_get_conduit or something similar. No
additional bindings for each and ever platform and driver that uses SMCCC
please.

> Of course if there is a better/cleaner way how this should be done I am
> happy to get more information about it.
> 

Let me know what you think about my thoughts stated above.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Fri, Oct 02, 2020 at 10:22:46AM +0000, Zulkifli, Muhammad Husaini wrote:
> Hi Sudeep,
> 
> >-----Original Message-----
> >From: Michal Simek <michal.simek@xilinx.com>
> >Sent: Friday, October 2, 2020 4:23 PM
> >To: Sudeep Holla <sudeep.holla@arm.com>; Zulkifli, Muhammad Husaini
> ><muhammad.husaini.zulkifli@intel.com>
> >Cc: Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com;
> >ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; linux-arm-
> >kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Raja Subramanian,
> >Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan
> >Mohamad, Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
> >Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
> >Firmware Service call
> >
> >Hi Sudeep,
> >
> >On 01. 10. 20 17:35, Sudeep Holla wrote:
> >> On Thu, Oct 01, 2020 at 10:21:48PM +0800,
> >muhammad.husaini.zulkifli@intel.com wrote:
> >>> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> >>>
> >>> Add generic firmware driver for Keem Bay SOC to support Arm Trusted
> >>> Firmware Services call.
> >>>
> >>> Signed-off-by: Muhammad Husaini Zulkifli
> >>> <muhammad.husaini.zulkifli@intel.com>
> >>> ---
> >>>  drivers/firmware/Kconfig                   |   1 +
> >>>  drivers/firmware/Makefile                  |   1 +
> >>>  drivers/firmware/intel/Kconfig             |  14 +++
> >>>  drivers/firmware/intel/Makefile            |   4 +
> >>>  drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
> >>>  include/linux/firmware/intel/keembay_smc.h |  27 +++++
> >>>  6 files changed, 166 insertions(+)
> >>>  create mode 100644 drivers/firmware/intel/Kconfig  create mode
> >>> 100644 drivers/firmware/intel/Makefile  create mode 100644
> >>> drivers/firmware/intel/keembay_smc.c
> >>>  create mode 100644 include/linux/firmware/intel/keembay_smc.h
> >>>
> >>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> >>> index fbd785dd0513..41de77d2720e 100644
> >>> --- a/drivers/firmware/Kconfig
> >>> +++ b/drivers/firmware/Kconfig
> >>> @@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
> >>>  source "drivers/firmware/smccc/Kconfig"
> >>>  source "drivers/firmware/tegra/Kconfig"
> >>>  source "drivers/firmware/xilinx/Kconfig"
> >>> +source "drivers/firmware/intel/Kconfig"
> >>>
> >>>  endmenu
> >>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> >>> index 99510be9f5ed..00f295ab9860 100644
> >>> --- a/drivers/firmware/Makefile
> >>> +++ b/drivers/firmware/Makefile
> >>> @@ -33,3 +33,4 @@ obj-y				+= psci/
> >>>  obj-y				+= smccc/
> >>>  obj-y				+= tegra/
> >>>  obj-y				+= xilinx/
> >>> +obj-y				+= intel/
> >>> diff --git a/drivers/firmware/intel/Kconfig
> >>> b/drivers/firmware/intel/Kconfig new file mode 100644 index
> >>> 000000000000..b2b7a4e5410b
> >>> --- /dev/null
> >>> +++ b/drivers/firmware/intel/Kconfig
> >>> @@ -0,0 +1,14 @@
> >>> +# SPDX-License-Identifier: GPL-2.0-only menu "Intel Firmware
> >>> +Drivers"
> >>> +
> >>> +config KEEMBAY_FIRMWARE
> >>> +	bool "Enable Keem Bay firmware interface support"
> >>> +	depends on HAVE_ARM_SMCCC
> >>
> >> What is the version of SMCCC implemented ?
> Our current Arm Trusted Firmware framework supports v1.1.
> Does it mean I should use HAVE_ARM_SMCCC_DISCOVERY?

Yes, HAVE_ARM_SMCCC_DISCOVERY is right dependency and allows you to
get smccc version via arm_smccc_get_version which may be useful in
future.

> Not really clear about this. As for HAVE_ARM_SMCCC will include 
> support for the SMC and HVC.
>

Yes, but for sake of correctness I prefer HAVE_ARM_SMCCC_DISCOVERY.

-- 
Regards,
Sudeep

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Fri, Oct 02, 2020 at 10:22:46AM +0000, Zulkifli, Muhammad Husaini wrote:
> Hi Sudeep,
> 
> >-----Original Message-----
> >From: Michal Simek <michal.simek@xilinx.com>
> >Sent: Friday, October 2, 2020 4:23 PM
> >To: Sudeep Holla <sudeep.holla@arm.com>; Zulkifli, Muhammad Husaini
> ><muhammad.husaini.zulkifli@intel.com>
> >Cc: Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com;
> >ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; linux-arm-
> >kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Raja Subramanian,
> >Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan
> >Mohamad, Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
> >Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
> >Firmware Service call
> >
> >Hi Sudeep,
> >
> >On 01. 10. 20 17:35, Sudeep Holla wrote:
> >> On Thu, Oct 01, 2020 at 10:21:48PM +0800,
> >muhammad.husaini.zulkifli@intel.com wrote:
> >>> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> >>>
> >>> Add generic firmware driver for Keem Bay SOC to support Arm Trusted
> >>> Firmware Services call.
> >>>
> >>> Signed-off-by: Muhammad Husaini Zulkifli
> >>> <muhammad.husaini.zulkifli@intel.com>
> >>> ---
> >>>  drivers/firmware/Kconfig                   |   1 +
> >>>  drivers/firmware/Makefile                  |   1 +
> >>>  drivers/firmware/intel/Kconfig             |  14 +++
> >>>  drivers/firmware/intel/Makefile            |   4 +
> >>>  drivers/firmware/intel/keembay_smc.c       | 119 +++++++++++++++++++++
> >>>  include/linux/firmware/intel/keembay_smc.h |  27 +++++
> >>>  6 files changed, 166 insertions(+)
> >>>  create mode 100644 drivers/firmware/intel/Kconfig  create mode
> >>> 100644 drivers/firmware/intel/Makefile  create mode 100644
> >>> drivers/firmware/intel/keembay_smc.c
> >>>  create mode 100644 include/linux/firmware/intel/keembay_smc.h
> >>>
> >>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> >>> index fbd785dd0513..41de77d2720e 100644
> >>> --- a/drivers/firmware/Kconfig
> >>> +++ b/drivers/firmware/Kconfig
> >>> @@ -305,5 +305,6 @@ source "drivers/firmware/psci/Kconfig"
> >>>  source "drivers/firmware/smccc/Kconfig"
> >>>  source "drivers/firmware/tegra/Kconfig"
> >>>  source "drivers/firmware/xilinx/Kconfig"
> >>> +source "drivers/firmware/intel/Kconfig"
> >>>
> >>>  endmenu
> >>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> >>> index 99510be9f5ed..00f295ab9860 100644
> >>> --- a/drivers/firmware/Makefile
> >>> +++ b/drivers/firmware/Makefile
> >>> @@ -33,3 +33,4 @@ obj-y				+= psci/
> >>>  obj-y				+= smccc/
> >>>  obj-y				+= tegra/
> >>>  obj-y				+= xilinx/
> >>> +obj-y				+= intel/
> >>> diff --git a/drivers/firmware/intel/Kconfig
> >>> b/drivers/firmware/intel/Kconfig new file mode 100644 index
> >>> 000000000000..b2b7a4e5410b
> >>> --- /dev/null
> >>> +++ b/drivers/firmware/intel/Kconfig
> >>> @@ -0,0 +1,14 @@
> >>> +# SPDX-License-Identifier: GPL-2.0-only menu "Intel Firmware
> >>> +Drivers"
> >>> +
> >>> +config KEEMBAY_FIRMWARE
> >>> +	bool "Enable Keem Bay firmware interface support"
> >>> +	depends on HAVE_ARM_SMCCC
> >>
> >> What is the version of SMCCC implemented ?
> Our current Arm Trusted Firmware framework supports v1.1.
> Does it mean I should use HAVE_ARM_SMCCC_DISCOVERY?

Yes, HAVE_ARM_SMCCC_DISCOVERY is right dependency and allows you to
get smccc version via arm_smccc_get_version which may be useful in
future.

> Not really clear about this. As for HAVE_ARM_SMCCC will include 
> support for the SMC and HVC.
>

Yes, but for sake of correctness I prefer HAVE_ARM_SMCCC_DISCOVERY.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Michal Simek]

Hi Sudeep,

On 02. 10. 20 12:58, Sudeep Holla wrote:
> Hi Michal,
> 
> On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>> Hi Sudeep,
>>
>> On 01. 10. 20 17:35, Sudeep Holla wrote:
> 
> [...]
> 
>>>
>>> What are the other uses of this KEEMBAY_SIP_* ?
>>> For now I tend to move this to the driver making use of it using
>>> arm_smccc_1_1_invoke directly if possible. I don't see the need for this
>>> to be separate driver. But do let us know the features implemented in the
>>> firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
>>> for some CPU errata implementation.
>>
>> This driver has been created based on my request to move it out the mmc
>> driver. It looks quite hacky to have arm_smccc_res and call
>> arm_smccc_smc() also with some IDs where it is visible that the part of
>> ID is just based on any spec.
> 
> OK, driver is fine but no dt-bindings as it is discoverable. It can
> also be just a wrapper library instead as it needs no explicit
> initialisation like drivers to setup.

I am fine with it. Do we have any example which we can point him to?


> 
>> Also in v1 he is just calling SMC. But maybe there is going a need to
>> call HVC instead which is something what device driver shouldn't decide
>> that's why IMHO doing step via firmware driver is much better approach.
> 
> Agreed and one must use arm_smccc_get_conduit or something similar. No
> additional bindings for each and ever platform and driver that uses SMCCC
> please.
> 
>> Of course if there is a better/cleaner way how this should be done I am
>> happy to get more information about it.
>>
> 
> Let me know what you think about my thoughts stated above.


I am fine with it. The key point is to have these sort it out because I
see that a lot of drivers just simply call that SMCs from drivers which
is IMHO wrong.


BTW: I see you have added soc id reading which you are saying is the
part of smcc v1.2 but I can't see any implementation in TF-A. Is this
spec publicly available?

Thanks,
Michal

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Michal Simek]

Hi Sudeep,

On 02. 10. 20 12:58, Sudeep Holla wrote:
> Hi Michal,
> 
> On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>> Hi Sudeep,
>>
>> On 01. 10. 20 17:35, Sudeep Holla wrote:
> 
> [...]
> 
>>>
>>> What are the other uses of this KEEMBAY_SIP_* ?
>>> For now I tend to move this to the driver making use of it using
>>> arm_smccc_1_1_invoke directly if possible. I don't see the need for this
>>> to be separate driver. But do let us know the features implemented in the
>>> firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
>>> for some CPU errata implementation.
>>
>> This driver has been created based on my request to move it out the mmc
>> driver. It looks quite hacky to have arm_smccc_res and call
>> arm_smccc_smc() also with some IDs where it is visible that the part of
>> ID is just based on any spec.
> 
> OK, driver is fine but no dt-bindings as it is discoverable. It can
> also be just a wrapper library instead as it needs no explicit
> initialisation like drivers to setup.

I am fine with it. Do we have any example which we can point him to?


> 
>> Also in v1 he is just calling SMC. But maybe there is going a need to
>> call HVC instead which is something what device driver shouldn't decide
>> that's why IMHO doing step via firmware driver is much better approach.
> 
> Agreed and one must use arm_smccc_get_conduit or something similar. No
> additional bindings for each and ever platform and driver that uses SMCCC
> please.
> 
>> Of course if there is a better/cleaner way how this should be done I am
>> happy to get more information about it.
>>
> 
> Let me know what you think about my thoughts stated above.


I am fine with it. The key point is to have these sort it out because I
see that a lot of drivers just simply call that SMCs from drivers which
is IMHO wrong.


BTW: I see you have added soc id reading which you are saying is the
part of smcc v1.2 but I can't see any implementation in TF-A. Is this
spec publicly available?

Thanks,
Michal



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

Hi Michal,

On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
> Hi Sudeep,
>
> On 02. 10. 20 12:58, Sudeep Holla wrote:
> > Hi Michal,
> >
> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
> >> Hi Sudeep,
> >>
> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
> >
> > [...]
> >
> >>>
> >>> What are the other uses of this KEEMBAY_SIP_* ?
> >>> For now I tend to move this to the driver making use of it using
> >>> arm_smccc_1_1_invoke directly if possible. I don't see the need for this
> >>> to be separate driver. But do let us know the features implemented in the
> >>> firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
> >>> for some CPU errata implementation.
> >>
> >> This driver has been created based on my request to move it out the mmc
> >> driver. It looks quite hacky to have arm_smccc_res and call
> >> arm_smccc_smc() also with some IDs where it is visible that the part of
> >> ID is just based on any spec.
> >
> > OK, driver is fine but no dt-bindings as it is discoverable. It can
> > also be just a wrapper library instead as it needs no explicit
> > initialisation like drivers to setup.
>
> I am fine with it. Do we have any example which we can point him to?
>

You seem to have figured that out already with SOC_ID example.
That was quick I must say 😄.

>
> >
> >> Also in v1 he is just calling SMC. But maybe there is going a need to
> >> call HVC instead which is something what device driver shouldn't decide
> >> that's why IMHO doing step via firmware driver is much better approach.
> >
> > Agreed and one must use arm_smccc_get_conduit or something similar. No
> > additional bindings for each and ever platform and driver that uses SMCCC
> > please.
> >
> >> Of course if there is a better/cleaner way how this should be done I am
> >> happy to get more information about it.
> >>
> >
> > Let me know what you think about my thoughts stated above.
>
>
> I am fine with it. The key point is to have these sort it out because I
> see that a lot of drivers just simply call that SMCs from drivers which
> is IMHO wrong.
>

Sure, sorry I didn't express my concern properly. I want to avoid dt bindings
for these and use the SMCCC discovery we have in place already if possible.

If this driver had consumers in the DT and it needs to be represented
in DT, it is a different story and I agree for need for a driver there.
But I don't see one in this usecase.

>
> BTW: I see you have added soc id reading which you are saying is the
> part of smcc v1.2 but I can't see any implementation in TF-A. Is this
> spec publicly available?
>

Spec is out[1], include/linux/arm-smccc.h points to the latest spec.
TF-A does have implementation as I tested with it and even reported
bug that I discovered when I tested with my patches that are now merged
upstream. Are you referring to master of TF-A or last release version ?
If latter, it had bug and may not be working. I may be wrong though, as
I am just telling what was told to me couple of months back and things
might have changed in TF-A land.

--
Regards,
Sudeep

[1] https://developer.arm.com/documentation/den0028/latest

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

Hi Michal,

On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
> Hi Sudeep,
>
> On 02. 10. 20 12:58, Sudeep Holla wrote:
> > Hi Michal,
> >
> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
> >> Hi Sudeep,
> >>
> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
> >
> > [...]
> >
> >>>
> >>> What are the other uses of this KEEMBAY_SIP_* ?
> >>> For now I tend to move this to the driver making use of it using
> >>> arm_smccc_1_1_invoke directly if possible. I don't see the need for this
> >>> to be separate driver. But do let us know the features implemented in the
> >>> firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
> >>> for some CPU errata implementation.
> >>
> >> This driver has been created based on my request to move it out the mmc
> >> driver. It looks quite hacky to have arm_smccc_res and call
> >> arm_smccc_smc() also with some IDs where it is visible that the part of
> >> ID is just based on any spec.
> >
> > OK, driver is fine but no dt-bindings as it is discoverable. It can
> > also be just a wrapper library instead as it needs no explicit
> > initialisation like drivers to setup.
>
> I am fine with it. Do we have any example which we can point him to?
>

You seem to have figured that out already with SOC_ID example.
That was quick I must say 😄.

>
> >
> >> Also in v1 he is just calling SMC. But maybe there is going a need to
> >> call HVC instead which is something what device driver shouldn't decide
> >> that's why IMHO doing step via firmware driver is much better approach.
> >
> > Agreed and one must use arm_smccc_get_conduit or something similar. No
> > additional bindings for each and ever platform and driver that uses SMCCC
> > please.
> >
> >> Of course if there is a better/cleaner way how this should be done I am
> >> happy to get more information about it.
> >>
> >
> > Let me know what you think about my thoughts stated above.
>
>
> I am fine with it. The key point is to have these sort it out because I
> see that a lot of drivers just simply call that SMCs from drivers which
> is IMHO wrong.
>

Sure, sorry I didn't express my concern properly. I want to avoid dt bindings
for these and use the SMCCC discovery we have in place already if possible.

If this driver had consumers in the DT and it needs to be represented
in DT, it is a different story and I agree for need for a driver there.
But I don't see one in this usecase.

>
> BTW: I see you have added soc id reading which you are saying is the
> part of smcc v1.2 but I can't see any implementation in TF-A. Is this
> spec publicly available?
>

Spec is out[1], include/linux/arm-smccc.h points to the latest spec.
TF-A does have implementation as I tested with it and even reported
bug that I discovered when I tested with my patches that are now merged
upstream. Are you referring to master of TF-A or last release version ?
If latter, it had bug and may not be working. I may be wrong though, as
I am just telling what was told to me couple of months back and things
might have changed in TF-A land.

--
Regards,
Sudeep

[1] https://developer.arm.com/documentation/den0028/latest

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Sudeep,

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Friday, October 2, 2020 10:51 PM
>To: Michal Simek <michal.simek@xilinx.com>
>Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
>Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Sudeep Holla
><sudeep.holla@arm.com>; Wan Mohamad, Wan Ahmad Zainie
><wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>Hi Michal,
>
>On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
>> Hi Sudeep,
>>
>> On 02. 10. 20 12:58, Sudeep Holla wrote:
>> > Hi Michal,
>> >
>> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>> >> Hi Sudeep,
>> >>
>> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
>> >
>> > [...]
>> >
>> >>>
>> >>> What are the other uses of this KEEMBAY_SIP_* ?
>> >>> For now I tend to move this to the driver making use of it using
>> >>> arm_smccc_1_1_invoke directly if possible. I don't see the need
>> >>> for this to be separate driver. But do let us know the features
>> >>> implemented in the firmware. If it is not v1.1+, reasons for not
>> >>> upgrading as you need v1.1 for some CPU errata implementation.
>> >>
>> >> This driver has been created based on my request to move it out the
>> >> mmc driver. It looks quite hacky to have arm_smccc_res and call
>> >> arm_smccc_smc() also with some IDs where it is visible that the
>> >> part of ID is just based on any spec.
>> >
>> > OK, driver is fine but no dt-bindings as it is discoverable. It can
>> > also be just a wrapper library instead as it needs no explicit
>> > initialisation like drivers to setup.
>>
>> I am fine with it. Do we have any example which we can point him to?
>>
>
>You seem to have figured that out already with SOC_ID example.
>That was quick I must say 😄.
>
>>
>> >
>> >> Also in v1 he is just calling SMC. But maybe there is going a need
>> >> to call HVC instead which is something what device driver shouldn't
>> >> decide that's why IMHO doing step via firmware driver is much better
>approach.
I will add func for HVC also in case in the future if someone want to use it.
>> >
>> > Agreed and one must use arm_smccc_get_conduit or something similar.
>> > No additional bindings for each and ever platform and driver that
>> > uses SMCCC please.
>> >
>> >> Of course if there is a better/cleaner way how this should be done
>> >> I am happy to get more information about it.
>> >>
>> >
>> > Let me know what you think about my thoughts stated above.
>>
>>
>> I am fine with it. The key point is to have these sort it out because
>> I see that a lot of drivers just simply call that SMCs from drivers
>> which is IMHO wrong.
>>
>
>Sure, sorry I didn't express my concern properly. I want to avoid dt bindings for
>these and use the SMCCC discovery we have in place already if possible.
>
>If this driver had consumers in the DT and it needs to be represented in DT, it is
>a different story and I agree for need for a driver there.
>But I don't see one in this usecase.

Does it ok if I do some checking in arasan controller driver as below and 
represented it in the DT of arasan,sdhci.yaml:
This is to ensure that for Keem Bay SOC specific, Keembay_firmware node must be consume.

	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
		struct device_node *dn;
		struct gpio_desc *uhs;

		dn = of_find_node_by_name(NULL, "keembay_firmware");
		if (!dn)
			return dev_err_probe(dev, PTR_ERR(dn),
						"can't find keembay_firmware node\n");
		of_node_put(dn);
..........
}

Thanks
>
>>
>> BTW: I see you have added soc id reading which you are saying is the
>> part of smcc v1.2 but I can't see any implementation in TF-A. Is this
>> spec publicly available?
>>
>
>Spec is out[1], include/linux/arm-smccc.h points to the latest spec.
>TF-A does have implementation as I tested with it and even reported bug that I
>discovered when I tested with my patches that are now merged upstream. Are
>you referring to master of TF-A or last release version ?
>If latter, it had bug and may not be working. I may be wrong though, as I am just
>telling what was told to me couple of months back and things might have
>changed in TF-A land.
>
>--
>Regards,
>Sudeep
>
>[1] https://developer.arm.com/documentation/den0028/latest

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Sudeep,

I am facing an error during sending yesterday.
I response again to your feedback as below

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Friday, October 2, 2020 10:51 PM
>To: Michal Simek <michal.simek@xilinx.com>
>Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
>Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Sudeep Holla
><sudeep.holla@arm.com>; Wan Mohamad, Wan Ahmad Zainie
><wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>Hi Michal,
>
>On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
>> Hi Sudeep,
>>
>> On 02. 10. 20 12:58, Sudeep Holla wrote:
>> > Hi Michal,
>> >
>> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>> >> Hi Sudeep,
>> >>
>> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
>> >
>> > [...]
>> >
>> >>>
>> >>> What are the other uses of this KEEMBAY_SIP_* ?
>> >>> For now I tend to move this to the driver making use of it using
>> >>> arm_smccc_1_1_invoke directly if possible. I don't see the need
>> >>> for this to be separate driver. But do let us know the features
>> >>> implemented in the firmware. If it is not v1.1+, reasons for not
>> >>> upgrading as you need v1.1 for some CPU errata implementation.
>> >>
>> >> This driver has been created based on my request to move it out the
>> >> mmc driver. It looks quite hacky to have arm_smccc_res and call
>> >> arm_smccc_smc() also with some IDs where it is visible that the
>> >> part of ID is just based on any spec.
>> >
>> > OK, driver is fine but no dt-bindings as it is discoverable. It can
>> > also be just a wrapper library instead as it needs no explicit
>> > initialisation like drivers to setup.
>>
>> I am fine with it. Do we have any example which we can point him to?
>>
>
>You seem to have figured that out already with SOC_ID example.
>That was quick I must say 😄.
>
>>
>> >
>> >> Also in v1 he is just calling SMC. But maybe there is going a need
>> >> to call HVC instead which is something what device driver shouldn't
>> >> decide that's why IMHO doing step via firmware driver is much better
>approach.
>> >
>> > Agreed and one must use arm_smccc_get_conduit or something similar.
>> > No additional bindings for each and ever platform and driver that
>> > uses SMCCC please.
>> >
>> >> Of course if there is a better/cleaner way how this should be done
>> >> I am happy to get more information about it.
>> >>
>> >
>> > Let me know what you think about my thoughts stated above.
>>
>>
>> I am fine with it. The key point is to have these sort it out because
>> I see that a lot of drivers just simply call that SMCs from drivers
>> which is IMHO wrong.
>>
>
>Sure, sorry I didn't express my concern properly. I want to avoid dt bindings for
>these and use the SMCCC discovery we have in place already if possible.
>
>If this driver had consumers in the DT and it needs to be represented in DT, it is
>a different story and I agree for need for a driver there.
>But I don't see one in this usecase.


Does it ok if I do some checking in arasan controller driver as below and represented it in the DT of arasan,sdhci.yaml:
This is to ensure that for Keem Bay SOC specific, the firmware driver must be consume.

	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
		struct device_node *dn;
		struct gpio_desc *uhs;

		dn = of_find_node_by_name(NULL, "keembay_firmware");
		if (!dn)
			return dev_err_probe(dev, PTR_ERR(dn),
						"can't find keembay_firmware node\n");
		of_node_put(dn);
..........
}
>
>>
>> BTW: I see you have added soc id reading which you are saying is the
>> part of smcc v1.2 but I can't see any implementation in TF-A. Is this
>> spec publicly available?
>>
>
>Spec is out[1], include/linux/arm-smccc.h points to the latest spec.
>TF-A does have implementation as I tested with it and even reported bug that I
>discovered when I tested with my patches that are now merged upstream. Are
>you referring to master of TF-A or last release version ?
>If latter, it had bug and may not be working. I may be wrong though, as I am just
>telling what was told to me couple of months back and things might have
>changed in TF-A land.
>
>--
>Regards,
>Sudeep
>
>[1] https://developer.arm.com/documentation/den0028/latest

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Sudeep,

I am facing an error during sending yesterday.
I response again to your feedback as below

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Friday, October 2, 2020 10:51 PM
>To: Michal Simek <michal.simek@xilinx.com>
>Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
>Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Sudeep Holla
><sudeep.holla@arm.com>; Wan Mohamad, Wan Ahmad Zainie
><wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>Hi Michal,
>
>On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
>> Hi Sudeep,
>>
>> On 02. 10. 20 12:58, Sudeep Holla wrote:
>> > Hi Michal,
>> >
>> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>> >> Hi Sudeep,
>> >>
>> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
>> >
>> > [...]
>> >
>> >>>
>> >>> What are the other uses of this KEEMBAY_SIP_* ?
>> >>> For now I tend to move this to the driver making use of it using
>> >>> arm_smccc_1_1_invoke directly if possible. I don't see the need
>> >>> for this to be separate driver. But do let us know the features
>> >>> implemented in the firmware. If it is not v1.1+, reasons for not
>> >>> upgrading as you need v1.1 for some CPU errata implementation.
>> >>
>> >> This driver has been created based on my request to move it out the
>> >> mmc driver. It looks quite hacky to have arm_smccc_res and call
>> >> arm_smccc_smc() also with some IDs where it is visible that the
>> >> part of ID is just based on any spec.
>> >
>> > OK, driver is fine but no dt-bindings as it is discoverable. It can
>> > also be just a wrapper library instead as it needs no explicit
>> > initialisation like drivers to setup.
>>
>> I am fine with it. Do we have any example which we can point him to?
>>
>
>You seem to have figured that out already with SOC_ID example.
>That was quick I must say 😄.
>
>>
>> >
>> >> Also in v1 he is just calling SMC. But maybe there is going a need
>> >> to call HVC instead which is something what device driver shouldn't
>> >> decide that's why IMHO doing step via firmware driver is much better
>approach.
>> >
>> > Agreed and one must use arm_smccc_get_conduit or something similar.
>> > No additional bindings for each and ever platform and driver that
>> > uses SMCCC please.
>> >
>> >> Of course if there is a better/cleaner way how this should be done
>> >> I am happy to get more information about it.
>> >>
>> >
>> > Let me know what you think about my thoughts stated above.
>>
>>
>> I am fine with it. The key point is to have these sort it out because
>> I see that a lot of drivers just simply call that SMCs from drivers
>> which is IMHO wrong.
>>
>
>Sure, sorry I didn't express my concern properly. I want to avoid dt bindings for
>these and use the SMCCC discovery we have in place already if possible.
>
>If this driver had consumers in the DT and it needs to be represented in DT, it is
>a different story and I agree for need for a driver there.
>But I don't see one in this usecase.


Does it ok if I do some checking in arasan controller driver as below and represented it in the DT of arasan,sdhci.yaml:
This is to ensure that for Keem Bay SOC specific, the firmware driver must be consume.

	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
		struct device_node *dn;
		struct gpio_desc *uhs;

		dn = of_find_node_by_name(NULL, "keembay_firmware");
		if (!dn)
			return dev_err_probe(dev, PTR_ERR(dn),
						"can't find keembay_firmware node\n");
		of_node_put(dn);
..........
}
>
>>
>> BTW: I see you have added soc id reading which you are saying is the
>> part of smcc v1.2 but I can't see any implementation in TF-A. Is this
>> spec publicly available?
>>
>
>Spec is out[1], include/linux/arm-smccc.h points to the latest spec.
>TF-A does have implementation as I tested with it and even reported bug that I
>discovered when I tested with my patches that are now merged upstream. Are
>you referring to master of TF-A or last release version ?
>If latter, it had bug and may not be working. I may be wrong though, as I am just
>telling what was told to me couple of months back and things might have
>changed in TF-A land.
>
>--
>Regards,
>Sudeep
>
>[1] https://developer.arm.com/documentation/den0028/latest
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Mon, Oct 05, 2020 at 08:37:13AM +0000, Zulkifli, Muhammad Husaini wrote:
> Hi Sudeep,
> 
> I am facing an error during sending yesterday.
> I response again to your feedback as below
> 
> >-----Original Message-----
> >From: Sudeep Holla <sudeep.holla@arm.com>
> >Sent: Friday, October 2, 2020 10:51 PM
> >To: Michal Simek <michal.simek@xilinx.com>
> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
> >Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
> >mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> >kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
> ><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Sudeep Holla
> ><sudeep.holla@arm.com>; Wan Mohamad, Wan Ahmad Zainie
> ><wan.ahmad.zainie.wan.mohamad@intel.com>
> >Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
> >Firmware Service call
> >
> >Hi Michal,
> >
> >On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
> >> Hi Sudeep,
> >>
> >> On 02. 10. 20 12:58, Sudeep Holla wrote:
> >> > Hi Michal,
> >> >
> >> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
> >> >> Hi Sudeep,
> >> >>
> >> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
> >> >
> >> > [...]
> >> >
> >> >>>
> >> >>> What are the other uses of this KEEMBAY_SIP_* ?
> >> >>> For now I tend to move this to the driver making use of it using
> >> >>> arm_smccc_1_1_invoke directly if possible. I don't see the need
> >> >>> for this to be separate driver. But do let us know the features
> >> >>> implemented in the firmware. If it is not v1.1+, reasons for not
> >> >>> upgrading as you need v1.1 for some CPU errata implementation.
> >> >>
> >> >> This driver has been created based on my request to move it out the
> >> >> mmc driver. It looks quite hacky to have arm_smccc_res and call
> >> >> arm_smccc_smc() also with some IDs where it is visible that the
> >> >> part of ID is just based on any spec.
> >> >
> >> > OK, driver is fine but no dt-bindings as it is discoverable. It can
> >> > also be just a wrapper library instead as it needs no explicit
> >> > initialisation like drivers to setup.
> >>
> >> I am fine with it. Do we have any example which we can point him to?
> >>
> >
> >You seem to have figured that out already with SOC_ID example.
> >That was quick I must say 😄.
> >
> >>
> >> >
> >> >> Also in v1 he is just calling SMC. But maybe there is going a need
> >> >> to call HVC instead which is something what device driver shouldn't
> >> >> decide that's why IMHO doing step via firmware driver is much better
> >approach.
> >> >
> >> > Agreed and one must use arm_smccc_get_conduit or something similar.
> >> > No additional bindings for each and ever platform and driver that
> >> > uses SMCCC please.
> >> >
> >> >> Of course if there is a better/cleaner way how this should be done
> >> >> I am happy to get more information about it.
> >> >>
> >> >
> >> > Let me know what you think about my thoughts stated above.
> >>
> >>
> >> I am fine with it. The key point is to have these sort it out because
> >> I see that a lot of drivers just simply call that SMCs from drivers
> >> which is IMHO wrong.
> >>
> >
> >Sure, sorry I didn't express my concern properly. I want to avoid dt bindings for
> >these and use the SMCCC discovery we have in place already if possible.
> >
> >If this driver had consumers in the DT and it needs to be represented in DT, it is
> >a different story and I agree for need for a driver there.
> >But I don't see one in this usecase.
> 
> 
> Does it ok if I do some checking in arasan controller driver as below and represented it in the DT of arasan,sdhci.yaml:
> This is to ensure that for Keem Bay SOC specific, the firmware driver must be consume.
> 
> 	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
> 		struct device_node *dn;
> 		struct gpio_desc *uhs;
> 
> 		dn = of_find_node_by_name(NULL, "keembay_firmware");

You have keembay_sd_voltage_selection function as Michal prefers, I have
no objections for that. But please no keembay_firmware node in DT.
You can implement this as a driver or simple smccc based function library
without DT node using SMCCC get_version. I hope the firmware gives error
for unimplemented FIDs, thereby eliminating the need for any DT node or
config option.

-- 
Regards,
Sudeep

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Mon, Oct 05, 2020 at 08:37:13AM +0000, Zulkifli, Muhammad Husaini wrote:
> Hi Sudeep,
> 
> I am facing an error during sending yesterday.
> I response again to your feedback as below
> 
> >-----Original Message-----
> >From: Sudeep Holla <sudeep.holla@arm.com>
> >Sent: Friday, October 2, 2020 10:51 PM
> >To: Michal Simek <michal.simek@xilinx.com>
> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
> >Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
> >mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> >kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
> ><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Sudeep Holla
> ><sudeep.holla@arm.com>; Wan Mohamad, Wan Ahmad Zainie
> ><wan.ahmad.zainie.wan.mohamad@intel.com>
> >Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
> >Firmware Service call
> >
> >Hi Michal,
> >
> >On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
> >> Hi Sudeep,
> >>
> >> On 02. 10. 20 12:58, Sudeep Holla wrote:
> >> > Hi Michal,
> >> >
> >> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
> >> >> Hi Sudeep,
> >> >>
> >> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
> >> >
> >> > [...]
> >> >
> >> >>>
> >> >>> What are the other uses of this KEEMBAY_SIP_* ?
> >> >>> For now I tend to move this to the driver making use of it using
> >> >>> arm_smccc_1_1_invoke directly if possible. I don't see the need
> >> >>> for this to be separate driver. But do let us know the features
> >> >>> implemented in the firmware. If it is not v1.1+, reasons for not
> >> >>> upgrading as you need v1.1 for some CPU errata implementation.
> >> >>
> >> >> This driver has been created based on my request to move it out the
> >> >> mmc driver. It looks quite hacky to have arm_smccc_res and call
> >> >> arm_smccc_smc() also with some IDs where it is visible that the
> >> >> part of ID is just based on any spec.
> >> >
> >> > OK, driver is fine but no dt-bindings as it is discoverable. It can
> >> > also be just a wrapper library instead as it needs no explicit
> >> > initialisation like drivers to setup.
> >>
> >> I am fine with it. Do we have any example which we can point him to?
> >>
> >
> >You seem to have figured that out already with SOC_ID example.
> >That was quick I must say 😄.
> >
> >>
> >> >
> >> >> Also in v1 he is just calling SMC. But maybe there is going a need
> >> >> to call HVC instead which is something what device driver shouldn't
> >> >> decide that's why IMHO doing step via firmware driver is much better
> >approach.
> >> >
> >> > Agreed and one must use arm_smccc_get_conduit or something similar.
> >> > No additional bindings for each and ever platform and driver that
> >> > uses SMCCC please.
> >> >
> >> >> Of course if there is a better/cleaner way how this should be done
> >> >> I am happy to get more information about it.
> >> >>
> >> >
> >> > Let me know what you think about my thoughts stated above.
> >>
> >>
> >> I am fine with it. The key point is to have these sort it out because
> >> I see that a lot of drivers just simply call that SMCs from drivers
> >> which is IMHO wrong.
> >>
> >
> >Sure, sorry I didn't express my concern properly. I want to avoid dt bindings for
> >these and use the SMCCC discovery we have in place already if possible.
> >
> >If this driver had consumers in the DT and it needs to be represented in DT, it is
> >a different story and I agree for need for a driver there.
> >But I don't see one in this usecase.
> 
> 
> Does it ok if I do some checking in arasan controller driver as below and represented it in the DT of arasan,sdhci.yaml:
> This is to ensure that for Keem Bay SOC specific, the firmware driver must be consume.
> 
> 	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
> 		struct device_node *dn;
> 		struct gpio_desc *uhs;
> 
> 		dn = of_find_node_by_name(NULL, "keembay_firmware");

You have keembay_sd_voltage_selection function as Michal prefers, I have
no objections for that. But please no keembay_firmware node in DT.
You can implement this as a driver or simple smccc based function library
without DT node using SMCCC get_version. I hope the firmware gives error
for unimplemented FIDs, thereby eliminating the need for any DT node or
config option.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Monday, October 5, 2020 4:45 PM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian
><adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan Mohamad,
>Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>On Mon, Oct 05, 2020 at 08:37:13AM +0000, Zulkifli, Muhammad Husaini wrote:
>> Hi Sudeep,
>>
>> I am facing an error during sending yesterday.
>> I response again to your feedback as below
>>
>> >-----Original Message-----
>> >From: Sudeep Holla <sudeep.holla@arm.com>
>> >Sent: Friday, October 2, 2020 10:51 PM
>> >To: Michal Simek <michal.simek@xilinx.com>
>> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
>> >Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org;
>> >linux- mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> >linux- kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
>> ><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Sudeep Holla
>> ><sudeep.holla@arm.com>; Wan Mohamad, Wan Ahmad Zainie
>> ><wan.ahmad.zainie.wan.mohamad@intel.com>
>> >Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm
>> >Trusted Firmware Service call
>> >
>> >Hi Michal,
>> >
>> >On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
>> >> Hi Sudeep,
>> >>
>> >> On 02. 10. 20 12:58, Sudeep Holla wrote:
>> >> > Hi Michal,
>> >> >
>> >> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>> >> >> Hi Sudeep,
>> >> >>
>> >> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
>> >> >
>> >> > [...]
>> >> >
>> >> >>>
>> >> >>> What are the other uses of this KEEMBAY_SIP_* ?
>> >> >>> For now I tend to move this to the driver making use of it
>> >> >>> using arm_smccc_1_1_invoke directly if possible. I don't see
>> >> >>> the need for this to be separate driver. But do let us know the
>> >> >>> features implemented in the firmware. If it is not v1.1+,
>> >> >>> reasons for not upgrading as you need v1.1 for some CPU errata
>implementation.
>> >> >>
>> >> >> This driver has been created based on my request to move it out
>> >> >> the mmc driver. It looks quite hacky to have arm_smccc_res and
>> >> >> call
>> >> >> arm_smccc_smc() also with some IDs where it is visible that the
>> >> >> part of ID is just based on any spec.
>> >> >
>> >> > OK, driver is fine but no dt-bindings as it is discoverable. It
>> >> > can also be just a wrapper library instead as it needs no
>> >> > explicit initialisation like drivers to setup.
>> >>
>> >> I am fine with it. Do we have any example which we can point him to?
>> >>
>> >
>> >You seem to have figured that out already with SOC_ID example.
>> >That was quick I must say 😄.
>> >
>> >>
>> >> >
>> >> >> Also in v1 he is just calling SMC. But maybe there is going a
>> >> >> need to call HVC instead which is something what device driver
>> >> >> shouldn't decide that's why IMHO doing step via firmware driver
>> >> >> is much better
>> >approach.
>> >> >
>> >> > Agreed and one must use arm_smccc_get_conduit or something similar.
>> >> > No additional bindings for each and ever platform and driver that
>> >> > uses SMCCC please.
>> >> >
>> >> >> Of course if there is a better/cleaner way how this should be
>> >> >> done I am happy to get more information about it.
>> >> >>
>> >> >
>> >> > Let me know what you think about my thoughts stated above.
>> >>
>> >>
>> >> I am fine with it. The key point is to have these sort it out
>> >> because I see that a lot of drivers just simply call that SMCs from
>> >> drivers which is IMHO wrong.
>> >>
>> >
>> >Sure, sorry I didn't express my concern properly. I want to avoid dt
>> >bindings for these and use the SMCCC discovery we have in place already if
>possible.
>> >
>> >If this driver had consumers in the DT and it needs to be represented
>> >in DT, it is a different story and I agree for need for a driver there.
>> >But I don't see one in this usecase.
>>
>>
>> Does it ok if I do some checking in arasan controller driver as below and
>represented it in the DT of arasan,sdhci.yaml:
>> This is to ensure that for Keem Bay SOC specific, the firmware driver must be
>consume.
>>
>> 	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
>> 		struct device_node *dn;
>> 		struct gpio_desc *uhs;
>>
>> 		dn = of_find_node_by_name(NULL, "keembay_firmware");
>
>You have keembay_sd_voltage_selection function as Michal prefers, I have no
>objections for that. But please no keembay_firmware node in DT.
>You can implement this as a driver or simple smccc based function library
>without DT node using SMCCC get_version. I hope the firmware gives error for
>unimplemented FIDs, thereby eliminating the need for any DT node or config
>option.
To be clarify keembay_sd_voltage_selection function as Michal's prefers is actually using the firmware driver. This function located in firmware driver. 
I will call this func during voltage switching from arasan controller. I believe this implementation require DT to specify the compatible name and method use either smc/hvc.

Are you saying that by using simple smcc based function library I should call below func() in arasan controller. For example
1) arm_smccc_get_version(void)
2) arm_smccc_version_init(arm_smccc_get_version(), SMCCC_CONDUIT_SMC);
3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, voltage_value ,  &res);

Please advices.
Thanks

>
>--
>Regards,
>Sudeep

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Monday, October 5, 2020 4:45 PM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian
><adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan Mohamad,
>Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>On Mon, Oct 05, 2020 at 08:37:13AM +0000, Zulkifli, Muhammad Husaini wrote:
>> Hi Sudeep,
>>
>> I am facing an error during sending yesterday.
>> I response again to your feedback as below
>>
>> >-----Original Message-----
>> >From: Sudeep Holla <sudeep.holla@arm.com>
>> >Sent: Friday, October 2, 2020 10:51 PM
>> >To: Michal Simek <michal.simek@xilinx.com>
>> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
>> >Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org;
>> >linux- mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> >linux- kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
>> ><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Sudeep Holla
>> ><sudeep.holla@arm.com>; Wan Mohamad, Wan Ahmad Zainie
>> ><wan.ahmad.zainie.wan.mohamad@intel.com>
>> >Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm
>> >Trusted Firmware Service call
>> >
>> >Hi Michal,
>> >
>> >On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
>> >> Hi Sudeep,
>> >>
>> >> On 02. 10. 20 12:58, Sudeep Holla wrote:
>> >> > Hi Michal,
>> >> >
>> >> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>> >> >> Hi Sudeep,
>> >> >>
>> >> >> On 01. 10. 20 17:35, Sudeep Holla wrote:
>> >> >
>> >> > [...]
>> >> >
>> >> >>>
>> >> >>> What are the other uses of this KEEMBAY_SIP_* ?
>> >> >>> For now I tend to move this to the driver making use of it
>> >> >>> using arm_smccc_1_1_invoke directly if possible. I don't see
>> >> >>> the need for this to be separate driver. But do let us know the
>> >> >>> features implemented in the firmware. If it is not v1.1+,
>> >> >>> reasons for not upgrading as you need v1.1 for some CPU errata
>implementation.
>> >> >>
>> >> >> This driver has been created based on my request to move it out
>> >> >> the mmc driver. It looks quite hacky to have arm_smccc_res and
>> >> >> call
>> >> >> arm_smccc_smc() also with some IDs where it is visible that the
>> >> >> part of ID is just based on any spec.
>> >> >
>> >> > OK, driver is fine but no dt-bindings as it is discoverable. It
>> >> > can also be just a wrapper library instead as it needs no
>> >> > explicit initialisation like drivers to setup.
>> >>
>> >> I am fine with it. Do we have any example which we can point him to?
>> >>
>> >
>> >You seem to have figured that out already with SOC_ID example.
>> >That was quick I must say 😄.
>> >
>> >>
>> >> >
>> >> >> Also in v1 he is just calling SMC. But maybe there is going a
>> >> >> need to call HVC instead which is something what device driver
>> >> >> shouldn't decide that's why IMHO doing step via firmware driver
>> >> >> is much better
>> >approach.
>> >> >
>> >> > Agreed and one must use arm_smccc_get_conduit or something similar.
>> >> > No additional bindings for each and ever platform and driver that
>> >> > uses SMCCC please.
>> >> >
>> >> >> Of course if there is a better/cleaner way how this should be
>> >> >> done I am happy to get more information about it.
>> >> >>
>> >> >
>> >> > Let me know what you think about my thoughts stated above.
>> >>
>> >>
>> >> I am fine with it. The key point is to have these sort it out
>> >> because I see that a lot of drivers just simply call that SMCs from
>> >> drivers which is IMHO wrong.
>> >>
>> >
>> >Sure, sorry I didn't express my concern properly. I want to avoid dt
>> >bindings for these and use the SMCCC discovery we have in place already if
>possible.
>> >
>> >If this driver had consumers in the DT and it needs to be represented
>> >in DT, it is a different story and I agree for need for a driver there.
>> >But I don't see one in this usecase.
>>
>>
>> Does it ok if I do some checking in arasan controller driver as below and
>represented it in the DT of arasan,sdhci.yaml:
>> This is to ensure that for Keem Bay SOC specific, the firmware driver must be
>consume.
>>
>> 	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
>> 		struct device_node *dn;
>> 		struct gpio_desc *uhs;
>>
>> 		dn = of_find_node_by_name(NULL, "keembay_firmware");
>
>You have keembay_sd_voltage_selection function as Michal prefers, I have no
>objections for that. But please no keembay_firmware node in DT.
>You can implement this as a driver or simple smccc based function library
>without DT node using SMCCC get_version. I hope the firmware gives error for
>unimplemented FIDs, thereby eliminating the need for any DT node or config
>option.
To be clarify keembay_sd_voltage_selection function as Michal's prefers is actually using the firmware driver. This function located in firmware driver. 
I will call this func during voltage switching from arasan controller. I believe this implementation require DT to specify the compatible name and method use either smc/hvc.

Are you saying that by using simple smcc based function library I should call below func() in arasan controller. For example
1) arm_smccc_get_version(void)
2) arm_smccc_version_init(arm_smccc_get_version(), SMCCC_CONDUIT_SMC);
3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, voltage_value ,  &res);

Please advices.
Thanks

>
>--
>Regards,
>Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Mon, Oct 05, 2020 at 05:04:10PM +0000, Zulkifli, Muhammad Husaini wrote:

> To be clarify keembay_sd_voltage_selection function as Michal's prefers is
> actually using the firmware driver. This function located in firmware
> driver.

OK, it can be just one function place it in any file you think is more
appropriate need not be arasan controller driver. Any reasons why this
can't work ? Can even be in some header.

int keembay_sd_voltage_selection(int volt)
{
	int res;

	arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, &res)

	/* appropriate error check if needed here */

	return res;
}

> I will call this func during voltage switching from arasan controller. I
> believe this implementation require DT to specify the compatible name and
> method use either smc/hvc.

No, use the standard one as detected by arm_smccc_1_1_invoke
(It calls arm_smccc_get_conduit internally and use SMC/HVC based on that)

> 
> Are you saying that by using simple smcc based function library I should
> call below func() in arasan controller. For example
> 1) arm_smccc_get_version(void)
> 2) arm_smccc_version_init(arm_smccc_get_version(), SMCCC_CONDUIT_SMC);

Nope

> 3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, voltage_value ,  &res);

Just this.

-- 
Regards,
Sudeep

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Mon, Oct 05, 2020 at 05:04:10PM +0000, Zulkifli, Muhammad Husaini wrote:

> To be clarify keembay_sd_voltage_selection function as Michal's prefers is
> actually using the firmware driver. This function located in firmware
> driver.

OK, it can be just one function place it in any file you think is more
appropriate need not be arasan controller driver. Any reasons why this
can't work ? Can even be in some header.

int keembay_sd_voltage_selection(int volt)
{
	int res;

	arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, &res)

	/* appropriate error check if needed here */

	return res;
}

> I will call this func during voltage switching from arasan controller. I
> believe this implementation require DT to specify the compatible name and
> method use either smc/hvc.

No, use the standard one as detected by arm_smccc_1_1_invoke
(It calls arm_smccc_get_conduit internally and use SMC/HVC based on that)

> 
> Are you saying that by using simple smcc based function library I should
> call below func() in arasan controller. For example
> 1) arm_smccc_get_version(void)
> 2) arm_smccc_version_init(arm_smccc_get_version(), SMCCC_CONDUIT_SMC);

Nope

> 3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, voltage_value ,  &res);

Just this.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Michal,

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Tuesday, October 6, 2020 4:08 AM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian
><adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan Mohamad,
>Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>On Mon, Oct 05, 2020 at 05:04:10PM +0000, Zulkifli, Muhammad Husaini wrote:
>
>> To be clarify keembay_sd_voltage_selection function as Michal's
>> prefers is actually using the firmware driver. This function located
>> in firmware driver.
>
>OK, it can be just one function place it in any file you think is more appropriate
>need not be arasan controller driver. Any reasons why this can't work ? Can even
>be in some header.
>
>int keembay_sd_voltage_selection(int volt) {
>	int res;
>
>	arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt,
>&res)
>
>	/* appropriate error check if needed here */
>
>	return res;
>}
>
>> I will call this func during voltage switching from arasan controller.
>> I believe this implementation require DT to specify the compatible
>> name and method use either smc/hvc.
>
>No, use the standard one as detected by arm_smccc_1_1_invoke (It calls
>arm_smccc_get_conduit internally and use SMC/HVC based on that)
>
>>
>> Are you saying that by using simple smcc based function library I
>> should call below func() in arasan controller. For example
>> 1) arm_smccc_get_version(void)
>> 2) arm_smccc_version_init(arm_smccc_get_version(),
>SMCCC_CONDUIT_SMC);
>
>Nope
>
>> 3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID,
>voltage_value
>> ,  &res);
>
>Just this.
Is it ok not using the centralize firmware drivers? 
I would just revert back everything as in V1 by directly call the arm_smccc_1_1_invoke in arasan controller.
 
>
>--
>Regards,
>Sudeep

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

Hi Michal,

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Tuesday, October 6, 2020 4:08 AM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian
><adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan Mohamad,
>Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>On Mon, Oct 05, 2020 at 05:04:10PM +0000, Zulkifli, Muhammad Husaini wrote:
>
>> To be clarify keembay_sd_voltage_selection function as Michal's
>> prefers is actually using the firmware driver. This function located
>> in firmware driver.
>
>OK, it can be just one function place it in any file you think is more appropriate
>need not be arasan controller driver. Any reasons why this can't work ? Can even
>be in some header.
>
>int keembay_sd_voltage_selection(int volt) {
>	int res;
>
>	arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt,
>&res)
>
>	/* appropriate error check if needed here */
>
>	return res;
>}
>
>> I will call this func during voltage switching from arasan controller.
>> I believe this implementation require DT to specify the compatible
>> name and method use either smc/hvc.
>
>No, use the standard one as detected by arm_smccc_1_1_invoke (It calls
>arm_smccc_get_conduit internally and use SMC/HVC based on that)
>
>>
>> Are you saying that by using simple smcc based function library I
>> should call below func() in arasan controller. For example
>> 1) arm_smccc_get_version(void)
>> 2) arm_smccc_version_init(arm_smccc_get_version(),
>SMCCC_CONDUIT_SMC);
>
>Nope
>
>> 3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID,
>voltage_value
>> ,  &res);
>
>Just this.
Is it ok not using the centralize firmware drivers? 
I would just revert back everything as in V1 by directly call the arm_smccc_1_1_invoke in arasan controller.
 
>
>--
>Regards,
>Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

HI Sudeep and Michal,

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Tuesday, October 6, 2020 4:08 AM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian
><adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan Mohamad,
>Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>On Mon, Oct 05, 2020 at 05:04:10PM +0000, Zulkifli, Muhammad Husaini wrote:
>
>> To be clarify keembay_sd_voltage_selection function as Michal's
>> prefers is actually using the firmware driver. This function located
>> in firmware driver.
>
>OK, it can be just one function place it in any file you think is more appropriate
>need not be arasan controller driver. Any reasons why this can't work ? Can even
>be in some header.
>
>int keembay_sd_voltage_selection(int volt) {
>	int res;
>
>	arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt,
>&res)
>
>	/* appropriate error check if needed here */
>
>	return res;
>}
>
Yeah I believe it can work. I will create one header file in include/linux/firmware/intel/Keembay_firmware.h 
To handle this func and arasan controller can call this func.
Are you guys ok with this?

>> I will call this func during voltage switching from arasan controller.
>> I believe this implementation require DT to specify the compatible
>> name and method use either smc/hvc.
>
>No, use the standard one as detected by arm_smccc_1_1_invoke (It calls
>arm_smccc_get_conduit internally and use SMC/HVC based on that)
>
>>
>> Are you saying that by using simple smcc based function library I
>> should call below func() in arasan controller. For example
>> 1) arm_smccc_get_version(void)
>> 2) arm_smccc_version_init(arm_smccc_get_version(),
>SMCCC_CONDUIT_SMC);
>
>Nope
>
>> 3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID,
>voltage_value
>> ,  &res);
>
>Just this.
>
>--
>Regards,
>Sudeep

RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Zulkifli, Muhammad Husaini]

HI Sudeep and Michal,

>-----Original Message-----
>From: Sudeep Holla <sudeep.holla@arm.com>
>Sent: Tuesday, October 6, 2020 4:08 AM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian
><adrian.hunter@intel.com>; ulf.hansson@linaro.org; linux-
>mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; arnd@arndb.de; Wan Mohamad,
>Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>On Mon, Oct 05, 2020 at 05:04:10PM +0000, Zulkifli, Muhammad Husaini wrote:
>
>> To be clarify keembay_sd_voltage_selection function as Michal's
>> prefers is actually using the firmware driver. This function located
>> in firmware driver.
>
>OK, it can be just one function place it in any file you think is more appropriate
>need not be arasan controller driver. Any reasons why this can't work ? Can even
>be in some header.
>
>int keembay_sd_voltage_selection(int volt) {
>	int res;
>
>	arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt,
>&res)
>
>	/* appropriate error check if needed here */
>
>	return res;
>}
>
Yeah I believe it can work. I will create one header file in include/linux/firmware/intel/Keembay_firmware.h 
To handle this func and arasan controller can call this func.
Are you guys ok with this?

>> I will call this func during voltage switching from arasan controller.
>> I believe this implementation require DT to specify the compatible
>> name and method use either smc/hvc.
>
>No, use the standard one as detected by arm_smccc_1_1_invoke (It calls
>arm_smccc_get_conduit internally and use SMC/HVC based on that)
>
>>
>> Are you saying that by using simple smcc based function library I
>> should call below func() in arasan controller. For example
>> 1) arm_smccc_get_version(void)
>> 2) arm_smccc_version_init(arm_smccc_get_version(),
>SMCCC_CONDUIT_SMC);
>
>Nope
>
>> 3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID,
>voltage_value
>> ,  &res);
>
>Just this.
>
>--
>Regards,
>Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Tue, Oct 06, 2020 at 01:22:31AM +0000, Zulkifli, Muhammad Husaini wrote:
> HI Sudeep and Michal,
>
> Yeah I believe it can work. I will create one header file in include/linux/firmware/intel/Keembay_firmware.h
> To handle this func and arasan controller can call this func.
> Are you guys ok with this?
>

Sounds good to me. No change w.r.t arasan controller as it still needs
to call the same api(keembay_sd_voltage_selection), just w/o a firmware
driver for it.

--
Regards,
Sudeep

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Sudeep Holla]

On Tue, Oct 06, 2020 at 01:22:31AM +0000, Zulkifli, Muhammad Husaini wrote:
> HI Sudeep and Michal,
>
> Yeah I believe it can work. I will create one header file in include/linux/firmware/intel/Keembay_firmware.h
> To handle this func and arasan controller can call this func.
> Are you guys ok with this?
>

Sounds good to me. No change w.r.t arasan controller as it still needs
to call the same api(keembay_sd_voltage_selection), just w/o a firmware
driver for it.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Michal Simek]

Hi,

On 06. 10. 20 13:13, Sudeep Holla wrote:
> On Tue, Oct 06, 2020 at 01:22:31AM +0000, Zulkifli, Muhammad Husaini wrote:
>> HI Sudeep and Michal,
>>
>> Yeah I believe it can work. I will create one header file in include/linux/firmware/intel/Keembay_firmware.h
>> To handle this func and arasan controller can call this func.
>> Are you guys ok with this?
>>
> 
> Sounds good to me. No change w.r.t arasan controller as it still needs
> to call the same api(keembay_sd_voltage_selection), just w/o a firmware
> driver for it.

I am also fine with it. Just please make sure that driver can be
compiled also on non ARM platforms.

Thanks,
Michal

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Michal Simek]

Hi,

On 06. 10. 20 13:13, Sudeep Holla wrote:
> On Tue, Oct 06, 2020 at 01:22:31AM +0000, Zulkifli, Muhammad Husaini wrote:
>> HI Sudeep and Michal,
>>
>> Yeah I believe it can work. I will create one header file in include/linux/firmware/intel/Keembay_firmware.h
>> To handle this func and arasan controller can call this func.
>> Are you guys ok with this?
>>
> 
> Sounds good to me. No change w.r.t arasan controller as it still needs
> to call the same api(keembay_sd_voltage_selection), just w/o a firmware
> driver for it.

I am also fine with it. Just please make sure that driver can be
compiled also on non ARM platforms.

Thanks,
Michal

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Michal Simek]

Hi,

On 02. 10. 20 16:51, Sudeep Holla wrote:
> Hi Michal,
> 
> On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
>> Hi Sudeep,
>>
>> On 02. 10. 20 12:58, Sudeep Holla wrote:
>>> Hi Michal,
>>>
>>> On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>>>> Hi Sudeep,
>>>>
>>>> On 01. 10. 20 17:35, Sudeep Holla wrote:
>>>
>>> [...]
>>>
>>>>>
>>>>> What are the other uses of this KEEMBAY_SIP_* ?
>>>>> For now I tend to move this to the driver making use of it using
>>>>> arm_smccc_1_1_invoke directly if possible. I don't see the need for this
>>>>> to be separate driver. But do let us know the features implemented in the
>>>>> firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
>>>>> for some CPU errata implementation.
>>>>
>>>> This driver has been created based on my request to move it out the mmc
>>>> driver. It looks quite hacky to have arm_smccc_res and call
>>>> arm_smccc_smc() also with some IDs where it is visible that the part of
>>>> ID is just based on any spec.
>>>
>>> OK, driver is fine but no dt-bindings as it is discoverable. It can
>>> also be just a wrapper library instead as it needs no explicit
>>> initialisation like drivers to setup.
>>
>> I am fine with it. Do we have any example which we can point him to?
>>
> 
> You seem to have figured that out already with SOC_ID example.
> That was quick I must say 😄.

I would expect that instead of of

	if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_2)
		return 0;

	if (arm_smccc_1_1_get_conduit() == SMCCC_CONDUIT_NONE) {
		pr_err("%s: invalid SMCCC conduit\n", __func__);
		return -EOPNOTSUPP;
	}

	arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
			     ARM_SMCCC_ARCH_SOC_ID, &res);


you will simply call

	arm_smccc_1_2_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
			     ARM_SMCCC_ARCH_SOC_ID, &res);
	...(check ret)

	arm_smccc_1_2_invoke(ARM_SMCCC_ARCH_SOC_ID, 0, &res);
	...(check ret)


where it is clear from 1_2 that it has to be at least 1.2 version.

<snip>

> 
>>
>> BTW: I see you have added soc id reading which you are saying is the
>> part of smcc v1.2 but I can't see any implementation in TF-A. Is this
>> spec publicly available?
>>
> 
> Spec is out[1], include/linux/arm-smccc.h points to the latest spec.
> TF-A does have implementation as I tested with it and even reported
> bug that I discovered when I tested with my patches that are now merged
> upstream. Are you referring to master of TF-A or last release version ?
> If latter, it had bug and may not be working. I may be wrong though, as
> I am just telling what was told to me couple of months back and things
> might have changed in TF-A land.

I will read it and take a look when I have time.

Thanks,
Michal

Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call

[Michal Simek]

Hi,

On 02. 10. 20 16:51, Sudeep Holla wrote:
> Hi Michal,
> 
> On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote:
>> Hi Sudeep,
>>
>> On 02. 10. 20 12:58, Sudeep Holla wrote:
>>> Hi Michal,
>>>
>>> On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote:
>>>> Hi Sudeep,
>>>>
>>>> On 01. 10. 20 17:35, Sudeep Holla wrote:
>>>
>>> [...]
>>>
>>>>>
>>>>> What are the other uses of this KEEMBAY_SIP_* ?
>>>>> For now I tend to move this to the driver making use of it using
>>>>> arm_smccc_1_1_invoke directly if possible. I don't see the need for this
>>>>> to be separate driver. But do let us know the features implemented in the
>>>>> firmware. If it is not v1.1+, reasons for not upgrading as you need v1.1
>>>>> for some CPU errata implementation.
>>>>
>>>> This driver has been created based on my request to move it out the mmc
>>>> driver. It looks quite hacky to have arm_smccc_res and call
>>>> arm_smccc_smc() also with some IDs where it is visible that the part of
>>>> ID is just based on any spec.
>>>
>>> OK, driver is fine but no dt-bindings as it is discoverable. It can
>>> also be just a wrapper library instead as it needs no explicit
>>> initialisation like drivers to setup.
>>
>> I am fine with it. Do we have any example which we can point him to?
>>
> 
> You seem to have figured that out already with SOC_ID example.
> That was quick I must say 😄.

I would expect that instead of of

	if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_2)
		return 0;

	if (arm_smccc_1_1_get_conduit() == SMCCC_CONDUIT_NONE) {
		pr_err("%s: invalid SMCCC conduit\n", __func__);
		return -EOPNOTSUPP;
	}

	arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
			     ARM_SMCCC_ARCH_SOC_ID, &res);


you will simply call

	arm_smccc_1_2_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
			     ARM_SMCCC_ARCH_SOC_ID, &res);
	...(check ret)

	arm_smccc_1_2_invoke(ARM_SMCCC_ARCH_SOC_ID, 0, &res);
	...(check ret)


where it is clear from 1_2 that it has to be at least 1.2 version.

<snip>

> 
>>
>> BTW: I see you have added soc id reading which you are saying is the
>> part of smcc v1.2 but I can't see any implementation in TF-A. Is this
>> spec publicly available?
>>
> 
> Spec is out[1], include/linux/arm-smccc.h points to the latest spec.
> TF-A does have implementation as I tested with it and even reported
> bug that I discovered when I tested with my patches that are now merged
> upstream. Are you referring to master of TF-A or last release version ?
> If latter, it had bug and may not be working. I may be wrong though, as
> I am just telling what was told to me couple of months back and things
> might have changed in TF-A land.

I will read it and take a look when I have time.

Thanks,
Michal


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel