* Re: [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Andy Shevchenko @ 2019-08-08 13:48 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, linux-gpio, Geert Uytterhoeven, Pavel Machek,
Linus Walleij, Greg Kroah-Hartman
In-Reply-To: <20190808132543.26274-2-sr@denx.de>
On Thu, Aug 08, 2019 at 03:25:43PM +0200, Stefan Roese wrote:
> This patch fixes a backward compatibility issue, when boards use the
> old style GPIO suffix "-gpio" instead of the new "-gpios". This
> potential problem has been introduced by commit d99482673f95 ("serial:
> mctrl_gpio: Check if GPIO property exisits before requesting it").
>
> This patch now fixes this issue by iterating over all supported GPIO
> suffixes by using the newly introduced for_each_gpio_suffix() helper.
>
> Also, the string buffer is now allocated on the stack to avoid the
> problem of allocation in a loop and its potential failure.
> for (i = 0; i < UART_GPIO_MAX; i++) {
> enum gpiod_flags flags;
> - char *gpio_str;
> + const char *suffix;
> + char gpio_str[32]; /* 32 is max size of property name */
Hmm... don't we have some define for the maximum length of property?
Or maybe we can still continue using kasprintf() approach?
> bool present;
> + int k;
> +
> + /*
> + * Check if GPIO property exists and continue if not. Iterate
> + * over all supported GPIO suffixes (foo-gpios vs. foo-gpio).
> + */
> + for_each_gpio_suffix(k, suffix) {
> + snprintf(gpio_str, sizeof(gpio_str), "%s-%s",
> + mctrl_gpios_desc[i].name, suffix);
> +
> + present = device_property_present(dev, gpio_str);
> + if (present)
> + break;
> + }
>
> - /* Check if GPIO property exists and continue if not */
> - gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
> - mctrl_gpios_desc[i].name);
> - if (!gpio_str)
> - continue;
> -
> - present = device_property_present(dev, gpio_str);
Because there is no more explicit assignment of present outside of the loop,
the compiler may warn about uninitialized variable in use...
> - kfree(gpio_str);
> if (!present)
...here.
> continue;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 1/2] gpiolib: Add for_each_gpio_suffix() helper
From: Andy Shevchenko @ 2019-08-08 13:44 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, linux-gpio, Geert Uytterhoeven, Pavel Machek,
Linus Walleij, Greg Kroah-Hartman
In-Reply-To: <20190808132543.26274-1-sr@denx.de>
On Thu, Aug 08, 2019 at 03:25:42PM +0200, Stefan Roese wrote:
> Add a helper macro to enable the interation over all supported GPIO
> suffixes (currently "gpios" & "gpio"). This will be used by the serial
> mctrl code to check, if a GPIO property exists before requesting it.
Thanks! My comments below.
> +#define for_each_gpio_suffix(idx, suffix) \
> + for (idx = 0; \
> + idx < ARRAY_SIZE(gpio_suffixes) && \
> + (suffix = gpio_suffixes[idx]) != NULL; \
> + idx++)
I think we can use comma here, like
for (idx = 0; idx < ARRAY_SIZE(...), suffix = ...; idx++;)
however, this needs to be checked, b/c I think the last operation makes a
return code, and probably we have to assign suffix first.
(and perhaps switch to one letter for idx makes it fit one line)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Stefan Roese @ 2019-08-08 13:25 UTC (permalink / raw)
To: linux-serial, linux-gpio
Cc: Andy Shevchenko, Geert Uytterhoeven, Pavel Machek, Linus Walleij,
Greg Kroah-Hartman
In-Reply-To: <20190808132543.26274-1-sr@denx.de>
This patch fixes a backward compatibility issue, when boards use the
old style GPIO suffix "-gpio" instead of the new "-gpios". This
potential problem has been introduced by commit d99482673f95 ("serial:
mctrl_gpio: Check if GPIO property exisits before requesting it").
This patch now fixes this issue by iterating over all supported GPIO
suffixes by using the newly introduced for_each_gpio_suffix() helper.
Also, the string buffer is now allocated on the stack to avoid the
problem of allocation in a loop and its potential failure.
Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Pavel Machek <pavel@denx.de>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/tty/serial/serial_mctrl_gpio.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 2b400189be91..d444fdaa280a 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -15,6 +15,7 @@
#include <linux/property.h>
#include "serial_mctrl_gpio.h"
+#include "../../gpio/gpiolib.h"
struct mctrl_gpios {
struct uart_port *port;
@@ -117,17 +118,24 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
for (i = 0; i < UART_GPIO_MAX; i++) {
enum gpiod_flags flags;
- char *gpio_str;
+ const char *suffix;
+ char gpio_str[32]; /* 32 is max size of property name */
bool present;
+ int k;
+
+ /*
+ * Check if GPIO property exists and continue if not. Iterate
+ * over all supported GPIO suffixes (foo-gpios vs. foo-gpio).
+ */
+ for_each_gpio_suffix(k, suffix) {
+ snprintf(gpio_str, sizeof(gpio_str), "%s-%s",
+ mctrl_gpios_desc[i].name, suffix);
+
+ present = device_property_present(dev, gpio_str);
+ if (present)
+ break;
+ }
- /* Check if GPIO property exists and continue if not */
- gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
- mctrl_gpios_desc[i].name);
- if (!gpio_str)
- continue;
-
- present = device_property_present(dev, gpio_str);
- kfree(gpio_str);
if (!present)
continue;
--
2.22.0
^ permalink raw reply related
* [PATCH 1/2] gpiolib: Add for_each_gpio_suffix() helper
From: Stefan Roese @ 2019-08-08 13:25 UTC (permalink / raw)
To: linux-serial, linux-gpio
Cc: Andy Shevchenko, Geert Uytterhoeven, Pavel Machek, Linus Walleij,
Greg Kroah-Hartman
Add a helper macro to enable the interation over all supported GPIO
suffixes (currently "gpios" & "gpio"). This will be used by the serial
mctrl code to check, if a GPIO property exists before requesting it.
Signed-off-by: Stefan Roese <sr@denx.de>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Pavel Machek <pavel@denx.de>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/gpio/gpiolib.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 7c52c2442173..a3add73f99d6 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -92,6 +92,12 @@ struct acpi_gpio_info {
/* gpio suffixes used for ACPI and device tree lookup */
static __maybe_unused const char * const gpio_suffixes[] = { "gpios", "gpio" };
+#define for_each_gpio_suffix(idx, suffix) \
+ for (idx = 0; \
+ idx < ARRAY_SIZE(gpio_suffixes) && \
+ (suffix = gpio_suffixes[idx]) != NULL; \
+ idx++)
+
#ifdef CONFIG_OF_GPIO
struct gpio_desc *of_find_gpio(struct device *dev,
const char *con_id,
--
2.22.0
^ permalink raw reply related
* [PATCH v2] pinctrl: intel: Allow to request locked pads
From: Andy Shevchenko @ 2019-08-08 13:21 UTC (permalink / raw)
To: Mika Westerberg, linux-gpio, Linus Walleij; +Cc: Andy Shevchenko
Some firmwares would like to protect pads from being modified by OS
and at the same time provide them to OS as a resource. So, the driver
in such circumstances may request pad and may not change its state.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
in v2:
- amended comment in intel_pad_locked() respectively to the change (Mika)
- described enum values (Linus)
- lowered case for locking flavour in debugfs for better looking
drivers/pinctrl/intel/pinctrl-intel.c | 67 ++++++++++++++++++++-------
1 file changed, 50 insertions(+), 17 deletions(-)
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index c949df07cbdf..b84a5579beee 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -220,47 +220,69 @@ static bool intel_pad_acpi_mode(struct intel_pinctrl *pctrl, unsigned int pin)
return !(readl(hostown) & BIT(gpp_offset));
}
-static bool intel_pad_locked(struct intel_pinctrl *pctrl, unsigned int pin)
+/*
+ * PAD_UNLOCKED: pad is fully controlled by the configuration registers
+ * PAD_LOCKED: pad configuration registers, except TX state, are locked
+ * PAD_LOCKED_TX: pad configuration TX state is locked
+ * PAD_LOCKED_FULL: pad configuration registers are locked completely
+ *
+ * Locking is considered as read-only mode for corresponding registers and
+ * their respective fields. That said, TX state bit is locked separately from
+ * the main locking scheme.
+ */
+enum {
+ PAD_UNLOCKED = 0,
+ PAD_LOCKED = 1,
+ PAD_LOCKED_TX = 2,
+ PAD_LOCKED_FULL = PAD_LOCKED | PAD_LOCKED_TX,
+};
+
+static int intel_pad_locked(struct intel_pinctrl *pctrl, unsigned int pin)
{
struct intel_community *community;
const struct intel_padgroup *padgrp;
unsigned int offset, gpp_offset;
u32 value;
+ int ret = PAD_UNLOCKED;
community = intel_get_community(pctrl, pin);
if (!community)
- return true;
+ return PAD_LOCKED_FULL;
if (!community->padcfglock_offset)
- return false;
+ return PAD_UNLOCKED;
padgrp = intel_community_get_padgroup(community, pin);
if (!padgrp)
- return true;
+ return PAD_LOCKED_FULL;
gpp_offset = padgroup_offset(padgrp, pin);
/*
* If PADCFGLOCK and PADCFGLOCKTX bits are both clear for this pad,
* the pad is considered unlocked. Any other case means that it is
- * either fully or partially locked and we don't touch it.
+ * either fully or partially locked.
*/
- offset = community->padcfglock_offset + padgrp->reg_num * 8;
+ offset = community->padcfglock_offset + 0 + padgrp->reg_num * 8;
value = readl(community->regs + offset);
if (value & BIT(gpp_offset))
- return true;
+ ret |= PAD_LOCKED;
offset = community->padcfglock_offset + 4 + padgrp->reg_num * 8;
value = readl(community->regs + offset);
if (value & BIT(gpp_offset))
- return true;
+ ret |= PAD_LOCKED_TX;
- return false;
+ return ret;
+}
+
+static bool intel_pad_is_unlocked(struct intel_pinctrl *pctrl, unsigned int pin)
+{
+ return (intel_pad_locked(pctrl, pin) & PAD_LOCKED) == PAD_UNLOCKED;
}
static bool intel_pad_usable(struct intel_pinctrl *pctrl, unsigned int pin)
{
- return intel_pad_owned_by_host(pctrl, pin) &&
- !intel_pad_locked(pctrl, pin);
+ return intel_pad_owned_by_host(pctrl, pin) && intel_pad_is_unlocked(pctrl, pin);
}
static int intel_get_groups_count(struct pinctrl_dev *pctldev)
@@ -294,7 +316,8 @@ static void intel_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
void __iomem *padcfg;
u32 cfg0, cfg1, mode;
- bool locked, acpi;
+ int locked;
+ bool acpi;
if (!intel_pad_owned_by_host(pctrl, pin)) {
seq_puts(s, "not available");
@@ -322,11 +345,16 @@ static void intel_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
if (locked || acpi) {
seq_puts(s, " [");
- if (locked) {
+ if (locked)
seq_puts(s, "LOCKED");
- if (acpi)
- seq_puts(s, ", ");
- }
+ if ((locked & PAD_LOCKED_FULL) == PAD_LOCKED_TX)
+ seq_puts(s, " tx");
+ else if ((locked & PAD_LOCKED_FULL) == PAD_LOCKED_FULL)
+ seq_puts(s, " full");
+
+ if (locked && acpi)
+ seq_puts(s, ", ");
+
if (acpi)
seq_puts(s, "ACPI");
seq_puts(s, "]");
@@ -448,11 +476,16 @@ static int intel_gpio_request_enable(struct pinctrl_dev *pctldev,
raw_spin_lock_irqsave(&pctrl->lock, flags);
- if (!intel_pad_usable(pctrl, pin)) {
+ if (!intel_pad_owned_by_host(pctrl, pin)) {
raw_spin_unlock_irqrestore(&pctrl->lock, flags);
return -EBUSY;
}
+ if (!intel_pad_is_unlocked(pctrl, pin)) {
+ raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+ return 0;
+ }
+
padcfg0 = intel_get_padcfg(pctrl, pin, PADCFG0);
intel_gpio_set_gpio_mode(padcfg0);
/* Disable TX buffer and enable RX (this will be input) */
--
2.20.1
^ permalink raw reply related
* [PATCH 6/6 v2] RFT: gpio: uniphier: Restrict valid interrupts
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Masahiro Yamada,
Thierry Reding, Brian Masney
In-Reply-To: <20190808123242.5359-1-linus.walleij@linaro.org>
The Uniphier GPIO can only create interrupt mappings on
GPIOs offset 0..23. Set the valid_mask in the struct
gpio_irqchip and clear bits 24..ngpios indicating that
these can not be mapped to interrupts.
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog RFT -> RTF v2:
- New patch to set the valid mask for the interrupts.
---
drivers/gpio/gpio-uniphier.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpio/gpio-uniphier.c b/drivers/gpio/gpio-uniphier.c
index 86e8446215fc..9840fcf7de57 100644
--- a/drivers/gpio/gpio-uniphier.c
+++ b/drivers/gpio/gpio-uniphier.c
@@ -338,6 +338,7 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
girq->child_to_parent_hwirq = uniphier_gpio_child_to_parent_hwirq;
girq->handler = handle_bad_irq;
girq->default_type = IRQ_TYPE_NONE;
+ girq->need_valid_mask = true;
uniphier_gpio_hw_init(priv);
@@ -345,6 +346,10 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
if (ret)
return ret;
+ /* Only GPIOs 0..UNIPHIER_GPIO_IRQ_MAX_NUM are valid for interrupts */
+ bitmap_clear(girq->valid_mask, UNIPHIER_GPIO_IRQ_MAX_NUM,
+ ngpios - UNIPHIER_GPIO_IRQ_MAX_NUM);
+
platform_set_drvdata(pdev, priv);
return 0;
--
2.21.0
^ permalink raw reply related
* [PATCH 5/6 v2] RFT: gpio: uniphier: Switch to GPIOLIB_IRQCHIP
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Masahiro Yamada,
Thierry Reding, Brian Masney
In-Reply-To: <20190808123242.5359-1-linus.walleij@linaro.org>
Use the new infrastructure for hierarchical irqchips in
gpiolib.
I have no chance to test or debug this so I need
help.
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog RFT v1 -> RFT v2:
- Drop noisy change of "data" parameter to "d"
- Use ->child_offset_to_irq() callback to account for the
UNIPHIER_GPIO_IRQ_OFFSET.
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-uniphier.c | 161 +++++++++--------------------------
2 files changed, 43 insertions(+), 119 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3125aca2db9f..cc07bbe4864e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -550,6 +550,7 @@ config GPIO_UNIPHIER
tristate "UniPhier GPIO support"
depends on ARCH_UNIPHIER || COMPILE_TEST
depends on OF_GPIO
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
help
Say yes here to support UniPhier GPIOs.
diff --git a/drivers/gpio/gpio-uniphier.c b/drivers/gpio/gpio-uniphier.c
index 93cdcc41e9fb..86e8446215fc 100644
--- a/drivers/gpio/gpio-uniphier.c
+++ b/drivers/gpio/gpio-uniphier.c
@@ -6,7 +6,6 @@
#include <linux/bits.h>
#include <linux/gpio/driver.h>
#include <linux/irq.h>
-#include <linux/irqdomain.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
@@ -30,7 +29,6 @@
struct uniphier_gpio_priv {
struct gpio_chip chip;
struct irq_chip irq_chip;
- struct irq_domain *domain;
void __iomem *regs;
spinlock_t lock;
u32 saved_vals[0];
@@ -162,28 +160,10 @@ static void uniphier_gpio_set_multiple(struct gpio_chip *chip,
}
}
-static int uniphier_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
-{
- struct irq_fwspec fwspec;
-
- if (offset < UNIPHIER_GPIO_IRQ_OFFSET)
- return -ENXIO;
-
- fwspec.fwnode = of_node_to_fwnode(chip->parent->of_node);
- fwspec.param_count = 2;
- fwspec.param[0] = offset - UNIPHIER_GPIO_IRQ_OFFSET;
- /*
- * IRQ_TYPE_NONE is rejected by the parent irq domain. Set LEVEL_HIGH
- * temporarily. Anyway, ->irq_set_type() will override it later.
- */
- fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
-
- return irq_create_fwspec_mapping(&fwspec);
-}
-
static void uniphier_gpio_irq_mask(struct irq_data *data)
{
- struct uniphier_gpio_priv *priv = data->chip_data;
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct uniphier_gpio_priv *priv = gpiochip_get_data(gc);
u32 mask = BIT(data->hwirq);
uniphier_gpio_reg_update(priv, UNIPHIER_GPIO_IRQ_EN, mask, 0);
@@ -193,7 +173,8 @@ static void uniphier_gpio_irq_mask(struct irq_data *data)
static void uniphier_gpio_irq_unmask(struct irq_data *data)
{
- struct uniphier_gpio_priv *priv = data->chip_data;
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct uniphier_gpio_priv *priv = gpiochip_get_data(gc);
u32 mask = BIT(data->hwirq);
uniphier_gpio_reg_update(priv, UNIPHIER_GPIO_IRQ_EN, mask, mask);
@@ -203,7 +184,8 @@ static void uniphier_gpio_irq_unmask(struct irq_data *data)
static int uniphier_gpio_irq_set_type(struct irq_data *data, unsigned int type)
{
- struct uniphier_gpio_priv *priv = data->chip_data;
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct uniphier_gpio_priv *priv = gpiochip_get_data(gc);
u32 mask = BIT(data->hwirq);
u32 val = 0;
@@ -245,82 +227,6 @@ static int uniphier_gpio_irq_get_parent_hwirq(struct uniphier_gpio_priv *priv,
return -ENOENT;
}
-static int uniphier_gpio_irq_domain_translate(struct irq_domain *domain,
- struct irq_fwspec *fwspec,
- unsigned long *out_hwirq,
- unsigned int *out_type)
-{
- if (WARN_ON(fwspec->param_count < 2))
- return -EINVAL;
-
- *out_hwirq = fwspec->param[0];
- *out_type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
-
- return 0;
-}
-
-static int uniphier_gpio_irq_domain_alloc(struct irq_domain *domain,
- unsigned int virq,
- unsigned int nr_irqs, void *arg)
-{
- struct uniphier_gpio_priv *priv = domain->host_data;
- struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq;
- unsigned int type;
- int ret;
-
- if (WARN_ON(nr_irqs != 1))
- return -EINVAL;
-
- ret = uniphier_gpio_irq_domain_translate(domain, arg, &hwirq, &type);
- if (ret)
- return ret;
-
- ret = uniphier_gpio_irq_get_parent_hwirq(priv, hwirq);
- if (ret < 0)
- return ret;
-
- /* parent is UniPhier AIDET */
- parent_fwspec.fwnode = domain->parent->fwnode;
- parent_fwspec.param_count = 2;
- parent_fwspec.param[0] = ret;
- parent_fwspec.param[1] = (type == IRQ_TYPE_EDGE_BOTH) ?
- IRQ_TYPE_EDGE_FALLING : type;
-
- ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
- &priv->irq_chip, priv);
- if (ret)
- return ret;
-
- return irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
-}
-
-static int uniphier_gpio_irq_domain_activate(struct irq_domain *domain,
- struct irq_data *data, bool early)
-{
- struct uniphier_gpio_priv *priv = domain->host_data;
- struct gpio_chip *chip = &priv->chip;
-
- return gpiochip_lock_as_irq(chip, data->hwirq + UNIPHIER_GPIO_IRQ_OFFSET);
-}
-
-static void uniphier_gpio_irq_domain_deactivate(struct irq_domain *domain,
- struct irq_data *data)
-{
- struct uniphier_gpio_priv *priv = domain->host_data;
- struct gpio_chip *chip = &priv->chip;
-
- gpiochip_unlock_as_irq(chip, data->hwirq + UNIPHIER_GPIO_IRQ_OFFSET);
-}
-
-static const struct irq_domain_ops uniphier_gpio_irq_domain_ops = {
- .alloc = uniphier_gpio_irq_domain_alloc,
- .free = irq_domain_free_irqs_common,
- .activate = uniphier_gpio_irq_domain_activate,
- .deactivate = uniphier_gpio_irq_domain_deactivate,
- .translate = uniphier_gpio_irq_domain_translate,
-};
-
static void uniphier_gpio_hw_init(struct uniphier_gpio_priv *priv)
{
/*
@@ -338,6 +244,32 @@ static unsigned int uniphier_gpio_get_nbanks(unsigned int ngpio)
return DIV_ROUND_UP(ngpio, UNIPHIER_GPIO_LINES_PER_BANK);
}
+static unsigned int uniphier_gpio_child_offset_to_irq(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ return offset + UNIPHIER_GPIO_IRQ_OFFSET;
+}
+
+static int uniphier_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+ unsigned int child,
+ unsigned int child_type,
+ unsigned int *parent,
+ unsigned int *parent_type)
+{
+ struct uniphier_gpio_priv *priv = gpiochip_get_data(gc);
+ int ret;
+
+ ret = uniphier_gpio_irq_get_parent_hwirq(priv, child);
+ if (ret < 0)
+ return ret;
+ *parent = ret;
+ if (child_type == IRQ_TYPE_EDGE_BOTH)
+ *parent_type = IRQ_TYPE_EDGE_FALLING;
+ else
+ *parent_type = child_type;
+ return 0;
+}
+
static int uniphier_gpio_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -345,6 +277,7 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
struct irq_domain *parent_domain;
struct uniphier_gpio_priv *priv;
struct gpio_chip *chip;
+ struct gpio_irq_chip *girq;
struct irq_chip *irq_chip;
unsigned int nregs;
u32 ngpios;
@@ -386,7 +319,6 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
chip->get = uniphier_gpio_get;
chip->set = uniphier_gpio_set;
chip->set_multiple = uniphier_gpio_set_multiple;
- chip->to_irq = uniphier_gpio_to_irq;
chip->base = -1;
chip->ngpio = ngpios;
@@ -398,34 +330,26 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
irq_chip->irq_set_affinity = irq_chip_set_affinity_parent;
irq_chip->irq_set_type = uniphier_gpio_irq_set_type;
+ girq = &chip->irq;
+ girq->chip = irq_chip;
+ girq->fwnode = of_node_to_fwnode(dev->of_node);
+ girq->parent_domain = parent_domain;
+ girq->child_offset_to_irq = uniphier_gpio_child_offset_to_irq;
+ girq->child_to_parent_hwirq = uniphier_gpio_child_to_parent_hwirq;
+ girq->handler = handle_bad_irq;
+ girq->default_type = IRQ_TYPE_NONE;
+
uniphier_gpio_hw_init(priv);
ret = devm_gpiochip_add_data(dev, chip, priv);
if (ret)
return ret;
- priv->domain = irq_domain_create_hierarchy(
- parent_domain, 0,
- UNIPHIER_GPIO_IRQ_MAX_NUM,
- of_node_to_fwnode(dev->of_node),
- &uniphier_gpio_irq_domain_ops, priv);
- if (!priv->domain)
- return -ENOMEM;
-
platform_set_drvdata(pdev, priv);
return 0;
}
-static int uniphier_gpio_remove(struct platform_device *pdev)
-{
- struct uniphier_gpio_priv *priv = platform_get_drvdata(pdev);
-
- irq_domain_remove(priv->domain);
-
- return 0;
-}
-
static int __maybe_unused uniphier_gpio_suspend(struct device *dev)
{
struct uniphier_gpio_priv *priv = dev_get_drvdata(dev);
@@ -485,7 +409,6 @@ MODULE_DEVICE_TABLE(of, uniphier_gpio_match);
static struct platform_driver uniphier_gpio_driver = {
.probe = uniphier_gpio_probe,
- .remove = uniphier_gpio_remove,
.driver = {
.name = "uniphier-gpio",
.of_match_table = uniphier_gpio_match,
--
2.21.0
^ permalink raw reply related
* [PATCH 4/6 v2] RFT: gpio: thunderx: Switch to GPIOLIB_IRQCHIP
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, David Daney, Thierry Reding,
Brian Masney
In-Reply-To: <20190808123242.5359-1-linus.walleij@linaro.org>
Use the new infrastructure for hierarchical irqchips in
gpiolib.
The major part of the rewrite was dues to the fact that
the driver was passing around a per-irq pointer to
struct thunderx_line * data container, and the central
handlers will assume struct gpio_chip * to be passed
to we need to use the hwirq as index to look up the
struct thunderx_line * for each IRQ.
The pushing and pop:ing of the irqdomain was confusing
because I've never seen this before, but I tried to
replicate it as best I could.
I have no chance to test or debug this so I need
help.
Cc: David Daney <david.daney@cavium.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog RFT v1-> RFT v2:
- Rebased.
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-thunderx.c | 163 ++++++++++++-----------------------
2 files changed, 57 insertions(+), 107 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b34e9b11a7ef..3125aca2db9f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -539,6 +539,7 @@ config GPIO_THUNDERX
tristate "Cavium ThunderX/OCTEON-TX GPIO"
depends on ARCH_THUNDER || (64BIT && COMPILE_TEST)
depends on PCI_MSI
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
select IRQ_FASTEOI_HIERARCHY_HANDLERS
help
diff --git a/drivers/gpio/gpio-thunderx.c b/drivers/gpio/gpio-thunderx.c
index 715371b5102a..ddad5c7ea617 100644
--- a/drivers/gpio/gpio-thunderx.c
+++ b/drivers/gpio/gpio-thunderx.c
@@ -53,7 +53,6 @@ struct thunderx_line {
struct thunderx_gpio {
struct gpio_chip chip;
u8 __iomem *register_base;
- struct irq_domain *irqd;
struct msix_entry *msix_entries; /* per line MSI-X */
struct thunderx_line *line_entries; /* per line irq info */
raw_spinlock_t lock;
@@ -283,54 +282,60 @@ static void thunderx_gpio_set_multiple(struct gpio_chip *chip,
}
}
-static void thunderx_gpio_irq_ack(struct irq_data *data)
+static void thunderx_gpio_irq_ack(struct irq_data *d)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
writeq(GPIO_INTR_INTR,
- txline->txgpio->register_base + intr_reg(txline->line));
+ txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
}
-static void thunderx_gpio_irq_mask(struct irq_data *data)
+static void thunderx_gpio_irq_mask(struct irq_data *d)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
writeq(GPIO_INTR_ENA_W1C,
- txline->txgpio->register_base + intr_reg(txline->line));
+ txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
}
-static void thunderx_gpio_irq_mask_ack(struct irq_data *data)
+static void thunderx_gpio_irq_mask_ack(struct irq_data *d)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
writeq(GPIO_INTR_ENA_W1C | GPIO_INTR_INTR,
- txline->txgpio->register_base + intr_reg(txline->line));
+ txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
}
-static void thunderx_gpio_irq_unmask(struct irq_data *data)
+static void thunderx_gpio_irq_unmask(struct irq_data *d)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
writeq(GPIO_INTR_ENA_W1S,
- txline->txgpio->register_base + intr_reg(txline->line));
+ txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
}
-static int thunderx_gpio_irq_set_type(struct irq_data *data,
+static int thunderx_gpio_irq_set_type(struct irq_data *d,
unsigned int flow_type)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
- struct thunderx_gpio *txgpio = txline->txgpio;
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
+ struct thunderx_line *txline =
+ &txgpio->line_entries[irqd_to_hwirq(d)];
u64 bit_cfg;
- irqd_set_trigger_type(data, flow_type);
+ irqd_set_trigger_type(d, flow_type);
bit_cfg = txline->fil_bits | GPIO_BIT_CFG_INT_EN;
if (flow_type & IRQ_TYPE_EDGE_BOTH) {
- irq_set_handler_locked(data, handle_fasteoi_ack_irq);
+ irq_set_handler_locked(d, handle_fasteoi_ack_irq);
bit_cfg |= GPIO_BIT_CFG_INT_TYPE;
} else {
- irq_set_handler_locked(data, handle_fasteoi_mask_irq);
+ irq_set_handler_locked(d, handle_fasteoi_mask_irq);
}
raw_spin_lock(&txgpio->lock);
@@ -359,33 +364,6 @@ static void thunderx_gpio_irq_disable(struct irq_data *data)
irq_chip_disable_parent(data);
}
-static int thunderx_gpio_irq_request_resources(struct irq_data *data)
-{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
- struct thunderx_gpio *txgpio = txline->txgpio;
- int r;
-
- r = gpiochip_lock_as_irq(&txgpio->chip, txline->line);
- if (r)
- return r;
-
- r = irq_chip_request_resources_parent(data);
- if (r)
- gpiochip_unlock_as_irq(&txgpio->chip, txline->line);
-
- return r;
-}
-
-static void thunderx_gpio_irq_release_resources(struct irq_data *data)
-{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
- struct thunderx_gpio *txgpio = txline->txgpio;
-
- irq_chip_release_resources_parent(data);
-
- gpiochip_unlock_as_irq(&txgpio->chip, txline->line);
-}
-
/*
* Interrupts are chained from underlying MSI-X vectors. We have
* these irq_chip functions to be able to handle level triggering
@@ -402,48 +380,22 @@ static struct irq_chip thunderx_gpio_irq_chip = {
.irq_unmask = thunderx_gpio_irq_unmask,
.irq_eoi = irq_chip_eoi_parent,
.irq_set_affinity = irq_chip_set_affinity_parent,
- .irq_request_resources = thunderx_gpio_irq_request_resources,
- .irq_release_resources = thunderx_gpio_irq_release_resources,
.irq_set_type = thunderx_gpio_irq_set_type,
.flags = IRQCHIP_SET_TYPE_MASKED
};
-static int thunderx_gpio_irq_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
- irq_hw_number_t *hwirq,
- unsigned int *type)
+static int thunderx_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+ unsigned int child,
+ unsigned int child_type,
+ unsigned int *parent,
+ unsigned int *parent_type)
{
- struct thunderx_gpio *txgpio = d->host_data;
-
- if (WARN_ON(fwspec->param_count < 2))
- return -EINVAL;
- if (fwspec->param[0] >= txgpio->chip.ngpio)
- return -EINVAL;
- *hwirq = fwspec->param[0];
- *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
- return 0;
-}
-
-static int thunderx_gpio_irq_alloc(struct irq_domain *d, unsigned int virq,
- unsigned int nr_irqs, void *arg)
-{
- struct thunderx_line *txline = arg;
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
- return irq_domain_set_hwirq_and_chip(d, virq, txline->line,
- &thunderx_gpio_irq_chip, txline);
-}
-
-static const struct irq_domain_ops thunderx_gpio_irqd_ops = {
- .alloc = thunderx_gpio_irq_alloc,
- .translate = thunderx_gpio_irq_translate
-};
-
-static int thunderx_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
-{
- struct thunderx_gpio *txgpio = gpiochip_get_data(chip);
-
- return irq_find_mapping(txgpio->irqd, offset);
+ *parent = txgpio->base_msi + (2 * child);
+ *parent_type = IRQ_TYPE_LEVEL_HIGH;
+ return 0;
}
static int thunderx_gpio_probe(struct pci_dev *pdev,
@@ -453,6 +405,7 @@ static int thunderx_gpio_probe(struct pci_dev *pdev,
struct device *dev = &pdev->dev;
struct thunderx_gpio *txgpio;
struct gpio_chip *chip;
+ struct gpio_irq_chip *girq;
int ngpio, i;
int err = 0;
@@ -497,8 +450,8 @@ static int thunderx_gpio_probe(struct pci_dev *pdev,
}
txgpio->msix_entries = devm_kcalloc(dev,
- ngpio, sizeof(struct msix_entry),
- GFP_KERNEL);
+ ngpio, sizeof(struct msix_entry),
+ GFP_KERNEL);
if (!txgpio->msix_entries) {
err = -ENOMEM;
goto out;
@@ -539,27 +492,6 @@ static int thunderx_gpio_probe(struct pci_dev *pdev,
if (err < 0)
goto out;
- /*
- * Push GPIO specific irqdomain on hierarchy created as a side
- * effect of the pci_enable_msix()
- */
- txgpio->irqd = irq_domain_create_hierarchy(irq_get_irq_data(txgpio->msix_entries[0].vector)->domain,
- 0, 0, of_node_to_fwnode(dev->of_node),
- &thunderx_gpio_irqd_ops, txgpio);
- if (!txgpio->irqd) {
- err = -ENOMEM;
- goto out;
- }
-
- /* Push on irq_data and the domain for each line. */
- for (i = 0; i < ngpio; i++) {
- err = irq_domain_push_irq(txgpio->irqd,
- txgpio->msix_entries[i].vector,
- &txgpio->line_entries[i]);
- if (err < 0)
- dev_err(dev, "irq_domain_push_irq: %d\n", err);
- }
-
chip->label = KBUILD_MODNAME;
chip->parent = dev;
chip->owner = THIS_MODULE;
@@ -574,11 +506,28 @@ static int thunderx_gpio_probe(struct pci_dev *pdev,
chip->set = thunderx_gpio_set;
chip->set_multiple = thunderx_gpio_set_multiple;
chip->set_config = thunderx_gpio_set_config;
- chip->to_irq = thunderx_gpio_to_irq;
+ girq = &chip->irq;
+ girq->chip = &thunderx_gpio_irq_chip;
+ girq->fwnode = of_node_to_fwnode(dev->of_node);
+ girq->parent_domain =
+ irq_get_irq_data(txgpio->msix_entries[0].vector)->domain;
+ girq->child_to_parent_hwirq = thunderx_gpio_child_to_parent_hwirq;
+ girq->handler = handle_bad_irq;
+ girq->default_type = IRQ_TYPE_NONE;
+
err = devm_gpiochip_add_data(dev, chip, txgpio);
if (err)
goto out;
+ /* Push on irq_data and the domain for each line. */
+ for (i = 0; i < ngpio; i++) {
+ err = irq_domain_push_irq(chip->irq.domain,
+ txgpio->msix_entries[i].vector,
+ chip);
+ if (err < 0)
+ dev_err(dev, "irq_domain_push_irq: %d\n", err);
+ }
+
dev_info(dev, "ThunderX GPIO: %d lines with base %d.\n",
ngpio, chip->base);
return 0;
@@ -593,10 +542,10 @@ static void thunderx_gpio_remove(struct pci_dev *pdev)
struct thunderx_gpio *txgpio = pci_get_drvdata(pdev);
for (i = 0; i < txgpio->chip.ngpio; i++)
- irq_domain_pop_irq(txgpio->irqd,
+ irq_domain_pop_irq(txgpio->chip.irq.domain,
txgpio->msix_entries[i].vector);
- irq_domain_remove(txgpio->irqd);
+ irq_domain_remove(txgpio->chip.irq.domain);
pci_set_drvdata(pdev, NULL);
}
--
2.21.0
^ permalink raw reply related
* [PATCH 3/6 v2] qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio; +Cc: Bartosz Golaszewski, Brian Masney, Linus Walleij
In-Reply-To: <20190808123242.5359-1-linus.walleij@linaro.org>
From: Brian Masney <masneyb@onstation.org>
Now that the GPIO core has support for hierarchical IRQ chips, convert
Qualcomm's spmi-gpio over to use these new helpers to reduce duplicated
code across drivers.
This change was tested on a LG Nexus 5 (hammerhead) phone.
Signed-off-by: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Change "child_pin_to_irq" into "child_offset_to_irq" so
we avoid confusion with pin control.
---
drivers/pinctrl/qcom/Kconfig | 1 +
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 92 +++++++-----------------
2 files changed, 26 insertions(+), 67 deletions(-)
diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 8e14a5f2e970..fa2c87821401 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -138,6 +138,7 @@ config PINCTRL_QCOM_SPMI_PMIC
select PINMUX
select PINCONF
select GENERIC_PINCONF
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
help
This is the pinctrl, pinmux, pinconf and gpiolib driver for the
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index f39da87ea185..442db15e0729 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -170,8 +170,6 @@ struct pmic_gpio_state {
struct regmap *map;
struct pinctrl_dev *ctrl;
struct gpio_chip chip;
- struct fwnode_handle *fwnode;
- struct irq_domain *domain;
};
static const struct pinconf_generic_params pmic_gpio_bindings[] = {
@@ -751,23 +749,6 @@ static int pmic_gpio_of_xlate(struct gpio_chip *chip,
return gpio_desc->args[0] - PMIC_GPIO_PHYSICAL_OFFSET;
}
-static int pmic_gpio_to_irq(struct gpio_chip *chip, unsigned pin)
-{
- struct pmic_gpio_state *state = gpiochip_get_data(chip);
- struct irq_fwspec fwspec;
-
- fwspec.fwnode = state->fwnode;
- fwspec.param_count = 2;
- fwspec.param[0] = pin + PMIC_GPIO_PHYSICAL_OFFSET;
- /*
- * Set the type to a safe value temporarily. This will be overwritten
- * later with the proper value by irq_set_type.
- */
- fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
-
- return irq_create_fwspec_mapping(&fwspec);
-}
-
static void pmic_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
struct pmic_gpio_state *state = gpiochip_get_data(chip);
@@ -787,7 +768,6 @@ static const struct gpio_chip pmic_gpio_gpio_template = {
.request = gpiochip_generic_request,
.free = gpiochip_generic_free,
.of_xlate = pmic_gpio_of_xlate,
- .to_irq = pmic_gpio_to_irq,
.dbg_show = pmic_gpio_dbg_show,
};
@@ -964,46 +944,24 @@ static int pmic_gpio_domain_translate(struct irq_domain *domain,
return 0;
}
-static int pmic_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq,
- unsigned int nr_irqs, void *data)
+static unsigned int pmic_gpio_child_offset_to_irq(struct gpio_chip *chip,
+ unsigned int offset)
{
- struct pmic_gpio_state *state = container_of(domain->host_data,
- struct pmic_gpio_state,
- chip);
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq;
- unsigned int type;
- int ret, i;
-
- ret = pmic_gpio_domain_translate(domain, fwspec, &hwirq, &type);
- if (ret)
- return ret;
-
- for (i = 0; i < nr_irqs; i++)
- irq_domain_set_info(domain, virq + i, hwirq + i,
- &pmic_gpio_irq_chip, state,
- handle_level_irq, NULL, NULL);
+ return offset + PMIC_GPIO_PHYSICAL_OFFSET;
+}
- parent_fwspec.fwnode = domain->parent->fwnode;
- parent_fwspec.param_count = 4;
- parent_fwspec.param[0] = 0;
- parent_fwspec.param[1] = hwirq + 0xc0;
- parent_fwspec.param[2] = 0;
- parent_fwspec.param[3] = fwspec->param[1];
+static int pmic_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
+ unsigned int child_hwirq,
+ unsigned int child_type,
+ unsigned int *parent_hwirq,
+ unsigned int *parent_type)
+{
+ *parent_hwirq = child_hwirq + 0xc0;
+ *parent_type = child_type;
- return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
- &parent_fwspec);
+ return 0;
}
-static const struct irq_domain_ops pmic_gpio_domain_ops = {
- .activate = gpiochip_irq_domain_activate,
- .alloc = pmic_gpio_domain_alloc,
- .deactivate = gpiochip_irq_domain_deactivate,
- .free = irq_domain_free_irqs_common,
- .translate = pmic_gpio_domain_translate,
-};
-
static int pmic_gpio_probe(struct platform_device *pdev)
{
struct irq_domain *parent_domain;
@@ -1013,6 +971,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
struct pinctrl_desc *pctrldesc;
struct pmic_gpio_pad *pad, *pads;
struct pmic_gpio_state *state;
+ struct gpio_irq_chip *girq;
int ret, npins, i;
u32 reg;
@@ -1092,19 +1051,21 @@ static int pmic_gpio_probe(struct platform_device *pdev)
if (!parent_domain)
return -ENXIO;
- state->fwnode = of_node_to_fwnode(state->dev->of_node);
- state->domain = irq_domain_create_hierarchy(parent_domain, 0,
- state->chip.ngpio,
- state->fwnode,
- &pmic_gpio_domain_ops,
- &state->chip);
- if (!state->domain)
- return -ENODEV;
+ girq = &state->chip.irq;
+ girq->chip = &pmic_gpio_irq_chip;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_level_irq;
+ girq->fwnode = of_node_to_fwnode(state->dev->of_node);
+ girq->parent_domain = parent_domain;
+ girq->child_to_parent_hwirq = pmic_gpio_child_to_parent_hwirq;
+ girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
+ girq->child_offset_to_irq = pmic_gpio_child_offset_to_irq;
+ girq->child_irq_domain_ops.translate = pmic_gpio_domain_translate;
ret = gpiochip_add_data(&state->chip, state);
if (ret) {
dev_err(state->dev, "can't add gpio chip\n");
- goto err_chip_add_data;
+ return ret;
}
/*
@@ -1130,8 +1091,6 @@ static int pmic_gpio_probe(struct platform_device *pdev)
err_range:
gpiochip_remove(&state->chip);
-err_chip_add_data:
- irq_domain_remove(state->domain);
return ret;
}
@@ -1140,7 +1099,6 @@ static int pmic_gpio_remove(struct platform_device *pdev)
struct pmic_gpio_state *state = platform_get_drvdata(pdev);
gpiochip_remove(&state->chip);
- irq_domain_remove(state->domain);
return 0;
}
--
2.21.0
^ permalink raw reply related
* [PATCH 2/6 v2] gpio: ixp4xx: Convert to hierarchical GPIOLIB_IRQCHIP
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Thomas Gleixner, Marc Zyngier,
Lina Iyer, Jon Hunter, Sowjanya Komatineni, Bitan Biswas,
linux-tegra, Thierry Reding, Brian Masney
In-Reply-To: <20190808123242.5359-1-linus.walleij@linaro.org>
This modifies the IXP4xx driver to use the new helpers
to handle the remapping of parent to child hardware irqs
in the gpiolib core.
This pulls the majority of the code out of the driver
and use the generic code in gpiolib.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lina Iyer <ilina@codeaurora.org>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Sowjanya Komatineni <skomatineni@nvidia.com>
Cc: Bitan Biswas <bbiswas@nvidia.com>
Cc: linux-tegra@vger.kernel.org
Cc: Thierry Reding <treding@nvidia.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebased.
ChangeLog RFC->v1:
- Fixed some bugs in dereferencing the gpio_chip first
from the irqchip data, then dereference the
local state container from the gpio_chip
- Adapted to changes in the core patch like provide
as translation callback rather than a table.
- Tested on the Linksys NSLU2 and works like a charm
---
drivers/gpio/Kconfig | 2 +-
drivers/gpio/gpio-ixp4xx.c | 277 +++++++++----------------------------
2 files changed, 63 insertions(+), 216 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index bb13c266c329..b34e9b11a7ef 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -288,7 +288,7 @@ config GPIO_IXP4XX
depends on ARM # For <asm/mach-types.h>
depends on ARCH_IXP4XX
select GPIO_GENERIC
- select IRQ_DOMAIN
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
help
Say yes here to support the GPIO functionality of a number of Intel
diff --git a/drivers/gpio/gpio-ixp4xx.c b/drivers/gpio/gpio-ixp4xx.c
index 670c2a85a35b..8bd23e80c61f 100644
--- a/drivers/gpio/gpio-ixp4xx.c
+++ b/drivers/gpio/gpio-ixp4xx.c
@@ -47,7 +47,6 @@
* @dev: containing device for this instance
* @fwnode: the fwnode for this GPIO chip
* @gc: gpiochip for this instance
- * @domain: irqdomain for this chip instance
* @base: remapped I/O-memory base
* @irq_edge: Each bit represents an IRQ: 1: edge-triggered,
* 0: level triggered
@@ -56,48 +55,22 @@ struct ixp4xx_gpio {
struct device *dev;
struct fwnode_handle *fwnode;
struct gpio_chip gc;
- struct irq_domain *domain;
void __iomem *base;
unsigned long long irq_edge;
};
-/**
- * struct ixp4xx_gpio_map - IXP4 GPIO to parent IRQ map
- * @gpio_offset: offset of the IXP4 GPIO line
- * @parent_hwirq: hwirq on the parent IRQ controller
- */
-struct ixp4xx_gpio_map {
- int gpio_offset;
- int parent_hwirq;
-};
-
-/* GPIO lines 0..12 have corresponding IRQs, GPIOs 13..15 have no IRQs */
-const struct ixp4xx_gpio_map ixp4xx_gpiomap[] = {
- { .gpio_offset = 0, .parent_hwirq = 6 },
- { .gpio_offset = 1, .parent_hwirq = 7 },
- { .gpio_offset = 2, .parent_hwirq = 19 },
- { .gpio_offset = 3, .parent_hwirq = 20 },
- { .gpio_offset = 4, .parent_hwirq = 21 },
- { .gpio_offset = 5, .parent_hwirq = 22 },
- { .gpio_offset = 6, .parent_hwirq = 23 },
- { .gpio_offset = 7, .parent_hwirq = 24 },
- { .gpio_offset = 8, .parent_hwirq = 25 },
- { .gpio_offset = 9, .parent_hwirq = 26 },
- { .gpio_offset = 10, .parent_hwirq = 27 },
- { .gpio_offset = 11, .parent_hwirq = 28 },
- { .gpio_offset = 12, .parent_hwirq = 29 },
-};
-
static void ixp4xx_gpio_irq_ack(struct irq_data *d)
{
- struct ixp4xx_gpio *g = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct ixp4xx_gpio *g = gpiochip_get_data(gc);
__raw_writel(BIT(d->hwirq), g->base + IXP4XX_REG_GPIS);
}
static void ixp4xx_gpio_irq_unmask(struct irq_data *d)
{
- struct ixp4xx_gpio *g = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct ixp4xx_gpio *g = gpiochip_get_data(gc);
/* ACK when unmasking if not edge-triggered */
if (!(g->irq_edge & BIT(d->hwirq)))
@@ -108,7 +81,8 @@ static void ixp4xx_gpio_irq_unmask(struct irq_data *d)
static int ixp4xx_gpio_irq_set_type(struct irq_data *d, unsigned int type)
{
- struct ixp4xx_gpio *g = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct ixp4xx_gpio *g = gpiochip_get_data(gc);
int line = d->hwirq;
unsigned long flags;
u32 int_style;
@@ -187,122 +161,31 @@ static struct irq_chip ixp4xx_gpio_irqchip = {
.irq_set_type = ixp4xx_gpio_irq_set_type,
};
-static int ixp4xx_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
-{
- struct ixp4xx_gpio *g = gpiochip_get_data(gc);
- struct irq_fwspec fwspec;
-
- fwspec.fwnode = g->fwnode;
- fwspec.param_count = 2;
- fwspec.param[0] = offset;
- fwspec.param[1] = IRQ_TYPE_NONE;
-
- return irq_create_fwspec_mapping(&fwspec);
-}
-
-static int ixp4xx_gpio_irq_domain_translate(struct irq_domain *domain,
- struct irq_fwspec *fwspec,
- unsigned long *hwirq,
- unsigned int *type)
+static int ixp4xx_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+ unsigned int child,
+ unsigned int child_type,
+ unsigned int *parent,
+ unsigned int *parent_type)
{
- int ret;
+ /* All these interrupts are level high in the CPU */
+ *parent_type = IRQ_TYPE_LEVEL_HIGH;
- /* We support standard DT translation */
- if (is_of_node(fwspec->fwnode) && fwspec->param_count == 2) {
- return irq_domain_translate_twocell(domain, fwspec,
- hwirq, type);
+ /* GPIO lines 0..12 have dedicated IRQs */
+ if (child == 0) {
+ *parent = 6;
+ return 0;
}
-
- /* This goes away when we transition to DT */
- if (is_fwnode_irqchip(fwspec->fwnode)) {
- ret = irq_domain_translate_twocell(domain, fwspec,
- hwirq, type);
- if (ret)
- return ret;
- WARN_ON(*type == IRQ_TYPE_NONE);
+ if (child == 1) {
+ *parent = 7;
return 0;
}
- return -EINVAL;
-}
-
-static int ixp4xx_gpio_irq_domain_alloc(struct irq_domain *d,
- unsigned int irq, unsigned int nr_irqs,
- void *data)
-{
- struct ixp4xx_gpio *g = d->host_data;
- irq_hw_number_t hwirq;
- unsigned int type = IRQ_TYPE_NONE;
- struct irq_fwspec *fwspec = data;
- int ret;
- int i;
-
- ret = ixp4xx_gpio_irq_domain_translate(d, fwspec, &hwirq, &type);
- if (ret)
- return ret;
-
- dev_dbg(g->dev, "allocate IRQ %d..%d, hwirq %lu..%lu\n",
- irq, irq + nr_irqs - 1,
- hwirq, hwirq + nr_irqs - 1);
-
- for (i = 0; i < nr_irqs; i++) {
- struct irq_fwspec parent_fwspec;
- const struct ixp4xx_gpio_map *map;
- int j;
-
- /* Not all lines support IRQs */
- for (j = 0; j < ARRAY_SIZE(ixp4xx_gpiomap); j++) {
- map = &ixp4xx_gpiomap[j];
- if (map->gpio_offset == hwirq)
- break;
- }
- if (j == ARRAY_SIZE(ixp4xx_gpiomap)) {
- dev_err(g->dev, "can't look up hwirq %lu\n", hwirq);
- return -EINVAL;
- }
- dev_dbg(g->dev, "found parent hwirq %u\n", map->parent_hwirq);
-
- /*
- * We set handle_bad_irq because the .set_type() should
- * always be invoked and set the right type of handler.
- */
- irq_domain_set_info(d,
- irq + i,
- hwirq + i,
- &ixp4xx_gpio_irqchip,
- g,
- handle_bad_irq,
- NULL, NULL);
- irq_set_probe(irq + i);
-
- /*
- * Create a IRQ fwspec to send up to the parent irqdomain:
- * specify the hwirq we address on the parent and tie it
- * all together up the chain.
- */
- parent_fwspec.fwnode = d->parent->fwnode;
- parent_fwspec.param_count = 2;
- parent_fwspec.param[0] = map->parent_hwirq;
- /* This parent only handles asserted level IRQs */
- parent_fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
- dev_dbg(g->dev, "alloc_irqs_parent for %d parent hwirq %d\n",
- irq + i, map->parent_hwirq);
- ret = irq_domain_alloc_irqs_parent(d, irq + i, 1,
- &parent_fwspec);
- if (ret)
- dev_err(g->dev,
- "failed to allocate parent hwirq %d for hwirq %lu\n",
- map->parent_hwirq, hwirq);
+ if (child >= 2 && child <= 12) {
+ *parent = child + 17;
+ return 0;
}
-
- return 0;
+ return -EINVAL;
}
-static const struct irq_domain_ops ixp4xx_gpio_irqdomain_ops = {
- .translate = ixp4xx_gpio_irq_domain_translate,
- .alloc = ixp4xx_gpio_irq_domain_alloc,
- .free = irq_domain_free_irqs_common,
-};
-
static int ixp4xx_gpio_probe(struct platform_device *pdev)
{
unsigned long flags;
@@ -311,8 +194,8 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
struct irq_domain *parent;
struct resource *res;
struct ixp4xx_gpio *g;
+ struct gpio_irq_chip *girq;
int ret;
- int i;
g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL);
if (!g)
@@ -326,6 +209,35 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
return PTR_ERR(g->base);
}
+ /*
+ * When we convert to device tree we will simply look up the
+ * parent irqdomain using irq_find_host(parent) as parent comes
+ * from IRQCHIP_DECLARE(), then use of_node_to_fwnode() to get
+ * the fwnode. For now we need this boardfile style code.
+ */
+ if (np) {
+ struct device_node *irq_parent;
+
+ irq_parent = of_irq_find_parent(np);
+ if (!irq_parent) {
+ dev_err(dev, "no IRQ parent node\n");
+ return -ENODEV;
+ }
+ parent = irq_find_host(irq_parent);
+ if (!parent) {
+ dev_err(dev, "no IRQ parent domain\n");
+ return -ENODEV;
+ }
+ g->fwnode = of_node_to_fwnode(np);
+ } else {
+ parent = ixp4xx_get_irq_domain();
+ g->fwnode = irq_domain_alloc_fwnode(g->base);
+ if (!g->fwnode) {
+ dev_err(dev, "no domain base\n");
+ return -ENODEV;
+ }
+ }
+
/*
* Make sure GPIO 14 and 15 are NOT used as clocks but GPIO on
* specific machines.
@@ -360,7 +272,6 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
dev_err(dev, "unable to init generic GPIO\n");
return ret;
}
- g->gc.to_irq = ixp4xx_gpio_to_irq;
g->gc.ngpio = 16;
g->gc.label = "IXP4XX_GPIO_CHIP";
/*
@@ -372,86 +283,22 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
g->gc.parent = &pdev->dev;
g->gc.owner = THIS_MODULE;
+ girq = &g->gc.irq;
+ girq->chip = &ixp4xx_gpio_irqchip;
+ girq->fwnode = g->fwnode;
+ girq->parent_domain = parent;
+ girq->child_to_parent_hwirq = ixp4xx_gpio_child_to_parent_hwirq;
+ girq->handler = handle_bad_irq;
+ girq->default_type = IRQ_TYPE_NONE;
+
ret = devm_gpiochip_add_data(dev, &g->gc, g);
if (ret) {
dev_err(dev, "failed to add SoC gpiochip\n");
return ret;
}
- /*
- * When we convert to device tree we will simply look up the
- * parent irqdomain using irq_find_host(parent) as parent comes
- * from IRQCHIP_DECLARE(), then use of_node_to_fwnode() to get
- * the fwnode. For now we need this boardfile style code.
- */
- if (np) {
- struct device_node *irq_parent;
-
- irq_parent = of_irq_find_parent(np);
- if (!irq_parent) {
- dev_err(dev, "no IRQ parent node\n");
- return -ENODEV;
- }
- parent = irq_find_host(irq_parent);
- if (!parent) {
- dev_err(dev, "no IRQ parent domain\n");
- return -ENODEV;
- }
- g->fwnode = of_node_to_fwnode(np);
- } else {
- parent = ixp4xx_get_irq_domain();
- g->fwnode = irq_domain_alloc_fwnode(g->base);
- if (!g->fwnode) {
- dev_err(dev, "no domain base\n");
- return -ENODEV;
- }
- }
- g->domain = irq_domain_create_hierarchy(parent,
- IRQ_DOMAIN_FLAG_HIERARCHY,
- ARRAY_SIZE(ixp4xx_gpiomap),
- g->fwnode,
- &ixp4xx_gpio_irqdomain_ops,
- g);
- if (!g->domain) {
- irq_domain_free_fwnode(g->fwnode);
- dev_err(dev, "no hierarchical irq domain\n");
- return ret;
- }
-
- /*
- * After adding OF support, this is no longer needed: irqs
- * will be allocated for the respective fwnodes.
- */
- if (!np) {
- for (i = 0; i < ARRAY_SIZE(ixp4xx_gpiomap); i++) {
- const struct ixp4xx_gpio_map *map = &ixp4xx_gpiomap[i];
- struct irq_fwspec fwspec;
-
- fwspec.fwnode = g->fwnode;
- /* This is the hwirq for the GPIO line side of things */
- fwspec.param[0] = map->gpio_offset;
- fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
- fwspec.param_count = 2;
- ret = __irq_domain_alloc_irqs(g->domain,
- -1, /* just pick something */
- 1,
- NUMA_NO_NODE,
- &fwspec,
- false,
- NULL);
- if (ret < 0) {
- irq_domain_free_fwnode(g->fwnode);
- dev_err(dev,
- "can not allocate irq for GPIO line %d parent hwirq %d in hierarchy domain: %d\n",
- map->gpio_offset, map->parent_hwirq,
- ret);
- return ret;
- }
- }
- }
-
platform_set_drvdata(pdev, g);
- dev_info(dev, "IXP4 GPIO @%p registered\n", g->base);
+ dev_info(dev, "IXP4 GPIO registered\n");
return 0;
}
--
2.21.0
^ permalink raw reply related
* [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Thomas Gleixner, Marc Zyngier,
Lina Iyer, Jon Hunter, Sowjanya Komatineni, Bitan Biswas,
linux-tegra, David Daney, Masahiro Yamada, Brian Masney,
Thierry Reding
Hierarchical IRQ domains can be used to stack different IRQ
controllers on top of each other.
Bring hierarchical IRQ domains into the GPIOLIB core with the
following basic idea:
Drivers that need their interrupts handled hierarchically
specify a callback to translate the child hardware IRQ and
IRQ type for each GPIO offset to a parent hardware IRQ and
parent hardware IRQ type.
Users have to pass the callback, fwnode, and parent irqdomain
before calling gpiochip_irqchip_add().
We use the new method of just filling in the struct
gpio_irq_chip before adding the gpiochip for all hierarchical
irqchips of this type.
The code path for device tree is pretty straight-forward,
while the code path for old boardfiles or anything else will
be more convoluted requireing upfront allocation of the
interrupts when adding the chip.
One specific use-case where this can be useful is if a power
management controller has top-level controls for wakeup
interrupts. In such cases, the power management controller can
be a parent to other interrupt controllers and program
additional registers when an IRQ has its wake capability
enabled or disabled.
The hierarchical irqchip helper code will only be available
when IRQ_DOMAIN_HIERARCHY is selected to GPIO chips using
this should select or depend on that symbol. When using
hierarchical IRQs, the parent interrupt controller must
also be hierarchical all the way up to the top interrupt
controller wireing directly into the CPU, so on systems
that do not have this we can get rid of all the extra
code for supporting hierarchical irqs.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lina Iyer <ilina@codeaurora.org>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Sowjanya Komatineni <skomatineni@nvidia.com>
Cc: Bitan Biswas <bbiswas@nvidia.com>
Cc: linux-tegra@vger.kernel.org
Cc: David Daney <david.daney@cavium.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Brian Masney <masneyb@onstation.org>
Co-developed-by: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Squash in changes contributed by Brian Masney to support
customization and solve some bugs.
- Add gpiochip_populate_parent_fwspec_{two,four}cell functions
so that irqchips using four cells can use this code.
- Pass girq->handler to the domain as default handler:
there are apparently cases where handle_bad_irq will
not quite cut it.
- Change "child_pin_to_irq" into "child_offset_to_irq" so
we avoid confusion with pin control.
- Diet down the parent domain presence checks, one time
is enough to check it.
- Change domain allocation error to -ENOMEM and do not
print errors.
- Diet some chip_info() blather.
- Assume nr_irqs to be 1 in .translate() and warn if not.
ChangeLog RFC->v1:
- Tested on real hardware
- Incorporate Thierry's idea to have a translation callback.
He was right about this approach, I was wrong in insisting
on IRQ maps.
---
Documentation/driver-api/gpio/driver.rst | 122 ++++++++-
drivers/gpio/gpiolib.c | 318 ++++++++++++++++++++++-
include/linux/gpio/driver.h | 111 ++++++++
3 files changed, 525 insertions(+), 26 deletions(-)
diff --git a/Documentation/driver-api/gpio/driver.rst b/Documentation/driver-api/gpio/driver.rst
index 921c71a3d683..5e96dbc7f4e7 100644
--- a/Documentation/driver-api/gpio/driver.rst
+++ b/Documentation/driver-api/gpio/driver.rst
@@ -259,7 +259,7 @@ most often cascaded off a parent interrupt controller, and in some special
cases the GPIO logic is melded with a SoC's primary interrupt controller.
The IRQ portions of the GPIO block are implemented using an irq_chip, using
-the header <linux/irq.h>. So basically such a driver is utilizing two sub-
+the header <linux/irq.h>. So this combined driver is utilizing two sub-
systems simultaneously: gpio and irq.
It is legal for any IRQ consumer to request an IRQ from any irqchip even if it
@@ -391,25 +391,119 @@ Infrastructure helpers for GPIO irqchips
----------------------------------------
To help out in handling the set-up and management of GPIO irqchips and the
-associated irqdomain and resource allocation callbacks, the gpiolib has
-some helpers that can be enabled by selecting the GPIOLIB_IRQCHIP Kconfig
-symbol:
-
-- gpiochip_irqchip_add(): adds a chained cascaded irqchip to a gpiochip. It
- will pass the struct gpio_chip* for the chip to all IRQ callbacks, so the
- callbacks need to embed the gpio_chip in its state container and obtain a
- pointer to the container using container_of().
- (See Documentation/driver-api/driver-model/design-patterns.rst)
+associated irqdomain and resource allocation callbacks. These are activated
+by selecting the Kconfig symbol GPIOLIB_IRQCHIP. If the symbol
+IRQ_DOMAIN_HIERARCHY is also selected, hierarchical helpers will also be
+provided. A big portion of overhead code will be managed by gpiolib,
+under the assumption that your interrupts are 1-to-1-mapped to the
+GPIO line index:
+
+ GPIO line offset Hardware IRQ
+ 0 0
+ 1 1
+ 2 2
+ ... ...
+ ngpio-1 ngpio-1
+
+If some GPIO lines do not have corresponding IRQs, the bitmask valid_mask
+and the flag need_valid_mask in gpio_irq_chip can be used to mask off some
+lines as invalid for associating with IRQs.
+
+The preferred way to set up the helpers is to fill in the
+struct gpio_irq_chip inside struct gpio_chip before adding the gpio_chip.
+If you do this, the additional irq_chip will be set up by gpiolib at the
+same time as setting up the rest of the GPIO functionality. The following
+is a typical example of a cascaded interrupt handler using gpio_irq_chip:
+
+ /* Typical state container with dynamic irqchip */
+ struct my_gpio {
+ struct gpio_chip gc;
+ struct irq_chip irq;
+ };
+
+ int irq; /* from platform etc */
+ struct my_gpio *g;
+ struct gpio_irq_chip *girq;
+
+ /* Set up the irqchip dynamically */
+ g->irq.name = "my_gpio_irq";
+ g->irq.irq_ack = my_gpio_ack_irq;
+ g->irq.irq_mask = my_gpio_mask_irq;
+ g->irq.irq_unmask = my_gpio_unmask_irq;
+ g->irq.irq_set_type = my_gpio_set_irq_type;
+
+ /* Get a pointer to the gpio_irq_chip */
+ girq = &g->gc.irq;
+ girq->chip = &g->irq;
+ girq->parent_handler = ftgpio_gpio_irq_handler;
+ girq->num_parents = 1;
+ girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents),
+ GFP_KERNEL);
+ if (!girq->parents)
+ return -ENOMEM;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_bad_irq;
+ girq->parents[0] = irq;
+
+ return devm_gpiochip_add_data(dev, &g->gc, g);
+
+The helper support using hierarchical interrupt controllers as well.
+In this case the typical set-up will look like this:
+
+ /* Typical state container with dynamic irqchip */
+ struct my_gpio {
+ struct gpio_chip gc;
+ struct irq_chip irq;
+ struct fwnode_handle *fwnode;
+ };
+
+ int irq; /* from platform etc */
+ struct my_gpio *g;
+ struct gpio_irq_chip *girq;
+
+ /* Set up the irqchip dynamically */
+ g->irq.name = "my_gpio_irq";
+ g->irq.irq_ack = my_gpio_ack_irq;
+ g->irq.irq_mask = my_gpio_mask_irq;
+ g->irq.irq_unmask = my_gpio_unmask_irq;
+ g->irq.irq_set_type = my_gpio_set_irq_type;
+
+ /* Get a pointer to the gpio_irq_chip */
+ girq = &g->gc.irq;
+ girq->chip = &g->irq;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_bad_irq;
+ girq->fwnode = g->fwnode;
+ girq->parent_domain = parent;
+ girq->child_to_parent_hwirq = my_gpio_child_to_parent_hwirq;
+
+ return devm_gpiochip_add_data(dev, &g->gc, g);
+
+As you can see pretty similar, but you do not supply a parent handler for
+the IRQ, instead a parent irqdomain, an fwnode for the hardware and
+a funcion .child_to_parent_hwirq() that has the purpose of looking up
+the parent hardware irq from a child (i.e. this gpio chip) hardware irq.
+As always it is good to look at examples in the kernel tree for advice
+on how to find the required pieces.
+
+The old way of adding irqchips to gpiochips after registration is also still
+available but we try to move away from this:
+
+- DEPRECATED: gpiochip_irqchip_add(): adds a chained cascaded irqchip to a
+ gpiochip. It will pass the struct gpio_chip* for the chip to all IRQ
+ callbacks, so the callbacks need to embed the gpio_chip in its state
+ container and obtain a pointer to the container using container_of().
+ (See Documentation/driver-model/design-patterns.txt)
- gpiochip_irqchip_add_nested(): adds a nested cascaded irqchip to a gpiochip,
as discussed above regarding different types of cascaded irqchips. The
cascaded irq has to be handled by a threaded interrupt handler.
Apart from that it works exactly like the chained irqchip.
-- gpiochip_set_chained_irqchip(): sets up a chained cascaded irq handler for a
- gpio_chip from a parent IRQ and passes the struct gpio_chip* as handler
- data. Notice that we pass is as the handler data, since the irqchip data is
- likely used by the parent irqchip.
+- DEPRECATED: gpiochip_set_chained_irqchip(): sets up a chained cascaded irq
+ handler for a gpio_chip from a parent IRQ and passes the struct gpio_chip*
+ as handler data. Notice that we pass is as the handler data, since the
+ irqchip data is likely used by the parent irqchip.
- gpiochip_set_nested_irqchip(): sets up a nested cascaded irq handler for a
gpio_chip from a parent IRQ. As the parent IRQ has usually been
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 537a37a89891..4a71a58ce47f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1720,6 +1720,273 @@ void gpiochip_set_nested_irqchip(struct gpio_chip *gpiochip,
}
EXPORT_SYMBOL_GPL(gpiochip_set_nested_irqchip);
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+
+/**
+ * gpiochip_set_hierarchical_irqchip() - connects a hierarchical irqchip
+ * to a gpiochip
+ * @gc: the gpiochip to set the irqchip hierarchical handler to
+ * @irqchip: the irqchip to handle this level of the hierarchy, the interrupt
+ * will then percolate up to the parent
+ */
+static void gpiochip_set_hierarchical_irqchip(struct gpio_chip *gc,
+ struct irq_chip *irqchip)
+{
+ /* DT will deal with mapping each IRQ as we go along */
+ if (is_of_node(gc->irq.fwnode))
+ return;
+
+ /*
+ * This is for legacy and boardfile "irqchip" fwnodes: allocate
+ * irqs upfront instead of dynamically since we don't have the
+ * dynamic type of allocation that hardware description languages
+ * provide. Once all GPIO drivers using board files are gone from
+ * the kernel we can delete this code, but for a transitional period
+ * it is necessary to keep this around.
+ */
+ if (is_fwnode_irqchip(gc->irq.fwnode)) {
+ int i;
+ int ret;
+
+ for (i = 0; i < gc->ngpio; i++) {
+ struct irq_fwspec fwspec;
+ unsigned int parent_hwirq;
+ unsigned int parent_type;
+ struct gpio_irq_chip *girq = &gc->irq;
+
+ /*
+ * We call the child to parent translation function
+ * only to check if the child IRQ is valid or not.
+ * Just pick the rising edge type here as that is what
+ * we likely need to support.
+ */
+ ret = girq->child_to_parent_hwirq(gc, i,
+ IRQ_TYPE_EDGE_RISING,
+ &parent_hwirq,
+ &parent_type);
+ if (ret) {
+ chip_err(gc, "skip set-up on hwirq %d\n",
+ i);
+ continue;
+ }
+
+ fwspec.fwnode = gc->irq.fwnode;
+ /* This is the hwirq for the GPIO line side of things */
+ fwspec.param[0] = girq->child_offset_to_irq(gc, i);
+ /* Just pick something */
+ fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
+ fwspec.param_count = 2;
+ ret = __irq_domain_alloc_irqs(gc->irq.domain,
+ /* just pick something */
+ -1,
+ 1,
+ NUMA_NO_NODE,
+ &fwspec,
+ false,
+ NULL);
+ if (ret < 0) {
+ chip_err(gc,
+ "can not allocate irq for GPIO line %d parent hwirq %d in hierarchy domain: %d\n",
+ i, parent_hwirq,
+ ret);
+ }
+ }
+ }
+
+ chip_err(gc, "%s unknown fwnode type proceed anyway\n", __func__);
+
+ return;
+}
+
+static int gpiochip_hierarchy_irq_domain_translate(struct irq_domain *d,
+ struct irq_fwspec *fwspec,
+ unsigned long *hwirq,
+ unsigned int *type)
+{
+ /* We support standard DT translation */
+ if (is_of_node(fwspec->fwnode) && fwspec->param_count == 2) {
+ return irq_domain_translate_twocell(d, fwspec, hwirq, type);
+ }
+
+ /* This is for board files and others not using DT */
+ if (is_fwnode_irqchip(fwspec->fwnode)) {
+ int ret;
+
+ ret = irq_domain_translate_twocell(d, fwspec, hwirq, type);
+ if (ret)
+ return ret;
+ WARN_ON(*type == IRQ_TYPE_NONE);
+ return 0;
+ }
+ return -EINVAL;
+}
+
+static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
+ unsigned int irq,
+ unsigned int nr_irqs,
+ void *data)
+{
+ struct gpio_chip *gc = d->host_data;
+ irq_hw_number_t hwirq;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = data;
+ struct irq_fwspec parent_fwspec;
+ unsigned int parent_hwirq;
+ unsigned int parent_type;
+ struct gpio_irq_chip *girq = &gc->irq;
+ int ret;
+
+ /*
+ * The nr_irqs parameter is always one except for PCI multi-MSI
+ * so this should not happen.
+ */
+ WARN_ON(nr_irqs != 1);
+
+ ret = gc->irq.child_irq_domain_ops.translate(d, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ chip_info(gc, "allocate IRQ %d, hwirq %lu\n", irq, hwirq);
+
+ ret = girq->child_to_parent_hwirq(gc, hwirq, type,
+ &parent_hwirq, &parent_type);
+ if (ret) {
+ chip_err(gc, "can't look up hwirq %lu\n", hwirq);
+ return ret;
+ }
+ chip_info(gc, "found parent hwirq %u\n", parent_hwirq);
+
+ /*
+ * We set handle_bad_irq because the .set_type() should
+ * always be invoked and set the right type of handler.
+ */
+ irq_domain_set_info(d,
+ irq,
+ hwirq,
+ gc->irq.chip,
+ gc,
+ girq->handler,
+ NULL, NULL);
+ irq_set_probe(irq);
+
+ /*
+ * Create a IRQ fwspec to send up to the parent irqdomain:
+ * specify the hwirq we address on the parent and tie it
+ * all together up the chain.
+ */
+ parent_fwspec.fwnode = d->parent->fwnode;
+ /* This parent only handles asserted level IRQs */
+ girq->populate_parent_fwspec(gc, &parent_fwspec, parent_hwirq,
+ parent_type);
+ chip_info(gc, "alloc_irqs_parent for %d parent hwirq %d\n",
+ irq, parent_hwirq);
+ ret = irq_domain_alloc_irqs_parent(d, irq, 1, &parent_fwspec);
+ if (ret)
+ chip_err(gc,
+ "failed to allocate parent hwirq %d for hwirq %lu\n",
+ parent_hwirq, hwirq);
+
+ return ret;
+}
+
+static unsigned int gpiochip_child_offset_to_irq_noop(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ return offset;
+}
+
+static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops)
+{
+ ops->activate = gpiochip_irq_domain_activate;
+ ops->deactivate = gpiochip_irq_domain_deactivate;
+ ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
+ ops->free = irq_domain_free_irqs_common;
+
+ /*
+ * We only allow overriding the translate() function for
+ * hierarchical chips, and this should only be done if the user
+ * really need something other than 1:1 translation.
+ */
+ if (!ops->translate)
+ ops->translate = gpiochip_hierarchy_irq_domain_translate;
+}
+
+static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
+{
+ if (!gc->irq.child_to_parent_hwirq ||
+ !gc->irq.fwnode) {
+ chip_err(gc, "missing irqdomain vital data\n");
+ return -EINVAL;
+ }
+
+ if (!gc->irq.child_offset_to_irq)
+ gc->irq.child_offset_to_irq = gpiochip_child_offset_to_irq_noop;
+
+ if (!gc->irq.populate_parent_fwspec)
+ gc->irq.populate_parent_fwspec =
+ gpiochip_populate_parent_fwspec_twocell;
+
+ gpiochip_hierarchy_setup_domain_ops(&gc->irq.child_irq_domain_ops);
+
+ gc->irq.domain = irq_domain_create_hierarchy(
+ gc->irq.parent_domain,
+ 0,
+ gc->ngpio,
+ gc->irq.fwnode,
+ &gc->irq.child_irq_domain_ops,
+ gc);
+
+ if (!gc->irq.domain)
+ return -ENOMEM;
+
+ gpiochip_set_hierarchical_irqchip(gc, gc->irq.chip);
+
+ return 0;
+}
+
+static bool gpiochip_hierarchy_is_hierarchical(struct gpio_chip *gc)
+{
+ return !!gc->irq.parent_domain;
+}
+
+void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+ struct irq_fwspec *fwspec,
+ unsigned int parent_hwirq,
+ unsigned int parent_type)
+{
+ fwspec->param_count = 2;
+ fwspec->param[0] = parent_hwirq;
+ fwspec->param[1] = parent_type;
+}
+EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_twocell);
+
+void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+ struct irq_fwspec *fwspec,
+ unsigned int parent_hwirq,
+ unsigned int parent_type)
+{
+ fwspec->param_count = 4;
+ fwspec->param[0] = 0;
+ fwspec->param[1] = parent_hwirq;
+ fwspec->param[2] = 0;
+ fwspec->param[3] = parent_type;
+}
+EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_fourcell);
+
+#else
+
+static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
+{
+ return -EINVAL;
+}
+
+static bool gpiochip_hierarchy_is_hierarchical(struct gpio_chip *gc)
+{
+ return false;
+}
+
+#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
+
/**
* gpiochip_irq_map() - maps an IRQ into a GPIO irqchip
* @d: the irqdomain used by this irqchip
@@ -1788,6 +2055,11 @@ static const struct irq_domain_ops gpiochip_domain_ops = {
.xlate = irq_domain_xlate_twocell,
};
+/*
+ * TODO: move these activate/deactivate in under the hierarchicial
+ * irqchip implementation as static once SPMI and SSBI (all external
+ * users) are phased over.
+ */
/**
* gpiochip_irq_domain_activate() - Lock a GPIO to be used as an IRQ
* @domain: The IRQ domain used by this IRQ chip
@@ -1827,10 +2099,23 @@ EXPORT_SYMBOL_GPL(gpiochip_irq_domain_deactivate);
static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
{
+ struct irq_domain *domain = chip->irq.domain;
+
if (!gpiochip_irqchip_irq_valid(chip, offset))
return -ENXIO;
- return irq_create_mapping(chip->irq.domain, offset);
+ if (irq_domain_is_hierarchy(domain)) {
+ struct irq_fwspec spec;
+
+ spec.fwnode = domain->fwnode;
+ spec.param_count = 2;
+ spec.param[0] = chip->irq.child_offset_to_irq(chip, offset);
+ spec.param[1] = IRQ_TYPE_NONE;
+
+ return irq_create_fwspec_mapping(&spec);
+ }
+
+ return irq_create_mapping(domain, offset);
}
static int gpiochip_irq_reqres(struct irq_data *d)
@@ -1907,7 +2192,7 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
struct lock_class_key *request_key)
{
struct irq_chip *irqchip = gpiochip->irq.chip;
- const struct irq_domain_ops *ops;
+ const struct irq_domain_ops *ops = NULL;
struct device_node *np;
unsigned int type;
unsigned int i;
@@ -1943,16 +2228,25 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
gpiochip->irq.lock_key = lock_key;
gpiochip->irq.request_key = request_key;
- if (gpiochip->irq.domain_ops)
- ops = gpiochip->irq.domain_ops;
- else
- ops = &gpiochip_domain_ops;
-
- gpiochip->irq.domain = irq_domain_add_simple(np, gpiochip->ngpio,
- gpiochip->irq.first,
- ops, gpiochip);
- if (!gpiochip->irq.domain)
- return -EINVAL;
+ /* If a parent irqdomain is provided, let's build a hierarchy */
+ if (gpiochip_hierarchy_is_hierarchical(gpiochip)) {
+ int ret = gpiochip_hierarchy_add_domain(gpiochip);
+ if (ret)
+ return ret;
+ } else {
+ /* Some drivers provide custom irqdomain ops */
+ if (gpiochip->irq.domain_ops)
+ ops = gpiochip->irq.domain_ops;
+
+ if (!ops)
+ ops = &gpiochip_domain_ops;
+ gpiochip->irq.domain = irq_domain_add_simple(np,
+ gpiochip->ngpio,
+ gpiochip->irq.first,
+ ops, gpiochip);
+ if (!gpiochip->irq.domain)
+ return -EINVAL;
+ }
if (gpiochip->irq.parent_handler) {
void *data = gpiochip->irq.parent_handler_data ?: gpiochip;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 6a0e420915a3..0e6d3b0c0211 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -23,6 +23,9 @@ enum gpio_lookup_flags;
#ifdef CONFIG_GPIOLIB
#ifdef CONFIG_GPIOLIB_IRQCHIP
+
+struct gpio_chip;
+
/**
* struct gpio_irq_chip - GPIO interrupt controller
*/
@@ -49,6 +52,84 @@ struct gpio_irq_chip {
*/
const struct irq_domain_ops *domain_ops;
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+ /**
+ * @fwnode:
+ *
+ * Firmware node corresponding to this gpiochip/irqchip, necessary
+ * for hierarchical irqdomain support.
+ */
+ struct fwnode_handle *fwnode;
+
+ /**
+ * @parent_domain:
+ *
+ * If non-NULL, will be set as the parent of this GPIO interrupt
+ * controller's IRQ domain to establish a hierarchical interrupt
+ * domain. The presence of this will activate the hierarchical
+ * interrupt support.
+ */
+ struct irq_domain *parent_domain;
+
+ /**
+ * @child_to_parent_hwirq:
+ *
+ * This callback translates a child hardware IRQ offset to a parent
+ * hardware IRQ offset on a hierarchical interrupt chip. The child
+ * hardware IRQs correspond to the GPIO index 0..ngpio-1 (see the
+ * ngpio field of struct gpio_chip) and the corresponding parent
+ * hardware IRQ and type (such as IRQ_TYPE_*) shall be returned by
+ * the driver. The driver can calculate this from an offset or using
+ * a lookup table or whatever method is best for this chip. Return
+ * 0 on successful translation in the driver.
+ *
+ * If some ranges of hardware IRQs do not have a corresponding parent
+ * HWIRQ, return -EINVAL, but also make sure to fill in @valid_mask and
+ * @need_valid_mask to make these GPIO lines unavailable for
+ * translation.
+ */
+ int (*child_to_parent_hwirq)(struct gpio_chip *chip,
+ unsigned int child_hwirq,
+ unsigned int child_type,
+ unsigned int *parent_hwirq,
+ unsigned int *parent_type);
+
+ /**
+ * @populate_parent_fwspec:
+ *
+ * This optional callback populates the &struct irq_fwspec for the
+ * parent's IRQ domain. If this is not specified, then
+ * &gpiochip_populate_parent_fwspec_twocell will be used. A four-cell
+ * variant named &gpiochip_populate_parent_fwspec_fourcell is also
+ * available.
+ */
+ void (*populate_parent_fwspec)(struct gpio_chip *chip,
+ struct irq_fwspec *fwspec,
+ unsigned int parent_hwirq,
+ unsigned int parent_type);
+
+ /**
+ * @child_offset_to_irq:
+ *
+ * This optional callback is used to translate the child's GPIO line
+ * offset on the GPIO chip to an IRQ number for the GPIO to_irq()
+ * callback. If this is not specified, then a default callback will be
+ * provided that returns the line offset.
+ */
+ unsigned int (*child_offset_to_irq)(struct gpio_chip *chip,
+ unsigned int pin);
+
+ /**
+ * @child_irq_domain_ops:
+ *
+ * The IRQ domain operations that will be used for this GPIO IRQ
+ * chip. If no operations are provided, then default callbacks will
+ * be populated to setup the IRQ hierarchy. Some drivers need to
+ * supply their own translate function.
+ */
+ struct irq_domain_ops child_irq_domain_ops;
+#endif
+
/**
* @handler:
*
@@ -449,6 +530,36 @@ struct bgpio_pdata {
int ngpio;
};
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+
+void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+ struct irq_fwspec *fwspec,
+ unsigned int parent_hwirq,
+ unsigned int parent_type);
+void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+ struct irq_fwspec *fwspec,
+ unsigned int parent_hwirq,
+ unsigned int parent_type);
+
+#else
+
+static void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+ struct irq_fwspec *fwspec,
+ unsigned int parent_hwirq,
+ unsigned int parent_type)
+{
+}
+
+static void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+ struct irq_fwspec *fwspec,
+ unsigned int parent_hwirq,
+ unsigned int parent_type)
+{
+}
+
+#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
+
+
#if IS_ENABLED(CONFIG_GPIO_GENERIC)
int bgpio_init(struct gpio_chip *gc, struct device *dev,
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 4/4 v1] RFT: gpio: uniphier: Switch to GPIOLIB_IRQCHIP
From: Linus Walleij @ 2019-08-08 11:57 UTC (permalink / raw)
To: Masahiro Yamada
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Thierry Reding,
Brian Masney
In-Reply-To: <CAK7LNATdYG-POvQQXwEiOD1eYwT081ohqACXV95fU01kfojXjQ@mail.gmail.com>
Hi Masahiro,
thanks for your review!
On Thu, Jul 18, 2019 at 1:10 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> On Mon, Jun 24, 2019 at 10:25 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > -static void uniphier_gpio_irq_unmask(struct irq_data *data)
> > +static void uniphier_gpio_irq_unmask(struct irq_data *d)
>
> Are you renaming 'data' -> 'd'
> just for your personal preference?
Yes, I am still looking for proof of what kind of terseness gives
the optimal perceptive qualities in written code, so I have only
intuitive ideas about what is the easiest code to read and maintain.
But it's your file, and since you seem not to like it I will
back out this change.
> > -static int uniphier_gpio_irq_set_type(struct irq_data *data, unsigned int type)
> > +static int uniphier_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>
> Again, this seems a noise change.
Contrary to popular belief, coding style changes while writing
new code is not universally seen as "noise", actually the opposite,
sending pure code style changes as singular patches, is seen as
noise. See the following paragraph from
Documentation/process/4.Coding.rst:
"pure coding style fixes are seen as noise by the development community;
they tend to get a chilly reception. So this type of patch is best
avoided. It is natural to fix the style of a piece of code while working
on it for other reasons, but coding style changes should not be made for
their own sake."
Given the number of pure coding style patches I get,
one could believe this does not apply ... but I try to be
accepting of it anyway.
> I did not test this patch, but probably it would break my board.
Oh too bad, let's see if we can make it more plausible to
work (I will not apply it while it is in RFT state).
> ->(de)activate hook has offset UNIPHIER_GPIO_IRQ_OFFSET (=120),
> but you are replacing it with generic gpiochip_irq_domain_activate,
> which as zero offset.
Ah! Brian gave me the tool to fix this properly, I will try to
iterate and get this right.
> > - priv->domain = irq_domain_create_hierarchy(
> > - parent_domain, 0,
> > - UNIPHIER_GPIO_IRQ_MAX_NUM,
>
> You are replacing UNIPHIER_GPIO_IRQ_MAX_NUM with gc->ngpio,
> which will much more irqs than needed.
>
> Is it possible to provide more flexibility?
UNIPHIER_GPIO_IRQ_MAX_NUM is 24 and ngpio comes
from the device tree and is compulsory. The current device
trees have:
arch/arm/boot/dts/uniphier-ld4.dtsi: ngpios = <136>;
arch/arm/boot/dts/uniphier-pro4.dtsi: ngpios = <248>;
arch/arm/boot/dts/uniphier-pro5.dtsi: ngpios = <248>;
arch/arm/boot/dts/uniphier-pxs2.dtsi: ngpios = <232>;
arch/arm/boot/dts/uniphier-sld8.dtsi: ngpios = <136>;
So I suppose that you mean that since only 24 GPIOs can
ever have assigned IRQs, making headroom for say 248 is
a waste of resources.
However irq descriptors are dynamically allocated, so saying
that the irqchip can have 24 descriptors rather than 248
is not going to save any memory.
What you might want is to only allow offset 0..23 to be mapped
to irqs. If I understand correctly this is how the hardware works:
the first 24 GPIOs have IRQs, the rest don't, is that right?
We have a facility for that: struct gpio_irq_chip field
.valid_mask
I will try to come up with a separate patch for that so you can
see if it works.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v1 2/3] arm64: dts: fix gpio node
From: Hui Song @ 2019-08-08 10:16 UTC (permalink / raw)
To: Shawn Guo, Li Yang, Rob Herring, Mark Rutland, Linus Walleij,
Bartosz Golaszewski
Cc: linux-arm-kernel, devicetree, linux-kernel, linux-gpio, Song Hui
In-Reply-To: <20190808101628.36782-1-hui.song_1@nxp.com>
From: Song Hui <hui.song_1@nxp.com>
Update the nodes to include little-endian
property to be consistent with the hardware
and add ls1088a gpio specify compatible.
Signed-off-by: Song Hui <hui.song_1@nxp.com>
---
arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index 20f5ebd..d58d203 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -269,43 +269,47 @@
};
gpio0: gpio@2300000 {
- compatible = "fsl,qoriq-gpio";
+ compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
reg = <0x0 0x2300000 0x0 0x10000>;
interrupts = <0 36 IRQ_TYPE_LEVEL_HIGH>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+ little-endian;
};
gpio1: gpio@2310000 {
- compatible = "fsl,qoriq-gpio";
+ compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
reg = <0x0 0x2310000 0x0 0x10000>;
interrupts = <0 36 IRQ_TYPE_LEVEL_HIGH>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+ little-endian;
};
gpio2: gpio@2320000 {
- compatible = "fsl,qoriq-gpio";
+ compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
reg = <0x0 0x2320000 0x0 0x10000>;
interrupts = <0 37 IRQ_TYPE_LEVEL_HIGH>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+ little-endian;
};
gpio3: gpio@2330000 {
- compatible = "fsl,qoriq-gpio";
+ compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
reg = <0x0 0x2330000 0x0 0x10000>;
interrupts = <0 37 IRQ_TYPE_LEVEL_HIGH>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+ little-endian;
};
ifc: ifc@2240000 {
--
2.9.5
^ permalink raw reply related
* [PATCH v1 3/3] gpio: mpc8xxx: add ls1088a platform special function
From: Hui Song @ 2019-08-08 10:16 UTC (permalink / raw)
To: Shawn Guo, Li Yang, Rob Herring, Mark Rutland, Linus Walleij,
Bartosz Golaszewski
Cc: linux-arm-kernel, devicetree, linux-kernel, linux-gpio, Song Hui
In-Reply-To: <20190808101628.36782-1-hui.song_1@nxp.com>
From: Song Hui <hui.song_1@nxp.com>
ls1028a and ls1088a platform share common special function.
The gpio hardware what they use is the same version.
Signed-off-by: Song Hui <hui.song_1@nxp.com>
---
drivers/gpio/gpio-mpc8xxx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index 1a680aa..16a47de 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -319,6 +319,7 @@ static const struct of_device_id mpc8xxx_gpio_ids[] = {
{ .compatible = "fsl,mpc5125-gpio", .data = &mpc5125_gpio_devtype, },
{ .compatible = "fsl,pq3-gpio", },
{ .compatible = "fsl,ls1028a-gpio", .data = &ls1028a_gpio_devtype, },
+ { .compatible = "fsl,ls1088a-gpio", .data = &ls1028a_gpio_devtype, },
{ .compatible = "fsl,qoriq-gpio", },
{}
};
--
2.9.5
^ permalink raw reply related
* [PATCH v1 1/3] gpio: mpc8xxx: add ls1088a platform gpio node DT binding description
From: Hui Song @ 2019-08-08 10:16 UTC (permalink / raw)
To: Shawn Guo, Li Yang, Rob Herring, Mark Rutland, Linus Walleij,
Bartosz Golaszewski
Cc: linux-arm-kernel, devicetree, linux-kernel, linux-gpio, Song Hui
From: Song Hui <hui.song_1@nxp.com>
ls1088a and ls1028a platform share common gpio node.
Signed-off-by: Song Hui <hui.song_1@nxp.com>
---
Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt b/Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt
index baf95d9..cd28e93 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt
@@ -4,7 +4,7 @@ Required properties:
- compatible : Should be "fsl,<soc>-gpio"
The following <soc>s are known to be supported:
mpc5121, mpc5125, mpc8349, mpc8572, mpc8610, pq3, qoriq,
- ls1021a, ls1043a, ls2080a, ls1028a.
+ ls1021a, ls1043a, ls2080a, ls1028a, ls1088a.
- reg : Address and length of the register set for the device
- interrupts : Should be the port interrupt shared by all 32 pins.
- #gpio-cells : Should be two. The first cell is the pin number and
@@ -39,10 +39,10 @@ gpio0: gpio@2300000 {
};
-Example of gpio-controller node for a ls1028a SoC:
+Example of gpio-controller node for a ls1028a/ls1088a SoC:
gpio1: gpio@2300000 {
- compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
+ compatible = "fsl,ls1028a-gpio", "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
reg = <0x0 0x2300000 0x0 0x10000>;
interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
gpio-controller;
--
2.9.5
^ permalink raw reply related
* Re: [PATCH v1] pinctrl: denverton: Update pin names according to v1.08
From: Mika Westerberg @ 2019-08-08 9:39 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-gpio, Linus Walleij
In-Reply-To: <20190731133958.51263-1-andriy.shevchenko@linux.intel.com>
On Wed, Jul 31, 2019 at 04:39:58PM +0300, Andy Shevchenko wrote:
> Version 1.08 of pin list has some changes in pin names for Intel Denverton.
>
> Update the driver accordingly.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v2 0/3] pinctrl: sh-pfc: Rollback to mux if requires when the gpio is freed
From: Geert Uytterhoeven @ 2019-08-08 8:31 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, open list:GPIO SUBSYSTEM,
Linux-Renesas
In-Reply-To: <1565245143-15018-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>
Hi Shimoda-san,
On Thu, Aug 8, 2019 at 8:20 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This patch series is based on renesas-drivers.git /
> renesas-drivers-2019-07-30-v5.3-rc2 tag.
>
> About R-Car PWM driver modification, it seems to need more time to achieve
> output duty zero about suitable gpio vs pinctrl handling. But, I believe
> this pinctrl patches could be applied into the mainline regardless
> the R-Car PWM modification.
Thank you! I plan to apply this next week, after I've sent a first sh-pfc pull
request for v5.4, and let it soak in renesas-drivers.
> Changes from v1:
> - Spin-off the pinctrl patches (from 1/7 to 3/7).
> - Add Geert-san's Reviewed-by on 1/3 and 2/3.
> - Add Fixes tag on 2/3.
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=143129
>
> Yoshihiro Shimoda (3):
> pinctrl: sh-pfc: add new flags into struct sh_pfc_pin_config
> pinctrl: sh-pfc: remove incomplete flag "cfg->type"
> pinctrl: sh-pfc: Rollback to mux if requires when the gpio is freed
>
> drivers/pinctrl/sh-pfc/pinctrl.c | 45 ++++++++++++++++++++--------------------
> 1 file changed, 23 insertions(+), 22 deletions(-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 3/3] pinctrl: sh-pfc: Rollback to mux if requires when the gpio is freed
From: Geert Uytterhoeven @ 2019-08-08 8:29 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, open list:GPIO SUBSYSTEM,
Linux-Renesas
In-Reply-To: <1565245143-15018-4-git-send-email-yoshihiro.shimoda.uh@renesas.com>
On Thu, Aug 8, 2019 at 8:20 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> R-Car PWM controller requires the gpio to output zero duty,
> this patch allows to roll it back from gpio to mux when the gpio
> is freed.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] pinctrl: sprd: Add of_node_put() before return to prevent memory leak
From: Baolin Wang @ 2019-08-08 7:55 UTC (permalink / raw)
To: Nishka Dasgupta
Cc: Linus Walleij, Orson Zhai, Chunyan Zhang,
open list:GPIO SUBSYSTEM
In-Reply-To: <20190808074329.15579-1-nishkadg.linux@gmail.com>
On Thu, 8 Aug 2019 at 15:43, Nishka Dasgupta <nishkadg.linux@gmail.com> wrote:
>
> Each iteration of for_each_child_of_node puts the previous node, but in
> the case of a return from the middle of the loop, there is no put, thus
> causing a memory leak. Hence add an of_node_put before the return in
> two places.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
Yes, thanks for your fix.
Reviewed-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> drivers/pinctrl/sprd/pinctrl-sprd.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/sprd/pinctrl-sprd.c b/drivers/pinctrl/sprd/pinctrl-sprd.c
> index c31b58168772..5abb64da2c0d 100644
> --- a/drivers/pinctrl/sprd/pinctrl-sprd.c
> +++ b/drivers/pinctrl/sprd/pinctrl-sprd.c
> @@ -940,8 +940,10 @@ static int sprd_pinctrl_parse_dt(struct sprd_pinctrl *sprd_pctl)
>
> for_each_child_of_node(np, child) {
> ret = sprd_pinctrl_parse_groups(child, sprd_pctl, grp);
> - if (ret)
> + if (ret) {
> + of_node_put(child);
> return ret;
> + }
>
> *temp++ = grp->name;
> grp++;
> @@ -950,8 +952,11 @@ static int sprd_pinctrl_parse_dt(struct sprd_pinctrl *sprd_pctl)
> for_each_child_of_node(child, sub_child) {
> ret = sprd_pinctrl_parse_groups(sub_child,
> sprd_pctl, grp);
> - if (ret)
> + if (ret) {
> + of_node_put(sub_child);
> + of_node_put(child);
> return ret;
> + }
>
> *temp++ = grp->name;
> grp++;
> --
> 2.19.1
>
--
Baolin Wang
Best Regards
^ permalink raw reply
* [PATCH] pinctrl: stm32: stm32: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-08 7:54 UTC (permalink / raw)
To: linus.walleij, mcoquelin.stm32, alexandre.torgue, linux-gpio,
linux-stm32, linux-arm-kernel
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node and
for_each_available_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return in
two places.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/stm32/pinctrl-stm32.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index b453aed1bbeb..2d5e0435af0a 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -615,6 +615,7 @@ static int stm32_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
&reserved_maps, num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map, *num_maps);
+ of_node_put(np);
return ret;
}
}
@@ -1468,8 +1469,10 @@ int stm32_pctl_probe(struct platform_device *pdev)
for_each_available_child_of_node(np, child) {
if (of_property_read_bool(child, "gpio-controller")) {
ret = stm32_gpiolib_register_bank(pctl, child);
- if (ret)
+ if (ret) {
+ of_node_put(child);
return ret;
+ }
pctl->nbanks++;
}
--
2.19.1
^ permalink raw reply related
* [PATCH] pinctrl: freescale: imx: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-08 7:47 UTC (permalink / raw)
To: aisheng.dong, festevam, shawnguo, stefan, kernel, linus.walleij,
s.hauer, linux-imx, linux-gpio, linux-arm-kernel
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node() puts the previous node;
however, in the case of a return from the middle of the loop, there is no
put, thus causing a memory leak. Hence put of_node_put() statements as
required before two mid-loop return statements.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/freescale/pinctrl-imx.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 83ff9532bae6..9f42036c5fbb 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -672,8 +672,10 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
grp = devm_kzalloc(ipctl->dev, sizeof(struct group_desc),
GFP_KERNEL);
- if (!grp)
+ if (!grp) {
+ of_node_put(child);
return -ENOMEM;
+ }
mutex_lock(&ipctl->mutex);
radix_tree_insert(&pctl->pin_group_tree,
@@ -697,12 +699,17 @@ static bool imx_pinctrl_dt_is_flat_functions(struct device_node *np)
struct device_node *pinctrl_np;
for_each_child_of_node(np, function_np) {
- if (of_property_read_bool(function_np, "fsl,pins"))
+ if (of_property_read_bool(function_np, "fsl,pins")) {
+ of_node_put(function_np);
return true;
+ }
for_each_child_of_node(function_np, pinctrl_np) {
- if (of_property_read_bool(pinctrl_np, "fsl,pins"))
+ if (of_property_read_bool(pinctrl_np, "fsl,pins")) {
+ of_node_put(pinctrl_np);
+ of_node_put(function_np);
return false;
+ }
}
}
--
2.19.1
^ permalink raw reply related
* [PATCH] pinctrl: sprd: Add of_node_put() before return to prevent memory leak
From: Nishka Dasgupta @ 2019-08-08 7:43 UTC (permalink / raw)
To: linus.walleij, orsonzhai, baolin.wang, zhang.lyra, linux-gpio
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return in
two places.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/sprd/pinctrl-sprd.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/sprd/pinctrl-sprd.c b/drivers/pinctrl/sprd/pinctrl-sprd.c
index c31b58168772..5abb64da2c0d 100644
--- a/drivers/pinctrl/sprd/pinctrl-sprd.c
+++ b/drivers/pinctrl/sprd/pinctrl-sprd.c
@@ -940,8 +940,10 @@ static int sprd_pinctrl_parse_dt(struct sprd_pinctrl *sprd_pctl)
for_each_child_of_node(np, child) {
ret = sprd_pinctrl_parse_groups(child, sprd_pctl, grp);
- if (ret)
+ if (ret) {
+ of_node_put(child);
return ret;
+ }
*temp++ = grp->name;
grp++;
@@ -950,8 +952,11 @@ static int sprd_pinctrl_parse_dt(struct sprd_pinctrl *sprd_pctl)
for_each_child_of_node(child, sub_child) {
ret = sprd_pinctrl_parse_groups(sub_child,
sprd_pctl, grp);
- if (ret)
+ if (ret) {
+ of_node_put(sub_child);
+ of_node_put(child);
return ret;
+ }
*temp++ = grp->name;
grp++;
--
2.19.1
^ permalink raw reply related
* Re: [PATCH RFC 6/7] pwm: rcar: Add gpio support to output duty zero
From: Uwe Kleine-König @ 2019-08-08 7:31 UTC (permalink / raw)
To: Yoshihiro Shimoda, linus.walleij@linaro.org
Cc: geert+renesas@glider.be, thierry.reding@gmail.com,
robh+dt@kernel.org, mark.rutland@arm.com,
linux-gpio@vger.kernel.org, linux-pwm@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <TYAPR01MB45445D854C1FDBB473A89559D8D70@TYAPR01MB4544.jpnprd01.prod.outlook.com>
Hello,
On Thu, Aug 08, 2019 at 03:52:52AM +0000, Yoshihiro Shimoda wrote:
> > From: Uwe Kleine-König, Sent: Wednesday, August 7, 2019 4:03 PM
> > On Mon, Jul 08, 2019 at 06:07:47PM +0900, Yoshihiro Shimoda wrote:
> > > @@ -119,8 +121,11 @@ static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns,
> > > ph = tmp & RCAR_PWMCNT_PH0_MASK;
> > >
> > > /* Avoid prohibited setting */
> > > - if (cyc == 0 || ph == 0)
> > > + if (cyc == 0)
> > > return -EINVAL;
> > > + /* Try to use GPIO to output duty zero */
> > > + if (ph == 0)
> > > + return -EAGAIN;
> >
> > If there is no gpio requesting cyc=0 should still yield an error.
>
> I'm sorry, I cannot understand this.
I meant that if getting the gpio failed but it's needed to yield the
right level this should result in an error. I thought I removed that
comment because this is taken care of further below.
> > > rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
> > >
> > > @@ -157,6 +162,28 @@ static void rcar_pwm_disable(struct rcar_pwm_chip *rp)
> > > rcar_pwm_update(rp, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
> > > }
> > >
> > > +static int rcar_pwm_gpiod_get(struct rcar_pwm_chip *rp)
> > > +{
> > > + if (rp->gpio)
> > > + return 0;
> > > +
> > > + rp->gpio = gpiod_get(rp->chip.dev, "renesas,duty-zero", GPIOD_OUT_LOW);
> > > + if (!IS_ERR(rp->gpio))
> > > + return 0;
> > > +
> > > + rp->gpio = NULL;
> > > + return -EINVAL;
> >
> > Does getting the gpio automatically switch the pinmuxing?
> >
> > If yes, this is IMHO a really surprising mis-feature of the gpio
> > subsystem. I'd prefer to "get" the gpio at probe time and only switch
> > the pinmuxing in .apply(). This makes .apply() quicker, ensures that all
> > resources necessary for pwm operation are available, handles
> > -EPROBE_DEFER (and maybe other errors) correctly.
>
> The current pinctrl subsystem only has .set_mux(). I checked the pinctrl subsystem
> history and the commit 2243a87d90b42eb38bc281957df3e57c712b5e56 removed the ".disable()" ops.
> So, IIUC, we cannot such a handling.
You'd need to reactivate the pwm setting when changing from 0% to
something bigger of course.
But as I understand it "getting" the gpio already implies that it is
muxed which is bad here. So it would be great if we could opt-out.
Linus?
The pwm-imx driver has a similar problem[1] where we already considered
using a gpio. There auto-muxing would be in the way, too. (Maybe it
isn't because it isn't implmented on i.MX?)
> > Note you're introducing a bug here because switching to gpio doesn't
> > ensure that the currently running period is completed.
>
> Umm, the hardware doesn't have such a condition so that the driver cannot manage it.
> So, I'll add this into the "Limitations" too.
yes please.
> > > +static void rcar_pwm_gpiod_put(struct rcar_pwm_chip *rp)
> > > +{
> > > + if (!rp->gpio)
> > > + return;
> > > +
> > > + gpiod_put(rp->gpio);
> > > + rp->gpio = NULL;
> > > +}
> > > +
> > > static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > > struct pwm_state *state)
> > > {
> > > @@ -171,6 +198,7 @@ static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > >
> > > if (!state->enabled) {
> > > rcar_pwm_disable(rp);
> > > + rcar_pwm_gpiod_put(rp);
> >
> > From the framework's POV disabling a PWM is quite similar to duty cycle
> > 0. Assuming disabling the PWM completes the currently running period[1]
> > it might be better and easier to disable instead of switching to gpio.
> > (Further assuming that disable really yields the inactive level which is
> > should and is another limitation if not.)
>
> If we disable the hardware, the duty cycle is 100% unfortunately. So,
> I think I should describe it as one of "Limitations".
It seems you got the idea .. :-)
Best regards
Uwe
[1] it goes to 0 on disable even if the polarity is inverted
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* RE: [PATCH RFC 6/7] pwm: rcar: Add gpio support to output duty zero
From: Yoshihiro Shimoda @ 2019-08-08 7:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Uwe Kleine-König, linus.walleij@linaro.org,
geert+renesas@glider.be, thierry.reding@gmail.com,
robh+dt@kernel.org, mark.rutland@arm.com,
linux-gpio@vger.kernel.org, linux-pwm@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <CAMuHMdUQpTvwk=PxhwJGbW8izBXSzHw0sNvypzONPovR7sZutA@mail.gmail.com>
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Thursday, August 8, 2019 3:49 PM
>
> Hi Shimoda-san,
>
> On Thu, Aug 8, 2019 at 5:53 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Uwe Kleine-König, Sent: Wednesday, August 7, 2019 4:03 PM
> > > While at it: If there is a publicly available reference manual adding a line:
> > >
> > > Reference Manual: https://...
> > >
> > > would be great, too.
> >
> > Unfortunately, the document is not public...
>
> RZ/G1 has the same hardware block, right?
> Its Hardware User's Manual is publicly available, e.g. for RZ/G1M:
> https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz/rzg/rzg1m.html#documents
You're right.
# Since I could get RZ/G2 series hardware user's manual from public website yesterday,
# I was thinking any manuals are not public...
Best regards,
Yoshihiro Shimoda
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH RFC 6/7] pwm: rcar: Add gpio support to output duty zero
From: Geert Uytterhoeven @ 2019-08-08 6:49 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Uwe Kleine-König, linus.walleij@linaro.org,
geert+renesas@glider.be, thierry.reding@gmail.com,
robh+dt@kernel.org, mark.rutland@arm.com,
linux-gpio@vger.kernel.org, linux-pwm@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <TYAPR01MB45445D854C1FDBB473A89559D8D70@TYAPR01MB4544.jpnprd01.prod.outlook.com>
Hi Shimoda-san,
On Thu, Aug 8, 2019 at 5:53 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Uwe Kleine-König, Sent: Wednesday, August 7, 2019 4:03 PM
> > While at it: If there is a publicly available reference manual adding a line:
> >
> > Reference Manual: https://...
> >
> > would be great, too.
>
> Unfortunately, the document is not public...
RZ/G1 has the same hardware block, right?
Its Hardware User's Manual is publicly available, e.g. for RZ/G1M:
https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz/rzg/rzg1m.html#documents
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox