Re: [PATCH 3/4] irqchip/meson-gpio: add select trigger type callback

From: qianggui.song <Qianggui.Song@amlogic.com>
To: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Kevin Hilman <khilman@baylibre.com>,
	 Neil Armstrong <narmstrong@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>
Subject: Re: [PATCH 3/4] irqchip/meson-gpio: add select trigger type callback
Date: Mon, 10 Jan 2022 20:28:00 +0800	[thread overview]
Message-ID: <0525de06-03b0-2321-5131-eb2710e5935c@amlogic.com> (raw)
In-Reply-To: <87sftytsk4.wl-maz@kernel.org>


On 1/8/22 6:44 PM, Marc Zyngier wrote:
> 
> On Sat, 08 Jan 2022 08:42:17 +0000,
> Qianggui Song <qianggui.song@amlogic.com> wrote:
>>
>> Due to some chips may use different registers and offset, provide
>> a set trigger type call back.
>>
>> Signed-off-by: Qianggui Song <qianggui.song@amlogic.com>
>> ---
>>   drivers/irqchip/irq-meson-gpio.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-meson-gpio.c b/drivers/irqchip/irq-meson-gpio.c
>> index 6a7b4fb13452..98419428fcbd 100644
>> --- a/drivers/irqchip/irq-meson-gpio.c
>> +++ b/drivers/irqchip/irq-meson-gpio.c
>> @@ -55,6 +55,8 @@ struct irq_ctl_ops {
>>   	void (*gpio_irq_sel_pin)(struct meson_gpio_irq_controller *ctl,
>>   				 unsigned int channel, unsigned long hwirq);
>>   	void (*gpio_irq_init)(struct meson_gpio_irq_controller *ctl);
>> +	unsigned int (*gpio_irq_sel_type)(struct meson_gpio_irq_controller *ctl,
>> +					  unsigned int idx, u32 val);
>>   };
>>   
>>   struct meson_gpio_irq_params {
>> @@ -68,16 +70,17 @@ struct meson_gpio_irq_params {
>>   	struct irq_ctl_ops ops;
>>   };
>>   
>> -#define INIT_MESON_COMMON(irqs, init, sel)			\
>> +#define INIT_MESON_COMMON(irqs, init, sel, type)		\
>>   	.nr_hwirq = irqs,					\
>>   	.ops = {						\
>>   		.gpio_irq_init = init,				\
>>   		.gpio_irq_sel_pin = sel,			\
>> +		.gpio_irq_sel_type = type,			\
>>   	},
>>   
>>   #define INIT_MESON8_COMMON_DATA(irqs)				\
>>   	INIT_MESON_COMMON(irqs, meson_gpio_irq_init_dummy,	\
>> -			  meson8_gpio_irq_sel_pin)		\
>> +			  meson8_gpio_irq_sel_pin, NULL)	\
>>   	.edge_single_offset = 0,				\
>>   	.pol_low_offset = 16,					\
>>   	.pin_sel_mask = 0xff,					\
>> @@ -85,7 +88,7 @@ struct meson_gpio_irq_params {
>>   
>>   #define INIT_MESON_A1_COMMON_DATA(irqs)				\
>>   	INIT_MESON_COMMON(irqs, meson_a1_gpio_irq_init,		\
>> -			  meson_a1_gpio_irq_sel_pin)		\
>> +			  meson_a1_gpio_irq_sel_pin, NULL)	\
>>   	.support_edge_both = true,				\
>>   	.edge_both_offset = 16,					\
>>   	.edge_single_offset = 8,				\
>> @@ -279,6 +282,10 @@ static int meson_gpio_irq_type_setup(struct meson_gpio_irq_controller *ctl,
>>   	 */
>>   	type &= IRQ_TYPE_SENSE_MASK;
>>   
>> +	/* Some controllers may have different calculation method*/
>> +	if (params->ops.gpio_irq_sel_type)
>> +		return params->ops.gpio_irq_sel_type(ctl, idx, type);
>> +
> 
> No. If you are going to indirect these things, indirect them for all
> implementations and keep the code clean.
Okay, I' ll try to implement a new macro
> 
> 	M.
> 

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic