mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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
Date: Wed, 29 May 2024 17:38:31 +0300	[thread overview]
Message-ID: <8689fbcd-3fa3-410b-8fc9-7a699bf163b8@moroto.mountain> (raw)
In-Reply-To: <2c265b6bcfcde7d2327b94c4f6e3ad6d4f1e2de7.1716967982.git.matthias.schiffer@ew.tq-group.com>

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"?

regards,
dan carpenter


  parent reply	other threads:[~2024-05-29 14:38 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-29  7:45 [PATCH 0/8] gpio-tqmx86 fixes Matthias Schiffer
2024-05-29  7:45 ` [PATCH 1/8] gpio: tqmx86: fix typo in Kconfig label Matthias Schiffer
2024-05-29 11:58   ` Andrew Lunn
2024-05-29  7:45 ` [PATCH 2/8] gpio: tqmx86: introduce shadow register for GPIO output value Matthias Schiffer
2024-05-29 12:02   ` Andrew Lunn
2024-05-29  7:45 ` [PATCH 3/8] gpio: tqmx86: change tqmx86_gpio_write() order of arguments to match regmap API Matthias Schiffer
2024-05-29 12:03   ` Bartosz Golaszewski
2024-05-29 12:11     ` Andrew Lunn
2024-05-29 12:23       ` Matthias Schiffer
2024-05-29  7:45 ` [PATCH 4/8] gpio: tqmx86: introduce _tqmx86_gpio_update_bits() helper Matthias Schiffer
2024-05-29 12:19   ` Andrew Lunn
2024-05-29 12:25     ` Matthias Schiffer
2024-05-29 12:31       ` Andrew Lunn
2024-05-29  7:45 ` [PATCH 5/8] gpio: tqmx86: add macros for interrupt configuration Matthias Schiffer
2024-05-29  7:45 ` [PATCH 6/8] gpio: tqmx86: store IRQ triggers without offsetting index Matthias Schiffer
2024-05-29  7:45 ` [PATCH 7/8] gpio: tqmx86: store IRQ trigger type and unmask status separately Matthias Schiffer
2024-05-29  7:45 ` [PATCH 8/8] gpio: tqmx86: fix broken IRQ_TYPE_EDGE_BOTH interrupt type Matthias Schiffer
2024-05-29 12:37   ` Andrew Lunn
2024-05-29 12:44     ` Matthias Schiffer
2024-05-29 14:38   ` Dan Carpenter [this message]
2024-05-30  8:39     ` Matthias Schiffer
2024-05-30 10:22       ` Dan Carpenter
2024-05-30 11:15         ` Matthias Schiffer
2024-05-30 11:36           ` Matthias Schiffer
2024-05-30 12:13       ` Andrew Lunn
2024-05-29 12:08 ` [PATCH 0/8] gpio-tqmx86 fixes Bartosz Golaszewski
2024-05-29 12:54   ` Matthias Schiffer
2024-05-29 17:02     ` Bartosz Golaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8689fbcd-3fa3-410b-8fc9-7a699bf163b8@moroto.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=andrew@lunn.ch \
    --cc=brgl@bgdev.pl \
    --cc=gregor.herburger@tq-group.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@ew.tq-group.com \
    --cc=matthias.schiffer@ew.tq-group.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®