From: Linus Walleij <linus.walleij@linaro.org>
To: Michael Walle <michael@walle.cc>
Cc: "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
linux-hwmon@vger.kernel.org, linux-pwm@vger.kernel.org,
LINUXWATCHDOG <linux-watchdog@vger.kernel.org>,
"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
"Rob Herring" <robh+dt@kernel.org>,
"Jean Delvare" <jdelvare@suse.com>,
"Guenter Roeck" <linux@roeck-us.net>,
"Lee Jones" <lee.jones@linaro.org>,
"Thierry Reding" <thierry.reding@gmail.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Wim Van Sebroeck" <wim@linux-watchdog.org>,
"Shawn Guo" <shawnguo@kernel.org>, "Li Yang" <leoyang.li@nxp.com>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Jason Cooper" <jason@lakedaemon.net>,
"Marc Zyngier" <maz@kernel.org>,
"Mark Brown" <broonie@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v2 10/16] gpio: add a reusable generic gpio_chip using regmap
Date: Thu, 16 Apr 2020 11:27:49 +0200 [thread overview]
Message-ID: <CACRpkdaqgHhPwdKdUai4zvi21qR-cSQUKyzZ3SyfWBLPN9us3w@mail.gmail.com> (raw)
In-Reply-To: <20200402203656.27047-11-michael@walle.cc>
On Thu, Apr 2, 2020 at 10:37 PM Michael Walle <michael@walle.cc> wrote:
> There are quite a lot simple GPIO controller which are using regmap to
> access the hardware. This driver tries to be a base to unify existing
> code into one place. This won't cover everything but it should be a good
> starting point.
>
> It does not implement its own irq_chip because there is already a
> generic one for regmap based devices. Instead, the irq_chip will be
> instanciated in the parent driver and its irq domain will be associate
> to this driver.
>
> For now it consists of the usual registers, like set (and an optional
> clear) data register, an input register and direction registers.
> Out-of-the-box, it supports consecutive register mappings and mappings
> where the registers have gaps between them with a linear mapping between
> GPIO offset and bit position. For weirder mappings the user can register
> its own .xlate().
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Overall I really like this driver and I think we should merge is as soon
as it is in reasonable shape and then improve on top so we can start
migrating drivers to it.
> +static int gpio_regmap_to_irq(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct gpio_regmap_data *data = gpiochip_get_data(chip);
> + struct gpio_regmap *gpio = data->gpio;
> +
> + /* the user might have its own .to_irq callback */
> + if (gpio->to_irq)
> + return gpio->to_irq(gpio, offset);
> +
> + return irq_create_mapping(gpio->irq_domain, offset);
I think that should at least be irq_find_mapping(), the mapping should
definately not be created by the .to_irq() callback since that is just
a convenience function.
> + if (gpio->irq_domain)
> + chip->to_irq = gpio_regmap_to_irq;
I don't know about this.
(...)
> + * @irq_domain: (Optional) IRQ domain if the controller is
> + * interrupt-capable
(...)
> + struct irq_domain *irq_domain;
I don't think this is a good storage place for the irqdomain, we already have
gpio_irq_chip inside gpio_chip and that has an irqdomain, we should
strive to reuse that infrastructure also for regmap GPIO I think, for now
I would just leave .to_irq() out of this and let the driver deal with any
irqs.
Yours,
Linus Walleij
WARNING: multiple messages have this Message-ID (diff)
From: Linus Walleij <linus.walleij@linaro.org>
To: Michael Walle <michael@walle.cc>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
"Thierry Reding" <thierry.reding@gmail.com>,
"Lee Jones" <lee.jones@linaro.org>,
"Jason Cooper" <jason@lakedaemon.net>,
"Marc Zyngier" <maz@kernel.org>,
"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Guenter Roeck" <linux@roeck-us.net>,
linux-pwm@vger.kernel.org, "Jean Delvare" <jdelvare@suse.com>,
LINUXWATCHDOG <linux-watchdog@vger.kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
"Mark Brown" <broonie@kernel.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Wim Van Sebroeck" <wim@linux-watchdog.org>,
"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
linux-hwmon@vger.kernel.org,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 10/16] gpio: add a reusable generic gpio_chip using regmap
Date: Thu, 16 Apr 2020 11:27:49 +0200 [thread overview]
Message-ID: <CACRpkdaqgHhPwdKdUai4zvi21qR-cSQUKyzZ3SyfWBLPN9us3w@mail.gmail.com> (raw)
In-Reply-To: <20200402203656.27047-11-michael@walle.cc>
On Thu, Apr 2, 2020 at 10:37 PM Michael Walle <michael@walle.cc> wrote:
> There are quite a lot simple GPIO controller which are using regmap to
> access the hardware. This driver tries to be a base to unify existing
> code into one place. This won't cover everything but it should be a good
> starting point.
>
> It does not implement its own irq_chip because there is already a
> generic one for regmap based devices. Instead, the irq_chip will be
> instanciated in the parent driver and its irq domain will be associate
> to this driver.
>
> For now it consists of the usual registers, like set (and an optional
> clear) data register, an input register and direction registers.
> Out-of-the-box, it supports consecutive register mappings and mappings
> where the registers have gaps between them with a linear mapping between
> GPIO offset and bit position. For weirder mappings the user can register
> its own .xlate().
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Overall I really like this driver and I think we should merge is as soon
as it is in reasonable shape and then improve on top so we can start
migrating drivers to it.
> +static int gpio_regmap_to_irq(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct gpio_regmap_data *data = gpiochip_get_data(chip);
> + struct gpio_regmap *gpio = data->gpio;
> +
> + /* the user might have its own .to_irq callback */
> + if (gpio->to_irq)
> + return gpio->to_irq(gpio, offset);
> +
> + return irq_create_mapping(gpio->irq_domain, offset);
I think that should at least be irq_find_mapping(), the mapping should
definately not be created by the .to_irq() callback since that is just
a convenience function.
> + if (gpio->irq_domain)
> + chip->to_irq = gpio_regmap_to_irq;
I don't know about this.
(...)
> + * @irq_domain: (Optional) IRQ domain if the controller is
> + * interrupt-capable
(...)
> + struct irq_domain *irq_domain;
I don't think this is a good storage place for the irqdomain, we already have
gpio_irq_chip inside gpio_chip and that has an irqdomain, we should
strive to reuse that infrastructure also for regmap GPIO I think, for now
I would just leave .to_irq() out of this and let the driver deal with any
irqs.
Yours,
Linus Walleij
WARNING: multiple messages have this Message-ID (diff)
From: Linus Walleij <linus.walleij@linaro.org>
To: Michael Walle <michael@walle.cc>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
"Thierry Reding" <thierry.reding@gmail.com>,
"Lee Jones" <lee.jones@linaro.org>,
"Jason Cooper" <jason@lakedaemon.net>,
"Marc Zyngier" <maz@kernel.org>,
"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Guenter Roeck" <linux@roeck-us.net>,
linux-pwm@vger.kernel.org, "Jean Delvare" <jdelvare@suse.com>,
LINUXWATCHDOG <linux-watchdog@vger.kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
"Mark Brown" <broonie@kernel.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Wim Van Sebroeck" <wim@linux-watchdog.org>,
"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
linux-hwmon@vger.kernel.org,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Li Yang" <leoyang.li@nxp.com>,
"Rob Herring" <robh+dt@kernel.org>,
"Shawn Guo" <shawnguo@kernel.org>
Subject: Re: [PATCH v2 10/16] gpio: add a reusable generic gpio_chip using regmap
Date: Thu, 16 Apr 2020 11:27:49 +0200 [thread overview]
Message-ID: <CACRpkdaqgHhPwdKdUai4zvi21qR-cSQUKyzZ3SyfWBLPN9us3w@mail.gmail.com> (raw)
In-Reply-To: <20200402203656.27047-11-michael@walle.cc>
On Thu, Apr 2, 2020 at 10:37 PM Michael Walle <michael@walle.cc> wrote:
> There are quite a lot simple GPIO controller which are using regmap to
> access the hardware. This driver tries to be a base to unify existing
> code into one place. This won't cover everything but it should be a good
> starting point.
>
> It does not implement its own irq_chip because there is already a
> generic one for regmap based devices. Instead, the irq_chip will be
> instanciated in the parent driver and its irq domain will be associate
> to this driver.
>
> For now it consists of the usual registers, like set (and an optional
> clear) data register, an input register and direction registers.
> Out-of-the-box, it supports consecutive register mappings and mappings
> where the registers have gaps between them with a linear mapping between
> GPIO offset and bit position. For weirder mappings the user can register
> its own .xlate().
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Overall I really like this driver and I think we should merge is as soon
as it is in reasonable shape and then improve on top so we can start
migrating drivers to it.
> +static int gpio_regmap_to_irq(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct gpio_regmap_data *data = gpiochip_get_data(chip);
> + struct gpio_regmap *gpio = data->gpio;
> +
> + /* the user might have its own .to_irq callback */
> + if (gpio->to_irq)
> + return gpio->to_irq(gpio, offset);
> +
> + return irq_create_mapping(gpio->irq_domain, offset);
I think that should at least be irq_find_mapping(), the mapping should
definately not be created by the .to_irq() callback since that is just
a convenience function.
> + if (gpio->irq_domain)
> + chip->to_irq = gpio_regmap_to_irq;
I don't know about this.
(...)
> + * @irq_domain: (Optional) IRQ domain if the controller is
> + * interrupt-capable
(...)
> + struct irq_domain *irq_domain;
I don't think this is a good storage place for the irqdomain, we already have
gpio_irq_chip inside gpio_chip and that has an irqdomain, we should
strive to reuse that infrastructure also for regmap GPIO I think, for now
I would just leave .to_irq() out of this and let the driver deal with any
irqs.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-04-16 9:28 UTC|newest]
Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-02 20:36 [PATCH v2 00/16] Add support for Kontron sl28cpld Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 01/16] include/linux/ioport.h: add helper to define REG resource constructs Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 02/16] mfd: mfd-core: Don't overwrite the dma_mask of the child device Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 03/16] mfd: mfd-core: match device tree node against reg property Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 04/16] regmap-irq: make it possible to add irq_chip do a specific device node Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-14 15:37 ` Applied "regmap-irq: make it possible to add irq_chip do a specific device node" to the regmap tree Mark Brown
2020-04-14 15:37 ` Mark Brown
2020-04-14 15:37 ` Mark Brown
2020-04-14 17:12 ` [PATCH v2 04/16] regmap-irq: make it possible to add irq_chip do a specific device node Mark Brown
2020-04-14 17:12 ` Mark Brown
2020-04-14 17:12 ` Mark Brown
2020-04-02 20:36 ` [PATCH v2 05/16] dt-bindings: mfd: Add bindings for sl28cpld Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 06/16] mfd: Add support for Kontron sl28cpld management controller Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 07/16] irqchip: add sl28cpld interrupt controller support Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 08/16] watchdog: add support for sl28cpld watchdog Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-03 6:25 ` Guenter Roeck
2020-04-03 6:25 ` Guenter Roeck
2020-04-03 6:25 ` Guenter Roeck
2020-04-02 20:36 ` [PATCH v2 09/16] pwm: add support for sl28cpld PWM controller Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 10/16] gpio: add a reusable generic gpio_chip using regmap Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-06 7:47 ` Bartosz Golaszewski
2020-04-06 7:47 ` Bartosz Golaszewski
2020-04-06 7:47 ` Bartosz Golaszewski
2020-04-06 10:10 ` Michael Walle
2020-04-06 10:10 ` Michael Walle
2020-04-06 10:10 ` Michael Walle
2020-04-14 9:50 ` Bartosz Golaszewski
2020-04-14 9:50 ` Bartosz Golaszewski
2020-04-14 9:50 ` Bartosz Golaszewski
2020-04-14 10:07 ` Michael Walle
2020-04-14 10:07 ` Michael Walle
2020-04-14 10:07 ` Michael Walle
2020-04-14 17:00 ` Bartosz Golaszewski
2020-04-14 17:00 ` Bartosz Golaszewski
2020-04-14 17:00 ` Bartosz Golaszewski
2020-04-14 18:41 ` Michael Walle
2020-04-14 18:41 ` Michael Walle
2020-04-14 18:41 ` Michael Walle
2020-04-14 19:57 ` Michael Walle
2020-04-14 19:57 ` Michael Walle
2020-04-16 9:20 ` Linus Walleij
2020-04-16 9:20 ` Linus Walleij
2020-04-16 9:20 ` Linus Walleij
2020-04-16 9:34 ` Michael Walle
2020-04-16 9:34 ` Michael Walle
2020-04-16 9:34 ` Michael Walle
2020-04-14 17:21 ` Mark Brown
2020-04-14 17:21 ` Mark Brown
2020-04-14 17:21 ` Mark Brown
2020-04-14 18:36 ` Michael Walle
2020-04-14 18:36 ` Michael Walle
2020-04-14 18:36 ` Michael Walle
2020-04-14 18:39 ` Mark Brown
2020-04-14 18:39 ` Mark Brown
2020-04-14 18:39 ` Mark Brown
2020-04-16 9:27 ` Linus Walleij [this message]
2020-04-16 9:27 ` Linus Walleij
2020-04-16 9:27 ` Linus Walleij
2020-04-17 6:34 ` Michael Walle
2020-04-17 6:34 ` Michael Walle
2020-04-17 6:34 ` Michael Walle
2020-04-21 10:50 ` Michael Walle
2020-04-21 10:50 ` Michael Walle
2020-04-21 10:50 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 11/16] gpio: add support for the sl28cpld GPIO controller Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-16 8:34 ` Linus Walleij
2020-04-16 8:34 ` Linus Walleij
2020-04-16 8:34 ` Linus Walleij
2020-04-16 8:55 ` Michael Walle
2020-04-16 8:55 ` Michael Walle
2020-04-16 8:55 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 12/16] hwmon: add support for the sl28cpld hardware monitoring controller Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 21:30 ` Guenter Roeck
2020-04-02 21:30 ` Guenter Roeck
2020-04-02 21:30 ` Guenter Roeck
2020-04-02 20:36 ` [PATCH v2 13/16] arm64: dts: freescale: sl28: enable sl28cpld Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 14/16] arm64: dts: freescale: sl28: map GPIOs to input events Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 15/16] arm64: dts: freescale: sl28: enable LED support Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` [PATCH v2 16/16] arm64: dts: freescale: sl28: enable fan support Michael Walle
2020-04-02 20:36 ` Michael Walle
2020-04-02 20:36 ` Michael Walle
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=CACRpkdaqgHhPwdKdUai4zvi21qR-cSQUKyzZ3SyfWBLPN9us3w@mail.gmail.com \
--to=linus.walleij@linaro.org \
--cc=bgolaszewski@baylibre.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jason@lakedaemon.net \
--cc=jdelvare@suse.com \
--cc=lee.jones@linaro.org \
--cc=leoyang.li@nxp.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=maz@kernel.org \
--cc=michael@walle.cc \
--cc=robh+dt@kernel.org \
--cc=shawnguo@kernel.org \
--cc=tglx@linutronix.de \
--cc=thierry.reding@gmail.com \
--cc=u.kleine-koenig@pengutronix.de \
--cc=wim@linux-watchdog.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.