* [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
@ 2023-12-15 11:56 Peng Fan (OSS)
2023-12-15 11:56 ` [PATCH 1/7] firmware: arm_scmi: introduce helper get_max_msg_size Peng Fan (OSS)
` (6 more replies)
0 siblings, 7 replies; 25+ messages in thread
From: Peng Fan (OSS) @ 2023-12-15 11:56 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan,
Oleksii Moisieiev
This patchset is a rework from Oleksii's RFC v5 patchset
https://lore.kernel.org/all/cover.1698353854.git.oleksii_moisieiev@epam.com/
This patchset introduces some changes based on RFC v5:
- introduce helper get_max_msg_size
- support compatible string
- iterate the id_table
- Support multiple configs in one command
- Added i.MX support
- Patch 5 firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
is almost same as RFCv5 expect multiple configs support.
- Patch 4 the dt-bindings includes compatible string to support i.MX
- Rebased on 2023-12-15 linux-next/master
If any comments from RFC v5 are missed, I am sorry in advance.
This PINCTRL Protocol is following Version 3.2 SCMI Spec Beta release.
On ARM-based systems, a separate Cortex-M based System Control Processor
(SCP) provides control on pins, as well as with power, clocks, reset
controllers. So implement the driver to support such cases.
The i.MX95 Example as below:
Configuration:
The scmi-pinctrl driver can be configured using DT bindings.
For example:
/ {
sram0: sram@445b1000 {
compatible = "mmio-sram";
reg = <0x0 0x445b1000 0x0 0x400>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x445b1000 0x400>;
scmi_buf0: scmi-sram-section@0 {
compatible = "arm,scmi-shmem";
reg = <0x0 0x80>;
};
scmi_buf1: scmi-sram-section@80 {
compatible = "arm,scmi-shmem";
reg = <0x80 0x80>;
};
};
firmware {
scmi {
compatible = "arm,scmi";
mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
shmem = <&scmi_buf0>, <&scmi_buf1>;
#address-cells = <1>;
#size-cells = <0>;
scmi_iomuxc: protocol@19 {
compatible = "fsl,imx95-scmi-pinctrl";
reg = <0x19>;
};
};
};
};
&scmi_iomuxc {
pinctrl_tpm3: tpm3grp {
fsl,pins = <
IMX95_PAD_GPIO_IO12__TPM3_CH2 0x51e
>;
};
};
This patchset has been tested on i.MX95-19x19-EVK board.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Oleksii Moisieiev (1):
firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
Peng Fan (6):
firmware: arm_scmi: introduce helper get_max_msg_size
firmware: scmi: support compatible string
firmware: arm_scmi: bus: iterate the id_table
dt-bindings: firmware: arm,scmi: support pinctrl protocol
pinctrl: Implementation of the generic scmi-pinctrl driver
pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map
.../devicetree/bindings/firmware/arm,scmi.yaml | 99 +++
MAINTAINERS | 7 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/bus.c | 49 +-
drivers/firmware/arm_scmi/common.h | 2 +-
drivers/firmware/arm_scmi/driver.c | 32 +-
drivers/firmware/arm_scmi/pinctrl.c | 927 +++++++++++++++++++++
drivers/firmware/arm_scmi/protocols.h | 2 +
drivers/pinctrl/Kconfig | 11 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-scmi-imx.c | 117 +++
drivers/pinctrl/pinctrl-scmi.c | 419 ++++++++++
drivers/pinctrl/pinctrl-scmi.h | 12 +
include/linux/scmi_protocol.h | 48 ++
14 files changed, 1710 insertions(+), 17 deletions(-)
---
base-commit: 17cb8a20bde66a520a2ca7aad1063e1ce7382240
change-id: 20231215-pinctrl-scmi-4c5b0374f4c6
Best regards,
--
Peng Fan <peng.fan@nxp.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/7] firmware: arm_scmi: introduce helper get_max_msg_size
2023-12-15 11:56 [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
@ 2023-12-15 11:56 ` Peng Fan (OSS)
2023-12-22 15:30 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 2/7] firmware: scmi: support compatible string Peng Fan (OSS)
` (5 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Peng Fan (OSS) @ 2023-12-15 11:56 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
When Agent sending data to SCMI server, the Agent driver could check
the size to avoid protocol buffer overflow. So introduce the helper
get_max_msg_size.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/driver.c | 15 +++++++++++++++
drivers/firmware/arm_scmi/protocols.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index a9f70e6e58ac..b4f8f190351b 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1481,6 +1481,20 @@ static int scmi_common_extended_name_get(const struct scmi_protocol_handle *ph,
return ret;
}
+/**
+ * scmi_common_get_max_msg_size - Get maximum message size
+ * @ph: A protocol handle reference.
+ *
+ * Return: Maximum message size for the current protocol.
+ */
+static int scmi_common_get_max_msg_size(const struct scmi_protocol_handle *ph)
+{
+ const struct scmi_protocol_instance *pi = ph_to_pi(ph);
+ struct scmi_info *info = handle_to_scmi_info(pi->handle);
+
+ return info->desc->max_msg_size;
+}
+
/**
* struct scmi_iterator - Iterator descriptor
* @msg: A reference to the message TX buffer; filled by @prepare_message with
@@ -1756,6 +1770,7 @@ static void scmi_common_fastchannel_db_ring(struct scmi_fc_db_info *db)
static const struct scmi_proto_helpers_ops helpers_ops = {
.extended_name_get = scmi_common_extended_name_get,
+ .get_max_msg_size = scmi_common_get_max_msg_size,
.iter_response_init = scmi_iterator_init,
.iter_response_run = scmi_iterator_run,
.fastchannel_init = scmi_common_fastchannel_init,
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index e683c26f24eb..08de8dc064c1 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -270,6 +270,7 @@ struct scmi_proto_helpers_ops {
void __iomem **p_addr,
struct scmi_fc_db_info **p_db);
void (*fastchannel_db_ring)(struct scmi_fc_db_info *db);
+ int (*get_max_msg_size)(const struct scmi_protocol_handle *ph);
};
/**
--
2.37.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/7] firmware: scmi: support compatible string
2023-12-15 11:56 [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
2023-12-15 11:56 ` [PATCH 1/7] firmware: arm_scmi: introduce helper get_max_msg_size Peng Fan (OSS)
@ 2023-12-15 11:56 ` Peng Fan (OSS)
2023-12-15 11:56 ` [PATCH 3/7] firmware: arm_scmi: bus: iterate the id_table Peng Fan (OSS)
` (4 subsequent siblings)
6 siblings, 0 replies; 25+ messages in thread
From: Peng Fan (OSS) @ 2023-12-15 11:56 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
i.MX95 use SCMI PINCTRL OEM config type, so it could not reuse
pinctrl-scmi.c which use generic pinconf. Then need introduce
saying pinctrl-imx-scmi.c to support i.MX95, however both
pinctrl-scmi and pinctrl-imx-scmi driver will register a pinctrl device
with the same of_node(protocl@19). This will introduce an issue, because
the pinctrl subsystem will use of_node to find the exact pinctrl
device. But with two pinctrl devices registered by two drivers, it is
unknown which pinctrl device will be chosen.
So introduce compatible string to support cases that Vendor SCMI
driver which does almost same thing as generic SCMI driver, but
has some specific Vendor settings to do.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/bus.c | 39 ++++++++++++++++++++++++++++++--------
drivers/firmware/arm_scmi/common.h | 2 +-
drivers/firmware/arm_scmi/driver.c | 15 ++++++++++-----
include/linux/scmi_protocol.h | 2 ++
4 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index c15928b8c5cc..c999edd18cd3 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -194,13 +194,21 @@ scmi_dev_match_id(struct scmi_device *scmi_dev, struct scmi_driver *scmi_drv)
if (!id)
return NULL;
- for (; id->protocol_id; id++)
- if (id->protocol_id == scmi_dev->protocol_id) {
- if (!id->name)
+ for (; id->protocol_id; id++) {
+ if (id->protocol_id != scmi_dev->protocol_id)
+ continue;
+ if (!id->name)
+ return id;
+
+ if (!strcmp(id->name, scmi_dev->name)) {
+ if (!id->compatible &&
+ device_property_read_string(&scmi_dev->dev, "compatible", NULL))
return id;
- else if (!strcmp(id->name, scmi_dev->name))
+
+ if (id->compatible && device_is_compatible(&scmi_dev->dev, id->compatible))
return id;
}
+ }
return NULL;
}
@@ -325,11 +333,14 @@ static void __scmi_device_destroy(struct scmi_device *scmi_dev)
static struct scmi_device *
__scmi_device_create(struct device_node *np, struct device *parent,
- int protocol, const char *name)
+ int protocol, const char *name, const char *compatible)
{
int id, retval;
struct scmi_device *scmi_dev;
+ if (compatible && !of_device_is_compatible(np, compatible))
+ return NULL;
+
/*
* If the same protocol/name device already exist under the same parent
* (i.e. SCMI instance) just return the existent device.
@@ -405,6 +416,7 @@ __scmi_device_create(struct device_node *np, struct device *parent,
* @name: The requested-name of the device to be created; this is optional
* and if no @name is provided, all the devices currently known to
* be requested on the SCMI bus for @protocol will be created.
+ * @compatible: The compatible string
*
* This method can be invoked to create a single well-defined device (like
* a transport device or a device requested by an SCMI driver loaded after
@@ -421,14 +433,14 @@ __scmi_device_create(struct device_node *np, struct device *parent,
*/
struct scmi_device *scmi_device_create(struct device_node *np,
struct device *parent, int protocol,
- const char *name)
+ const char *name, const char *compatible)
{
struct list_head *phead;
struct scmi_requested_dev *rdev;
struct scmi_device *scmi_dev = NULL;
if (name)
- return __scmi_device_create(np, parent, protocol, name);
+ return __scmi_device_create(np, parent, protocol, name, compatible);
mutex_lock(&scmi_requested_devices_mtx);
phead = idr_find(&scmi_requested_devices, protocol);
@@ -442,9 +454,20 @@ struct scmi_device *scmi_device_create(struct device_node *np,
list_for_each_entry(rdev, phead, node) {
struct scmi_device *sdev;
+ if (compatible) {
+ if (!rdev->id_table->compatible)
+ continue;
+ if (strcmp(compatible, rdev->id_table->compatible))
+ continue;
+ } else {
+ if (rdev->id_table->compatible)
+ continue;
+ }
+
sdev = __scmi_device_create(np, parent,
rdev->id_table->protocol_id,
- rdev->id_table->name);
+ rdev->id_table->name,
+ rdev->id_table->compatible);
/* Report errors and carry on... */
if (sdev)
scmi_dev = sdev;
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index c46dc5215af7..930a24e0d712 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -149,7 +149,7 @@ extern struct blocking_notifier_head scmi_requested_devices_nh;
struct scmi_device *scmi_device_create(struct device_node *np,
struct device *parent, int protocol,
- const char *name);
+ const char *name, const char *compatible);
void scmi_device_destroy(struct device *parent, int protocol, const char *name);
int scmi_protocol_acquire(const struct scmi_handle *handle, u8 protocol_id);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index b4f8f190351b..3174da57d832 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -261,15 +261,17 @@ EXPORT_SYMBOL_GPL(scmi_protocol_unregister);
* @name: The optional name of the device to be created: if not provided this
* call will lead to the creation of all the devices currently requested
* for the specified protocol.
+ * @compatible: optional, the compatible string
*/
static void scmi_create_protocol_devices(struct device_node *np,
struct scmi_info *info,
- int prot_id, const char *name)
+ int prot_id, const char *name,
+ const char *compatible)
{
struct scmi_device *sdev;
mutex_lock(&info->devreq_mtx);
- sdev = scmi_device_create(np, info->dev, prot_id, name);
+ sdev = scmi_device_create(np, info->dev, prot_id, name, compatible);
if (name && !sdev)
dev_err(info->dev,
"failed to create device for protocol 0x%X (%s)\n",
@@ -2354,7 +2356,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
snprintf(name, 32, "__scmi_transport_device_%s_%02X",
idx ? "rx" : "tx", prot_id);
/* Create a uniquely named, dedicated transport device for this chan */
- tdev = scmi_device_create(of_node, info->dev, prot_id, name);
+ tdev = scmi_device_create(of_node, info->dev, prot_id, name, NULL);
if (!tdev) {
dev_err(info->dev,
"failed to create transport device (%s)\n", name);
@@ -2550,7 +2552,7 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
switch (action) {
case SCMI_BUS_NOTIFY_DEVICE_REQUEST:
scmi_create_protocol_devices(np, info, id_table->protocol_id,
- id_table->name);
+ id_table->name, id_table->compatible);
break;
case SCMI_BUS_NOTIFY_DEVICE_UNREQUEST:
scmi_destroy_protocol_devices(info, id_table->protocol_id,
@@ -2802,10 +2804,13 @@ static int scmi_probe(struct platform_device *pdev)
for_each_available_child_of_node(np, child) {
u32 prot_id;
+ const char *s = NULL;
if (of_property_read_u32(child, "reg", &prot_id))
continue;
+ of_property_read_string(child, "compatible", &s);
+
if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
dev_err(dev, "Out of range protocol %d\n", prot_id);
@@ -2828,7 +2833,7 @@ static int scmi_probe(struct platform_device *pdev)
}
of_node_get(child);
- scmi_create_protocol_devices(child, info, prot_id, NULL);
+ scmi_create_protocol_devices(child, info, prot_id, NULL, s);
}
return 0;
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index f2f05fb42d28..19a06b872afe 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -839,6 +839,7 @@ struct scmi_device {
u32 id;
u8 protocol_id;
const char *name;
+ const char *compatible; /* Optional */
struct device dev;
struct scmi_handle *handle;
};
@@ -848,6 +849,7 @@ struct scmi_device {
struct scmi_device_id {
u8 protocol_id;
const char *name;
+ const char *compatible; /* Optional */
};
struct scmi_driver {
--
2.37.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/7] firmware: arm_scmi: bus: iterate the id_table
2023-12-15 11:56 [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
2023-12-15 11:56 ` [PATCH 1/7] firmware: arm_scmi: introduce helper get_max_msg_size Peng Fan (OSS)
2023-12-15 11:56 ` [PATCH 2/7] firmware: scmi: support compatible string Peng Fan (OSS)
@ 2023-12-15 11:56 ` Peng Fan (OSS)
2023-12-22 15:40 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol Peng Fan (OSS)
` (3 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Peng Fan (OSS) @ 2023-12-15 11:56 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
There maybe more entries in driver->id_table, just like platform
driver of_match_table. So iterate the id_table, not only use the 1st
entry.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/bus.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index c999edd18cd3..aeb7da841b9d 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -282,14 +282,18 @@ EXPORT_SYMBOL_GPL(scmi_bus_type);
int scmi_driver_register(struct scmi_driver *driver, struct module *owner,
const char *mod_name)
{
+ const struct scmi_device_id *id_table = driver->id_table;
int retval;
if (!driver->probe)
return -EINVAL;
- retval = scmi_protocol_device_request(driver->id_table);
- if (retval)
- return retval;
+ while (id_table->name) {
+ retval = scmi_protocol_device_request(id_table);
+ if (retval)
+ return retval;
+ id_table++;
+ }
driver->driver.bus = &scmi_bus_type;
driver->driver.name = driver->name;
--
2.37.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-15 11:56 [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
` (2 preceding siblings ...)
2023-12-15 11:56 ` [PATCH 3/7] firmware: arm_scmi: bus: iterate the id_table Peng Fan (OSS)
@ 2023-12-15 11:56 ` Peng Fan (OSS)
2023-12-15 20:11 ` Rob Herring
` (3 more replies)
2023-12-15 11:56 ` [PATCH 5/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
` (2 subsequent siblings)
6 siblings, 4 replies; 25+ messages in thread
From: Peng Fan (OSS) @ 2023-12-15 11:56 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Add SCMI v3.2 pinctrl protocol bindings and example.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
.../devicetree/bindings/firmware/arm,scmi.yaml | 99 ++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 4591523b51a0..bfd2b6a89979 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -247,6 +247,85 @@ properties:
reg:
const: 0x18
+ protocol@19:
+ type: object
+ allOf:
+ - $ref: '#/$defs/protocol-node'
+ - $ref: /schemas/pinctrl/pinctrl.yaml
+ - if:
+ properties:
+ compatible:
+ const: fsl,imx95-scmi-pinctrl
+ then:
+ patternProperties:
+ "grp$": false
+ "-pins$": true
+ else:
+ patternProperties:
+ "grp$": false
+ "-pins$": true
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ const: 0x19
+
+ '#pinctrl-cells':
+ const: 0
+
+ compatible:
+ const: fsl,imx95-scmi-pinctrl
+
+ patternProperties:
+ '-pins$':
+ type: object
+ allOf:
+ - $ref: /schemas/pinctrl/pincfg-node.yaml#
+ - $ref: /schemas/pinctrl/pinmux-node.yaml#
+ unevaluatedProperties: false
+
+ description:
+ A pin multiplexing sub-node describe how to configure a
+ set of pins is some desired function.
+ A single sub-node may define several pin configurations.
+ This sub-node is using default pinctrl bindings to configure
+ pin multiplexing and using SCMI protocol to apply specified
+ configuration using SCMI protocol.
+
+ 'grp$':
+ type: object
+ description:
+ Pinctrl node's client devices use subnodes for desired pin configuration.
+ Client device subnodes use below standard properties.
+
+ properties:
+ fsl,pins:
+ description:
+ each entry consists of 6 integers and represents the mux and config
+ setting for one pin. The first 5 integers <mux_reg conf_reg input_reg
+ mux_val input_val> are specified using a PIN_FUNC_ID macro, which can
+ be found in <arch/arm64/boot/dts/freescale/imx95-pinfunc.h>. The last
+ integer CONFIG is the pad setting value like pull-up on this pin. Please
+ refer to i.MX95 Plus Reference Manual for detailed CONFIG settings.
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+ items:
+ items:
+ - description: |
+ "mux_reg" indicates the offset of mux register.
+ - description: |
+ "conf_reg" indicates the offset of pad configuration register.
+ - description: |
+ "input_reg" indicates the offset of select input register.
+ - description: |
+ "mux_val" indicates the mux value to be applied.
+ - description: |
+ "input_val" indicates the select input value to be applied.
+ - description: |
+ "pad_setting" indicates the pad configuration value to be applied.
+
+ required:
+ - reg
+
additionalProperties: false
$defs:
@@ -401,6 +480,26 @@ examples:
scmi_powercap: protocol@18 {
reg = <0x18>;
};
+
+ scmi_pinctrl: protocol@19 {
+ reg = <0x19>;
+ #pinctrl-cells = <0>;
+
+ i2c2-pins {
+ groups = "i2c2_a", "i2c2_b";
+ function = "i2c2";
+ };
+
+ mdio-pins {
+ groups = "avb_mdio";
+ drive-strength = <24>;
+ };
+
+ keys_pins: keys-pins {
+ pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
+ bias-pull-up;
+ };
+ };
};
};
--
2.37.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
2023-12-15 11:56 [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
` (3 preceding siblings ...)
2023-12-15 11:56 ` [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol Peng Fan (OSS)
@ 2023-12-15 11:56 ` Peng Fan (OSS)
2023-12-22 15:58 ` Cristian Marussi
2023-12-22 16:26 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 6/7] pinctrl: Implementation of the generic scmi-pinctrl driver Peng Fan (OSS)
2023-12-15 11:56 ` [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map Peng Fan (OSS)
6 siblings, 2 replies; 25+ messages in thread
From: Peng Fan (OSS) @ 2023-12-15 11:56 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan,
Oleksii Moisieiev
From: Oleksii Moisieiev <Oleksii_Moisieiev@epam.com>
Add basic implementation of the SCMI v3.2 pincontrol protocol.
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Co-developed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
MAINTAINERS | 6 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/driver.c | 2 +
drivers/firmware/arm_scmi/pinctrl.c | 927 ++++++++++++++++++++++++++++++++++
drivers/firmware/arm_scmi/protocols.h | 1 +
include/linux/scmi_protocol.h | 46 ++
6 files changed, 983 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b589218605b4..8d971adeee22 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21180,6 +21180,12 @@ F: include/linux/sc[mp]i_protocol.h
F: include/trace/events/scmi.h
F: include/uapi/linux/virtio_scmi.h
+SYSTEM CONTROL MANAGEMENT INTERFACE (SCMI) PINCTRL DRIVER
+M: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
+L: linux-arm-kernel@lists.infradead.org
+S: Maintained
+F: drivers/firmware/arm_scmi/pinctrl.c
+
SYSTEM RESET/SHUTDOWN DRIVERS
M: Sebastian Reichel <sre@kernel.org>
L: linux-pm@vger.kernel.org
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index a7bc4796519c..8e3874ff1544 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -11,6 +11,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
+scmi-protocols-y += pinctrl.o
scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 3174da57d832..1cf9f5d4f7bd 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3057,6 +3057,7 @@ static int __init scmi_driver_init(void)
scmi_voltage_register();
scmi_system_register();
scmi_powercap_register();
+ scmi_pinctrl_register();
return platform_driver_register(&scmi_driver);
}
@@ -3074,6 +3075,7 @@ static void __exit scmi_driver_exit(void)
scmi_voltage_unregister();
scmi_system_unregister();
scmi_powercap_unregister();
+ scmi_pinctrl_unregister();
scmi_transports_exit();
diff --git a/drivers/firmware/arm_scmi/pinctrl.c b/drivers/firmware/arm_scmi/pinctrl.c
new file mode 100644
index 000000000000..a25c8edcedd2
--- /dev/null
+++ b/drivers/firmware/arm_scmi/pinctrl.c
@@ -0,0 +1,927 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Pinctrl Protocol
+ *
+ * Copyright (C) 2023 EPAM
+ */
+
+#include <linux/module.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/scmi_protocol.h>
+#include <linux/slab.h>
+
+#include "common.h"
+#include "protocols.h"
+
+/* Updated only after ALL the mandatory features for that version are merged */
+#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x40000
+
+#define REG_TYPE_BITS GENMASK(9, 8)
+#define REG_CONFIG GENMASK(7, 0)
+
+#define GET_GROUPS_NR(x) le32_get_bits((x), GENMASK(31, 16))
+#define GET_PINS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+#define GET_FUNCTIONS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+
+#define EXT_NAME_FLAG(x) le32_get_bits((x), BIT(31))
+#define NUM_ELEMS(x) le32_get_bits((x), GENMASK(15, 0))
+
+#define REMAINING(x) le32_get_bits((x), GENMASK(31, 16))
+#define RETURNED(x) le32_get_bits((x), GENMASK(11, 0))
+
+enum scmi_pinctrl_protocol_cmd {
+ PINCTRL_ATTRIBUTES = 0x3,
+ PINCTRL_LIST_ASSOCIATIONS = 0x4,
+ PINCTRL_CONFIG_GET = 0x5,
+ PINCTRL_CONFIG_SET = 0x6,
+ PINCTRL_FUNCTION_SELECT = 0x7,
+ PINCTRL_REQUEST = 0x8,
+ PINCTRL_RELEASE = 0x9,
+ PINCTRL_NAME_GET = 0xa,
+ PINCTRL_SET_PERMISSIONS = 0xb
+};
+
+struct scmi_msg_conf_set {
+ __le32 identifier;
+ __le32 attributes;
+ __le32 configs[];
+};
+
+struct scmi_msg_conf_get {
+ __le32 identifier;
+ __le32 attributes;
+};
+
+struct scmi_resp_conf_get {
+ __le32 num_configs;
+ __le32 configs[];
+};
+
+struct scmi_msg_pinctrl_protocol_attributes {
+ __le32 attributes_low;
+ __le32 attributes_high;
+};
+
+struct scmi_msg_pinctrl_attributes {
+ __le32 identifier;
+ __le32 flags;
+};
+
+struct scmi_resp_pinctrl_attributes {
+ __le32 attributes;
+ u8 name[SCMI_SHORT_NAME_MAX_SIZE];
+};
+
+struct scmi_msg_pinctrl_list_assoc {
+ __le32 identifier;
+ __le32 flags;
+ __le32 index;
+};
+
+struct scmi_resp_pinctrl_list_assoc {
+ __le32 flags;
+ __le16 array[];
+};
+
+struct scmi_msg_func_set {
+ __le32 identifier;
+ __le32 function_id;
+ __le32 flags;
+};
+
+struct scmi_msg_request {
+ __le32 identifier;
+ __le32 flags;
+};
+
+struct scmi_group_info {
+ char name[SCMI_MAX_STR_SIZE];
+ bool present;
+ unsigned int *group_pins;
+ unsigned int nr_pins;
+};
+
+struct scmi_function_info {
+ char name[SCMI_MAX_STR_SIZE];
+ bool present;
+ unsigned int *groups;
+ unsigned int nr_groups;
+};
+
+struct scmi_pin_info {
+ char name[SCMI_MAX_STR_SIZE];
+ bool present;
+};
+
+struct scmi_pinctrl_info {
+ u32 version;
+ int nr_groups;
+ int nr_functions;
+ int nr_pins;
+ struct scmi_group_info *groups;
+ struct scmi_function_info *functions;
+ struct scmi_pin_info *pins;
+};
+
+static int scmi_pinctrl_attributes_get(const struct scmi_protocol_handle *ph,
+ struct scmi_pinctrl_info *pi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_pinctrl_protocol_attributes *attr;
+
+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ pi->nr_functions = GET_FUNCTIONS_NR(attr->attributes_high);
+ pi->nr_groups = GET_GROUPS_NR(attr->attributes_low);
+ pi->nr_pins = GET_PINS_NR(attr->attributes_low);
+ }
+
+ ph->xops->xfer_put(ph, t);
+ return ret;
+}
+
+static int scmi_pinctrl_count_get(const struct scmi_protocol_handle *ph,
+ enum scmi_pinctrl_selector_type type)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ switch (type) {
+ case PIN_TYPE:
+ return pi->nr_pins;
+ case GROUP_TYPE:
+ return pi->nr_groups;
+ case FUNCTION_TYPE:
+ return pi->nr_functions;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int scmi_pinctrl_validate_id(const struct scmi_protocol_handle *ph,
+ u32 identifier,
+ enum scmi_pinctrl_selector_type type)
+{
+ int value;
+
+ value = scmi_pinctrl_count_get(ph, type);
+ if (value < 0)
+ return value;
+
+ if (identifier >= value)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int scmi_pinctrl_attributes(const struct scmi_protocol_handle *ph,
+ enum scmi_pinctrl_selector_type type,
+ u32 selector, char *name,
+ unsigned int *n_elems)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_pinctrl_attributes *tx;
+ struct scmi_resp_pinctrl_attributes *rx;
+
+ if (!name)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, PINCTRL_ATTRIBUTES, sizeof(*tx), sizeof(*rx), &t);
+ if (ret)
+ return ret;
+
+ tx = t->tx.buf;
+ rx = t->rx.buf;
+ tx->identifier = cpu_to_le32(selector);
+ tx->flags = cpu_to_le32(type);
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ if (n_elems)
+ *n_elems = NUM_ELEMS(rx->attributes);
+
+ strscpy(name, rx->name, SCMI_SHORT_NAME_MAX_SIZE);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ /*
+ * If supported overwrite short name with the extended one;
+ * on error just carry on and use already provided short name.
+ */
+ if (!ret && EXT_NAME_FLAG(rx->attributes))
+ ph->hops->extended_name_get(ph, PINCTRL_NAME_GET, selector,
+ (u32 *)&type, name,
+ SCMI_MAX_STR_SIZE);
+ return ret;
+}
+
+struct scmi_pinctrl_ipriv {
+ u32 selector;
+ enum scmi_pinctrl_selector_type type;
+ unsigned int *array;
+};
+
+static void iter_pinctrl_assoc_prepare_message(void *message,
+ unsigned int desc_index,
+ const void *priv)
+{
+ struct scmi_msg_pinctrl_list_assoc *msg = message;
+ const struct scmi_pinctrl_ipriv *p = priv;
+
+ msg->identifier = cpu_to_le32(p->selector);
+ msg->flags = cpu_to_le32(p->type);
+ /* Set the number of OPPs to be skipped/already read */
+ msg->index = cpu_to_le32(desc_index);
+}
+
+static int iter_pinctrl_assoc_update_state(struct scmi_iterator_state *st,
+ const void *response, void *priv)
+{
+ const struct scmi_resp_pinctrl_list_assoc *r = response;
+
+ st->num_returned = RETURNED(r->flags);
+ st->num_remaining = REMAINING(r->flags);
+
+ return 0;
+}
+
+static int
+iter_pinctrl_assoc_process_response(const struct scmi_protocol_handle *ph,
+ const void *response,
+ struct scmi_iterator_state *st, void *priv)
+{
+ const struct scmi_resp_pinctrl_list_assoc *r = response;
+ struct scmi_pinctrl_ipriv *p = priv;
+
+ p->array[st->desc_index + st->loop_idx] =
+ le16_to_cpu(r->array[st->loop_idx]);
+
+ return 0;
+}
+
+static int scmi_pinctrl_list_associations(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ u16 size, unsigned int *array)
+{
+ int ret;
+ void *iter;
+ struct scmi_iterator_ops ops = {
+ .prepare_message = iter_pinctrl_assoc_prepare_message,
+ .update_state = iter_pinctrl_assoc_update_state,
+ .process_response = iter_pinctrl_assoc_process_response,
+ };
+ struct scmi_pinctrl_ipriv ipriv = {
+ .selector = selector,
+ .type = type,
+ .array = array,
+ };
+
+ if (!array || !size || type == PIN_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ iter = ph->hops->iter_response_init(ph, &ops, size,
+ PINCTRL_LIST_ASSOCIATIONS,
+ sizeof(struct scmi_msg_pinctrl_list_assoc),
+ &ipriv);
+
+ if (IS_ERR(iter))
+ return PTR_ERR(iter);
+
+ return ph->hops->iter_response_run(iter);
+}
+
+struct scmi_conf_get_ipriv {
+ u32 selector;
+ enum scmi_pinctrl_selector_type type;
+ u8 all;
+ u8 *config_types;
+ unsigned long *config_values;
+};
+
+static void iter_pinctrl_conf_get_prepare_message(void *message,
+ unsigned int desc_index,
+ const void *priv)
+{
+ struct scmi_msg_conf_get *msg = message;
+ const struct scmi_conf_get_ipriv *p = priv;
+ u32 attributes;
+
+ msg->identifier = cpu_to_le32(p->selector);
+ attributes = FIELD_PREP(BIT(18), p->all) |
+ FIELD_PREP(GENMASK(17, 16), p->type);
+
+ if (p->all)
+ attributes |= FIELD_PREP(GENMASK(15, 8), desc_index);
+ else
+ attributes |= FIELD_PREP(GENMASK(7, 0), p->config_types[0]);
+
+ msg->attributes = cpu_to_le32(attributes);
+ msg->identifier = cpu_to_le32(p->selector);
+}
+
+static int iter_pinctrl_conf_get_update_state(struct scmi_iterator_state *st,
+ const void *response, void *priv)
+{
+ const struct scmi_resp_conf_get *r = response;
+
+ st->num_returned = le32_get_bits(r->num_configs, GENMASK(7, 0));
+ st->num_remaining = le32_get_bits(r->num_configs, GENMASK(31, 24));
+
+ return 0;
+}
+
+static int iter_pinctrl_conf_get_process_response(const struct scmi_protocol_handle *ph,
+ const void *response,
+ struct scmi_iterator_state *st, void *priv)
+{
+ const struct scmi_resp_conf_get *r = response;
+ struct scmi_conf_get_ipriv *p = priv;
+
+ if (!p->all) {
+ if (p->config_types[0] !=
+ le32_get_bits(r->configs[st->loop_idx * 2], GENMASK(7, 0)))
+ return -EINVAL;
+ } else {
+ p->config_types[st->desc_index + st->loop_idx] =
+ le32_get_bits(r->configs[st->loop_idx * 2], GENMASK(7, 0));
+ }
+
+ p->config_values[st->desc_index + st->loop_idx] =
+ le32_to_cpu(r->configs[st->loop_idx * 2 + 1]);
+
+ return 0;
+}
+
+static int scmi_pinctrl_config_get(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ u8 config_type, unsigned long *config_value)
+{
+ int ret;
+ void *iter;
+ struct scmi_iterator_ops ops = {
+ .prepare_message = iter_pinctrl_conf_get_prepare_message,
+ .update_state = iter_pinctrl_conf_get_update_state,
+ .process_response = iter_pinctrl_conf_get_process_response,
+ };
+ struct scmi_conf_get_ipriv ipriv = {
+ .selector = selector,
+ .type = type,
+ .all = 0,
+ .config_types = &config_type,
+ .config_values = config_value,
+ };
+
+ if (!config_value || type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ iter = ph->hops->iter_response_init(ph, &ops, 1, PINCTRL_CONFIG_GET,
+ sizeof(struct scmi_msg_conf_get),
+ &ipriv);
+
+ if (IS_ERR(iter))
+ return PTR_ERR(iter);
+
+ return ph->hops->iter_response_run(iter);
+}
+
+static int scmi_pinctrl_config_get_all(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ u16 size, u8 *config_types,
+ unsigned long *config_values)
+{
+ int ret;
+ void *iter;
+ struct scmi_iterator_ops ops = {
+ .prepare_message = iter_pinctrl_conf_get_prepare_message,
+ .update_state = iter_pinctrl_conf_get_update_state,
+ .process_response = iter_pinctrl_conf_get_process_response,
+ };
+ struct scmi_conf_get_ipriv ipriv = {
+ .selector = selector,
+ .type = type,
+ .all = 1,
+ .config_types = config_types,
+ .config_values = config_values,
+ };
+
+ if (!config_values || !config_types || type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ iter = ph->hops->iter_response_init(ph, &ops, size, PINCTRL_CONFIG_GET,
+ sizeof(struct scmi_msg_conf_get),
+ &ipriv);
+
+ if (IS_ERR(iter))
+ return PTR_ERR(iter);
+
+ return ph->hops->iter_response_run(iter);
+}
+
+static int scmi_pinctrl_config_set(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ unsigned long *configs, unsigned int nr_configs)
+{
+ struct scmi_xfer *t;
+ struct scmi_msg_conf_set *tx;
+ u32 attributes;
+ int ret, i;
+ unsigned int configs_in_chunk, conf_num = 0;
+ unsigned int chunk;
+ int max_msg_size = ph->hops->get_max_msg_size(ph);
+
+ if (!configs || type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ configs_in_chunk = (max_msg_size - sizeof(*tx)) / (sizeof(unsigned long) * 2);
+ while (conf_num < nr_configs) {
+ chunk = (nr_configs - conf_num > configs_in_chunk) ? configs_in_chunk :
+ nr_configs - conf_num;
+
+ ret = ph->xops->xfer_get_init(ph, PINCTRL_CONFIG_SET,
+ sizeof(*tx) + chunk * 2 * sizeof(unsigned long),
+ 0, &t);
+ if (ret)
+ return ret;
+
+ tx = t->tx.buf;
+ tx->identifier = cpu_to_le32(selector);
+ attributes = FIELD_PREP(GENMASK(1, 0), type) |
+ FIELD_PREP(GENMASK(9, 2), chunk);
+ tx->attributes = cpu_to_le32(attributes);
+
+ for (i = 0; i < chunk; i++) {
+ tx->configs[i * 2] = cpu_to_le32(pinconf_to_config_param(configs[i]));
+ tx->configs[i * 2 + 1] =
+ cpu_to_le32(pinconf_to_config_argument(configs[i]));
+ }
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ if (ret)
+ break;
+
+ conf_num += chunk;
+ }
+
+ return ret;
+}
+
+static int scmi_pinctrl_function_select(const struct scmi_protocol_handle *ph,
+ u32 identifier,
+ enum scmi_pinctrl_selector_type type,
+ u32 function_id)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_func_set *tx;
+
+ if (type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, identifier, type);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, PINCTRL_FUNCTION_SELECT, sizeof(*tx), 0, &t);
+ if (ret)
+ return ret;
+
+ tx = t->tx.buf;
+ tx->identifier = cpu_to_le32(identifier);
+ tx->function_id = cpu_to_le32(function_id);
+ tx->flags = cpu_to_le32(type);
+
+ ret = ph->xops->do_xfer(ph, t);
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_pinctrl_request(const struct scmi_protocol_handle *ph,
+ u32 identifier,
+ enum scmi_pinctrl_selector_type type)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_request *tx;
+
+ if (type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, identifier, type);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, PINCTRL_REQUEST, sizeof(*tx), 0, &t);
+
+ tx = t->tx.buf;
+ tx->identifier = cpu_to_le32(identifier);
+ tx->flags = cpu_to_le32(type);
+
+ ret = ph->xops->do_xfer(ph, t);
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_pinctrl_pin_request(const struct scmi_protocol_handle *ph,
+ u32 pin)
+{
+ return scmi_pinctrl_request(ph, pin, PIN_TYPE);
+}
+
+static int scmi_pinctrl_free(const struct scmi_protocol_handle *ph,
+ u32 identifier,
+ enum scmi_pinctrl_selector_type type)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_request *tx;
+
+ if (type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, identifier, type);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, PINCTRL_RELEASE, sizeof(*tx), 0, &t);
+
+ tx = t->tx.buf;
+ tx->identifier = cpu_to_le32(identifier);
+ tx->flags = cpu_to_le32(type);
+
+ ret = ph->xops->do_xfer(ph, t);
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_pinctrl_pin_free(const struct scmi_protocol_handle *ph, u32 pin)
+{
+ return scmi_pinctrl_free(ph, pin, PIN_TYPE);
+}
+
+static int scmi_pinctrl_get_group_info(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ struct scmi_group_info *group)
+{
+ int ret;
+
+ if (!group)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_attributes(ph, GROUP_TYPE, selector,
+ group->name,
+ &group->nr_pins);
+ if (ret)
+ return ret;
+
+ if (!group->nr_pins) {
+ dev_err(ph->dev, "Group %d has 0 elements", selector);
+ return -ENODATA;
+ }
+
+ group->group_pins = kmalloc_array(group->nr_pins, sizeof(*group->group_pins), GFP_KERNEL);
+ if (!group->group_pins)
+ return -ENOMEM;
+
+ ret = scmi_pinctrl_list_associations(ph, selector, GROUP_TYPE,
+ group->nr_pins, group->group_pins);
+ if (ret) {
+ kfree(group->group_pins);
+ return ret;
+ }
+
+ group->present = true;
+ return 0;
+}
+
+static int scmi_pinctrl_get_group_name(const struct scmi_protocol_handle *ph,
+ u32 selector, const char **name)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!name)
+ return -EINVAL;
+
+ if (selector >= pi->nr_groups)
+ return -EINVAL;
+
+ if (!pi->groups[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_group_info(ph, selector,
+ &pi->groups[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *name = pi->groups[selector].name;
+
+ return 0;
+}
+
+static int scmi_pinctrl_group_pins_get(const struct scmi_protocol_handle *ph,
+ u32 selector, const unsigned int **pins,
+ unsigned int *nr_pins)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!pins || !nr_pins)
+ return -EINVAL;
+
+ if (selector >= pi->nr_groups)
+ return -EINVAL;
+
+ if (!pi->groups[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_group_info(ph, selector,
+ &pi->groups[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *pins = pi->groups[selector].group_pins;
+ *nr_pins = pi->groups[selector].nr_pins;
+
+ return 0;
+}
+
+static int scmi_pinctrl_get_function_info(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ struct scmi_function_info *func)
+{
+ int ret;
+
+ if (!func)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_attributes(ph, FUNCTION_TYPE, selector,
+ func->name,
+ &func->nr_groups);
+ if (ret)
+ return ret;
+
+ if (!func->nr_groups) {
+ dev_err(ph->dev, "Function %d has 0 elements", selector);
+ return -ENODATA;
+ }
+
+ func->groups = kmalloc_array(func->nr_groups, sizeof(*func->groups), GFP_KERNEL);
+ if (!func->groups)
+ return -ENOMEM;
+
+ ret = scmi_pinctrl_list_associations(ph, selector, FUNCTION_TYPE,
+ func->nr_groups, func->groups);
+ if (ret) {
+ kfree(func->groups);
+ return ret;
+ }
+
+ func->present = true;
+ return 0;
+}
+
+static int scmi_pinctrl_get_function_name(const struct scmi_protocol_handle *ph,
+ u32 selector, const char **name)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!name)
+ return -EINVAL;
+
+ if (selector >= pi->nr_functions)
+ return -EINVAL;
+
+ if (!pi->functions[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_function_info(ph, selector,
+ &pi->functions[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *name = pi->functions[selector].name;
+ return 0;
+}
+
+static int scmi_pinctrl_function_groups_get(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ unsigned int *nr_groups,
+ const unsigned int **groups)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!groups || !nr_groups)
+ return -EINVAL;
+
+ if (selector >= pi->nr_functions)
+ return -EINVAL;
+
+ if (!pi->functions[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_function_info(ph, selector,
+ &pi->functions[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *groups = pi->functions[selector].groups;
+ *nr_groups = pi->functions[selector].nr_groups;
+
+ return 0;
+}
+
+static int scmi_pinctrl_mux_set(const struct scmi_protocol_handle *ph,
+ u32 selector, u32 group)
+{
+ return scmi_pinctrl_function_select(ph, group, GROUP_TYPE,
+ selector);
+}
+
+static int scmi_pinctrl_get_pin_info(const struct scmi_protocol_handle *ph,
+ u32 selector, struct scmi_pin_info *pin)
+{
+ int ret;
+
+ if (!pin)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_attributes(ph, PIN_TYPE, selector,
+ pin->name, NULL);
+ if (ret)
+ return ret;
+
+ pin->present = true;
+ return 0;
+}
+
+static int scmi_pinctrl_get_pin_name(const struct scmi_protocol_handle *ph,
+ u32 selector, const char **name)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!name)
+ return -EINVAL;
+
+ if (selector >= pi->nr_pins)
+ return -EINVAL;
+
+ if (!pi->pins[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_pin_info(ph, selector,
+ &pi->pins[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *name = pi->pins[selector].name;
+
+ return 0;
+}
+
+static int scmi_pinctrl_name_get(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ const char **name)
+{
+ switch (type) {
+ case PIN_TYPE:
+ return scmi_pinctrl_get_pin_name(ph, selector, name);
+ case GROUP_TYPE:
+ return scmi_pinctrl_get_group_name(ph, selector, name);
+ case FUNCTION_TYPE:
+ return scmi_pinctrl_get_function_name(ph, selector, name);
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct scmi_pinctrl_proto_ops pinctrl_proto_ops = {
+ .count_get = scmi_pinctrl_count_get,
+ .name_get = scmi_pinctrl_name_get,
+ .group_pins_get = scmi_pinctrl_group_pins_get,
+ .function_groups_get = scmi_pinctrl_function_groups_get,
+ .mux_set = scmi_pinctrl_mux_set,
+ .config_get = scmi_pinctrl_config_get,
+ .config_get_all = scmi_pinctrl_config_get_all,
+ .config_set = scmi_pinctrl_config_set,
+ .pin_request = scmi_pinctrl_pin_request,
+ .pin_free = scmi_pinctrl_pin_free,
+};
+
+static int scmi_pinctrl_protocol_init(const struct scmi_protocol_handle *ph)
+{
+ int ret;
+ u32 version;
+ struct scmi_pinctrl_info *pinfo;
+
+ ret = ph->xops->version_get(ph, &version);
+ if (ret)
+ return ret;
+
+ dev_dbg(ph->dev, "Pinctrl Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ pinfo = devm_kzalloc(ph->dev, sizeof(*pinfo), GFP_KERNEL);
+ if (!pinfo)
+ return -ENOMEM;
+
+ ret = scmi_pinctrl_attributes_get(ph, pinfo);
+ if (ret)
+ return ret;
+
+ pinfo->pins = devm_kcalloc(ph->dev, pinfo->nr_pins,
+ sizeof(*pinfo->pins),
+ GFP_KERNEL);
+ if (!pinfo->pins)
+ return -ENOMEM;
+
+ pinfo->groups = devm_kcalloc(ph->dev, pinfo->nr_groups,
+ sizeof(*pinfo->groups),
+ GFP_KERNEL);
+ if (!pinfo->groups)
+ return -ENOMEM;
+
+ pinfo->functions = devm_kcalloc(ph->dev, pinfo->nr_functions,
+ sizeof(*pinfo->functions),
+ GFP_KERNEL);
+ if (!pinfo->functions)
+ return -ENOMEM;
+
+ pinfo->version = version;
+
+ return ph->set_priv(ph, pinfo, version);
+}
+
+static int scmi_pinctrl_protocol_deinit(const struct scmi_protocol_handle *ph)
+{
+ int i;
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ for (i = 0; i < pi->nr_groups; i++) {
+ if (pi->groups[i].present) {
+ kfree(pi->groups[i].group_pins);
+ pi->groups[i].present = false;
+ }
+ }
+
+ for (i = 0; i < pi->nr_functions; i++) {
+ if (pi->functions[i].present) {
+ kfree(pi->functions[i].groups);
+ pi->functions[i].present = false;
+ }
+ }
+
+ return 0;
+}
+
+static const struct scmi_protocol scmi_pinctrl = {
+ .id = SCMI_PROTOCOL_PINCTRL,
+ .owner = THIS_MODULE,
+ .instance_init = &scmi_pinctrl_protocol_init,
+ .instance_deinit = &scmi_pinctrl_protocol_deinit,
+ .ops = &pinctrl_proto_ops,
+ .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
+};
+
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(pinctrl, scmi_pinctrl)
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index 08de8dc064c1..e30f91ce53c8 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -347,6 +347,7 @@ void __exit scmi_##name##_unregister(void) \
DECLARE_SCMI_REGISTER_UNREGISTER(base);
DECLARE_SCMI_REGISTER_UNREGISTER(clock);
DECLARE_SCMI_REGISTER_UNREGISTER(perf);
+DECLARE_SCMI_REGISTER_UNREGISTER(pinctrl);
DECLARE_SCMI_REGISTER_UNREGISTER(power);
DECLARE_SCMI_REGISTER_UNREGISTER(reset);
DECLARE_SCMI_REGISTER_UNREGISTER(sensors);
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 19a06b872afe..516c98252732 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -717,6 +717,51 @@ struct scmi_powercap_proto_ops {
u32 *power_thresh_high);
};
+enum scmi_pinctrl_selector_type {
+ PIN_TYPE = 0,
+ GROUP_TYPE,
+ FUNCTION_TYPE,
+};
+
+/**
+ * struct scmi_pinctrl_proto_ops - represents the various operations provided
+ * by SCMI Pinctrl Protocol
+ *
+ * @count_get: returns count of the registered elements in given type
+ * @name_get: returns name by index of given type
+ * @group_pins_get: returns the set of pins, assigned to the specified group
+ * @function_groups_get: returns the set of groups, assigned to the specified
+ * function
+ * @mux_set: set muxing function for groups of pins
+ * @config_get: returns configuration parameter for pin or group
+ * @config_set: sets the configuration parameter for pin or group
+ * @pin_request: aquire pin before selecting mux setting
+ * @pin_free: frees pin, acquired by request_pin call
+ */
+struct scmi_pinctrl_proto_ops {
+ int (*count_get)(const struct scmi_protocol_handle *ph,
+ enum scmi_pinctrl_selector_type type);
+ int (*name_get)(const struct scmi_protocol_handle *ph, u32 selector,
+ enum scmi_pinctrl_selector_type type, const char **name);
+ int (*group_pins_get)(const struct scmi_protocol_handle *ph, u32 selector,
+ const unsigned int **pins, unsigned int *nr_pins);
+ int (*function_groups_get)(const struct scmi_protocol_handle *ph, u32 selector,
+ unsigned int *nr_groups, const unsigned int **groups);
+ int (*mux_set)(const struct scmi_protocol_handle *ph, u32 selector, u32 group);
+ int (*config_get)(const struct scmi_protocol_handle *ph, u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ u8 config_type, unsigned long *config_value);
+ int (*config_get_all)(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type, u16 size,
+ u8 *config_types, unsigned long *config_values);
+ int (*config_set)(const struct scmi_protocol_handle *ph, u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ unsigned long *configs, unsigned int nr_configs);
+ int (*pin_request)(const struct scmi_protocol_handle *ph, u32 pin);
+ int (*pin_free)(const struct scmi_protocol_handle *ph, u32 pin);
+};
+
/**
* struct scmi_notify_ops - represents notifications' operations provided by
* SCMI core
@@ -824,6 +869,7 @@ enum scmi_std_protocol {
SCMI_PROTOCOL_RESET = 0x16,
SCMI_PROTOCOL_VOLTAGE = 0x17,
SCMI_PROTOCOL_POWERCAP = 0x18,
+ SCMI_PROTOCOL_PINCTRL = 0x19,
};
enum scmi_system_events {
--
2.37.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 6/7] pinctrl: Implementation of the generic scmi-pinctrl driver
2023-12-15 11:56 [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
` (4 preceding siblings ...)
2023-12-15 11:56 ` [PATCH 5/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
@ 2023-12-15 11:56 ` Peng Fan (OSS)
2023-12-22 16:35 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map Peng Fan (OSS)
6 siblings, 1 reply; 25+ messages in thread
From: Peng Fan (OSS) @ 2023-12-15 11:56 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
scmi-pinctrl driver implements pinctrl driver interface and using
SCMI protocol to redirect messages from pinctrl subsystem SDK to
SCMI platform firmware, which does the changes in HW.
Co-developed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
MAINTAINERS | 1 +
drivers/pinctrl/Kconfig | 11 ++
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-scmi.c | 403 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 416 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8d971adeee22..0ca47b9c9f55 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21172,6 +21172,7 @@ F: drivers/clk/clk-sc[mp]i.c
F: drivers/cpufreq/sc[mp]i-cpufreq.c
F: drivers/firmware/arm_scmi/
F: drivers/firmware/arm_scpi.c
+F: drivers/pinctrl/pinctrl-scmi.c
F: drivers/pmdomain/arm/
F: drivers/powercap/arm_scmi_powercap.c
F: drivers/regulator/scmi-regulator.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 8163a5983166..27b26e428f60 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -432,6 +432,17 @@ config PINCTRL_ROCKCHIP
help
This support pinctrl and GPIO driver for Rockchip SoCs.
+config PINCTRL_SCMI
+ tristate "Pinctrl driver using SCMI protocol interface"
+ depends on ARM_SCMI_PROTOCOL || COMPILE_TEST
+ select PINMUX
+ select GENERIC_PINCONF
+ help
+ This driver provides support for pinctrl which is controlled
+ by firmware that implements the SCMI interface.
+ It uses SCMI Message Protocol to interact with the
+ firmware providing all the pinctrl controls.
+
config PINCTRL_SINGLE
tristate "One-register-per-pin type device tree based pinctrl driver"
depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 1071f301cc70..ba755ed2d46c 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_PINCTRL_PIC32) += pinctrl-pic32.o
obj-$(CONFIG_PINCTRL_PISTACHIO) += pinctrl-pistachio.o
obj-$(CONFIG_PINCTRL_RK805) += pinctrl-rk805.o
obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o
+obj-$(CONFIG_PINCTRL_SCMI) += pinctrl-scmi.o
obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o
obj-$(CONFIG_PINCTRL_ST) += pinctrl-st.o
obj-$(CONFIG_PINCTRL_STMFX) += pinctrl-stmfx.o
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
new file mode 100644
index 000000000000..0710ce9a1b42
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -0,0 +1,403 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Power Interface (SCMI) Protocol based pinctrl driver
+ *
+ * Copyright (C) 2023 EPAM
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/seq_file.h>
+#include <linux/scmi_protocol.h>
+#include <linux/slab.h>
+
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "pinctrl-utils.h"
+#include "core.h"
+#include "pinconf.h"
+
+#define DRV_NAME "scmi-pinctrl"
+
+static const struct scmi_pinctrl_proto_ops *pinctrl_ops;
+
+struct scmi_pinctrl_funcs {
+ unsigned int num_groups;
+ const char **groups;
+};
+
+struct scmi_pinctrl {
+ struct device *dev;
+ struct scmi_protocol_handle *ph;
+ struct pinctrl_dev *pctldev;
+ struct pinctrl_desc pctl_desc;
+ struct scmi_pinctrl_funcs *functions;
+ unsigned int nr_functions;
+ char **groups;
+ unsigned int nr_groups;
+ struct pinctrl_pin_desc *pins;
+ unsigned int nr_pins;
+};
+
+static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->count_get(pmx->ph, GROUP_TYPE);
+}
+
+static const char *pinctrl_scmi_get_group_name(struct pinctrl_dev *pctldev, unsigned int selector)
+{
+ int ret;
+ const char *name;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ ret = pinctrl_ops->name_get(pmx->ph, selector, GROUP_TYPE, &name);
+ if (ret) {
+ dev_err(pmx->dev, "get name failed with err %d", ret);
+ return NULL;
+ }
+
+ return name;
+}
+
+static int pinctrl_scmi_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->group_pins_get(pmx->ph, selector, pins, num_pins);
+}
+
+static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
+ .get_groups_count = pinctrl_scmi_get_groups_count,
+ .get_group_name = pinctrl_scmi_get_group_name,
+ .get_group_pins = pinctrl_scmi_get_group_pins,
+#ifdef CONFIG_OF
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
+ .dt_free_map = pinconf_generic_dt_free_map,
+#endif
+};
+
+static int pinctrl_scmi_get_functions_count(struct pinctrl_dev *pctldev)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->count_get(pmx->ph, FUNCTION_TYPE);
+}
+
+static const char *pinctrl_scmi_get_function_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ int ret;
+ const char *name;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ ret = pinctrl_ops->name_get(pmx->ph, selector, FUNCTION_TYPE, &name);
+ if (ret) {
+ dev_err(pmx->dev, "get name failed with err %d", ret);
+ return NULL;
+ }
+
+ return name;
+}
+
+static int pinctrl_scmi_get_function_groups(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const char * const **groups,
+ unsigned int * const num_groups)
+{
+ const unsigned int *group_ids;
+ int ret, i;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ if (!groups || !num_groups)
+ return -EINVAL;
+
+ if (selector < pmx->nr_functions &&
+ pmx->functions[selector].num_groups) {
+ *groups = (const char * const *)pmx->functions[selector].groups;
+ *num_groups = pmx->functions[selector].num_groups;
+ return 0;
+ }
+
+ ret = pinctrl_ops->function_groups_get(pmx->ph, selector,
+ &pmx->functions[selector].num_groups,
+ &group_ids);
+ if (ret) {
+ dev_err(pmx->dev, "Unable to get function groups, err %d", ret);
+ return ret;
+ }
+
+ *num_groups = pmx->functions[selector].num_groups;
+ if (!*num_groups)
+ return -EINVAL;
+
+ pmx->functions[selector].groups =
+ devm_kcalloc(pmx->dev, *num_groups, sizeof(*pmx->functions[selector].groups),
+ GFP_KERNEL);
+ if (!pmx->functions[selector].groups)
+ return -ENOMEM;
+
+ for (i = 0; i < *num_groups; i++) {
+ pmx->functions[selector].groups[i] =
+ pinctrl_scmi_get_group_name(pmx->pctldev,
+ group_ids[i]);
+ if (!pmx->functions[selector].groups[i]) {
+ ret = -ENOMEM;
+ goto err_free;
+ }
+ }
+
+ *groups = (const char * const *)pmx->functions[selector].groups;
+
+ return 0;
+
+err_free:
+ devm_kfree(pmx->dev, pmx->functions[selector].groups);
+
+ return ret;
+}
+
+static int pinctrl_scmi_func_set_mux(struct pinctrl_dev *pctldev, unsigned int selector,
+ unsigned int group)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->mux_set(pmx->ph, selector, group);
+}
+
+static int pinctrl_scmi_request(struct pinctrl_dev *pctldev, unsigned int offset)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->pin_request(pmx->ph, offset);
+}
+
+static int pinctrl_scmi_free(struct pinctrl_dev *pctldev, unsigned int offset)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->pin_free(pmx->ph, offset);
+}
+
+static const struct pinmux_ops pinctrl_scmi_pinmux_ops = {
+ .request = pinctrl_scmi_request,
+ .free = pinctrl_scmi_free,
+ .get_functions_count = pinctrl_scmi_get_functions_count,
+ .get_function_name = pinctrl_scmi_get_function_name,
+ .get_function_groups = pinctrl_scmi_get_function_groups,
+ .set_mux = pinctrl_scmi_func_set_mux,
+};
+
+static int pinctrl_scmi_pinconf_get(struct pinctrl_dev *pctldev, unsigned int _pin,
+ unsigned long *config)
+{
+ int ret;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param config_type;
+ unsigned long config_value;
+
+ if (!config)
+ return -EINVAL;
+
+ config_type = pinconf_to_config_param(*config);
+
+ ret = pinctrl_ops->config_get(pmx->ph, _pin, PIN_TYPE, config_type, &config_value);
+ if (ret)
+ return ret;
+
+ *config = pinconf_to_config_packed(config_type, config_value);
+
+ return 0;
+}
+
+static int pinctrl_scmi_pinconf_set(struct pinctrl_dev *pctldev,
+ unsigned int _pin,
+ unsigned long *configs,
+ unsigned int num_configs)
+{
+ int ret;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ if (!configs || !num_configs)
+ return -EINVAL;
+
+ ret = pinctrl_ops->config_set(pmx->ph, _pin, PIN_TYPE, configs, num_configs);
+ if (ret)
+ dev_err(pmx->dev, "Error parsing config %d\n", ret);
+
+ return ret;
+}
+
+static int pinctrl_scmi_pinconf_group_set(struct pinctrl_dev *pctldev,
+ unsigned int group,
+ unsigned long *configs,
+ unsigned int num_configs)
+{
+ int ret;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ if (!configs || !num_configs)
+ return -EINVAL;
+
+ ret = pinctrl_ops->config_set(pmx->ph, group, GROUP_TYPE, configs, num_configs);
+ if (ret)
+ dev_err(pmx->dev, "Error parsing config %d", ret);
+
+ return ret;
+};
+
+static int pinctrl_scmi_pinconf_group_get(struct pinctrl_dev *pctldev,
+ unsigned int group,
+ unsigned long *config)
+{
+ int ret;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param config_type;
+ unsigned long config_value;
+
+ if (!config)
+ return -EINVAL;
+
+ config_type = pinconf_to_config_param(*config);
+
+ ret = pinctrl_ops->config_get(pmx->ph, group, GROUP_TYPE, config_type, &config_value);
+ if (ret)
+ return ret;
+
+ *config = pinconf_to_config_packed(config_type, config_value);
+
+ return 0;
+}
+
+static const struct pinconf_ops pinctrl_scmi_pinconf_ops = {
+ .is_generic = true,
+ .pin_config_get = pinctrl_scmi_pinconf_get,
+ .pin_config_set = pinctrl_scmi_pinconf_set,
+ .pin_config_group_set = pinctrl_scmi_pinconf_group_set,
+ .pin_config_group_get = pinctrl_scmi_pinconf_group_get,
+ .pin_config_config_dbg_show = pinconf_generic_dump_config,
+};
+
+static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
+ unsigned int *nr_pins,
+ const struct pinctrl_pin_desc **pins)
+{
+ int ret, i;
+
+ if (!pins || !nr_pins)
+ return -EINVAL;
+
+ if (pmx->nr_pins) {
+ *pins = pmx->pins;
+ *nr_pins = pmx->nr_pins;
+ return 0;
+ }
+
+ *nr_pins = pinctrl_ops->count_get(pmx->ph, PIN_TYPE);
+
+ pmx->nr_pins = *nr_pins;
+ pmx->pins = devm_kmalloc_array(pmx->dev, *nr_pins, sizeof(*pmx->pins), GFP_KERNEL);
+ if (!pmx->pins)
+ return -ENOMEM;
+
+ for (i = 0; i < *nr_pins; i++) {
+ pmx->pins[i].number = i;
+ ret = pinctrl_ops->name_get(pmx->ph, i, PIN_TYPE, &pmx->pins[i].name);
+ if (ret) {
+ dev_err(pmx->dev, "Can't get name for pin %d: rc %d", i, ret);
+ pmx->nr_pins = 0;
+ return ret;
+ }
+ }
+
+ *pins = pmx->pins;
+ dev_dbg(pmx->dev, "got pins %d", *nr_pins);
+
+ return 0;
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_PINCTRL, "pinctrl" },
+ { }
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static int scmi_pinctrl_probe(struct scmi_device *sdev)
+{
+ int ret;
+ struct device *dev = &sdev->dev;
+ struct scmi_pinctrl *pmx;
+ const struct scmi_handle *handle;
+ struct scmi_protocol_handle *ph;
+
+ if (!sdev || !sdev->handle)
+ return -EINVAL;
+
+ handle = sdev->handle;
+
+ pinctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PINCTRL, &ph);
+ if (IS_ERR(pinctrl_ops))
+ return PTR_ERR(pinctrl_ops);
+
+ pmx = devm_kzalloc(dev, sizeof(*pmx), GFP_KERNEL);
+ if (!pmx)
+ return -ENOMEM;
+
+ pmx->ph = ph;
+
+ pmx->dev = dev;
+ pmx->pctl_desc.name = DRV_NAME;
+ pmx->pctl_desc.owner = THIS_MODULE;
+ pmx->pctl_desc.pctlops = &pinctrl_scmi_pinctrl_ops;
+ pmx->pctl_desc.pmxops = &pinctrl_scmi_pinmux_ops;
+ pmx->pctl_desc.confops = &pinctrl_scmi_pinconf_ops;
+
+ ret = pinctrl_scmi_get_pins(pmx, &pmx->pctl_desc.npins,
+ &pmx->pctl_desc.pins);
+ if (ret)
+ return ret;
+
+ ret = devm_pinctrl_register_and_init(dev, &pmx->pctl_desc, pmx, &pmx->pctldev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register pinctrl\n");
+
+ pmx->nr_functions = pinctrl_scmi_get_functions_count(pmx->pctldev);
+ pmx->nr_groups = pinctrl_scmi_get_groups_count(pmx->pctldev);
+
+ if (pmx->nr_functions) {
+ pmx->functions =
+ devm_kcalloc(dev, pmx->nr_functions, sizeof(*pmx->functions),
+ GFP_KERNEL);
+ if (!pmx->functions)
+ return -ENOMEM;
+ }
+
+ if (pmx->nr_groups) {
+ pmx->groups =
+ devm_kcalloc(dev, pmx->nr_groups, sizeof(*pmx->groups), GFP_KERNEL);
+ if (!pmx->groups)
+ return -ENOMEM;
+ }
+
+ return pinctrl_enable(pmx->pctldev);
+}
+
+static struct scmi_driver scmi_pinctrl_driver = {
+ .name = DRV_NAME,
+ .probe = scmi_pinctrl_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_pinctrl_driver);
+
+MODULE_AUTHOR("Oleksii Moisieiev <oleksii_moisieiev@epam.com>");
+MODULE_DESCRIPTION("ARM SCMI pin controller driver");
+MODULE_LICENSE("GPL");
--
2.37.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map
2023-12-15 11:56 [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
` (5 preceding siblings ...)
2023-12-15 11:56 ` [PATCH 6/7] pinctrl: Implementation of the generic scmi-pinctrl driver Peng Fan (OSS)
@ 2023-12-15 11:56 ` Peng Fan (OSS)
2023-12-22 17:43 ` Cristian Marussi
6 siblings, 1 reply; 25+ messages in thread
From: Peng Fan (OSS) @ 2023-12-15 11:56 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
i.MX95 System Manager FW supports SCMI PINCTRL protocol, but uses
OEM Pin Configuration type, so need i.MX specific dt_node_to_map.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/pinctrl/Makefile | 2 +-
drivers/pinctrl/pinctrl-scmi-imx.c | 117 +++++++++++++++++++++++++++++++++++++
drivers/pinctrl/pinctrl-scmi.c | 16 +++++
drivers/pinctrl/pinctrl-scmi.h | 12 ++++
4 files changed, 146 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index ba755ed2d46c..d96b7ede1355 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -44,7 +44,7 @@ obj-$(CONFIG_PINCTRL_PIC32) += pinctrl-pic32.o
obj-$(CONFIG_PINCTRL_PISTACHIO) += pinctrl-pistachio.o
obj-$(CONFIG_PINCTRL_RK805) += pinctrl-rk805.o
obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o
-obj-$(CONFIG_PINCTRL_SCMI) += pinctrl-scmi.o
+obj-$(CONFIG_PINCTRL_SCMI) += pinctrl-scmi.o pinctrl-scmi-imx.o
obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o
obj-$(CONFIG_PINCTRL_ST) += pinctrl-st.o
obj-$(CONFIG_PINCTRL_STMFX) += pinctrl-stmfx.o
diff --git a/drivers/pinctrl/pinctrl-scmi-imx.c b/drivers/pinctrl/pinctrl-scmi-imx.c
new file mode 100644
index 000000000000..e9d02e4c2cc1
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-scmi-imx.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2023 NXP
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "pinctrl-scmi.h"
+#include "pinctrl-utils.h"
+#include "core.h"
+#include "pinconf.h"
+
+/* SCMI pin control types, aligned with SCMI firmware */
+#define IMX_SCMI_PIN_TYPE_MUX 192
+#define IMX_SCMI_PIN_TYPE_CONFIG 193
+#define IMX_SCMI_PIN_TYPE_DAISY_ID 194
+#define IMX_SCMI_PIN_TYPE_DAISY_CFG 195
+
+#define IMX_SCMI_NO_PAD_CTL BIT(31)
+#define IMX_SCMI_PAD_SION BIT(30)
+#define IMX_SCMI_IOMUXC_CONFIG_SION BIT(4)
+
+#define IMX_SCMI_NUM_CFG 4
+#define IMX_SCMI_PIN_SIZE 24
+
+int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node *np,
+ struct pinctrl_map **map, unsigned int *num_maps)
+{
+ struct pinctrl_map *new_map;
+ const __be32 *list;
+ unsigned long *configs = NULL;
+ unsigned long cfg[IMX_SCMI_NUM_CFG];
+ int map_num, size, pin_size, pin_id, num_pins;
+ int mux_reg, conf_reg, input_reg, mux_val, conf_val, input_val;
+ int i, j;
+ uint32_t ncfg;
+ static uint32_t daisy_off;
+
+ if (!daisy_off) {
+ if (of_machine_is_compatible("fsl,imx95"))
+ daisy_off = 0x408;
+ else
+ dev_err(pctldev->dev, "platform not support scmi pinctrl\n");
+ }
+
+ list = of_get_property(np, "fsl,pins", &size);
+ if (!list) {
+ dev_err(pctldev->dev, "no fsl,pins property in node %pOF\n", np);
+ return -EINVAL;
+ }
+
+ pin_size = IMX_SCMI_PIN_SIZE;
+
+ if (!size || size % pin_size) {
+ dev_err(pctldev->dev, "Invalid fsl,pins or pins property in node %pOF\n", np);
+ return -EINVAL;
+ }
+
+ num_pins = size / pin_size;
+ map_num = num_pins;
+
+ new_map = kmalloc_array(map_num, sizeof(struct pinctrl_map),
+ GFP_KERNEL);
+ if (!new_map)
+ return -ENOMEM;
+
+ *map = new_map;
+ *num_maps = map_num;
+
+ /* create config map */
+ for (i = 0; i < num_pins; i++) {
+ j = 0;
+ ncfg = IMX_SCMI_NUM_CFG;
+ mux_reg = be32_to_cpu(*list++);
+ conf_reg = be32_to_cpu(*list++);
+ input_reg = be32_to_cpu(*list++);
+ mux_val = be32_to_cpu(*list++);
+ input_val = be32_to_cpu(*list++);
+ conf_val = be32_to_cpu(*list++);
+ if (conf_val & IMX_SCMI_PAD_SION)
+ mux_val |= IMX_SCMI_IOMUXC_CONFIG_SION;
+
+ pin_id = mux_reg / 4;
+
+ cfg[j++] = pinconf_to_config_packed(IMX_SCMI_PIN_TYPE_MUX, mux_val);
+
+ if (!conf_reg || (conf_val & IMX_SCMI_NO_PAD_CTL))
+ ncfg--;
+ else
+ cfg[j++] = pinconf_to_config_packed(IMX_SCMI_PIN_TYPE_CONFIG, conf_val);
+
+ if (!input_reg) {
+ ncfg -= 2;
+ } else {
+ cfg[j++] = pinconf_to_config_packed(IMX_SCMI_PIN_TYPE_DAISY_ID,
+ (input_reg - daisy_off) / 4);
+ cfg[j++] = pinconf_to_config_packed(IMX_SCMI_PIN_TYPE_DAISY_CFG, input_val);
+ }
+
+ configs = kmemdup(cfg, ncfg * sizeof(unsigned long), GFP_KERNEL);
+
+ new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
+ new_map[i].data.configs.group_or_pin = pin_get_name(pctldev, pin_id);
+ new_map[i].data.configs.configs = configs;
+ new_map[i].data.configs.num_configs = ncfg;
+ }
+
+ return 0;
+}
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 0710ce9a1b42..e677e2064ba7 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -8,6 +8,7 @@
#include <linux/device.h>
#include <linux/err.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/seq_file.h>
#include <linux/scmi_protocol.h>
#include <linux/slab.h>
@@ -18,6 +19,7 @@
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
+#include "pinctrl-scmi.h"
#include "pinctrl-utils.h"
#include "core.h"
#include "pinconf.h"
@@ -86,6 +88,16 @@ static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
#endif
};
+static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
+ .get_groups_count = pinctrl_scmi_get_groups_count,
+ .get_group_name = pinctrl_scmi_get_group_name,
+ .get_group_pins = pinctrl_scmi_get_group_pins,
+#ifdef CONFIG_OF
+ .dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
+ .dt_free_map = pinconf_generic_dt_free_map,
+#endif
+};
+
static int pinctrl_scmi_get_functions_count(struct pinctrl_dev *pctldev)
{
struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
@@ -327,6 +339,7 @@ static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
static const struct scmi_device_id scmi_id_table[] = {
{ SCMI_PROTOCOL_PINCTRL, "pinctrl" },
+ { SCMI_PROTOCOL_PINCTRL, "pinctrl-scmi-imx", "fsl,imx95-scmi-pinctrl" },
{ }
};
MODULE_DEVICE_TABLE(scmi, scmi_id_table);
@@ -361,6 +374,9 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
pmx->pctl_desc.pmxops = &pinctrl_scmi_pinmux_ops;
pmx->pctl_desc.confops = &pinctrl_scmi_pinconf_ops;
+ if (device_is_compatible(dev, "fsl,imx95-scmi-pinctrl"))
+ pmx->pctl_desc.pctlops = &pinctrl_scmi_imx_pinctrl_ops;
+
ret = pinctrl_scmi_get_pins(pmx, &pmx->pctl_desc.npins,
&pmx->pctl_desc.pins);
if (ret)
diff --git a/drivers/pinctrl/pinctrl-scmi.h b/drivers/pinctrl/pinctrl-scmi.h
new file mode 100644
index 000000000000..25863b4428fe
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-scmi.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2023 NXP
+ */
+
+#ifndef __DRIVERS_PINCTRL_SCMI_H
+#define __DRIVERS_PINCTRL_SCMI_H
+
+int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node *np,
+ struct pinctrl_map **map, unsigned int *num_maps);
+
+#endif /* __DRIVERS_PINCTRL_SCMI_H */
--
2.37.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-15 11:56 ` [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol Peng Fan (OSS)
@ 2023-12-15 20:11 ` Rob Herring
2023-12-16 4:44 ` Peng Fan
2023-12-19 19:29 ` Marco Felsch
` (2 subsequent siblings)
3 siblings, 1 reply; 25+ messages in thread
From: Rob Herring @ 2023-12-15 20:11 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Cristian Marussi, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
On Fri, Dec 15, 2023 at 07:56:32PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add SCMI v3.2 pinctrl protocol bindings and example.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> .../devicetree/bindings/firmware/arm,scmi.yaml | 99 ++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 4591523b51a0..bfd2b6a89979 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -247,6 +247,85 @@ properties:
> reg:
> const: 0x18
>
> + protocol@19:
> + type: object
> + allOf:
> + - $ref: '#/$defs/protocol-node'
> + - $ref: /schemas/pinctrl/pinctrl.yaml
> + - if:
> + properties:
> + compatible:
> + const: fsl,imx95-scmi-pinctrl
> + then:
> + patternProperties:
> + "grp$": false
> + "-pins$": true
> + else:
> + patternProperties:
> + "grp$": false
> + "-pins$": true
> + unevaluatedProperties: false
This will not scale if each vendor adds to arm,scmi.yaml. You need to
move this to its own file. That can have a ref to
/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node.
Here you can just say compatible is "fsl,imx95-scmi-pinctrl" and
'additionalProperties: true'.
> +
> + properties:
> + reg:
> + const: 0x19
> +
> + '#pinctrl-cells':
> + const: 0
Generally not used if 0.
> +
> + compatible:
> + const: fsl,imx95-scmi-pinctrl
> +
> + patternProperties:
> + '-pins$':
> + type: object
> + allOf:
> + - $ref: /schemas/pinctrl/pincfg-node.yaml#
> + - $ref: /schemas/pinctrl/pinmux-node.yaml#
> + unevaluatedProperties: false
> +
> + description:
> + A pin multiplexing sub-node describe how to configure a
> + set of pins is some desired function.
> + A single sub-node may define several pin configurations.
> + This sub-node is using default pinctrl bindings to configure
> + pin multiplexing and using SCMI protocol to apply specified
> + configuration using SCMI protocol.
> +
> + 'grp$':
> + type: object
> + description:
> + Pinctrl node's client devices use subnodes for desired pin configuration.
> + Client device subnodes use below standard properties.
> +
> + properties:
> + fsl,pins:
> + description:
> + each entry consists of 6 integers and represents the mux and config
> + setting for one pin. The first 5 integers <mux_reg conf_reg input_reg
> + mux_val input_val> are specified using a PIN_FUNC_ID macro, which can
> + be found in <arch/arm64/boot/dts/freescale/imx95-pinfunc.h>. The last
> + integer CONFIG is the pad setting value like pull-up on this pin. Please
> + refer to i.MX95 Plus Reference Manual for detailed CONFIG settings.
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + items:
> + items:
> + - description: |
> + "mux_reg" indicates the offset of mux register.
> + - description: |
> + "conf_reg" indicates the offset of pad configuration register.
> + - description: |
> + "input_reg" indicates the offset of select input register.
> + - description: |
> + "mux_val" indicates the mux value to be applied.
> + - description: |
> + "input_val" indicates the select input value to be applied.
> + - description: |
> + "pad_setting" indicates the pad configuration value to be applied.
> +
> + required:
> + - reg
> +
> additionalProperties: false
>
> $defs:
> @@ -401,6 +480,26 @@ examples:
> scmi_powercap: protocol@18 {
> reg = <0x18>;
> };
> +
> + scmi_pinctrl: protocol@19 {
> + reg = <0x19>;
> + #pinctrl-cells = <0>;
Missing compatible. Schema should catch that...
> +
> + i2c2-pins {
> + groups = "i2c2_a", "i2c2_b";
> + function = "i2c2";
> + };
> +
> + mdio-pins {
> + groups = "avb_mdio";
> + drive-strength = <24>;
> + };
> +
> + keys_pins: keys-pins {
> + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> + bias-pull-up;
> + };
> + };
> };
> };
>
>
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-15 20:11 ` Rob Herring
@ 2023-12-16 4:44 ` Peng Fan
0 siblings, 0 replies; 25+ messages in thread
From: Peng Fan @ 2023-12-16 4:44 UTC (permalink / raw)
To: Rob Herring, Peng Fan (OSS)
Cc: Sudeep Holla, Cristian Marussi, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-gpio@vger.kernel.org
Hi Rob,
> Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl
> protocol
>
> On Fri, Dec 15, 2023 at 07:56:32PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add SCMI v3.2 pinctrl protocol bindings and example.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > .../devicetree/bindings/firmware/arm,scmi.yaml | 99
> ++++++++++++++++++++++
> > 1 file changed, 99 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > index 4591523b51a0..bfd2b6a89979 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > @@ -247,6 +247,85 @@ properties:
> > reg:
> > const: 0x18
> >
> > + protocol@19:
> > + type: object
> > + allOf:
> > + - $ref: '#/$defs/protocol-node'
> > + - $ref: /schemas/pinctrl/pinctrl.yaml
> > + - if:
> > + properties:
> > + compatible:
> > + const: fsl,imx95-scmi-pinctrl
> > + then:
> > + patternProperties:
> > + "grp$": false
> > + "-pins$": true
> > + else:
> > + patternProperties:
> > + "grp$": false
> > + "-pins$": true
> > + unevaluatedProperties: false
>
> This will not scale if each vendor adds to arm,scmi.yaml. You need to move
> this to its own file. That can have a ref to
> /schemas/firmware/arm,scmi.yaml#/$defs/protocol-node.
>
> Here you can just say compatible is "fsl,imx95-scmi-pinctrl" and
> 'additionalProperties: true'.
Do you have an example for this? I am not able write a correct
one to pass the dt check.
- if:
properties:
compatible:
const: fsl,imx95-scmi-pinctrl
then:
properties:
additionalProperties: true
.......
compatible: true
And in fsl,imx95-scmi-pinctrl.yaml
allOf:
- $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'
properties:
reg:
const: 0x19
compatible:
const: fsl,imx95-scmi-pinctrl
patternProperties:
'grp$':
type: object
description:
Pinctrl node's client devices use subnodes for desired pin configuration.
Client device subnodes use below standard properties.
properties:
fsl,pins:
description:
.....
>
> > +
> > + properties:
> > + reg:
> > + const: 0x19
> > +
> > + '#pinctrl-cells':
> > + const: 0
>
> Generally not used if 0.
Will drop this property.
>
> > +
> > + compatible:
> > + const: fsl,imx95-scmi-pinctrl
> > +
> > + patternProperties:
> > + '-pins$':
> > + type: object
> > + allOf:
> > + - $ref: /schemas/pinctrl/pincfg-node.yaml#
> > + - $ref: /schemas/pinctrl/pinmux-node.yaml#
> > + unevaluatedProperties: false
> > +
> > + description:
> > + A pin multiplexing sub-node describe how to configure a
> > + set of pins is some desired function.
> > + A single sub-node may define several pin configurations.
> > + This sub-node is using default pinctrl bindings to configure
> > + pin multiplexing and using SCMI protocol to apply specified
> > + configuration using SCMI protocol.
> > +
> > + 'grp$':
> > + type: object
> > + description:
> > + Pinctrl node's client devices use subnodes for desired pin
> configuration.
> > + Client device subnodes use below standard properties.
> > +
> > + properties:
> > + fsl,pins:
> > + description:
> > + each entry consists of 6 integers and represents the mux and
> config
> > + setting for one pin. The first 5 integers <mux_reg conf_reg
> input_reg
> > + mux_val input_val> are specified using a PIN_FUNC_ID macro,
> which can
> > + be found in <arch/arm64/boot/dts/freescale/imx95-pinfunc.h>.
> The last
> > + integer CONFIG is the pad setting value like pull-up on this pin.
> Please
> > + refer to i.MX95 Plus Reference Manual for detailed CONFIG
> settings.
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > + items:
> > + items:
> > + - description: |
> > + "mux_reg" indicates the offset of mux register.
> > + - description: |
> > + "conf_reg" indicates the offset of pad configuration register.
> > + - description: |
> > + "input_reg" indicates the offset of select input register.
> > + - description: |
> > + "mux_val" indicates the mux value to be applied.
> > + - description: |
> > + "input_val" indicates the select input value to be applied.
> > + - description: |
> > + "pad_setting" indicates the pad configuration value to be
> applied.
> > +
> > + required:
> > + - reg
> > +
> > additionalProperties: false
> >
> > $defs:
> > @@ -401,6 +480,26 @@ examples:
> > scmi_powercap: protocol@18 {
> > reg = <0x18>;
> > };
> > +
> > + scmi_pinctrl: protocol@19 {
> > + reg = <0x19>;
> > + #pinctrl-cells = <0>;
>
> Missing compatible. Schema should catch that...
Only i.MX need compatible here.
Thanks,
Peng.
>
> > +
> > + i2c2-pins {
> > + groups = "i2c2_a", "i2c2_b";
> > + function = "i2c2";
> > + };
> > +
> > + mdio-pins {
> > + groups = "avb_mdio";
> > + drive-strength = <24>;
> > + };
> > +
> > + keys_pins: keys-pins {
> > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> > + bias-pull-up;
> > + };
> > + };
> > };
> > };
> >
> >
> > --
> > 2.37.1
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-15 11:56 ` [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol Peng Fan (OSS)
2023-12-15 20:11 ` Rob Herring
@ 2023-12-19 19:29 ` Marco Felsch
2023-12-20 1:58 ` Peng Fan
2023-12-20 11:42 ` Linus Walleij
2023-12-22 17:46 ` Cristian Marussi
3 siblings, 1 reply; 25+ messages in thread
From: Marco Felsch @ 2023-12-19 19:29 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, Peng Fan, linux-kernel,
linux-arm-kernel, linux-gpio
Hi Peng,
On 23-12-15, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add SCMI v3.2 pinctrl protocol bindings and example.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> .../devicetree/bindings/firmware/arm,scmi.yaml | 99 ++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 4591523b51a0..bfd2b6a89979 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -247,6 +247,85 @@ properties:
> reg:
> const: 0x18
>
> + protocol@19:
...
> @@ -401,6 +480,26 @@ examples:
> scmi_powercap: protocol@18 {
> reg = <0x18>;
> };
> +
> + scmi_pinctrl: protocol@19 {
> + reg = <0x19>;
> + #pinctrl-cells = <0>;
> +
> + i2c2-pins {
> + groups = "i2c2_a", "i2c2_b";
> + function = "i2c2";
> + };
> +
> + mdio-pins {
> + groups = "avb_mdio";
> + drive-strength = <24>;
> + };
> +
> + keys_pins: keys-pins {
> + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> + bias-pull-up;
> + };
> + };
This example is different to the one you mentioned within the
cover-letter. I didn't checked all patches just want to ask which API
will be implemented by this patchset?
Regards,
Marco
> };
> };
>
>
> --
> 2.37.1
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-19 19:29 ` Marco Felsch
@ 2023-12-20 1:58 ` Peng Fan
2023-12-20 12:30 ` Marco Felsch
0 siblings, 1 reply; 25+ messages in thread
From: Peng Fan @ 2023-12-20 1:58 UTC (permalink / raw)
To: Marco Felsch, Peng Fan (OSS)
Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
dl-linux-imx, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org
> Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl
> protocol
>
> Hi Peng,
>
> On 23-12-15, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add SCMI v3.2 pinctrl protocol bindings and example.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > .../devicetree/bindings/firmware/arm,scmi.yaml | 99
> ++++++++++++++++++++++
> > 1 file changed, 99 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > index 4591523b51a0..bfd2b6a89979 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > @@ -247,6 +247,85 @@ properties:
> > reg:
> > const: 0x18
> >
> > + protocol@19:
>
> ...
>
> > @@ -401,6 +480,26 @@ examples:
> > scmi_powercap: protocol@18 {
> > reg = <0x18>;
> > };
> > +
> > + scmi_pinctrl: protocol@19 {
> > + reg = <0x19>;
> > + #pinctrl-cells = <0>;
> > +
> > + i2c2-pins {
> > + groups = "i2c2_a", "i2c2_b";
> > + function = "i2c2";
> > + };
> > +
> > + mdio-pins {
> > + groups = "avb_mdio";
> > + drive-strength = <24>;
> > + };
> > +
> > + keys_pins: keys-pins {
> > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> > + bias-pull-up;
> > + };
> > + };
>
> This example is different to the one you mentioned within the cover-letter. I
> didn't checked all patches just want to ask which API will be implemented by
> this patchset?
I kept this change since it was tested by Oleksii, but anyway i.MX not use these.
The API, I suppose you are asking about this?
static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
.get_groups_count = pinctrl_scmi_get_groups_count,
.get_group_name = pinctrl_scmi_get_group_name,
.get_group_pins = pinctrl_scmi_get_group_pins,
#ifdef CONFIG_OF
.dt_node_to_map = pinconf_generic_dt_node_to_map_all,
.dt_free_map = pinconf_generic_dt_free_map,
#endif
};
static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
.get_groups_count = pinctrl_scmi_get_groups_count,
.get_group_name = pinctrl_scmi_get_group_name,
.get_group_pins = pinctrl_scmi_get_group_pins,
#ifdef CONFIG_OF
.dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
.dt_free_map = pinconf_generic_dt_free_map,
#endif
};
Thanks,
Peng.
>
> Regards,
> Marco
>
> > };
> > };
> >
> >
> > --
> > 2.37.1
> >
> >
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-15 11:56 ` [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol Peng Fan (OSS)
2023-12-15 20:11 ` Rob Herring
2023-12-19 19:29 ` Marco Felsch
@ 2023-12-20 11:42 ` Linus Walleij
2023-12-22 17:46 ` Cristian Marussi
3 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2023-12-20 11:42 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Oleksii Moisieiev, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan,
Takahiro Akashi
Hi Peng,
On Fri, Dec 15, 2023 at 12:52 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
In this example, as it is not intended to reflect any specific hardware,
use the latest canonical naming:
> +
> + scmi_pinctrl: protocol@19 {
> + reg = <0x19>;
> + #pinctrl-cells = <0>;
> +
> + i2c2-pins {
> + groups = "i2c2_a", "i2c2_b";
groups = "g_i2c2_a", "g_i2c2_b";
> + function = "i2c2";
function = "f_i2c2";
> + };
> +
> + mdio-pins {
> + groups = "avb_mdio";
groups = "g_avb_mdio";
> + drive-strength = <24>;
> + };
> +
> + keys_pins: keys-pins {
> + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
pins = "gpio_5_17", "gpio_5_20", "gpio_5_22", "gpio_2_1";
These names look odd to me, like these are actually groups
with pins 5..17 etc. Should it be groups = "g_gpio_5_17" etc?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-20 1:58 ` Peng Fan
@ 2023-12-20 12:30 ` Marco Felsch
2023-12-20 12:33 ` Peng Fan
0 siblings, 1 reply; 25+ messages in thread
From: Marco Felsch @ 2023-12-20 12:30 UTC (permalink / raw)
To: Peng Fan
Cc: Peng Fan (OSS), linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, Conor Dooley, Pengutronix Kernel Team,
Fabio Estevam, Linus Walleij, linux-kernel@vger.kernel.org,
linux-gpio@vger.kernel.org, Rob Herring, Oleksii Moisieiev,
Krzysztof Kozlowski, Sudeep Holla, Shawn Guo, Sascha Hauer,
Cristian Marussi, dl-linux-imx
On 23-12-20, Peng Fan wrote:
> > Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl
> > protocol
> >
> > Hi Peng,
> >
> > On 23-12-15, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > Add SCMI v3.2 pinctrl protocol bindings and example.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > > .../devicetree/bindings/firmware/arm,scmi.yaml | 99
> > ++++++++++++++++++++++
> > > 1 file changed, 99 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > index 4591523b51a0..bfd2b6a89979 100644
> > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > @@ -247,6 +247,85 @@ properties:
> > > reg:
> > > const: 0x18
> > >
> > > + protocol@19:
> >
> > ...
> >
> > > @@ -401,6 +480,26 @@ examples:
> > > scmi_powercap: protocol@18 {
> > > reg = <0x18>;
> > > };
> > > +
> > > + scmi_pinctrl: protocol@19 {
> > > + reg = <0x19>;
> > > + #pinctrl-cells = <0>;
> > > +
> > > + i2c2-pins {
> > > + groups = "i2c2_a", "i2c2_b";
> > > + function = "i2c2";
> > > + };
> > > +
> > > + mdio-pins {
> > > + groups = "avb_mdio";
> > > + drive-strength = <24>;
> > > + };
> > > +
> > > + keys_pins: keys-pins {
> > > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> > > + bias-pull-up;
> > > + };
> > > + };
> >
> > This example is different to the one you mentioned within the cover-letter. I
> > didn't checked all patches just want to ask which API will be implemented by
> > this patchset?
>
> I kept this change since it was tested by Oleksii, but anyway i.MX not use these.
>
> The API, I suppose you are asking about this?
> static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
> .get_groups_count = pinctrl_scmi_get_groups_count,
> .get_group_name = pinctrl_scmi_get_group_name,
> .get_group_pins = pinctrl_scmi_get_group_pins,
> #ifdef CONFIG_OF
> .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
> .dt_free_map = pinconf_generic_dt_free_map,
> #endif
> };
>
> static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
> .get_groups_count = pinctrl_scmi_get_groups_count,
> .get_group_name = pinctrl_scmi_get_group_name,
> .get_group_pins = pinctrl_scmi_get_group_pins,
> #ifdef CONFIG_OF
> .dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
> .dt_free_map = pinconf_generic_dt_free_map,
> #endif
> };
I see, thanks for the clarification. In short: the i.MX SMCI pinctrl
DT-API is the same as the non-SCMI pinctrl API since the dt_node_to_map
will convert it.
Regards,
Marco
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-20 12:30 ` Marco Felsch
@ 2023-12-20 12:33 ` Peng Fan
2023-12-20 12:37 ` Marco Felsch
0 siblings, 1 reply; 25+ messages in thread
From: Peng Fan @ 2023-12-20 12:33 UTC (permalink / raw)
To: Marco Felsch
Cc: Peng Fan (OSS), linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, Conor Dooley, Pengutronix Kernel Team,
Fabio Estevam, Linus Walleij, linux-kernel@vger.kernel.org,
linux-gpio@vger.kernel.org, Rob Herring, Oleksii Moisieiev,
Krzysztof Kozlowski, Sudeep Holla, Shawn Guo, Sascha Hauer,
Cristian Marussi, dl-linux-imx
> Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl
> protocol
>
> On 23-12-20, Peng Fan wrote:
> > > Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support
> > > pinctrl protocol
> > >
> > > Hi Peng,
> > >
> > > On 23-12-15, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > Add SCMI v3.2 pinctrl protocol bindings and example.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > > .../devicetree/bindings/firmware/arm,scmi.yaml | 99
> > > ++++++++++++++++++++++
> > > > 1 file changed, 99 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > index 4591523b51a0..bfd2b6a89979 100644
> > > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > @@ -247,6 +247,85 @@ properties:
> > > > reg:
> > > > const: 0x18
> > > >
> > > > + protocol@19:
> > >
> > > ...
> > >
> > > > @@ -401,6 +480,26 @@ examples:
> > > > scmi_powercap: protocol@18 {
> > > > reg = <0x18>;
> > > > };
> > > > +
> > > > + scmi_pinctrl: protocol@19 {
> > > > + reg = <0x19>;
> > > > + #pinctrl-cells = <0>;
> > > > +
> > > > + i2c2-pins {
> > > > + groups = "i2c2_a", "i2c2_b";
> > > > + function = "i2c2";
> > > > + };
> > > > +
> > > > + mdio-pins {
> > > > + groups = "avb_mdio";
> > > > + drive-strength = <24>;
> > > > + };
> > > > +
> > > > + keys_pins: keys-pins {
> > > > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> > > > + bias-pull-up;
> > > > + };
> > > > + };
> > >
> > > This example is different to the one you mentioned within the
> > > cover-letter. I didn't checked all patches just want to ask which
> > > API will be implemented by this patchset?
> >
> > I kept this change since it was tested by Oleksii, but anyway i.MX not use
> these.
> >
> > The API, I suppose you are asking about this?
> > static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
> > .get_groups_count = pinctrl_scmi_get_groups_count,
> > .get_group_name = pinctrl_scmi_get_group_name,
> > .get_group_pins = pinctrl_scmi_get_group_pins,
> > #ifdef CONFIG_OF
> > .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
> > .dt_free_map = pinconf_generic_dt_free_map,
> > #endif
> > };
> >
> > static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
> > .get_groups_count = pinctrl_scmi_get_groups_count,
> > .get_group_name = pinctrl_scmi_get_group_name,
> > .get_group_pins = pinctrl_scmi_get_group_pins,
> > #ifdef CONFIG_OF
> > .dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
> > .dt_free_map = pinconf_generic_dt_free_map,
> > #endif
> > };
>
> I see, thanks for the clarification. In short: the i.MX SMCI pinctrl DT-API is the
> same as the non-SCMI pinctrl API since the dt_node_to_map will convert it.
Yes, the fsl,pins format is same whether SCMI or non-SCMI. But we need
to pack the data to a format that matches the i.MX OEM SCMI PINCTRL
protocol, so we need to dedicated dt_node_to_map here.
Thanks,
Peng.
>
> Regards,
> Marco
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-20 12:33 ` Peng Fan
@ 2023-12-20 12:37 ` Marco Felsch
2023-12-21 1:31 ` Peng Fan
0 siblings, 1 reply; 25+ messages in thread
From: Marco Felsch @ 2023-12-20 12:37 UTC (permalink / raw)
To: Peng Fan
Cc: devicetree@vger.kernel.org, Conor Dooley, Cristian Marussi,
Peng Fan (OSS), Krzysztof Kozlowski, Shawn Guo, Linus Walleij,
linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
Rob Herring, Oleksii Moisieiev, Pengutronix Kernel Team,
Sudeep Holla, Fabio Estevam, Sascha Hauer,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
On 23-12-20, Peng Fan wrote:
> > Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl
> > protocol
> >
> > On 23-12-20, Peng Fan wrote:
> > > > Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support
> > > > pinctrl protocol
> > > >
> > > > Hi Peng,
> > > >
> > > > On 23-12-15, Peng Fan (OSS) wrote:
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > Add SCMI v3.2 pinctrl protocol bindings and example.
> > > > >
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > ---
> > > > > .../devicetree/bindings/firmware/arm,scmi.yaml | 99
> > > > ++++++++++++++++++++++
> > > > > 1 file changed, 99 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > index 4591523b51a0..bfd2b6a89979 100644
> > > > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > @@ -247,6 +247,85 @@ properties:
> > > > > reg:
> > > > > const: 0x18
> > > > >
> > > > > + protocol@19:
> > > >
> > > > ...
> > > >
> > > > > @@ -401,6 +480,26 @@ examples:
> > > > > scmi_powercap: protocol@18 {
> > > > > reg = <0x18>;
> > > > > };
> > > > > +
> > > > > + scmi_pinctrl: protocol@19 {
> > > > > + reg = <0x19>;
> > > > > + #pinctrl-cells = <0>;
> > > > > +
> > > > > + i2c2-pins {
> > > > > + groups = "i2c2_a", "i2c2_b";
> > > > > + function = "i2c2";
> > > > > + };
> > > > > +
> > > > > + mdio-pins {
> > > > > + groups = "avb_mdio";
> > > > > + drive-strength = <24>;
> > > > > + };
> > > > > +
> > > > > + keys_pins: keys-pins {
> > > > > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> > > > > + bias-pull-up;
> > > > > + };
> > > > > + };
> > > >
> > > > This example is different to the one you mentioned within the
> > > > cover-letter. I didn't checked all patches just want to ask which
> > > > API will be implemented by this patchset?
> > >
> > > I kept this change since it was tested by Oleksii, but anyway i.MX not use
> > these.
> > >
> > > The API, I suppose you are asking about this?
> > > static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
> > > .get_groups_count = pinctrl_scmi_get_groups_count,
> > > .get_group_name = pinctrl_scmi_get_group_name,
> > > .get_group_pins = pinctrl_scmi_get_group_pins,
> > > #ifdef CONFIG_OF
> > > .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
> > > .dt_free_map = pinconf_generic_dt_free_map,
> > > #endif
> > > };
> > >
> > > static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
> > > .get_groups_count = pinctrl_scmi_get_groups_count,
> > > .get_group_name = pinctrl_scmi_get_group_name,
> > > .get_group_pins = pinctrl_scmi_get_group_pins,
> > > #ifdef CONFIG_OF
> > > .dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
> > > .dt_free_map = pinconf_generic_dt_free_map,
> > > #endif
> > > };
> >
> > I see, thanks for the clarification. In short: the i.MX SMCI pinctrl DT-API is the
> > same as the non-SCMI pinctrl API since the dt_node_to_map will convert it.
>
> Yes, the fsl,pins format is same whether SCMI or non-SCMI. But we need
> to pack the data to a format that matches the i.MX OEM SCMI PINCTRL
> protocol, so we need to dedicated dt_node_to_map here.
Yes, I saw that you're using the enum values 192-255 for the OEM
specific part and the packing. Does you have public access to the FW
implementing the SCMI?
Regards,
Marco
>
> Thanks,
> Peng.
>
> >
> > Regards,
> > Marco
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-20 12:37 ` Marco Felsch
@ 2023-12-21 1:31 ` Peng Fan
0 siblings, 0 replies; 25+ messages in thread
From: Peng Fan @ 2023-12-21 1:31 UTC (permalink / raw)
To: Marco Felsch
Cc: devicetree@vger.kernel.org, Conor Dooley, Cristian Marussi,
Peng Fan (OSS), Krzysztof Kozlowski, Shawn Guo, Linus Walleij,
linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
Rob Herring, Oleksii Moisieiev, Pengutronix Kernel Team,
Sudeep Holla, Fabio Estevam, Sascha Hauer,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
> Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl
> protocol
>
> On 23-12-20, Peng Fan wrote:
> > > Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support
> > > pinctrl protocol
> > >
> > > On 23-12-20, Peng Fan wrote:
> > > > > Subject: Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi:
> > > > > support pinctrl protocol
> > > > >
> > > > > Hi Peng,
> > > > >
> > > > > On 23-12-15, Peng Fan (OSS) wrote:
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > >
> > > > > > Add SCMI v3.2 pinctrl protocol bindings and example.
> > > > > >
> > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > ---
> > > > > > .../devicetree/bindings/firmware/arm,scmi.yaml | 99
> > > > > ++++++++++++++++++++++
> > > > > > 1 file changed, 99 insertions(+)
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > index 4591523b51a0..bfd2b6a89979 100644
> > > > > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > @@ -247,6 +247,85 @@ properties:
> > > > > > reg:
> > > > > > const: 0x18
> > > > > >
> > > > > > + protocol@19:
> > > > >
> > > > > ...
> > > > >
> > > > > > @@ -401,6 +480,26 @@ examples:
> > > > > > scmi_powercap: protocol@18 {
> > > > > > reg = <0x18>;
> > > > > > };
> > > > > > +
> > > > > > + scmi_pinctrl: protocol@19 {
> > > > > > + reg = <0x19>;
> > > > > > + #pinctrl-cells = <0>;
> > > > > > +
> > > > > > + i2c2-pins {
> > > > > > + groups = "i2c2_a", "i2c2_b";
> > > > > > + function = "i2c2";
> > > > > > + };
> > > > > > +
> > > > > > + mdio-pins {
> > > > > > + groups = "avb_mdio";
> > > > > > + drive-strength = <24>;
> > > > > > + };
> > > > > > +
> > > > > > + keys_pins: keys-pins {
> > > > > > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> > > > > > + bias-pull-up;
> > > > > > + };
> > > > > > + };
> > > > >
> > > > > This example is different to the one you mentioned within the
> > > > > cover-letter. I didn't checked all patches just want to ask
> > > > > which API will be implemented by this patchset?
> > > >
> > > > I kept this change since it was tested by Oleksii, but anyway i.MX
> > > > not use
> > > these.
> > > >
> > > > The API, I suppose you are asking about this?
> > > > static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
> > > > .get_groups_count = pinctrl_scmi_get_groups_count,
> > > > .get_group_name = pinctrl_scmi_get_group_name,
> > > > .get_group_pins = pinctrl_scmi_get_group_pins, #ifdef
> > > > CONFIG_OF
> > > > .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
> > > > .dt_free_map = pinconf_generic_dt_free_map, #endif };
> > > >
> > > > static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
> > > > .get_groups_count = pinctrl_scmi_get_groups_count,
> > > > .get_group_name = pinctrl_scmi_get_group_name,
> > > > .get_group_pins = pinctrl_scmi_get_group_pins, #ifdef
> > > > CONFIG_OF
> > > > .dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
> > > > .dt_free_map = pinconf_generic_dt_free_map, #endif };
> > >
> > > I see, thanks for the clarification. In short: the i.MX SMCI pinctrl
> > > DT-API is the same as the non-SCMI pinctrl API since the dt_node_to_map
> will convert it.
> >
> > Yes, the fsl,pins format is same whether SCMI or non-SCMI. But we need
> > to pack the data to a format that matches the i.MX OEM SCMI PINCTRL
> > protocol, so we need to dedicated dt_node_to_map here.
>
> Yes, I saw that you're using the enum values 192-255 for the OEM specific
> part and the packing. Does you have public access to the FW implementing
> the SCMI?
>
The system manager firmware:
https://github.com/nxp-imx/imx-sm
Regards,
Peng.
> Regards,
> Marco
>
> >
> > Thanks,
> > Peng.
> >
> > >
> > > Regards,
> > > Marco
> >
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/7] firmware: arm_scmi: introduce helper get_max_msg_size
2023-12-15 11:56 ` [PATCH 1/7] firmware: arm_scmi: introduce helper get_max_msg_size Peng Fan (OSS)
@ 2023-12-22 15:30 ` Cristian Marussi
0 siblings, 0 replies; 25+ messages in thread
From: Cristian Marussi @ 2023-12-22 15:30 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
On Fri, Dec 15, 2023 at 07:56:29PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> When Agent sending data to SCMI server, the Agent driver could check
> the size to avoid protocol buffer overflow. So introduce the helper
> get_max_msg_size.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/arm_scmi/driver.c | 15 +++++++++++++++
> drivers/firmware/arm_scmi/protocols.h | 1 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index a9f70e6e58ac..b4f8f190351b 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -1481,6 +1481,20 @@ static int scmi_common_extended_name_get(const struct scmi_protocol_handle *ph,
> return ret;
> }
>
> +/**
> + * scmi_common_get_max_msg_size - Get maximum message size
> + * @ph: A protocol handle reference.
> + *
> + * Return: Maximum message size for the current protocol.
> + */
> +static int scmi_common_get_max_msg_size(const struct scmi_protocol_handle *ph)
> +{
> + const struct scmi_protocol_instance *pi = ph_to_pi(ph);
> + struct scmi_info *info = handle_to_scmi_info(pi->handle);
> +
> + return info->desc->max_msg_size;
> +}
> +
> /**
> * struct scmi_iterator - Iterator descriptor
> * @msg: A reference to the message TX buffer; filled by @prepare_message with
> @@ -1756,6 +1770,7 @@ static void scmi_common_fastchannel_db_ring(struct scmi_fc_db_info *db)
>
> static const struct scmi_proto_helpers_ops helpers_ops = {
> .extended_name_get = scmi_common_extended_name_get,
> + .get_max_msg_size = scmi_common_get_max_msg_size,
> .iter_response_init = scmi_iterator_init,
> .iter_response_run = scmi_iterator_run,
> .fastchannel_init = scmi_common_fastchannel_init,
> diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
> index e683c26f24eb..08de8dc064c1 100644
> --- a/drivers/firmware/arm_scmi/protocols.h
> +++ b/drivers/firmware/arm_scmi/protocols.h
> @@ -270,6 +270,7 @@ struct scmi_proto_helpers_ops {
> void __iomem **p_addr,
> struct scmi_fc_db_info **p_db);
> void (*fastchannel_db_ring)(struct scmi_fc_db_info *db);
> + int (*get_max_msg_size)(const struct scmi_protocol_handle *ph);
> };
This looks good to me, but please add a comment for this new helper in
the comment block above.
Other than that:
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Thanks,
Cristian
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/7] firmware: arm_scmi: bus: iterate the id_table
2023-12-15 11:56 ` [PATCH 3/7] firmware: arm_scmi: bus: iterate the id_table Peng Fan (OSS)
@ 2023-12-22 15:40 ` Cristian Marussi
0 siblings, 0 replies; 25+ messages in thread
From: Cristian Marussi @ 2023-12-22 15:40 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
On Fri, Dec 15, 2023 at 07:56:31PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> There maybe more entries in driver->id_table, just like platform
> driver of_match_table. So iterate the id_table, not only use the 1st
> entry.
>
I understand the need, but you dont consider the unbind/unload part.
Moreover since I needed a similar mechanism for testing (multiple
protocols in a single driver), I posted yesterday a patch that does this
same thing that since I was using since ages but never posted (and it
takes care of unload/unbind too.).
https://lore.kernel.org/all/20231221151129.325749-1-cristian.marussi@arm.com/
Please drop this patch and use the above (that soon should be in -next)
Thanks,
Cristian
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
2023-12-15 11:56 ` [PATCH 5/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
@ 2023-12-22 15:58 ` Cristian Marussi
2023-12-22 16:26 ` Cristian Marussi
1 sibling, 0 replies; 25+ messages in thread
From: Cristian Marussi @ 2023-12-22 15:58 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
On Fri, Dec 15, 2023 at 07:56:33PM +0800, Peng Fan (OSS) wrote:
> From: Oleksii Moisieiev <Oleksii_Moisieiev@epam.com>
>
> Add basic implementation of the SCMI v3.2 pincontrol protocol.
>
Hi
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Co-developed-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> MAINTAINERS | 6 +
> drivers/firmware/arm_scmi/Makefile | 1 +
> drivers/firmware/arm_scmi/driver.c | 2 +
> drivers/firmware/arm_scmi/pinctrl.c | 927 ++++++++++++++++++++++++++++++++++
> drivers/firmware/arm_scmi/protocols.h | 1 +
> include/linux/scmi_protocol.h | 46 ++
> 6 files changed, 983 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b589218605b4..8d971adeee22 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21180,6 +21180,12 @@ F: include/linux/sc[mp]i_protocol.h
> F: include/trace/events/scmi.h
> F: include/uapi/linux/virtio_scmi.h
>
> +SYSTEM CONTROL MANAGEMENT INTERFACE (SCMI) PINCTRL DRIVER
> +M: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> +L: linux-arm-kernel@lists.infradead.org
> +S: Maintained
> +F: drivers/firmware/arm_scmi/pinctrl.c
> +
> SYSTEM RESET/SHUTDOWN DRIVERS
> M: Sebastian Reichel <sre@kernel.org>
> L: linux-pm@vger.kernel.org
> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> index a7bc4796519c..8e3874ff1544 100644
> --- a/drivers/firmware/arm_scmi/Makefile
> +++ b/drivers/firmware/arm_scmi/Makefile
> @@ -11,6 +11,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
> scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
> +scmi-protocols-y += pinctrl.o
> scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 3174da57d832..1cf9f5d4f7bd 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -3057,6 +3057,7 @@ static int __init scmi_driver_init(void)
> scmi_voltage_register();
> scmi_system_register();
> scmi_powercap_register();
> + scmi_pinctrl_register();
>
> return platform_driver_register(&scmi_driver);
> }
> @@ -3074,6 +3075,7 @@ static void __exit scmi_driver_exit(void)
> scmi_voltage_unregister();
> scmi_system_unregister();
> scmi_powercap_unregister();
> + scmi_pinctrl_unregister();
>
> scmi_transports_exit();
>
> diff --git a/drivers/firmware/arm_scmi/pinctrl.c b/drivers/firmware/arm_scmi/pinctrl.c
> new file mode 100644
> index 000000000000..a25c8edcedd2
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/pinctrl.c
> @@ -0,0 +1,927 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System Control and Management Interface (SCMI) Pinctrl Protocol
> + *
> + * Copyright (C) 2023 EPAM
> + */
> +
> +#include <linux/module.h>
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/slab.h>
> +
> +#include "common.h"
> +#include "protocols.h"
> +
> +/* Updated only after ALL the mandatory features for that version are merged */
> +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x40000
> +
> +#define REG_TYPE_BITS GENMASK(9, 8)
> +#define REG_CONFIG GENMASK(7, 0)
> +
> +#define GET_GROUPS_NR(x) le32_get_bits((x), GENMASK(31, 16))
> +#define GET_PINS_NR(x) le32_get_bits((x), GENMASK(15, 0))
> +#define GET_FUNCTIONS_NR(x) le32_get_bits((x), GENMASK(15, 0))
> +
> +#define EXT_NAME_FLAG(x) le32_get_bits((x), BIT(31))
> +#define NUM_ELEMS(x) le32_get_bits((x), GENMASK(15, 0))
> +
> +#define REMAINING(x) le32_get_bits((x), GENMASK(31, 16))
> +#define RETURNED(x) le32_get_bits((x), GENMASK(11, 0))
> +
> +enum scmi_pinctrl_protocol_cmd {
> + PINCTRL_ATTRIBUTES = 0x3,
> + PINCTRL_LIST_ASSOCIATIONS = 0x4,
> + PINCTRL_CONFIG_GET = 0x5,
> + PINCTRL_CONFIG_SET = 0x6,
> + PINCTRL_FUNCTION_SELECT = 0x7,
> + PINCTRL_REQUEST = 0x8,
> + PINCTRL_RELEASE = 0x9,
> + PINCTRL_NAME_GET = 0xa,
> + PINCTRL_SET_PERMISSIONS = 0xb
> +};
> +
> +struct scmi_msg_conf_set {
> + __le32 identifier;
> + __le32 attributes;
> + __le32 configs[];
> +};
> +
> +struct scmi_msg_conf_get {
> + __le32 identifier;
> + __le32 attributes;
> +};
> +
> +struct scmi_resp_conf_get {
> + __le32 num_configs;
> + __le32 configs[];
> +};
> +
> +struct scmi_msg_pinctrl_protocol_attributes {
> + __le32 attributes_low;
> + __le32 attributes_high;
> +};
> +
> +struct scmi_msg_pinctrl_attributes {
> + __le32 identifier;
> + __le32 flags;
> +};
> +
> +struct scmi_resp_pinctrl_attributes {
> + __le32 attributes;
> + u8 name[SCMI_SHORT_NAME_MAX_SIZE];
> +};
> +
> +struct scmi_msg_pinctrl_list_assoc {
> + __le32 identifier;
> + __le32 flags;
> + __le32 index;
> +};
> +
> +struct scmi_resp_pinctrl_list_assoc {
> + __le32 flags;
> + __le16 array[];
> +};
> +
> +struct scmi_msg_func_set {
> + __le32 identifier;
> + __le32 function_id;
> + __le32 flags;
> +};
> +
> +struct scmi_msg_request {
> + __le32 identifier;
> + __le32 flags;
> +};
> +
> +struct scmi_group_info {
> + char name[SCMI_MAX_STR_SIZE];
> + bool present;
> + unsigned int *group_pins;
> + unsigned int nr_pins;
> +};
> +
> +struct scmi_function_info {
> + char name[SCMI_MAX_STR_SIZE];
> + bool present;
> + unsigned int *groups;
> + unsigned int nr_groups;
> +};
> +
> +struct scmi_pin_info {
> + char name[SCMI_MAX_STR_SIZE];
> + bool present;
> +};
> +
> +struct scmi_pinctrl_info {
> + u32 version;
> + int nr_groups;
> + int nr_functions;
> + int nr_pins;
> + struct scmi_group_info *groups;
> + struct scmi_function_info *functions;
> + struct scmi_pin_info *pins;
> +};
> +
> +static int scmi_pinctrl_attributes_get(const struct scmi_protocol_handle *ph,
> + struct scmi_pinctrl_info *pi)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_pinctrl_protocol_attributes *attr;
> +
> + ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr), &t);
> + if (ret)
> + return ret;
> +
> + attr = t->rx.buf;
> +
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + pi->nr_functions = GET_FUNCTIONS_NR(attr->attributes_high);
> + pi->nr_groups = GET_GROUPS_NR(attr->attributes_low);
> + pi->nr_pins = GET_PINS_NR(attr->attributes_low);
> + }
> +
> + ph->xops->xfer_put(ph, t);
> + return ret;
> +}
> +
> +static int scmi_pinctrl_count_get(const struct scmi_protocol_handle *ph,
> + enum scmi_pinctrl_selector_type type)
> +{
> + struct scmi_pinctrl_info *pi = ph->get_priv(ph);
> +
> + switch (type) {
> + case PIN_TYPE:
> + return pi->nr_pins;
> + case GROUP_TYPE:
> + return pi->nr_groups;
> + case FUNCTION_TYPE:
> + return pi->nr_functions;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int scmi_pinctrl_validate_id(const struct scmi_protocol_handle *ph,
> + u32 identifier,
> + enum scmi_pinctrl_selector_type type)
> +{
> + int value;
> +
> + value = scmi_pinctrl_count_get(ph, type);
> + if (value < 0)
> + return value;
> +
> + if (identifier >= value)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_attributes(const struct scmi_protocol_handle *ph,
> + enum scmi_pinctrl_selector_type type,
> + u32 selector, char *name,
> + unsigned int *n_elems)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_pinctrl_attributes *tx;
> + struct scmi_resp_pinctrl_attributes *rx;
> +
> + if (!name)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, PINCTRL_ATTRIBUTES, sizeof(*tx), sizeof(*rx), &t);
> + if (ret)
> + return ret;
> +
> + tx = t->tx.buf;
> + rx = t->rx.buf;
> + tx->identifier = cpu_to_le32(selector);
> + tx->flags = cpu_to_le32(type);
> +
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + if (n_elems)
> + *n_elems = NUM_ELEMS(rx->attributes);
> +
> + strscpy(name, rx->name, SCMI_SHORT_NAME_MAX_SIZE);
> + }
> +
> + ph->xops->xfer_put(ph, t);
> +
> + /*
> + * If supported overwrite short name with the extended one;
> + * on error just carry on and use already provided short name.
> + */
> + if (!ret && EXT_NAME_FLAG(rx->attributes))
> + ph->hops->extended_name_get(ph, PINCTRL_NAME_GET, selector,
> + (u32 *)&type, name,
> + SCMI_MAX_STR_SIZE);
> + return ret;
> +}
> +
> +struct scmi_pinctrl_ipriv {
> + u32 selector;
> + enum scmi_pinctrl_selector_type type;
> + unsigned int *array;
> +};
> +
> +static void iter_pinctrl_assoc_prepare_message(void *message,
> + unsigned int desc_index,
> + const void *priv)
> +{
> + struct scmi_msg_pinctrl_list_assoc *msg = message;
> + const struct scmi_pinctrl_ipriv *p = priv;
> +
> + msg->identifier = cpu_to_le32(p->selector);
> + msg->flags = cpu_to_le32(p->type);
> + /* Set the number of OPPs to be skipped/already read */
> + msg->index = cpu_to_le32(desc_index);
> +}
> +
> +static int iter_pinctrl_assoc_update_state(struct scmi_iterator_state *st,
> + const void *response, void *priv)
> +{
> + const struct scmi_resp_pinctrl_list_assoc *r = response;
> +
> + st->num_returned = RETURNED(r->flags);
> + st->num_remaining = REMAINING(r->flags);
> +
> + return 0;
> +}
> +
> +static int
> +iter_pinctrl_assoc_process_response(const struct scmi_protocol_handle *ph,
> + const void *response,
> + struct scmi_iterator_state *st, void *priv)
> +{
> + const struct scmi_resp_pinctrl_list_assoc *r = response;
> + struct scmi_pinctrl_ipriv *p = priv;
> +
> + p->array[st->desc_index + st->loop_idx] =
> + le16_to_cpu(r->array[st->loop_idx]);
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_list_associations(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + u16 size, unsigned int *array)
> +{
> + int ret;
> + void *iter;
> + struct scmi_iterator_ops ops = {
> + .prepare_message = iter_pinctrl_assoc_prepare_message,
> + .update_state = iter_pinctrl_assoc_update_state,
> + .process_response = iter_pinctrl_assoc_process_response,
> + };
> + struct scmi_pinctrl_ipriv ipriv = {
> + .selector = selector,
> + .type = type,
> + .array = array,
> + };
> +
> + if (!array || !size || type == PIN_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + iter = ph->hops->iter_response_init(ph, &ops, size,
> + PINCTRL_LIST_ASSOCIATIONS,
> + sizeof(struct scmi_msg_pinctrl_list_assoc),
> + &ipriv);
> +
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
> +
> + return ph->hops->iter_response_run(iter);
> +}
> +
> +struct scmi_conf_get_ipriv {
> + u32 selector;
> + enum scmi_pinctrl_selector_type type;
> + u8 all;
> + u8 *config_types;
> + unsigned long *config_values;
> +};
> +
> +static void iter_pinctrl_conf_get_prepare_message(void *message,
> + unsigned int desc_index,
> + const void *priv)
> +{
> + struct scmi_msg_conf_get *msg = message;
> + const struct scmi_conf_get_ipriv *p = priv;
> + u32 attributes;
> +
> + msg->identifier = cpu_to_le32(p->selector);
^^^^^
> + attributes = FIELD_PREP(BIT(18), p->all) |
> + FIELD_PREP(GENMASK(17, 16), p->type);
> +
> + if (p->all)
> + attributes |= FIELD_PREP(GENMASK(15, 8), desc_index);
> + else
> + attributes |= FIELD_PREP(GENMASK(7, 0), p->config_types[0]);
> +
> + msg->attributes = cpu_to_le32(attributes);
> + msg->identifier = cpu_to_le32(p->selector);
Duplicated .. see above ^^^^
> +}
> +
> +static int iter_pinctrl_conf_get_update_state(struct scmi_iterator_state *st,
> + const void *response, void *priv)
> +{
> + const struct scmi_resp_conf_get *r = response;
> +
> + st->num_returned = le32_get_bits(r->num_configs, GENMASK(7, 0));
> + st->num_remaining = le32_get_bits(r->num_configs, GENMASK(31, 24));
> +
You use the same iterators for both config_get and config_get_all() with proper
conditional ifs (and that is fine) BUT also here you should take care of
the case in which !p->all because in such a case the spec says that the
r->num_configs field should be ignored, which means that the platform
could return 0 or garbage, and you should assume insetad that
if (!p->all) {
st->num_returned = 1;
st->num_remaining = 0;
}
...if not the iterator could not work.
(I still have to emulate and test this on my setup though...but at first
sight it is out of spec as it is now...)
> + return 0;
> +}
> +
> +static int iter_pinctrl_conf_get_process_response(const struct scmi_protocol_handle *ph,
> + const void *response,
> + struct scmi_iterator_state *st, void *priv)
> +{
> + const struct scmi_resp_conf_get *r = response;
> + struct scmi_conf_get_ipriv *p = priv;
> +
> + if (!p->all) {
> + if (p->config_types[0] !=
> + le32_get_bits(r->configs[st->loop_idx * 2], GENMASK(7, 0)))
> + return -EINVAL;
> + } else {
> + p->config_types[st->desc_index + st->loop_idx] =
> + le32_get_bits(r->configs[st->loop_idx * 2], GENMASK(7, 0));
> + }
> +
> + p->config_values[st->desc_index + st->loop_idx] =
> + le32_to_cpu(r->configs[st->loop_idx * 2 + 1]);
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_config_get(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + u8 config_type, unsigned long *config_value)
> +{
> + int ret;
> + void *iter;
> + struct scmi_iterator_ops ops = {
> + .prepare_message = iter_pinctrl_conf_get_prepare_message,
> + .update_state = iter_pinctrl_conf_get_update_state,
> + .process_response = iter_pinctrl_conf_get_process_response,
> + };
> + struct scmi_conf_get_ipriv ipriv = {
> + .selector = selector,
> + .type = type,
> + .all = 0,
> + .config_types = &config_type,
> + .config_values = config_value,
> + };
> +
> + if (!config_value || type == FUNCTION_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + iter = ph->hops->iter_response_init(ph, &ops, 1, PINCTRL_CONFIG_GET,
> + sizeof(struct scmi_msg_conf_get),
> + &ipriv);
> +
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
> +
> + return ph->hops->iter_response_run(iter);
> +}
> +
> +static int scmi_pinctrl_config_get_all(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + u16 size, u8 *config_types,
> + unsigned long *config_values)
> +{
> + int ret;
> + void *iter;
> + struct scmi_iterator_ops ops = {
> + .prepare_message = iter_pinctrl_conf_get_prepare_message,
> + .update_state = iter_pinctrl_conf_get_update_state,
> + .process_response = iter_pinctrl_conf_get_process_response,
> + };
> + struct scmi_conf_get_ipriv ipriv = {
> + .selector = selector,
> + .type = type,
> + .all = 1,
> + .config_types = config_types,
> + .config_values = config_values,
> + };
> +
> + if (!config_values || !config_types || type == FUNCTION_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + iter = ph->hops->iter_response_init(ph, &ops, size, PINCTRL_CONFIG_GET,
> + sizeof(struct scmi_msg_conf_get),
> + &ipriv);
> +
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
> +
> + return ph->hops->iter_response_run(iter);
> +}
> +
> +static int scmi_pinctrl_config_set(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + unsigned long *configs, unsigned int nr_configs)
> +{
> + struct scmi_xfer *t;
> + struct scmi_msg_conf_set *tx;
> + u32 attributes;
> + int ret, i;
> + unsigned int configs_in_chunk, conf_num = 0;
> + unsigned int chunk;
> + int max_msg_size = ph->hops->get_max_msg_size(ph);
> +
> + if (!configs || type == FUNCTION_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + configs_in_chunk = (max_msg_size - sizeof(*tx)) / (sizeof(unsigned long) * 2);
> + while (conf_num < nr_configs) {
> + chunk = (nr_configs - conf_num > configs_in_chunk) ? configs_in_chunk :
> + nr_configs - conf_num;
> +
> + ret = ph->xops->xfer_get_init(ph, PINCTRL_CONFIG_SET,
> + sizeof(*tx) + chunk * 2 * sizeof(unsigned long),
> + 0, &t);
> + if (ret)
> + return ret;
> +
> + tx = t->tx.buf;
> + tx->identifier = cpu_to_le32(selector);
> + attributes = FIELD_PREP(GENMASK(1, 0), type) |
> + FIELD_PREP(GENMASK(9, 2), chunk);
> + tx->attributes = cpu_to_le32(attributes);
> +
The chunking seems good to me till here, BUT these last lines should not be like:
> + for (i = 0; i < chunk; i++) {
> + tx->configs[i * 2] = cpu_to_le32(pinconf_to_config_param(configs[i]));
tx->configs[i * 2] = cpu_to_le32(pinconf_to_config_param(configs[conf_num + i]));
> + tx->configs[i * 2 + 1] =
> + cpu_to_le32(pinconf_to_config_argument(configs[i]));
cpu_to_le32(pinconf_to_config_argument(configs[conf_num + i]));
..sice you are indexing the provided configs inside a chunk loop ?
(still to be properly 'fully' tested too...)
> + }
> +
> + ret = ph->xops->do_xfer(ph, t);
> +
> + ph->xops->xfer_put(ph, t);
> +
> + if (ret)
> + break;
> +
> + conf_num += chunk;
> + }
> +
> + return ret;
> +}
As said, I'll plan to exercise a bit more these interfaces directly BUT
as it is now my basic test case using the original generic pinctrl scmi
driver is still working after these latest changes.
Thanks,
Cristian
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
2023-12-15 11:56 ` [PATCH 5/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
2023-12-22 15:58 ` Cristian Marussi
@ 2023-12-22 16:26 ` Cristian Marussi
1 sibling, 0 replies; 25+ messages in thread
From: Cristian Marussi @ 2023-12-22 16:26 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
On Fri, Dec 15, 2023 at 07:56:33PM +0800, Peng Fan (OSS) wrote:
> From: Oleksii Moisieiev <Oleksii_Moisieiev@epam.com>
>
> Add basic implementation of the SCMI v3.2 pincontrol protocol.
>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Co-developed-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> MAINTAINERS | 6 +
> drivers/firmware/arm_scmi/Makefile | 1 +
> drivers/firmware/arm_scmi/driver.c | 2 +
> drivers/firmware/arm_scmi/pinctrl.c | 927 ++++++++++++++++++++++++++++++++++
> drivers/firmware/arm_scmi/protocols.h | 1 +
> include/linux/scmi_protocol.h | 46 ++
> 6 files changed, 983 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b589218605b4..8d971adeee22 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21180,6 +21180,12 @@ F: include/linux/sc[mp]i_protocol.h
> F: include/trace/events/scmi.h
> F: include/uapi/linux/virtio_scmi.h
>
> +SYSTEM CONTROL MANAGEMENT INTERFACE (SCMI) PINCTRL DRIVER
> +M: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> +L: linux-arm-kernel@lists.infradead.org
> +S: Maintained
> +F: drivers/firmware/arm_scmi/pinctrl.c
> +
> SYSTEM RESET/SHUTDOWN DRIVERS
> M: Sebastian Reichel <sre@kernel.org>
> L: linux-pm@vger.kernel.org
> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> index a7bc4796519c..8e3874ff1544 100644
> --- a/drivers/firmware/arm_scmi/Makefile
> +++ b/drivers/firmware/arm_scmi/Makefile
> @@ -11,6 +11,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
> scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
> +scmi-protocols-y += pinctrl.o
> scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 3174da57d832..1cf9f5d4f7bd 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -3057,6 +3057,7 @@ static int __init scmi_driver_init(void)
> scmi_voltage_register();
> scmi_system_register();
> scmi_powercap_register();
> + scmi_pinctrl_register();
>
> return platform_driver_register(&scmi_driver);
> }
> @@ -3074,6 +3075,7 @@ static void __exit scmi_driver_exit(void)
> scmi_voltage_unregister();
> scmi_system_unregister();
> scmi_powercap_unregister();
> + scmi_pinctrl_unregister();
>
> scmi_transports_exit();
>
> diff --git a/drivers/firmware/arm_scmi/pinctrl.c b/drivers/firmware/arm_scmi/pinctrl.c
> new file mode 100644
> index 000000000000..a25c8edcedd2
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/pinctrl.c
> @@ -0,0 +1,927 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System Control and Management Interface (SCMI) Pinctrl Protocol
> + *
> + * Copyright (C) 2023 EPAM
> + */
> +
> +#include <linux/module.h>
> +#include <linux/pinctrl/pinconf-generic.h>
I spotted this only later while looking at the SCMI Pinctrl driver...
...Get rid of this and please make these conversions in the SCMI pinctrl driver
NOT here in the protocol layer....these ops should receive SCMI valid requests
and should not have any need to invoke some other subsystem helpers to
pack/unpack.
See below..
> +#include <linux/scmi_protocol.h>
> +#include <linux/slab.h>
[snip]
> +
> +static int scmi_pinctrl_config_set(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + unsigned long *configs, unsigned int nr_configs)
> +{
> + struct scmi_xfer *t;
> + struct scmi_msg_conf_set *tx;
> + u32 attributes;
> + int ret, i;
> + unsigned int configs_in_chunk, conf_num = 0;
> + unsigned int chunk;
> + int max_msg_size = ph->hops->get_max_msg_size(ph);
> +
> + if (!configs || type == FUNCTION_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + configs_in_chunk = (max_msg_size - sizeof(*tx)) / (sizeof(unsigned long) * 2);
^^
sizeof(__le32))
> + while (conf_num < nr_configs) {
> + chunk = (nr_configs - conf_num > configs_in_chunk) ? configs_in_chunk :
> + nr_configs - conf_num;
> +
> + ret = ph->xops->xfer_get_init(ph, PINCTRL_CONFIG_SET,
> + sizeof(*tx) + chunk * 2 * sizeof(unsigned long),
^^
sizeof(__le32))
> + 0, &t);
> + if (ret)
> + return ret;
> +
> + tx = t->tx.buf;
> + tx->identifier = cpu_to_le32(selector);
> + attributes = FIELD_PREP(GENMASK(1, 0), type) |
> + FIELD_PREP(GENMASK(9, 2), chunk);
> + tx->attributes = cpu_to_le32(attributes);
> +
> + for (i = 0; i < chunk; i++) {
> + tx->configs[i * 2] = cpu_to_le32(pinconf_to_config_param(configs[i]));
This should be the bare config_type as received already in SCMI format
as a param.
> + tx->configs[i * 2 + 1] =
> + cpu_to_le32(pinconf_to_config_argument(configs[i]));
and here the config_values...this also means you will have to change the
parameters to this function to pass a
uint8_t *config_types
uint32_t *config_values
unsigne int num_configs
or something like that....there is also a subtle need to remap the types
from Pinctrl to SCMI in the pinctrl SCMI driver (I commented this on
that patch)
Thanks,
Cristian
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/7] pinctrl: Implementation of the generic scmi-pinctrl driver
2023-12-15 11:56 ` [PATCH 6/7] pinctrl: Implementation of the generic scmi-pinctrl driver Peng Fan (OSS)
@ 2023-12-22 16:35 ` Cristian Marussi
0 siblings, 0 replies; 25+ messages in thread
From: Cristian Marussi @ 2023-12-22 16:35 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan,
AKASHI Takahiro
On Fri, Dec 15, 2023 at 07:56:34PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> scmi-pinctrl driver implements pinctrl driver interface and using
> SCMI protocol to redirect messages from pinctrl subsystem SDK to
> SCMI platform firmware, which does the changes in HW.
>
[CC: Akashi which was interested in this series]
This generic driver still works fine for me as of now in my emulated
setup using the generic SCMI bindings as in the initial Oleksii example
and a dummy driver consumer for this pins, BUT there is still an open
issue as reported by Akashi previously ..see below.
> Co-developed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> MAINTAINERS | 1 +
> drivers/pinctrl/Kconfig | 11 ++
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-scmi.c | 403 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 416 insertions(+)
[snip]
> +static const struct pinmux_ops pinctrl_scmi_pinmux_ops = {
> + .request = pinctrl_scmi_request,
> + .free = pinctrl_scmi_free,
> + .get_functions_count = pinctrl_scmi_get_functions_count,
> + .get_function_name = pinctrl_scmi_get_function_name,
> + .get_function_groups = pinctrl_scmi_get_function_groups,
> + .set_mux = pinctrl_scmi_func_set_mux,
> +};
> +
> +static int pinctrl_scmi_pinconf_get(struct pinctrl_dev *pctldev, unsigned int _pin,
> + unsigned long *config)
> +{
> + int ret;
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> + enum pin_config_param config_type;
> + unsigned long config_value;
> +
> + if (!config)
> + return -EINVAL;
> +
> + config_type = pinconf_to_config_param(*config);
This config types that are packed/unpacked from the generic Pinctrl
subsystem have also to be mapped/umapped, here and below, to the SCMI
ones since they are slightly different/.
IOW SCMI V3.2 Table_24 in the spec, which defines Pins Config Types as
understood by an SCMI platform FW is NOT exactly mapping to the enum
pin_config_param used by Pinctrl in its pinconf_to_config so you risk
passing a config_type to the SCMI fw that does NOT mean at all what
intended...if the FW is compliant with SCMI.
> +
> + ret = pinctrl_ops->config_get(pmx->ph, _pin, PIN_TYPE, config_type, &config_value);
> + if (ret)
> + return ret;
> +
> + *config = pinconf_to_config_packed(config_type, config_value);
> +
> + return 0;
> +}
> +
> +static int pinctrl_scmi_pinconf_set(struct pinctrl_dev *pctldev,
> + unsigned int _pin,
> + unsigned long *configs,
> + unsigned int num_configs)
> +{
> + int ret;
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> +
> + if (!configs || !num_configs)
> + return -EINVAL;
> +
Same here, as said in the previous patch about pinctrl protocol, you
should pack/unpack and map/unmap from Pinctrl packed configs and types
to SCMI types and unpacked config/values, to fix the mismatch between
Pinctrl and SCMI types and also to avoid the usage of Pinctrl helpers to
extract the types from the SCMI protocol layer so that we can keep it
agnostic in these regards.
Thanks,
Cristian
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map
2023-12-15 11:56 ` [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map Peng Fan (OSS)
@ 2023-12-22 17:43 ` Cristian Marussi
2023-12-23 2:14 ` Peng Fan
0 siblings, 1 reply; 25+ messages in thread
From: Cristian Marussi @ 2023-12-22 17:43 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
On Fri, Dec 15, 2023 at 07:56:35PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> i.MX95 System Manager FW supports SCMI PINCTRL protocol, but uses
> OEM Pin Configuration type, so need i.MX specific dt_node_to_map.
>
This does not even compile for me, as of now, when configuring the Pinctrl
SCMI driver as a module with your IMX custom additions. (I think the
Makefile with the additional pinctrl-imx is wrong in how describes the
objects composing the pinctrl-scmi module with IMX addons...)
ERROR: modpost: "pinctrl_scmi_imx_dt_node_to_map" [drivers/pinctrl/pinctrl-scmi.ko] undefined!
make[3]: *** [dev/src/linux/scripts/Makefile.modpost:145: Module.symvers] Error 1
make[2]: *** [dev/src/linux/Makefile:1863: modpost] Error 2
make[1]: *** [dev/src/linux/Makefile:234: __sub-make] Error 2
make[1]: Leaving directory dev/out_linux
make: *** [Makefile:234: __sub-make] Error 2
More in general, I think that this NXP OEM specific additions, which are
in general welcome (and indeed as you know part of the spec was modified
to allow for OEM specific needs), do NOT belong to this generic SCMI
Pinctrl driver, because the driver from Oleksii/EPAM was born as a
generic SCMI driver and it fits perfectly with the Generic Pinctrl
Linux subsystem and related generic bindings parsing: now with this
you are trying to stick a custom OEM slight varied behaviour (and
related binding) on top of a generic thing.
And this choice leads to a number of additional changes in the SCMI core
to support an even more complex handling of SCMI devices, which is
already too complex IMO..
IOW...I dont think that the whole idea of the per-protocol optional
compatible to be able to select slightly different behaviours/parsing
would have a great chance to fly sincerely...
I know there is an issue with having a completely distinct SCMI IMX
pinctrl driver that uses the same protocol node @19 (without the need for
the compatible trick) due to the way in which the Pinctrl subsystem
searches for devices (by of_node)...I'll think about an alternative
way to allow this but I am not sure (as you saw) that would be so easily
doable...
Also, I am wondering if this is really a problem in reality since I would
NOT expect you to load/ship both the OEM/NXP custom specific SCMI pinctrl
driver AND the generic one on the same platform (after having made them
distinct I mean...) am I wrong ?
(so you could even made them exclude each other at compile time...far
from being the best option I agree...)
Thanks,
Cristian
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol
2023-12-15 11:56 ` [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol Peng Fan (OSS)
` (2 preceding siblings ...)
2023-12-20 11:42 ` Linus Walleij
@ 2023-12-22 17:46 ` Cristian Marussi
3 siblings, 0 replies; 25+ messages in thread
From: Cristian Marussi @ 2023-12-22 17:46 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
On Fri, Dec 15, 2023 at 07:56:32PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add SCMI v3.2 pinctrl protocol bindings and example.
>
Hi
as I mentioned on the last patch review, I dont see adding all this
vendor specific stuff in the ARM SCMI Pinctrl generic binding as a
viable option, nor the per-protocol compatible to select slightly
different behaviours.
Thanks,
Cristian
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map
2023-12-22 17:43 ` Cristian Marussi
@ 2023-12-23 2:14 ` Peng Fan
0 siblings, 0 replies; 25+ messages in thread
From: Peng Fan @ 2023-12-23 2:14 UTC (permalink / raw)
To: Cristian Marussi, Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-gpio@vger.kernel.org
> Subject: Re: [PATCH 7/7] pinctrl: scmi: implement
> pinctrl_scmi_imx_dt_node_to_map
>
> On Fri, Dec 15, 2023 at 07:56:35PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX95 System Manager FW supports SCMI PINCTRL protocol, but uses
> OEM
> > Pin Configuration type, so need i.MX specific dt_node_to_map.
> >
>
> This does not even compile for me, as of now, when configuring the Pinctrl
> SCMI driver as a module with your IMX custom additions. (I think the
> Makefile with the additional pinctrl-imx is wrong in how describes the objects
> composing the pinctrl-scmi module with IMX addons...)
>
> ERROR: modpost: "pinctrl_scmi_imx_dt_node_to_map"
> [drivers/pinctrl/pinctrl-scmi.ko] undefined!
> make[3]: *** [dev/src/linux/scripts/Makefile.modpost:145: Module.symvers]
> Error 1
> make[2]: *** [dev/src/linux/Makefile:1863: modpost] Error 2
> make[1]: *** [dev/src/linux/Makefile:234: __sub-make] Error 2
> make[1]: Leaving directory dev/out_linux
> make: *** [Makefile:234: __sub-make] Error 2
Oh, sorry for this. I could post a new version if you require. But before
that we may better align on the approach on how to support i.MX.
>
> More in general, I think that this NXP OEM specific additions, which are in
> general welcome (and indeed as you know part of the spec was modified to
> allow for OEM specific needs), do NOT belong to this generic SCMI Pinctrl
> driver, because the driver from Oleksii/EPAM was born as a generic SCMI
> driver and it fits perfectly with the Generic Pinctrl Linux subsystem and
> related generic bindings parsing: now with this you are trying to stick a
> custom OEM slight varied behaviour (and related binding) on top of a generic
> thing.
>
> And this choice leads to a number of additional changes in the SCMI core to
> support an even more complex handling of SCMI devices, which is already too
> complex IMO..
>
> IOW...I dont think that the whole idea of the per-protocol optional
> compatible to be able to select slightly different behaviours/parsing would
> have a great chance to fly sincerely...
>
> I know there is an issue with having a completely distinct SCMI IMX pinctrl
> driver that uses the same protocol node @19 (without the need for the
> compatible trick) due to the way in which the Pinctrl subsystem searches for
> devices (by of_node)...I'll think about an alternative way to allow this but I am
> not sure (as you saw) that would be so easily doable...
For all protocols supports VENDOR extension attributes, we need a way
to handle I think.
As Linus wrote in
https://lore.kernel.org/all/CACRpkdaRY+rU+md-r5gVyFH5ATt3Pqp9=M4=+WArYkfVLAFdpw@mail.gmail.com/:
We may need:
protocol@19 {
compatible = "vendor,soc-scmi-pinctrl";
(...)
>
> Also, I am wondering if this is really a problem in reality since I would NOT
> expect you to load/ship both the OEM/NXP custom specific SCMI pinctrl
> driver AND the generic one on the same platform (after having made them
> distinct I mean...) am I wrong ?
You are right, but that means the upstream ARM64 defconfig will not
able to support both i.MX9 and others.
Thanks,
Peng.
> (so you could even made them exclude each other at compile time...far from
> being the best option I agree...)
>
> Thanks,
> Cristian
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2023-12-23 2:14 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-15 11:56 [PATCH 0/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
2023-12-15 11:56 ` [PATCH 1/7] firmware: arm_scmi: introduce helper get_max_msg_size Peng Fan (OSS)
2023-12-22 15:30 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 2/7] firmware: scmi: support compatible string Peng Fan (OSS)
2023-12-15 11:56 ` [PATCH 3/7] firmware: arm_scmi: bus: iterate the id_table Peng Fan (OSS)
2023-12-22 15:40 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 4/7] dt-bindings: firmware: arm,scmi: support pinctrl protocol Peng Fan (OSS)
2023-12-15 20:11 ` Rob Herring
2023-12-16 4:44 ` Peng Fan
2023-12-19 19:29 ` Marco Felsch
2023-12-20 1:58 ` Peng Fan
2023-12-20 12:30 ` Marco Felsch
2023-12-20 12:33 ` Peng Fan
2023-12-20 12:37 ` Marco Felsch
2023-12-21 1:31 ` Peng Fan
2023-12-20 11:42 ` Linus Walleij
2023-12-22 17:46 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 5/7] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (OSS)
2023-12-22 15:58 ` Cristian Marussi
2023-12-22 16:26 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 6/7] pinctrl: Implementation of the generic scmi-pinctrl driver Peng Fan (OSS)
2023-12-22 16:35 ` Cristian Marussi
2023-12-15 11:56 ` [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map Peng Fan (OSS)
2023-12-22 17:43 ` Cristian Marussi
2023-12-23 2:14 ` Peng Fan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).