* [PATCH] pinctrl: bcm-iproc: Use SPDX header
From: Linus Walleij @ 2019-08-12 13:04 UTC (permalink / raw)
To: linux-gpio; +Cc: Linus Walleij, Pramod Kumar, Ray Jui, Scott Branden
This convert the BCM IPROC driver to use the SPDX header
for indicating GPL v2.0 only licensing.
Cc: Pramod Kumar <pramodku@broadcom.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index b70058caee50..18ff01727e0e 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -1,17 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (C) 2014-2017 Broadcom
*
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation version 2.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-/*
* This file contains the Broadcom Iproc GPIO driver that supports 3
* GPIO controllers on Iproc including the ASIU GPIO controller, the
* chipCommonG GPIO controller, and the always-on GPIO controller. Basic
--
2.21.0
^ permalink raw reply related
* [PATCH] gpio: ep93xx: Pass irqchip when adding gpiochip
From: Linus Walleij @ 2019-08-12 13:00 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Alexander Sverdlin,
H Hartley Sweeten, Thierry Reding
We need to convert all old gpio irqchips to pass the irqchip
setup along when adding the gpio_chip. For more info see
drivers/gpio/TODO.
For chained irqchips this is a pretty straight-forward
conversion.
Cc: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-ep93xx.c | 140 +++++++++++++++++++------------------
1 file changed, 73 insertions(+), 67 deletions(-)
diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index a90870a60c15..226da8df6f10 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -269,56 +269,6 @@ static struct irq_chip ep93xx_gpio_irq_chip = {
.irq_set_type = ep93xx_gpio_irq_type,
};
-static int ep93xx_gpio_init_irq(struct platform_device *pdev,
- struct ep93xx_gpio *epg)
-{
- int ab_parent_irq = platform_get_irq(pdev, 0);
- struct device *dev = &pdev->dev;
- int gpio_irq;
- int ret;
- int i;
-
- /* The A bank */
- ret = gpiochip_irqchip_add(&epg->gc[0], &ep93xx_gpio_irq_chip,
- 64, handle_level_irq,
- IRQ_TYPE_NONE);
- if (ret) {
- dev_err(dev, "Could not add irqchip 0\n");
- return ret;
- }
- gpiochip_set_chained_irqchip(&epg->gc[0], &ep93xx_gpio_irq_chip,
- ab_parent_irq,
- ep93xx_gpio_ab_irq_handler);
-
- /* The B bank */
- ret = gpiochip_irqchip_add(&epg->gc[1], &ep93xx_gpio_irq_chip,
- 72, handle_level_irq,
- IRQ_TYPE_NONE);
- if (ret) {
- dev_err(dev, "Could not add irqchip 1\n");
- return ret;
- }
- gpiochip_set_chained_irqchip(&epg->gc[1], &ep93xx_gpio_irq_chip,
- ab_parent_irq,
- ep93xx_gpio_ab_irq_handler);
-
- /* The F bank */
- for (i = 0; i < 8; i++) {
- gpio_irq = EP93XX_GPIO_F_IRQ_BASE + i;
- irq_set_chip_data(gpio_irq, &epg->gc[5]);
- irq_set_chip_and_handler(gpio_irq, &ep93xx_gpio_irq_chip,
- handle_level_irq);
- irq_clear_status_flags(gpio_irq, IRQ_NOREQUEST);
- }
-
- for (i = 1; i <= 8; i++)
- irq_set_chained_handler_and_data(platform_get_irq(pdev, i),
- ep93xx_gpio_f_irq_handler,
- &epg->gc[i]);
- return 0;
-}
-
-
/*************************************************************************
* gpiolib interface for EP93xx on-chip GPIOs
*************************************************************************/
@@ -328,26 +278,33 @@ struct ep93xx_gpio_bank {
int dir;
int base;
bool has_irq;
+ bool has_hierarchical_irq;
+ unsigned int irq_base;
};
-#define EP93XX_GPIO_BANK(_label, _data, _dir, _base, _has_irq) \
+#define EP93XX_GPIO_BANK(_label, _data, _dir, _base, _has_irq, _has_hier, _irq_base) \
{ \
.label = _label, \
.data = _data, \
.dir = _dir, \
.base = _base, \
.has_irq = _has_irq, \
+ .has_hierarchical_irq = _has_hier, \
+ .irq_base = _irq_base, \
}
static struct ep93xx_gpio_bank ep93xx_gpio_banks[] = {
- EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true), /* Bank A has 8 IRQs */
- EP93XX_GPIO_BANK("B", 0x04, 0x14, 8, true), /* Bank B has 8 IRQs */
- EP93XX_GPIO_BANK("C", 0x08, 0x18, 40, false),
- EP93XX_GPIO_BANK("D", 0x0c, 0x1c, 24, false),
- EP93XX_GPIO_BANK("E", 0x20, 0x24, 32, false),
- EP93XX_GPIO_BANK("F", 0x30, 0x34, 16, true), /* Bank F has 8 IRQs */
- EP93XX_GPIO_BANK("G", 0x38, 0x3c, 48, false),
- EP93XX_GPIO_BANK("H", 0x40, 0x44, 56, false),
+ /* Bank A has 8 IRQs */
+ EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true, false, 64),
+ /* Bank B has 8 IRQs */
+ EP93XX_GPIO_BANK("B", 0x04, 0x14, 8, true, false, 72),
+ EP93XX_GPIO_BANK("C", 0x08, 0x18, 40, false, false, 0),
+ EP93XX_GPIO_BANK("D", 0x0c, 0x1c, 24, false, false, 0),
+ EP93XX_GPIO_BANK("E", 0x20, 0x24, 32, false, false, 0),
+ /* Bank F has 8 IRQs */
+ EP93XX_GPIO_BANK("F", 0x30, 0x34, 16, false, true, 0),
+ EP93XX_GPIO_BANK("G", 0x38, 0x3c, 48, false, false, 0),
+ EP93XX_GPIO_BANK("H", 0x40, 0x44, 56, false, false, 0),
};
static int ep93xx_gpio_set_config(struct gpio_chip *gc, unsigned offset,
@@ -369,12 +326,15 @@ static int ep93xx_gpio_f_to_irq(struct gpio_chip *gc, unsigned offset)
return EP93XX_GPIO_F_IRQ_BASE + offset;
}
-static int ep93xx_gpio_add_bank(struct gpio_chip *gc, struct device *dev,
+static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
+ struct platform_device *pdev,
struct ep93xx_gpio *epg,
struct ep93xx_gpio_bank *bank)
{
void __iomem *data = epg->base + bank->data;
void __iomem *dir = epg->base + bank->dir;
+ struct device *dev = &pdev->dev;
+ struct gpio_irq_chip *girq;
int err;
err = bgpio_init(gc, dev, 1, data, NULL, NULL, dir, NULL, 0);
@@ -384,8 +344,59 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc, struct device *dev,
gc->label = bank->label;
gc->base = bank->base;
- if (bank->has_irq)
+ girq = &gc->irq;
+ if (bank->has_irq || bank->has_hierarchical_irq) {
gc->set_config = ep93xx_gpio_set_config;
+ girq->chip = &ep93xx_gpio_irq_chip;
+ }
+
+ if (bank->has_irq) {
+ int ab_parent_irq = platform_get_irq(pdev, 0);
+
+ girq->parent_handler = ep93xx_gpio_ab_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_level_irq;
+ girq->parents[0] = ab_parent_irq;
+ girq->first = bank->irq_base;
+ }
+
+ /* Only bank F has especially funky IRQ handling */
+ if (bank->has_hierarchical_irq) {
+ int gpio_irq;
+ int i;
+
+ /*
+ * FIXME: convert this to use hierarchical IRQ support!
+ * this requires fixing the root irqchip to be hierarchial.
+ */
+ girq->parent_handler = ep93xx_gpio_f_irq_handler;
+ girq->num_parents = 8;
+ girq->parents = devm_kcalloc(dev, 8,
+ sizeof(*girq->parents),
+ GFP_KERNEL);
+ if (!girq->parents)
+ return -ENOMEM;
+ /* Pick resources 1..8 for these IRQs */
+ for (i = 1; i <= 8; i++)
+ girq->parents[i - 1] = platform_get_irq(pdev, i);
+ for (i = 0; i < 8; i++) {
+ gpio_irq = EP93XX_GPIO_F_IRQ_BASE + i;
+ irq_set_chip_data(gpio_irq, &epg->gc[5]);
+ irq_set_chip_and_handler(gpio_irq,
+ &ep93xx_gpio_irq_chip,
+ handle_level_irq);
+ irq_clear_status_flags(gpio_irq, IRQ_NOREQUEST);
+ }
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_level_irq;
+ gc->to_irq = ep93xx_gpio_f_to_irq;
+ }
return devm_gpiochip_add_data(dev, gc, epg);
}
@@ -407,16 +418,11 @@ static int ep93xx_gpio_probe(struct platform_device *pdev)
struct gpio_chip *gc = &epg->gc[i];
struct ep93xx_gpio_bank *bank = &ep93xx_gpio_banks[i];
- if (ep93xx_gpio_add_bank(gc, &pdev->dev, epg, bank))
+ if (ep93xx_gpio_add_bank(gc, pdev, epg, bank))
dev_warn(&pdev->dev, "Unable to add gpio bank %s\n",
bank->label);
- /* Only bank F has especially funky IRQ handling */
- if (i == 5)
- gc->to_irq = ep93xx_gpio_f_to_irq;
}
- ep93xx_gpio_init_irq(pdev, epg);
-
return 0;
}
--
2.21.0
^ permalink raw reply related
* [PATCH 2/2] irq/irq_sim: use irq domain
From: Bartosz Golaszewski @ 2019-08-12 12:52 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Thomas Gleixner,
Marc Zyngier
Cc: linux-gpio, linux-kernel, linux-iio, Bartosz Golaszewski
In-Reply-To: <20190812125256.9690-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We currently have a dedicated function to map the interrupt simulator
offsets to global interrupt numbers. This is something that irq_domain
should handle.
Create a linear irq_domain when initializing the interrupt simulator
and modify the irq_sim_fire() function to only take as parameter the
global interrupt number.
Convert both users in the same patch to using the new interface.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/gpio/gpio-mockup.c | 11 ++-
drivers/iio/dummy/iio_dummy_evgen.c | 22 ++++--
include/linux/irq_sim.h | 5 +-
kernel/irq/Kconfig | 1 +
kernel/irq/irq_sim.c | 110 +++++++++++++++++-----------
5 files changed, 94 insertions(+), 55 deletions(-)
diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index 9b28ffec5826..4cf594f0e7cd 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -186,7 +186,7 @@ static int gpio_mockup_to_irq(struct gpio_chip *gc, unsigned int offset)
{
struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
- return irq_sim_irqnum(chip->irqsim, offset);
+ return irq_create_mapping(irq_sim_get_domain(chip->irqsim), offset);
}
static void gpio_mockup_free(struct gpio_chip *gc, unsigned int offset)
@@ -228,6 +228,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file,
struct gpio_mockup_dbgfs_private *priv;
int rv, val, curr, irq, irq_type;
struct gpio_mockup_chip *chip;
+ struct irq_domain *domain;
struct seq_file *sfile;
struct gpio_desc *desc;
struct gpio_chip *gc;
@@ -248,6 +249,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file,
gc = &chip->gc;
desc = &gc->gpiodev->descs[priv->offset];
sim = chip->irqsim;
+ domain = irq_sim_get_domain(sim);
mutex_lock(&chip->lock);
@@ -257,12 +259,15 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file,
if (curr == val)
goto out;
- irq = irq_sim_irqnum(sim, priv->offset);
+ irq = irq_find_mapping(domain, priv->offset);
+ if (!irq)
+ return -ENOENT;
+
irq_type = irq_get_trigger_type(irq);
if ((val == 1 && (irq_type & IRQ_TYPE_EDGE_RISING)) ||
(val == 0 && (irq_type & IRQ_TYPE_EDGE_FALLING)))
- irq_sim_fire(sim, priv->offset);
+ irq_sim_fire(irq);
}
/* Change the value unless we're actively driving the line. */
diff --git a/drivers/iio/dummy/iio_dummy_evgen.c b/drivers/iio/dummy/iio_dummy_evgen.c
index efbcd4a5609e..cc827f60a535 100644
--- a/drivers/iio/dummy/iio_dummy_evgen.c
+++ b/drivers/iio/dummy/iio_dummy_evgen.c
@@ -31,14 +31,13 @@
* @lock: protect the evgen state
* @inuse: mask of which irqs are connected
* @irq_sim: interrupt simulator
- * @base: base of irq range
*/
struct iio_dummy_eventgen {
struct iio_dummy_regs regs[IIO_EVENTGEN_NO];
struct mutex lock;
bool inuse[IIO_EVENTGEN_NO];
struct irq_sim *irq_sim;
- int base;
+ struct irq_domain *domain;
};
/* We can only ever have one instance of this 'device' */
@@ -56,7 +55,7 @@ static int iio_dummy_evgen_create(void)
return PTR_ERR(iio_evgen->irq_sim);
}
- iio_evgen->base = irq_sim_irqnum(iio_evgen->irq_sim, 0);
+ iio_evgen->domain = irq_sim_get_domain(iio_evgen->irq_sim);
mutex_init(&iio_evgen->lock);
return 0;
@@ -78,7 +77,7 @@ int iio_dummy_evgen_get_irq(void)
mutex_lock(&iio_evgen->lock);
for (i = 0; i < IIO_EVENTGEN_NO; i++) {
if (!iio_evgen->inuse[i]) {
- ret = irq_sim_irqnum(iio_evgen->irq_sim, i);
+ ret = irq_create_mapping(iio_evgen->domain, i);
iio_evgen->inuse[i] = true;
break;
}
@@ -99,15 +98,21 @@ EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_irq);
*/
void iio_dummy_evgen_release_irq(int irq)
{
+ struct irq_data *irqd;
+
+ irqd = irq_get_irq_data(irq);
+
mutex_lock(&iio_evgen->lock);
- iio_evgen->inuse[irq - iio_evgen->base] = false;
+ iio_evgen->inuse[irqd_to_hwirq(irqd)] = false;
mutex_unlock(&iio_evgen->lock);
}
EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq);
struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq)
{
- return &iio_evgen->regs[irq - iio_evgen->base];
+ struct irq_data *irqd = irq_get_irq_data(irq);
+
+ return &iio_evgen->regs[irqd_to_hwirq(irqd)];
}
EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs);
@@ -129,7 +134,7 @@ static ssize_t iio_evgen_poke(struct device *dev,
{
struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
unsigned long event;
- int ret;
+ int ret, irq;
ret = kstrtoul(buf, 10, &event);
if (ret)
@@ -138,7 +143,8 @@ static ssize_t iio_evgen_poke(struct device *dev,
iio_evgen->regs[this_attr->address].reg_id = this_attr->address;
iio_evgen->regs[this_attr->address].reg_data = event;
- irq_sim_fire(iio_evgen->irq_sim, this_attr->address);
+ irq = irq_create_mapping(iio_evgen->domain, this_attr->address);
+ irq_sim_fire(irq);
return len;
}
diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h
index 4bbf036145e2..4056d0e7f0b4 100644
--- a/include/linux/irq_sim.h
+++ b/include/linux/irq_sim.h
@@ -7,6 +7,7 @@
#define _LINUX_IRQ_SIM_H
#include <linux/irq_work.h>
+#include <linux/irqdomain.h>
#include <linux/device.h>
/*
@@ -19,7 +20,7 @@ struct irq_sim;
struct irq_sim *irq_sim_new(unsigned int num_irqs);
struct irq_sim *devm_irq_sim_new(struct device *dev, unsigned int num_irqs);
void irq_sim_free(struct irq_sim *sim);
-void irq_sim_fire(struct irq_sim *sim, unsigned int offset);
-int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset);
+void irq_sim_fire(int virq);
+struct irq_domain *irq_sim_get_domain(struct irq_sim *sim);
#endif /* _LINUX_IRQ_SIM_H */
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index f92d9a687372..d0890f7729d4 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -68,6 +68,7 @@ config IRQ_DOMAIN
config IRQ_SIM
bool
select IRQ_WORK
+ select IRQ_DOMAIN
# Support for hierarchical irq domains
config IRQ_DOMAIN_HIERARCHY
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index 79f0a6494b6c..a1c91aefb6cd 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -15,13 +15,14 @@ struct irq_sim_work_ctx {
struct irq_sim_irq_ctx {
int irqnum;
bool enabled;
+ struct irq_sim_work_ctx *work_ctx;
};
struct irq_sim {
struct irq_sim_work_ctx work_ctx;
int irq_base;
unsigned int irq_count;
- struct irq_sim_irq_ctx *irqs;
+ struct irq_domain *domain;
};
struct irq_sim_devres {
@@ -74,11 +75,46 @@ static void irq_sim_handle_irq(struct irq_work *work)
offset = find_next_bit(work_ctx->pending,
sim->irq_count, offset);
clear_bit(offset, work_ctx->pending);
- irqnum = irq_sim_irqnum(sim, offset);
+ irqnum = irq_find_mapping(sim->domain, offset);
handle_simple_irq(irq_to_desc(irqnum));
}
}
+static int irq_sim_domain_map(struct irq_domain *domain,
+ unsigned int virq, irq_hw_number_t hw)
+{
+ struct irq_sim *sim = domain->host_data;
+ struct irq_sim_irq_ctx *ctx;
+
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ irq_set_chip(virq, &irq_sim_irqchip);
+ irq_set_chip_data(virq, ctx);
+ irq_set_handler(virq, handle_simple_irq);
+ irq_modify_status(virq, IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
+ ctx->work_ctx = &sim->work_ctx;
+
+ return 0;
+}
+
+static void irq_sim_domain_unmap(struct irq_domain *domain, unsigned int virq)
+{
+ struct irq_sim_irq_ctx *ctx;
+ struct irq_data *irqd;
+
+ irqd = irq_domain_get_irq_data(domain, virq);
+ ctx = irq_data_get_irq_chip_data(irqd);
+
+ kfree(ctx);
+}
+
+static const struct irq_domain_ops irq_sim_domain_ops = {
+ .map = irq_sim_domain_map,
+ .unmap = irq_sim_domain_unmap,
+};
+
/**
* irq_sim_new - Create a new interrupt simulator: allocate a range of
* dummy interrupts.
@@ -91,42 +127,21 @@ static void irq_sim_handle_irq(struct irq_work *work)
struct irq_sim *irq_sim_new(unsigned int num_irqs)
{
struct irq_sim *sim;
- int i;
sim = kmalloc(sizeof(*sim), GFP_KERNEL);
if (!sim)
return ERR_PTR(-ENOMEM);
- sim->irqs = kmalloc_array(num_irqs, sizeof(*sim->irqs), GFP_KERNEL);
- if (!sim->irqs) {
- kfree(sim);
- return ERR_PTR(-ENOMEM);
- }
-
- sim->irq_base = irq_alloc_descs(-1, 0, num_irqs, 0);
- if (sim->irq_base < 0) {
- kfree(sim->irqs);
- kfree(sim);
- return ERR_PTR(sim->irq_base);
- }
-
sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
if (!sim->work_ctx.pending) {
- kfree(sim->irqs);
kfree(sim);
- irq_free_descs(sim->irq_base, num_irqs);
return ERR_PTR(-ENOMEM);
}
- for (i = 0; i < num_irqs; i++) {
- sim->irqs[i].irqnum = sim->irq_base + i;
- sim->irqs[i].enabled = false;
- irq_set_chip(sim->irq_base + i, &irq_sim_irqchip);
- irq_set_chip_data(sim->irq_base + i, &sim->irqs[i]);
- irq_set_handler(sim->irq_base + i, &handle_simple_irq);
- irq_modify_status(sim->irq_base + i,
- IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
- }
+ sim->domain = irq_domain_create_linear(NULL, num_irqs,
+ &irq_sim_domain_ops, sim);
+ if (!sim->domain)
+ return ERR_PTR(-ENOMEM);
init_irq_work(&sim->work_ctx.work, irq_sim_handle_irq);
sim->irq_count = num_irqs;
@@ -143,10 +158,17 @@ EXPORT_SYMBOL_GPL(irq_sim_new);
*/
void irq_sim_free(struct irq_sim *sim)
{
+ int i, irq;
+
+ for (i = 0; i < sim->irq_count; i++) {
+ irq = irq_find_mapping(sim->domain, i);
+ if (irq)
+ irq_dispose_mapping(irq);
+ }
+
+ irq_domain_remove(sim->domain);
irq_work_sync(&sim->work_ctx.work);
bitmap_free(sim->work_ctx.pending);
- irq_free_descs(sim->irq_base, sim->irq_count);
- kfree(sim->irqs);
kfree(sim);
}
EXPORT_SYMBOL_GPL(irq_sim_free);
@@ -189,27 +211,31 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_new);
/**
* irq_sim_fire - Enqueue an interrupt.
*
- * @sim: The interrupt simulator object.
- * @offset: Offset of the simulated interrupt which should be fired.
+ * @virq: Virtual interrupt number to fire. It must be associated with
+ * an existing interrupt simulator.
*/
-void irq_sim_fire(struct irq_sim *sim, unsigned int offset)
+void irq_sim_fire(int virq)
{
- if (sim->irqs[offset].enabled) {
- set_bit(offset, sim->work_ctx.pending);
- irq_work_queue(&sim->work_ctx.work);
+ struct irq_sim_irq_ctx *ctx;
+ struct irq_data *irqd;
+
+ irqd = irq_get_irq_data(virq);
+ ctx = irq_data_get_irq_chip_data(irqd);
+
+ if (ctx->enabled) {
+ set_bit(irqd_to_hwirq(irqd), ctx->work_ctx->pending);
+ irq_work_queue(&ctx->work_ctx->work);
}
}
EXPORT_SYMBOL_GPL(irq_sim_fire);
/**
- * irq_sim_irqnum - Get the allocated number of a dummy interrupt.
+ * irq_sim_get_domain - Retrieve the interrupt domain for this simulator.
*
- * @sim: The interrupt simulator object.
- * @offset: Offset of the simulated interrupt for which to retrieve
- * the number.
+ * @sim: The interrupt simulator the domain of which we retrieve.
*/
-int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset)
+struct irq_domain *irq_sim_get_domain(struct irq_sim *sim)
{
- return sim->irqs[offset].irqnum;
+ return sim->domain;
}
-EXPORT_SYMBOL_GPL(irq_sim_irqnum);
+EXPORT_SYMBOL(irq_sim_get_domain);
--
2.21.0
^ permalink raw reply related
* [PATCH 1/2] irq/irq_sim: make the irq_sim structure opaque
From: Bartosz Golaszewski @ 2019-08-12 12:52 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Thomas Gleixner,
Marc Zyngier
Cc: linux-gpio, linux-kernel, linux-iio, Bartosz Golaszewski
In-Reply-To: <20190812125256.9690-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
There's no good reason to export the interrupt simulator structure to
users and have them provide memory for it. Let's make all the related
data structures opaque and convert both users. This way we have a lot
less APIs exposed in the header.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/gpio/gpio-mockup.c | 12 ++---
drivers/iio/dummy/iio_dummy_evgen.c | 18 +++----
include/linux/irq_sim.h | 24 ++-------
kernel/irq/irq_sim.c | 83 ++++++++++++++++++-----------
4 files changed, 70 insertions(+), 67 deletions(-)
diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index f1a9c0544e3f..9b28ffec5826 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -53,7 +53,7 @@ struct gpio_mockup_line_status {
struct gpio_mockup_chip {
struct gpio_chip gc;
struct gpio_mockup_line_status *lines;
- struct irq_sim irqsim;
+ struct irq_sim *irqsim;
struct dentry *dbg_dir;
struct mutex lock;
};
@@ -186,7 +186,7 @@ static int gpio_mockup_to_irq(struct gpio_chip *gc, unsigned int offset)
{
struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
- return irq_sim_irqnum(&chip->irqsim, offset);
+ return irq_sim_irqnum(chip->irqsim, offset);
}
static void gpio_mockup_free(struct gpio_chip *gc, unsigned int offset)
@@ -247,7 +247,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file,
chip = priv->chip;
gc = &chip->gc;
desc = &gc->gpiodev->descs[priv->offset];
- sim = &chip->irqsim;
+ sim = chip->irqsim;
mutex_lock(&chip->lock);
@@ -431,9 +431,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
return rv;
}
- rv = devm_irq_sim_init(dev, &chip->irqsim, gc->ngpio);
- if (rv < 0)
- return rv;
+ chip->irqsim = devm_irq_sim_new(dev, gc->ngpio);
+ if (IS_ERR(chip->irqsim))
+ return PTR_ERR(chip->irqsim);
rv = devm_gpiochip_add_data(dev, &chip->gc, chip);
if (rv)
diff --git a/drivers/iio/dummy/iio_dummy_evgen.c b/drivers/iio/dummy/iio_dummy_evgen.c
index a6edf30567aa..efbcd4a5609e 100644
--- a/drivers/iio/dummy/iio_dummy_evgen.c
+++ b/drivers/iio/dummy/iio_dummy_evgen.c
@@ -37,7 +37,7 @@ struct iio_dummy_eventgen {
struct iio_dummy_regs regs[IIO_EVENTGEN_NO];
struct mutex lock;
bool inuse[IIO_EVENTGEN_NO];
- struct irq_sim irq_sim;
+ struct irq_sim *irq_sim;
int base;
};
@@ -46,19 +46,17 @@ static struct iio_dummy_eventgen *iio_evgen;
static int iio_dummy_evgen_create(void)
{
- int ret;
-
iio_evgen = kzalloc(sizeof(*iio_evgen), GFP_KERNEL);
if (!iio_evgen)
return -ENOMEM;
- ret = irq_sim_init(&iio_evgen->irq_sim, IIO_EVENTGEN_NO);
- if (ret < 0) {
+ iio_evgen->irq_sim = irq_sim_new(IIO_EVENTGEN_NO);
+ if (IS_ERR(iio_evgen->irq_sim)) {
kfree(iio_evgen);
- return ret;
+ return PTR_ERR(iio_evgen->irq_sim);
}
- iio_evgen->base = irq_sim_irqnum(&iio_evgen->irq_sim, 0);
+ iio_evgen->base = irq_sim_irqnum(iio_evgen->irq_sim, 0);
mutex_init(&iio_evgen->lock);
return 0;
@@ -80,7 +78,7 @@ int iio_dummy_evgen_get_irq(void)
mutex_lock(&iio_evgen->lock);
for (i = 0; i < IIO_EVENTGEN_NO; i++) {
if (!iio_evgen->inuse[i]) {
- ret = irq_sim_irqnum(&iio_evgen->irq_sim, i);
+ ret = irq_sim_irqnum(iio_evgen->irq_sim, i);
iio_evgen->inuse[i] = true;
break;
}
@@ -115,7 +113,7 @@ EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs);
static void iio_dummy_evgen_free(void)
{
- irq_sim_fini(&iio_evgen->irq_sim);
+ irq_sim_free(iio_evgen->irq_sim);
kfree(iio_evgen);
}
@@ -140,7 +138,7 @@ static ssize_t iio_evgen_poke(struct device *dev,
iio_evgen->regs[this_attr->address].reg_id = this_attr->address;
iio_evgen->regs[this_attr->address].reg_data = event;
- irq_sim_fire(&iio_evgen->irq_sim, this_attr->address);
+ irq_sim_fire(iio_evgen->irq_sim, this_attr->address);
return len;
}
diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h
index 4500d453a63e..4bbf036145e2 100644
--- a/include/linux/irq_sim.h
+++ b/include/linux/irq_sim.h
@@ -14,27 +14,11 @@
* requested like normal irqs and enqueued from process context.
*/
-struct irq_sim_work_ctx {
- struct irq_work work;
- unsigned long *pending;
-};
+struct irq_sim;
-struct irq_sim_irq_ctx {
- int irqnum;
- bool enabled;
-};
-
-struct irq_sim {
- struct irq_sim_work_ctx work_ctx;
- int irq_base;
- unsigned int irq_count;
- struct irq_sim_irq_ctx *irqs;
-};
-
-int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs);
-int devm_irq_sim_init(struct device *dev, struct irq_sim *sim,
- unsigned int num_irqs);
-void irq_sim_fini(struct irq_sim *sim);
+struct irq_sim *irq_sim_new(unsigned int num_irqs);
+struct irq_sim *devm_irq_sim_new(struct device *dev, unsigned int num_irqs);
+void irq_sim_free(struct irq_sim *sim);
void irq_sim_fire(struct irq_sim *sim, unsigned int offset);
int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset);
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index b992f88c5613..79f0a6494b6c 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -7,6 +7,23 @@
#include <linux/irq_sim.h>
#include <linux/irq.h>
+struct irq_sim_work_ctx {
+ struct irq_work work;
+ unsigned long *pending;
+};
+
+struct irq_sim_irq_ctx {
+ int irqnum;
+ bool enabled;
+};
+
+struct irq_sim {
+ struct irq_sim_work_ctx work_ctx;
+ int irq_base;
+ unsigned int irq_count;
+ struct irq_sim_irq_ctx *irqs;
+};
+
struct irq_sim_devres {
struct irq_sim *sim;
};
@@ -63,34 +80,42 @@ static void irq_sim_handle_irq(struct irq_work *work)
}
/**
- * irq_sim_init - Initialize the interrupt simulator: allocate a range of
- * dummy interrupts.
+ * irq_sim_new - Create a new interrupt simulator: allocate a range of
+ * dummy interrupts.
*
- * @sim: The interrupt simulator object to initialize.
* @num_irqs: Number of interrupts to allocate
*
- * On success: return the base of the allocated interrupt range.
- * On failure: a negative errno.
+ * On success: return the new irq_sim object.
+ * On failure: a negative errno wrapped with ERR_PTR().
*/
-int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)
+struct irq_sim *irq_sim_new(unsigned int num_irqs)
{
+ struct irq_sim *sim;
int i;
+ sim = kmalloc(sizeof(*sim), GFP_KERNEL);
+ if (!sim)
+ return ERR_PTR(-ENOMEM);
+
sim->irqs = kmalloc_array(num_irqs, sizeof(*sim->irqs), GFP_KERNEL);
- if (!sim->irqs)
- return -ENOMEM;
+ if (!sim->irqs) {
+ kfree(sim);
+ return ERR_PTR(-ENOMEM);
+ }
sim->irq_base = irq_alloc_descs(-1, 0, num_irqs, 0);
if (sim->irq_base < 0) {
kfree(sim->irqs);
- return sim->irq_base;
+ kfree(sim);
+ return ERR_PTR(sim->irq_base);
}
sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
if (!sim->work_ctx.pending) {
kfree(sim->irqs);
+ kfree(sim);
irq_free_descs(sim->irq_base, num_irqs);
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
}
for (i = 0; i < num_irqs; i++) {
@@ -106,64 +131,60 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)
init_irq_work(&sim->work_ctx.work, irq_sim_handle_irq);
sim->irq_count = num_irqs;
- return sim->irq_base;
+ return sim;
}
-EXPORT_SYMBOL_GPL(irq_sim_init);
+EXPORT_SYMBOL_GPL(irq_sim_new);
/**
- * irq_sim_fini - Deinitialize the interrupt simulator: free the interrupt
+ * irq_sim_free - Deinitialize the interrupt simulator: free the interrupt
* descriptors and allocated memory.
*
* @sim: The interrupt simulator to tear down.
*/
-void irq_sim_fini(struct irq_sim *sim)
+void irq_sim_free(struct irq_sim *sim)
{
irq_work_sync(&sim->work_ctx.work);
bitmap_free(sim->work_ctx.pending);
irq_free_descs(sim->irq_base, sim->irq_count);
kfree(sim->irqs);
+ kfree(sim);
}
-EXPORT_SYMBOL_GPL(irq_sim_fini);
+EXPORT_SYMBOL_GPL(irq_sim_free);
static void devm_irq_sim_release(struct device *dev, void *res)
{
struct irq_sim_devres *this = res;
- irq_sim_fini(this->sim);
+ irq_sim_free(this->sim);
}
/**
- * irq_sim_init - Initialize the interrupt simulator for a managed device.
+ * devm_irq_sim_new - Create a new interrupt simulator for a managed device.
*
* @dev: Device to initialize the simulator object for.
- * @sim: The interrupt simulator object to initialize.
* @num_irqs: Number of interrupts to allocate
*
- * On success: return the base of the allocated interrupt range.
- * On failure: a negative errno.
+ * On success: return a new irq_sim object.
+ * On failure: a negative errno wrapped with ERR_PTR().
*/
-int devm_irq_sim_init(struct device *dev, struct irq_sim *sim,
- unsigned int num_irqs)
+struct irq_sim *devm_irq_sim_new(struct device *dev, unsigned int num_irqs)
{
struct irq_sim_devres *dr;
- int rv;
dr = devres_alloc(devm_irq_sim_release, sizeof(*dr), GFP_KERNEL);
if (!dr)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
- rv = irq_sim_init(sim, num_irqs);
- if (rv < 0) {
+ dr->sim = irq_sim_new(num_irqs);
+ if (IS_ERR(dr->sim)) {
devres_free(dr);
- return rv;
+ return dr->sim;
}
- dr->sim = sim;
devres_add(dev, dr);
-
- return rv;
+ return dr->sim;
}
-EXPORT_SYMBOL_GPL(devm_irq_sim_init);
+EXPORT_SYMBOL_GPL(devm_irq_sim_new);
/**
* irq_sim_fire - Enqueue an interrupt.
--
2.21.0
^ permalink raw reply related
* [PATCH 0/2] irq/irq_sim: try to improve the API
From: Bartosz Golaszewski @ 2019-08-12 12:52 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Thomas Gleixner,
Marc Zyngier
Cc: linux-gpio, linux-kernel, linux-iio, Bartosz Golaszewski
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Currently the interrupt simulator exposes structures that don't need
to be public and has a helper that manually maps the simulator's irq
offsets to the global interrupt numberspace - something that should
be preferably handles by an irq_domain.
The first patch addresses the public structures: it moves them into
the relevant .c file and makes the init function return an opaque
pointer.
The second patch adds a linear irq_domain to the simulator and removes
the irq_sim_irqnum() routine. Users should now use the standard
irq_domain functions.
Both users of the irq_sim are converted at the same time as it's much
easier than trying to transition them step by step.
Tested both the gpio-mockup module as well as the iio_dummy_evgen.
Bartosz Golaszewski (2):
irq/irq_sim: make the irq_sim structure opaque
irq/irq_sim: use irq domain
drivers/gpio/gpio-mockup.c | 21 ++--
drivers/iio/dummy/iio_dummy_evgen.c | 34 +++---
include/linux/irq_sim.h | 29 ++---
kernel/irq/Kconfig | 1 +
kernel/irq/irq_sim.c | 177 ++++++++++++++++++----------
5 files changed, 152 insertions(+), 110 deletions(-)
--
2.21.0
^ permalink raw reply
* Re: [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Andy Shevchenko @ 2019-08-12 12:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Stefan Roese, open list:SERIAL DRIVERS, open list:GPIO SUBSYSTEM,
Pavel Machek, Linus Walleij, Greg Kroah-Hartman, Mika Westerberg
In-Reply-To: <CAMuHMdXs=AdURCgB1GWRT=huKbXX7rGn6=upi1MMuFDHhKqX0g@mail.gmail.com>
On Mon, Aug 12, 2019 at 02:11:42PM +0200, Geert Uytterhoeven wrote:
> On Mon, Aug 12, 2019 at 1:54 PM Stefan Roese <sr@denx.de> wrote:
> > On 12.08.19 13:17, Geert Uytterhoeven wrote:
> > If we all agree, that only the documented "-gpios" variant needs to
> > be supported, then we can drop this patch.
>
> BTW, I'm still wondering if d99482673f950817 ("serial: mctrl_gpio: Check
> if GPIO property exisits before requesting it") is the right fix.
> This is a place where we rely explicitly on named GPIOs being present, so
> IMHO the ACPI code should not return a random GPIO instead.
ACPI is not taking it to consideration for now. That is, there is no support of
mctrl gpio for ACPI.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Geert Uytterhoeven @ 2019-08-12 12:11 UTC (permalink / raw)
To: Stefan Roese
Cc: open list:SERIAL DRIVERS, open list:GPIO SUBSYSTEM,
Andy Shevchenko, Pavel Machek, Linus Walleij, Greg Kroah-Hartman,
Mika Westerberg
In-Reply-To: <ad77e973-912f-5ff3-9dd4-610695ec57eb@denx.de>
Hi Stefan,
On Mon, Aug 12, 2019 at 1:54 PM Stefan Roese <sr@denx.de> wrote:
> On 12.08.19 13:17, Geert Uytterhoeven wrote:
> > On Thu, Aug 8, 2019 at 3:25 PM Stefan Roese <sr@denx.de> 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.
> >>
> >> Signed-off-by: Stefan Roese <sr@denx.de>
> >
> > Do we really need to spread this *-gpio" legacy support all over the kernel?
> >
> > Seeing the only in-kernel users of legacy "rts-gpio" are
> >
> > arch/arm/boot/dts/am335x-nano.dts: rts-gpio = <&gpio0 13
> > GPIO_ACTIVE_HIGH>;
> > arch/arm/boot/dts/am335x-nano.dts: rts-gpio = <&gpio2 15
> > GPIO_ACTIVE_HIGH>;
> > arch/arm/boot/dts/am335x-pdu001.dts: rts-gpio = <&gpio1 9
> > GPIO_ACTIVE_HIGH>;
> >
> > and this is handled by omap-serial.c, predating mctrl_gpio, I'd like to
> > reconsider.
> >
> > Documentation/devicetree/bindings/serial/serial.txt always described
> > the "*-gpios"
> > variants, so there should be no users of the legacy "*-gpio" variants.
>
> Hmmm, you were the one to comment about supporting (not breaking) the
> "-gpio" variant:
>
> https://lkml.org/lkml/2019/6/24/248
>
> That was my main motivation to work on this patch.
I know. That's why I wrote "reconsider". Before, I assumed there were actual
users of the legacy properties, which turned out to be wrong.
IMHO no driver should be aware there can be multiple possible suffixes.
Sorry for the trouble.
> If we all agree, that only the documented "-gpios" variant needs to
> be supported, then we can drop this patch.
BTW, I'm still wondering if d99482673f950817 ("serial: mctrl_gpio: Check
if GPIO property exisits before requesting it") is the right fix.
This is a place where we rely explicitly on named GPIOs being present, so
IMHO the ACPI code should not return a random GPIO instead.
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 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Stefan Roese @ 2019-08-12 11:53 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:SERIAL DRIVERS, open list:GPIO SUBSYSTEM,
Andy Shevchenko, Pavel Machek, Linus Walleij, Greg Kroah-Hartman
In-Reply-To: <CAMuHMdUzry6f_AqcjevgSRgJ2Q8Nqr_kEyYz+1QEVft6BTrC2g@mail.gmail.com>
Hi Geert,
On 12.08.19 13:17, Geert Uytterhoeven wrote:
> Hi Stefan,
>
> On Thu, Aug 8, 2019 at 3:25 PM Stefan Roese <sr@denx.de> 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.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>
> Do we really need to spread this *-gpio" legacy support all over the kernel?
>
> Seeing the only in-kernel users of legacy "rts-gpio" are
>
> arch/arm/boot/dts/am335x-nano.dts: rts-gpio = <&gpio0 13
> GPIO_ACTIVE_HIGH>;
> arch/arm/boot/dts/am335x-nano.dts: rts-gpio = <&gpio2 15
> GPIO_ACTIVE_HIGH>;
> arch/arm/boot/dts/am335x-pdu001.dts: rts-gpio = <&gpio1 9
> GPIO_ACTIVE_HIGH>;
>
> and this is handled by omap-serial.c, predating mctrl_gpio, I'd like to
> reconsider.
>
> Documentation/devicetree/bindings/serial/serial.txt always described
> the "*-gpios"
> variants, so there should be no users of the legacy "*-gpio" variants.
Hmmm, you were the one to comment about supporting (not breaking) the
"-gpio" variant:
https://lkml.org/lkml/2019/6/24/248
That was my main motivation to work on this patch.
If we all agree, that only the documented "-gpios" variant needs to
be supported, then we can drop this patch.
Thanks,
Stefan
^ permalink raw reply
* [PATCH] gpio: pl061: Fix the issue failed to register the ACPI interruption
From: Wei Xu @ 2019-08-12 11:28 UTC (permalink / raw)
To: linux-gpio, linux-kernel, linux-arm-kernel@lists.infradead.org,
linus.walleij
Cc: Linuxarm, xuwei5, Shameerali Kolothum Thodi, Jonathan Cameron,
John Garry, Salil Mehta, Shiju Jose, jinying, Zhangyi ac,
Liguozhu (Kenneth), Tangkunshan, huangdaode
Invoke acpi_gpiochip_request_interrupts after the acpi data has been
attached to the pl061 acpi node to register interruption.
Otherwise it will be failed to register interruption for the ACPI case.
Because in the gpiochip_add_data_with_key, acpi_gpiochip_add is invoked
after gpiochip_add_irqchip but at that time the acpi data has not been
attached yet.
Tested with below steps on QEMU v4.1.0-rc3 and Linux kernel v5.3-rc4,
and found pl061 interruption is missed in the /proc/interrupts:
1.
qemu-system-aarch64 \
-machine virt,gic-version=3 -cpu cortex-a57 \
-m 1G,maxmem=4G,slots=4 \
-kernel Image -initrd rootfs.cpio.gz \
-net none -nographic \
-bios QEMU_EFI.fd \
-append "console=ttyAMA0 acpi=force earlycon=pl011,0x9000000"
2. cat /proc/interrupts in the guest console:
estuary:/$ cat /proc/interrupts
CPU0
2: 3228 GICv3 27 Level arch_timer
4: 15 GICv3 33 Level uart-pl011
42: 0 GICv3 23 Level arm-pmu
IPI0: 0 Rescheduling interrupts
IPI1: 0 Function call interrupts
IPI2: 0 CPU stop interrupts
IPI3: 0 CPU stop (for crash dump) interrupts
IPI4: 0 Timer broadcast interrupts
IPI5: 0 IRQ work interrupts
IPI6: 0 CPU wake-up interrupts
Err: 0
Fixes: 04ce935c6b2a ("gpio: pl061: Pass irqchip when adding gpiochip")
Signed-off-by: Wei Xu <xuwei5@hisilicon.com>
---
drivers/gpio/gpio-pl061.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 722ce5c..e1a434e 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -8,6 +8,7 @@
*
* Data sheet: ARM DDI 0190B, September 2000
*/
+#include <linux/acpi.h>
#include <linux/spinlock.h>
#include <linux/errno.h>
#include <linux/init.h>
@@ -24,6 +25,9 @@
#include <linux/pinctrl/consumer.h>
#include <linux/pm.h>
+#include "gpiolib.h"
+#include "gpiolib-acpi.h"
+
#define GPIODIR 0x400
#define GPIOIS 0x404
#define GPIOIBE 0x408
@@ -345,6 +349,9 @@ static int pl061_probe(struct amba_device *adev,
const struct amba_id *id)
if (ret)
return ret;
+ if (has_acpi_companion(dev))
+ acpi_gpiochip_request_interrupts(&pl061->gc);
+
amba_set_drvdata(adev, pl061);
dev_info(dev, "PL061 GPIO chip registered\n");
--
2.8.1
.
^ permalink raw reply related
* Re: [PATCH 1/2] gpiolib: Add for_each_gpio_suffix() helper
From: Geert Uytterhoeven @ 2019-08-12 11:18 UTC (permalink / raw)
To: Linus Walleij
Cc: Stefan Roese, open list:SERIAL DRIVERS, open list:GPIO SUBSYSTEM,
Andy Shevchenko, Pavel Machek, Greg Kroah-Hartman
In-Reply-To: <CACRpkdYzg0At4qf1Nv5_+SzgqQ-iLU1ND9Svhj47=pXJf9E7Mg@mail.gmail.com>
Hi Linus,
On Sat, Aug 10, 2019 at 10:27 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Aug 8, 2019 at 3:25 PM Stefan Roese <sr@denx.de> 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.
> >
> > 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>
>
> I really like this patch, it makes things so much more readable.
Do we really need to spread this *-gpio" legacy support all over the kernel?
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 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Geert Uytterhoeven @ 2019-08-12 11:17 UTC (permalink / raw)
To: Stefan Roese
Cc: open list:SERIAL DRIVERS, open list:GPIO SUBSYSTEM,
Andy Shevchenko, Pavel Machek, Linus Walleij, Greg Kroah-Hartman
In-Reply-To: <20190808132543.26274-2-sr@denx.de>
Hi Stefan,
On Thu, Aug 8, 2019 at 3:25 PM Stefan Roese <sr@denx.de> 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.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
Do we really need to spread this *-gpio" legacy support all over the kernel?
Seeing the only in-kernel users of legacy "rts-gpio" are
arch/arm/boot/dts/am335x-nano.dts: rts-gpio = <&gpio0 13
GPIO_ACTIVE_HIGH>;
arch/arm/boot/dts/am335x-nano.dts: rts-gpio = <&gpio2 15
GPIO_ACTIVE_HIGH>;
arch/arm/boot/dts/am335x-pdu001.dts: rts-gpio = <&gpio1 9
GPIO_ACTIVE_HIGH>;
and this is handled by omap-serial.c, predating mctrl_gpio, I'd like to
reconsider.
Documentation/devicetree/bindings/serial/serial.txt always described
the "*-gpios"
variants, so there should be no users of the legacy "*-gpio" variants.
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] gpio: intel-mid: Pass irqchip when adding gpiochip
From: Andy Shevchenko @ 2019-08-12 11:05 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio, Bartosz Golaszewski, Mika Westerberg, David Cohen,
Thierry Reding
In-Reply-To: <20190811080539.15647-1-linus.walleij@linaro.org>
On Sun, Aug 11, 2019 at 10:05:39AM +0200, Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
>
> For chained irqchips this is a pretty straight-forward
> conversion.
>
Same comments as per lynxpoint.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] gpio: merrifield: Pass irqchip when adding gpiochip
From: Andy Shevchenko @ 2019-08-12 11:00 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio, Bartosz Golaszewski, Mika Westerberg, David Cohen,
Thierry Reding
In-Reply-To: <20190812082331.22674-1-linus.walleij@linaro.org>
On Mon, Aug 12, 2019 at 10:23:31AM +0200, Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
>
> For chained irqchips this is a pretty straight-forward
> conversion.
>
Same comment as per lynxpoint patch.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] gpio: lynxpoint: Pass irqchip when adding gpiochip
From: Andy Shevchenko @ 2019-08-12 10:58 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio, Bartosz Golaszewski, Mika Westerberg, David Cohen,
Thierry Reding
In-Reply-To: <20190812081351.21284-1-linus.walleij@linaro.org>
On Mon, Aug 12, 2019 at 10:13:51AM +0200, Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
>
> For chained irqchips this is a pretty straight-forward
> conversion.
>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: David Cohen <david.a.cohen@linux.intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Andy: when you're happy with this you can either supply an
> ACK and I will merge it or you can merge it into your tree
> for a later pull request, just tell me what you prefer.
I'll take through my tree.
See comment below.
> + girq->num_parents = 1;
> + girq->parents = devm_kcalloc(&pdev->dev, 1,
> + sizeof(*girq->parents),
> + GFP_KERNEL);
> + if (!girq->parents)
> + return -ENOMEM;
I understand the point to use kcalloc() for one entry, though I would make
intention more explicitly, i.e. use girq->num_parents in it instead of hard
coded value.
> + girq->parents[0] = (unsigned)irq_rc->start;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_simple_irq;
> - ret = gpiochip_irqchip_add(gc, &lp_irqchip, 0,
> - handle_simple_irq, IRQ_TYPE_NONE);
Hmm... Now I'm wondering, shall we use handle_bad_irq() here?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Andy Shevchenko @ 2019-08-12 10:53 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, linux-gpio, Geert Uytterhoeven, Pavel Machek,
Linus Walleij, Greg Kroah-Hartman
In-Reply-To: <c4d14b64-6c2f-7e87-ea45-aa780dca85b8@denx.de>
On Thu, Aug 08, 2019 at 03:59:36PM +0200, Stefan Roese wrote:
> On 08.08.19 15:48, Andy Shevchenko wrote:
> > 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?
>
> I've come up with this assumption from this code (identical comment):
>
> https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpiolib-of.c#L293
>
> (and other places in drivers/gpio/*)
I tried hard to find an evidence of this in Linux kernel, I assume that comes
from DT compiler or something, but fail. Linux kernel OF properties handling is
written in the assumption of arbitrary length of the property name.
It might be that my hard was not hard at all and I missed something.
> > Or maybe we can still continue using kasprintf() approach?
>
> Frankly, I was feeling a bit uncomfortable with this memory allocation
> in a loop. And Pavel also commented on this:
>
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2066286.html
If memory allocator fails, it's a big issue, and what will happen next probably
much less important.
> So I would really prefer to move this buffer to the stack instead.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v8 14/21] clk: tegra210: Add suspend and resume support
From: Thierry Reding @ 2019-08-12 10:17 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-15-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 991 bytes --]
On Thu, Aug 08, 2019 at 04:46:53PM -0700, Sowjanya Komatineni wrote:
> This patch adds support for clk: tegra210: suspend-resume.
>
> All the CAR controller settings are lost on suspend when core
> power goes off.
>
> This patch has implementation for saving and restoring all PLLs
> and clocks context during system suspend and resume to have the
> clocks back to same state for normal operation.
>
> Clock driver suspend and resume are registered as syscore_ops as clocks
> restore need to happen before the other drivers resume to have all their
> clocks back to the same state as before suspend.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
> drivers/clk/tegra/clk.c | 64 ++++++++++++++++++++++++
> drivers/clk/tegra/clk.h | 3 ++
> 3 files changed, 166 insertions(+), 4 deletions(-)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] dt-bindings: aspeed: Remove mention of deprecated compatibles
From: Lee Jones @ 2019-08-12 10:15 UTC (permalink / raw)
To: Linus Walleij
Cc: Andrew Jeffery, linux-aspeed, Rob Herring, Mark Rutland,
Joel Stanley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM
In-Reply-To: <CACRpkdZCJWeZO6CFvkq4uhnX+o_q_AfkDZ=a2kmUgbS3JtDqfA@mail.gmail.com>
On Mon, 05 Aug 2019, Linus Walleij wrote:
> On Wed, Jul 24, 2019 at 10:13 AM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> > Guide readers away from using the aspeed,g[45].* compatible patterns.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>
> Patch applied to the pinctrl tree.
With my Ack?
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v8 12/21] cpufreq: tegra124: Add suspend and resume support
From: Thierry Reding @ 2019-08-12 10:07 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-13-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]
On Thu, Aug 08, 2019 at 04:46:51PM -0700, Sowjanya Komatineni wrote:
> This patch adds suspend and resume pm ops for cpufreq driver.
>
> PLLP is the safe clock source for CPU during system suspend and
> resume as PLLP rate is below the CPU Fmax at Vmin.
>
> CPUFreq driver suspend switches the CPU clock source to PLLP and
> disables the DFLL clock.
>
> During system resume, warmboot code powers up the CPU with PLLP
> clock source. So CPUFreq driver resume enabled DFLL clock and
> switches CPU back to DFLL clock source.
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/cpufreq/tegra124-cpufreq.c | 60 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> index 4f0c637b3b49..e979a3370988 100644
> --- a/drivers/cpufreq/tegra124-cpufreq.c
> +++ b/drivers/cpufreq/tegra124-cpufreq.c
> @@ -6,6 +6,7 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/clk.h>
> +#include <linux/cpufreq.h>
> #include <linux/err.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> @@ -128,8 +129,67 @@ static int tegra124_cpufreq_probe(struct platform_device *pdev)
> return ret;
> }
>
> +static int __maybe_unused tegra124_cpufreq_suspend(struct device *dev)
> +{
> + struct tegra124_cpufreq_priv *priv = dev_get_drvdata(dev);
> + int err;
> +
> + /*
> + * PLLP rate 408Mhz is below the CPU Fmax at Vmin and is safe to
> + * use during suspend and resume. So, switch the CPU clock source
> + * to PLLP and disable DFLL.
> + */
> + err = clk_set_parent(priv->cpu_clk, priv->pllp_clk);
> + if (err < 0) {
> + dev_err(dev, "failed to reparent to PLLP: %d\n", err);
> + return err;
> + }
> +
> + /* disable DFLL clock */
> + clk_disable_unprepare(priv->dfll_clk);
This comment is superfluous since it doesn't explain anything that the
code below doesn't explain already.
Not sure who will end up merging this. If not me, then this is:
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v8 11/21] clk: tegra: clk-dfll: Add suspend and resume support
From: Thierry Reding @ 2019-08-12 10:01 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-12-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 2446 bytes --]
On Thu, Aug 08, 2019 at 04:46:50PM -0700, Sowjanya Komatineni wrote:
> This patch implements DFLL suspend and resume operation.
>
> During system suspend entry, CPU clock will switch CPU to safe
> clock source of PLLP and disables DFLL clock output.
>
> DFLL driver suspend confirms DFLL disable state and errors out on
> being active.
>
> DFLL is re-initialized during the DFLL driver resume as it goes
> through complete reset during suspend entry.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-dfll.c | 56 ++++++++++++++++++++++++++++++
> drivers/clk/tegra/clk-dfll.h | 2 ++
> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 1 +
> 3 files changed, 59 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> index f8688c2ddf1a..eb298a5d7be9 100644
> --- a/drivers/clk/tegra/clk-dfll.c
> +++ b/drivers/clk/tegra/clk-dfll.c
> @@ -1487,6 +1487,7 @@ static int dfll_init(struct tegra_dfll *td)
> td->last_unrounded_rate = 0;
>
> pm_runtime_enable(td->dev);
> + pm_runtime_irq_safe(td->dev);
> pm_runtime_get_sync(td->dev);
>
> dfll_set_mode(td, DFLL_DISABLED);
> @@ -1513,6 +1514,61 @@ static int dfll_init(struct tegra_dfll *td)
> return ret;
> }
>
> +/**
> + * tegra_dfll_suspend - check DFLL is disabled
> + * @dev: DFLL device *
> + *
> + * DFLL clock should be disabled by the CPUFreq driver. So, make
> + * sure it is disabled and disable all clocks needed by the DFLL.
> + */
> +int tegra_dfll_suspend(struct device *dev)
> +{
> + struct tegra_dfll *td = dev_get_drvdata(dev);
> +
> + if (dfll_is_running(td)) {
> + dev_err(td->dev, "dfll is enabled while shouldn't be\n");
Minor nit: "DFLL" in the error message, just like you have in the
kerneldoc comment above. Perhaps also make the error message a little
more specific. "while shouldn't be" makes the user guess what that
means. Perhaps better to say something like:
"DFLL still enabled while suspending\n"
or perhaps even add a hint as to what could be the culprit:
"DFLL still enabled while suspending, possibly a cpufreq driverbug\n"
The latter is somewhat long and the former is enough because the
kerneldoc comment already explains that cpufreq might be the reason for
this.
With a more specific error message, this is:
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v8 10/21] clk: tegra: clk-super: Add restore-context support
From: Thierry Reding @ 2019-08-12 9:55 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-11-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 588 bytes --]
On Thu, Aug 08, 2019 at 04:46:49PM -0700, Sowjanya Komatineni wrote:
> This patch implements restore_context for clk_super_mux and clk_super.
>
> During system supend, core power goes off the and context of Tegra
> CAR registers is lost.
>
> So on system resume, context of super clock registers are restored
> to have them in same state as before suspend.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-super.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v8 09/21] clk: tegra: clk-super: Fix to enable PLLP branches to CPU
From: Thierry Reding @ 2019-08-12 9:53 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-10-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 6124 bytes --]
On Thu, Aug 08, 2019 at 04:46:48PM -0700, Sowjanya Komatineni wrote:
> This patch has a fix to enable PLLP branches to CPU before changing
> the CPU cluster clock source to PLLP for Gen5 Super clock and
> disables PLLP branches to CPU when not in use.
>
> During system suspend entry and exit, CPU source will be switched
> to PLLP and this needs PLLP branches to be enabled to CPU prior to
> the switch.
>
> On system resume, warmboot code enables PLLP branches to CPU and
> powers up the CPU with PLLP clock source.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-super.c | 14 ++++++++++++++
> drivers/clk/tegra/clk-tegra-super-gen4.c | 7 ++++++-
> drivers/clk/tegra/clk.c | 14 ++++++++++++++
> drivers/clk/tegra/clk.h | 5 +++++
> 4 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
> index 39ef31b46df5..e2a1e95a8db7 100644
> --- a/drivers/clk/tegra/clk-super.c
> +++ b/drivers/clk/tegra/clk-super.c
> @@ -28,6 +28,9 @@
> #define super_state_to_src_shift(m, s) ((m->width * s))
> #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>
> +#define CCLK_SRC_PLLP_OUT0 4
> +#define CCLK_SRC_PLLP_OUT4 5
> +
> static u8 clk_super_get_parent(struct clk_hw *hw)
> {
> struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
> @@ -97,12 +100,23 @@ static int clk_super_set_parent(struct clk_hw *hw, u8 index)
> if (index == mux->div2_index)
> index = mux->pllx_index;
> }
> +
> + /* enable PLLP branches to CPU before selecting PLLP source */
> + if ((mux->flags & TEGRA210_CPU_CLK) &&
> + (index == CCLK_SRC_PLLP_OUT0 || index == CCLK_SRC_PLLP_OUT4))
> + tegra_clk_set_pllp_out_cpu(true);
> +
> val &= ~((super_state_to_src_mask(mux)) << shift);
> val |= (index & (super_state_to_src_mask(mux))) << shift;
>
> writel_relaxed(val, mux->reg);
> udelay(2);
>
> + /* disable PLLP branches to CPU if not used */
> + if ((mux->flags & TEGRA210_CPU_CLK) &&
> + index != CCLK_SRC_PLLP_OUT0 && index != CCLK_SRC_PLLP_OUT4)
> + tegra_clk_set_pllp_out_cpu(false);
> +
> out:
> if (mux->lock)
> spin_unlock_irqrestore(mux->lock, flags);
> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
> index cdfe7c9697e1..98538f79b0c4 100644
> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
> @@ -180,7 +180,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
> gen_info->num_cclk_g_parents,
> CLK_SET_RATE_PARENT,
> clk_base + CCLKG_BURST_POLICY,
> - 0, 4, 8, 0, NULL);
> + TEGRA210_CPU_CLK, 4, 8, 0, NULL);
> } else {
> clk = tegra_clk_register_super_mux("cclk_g",
> gen_info->cclk_g_parents,
> @@ -196,6 +196,11 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
> dt_clk = tegra_lookup_dt_id(tegra_clk_cclk_lp, tegra_clks);
> if (dt_clk) {
> if (gen_info->gen == gen5) {
> + /*
> + * TEGRA210_CPU_CLK flag is not needed for cclk_lp as cluster
> + * switching is not currently supported on Tegra210 and also
> + * cpu_lp is not used.
> + */
Indentation looks odd here. If you want to comment the whole block, put
the comment above the "if (...) {". If you want to comment the contents
of the block, indent one level further so it aligns with the "clk = ..."
below.
Otherwise looks good, so with the indentation fixed:
Acked-by: Thierry Reding <treding@nvidia.com>
> clk = tegra_clk_register_super_mux("cclk_lp",
> gen_info->cclk_lp_parents,
> gen_info->num_cclk_lp_parents,
> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> index 573e3c967ae1..eb08047fd02f 100644
> --- a/drivers/clk/tegra/clk.c
> +++ b/drivers/clk/tegra/clk.c
> @@ -23,6 +23,7 @@
> #define CLK_OUT_ENB_W 0x364
> #define CLK_OUT_ENB_X 0x280
> #define CLK_OUT_ENB_Y 0x298
> +#define CLK_ENB_PLLP_OUT_CPU BIT(31)
> #define CLK_OUT_ENB_SET_L 0x320
> #define CLK_OUT_ENB_CLR_L 0x324
> #define CLK_OUT_ENB_SET_H 0x328
> @@ -199,6 +200,19 @@ const struct tegra_clk_periph_regs *get_reg_bank(int clkid)
> }
> }
>
> +void tegra_clk_set_pllp_out_cpu(bool enable)
> +{
> + u32 val;
> +
> + val = readl_relaxed(clk_base + CLK_OUT_ENB_Y);
> + if (enable)
> + val |= CLK_ENB_PLLP_OUT_CPU;
> + else
> + val &= ~CLK_ENB_PLLP_OUT_CPU;
> +
> + writel_relaxed(val, clk_base + CLK_OUT_ENB_Y);
> +}
> +
> struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
> {
> clk_base = regs;
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index 8a9af45b6084..560e2bcb3d7d 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -677,6 +677,9 @@ struct clk *tegra_clk_register_periph_data(void __iomem *clk_base,
> * Flags:
> * TEGRA_DIVIDER_2 - LP cluster has additional divider. This flag indicates
> * that this is LP cluster clock.
> + * TEGRA210_CPU_CLK - This flag is used to identify CPU cluster for gen5
> + * super mux parent using PLLP branches. To use PLLP branches to CPU, need
> + * to configure additional bit PLLP_OUT_CPU in the clock registers.
> */
> struct tegra_clk_super_mux {
> struct clk_hw hw;
> @@ -693,6 +696,7 @@ struct tegra_clk_super_mux {
> #define to_clk_super_mux(_hw) container_of(_hw, struct tegra_clk_super_mux, hw)
>
> #define TEGRA_DIVIDER_2 BIT(0)
> +#define TEGRA210_CPU_CLK BIT(1)
>
> extern const struct clk_ops tegra_clk_super_ops;
> struct clk *tegra_clk_register_super_mux(const char *name,
> @@ -838,6 +842,7 @@ int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
> u8 frac_width, u8 flags);
> void tegra_clk_osc_resume(void __iomem *clk_base);
> +void tegra_clk_set_pllp_out_cpu(bool enable);
>
>
> /* Combined read fence with delay */
> --
> 2.7.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] gpio: cadence: Pass irqchip when adding gpiochip
From: Jan Kotas @ 2019-08-12 9:53 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio@vger.kernel.org, Bartosz Golaszewski, Jan Kotas,
Thierry Reding
In-Reply-To: <20190809131804.20352-1-linus.walleij@linaro.org>
> On 9 Aug 2019, at 15:18, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
>
> For chained irqchips this is a pretty straight-forward
> conversion.
>
> Cc: Jan Kotas <jank@cadence.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Hi Jan, it'd be great if you could test/review this
> patch.
Everything seems to be OK in my tests.
Regards,
Jan
^ permalink raw reply
* Re: [PATCH v8 08/21] clk: tegra: periph: Add restore_context support
From: Thierry Reding @ 2019-08-12 9:50 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-9-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 747 bytes --]
On Thu, Aug 08, 2019 at 04:46:47PM -0700, Sowjanya Komatineni wrote:
> This patch implements restore_context support for clk-periph and
> clk-sdmmc-mux clock operations to restore clock parent and rates
> on system resume.
>
> During system suspend, core power goes off and looses the context
> of the Tegra clock controller registers.
>
> So on system resume, clocks parent and rate are restored back to
> the context before suspend based on cached data.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-periph.c | 18 ++++++++++++++++++
> drivers/clk/tegra/clk-sdmmc-mux.c | 12 ++++++++++++
> 2 files changed, 30 insertions(+)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v8 07/21] clk: Add API to get index of the clock parent
From: Thierry Reding @ 2019-08-12 9:47 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-8-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]
On Thu, Aug 08, 2019 at 04:46:46PM -0700, Sowjanya Komatineni wrote:
> This patch adds an API clk_hw_get_parent_index to get index of the
> clock parent to use during the clock restore operations on system
> resume.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/clk.c | 17 +++++++++++++++++
> include/linux/clk-provider.h | 1 +
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..f26252e48f73 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1643,6 +1643,23 @@ static int clk_fetch_parent_index(struct clk_core *core,
> return i;
> }
>
> +/**
> + * clk_hw_get_parent_index - return the index of parent clock
> + * @hw: clk_hw associated with the clk being consumed
> + * @parent_hw: clk_hw associated with the parent of clk
> + *
> + * Fetches and returns the index of parent clock.
> + * if hw or parent_hw is NULL, returns -EINVAL.
"If" because it's at the beginning of a sentence. You may also want to
turn this into a "Return:" section as described in:
Documentation/doc-guide/kernel-doc.rst
That said, other functions in this file don't use that construct either,
so I suppose this is fine as-is, for consistency.
So with the capitalization of "If" fixed, this is:
Reviewed-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/7] [RFC] ARM: remove Intel iop33x and iop13xx support
From: Martin Michlmayr @ 2019-08-12 9:44 UTC (permalink / raw)
To: Dan Williams
Cc: Arnd Bergmann, soc, Russell King, Vinod Koul, Linus Walleij,
Bartosz Golaszewski, linux-arm-kernel, Linux Kernel Mailing List,
dmaengine, linux-gpio, linux-i2c, Peter Teichmann
In-Reply-To: <CAA9_cmdDbBm0ookyqGJMcyLVFHkYHuR3mEeawQKS2UqYJoWWaQ@mail.gmail.com>
* Dan Williams <dan.j.williams@intel.com> [2019-08-09 11:34]:
> > Earlier versions of OpenWRT and Debian both had support for iop32x
> > but not the others, and they both dropped iop32x as well in their 2015
> > releases.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > I'm just guessing that iop32x is still needed, and the other two are
> > not. If anyone disagrees with that assessment, let me know so we
> > can come up with an alternative approach.
>
> I'm not sure who would scream if iop32x support went away as well, but
> I have not followed this space in years hence copying Martin.
I believe iop13xx were mostly Intel dev boards. I'm not aware of any
major devices based on iop33x.
As Arnd points out, Debian used to have support for various iop32x
devices. While Debian hasn't supported iop32x in a number of years,
these devices are still usable and in use (RMK being a prime example).
So I think it's safe to drop iop33x/iop13xx while retaining support
for iop32x.
As I was looking at my email archives, I saw an email from Peter
Teichmann who was working on an iop33x based platform (around 2009) so
I've copied him as well.
> In any event:
>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Martin Michlmayr <tbm@cyrius.com>
--
Martin Michlmayr
https://www.cyrius.com/
^ 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