On 29/08/2025 11:41, Christophe Leroy wrote:

quoted

quoted

That's more or less the same here with my series, patch 1 implements an
interrupt controller (documented in patch 6) and then the GPIO
controllers consume the interrupts, for instance in gpiolib functions
gpio_sysfs_request_irq() [drivers/gpio/gpiolib-sysfs.c] or
edge_detector_setup() or debounce_setup() [drivers/gpio/gpiolib-cdev.c]

External drivers also use interrupts indirectly. For example driver
sound/soc/soc-jack.c, it doesn't have any direct reference to an
interrupt. The driver is given an array of GPIOs and then installs an
IRQ in function snd_soc_jack_add_gpios() by doing

	request_any_context_irq(gpiod_to_irq(gpios[i].desc),
					      gpio_handler,
					      IRQF_SHARED |
					      IRQF_TRIGGER_RISING |
					      IRQF_TRIGGER_FALLING,
					      gpios[i].name,
					      &gpios[i]);

External drivers do not matter then. Your GPIO controller receives
specific interrupts (that's the interrupt property) and knows exactly
how each GPIO maps to it.

Do you mean then that the GPIO driver should already know which line has 

The SoC knows, that's fixed information, so shall GPIO driver know as well.

an interrupt and which one doesn't ?

The interrupts are fixed per soc, but today the GPIO driver is generic 
and used on different SOCs that don't have interrupts on the same lines. 

How you write drivers is one thing, but that's never a reason alone to
add properties to the DT.

And even on the given SOCs, not all ports have interrupts on the same 

That's pretty standard between all GPIO/pinctrl drivers. I would
generalize that's pretty standard for all SoCs - they have differences
within devices, some pins do that, some do different things.

lines. Should all possibilities be hard-coded inside the driver for each 
possible compatible ? The property 'fsl,qe-gpio-irq-mask' is there to 

There are many ways how to do it in the driver, that feels like one of
them, so yes, it should.

avoid that and keep the GPIO driver as generic as possible with a single 

Sorry, that approach, which leads to moving such stuff to DT, was many
times on mailing list rejected. You use the same argument as that "one
clock, one device node" TI approach. It got it way to kernel long time
ago but since then was pretty discouraged (rejected for new SoCs). It
re-appeared again few months ago in a form of "I have two registers, so
I have two device nodes in DT", so it seems poor code keeps re-appearing.

compatible 'fsl,mpc8323-qe-pario-bank' that is found in the DTS of 8323 
but also in DTS of 8360, in DTS of 8569, etc... :

arch/powerpc/boot/dts/fsl/mpc8569mds.dts: 
             "fsl,mpc8323-qe-pario-bank";
arch/powerpc/boot/dts/fsl/mpc8569mds.dts: 
             "fsl,mpc8323-qe-pario-bank";
arch/powerpc/boot/dts/kmeter1.dts: 
     "fsl,mpc8323-qe-pario-bank";
arch/powerpc/boot/dts/mpc832x_rdb.dts: 
compatible = "fsl,mpc8323-qe-pario-bank";
arch/powerpc/boot/dts/mpc836x_rdk.dts: 
"fsl,mpc8323-qe-pario-bank";
arch/powerpc/boot/dts/mpc836x_rdk.dts: 
"fsl,mpc8323-qe-pario-bank";

Do you mean we should define one compatible for each of the ports of 
each soc, and encode the mask of interrupts that define which line of 
the port has interrupts in the data field ?

I don't know that good your hardware to tell.

Something like:

static const struct of_device_id qe_gpio_match[] = {
	{
		.compatible = "fsl,mpc8323-qe-pario-bank-a",
		.data = (void *)0x00a00028,

There is no DTS in your patchset, so I cannot help really. I just don't
have time to imagine such DTS and try to figure out how it can be
written. Posting complete picture usually helps.

I don't follow what is the bank.

You have a device, yes?
It has some grouped GPIOs (banks?) and some within group/bank can handle
interrupts?
Are these fixed per SoC? Yes. Well, that's standard and every other
vendor has the same. They solve it in the drivers differently, though.

	},
	{
		.compatible = "fsl,mpc8323-qe-pario-bank-b",
		.data = (void *)0x01400050,
	},
	{
		.compatible = "fsl,mpc8323-qe-pario-bank-c",
		.data = (void *)0x00000084,
	},
	{
		.compatible = "fsl,mpc8323-qe-pario-bank-d",
		.data = (void *)0,
	},
	{
		.compatible = "fsl,mpc8360-qe-pario-bank-a",
		.data = (void *)0xXXXXXXXX,
	},
	{
		.compatible = "fsl,mpc8360-qe-pario-bank-b",
		.data = (void *)0xXXXXXXXX,
	},
....
	{},
};
MODULE_DEVICE_TABLE(of, qe_gpio_match);

That would be feasible but would mean updating the driver each time a 
new SOC is added.

BTW, like every other platform.

Do you mean it should be done that way ?

Isn't the purpose of the device tree to keep drivers as generic as 
possible ?

Not at all, sorry. The purpose of DT is not to keep drivers generic.


Best regards,
Krzysztof