Received-SPF: pass (google.com: domain of linux-kernel+bounces-195259-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 30 May 2024 13:22:53 +0300
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <brgl@bgdev.pl>, Andrew Lunn <andrew@lunn.ch>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	Gregor Herburger <gregor.herburger@tq-group.com>,
	linux@ew.tq-group.com
Subject: Re: [PATCH 8/8] gpio: tqmx86: fix broken IRQ_TYPE_EDGE_BOTH
 interrupt type
Message-ID: <876eb824-2898-4ffe-9d0e-69dd0781729f@moroto.mountain>
References: <cover.1716967982.git.matthias.schiffer@ew.tq-group.com>
 <2c265b6bcfcde7d2327b94c4f6e3ad6d4f1e2de7.1716967982.git.matthias.schiffer@ew.tq-group.com>
 <8689fbcd-3fa3-410b-8fc9-7a699bf163b8@moroto.mountain>
 <0e971f0b885bd360e33ef472d96e3d9e0ab56405.camel@ew.tq-group.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0e971f0b885bd360e33ef472d96e3d9e0ab56405.camel@ew.tq-group.com>

On Thu, May 30, 2024 at 10:39:25AM +0200, Matthias Schiffer wrote:
> On Wed, 2024-05-29 at 17:38 +0300, Dan Carpenter wrote:
> > 
> > On Wed, May 29, 2024 at 09:45:20AM +0200, Matthias Schiffer wrote:
> > > diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
> > > index c957be3341774..400415676ad5d 100644
> > > --- a/drivers/gpio/gpio-tqmx86.c
> > > +++ b/drivers/gpio/gpio-tqmx86.c
> > > @@ -126,9 +126,15 @@ static void _tqmx86_gpio_irq_config(struct tqmx86_gpio_data *gpio, int hwirq)
> > >  	unsigned int offset = hwirq - TQMX86_NGPO;
> > >  	u8 type = TQMX86_INT_TRIG_NONE, mask, val;
> > >  
> > > -	if (gpio->irq_type[hwirq] & TQMX86_INT_UNMASKED)
> > > +	if (gpio->irq_type[hwirq] & TQMX86_INT_UNMASKED) {
> > >  		type = gpio->irq_type[hwirq] & TQMX86_INT_TRIG_MASK;
> > >  
> > > +		if (type == TQMX86_INT_TRIG_BOTH)
> > > +			type = tqmx86_gpio_get(&gpio->chip, hwirq)
> >                                                             ^^^^^
> > 
> > > +				? TQMX86_INT_TRIG_FALLING
> > > +				: TQMX86_INT_TRIG_RISING;
> > > +	}
> > > +
> > >  	mask = TQMX86_GPII_MASK(offset);
> >                                 ^^^^^^
> > >  	val = TQMX86_GPII_CONFIG(offset, type);
> >                                  ^^^^^^
> > >  	_tqmx86_gpio_update_bits(gpio, TQMX86_GPIIC, mask, val);
> > 
> > The offset stuff wasn't beautiful and I'm glad you are deleting it.  My
> > understanding is that a hwirq is 0-3 for output or 4-7 input.  An offset
> > is "hwirq % 4"?
> > 
> > There are a bunch of places which are still marked as taking an offset
> > but they all actually take a hwirq.  For example, tqmx86_gpio_get()
> > above.  The only things which still actually take an offset are the
> > TQMX86_GPII_MASK() and TQMX86_GPII_CONFIG() macros.
> > 
> > Could you:
> > 1) Modify TQMX86_GPII_MASK() and TQMX86_GPII_CONFIG() to take a hwirq?
> > 2) Rename all the "offset" variables to "hwirq"?
> 
> Unfortunately, the TQMx86 GPIO is a huge mess, and the mapping between GPIO numbers and IRQ numbers
> depends on the hardware generation/variant. I don't think it is possible to have GPIO numbers and
> hwirq numbers differ, is it?
> 
> Currently, the driver only supports COM Express modules, where IRQs 0-3 correspond to GPIOs 4-7,
> while GPIOs 0-3 don't have interrupt support.

I'm so confused.

So "offset" is the GPIO number and "hwirq" is the IRQ number?  If the
IRQ numbers are 0-3 then why do we subtract 4 to get the GPIO number in
_tqmx86_gpio_irq_config()?

	unsigned int offset = hwirq - TQMX86_NGPO;

And again, it's just weird to call:

		type = tqmx86_gpio_get(&gpio->chip, hwirq);

where we're passing "hwirq" when tqmx86_gpio_get() takes an "offset" as
an argument.

regards,
dan carpenter