[RFC 0/6] firmware: scmi: add SCMI pinctrl protocol support
This is an RFC and meant to get feedback from other developers as - the specification (pinctrl part) is still in a draft - the upstream patch for linux, including dt bindings, is still WIP - I'm not confident the drivers are generic enough to cover most HWs - The tests ("ut") doesn't cover all the features yet
This patch series allows users to access SCMI pin control protocol provided by SCMI server (platform). See SCMI specification document v3.2 beta 2[1] for more details about SCMI pin control protocol.
The implementation consists of two layers: - basic helper functions for SCMI pin control protocol in drivers/firmware/scmi/pinctrl.c (patch#2) - DM-compliant pinctrl/gpio drivers, which utilizes the helper functions, in drivers/pinctrl/pinctrl-scmi.c (patch#3,#4)
[1] https://developer.arm.com/documentation/den0056/e/?lang=en
DT bindings =========== Upstream pinctrl patch for linux defines the bindings in [2] though it doesn't say much. I expect that my implementation basically complies with U-Boot's generic bindings described in [3], but not all the features are verified.
As for gpio, unless you hard-code pin assignments directly in a device driver, my implementation allows the following alternatives in DT. Either way, we may need an additional binding description for gpio.
(A) scmi { ... // other protocols scmi_pinctrl: protocol@19 { // Pin control protocol ... {pinmux definitions}... // if any, including GPIO? } } scmi_gpio: scmi_gpio { compatible = "arm,scmi-gpio-generic"; gpio-controller; #gpio-cells = <2>; gpio-ranges = <&scmi_pinctrl 0 5 4>, <&scmi_pinctrl 4 0 0>; gpio-ranges-group-names = "", "ANOTHER_GPIO_GROUP"; } some_device { ... reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>; } (B) scmi { ... // other protocols scmi_pinctrl: protocol@19 { // Pin control protocol ... {pinmux definitions}... // if any, including GPIO?
scmi_gpio: scmi_gpio { // no need for "compatible" gpio-controller; #gpio-cells = <2>; gpio-ranges = <&scmi_pinctrl 0 5 4>, <&scmi_pinctrl 4 0 0>; gpio-ranges-group-names = "", "ANOTHER_GPIO_GROUP"; } } } some_device { ... reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>; } (C) if "gpio-ranges" is missing in gpio definition, assume 1:1 mapping, i.e. use a native pinctrl pin number (5). some_device { ... reset-gpios = <&scmi_gpio 5 GPIO_ACTIVE_HIGH>; }
[2] https://lkml.iu.edu/hypermail/linux/kernel/2308.1/01084.html [3] <u-boot>/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt <u-boot>/doc/device-tree-bindings/gpio/gpio.txt
Test ==== The patch series was tested on the following platforms: * sandbox ("ut dm pinmux" and manually using gpio command)
Prerequisite: ============= * This patch series is based on my WIP "Base protocol support" patches on v2023.10-rc3. You can fetch the whole code from [4].
[4] https://git.linaro.org/people/takahiro.akashi/u-boot.git branch:scmi/pinctrl
Patches: ======== Patch#1: Add SCMI base protocol driver Patch#2-#4: Add drivers Patch#5-#6: Test related
Change history: =============== RFC (Sep 6, 2023) * initial release as RFC
AKASHI Takahiro (6): firmware: scmi: fix protocol enumeration logic firmware: scmi: add pinctrl protocol support pinctrl: add scmi driver gpio: add scmi driver based on pinctrl firmware: scmi: add pseudo pinctrl protocol support on sandbox test: dm: add SCMI pinctrl test
arch/sandbox/dts/test.dts | 115 +++ cmd/scmi.c | 1 + drivers/firmware/scmi/Kconfig | 3 + drivers/firmware/scmi/Makefile | 1 + drivers/firmware/scmi/pinctrl.c | 412 ++++++++ drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +++++++++++++ drivers/firmware/scmi/scmi_agent-uclass.c | 18 +- drivers/pinctrl/Kconfig | 11 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-scmi.c | 1071 ++++++++++++++++++++ include/scmi_agent-uclass.h | 2 + include/scmi_protocols.h | 435 ++++++++ test/dm/scmi.c | 62 ++ 13 files changed, 2852 insertions(+), 2 deletions(-) create mode 100644 drivers/firmware/scmi/pinctrl.c create mode 100644 drivers/pinctrl/pinctrl-scmi.c
The original logic in enumerating all the protocols accidentally modifies a *loop* variable, node, at Voltage domain protocol. So subsequent protocol nodes in a device tree won't be detected.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- drivers/firmware/scmi/scmi_agent-uclass.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c index 46a2933d51a4..79584c00a066 100644 --- a/drivers/firmware/scmi/scmi_agent-uclass.c +++ b/drivers/firmware/scmi/scmi_agent-uclass.c @@ -422,8 +422,11 @@ static int scmi_bind_protocols(struct udevice *dev) case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN: if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI) && scmi_protocol_is_supported(dev, protocol_id)) { - node = ofnode_find_subnode(node, "regulators"); - if (!ofnode_valid(node)) { + ofnode sub_node; + + sub_node = ofnode_find_subnode(node, + "regulators"); + if (!ofnode_valid(sub_node)) { dev_err(dev, "no regulators node\n"); return -ENXIO; }
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
The original logic in enumerating all the protocols accidentally modifies a *loop* variable, node, at Voltage domain protocol. So subsequent protocol nodes in a device tree won't be detected.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/firmware/scmi/scmi_agent-uclass.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c index 46a2933d51a4..79584c00a066 100644 --- a/drivers/firmware/scmi/scmi_agent-uclass.c +++ b/drivers/firmware/scmi/scmi_agent-uclass.c @@ -422,8 +422,11 @@ static int scmi_bind_protocols(struct udevice *dev) case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN: if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI) && scmi_protocol_is_supported(dev, protocol_id)) {
node = ofnode_find_subnode(node, "regulators");if (!ofnode_valid(node)) {
ofnode sub_node;sub_node = ofnode_find_subnode(node,"regulators");if (!ofnode_valid(sub_node)) { dev_err(dev, "no regulators node\n"); return -ENXIO; }-- 2.34.1
This function is very ugly...could we instead rely on driver model to bind the devices? It seems unfortunate to have to write code for something which could be done with no extra code.
Regards, Simon
Hi Simon,
On Sun, Sep 10, 2023 at 01:13:26PM -0600, Simon Glass wrote:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
The original logic in enumerating all the protocols accidentally modifies a *loop* variable, node, at Voltage domain protocol. So subsequent protocol nodes in a device tree won't be detected.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/firmware/scmi/scmi_agent-uclass.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c index 46a2933d51a4..79584c00a066 100644 --- a/drivers/firmware/scmi/scmi_agent-uclass.c +++ b/drivers/firmware/scmi/scmi_agent-uclass.c @@ -422,8 +422,11 @@ static int scmi_bind_protocols(struct udevice *dev) case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN: if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI) && scmi_protocol_is_supported(dev, protocol_id)) {
node = ofnode_find_subnode(node, "regulators");if (!ofnode_valid(node)) {
ofnode sub_node;sub_node = ofnode_find_subnode(node,"regulators");if (!ofnode_valid(sub_node)) { dev_err(dev, "no regulators node\n"); return -ENXIO; }-- 2.34.1
This function is very ugly...could we instead rely on driver model to
I think you are talking about scmi_bind_protocols() itself, not the hunk above in my patch, which is merely a bug fix in the existing code. If so,
bind the devices? It seems unfortunate to have to write code for something which could be done with no extra code.
We will discuss in the threads on your comment [1].
[1] https://lists.denx.de/pipermail/u-boot/2023-September/530200.html
-Takahiro Akashi
Regards, Simon
With this patch, transport-level utility functions for SCMI pinctrl protocol are added. DM-compliant device drivers, pinctrl and gpio, are added in the following patches.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/scmi.c | 1 + drivers/firmware/scmi/Kconfig | 3 + drivers/firmware/scmi/Makefile | 1 + drivers/firmware/scmi/pinctrl.c | 412 ++++++++++++++++++++ drivers/firmware/scmi/scmi_agent-uclass.c | 11 + include/scmi_agent-uclass.h | 2 + include/scmi_protocols.h | 435 ++++++++++++++++++++++ 7 files changed, 865 insertions(+) create mode 100644 drivers/firmware/scmi/pinctrl.c
diff --git a/cmd/scmi.c b/cmd/scmi.c index 5efec8ad87fd..ae7892381e0a 100644 --- a/cmd/scmi.c +++ b/cmd/scmi.c @@ -33,6 +33,7 @@ struct { {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"}, {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"}, {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"}, + {SCMI_PROTOCOL_ID_PIN_CONTROL, "Pin control"}, };
/** diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig index 8cf85f0d7a12..e0cb7b70dede 100644 --- a/drivers/firmware/scmi/Kconfig +++ b/drivers/firmware/scmi/Kconfig @@ -41,3 +41,6 @@ config SCMI_AGENT_OPTEE help Enable the SCMI communication channel based on OP-TEE transport for compatible "linaro,scmi-optee". + +config SCMI_PINCTRL + bool diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile index 1a23d4981709..bf6381332e9e 100644 --- a/drivers/firmware/scmi/Makefile +++ b/drivers/firmware/scmi/Makefile @@ -4,4 +4,5 @@ obj-y += smt.o obj-$(CONFIG_SCMI_AGENT_SMCCC) += smccc_agent.o obj-$(CONFIG_SCMI_AGENT_MAILBOX) += mailbox_agent.o obj-$(CONFIG_SCMI_AGENT_OPTEE) += optee_agent.o +obj-$(CONFIG_SCMI_PINCTRL) += pinctrl.o obj-$(CONFIG_SANDBOX) += sandbox-scmi_agent.o sandbox-scmi_devices.o diff --git a/drivers/firmware/scmi/pinctrl.c b/drivers/firmware/scmi/pinctrl.c new file mode 100644 index 000000000000..88ec57dc3d53 --- /dev/null +++ b/drivers/firmware/scmi/pinctrl.c @@ -0,0 +1,412 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * SCMI Pin control protocol as U-Boot device + * + * Copyright (C) 2023 Linaro Limited + * author: AKASHI Takahiro takahiro.akashi@linaro.org + */ + +#include <common.h> +#include <dm.h> +#include <malloc.h> +#include <scmi_agent.h> +#include <scmi_protocols.h> +#include <string.h> +#include <asm/types.h> +#include <dm/device_compat.h> +#include <linux/kernel.h> + +/** + * struct scmi_pinctrl_priv - Private data for SCMI Pin control protocol + * @channel: Reference to the SCMI channel to use + */ +struct scmi_pinctrl_priv { + struct scmi_channel *channel; +}; + +int scmi_pinctrl_protocol_attrs(struct udevice *dev, u32 *num_pins, + u32 *num_groups, u32 *num_functions) +{ + struct scmi_pinctrl_protocol_attrs_out out; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PROTOCOL_ATTRIBUTES, + .out_msg = (u8 *)&out, + .out_msg_sz = sizeof(out), + }; + int ret; + + if (!dev || !num_pins || !num_groups || !num_functions) + return -EINVAL; + + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (out.status) + return scmi_to_linux_errno(out.status); + + *num_groups = SCMI_PINCTRL_ATTRS_NUM_GROUPS(out.attributes_low); + *num_pins = SCMI_PINCTRL_ATTRS_NUM_PINS(out.attributes_low); + *num_functions = SCMI_PINCTRL_ATTRS_NUM_FUNCTIONS(out.attributes_high); + + return 0; +} + +int scmi_pinctrl_attrs(struct udevice *dev, u32 id, u32 flags, + bool *extended_name, u8 **name) +{ + struct scmi_pinctrl_attrs_in in; + struct scmi_pinctrl_attrs_out out; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_ATTRIBUTES, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&out, + .out_msg_sz = sizeof(out), + }; + int ret; + + if (!dev || !extended_name || !name) + return -EINVAL; + + in.id = id; + in.flags = flags; + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (out.status) + return scmi_to_linux_errno(out.status); + + *extended_name = SCMI_PINCTRL_ATTRS_EXTENDED_NAME(out.attributes); + *name = strdup(out.name); + if (!*name) + return -ENOMEM; + + return 0; +} + +int scmi_pinctrl_list_assocs(struct udevice *dev, u32 id, u32 flags, + u16 **array) +{ + struct scmi_pinctrl_list_assocs_in in; + struct scmi_pinctrl_list_assocs_out out; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_LIST_ASSOCIATIONS, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&out, + .out_msg_sz = sizeof(out), + }; + u32 index; + u16 *buf; + int i, ret; + + if (!dev || !array) + return -EINVAL; + + in.id = id; + in.flags = flags; + + buf = calloc(sizeof(u16), SCMI_PINCTRL_ARRAY_SIZE); + if (!buf) + return -ENOMEM; + + index = 0; + while (1) { + in.index = index; + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + goto err; + if (out.status) { + ret = scmi_to_linux_errno(out.status); + goto err; + } + + for (i = 0; i < SCMI_PINCTRL_LIST_NUM_PINGROUP(out.flags); + i++, index++) + buf[index] = out.array[i]; + + /* realloc should happen only once */ + if (SCMI_PINCTRL_LIST_NUM_REMAINING(out.flags)) { + buf = realloc(buf, sizeof(u16) * + (SCMI_PINCTRL_ARRAY_SIZE + + SCMI_PINCTRL_LIST_NUM_REMAINING(out.flags))); + if (!buf) + return -ENOMEM; + } else { + break; + } + } + + *array = buf; + + return index; +err: + free(buf); + + return ret; +} + +int scmi_pinctrl_config_get(struct udevice *dev, u32 id, bool all_configs, + int selector, unsigned int config_type, + struct scmi_pin_entry **configs) + +{ + struct scmi_pinctrl_config_get_in in; + struct scmi_pinctrl_config_get_out out; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_CONFIG_GET, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&out, + .out_msg_sz = sizeof(out), + }; + struct scmi_pin_entry *entry; + int cur, num, remaining, ret; + + if (!dev) + return -EINVAL; + + if (!all_configs) { + in.id = id; + in.attributes = SCMI_PINCTRL_CONFIG_GET_ATTRS(0, selector, 0, + config_type); + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (out.status) + return scmi_to_linux_errno(out.status); + + entry = malloc(sizeof(*entry)); + if (!entry) + return -ENOMEM; + + *entry = out.configs[0]; + *configs = entry; + + return 0; + } + + entry = NULL; + cur = 0; + while (1) { + in.id = id; + in.attributes = SCMI_PINCTRL_CONFIG_GET_ATTRS(1, selector, cur, + 0); + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (out.status) + return scmi_to_linux_errno(out.status); + + num = SCMI_PINCTRL_CONFIG_GET_NUM_RET(out.num_configs); + remaining = SCMI_PINCTRL_CONFIG_GET_REMAINING(out.num_configs); + + if (!entry) + entry = malloc(sizeof(*entry) * (num + remaining)); + if (!entry) + return -ENOMEM; + + memcpy(entry + cur, &out.configs, sizeof(*entry) * num); + + if (!remaining) + break; + + cur += num; + } + + return 0; +} + +int scmi_pinctrl_config_set(struct udevice *dev, u32 id, int selector, + int num_configs, struct scmi_pin_entry *configs) +{ + s32 status; + struct scmi_pinctrl_config_set_in in; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_CONFIG_SET, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&status, + .out_msg_sz = sizeof(status), + }; + int cur, num, ret; + + if (!dev) + return -EINVAL; + + cur = 0; + while (num_configs) { + if (num_configs < SCMI_PINCTRL_CONFIG_ENTRY_MAX) + num = num_configs; + else + num = SCMI_PINCTRL_CONFIG_ENTRY_MAX; + + in.id = id; + in.attributes = SCMI_PINCTRL_CONFIG_SET_ATTRS(num, selector); + memcpy(&in.configs[cur], configs, + sizeof(struct scmi_pin_entry) * num); + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (status) + return scmi_to_linux_errno(status); + + num_configs -= num; + cur += num; + } + + return 0; +} + +int scmi_pinctrl_function_select(struct udevice *dev, u32 id, u32 function_id, + u32 flags) +{ + s32 status; + struct scmi_pinctrl_function_select_in in; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_FUNCTION_SELECT, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&status, + .out_msg_sz = sizeof(status), + }; + int ret; + + if (!dev) + return -EINVAL; + + in.id = id; + in.function_id = function_id; + in.flags = flags; + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (status) + return scmi_to_linux_errno(status); + + return 0; +} + +int scmi_pinctrl_request(struct udevice *dev, u32 id, u32 flags) +{ + s32 status; + struct scmi_pinctrl_request_in in; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_REQUEST, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&status, + .out_msg_sz = sizeof(status), + }; + int ret; + + if (!dev) + return -EINVAL; + + in.id = id; + in.flags = flags; + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (status) + return scmi_to_linux_errno(status); + + return 0; +} + +int scmi_pinctrl_release(struct udevice *dev, u32 id, u32 flags) +{ + s32 status; + struct scmi_pinctrl_release_in in; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_RELEASE, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&status, + .out_msg_sz = sizeof(status), + }; + int ret; + + if (!dev) + return -EINVAL; + + in.id = id; + in.flags = flags; + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (status) + return scmi_to_linux_errno(status); + + return 0; +} + +int scmi_pinctrl_name_get(struct udevice *dev, u32 id, u32 flags, u8 **name) +{ + struct scmi_pinctrl_name_get_in in; + struct scmi_pinctrl_name_get_out out; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_NAME_GET, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&out, + .out_msg_sz = sizeof(out), + }; + int ret; + + if (!dev || !name) + return -EINVAL; + + in.id = id; + in.flags = flags; + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (out.status) + return scmi_to_linux_errno(out.status); + + *name = strdup(out.name); + if (!*name) + return -ENOMEM; + + return 0; +} + +int scmi_pinctrl_set_permissions(struct udevice *dev, u32 agent_id, u32 id, + u32 flags) +{ + s32 status; + struct scmi_pinctrl_set_permissions_in in; + struct scmi_msg msg = { + .protocol_id = SCMI_PROTOCOL_ID_PIN_CONTROL, + .message_id = SCMI_PINCTRL_SET_PERMISSIONS, + .in_msg = (u8 *)&in, + .in_msg_sz = sizeof(in), + .out_msg = (u8 *)&status, + .out_msg_sz = sizeof(status), + }; + int ret; + + if (!dev) + return -EINVAL; + + in.agent_id = agent_id; + in.id = id; + in.flags = flags; + ret = devm_scmi_process_msg(dev, &msg); + if (ret) + return ret; + if (status) + return scmi_to_linux_errno(status); + + return 0; +} diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c index 79584c00a066..a5953cabbcee 100644 --- a/drivers/firmware/scmi/scmi_agent-uclass.c +++ b/drivers/firmware/scmi/scmi_agent-uclass.c @@ -95,6 +95,9 @@ struct udevice *scmi_get_protocol(struct udevice *dev, case SCMI_PROTOCOL_ID_CLOCK: proto = priv->clock_dev; break; + case SCMI_PROTOCOL_ID_PIN_CONTROL: + proto = priv->pinctrl_dev; + break; case SCMI_PROTOCOL_ID_RESET_DOMAIN: proto = priv->resetdom_dev; break; @@ -142,6 +145,9 @@ static int scmi_add_protocol(struct udevice *dev, case SCMI_PROTOCOL_ID_CLOCK: priv->clock_dev = proto; break; + case SCMI_PROTOCOL_ID_PIN_CONTROL: + priv->pinctrl_dev = proto; + break; case SCMI_PROTOCOL_ID_RESET_DOMAIN: priv->resetdom_dev = proto; break; @@ -414,6 +420,11 @@ static int scmi_bind_protocols(struct udevice *dev) scmi_protocol_is_supported(dev, protocol_id)) drv = DM_DRIVER_GET(scmi_clock); break; + case SCMI_PROTOCOL_ID_PIN_CONTROL: + if (IS_ENABLED(CONFIG_PINCTRL_SCMI) && + scmi_protocol_is_supported(dev, protocol_id)) + drv = DM_DRIVER_GET(scmi_pinctrl); + break; case SCMI_PROTOCOL_ID_RESET_DOMAIN: if (IS_ENABLED(CONFIG_RESET_SCMI) && scmi_protocol_is_supported(dev, protocol_id)) diff --git a/include/scmi_agent-uclass.h b/include/scmi_agent-uclass.h index 6fd6d54d6a5a..993ea81e09c8 100644 --- a/include/scmi_agent-uclass.h +++ b/include/scmi_agent-uclass.h @@ -25,6 +25,7 @@ struct scmi_channel; * @base_dev: SCMI base protocol device * @clock_dev: SCMI clock protocol device * @resetdom_dev: SCMI reset domain protocol device + * @pinctrl_dev: SCMI pin control protocol device * @voltagedom_dev: SCMI voltage domain protocol device */ struct scmi_agent_priv { @@ -40,6 +41,7 @@ struct scmi_agent_priv { struct udevice *base_dev; struct udevice *clock_dev; struct udevice *resetdom_dev; + struct udevice *pinctrl_dev; struct udevice *voltagedom_dev; };
diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h index f41d3f317ce2..699e27e0a84f 100644 --- a/include/scmi_protocols.h +++ b/include/scmi_protocols.h @@ -24,6 +24,7 @@ enum scmi_std_protocol { SCMI_PROTOCOL_ID_SENSOR = 0x15, SCMI_PROTOCOL_ID_RESET_DOMAIN = 0x16, SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN = 0x17, + SCMI_PROTOCOL_ID_PIN_CONTROL = 0x19, };
enum scmi_status_code { @@ -683,4 +684,438 @@ struct scmi_voltd_level_get_out { s32 voltage_level; };
+/* + * SCMI Pin Control Protocol + */ +#define SCMI_PIN_CONTROL_PROTOCOL_VERSION 0x10000 + +enum scmi_pin_control_id { + SCMI_PINCTRL_ATTRIBUTES = 0x3, + SCMI_PINCTRL_LIST_ASSOCIATIONS = 0x4, + SCMI_PINCTRL_CONFIG_GET = 0x5, + SCMI_PINCTRL_CONFIG_SET = 0x6, + SCMI_PINCTRL_FUNCTION_SELECT = 0x7, + SCMI_PINCTRL_REQUEST = 0x8, + SCMI_PINCTRL_RELEASE = 0x9, + SCMI_PINCTRL_NAME_GET = 0xa, + SCMI_PINCTRL_SET_PERMISSIONS = 0xb, +}; + +#define SCMI_PINCTRL_NAME_LENGTH_MAX 16 + +/** + * struct scmi_pinctrl_protocol_attrs_out - Response for PINCTRL's + * PROTOCOL_ATTRIBUTES command + * @status: SCMI command status + * @attributes_low: Protocol attributes (lower bytes) + * @attributes_high: Protocol attributes (higher bytes) + */ +struct scmi_pinctrl_protocol_attrs_out { + s32 status; + u32 attributes_low; + u32 attributes_high; +}; + +#define SCMI_PINCTRL_ATTRS_NUM_GROUPS(attributes_low) \ + (((attributes_low) & GENMASK(31, 16)) >> 16) +#define SCMI_PINCTRL_ATTRS_NUM_PINS(attributes_low) \ + ((attributes_low) & GENMASK(15, 0)) +#define SCMI_PINCTRL_ATTRS_NUM_FUNCTIONS(attributes_high) \ + ((attributes_high) & GENMASK(15, 0)) + +/** + * struct scmi_pinctrl_attrs_in - Message payload for + * SCMI_PINCTRL_ATTRIBUTES command + * @id: Identifier of pin, group or function + * @flags: A set of flags + */ +struct scmi_pinctrl_attrs_in { + u32 id; + u32 flags; +}; + +/** + * struct scmi_pinctrl_attrs_out - Response payload for + * SCMI_PINCTRL_ATTRIBUTES command + * @status: SCMI command status + * @attributes: Attributes of pin, group for function + * @name: Name of pin, group or function + */ +struct scmi_pinctrl_attrs_out { + u32 status; + u32 attributes; + u8 name[SCMI_PINCTRL_NAME_LENGTH_MAX]; +}; + +#define SCMI_PINCTRL_TYPE(flags) ((flags) & GENMASK(1, 0)) +#define SCMI_PINCTRL_TYPE_PIN 0x0 +#define SCMI_PINCTRL_TYPE_GROUP 0x1 +#define SCMI_PINCTRL_TYPE_FUNCTION 0x2 + +#define SCMI_PINCTRL_ATTRS_EXTENDED_NAME(attributes) \ + ((attributes) & BIT(31)) +#define SCMI_PINCTRL_ATTRS_NUM_PINGROUP(attributes) \ + ((attributes) & GENMASK(15, 0)) + +/** + * struct scmi_pinctrl_list_assocs_in - Message payload for + * SCMI_PINCTRL_LIST_ASSOCIATIONS command + * @id: Identifier of group or function + * @flags: A set of flags + * @index: Index of first group to be described + */ +struct scmi_pinctrl_list_assocs_in { + u32 id; + u32 flags; + u32 index; +}; + +#define SCMI_PINCTRL_ARRAY_SIZE 8 + +/** + * struct scmi_pinctrl_list_assocs_out - Response payload for + * SCMI_PINCTRL_LIST_ASSOCIATIONS command + * @status: SCMI command status + * @attributes: A set of flags + * @array: Array of pin or group identifiers + */ +struct scmi_pinctrl_list_assocs_out { + u32 status; + u32 flags; + u16 array[SCMI_PINCTRL_ARRAY_SIZE]; +}; + +#define SCMI_PINCTRL_LIST_NUM_REMAINING(flags) \ + (((flags) & GENMASK(31, 16)) >> 16) +#define SCMI_PINCTRL_LIST_NUM_PINGROUP(flags) \ + ((flags) & GENMASK(11, 0)) + +/* FIXME: depends on transport */ +#define SCMI_PINCTRL_CONFIG_ENTRY_MAX 8 + +struct scmi_pin_entry { + u32 type; + u32 value; +}; + +/** + * struct scmi_pinctrl_config_get_in - Message payload for + * SCMI_PINCTRL_CONFIG_GET command + * @id: Identifier of pin or group + * @attributes: Configuration attributes + */ +struct scmi_pinctrl_config_get_in { + u32 id; + u32 attributes; +}; + +#define SCMI_PINCTRL_CONFIG_GET_ATTRS(all, selector, skip, type) \ + (((all) << 18) | ((selector) << 16) | \ + (((skip) << 8) | (type))) + +#define SCMI_PINCTRL_CONFIG_GET_ALL BIT(18) +#define SCMI_PINCTRL_CONFIG_GET_PINCTRL_TYPE(attributes) \ + (((attributes) & GENMASK(17, 16)) >> 16) +#define SCMI_PINCTRL_CONFIG_GET_SKIP(attributes) \ + (((attributes) & GENMASK(15, 8)) >> 8) +#define SCMI_PINCTRL_CONFIG_GET_TYPE(attributes) \ + ((attributes) & GENMASK(7, 0)) + +/** + * struct scmi_pinctrl_confige_get_out - Response payload for + * SCMI_PINCTRL_CONFIG_GET command + * @status: SCMI command status + * @num_configs: Number of configuration types + * @configs: Values of configuration types + */ +struct scmi_pinctrl_config_get_out { + u32 status; + u32 num_configs; + struct scmi_pin_entry configs[SCMI_PINCTRL_CONFIG_ENTRY_MAX]; +}; + +#define SCMI_PINCTRL_CONFIG_GET_REMAINING(attributes) \ + (((attributes) & GENMASK(31, 24)) >> 24) +#define SCMI_PINCTRL_CONFIG_GET_NUM_RET(attributes) \ + ((attributes) & GENMASK(7, 0)) +#define SCMI_PINCTRL_CONFIG_GET_NUM_CONFIGS(remaining, num) \ + (((remaining) << 24) | (num)) + +#define SCMI_PINCTRL_CONFIG_ID_SHIFT 8 + +#define SCMI_PINCTRL_CONFIG_DEFAULT 0 +#define SCMI_PINCTRL_CONFIG_BIAS_BUS_HOLD 1 +#define SCMI_PINCTRL_CONFIG_BIAS_DISABLE 2 +#define SCMI_PINCTRL_CONFIG_BIAS_HI_IMPEDANCE 3 +#define SCMI_PINCTRL_CONFIG_BIAS_PULL_UP 4 +#define SCMI_PINCTRL_CONFIG_BIAS_PULL_DEF 5 +#define SCMI_PINCTRL_CONFIG_BIAS_PULL_DOWN 6 +#define SCMI_PINCTRL_CONFIG_DRIVE_OPEN_DRAIN 7 +#define SCMI_PINCTRL_CONFIG_DRIVE_OPEN_SOURCE 8 +#define SCMI_PINCTRL_CONFIG_DRIVE_PUSH_PULL 9 +#define SCMI_PINCTRL_CONFIG_DRIVE_STRENGTH 10 +#define SCMI_PINCTRL_CONFIG_INPUT_DEBOUNCE 11 +#define SCMI_PINCTRL_CONFIG_INPUT_MODE 12 +#define SCMI_PINCTRL_CONFIG_PULL_MODE 13 +#define SCMI_PINCTRL_CONFIG_INPUT_VALUE 14 +#define SCMI_PINCTRL_CONFIG_INPUT_SCHMITT 15 +#define SCMI_PINCTRL_CONFIG_LOW_POWER_MODE 16 +#define SCMI_PINCTRL_CONFIG_OUTPUT_MODE 17 +#define SCMI_PINCTRL_CONFIG_OUTPUT_VALUE 18 +#define SCMI_PINCTRL_CONFIG_POWER_SOURCE 19 +#define SCMI_PINCTRL_CONFIG_SLEW_RATE 20 +#define SCMI_PINCTRL_CONFIG_RESERVED 21 +#define SCMI_PINCTRL_CONFIG_OEM 192 + +/** + * struct scmi_pinctrl_config_set_in - Message payload for + * SCMI_PINCTRL_CONFIG_GET command + * @id: Identifier of pin or group + * @attributes: Configuration attributes + * @configs: Values of configuration types + */ +struct scmi_pinctrl_config_set_in { + u32 id; + u32 attributes; + struct scmi_pin_entry configs[SCMI_PINCTRL_CONFIG_ENTRY_MAX]; +}; + +#define SCMI_PINCTRL_CONFIG_SET_ATTRS(num, type) \ + (((num) << 2) | (type)) +#define SCMI_PINCTRL_CONFIG_SET_NUM_CONFIGS(attributes) \ + (((attributes) & GENMASK(9, 2)) >> 2) +#define SCMI_PINCTRL_CONFIG_SET_PINCTRL_TYPE(attributes) \ + ((attributes) & GENMASK(1, 0)) + +/** + * struct scmi_pinctrl_function_select_in - Message payload for + * SCMI_PINCTRL_FUNCTION_SELECT command + * @id: Identifier of pin or group + * @function_id: Identifier of function + * @flags: A set of flags + */ +struct scmi_pinctrl_function_select_in { + u32 id; + u32 function_id; + u32 flags; +}; + +/** + * struct scmi_pinctrl_request_in - Message payload for + * SCMI_PINCTRL_REQUEST command + * @id: Identifier of pin or group + * @flags: A set of flags + */ +struct scmi_pinctrl_request_in { + u32 id; + u32 flags; +}; + +/** + * struct scmi_pinctrl_release_in - Message payload for + * SCMI_PINCTRL_RELEASE command + * @id: Identifier of pin or group + * @flags: A set of flags + */ +struct scmi_pinctrl_release_in { + u32 id; + u32 flags; +}; + +/** + * struct scmi_pinctrl_name_get_in - Message payload for + * SCMI_PINCTRL_NAME_GET command + * @id: Identifier of pin, group or function + * @flags: A set of flags + */ +struct scmi_pinctrl_name_get_in { + u32 id; + u32 flags; +}; + +/** + * struct scmi_pinctrl_name_get_out - Response payload for + * SCMI_PINCTRL_NAME_GET command + * @status: SCMI command status + * @flags: A set of flags (reserved) + * @name: Name in string + */ +struct scmi_pinctrl_name_get_out { + u32 status; + u32 flags; + u8 name[64]; +}; + +/** + * struct scmi_pinctrl_set_permissions_in - Message payload for + * SCMI_PINCTRL_SET_PERMISSIONS command + * @agent_id: Identifier of the agent + * @id: Identifier of pin or group + * @flags: A set of flags + */ +struct scmi_pinctrl_set_permissions_in { + u32 agent_id; + u32 id; + u32 flags; +}; + +#define SCMI_PINCTRL_PERMISSION BIT(2) + +/** + * scmi_pinctrl_protocol_attrs - get attributes of pinctrl protocol + * @dev: SCMI pinctrl device + * @num_pins: A number of pins + * @num_groups: A number of groups + * @num_functions: A number of functions + * + * Get a number of available pins, groups and functions, respectively, + * in @num_pins, @num_groups, @num_functions. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_protocol_attrs(struct udevice *dev, u32 *num_pins, + u32 *num_groups, u32 *num_functions); + +/** + * scmi_pinctrl_attrs - get attributes of pin, group or function + * @dev: SCMI pinctrl device + * @id: Identifier of pin, group or function + * @flags: A set of flags + * @extended_name: Bit flag for extended name + * @name: Name of pin, group or function + * + * Get the name of @id in @name. + * @flags can be SCMI_PINCTRL_TYPE_PIN, GROUP or FUNCTION. + * If @id has an extended name, @extended_name is set to true. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_attrs(struct udevice *dev, u32 id, u32 flags, + bool *extended_name, u8 **name); + +/** + * scmi_pinctrl_list_assocs - Get a list of pins or groups + * @dev: SCMI pinctrl device + * @id: Identifier of group or function + * @flags: A set of flags + * @array: A pointer to a list of pins, groups or functions + * + * Get a list of pins or groups associated with @id in @array. + * @flags can be SCMI_PINCTRL_TYPE_GROUP or FUNCTION. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_list_assocs(struct udevice *dev, u32 id, u32 flags, + u16 **array); + +/** + * scmi_pinctrl_config_get - Get configuration + * @dev: SCMI pinctrl device + * @id: Identifier of pin or group + * @all_configs: Identifier of pin or group + * @selector: Type of id + * @config_type: Type of configuration + * @configs: A pointer to an array of configuration data + * + * If @all_configs is set, get a list of values of all configuration types + * associated with @id in @configs, otherwise get the configuration for + * a specific @config_type. Each configuration data is presented in a form + * of struct scmi_pin_entry. + * @selector can be SCMI_PINCTRL_TYPE_PIN or GROUP. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_config_get(struct udevice *dev, u32 id, bool all_configs, + int selector, unsigned int config_type, + struct scmi_pin_entry **configs); + +/** + * scmi_pinctrl_config_set - Set configuration + * @dev: SCMI pinctrl device + * @id: Identifier of pin or group + * @selector: Type of id + * @num_configs: A number of configuration data + * @configs: A pointer to an array of configuration data + * + * Configure a pin or group associated with @id using @configs. + * Each configuration data is presented in a form of struct scmi_pin_entry. + * @selector can be SCMI_PINCTRL_TYPE_PIN or GROUP. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_config_set(struct udevice *dev, u32 id, int selector, + int num_configs, struct scmi_pin_entry *configs); + +/** + * scmi_pinctrl_function_select - Select a function + * @dev: SCMI pinctrl device + * @id: Identifier of pin or group + * @function_id: Identifier of function + * @flags: A set of flags + * + * Select a function, @function_id, for a pin or group with @id. + * @flags can be SCMI_PINCTRL_TYPE_PIN or GROUP. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_function_select(struct udevice *dev, u32 id, u32 function_id, + u32 flags); + +/** + * scmi_pinctrl_request - Request exclusive control + * @dev: SCMI pinctrl device + * @id: Identifier of pin or group + * @flags: A set of flags + * + * Make a request for exclusive control of pin or group with @id by this agent. + * Once granted, no other agent can access @id until released. + * @flags can be SCMI_PINCTRL_TYPE_PIN or GROUP. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_request(struct udevice *dev, u32 id, u32 flags); + +/** + * scmi_pinctrl_release - Release exclusive control + * @dev: SCMI pinctrl device + * @id: Identifier of pin or group + * @flags: A set of flags + * + * Release exclusive control of pin or group with @id by this agent. + * @flags can be SCMI_PINCTRL_TYPE_PIN or GROUP. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_release(struct udevice *dev, u32 id, u32 flags); + +/** + * scmi_pinctrl_name_get - Get extended name + * @dev: SCMI pinctrl device + * @id: Identifier of pin, group or function + * @flags: A set of flags + * + * Get an extended name of pin, group or function with @id. + * @flags can be SCMI_PINCTRL_TYPE_PIN, GROUP or FUNCTION. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_name_get(struct udevice *dev, u32 id, u32 flags, u8 **name); + +/** + * scmi_pinctrl_set_permissions - Claim access permission + * @dev: SCMI pinctrl device + * @agent_id: Identifier of SCMI agent + * @id: Identifier of pin or group + * @flags: A set of flags + * + * Request to set or deny access permission against @id for the agent + * with @agent_id. + * Bit[2] of @flags indicates "allow access" if set, otherwise "deny access". + * @flags should have SCMI_PINCTRL_TYPE_PIN or GROUP. + * + * Return: 0 on success, error code otherwise + */ +int scmi_pinctrl_set_permissions(struct udevice *dev, u32 agent_id, u32 id, + u32 flags); + #endif /* _SCMI_PROTOCOLS_H */
This DM-compliant driver deals with SCMI pinctrl protocol and presents pinctrl devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- drivers/pinctrl/Kconfig | 11 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-scmi.c | 537 +++++++++++++++++++++++++++++++++ 3 files changed, 549 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-scmi.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 75b3ff47a2e8..d02f5db550c8 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -256,6 +256,17 @@ config PINCTRL_SANDBOX Currently, this driver actually does nothing but print debug messages when pinctrl operations are invoked.
+config PINCTRL_SCMI + bool "SCMI pinctrl driver" + depends on SCMI_FIRMWARE + select SCMI_PINCTRL + help + This enables pinctrl driver base on SCMI. + + The driver is controlled by a device tree node which contains + both the GPIO definitions and pin control functions for each + available multiplex function. + config PINCTRL_SINGLE bool "Single register pin-control and pin-multiplex driver" depends on DM diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index fc1f01a02cbd..a791df022b7d 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_PINCTRL_MSCC) += mscc/ obj-$(CONFIG_ARCH_MVEBU) += mvebu/ obj-$(CONFIG_ARCH_NEXELL) += nexell/ obj-$(CONFIG_PINCTRL_QE) += pinctrl-qe-io.o +obj-$(CONFIG_PINCTRL_SCMI) += pinctrl-scmi.o obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o obj-$(CONFIG_PINCTRL_STI) += pinctrl-sti.o obj-$(CONFIG_PINCTRL_STM32) += pinctrl_stm32.o diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c new file mode 100644 index 000000000000..3ebdad57b86c --- /dev/null +++ b/drivers/pinctrl/pinctrl-scmi.c @@ -0,0 +1,537 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2023 Linaro Limited + * Author: AKASHI Takahiro takahiro.akashi@linaro.org + */ + +#define LOG_CATEGORY UCLASS_PINCTRL + +#include <common.h> +#include <dm.h> +#include <malloc.h> +#include <scmi_agent.h> +#include <scmi_protocols.h> +#include <dm/device_compat.h> +#include <dm/pinctrl.h> + +/** + * struct scmi_pin - attributes for a pin + * @name: Name of pin + * @value: Value of pin + * @flags: A set of flags + * @function: Function selected + * @status: An array of status of configuration types + */ +struct scmi_pin { + char *name; + u32 value; + u32 flags; + unsigned int function; + u32 status[SCMI_PINCTRL_CONFIG_RESERVED]; +}; + +/** + * struct scmi_group - attributes for a group + * @name: Name of group + * @num_pins: A number of pins + * @pins: An array of pin id's + */ +struct scmi_group { + char *name; + unsigned int num_pins; + u16 *pins; +}; + +/** + * struct scmi_pinctrl_priv - private data for pinctrl device + * @num_pins: A number of pins + * @num_groups: A number of groups + * @num_functions: A number of functions + * @pins: An array of pins + * @groups: An array of groups + * @functions: An array of function names + */ +struct scmi_pinctrl_priv { + unsigned int num_pins; + unsigned int num_groups; + unsigned int num_functions; + struct scmi_pin *pins; + struct scmi_group *groups; + char **functions; +}; + +static const struct pinconf_param scmi_conf_params[] = { + { "default", SCMI_PINCTRL_CONFIG_DEFAULT, 1 }, + { "bias-bus-hold", SCMI_PINCTRL_CONFIG_BIAS_BUS_HOLD, 1 }, + { "bias-disable", SCMI_PINCTRL_CONFIG_BIAS_DISABLE, 1 }, + { "bias-high-impedance", SCMI_PINCTRL_CONFIG_BIAS_HI_IMPEDANCE, 1 }, + { "bias-pull-up", SCMI_PINCTRL_CONFIG_BIAS_PULL_UP, 1 }, + { "bias-pull-default", SCMI_PINCTRL_CONFIG_BIAS_PULL_DEF, 1 }, + { "bias-pull-down", SCMI_PINCTRL_CONFIG_BIAS_PULL_DOWN, 1 }, + { "drive-open-drain", SCMI_PINCTRL_CONFIG_DRIVE_OPEN_DRAIN, 1 }, + { "drive-open-source", SCMI_PINCTRL_CONFIG_DRIVE_OPEN_SOURCE, 1 }, + { "drive-push-pull", SCMI_PINCTRL_CONFIG_DRIVE_PUSH_PULL, 1 }, + { "drive-strength", SCMI_PINCTRL_CONFIG_DRIVE_STRENGTH, 0 }, + { "input-debounce", SCMI_PINCTRL_CONFIG_INPUT_DEBOUNCE, 0 }, + { "input-mode", SCMI_PINCTRL_CONFIG_INPUT_MODE, 1 }, + { "pull-mode", SCMI_PINCTRL_CONFIG_PULL_MODE, 0 }, + { "input-value", SCMI_PINCTRL_CONFIG_INPUT_VALUE, 0 }, + { "input-schmitt", SCMI_PINCTRL_CONFIG_INPUT_SCHMITT, 1 }, + { "low-power-mode", SCMI_PINCTRL_CONFIG_LOW_POWER_MODE, 1 }, + { "output-mode", SCMI_PINCTRL_CONFIG_OUTPUT_MODE, 1 }, + { "output-value", SCMI_PINCTRL_CONFIG_OUTPUT_VALUE, 0 }, + { "power-source", SCMI_PINCTRL_CONFIG_POWER_SOURCE, 0 }, + { "slew-rate", SCMI_PINCTRL_CONFIG_SLEW_RATE, 0 }, +}; + +/** + * pinctrl_get_name - get a name + * @dev: SCMI pinctrl device + * @type: Type of id + * @id: Identifier of pin, group or function + * + * Get a name of @id. + * @type can be SCMI_PINCTRL_TYPE_PIN, GROUP or FUNCTION. + * An extended name is returned if it is provided. + * + * Return: A pointer to the name, NULL if failed. + */ +static char *pinctrl_get_name(struct udevice *dev, unsigned int type, + unsigned int id) +{ + u8 *name, *extended_name; + bool extended; + int ret; + + ret = scmi_pinctrl_attrs(dev, id, type, &extended, &name); + if (ret) { + dev_err(dev, "failed to get attributes (%d)\n", ret); + return NULL; + } + + if (!extended) + return name; + + ret = scmi_pinctrl_name_get(dev, id, type, &extended_name); + if (ret) { + dev_err(dev, "failed to get extended_name (%d)\n", ret); + return name; + } + + free(name); + return extended_name; +} + +/** + * get_pins_count - Get the number of selectable pins + * @dev: SCMI pinctrl device to use + * + * Get the number of selectable named pins available in this driver + * + * Return: a number of pins + */ +static int scmi_get_pins_count(struct udevice *dev) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + + return priv->num_pins; +} + +/** + * get_pin_name - Get the name of a pin + * @dev: SCMI pinctrl device of the pin + * @selector: The pin selector + * + * Get the name of a pin + * + * Return: a pointer to the name of the pin + */ +static const char *scmi_get_pin_name(struct udevice *dev, unsigned int selector) +{ + return pinctrl_get_name(dev, SCMI_PINCTRL_TYPE_PIN, selector); +} + +/** + * get_pin_muxing - Show pin muxing + * @dev: SCMI pinctrl device to use + * @selector: Pin selector + * @buf: Buffer to fill with pin muxing description + * @size: Size of @buf + * + * Create a displayable information in @buf about the muxing of a given pin. + * + * @Return: 0 if OK, or negative error code on failure + */ +static int scmi_get_pin_muxing(struct udevice *dev, unsigned int selector, + char *buf, int size) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + char tmp[100]; + int i; + + if (priv->pins[selector].function == UINT_MAX) { + strlcpy(buf, "<unknown>", size); + return 0; + } + + sprintf(tmp, "%s", priv->functions[priv->pins[selector].function]); + strlcpy(buf, tmp, size); + + for (i = 0; i < SCMI_PINCTRL_CONFIG_RESERVED; i++) { + /* TODO: distinguish 0 and "disabled" in status */ + if (priv->pins[selector].status[i]) { + strlcat(buf, " ", size); + strlcat(buf, scmi_conf_params[i].property, size); + } + } + strlcat(buf, ".", size); + + return 0; +} + +/** + * get_groups_count - Get the number of selectable groups + * @dev: SCMI pinctrl device to use + * + * Get a number of selectable groups + * + * Return: a number of selectable named groups available in the driver + */ +static int scmi_get_groups_count(struct udevice *dev) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + + return priv->num_groups; +} + +/** + * get_group_name - Get the name of a group + * @dev: SCMI pinctrl device of the group + * @selector: The group selector + * + * Ge the name of a group + * + * Return: a pointer to the name of the group + */ +static const char *scmi_get_group_name(struct udevice *dev, + unsigned int selector) +{ + return pinctrl_get_name(dev, SCMI_PINCTRL_TYPE_GROUP, selector); +} + +/** + * get_functions_count - Get the number of selectable functions + * @dev: SCMI pinctrl device to use + * + * Get a number of selectable functions + * + * Return: a number of selectable named functions available in this driver + */ +static int scmi_get_functions_count(struct udevice *dev) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + + return priv->num_functions; +} + +/** + * get_function_name - Get the name of a function + * @dev: SCMI pinmux device of the function + * @selector: The function selector + * + * Get a name of a function + * + * Return: a pointer to the function name of the muxing selector + */ +static const char *scmi_get_function_name(struct udevice *dev, + unsigned int selector) +{ + return pinctrl_get_name(dev, SCMI_PINCTRL_TYPE_FUNCTION, selector); +} + +/** + * pinmux_set - Mux a pin to a function + * @dev: SCMI pinctrl device to use + * @pin_selector: The pin selector + * @func_selector: The func selector + * + * Set a function, @function_selector, to @pin_selector. + * + * Return: 0 if OK, or negative error code on failure + */ +static int scmi_pinmux_set(struct udevice *dev, unsigned int pin_selector, + unsigned int func_selector) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + int ret; + + ret = scmi_pinctrl_function_select(dev, pin_selector, func_selector, + SCMI_PINCTRL_TYPE_PIN); + if (ret) { + dev_err(dev, "failed to select function (%d)\n", ret); + return ret; + } + + priv->pins[pin_selector].function = func_selector; + + return 0; +} + +/** + * pinmux_group_set - Mux a group of pins to a function + * @dev: SCMI pinctrl device to use + * @group_selector: The group selector + * @func_selector: The func selector + * + * Set a function, @function_selector, to @group_selector. + * + * @Return: 0 if OK, or negative error code on failure + */ +static int scmi_pinmux_group_set(struct udevice *dev, + unsigned int group_selector, + unsigned int func_selector) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + int i, ret; + + ret = scmi_pinctrl_function_select(dev, group_selector, func_selector, + SCMI_PINCTRL_TYPE_GROUP); + if (ret) { + dev_err(dev, "failed to select function (%d)\n", ret); + return ret; + } + + for (i = 0; i < priv->groups[group_selector].num_pins; i++) + priv->pins[priv->groups[group_selector].pins[i]].function = + func_selector; + + return 0; +} + +/* TODO: may be driver-specific */ +/** + * pinmux_property_set - Enable a pinmux group + * @dev: SCMI pinctrl device to use + * @pinmux_group: A u32 representing the pin identifier and mux + * settings. + * + * Mux a single pin to a single function based on a driver-specific + * pinmux group. + * The format of @pinmux_group follows ... + * + * Return: Pin selector for the muxed pin if OK, or negative error code on + * failure + */ +static int scmi_pinmux_property_set(struct udevice *dev, u32 pinmux_group) +{ + unsigned int pin_selector = pinmux_group & 0xFFFF; + unsigned int func_selector = pinmux_group >> 16; + int ret; + + ret = scmi_pinmux_set(dev, pin_selector, func_selector); + + return ret ? ret : pin_selector; +} + +/** + * pinconf_set - Configure an individual pin with a parameter + * @dev: SCMI pinctrl device to use + * @pin_selector: The pin selector + * @param: An &enum pin_config_param from @pinconf_params + * @argument: The argument to this param from the device tree, or + * @pinconf_params.default_value + * + * Configure @param of a pin, @pin_selector, with @argument. + * + * @Return: 0 if OK, or negative error code on failure + */ +static int scmi_pinconf_set(struct udevice *dev, unsigned int pin_selector, + unsigned int param, unsigned int argument) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + struct scmi_pin_entry config; + int ret; + + config.type = param; + config.value = argument; + ret = scmi_pinctrl_config_set(dev, pin_selector, SCMI_PINCTRL_TYPE_PIN, + 1, &config); + if (ret) { + dev_err(dev, "failed to set config (%d)\n", ret); + return ret; + } + + if (param < SCMI_PINCTRL_CONFIG_RESERVED) + priv->pins[pin_selector].status[param] = argument; + + return 0; +} + +/** + * pinconf_group_set - Configure all pins in a group with a parameter + * @dev: SCmi pinctrl device to use + * @group_selector: The group selector + * @param: A &enum pin_config_param from @pinconf_params + * @argument: The argument to this param from the device tree, or + * @pinconf_params.default_value + * + * Configure @param of all the pins in a group, @group_selector, with @argument. + * + * @Return: 0 if OK, or negative error code on failure + */ +static int scmi_pinconf_group_set(struct udevice *dev, + unsigned int group_selector, + unsigned int param, unsigned int argument) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + struct scmi_pin_entry config; + int i, ret; + + config.type = param; + config.value = argument; + ret = scmi_pinctrl_config_set(dev, group_selector, + SCMI_PINCTRL_TYPE_GROUP, 1, &config); + if (ret) { + dev_err(dev, "failed to set config (%d)\n", ret); + return ret; + } + + if (param >= SCMI_PINCTRL_CONFIG_RESERVED) + return 0; + + for (i = 0; i < priv->groups[group_selector].num_pins; i++) + priv->pins[priv->groups[group_selector].pins[i]].status[param] = + argument; + + return 0; +} + +const struct pinctrl_ops scmi_pinctrl_ops = { + .get_pins_count = scmi_get_pins_count, + .get_pin_name = scmi_get_pin_name, + .get_pin_muxing = scmi_get_pin_muxing, + .get_groups_count = scmi_get_groups_count, + .get_group_name = scmi_get_group_name, + .get_functions_count = scmi_get_functions_count, + .get_function_name = scmi_get_function_name, + .pinmux_set = scmi_pinmux_set, + .pinmux_group_set = scmi_pinmux_group_set, + .pinmux_property_set = scmi_pinmux_property_set, + .pinconf_num_params = ARRAY_SIZE(scmi_conf_params), + .pinconf_params = scmi_conf_params, + .pinconf_set = scmi_pinconf_set, + .pinconf_group_set = scmi_pinconf_group_set, + .set_state = pinctrl_generic_set_state, +}; + +/** + * scmi_pinctrl_probe - probe a device + * @dev: SCMI pinctrl device + * + * Probe and initialize a pinctrl device. + * + * Return: 0 on success, error code on failure + */ +static int scmi_pinctrl_probe(struct udevice *dev) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + u32 version; + char *name; + int i, ret; + + ret = devm_scmi_of_get_channel(dev); + if (ret) { + dev_err(dev, "failed to get channel (%d)\n", ret); + return ret; + } + + ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_PIN_CONTROL, + &version); + if (ret || version < SCMI_PIN_CONTROL_PROTOCOL_VERSION) { + dev_err(dev, "protocol version doesn't match (%d)\n", version); + return -EINVAL; + } + + ret = scmi_pinctrl_protocol_attrs(dev, &priv->num_pins, + &priv->num_groups, + &priv->num_functions); + if (ret) { + dev_err(dev, "failed to get protocol attributes (%d)\n", ret); + return ret; + } + + priv->pins = calloc(sizeof(struct scmi_pin), priv->num_pins); + if (!priv->pins) { + dev_err(dev, "memory not available\n"); + return -ENOMEM; + } + for (i = 0; i < priv->num_pins; i++) { + priv->pins[i].function = UINT_MAX; /* unknown yet */ + name = scmi_get_pin_name(dev, i); + if (!name) { + dev_err(dev, "failed to get pin name\n"); + return ret; + } + priv->pins[i].name = strdup(name); + free(name); + if (!priv->pins[i].name) { + dev_err(dev, "memory not available\n"); + return -ENOMEM; + } + } + + priv->groups = calloc(sizeof(struct scmi_group), priv->num_groups); + if (!priv->groups) + return -ENOMEM; + for (i = 0; i < priv->num_groups; i++) { + name = scmi_get_group_name(dev, i); + if (!name) { + dev_err(dev, "failed to get group name\n"); + return ret; + } + priv->groups[i].name = strdup(name); + free(name); + if (!priv->groups[i].name) { + dev_err(dev, "memory not available\n"); + return -ENOMEM; + } + + ret = scmi_pinctrl_list_assocs(dev, i, SCMI_PINCTRL_TYPE_GROUP, + &priv->groups[i].pins); + if (ret < 0) { + dev_err(dev, "failed to enumerate pins (%d)\n", ret); + return ret; + } + + priv->groups[i].num_pins = ret; + } + + priv->functions = calloc(sizeof(char *), priv->num_functions); + if (!priv->functions) { + dev_err(dev, "memory not available\n"); + return -ENOMEM; + } + for (i = 0; i < priv->num_functions; i++) { + name = scmi_get_function_name(dev, i); + if (!name) { + dev_err(dev, "failed to get group name\n"); + return ret; + } + priv->functions[i] = strdup(name); + free(name); + if (!priv->functions[i]) { + dev_err(dev, "memory not available\n"); + return -ENOMEM; + } + } + + return 0; +} + +U_BOOT_DRIVER(scmi_pinctrl) = { + .name = "scmi_pinctrl", + .id = UCLASS_PINCTRL, + .ops = &scmi_pinctrl_ops, + .probe = scmi_pinctrl_probe, + .priv_auto = sizeof(struct scmi_pinctrl_priv), +};
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This DM-compliant driver deals with SCMI pinctrl protocol and presents pinctrl devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/pinctrl/Kconfig | 11 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-scmi.c | 537 +++++++++++++++++++++++++++++++++ 3 files changed, 549 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-scmi.c
[..]
+
+U_BOOT_DRIVER(scmi_pinctrl) = {
.name = "scmi_pinctrl",.id = UCLASS_PINCTRL,.ops = &scmi_pinctrl_ops,.probe = scmi_pinctrl_probe,.priv_auto = sizeof(struct scmi_pinctrl_priv),+};
2.34.1
Where is the compatible string for this driver?
Regards, Simon
Hi Simon,
On Sun, Sep 10, 2023 at 01:13:27PM -0600, Simon Glass wrote:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This DM-compliant driver deals with SCMI pinctrl protocol and presents pinctrl devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/pinctrl/Kconfig | 11 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-scmi.c | 537 +++++++++++++++++++++++++++++++++ 3 files changed, 549 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-scmi.c
[..]
+U_BOOT_DRIVER(scmi_pinctrl) = {
.name = "scmi_pinctrl",.id = UCLASS_PINCTRL,.ops = &scmi_pinctrl_ops,.probe = scmi_pinctrl_probe,.priv_auto = sizeof(struct scmi_pinctrl_priv),+};
2.34.1
Where is the compatible string for this driver?
Nothing defined.
As I mentioned in the cover letter, pinctrl driver consists of two layers: - helper functions which directly manipulate protocol interfaces. - DM-compliant (that you doubt about :) pinctrl driver.
All the SCMI-based drivers (existing ones, clock, reset and voltage domain, except base) follow this scheme.
According to the DT bindings for SCMI protocols, they are sub-nodes of scmi node and identified by "reg" properties. When the *bind* takes place, scmi_bind_protocols() will enumerate all the sub-nodes and try to bind associated protocol drivers (pinctrl in my patch) based on "reg" value.
So there is no need to have a "compatible" property for each protocol. Do you think it is ugly? I suppose the corresponding kernel code, scmi_probe() in drivers/firmware/arm_scmi/driver.c, has a similar logic although it seems a bit more complicated.
-Takahiro Akashi
Regards, Simon
This DM-compliant driver deals with SCMI pinctrl protocol and presents gpio devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++- 1 file changed, 539 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c index 3ebdad57b86c..73d385bdbfcc 100644 --- a/drivers/pinctrl/pinctrl-scmi.c +++ b/drivers/pinctrl/pinctrl-scmi.c @@ -11,21 +11,20 @@ #include <malloc.h> #include <scmi_agent.h> #include <scmi_protocols.h> +#include <asm-generic/gpio.h> #include <dm/device_compat.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <dm/pinctrl.h>
/** * struct scmi_pin - attributes for a pin * @name: Name of pin - * @value: Value of pin - * @flags: A set of flags * @function: Function selected * @status: An array of status of configuration types */ struct scmi_pin { char *name; - u32 value; - u32 flags; unsigned int function; u32 status[SCMI_PINCTRL_CONFIG_RESERVED]; }; @@ -308,7 +307,6 @@ static int scmi_pinmux_group_set(struct udevice *dev, return 0; }
-/* TODO: may be driver-specific */ /** * pinmux_property_set - Enable a pinmux group * @dev: SCMI pinctrl device to use @@ -424,6 +422,539 @@ const struct pinctrl_ops scmi_pinctrl_ops = { .set_state = pinctrl_generic_set_state, };
+#if CONFIG_IS_ENABLED(DM_GPIO) +/* + * GPIO part + */ + +/** + * struct scmi_pinctrl_gpio_plat - platform data + * @pinctrl_dev: Associated SCMI pinctrl device + * @gpio_map: A list of gpio mapping information + * @name: Name of ? + * @ngpios: A number of gpio pins + */ +struct scmi_pinctrl_gpio_plat { + struct list_head gpio_map; + char *name; + u32 ngpios; +}; + +/** + * struct scmi_pinctrl_gpio_map - gpio mapping information + * @gpio_pin: Start gpio pin selector + * @pinctrl_pin: Mapped start pinctrl pin selector, used for linear mapping + * @pinctrl_group: Name of an associated group, used for group namee mapping + * @num_pins: A number of pins + * @node: Node for a mapping list + */ +struct scmi_pinctrl_gpio_map { + struct udevice *pinctrl_dev; + u32 gpio_pin; + u32 pinctrl_pin; + u16 *pinctrl_group; + u32 num_pins; + struct list_head node; +}; + +/** + * map_to_id - Map a gpio pin offset to a pinctrl pin selector + * @dev: SCMI pinctrl device + * @offset: Pin offset + * @id: Pinctrl pin selector + * + * Map a gpio pin offset, @offset, to an associated pinctrl pin selector + * + * Return: 0 on success, -1 on failure + */ +static int map_to_id(struct udevice *dev, unsigned int offset, + struct udevice **pinctrl_dev, u32 *id) +{ + struct scmi_pinctrl_gpio_plat *plat = dev_get_plat(dev); + struct scmi_pinctrl_gpio_map *map; + + /* if no mapping is defined, return 1:1 pin */ + if (list_empty(&plat->gpio_map)) { + uclass_first_device(UCLASS_PINCTRL, pinctrl_dev); + if (!*pinctrl_dev) + return -1; + + *id = offset; + return 0; + } + + list_for_each_entry(map, &plat->gpio_map, node) { + if (offset >= map->gpio_pin && + offset < (map->gpio_pin + map->num_pins)) { + *pinctrl_dev = map->pinctrl_dev; + if (!pinctrl_dev) + return -1; + + if (map->pinctrl_group) + *id = map->pinctrl_group[offset + - map->gpio_pin]; + else + *id = map->pinctrl_pin + + (offset - map->gpio_pin); + + return 0; + } + } + + return -1; +} + +/** + * scmi_gpio_get_value - Get a value of a gpio pin + * @dev: SCMI gpio device + * @offset: Pin offset + * + * Get a value of a gpio pin in @offset + * + * Return: Value of a pin on success, error code on failure + */ +static int scmi_gpio_get_value(struct udevice *dev, unsigned int offset) +{ + struct scmi_pinctrl_priv *priv; + u32 id; + struct udevice *pinctrl_dev; + struct scmi_pin_entry *config; + int config_type, ret; + + if (map_to_id(dev, offset, &pinctrl_dev, &id)) { + dev_err(dev, "Invalid pin: %u\n", offset); + return -EINVAL; + } + + priv = dev_get_priv(pinctrl_dev); + if (priv->pins[id].status[SCMI_PINCTRL_CONFIG_INPUT_MODE]) { + config_type = SCMI_PINCTRL_CONFIG_INPUT_VALUE; + } else if (priv->pins[id].status[SCMI_PINCTRL_CONFIG_OUTPUT_MODE]) { + config_type = SCMI_PINCTRL_CONFIG_OUTPUT_VALUE; + } else { + dev_err(dev, "Invalid pin mode: %u\n", offset); + return -EINVAL; + } + + ret = scmi_pinctrl_config_get(pinctrl_dev, id, false, + SCMI_PINCTRL_TYPE_PIN, config_type, + &config); + if (ret) { + dev_err(dev, "config_get failed (%d)\n", ret); + return ret; + } + + return config->value; +} + +/** + * config_pin_set - Configure a pinctrl pin + * @dev: SCMI pinctrl device + * @id: Pin selector + * @config_type: Configuration type + * @value: Value + * + * Set a configuration of @config_type of a pinctrl pin, @id, to @value. + * + * Return: 0 on success, error code on failure + */ +static int config_pin_set(struct udevice *dev, u32 id, u32 config_type, + u32 value) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + struct scmi_pin_entry config; + int ret; + + config.type = config_type; + config.value = value; + ret = scmi_pinctrl_config_set(dev, id, SCMI_PINCTRL_TYPE_PIN, + 1, &config); + if (ret) { + dev_err(dev, "config_set failed (%d)\n", ret); + return ret; + } + + priv->pins[id].status[config_type] = value; + + return 0; +} + +/** + * scmi_gpio_set_value - Set a value of a gpio pin + * @dev: SCMI gpio device + * @offset: Pin offset + * @value: Value + * + * Set a value of a gpio pin in @offset to @value. + * + * Return: 0 on success, error code on failure + */ +static int scmi_gpio_set_value(struct udevice *dev, unsigned int offset, + int value) +{ + struct udevice *pinctrl_dev; + u32 id; + + if (map_to_id(dev, offset, &pinctrl_dev, &id)) { + dev_err(dev, "Invalid pin: %u\n", offset); + return -EINVAL; + } + + return config_pin_set(pinctrl_dev, id, + SCMI_PINCTRL_CONFIG_OUTPUT_VALUE, + (u32)value); +} + +/** + * scmi_gpio_get_function - Get a gpio function of a gpio pin + * @dev: SCMI gpio device + * @offset: Pin offset + * + * Get a gpio function of a gpio pin in @offset. + * + * Return: GPIOF_OUTPUT or GPIO_INPUT on success, otherwise GPIOF_UNKNOWN + */ +static int scmi_gpio_get_function(struct udevice *dev, unsigned int offset) +{ + struct udevice *pinctrl_dev; + u32 id; + struct scmi_pin_entry *config; + int ret; + + if (map_to_id(dev, offset, &pinctrl_dev, &id)) { + dev_err(dev, "Invalid pin: %u\n", offset); + return -EINVAL; + } + + ret = scmi_pinctrl_config_get(pinctrl_dev, id, false, + SCMI_PINCTRL_TYPE_PIN, + SCMI_PINCTRL_CONFIG_INPUT_MODE, &config); + if (!ret && config->value == 1) + return GPIOF_INPUT; + + ret = scmi_pinctrl_config_get(pinctrl_dev, id, false, + SCMI_PINCTRL_TYPE_PIN, + SCMI_PINCTRL_CONFIG_OUTPUT_MODE, + &config); + if (!ret && config->value == 1) + return GPIOF_OUTPUT; + + return GPIOF_UNKNOWN; +} + +/** + * set_flags - Adjust GPIO flags + * @dev: SCMI gpio device + * @offset: Pin offset + * @flags: New flags value + * + * This function should set up the GPIO configuration according to information + * provided by @flags (GPIOD_xxx). + * + * @return 0 if OK, otherwise -ve on error + */ +static int scmi_gpio_set_flags(struct udevice *dev, unsigned int offset, + ulong flags) +{ + struct udevice *pinctrl_dev; + struct scmi_pin_entry config[5]; /* 5 is enough for now */ + u32 id; + struct scmi_pinctrl_priv *priv; + int i = 0, ret; + + if (map_to_id(dev, offset, &pinctrl_dev, &id)) { + dev_err(dev, "Invalid pin: %u\n", offset); + return -EINVAL; + } + + if (flags & GPIOD_IS_OUT) { + config[i].type = SCMI_PINCTRL_CONFIG_INPUT_MODE; + config[i++].value = 0; + + config[i].type = SCMI_PINCTRL_CONFIG_OUTPUT_MODE; + config[i++].value = 1; + config[i].type = SCMI_PINCTRL_CONFIG_OUTPUT_VALUE; + config[i++].value = (u32)!!(flags & GPIOD_IS_OUT_ACTIVE); + } else if (flags & GPIOD_IS_IN) { + config[i].type = SCMI_PINCTRL_CONFIG_OUTPUT_MODE; + config[i++].value = 0; + + config[i].type = SCMI_PINCTRL_CONFIG_INPUT_MODE; + config[i++].value = 1; + } + + if (flags & GPIOD_OPEN_DRAIN) { + config[i].type = SCMI_PINCTRL_CONFIG_DRIVE_OPEN_DRAIN; + config[i++].value = 1; + } else if (flags & GPIOD_OPEN_SOURCE) { + config[i].type = SCMI_PINCTRL_CONFIG_DRIVE_OPEN_SOURCE; + config[i++].value = 1; + } + + if (flags & GPIOD_PULL_UP) { + config[i].type = SCMI_PINCTRL_CONFIG_BIAS_PULL_UP; + config[i++].value = 1; + } else if (flags & GPIOD_PULL_DOWN) { + config[i].type = SCMI_PINCTRL_CONFIG_BIAS_PULL_DOWN; + config[i++].value = 1; + } + + ret = scmi_pinctrl_config_set(pinctrl_dev, id, SCMI_PINCTRL_TYPE_PIN, + i, config); + if (ret) { + dev_err(dev, "config_set failed (%d)\n", ret); + return ret; + } + + /* update local status */ + priv = dev_get_priv(pinctrl_dev); + + if (flags & GPIOD_IS_OUT) { + priv->pins[id].status[SCMI_PINCTRL_CONFIG_OUTPUT_MODE] = 1; + priv->pins[id].status[SCMI_PINCTRL_CONFIG_OUTPUT_VALUE] = 1; + + priv->pins[id].status[SCMI_PINCTRL_CONFIG_INPUT_MODE] = 0; + } else if (flags & GPIOD_IS_IN) { + priv->pins[id].status[SCMI_PINCTRL_CONFIG_INPUT_MODE] = 1; + priv->pins[id].status[SCMI_PINCTRL_CONFIG_OUTPUT_MODE] = 0; + } + + if (flags & GPIOD_OPEN_DRAIN) + priv->pins[id].status[SCMI_PINCTRL_CONFIG_DRIVE_OPEN_DRAIN] = 1; + else if (flags & GPIOD_OPEN_SOURCE) + priv->pins[id].status[SCMI_PINCTRL_CONFIG_DRIVE_OPEN_SOURCE] = + 1; + + if (flags & GPIOD_PULL_UP) + priv->pins[id].status[SCMI_PINCTRL_CONFIG_BIAS_PULL_UP] = 1; + else if (flags & GPIOD_PULL_DOWN) + priv->pins[id].status[SCMI_PINCTRL_CONFIG_BIAS_PULL_DOWN] = 1; + + return 0; +} + +/** + * lookup_pin_group - find out a list of pins for group name + * @dev: SCMI pinctrl device + * @name: Name of a group + * @num_pins: Pointer to a number of pins + * @pins: Pointer to array of pin selectors + * + * Look up a group named @name and return a list of pin selectors associated with + * the group in @pins. + * + * Return: 0 on success, -1 on failure + */ +static int lookup_pin_group(struct udevice *dev, const char *name, + u32 *num_pins, u16 **pins) +{ + struct scmi_pinctrl_priv *priv = dev_get_priv(dev); + int i; + + for (i = 0; i < priv->num_groups; i++) + if (!strcmp(name, priv->groups[i].name)) { + *num_pins = priv->groups[i].num_pins; + *pins = priv->groups[i].pins; + return 0; + } + + return -1; +} + +/** + * get_pinctrl_dev - Get a pin control device at node + * @node: Reference to the node + * + * Get a pin control device at @node. + * + * Return: 0 on success, otherwise NULL + */ +static struct udevice *get_pinctrl_dev(ofnode *node) +{ + struct udevice *dev; + struct uclass *uc; + + if (uclass_get(UCLASS_PINCTRL, &uc)) + return NULL; + + uclass_foreach_dev(dev, uc) + if (dev->node_.np == node->np) + return dev; + + return NULL; +} + +/** + * scmi_gpio_probe - Probe a SCMI gpio device + * @dev: SCMI gpio device + * + * Probe and initialize a SCMI gpio device. + * + * 0 on success, error code on failure + */ +static int scmi_gpio_probe(struct udevice *dev) +{ + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); + struct scmi_pinctrl_gpio_plat *plat = dev_get_plat(dev); + + struct scmi_pinctrl_gpio_map *range; + struct ofnode_phandle_args args; + const char *name; + int num_pins = 0, index, ret; + + uc_priv->bank_name = dev_read_string(dev, "gpio-bank-name"); + if (!uc_priv->bank_name) + uc_priv->bank_name = dev->name; + + INIT_LIST_HEAD(&plat->gpio_map); + for (index = 0; ; index++) { + ret = dev_read_phandle_with_args(dev, "gpio-ranges", + NULL, 3, index, &args); + if (ret) + break; + + range = malloc(sizeof(*range)); + if (!range) { + dev_err(dev, "Memory not available\n"); + return -ENOMEM; + } + + range->pinctrl_dev = get_pinctrl_dev(&args.node); + if (!range->pinctrl_dev) { + dev_err(dev, "Pin control device not found\n"); + return -ENODEV; + } + + /* make sure pinctrl is activated */ + ret = device_probe(range->pinctrl_dev); + if (ret) + return ret; + + if (args.args[2]) { + range->gpio_pin = args.args[0]; + range->pinctrl_pin = args.args[1]; + range->num_pins = args.args[2]; + range->pinctrl_group = NULL; + } else { + ret = of_property_read_string_index(dev_np(dev), + "gpio-ranges-group-names", + index, &name); + if (ret) { + dev_err(dev, + "gpio-ranges-group-names required\n"); + return ret; + } + + ret = lookup_pin_group(range->pinctrl_dev, name, + &range->num_pins, + &range->pinctrl_group); + if (ret) { + dev_err(dev, "Group not found: %s\n", name); + return -EINVAL; + } + + range->gpio_pin = args.args[0]; + } + num_pins += range->num_pins; + list_add_tail(&range->node, &plat->gpio_map); + } + + uc_priv->gpio_count = num_pins; + + return 0; +} + +static const struct dm_gpio_ops scmi_gpio_ops = { + .get_function = scmi_gpio_get_function, + .get_value = scmi_gpio_get_value, + .set_value = scmi_gpio_set_value, + .set_flags = scmi_gpio_set_flags, +}; + +static const struct udevice_id scmi_gpio_ids[] = { + { .compatible = "arm,scmi-gpio-generic" }, + { } +}; + +U_BOOT_DRIVER(scmi_gpio) = { + .name = "scmi_gpio", + .id = UCLASS_GPIO, + .of_match = scmi_gpio_ids, + .of_to_plat = scmi_gpio_probe, + .ops = &scmi_gpio_ops, + .plat_auto = sizeof(struct scmi_pinctrl_gpio_plat), +}; + +/** + * scmi_gpiochip_register - Create a pinctrl-controlled gpio device + * @parent: SCMI pinctrl device + * + * Create a pinctrl-controlled gpio device + * + * Return: 0 on success, error code on failure + */ +static int scmi_gpiochip_register(struct udevice *parent) +{ + ofnode node; + struct driver *drv; + struct udevice *gpio_dev; + int ret; + + /* TODO: recovery if failed */ + dev_for_each_subnode(node, parent) { + if (!ofnode_is_enabled(node)) + continue; + + if (!ofnode_read_prop(node, "gpio-controller", NULL)) + /* not a GPIO node */ + continue; + + drv = DM_DRIVER_GET(scmi_gpio); + if (!drv) { + dev_err(parent, "No gpio driver?\n"); + return -ENODEV; + } + + ret = device_bind(parent, drv, ofnode_get_name(node), NULL, + node, &gpio_dev); + if (ret) { + dev_err(parent, "failed to bind %s to gpio (%d)\n", + ofnode_get_name(node), ret); + return -ENODEV; + } + + return 0; + } + + return -ENODEV; +} + +/** + * scmi_pinctrl_bind - Bind a pinctrl-controlled gpio device + * @dev: SCMI pinctrl device + * + * Bind a pinctrl-controlled gpio device + * + * Return: 0 on success, error code on failure + */ +static int scmi_pinctrl_bind(struct udevice *dev) +{ + int ret; + + /* gpiochip register */ + if (CONFIG_IS_ENABLED(DM_GPIO)) { + ret = scmi_gpiochip_register(dev); + if (ret && ret != -ENODEV) { + dev_err(dev, "failed to register gpio driver (%d)\n", + ret); + return ret; + } + } + + return 0; +} +#endif /* CONFIG_DM_GPIO */ + /** * scmi_pinctrl_probe - probe a device * @dev: SCMI pinctrl device @@ -532,6 +1063,9 @@ U_BOOT_DRIVER(scmi_pinctrl) = { .name = "scmi_pinctrl", .id = UCLASS_PINCTRL, .ops = &scmi_pinctrl_ops, +#if CONFIG_IS_ENABLED(DM_GPIO) + .bind = scmi_pinctrl_bind, +#endif .probe = scmi_pinctrl_probe, .priv_auto = sizeof(struct scmi_pinctrl_priv), };
On 9/6/23 04:40, AKASHI Takahiro wrote:
This DM-compliant driver deals with SCMI pinctrl protocol and presents gpio devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++- 1 file changed, 539 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c index 3ebdad57b86c..73d385bdbfcc 100644 --- a/drivers/pinctrl/pinctrl-scmi.c +++ b/drivers/pinctrl/pinctrl-scmi.c @@ -11,21 +11,20 @@ #include <malloc.h> #include <scmi_agent.h> #include <scmi_protocols.h> +#include <asm-generic/gpio.h> #include <dm/device_compat.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <dm/pinctrl.h>
/**
- struct scmi_pin - attributes for a pin
- @name: Name of pin
- @value: Value of pin
*/ struct scmi_pin { char *name;
- @flags: A set of flags
- @function: Function selected
- @status: An array of status of configuration types
- u32 value;
- u32 flags;
You have added this in 3/6 then there is no reason to remove it in this version.
M
On Wed, Sep 06, 2023 at 04:56:56PM +0200, Michal Simek wrote:
On 9/6/23 04:40, AKASHI Takahiro wrote:
This DM-compliant driver deals with SCMI pinctrl protocol and presents gpio devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++- 1 file changed, 539 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c index 3ebdad57b86c..73d385bdbfcc 100644 --- a/drivers/pinctrl/pinctrl-scmi.c +++ b/drivers/pinctrl/pinctrl-scmi.c @@ -11,21 +11,20 @@ #include <malloc.h> #include <scmi_agent.h> #include <scmi_protocols.h> +#include <asm-generic/gpio.h> #include <dm/device_compat.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <dm/pinctrl.h> /**
- struct scmi_pin - attributes for a pin
- @name: Name of pin
- @value: Value of pin
*/ struct scmi_pin { char *name;
- @flags: A set of flags
- @function: Function selected
- @status: An array of status of configuration types
- u32 value;
- u32 flags;
You have added this in 3/6 then there is no reason to remove it in this version.
Right. It was my mistake in a last-minute cleanup. The hunk should be merged in 3/6.
BTW, this part of code, holding status of every pin's pinconf properties locally, is a bit clumsy. I'd remove the whole code if possible.
-Takahiro Akashi
M
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This DM-compliant driver deals with SCMI pinctrl protocol and presents gpio devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++- 1 file changed, 539 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c index 3ebdad57b86c..73d385bdbfcc 100644 --- a/drivers/pinctrl/pinctrl-scmi.c +++ b/drivers/pinctrl/pinctrl-scmi.c @@ -11,21 +11,20 @@ #include <malloc.h> #include <scmi_agent.h> #include <scmi_protocols.h> +#include <asm-generic/gpio.h> #include <dm/device_compat.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <dm/pinctrl.h>
[..]
+U_BOOT_DRIVER(scmi_gpio) = {
.name = "scmi_gpio",.id = UCLASS_GPIO,.of_match = scmi_gpio_ids,.of_to_plat = scmi_gpio_probe,.ops = &scmi_gpio_ops,.plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),+};
+/**
- scmi_gpiochip_register - Create a pinctrl-controlled gpio device
- @parent: SCMI pinctrl device
- Create a pinctrl-controlled gpio device
- Return: 0 on success, error code on failure
- */
+static int scmi_gpiochip_register(struct udevice *parent) +{
ofnode node;struct driver *drv;struct udevice *gpio_dev;int ret;/* TODO: recovery if failed */dev_for_each_subnode(node, parent) {if (!ofnode_is_enabled(node))continue;
Can we not rely on the normal driver model binding to bind these devices? Why does it need to be done manually here?
if (!ofnode_read_prop(node, "gpio-controller", NULL))/* not a GPIO node */continue;drv = DM_DRIVER_GET(scmi_gpio);if (!drv) {dev_err(parent, "No gpio driver?\n");return -ENODEV;}ret = device_bind(parent, drv, ofnode_get_name(node), NULL,node, &gpio_dev);if (ret) {dev_err(parent, "failed to bind %s to gpio (%d)\n",ofnode_get_name(node), ret);return -ENODEV;}return 0;}return -ENODEV;+}
Regards, Simon
Hi Simon,
On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This DM-compliant driver deals with SCMI pinctrl protocol and presents gpio devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++- 1 file changed, 539 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c index 3ebdad57b86c..73d385bdbfcc 100644 --- a/drivers/pinctrl/pinctrl-scmi.c +++ b/drivers/pinctrl/pinctrl-scmi.c @@ -11,21 +11,20 @@ #include <malloc.h> #include <scmi_agent.h> #include <scmi_protocols.h> +#include <asm-generic/gpio.h> #include <dm/device_compat.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <dm/pinctrl.h>
[..]
+U_BOOT_DRIVER(scmi_gpio) = {
.name = "scmi_gpio",.id = UCLASS_GPIO,.of_match = scmi_gpio_ids,.of_to_plat = scmi_gpio_probe,.ops = &scmi_gpio_ops,.plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),+};
+/**
- scmi_gpiochip_register - Create a pinctrl-controlled gpio device
- @parent: SCMI pinctrl device
- Create a pinctrl-controlled gpio device
- Return: 0 on success, error code on failure
- */
+static int scmi_gpiochip_register(struct udevice *parent) +{
ofnode node;struct driver *drv;struct udevice *gpio_dev;int ret;/* TODO: recovery if failed */dev_for_each_subnode(node, parent) {if (!ofnode_is_enabled(node))continue;Can we not rely on the normal driver model binding to bind these devices? Why does it need to be done manually here?
First, please take a look at the cover letter. In this RFC, I basically assume two patterns of DT bindings, (A) and (B) in the cover letter (or sandbox's test.dts in patch#5).
In (B), a DT node as a gpio device, which is essentially a child of pinctrl device, is located *under* a pinctrl device. It need to be probed manually as there is no implicit method to enumerate it as a DM device automatically.
On the other hand, in (A), the same node can be put anywhere in a DT as it contains a "compatible" property to identify itself. So if we want to only support (A), scmi_gpiochip_register() and scmi_pinctrl_bind() are not necessary.
Since there is no discussion about bindings for GPIO managed by SCMI pinctrl device yet on the kernel side, I have left two solutions in this RFC.
Thanks, -Takakahiro Akashi
if (!ofnode_read_prop(node, "gpio-controller", NULL))/* not a GPIO node */continue;drv = DM_DRIVER_GET(scmi_gpio);if (!drv) {dev_err(parent, "No gpio driver?\n");return -ENODEV;}ret = device_bind(parent, drv, ofnode_get_name(node), NULL,node, &gpio_dev);if (ret) {dev_err(parent, "failed to bind %s to gpio (%d)\n",ofnode_get_name(node), ret);return -ENODEV;}return 0;}return -ENODEV;+}
Regards, Simon
Hi,
On Thu, 7 Sept 2023 at 22:32, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
Hi Simon,
On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This DM-compliant driver deals with SCMI pinctrl protocol and presents gpio devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++- 1 file changed, 539 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c index 3ebdad57b86c..73d385bdbfcc 100644 --- a/drivers/pinctrl/pinctrl-scmi.c +++ b/drivers/pinctrl/pinctrl-scmi.c @@ -11,21 +11,20 @@ #include <malloc.h> #include <scmi_agent.h> #include <scmi_protocols.h> +#include <asm-generic/gpio.h> #include <dm/device_compat.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <dm/pinctrl.h>
[..]
+U_BOOT_DRIVER(scmi_gpio) = {
.name = "scmi_gpio",.id = UCLASS_GPIO,.of_match = scmi_gpio_ids,.of_to_plat = scmi_gpio_probe,.ops = &scmi_gpio_ops,.plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),+};
+/**
- scmi_gpiochip_register - Create a pinctrl-controlled gpio device
- @parent: SCMI pinctrl device
- Create a pinctrl-controlled gpio device
- Return: 0 on success, error code on failure
- */
+static int scmi_gpiochip_register(struct udevice *parent) +{
ofnode node;struct driver *drv;struct udevice *gpio_dev;int ret;/* TODO: recovery if failed */dev_for_each_subnode(node, parent) {if (!ofnode_is_enabled(node))continue;Can we not rely on the normal driver model binding to bind these devices? Why does it need to be done manually here?
First, please take a look at the cover letter. In this RFC, I basically assume two patterns of DT bindings, (A) and (B) in the cover letter (or sandbox's test.dts in patch#5).
In (B), a DT node as a gpio device, which is essentially a child of pinctrl device, is located *under* a pinctrl device. It need to be probed manually as there is no implicit method to enumerate it as a DM device automatically.
You could add a post_bind() method to the pinctrl uclass to call dm_scan_fdt_dev() as we do with TPM.
On the other hand, in (A), the same node can be put anywhere in a DT as it contains a "compatible" property to identify itself. So if we want to only support (A), scmi_gpiochip_register() and scmi_pinctrl_bind() are not necessary.
Since there is no discussion about bindings for GPIO managed by SCMI pinctrl device yet on the kernel side, I have left two solutions in this RFC.
OK.
[..]
Regards, Simon
Hi Simon,
On Sun, Sep 10, 2023 at 01:13:24PM -0600, Simon Glass wrote:
Hi,
On Thu, 7 Sept 2023 at 22:32, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
Hi Simon,
On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This DM-compliant driver deals with SCMI pinctrl protocol and presents gpio devices exposed by SCMI firmware (server).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++- 1 file changed, 539 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c index 3ebdad57b86c..73d385bdbfcc 100644 --- a/drivers/pinctrl/pinctrl-scmi.c +++ b/drivers/pinctrl/pinctrl-scmi.c @@ -11,21 +11,20 @@ #include <malloc.h> #include <scmi_agent.h> #include <scmi_protocols.h> +#include <asm-generic/gpio.h> #include <dm/device_compat.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <dm/pinctrl.h>
[..]
+U_BOOT_DRIVER(scmi_gpio) = {
.name = "scmi_gpio",.id = UCLASS_GPIO,.of_match = scmi_gpio_ids,.of_to_plat = scmi_gpio_probe,.ops = &scmi_gpio_ops,.plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),+};
+/**
- scmi_gpiochip_register - Create a pinctrl-controlled gpio device
- @parent: SCMI pinctrl device
- Create a pinctrl-controlled gpio device
- Return: 0 on success, error code on failure
- */
+static int scmi_gpiochip_register(struct udevice *parent) +{
ofnode node;struct driver *drv;struct udevice *gpio_dev;int ret;/* TODO: recovery if failed */dev_for_each_subnode(node, parent) {if (!ofnode_is_enabled(node))continue;Can we not rely on the normal driver model binding to bind these devices? Why does it need to be done manually here?
First, please take a look at the cover letter. In this RFC, I basically assume two patterns of DT bindings, (A) and (B) in the cover letter (or sandbox's test.dts in patch#5).
In (B), a DT node as a gpio device, which is essentially a child of pinctrl device, is located *under* a pinctrl device. It need to be probed manually as there is no implicit method to enumerate it as a DM device automatically.
You could add a post_bind() method to the pinctrl uclass to call dm_scan_fdt_dev() as we do with TPM.
Okay, but in that case, defining "compatible" property will become mandatory, while I invented (B) so that we don't need extra bindings.
Please note that no discussion started about the binding of gpio devices.
-Takahiro Akashi
On the other hand, in (A), the same node can be put anywhere in a DT as it contains a "compatible" property to identify itself. So if we want to only support (A), scmi_gpiochip_register() and scmi_pinctrl_bind() are not necessary.
Since there is no discussion about bindings for GPIO managed by SCMI pinctrl device yet on the kernel side, I have left two solutions in this RFC.
OK.
[..]
Regards, Simon
With this patch, sandbox SCMI agent can handle pinctrl protocol. This feature is used in an unit test for SCMI pinctrl.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/sandbox/dts/test.dts | 115 ++++ drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +++++++++++++++++++++ 2 files changed, 837 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index dc0bfdfb6e4b..d2ddea801995 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -723,9 +723,124 @@ }; }; }; + + pinctrl_scmi: protocol@19 { + reg = <0x19>; + + pinctrl-names = "default","alternate"; + pinctrl-0 = <&scmi_pinctrl_gpios>, <&scmi_pinctrl_i2s>; + pinctrl-1 = <&scmi_pinctrl_spi>, <&scmi_pinctrl_i2c>; + +#if 0 + scmi_pinctrl_gpio_a: scmi_gpios { + gpio-controller; + #gpio-cells = <2>; + gpio-bank-name = "scmi_gpios"; + ngpios = <4>; + gpio-ranges = <&pinctrl_scmi 0 5 4>, + <&pinctrl_scmi 4 0 0>; + gpio-ranges-group-names = "", + "GPIO_B"; + + hog_input_1 { + gpio-hog; + input; + gpios = <1 GPIO_ACTIVE_HIGH>; + }; + hog_output_3 { + gpio-hog; + output-high; + output-mode; + output-value = <1>; + gpios = <3 GPIO_ACTIVE_HIGH>; + }; + }; +#endif + + scmi_pinctrl_gpios: gpios-pins { + gpio0 { + pins = "P5"; + function = "GPIO"; + bias-pull-up; + // input-disable; + input-mode = <0>; + }; + gpio1 { + pins = "P6"; + function = "GPIO"; + // output-high; + output-mode; + output-value = <1>; + drive-open-drain; + }; + gpio2 { + pinmux = <SANDBOX_PINMUX(7, SANDBOX_PINMUX_GPIO)>; + bias-pull-down; + // input-enable; + input-mode; + }; + gpio3 { + pinmux = <SANDBOX_PINMUX(8, SANDBOX_PINMUX_GPIO)>; + bias-disable; + }; + }; + + scmi_pinctrl_i2c: i2c-pins { + groups { + groups = "I2C_UART"; + function = "I2C"; + }; + + pins { + pins = "P0", "P1"; + drive-open-drain; + }; + }; + + scmi_pinctrl_i2s: i2s-pins { + groups = "SPI_I2S"; + function = "I2S"; + }; + + scmi_pinctrl_spi: spi-pins { + groups = "SPI_I2S"; + function = "SPI"; + + cs { + pinmux = <SANDBOX_PINMUX(5, SANDBOX_PINMUX_CS)>, + <SANDBOX_PINMUX(6, SANDBOX_PINMUX_CS)>; + }; + }; + }; }; };
+#if 1 + scmi_pinctrl_gpio_a: scmi_gpios { + compatible = "arm,scmi-gpio-generic"; + gpio-controller; + #gpio-cells = <2>; + gpio-bank-name = "scmi_gpios"; + gpio-ranges = <&pinctrl_scmi 0 5 4>, + <&pinctrl_scmi 4 0 0>; + gpio-ranges-group-names = "", + "GPIO_B"; + + hog_input_1 { + gpio-hog; + input; + gpios = <1 GPIO_ACTIVE_HIGH>; + }; + hog_output_3 { + gpio-hog; + output-high; + output-mode; + output-value = <1>; + gpios = <3 GPIO_ACTIVE_HIGH>; + }; + }; +#endif + fpga { compatible = "sandbox,fpga"; }; diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c index 27d17809be43..d5ff8a2b1c79 100644 --- a/drivers/firmware/scmi/sandbox-scmi_agent.c +++ b/drivers/firmware/scmi/sandbox-scmi_agent.c @@ -14,6 +14,7 @@ #include <asm/io.h> #include <asm/scmi_test.h> #include <dm/device_compat.h> +#include <dt-bindings/pinctrl/sandbox-pinmux.h> #include <linux/bitfield.h> #include <linux/kernel.h>
@@ -43,8 +44,11 @@ #define SANDBOX_SCMI_AGENT_NAME "OSPM" #define SANDBOX_SCMI_PLATFORM_NAME "platform"
+#define SANDBOX_SCMI_PIN_CONTROL_PROTOCOL_VERSION SCMI_PIN_CONTROL_PROTOCOL_VERSION + static u8 protocols[] = { SCMI_PROTOCOL_ID_CLOCK, + SCMI_PROTOCOL_ID_PIN_CONTROL, SCMI_PROTOCOL_ID_RESET_DOMAIN, SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, }; @@ -796,6 +800,696 @@ static int sandbox_scmi_voltd_level_get(struct udevice *dev, return 0; }
+/* Pin control protocol */ + +/* + * This driver emulates a pin controller with the following rules: + * - The pinctrl config for each pin must be set individually + * - The first two pins (P0-P1) must be muxed as a group + * - The next three pins (P2-P4) must be muxed as a group + * - The next four pins (P5-P8) must be muxed individually + * - The last two pins (P9-P10) must be fixed as a GPIO group only + + P0: "UART TX", "I2C SCL" + P1: "UART RX", "I2C SDA" + P2: "SPI SCLK", "I2S SCK" + P3: "SPI MOSI", "I2S SD" + P4: "SPI MISO", "I2S WS" + P5: "GPIO0", "SPI CS0" + P6: "GPIO1", "SPI CS1" + P7: "GPIO2", "PWM0" + P8: "GPIO3", "PWM1" + P9: "GPIO_B" + P10: "GPIO_B" + + */ + +static const char * const sandbox_pins[] = { +#define PIN(x) \ + [x] = "P" #x + PIN(0), + PIN(1), + PIN(2), + PIN(3), + PIN(4), + PIN(5), + PIN(6), + PIN(7), + PIN(8), + PIN(9), + PIN(10), +#undef PIN +}; + +static unsigned int sandbox_pin_functions[9]; +static u32 sandbox_pin_states[9][SCMI_PINCTRL_CONFIG_RESERVED]; + +#define SANDBOX_GROUP_I2C_UART 0 +#define SANDBOX_GROUP_SPI_I2S 1 +#define SANDBOX_GROUP_GPIO_B 2 + +static const char * const sandbox_groups[] = { + /* P0-P1 */ + [SANDBOX_GROUP_I2C_UART] = "I2C_UART", + /* P2-P4 */ + [SANDBOX_GROUP_SPI_I2S] = "SPI_I2S", + /* P9-P10 */ + [SANDBOX_GROUP_GPIO_B] = "GPIO_B", +}; + +static const char * const sandbox_functions[] = { +#define FUNC(id) \ + [SANDBOX_PINMUX_##id] = #id + FUNC(UART), + FUNC(I2C), + FUNC(SPI), + FUNC(I2S), + FUNC(GPIO), + FUNC(CS), + FUNC(PWM), + /* FUNC(GPIO_B) */ +#undef FUNC +}; + +static int sandbox_scmi_pinctrl_protocol_version(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_protocol_version_out *out = NULL; + + if (!msg->out_msg || msg->out_msg_sz < sizeof(*out)) + return -EINVAL; + + out = (struct scmi_protocol_version_out *)msg->out_msg; + out->version = SANDBOX_SCMI_PIN_CONTROL_PROTOCOL_VERSION; + out->status = SCMI_SUCCESS; + + return 0; +} + +static int sandbox_scmi_pinctrl_protocol_attrs(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_protocol_attrs_out *out = NULL; + + if (!msg->out_msg || msg->out_msg_sz < sizeof(*out)) + return -EINVAL; + + out = (struct scmi_pinctrl_protocol_attrs_out *)msg->out_msg; + out->attributes_low = (ARRAY_SIZE(sandbox_groups) << 16) + + ARRAY_SIZE(sandbox_pins); + out->attributes_high = ARRAY_SIZE(sandbox_functions); + out->status = SCMI_SUCCESS; + + return 0; +} + +static int sandbox_scmi_pinctrl_attrs(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_attrs_in *in; + struct scmi_pinctrl_attrs_out *out; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*out)) + return -EINVAL; + + in = (struct scmi_pinctrl_attrs_in *)msg->in_msg; + out = (struct scmi_pinctrl_attrs_out *)msg->out_msg; + + /* + * Currently all pins have a name with less than 16 characters + * (SCMI_PINCTRL_NAME_LENGTH_MAX). + */ + switch (SCMI_PINCTRL_TYPE(in->flags)) { + case SCMI_PINCTRL_TYPE_PIN: + if (in->id < ARRAY_SIZE(sandbox_pins)) { + strcpy(out->name, sandbox_pins[in->id]); + out->attributes = 0; + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_GROUP: + if (in->id < ARRAY_SIZE(sandbox_groups)) { + strcpy(out->name, sandbox_groups[in->id]); + out->attributes = in->id ? 3 : 2; + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_FUNCTION: + if (in->id < ARRAY_SIZE(sandbox_functions)) { + strcpy(out->name, sandbox_functions[in->id]); + if (in->id == 4) /* UART */ + out->attributes = 4; + else if (in->id == 5 || in->id == 6) /* CS or PWM */ + out->attributes = 2; + else + out->attributes = 1; + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + default: + out->status = SCMI_INVALID_PARAMETERS; + goto err; + } + + out->status = SCMI_SUCCESS; + +err: + return 0; +} + +static int sandbox_scmi_pinctrl_list_assocs(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_list_assocs_in *in; + struct scmi_pinctrl_list_assocs_out *out; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*out)) + return -EINVAL; + + in = (struct scmi_pinctrl_list_assocs_in *)msg->in_msg; + out = (struct scmi_pinctrl_list_assocs_out *)msg->out_msg; + + /* + * UART -> GROUP0 + * I2C -> GROUP0 + * SPI -> GROUP1 + * I2S -> GROUP1 + * GPIO -> PIN5, 6, 7, 8 + * CS -> PIN5, 6 + * PWM -> PIN7, 8 + * (GPIO_B -> GROUP2) + */ + switch (SCMI_PINCTRL_TYPE(in->flags)) { + case SCMI_PINCTRL_TYPE_GROUP: + if (in->id == SANDBOX_GROUP_I2C_UART) { + if (in->index == 0) { + out->array[0] = 0; + out->array[1] = 1; + out->flags = 2; + } else if (in->index == 1) { + out->array[0] = 0; + out->flags = 1; + } else { + out->status = SCMI_OUT_OF_RANGE; + goto err; + } + } else if (in->id == SANDBOX_GROUP_SPI_I2S) { + if (in->index == 0) { + out->array[0] = 2; + out->array[1] = 3; + out->array[2] = 4; + out->flags = 3; + } else if (in->index == 1) { + out->array[0] = 3; + out->array[1] = 4; + out->flags = 2; + } else if (in->index == 2) { + out->array[0] = 4; + out->flags = 1; + } else { + out->status = SCMI_OUT_OF_RANGE; + goto err; + } + } else if (in->id == SANDBOX_GROUP_GPIO_B) { + if (in->index == 0) { + out->array[0] = 9; + out->array[1] = 10; + out->flags = 2; + } else if (in->index == 1) { + out->array[0] = 10; + out->flags = 1; + } else { + out->status = SCMI_OUT_OF_RANGE; + goto err; + } + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_FUNCTION: + if (in->id == SANDBOX_PINMUX_UART) { + if (in->index > 0) { + out->status = SCMI_OUT_OF_RANGE; + goto err; + } + out->array[0] = SANDBOX_GROUP_I2C_UART; + out->flags = 1; + } else if (in->id == SANDBOX_PINMUX_I2C) { + if (in->index > 0) { + out->status = SCMI_OUT_OF_RANGE; + goto err; + } + out->array[0] = SANDBOX_GROUP_I2C_UART; + out->flags = 1; + } else if (in->id == SANDBOX_PINMUX_SPI) { + if (in->index > 0) { + out->status = SCMI_OUT_OF_RANGE; + goto err; + } + out->array[0] = SANDBOX_GROUP_SPI_I2S; + out->flags = 1; + } else if (in->id == SANDBOX_PINMUX_I2S) { + if (in->index > 0) { + out->status = SCMI_OUT_OF_RANGE; + goto err; + } + out->array[0] = SANDBOX_GROUP_SPI_I2S; + out->flags = 1; + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + default: + out->status = SCMI_INVALID_PARAMETERS; + goto err; + } + + out->status = SCMI_SUCCESS; + +err: + return 0; +} + +static void copy_config(struct scmi_pin_entry *configs, u32 selector, int skip, + u32 *num, u32 *remaining) +{ + int max_num, i; + + if ((skip + SCMI_PINCTRL_CONFIG_ENTRY_MAX) + > SCMI_PINCTRL_CONFIG_RESERVED) + max_num = SCMI_PINCTRL_CONFIG_RESERVED - skip; + else + max_num = SCMI_PINCTRL_CONFIG_ENTRY_MAX; + + /* TODO: eliminate disabled properties? */ + for (i = 0; i < max_num; i++) { + configs[i].type = skip + i; + configs[i].value = sandbox_pin_states[selector][skip + i]; + } + + *num = max_num; + *remaining = SCMI_PINCTRL_CONFIG_RESERVED - (skip + max_num); +} + +static int sandbox_scmi_pinctrl_config_get(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_config_get_in *in; + struct scmi_pinctrl_config_get_out *out; + u32 type, num, remaining; + int all_configs, skip; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*out)) + return -EINVAL; + + in = (struct scmi_pinctrl_config_get_in *)msg->in_msg; + out = (struct scmi_pinctrl_config_get_out *)msg->out_msg; + + all_configs = in->attributes & SCMI_PINCTRL_CONFIG_GET_ALL; + skip = SCMI_PINCTRL_CONFIG_GET_SKIP(in->attributes); + type = SCMI_PINCTRL_CONFIG_GET_TYPE(in->attributes); + if (type >= SCMI_PINCTRL_CONFIG_RESERVED) { + out->status = SCMI_INVALID_PARAMETERS; + goto err; + } + + switch (SCMI_PINCTRL_CONFIG_GET_PINCTRL_TYPE(in->attributes)) { + case SCMI_PINCTRL_TYPE_PIN: + if (in->id < ARRAY_SIZE(sandbox_pins)) { + if (all_configs) { + if (skip >= SCMI_PINCTRL_CONFIG_RESERVED) { + out->status = SCMI_INVALID_PARAMETERS; + goto err; + } + num = 0; /* avoid compiler warning */ + remaining = 0; + copy_config(&out->configs[0], in->id, skip, + &num, &remaining); + } else { + out->configs[0].type = type; + out->configs[0].value = + sandbox_pin_states[in->id][type]; + num = 1; + remaining = 0; + } + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + out->num_configs = + SCMI_PINCTRL_CONFIG_GET_NUM_CONFIGS(remaining, num); + break; + case SCMI_PINCTRL_TYPE_GROUP: + if (in->id < ARRAY_SIZE(sandbox_groups)) { + /* TODO */; + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + default: + out->status = SCMI_INVALID_PARAMETERS; + goto err; + } + + out->status = SCMI_SUCCESS; + +err: + return 0; +} + +static int sandbox_scmi_pinctrl_config_set(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_config_set_in *in; + u32 *status; + int i, num; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*status)) + return -EINVAL; + + in = (struct scmi_pinctrl_config_set_in *)msg->in_msg; + status = (u32 *)msg->out_msg; + + num = SCMI_PINCTRL_CONFIG_SET_NUM_CONFIGS(in->attributes); + if (num > SCMI_PINCTRL_CONFIG_ENTRY_MAX) { + *status = SCMI_PROTOCOL_ERROR; + goto err; + } + + switch (SCMI_PINCTRL_CONFIG_SET_PINCTRL_TYPE(in->attributes)) { + case SCMI_PINCTRL_TYPE_PIN: + if (in->id < ARRAY_SIZE(sandbox_pins)) { + /* TODO: check value range */ + for (i = 0; i < num; i++) + sandbox_pin_states[in->id][in->configs[i].type] + = in->configs[i].value; + } else { + *status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_GROUP: + /* TODO: check value range */ + if (!in->id) { + for (i = 0; i < num; i++) { + sandbox_pin_states[0][in->configs[i].type] = + in->configs[i].value; + sandbox_pin_states[1][in->configs[i].type] = + in->configs[i].value; + } + } else if (in->id == 1) { + for (i = 0; i < num; i++) { + sandbox_pin_states[2][in->configs[i].type] = + in->configs[i].value; + sandbox_pin_states[3][in->configs[i].type] = + in->configs[i].value; + sandbox_pin_states[4][in->configs[i].type] = + in->configs[i].value; + } + } else if (in->id == 2) { + for (i = 0; i < num; i++) { + sandbox_pin_states[9][in->configs[i].type] = + in->configs[i].value; + sandbox_pin_states[10][in->configs[i].type] = + in->configs[i].value; + } + } else { + *status = SCMI_NOT_FOUND; + goto err; + } + break; + default: + *status = SCMI_INVALID_PARAMETERS; + goto err; + } + + *status = SCMI_SUCCESS; +err: + return 0; +} + +static int sandbox_scmi_pinctrl_function_select(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_function_select_in *in; + u32 *status; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*status)) + return -EINVAL; + + in = (struct scmi_pinctrl_function_select_in *)msg->in_msg; + status = (u32 *)msg->out_msg; + + switch (SCMI_PINCTRL_TYPE(in->flags)) { + case SCMI_PINCTRL_TYPE_PIN: + if (in->id == 5 || in->id == 6) { + if (in->function_id == SANDBOX_PINMUX_GPIO || + in->function_id == SANDBOX_PINMUX_CS) { + sandbox_pin_functions[in->id] = in->function_id; + *status = SCMI_SUCCESS; + } else { + *status = SCMI_NOT_SUPPORTED; + } + } else if (in->id == 7 || in->id == 8) { + if (in->function_id == SANDBOX_PINMUX_GPIO || + in->function_id == SANDBOX_PINMUX_PWM) { + sandbox_pin_functions[in->id] = in->function_id; + *status = SCMI_SUCCESS; + } else { + *status = SCMI_NOT_SUPPORTED; + } + } + break; + case SCMI_PINCTRL_TYPE_GROUP: + if (in->id == SANDBOX_GROUP_I2C_UART) { + if (in->function_id == SANDBOX_PINMUX_UART || + in->function_id == SANDBOX_PINMUX_I2C) { + sandbox_pin_functions[0] = in->function_id; + sandbox_pin_functions[1] = in->function_id; + *status = SCMI_SUCCESS; + } else { + *status = SCMI_NOT_SUPPORTED; + } + } else if (in->id == SANDBOX_GROUP_SPI_I2S) { + if (in->function_id == SANDBOX_PINMUX_SPI || + in->function_id == SANDBOX_PINMUX_I2S) { + sandbox_pin_functions[2] = in->function_id; + sandbox_pin_functions[3] = in->function_id; + sandbox_pin_functions[4] = in->function_id; + *status = SCMI_SUCCESS; + } else { + *status = SCMI_NOT_SUPPORTED; + } + } + break; + default: + *status = SCMI_INVALID_PARAMETERS; + goto err; + } + + *status = SCMI_SUCCESS; +err: + return 0; +} + +static int sandbox_scmi_pinctrl_request(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_request_in *in; + u32 *status; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*status)) + return -EINVAL; + + in = (struct scmi_pinctrl_request_in *)msg->in_msg; + status = (u32 *)msg->out_msg; + + /* + * No other agent, so always accept the request + */ + switch (SCMI_PINCTRL_TYPE(in->flags)) { + case SCMI_PINCTRL_TYPE_PIN: + if (in->id >= ARRAY_SIZE(sandbox_pins)) { + *status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_GROUP: + if (in->id >= ARRAY_SIZE(sandbox_groups)) { + *status = SCMI_NOT_FOUND; + goto err; + } + break; + default: + *status = SCMI_INVALID_PARAMETERS; + goto err; + } + + *status = SCMI_SUCCESS; +err: + return 0; +} + +static int sandbox_scmi_pinctrl_release(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_release_in *in; + u32 *status; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*status)) + return -EINVAL; + + in = (struct scmi_pinctrl_release_in *)msg->in_msg; + status = (u32 *)msg->out_msg; + + /* + * No other agent, so always accept the release + */ + switch (SCMI_PINCTRL_TYPE(in->flags)) { + case SCMI_PINCTRL_TYPE_PIN: + if (in->id >= ARRAY_SIZE(sandbox_pins)) { + *status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_GROUP: + if (in->id >= ARRAY_SIZE(sandbox_groups)) { + *status = SCMI_NOT_FOUND; + goto err; + } + break; + default: + *status = SCMI_INVALID_PARAMETERS; + goto err; + } + + *status = SCMI_SUCCESS; +err: + return 0; +} + +static int sandbox_scmi_pinctrl_name_get(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_name_get_in *in; + struct scmi_pinctrl_name_get_out *out; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*out)) + return -EINVAL; + + in = (struct scmi_pinctrl_name_get_in *)msg->in_msg; + out = (struct scmi_pinctrl_name_get_out *)msg->out_msg; + + /* + * Currently all pins have a name with less than 64 characters + */ + switch (SCMI_PINCTRL_TYPE(in->flags)) { + case SCMI_PINCTRL_TYPE_PIN: + if (in->id < ARRAY_SIZE(sandbox_pins)) { + strcpy(out->name, sandbox_pins[in->id]); + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_GROUP: + if (in->id < ARRAY_SIZE(sandbox_groups)) { + strcpy(out->name, sandbox_groups[in->id]); + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_FUNCTION: + if (in->id < ARRAY_SIZE(sandbox_functions)) { + strcpy(out->name, sandbox_functions[in->id]); + } else { + out->status = SCMI_NOT_FOUND; + goto err; + } + break; + default: + out->status = SCMI_INVALID_PARAMETERS; + goto err; + } + + out->flags = 0; + out->status = SCMI_SUCCESS; + +err: + return 0; +} + +static int sandbox_scmi_pinctrl_set_permissions(struct udevice *dev, + struct scmi_msg *msg) +{ + struct scmi_pinctrl_set_permissions_in *in; + u32 *status; + + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) || + !msg->out_msg || msg->out_msg_sz < sizeof(*status)) + return -EINVAL; + + in = (struct scmi_pinctrl_set_permissions_in *)msg->in_msg; + status = (u32 *)msg->out_msg; + + if (in->agent_id != 1) { + *status = SCMI_NOT_FOUND; + goto err; + } + + switch (SCMI_PINCTRL_TYPE(in->flags)) { + case SCMI_PINCTRL_TYPE_PIN: + if (in->id < ARRAY_SIZE(sandbox_pins)) { + if (in->flags & SCMI_PINCTRL_PERMISSION) + *status = SCMI_SUCCESS; + else + /* unset not allowed */ + *status = SCMI_DENIED; + } else { + *status = SCMI_NOT_FOUND; + goto err; + } + break; + case SCMI_PINCTRL_TYPE_GROUP: + if (in->id < ARRAY_SIZE(sandbox_groups)) { + if (in->flags & SCMI_PINCTRL_PERMISSION) + *status = SCMI_SUCCESS; + else + /* unset not allowed */ + *status = SCMI_DENIED; + } else { + *status = SCMI_NOT_FOUND; + goto err; + } + break; + default: + *status = SCMI_INVALID_PARAMETERS; + goto err; + } + + *status = SCMI_SUCCESS; +err: + return 0; +} + static int sandbox_scmi_test_process_msg(struct udevice *dev, struct scmi_channel *channel, struct scmi_msg *msg) @@ -847,6 +1541,34 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev, break; } break; + case SCMI_PROTOCOL_ID_PIN_CONTROL: + switch (msg->message_id) { + case SCMI_PROTOCOL_VERSION: + return sandbox_scmi_pinctrl_protocol_version(dev, msg); + case SCMI_PROTOCOL_ATTRIBUTES: + return sandbox_scmi_pinctrl_protocol_attrs(dev, msg); + case SCMI_PINCTRL_ATTRIBUTES: + return sandbox_scmi_pinctrl_attrs(dev, msg); + case SCMI_PINCTRL_LIST_ASSOCIATIONS: + return sandbox_scmi_pinctrl_list_assocs(dev, msg); + case SCMI_PINCTRL_CONFIG_GET: + return sandbox_scmi_pinctrl_config_get(dev, msg); + case SCMI_PINCTRL_CONFIG_SET: + return sandbox_scmi_pinctrl_config_set(dev, msg); + case SCMI_PINCTRL_FUNCTION_SELECT: + return sandbox_scmi_pinctrl_function_select(dev, msg); + case SCMI_PINCTRL_REQUEST: + return sandbox_scmi_pinctrl_request(dev, msg); + case SCMI_PINCTRL_RELEASE: + return sandbox_scmi_pinctrl_release(dev, msg); + case SCMI_PINCTRL_NAME_GET: + return sandbox_scmi_pinctrl_name_get(dev, msg); + case SCMI_PINCTRL_SET_PERMISSIONS: + return sandbox_scmi_pinctrl_set_permissions(dev, msg); + default: + break; + } + break; case SCMI_PROTOCOL_ID_RESET_DOMAIN: switch (msg->message_id) { case SCMI_RESET_DOMAIN_ATTRIBUTES:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
With this patch, sandbox SCMI agent can handle pinctrl protocol. This feature is used in an unit test for SCMI pinctrl.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
arch/sandbox/dts/test.dts | 115 ++++ drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +++++++++++++++++++++ 2 files changed, 837 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index dc0bfdfb6e4b..d2ddea801995 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -723,9 +723,124 @@ }; }; };
pinctrl_scmi: protocol@19 {reg = <0x19>;pinctrl-names = "default","alternate";pinctrl-0 = <&scmi_pinctrl_gpios>, <&scmi_pinctrl_i2s>;pinctrl-1 = <&scmi_pinctrl_spi>, <&scmi_pinctrl_i2c>;+#if 0
Are these alternatives that you are testing?
The bindings look OK to me - is this how it is done in Linux?
Regards, Simon
On Sun, Sep 10, 2023 at 01:13:28PM -0600, Simon Glass wrote:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
With this patch, sandbox SCMI agent can handle pinctrl protocol. This feature is used in an unit test for SCMI pinctrl.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
arch/sandbox/dts/test.dts | 115 ++++ drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +++++++++++++++++++++ 2 files changed, 837 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index dc0bfdfb6e4b..d2ddea801995 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -723,9 +723,124 @@ }; }; };
pinctrl_scmi: protocol@19 {reg = <0x19>;pinctrl-names = "default","alternate";pinctrl-0 = <&scmi_pinctrl_gpios>, <&scmi_pinctrl_i2s>;pinctrl-1 = <&scmi_pinctrl_spi>, <&scmi_pinctrl_i2c>;+#if 0
Are these alternatives that you are testing?
I should have had more explanation. Yes, as I mentioned in the cover letter (and patch#4), there are two alternatives for defining SCMI pinctrl based gpio devices. Actually, this "#if" corresponds to the case (B) where gpio node is located under scmi node.
Since I didn't come up with any good way to switch two cases dynamically in the test, I had modified this option manually. (I left two "#if" here as the patch was an RFC, any way.)
Thanks, -Takahiro Akashi
The bindings look OK to me - is this how it is done in Linux?
Regards, Simon
In this test, SCMI-based pinmux feature is exercised.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- test/dm/scmi.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
diff --git a/test/dm/scmi.c b/test/dm/scmi.c index 423b6ef70b29..ca5a2e9c781e 100644 --- a/test/dm/scmi.c +++ b/test/dm/scmi.c @@ -21,6 +21,7 @@ #include <scmi_protocols.h> #include <asm/scmi_test.h> #include <dm/device-internal.h> +#include <dm/pinctrl.h> #include <dm/test.h> #include <linux/kconfig.h> #include <power/regulator.h> @@ -395,3 +396,64 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts) return release_sandbox_scmi_test_devices(uts, dev); } DM_TEST(dm_test_scmi_voltage_domains, UT_TESTF_SCAN_FDT); + +/* + * This part is derived from test/dm/pinmux.c, So + * + * Copyright (C) 2020 Sean Anderson seanga2@gmail.com + */ + +static char buf[64]; +#define test_muxing(selector, expected) do { \ + ut_assertok(pinctrl_get_pin_muxing(dev, selector, buf, sizeof(buf))); \ + ut_asserteq_str(expected, (char *)&buf); \ +} while (0) + +#define test_name(selector, expected) do { \ + ut_assertok(pinctrl_get_pin_name(dev, selector, buf, sizeof(buf))); \ + ut_asserteq_str(expected, (char *)&buf); \ +} while (0) + +static int dm_test_scmi_pinmux(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(uclass_get_device_by_name(UCLASS_PINCTRL, "protocol@19", + &dev)); + ut_assertok(pinctrl_select_state(dev, "default")); + test_muxing(0, "<unknown>"); + test_muxing(1, "<unknown>"); + test_muxing(2, "I2S."); + test_muxing(3, "I2S."); + test_muxing(4, "I2S."); + test_muxing(5, "GPIO bias-pull-up."); + test_muxing(6, "GPIO drive-open-drain output-mode output-value."); + test_muxing(7, "GPIO bias-pull-down input-mode."); + test_muxing(8, "GPIO bias-disable."); + + ut_assertok(pinctrl_select_state(dev, "alternate")); + test_muxing(0, "I2C drive-open-drain."); + test_muxing(1, "I2C drive-open-drain."); + test_muxing(2, "SPI."); + test_muxing(3, "SPI."); + test_muxing(4, "SPI."); + test_muxing(5, "CS bias-pull-up."); + test_muxing(6, "CS drive-open-drain output-mode output-value."); + test_muxing(7, "GPIO bias-pull-down input-mode."); + test_muxing(8, "GPIO bias-disable."); + + ut_assertok(pinctrl_select_state(dev, "0")); + test_muxing(0, "I2C drive-open-drain."); + test_muxing(1, "I2C drive-open-drain."); + test_muxing(2, "I2S."); + test_muxing(3, "I2S."); + test_muxing(4, "I2S."); + test_muxing(5, "GPIO bias-pull-up."); + test_muxing(6, "GPIO drive-open-drain output-mode output-value."); + test_muxing(7, "GPIO bias-pull-down input-mode."); + test_muxing(8, "GPIO bias-disable."); + + return 0; +} + +DM_TEST(dm_test_scmi_pinmux, UT_TESTF_SCAN_FDT);
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
In this test, SCMI-based pinmux feature is exercised.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
test/dm/scmi.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
diff --git a/test/dm/scmi.c b/test/dm/scmi.c index 423b6ef70b29..ca5a2e9c781e 100644 --- a/test/dm/scmi.c +++ b/test/dm/scmi.c @@ -21,6 +21,7 @@ #include <scmi_protocols.h> #include <asm/scmi_test.h> #include <dm/device-internal.h> +#include <dm/pinctrl.h> #include <dm/test.h> #include <linux/kconfig.h> #include <power/regulator.h> @@ -395,3 +396,64 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts) return release_sandbox_scmi_test_devices(uts, dev); } DM_TEST(dm_test_scmi_voltage_domains, UT_TESTF_SCAN_FDT);
+/*
- This part is derived from test/dm/pinmux.c, So
- Copyright (C) 2020 Sean Anderson seanga2@gmail.com
- */
+static char buf[64]; +#define test_muxing(selector, expected) do { \
ut_assertok(pinctrl_get_pin_muxing(dev, selector, buf, sizeof(buf))); \ut_asserteq_str(expected, (char *)&buf); \
Can you instead make this a function?
+} while (0)
+#define test_name(selector, expected) do { \
ut_assertok(pinctrl_get_pin_name(dev, selector, buf, sizeof(buf))); \ut_asserteq_str(expected, (char *)&buf); \+} while (0)
+static int dm_test_scmi_pinmux(struct unit_test_state *uts) +{
struct udevice *dev;ut_assertok(uclass_get_device_by_name(UCLASS_PINCTRL, "protocol@19",&dev));ut_assertok(pinctrl_select_state(dev, "default"));test_muxing(0, "<unknown>");
ut_assertok(check_muxing(uts, ...));
test_muxing(1, "<unknown>");test_muxing(2, "I2S.");test_muxing(3, "I2S.");test_muxing(4, "I2S.");test_muxing(5, "GPIO bias-pull-up.");test_muxing(6, "GPIO drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");ut_assertok(pinctrl_select_state(dev, "alternate"));test_muxing(0, "I2C drive-open-drain.");test_muxing(1, "I2C drive-open-drain.");test_muxing(2, "SPI.");test_muxing(3, "SPI.");test_muxing(4, "SPI.");test_muxing(5, "CS bias-pull-up.");test_muxing(6, "CS drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");ut_assertok(pinctrl_select_state(dev, "0"));test_muxing(0, "I2C drive-open-drain.");test_muxing(1, "I2C drive-open-drain.");test_muxing(2, "I2S.");test_muxing(3, "I2S.");test_muxing(4, "I2S.");test_muxing(5, "GPIO bias-pull-up.");test_muxing(6, "GPIO drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");return 0;+}
+DM_TEST(dm_test_scmi_pinmux, UT_TESTF_SCAN_FDT);
2.34.1
Regards, Simon
On Sun, Sep 10, 2023 at 01:13:30PM -0600, Simon Glass wrote:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
In this test, SCMI-based pinmux feature is exercised.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
test/dm/scmi.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
diff --git a/test/dm/scmi.c b/test/dm/scmi.c index 423b6ef70b29..ca5a2e9c781e 100644 --- a/test/dm/scmi.c +++ b/test/dm/scmi.c @@ -21,6 +21,7 @@ #include <scmi_protocols.h> #include <asm/scmi_test.h> #include <dm/device-internal.h> +#include <dm/pinctrl.h> #include <dm/test.h> #include <linux/kconfig.h> #include <power/regulator.h> @@ -395,3 +396,64 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts) return release_sandbox_scmi_test_devices(uts, dev); } DM_TEST(dm_test_scmi_voltage_domains, UT_TESTF_SCAN_FDT);
+/*
- This part is derived from test/dm/pinmux.c, So
- Copyright (C) 2020 Sean Anderson seanga2@gmail.com
- */
+static char buf[64]; +#define test_muxing(selector, expected) do { \
ut_assertok(pinctrl_get_pin_muxing(dev, selector, buf, sizeof(buf))); \ut_asserteq_str(expected, (char *)&buf); \Can you instead make this a function?
Sure, but please note that the whole scheme was borrowed from test/dm/pinmux.c.
+} while (0)
+#define test_name(selector, expected) do { \
ut_assertok(pinctrl_get_pin_name(dev, selector, buf, sizeof(buf))); \ut_asserteq_str(expected, (char *)&buf); \+} while (0)
+static int dm_test_scmi_pinmux(struct unit_test_state *uts) +{
struct udevice *dev;ut_assertok(uclass_get_device_by_name(UCLASS_PINCTRL, "protocol@19",&dev));ut_assertok(pinctrl_select_state(dev, "default"));test_muxing(0, "<unknown>");ut_assertok(check_muxing(uts, ...));
Assertion for
test_muxing(1, "<unknown>");test_muxing(2, "I2S.");test_muxing(3, "I2S.");test_muxing(4, "I2S.");test_muxing(5, "GPIO bias-pull-up.");test_muxing(6, "GPIO drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");
this chunk of code? It is not clear to me what is the advantage, though.
-Takahiro Akashi
ut_assertok(pinctrl_select_state(dev, "alternate"));test_muxing(0, "I2C drive-open-drain.");test_muxing(1, "I2C drive-open-drain.");test_muxing(2, "SPI.");test_muxing(3, "SPI.");test_muxing(4, "SPI.");test_muxing(5, "CS bias-pull-up.");test_muxing(6, "CS drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");ut_assertok(pinctrl_select_state(dev, "0"));test_muxing(0, "I2C drive-open-drain.");test_muxing(1, "I2C drive-open-drain.");test_muxing(2, "I2S.");test_muxing(3, "I2S.");test_muxing(4, "I2S.");test_muxing(5, "GPIO bias-pull-up.");test_muxing(6, "GPIO drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");return 0;+}
+DM_TEST(dm_test_scmi_pinmux, UT_TESTF_SCAN_FDT);
2.34.1
Regards, Simon
Hi,
On Sun, 10 Sept 2023 at 23:47, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
On Sun, Sep 10, 2023 at 01:13:30PM -0600, Simon Glass wrote:
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
In this test, SCMI-based pinmux feature is exercised.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
test/dm/scmi.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
diff --git a/test/dm/scmi.c b/test/dm/scmi.c index 423b6ef70b29..ca5a2e9c781e 100644 --- a/test/dm/scmi.c +++ b/test/dm/scmi.c @@ -21,6 +21,7 @@ #include <scmi_protocols.h> #include <asm/scmi_test.h> #include <dm/device-internal.h> +#include <dm/pinctrl.h> #include <dm/test.h> #include <linux/kconfig.h> #include <power/regulator.h> @@ -395,3 +396,64 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts) return release_sandbox_scmi_test_devices(uts, dev); } DM_TEST(dm_test_scmi_voltage_domains, UT_TESTF_SCAN_FDT);
+/*
- This part is derived from test/dm/pinmux.c, So
- Copyright (C) 2020 Sean Anderson seanga2@gmail.com
- */
+static char buf[64]; +#define test_muxing(selector, expected) do { \
ut_assertok(pinctrl_get_pin_muxing(dev, selector, buf, sizeof(buf))); \ut_asserteq_str(expected, (char *)&buf); \Can you instead make this a function?
Sure, but please note that the whole scheme was borrowed from test/dm/pinmux.c.
I'm not sure if it is needed there, either. It is pretty ugly.
+} while (0)
+#define test_name(selector, expected) do { \
ut_assertok(pinctrl_get_pin_name(dev, selector, buf, sizeof(buf))); \ut_asserteq_str(expected, (char *)&buf); \+} while (0)
+static int dm_test_scmi_pinmux(struct unit_test_state *uts) +{
struct udevice *dev;ut_assertok(uclass_get_device_by_name(UCLASS_PINCTRL, "protocol@19",&dev));ut_assertok(pinctrl_select_state(dev, "default"));test_muxing(0, "<unknown>");ut_assertok(check_muxing(uts, ...));
Assertion for
test_muxing(1, "<unknown>");test_muxing(2, "I2S.");test_muxing(3, "I2S.");test_muxing(4, "I2S.");test_muxing(5, "GPIO bias-pull-up.");test_muxing(6, "GPIO drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");this chunk of code? It is not clear to me what is the advantage, though.
Avoiding macros, which are not needed.
-Takahiro Akashi
ut_assertok(pinctrl_select_state(dev, "alternate"));test_muxing(0, "I2C drive-open-drain.");test_muxing(1, "I2C drive-open-drain.");test_muxing(2, "SPI.");test_muxing(3, "SPI.");test_muxing(4, "SPI.");test_muxing(5, "CS bias-pull-up.");test_muxing(6, "CS drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");ut_assertok(pinctrl_select_state(dev, "0"));test_muxing(0, "I2C drive-open-drain.");test_muxing(1, "I2C drive-open-drain.");test_muxing(2, "I2S.");test_muxing(3, "I2S.");test_muxing(4, "I2S.");test_muxing(5, "GPIO bias-pull-up.");test_muxing(6, "GPIO drive-open-drain output-mode output-value.");test_muxing(7, "GPIO bias-pull-down input-mode.");test_muxing(8, "GPIO bias-disable.");return 0;+}
+DM_TEST(dm_test_scmi_pinmux, UT_TESTF_SCAN_FDT);
2.34.1
Regards, Simon
Regards, Simon
Adding Peng Fan, who is working on scmi/pinctrl support for i.MX9:
https://lore.kernel.org/all/ZO9GLG5tQynYyAvR@pluto/T/
On Tue, Sep 5, 2023 at 11:41 PM AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This is an RFC and meant to get feedback from other developers as
- the specification (pinctrl part) is still in a draft
- the upstream patch for linux, including dt bindings, is still WIP
- I'm not confident the drivers are generic enough to cover most HWs
- The tests ("ut") doesn't cover all the features yet
This patch series allows users to access SCMI pin control protocol provided by SCMI server (platform). See SCMI specification document v3.2 beta 2[1] for more details about SCMI pin control protocol.
The implementation consists of two layers:
- basic helper functions for SCMI pin control protocol in drivers/firmware/scmi/pinctrl.c (patch#2)
- DM-compliant pinctrl/gpio drivers, which utilizes the helper functions, in drivers/pinctrl/pinctrl-scmi.c (patch#3,#4)
[1] https://developer.arm.com/documentation/den0056/e/?lang=en
DT bindings
Upstream pinctrl patch for linux defines the bindings in [2] though it doesn't say much. I expect that my implementation basically complies with U-Boot's generic bindings described in [3], but not all the features are verified.
As for gpio, unless you hard-code pin assignments directly in a device driver, my implementation allows the following alternatives in DT. Either way, we may need an additional binding description for gpio.
(A) scmi { ... // other protocols scmi_pinctrl: protocol@19 { // Pin control protocol ... {pinmux definitions}... // if any, including GPIO? } } scmi_gpio: scmi_gpio { compatible = "arm,scmi-gpio-generic"; gpio-controller; #gpio-cells = <2>; gpio-ranges = <&scmi_pinctrl 0 5 4>, <&scmi_pinctrl 4 0 0>; gpio-ranges-group-names = "", "ANOTHER_GPIO_GROUP"; } some_device { ... reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>; } (B) scmi { ... // other protocols scmi_pinctrl: protocol@19 { // Pin control protocol ... {pinmux definitions}... // if any, including GPIO?
scmi_gpio: scmi_gpio { // no need for "compatible" gpio-controller; #gpio-cells = <2>; gpio-ranges = <&scmi_pinctrl 0 5 4>, <&scmi_pinctrl 4 0 0>; gpio-ranges-group-names = "", "ANOTHER_GPIO_GROUP"; } } } some_device { ... reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>; }(C) if "gpio-ranges" is missing in gpio definition, assume 1:1 mapping, i.e. use a native pinctrl pin number (5). some_device { ... reset-gpios = <&scmi_gpio 5 GPIO_ACTIVE_HIGH>; }
[2] https://lkml.iu.edu/hypermail/linux/kernel/2308.1/01084.html [3] <u-boot>/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt <u-boot>/doc/device-tree-bindings/gpio/gpio.txt
Test
The patch series was tested on the following platforms:
- sandbox ("ut dm pinmux" and manually using gpio command)
Prerequisite:
- This patch series is based on my WIP "Base protocol support" patches on v2023.10-rc3. You can fetch the whole code from [4].
[4] https://git.linaro.org/people/takahiro.akashi/u-boot.git branch:scmi/pinctrl
Patches:
Patch#1: Add SCMI base protocol driver Patch#2-#4: Add drivers Patch#5-#6: Test related
Change history:
RFC (Sep 6, 2023)
- initial release as RFC
AKASHI Takahiro (6): firmware: scmi: fix protocol enumeration logic firmware: scmi: add pinctrl protocol support pinctrl: add scmi driver gpio: add scmi driver based on pinctrl firmware: scmi: add pseudo pinctrl protocol support on sandbox test: dm: add SCMI pinctrl test
arch/sandbox/dts/test.dts | 115 +++ cmd/scmi.c | 1 + drivers/firmware/scmi/Kconfig | 3 + drivers/firmware/scmi/Makefile | 1 + drivers/firmware/scmi/pinctrl.c | 412 ++++++++ drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +++++++++++++ drivers/firmware/scmi/scmi_agent-uclass.c | 18 +- drivers/pinctrl/Kconfig | 11 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-scmi.c | 1071 ++++++++++++++++++++ include/scmi_agent-uclass.h | 2 + include/scmi_protocols.h | 435 ++++++++ test/dm/scmi.c | 62 ++ 13 files changed, 2852 insertions(+), 2 deletions(-) create mode 100644 drivers/firmware/scmi/pinctrl.c create mode 100644 drivers/pinctrl/pinctrl-scmi.c
-- 2.34.1
Hi Peng,
On Wed, Sep 06, 2023 at 12:09:45AM -0300, Fabio Estevam wrote:
Adding Peng Fan, who is working on scmi/pinctrl support for i.MX9:
I made a comment there.
BTW, do you already have your own implementation of SCMI pin control protocol at SCMI firmware(server)? Is it public available?
-Takahiro Akashi
On Tue, Sep 5, 2023 at 11:41 PM AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This is an RFC and meant to get feedback from other developers as
- the specification (pinctrl part) is still in a draft
- the upstream patch for linux, including dt bindings, is still WIP
- I'm not confident the drivers are generic enough to cover most HWs
- The tests ("ut") doesn't cover all the features yet
This patch series allows users to access SCMI pin control protocol provided by SCMI server (platform). See SCMI specification document v3.2 beta 2[1] for more details about SCMI pin control protocol.
The implementation consists of two layers:
- basic helper functions for SCMI pin control protocol in drivers/firmware/scmi/pinctrl.c (patch#2)
- DM-compliant pinctrl/gpio drivers, which utilizes the helper functions, in drivers/pinctrl/pinctrl-scmi.c (patch#3,#4)
[1] https://developer.arm.com/documentation/den0056/e/?lang=en
DT bindings
Upstream pinctrl patch for linux defines the bindings in [2] though it doesn't say much. I expect that my implementation basically complies with U-Boot's generic bindings described in [3], but not all the features are verified.
As for gpio, unless you hard-code pin assignments directly in a device driver, my implementation allows the following alternatives in DT. Either way, we may need an additional binding description for gpio.
(A) scmi { ... // other protocols scmi_pinctrl: protocol@19 { // Pin control protocol ... {pinmux definitions}... // if any, including GPIO? } } scmi_gpio: scmi_gpio { compatible = "arm,scmi-gpio-generic"; gpio-controller; #gpio-cells = <2>; gpio-ranges = <&scmi_pinctrl 0 5 4>, <&scmi_pinctrl 4 0 0>; gpio-ranges-group-names = "", "ANOTHER_GPIO_GROUP"; } some_device { ... reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>; } (B) scmi { ... // other protocols scmi_pinctrl: protocol@19 { // Pin control protocol ... {pinmux definitions}... // if any, including GPIO?
scmi_gpio: scmi_gpio { // no need for "compatible" gpio-controller; #gpio-cells = <2>; gpio-ranges = <&scmi_pinctrl 0 5 4>, <&scmi_pinctrl 4 0 0>; gpio-ranges-group-names = "", "ANOTHER_GPIO_GROUP"; } } } some_device { ... reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>; }(C) if "gpio-ranges" is missing in gpio definition, assume 1:1 mapping, i.e. use a native pinctrl pin number (5). some_device { ... reset-gpios = <&scmi_gpio 5 GPIO_ACTIVE_HIGH>; }
[2] https://lkml.iu.edu/hypermail/linux/kernel/2308.1/01084.html [3] <u-boot>/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt <u-boot>/doc/device-tree-bindings/gpio/gpio.txt
Test
The patch series was tested on the following platforms:
- sandbox ("ut dm pinmux" and manually using gpio command)
Prerequisite:
- This patch series is based on my WIP "Base protocol support" patches on v2023.10-rc3. You can fetch the whole code from [4].
[4] https://git.linaro.org/people/takahiro.akashi/u-boot.git branch:scmi/pinctrl
Patches:
Patch#1: Add SCMI base protocol driver Patch#2-#4: Add drivers Patch#5-#6: Test related
Change history:
RFC (Sep 6, 2023)
- initial release as RFC
AKASHI Takahiro (6): firmware: scmi: fix protocol enumeration logic firmware: scmi: add pinctrl protocol support pinctrl: add scmi driver gpio: add scmi driver based on pinctrl firmware: scmi: add pseudo pinctrl protocol support on sandbox test: dm: add SCMI pinctrl test
arch/sandbox/dts/test.dts | 115 +++ cmd/scmi.c | 1 + drivers/firmware/scmi/Kconfig | 3 + drivers/firmware/scmi/Makefile | 1 + drivers/firmware/scmi/pinctrl.c | 412 ++++++++ drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +++++++++++++ drivers/firmware/scmi/scmi_agent-uclass.c | 18 +- drivers/pinctrl/Kconfig | 11 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-scmi.c | 1071 ++++++++++++++++++++ include/scmi_agent-uclass.h | 2 + include/scmi_protocols.h | 435 ++++++++ test/dm/scmi.c | 62 ++ 13 files changed, 2852 insertions(+), 2 deletions(-) create mode 100644 drivers/firmware/scmi/pinctrl.c create mode 100644 drivers/pinctrl/pinctrl-scmi.c
-- 2.34.1
On 9/7/2023 5:58 PM, AKASHI Takahiro wrote:
Hi Peng,
On Wed, Sep 06, 2023 at 12:09:45AM -0300, Fabio Estevam wrote:
Adding Peng Fan, who is working on scmi/pinctrl support for i.MX9:
Thanks for looping me.
I made a comment there.
Thanks for help reviewing.
BTW, do you already have your own implementation of SCMI pin control protocol at SCMI firmware(server)?
We follow the SCMI 3.2 pinctrl protocol, but use OEM config type.
Is it public available?
The firmware is not public for now, but I could share the config set array, something like this: MUX TYPE, MUX REG, CONF_TYPE, CONF REG, DAISY TYPE, DAISY ID, DAISY VALUE.
Regards, Peng.
-Takahiro Akashi
On Tue, Sep 5, 2023 at 11:41 PM AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This is an RFC and meant to get feedback from other developers as
- the specification (pinctrl part) is still in a draft
- the upstream patch for linux, including dt bindings, is still WIP
- I'm not confident the drivers are generic enough to cover most HWs
- The tests ("ut") doesn't cover all the features yet
This patch series allows users to access SCMI pin control protocol provided by SCMI server (platform). See SCMI specification document v3.2 beta 2[1] for more details about SCMI pin control protocol.
The implementation consists of two layers:
- basic helper functions for SCMI pin control protocol in drivers/firmware/scmi/pinctrl.c (patch#2)
- DM-compliant pinctrl/gpio drivers, which utilizes the helper functions, in drivers/pinctrl/pinctrl-scmi.c (patch#3,#4)
[1] https://developer.arm.com/documentation/den0056/e/?lang=en
DT bindings
Upstream pinctrl patch for linux defines the bindings in [2] though it doesn't say much. I expect that my implementation basically complies with U-Boot's generic bindings described in [3], but not all the features are verified.
As for gpio, unless you hard-code pin assignments directly in a device driver, my implementation allows the following alternatives in DT. Either way, we may need an additional binding description for gpio.
(A) scmi { ... // other protocols scmi_pinctrl: protocol@19 { // Pin control protocol ... {pinmux definitions}... // if any, including GPIO? } } scmi_gpio: scmi_gpio { compatible = "arm,scmi-gpio-generic"; gpio-controller; #gpio-cells = <2>; gpio-ranges = <&scmi_pinctrl 0 5 4>, <&scmi_pinctrl 4 0 0>; gpio-ranges-group-names = "", "ANOTHER_GPIO_GROUP"; } some_device { ... reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>; } (B) scmi { ... // other protocols scmi_pinctrl: protocol@19 { // Pin control protocol ... {pinmux definitions}... // if any, including GPIO?
scmi_gpio: scmi_gpio { // no need for "compatible" gpio-controller; #gpio-cells = <2>; gpio-ranges = <&scmi_pinctrl 0 5 4>, <&scmi_pinctrl 4 0 0>; gpio-ranges-group-names = "", "ANOTHER_GPIO_GROUP"; } } } some_device { ... reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>; }(C) if "gpio-ranges" is missing in gpio definition, assume 1:1 mapping, i.e. use a native pinctrl pin number (5). some_device { ... reset-gpios = <&scmi_gpio 5 GPIO_ACTIVE_HIGH>; }
[2] https://lkml.iu.edu/hypermail/linux/kernel/2308.1/01084.html [3] <u-boot>/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt <u-boot>/doc/device-tree-bindings/gpio/gpio.txt
Test
The patch series was tested on the following platforms:
- sandbox ("ut dm pinmux" and manually using gpio command)
Prerequisite:
- This patch series is based on my WIP "Base protocol support" patches on v2023.10-rc3. You can fetch the whole code from [4].
[4] https://git.linaro.org/people/takahiro.akashi/u-boot.git branch:scmi/pinctrl
Patches:
Patch#1: Add SCMI base protocol driver Patch#2-#4: Add drivers Patch#5-#6: Test related
Change history:
RFC (Sep 6, 2023)
- initial release as RFC
AKASHI Takahiro (6): firmware: scmi: fix protocol enumeration logic firmware: scmi: add pinctrl protocol support pinctrl: add scmi driver gpio: add scmi driver based on pinctrl firmware: scmi: add pseudo pinctrl protocol support on sandbox test: dm: add SCMI pinctrl test
arch/sandbox/dts/test.dts | 115 +++ cmd/scmi.c | 1 + drivers/firmware/scmi/Kconfig | 3 + drivers/firmware/scmi/Makefile | 1 + drivers/firmware/scmi/pinctrl.c | 412 ++++++++ drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +++++++++++++ drivers/firmware/scmi/scmi_agent-uclass.c | 18 +- drivers/pinctrl/Kconfig | 11 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-scmi.c | 1071 ++++++++++++++++++++ include/scmi_agent-uclass.h | 2 + include/scmi_protocols.h | 435 ++++++++ test/dm/scmi.c | 62 ++ 13 files changed, 2852 insertions(+), 2 deletions(-) create mode 100644 drivers/firmware/scmi/pinctrl.c create mode 100644 drivers/pinctrl/pinctrl-scmi.c
-- 2.34.1
participants (5)
-
AKASHI Takahiro -
Fabio Estevam -
Michal Simek -
Peng Fan -
Simon Glass