All:
There was a v6 that use generic properties, but at a late stage, NXP
internals decides to switch to fsl,pins property to align with other
i.MXs. Since new properties, drivers rewrite, I start this patchset
from v1 with a new patch title. A RFC patch for binding was posted,
since Rob said he is fine, so post this patchset out.
Whether v6 or this patchset, patch 2 is a must and was not changed from
v6.
The pinctrl stuff has been pending for quite sometime, I would be
apprecaited if any quick comments.
v6:
https://lore.kernel.org/all/[email protected]/
RFC:
https://lore.kernel.org/all/[email protected]/
Thanks,
Peng.
ARM SCMI v3.2 Table 24 Pin Configuration Type and Enumerations:
'192 -255 OEM specific units'.
i.MX95 System Manager FW supports SCMI PINCTRL protocol, but it has zero
functions, groups. So pinctrl-scmi.c could not be reused for i.MX95.
Because nxp,pin-func, nxp,pin-conf properties are rejected by dt
maintainers, so use 'fsl,pins' which requires a new driver
pinctrl-imx-scmi.c
The node will be as below:
pinctrl_usdhc1: usdhc1grp {
fsl,pins = <
IMX95_PAD_SD1_CLK__USDHC1_CLK 0x158e
IMX95_PAD_SD1_CMD__USDHC1_CMD 0x138e
IMX95_PAD_SD1_DATA0__USDHC1_DATA0 0x138e
IMX95_PAD_SD1_DATA1__USDHC1_DATA1 0x138e
IMX95_PAD_SD1_DATA2__USDHC1_DATA2 0x138e
IMX95_PAD_SD1_DATA3__USDHC1_DATA3 0x138e
IMX95_PAD_SD1_DATA4__USDHC1_DATA4 0x138e
IMX95_PAD_SD1_DATA5__USDHC1_DATA5 0x138e
IMX95_PAD_SD1_DATA6__USDHC1_DATA6 0x138e
IMX95_PAD_SD1_DATA7__USDHC1_DATA7 0x138e
IMX95_PAD_SD1_STROBE__USDHC1_STROBE 0x158e
>;
};
Signed-off-by: Peng Fan <[email protected]>
---
Peng Fan (3):
dt-bindings: firmware: arm,scmi: Add properties for i.MX95 Pinctrl OEM extensions
pinctrl: scmi: add blocklist
pinctrl: imx: support SCMI pinctrl protocol for i.MX95
.../devicetree/bindings/firmware/arm,scmi.yaml | 4 +-
.../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml | 53 +++
drivers/pinctrl/freescale/Kconfig | 9 +
drivers/pinctrl/freescale/Makefile | 1 +
drivers/pinctrl/freescale/pinctrl-imx-scmi.c | 357 +++++++++++++++++++++
drivers/pinctrl/pinctrl-scmi.c | 9 +
6 files changed, 432 insertions(+), 1 deletion(-)
---
base-commit: 632483ea8004edfadd035de36e1ab2c7c4f53158
change-id: 20240521-pinctrl-scmi-imx95-867bea9595cf
Best regards,
--
Peng Fan <[email protected]>
From: Peng Fan <[email protected]>
i.MX95 Pinctrl is managed by System Control Management Interface(SCMI)
firmware using OEM extensions. No functions, no groups are provided by
the firmware. So add i.MX95 specific properties.
To keep aligned with current i.MX pinctrl bindings, still use "fsl,pins"
for i.MX95.
Signed-off-by: Peng Fan <[email protected]>
---
.../devicetree/bindings/firmware/arm,scmi.yaml | 4 +-
.../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml | 53 ++++++++++++++++++++++
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 7de2c29606e5..f7a48b1e9f62 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -251,7 +251,9 @@ properties:
type: object
allOf:
- $ref: '#/$defs/protocol-node'
- - $ref: /schemas/pinctrl/pinctrl.yaml
+ - anyOf:
+ - $ref: /schemas/pinctrl/pinctrl.yaml
+ - $ref: /schemas/firmware/nxp,imx95-scmi-pinctrl.yaml
unevaluatedProperties: false
diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
new file mode 100644
index 000000000000..a96fc6cce502
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/nxp,imx95-scmi-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX System Control and Management Interface (SCMI) Pinctrl Protocol
+
+maintainers:
+ - Peng Fan <[email protected]>
+
+allOf:
+ - $ref: /schemas/pinctrl/pinctrl.yaml
+
+patternProperties:
+ 'grp$':
+ type: object
+ description:
+ Pinctrl node's client devices use subnodes for desired pin configuration.
+ Client device subnodes use below standard properties.
+
+ unevaluatedProperties: false
+
+ 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 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:
+ - fsl,pins
+
+additionalProperties: true
--
2.37.1
From: Peng Fan <[email protected]>
i.MX95 will have its own pinctrl scmi driver, so need block
pinctrl-scmi driver for i.MX95, otherwise there will be two pinctrl
devices for a single scmi protocol@19.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/pinctrl-scmi.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 036bc1e3fc6c..df4bbcd7d1d5 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -11,6 +11,7 @@
#include <linux/errno.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
+#include <linux/of.h>
#include <linux/scmi_protocol.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -504,6 +505,11 @@ static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
return 0;
}
+static const char * const scmi_pinctrl_blocklist[] = {
+ "fsl,imx95",
+ NULL
+};
+
static int scmi_pinctrl_probe(struct scmi_device *sdev)
{
int ret;
@@ -515,6 +521,9 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
if (!sdev->handle)
return -EINVAL;
+ if (of_machine_compatible_match(scmi_pinctrl_blocklist))
+ return -ENODEV;
+
handle = sdev->handle;
pinctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PINCTRL, &ph);
--
2.37.1
From: Peng Fan <[email protected]>
The generic pinctrl-scmi.c driver could not be used for i.MX95 because
i.MX95 SCMI firmware not supports functions, groups or generic
'Pin Configuration Type and Enumerations' listed in SCMI Specification.
i.MX95 System Control Management Interface(SCMI) firmware only supports
below pin configuration types which are OEM specific types:
192: PIN MUX
193: PIN CONF
194: DAISY ID
195: DAISY VAL
To Support fsl,pins property together with SCMI OEM protocol, add this
driver.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/freescale/Kconfig | 9 +
drivers/pinctrl/freescale/Makefile | 1 +
drivers/pinctrl/freescale/pinctrl-imx-scmi.c | 357 +++++++++++++++++++++++++++
3 files changed, 367 insertions(+)
diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 27bdc548f3a7..711a5ab3ceb1 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -7,6 +7,15 @@ config PINCTRL_IMX
select PINCONF
select REGMAP
+config PINCTRL_IMX_SCMI
+ tristate "i.MX95 pinctrl driver using SCMI protocol interface"
+ depends on ARM_SCMI_PROTOCOL && OF || COMPILE_TEST
+ select PINMUX
+ select GENERIC_PINCONF
+ help
+ i.MX95 SCMI firmware provides pinctrl protocol. This driver
+ utilizes the SCMI interface to do pinctrl configuration.
+
config PINCTRL_IMX_SCU
tristate
depends on IMX_SCU
diff --git a/drivers/pinctrl/freescale/Makefile b/drivers/pinctrl/freescale/Makefile
index 647dff060477..e79b4b06e71b 100644
--- a/drivers/pinctrl/freescale/Makefile
+++ b/drivers/pinctrl/freescale/Makefile
@@ -2,6 +2,7 @@
# Freescale pin control drivers
obj-$(CONFIG_PINCTRL_IMX) += pinctrl-imx.o
obj-$(CONFIG_PINCTRL_IMX_SCU) += pinctrl-scu.o
+obj-$(CONFIG_PINCTRL_IMX_SCMI) += pinctrl-imx-scmi.o
obj-$(CONFIG_PINCTRL_IMX1_CORE) += pinctrl-imx1-core.o
obj-$(CONFIG_PINCTRL_IMX1) += pinctrl-imx1.o
obj-$(CONFIG_PINCTRL_IMX27) += pinctrl-imx27.o
diff --git a/drivers/pinctrl/freescale/pinctrl-imx-scmi.c b/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
new file mode 100644
index 000000000000..2991047535bc
--- /dev/null
+++ b/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Power Interface (SCMI) Protocol based i.MX pinctrl driver
+ *
+ * Copyright 2024 NXP
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/scmi_protocol.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/types.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"
+#include "../pinmux.h"
+
+#define DRV_NAME "scmi-pinctrl-imx"
+
+struct scmi_pinctrl_imx {
+ struct device *dev;
+ struct scmi_protocol_handle *ph;
+ struct pinctrl_dev *pctldev;
+ struct pinctrl_desc pctl_desc;
+ const struct scmi_pinctrl_proto_ops *ops;
+};
+
+/* SCMI pin control types, aligned with SCMI firmware */
+#define IMX_SCMI_NUM_CFG 4
+#define IMX_SCMI_PIN_MUX 192
+#define IMX_SCMI_PIN_CONFIG 193
+#define IMX_SCMI_PIN_DAISY_ID 194
+#define IMX_SCMI_PIN_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_PIN_SIZE 24
+
+#define IMX95_DAISY_OFF 0x408
+
+static 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 = IMX95_DAISY_OFF;
+ } else {
+ dev_err(pctldev->dev, "platform not support scmi pinctrl\n");
+ return -EINVAL;
+ }
+ }
+
+ 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_MUX, mux_val);
+
+ if (!conf_reg || (conf_val & IMX_SCMI_NO_PAD_CTL))
+ ncfg--;
+ else
+ cfg[j++] = pinconf_to_config_packed(IMX_SCMI_PIN_CONFIG, conf_val);
+
+ if (!input_reg) {
+ ncfg -= 2;
+ } else {
+ cfg[j++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_ID,
+ (input_reg - daisy_off) / 4);
+ cfg[j++] = pinconf_to_config_packed(IMX_SCMI_PIN_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;
+}
+
+static void pinctrl_scmi_imx_dt_free_map(struct pinctrl_dev *pctldev,
+ struct pinctrl_map *map, unsigned int num_maps)
+{
+ kfree(map);
+}
+
+static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
+ .get_groups_count = pinctrl_generic_get_group_count,
+ .get_group_name = pinctrl_generic_get_group_name,
+ .get_group_pins = pinctrl_generic_get_group_pins,
+ .dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
+ .dt_free_map = pinctrl_scmi_imx_dt_free_map,
+};
+
+static int pinctrl_scmi_imx_func_set_mux(struct pinctrl_dev *pctldev,
+ unsigned int selector, unsigned int group)
+{
+ /*
+ * For i.MX SCMI PINCTRL , postpone the mux setting
+ * until config is set as they can be set together
+ * in one IPC call
+ */
+ return 0;
+}
+
+static const struct pinmux_ops pinctrl_scmi_imx_pinmux_ops = {
+ .get_functions_count = pinmux_generic_get_function_count,
+ .get_function_name = pinmux_generic_get_function_name,
+ .get_function_groups = pinmux_generic_get_function_groups,
+ .set_mux = pinctrl_scmi_imx_func_set_mux,
+};
+
+static int pinctrl_scmi_imx_pinconf_get(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *config)
+{
+ int ret;
+ struct scmi_pinctrl_imx *pmx = pinctrl_dev_get_drvdata(pctldev);
+ u32 config_type, val;
+
+ if (!config)
+ return -EINVAL;
+
+ config_type = pinconf_to_config_param(*config);
+
+ ret = pmx->ops->settings_get_one(pmx->ph, pin, PIN_TYPE, config_type, &val);
+ /* Convert SCMI error code to PINCTRL expected error code */
+ if (ret == -EOPNOTSUPP)
+ return -ENOTSUPP;
+ if (ret)
+ return ret;
+
+ *config = pinconf_to_config_packed(config_type, val);
+
+ dev_dbg(pmx->dev, "pin:%s, conf:0x%x", pin_get_name(pctldev, pin), val);
+
+ return 0;
+}
+
+static int pinctrl_scmi_imx_pinconf_set(struct pinctrl_dev *pctldev,
+ unsigned int pin,
+ unsigned long *configs,
+ unsigned int num_configs)
+{
+ struct scmi_pinctrl_imx *pmx = pinctrl_dev_get_drvdata(pctldev);
+ enum scmi_pinctrl_conf_type config_type[IMX_SCMI_NUM_CFG];
+ u32 config_value[IMX_SCMI_NUM_CFG];
+ enum scmi_pinctrl_conf_type *p_config_type = config_type;
+ u32 *p_config_value = config_value;
+ int ret;
+ int i;
+
+ if (!configs || !num_configs)
+ return -EINVAL;
+
+ if (num_configs > IMX_SCMI_NUM_CFG) {
+ dev_err(pmx->dev, "num_configs(%d) too large\n", num_configs);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num_configs; i++) {
+ /* cast to avoid build warning */
+ p_config_type[i] =
+ (enum scmi_pinctrl_conf_type)pinconf_to_config_param(configs[i]);
+ p_config_value[i] = pinconf_to_config_argument(configs[i]);
+
+ dev_dbg(pmx->dev, "pin: %u, type: %u, val: 0x%x\n",
+ pin, p_config_type[i], p_config_value[i]);
+ }
+
+ ret = pmx->ops->settings_conf(pmx->ph, pin, PIN_TYPE, num_configs,
+ p_config_type, p_config_value);
+ if (ret)
+ dev_err(pmx->dev, "Error set config %d\n", ret);
+
+ return ret;
+}
+
+static void pinctrl_scmi_imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
+ struct seq_file *s, unsigned int pin_id)
+{
+ unsigned long config = pinconf_to_config_packed(IMX_SCMI_PIN_CONFIG, 0);
+ int ret;
+
+ ret = pinctrl_scmi_imx_pinconf_get(pctldev, pin_id, &config);
+ if (ret)
+ config = 0;
+ else
+ config = pinconf_to_config_argument(config);
+
+ seq_printf(s, "0x%lx", config);
+}
+
+static const struct pinconf_ops pinctrl_scmi_imx_pinconf_ops = {
+ .pin_config_get = pinctrl_scmi_imx_pinconf_get,
+ .pin_config_set = pinctrl_scmi_imx_pinconf_set,
+ .pin_config_dbg_show = pinctrl_scmi_imx_pinconf_dbg_show,
+};
+
+static int
+scmi_pinctrl_imx_get_pins(struct scmi_pinctrl_imx *pmx, struct pinctrl_desc *desc)
+{
+ struct pinctrl_pin_desc *pins;
+ unsigned int npins;
+ int ret, i;
+
+ npins = pmx->ops->count_get(pmx->ph, PIN_TYPE);
+ pins = devm_kmalloc_array(pmx->dev, npins, sizeof(*pins), GFP_KERNEL);
+ if (!pins)
+ return -ENOMEM;
+
+ for (i = 0; i < npins; i++) {
+ pins[i].number = i;
+ /* no need free name, firmware driver handles it */
+ ret = pmx->ops->name_get(pmx->ph, i, PIN_TYPE, &pins[i].name);
+ if (ret)
+ return dev_err_probe(pmx->dev, ret,
+ "Can't get name for pin %d", i);
+ }
+
+ desc->npins = npins;
+ desc->pins = pins;
+ dev_dbg(pmx->dev, "got pins %u", npins);
+
+ return 0;
+}
+
+static const char * const scmi_pinctrl_imx_allowlist[] = {
+ "fsl,imx95",
+ NULL
+};
+
+static int scmi_pinctrl_imx_probe(struct scmi_device *sdev)
+{
+ struct device *dev = &sdev->dev;
+ const struct scmi_handle *handle = sdev->handle;
+ struct scmi_pinctrl_imx *pmx;
+ struct scmi_protocol_handle *ph;
+ const struct scmi_pinctrl_proto_ops *pinctrl_ops;
+ int ret;
+
+ if (!handle)
+ return -EINVAL;
+
+ if (!of_machine_compatible_match(scmi_pinctrl_imx_allowlist))
+ return -ENODEV;
+
+ 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->ops = pinctrl_ops;
+
+ pmx->dev = dev;
+ pmx->pctl_desc.name = DRV_NAME;
+ pmx->pctl_desc.owner = THIS_MODULE;
+ pmx->pctl_desc.pctlops = &pinctrl_scmi_imx_pinctrl_ops;
+ pmx->pctl_desc.pmxops = &pinctrl_scmi_imx_pinmux_ops;
+ pmx->pctl_desc.confops = &pinctrl_scmi_imx_pinconf_ops;
+
+ ret = scmi_pinctrl_imx_get_pins(pmx, &pmx->pctl_desc);
+ if (ret)
+ return ret;
+
+ pmx->dev = &sdev->dev;
+
+ 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");
+
+ return pinctrl_enable(pmx->pctldev);
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_PINCTRL, "pinctrl-imx" },
+ { }
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_pinctrl_imx_driver = {
+ .name = DRV_NAME,
+ .probe = scmi_pinctrl_imx_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_pinctrl_imx_driver);
+
+MODULE_AUTHOR("Peng Fan <[email protected]>");
+MODULE_DESCRIPTION("i.MX SCMI pin controller driver");
+MODULE_LICENSE("GPL");
--
2.37.1
On Tue, 21 May 2024 14:25:57 +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> i.MX95 Pinctrl is managed by System Control Management Interface(SCMI)
> firmware using OEM extensions. No functions, no groups are provided by
> the firmware. So add i.MX95 specific properties.
>
> To keep aligned with current i.MX pinctrl bindings, still use "fsl,pins"
> for i.MX95.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> .../devicetree/bindings/firmware/arm,scmi.yaml | 4 +-
> .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml | 53 ++++++++++++++++++++++
> 2 files changed, 56 insertions(+), 1 deletion(-)
>
Reviewed-by: Rob Herring (Arm) <[email protected]>
Hi Linus, Sudeep, Cristian,
> Subject: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions with
> fsl,pins property
Sorry if this is an early ping to you. Just wanna this not blocking i.MX95
upstream support.
Are you ok with this patchset?
Thanks,
Peng.
>
> All:
> There was a v6 that use generic properties, but at a late stage, NXP internals
> decides to switch to fsl,pins property to align with other i.MXs. Since new
> properties, drivers rewrite, I start this patchset from v1 with a new patch title.
> A RFC patch for binding was posted, since Rob said he is fine, so post this
> patchset out.
>
> Whether v6 or this patchset, patch 2 is a must and was not changed from v6.
>
> The pinctrl stuff has been pending for quite sometime, I would be
> apprecaited if any quick comments.
>
> v6:
> https://lore.kernel.org/all/20240513-pinctrl-scmi-oem-v3-v6-0-
> [email protected]/
> RFC:
> https://lore.kernel.org/all/[email protected]/
>
> Thanks,
> Peng.
>
> ARM SCMI v3.2 Table 24 Pin Configuration Type and Enumerations:
> '192 -255 OEM specific units'.
>
> i.MX95 System Manager FW supports SCMI PINCTRL protocol, but it has zero
> functions, groups. So pinctrl-scmi.c could not be reused for i.MX95.
> Because nxp,pin-func, nxp,pin-conf properties are rejected by dt maintainers,
> so use 'fsl,pins' which requires a new driver pinctrl-imx-scmi.c
>
> The node will be as below:
> pinctrl_usdhc1: usdhc1grp {
> fsl,pins = <
> IMX95_PAD_SD1_CLK__USDHC1_CLK
> 0x158e
> IMX95_PAD_SD1_CMD__USDHC1_CMD
> 0x138e
> IMX95_PAD_SD1_DATA0__USDHC1_DATA0
> 0x138e
> IMX95_PAD_SD1_DATA1__USDHC1_DATA1
> 0x138e
> IMX95_PAD_SD1_DATA2__USDHC1_DATA2
> 0x138e
> IMX95_PAD_SD1_DATA3__USDHC1_DATA3
> 0x138e
> IMX95_PAD_SD1_DATA4__USDHC1_DATA4
> 0x138e
> IMX95_PAD_SD1_DATA5__USDHC1_DATA5
> 0x138e
> IMX95_PAD_SD1_DATA6__USDHC1_DATA6
> 0x138e
> IMX95_PAD_SD1_DATA7__USDHC1_DATA7
> 0x138e
> IMX95_PAD_SD1_STROBE__USDHC1_STROBE
> 0x158e
> >;
> };
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> Peng Fan (3):
> dt-bindings: firmware: arm,scmi: Add properties for i.MX95 Pinctrl OEM
> extensions
> pinctrl: scmi: add blocklist
> pinctrl: imx: support SCMI pinctrl protocol for i.MX95
>
> .../devicetree/bindings/firmware/arm,scmi.yaml | 4 +-
> .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml | 53 +++
> drivers/pinctrl/freescale/Kconfig | 9 +
> drivers/pinctrl/freescale/Makefile | 1 +
> drivers/pinctrl/freescale/pinctrl-imx-scmi.c | 357
> +++++++++++++++++++++
> drivers/pinctrl/pinctrl-scmi.c | 9 +
> 6 files changed, 432 insertions(+), 1 deletion(-)
> ---
> base-commit: 632483ea8004edfadd035de36e1ab2c7c4f53158
> change-id: 20240521-pinctrl-scmi-imx95-867bea9595cf
>
> Best regards,
> --
> Peng Fan <[email protected]>
On Mon, May 27, 2024 at 10:36 AM Peng Fan <[email protected]> wrote:
> Hi Linus, Sudeep, Cristian,
>
> > Subject: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions with
> > fsl,pins property
>
> Sorry if this is an early ping to you. Just wanna this not blocking i.MX95
> upstream support.
>
> Are you ok with this patchset?
Patch 1 is fine as it's ACKed by the DT maintainers.
Patch 2 patches the SCMI pinctrl driver and patch 3 uses
module_scmi_driver(), so I cannot merge any of these patches
without ACK from the SCMI maintainers.
The code in patch 3 looks OK but I will comment on it, no big
deal though.
Yours,
Linus Walleij
On Tue, May 21, 2024 at 8:17 AM Peng Fan (OSS) <[email protected]> wrote:
> +static int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev,
> + struct device_node *np,
> + struct pinctrl_map **map,
> + unsigned int *num_maps)
(...)
> +static int pinctrl_scmi_imx_pinconf_set(struct pinctrl_dev *pctldev,
> + unsigned int pin,
> + unsigned long *configs,
> + unsigned int num_configs)
The code in these functions look suspiciously similar to same code
in pinctrl-imx.c, I bet it is copy/pase/modify.
Can you look a second time if it is possible to share code between the
drivers?
It's not super much code, I'm mostly worried about bugs having to be
fixed in two places.
What is the opinion of the othe i.MX pinctrl maintainers?
Yours,
Linus Walleij
Hi Linus,
> Subject: Re: [PATCH 3/3] pinctrl: imx: support SCMI pinctrl protocol for
> i.MX95
>
> On Tue, May 21, 2024 at 8:17 AM Peng Fan (OSS) <[email protected]>
> wrote:
>
> > +static int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev,
> > + struct device_node *np,
> > + struct pinctrl_map **map,
> > + unsigned int *num_maps)
> (...)
> > +static int pinctrl_scmi_imx_pinconf_set(struct pinctrl_dev *pctldev,
> > + unsigned int pin,
> > + unsigned long *configs,
> > + unsigned int num_configs)
>
> The code in these functions look suspiciously similar to same code in pinctrl-
> imx.c, I bet it is copy/pase/modify.
I only took the imx_pinctrl_parse_pin_mmio as example to get parse the node
and do the pinctrl_scmi_imx_dt_node_to_map here. Only the pieces:
"be32_to_cpu(*list++); "
For other parts, they are different. There is no MUX here, configs
has vendor SCMI "IMX_SCMI_PIN_X", and more.
>
> Can you look a second time if it is possible to share code between the drivers?
I thought about this. Just trying what did for i.MX8 SCU pinctrl API by adding
IMX_USE_SCMI flag.
But because that means more if else check in pinctrl-imx.c and
scmi requires different configs layout, which makes pinctrl-imx.c looks
messy. And scmi pinctrl requires a totally different probe function,
not imx_pinctrl_probe. So I decided to write a new driver for i.MX95.
>
> It's not super much code, I'm mostly worried about bugs having to be fixed in
> two places.
I could switch back to my initial try to share pinctrl-imx.c, but I hope not.
>
> What is the opinion of the othe i.MX pinctrl maintainers?
Aisheng, Fabio, Shawn, Jacky, any comments?
Thanks,
Peng.
>
> Yours,
> Linus Walleij
Hi Linus,
Thanks for your quick response.
> Subject: Re: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions with
> fsl,pins property
>
> On Mon, May 27, 2024 at 10:36 AM Peng Fan <[email protected]> wrote:
>
> > Hi Linus, Sudeep, Cristian,
> >
> > > Subject: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions
> > > with fsl,pins property
> >
> > Sorry if this is an early ping to you. Just wanna this not blocking
> > i.MX95 upstream support.
> >
> > Are you ok with this patchset?
>
> Patch 1 is fine as it's ACKed by the DT maintainers.
>
> Patch 2 patches the SCMI pinctrl driver and patch 3 uses
> module_scmi_driver(), so I cannot merge any of these patches without ACK
> from the SCMI maintainers.
Sudeep, Cristian, May I get your ACK for the patches?
>
> The code in patch 3 looks OK but I will comment on it, no big deal though.
Thanks, I just replied there.
Thanks,
Peng.
>
> Yours,
> Linus Walleij
> From: Peng Fan <[email protected]>
> Sent: 2024年5月27日 21:18
>
> Hi Linus,
>
> > Subject: Re: [PATCH 3/3] pinctrl: imx: support SCMI pinctrl protocol
> > for
> > i.MX95
> >
> > On Tue, May 21, 2024 at 8:17 AM Peng Fan (OSS) <[email protected]>
> > wrote:
> >
> > > +static int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev,
> > > + struct device_node *np,
> > > + struct pinctrl_map
> **map,
> > > + unsigned int
> *num_maps)
> > (...)
> > > +static int pinctrl_scmi_imx_pinconf_set(struct pinctrl_dev *pctldev,
> > > + unsigned int pin,
> > > + unsigned long *configs,
> > > + unsigned int num_configs)
> >
> > The code in these functions look suspiciously similar to same code in
> > pinctrl- imx.c, I bet it is copy/pase/modify.
>
> I only took the imx_pinctrl_parse_pin_mmio as example to get parse the node
> and do the pinctrl_scmi_imx_dt_node_to_map here. Only the pieces:
> "be32_to_cpu(*list++); "
>
> For other parts, they are different. There is no MUX here, configs has vendor
> SCMI "IMX_SCMI_PIN_X", and more.
>
> >
> > Can you look a second time if it is possible to share code between the
> drivers?
>
> I thought about this. Just trying what did for i.MX8 SCU pinctrl API by adding
> IMX_USE_SCMI flag.
>
> But because that means more if else check in pinctrl-imx.c and scmi requires
> different configs layout, which makes pinctrl-imx.c looks messy. And scmi
> pinctrl requires a totally different probe function, not imx_pinctrl_probe. So I
> decided to write a new driver for i.MX95.
>
Agree. We once had a local discussion before on whether can reuse the pinctrl-imx.c.
Current implementation is more tiny and clean for SCMI only which has many logic difference
from the legacy platforms in pinctrl-imx.c. (e.g. no grp/function, no static pin
definition/registration, pin configuration packing). Reuse requires adding more code.
So we think probably better to keep it a separate driver for SCMI only.
Regards
Aisheng
> >
> > It's not super much code, I'm mostly worried about bugs having to be
> > fixed in two places.
>
> I could switch back to my initial try to share pinctrl-imx.c, but I hope not.
>
> >
> > What is the opinion of the othe i.MX pinctrl maintainers?
>
> Aisheng, Fabio, Shawn, Jacky, any comments?
>
> Thanks,
> Peng.
>
> >
> > Yours,
> > Linus Walleij
> From: Peng Fan (OSS) <[email protected]>
> Sent: 2024年5月21日 14:26
>
> All:
> There was a v6 that use generic properties, but at a late stage, NXP internals
> decides to switch to fsl,pins property to align with other i.MXs. Since new
> properties, drivers rewrite, I start this patchset from v1 with a new patch title.
> A RFC patch for binding was posted, since Rob said he is fine, so post this
> patchset out.
>
> Whether v6 or this patchset, patch 2 is a must and was not changed from
> v6.
>
> The pinctrl stuff has been pending for quite sometime, I would be
> apprecaited if any quick comments.
>
> v6:
>
> https://lore.kernel.org/all/20240513-pinctrl-scmi-oem-v3-v6-0-904975c99cc4
> @nxp.com/
> RFC:
> https://lore.kernel.org/all/[email protected]/
>
> Thanks,
> Peng.
>
> ARM SCMI v3.2 Table 24 Pin Configuration Type and Enumerations:
> '192 -255 OEM specific units'.
>
> i.MX95 System Manager FW supports SCMI PINCTRL protocol, but it has zero
> functions, groups. So pinctrl-scmi.c could not be reused for i.MX95.
> Because nxp,pin-func, nxp,pin-conf properties are rejected by dt maintainers,
> so use 'fsl,pins' which requires a new driver pinctrl-imx-scmi.c
>
> The node will be as below:
> pinctrl_usdhc1: usdhc1grp {
> fsl,pins = <
> IMX95_PAD_SD1_CLK__USDHC1_CLK 0x158e
> IMX95_PAD_SD1_CMD__USDHC1_CMD 0x138e
> IMX95_PAD_SD1_DATA0__USDHC1_DATA0 0x138e
> IMX95_PAD_SD1_DATA1__USDHC1_DATA1 0x138e
> IMX95_PAD_SD1_DATA2__USDHC1_DATA2 0x138e
> IMX95_PAD_SD1_DATA3__USDHC1_DATA3 0x138e
> IMX95_PAD_SD1_DATA4__USDHC1_DATA4 0x138e
> IMX95_PAD_SD1_DATA5__USDHC1_DATA5 0x138e
> IMX95_PAD_SD1_DATA6__USDHC1_DATA6 0x138e
> IMX95_PAD_SD1_DATA7__USDHC1_DATA7 0x138e
> IMX95_PAD_SD1_STROBE__USDHC1_STROBE 0x158e
> >;
> };
>
> Signed-off-by: Peng Fan <[email protected]>
For the series:
Reviewed-by: Dong Aisheng <[email protected]>
Regards
Aisheng
On Tue, May 21, 2024 at 02:25:58PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> i.MX95 will have its own pinctrl scmi driver, so need block
> pinctrl-scmi driver for i.MX95, otherwise there will be two pinctrl
> devices for a single scmi protocol@19.
>
I am not happy with NXP's we will use SCMI but in our own custom way.
So take one last change to crib about it and hence reluctantly,
Acked-by: Sudeep Holla <[email protected]>
--
Regards,
Sudeep
On Mon, May 27, 2024 at 08:36:27AM +0000, Peng Fan wrote:
> Hi Linus, Sudeep, Cristian,
>
> > Subject: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions with
> > fsl,pins property
>
> Sorry if this is an early ping to you. Just wanna this not blocking i.MX95
> upstream support.
>
I would say yes as this was posted bang in the middle of the merge window.
So it is possible for people to miss this if they are busy otherwise.
I wouldn't have responded in general or if someone is new to the Linux
kernel development. But you are no new to kernel development.
In general I also have a suggestion for you. Avoid churning the dependent
patch series if the base set of patches are not yet reviewed or agreed upon.
I was super confused with the amount of different concurrent but dependent
patch series you had for this whole i.MX SCMI pinmux support. I had ignored
and not responded in the past but thought it would be good to respond in
this thread.
--
Regards,
Sudeep
Hi Sudeep,
> Subject: Re: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions with
> fsl,pins property
>
> On Mon, May 27, 2024 at 08:36:27AM +0000, Peng Fan wrote:
> > Hi Linus, Sudeep, Cristian,
> >
> > > Subject: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions
> > > with fsl,pins property
> >
> > Sorry if this is an early ping to you. Just wanna this not blocking
> > i.MX95 upstream support.
> >
>
> I would say yes as this was posted bang in the middle of the merge window.
> So it is possible for people to miss this if they are busy otherwise.
>
> I wouldn't have responded in general or if someone is new to the Linux kernel
> development. But you are no new to kernel development.
>
> In general I also have a suggestion for you. Avoid churning the dependent
> patch series if the base set of patches are not yet reviewed or agreed upon.
> I was super confused with the amount of different concurrent but dependent
> patch series you had for this whole i.MX SCMI pinmux support. I had ignored
> and not responded in the past but thought it would be good to respond in
> this thread.
Thanks for your suggestion. I tried to do different implementations that
could make all of us agree, so it was indeed many versions with different
implementations. Sorry. I will improve.
BTW: would you please also give an ACK for patch 3, because patch 3 uses
module_scmi_driver?
Thanks,
Peng.
>
> --
> Regards,
> Sudeep
On Tue, May 21, 2024 at 02:25:59PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> The generic pinctrl-scmi.c driver could not be used for i.MX95 because
> i.MX95 SCMI firmware not supports functions, groups or generic
> 'Pin Configuration Type and Enumerations' listed in SCMI Specification.
>
> i.MX95 System Control Management Interface(SCMI) firmware only supports
> below pin configuration types which are OEM specific types:
> 192: PIN MUX
> 193: PIN CONF
> 194: DAISY ID
> 195: DAISY VAL
>
> To Support fsl,pins property together with SCMI OEM protocol, add this
> driver.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/pinctrl/freescale/Kconfig | 9 +
> drivers/pinctrl/freescale/Makefile | 1 +
> drivers/pinctrl/freescale/pinctrl-imx-scmi.c | 357 +++++++++++++++++++++++++++
> 3 files changed, 367 insertions(+)
>
Only tested compilation and interactions with Generic Pinctrl SCMI
driver, LGTM.
Reviewed-by: Cristian Marussi <[email protected]>
Thanks,
Cristian
On Tue, May 21, 2024 at 02:25:59PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> The generic pinctrl-scmi.c driver could not be used for i.MX95 because
> i.MX95 SCMI firmware not supports functions, groups or generic
> 'Pin Configuration Type and Enumerations' listed in SCMI Specification.
>
> i.MX95 System Control Management Interface(SCMI) firmware only supports
> below pin configuration types which are OEM specific types:
> 192: PIN MUX
> 193: PIN CONF
> 194: DAISY ID
> 195: DAISY VAL
>
> To Support fsl,pins property together with SCMI OEM protocol, add this
> driver.
>
(For the scmi interface usage part)
Acked-by: Sudeep Holla <[email protected]>
--
Regards,
Sudeep
On Tue, Jun 04, 2024 at 12:49:13AM +0000, Peng Fan wrote:
> Hi Sudeep,
>
> > Subject: Re: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions with
> > fsl,pins property
> >
> > On Mon, May 27, 2024 at 08:36:27AM +0000, Peng Fan wrote:
> > > Hi Linus, Sudeep, Cristian,
> > >
> > > > Subject: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions
> > > > with fsl,pins property
> > >
> > > Sorry if this is an early ping to you. Just wanna this not blocking
> > > i.MX95 upstream support.
> > >
> >
> > I would say yes as this was posted bang in the middle of the merge window.
> > So it is possible for people to miss this if they are busy otherwise.
> >
> > I wouldn't have responded in general or if someone is new to the Linux kernel
> > development. But you are no new to kernel development.
> >
> > In general I also have a suggestion for you. Avoid churning the dependent
> > patch series if the base set of patches are not yet reviewed or agreed upon.
> > I was super confused with the amount of different concurrent but dependent
> > patch series you had for this whole i.MX SCMI pinmux support. I had ignored
> > and not responded in the past but thought it would be good to respond in
> > this thread.
>
> Thanks for your suggestion. I tried to do different implementations that
> could make all of us agree, so it was indeed many versions with different
> implementations. Sorry. I will improve.
>
Thanks and sorry again if it is harsh but it was indeed confusing.
> BTW: would you please also give an ACK for patch 3, because patch 3 uses
> module_scmi_driver?
Done.
--
Regards,
Sudeep
Hi Linus,
> Subject: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions with
> fsl,pins property
>
Is this patchset good for you to pick up?
SCMI and i.MX maintainers has gave A-b and R-b a few days ago.
Thanks,
Peng.
On Tue, May 21, 2024 at 8:17 AM Peng Fan (OSS) <[email protected]> wrote:
> There was a v6 that use generic properties, but at a late stage, NXP
> internals decides to switch to fsl,pins property to align with other
> i.MXs. Since new properties, drivers rewrite, I start this patchset
> from v1 with a new patch title. A RFC patch for binding was posted,
> since Rob said he is fine, so post this patchset out.
>
> Whether v6 or this patchset, patch 2 is a must and was not changed from
> v6.
>
> The pinctrl stuff has been pending for quite sometime, I would be
> apprecaited if any quick comments.
>
> v6:
> https://lore.kernel.org/all/[email protected]/
> RFC:
> https://lore.kernel.org/all/[email protected]/
Patches applied.
Yours,
Linus Walleij
Hi Linus,
> Subject: Re: [PATCH 0/3] pinctrl: scmi: support i.MX95 OEM extensions with
> fsl,pins property
>
> On Tue, May 21, 2024 at 8:17 AM Peng Fan (OSS) <[email protected]>
> wrote:
>
> > There was a v6 that use generic properties, but at a late stage, NXP
> > internals decides to switch to fsl,pins property to align with other
> > i.MXs. Since new properties, drivers rewrite, I start this patchset
> > from v1 with a new patch title. A RFC patch for binding was posted,
> > since Rob said he is fine, so post this patchset out.
>
> Patches applied.
Just checked your repo in git.kernel.org, not see the patches.
No big deal, just wonder if they got forgotten.
Thanks,
Peng.
>
> Yours,
> Linus Walleij