Re: [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad

[Mathieu Dubois-Briand]

On Thu Dec 19, 2024 at 8:35 PM CET, Dmitry Torokhov wrote:
> Hi Mathieu,
>
> On Thu, Dec 19, 2024 at 05:21:23PM +0100, Mathieu Dubois-Briand wrote:
> > +
> > +	input_set_capability(input, EV_MSC, MSC_SCAN);
> > +	if (!max7360_keypad->no_autorepeat)
> > +		__set_bit(EV_REP, input->evbit);
> > +
> > +	input_set_drvdata(input, max7360_keypad);
> > +
> > +	flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
>
> Shared? Why? And why do you need a temp variable?
>

Ok, this is probably wrong.

I have a board using the MAX7360, were both interrupt lines of the
chipset (INTI, interrupt for GPIO and rotary encoder and INTK, interrupt
for the keypad) have been both wired to the same pin on the CPU side. So
on this board, the interrupt line is indeed shared between the three
corresponding drivers.

Yet this is probably very specific to my board, as I have seen no data
about MAXIM suggesting this design. So having IRQF_SHARED is probably
more a hack than a valid implementation. I will drop it in my next
series.

>
> Thanks.

Thanks a lot for you review, I am preparing a new version of this series
that should address all your comments.


-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com