* [PATCH v2 0/4] Add support for SCMIv3.0 Voltage Domain Protocol and SCMI-Regulator
@ 2020-10-15 15:39 Cristian Marussi
2020-10-15 15:39 ` [PATCH v2 1/4] firmware: arm_scmi: Add Voltage Domain Support Cristian Marussi
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Cristian Marussi @ 2020-10-15 15:39 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: robh, satyakim, f.fainelli, vincent.guittot, sudeep.holla,
broonie, cristian.marussi, james.quinlan, Jonathan.Cameron,
souvik.chakravarty, etienne.carriere, lukasz.luba
Hi,
this series introduces the support for the new SCMI Voltage Domain Protocol
defined by the upcoming SCMIv3.0 specification, whose BETA release is
available at [1].
Afterwards, a new generic SCMI Regulator driver is developed on top of the
new SCMI VD Protocol.
The series is currently based on for-next/scmi [2] on top of:
commit fd7c58ee3026 ("firmware: arm_scmi: Fix locking in notifications")
Any feedback welcome,
Thanks,
Cristian
---
v1 --> v2
- rebased on for-next/scmi v5.10
- DT bindings
- removed any reference to negative voltages
- SCMI Regulator
- removed duplicate regulator naming
- removed redundant .get/set_voltage ops: only _sel variants implemented
- removed condexpr on fail path to increase readability
- VD Protocol
- fix voltage levels query loop to reload full cmd description
between iterations as reported by Etienne Carriere
- ensure transport rx buffer is properly sized calli scmi_reset_rx_to_maxsz
between transfers
[1]:https://developer.arm.com/documentation/den0056/c/
[2]:https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/log/?h=for-next/scmi
Cristian Marussi (4):
firmware: arm_scmi: Add Voltage Domain Support
firmware: arm_scmi: add SCMI Voltage Domain devname
regulator: add SCMI driver
dt-bindings: arm: add support for SCMI Regulators
.../devicetree/bindings/arm/arm,scmi.txt | 42 ++
drivers/firmware/arm_scmi/Makefile | 2 +-
drivers/firmware/arm_scmi/common.h | 1 +
drivers/firmware/arm_scmi/driver.c | 3 +
drivers/firmware/arm_scmi/voltage.c | 382 +++++++++++++++
drivers/regulator/Kconfig | 9 +
drivers/regulator/Makefile | 1 +
drivers/regulator/scmi-regulator.c | 453 ++++++++++++++++++
include/linux/scmi_protocol.h | 64 +++
9 files changed, 956 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/arm_scmi/voltage.c
create mode 100644 drivers/regulator/scmi-regulator.c
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/4] firmware: arm_scmi: Add Voltage Domain Support
2020-10-15 15:39 [PATCH v2 0/4] Add support for SCMIv3.0 Voltage Domain Protocol and SCMI-Regulator Cristian Marussi
@ 2020-10-15 15:39 ` Cristian Marussi
2020-10-16 13:36 ` Etienne Carriere
2020-10-15 15:39 ` [PATCH v2 2/4] firmware: arm_scmi: add SCMI Voltage Domain devname Cristian Marussi
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Cristian Marussi @ 2020-10-15 15:39 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: robh, satyakim, f.fainelli, vincent.guittot, sudeep.holla,
broonie, cristian.marussi, james.quinlan, Jonathan.Cameron,
souvik.chakravarty, etienne.carriere, lukasz.luba
Add SCMI Voltage Domain protocol support.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v1 --> v2
- fix voltage levels query loop to reload full cmd description
between iterations as reported by Etienne Carriere
- ensure rx buffer is properly sized calli scmi_reset_rx_to_maxsz
between transfers
---
drivers/firmware/arm_scmi/Makefile | 2 +-
drivers/firmware/arm_scmi/common.h | 1 +
drivers/firmware/arm_scmi/driver.c | 2 +
drivers/firmware/arm_scmi/voltage.c | 382 ++++++++++++++++++++++++++++
include/linux/scmi_protocol.h | 64 +++++
5 files changed, 450 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/arm_scmi/voltage.c
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index bc0d54f8e861..6a2ef63306d6 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -4,7 +4,7 @@ scmi-driver-y = driver.o notify.o
scmi-transport-y = shmem.o
scmi-transport-$(CONFIG_MAILBOX) += mailbox.o
scmi-transport-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smc.o
-scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o
+scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o
scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \
$(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 65063fa948d4..c0fb45e7c3e8 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -169,6 +169,7 @@ DECLARE_SCMI_REGISTER_UNREGISTER(perf);
DECLARE_SCMI_REGISTER_UNREGISTER(power);
DECLARE_SCMI_REGISTER_UNREGISTER(reset);
DECLARE_SCMI_REGISTER_UNREGISTER(sensors);
+DECLARE_SCMI_REGISTER_UNREGISTER(voltage);
DECLARE_SCMI_REGISTER_UNREGISTER(system);
#define DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(id, name) \
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 3dfd8b6a0ebf..ada35e63feae 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -946,6 +946,7 @@ static int __init scmi_driver_init(void)
scmi_power_register();
scmi_reset_register();
scmi_sensors_register();
+ scmi_voltage_register();
scmi_system_register();
return platform_driver_register(&scmi_driver);
@@ -961,6 +962,7 @@ static void __exit scmi_driver_exit(void)
scmi_power_unregister();
scmi_reset_unregister();
scmi_sensors_unregister();
+ scmi_voltage_unregister();
scmi_system_unregister();
platform_driver_unregister(&scmi_driver);
diff --git a/drivers/firmware/arm_scmi/voltage.c b/drivers/firmware/arm_scmi/voltage.c
new file mode 100644
index 000000000000..a20da5128de1
--- /dev/null
+++ b/drivers/firmware/arm_scmi/voltage.c
@@ -0,0 +1,382 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Voltage Protocol
+ *
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#include <linux/scmi_protocol.h>
+
+#include "common.h"
+
+#define VOLTAGE_DOMS_NUM_MASK GENMASK(15, 0)
+#define REMAINING_LEVELS_MASK GENMASK(31, 16)
+#define RETURNED_LEVELS_MASK GENMASK(11, 0)
+
+enum scmi_voltage_protocol_cmd {
+ VOLTAGE_DOMAIN_ATTRIBUTES = 0x3,
+ VOLTAGE_DESCRIBE_LEVELS = 0x4,
+ VOLTAGE_CONFIG_SET = 0x5,
+ VOLTAGE_CONFIG_GET = 0x6,
+ VOLTAGE_LEVEL_SET = 0x7,
+ VOLTAGE_LEVEL_GET = 0x8,
+};
+
+struct scmi_msg_resp_protocol_attributes {
+ __le32 attr;
+#define NUM_VOLTAGE_DOMAINS(x) (u16)(FIELD_GET(VOLTAGE_DOMS_NUM_MASK, (x)))
+};
+
+struct scmi_msg_resp_domain_attributes {
+ __le32 attr;
+ u8 name[SCMI_MAX_STR_SIZE];
+};
+
+struct scmi_msg_cmd_describe_levels {
+ __le32 domain_id;
+ __le32 level_index;
+};
+
+struct scmi_msg_resp_describe_levels {
+ __le32 flags;
+#define NUM_REMAINING_LEVELS(f) (u16)(FIELD_GET(REMAINING_LEVELS_MASK, (f)))
+#define NUM_RETURNED_LEVELS(f) (u16)(FIELD_GET(RETURNED_LEVELS_MASK, (f)))
+#define SUPPORTS_SEGMENTED_LEVELS(f) ((f) & BIT(12))
+ __le32 voltage[];
+};
+
+struct scmi_msg_cmd_config_set {
+ __le32 domain_id;
+ __le32 config;
+};
+
+struct scmi_msg_cmd_level_set {
+ __le32 domain_id;
+ __le32 flags;
+ __le32 voltage_level;
+};
+
+struct voltage_info {
+ u32 version;
+ u16 num_domains;
+ struct scmi_voltage_info **domains;
+};
+
+static int scmi_protocol_attributes_get(const struct scmi_handle *handle,
+ struct voltage_info *vinfo)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_resp_protocol_attributes *resp;
+
+ ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
+ SCMI_PROTOCOL_VOLTAGE, 0, sizeof(*resp), &t);
+ if (ret)
+ return ret;
+
+ resp = t->rx.buf;
+ ret = scmi_do_xfer(handle, t);
+ if (!ret)
+ vinfo->num_domains =
+ NUM_VOLTAGE_DOMAINS(le32_to_cpu(resp->attr));
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static inline int scmi_init_voltage_levels(struct device *dev,
+ struct scmi_voltage_info *v,
+ u32 flags, u32 num_levels)
+{
+ bool segmented;
+
+ segmented = SUPPORTS_SEGMENTED_LEVELS(flags);
+ /* segmented levels array's entries must be multiple-of-3 */
+ if (!num_levels || (segmented && num_levels % 3))
+ return -EINVAL;
+
+ v->levels_uv = devm_kcalloc(dev, num_levels, sizeof(u32), GFP_KERNEL);
+ if (!v->levels_uv)
+ return -ENOMEM;
+
+ v->num_levels = num_levels;
+ v->segmented = segmented;
+
+ return 0;
+}
+
+static int scmi_voltage_descriptors_get(const struct scmi_handle *handle,
+ struct voltage_info *vinfo)
+{
+ int ret, dom;
+ struct scmi_xfer *td, *tl;
+ struct device *dev = handle->dev;
+ struct scmi_msg_resp_domain_attributes *resp_dom;
+ struct scmi_msg_resp_describe_levels *resp_levels;
+
+ ret = scmi_xfer_get_init(handle, VOLTAGE_DOMAIN_ATTRIBUTES,
+ SCMI_PROTOCOL_VOLTAGE, sizeof(__le32),
+ sizeof(*resp_dom), &td);
+ if (ret)
+ return ret;
+ resp_dom = td->rx.buf;
+
+ ret = scmi_xfer_get_init(handle, VOLTAGE_DESCRIBE_LEVELS,
+ SCMI_PROTOCOL_VOLTAGE, sizeof(__le64), 0, &tl);
+ if (ret)
+ goto outd;
+ resp_levels = tl->rx.buf;
+
+ for (dom = 0; dom < vinfo->num_domains; dom++) {
+ u32 desc_index = 0;
+ u16 num_returned = 0, num_remaining = 0;
+ struct scmi_msg_cmd_describe_levels *cmd;
+ struct scmi_voltage_info *v;
+
+ /* Retrieve domain attributes at first ... */
+ put_unaligned_le32(dom, td->tx.buf);
+ ret = scmi_do_xfer(handle, td);
+ if (ret)
+ continue;
+
+ v = devm_kzalloc(dev, sizeof(*v), GFP_KERNEL);
+ if (!v) {
+ ret = -ENOMEM;
+ break;
+ }
+
+ v->id = dom;
+ v->attributes = le32_to_cpu(resp_dom->attr);
+ strlcpy(v->name, resp_dom->name, SCMI_MAX_STR_SIZE);
+
+ cmd = tl->tx.buf;
+ /* ...then retrieve domain levels descriptions */
+ do {
+ u32 flags;
+ int cnt;
+
+ cmd->domain_id = cpu_to_le32(v->id);
+ cmd->level_index = desc_index;
+ ret = scmi_do_xfer(handle, tl);
+ if (ret)
+ break;
+
+ flags = le32_to_cpu(resp_levels->flags);
+ num_returned = NUM_RETURNED_LEVELS(flags);
+ num_remaining = NUM_REMAINING_LEVELS(flags);
+
+ /* Allocate space for num_levels if not already done */
+ if (!v->num_levels) {
+ ret = scmi_init_voltage_levels(dev, v, flags,
+ num_returned +
+ num_remaining);
+ if (ret)
+ break;
+ }
+
+ if (desc_index + num_returned > v->num_levels) {
+ dev_err(handle->dev,
+ "No. of voltage levels can't exceed %d",
+ v->num_levels);
+ ret = -EINVAL;
+ break;
+ }
+
+ for (cnt = 0; cnt < num_returned; cnt++) {
+ s32 val;
+
+ val =
+ (s32)le32_to_cpu(resp_levels->voltage[cnt]);
+ v->levels_uv[desc_index + cnt] = val;
+ if (!v->negative_volts_allowed && val < 0)
+ v->negative_volts_allowed = true;
+ }
+
+ desc_index += num_returned;
+
+ scmi_reset_rx_to_maxsz(handle, tl);
+ /* check both to avoid infinite loop due to buggy fw */
+ } while (num_returned && num_remaining);
+
+ /*
+ * Bail out on memory errors, just skip domain on any
+ * other error.
+ */
+ if (!ret)
+ vinfo->domains[dom] = v;
+ else if (ret == -ENOMEM)
+ break;
+
+ scmi_reset_rx_to_maxsz(handle, td);
+ }
+
+ scmi_xfer_put(handle, tl);
+outd:
+ scmi_xfer_put(handle, td);
+
+ return ret;
+}
+
+static inline int __scmi_voltage_get_u32(const struct scmi_handle *handle,
+ u8 cmd_id, u32 domain_id, u32 *value)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct voltage_info *vinfo = handle->voltage_priv;
+
+ if (domain_id >= vinfo->num_domains)
+ return -EINVAL;
+
+ ret = scmi_xfer_get_init(handle, cmd_id,
+ SCMI_PROTOCOL_VOLTAGE,
+ sizeof(__le32), 0, &t);
+ if (ret)
+ return ret;
+
+ put_unaligned_le32(domain_id, t->tx.buf);
+ ret = scmi_do_xfer(handle, t);
+ if (!ret)
+ *value = get_unaligned_le32(t->rx.buf);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static int scmi_voltage_config_set(const struct scmi_handle *handle,
+ u32 domain_id, u32 config)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct voltage_info *vinfo = handle->voltage_priv;
+ struct scmi_msg_cmd_config_set *cmd;
+
+ if (domain_id >= vinfo->num_domains)
+ return -EINVAL;
+
+ ret = scmi_xfer_get_init(handle, VOLTAGE_CONFIG_SET,
+ SCMI_PROTOCOL_VOLTAGE,
+ sizeof(*cmd), 0, &t);
+ if (ret)
+ return ret;
+
+ cmd = t->tx.buf;
+ cmd->domain_id = cpu_to_le32(domain_id);
+ cmd->config = cpu_to_le32(config & GENMASK(3, 0));
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static int scmi_voltage_config_get(const struct scmi_handle *handle,
+ u32 domain_id, u32 *config)
+{
+ return __scmi_voltage_get_u32(handle, VOLTAGE_CONFIG_GET,
+ domain_id, config);
+}
+
+static int scmi_voltage_level_set(const struct scmi_handle *handle,
+ u32 domain_id, u32 flags, s32 volt_uV)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct voltage_info *vinfo = handle->voltage_priv;
+ struct scmi_msg_cmd_level_set *cmd;
+
+ if (domain_id >= vinfo->num_domains)
+ return -EINVAL;
+
+ ret = scmi_xfer_get_init(handle, VOLTAGE_LEVEL_SET,
+ SCMI_PROTOCOL_VOLTAGE,
+ sizeof(*cmd), 0, &t);
+ if (ret)
+ return ret;
+
+ cmd = t->tx.buf;
+ cmd->domain_id = cpu_to_le32(domain_id);
+ cmd->flags = cpu_to_le32(flags);
+ cmd->voltage_level = cpu_to_le32(volt_uV);
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static int scmi_voltage_level_get(const struct scmi_handle *handle,
+ u32 domain_id, s32 *volt_uV)
+{
+ return __scmi_voltage_get_u32(handle, VOLTAGE_LEVEL_GET,
+ domain_id, (u32 *)volt_uV);
+}
+
+static const struct scmi_voltage_info *
+scmi_voltage_info_get(const struct scmi_handle *handle, u32 domain_id)
+{
+ struct voltage_info *vinfo = handle->voltage_priv;
+
+ if (domain_id >= vinfo->num_domains)
+ return NULL;
+
+ return vinfo->domains[domain_id];
+}
+
+static int scmi_voltage_domains_num_get(const struct scmi_handle *handle)
+{
+ struct voltage_info *vinfo = handle->voltage_priv;
+
+ return vinfo->num_domains;
+}
+
+static struct scmi_voltage_ops voltage_ops = {
+ .num_domains_get = scmi_voltage_domains_num_get,
+ .info_get = scmi_voltage_info_get,
+ .config_set = scmi_voltage_config_set,
+ .config_get = scmi_voltage_config_get,
+ .level_set = scmi_voltage_level_set,
+ .level_get = scmi_voltage_level_get,
+};
+
+static int scmi_voltage_protocol_init(struct scmi_handle *handle)
+{
+ int ret;
+ u32 version;
+ struct voltage_info *vinfo;
+
+ ret = scmi_version_get(handle, SCMI_PROTOCOL_VOLTAGE, &version);
+ if (ret)
+ return ret;
+
+ dev_dbg(handle->dev, "Voltage Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ vinfo = devm_kzalloc(handle->dev, sizeof(*vinfo), GFP_KERNEL);
+ if (!vinfo)
+ return -ENOMEM;
+ vinfo->version = version;
+
+ ret = scmi_protocol_attributes_get(handle, vinfo);
+ if (ret)
+ return ret;
+
+ if (vinfo->num_domains) {
+ vinfo->domains = devm_kcalloc(handle->dev, vinfo->num_domains,
+ sizeof(vinfo->domains),
+ GFP_KERNEL);
+ if (!vinfo->domains)
+ return -ENOMEM;
+ ret = scmi_voltage_descriptors_get(handle, vinfo);
+ if (ret)
+ return ret;
+ } else {
+ dev_warn(handle->dev, "No Voltage domains found.\n");
+ }
+
+ handle->voltage_ops = &voltage_ops;
+ handle->voltage_priv = vinfo;
+
+ return 0;
+}
+
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_VOLTAGE, voltage)
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 9cd312a1ff92..032ad9bb2a53 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -209,6 +209,64 @@ struct scmi_reset_ops {
int (*deassert)(const struct scmi_handle *handle, u32 domain);
};
+/**
+ * struct scmi_voltage_info - describe one available SCMI Voltage Domain
+ *
+ * @id: the domain ID as advertised by the platform
+ * @segmented: defines the layout of the entries of array @levels_uv.
+ * - when True the entries are to be interpreted as triplets,
+ * each defining a segment representing a range of equally
+ * space voltages: <lowest_volts>, <highest_volt>, <step_uV>
+ * - when False the entries simply represent a single discrete
+ * supported voltage level
+ * @negative_volts_allowed: True if any of the entries of @levels_uv represent
+ * a negative voltage.
+ * @attributes: represents Voltage Domain advertised attributes
+ * @name: name assigned to the Voltage Domain by platform
+ * @num_levels: number of total entries in @levels_uv.
+ * @levels_uv: array of entries describing the available voltage levels for
+ * this domain.
+ */
+struct scmi_voltage_info {
+ u32 id;
+ bool segmented;
+#define SCMI_VOLTAGE_SEGMENT_LOW 0
+#define SCMI_VOLTAGE_SEGMENT_HIGH 1
+#define SCMI_VOLTAGE_SEGMENT_STEP 2
+ bool negative_volts_allowed;
+ u32 attributes;
+ char name[SCMI_MAX_STR_SIZE];
+ u16 num_levels;
+ s32 *levels_uv;
+};
+
+/**
+ * struct scmi_voltage_ops - represents the various operations provided
+ * by SCMI Voltage Protocol
+ *
+ * @num_domains_get: get the count of voltage domains provided by SCMI
+ * @info_get: get the information of the specified domain
+ * @config_set: set the config for the specified domain
+ * @config_get: get the config of the specified domain
+ * @level_set: set the voltage level for the specified domain
+ * @level_get: get the voltage level of the specified domain
+ */
+struct scmi_voltage_ops {
+ int (*num_domains_get)(const struct scmi_handle *handle);
+ const struct scmi_voltage_info *(*info_get)
+ (const struct scmi_handle *handle, u32 domain_id);
+ int (*config_set)(const struct scmi_handle *handle, u32 domain_id,
+ u32 config);
+#define SCMI_VOLTAGE_ARCH_STATE_OFF 0x0
+#define SCMI_VOLTAGE_ARCH_STATE_ON 0x7
+ int (*config_get)(const struct scmi_handle *handle, u32 domain_id,
+ u32 *config);
+ int (*level_set)(const struct scmi_handle *handle, u32 domain_id,
+ u32 flags, s32 volt_uV);
+ int (*level_get)(const struct scmi_handle *handle, u32 domain_id,
+ s32 *volt_uV);
+};
+
/**
* struct scmi_notify_ops - represents notifications' operations provided by
* SCMI core
@@ -262,6 +320,7 @@ struct scmi_notify_ops {
* @clk_ops: pointer to set of clock protocol operations
* @sensor_ops: pointer to set of sensor protocol operations
* @reset_ops: pointer to set of reset protocol operations
+ * @voltage_ops: pointer to set of voltage protocol operations
* @notify_ops: pointer to set of notifications related operations
* @perf_priv: pointer to private data structure specific to performance
* protocol(for internal use only)
@@ -273,6 +332,8 @@ struct scmi_notify_ops {
* protocol(for internal use only)
* @reset_priv: pointer to private data structure specific to reset
* protocol(for internal use only)
+ * @voltage_priv: pointer to private data structure specific to voltage
+ * protocol(for internal use only)
* @notify_priv: pointer to private data structure specific to notifications
* (for internal use only)
*/
@@ -284,6 +345,7 @@ struct scmi_handle {
const struct scmi_power_ops *power_ops;
const struct scmi_sensor_ops *sensor_ops;
const struct scmi_reset_ops *reset_ops;
+ const struct scmi_voltage_ops *voltage_ops;
const struct scmi_notify_ops *notify_ops;
/* for protocol internal use */
void *perf_priv;
@@ -291,6 +353,7 @@ struct scmi_handle {
void *power_priv;
void *sensor_priv;
void *reset_priv;
+ void *voltage_priv;
void *notify_priv;
void *system_priv;
};
@@ -303,6 +366,7 @@ enum scmi_std_protocol {
SCMI_PROTOCOL_CLOCK = 0x14,
SCMI_PROTOCOL_SENSOR = 0x15,
SCMI_PROTOCOL_RESET = 0x16,
+ SCMI_PROTOCOL_VOLTAGE = 0x17,
};
enum scmi_system_events {
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/4] firmware: arm_scmi: add SCMI Voltage Domain devname
2020-10-15 15:39 [PATCH v2 0/4] Add support for SCMIv3.0 Voltage Domain Protocol and SCMI-Regulator Cristian Marussi
2020-10-15 15:39 ` [PATCH v2 1/4] firmware: arm_scmi: Add Voltage Domain Support Cristian Marussi
@ 2020-10-15 15:39 ` Cristian Marussi
2020-10-15 15:40 ` [PATCH v2 3/4] regulator: add SCMI driver Cristian Marussi
2020-10-15 15:40 ` [PATCH v2 4/4] dt-bindings: arm: add support for SCMI Regulators Cristian Marussi
3 siblings, 0 replies; 9+ messages in thread
From: Cristian Marussi @ 2020-10-15 15:39 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: robh, satyakim, f.fainelli, vincent.guittot, sudeep.holla,
broonie, cristian.marussi, james.quinlan, Jonathan.Cameron,
souvik.chakravarty, etienne.carriere, lukasz.luba
Add SCMI Voltage Domain device name to the core list of supported protocol
devices.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index ada35e63feae..5392e1fc6b4e 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -743,6 +743,7 @@ static struct scmi_prot_devnames devnames[] = {
{ SCMI_PROTOCOL_CLOCK, { "clocks" },},
{ SCMI_PROTOCOL_SENSOR, { "hwmon" },},
{ SCMI_PROTOCOL_RESET, { "reset" },},
+ { SCMI_PROTOCOL_VOLTAGE, { "regulator" },},
};
static inline void
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] regulator: add SCMI driver
2020-10-15 15:39 [PATCH v2 0/4] Add support for SCMIv3.0 Voltage Domain Protocol and SCMI-Regulator Cristian Marussi
2020-10-15 15:39 ` [PATCH v2 1/4] firmware: arm_scmi: Add Voltage Domain Support Cristian Marussi
2020-10-15 15:39 ` [PATCH v2 2/4] firmware: arm_scmi: add SCMI Voltage Domain devname Cristian Marussi
@ 2020-10-15 15:40 ` Cristian Marussi
2020-10-16 14:09 ` Etienne Carriere
2020-10-15 15:40 ` [PATCH v2 4/4] dt-bindings: arm: add support for SCMI Regulators Cristian Marussi
3 siblings, 1 reply; 9+ messages in thread
From: Cristian Marussi @ 2020-10-15 15:40 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: robh, satyakim, f.fainelli, vincent.guittot, sudeep.holla,
broonie, cristian.marussi, james.quinlan, Jonathan.Cameron,
souvik.chakravarty, etienne.carriere, lukasz.luba
Add a simple regulator based on SCMI Voltage Domain Protocol.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
----
v1 --> v2
- removed duplicate regulator naming
- removed redundant .get/set_voltage ops: only _sel variants implemented
- removed condexpr on fail path to increase readability
v0 --> v1
- fixed init_data constraint parsing
- fixes for v5.8 (linear_range.h)
- fixed commit message content and subject line format
- factored out SCMI core specific changes to distinct patch
- reworked Kconfig and Makefile to keep proper alphabetic order
- fixed SPDX comment style
- removed unneeded inline functions
- reworked conditionals for legibility
- fixed some return paths to properly report SCMI original errors codes
- added some more descriptive error messages when fw returns invalid ranges
- removed unneeded explicit devm_regulator_unregister from .remove()
---
drivers/regulator/Kconfig | 9 +
drivers/regulator/Makefile | 1 +
drivers/regulator/scmi-regulator.c | 453 +++++++++++++++++++++++++++++
3 files changed, 463 insertions(+)
create mode 100644 drivers/regulator/scmi-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index de17ef7e18f0..6d3a10cb9833 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -155,6 +155,15 @@ config REGULATOR_ARIZONA_MICSUPP
and Wolfson Microelectronic Arizona codecs
devices.
+config REGULATOR_ARM_SCMI
+ tristate "SCMI based regulator driver"
+ depends on ARM_SCMI_PROTOCOL && OF
+ help
+ This adds the regulator driver support for ARM platforms using SCMI
+ protocol for device voltage management.
+ This driver uses SCMI Message Protocol driver to interact with the
+ firmware providing the device Voltage functionality.
+
config REGULATOR_AS3711
tristate "AS3711 PMIC"
depends on MFD_AS3711
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index d8d3ecf526a8..0532a7393d5d 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_REGULATOR_AD5398) += ad5398.o
obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o
obj-$(CONFIG_REGULATOR_ARIZONA_LDO1) += arizona-ldo1.o
obj-$(CONFIG_REGULATOR_ARIZONA_MICSUPP) += arizona-micsupp.o
+obj-$(CONFIG_REGULATOR_ARM_SCMI) += scmi-regulator.o
obj-$(CONFIG_REGULATOR_AS3711) += as3711-regulator.o
obj-$(CONFIG_REGULATOR_AS3722) += as3722-regulator.o
obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o
diff --git a/drivers/regulator/scmi-regulator.c b/drivers/regulator/scmi-regulator.c
new file mode 100644
index 000000000000..e4e7d0345723
--- /dev/null
+++ b/drivers/regulator/scmi-regulator.c
@@ -0,0 +1,453 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// System Control and Management Interface (SCMI) based regulator driver
+//
+// Copyright (C) 2020 ARM Ltd.
+//
+// Implements a regulator driver on top of the SCMI Voltage Protocol.
+//
+// The ARM SCMI Protocol aims in general to hide as much as possible all the
+// underlying operational details while providing an abstracted interface for
+// its users to operate upon: as a consequence the resulting operational
+// capabilities and configurability of this regulator device are much more
+// limited than the ones usually available on a standard physical regulator.
+//
+// The supported SCMI regulator ops are restricted to the bare minimum:
+//
+// - 'status_ops': enable/disable/is_enabled
+// - 'voltage_ops': get_voltage_sel/set_voltage_sel
+// list_voltage/map_voltage
+//
+// Each SCMI regulator instance is associated, through the means of a proper DT
+// entry description, to a specific SCMI Voltage Domain.
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/linear_range.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/scmi_protocol.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+struct scmi_regulator {
+ u32 id;
+ struct scmi_device *sdev;
+ struct regulator_dev *rdev;
+ struct device_node *of_node;
+ struct regulator_desc desc;
+ struct regulator_config conf;
+};
+
+struct scmi_regulator_info {
+ int num_doms;
+ struct scmi_regulator **sregv;
+};
+
+static int scmi_reg_enable(struct regulator_dev *rdev)
+{
+ struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
+ const struct scmi_handle *handle = sreg->sdev->handle;
+
+ return handle->voltage_ops->config_set(handle, sreg->id,
+ SCMI_VOLTAGE_ARCH_STATE_ON);
+}
+
+static int scmi_reg_disable(struct regulator_dev *rdev)
+{
+ struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
+ const struct scmi_handle *handle = sreg->sdev->handle;
+
+ return handle->voltage_ops->config_set(handle, sreg->id,
+ SCMI_VOLTAGE_ARCH_STATE_OFF);
+}
+
+static int scmi_reg_is_enabled(struct regulator_dev *rdev)
+{
+ int ret;
+ u32 config;
+ struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
+ const struct scmi_handle *handle = sreg->sdev->handle;
+
+ ret = handle->voltage_ops->config_get(handle, sreg->id,
+ &config);
+ if (ret) {
+ dev_err(&sreg->sdev->dev,
+ "Error %d reading regulator %s status.\n",
+ ret, sreg->desc.name);
+ return 0;
+ }
+
+ return config & SCMI_VOLTAGE_ARCH_STATE_ON;
+}
+
+static int scmi_reg_get_voltage_sel(struct regulator_dev *rdev)
+{
+ int ret;
+ s32 volt_uV;
+ struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
+ const struct scmi_handle *handle = sreg->sdev->handle;
+
+ ret = handle->voltage_ops->level_get(handle, sreg->id, &volt_uV);
+ if (ret)
+ return ret;
+
+ return sreg->desc.ops->map_voltage(rdev, volt_uV, volt_uV);
+}
+
+static int scmi_reg_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int selector)
+{
+ int ret;
+ s32 volt_uV;
+ struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
+ const struct scmi_handle *handle = sreg->sdev->handle;
+
+ volt_uV = sreg->desc.ops->list_voltage(rdev, selector);
+ if (volt_uV <= 0)
+ return -EINVAL;
+
+ ret = handle->voltage_ops->level_set(handle, sreg->id, 0x0, volt_uV);
+ if (ret)
+ return ret;
+
+ return ret;
+}
+
+static const struct regulator_ops scmi_reg_fixed_ops = {
+ .enable = scmi_reg_enable,
+ .disable = scmi_reg_disable,
+ .is_enabled = scmi_reg_is_enabled,
+};
+
+static const struct regulator_ops scmi_reg_linear_ops = {
+ .enable = scmi_reg_enable,
+ .disable = scmi_reg_disable,
+ .is_enabled = scmi_reg_is_enabled,
+ .get_voltage_sel = scmi_reg_get_voltage_sel,
+ .set_voltage_sel = scmi_reg_set_voltage_sel,
+ .list_voltage = regulator_list_voltage_linear,
+ .map_voltage = regulator_map_voltage_linear,
+};
+
+static const struct regulator_ops scmi_reg_range_ops = {
+ .enable = scmi_reg_enable,
+ .disable = scmi_reg_disable,
+ .is_enabled = scmi_reg_is_enabled,
+ .get_voltage_sel = scmi_reg_get_voltage_sel,
+ .set_voltage_sel = scmi_reg_set_voltage_sel,
+ .list_voltage = regulator_list_voltage_linear_range,
+ .map_voltage = regulator_map_voltage_linear_range,
+};
+
+static const struct regulator_ops scmi_reg_discrete_ops = {
+ .enable = scmi_reg_enable,
+ .disable = scmi_reg_disable,
+ .is_enabled = scmi_reg_is_enabled,
+ .get_voltage_sel = scmi_reg_get_voltage_sel,
+ .set_voltage_sel = scmi_reg_set_voltage_sel,
+ .list_voltage = regulator_list_voltage_table,
+ .map_voltage = regulator_map_voltage_iterate,
+};
+
+static int
+scmi_config_linear_regulator_mappings(struct scmi_regulator *sreg,
+ const struct scmi_voltage_info *vinfo)
+{
+ /*
+ * Note that SCMI voltage domains describable by linear ranges
+ * (segments) {low, high, step} are guaranteed to come in triplets by
+ * the SCMI Voltage Domain protocol support itself.
+ */
+ if (vinfo->num_levels == 3) {
+ s32 delta_uV;
+
+ delta_uV = (vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH] -
+ vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW]);
+ /* Rule out buggy negative-intervals answers from fw */
+ if (delta_uV < 0) {
+ dev_err(&sreg->sdev->dev,
+ "Invalid volt-range %d-%duV for domain %d\n",
+ vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW],
+ vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH],
+ sreg->id);
+ return -EINVAL;
+ }
+
+ if (!delta_uV) {
+ /* Just one fixed voltage exposed by SCMI */
+ sreg->desc.fixed_uV =
+ vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW];
+ sreg->desc.n_voltages = 1;
+ sreg->desc.ops = &scmi_reg_fixed_ops;
+ } else {
+ /* One simple linear mapping. */
+ sreg->desc.min_uV =
+ vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW];
+ sreg->desc.uV_step =
+ vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_STEP];
+ sreg->desc.linear_min_sel = 0;
+ sreg->desc.n_voltages = delta_uV / sreg->desc.uV_step;
+ sreg->desc.ops = &scmi_reg_linear_ops;
+ }
+ } else {
+ /* Multiple linear mappings. */
+ int i, num_ranges, last_max = -1;
+ struct linear_range *lr;
+
+ num_ranges = vinfo->num_levels / 3;
+ lr = devm_kcalloc(&sreg->sdev->dev, num_ranges,
+ sizeof(*lr), GFP_KERNEL);
+ if (!lr)
+ return -ENOMEM;
+
+ sreg->desc.n_linear_ranges = num_ranges;
+ sreg->desc.linear_ranges = lr;
+ for (i = 0; num_ranges; num_ranges--, i += 3, lr++) {
+ s32 delta_uV;
+
+ lr->min =
+ vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_LOW];
+ lr->step =
+ vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_STEP];
+ delta_uV =
+ vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_HIGH] -
+ lr->min;
+ if (delta_uV <= 0 || !(delta_uV / lr->step)) {
+ dev_err(&sreg->sdev->dev,
+ "Invalid volt-range %d-%duV for domain %d\n",
+ vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW],
+ vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH],
+ sreg->id);
+ return -EINVAL;
+ }
+ lr->max_sel = delta_uV / lr->step - 1;
+ lr->min_sel = last_max + 1;
+ last_max = lr->max_sel;
+ }
+ sreg->desc.n_voltages = last_max + 1;
+ sreg->desc.ops = &scmi_reg_range_ops;
+ }
+
+ return 0;
+}
+
+static int
+scmi_config_discrete_regulator_mappings(struct scmi_regulator *sreg,
+ const struct scmi_voltage_info *vinfo)
+{
+ /* Discrete non linear levels are mapped to volt_table */
+ sreg->desc.n_voltages = vinfo->num_levels;
+ if (sreg->desc.n_voltages > 1) {
+ sreg->desc.volt_table = (const unsigned int *)vinfo->levels_uv;
+ sreg->desc.ops = &scmi_reg_discrete_ops;
+ } else {
+ sreg->desc.fixed_uV = vinfo->levels_uv[0];
+ sreg->desc.ops = &scmi_reg_fixed_ops;
+ }
+
+ return 0;
+}
+
+static int scmi_regulator_common_init(struct scmi_regulator *sreg)
+{
+ int ret;
+ const struct scmi_handle *handle = sreg->sdev->handle;
+ struct device *dev = &sreg->sdev->dev;
+ const struct scmi_voltage_info *vinfo;
+
+ vinfo = handle->voltage_ops->info_get(handle, sreg->id);
+ if (!vinfo)
+ return -ENODEV;
+
+ if (!vinfo->num_levels)
+ return -EINVAL;
+
+ /*
+ * Regulator framework does not fully support negative voltages
+ * so we discard any voltage domain reported as supporting negative
+ * voltages: as a consequence each levels_uv entry is guaranteed to
+ * be non-negative from here on.
+ */
+ if (vinfo->negative_volts_allowed) {
+ dev_warn(dev, "Negative voltages NOT supported...skip %s\n",
+ sreg->of_node->full_name);
+ return -EOPNOTSUPP;
+ }
+
+ sreg->desc.name = devm_kasprintf(dev, GFP_KERNEL, "%s", vinfo->name);
+ if (!sreg->desc.name)
+ return -ENOMEM;
+
+ sreg->desc.id = sreg->id;
+ sreg->desc.type = REGULATOR_VOLTAGE;
+ sreg->desc.owner = THIS_MODULE;
+ sreg->desc.of_match = sreg->of_node->name;
+ sreg->desc.regulators_node = "regulators";
+ if (vinfo->segmented)
+ ret = scmi_config_linear_regulator_mappings(sreg, vinfo);
+ else
+ ret = scmi_config_discrete_regulator_mappings(sreg, vinfo);
+ if (ret)
+ return ret;
+
+ sreg->conf.dev = dev;
+ sreg->conf.driver_data = sreg;
+
+ return 0;
+}
+
+static int process_scmi_regulator_of_node(struct scmi_device *sdev,
+ struct device_node *np,
+ struct scmi_regulator_info *rinfo)
+{
+ u32 dom, ret;
+
+ ret = of_property_read_u32(np, "reg", &dom);
+ if (ret)
+ return ret;
+
+ if (dom >= rinfo->num_doms)
+ return -ENODEV;
+
+ if (rinfo->sregv[dom]) {
+ dev_err(&sdev->dev,
+ "SCMI Voltage Domain %d already in use. Skipping: %s\n",
+ dom, np->full_name);
+ return -EINVAL;
+ }
+
+ rinfo->sregv[dom] = devm_kzalloc(&sdev->dev,
+ sizeof(struct scmi_regulator),
+ GFP_KERNEL);
+ if (!rinfo->sregv[dom])
+ return -ENOMEM;
+
+ rinfo->sregv[dom]->id = dom;
+ rinfo->sregv[dom]->sdev = sdev;
+ /* get hold of good nodes */
+ of_node_get(np);
+ rinfo->sregv[dom]->of_node = np;
+ dev_info(&sdev->dev,
+ "Found valid SCMI Regulator -- OF node [%d] -> %s\n",
+ dom, np->full_name);
+
+ return ret;
+}
+
+static int scmi_regulator_probe(struct scmi_device *sdev)
+{
+ int d, ret, num_doms;
+ struct device_node *np, *child;
+ const struct scmi_handle *handle = sdev->handle;
+ struct scmi_regulator_info *rinfo;
+
+ if (!handle || !handle->voltage_ops)
+ return -ENODEV;
+
+ num_doms = handle->voltage_ops->num_domains_get(handle);
+ if (num_doms <= 0) {
+ if (!num_doms) {
+ dev_err(&sdev->dev,
+ "number of voltage domains invalid\n");
+ num_doms = -EINVAL;
+ } else {
+ dev_err(&sdev->dev,
+ "failed to get voltage domains - err:%d\n",
+ num_doms);
+ }
+
+ return num_doms;
+ }
+
+ rinfo = devm_kzalloc(&sdev->dev, sizeof(*rinfo), GFP_KERNEL);
+ if (!rinfo)
+ return -ENOMEM;
+
+ /* Allocate pointers' array for all possible domains */
+ rinfo->sregv = devm_kcalloc(&sdev->dev, num_doms,
+ sizeof(rinfo->sregv), GFP_KERNEL);
+ if (!rinfo->sregv)
+ return -ENOMEM;
+
+ rinfo->num_doms = num_doms;
+ /*
+ * Start collecting into rinfo->sregv possibly good SCMI Regulators as
+ * described by a well-formed DT entry and associated with an existing
+ * plausible SCMI Voltage Domain number, all belonging to this SCMI
+ * platform instance node (handle->dev->of_node).
+ */
+ np = of_find_node_by_name(handle->dev->of_node, "regulators");
+ for_each_child_of_node(np, child) {
+ ret = process_scmi_regulator_of_node(sdev, child, rinfo);
+ /* abort on any mem issue */
+ if (ret == -ENOMEM)
+ return ret;
+ }
+
+ /*
+ * Register a regulator for each valid regulator-DT-entry that we
+ * can successfully reach via SCMI
+ */
+ for (d = 0; d < num_doms; d++) {
+ struct scmi_regulator *sreg = rinfo->sregv[d];
+
+ if (!sreg)
+ continue;
+ ret = scmi_regulator_common_init(sreg);
+ if (ret)
+ continue;
+ sreg->rdev = devm_regulator_register(&sdev->dev, &sreg->desc,
+ &sreg->conf);
+ if (IS_ERR(sreg->rdev)) {
+ sreg->rdev = NULL;
+ continue;
+ }
+
+ dev_info(&sdev->dev,
+ "Regulator %s registered for domain [%d](%s)\n",
+ sreg->desc.name, sreg->id, sreg->desc.name);
+ }
+
+ dev_set_drvdata(&sdev->dev, rinfo);
+
+ return 0;
+}
+
+static void scmi_regulator_remove(struct scmi_device *sdev)
+{
+ int d;
+ struct scmi_regulator_info *rinfo;
+
+ rinfo = dev_get_drvdata(&sdev->dev);
+ if (!rinfo)
+ return;
+
+ for (d = 0; d < rinfo->num_doms; d++) {
+ if (!rinfo->sregv[d])
+ continue;
+ of_node_put(rinfo->sregv[d]->of_node);
+ }
+}
+
+static const struct scmi_device_id scmi_regulator_id_table[] = {
+ { SCMI_PROTOCOL_VOLTAGE, "regulator" },
+ { },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_regulator_id_table);
+
+static struct scmi_driver scmi_drv = {
+ .name = "scmi-regulator",
+ .probe = scmi_regulator_probe,
+ .remove = scmi_regulator_remove,
+ .id_table = scmi_regulator_id_table,
+};
+
+module_scmi_driver(scmi_drv);
+
+MODULE_AUTHOR("Cristian Marussi <cristian.marussi@arm.com>");
+MODULE_DESCRIPTION("ARM SCMI regulator driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] dt-bindings: arm: add support for SCMI Regulators
2020-10-15 15:39 [PATCH v2 0/4] Add support for SCMIv3.0 Voltage Domain Protocol and SCMI-Regulator Cristian Marussi
` (2 preceding siblings ...)
2020-10-15 15:40 ` [PATCH v2 3/4] regulator: add SCMI driver Cristian Marussi
@ 2020-10-15 15:40 ` Cristian Marussi
3 siblings, 0 replies; 9+ messages in thread
From: Cristian Marussi @ 2020-10-15 15:40 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: robh, satyakim, f.fainelli, vincent.guittot, sudeep.holla,
broonie, cristian.marussi, james.quinlan, Jonathan.Cameron,
souvik.chakravarty, etienne.carriere, lukasz.luba
Add devicetree bindings to support regulators based on SCMI Voltage
Domain Protocol.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v1 --> v2
- removed any reference to negative voltages
---
.../devicetree/bindings/arm/arm,scmi.txt | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
index 55deb68230eb..6d3d0bdab322 100644
--- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
@@ -62,6 +62,28 @@ Required properties:
- #power-domain-cells : Should be 1. Contains the device or the power
domain ID value used by SCMI commands.
+Regulator bindings for the SCMI Regulator based on SCMI Message Protocol
+------------------------------------------------------------
+
+An SCMI Regulator is permanently bound to a well defined SCMI Voltage Domain,
+and should be always positioned as a root regulator.
+It does not support any current operation.
+
+This binding uses the common regulator binding[6].
+
+SCMI Regulators are grouped under a 'regulators' node which in turn is a child
+of the SCMI Voltage protocol node inside the desired SCMI instance node.
+
+Required properties:
+ - reg : shall identify an existent SCMI Voltage Domain.
+
+Optional properties:
+ - all of the other standard regulator bindings as in [6]: note that, since
+ the SCMI Protocol itself aims in fact to hide away many of the operational
+ capabilities usually exposed by the properties of a standard regulator,
+ most of the usual regulator bindings could have just no effect in the
+ context of this SCMI regulator.
+
Sensor bindings for the sensors based on SCMI Message Protocol
--------------------------------------------------------------
SCMI provides an API to access the various sensors on the SoC.
@@ -105,6 +127,7 @@ Required sub-node properties:
[3] Documentation/devicetree/bindings/thermal/thermal*.yaml
[4] Documentation/devicetree/bindings/sram/sram.yaml
[5] Documentation/devicetree/bindings/reset/reset.txt
+[6] Documentation/devicetree/bindings/regulator/regulator.yaml
Example:
@@ -169,6 +192,25 @@ firmware {
reg = <0x16>;
#reset-cells = <1>;
};
+
+ scmi_voltage: protocol@17 {
+ reg = <0x17>;
+
+ regulators {
+ regulator_cpu: regulator_scmi_cpu@0 {
+ reg = <0x0>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ regulator_gpu: regulator_scmi_gpu@9 {
+ reg = <0x9>;
+ regulator-min-microvolt = <500000>;
+ regulator-max-microvolt = <4200000>;
+ };
+
+ ...
+ };
+ };
};
};
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] firmware: arm_scmi: Add Voltage Domain Support
2020-10-15 15:39 ` [PATCH v2 1/4] firmware: arm_scmi: Add Voltage Domain Support Cristian Marussi
@ 2020-10-16 13:36 ` Etienne Carriere
2020-10-16 16:51 ` Cristian Marussi
0 siblings, 1 reply; 9+ messages in thread
From: Etienne Carriere @ 2020-10-16 13:36 UTC (permalink / raw)
To: Cristian Marussi
Cc: Rob Herring, satyakim, f.fainelli, Vincent Guittot, Sudeep Holla,
linux-kernel, broonie,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Jim Quinlan, Jonathan.Cameron, Souvik Chakravarty, lukasz.luba
Hello Cristian,
Thanks for the update.
Some minor comments below.
FYI I successfully tested this series.
Regards,
Etienne
On Thu, 15 Oct 2020 at 17:40, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> Add SCMI Voltage Domain protocol support.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v1 --> v2
> - fix voltage levels query loop to reload full cmd description
> between iterations as reported by Etienne Carriere
> - ensure rx buffer is properly sized calli scmi_reset_rx_to_maxsz
> between transfers
> ---
> drivers/firmware/arm_scmi/Makefile | 2 +-
> drivers/firmware/arm_scmi/common.h | 1 +
> drivers/firmware/arm_scmi/driver.c | 2 +
> drivers/firmware/arm_scmi/voltage.c | 382 ++++++++++++++++++++++++++++
> include/linux/scmi_protocol.h | 64 +++++
> 5 files changed, 450 insertions(+), 1 deletion(-)
> create mode 100644 drivers/firmware/arm_scmi/voltage.c
>
> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> index bc0d54f8e861..6a2ef63306d6 100644
> --- a/drivers/firmware/arm_scmi/Makefile
> +++ b/drivers/firmware/arm_scmi/Makefile
> @@ -4,7 +4,7 @@ scmi-driver-y = driver.o notify.o
> scmi-transport-y = shmem.o
> scmi-transport-$(CONFIG_MAILBOX) += mailbox.o
> scmi-transport-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smc.o
> -scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o
> +scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o
> scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \
> $(scmi-transport-y)
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 65063fa948d4..c0fb45e7c3e8 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -169,6 +169,7 @@ DECLARE_SCMI_REGISTER_UNREGISTER(perf);
> DECLARE_SCMI_REGISTER_UNREGISTER(power);
> DECLARE_SCMI_REGISTER_UNREGISTER(reset);
> DECLARE_SCMI_REGISTER_UNREGISTER(sensors);
> +DECLARE_SCMI_REGISTER_UNREGISTER(voltage);
> DECLARE_SCMI_REGISTER_UNREGISTER(system);
>
> #define DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(id, name) \
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 3dfd8b6a0ebf..ada35e63feae 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -946,6 +946,7 @@ static int __init scmi_driver_init(void)
> scmi_power_register();
> scmi_reset_register();
> scmi_sensors_register();
> + scmi_voltage_register();
> scmi_system_register();
>
> return platform_driver_register(&scmi_driver);
> @@ -961,6 +962,7 @@ static void __exit scmi_driver_exit(void)
> scmi_power_unregister();
> scmi_reset_unregister();
> scmi_sensors_unregister();
> + scmi_voltage_unregister();
> scmi_system_unregister();
>
> platform_driver_unregister(&scmi_driver);
> diff --git a/drivers/firmware/arm_scmi/voltage.c b/drivers/firmware/arm_scmi/voltage.c
> new file mode 100644
> index 000000000000..a20da5128de1
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/voltage.c
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System Control and Management Interface (SCMI) Voltage Protocol
> + *
> + * Copyright (C) 2020 ARM Ltd.
> + */
> +
> +#include <linux/scmi_protocol.h>
> +
> +#include "common.h"
> +
> +#define VOLTAGE_DOMS_NUM_MASK GENMASK(15, 0)
> +#define REMAINING_LEVELS_MASK GENMASK(31, 16)
> +#define RETURNED_LEVELS_MASK GENMASK(11, 0)
> +
> +enum scmi_voltage_protocol_cmd {
> + VOLTAGE_DOMAIN_ATTRIBUTES = 0x3,
> + VOLTAGE_DESCRIBE_LEVELS = 0x4,
> + VOLTAGE_CONFIG_SET = 0x5,
> + VOLTAGE_CONFIG_GET = 0x6,
> + VOLTAGE_LEVEL_SET = 0x7,
> + VOLTAGE_LEVEL_GET = 0x8,
> +};
> +
> +struct scmi_msg_resp_protocol_attributes {
> + __le32 attr;
> +#define NUM_VOLTAGE_DOMAINS(x) (u16)(FIELD_GET(VOLTAGE_DOMS_NUM_MASK, (x)))
> +};
> +
> +struct scmi_msg_resp_domain_attributes {
> + __le32 attr;
> + u8 name[SCMI_MAX_STR_SIZE];
> +};
> +
> +struct scmi_msg_cmd_describe_levels {
> + __le32 domain_id;
> + __le32 level_index;
> +};
> +
> +struct scmi_msg_resp_describe_levels {
> + __le32 flags;
> +#define NUM_REMAINING_LEVELS(f) (u16)(FIELD_GET(REMAINING_LEVELS_MASK, (f)))
> +#define NUM_RETURNED_LEVELS(f) (u16)(FIELD_GET(RETURNED_LEVELS_MASK, (f)))
> +#define SUPPORTS_SEGMENTED_LEVELS(f) ((f) & BIT(12))
> + __le32 voltage[];
> +};
> +
> +struct scmi_msg_cmd_config_set {
> + __le32 domain_id;
> + __le32 config;
> +};
> +
> +struct scmi_msg_cmd_level_set {
> + __le32 domain_id;
> + __le32 flags;
> + __le32 voltage_level;
> +};
> +
> +struct voltage_info {
> + u32 version;
> + u16 num_domains;
> + struct scmi_voltage_info **domains;
> +};
> +
> +static int scmi_protocol_attributes_get(const struct scmi_handle *handle,
> + struct voltage_info *vinfo)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_resp_protocol_attributes *resp;
> +
> + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
> + SCMI_PROTOCOL_VOLTAGE, 0, sizeof(*resp), &t);
> + if (ret)
> + return ret;
> +
> + resp = t->rx.buf;
> + ret = scmi_do_xfer(handle, t);
> + if (!ret)
> + vinfo->num_domains =
> + NUM_VOLTAGE_DOMAINS(le32_to_cpu(resp->attr));
> +
> + scmi_xfer_put(handle, t);
> + return ret;
> +}
> +
> +static inline int scmi_init_voltage_levels(struct device *dev,
> + struct scmi_voltage_info *v,
> + u32 flags, u32 num_levels)
> +{
> + bool segmented;
> +
> + segmented = SUPPORTS_SEGMENTED_LEVELS(flags);
> + /* segmented levels array's entries must be multiple-of-3 */
> + if (!num_levels || (segmented && num_levels % 3))
> + return -EINVAL;
I think segment levels are described by a single triplet.
If right, should test (!num_levels || (segmented && num_levels == 3)).
> +
> + v->levels_uv = devm_kcalloc(dev, num_levels, sizeof(u32), GFP_KERNEL);
> + if (!v->levels_uv)
> + return -ENOMEM;
> +
> + v->num_levels = num_levels;
> + v->segmented = segmented;
> +
> + return 0;
> +}
> +
> +static int scmi_voltage_descriptors_get(const struct scmi_handle *handle,
> + struct voltage_info *vinfo)
> +{
> + int ret, dom;
> + struct scmi_xfer *td, *tl;
> + struct device *dev = handle->dev;
> + struct scmi_msg_resp_domain_attributes *resp_dom;
> + struct scmi_msg_resp_describe_levels *resp_levels;
> +
> + ret = scmi_xfer_get_init(handle, VOLTAGE_DOMAIN_ATTRIBUTES,
> + SCMI_PROTOCOL_VOLTAGE, sizeof(__le32),
> + sizeof(*resp_dom), &td);
> + if (ret)
> + return ret;
> + resp_dom = td->rx.buf;
> +
> + ret = scmi_xfer_get_init(handle, VOLTAGE_DESCRIBE_LEVELS,
> + SCMI_PROTOCOL_VOLTAGE, sizeof(__le64), 0, &tl);
> + if (ret)
> + goto outd;
> + resp_levels = tl->rx.buf;
> +
> + for (dom = 0; dom < vinfo->num_domains; dom++) {
> + u32 desc_index = 0;
> + u16 num_returned = 0, num_remaining = 0;
> + struct scmi_msg_cmd_describe_levels *cmd;
> + struct scmi_voltage_info *v;
> +
> + /* Retrieve domain attributes at first ... */
> + put_unaligned_le32(dom, td->tx.buf);
> + ret = scmi_do_xfer(handle, td);
> + if (ret)
> + continue;
Continue and break and report the error code?
Seems you prefer to abort only on local error (ENOMEM), not comm error.
Maybe state with nice inline comment as done below (/* Skip domain on error */).
> +
> + v = devm_kzalloc(dev, sizeof(*v), GFP_KERNEL);
> + if (!v) {
> + ret = -ENOMEM;
> + break;
> + }
> +
> + v->id = dom;
> + v->attributes = le32_to_cpu(resp_dom->attr);
> + strlcpy(v->name, resp_dom->name, SCMI_MAX_STR_SIZE);
> +
> + cmd = tl->tx.buf;
> + /* ...then retrieve domain levels descriptions */
> + do {
> + u32 flags;
> + int cnt;
> +
> + cmd->domain_id = cpu_to_le32(v->id);
> + cmd->level_index = desc_index;
> + ret = scmi_do_xfer(handle, tl);
> + if (ret)
> + break;
> +
> + flags = le32_to_cpu(resp_levels->flags);
> + num_returned = NUM_RETURNED_LEVELS(flags);
> + num_remaining = NUM_REMAINING_LEVELS(flags);
> +
> + /* Allocate space for num_levels if not already done */
> + if (!v->num_levels) {
> + ret = scmi_init_voltage_levels(dev, v, flags,
> + num_returned +
> + num_remaining);
> + if (ret)
> + break;
> + }
> +
> + if (desc_index + num_returned > v->num_levels) {
> + dev_err(handle->dev,
> + "No. of voltage levels can't exceed %d",
Missing '\n'.
> + v->num_levels);
> + ret = -EINVAL;
> + break;
> + }
> +
> + for (cnt = 0; cnt < num_returned; cnt++) {
> + s32 val;
> +
> + val =
> + (s32)le32_to_cpu(resp_levels->voltage[cnt]);
> + v->levels_uv[desc_index + cnt] = val;
> + if (!v->negative_volts_allowed && val < 0)
could skip testing v->negative_volts_allowed.
if (val < 0)
v->negative_volts_allowed = true;
> + v->negative_volts_allowed = true;
> + }
> +
> + desc_index += num_returned;
> +
> + scmi_reset_rx_to_maxsz(handle, tl);
> + /* check both to avoid infinite loop due to buggy fw */
> + } while (num_returned && num_remaining);
> +
> + /*
> + * Bail out on memory errors, just skip domain on any
> + * other error.
> + */
> + if (!ret)
> + vinfo->domains[dom] = v;
> + else if (ret == -ENOMEM)
> + break;
> +
> + scmi_reset_rx_to_maxsz(handle, td);
> + }
> +
> + scmi_xfer_put(handle, tl);
> +outd:
> + scmi_xfer_put(handle, td);
> +
> + return ret;
> +}
> +
> +static inline int __scmi_voltage_get_u32(const struct scmi_handle *handle,
inline needed ?
> + u8 cmd_id, u32 domain_id, u32 *value)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct voltage_info *vinfo = handle->voltage_priv;
> +
> + if (domain_id >= vinfo->num_domains)
> + return -EINVAL;
> +
> + ret = scmi_xfer_get_init(handle, cmd_id,
> + SCMI_PROTOCOL_VOLTAGE,
> + sizeof(__le32), 0, &t);
> + if (ret)
> + return ret;
> +
> + put_unaligned_le32(domain_id, t->tx.buf);
> + ret = scmi_do_xfer(handle, t);
> + if (!ret)
> + *value = get_unaligned_le32(t->rx.buf);
> +
> + scmi_xfer_put(handle, t);
> + return ret;
> +}
> +
> +static int scmi_voltage_config_set(const struct scmi_handle *handle,
> + u32 domain_id, u32 config)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct voltage_info *vinfo = handle->voltage_priv;
> + struct scmi_msg_cmd_config_set *cmd;
> +
> + if (domain_id >= vinfo->num_domains)
> + return -EINVAL;
> +
> + ret = scmi_xfer_get_init(handle, VOLTAGE_CONFIG_SET,
> + SCMI_PROTOCOL_VOLTAGE,
> + sizeof(*cmd), 0, &t);
> + if (ret)
> + return ret;
> +
> + cmd = t->tx.buf;
> + cmd->domain_id = cpu_to_le32(domain_id);
> + cmd->config = cpu_to_le32(config & GENMASK(3, 0));
> +
> + ret = scmi_do_xfer(handle, t);
> +
> + scmi_xfer_put(handle, t);
> + return ret;
> +}
> +
> +static int scmi_voltage_config_get(const struct scmi_handle *handle,
> + u32 domain_id, u32 *config)
> +{
> + return __scmi_voltage_get_u32(handle, VOLTAGE_CONFIG_GET,
> + domain_id, config);
> +}
> +
> +static int scmi_voltage_level_set(const struct scmi_handle *handle,
> + u32 domain_id, u32 flags, s32 volt_uV)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct voltage_info *vinfo = handle->voltage_priv;
> + struct scmi_msg_cmd_level_set *cmd;
> +
> + if (domain_id >= vinfo->num_domains)
> + return -EINVAL;
> +
> + ret = scmi_xfer_get_init(handle, VOLTAGE_LEVEL_SET,
> + SCMI_PROTOCOL_VOLTAGE,
> + sizeof(*cmd), 0, &t);
> + if (ret)
> + return ret;
> +
> + cmd = t->tx.buf;
> + cmd->domain_id = cpu_to_le32(domain_id);
> + cmd->flags = cpu_to_le32(flags);
> + cmd->voltage_level = cpu_to_le32(volt_uV);
> +
> + ret = scmi_do_xfer(handle, t);
> +
> + scmi_xfer_put(handle, t);
> + return ret;
> +}
> +
> +static int scmi_voltage_level_get(const struct scmi_handle *handle,
> + u32 domain_id, s32 *volt_uV)
> +{
> + return __scmi_voltage_get_u32(handle, VOLTAGE_LEVEL_GET,
> + domain_id, (u32 *)volt_uV);
> +}
> +
> +static const struct scmi_voltage_info *
> +scmi_voltage_info_get(const struct scmi_handle *handle, u32 domain_id)
> +{
> + struct voltage_info *vinfo = handle->voltage_priv;
> +
> + if (domain_id >= vinfo->num_domains)
> + return NULL;
> +
> + return vinfo->domains[domain_id];
> +}
> +
> +static int scmi_voltage_domains_num_get(const struct scmi_handle *handle)
> +{
> + struct voltage_info *vinfo = handle->voltage_priv;
> +
> + return vinfo->num_domains;
> +}
> +
> +static struct scmi_voltage_ops voltage_ops = {
> + .num_domains_get = scmi_voltage_domains_num_get,
> + .info_get = scmi_voltage_info_get,
> + .config_set = scmi_voltage_config_set,
> + .config_get = scmi_voltage_config_get,
> + .level_set = scmi_voltage_level_set,
> + .level_get = scmi_voltage_level_get,
> +};
> +
> +static int scmi_voltage_protocol_init(struct scmi_handle *handle)
> +{
> + int ret;
> + u32 version;
> + struct voltage_info *vinfo;
> +
> + ret = scmi_version_get(handle, SCMI_PROTOCOL_VOLTAGE, &version);
> + if (ret)
> + return ret;
> +
> + dev_dbg(handle->dev, "Voltage Version %d.%d\n",
> + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
> +
> + vinfo = devm_kzalloc(handle->dev, sizeof(*vinfo), GFP_KERNEL);
> + if (!vinfo)
> + return -ENOMEM;
> + vinfo->version = version;
> +
> + ret = scmi_protocol_attributes_get(handle, vinfo);
> + if (ret)
> + return ret;
> +
> + if (vinfo->num_domains) {
> + vinfo->domains = devm_kcalloc(handle->dev, vinfo->num_domains,
> + sizeof(vinfo->domains),
> + GFP_KERNEL);
> + if (!vinfo->domains)
> + return -ENOMEM;
> + ret = scmi_voltage_descriptors_get(handle, vinfo);
> + if (ret)
> + return ret;
> + } else {
> + dev_warn(handle->dev, "No Voltage domains found.\n");
> + }
> +
> + handle->voltage_ops = &voltage_ops;
> + handle->voltage_priv = vinfo;
> +
> + return 0;
> +}
> +
> +DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_VOLTAGE, voltage)
> diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
> index 9cd312a1ff92..032ad9bb2a53 100644
> --- a/include/linux/scmi_protocol.h
> +++ b/include/linux/scmi_protocol.h
> @@ -209,6 +209,64 @@ struct scmi_reset_ops {
> int (*deassert)(const struct scmi_handle *handle, u32 domain);
> };
>
> +/**
> + * struct scmi_voltage_info - describe one available SCMI Voltage Domain
> + *
> + * @id: the domain ID as advertised by the platform
> + * @segmented: defines the layout of the entries of array @levels_uv.
> + * - when True the entries are to be interpreted as triplets,
> + * each defining a segment representing a range of equally
> + * space voltages: <lowest_volts>, <highest_volt>, <step_uV>
> + * - when False the entries simply represent a single discrete
> + * supported voltage level
> + * @negative_volts_allowed: True if any of the entries of @levels_uv represent
> + * a negative voltage.
> + * @attributes: represents Voltage Domain advertised attributes
> + * @name: name assigned to the Voltage Domain by platform
> + * @num_levels: number of total entries in @levels_uv.
> + * @levels_uv: array of entries describing the available voltage levels for
> + * this domain.
> + */
> +struct scmi_voltage_info {
> + u32 id;
> + bool segmented;
> +#define SCMI_VOLTAGE_SEGMENT_LOW 0
> +#define SCMI_VOLTAGE_SEGMENT_HIGH 1
> +#define SCMI_VOLTAGE_SEGMENT_STEP 2
> + bool negative_volts_allowed;
> + u32 attributes;
> + char name[SCMI_MAX_STR_SIZE];
> + u16 num_levels;
> + s32 *levels_uv;
> +};
> +
> +/**
> + * struct scmi_voltage_ops - represents the various operations provided
> + * by SCMI Voltage Protocol
> + *
> + * @num_domains_get: get the count of voltage domains provided by SCMI
> + * @info_get: get the information of the specified domain
> + * @config_set: set the config for the specified domain
> + * @config_get: get the config of the specified domain
> + * @level_set: set the voltage level for the specified domain
> + * @level_get: get the voltage level of the specified domain
> + */
> +struct scmi_voltage_ops {
> + int (*num_domains_get)(const struct scmi_handle *handle);
> + const struct scmi_voltage_info *(*info_get)
> + (const struct scmi_handle *handle, u32 domain_id);
> + int (*config_set)(const struct scmi_handle *handle, u32 domain_id,
> + u32 config);
> +#define SCMI_VOLTAGE_ARCH_STATE_OFF 0x0
> +#define SCMI_VOLTAGE_ARCH_STATE_ON 0x7
> + int (*config_get)(const struct scmi_handle *handle, u32 domain_id,
> + u32 *config);
> + int (*level_set)(const struct scmi_handle *handle, u32 domain_id,
> + u32 flags, s32 volt_uV);
> + int (*level_get)(const struct scmi_handle *handle, u32 domain_id,
> + s32 *volt_uV);
> +};
> +
> /**
> * struct scmi_notify_ops - represents notifications' operations provided by
> * SCMI core
> @@ -262,6 +320,7 @@ struct scmi_notify_ops {
> * @clk_ops: pointer to set of clock protocol operations
> * @sensor_ops: pointer to set of sensor protocol operations
> * @reset_ops: pointer to set of reset protocol operations
> + * @voltage_ops: pointer to set of voltage protocol operations
> * @notify_ops: pointer to set of notifications related operations
> * @perf_priv: pointer to private data structure specific to performance
> * protocol(for internal use only)
> @@ -273,6 +332,8 @@ struct scmi_notify_ops {
> * protocol(for internal use only)
> * @reset_priv: pointer to private data structure specific to reset
> * protocol(for internal use only)
> + * @voltage_priv: pointer to private data structure specific to voltage
> + * protocol(for internal use only)
> * @notify_priv: pointer to private data structure specific to notifications
> * (for internal use only)
> */
> @@ -284,6 +345,7 @@ struct scmi_handle {
> const struct scmi_power_ops *power_ops;
> const struct scmi_sensor_ops *sensor_ops;
> const struct scmi_reset_ops *reset_ops;
> + const struct scmi_voltage_ops *voltage_ops;
> const struct scmi_notify_ops *notify_ops;
> /* for protocol internal use */
> void *perf_priv;
> @@ -291,6 +353,7 @@ struct scmi_handle {
> void *power_priv;
> void *sensor_priv;
> void *reset_priv;
> + void *voltage_priv;
> void *notify_priv;
> void *system_priv;
> };
> @@ -303,6 +366,7 @@ enum scmi_std_protocol {
> SCMI_PROTOCOL_CLOCK = 0x14,
> SCMI_PROTOCOL_SENSOR = 0x15,
> SCMI_PROTOCOL_RESET = 0x16,
> + SCMI_PROTOCOL_VOLTAGE = 0x17,
> };
>
> enum scmi_system_events {
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/4] regulator: add SCMI driver
2020-10-15 15:40 ` [PATCH v2 3/4] regulator: add SCMI driver Cristian Marussi
@ 2020-10-16 14:09 ` Etienne Carriere
2020-10-16 16:55 ` Cristian Marussi
0 siblings, 1 reply; 9+ messages in thread
From: Etienne Carriere @ 2020-10-16 14:09 UTC (permalink / raw)
To: Cristian Marussi
Cc: Rob Herring, satyakim, f.fainelli, Vincent Guittot, Sudeep Holla,
linux-kernel, broonie,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Jim Quinlan, Jonathan.Cameron, Souvik Chakravarty, lukasz.luba
Hi Cristian,
Some minor comments...
Etienne
On Thu, 15 Oct 2020 at 17:40, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> Add a simple regulator based on SCMI Voltage Domain Protocol.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ----
> v1 --> v2
> - removed duplicate regulator naming
> - removed redundant .get/set_voltage ops: only _sel variants implemented
> - removed condexpr on fail path to increase readability
>
> v0 --> v1
> - fixed init_data constraint parsing
> - fixes for v5.8 (linear_range.h)
> - fixed commit message content and subject line format
> - factored out SCMI core specific changes to distinct patch
> - reworked Kconfig and Makefile to keep proper alphabetic order
> - fixed SPDX comment style
> - removed unneeded inline functions
> - reworked conditionals for legibility
> - fixed some return paths to properly report SCMI original errors codes
> - added some more descriptive error messages when fw returns invalid ranges
> - removed unneeded explicit devm_regulator_unregister from .remove()
> ---
> drivers/regulator/Kconfig | 9 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/scmi-regulator.c | 453 +++++++++++++++++++++++++++++
> 3 files changed, 463 insertions(+)
> create mode 100644 drivers/regulator/scmi-regulator.c
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index de17ef7e18f0..6d3a10cb9833 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -155,6 +155,15 @@ config REGULATOR_ARIZONA_MICSUPP
> and Wolfson Microelectronic Arizona codecs
> devices.
>
> +config REGULATOR_ARM_SCMI
> + tristate "SCMI based regulator driver"
> + depends on ARM_SCMI_PROTOCOL && OF
> + help
> + This adds the regulator driver support for ARM platforms using SCMI
> + protocol for device voltage management.
> + This driver uses SCMI Message Protocol driver to interact with the
> + firmware providing the device Voltage functionality.
> +
> config REGULATOR_AS3711
> tristate "AS3711 PMIC"
> depends on MFD_AS3711
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index d8d3ecf526a8..0532a7393d5d 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_REGULATOR_AD5398) += ad5398.o
> obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o
> obj-$(CONFIG_REGULATOR_ARIZONA_LDO1) += arizona-ldo1.o
> obj-$(CONFIG_REGULATOR_ARIZONA_MICSUPP) += arizona-micsupp.o
> +obj-$(CONFIG_REGULATOR_ARM_SCMI) += scmi-regulator.o
> obj-$(CONFIG_REGULATOR_AS3711) += as3711-regulator.o
> obj-$(CONFIG_REGULATOR_AS3722) += as3722-regulator.o
> obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o
> diff --git a/drivers/regulator/scmi-regulator.c b/drivers/regulator/scmi-regulator.c
> new file mode 100644
> index 000000000000..e4e7d0345723
> --- /dev/null
> +++ b/drivers/regulator/scmi-regulator.c
> @@ -0,0 +1,453 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// System Control and Management Interface (SCMI) based regulator driver
> +//
> +// Copyright (C) 2020 ARM Ltd.
> +//
> +// Implements a regulator driver on top of the SCMI Voltage Protocol.
> +//
> +// The ARM SCMI Protocol aims in general to hide as much as possible all the
> +// underlying operational details while providing an abstracted interface for
> +// its users to operate upon: as a consequence the resulting operational
> +// capabilities and configurability of this regulator device are much more
> +// limited than the ones usually available on a standard physical regulator.
> +//
> +// The supported SCMI regulator ops are restricted to the bare minimum:
> +//
> +// - 'status_ops': enable/disable/is_enabled
> +// - 'voltage_ops': get_voltage_sel/set_voltage_sel
> +// list_voltage/map_voltage
> +//
> +// Each SCMI regulator instance is associated, through the means of a proper DT
> +// entry description, to a specific SCMI Voltage Domain.
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/linear_range.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/of_regulator.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +struct scmi_regulator {
> + u32 id;
> + struct scmi_device *sdev;
> + struct regulator_dev *rdev;
> + struct device_node *of_node;
> + struct regulator_desc desc;
> + struct regulator_config conf;
> +};
> +
> +struct scmi_regulator_info {
> + int num_doms;
> + struct scmi_regulator **sregv;
> +};
> +
> +static int scmi_reg_enable(struct regulator_dev *rdev)
> +{
> + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> + const struct scmi_handle *handle = sreg->sdev->handle;
> +
> + return handle->voltage_ops->config_set(handle, sreg->id,
> + SCMI_VOLTAGE_ARCH_STATE_ON);
> +}
> +
> +static int scmi_reg_disable(struct regulator_dev *rdev)
> +{
> + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> + const struct scmi_handle *handle = sreg->sdev->handle;
> +
> + return handle->voltage_ops->config_set(handle, sreg->id,
> + SCMI_VOLTAGE_ARCH_STATE_OFF);
> +}
> +
> +static int scmi_reg_is_enabled(struct regulator_dev *rdev)
> +{
> + int ret;
> + u32 config;
> + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> + const struct scmi_handle *handle = sreg->sdev->handle;
> +
> + ret = handle->voltage_ops->config_get(handle, sreg->id,
> + &config);
> + if (ret) {
> + dev_err(&sreg->sdev->dev,
> + "Error %d reading regulator %s status.\n",
> + ret, sreg->desc.name);
> + return 0;
> + }
> +
> + return config & SCMI_VOLTAGE_ARCH_STATE_ON;
> +}
> +
> +static int scmi_reg_get_voltage_sel(struct regulator_dev *rdev)
> +{
> + int ret;
> + s32 volt_uV;
> + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> + const struct scmi_handle *handle = sreg->sdev->handle;
> +
> + ret = handle->voltage_ops->level_get(handle, sreg->id, &volt_uV);
> + if (ret)
> + return ret;
> +
> + return sreg->desc.ops->map_voltage(rdev, volt_uV, volt_uV);
> +}
> +
> +static int scmi_reg_set_voltage_sel(struct regulator_dev *rdev,
> + unsigned int selector)
> +{
> + int ret;
> + s32 volt_uV;
> + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> + const struct scmi_handle *handle = sreg->sdev->handle;
> +
> + volt_uV = sreg->desc.ops->list_voltage(rdev, selector);
> + if (volt_uV <= 0)
> + return -EINVAL;
> +
> + ret = handle->voltage_ops->level_set(handle, sreg->id, 0x0, volt_uV);
> + if (ret)
> + return ret;
> +
> + return ret;
return handle->voltage_ops->level_set(handle, sreg->id, 0x0, volt_uV);
> +}
> +
> +static const struct regulator_ops scmi_reg_fixed_ops = {
> + .enable = scmi_reg_enable,
> + .disable = scmi_reg_disable,
> + .is_enabled = scmi_reg_is_enabled,
> +};
> +
> +static const struct regulator_ops scmi_reg_linear_ops = {
> + .enable = scmi_reg_enable,
> + .disable = scmi_reg_disable,
> + .is_enabled = scmi_reg_is_enabled,
> + .get_voltage_sel = scmi_reg_get_voltage_sel,
> + .set_voltage_sel = scmi_reg_set_voltage_sel,
> + .list_voltage = regulator_list_voltage_linear,
> + .map_voltage = regulator_map_voltage_linear,
> +};
> +
> +static const struct regulator_ops scmi_reg_range_ops = {
> + .enable = scmi_reg_enable,
> + .disable = scmi_reg_disable,
> + .is_enabled = scmi_reg_is_enabled,
> + .get_voltage_sel = scmi_reg_get_voltage_sel,
> + .set_voltage_sel = scmi_reg_set_voltage_sel,
> + .list_voltage = regulator_list_voltage_linear_range,
> + .map_voltage = regulator_map_voltage_linear_range,
> +};
> +
> +static const struct regulator_ops scmi_reg_discrete_ops = {
> + .enable = scmi_reg_enable,
> + .disable = scmi_reg_disable,
> + .is_enabled = scmi_reg_is_enabled,
> + .get_voltage_sel = scmi_reg_get_voltage_sel,
> + .set_voltage_sel = scmi_reg_set_voltage_sel,
> + .list_voltage = regulator_list_voltage_table,
> + .map_voltage = regulator_map_voltage_iterate,
> +};
> +
> +static int
> +scmi_config_linear_regulator_mappings(struct scmi_regulator *sreg,
> + const struct scmi_voltage_info *vinfo)
> +{
> + /*
> + * Note that SCMI voltage domains describable by linear ranges
> + * (segments) {low, high, step} are guaranteed to come in triplets by
> + * the SCMI Voltage Domain protocol support itself.
> + */
> + if (vinfo->num_levels == 3) {
> + s32 delta_uV;
> +
> + delta_uV = (vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH] -
> + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW]);
> + /* Rule out buggy negative-intervals answers from fw */
> + if (delta_uV < 0) {
> + dev_err(&sreg->sdev->dev,
> + "Invalid volt-range %d-%duV for domain %d\n",
> + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW],
> + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH],
> + sreg->id);
> + return -EINVAL;
> + }
> +
> + if (!delta_uV) {
> + /* Just one fixed voltage exposed by SCMI */
> + sreg->desc.fixed_uV =
> + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW];
> + sreg->desc.n_voltages = 1;
> + sreg->desc.ops = &scmi_reg_fixed_ops;
> + } else {
> + /* One simple linear mapping. */
> + sreg->desc.min_uV =
> + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW];
> + sreg->desc.uV_step =
> + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_STEP];
> + sreg->desc.linear_min_sel = 0;
> + sreg->desc.n_voltages = delta_uV / sreg->desc.uV_step;
> + sreg->desc.ops = &scmi_reg_linear_ops;
> + }
> + } else {
> + /* Multiple linear mappings. */
I don't see this multi-linear mapping in the v3.0 beta of the spec.
Is it something planned?
> + int i, num_ranges, last_max = -1;
> + struct linear_range *lr;
> +
> + num_ranges = vinfo->num_levels / 3;
> + lr = devm_kcalloc(&sreg->sdev->dev, num_ranges,
> + sizeof(*lr), GFP_KERNEL);
> + if (!lr)
> + return -ENOMEM;
> +
> + sreg->desc.n_linear_ranges = num_ranges;
> + sreg->desc.linear_ranges = lr;
> + for (i = 0; num_ranges; num_ranges--, i += 3, lr++) {
> + s32 delta_uV;
> +
> + lr->min =
> + vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_LOW];
> + lr->step =
> + vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_STEP];
> + delta_uV =
> + vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_HIGH] -
> + lr->min;
> + if (delta_uV <= 0 || !(delta_uV / lr->step)) {
> + dev_err(&sreg->sdev->dev,
> + "Invalid volt-range %d-%duV for domain %d\n",
> + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW],
> + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH],
> + sreg->id);
> + return -EINVAL;
> + }
> + lr->max_sel = delta_uV / lr->step - 1;
> + lr->min_sel = last_max + 1;
> + last_max = lr->max_sel;
> + }
> + sreg->desc.n_voltages = last_max + 1;
> + sreg->desc.ops = &scmi_reg_range_ops;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +scmi_config_discrete_regulator_mappings(struct scmi_regulator *sreg,
> + const struct scmi_voltage_info *vinfo)
> +{
> + /* Discrete non linear levels are mapped to volt_table */
> + sreg->desc.n_voltages = vinfo->num_levels;
> + if (sreg->desc.n_voltages > 1) {
> + sreg->desc.volt_table = (const unsigned int *)vinfo->levels_uv;
> + sreg->desc.ops = &scmi_reg_discrete_ops;
> + } else {
> + sreg->desc.fixed_uV = vinfo->levels_uv[0];
> + sreg->desc.ops = &scmi_reg_fixed_ops;
> + }
> +
> + return 0;
> +}
> +
> +static int scmi_regulator_common_init(struct scmi_regulator *sreg)
> +{
> + int ret;
> + const struct scmi_handle *handle = sreg->sdev->handle;
> + struct device *dev = &sreg->sdev->dev;
> + const struct scmi_voltage_info *vinfo;
> +
> + vinfo = handle->voltage_ops->info_get(handle, sreg->id);
> + if (!vinfo)
> + return -ENODEV;
> +
> + if (!vinfo->num_levels)
> + return -EINVAL;
> +
> + /*
> + * Regulator framework does not fully support negative voltages
> + * so we discard any voltage domain reported as supporting negative
> + * voltages: as a consequence each levels_uv entry is guaranteed to
> + * be non-negative from here on.
> + */
> + if (vinfo->negative_volts_allowed) {
> + dev_warn(dev, "Negative voltages NOT supported...skip %s\n",
> + sreg->of_node->full_name);
> + return -EOPNOTSUPP;
> + }
> +
> + sreg->desc.name = devm_kasprintf(dev, GFP_KERNEL, "%s", vinfo->name);
> + if (!sreg->desc.name)
> + return -ENOMEM;
> +
> + sreg->desc.id = sreg->id;
> + sreg->desc.type = REGULATOR_VOLTAGE;
> + sreg->desc.owner = THIS_MODULE;
> + sreg->desc.of_match = sreg->of_node->name;
> + sreg->desc.regulators_node = "regulators";
> + if (vinfo->segmented)
> + ret = scmi_config_linear_regulator_mappings(sreg, vinfo);
> + else
> + ret = scmi_config_discrete_regulator_mappings(sreg, vinfo);
> + if (ret)
> + return ret;
> +
> + sreg->conf.dev = dev;
> + sreg->conf.driver_data = sreg;
> +
> + return 0;
> +}
> +
> +static int process_scmi_regulator_of_node(struct scmi_device *sdev,
> + struct device_node *np,
> + struct scmi_regulator_info *rinfo)
> +{
> + u32 dom, ret;
> +
> + ret = of_property_read_u32(np, "reg", &dom);
> + if (ret)
> + return ret;
> +
> + if (dom >= rinfo->num_doms)
> + return -ENODEV;
> +
> + if (rinfo->sregv[dom]) {
> + dev_err(&sdev->dev,
> + "SCMI Voltage Domain %d already in use. Skipping: %s\n",
> + dom, np->full_name);
> + return -EINVAL;
> + }
> +
> + rinfo->sregv[dom] = devm_kzalloc(&sdev->dev,
> + sizeof(struct scmi_regulator),
> + GFP_KERNEL);
> + if (!rinfo->sregv[dom])
> + return -ENOMEM;
> +
> + rinfo->sregv[dom]->id = dom;
> + rinfo->sregv[dom]->sdev = sdev;
> + /* get hold of good nodes */
> + of_node_get(np);
> + rinfo->sregv[dom]->of_node = np;
> + dev_info(&sdev->dev,
> + "Found valid SCMI Regulator -- OF node [%d] -> %s\n",
> + dom, np->full_name);
> +
> + return ret;
suggest return 0; here.
> +}
> +
> +static int scmi_regulator_probe(struct scmi_device *sdev)
> +{
> + int d, ret, num_doms;
> + struct device_node *np, *child;
> + const struct scmi_handle *handle = sdev->handle;
> + struct scmi_regulator_info *rinfo;
> +
> + if (!handle || !handle->voltage_ops)
> + return -ENODEV;
> +
> + num_doms = handle->voltage_ops->num_domains_get(handle);
> + if (num_doms <= 0) {
> + if (!num_doms) {
> + dev_err(&sdev->dev,
> + "number of voltage domains invalid\n");
> + num_doms = -EINVAL;
> + } else {
> + dev_err(&sdev->dev,
> + "failed to get voltage domains - err:%d\n",
> + num_doms);
> + }
> +
> + return num_doms;
> + }
> +
> + rinfo = devm_kzalloc(&sdev->dev, sizeof(*rinfo), GFP_KERNEL);
> + if (!rinfo)
> + return -ENOMEM;
> +
> + /* Allocate pointers' array for all possible domains */
> + rinfo->sregv = devm_kcalloc(&sdev->dev, num_doms,
> + sizeof(rinfo->sregv), GFP_KERNEL);
> + if (!rinfo->sregv)
> + return -ENOMEM;
> +
> + rinfo->num_doms = num_doms;
> + /*
> + * Start collecting into rinfo->sregv possibly good SCMI Regulators as
> + * described by a well-formed DT entry and associated with an existing
> + * plausible SCMI Voltage Domain number, all belonging to this SCMI
> + * platform instance node (handle->dev->of_node).
> + */
> + np = of_find_node_by_name(handle->dev->of_node, "regulators");
> + for_each_child_of_node(np, child) {
> + ret = process_scmi_regulator_of_node(sdev, child, rinfo);
> + /* abort on any mem issue */
> + if (ret == -ENOMEM)
> + return ret;
> + }
> +
> + /*
> + * Register a regulator for each valid regulator-DT-entry that we
> + * can successfully reach via SCMI
> + */
> + for (d = 0; d < num_doms; d++) {
> + struct scmi_regulator *sreg = rinfo->sregv[d];
> +
> + if (!sreg)
> + continue;
> + ret = scmi_regulator_common_init(sreg);
> + if (ret)
> + continue;
> + sreg->rdev = devm_regulator_register(&sdev->dev, &sreg->desc,
> + &sreg->conf);
> + if (IS_ERR(sreg->rdev)) {
> + sreg->rdev = NULL;
> + continue;
> + }
> +
> + dev_info(&sdev->dev,
> + "Regulator %s registered for domain [%d](%s)\n",
> + sreg->desc.name, sreg->id, sreg->desc.name);
Maybe not display sreg->desc.name twice.
> + }
> +
> + dev_set_drvdata(&sdev->dev, rinfo);
> +
> + return 0;
> +}
> +
> +static void scmi_regulator_remove(struct scmi_device *sdev)
> +{
> + int d;
> + struct scmi_regulator_info *rinfo;
> +
> + rinfo = dev_get_drvdata(&sdev->dev);
> + if (!rinfo)
> + return;
> +
> + for (d = 0; d < rinfo->num_doms; d++) {
> + if (!rinfo->sregv[d])
> + continue;
> + of_node_put(rinfo->sregv[d]->of_node);
> + }
> +}
> +
> +static const struct scmi_device_id scmi_regulator_id_table[] = {
> + { SCMI_PROTOCOL_VOLTAGE, "regulator" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(scmi, scmi_regulator_id_table);
> +
> +static struct scmi_driver scmi_drv = {
> + .name = "scmi-regulator",
> + .probe = scmi_regulator_probe,
> + .remove = scmi_regulator_remove,
> + .id_table = scmi_regulator_id_table,
> +};
> +
> +module_scmi_driver(scmi_drv);
> +
> +MODULE_AUTHOR("Cristian Marussi <cristian.marussi@arm.com>");
> +MODULE_DESCRIPTION("ARM SCMI regulator driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] firmware: arm_scmi: Add Voltage Domain Support
2020-10-16 13:36 ` Etienne Carriere
@ 2020-10-16 16:51 ` Cristian Marussi
0 siblings, 0 replies; 9+ messages in thread
From: Cristian Marussi @ 2020-10-16 16:51 UTC (permalink / raw)
To: Etienne Carriere
Cc: Rob Herring, satyakim, f.fainelli, Vincent Guittot, Sudeep Holla,
linux-kernel, broonie,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Jim Quinlan, Jonathan.Cameron, Souvik Chakravarty, lukasz.luba
On Fri, Oct 16, 2020 at 03:36:51PM +0200, Etienne Carriere wrote:
> Hello Cristian,
>
> Thanks for the update.
> Some minor comments below.
> FYI I successfully tested this series.
>
Hi Etienne
thanks for the review and for testing !
> Regards,
> Etienne
>
> On Thu, 15 Oct 2020 at 17:40, Cristian Marussi <cristian.marussi@arm.com> wrote:
> >
> > Add SCMI Voltage Domain protocol support.
> >
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> > v1 --> v2
> > - fix voltage levels query loop to reload full cmd description
> > between iterations as reported by Etienne Carriere
> > - ensure rx buffer is properly sized calli scmi_reset_rx_to_maxsz
> > between transfers
> > ---
> > drivers/firmware/arm_scmi/Makefile | 2 +-
> > drivers/firmware/arm_scmi/common.h | 1 +
> > drivers/firmware/arm_scmi/driver.c | 2 +
> > drivers/firmware/arm_scmi/voltage.c | 382 ++++++++++++++++++++++++++++
> > include/linux/scmi_protocol.h | 64 +++++
> > 5 files changed, 450 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/firmware/arm_scmi/voltage.c
> >
> > diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> > index bc0d54f8e861..6a2ef63306d6 100644
> > --- a/drivers/firmware/arm_scmi/Makefile
> > +++ b/drivers/firmware/arm_scmi/Makefile
> > @@ -4,7 +4,7 @@ scmi-driver-y = driver.o notify.o
> > scmi-transport-y = shmem.o
> > scmi-transport-$(CONFIG_MAILBOX) += mailbox.o
> > scmi-transport-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smc.o
> > -scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o
> > +scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o
> > scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \
> > $(scmi-transport-y)
> > obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
> > diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> > index 65063fa948d4..c0fb45e7c3e8 100644
> > --- a/drivers/firmware/arm_scmi/common.h
> > +++ b/drivers/firmware/arm_scmi/common.h
> > @@ -169,6 +169,7 @@ DECLARE_SCMI_REGISTER_UNREGISTER(perf);
> > DECLARE_SCMI_REGISTER_UNREGISTER(power);
> > DECLARE_SCMI_REGISTER_UNREGISTER(reset);
> > DECLARE_SCMI_REGISTER_UNREGISTER(sensors);
> > +DECLARE_SCMI_REGISTER_UNREGISTER(voltage);
> > DECLARE_SCMI_REGISTER_UNREGISTER(system);
> >
> > #define DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(id, name) \
> > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> > index 3dfd8b6a0ebf..ada35e63feae 100644
> > --- a/drivers/firmware/arm_scmi/driver.c
> > +++ b/drivers/firmware/arm_scmi/driver.c
> > @@ -946,6 +946,7 @@ static int __init scmi_driver_init(void)
> > scmi_power_register();
> > scmi_reset_register();
> > scmi_sensors_register();
> > + scmi_voltage_register();
> > scmi_system_register();
> >
> > return platform_driver_register(&scmi_driver);
> > @@ -961,6 +962,7 @@ static void __exit scmi_driver_exit(void)
> > scmi_power_unregister();
> > scmi_reset_unregister();
> > scmi_sensors_unregister();
> > + scmi_voltage_unregister();
> > scmi_system_unregister();
> >
> > platform_driver_unregister(&scmi_driver);
> > diff --git a/drivers/firmware/arm_scmi/voltage.c b/drivers/firmware/arm_scmi/voltage.c
> > new file mode 100644
> > index 000000000000..a20da5128de1
> > --- /dev/null
> > +++ b/drivers/firmware/arm_scmi/voltage.c
> > @@ -0,0 +1,382 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * System Control and Management Interface (SCMI) Voltage Protocol
> > + *
> > + * Copyright (C) 2020 ARM Ltd.
> > + */
> > +
> > +#include <linux/scmi_protocol.h>
> > +
> > +#include "common.h"
> > +
> > +#define VOLTAGE_DOMS_NUM_MASK GENMASK(15, 0)
> > +#define REMAINING_LEVELS_MASK GENMASK(31, 16)
> > +#define RETURNED_LEVELS_MASK GENMASK(11, 0)
> > +
> > +enum scmi_voltage_protocol_cmd {
> > + VOLTAGE_DOMAIN_ATTRIBUTES = 0x3,
> > + VOLTAGE_DESCRIBE_LEVELS = 0x4,
> > + VOLTAGE_CONFIG_SET = 0x5,
> > + VOLTAGE_CONFIG_GET = 0x6,
> > + VOLTAGE_LEVEL_SET = 0x7,
> > + VOLTAGE_LEVEL_GET = 0x8,
> > +};
> > +
> > +struct scmi_msg_resp_protocol_attributes {
> > + __le32 attr;
> > +#define NUM_VOLTAGE_DOMAINS(x) (u16)(FIELD_GET(VOLTAGE_DOMS_NUM_MASK, (x)))
> > +};
> > +
> > +struct scmi_msg_resp_domain_attributes {
> > + __le32 attr;
> > + u8 name[SCMI_MAX_STR_SIZE];
> > +};
> > +
> > +struct scmi_msg_cmd_describe_levels {
> > + __le32 domain_id;
> > + __le32 level_index;
> > +};
> > +
> > +struct scmi_msg_resp_describe_levels {
> > + __le32 flags;
> > +#define NUM_REMAINING_LEVELS(f) (u16)(FIELD_GET(REMAINING_LEVELS_MASK, (f)))
> > +#define NUM_RETURNED_LEVELS(f) (u16)(FIELD_GET(RETURNED_LEVELS_MASK, (f)))
> > +#define SUPPORTS_SEGMENTED_LEVELS(f) ((f) & BIT(12))
> > + __le32 voltage[];
> > +};
> > +
> > +struct scmi_msg_cmd_config_set {
> > + __le32 domain_id;
> > + __le32 config;
> > +};
> > +
> > +struct scmi_msg_cmd_level_set {
> > + __le32 domain_id;
> > + __le32 flags;
> > + __le32 voltage_level;
> > +};
> > +
> > +struct voltage_info {
> > + u32 version;
> > + u16 num_domains;
> > + struct scmi_voltage_info **domains;
> > +};
> > +
> > +static int scmi_protocol_attributes_get(const struct scmi_handle *handle,
> > + struct voltage_info *vinfo)
> > +{
> > + int ret;
> > + struct scmi_xfer *t;
> > + struct scmi_msg_resp_protocol_attributes *resp;
> > +
> > + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
> > + SCMI_PROTOCOL_VOLTAGE, 0, sizeof(*resp), &t);
> > + if (ret)
> > + return ret;
> > +
> > + resp = t->rx.buf;
> > + ret = scmi_do_xfer(handle, t);
> > + if (!ret)
> > + vinfo->num_domains =
> > + NUM_VOLTAGE_DOMAINS(le32_to_cpu(resp->attr));
> > +
> > + scmi_xfer_put(handle, t);
> > + return ret;
> > +}
> > +
> > +static inline int scmi_init_voltage_levels(struct device *dev,
> > + struct scmi_voltage_info *v,
> > + u32 flags, u32 num_levels)
> > +{
> > + bool segmented;
> > +
> > + segmented = SUPPORTS_SEGMENTED_LEVELS(flags);
> > + /* segmented levels array's entries must be multiple-of-3 */
> > + if (!num_levels || (segmented && num_levels % 3))
> > + return -EINVAL;
>
> I think segment levels are described by a single triplet.
> If right, should test (!num_levels || (segmented && num_levels == 3)).
>
>
You're right, multiple triplets were initially described but dropped
from the final spec, so my bad I forgot to update this. (same goes with
the correspondent regulator support for multiple linear mappings as you
spotted there).
I'll fix in V3 like
if (!num_levels || (segmented && num_levels != 3))
return -EINVAL;
> > +
> > + v->levels_uv = devm_kcalloc(dev, num_levels, sizeof(u32), GFP_KERNEL);
> > + if (!v->levels_uv)
> > + return -ENOMEM;
> > +
> > + v->num_levels = num_levels;
> > + v->segmented = segmented;
> > +
> > + return 0;
> > +}
> > +
> > +static int scmi_voltage_descriptors_get(const struct scmi_handle *handle,
> > + struct voltage_info *vinfo)
> > +{
> > + int ret, dom;
> > + struct scmi_xfer *td, *tl;
> > + struct device *dev = handle->dev;
> > + struct scmi_msg_resp_domain_attributes *resp_dom;
> > + struct scmi_msg_resp_describe_levels *resp_levels;
> > +
> > + ret = scmi_xfer_get_init(handle, VOLTAGE_DOMAIN_ATTRIBUTES,
> > + SCMI_PROTOCOL_VOLTAGE, sizeof(__le32),
> > + sizeof(*resp_dom), &td);
> > + if (ret)
> > + return ret;
> > + resp_dom = td->rx.buf;
> > +
> > + ret = scmi_xfer_get_init(handle, VOLTAGE_DESCRIBE_LEVELS,
> > + SCMI_PROTOCOL_VOLTAGE, sizeof(__le64), 0, &tl);
> > + if (ret)
> > + goto outd;
> > + resp_levels = tl->rx.buf;
> > +
> > + for (dom = 0; dom < vinfo->num_domains; dom++) {
> > + u32 desc_index = 0;
> > + u16 num_returned = 0, num_remaining = 0;
> > + struct scmi_msg_cmd_describe_levels *cmd;
> > + struct scmi_voltage_info *v;
> > +
> > + /* Retrieve domain attributes at first ... */
> > + put_unaligned_le32(dom, td->tx.buf);
> > + ret = scmi_do_xfer(handle, td);
> > + if (ret)
> > + continue;
>
> Continue and break and report the error code?
> Seems you prefer to abort only on local error (ENOMEM), not comm error.
> Maybe state with nice inline comment as done below (/* Skip domain on error */).
>
Ok I'll do.
> > +
> > + v = devm_kzalloc(dev, sizeof(*v), GFP_KERNEL);
> > + if (!v) {
> > + ret = -ENOMEM;
> > + break;
> > + }
> > +
> > + v->id = dom;
> > + v->attributes = le32_to_cpu(resp_dom->attr);
> > + strlcpy(v->name, resp_dom->name, SCMI_MAX_STR_SIZE);
> > +
> > + cmd = tl->tx.buf;
> > + /* ...then retrieve domain levels descriptions */
> > + do {
> > + u32 flags;
> > + int cnt;
> > +
> > + cmd->domain_id = cpu_to_le32(v->id);
> > + cmd->level_index = desc_index;
> > + ret = scmi_do_xfer(handle, tl);
> > + if (ret)
> > + break;
> > +
> > + flags = le32_to_cpu(resp_levels->flags);
> > + num_returned = NUM_RETURNED_LEVELS(flags);
> > + num_remaining = NUM_REMAINING_LEVELS(flags);
> > +
> > + /* Allocate space for num_levels if not already done */
> > + if (!v->num_levels) {
> > + ret = scmi_init_voltage_levels(dev, v, flags,
> > + num_returned +
> > + num_remaining);
> > + if (ret)
> > + break;
> > + }
> > +
> > + if (desc_index + num_returned > v->num_levels) {
> > + dev_err(handle->dev,
> > + "No. of voltage levels can't exceed %d",
>
> Missing '\n'.
>
Right.
> > + v->num_levels);
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + for (cnt = 0; cnt < num_returned; cnt++) {
> > + s32 val;
> > +
> > + val =
> > + (s32)le32_to_cpu(resp_levels->voltage[cnt]);
> > + v->levels_uv[desc_index + cnt] = val;
> > + if (!v->negative_volts_allowed && val < 0)
>
> could skip testing v->negative_volts_allowed.
> if (val < 0)
> v->negative_volts_allowed = true;
>
Ok.
>
> > + v->negative_volts_allowed = true;
> > + }
> > +
> > + desc_index += num_returned;
> > +
> > + scmi_reset_rx_to_maxsz(handle, tl);
> > + /* check both to avoid infinite loop due to buggy fw */
> > + } while (num_returned && num_remaining);
> > +
> > + /*
> > + * Bail out on memory errors, just skip domain on any
> > + * other error.
> > + */
> > + if (!ret)
> > + vinfo->domains[dom] = v;
> > + else if (ret == -ENOMEM)
> > + break;
> > +
> > + scmi_reset_rx_to_maxsz(handle, td);
> > + }
> > +
> > + scmi_xfer_put(handle, tl);
> > +outd:
> > + scmi_xfer_put(handle, td);
> > +
> > + return ret;
> > +}
> > +
> > +static inline int __scmi_voltage_get_u32(const struct scmi_handle *handle,
>
> inline needed ?
>
Mmm, probably not.
Thanks
Cristian
> > + u8 cmd_id, u32 domain_id, u32 *value)
> > +{
> > + int ret;
> > + struct scmi_xfer *t;
> > + struct voltage_info *vinfo = handle->voltage_priv;
> > +
> > + if (domain_id >= vinfo->num_domains)
> > + return -EINVAL;
> > +
> > + ret = scmi_xfer_get_init(handle, cmd_id,
> > + SCMI_PROTOCOL_VOLTAGE,
> > + sizeof(__le32), 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + put_unaligned_le32(domain_id, t->tx.buf);
> > + ret = scmi_do_xfer(handle, t);
> > + if (!ret)
> > + *value = get_unaligned_le32(t->rx.buf);
> > +
> > + scmi_xfer_put(handle, t);
> > + return ret;
> > +}
> > +
> > +static int scmi_voltage_config_set(const struct scmi_handle *handle,
> > + u32 domain_id, u32 config)
> > +{
> > + int ret;
> > + struct scmi_xfer *t;
> > + struct voltage_info *vinfo = handle->voltage_priv;
> > + struct scmi_msg_cmd_config_set *cmd;
> > +
> > + if (domain_id >= vinfo->num_domains)
> > + return -EINVAL;
> > +
> > + ret = scmi_xfer_get_init(handle, VOLTAGE_CONFIG_SET,
> > + SCMI_PROTOCOL_VOLTAGE,
> > + sizeof(*cmd), 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + cmd = t->tx.buf;
> > + cmd->domain_id = cpu_to_le32(domain_id);
> > + cmd->config = cpu_to_le32(config & GENMASK(3, 0));
> > +
> > + ret = scmi_do_xfer(handle, t);
> > +
> > + scmi_xfer_put(handle, t);
> > + return ret;
> > +}
> > +
> > +static int scmi_voltage_config_get(const struct scmi_handle *handle,
> > + u32 domain_id, u32 *config)
> > +{
> > + return __scmi_voltage_get_u32(handle, VOLTAGE_CONFIG_GET,
> > + domain_id, config);
> > +}
> > +
> > +static int scmi_voltage_level_set(const struct scmi_handle *handle,
> > + u32 domain_id, u32 flags, s32 volt_uV)
> > +{
> > + int ret;
> > + struct scmi_xfer *t;
> > + struct voltage_info *vinfo = handle->voltage_priv;
> > + struct scmi_msg_cmd_level_set *cmd;
> > +
> > + if (domain_id >= vinfo->num_domains)
> > + return -EINVAL;
> > +
> > + ret = scmi_xfer_get_init(handle, VOLTAGE_LEVEL_SET,
> > + SCMI_PROTOCOL_VOLTAGE,
> > + sizeof(*cmd), 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + cmd = t->tx.buf;
> > + cmd->domain_id = cpu_to_le32(domain_id);
> > + cmd->flags = cpu_to_le32(flags);
> > + cmd->voltage_level = cpu_to_le32(volt_uV);
> > +
> > + ret = scmi_do_xfer(handle, t);
> > +
> > + scmi_xfer_put(handle, t);
> > + return ret;
> > +}
> > +
> > +static int scmi_voltage_level_get(const struct scmi_handle *handle,
> > + u32 domain_id, s32 *volt_uV)
> > +{
> > + return __scmi_voltage_get_u32(handle, VOLTAGE_LEVEL_GET,
> > + domain_id, (u32 *)volt_uV);
> > +}
> > +
> > +static const struct scmi_voltage_info *
> > +scmi_voltage_info_get(const struct scmi_handle *handle, u32 domain_id)
> > +{
> > + struct voltage_info *vinfo = handle->voltage_priv;
> > +
> > + if (domain_id >= vinfo->num_domains)
> > + return NULL;
> > +
> > + return vinfo->domains[domain_id];
> > +}
> > +
> > +static int scmi_voltage_domains_num_get(const struct scmi_handle *handle)
> > +{
> > + struct voltage_info *vinfo = handle->voltage_priv;
> > +
> > + return vinfo->num_domains;
> > +}
> > +
> > +static struct scmi_voltage_ops voltage_ops = {
> > + .num_domains_get = scmi_voltage_domains_num_get,
> > + .info_get = scmi_voltage_info_get,
> > + .config_set = scmi_voltage_config_set,
> > + .config_get = scmi_voltage_config_get,
> > + .level_set = scmi_voltage_level_set,
> > + .level_get = scmi_voltage_level_get,
> > +};
> > +
> > +static int scmi_voltage_protocol_init(struct scmi_handle *handle)
> > +{
> > + int ret;
> > + u32 version;
> > + struct voltage_info *vinfo;
> > +
> > + ret = scmi_version_get(handle, SCMI_PROTOCOL_VOLTAGE, &version);
> > + if (ret)
> > + return ret;
> > +
> > + dev_dbg(handle->dev, "Voltage Version %d.%d\n",
> > + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
> > +
> > + vinfo = devm_kzalloc(handle->dev, sizeof(*vinfo), GFP_KERNEL);
> > + if (!vinfo)
> > + return -ENOMEM;
> > + vinfo->version = version;
> > +
> > + ret = scmi_protocol_attributes_get(handle, vinfo);
> > + if (ret)
> > + return ret;
> > +
> > + if (vinfo->num_domains) {
> > + vinfo->domains = devm_kcalloc(handle->dev, vinfo->num_domains,
> > + sizeof(vinfo->domains),
> > + GFP_KERNEL);
> > + if (!vinfo->domains)
> > + return -ENOMEM;
> > + ret = scmi_voltage_descriptors_get(handle, vinfo);
> > + if (ret)
> > + return ret;
> > + } else {
> > + dev_warn(handle->dev, "No Voltage domains found.\n");
> > + }
> > +
> > + handle->voltage_ops = &voltage_ops;
> > + handle->voltage_priv = vinfo;
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_VOLTAGE, voltage)
> > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
> > index 9cd312a1ff92..032ad9bb2a53 100644
> > --- a/include/linux/scmi_protocol.h
> > +++ b/include/linux/scmi_protocol.h
> > @@ -209,6 +209,64 @@ struct scmi_reset_ops {
> > int (*deassert)(const struct scmi_handle *handle, u32 domain);
> > };
> >
> > +/**
> > + * struct scmi_voltage_info - describe one available SCMI Voltage Domain
> > + *
> > + * @id: the domain ID as advertised by the platform
> > + * @segmented: defines the layout of the entries of array @levels_uv.
> > + * - when True the entries are to be interpreted as triplets,
> > + * each defining a segment representing a range of equally
> > + * space voltages: <lowest_volts>, <highest_volt>, <step_uV>
> > + * - when False the entries simply represent a single discrete
> > + * supported voltage level
> > + * @negative_volts_allowed: True if any of the entries of @levels_uv represent
> > + * a negative voltage.
> > + * @attributes: represents Voltage Domain advertised attributes
> > + * @name: name assigned to the Voltage Domain by platform
> > + * @num_levels: number of total entries in @levels_uv.
> > + * @levels_uv: array of entries describing the available voltage levels for
> > + * this domain.
> > + */
> > +struct scmi_voltage_info {
> > + u32 id;
> > + bool segmented;
> > +#define SCMI_VOLTAGE_SEGMENT_LOW 0
> > +#define SCMI_VOLTAGE_SEGMENT_HIGH 1
> > +#define SCMI_VOLTAGE_SEGMENT_STEP 2
> > + bool negative_volts_allowed;
> > + u32 attributes;
> > + char name[SCMI_MAX_STR_SIZE];
> > + u16 num_levels;
> > + s32 *levels_uv;
> > +};
> > +
> > +/**
> > + * struct scmi_voltage_ops - represents the various operations provided
> > + * by SCMI Voltage Protocol
> > + *
> > + * @num_domains_get: get the count of voltage domains provided by SCMI
> > + * @info_get: get the information of the specified domain
> > + * @config_set: set the config for the specified domain
> > + * @config_get: get the config of the specified domain
> > + * @level_set: set the voltage level for the specified domain
> > + * @level_get: get the voltage level of the specified domain
> > + */
> > +struct scmi_voltage_ops {
> > + int (*num_domains_get)(const struct scmi_handle *handle);
> > + const struct scmi_voltage_info *(*info_get)
> > + (const struct scmi_handle *handle, u32 domain_id);
> > + int (*config_set)(const struct scmi_handle *handle, u32 domain_id,
> > + u32 config);
> > +#define SCMI_VOLTAGE_ARCH_STATE_OFF 0x0
> > +#define SCMI_VOLTAGE_ARCH_STATE_ON 0x7
> > + int (*config_get)(const struct scmi_handle *handle, u32 domain_id,
> > + u32 *config);
> > + int (*level_set)(const struct scmi_handle *handle, u32 domain_id,
> > + u32 flags, s32 volt_uV);
> > + int (*level_get)(const struct scmi_handle *handle, u32 domain_id,
> > + s32 *volt_uV);
> > +};
> > +
> > /**
> > * struct scmi_notify_ops - represents notifications' operations provided by
> > * SCMI core
> > @@ -262,6 +320,7 @@ struct scmi_notify_ops {
> > * @clk_ops: pointer to set of clock protocol operations
> > * @sensor_ops: pointer to set of sensor protocol operations
> > * @reset_ops: pointer to set of reset protocol operations
> > + * @voltage_ops: pointer to set of voltage protocol operations
> > * @notify_ops: pointer to set of notifications related operations
> > * @perf_priv: pointer to private data structure specific to performance
> > * protocol(for internal use only)
> > @@ -273,6 +332,8 @@ struct scmi_notify_ops {
> > * protocol(for internal use only)
> > * @reset_priv: pointer to private data structure specific to reset
> > * protocol(for internal use only)
> > + * @voltage_priv: pointer to private data structure specific to voltage
> > + * protocol(for internal use only)
> > * @notify_priv: pointer to private data structure specific to notifications
> > * (for internal use only)
> > */
> > @@ -284,6 +345,7 @@ struct scmi_handle {
> > const struct scmi_power_ops *power_ops;
> > const struct scmi_sensor_ops *sensor_ops;
> > const struct scmi_reset_ops *reset_ops;
> > + const struct scmi_voltage_ops *voltage_ops;
> > const struct scmi_notify_ops *notify_ops;
> > /* for protocol internal use */
> > void *perf_priv;
> > @@ -291,6 +353,7 @@ struct scmi_handle {
> > void *power_priv;
> > void *sensor_priv;
> > void *reset_priv;
> > + void *voltage_priv;
> > void *notify_priv;
> > void *system_priv;
> > };
> > @@ -303,6 +366,7 @@ enum scmi_std_protocol {
> > SCMI_PROTOCOL_CLOCK = 0x14,
> > SCMI_PROTOCOL_SENSOR = 0x15,
> > SCMI_PROTOCOL_RESET = 0x16,
> > + SCMI_PROTOCOL_VOLTAGE = 0x17,
> > };
> >
> > enum scmi_system_events {
> > --
> > 2.17.1
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/4] regulator: add SCMI driver
2020-10-16 14:09 ` Etienne Carriere
@ 2020-10-16 16:55 ` Cristian Marussi
0 siblings, 0 replies; 9+ messages in thread
From: Cristian Marussi @ 2020-10-16 16:55 UTC (permalink / raw)
To: Etienne Carriere
Cc: Rob Herring, satyakim, f.fainelli, Vincent Guittot, Sudeep Holla,
linux-kernel, broonie,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Jim Quinlan, Jonathan.Cameron, Souvik Chakravarty, lukasz.luba
On Fri, Oct 16, 2020 at 04:09:42PM +0200, Etienne Carriere wrote:
> Hi Cristian,
>
> Some minor comments...
>
> Etienne
>
>
> On Thu, 15 Oct 2020 at 17:40, Cristian Marussi <cristian.marussi@arm.com> wrote:
> >
> > Add a simple regulator based on SCMI Voltage Domain Protocol.
> >
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ----
> > v1 --> v2
> > - removed duplicate regulator naming
> > - removed redundant .get/set_voltage ops: only _sel variants implemented
> > - removed condexpr on fail path to increase readability
> >
> > v0 --> v1
> > - fixed init_data constraint parsing
> > - fixes for v5.8 (linear_range.h)
> > - fixed commit message content and subject line format
> > - factored out SCMI core specific changes to distinct patch
> > - reworked Kconfig and Makefile to keep proper alphabetic order
> > - fixed SPDX comment style
> > - removed unneeded inline functions
> > - reworked conditionals for legibility
> > - fixed some return paths to properly report SCMI original errors codes
> > - added some more descriptive error messages when fw returns invalid ranges
> > - removed unneeded explicit devm_regulator_unregister from .remove()
> > ---
> > drivers/regulator/Kconfig | 9 +
> > drivers/regulator/Makefile | 1 +
> > drivers/regulator/scmi-regulator.c | 453 +++++++++++++++++++++++++++++
> > 3 files changed, 463 insertions(+)
> > create mode 100644 drivers/regulator/scmi-regulator.c
> >
> > diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> > index de17ef7e18f0..6d3a10cb9833 100644
> > --- a/drivers/regulator/Kconfig
> > +++ b/drivers/regulator/Kconfig
> > @@ -155,6 +155,15 @@ config REGULATOR_ARIZONA_MICSUPP
> > and Wolfson Microelectronic Arizona codecs
> > devices.
> >
> > +config REGULATOR_ARM_SCMI
> > + tristate "SCMI based regulator driver"
> > + depends on ARM_SCMI_PROTOCOL && OF
> > + help
> > + This adds the regulator driver support for ARM platforms using SCMI
> > + protocol for device voltage management.
> > + This driver uses SCMI Message Protocol driver to interact with the
> > + firmware providing the device Voltage functionality.
> > +
> > config REGULATOR_AS3711
> > tristate "AS3711 PMIC"
> > depends on MFD_AS3711
> > diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> > index d8d3ecf526a8..0532a7393d5d 100644
> > --- a/drivers/regulator/Makefile
> > +++ b/drivers/regulator/Makefile
> > @@ -24,6 +24,7 @@ obj-$(CONFIG_REGULATOR_AD5398) += ad5398.o
> > obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o
> > obj-$(CONFIG_REGULATOR_ARIZONA_LDO1) += arizona-ldo1.o
> > obj-$(CONFIG_REGULATOR_ARIZONA_MICSUPP) += arizona-micsupp.o
> > +obj-$(CONFIG_REGULATOR_ARM_SCMI) += scmi-regulator.o
> > obj-$(CONFIG_REGULATOR_AS3711) += as3711-regulator.o
> > obj-$(CONFIG_REGULATOR_AS3722) += as3722-regulator.o
> > obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o
> > diff --git a/drivers/regulator/scmi-regulator.c b/drivers/regulator/scmi-regulator.c
> > new file mode 100644
> > index 000000000000..e4e7d0345723
> > --- /dev/null
> > +++ b/drivers/regulator/scmi-regulator.c
> > @@ -0,0 +1,453 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// System Control and Management Interface (SCMI) based regulator driver
> > +//
> > +// Copyright (C) 2020 ARM Ltd.
> > +//
> > +// Implements a regulator driver on top of the SCMI Voltage Protocol.
> > +//
> > +// The ARM SCMI Protocol aims in general to hide as much as possible all the
> > +// underlying operational details while providing an abstracted interface for
> > +// its users to operate upon: as a consequence the resulting operational
> > +// capabilities and configurability of this regulator device are much more
> > +// limited than the ones usually available on a standard physical regulator.
> > +//
> > +// The supported SCMI regulator ops are restricted to the bare minimum:
> > +//
> > +// - 'status_ops': enable/disable/is_enabled
> > +// - 'voltage_ops': get_voltage_sel/set_voltage_sel
> > +// list_voltage/map_voltage
> > +//
> > +// Each SCMI regulator instance is associated, through the means of a proper DT
> > +// entry description, to a specific SCMI Voltage Domain.
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/linear_range.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/regulator/driver.h>
> > +#include <linux/regulator/machine.h>
> > +#include <linux/regulator/of_regulator.h>
> > +#include <linux/scmi_protocol.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +
> > +struct scmi_regulator {
> > + u32 id;
> > + struct scmi_device *sdev;
> > + struct regulator_dev *rdev;
> > + struct device_node *of_node;
> > + struct regulator_desc desc;
> > + struct regulator_config conf;
> > +};
> > +
> > +struct scmi_regulator_info {
> > + int num_doms;
> > + struct scmi_regulator **sregv;
> > +};
> > +
> > +static int scmi_reg_enable(struct regulator_dev *rdev)
> > +{
> > + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> > + const struct scmi_handle *handle = sreg->sdev->handle;
> > +
> > + return handle->voltage_ops->config_set(handle, sreg->id,
> > + SCMI_VOLTAGE_ARCH_STATE_ON);
> > +}
> > +
> > +static int scmi_reg_disable(struct regulator_dev *rdev)
> > +{
> > + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> > + const struct scmi_handle *handle = sreg->sdev->handle;
> > +
> > + return handle->voltage_ops->config_set(handle, sreg->id,
> > + SCMI_VOLTAGE_ARCH_STATE_OFF);
> > +}
> > +
> > +static int scmi_reg_is_enabled(struct regulator_dev *rdev)
> > +{
> > + int ret;
> > + u32 config;
> > + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> > + const struct scmi_handle *handle = sreg->sdev->handle;
> > +
> > + ret = handle->voltage_ops->config_get(handle, sreg->id,
> > + &config);
> > + if (ret) {
> > + dev_err(&sreg->sdev->dev,
> > + "Error %d reading regulator %s status.\n",
> > + ret, sreg->desc.name);
> > + return 0;
> > + }
> > +
> > + return config & SCMI_VOLTAGE_ARCH_STATE_ON;
> > +}
> > +
> > +static int scmi_reg_get_voltage_sel(struct regulator_dev *rdev)
> > +{
> > + int ret;
> > + s32 volt_uV;
> > + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> > + const struct scmi_handle *handle = sreg->sdev->handle;
> > +
> > + ret = handle->voltage_ops->level_get(handle, sreg->id, &volt_uV);
> > + if (ret)
> > + return ret;
> > +
> > + return sreg->desc.ops->map_voltage(rdev, volt_uV, volt_uV);
> > +}
> > +
> > +static int scmi_reg_set_voltage_sel(struct regulator_dev *rdev,
> > + unsigned int selector)
> > +{
> > + int ret;
> > + s32 volt_uV;
> > + struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
> > + const struct scmi_handle *handle = sreg->sdev->handle;
> > +
> > + volt_uV = sreg->desc.ops->list_voltage(rdev, selector);
> > + if (volt_uV <= 0)
> > + return -EINVAL;
> > +
> > + ret = handle->voltage_ops->level_set(handle, sreg->id, 0x0, volt_uV);
> > + if (ret)
> > + return ret;
> > +
> > + return ret;
>
> return handle->voltage_ops->level_set(handle, sreg->id, 0x0, volt_uV);
>
> > +}
> > +
> > +static const struct regulator_ops scmi_reg_fixed_ops = {
> > + .enable = scmi_reg_enable,
> > + .disable = scmi_reg_disable,
> > + .is_enabled = scmi_reg_is_enabled,
> > +};
> > +
> > +static const struct regulator_ops scmi_reg_linear_ops = {
> > + .enable = scmi_reg_enable,
> > + .disable = scmi_reg_disable,
> > + .is_enabled = scmi_reg_is_enabled,
> > + .get_voltage_sel = scmi_reg_get_voltage_sel,
> > + .set_voltage_sel = scmi_reg_set_voltage_sel,
> > + .list_voltage = regulator_list_voltage_linear,
> > + .map_voltage = regulator_map_voltage_linear,
> > +};
> > +
> > +static const struct regulator_ops scmi_reg_range_ops = {
> > + .enable = scmi_reg_enable,
> > + .disable = scmi_reg_disable,
> > + .is_enabled = scmi_reg_is_enabled,
> > + .get_voltage_sel = scmi_reg_get_voltage_sel,
> > + .set_voltage_sel = scmi_reg_set_voltage_sel,
> > + .list_voltage = regulator_list_voltage_linear_range,
> > + .map_voltage = regulator_map_voltage_linear_range,
> > +};
> > +
> > +static const struct regulator_ops scmi_reg_discrete_ops = {
> > + .enable = scmi_reg_enable,
> > + .disable = scmi_reg_disable,
> > + .is_enabled = scmi_reg_is_enabled,
> > + .get_voltage_sel = scmi_reg_get_voltage_sel,
> > + .set_voltage_sel = scmi_reg_set_voltage_sel,
> > + .list_voltage = regulator_list_voltage_table,
> > + .map_voltage = regulator_map_voltage_iterate,
> > +};
> > +
> > +static int
> > +scmi_config_linear_regulator_mappings(struct scmi_regulator *sreg,
> > + const struct scmi_voltage_info *vinfo)
> > +{
> > + /*
> > + * Note that SCMI voltage domains describable by linear ranges
> > + * (segments) {low, high, step} are guaranteed to come in triplets by
> > + * the SCMI Voltage Domain protocol support itself.
> > + */
> > + if (vinfo->num_levels == 3) {
> > + s32 delta_uV;
> > +
> > + delta_uV = (vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH] -
> > + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW]);
> > + /* Rule out buggy negative-intervals answers from fw */
> > + if (delta_uV < 0) {
> > + dev_err(&sreg->sdev->dev,
> > + "Invalid volt-range %d-%duV for domain %d\n",
> > + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW],
> > + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH],
> > + sreg->id);
> > + return -EINVAL;
> > + }
> > +
> > + if (!delta_uV) {
> > + /* Just one fixed voltage exposed by SCMI */
> > + sreg->desc.fixed_uV =
> > + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW];
> > + sreg->desc.n_voltages = 1;
> > + sreg->desc.ops = &scmi_reg_fixed_ops;
> > + } else {
> > + /* One simple linear mapping. */
> > + sreg->desc.min_uV =
> > + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW];
> > + sreg->desc.uV_step =
> > + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_STEP];
> > + sreg->desc.linear_min_sel = 0;
> > + sreg->desc.n_voltages = delta_uV / sreg->desc.uV_step;
> > + sreg->desc.ops = &scmi_reg_linear_ops;
> > + }
> > + } else {
> > + /* Multiple linear mappings. */
>
> I don't see this multi-linear mapping in the v3.0 beta of the spec.
> Is it something planned?
>
No, it was indeed something included in previous spec iterations (multiple triplets)
but now removed, as said in previous patch, I forgot to remove this
redundant support. I'll do in V3.
> > + int i, num_ranges, last_max = -1;
> > + struct linear_range *lr;
> > +
> > + num_ranges = vinfo->num_levels / 3;
> > + lr = devm_kcalloc(&sreg->sdev->dev, num_ranges,
> > + sizeof(*lr), GFP_KERNEL);
> > + if (!lr)
> > + return -ENOMEM;
> > +
> > + sreg->desc.n_linear_ranges = num_ranges;
> > + sreg->desc.linear_ranges = lr;
> > + for (i = 0; num_ranges; num_ranges--, i += 3, lr++) {
> > + s32 delta_uV;
> > +
> > + lr->min =
> > + vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_LOW];
> > + lr->step =
> > + vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_STEP];
> > + delta_uV =
> > + vinfo->levels_uv[i + SCMI_VOLTAGE_SEGMENT_HIGH] -
> > + lr->min;
> > + if (delta_uV <= 0 || !(delta_uV / lr->step)) {
> > + dev_err(&sreg->sdev->dev,
> > + "Invalid volt-range %d-%duV for domain %d\n",
> > + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_LOW],
> > + vinfo->levels_uv[SCMI_VOLTAGE_SEGMENT_HIGH],
> > + sreg->id);
> > + return -EINVAL;
> > + }
> > + lr->max_sel = delta_uV / lr->step - 1;
> > + lr->min_sel = last_max + 1;
> > + last_max = lr->max_sel;
> > + }
> > + sreg->desc.n_voltages = last_max + 1;
> > + sreg->desc.ops = &scmi_reg_range_ops;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +scmi_config_discrete_regulator_mappings(struct scmi_regulator *sreg,
> > + const struct scmi_voltage_info *vinfo)
> > +{
> > + /* Discrete non linear levels are mapped to volt_table */
> > + sreg->desc.n_voltages = vinfo->num_levels;
> > + if (sreg->desc.n_voltages > 1) {
> > + sreg->desc.volt_table = (const unsigned int *)vinfo->levels_uv;
> > + sreg->desc.ops = &scmi_reg_discrete_ops;
> > + } else {
> > + sreg->desc.fixed_uV = vinfo->levels_uv[0];
> > + sreg->desc.ops = &scmi_reg_fixed_ops;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int scmi_regulator_common_init(struct scmi_regulator *sreg)
> > +{
> > + int ret;
> > + const struct scmi_handle *handle = sreg->sdev->handle;
> > + struct device *dev = &sreg->sdev->dev;
> > + const struct scmi_voltage_info *vinfo;
> > +
> > + vinfo = handle->voltage_ops->info_get(handle, sreg->id);
> > + if (!vinfo)
> > + return -ENODEV;
> > +
> > + if (!vinfo->num_levels)
> > + return -EINVAL;
> > +
> > + /*
> > + * Regulator framework does not fully support negative voltages
> > + * so we discard any voltage domain reported as supporting negative
> > + * voltages: as a consequence each levels_uv entry is guaranteed to
> > + * be non-negative from here on.
> > + */
> > + if (vinfo->negative_volts_allowed) {
> > + dev_warn(dev, "Negative voltages NOT supported...skip %s\n",
> > + sreg->of_node->full_name);
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + sreg->desc.name = devm_kasprintf(dev, GFP_KERNEL, "%s", vinfo->name);
> > + if (!sreg->desc.name)
> > + return -ENOMEM;
> > +
> > + sreg->desc.id = sreg->id;
> > + sreg->desc.type = REGULATOR_VOLTAGE;
> > + sreg->desc.owner = THIS_MODULE;
> > + sreg->desc.of_match = sreg->of_node->name;
> > + sreg->desc.regulators_node = "regulators";
> > + if (vinfo->segmented)
> > + ret = scmi_config_linear_regulator_mappings(sreg, vinfo);
> > + else
> > + ret = scmi_config_discrete_regulator_mappings(sreg, vinfo);
> > + if (ret)
> > + return ret;
> > +
> > + sreg->conf.dev = dev;
> > + sreg->conf.driver_data = sreg;
> > +
> > + return 0;
> > +}
> > +
> > +static int process_scmi_regulator_of_node(struct scmi_device *sdev,
> > + struct device_node *np,
> > + struct scmi_regulator_info *rinfo)
> > +{
> > + u32 dom, ret;
> > +
> > + ret = of_property_read_u32(np, "reg", &dom);
> > + if (ret)
> > + return ret;
> > +
> > + if (dom >= rinfo->num_doms)
> > + return -ENODEV;
> > +
> > + if (rinfo->sregv[dom]) {
> > + dev_err(&sdev->dev,
> > + "SCMI Voltage Domain %d already in use. Skipping: %s\n",
> > + dom, np->full_name);
> > + return -EINVAL;
> > + }
> > +
> > + rinfo->sregv[dom] = devm_kzalloc(&sdev->dev,
> > + sizeof(struct scmi_regulator),
> > + GFP_KERNEL);
> > + if (!rinfo->sregv[dom])
> > + return -ENOMEM;
> > +
> > + rinfo->sregv[dom]->id = dom;
> > + rinfo->sregv[dom]->sdev = sdev;
> > + /* get hold of good nodes */
> > + of_node_get(np);
> > + rinfo->sregv[dom]->of_node = np;
> > + dev_info(&sdev->dev,
> > + "Found valid SCMI Regulator -- OF node [%d] -> %s\n",
> > + dom, np->full_name);
> > +
> > + return ret;
>
> suggest return 0; here.
Ok.
>
> > +}
> > +
> > +static int scmi_regulator_probe(struct scmi_device *sdev)
> > +{
> > + int d, ret, num_doms;
> > + struct device_node *np, *child;
> > + const struct scmi_handle *handle = sdev->handle;
> > + struct scmi_regulator_info *rinfo;
> > +
> > + if (!handle || !handle->voltage_ops)
> > + return -ENODEV;
> > +
> > + num_doms = handle->voltage_ops->num_domains_get(handle);
> > + if (num_doms <= 0) {
> > + if (!num_doms) {
> > + dev_err(&sdev->dev,
> > + "number of voltage domains invalid\n");
> > + num_doms = -EINVAL;
> > + } else {
> > + dev_err(&sdev->dev,
> > + "failed to get voltage domains - err:%d\n",
> > + num_doms);
> > + }
> > +
> > + return num_doms;
> > + }
> > +
> > + rinfo = devm_kzalloc(&sdev->dev, sizeof(*rinfo), GFP_KERNEL);
> > + if (!rinfo)
> > + return -ENOMEM;
> > +
> > + /* Allocate pointers' array for all possible domains */
> > + rinfo->sregv = devm_kcalloc(&sdev->dev, num_doms,
> > + sizeof(rinfo->sregv), GFP_KERNEL);
> > + if (!rinfo->sregv)
> > + return -ENOMEM;
> > +
> > + rinfo->num_doms = num_doms;
> > + /*
> > + * Start collecting into rinfo->sregv possibly good SCMI Regulators as
> > + * described by a well-formed DT entry and associated with an existing
> > + * plausible SCMI Voltage Domain number, all belonging to this SCMI
> > + * platform instance node (handle->dev->of_node).
> > + */
> > + np = of_find_node_by_name(handle->dev->of_node, "regulators");
> > + for_each_child_of_node(np, child) {
> > + ret = process_scmi_regulator_of_node(sdev, child, rinfo);
> > + /* abort on any mem issue */
> > + if (ret == -ENOMEM)
> > + return ret;
> > + }
> > +
> > + /*
> > + * Register a regulator for each valid regulator-DT-entry that we
> > + * can successfully reach via SCMI
> > + */
> > + for (d = 0; d < num_doms; d++) {
> > + struct scmi_regulator *sreg = rinfo->sregv[d];
> > +
> > + if (!sreg)
> > + continue;
> > + ret = scmi_regulator_common_init(sreg);
> > + if (ret)
> > + continue;
> > + sreg->rdev = devm_regulator_register(&sdev->dev, &sreg->desc,
> > + &sreg->conf);
> > + if (IS_ERR(sreg->rdev)) {
> > + sreg->rdev = NULL;
> > + continue;
> > + }
> > +
> > + dev_info(&sdev->dev,
> > + "Regulator %s registered for domain [%d](%s)\n",
> > + sreg->desc.name, sreg->id, sreg->desc.name);
>
> Maybe not display sreg->desc.name twice.
>
Yes does not make any sense indeed now that I have only one name o_O
> > + }
> > +
> > + dev_set_drvdata(&sdev->dev, rinfo);
> > +
> > + return 0;
> > +}
> > +
> > +static void scmi_regulator_remove(struct scmi_device *sdev)
> > +{
> > + int d;
> > + struct scmi_regulator_info *rinfo;
> > +
> > + rinfo = dev_get_drvdata(&sdev->dev);
> > + if (!rinfo)
> > + return;
> > +
> > + for (d = 0; d < rinfo->num_doms; d++) {
> > + if (!rinfo->sregv[d])
> > + continue;
> > + of_node_put(rinfo->sregv[d]->of_node);
> > + }
> > +}
> > +
> > +static const struct scmi_device_id scmi_regulator_id_table[] = {
> > + { SCMI_PROTOCOL_VOLTAGE, "regulator" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(scmi, scmi_regulator_id_table);
> > +
> > +static struct scmi_driver scmi_drv = {
> > + .name = "scmi-regulator",
> > + .probe = scmi_regulator_probe,
> > + .remove = scmi_regulator_remove,
> > + .id_table = scmi_regulator_id_table,
> > +};
> > +
> > +module_scmi_driver(scmi_drv);
> > +
> > +MODULE_AUTHOR("Cristian Marussi <cristian.marussi@arm.com>");
> > +MODULE_DESCRIPTION("ARM SCMI regulator driver");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.17.1
> >
Thanks
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-16 16:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15 15:39 [PATCH v2 0/4] Add support for SCMIv3.0 Voltage Domain Protocol and SCMI-Regulator Cristian Marussi
2020-10-15 15:39 ` [PATCH v2 1/4] firmware: arm_scmi: Add Voltage Domain Support Cristian Marussi
2020-10-16 13:36 ` Etienne Carriere
2020-10-16 16:51 ` Cristian Marussi
2020-10-15 15:39 ` [PATCH v2 2/4] firmware: arm_scmi: add SCMI Voltage Domain devname Cristian Marussi
2020-10-15 15:40 ` [PATCH v2 3/4] regulator: add SCMI driver Cristian Marussi
2020-10-16 14:09 ` Etienne Carriere
2020-10-16 16:55 ` Cristian Marussi
2020-10-15 15:40 ` [PATCH v2 4/4] dt-bindings: arm: add support for SCMI Regulators Cristian Marussi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).