* [PATCH 1/4] soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
2025-08-12 11:02 [PATCH 0/4] Add support of IRQs to QUICC ENGINE GPIOs Christophe Leroy
@ 2025-08-12 11:02 ` Christophe Leroy
2025-08-13 7:49 ` kernel test robot
2025-08-12 11:02 ` [PATCH 2/4] soc: fsl: qe: Change GPIO driver to a proper platform driver Christophe Leroy
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Christophe Leroy @ 2025-08-12 11:02 UTC (permalink / raw)
To: Qiang Zhao, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Christophe Leroy, linux-kernel, linuxppc-dev, linux-arm-kernel,
linux-gpio, devicetree
The QUICC Engine provides interrupts for a few I/O ports. This is
handled via a separate interrupt ID and managed via a triplet of
dedicated registers hosted by the SoC.
Implement an interrupt driver for it for that those IRQs can then
be linked to the related GPIOs.
The number of ports for which interrupts are supported depends on
the microcontroller:
- mpc8323 has 10 interrupts
- mpc8360 has 28 interrupts
- mpc8568 has 18 interrupts
So add this information as data of the compatible.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
drivers/soc/fsl/qe/Makefile | 2 +-
drivers/soc/fsl/qe/qe_ports_ic.c | 156 +++++++++++++++++++++++++++++++
2 files changed, 157 insertions(+), 1 deletion(-)
create mode 100644 drivers/soc/fsl/qe/qe_ports_ic.c
diff --git a/drivers/soc/fsl/qe/Makefile b/drivers/soc/fsl/qe/Makefile
index ec8506e131136..901a9c40d5eb7 100644
--- a/drivers/soc/fsl/qe/Makefile
+++ b/drivers/soc/fsl/qe/Makefile
@@ -11,4 +11,4 @@ obj-$(CONFIG_UCC_SLOW) += ucc_slow.o
obj-$(CONFIG_UCC_FAST) += ucc_fast.o
obj-$(CONFIG_QE_TDM) += qe_tdm.o
obj-$(CONFIG_QE_USB) += usb.o
-obj-$(CONFIG_QE_GPIO) += gpio.o
+obj-$(CONFIG_QE_GPIO) += gpio.o qe_ports_ic.o
diff --git a/drivers/soc/fsl/qe/qe_ports_ic.c b/drivers/soc/fsl/qe/qe_ports_ic.c
new file mode 100644
index 0000000000000..2ab82ac259564
--- /dev/null
+++ b/drivers/soc/fsl/qe/qe_ports_ic.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * QUICC ENGINE I/O Ports Interrupt Controller
+ *
+ * Copyright (c) 2025 Christophe Leroy CS GROUP France (christophe.leroy@csgroup.eu)
+ */
+
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/platform_device.h>
+
+/* QE IC registers offset */
+#define CEPIER 0x0c
+#define CEPIMR 0x10
+#define CEPICR 0x14
+
+struct qepic_data {
+ void __iomem *reg;
+ struct irq_domain *host;
+};
+
+static void qepic_mask(struct irq_data *d)
+{
+ struct qepic_data *data = irq_data_get_irq_chip_data(d);
+
+ clrbits32(data->reg + CEPIMR, 1 << (31 - irqd_to_hwirq(d)));
+}
+
+static void qepic_unmask(struct irq_data *d)
+{
+ struct qepic_data *data = irq_data_get_irq_chip_data(d);
+
+ setbits32(data->reg + CEPIMR, 1 << (31 - irqd_to_hwirq(d)));
+}
+
+static void qepic_end(struct irq_data *d)
+{
+ struct qepic_data *data = irq_data_get_irq_chip_data(d);
+
+ out_be32(data->reg + CEPIER, 1 << (31 - irqd_to_hwirq(d)));
+}
+
+static int qepic_set_type(struct irq_data *d, unsigned int flow_type)
+{
+ struct qepic_data *data = irq_data_get_irq_chip_data(d);
+ unsigned int vec = (unsigned int)irqd_to_hwirq(d);
+
+ switch (flow_type & IRQ_TYPE_SENSE_MASK) {
+ case IRQ_TYPE_EDGE_FALLING:
+ setbits32(data->reg + CEPICR, 1 << (31 - vec));
+ return 0;
+ case IRQ_TYPE_EDGE_BOTH:
+ case IRQ_TYPE_NONE:
+ clrbits32(data->reg + CEPICR, 1 << (31 - vec));
+ return 0;
+ }
+ return -EINVAL;
+}
+
+static struct irq_chip qepic = {
+ .name = "QEPIC",
+ .irq_mask = qepic_mask,
+ .irq_unmask = qepic_unmask,
+ .irq_eoi = qepic_end,
+ .irq_set_type = qepic_set_type,
+};
+
+static int qepic_get_irq(struct irq_desc *desc)
+{
+ struct qepic_data *data = irq_desc_get_handler_data(desc);
+ u32 event = in_be32(data->reg + CEPIER);
+
+ if (!event)
+ return -1;
+
+ return irq_find_mapping(data->host, 32 - ffs(event));
+}
+
+static void qepic_cascade(struct irq_desc *desc)
+{
+ generic_handle_irq(qepic_get_irq(desc));
+}
+
+static int qepic_host_map(struct irq_domain *h, unsigned int virq, irq_hw_number_t hw)
+{
+ irq_set_chip_data(virq, h->host_data);
+ irq_set_chip_and_handler(virq, &qepic, handle_fasteoi_irq);
+ return 0;
+}
+
+static const struct irq_domain_ops qepic_host_ops = {
+ .map = qepic_host_map,
+};
+
+static int qepic_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct qepic_data *data;
+ int irq;
+ int nb;
+
+ nb = (int)of_device_get_match_data(dev);
+ if (nb < 1 || nb > 32)
+ return -EINVAL;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->reg = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(data->reg))
+ return PTR_ERR(data->reg);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ data->host = irq_domain_add_linear(dev->of_node, nb, &qepic_host_ops, data);
+ if (!data->host)
+ return -ENODEV;
+
+ irq_set_handler_data(irq, data);
+ irq_set_chained_handler(irq, qepic_cascade);
+
+ return 0;
+}
+
+static const struct of_device_id qepic_match[] = {
+ {
+ .compatible = "fsl,mpc8323-qe-ports-ic",
+ .data = (void *)10,
+ },
+ {
+ .compatible = "fsl,mpc8360-qe-ports-ic",
+ .data = (void *)28,
+ },
+ {
+ .compatible = "fsl,mpc8568-qe-ports-ic",
+ .data = (void *)18,
+ },
+ {},
+};
+
+static struct platform_driver qepic_driver = {
+ .driver = {
+ .name = "qe_ports_ic",
+ .of_match_table = qepic_match,
+ },
+ .probe = qepic_probe,
+};
+
+static int __init qepic_init(void)
+{
+ return platform_driver_register(&qepic_driver);
+}
+arch_initcall(qepic_init);
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
2025-08-12 11:02 ` [PATCH 1/4] soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports Christophe Leroy
@ 2025-08-13 7:49 ` kernel test robot
0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2025-08-13 7:49 UTC (permalink / raw)
To: Christophe Leroy, Qiang Zhao, Linus Walleij, Bartosz Golaszewski,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: oe-kbuild-all, Christophe Leroy, linux-kernel, linuxppc-dev,
linux-arm-kernel, linux-gpio, devicetree
Hi Christophe,
kernel test robot noticed the following build warnings:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.17-rc1 next-20250813]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christophe-Leroy/soc-fsl-qe-Add-an-interrupt-controller-for-QUICC-Engine-Ports/20250812-195423
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/1dcc9528e97d228ea7889caa00cc254ef0375ed4.1754996033.git.christophe.leroy%40csgroup.eu
patch subject: [PATCH 1/4] soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
config: powerpc64-randconfig-002-20250813 (https://download.01.org/0day-ci/archive/20250813/202508131517.P1Nfz0RF-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250813/202508131517.P1Nfz0RF-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508131517.P1Nfz0RF-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/soc/fsl/qe/qe_ports_ic.c: In function 'qepic_probe':
>> drivers/soc/fsl/qe/qe_ports_ic.c:102:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
nb = (int)of_device_get_match_data(dev);
^
vim +102 drivers/soc/fsl/qe/qe_ports_ic.c
94
95 static int qepic_probe(struct platform_device *pdev)
96 {
97 struct device *dev = &pdev->dev;
98 struct qepic_data *data;
99 int irq;
100 int nb;
101
> 102 nb = (int)of_device_get_match_data(dev);
103 if (nb < 1 || nb > 32)
104 return -EINVAL;
105
106 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
107 if (!data)
108 return -ENOMEM;
109
110 data->reg = devm_platform_ioremap_resource(pdev, 0);
111 if (IS_ERR(data->reg))
112 return PTR_ERR(data->reg);
113
114 irq = platform_get_irq(pdev, 0);
115 if (irq < 0)
116 return irq;
117
118 data->host = irq_domain_add_linear(dev->of_node, nb, &qepic_host_ops, data);
119 if (!data->host)
120 return -ENODEV;
121
122 irq_set_handler_data(irq, data);
123 irq_set_chained_handler(irq, qepic_cascade);
124
125 return 0;
126 }
127
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/4] soc: fsl: qe: Change GPIO driver to a proper platform driver
2025-08-12 11:02 [PATCH 0/4] Add support of IRQs to QUICC ENGINE GPIOs Christophe Leroy
2025-08-12 11:02 ` [PATCH 1/4] soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports Christophe Leroy
@ 2025-08-12 11:02 ` Christophe Leroy
2025-08-12 14:16 ` Bartosz Golaszewski
2025-08-12 14:20 ` Bartosz Golaszewski
2025-08-12 11:02 ` [PATCH 3/4] soc: fsl: qe: Add support of IRQ in QE GPIO Christophe Leroy
2025-08-12 11:02 ` [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports Christophe Leroy
3 siblings, 2 replies; 16+ messages in thread
From: Christophe Leroy @ 2025-08-12 11:02 UTC (permalink / raw)
To: Qiang Zhao, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Christophe Leroy, linux-kernel, linuxppc-dev, linux-arm-kernel,
linux-gpio, devicetree
In order to be able to add interrupts to the GPIOs, first change the
QE GPIO driver to the proper platform driver in order to allow
initialisation to be done in the right order, otherwise the GPIOs
get added before the interrupts are registered.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
drivers/soc/fsl/qe/gpio.c | 88 +++++++++++++++++++++++----------------
1 file changed, 53 insertions(+), 35 deletions(-)
diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
index 8df1e8fa86a5f..b502377193192 100644
--- a/drivers/soc/fsl/qe/gpio.c
+++ b/drivers/soc/fsl/qe/gpio.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/export.h>
#include <linux/property.h>
+#include <linux/platform_device.h>
#include <soc/fsl/qe/qe.h>
@@ -295,45 +296,62 @@ void qe_pin_set_gpio(struct qe_pin *qe_pin)
}
EXPORT_SYMBOL(qe_pin_set_gpio);
-static int __init qe_add_gpiochips(void)
+static int qe_gpio_probe(struct platform_device *ofdev)
{
- struct device_node *np;
-
- for_each_compatible_node(np, NULL, "fsl,mpc8323-qe-pario-bank") {
- int ret;
- struct qe_gpio_chip *qe_gc;
- struct of_mm_gpio_chip *mm_gc;
- struct gpio_chip *gc;
-
- qe_gc = kzalloc(sizeof(*qe_gc), GFP_KERNEL);
- if (!qe_gc) {
- ret = -ENOMEM;
- goto err;
- }
+ struct device *dev = &ofdev->dev;
+ struct device_node *np = dev->of_node;
+ int ret;
+ struct qe_gpio_chip *qe_gc;
+ struct of_mm_gpio_chip *mm_gc;
+ struct gpio_chip *gc;
+
+ qe_gc = kzalloc(sizeof(*qe_gc), GFP_KERNEL);
+ if (!qe_gc) {
+ ret = -ENOMEM;
+ goto err;
+ }
- spin_lock_init(&qe_gc->lock);
+ spin_lock_init(&qe_gc->lock);
- mm_gc = &qe_gc->mm_gc;
- gc = &mm_gc->gc;
+ mm_gc = &qe_gc->mm_gc;
+ gc = &mm_gc->gc;
- mm_gc->save_regs = qe_gpio_save_regs;
- gc->ngpio = QE_PIO_PINS;
- gc->direction_input = qe_gpio_dir_in;
- gc->direction_output = qe_gpio_dir_out;
- gc->get = qe_gpio_get;
- gc->set = qe_gpio_set;
- gc->set_multiple = qe_gpio_set_multiple;
+ mm_gc->save_regs = qe_gpio_save_regs;
+ gc->ngpio = QE_PIO_PINS;
+ gc->direction_input = qe_gpio_dir_in;
+ gc->direction_output = qe_gpio_dir_out;
+ gc->get = qe_gpio_get;
+ gc->set = qe_gpio_set;
+ gc->set_multiple = qe_gpio_set_multiple;
- ret = of_mm_gpiochip_add_data(np, mm_gc, qe_gc);
- if (ret)
- goto err;
- continue;
+ ret = of_mm_gpiochip_add_data(np, mm_gc, qe_gc);
+ if (!ret)
+ return 0;
err:
- pr_err("%pOF: registration failed with status %d\n",
- np, ret);
- kfree(qe_gc);
- /* try others anyway */
- }
- return 0;
+ dev_err(dev, "registration failed with status %d\n", ret);
+ kfree(qe_gc);
+
+ return ret;
+}
+
+static const struct of_device_id qe_gpio_match[] = {
+ {
+ .compatible = "fsl,mpc8323-qe-pario-bank",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, qe_gpio_match);
+
+static struct platform_driver qe_gpio_driver = {
+ .probe = qe_gpio_probe,
+ .driver = {
+ .name = "qe-gpio",
+ .of_match_table = qe_gpio_match,
+ },
+};
+
+static int __init qe_gpio_init(void)
+{
+ return platform_driver_register(&qe_gpio_driver);
}
-arch_initcall(qe_add_gpiochips);
+arch_initcall(qe_gpio_init);
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] soc: fsl: qe: Change GPIO driver to a proper platform driver
2025-08-12 11:02 ` [PATCH 2/4] soc: fsl: qe: Change GPIO driver to a proper platform driver Christophe Leroy
@ 2025-08-12 14:16 ` Bartosz Golaszewski
2025-08-12 14:20 ` Bartosz Golaszewski
1 sibling, 0 replies; 16+ messages in thread
From: Bartosz Golaszewski @ 2025-08-12 14:16 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, linuxppc-dev, linux-arm-kernel, linux-gpio,
devicetree, Qiang Zhao, Linus Walleij, Bartosz Golaszewski,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
On Tue, 12 Aug 2025 13:02:52 +0200, Christophe Leroy
<christophe.leroy@csgroup.eu> said:
> In order to be able to add interrupts to the GPIOs, first change the
> QE GPIO driver to the proper platform driver in order to allow
> initialisation to be done in the right order, otherwise the GPIOs
> get added before the interrupts are registered.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] soc: fsl: qe: Change GPIO driver to a proper platform driver
2025-08-12 11:02 ` [PATCH 2/4] soc: fsl: qe: Change GPIO driver to a proper platform driver Christophe Leroy
2025-08-12 14:16 ` Bartosz Golaszewski
@ 2025-08-12 14:20 ` Bartosz Golaszewski
1 sibling, 0 replies; 16+ messages in thread
From: Bartosz Golaszewski @ 2025-08-12 14:20 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, linuxppc-dev, linux-arm-kernel, linux-gpio,
devicetree, Qiang Zhao, Linus Walleij, Bartosz Golaszewski,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
On Tue, 12 Aug 2025 13:02:52 +0200, Christophe Leroy
<christophe.leroy@csgroup.eu> said:
> In order to be able to add interrupts to the GPIOs, first change the
> QE GPIO driver to the proper platform driver in order to allow
> initialisation to be done in the right order, otherwise the GPIOs
> get added before the interrupts are registered.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> drivers/soc/fsl/qe/gpio.c | 88 +++++++++++++++++++++++----------------
> 1 file changed, 53 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
> index 8df1e8fa86a5f..b502377193192 100644
> --- a/drivers/soc/fsl/qe/gpio.c
> +++ b/drivers/soc/fsl/qe/gpio.c
> @@ -19,6 +19,7 @@
> #include <linux/slab.h>
> #include <linux/export.h>
> #include <linux/property.h>
> +#include <linux/platform_device.h>
>
> #include <soc/fsl/qe/qe.h>
>
> @@ -295,45 +296,62 @@ void qe_pin_set_gpio(struct qe_pin *qe_pin)
> }
> EXPORT_SYMBOL(qe_pin_set_gpio);
>
> -static int __init qe_add_gpiochips(void)
> +static int qe_gpio_probe(struct platform_device *ofdev)
> {
> - struct device_node *np;
> -
> - for_each_compatible_node(np, NULL, "fsl,mpc8323-qe-pario-bank") {
> - int ret;
> - struct qe_gpio_chip *qe_gc;
> - struct of_mm_gpio_chip *mm_gc;
> - struct gpio_chip *gc;
> -
> - qe_gc = kzalloc(sizeof(*qe_gc), GFP_KERNEL);
> - if (!qe_gc) {
> - ret = -ENOMEM;
> - goto err;
> - }
> + struct device *dev = &ofdev->dev;
> + struct device_node *np = dev->of_node;
> + int ret;
> + struct qe_gpio_chip *qe_gc;
> + struct of_mm_gpio_chip *mm_gc;
> + struct gpio_chip *gc;
> +
> + qe_gc = kzalloc(sizeof(*qe_gc), GFP_KERNEL);
> + if (!qe_gc) {
> + ret = -ENOMEM;
> + goto err;
> + }
>
> - spin_lock_init(&qe_gc->lock);
> + spin_lock_init(&qe_gc->lock);
>
> - mm_gc = &qe_gc->mm_gc;
> - gc = &mm_gc->gc;
> + mm_gc = &qe_gc->mm_gc;
> + gc = &mm_gc->gc;
>
> - mm_gc->save_regs = qe_gpio_save_regs;
> - gc->ngpio = QE_PIO_PINS;
> - gc->direction_input = qe_gpio_dir_in;
> - gc->direction_output = qe_gpio_dir_out;
> - gc->get = qe_gpio_get;
> - gc->set = qe_gpio_set;
> - gc->set_multiple = qe_gpio_set_multiple;
> + mm_gc->save_regs = qe_gpio_save_regs;
> + gc->ngpio = QE_PIO_PINS;
> + gc->direction_input = qe_gpio_dir_in;
> + gc->direction_output = qe_gpio_dir_out;
> + gc->get = qe_gpio_get;
> + gc->set = qe_gpio_set;
> + gc->set_multiple = qe_gpio_set_multiple;
>
> - ret = of_mm_gpiochip_add_data(np, mm_gc, qe_gc);
> - if (ret)
> - goto err;
> - continue;
> + ret = of_mm_gpiochip_add_data(np, mm_gc, qe_gc);
Actually scratch my R-b, on second glance - this should now be
replaced with devm_gpiochip_add_data(). I don't see anything that
would be in the way now that it's an actuall platform device.
Bartosz
> + if (!ret)
> + return 0;
> err:
> - pr_err("%pOF: registration failed with status %d\n",
> - np, ret);
> - kfree(qe_gc);
> - /* try others anyway */
> - }
> - return 0;
> + dev_err(dev, "registration failed with status %d\n", ret);
> + kfree(qe_gc);
> +
> + return ret;
> +}
> +
> +static const struct of_device_id qe_gpio_match[] = {
> + {
> + .compatible = "fsl,mpc8323-qe-pario-bank",
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, qe_gpio_match);
> +
> +static struct platform_driver qe_gpio_driver = {
> + .probe = qe_gpio_probe,
> + .driver = {
> + .name = "qe-gpio",
> + .of_match_table = qe_gpio_match,
> + },
> +};
> +
> +static int __init qe_gpio_init(void)
> +{
> + return platform_driver_register(&qe_gpio_driver);
> }
> -arch_initcall(qe_add_gpiochips);
> +arch_initcall(qe_gpio_init);
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] soc: fsl: qe: Add support of IRQ in QE GPIO
2025-08-12 11:02 [PATCH 0/4] Add support of IRQs to QUICC ENGINE GPIOs Christophe Leroy
2025-08-12 11:02 ` [PATCH 1/4] soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports Christophe Leroy
2025-08-12 11:02 ` [PATCH 2/4] soc: fsl: qe: Change GPIO driver to a proper platform driver Christophe Leroy
@ 2025-08-12 11:02 ` Christophe Leroy
2025-08-12 14:21 ` Bartosz Golaszewski
2025-08-12 15:30 ` Krzysztof Kozlowski
2025-08-12 11:02 ` [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports Christophe Leroy
3 siblings, 2 replies; 16+ messages in thread
From: Christophe Leroy @ 2025-08-12 11:02 UTC (permalink / raw)
To: Qiang Zhao, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Christophe Leroy, linux-kernel, linuxppc-dev, linux-arm-kernel,
linux-gpio, devicetree
In the QE, a few GPIOs are IRQ capable. Similarly to
commit 726bd223105c ("powerpc/8xx: Adding support of IRQ in MPC8xx
GPIO"), add IRQ support to QE GPIO.
Add property 'fsl,qe-gpio-irq-mask' similar to
'fsl,cpm1-gpio-irq-mask' that define which of the GPIOs have IRQs.
Here is an exemple for port B of mpc8323 which has IRQs for
GPIOs PB7, PB9, PB25 and PB27.
qe_pio_b: gpio-controller@1418 {
#gpio-cells = <2>;
compatible = "fsl,mpc8323-qe-pario-bank";
reg = <0x1418 0x18>;
interrupts = <4 5 6 7>;
fsl,qe-gpio-irq-mask = <0x01400050>;
interrupt-parent = <&qepic>;
gpio-controller;
};
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
drivers/soc/fsl/qe/gpio.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
index b502377193192..59145652ad850 100644
--- a/drivers/soc/fsl/qe/gpio.c
+++ b/drivers/soc/fsl/qe/gpio.c
@@ -13,6 +13,7 @@
#include <linux/err.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/gpio/legacy-of-mm-gpiochip.h>
#include <linux/gpio/consumer.h>
#include <linux/gpio/driver.h>
@@ -32,6 +33,8 @@ struct qe_gpio_chip {
/* saved_regs used to restore dedicated functions */
struct qe_pio_regs saved_regs;
+
+ int irq[32];
};
static void qe_gpio_save_regs(struct of_mm_gpio_chip *mm_gc)
@@ -141,6 +144,13 @@ static int qe_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
return 0;
}
+static int qe_gpio_to_irq(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct qe_gpio_chip *qe_gc = gpiochip_get_data(gc);
+
+ return qe_gc->irq[gpio] ? : -ENXIO;
+}
+
struct qe_pin {
/*
* The qe_gpio_chip name is unfortunate, we should change that to
@@ -304,6 +314,7 @@ static int qe_gpio_probe(struct platform_device *ofdev)
struct qe_gpio_chip *qe_gc;
struct of_mm_gpio_chip *mm_gc;
struct gpio_chip *gc;
+ u32 mask;
qe_gc = kzalloc(sizeof(*qe_gc), GFP_KERNEL);
if (!qe_gc) {
@@ -313,6 +324,14 @@ static int qe_gpio_probe(struct platform_device *ofdev)
spin_lock_init(&qe_gc->lock);
+ if (!of_property_read_u32(np, "fsl,qe-gpio-irq-mask", &mask)) {
+ int i, j;
+
+ for (i = 0, j = 0; i < 32; i++)
+ if (mask & (1 << (31 - i)))
+ qe_gc->irq[i] = irq_of_parse_and_map(np, j++);
+ }
+
mm_gc = &qe_gc->mm_gc;
gc = &mm_gc->gc;
@@ -323,6 +342,7 @@ static int qe_gpio_probe(struct platform_device *ofdev)
gc->get = qe_gpio_get;
gc->set = qe_gpio_set;
gc->set_multiple = qe_gpio_set_multiple;
+ gc->to_irq = qe_gpio_to_irq;
ret = of_mm_gpiochip_add_data(np, mm_gc, qe_gc);
if (!ret)
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] soc: fsl: qe: Add support of IRQ in QE GPIO
2025-08-12 11:02 ` [PATCH 3/4] soc: fsl: qe: Add support of IRQ in QE GPIO Christophe Leroy
@ 2025-08-12 14:21 ` Bartosz Golaszewski
2025-08-18 8:33 ` Christophe Leroy
2025-08-12 15:30 ` Krzysztof Kozlowski
1 sibling, 1 reply; 16+ messages in thread
From: Bartosz Golaszewski @ 2025-08-12 14:21 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, linuxppc-dev, linux-arm-kernel, linux-gpio,
devicetree, Qiang Zhao, Linus Walleij, Bartosz Golaszewski,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
On Tue, 12 Aug 2025 13:02:53 +0200, Christophe Leroy
<christophe.leroy@csgroup.eu> said:
> In the QE, a few GPIOs are IRQ capable. Similarly to
> commit 726bd223105c ("powerpc/8xx: Adding support of IRQ in MPC8xx
> GPIO"), add IRQ support to QE GPIO.
>
> Add property 'fsl,qe-gpio-irq-mask' similar to
> 'fsl,cpm1-gpio-irq-mask' that define which of the GPIOs have IRQs.
>
> Here is an exemple for port B of mpc8323 which has IRQs for
> GPIOs PB7, PB9, PB25 and PB27.
>
> qe_pio_b: gpio-controller@1418 {
> #gpio-cells = <2>;
> compatible = "fsl,mpc8323-qe-pario-bank";
> reg = <0x1418 0x18>;
> interrupts = <4 5 6 7>;
> fsl,qe-gpio-irq-mask = <0x01400050>;
> interrupt-parent = <&qepic>;
> gpio-controller;
> };
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> drivers/soc/fsl/qe/gpio.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
> index b502377193192..59145652ad850 100644
> --- a/drivers/soc/fsl/qe/gpio.c
> +++ b/drivers/soc/fsl/qe/gpio.c
> @@ -13,6 +13,7 @@
> #include <linux/err.h>
> #include <linux/io.h>
> #include <linux/of.h>
> +#include <linux/of_irq.h>
> #include <linux/gpio/legacy-of-mm-gpiochip.h>
> #include <linux/gpio/consumer.h>
> #include <linux/gpio/driver.h>
> @@ -32,6 +33,8 @@ struct qe_gpio_chip {
>
> /* saved_regs used to restore dedicated functions */
> struct qe_pio_regs saved_regs;
> +
> + int irq[32];
> };
>
> static void qe_gpio_save_regs(struct of_mm_gpio_chip *mm_gc)
> @@ -141,6 +144,13 @@ static int qe_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> return 0;
> }
>
> +static int qe_gpio_to_irq(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct qe_gpio_chip *qe_gc = gpiochip_get_data(gc);
> +
> + return qe_gc->irq[gpio] ? : -ENXIO;
> +}
> +
> struct qe_pin {
> /*
> * The qe_gpio_chip name is unfortunate, we should change that to
> @@ -304,6 +314,7 @@ static int qe_gpio_probe(struct platform_device *ofdev)
> struct qe_gpio_chip *qe_gc;
> struct of_mm_gpio_chip *mm_gc;
> struct gpio_chip *gc;
> + u32 mask;
>
> qe_gc = kzalloc(sizeof(*qe_gc), GFP_KERNEL);
> if (!qe_gc) {
> @@ -313,6 +324,14 @@ static int qe_gpio_probe(struct platform_device *ofdev)
>
> spin_lock_init(&qe_gc->lock);
>
> + if (!of_property_read_u32(np, "fsl,qe-gpio-irq-mask", &mask)) {
AFAICT: you can drop the of.h include and just use
device_property_present() here.
> + int i, j;
> +
> + for (i = 0, j = 0; i < 32; i++)
> + if (mask & (1 << (31 - i)))
> + qe_gc->irq[i] = irq_of_parse_and_map(np, j++);
> + }
> +
> mm_gc = &qe_gc->mm_gc;
> gc = &mm_gc->gc;
>
> @@ -323,6 +342,7 @@ static int qe_gpio_probe(struct platform_device *ofdev)
> gc->get = qe_gpio_get;
> gc->set = qe_gpio_set;
> gc->set_multiple = qe_gpio_set_multiple;
> + gc->to_irq = qe_gpio_to_irq;
>
> ret = of_mm_gpiochip_add_data(np, mm_gc, qe_gc);
> if (!ret)
> --
> 2.49.0
>
>
Bart
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] soc: fsl: qe: Add support of IRQ in QE GPIO
2025-08-12 14:21 ` Bartosz Golaszewski
@ 2025-08-18 8:33 ` Christophe Leroy
0 siblings, 0 replies; 16+ messages in thread
From: Christophe Leroy @ 2025-08-18 8:33 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-kernel, linuxppc-dev, linux-arm-kernel, linux-gpio,
devicetree, Qiang Zhao, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Le 12/08/2025 à 16:21, Bartosz Golaszewski a écrit :
> On Tue, 12 Aug 2025 13:02:53 +0200, Christophe Leroy
> <christophe.leroy@csgroup.eu> said:
>> In the QE, a few GPIOs are IRQ capable. Similarly to
>> commit 726bd223105c ("powerpc/8xx: Adding support of IRQ in MPC8xx
>> GPIO"), add IRQ support to QE GPIO.
>>
>> Add property 'fsl,qe-gpio-irq-mask' similar to
>> 'fsl,cpm1-gpio-irq-mask' that define which of the GPIOs have IRQs.
>>
>> Here is an exemple for port B of mpc8323 which has IRQs for
>> GPIOs PB7, PB9, PB25 and PB27.
>>
>> qe_pio_b: gpio-controller@1418 {
>> #gpio-cells = <2>;
>> compatible = "fsl,mpc8323-qe-pario-bank";
>> reg = <0x1418 0x18>;
>> interrupts = <4 5 6 7>;
>> fsl,qe-gpio-irq-mask = <0x01400050>;
>> interrupt-parent = <&qepic>;
>> gpio-controller;
>> };
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> drivers/soc/fsl/qe/gpio.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
>> index b502377193192..59145652ad850 100644
>> --- a/drivers/soc/fsl/qe/gpio.c
>> +++ b/drivers/soc/fsl/qe/gpio.c
>> @@ -13,6 +13,7 @@
>> #include <linux/err.h>
>> #include <linux/io.h>
>> #include <linux/of.h>
>> +#include <linux/of_irq.h>
>> #include <linux/gpio/legacy-of-mm-gpiochip.h>
>> #include <linux/gpio/consumer.h>
>> #include <linux/gpio/driver.h>
>> @@ -32,6 +33,8 @@ struct qe_gpio_chip {
>>
>> /* saved_regs used to restore dedicated functions */
>> struct qe_pio_regs saved_regs;
>> +
>> + int irq[32];
>> };
>>
>> static void qe_gpio_save_regs(struct of_mm_gpio_chip *mm_gc)
>> @@ -141,6 +144,13 @@ static int qe_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>> return 0;
>> }
>>
>> +static int qe_gpio_to_irq(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct qe_gpio_chip *qe_gc = gpiochip_get_data(gc);
>> +
>> + return qe_gc->irq[gpio] ? : -ENXIO;
>> +}
>> +
>> struct qe_pin {
>> /*
>> * The qe_gpio_chip name is unfortunate, we should change that to
>> @@ -304,6 +314,7 @@ static int qe_gpio_probe(struct platform_device *ofdev)
>> struct qe_gpio_chip *qe_gc;
>> struct of_mm_gpio_chip *mm_gc;
>> struct gpio_chip *gc;
>> + u32 mask;
>>
>> qe_gc = kzalloc(sizeof(*qe_gc), GFP_KERNEL);
>> if (!qe_gc) {
>> @@ -313,6 +324,14 @@ static int qe_gpio_probe(struct platform_device *ofdev)
>>
>> spin_lock_init(&qe_gc->lock);
>>
>> + if (!of_property_read_u32(np, "fsl,qe-gpio-irq-mask", &mask)) {
>
> AFAICT: you can drop the of.h include and just use
> device_property_present() here.
This line reads the value of the mask, I can't see how it can be
replaced by device_property_present().
>
>> + int i, j;
>> +
>> + for (i = 0, j = 0; i < 32; i++)
>> + if (mask & (1 << (31 - i)))
>> + qe_gc->irq[i] = irq_of_parse_and_map(np, j++);
>> + }
>> +
>> mm_gc = &qe_gc->mm_gc;
>> gc = &mm_gc->gc;
>>
>> @@ -323,6 +342,7 @@ static int qe_gpio_probe(struct platform_device *ofdev)
>> gc->get = qe_gpio_get;
>> gc->set = qe_gpio_set;
>> gc->set_multiple = qe_gpio_set_multiple;
>> + gc->to_irq = qe_gpio_to_irq;
>>
>> ret = of_mm_gpiochip_add_data(np, mm_gc, qe_gc);
>> if (!ret)
>> --
>> 2.49.0
>>
>>
>
> Bart
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] soc: fsl: qe: Add support of IRQ in QE GPIO
2025-08-12 11:02 ` [PATCH 3/4] soc: fsl: qe: Add support of IRQ in QE GPIO Christophe Leroy
2025-08-12 14:21 ` Bartosz Golaszewski
@ 2025-08-12 15:30 ` Krzysztof Kozlowski
1 sibling, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-12 15:30 UTC (permalink / raw)
To: Christophe Leroy, Qiang Zhao, Linus Walleij, Bartosz Golaszewski,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linuxppc-dev, linux-arm-kernel, linux-gpio,
devicetree
On 12/08/2025 13:02, Christophe Leroy wrote:
>
> qe_gc = kzalloc(sizeof(*qe_gc), GFP_KERNEL);
> if (!qe_gc) {
> @@ -313,6 +324,14 @@ static int qe_gpio_probe(struct platform_device *ofdev)
>
> spin_lock_init(&qe_gc->lock);
>
> + if (!of_property_read_u32(np, "fsl,qe-gpio-irq-mask", &mask)) {
Undocumented ABI. You cannot add such stuff and testing your DTS would
point it out.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
2025-08-12 11:02 [PATCH 0/4] Add support of IRQs to QUICC ENGINE GPIOs Christophe Leroy
` (2 preceding siblings ...)
2025-08-12 11:02 ` [PATCH 3/4] soc: fsl: qe: Add support of IRQ in QE GPIO Christophe Leroy
@ 2025-08-12 11:02 ` Christophe Leroy
2025-08-12 14:32 ` Rob Herring (Arm)
2025-08-12 15:23 ` Krzysztof Kozlowski
3 siblings, 2 replies; 16+ messages in thread
From: Christophe Leroy @ 2025-08-12 11:02 UTC (permalink / raw)
To: Qiang Zhao, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Christophe Leroy, linux-kernel, linuxppc-dev, linux-arm-kernel,
linux-gpio, devicetree
The QUICC Engine provides interrupts for a few I/O ports. This is
handled via a separate interrupt ID and managed via a triplet of
dedicated registers hosted by the SoC.
Implement an interrupt driver for it for that those IRQs can then
be linked to the related GPIOs.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
.../soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml | 63 +++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
new file mode 100644
index 0000000000000..7c98706d03dd1
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+title: Freescale QUICC Engine I/O Ports Interrupt Controller
+
+maintainers:
+ - name: Christophe Leroy
+ email: christophe.leroy@csgroup.eu
+
+description: |
+ Interrupt controller for the QUICC Engine I/O ports found on some
+ Freescale/NXP PowerQUICC and QorIQ SoCs.
+
+properties:
+ compatible:
+ enum:
+ - fsl,mpc8323-qe-ports-ic
+ - fsl,mpc8360-qe-ports-ic
+ - fsl,mpc8568-qe-ports-ic
+
+ reg:
+ description: Base address and size of the QE I/O Ports Interrupt Controller registers.
+ minItems: 1
+ maxItems: 1
+
+ interrupt-controller:
+ type: boolean
+ description: Indicates this node is an interrupt controller.
+
+ '#address-cells':
+ const: 0
+ description: Must be 0.
+
+ '#interrupt-cells':
+ const: 1
+ description: Number of cells to encode an interrupt specifier.
+
+ interrupts:
+ minItems: 1
+ maxItems: 1
+ description: Interrupt line to which the QE I/O Ports controller is connected.
+
+ interrupt-parent:
+ description: Phandle to the parent interrupt controller.
+
+required:
+ - compatible
+ - reg
+ - interrupt-controller
+ - '#address-cells'
+ - '#interrupt-cells'
+ - interrupts
+ - interrupt-parent
+
+examples:
+ - |
+ interrupt-controller@c00 {
+ interrupt-controller;
+ compatible = "fsl,mpc8323-qe-ports-ic";
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ reg = <0xc00 0x18>;
+ interrupts = <74 0x8>;
+ interrupt-parent = <&ipic>;
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
2025-08-12 11:02 ` [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports Christophe Leroy
@ 2025-08-12 14:32 ` Rob Herring (Arm)
2025-08-12 15:23 ` Krzysztof Kozlowski
1 sibling, 0 replies; 16+ messages in thread
From: Rob Herring (Arm) @ 2025-08-12 14:32 UTC (permalink / raw)
To: Christophe Leroy
Cc: Krzysztof Kozlowski, Linus Walleij, linux-kernel,
linux-arm-kernel, devicetree, linuxppc-dev, Bartosz Golaszewski,
Conor Dooley, linux-gpio, Qiang Zhao
On Tue, 12 Aug 2025 13:02:54 +0200, Christophe Leroy wrote:
> The QUICC Engine provides interrupts for a few I/O ports. This is
> handled via a separate interrupt ID and managed via a triplet of
> dedicated registers hosted by the SoC.
>
> Implement an interrupt driver for it for that those IRQs can then
> be linked to the related GPIOs.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> .../soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml | 63 +++++++++++++++++++
> 1 file changed, 63 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml:3:1: [error] missing document start "---" (document-start)
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml: ignoring, error parsing file
Traceback (most recent call last):
File "/usr/local/bin/dt-doc-validate", line 8, in <module>
sys.exit(main())
^^^^^^
File "/usr/local/lib/python3.11/dist-packages/dtschema/doc_validate.py", line 66, in main
ret |= check_doc(f)
^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/dtschema/doc_validate.py", line 22, in check_doc
dtsch = dtschema.DTSchema(filename, line_numbers=line_number)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/dtschema/schema.py", line 83, in __init__
id = schema['$id'].rstrip('#')
~~~~~~^^^^^^^
KeyError: '$id'
Error: Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.example.dts:34.3-35.1 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.dtbs:132: Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1527: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/0b56ef403a7c8d0f8305e847d68959a1037d365e.1754996033.git.christophe.leroy@csgroup.eu
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
2025-08-12 11:02 ` [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports Christophe Leroy
2025-08-12 14:32 ` Rob Herring (Arm)
@ 2025-08-12 15:23 ` Krzysztof Kozlowski
2025-08-12 17:16 ` Rob Herring
2025-08-18 8:37 ` Christophe Leroy
1 sibling, 2 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-12 15:23 UTC (permalink / raw)
To: Christophe Leroy, Qiang Zhao, Linus Walleij, Bartosz Golaszewski,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linuxppc-dev, linux-arm-kernel, linux-gpio,
devicetree
On 12/08/2025 13:02, Christophe Leroy wrote:
> The QUICC Engine provides interrupts for a few I/O ports. This is
> handled via a separate interrupt ID and managed via a triplet of
> dedicated registers hosted by the SoC.
>
> Implement an interrupt driver for it for that those IRQs can then
> be linked to the related GPIOs.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> .../soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml | 63 +++++++++++++++++++
> 1 file changed, 63 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
> new file mode 100644
> index 0000000000000..7c98706d03dd1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +
> +title: Freescale QUICC Engine I/O Ports Interrupt Controller
> +
> +maintainers:
> + - name: Christophe Leroy
> + email: christophe.leroy@csgroup.eu
Oh no...
> +
> +description: |
> + Interrupt controller for the QUICC Engine I/O ports found on some
> + Freescale/NXP PowerQUICC and QorIQ SoCs.
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,mpc8323-qe-ports-ic
> + - fsl,mpc8360-qe-ports-ic
> + - fsl,mpc8568-qe-ports-ic
> +
> + reg:
> + description: Base address and size of the QE I/O Ports Interrupt Controller registers.
> + minItems: 1
> + maxItems: 1
This was never tested but more important this and everything further
looks like generated by AI. Please don't do that or at least mark it
clearly, so I will prioritize accordingly (hint: AI generates poor code
and burden to decipher AI slop should not be on open source reviewers
but on users of AI, but as one of maintainers probably you already know
that, so sorry for lecturing).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
2025-08-12 15:23 ` Krzysztof Kozlowski
@ 2025-08-12 17:16 ` Rob Herring
2025-08-18 8:39 ` Christophe Leroy
2025-08-18 8:37 ` Christophe Leroy
1 sibling, 1 reply; 16+ messages in thread
From: Rob Herring @ 2025-08-12 17:16 UTC (permalink / raw)
To: Krzysztof Kozlowski, Christophe Leroy
Cc: Qiang Zhao, Linus Walleij, Bartosz Golaszewski,
Krzysztof Kozlowski, Conor Dooley, linux-kernel, linuxppc-dev,
linux-arm-kernel, linux-gpio, devicetree
On Tue, Aug 12, 2025 at 10:23 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 12/08/2025 13:02, Christophe Leroy wrote:
> > The QUICC Engine provides interrupts for a few I/O ports. This is
> > handled via a separate interrupt ID and managed via a triplet of
> > dedicated registers hosted by the SoC.
> >
> > Implement an interrupt driver for it for that those IRQs can then
> > be linked to the related GPIOs.
> >
> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > ---
> > .../soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml | 63 +++++++++++++++++++
> > 1 file changed, 63 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
> > new file mode 100644
> > index 0000000000000..7c98706d03dd1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
> > @@ -0,0 +1,63 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +
> > +title: Freescale QUICC Engine I/O Ports Interrupt Controller
> > +
> > +maintainers:
> > + - name: Christophe Leroy
> > + email: christophe.leroy@csgroup.eu
>
> Oh no...
>
> > +
> > +description: |
> > + Interrupt controller for the QUICC Engine I/O ports found on some
> > + Freescale/NXP PowerQUICC and QorIQ SoCs.
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - fsl,mpc8323-qe-ports-ic
> > + - fsl,mpc8360-qe-ports-ic
> > + - fsl,mpc8568-qe-ports-ic
> > +
> > + reg:
> > + description: Base address and size of the QE I/O Ports Interrupt Controller registers.
> > + minItems: 1
> > + maxItems: 1
>
> This was never tested but more important this and everything further
> looks like generated by AI. Please don't do that or at least mark it
> clearly, so I will prioritize accordingly (hint: AI generates poor code
> and burden to decipher AI slop should not be on open source reviewers
> but on users of AI, but as one of maintainers probably you already know
> that, so sorry for lecturing).
If anyone needs some AI (chatgpt) converted bindings, my "dt-convert"
branch has ~800 of them. Feeding the warnings back to AI to fix was
somewhat effective. The result is not the worst I've seen submitted.
It saves some of the boilerplate, but can't fix things that are just
wrong or unclear in .txt bindings. Despite my 'prompt engineering'
attempts, it still tends to get the same things wrong over and over.
Rob
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
2025-08-12 17:16 ` Rob Herring
@ 2025-08-18 8:39 ` Christophe Leroy
0 siblings, 0 replies; 16+ messages in thread
From: Christophe Leroy @ 2025-08-18 8:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski
Cc: Qiang Zhao, Linus Walleij, Bartosz Golaszewski,
Krzysztof Kozlowski, Conor Dooley, linux-kernel, linuxppc-dev,
linux-arm-kernel, linux-gpio, devicetree
Le 12/08/2025 à 19:16, Rob Herring a écrit :
> On Tue, Aug 12, 2025 at 10:23 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 12/08/2025 13:02, Christophe Leroy wrote:
>>> The QUICC Engine provides interrupts for a few I/O ports. This is
>>> handled via a separate interrupt ID and managed via a triplet of
>>> dedicated registers hosted by the SoC.
>>>
>>> Implement an interrupt driver for it for that those IRQs can then
>>> be linked to the related GPIOs.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>> ---
>>> .../soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml | 63 +++++++++++++++++++
>>> 1 file changed, 63 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
>>> new file mode 100644
>>> index 0000000000000..7c98706d03dd1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
>>> @@ -0,0 +1,63 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +
>>> +title: Freescale QUICC Engine I/O Ports Interrupt Controller
>>> +
>>> +maintainers:
>>> + - name: Christophe Leroy
>>> + email: christophe.leroy@csgroup.eu
>>
>> Oh no...
>>
>>> +
>>> +description: |
>>> + Interrupt controller for the QUICC Engine I/O ports found on some
>>> + Freescale/NXP PowerQUICC and QorIQ SoCs.
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - fsl,mpc8323-qe-ports-ic
>>> + - fsl,mpc8360-qe-ports-ic
>>> + - fsl,mpc8568-qe-ports-ic
>>> +
>>> + reg:
>>> + description: Base address and size of the QE I/O Ports Interrupt Controller registers.
>>> + minItems: 1
>>> + maxItems: 1
>>
>> This was never tested but more important this and everything further
>> looks like generated by AI. Please don't do that or at least mark it
>> clearly, so I will prioritize accordingly (hint: AI generates poor code
>> and burden to decipher AI slop should not be on open source reviewers
>> but on users of AI, but as one of maintainers probably you already know
>> that, so sorry for lecturing).
>
> If anyone needs some AI (chatgpt) converted bindings, my "dt-convert"
> branch has ~800 of them. Feeding the warnings back to AI to fix was
> somewhat effective. The result is not the worst I've seen submitted.
> It saves some of the boilerplate, but can't fix things that are just
> wrong or unclear in .txt bindings. Despite my 'prompt engineering'
> attempts, it still tends to get the same things wrong over and over.
By the way, the new binding was not generated from text binding. I fed
the AI with the driver C source file.
Christophe
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] dt-bindings: soc: fsl: qe: Add an interrupt controller for QUICC Engine Ports
2025-08-12 15:23 ` Krzysztof Kozlowski
2025-08-12 17:16 ` Rob Herring
@ 2025-08-18 8:37 ` Christophe Leroy
1 sibling, 0 replies; 16+ messages in thread
From: Christophe Leroy @ 2025-08-18 8:37 UTC (permalink / raw)
To: Krzysztof Kozlowski, Qiang Zhao, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-kernel, linuxppc-dev, linux-arm-kernel, linux-gpio,
devicetree
Le 12/08/2025 à 17:23, Krzysztof Kozlowski a écrit :
> On 12/08/2025 13:02, Christophe Leroy wrote:
>> The QUICC Engine provides interrupts for a few I/O ports. This is
>> handled via a separate interrupt ID and managed via a triplet of
>> dedicated registers hosted by the SoC.
>>
>> Implement an interrupt driver for it for that those IRQs can then
>> be linked to the related GPIOs.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> .../soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml | 63 +++++++++++++++++++
>> 1 file changed, 63 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
>> new file mode 100644
>> index 0000000000000..7c98706d03dd1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-ports-ic.yaml
>> @@ -0,0 +1,63 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +
>> +title: Freescale QUICC Engine I/O Ports Interrupt Controller
>> +
>> +maintainers:
>> + - name: Christophe Leroy
>> + email: christophe.leroy@csgroup.eu
>
> Oh no...
>
>> +
>> +description: |
>> + Interrupt controller for the QUICC Engine I/O ports found on some
>> + Freescale/NXP PowerQUICC and QorIQ SoCs.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - fsl,mpc8323-qe-ports-ic
>> + - fsl,mpc8360-qe-ports-ic
>> + - fsl,mpc8568-qe-ports-ic
>> +
>> + reg:
>> + description: Base address and size of the QE I/O Ports Interrupt Controller registers.
>> + minItems: 1
>> + maxItems: 1
>
> This was never tested but more important this and everything further
> looks like generated by AI. Please don't do that or at least mark it
> clearly, so I will prioritize accordingly (hint: AI generates poor code
> and burden to decipher AI slop should not be on open source reviewers
> but on users of AI, but as one of maintainers probably you already know
> that, so sorry for lecturing).
Yes sorry, overconfidence into AI. Until now I knew almost nothing about
YAML and the generated file had a good look. I didn't know there was a
special procedure to test bindings, I thought checkpatch was doing all
necessary checks.
Fixed in v2.
^ permalink raw reply [flat|nested] 16+ messages in thread