* [PATCH] gpio: pca953x: Use irqchip template
From: Linus Walleij @ 2020-07-17 14:40 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Andy Shevchenko, Marek Vasut,
Uwe Kleine-König, Adam Ford, Vignesh Raghavendra
This makes the driver use the irqchip template to assign
properties to the gpio_irq_chip instead of using the
explicit calls to gpiochip_irqchip_add_nested() and
gpiochip_set_nested_irqchip(). The irqchip is instead
added while adding the gpiochip.
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Marek Vasut <marek.vasut@gmail.com>
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Adam Ford <aford173@gmail.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-pca953x.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 9c90cf3aac5a..ab22152bf3e8 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -834,6 +834,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, int irq_base)
struct irq_chip *irq_chip = &chip->irq_chip;
DECLARE_BITMAP(reg_direction, MAX_LINE);
DECLARE_BITMAP(irq_stat, MAX_LINE);
+ struct gpio_irq_chip *girq;
int ret;
if (dmi_first_match(pca953x_dmi_acpi_irq_info)) {
@@ -883,16 +884,16 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, int irq_base)
irq_chip->irq_set_type = pca953x_irq_set_type;
irq_chip->irq_shutdown = pca953x_irq_shutdown;
- ret = gpiochip_irqchip_add_nested(&chip->gpio_chip, irq_chip,
- irq_base, handle_simple_irq,
- IRQ_TYPE_NONE);
- if (ret) {
- dev_err(&client->dev,
- "could not connect irqchip to gpiochip\n");
- return ret;
- }
-
- gpiochip_set_nested_irqchip(&chip->gpio_chip, irq_chip, client->irq);
+ girq = &chip->gpio_chip.irq;
+ girq->chip = irq_chip;
+ /* This will let us handle the parent IRQ in the driver */
+ girq->parent_handler = NULL;
+ girq->num_parents = 0;
+ girq->parents = NULL;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_simple_irq;
+ girq->threaded = true;
+ girq->first = irq_base; /* FIXME: get rid of this */
return 0;
}
@@ -1080,11 +1081,11 @@ static int pca953x_probe(struct i2c_client *client,
if (ret)
goto err_exit;
- ret = devm_gpiochip_add_data(&client->dev, &chip->gpio_chip, chip);
+ ret = pca953x_irq_setup(chip, irq_base);
if (ret)
goto err_exit;
- ret = pca953x_irq_setup(chip, irq_base);
+ ret = devm_gpiochip_add_data(&client->dev, &chip->gpio_chip, chip);
if (ret)
goto err_exit;
--
2.26.2
^ permalink raw reply related
* [PATCH] gpio: max732x: Use irqchip template
From: Linus Walleij @ 2020-07-17 14:19 UTC (permalink / raw)
To: linux-gpio; +Cc: Bartosz Golaszewski, Linus Walleij, Sam Protsenko
This makes the driver use the irqchip template to assign
properties to the gpio_irq_chip instead of using the
explicit calls to gpiochip_irqchip_add_nested() and
gpiochip_set_nested_irqchip(). The irqchip is instead
added while adding the gpiochip.
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-max732x.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpio/gpio-max732x.c b/drivers/gpio/gpio-max732x.c
index 63472f308857..347415344a20 100644
--- a/drivers/gpio/gpio-max732x.c
+++ b/drivers/gpio/gpio-max732x.c
@@ -503,6 +503,8 @@ static int max732x_irq_setup(struct max732x_chip *chip,
if (((pdata && pdata->irq_base) || client->irq)
&& has_irq != INT_NONE) {
+ struct gpio_irq_chip *girq;
+
if (pdata)
irq_base = pdata->irq_base;
chip->irq_features = has_irq;
@@ -517,19 +519,17 @@ static int max732x_irq_setup(struct max732x_chip *chip,
client->irq);
return ret;
}
- ret = gpiochip_irqchip_add_nested(&chip->gpio_chip,
- &max732x_irq_chip,
- irq_base,
- handle_simple_irq,
- IRQ_TYPE_NONE);
- if (ret) {
- dev_err(&client->dev,
- "could not connect irqchip to gpiochip\n");
- return ret;
- }
- gpiochip_set_nested_irqchip(&chip->gpio_chip,
- &max732x_irq_chip,
- client->irq);
+
+ girq = &chip->gpio_chip.irq;
+ girq->chip = &max732x_irq_chip;
+ /* This will let us handle the parent IRQ in the driver */
+ girq->parent_handler = NULL;
+ girq->num_parents = 0;
+ girq->parents = NULL;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_simple_irq;
+ girq->threaded = true;
+ girq->first = irq_base; /* FIXME: get rid of this */
}
return 0;
--
2.26.2
^ permalink raw reply related
* Re: gpiolib gpio_chrdev_release duration is about 30 ms
From: Maxim Kochetkov @ 2020-07-17 14:17 UTC (permalink / raw)
To: Linus Walleij; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski
In-Reply-To: <CACRpkda-pXF71vr5v90yipKubc14tbZW5Ryw1o7rdn4FbWwsTw@mail.gmail.com>
I need a small userspace program to do some GPIO magic to communicate
other hardware like devmem. This program takes about 2,5 seconds just to
find GPIO lines by name.
replacing synchronize_rcu to synchronize_rcu_expedited in
atomic_notifier_chain_unregister gives the same boost as removing
synchronize_rcu
17.07.2020 16:37, Linus Walleij пишет:
> Are you just testing because
> curious or do you need to meet a design objective?
^ permalink raw reply
* Re: [PATCH] gpio: crystalcove: Use irqchip template
From: Linus Walleij @ 2020-07-17 14:14 UTC (permalink / raw)
To: Hans de Goede
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Andy Shevchenko,
Kuppuswamy Sathyanarayanan
In-Reply-To: <fb6bb42b-b657-5cd7-7a58-236e10bfb547@redhat.com>
On Fri, Jul 17, 2020 at 4:02 PM Hans de Goede <hdegoede@redhat.com> wrote:
> Erm, it does not even compile:
>
> drivers/gpio/gpio-crystalcove.c: In function ‘crystalcove_gpio_probe’:
> drivers/gpio/gpio-crystalcove.c:357:10: error: ‘ch’ undeclared (first use in this function); did you mean ‘cg’?
> 357 | girq = &ch->chip.irq;
> | ^~
> | cg
> drivers/gpio/gpio-crystalcove.c:357:10: note: each undeclared identifier is reported only once for each function it appears in
>
> I've fixed this up locally.
Thanks, the SOC_PMIC isn't in the Intel default build, and you know
me and Intel ...
Sorry for that.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Greg KH @ 2020-07-17 14:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Linus Walleij, Anson Huang, John Stultz,
Russell King, Shawn Guo, Sascha Hauer, Sascha Hauer,
Fabio Estevam, Catalin Marinas, Will Deacon, Bartosz Golaszewski,
oleksandr.suvorov@toradex.com, Adam Ford, Andreas Kemnade,
hverkuil-cisco@xs4all.nl, Bjorn Andersson, Leo Li, Vinod Koul,
Linux ARM, linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx, Jon Corbet, Olof Johansson, Kevin Hilman
In-Reply-To: <CAK8P3a3fwwx-2gQTXZ9dbko+DuLELGm=nKrYFXfwcJJOf0Xz5g@mail.gmail.com>
On Fri, Jul 17, 2020 at 03:54:49PM +0200, Arnd Bergmann wrote:
> > And look at the driver core work for many driver subsystems to be fixed
> > up just to get a single kernel image to work on multiple platforms.
> > Just because older ones did it, doesn't mean it actually works today :)
>
> Can you give a specific example? The only problem I'm aware of for
> those SoCs is drivers being outside of the mainline kernel. Clearly
> having support for loadable modules helps SoC vendors because it
> allows them to support a new platform with an existing binary kernel
> by shipping third-party driver modules, but for stuff that is already
> in mainline, we could in theory support all hardware in a single gigantic
> binary kernel with no support for loadable modules at all.
That did not work for many drivers for some reason, look at all the work
Saravana had to do in the driver core and device tree code for it to
happen correctly over the past year.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 16/25] arch: arm: mach-at91: pm: Move prototypes to mutually included header
From: Alexandre Belloni @ 2020-07-17 14:10 UTC (permalink / raw)
To: Lee Jones
Cc: linus.walleij, linux-kernel, linux-gpio, Russell King,
Nicolas Ferre, Ludovic Desroches
In-Reply-To: <20200716134231.GP3165313@dell>
On 16/07/2020 14:42:31+0100, Lee Jones wrote:
> > > diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> > > index 9c52130876597..37997e5ab0538 100644
> > > --- a/drivers/pinctrl/pinctrl-at91.c
> > > +++ b/drivers/pinctrl/pinctrl-at91.c
> > > @@ -22,6 +22,7 @@
> > > #include <linux/pinctrl/pinmux.h>
> > > /* Since we request GPIOs from ourself */
> > > #include <linux/pinctrl/consumer.h>
> > > +#include <linux/platform_data/atmel.h>
> > >
> > > #include "pinctrl-at91.h"
> > > #include "core.h"
> > > diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
> > > index 99e6069c5fd89..666ef482ea8c0 100644
> > > --- a/include/linux/platform_data/atmel.h
> > > +++ b/include/linux/platform_data/atmel.h
> >
> > The plan is to get rid of that file so you should probably find a better
> > location.
>
> Suggestions welcome.
>
Something like include/soc/at91/pm.h so we can also move
at91_suspend_entering_slow_clock there and then kill
platform_data/atmel.h.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH] gpio: crystalcove: Use irqchip template
From: Andy Shevchenko @ 2020-07-17 14:08 UTC (permalink / raw)
To: Hans de Goede
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
Andy Shevchenko, Kuppuswamy Sathyanarayanan
In-Reply-To: <d97d8c70-528e-f06b-3bf6-4faf51857a9c@redhat.com>
On Fri, Jul 17, 2020 at 5:00 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 7/17/20 1:25 PM, Linus Walleij wrote:
...
> Hmm, testing this might be tricky, I don't think any boards
> actually use any GPIOs on the PMIC (which this driver is for)
> as interrupts...
>
> So the best I can do is boot a machine and test there are no
> regressions I guess.
I was about to send a PR to Linus, but I will wait for your answer.
From my perspective the approach is fine. We might have needed some
kind of masking, but I have no idea what those GPIOs can be used
for...
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2] irqchip/stm32-exti: map direct event to irq parent
From: Alexandre Torgue @ 2020-07-17 14:07 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier
Cc: linux-arm-kernel, linux-kernel, linux-gpio, linux-stm32, marex,
alexandre.torgue
EXTI lines are mainly used to wake-up system from CStop low power mode.
Currently, if a device wants to use a EXTI (direct) line as wakeup line,
it has to declare 2 interrupts:
- one for EXTI used to wake-up system (with dedicated_wake_irq api).
- one for GIC used to get the wake up reason inside the concerned IP.
This split is not really needed as each EXTI line is actually "linked " to
a GIC. So to avoid this useless double interrupt management in each
wake-up driver, this patch lets the STM32 EXTI driver abstract it by
mapping each EXTI line to his corresponding GIC.
Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
---
Changes since v1:
- Fix warning reported by test robot.
diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
index faa8482c8246..732b04a121b6 100644
--- a/drivers/irqchip/irq-stm32-exti.c
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -42,6 +42,7 @@ struct stm32_exti_bank {
struct stm32_desc_irq {
u32 exti;
u32 irq_parent;
+ struct irq_chip *chip;
};
struct stm32_exti_drv_data {
@@ -166,27 +167,41 @@ static const struct stm32_exti_bank *stm32mp1_exti_banks[] = {
&stm32mp1_exti_b3,
};
+static struct irq_chip stm32_exti_h_chip;
+static struct irq_chip stm32_exti_h_chip_direct;
+
static const struct stm32_desc_irq stm32mp1_desc_irq[] = {
- { .exti = 0, .irq_parent = 6 },
- { .exti = 1, .irq_parent = 7 },
- { .exti = 2, .irq_parent = 8 },
- { .exti = 3, .irq_parent = 9 },
- { .exti = 4, .irq_parent = 10 },
- { .exti = 5, .irq_parent = 23 },
- { .exti = 6, .irq_parent = 64 },
- { .exti = 7, .irq_parent = 65 },
- { .exti = 8, .irq_parent = 66 },
- { .exti = 9, .irq_parent = 67 },
- { .exti = 10, .irq_parent = 40 },
- { .exti = 11, .irq_parent = 42 },
- { .exti = 12, .irq_parent = 76 },
- { .exti = 13, .irq_parent = 77 },
- { .exti = 14, .irq_parent = 121 },
- { .exti = 15, .irq_parent = 127 },
- { .exti = 16, .irq_parent = 1 },
- { .exti = 65, .irq_parent = 144 },
- { .exti = 68, .irq_parent = 143 },
- { .exti = 73, .irq_parent = 129 },
+ { .exti = 0, .irq_parent = 6, .chip = &stm32_exti_h_chip },
+ { .exti = 1, .irq_parent = 7, .chip = &stm32_exti_h_chip },
+ { .exti = 2, .irq_parent = 8, .chip = &stm32_exti_h_chip },
+ { .exti = 3, .irq_parent = 9, .chip = &stm32_exti_h_chip },
+ { .exti = 4, .irq_parent = 10, .chip = &stm32_exti_h_chip },
+ { .exti = 5, .irq_parent = 23, .chip = &stm32_exti_h_chip },
+ { .exti = 6, .irq_parent = 64, .chip = &stm32_exti_h_chip },
+ { .exti = 7, .irq_parent = 65, .chip = &stm32_exti_h_chip },
+ { .exti = 8, .irq_parent = 66, .chip = &stm32_exti_h_chip },
+ { .exti = 9, .irq_parent = 67, .chip = &stm32_exti_h_chip },
+ { .exti = 10, .irq_parent = 40, .chip = &stm32_exti_h_chip },
+ { .exti = 11, .irq_parent = 42, .chip = &stm32_exti_h_chip },
+ { .exti = 12, .irq_parent = 76, .chip = &stm32_exti_h_chip },
+ { .exti = 13, .irq_parent = 77, .chip = &stm32_exti_h_chip },
+ { .exti = 14, .irq_parent = 121, .chip = &stm32_exti_h_chip },
+ { .exti = 15, .irq_parent = 127, .chip = &stm32_exti_h_chip },
+ { .exti = 16, .irq_parent = 1, .chip = &stm32_exti_h_chip },
+ { .exti = 19, .irq_parent = 3, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 21, .irq_parent = 31, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 22, .irq_parent = 33, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 23, .irq_parent = 72, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 24, .irq_parent = 95, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 25, .irq_parent = 107, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 30, .irq_parent = 52, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 47, .irq_parent = 93, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 54, .irq_parent = 135, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 61, .irq_parent = 100, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 65, .irq_parent = 144, .chip = &stm32_exti_h_chip },
+ { .exti = 68, .irq_parent = 143, .chip = &stm32_exti_h_chip },
+ { .exti = 70, .irq_parent = 62, .chip = &stm32_exti_h_chip_direct },
+ { .exti = 73, .irq_parent = 129, .chip = &stm32_exti_h_chip },
};
static const struct stm32_exti_drv_data stm32mp1_drv_data = {
@@ -196,22 +211,23 @@ static const struct stm32_exti_drv_data stm32mp1_drv_data = {
.irq_nr = ARRAY_SIZE(stm32mp1_desc_irq),
};
-static int stm32_exti_to_irq(const struct stm32_exti_drv_data *drv_data,
- irq_hw_number_t hwirq)
+static const struct
+stm32_desc_irq *stm32_exti_get_desc(const struct stm32_exti_drv_data *drv_data,
+ irq_hw_number_t hwirq)
{
- const struct stm32_desc_irq *desc_irq;
+ const struct stm32_desc_irq *desc = NULL;
int i;
if (!drv_data->desc_irqs)
- return -EINVAL;
+ return NULL;
for (i = 0; i < drv_data->irq_nr; i++) {
- desc_irq = &drv_data->desc_irqs[i];
- if (desc_irq->exti == hwirq)
- return desc_irq->irq_parent;
+ desc = &drv_data->desc_irqs[i];
+ if (desc->exti == hwirq)
+ break;
}
- return -EINVAL;
+ return desc;
}
static unsigned long stm32_exti_pending(struct irq_chip_generic *gc)
@@ -628,30 +644,47 @@ static struct irq_chip stm32_exti_h_chip = {
.irq_set_affinity = IS_ENABLED(CONFIG_SMP) ? stm32_exti_h_set_affinity : NULL,
};
+static struct irq_chip stm32_exti_h_chip_direct = {
+ .name = "stm32-exti-h-direct",
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_ack = irq_chip_ack_parent,
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_retrigger = irq_chip_retrigger_hierarchy,
+ .irq_set_type = irq_chip_set_type_parent,
+ .irq_set_wake = stm32_exti_h_set_wake,
+ .flags = IRQCHIP_MASK_ON_SUSPEND,
+ .irq_set_affinity = IS_ENABLED(CONFIG_SMP) ? irq_chip_set_affinity_parent : NULL,
+};
+
static int stm32_exti_h_domain_alloc(struct irq_domain *dm,
unsigned int virq,
unsigned int nr_irqs, void *data)
{
struct stm32_exti_host_data *host_data = dm->host_data;
struct stm32_exti_chip_data *chip_data;
+ const struct stm32_desc_irq *desc;
struct irq_fwspec *fwspec = data;
struct irq_fwspec p_fwspec;
irq_hw_number_t hwirq;
- int p_irq, bank;
+ int bank;
hwirq = fwspec->param[0];
bank = hwirq / IRQS_PER_BANK;
chip_data = &host_data->chips_data[bank];
- irq_domain_set_hwirq_and_chip(dm, virq, hwirq,
- &stm32_exti_h_chip, chip_data);
- p_irq = stm32_exti_to_irq(host_data->drv_data, hwirq);
- if (p_irq >= 0) {
+ desc = stm32_exti_get_desc(host_data->drv_data, hwirq);
+ if (!desc)
+ return -EINVAL;
+
+ irq_domain_set_hwirq_and_chip(dm, virq, hwirq, desc->chip,
+ chip_data);
+ if (desc->irq_parent) {
p_fwspec.fwnode = dm->parent->fwnode;
p_fwspec.param_count = 3;
p_fwspec.param[0] = GIC_SPI;
- p_fwspec.param[1] = p_irq;
+ p_fwspec.param[1] = desc->irq_parent;
p_fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
return irq_domain_alloc_irqs_parent(dm, virq, 1, &p_fwspec);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] gpio: crystalcove: Use irqchip template
From: Hans de Goede @ 2020-07-17 14:02 UTC (permalink / raw)
To: Linus Walleij, linux-gpio
Cc: Bartosz Golaszewski, Andy Shevchenko, Kuppuswamy Sathyanarayanan
In-Reply-To: <d97d8c70-528e-f06b-3bf6-4faf51857a9c@redhat.com>
Hi,
On 7/17/20 3:59 PM, Hans de Goede wrote:
> Hi,
>
> On 7/17/20 1:25 PM, Linus Walleij wrote:
>> This makes the driver use the irqchip template to assign
>> properties to the gpio_irq_chip instead of using the
>> explicit calls to gpiochip_irqchip_add_nested() and
>> gpiochip_set_nested_irqchip(). The irqchip is instead
>> added while adding the gpiochip.
>>
>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> Intel folks and Hans: I hope someone can test this, I'm
>> a bit uncertain if IRQs could fire before registering
>> the chip and if we need a hw_init() in this driver to cope.
>
> I've added this to my personal tree for testing. I will get back
> to you when I've either hit an issue, or used it for a while without
> issues :)
>
> Hmm, testing this might be tricky, I don't think any boards
> actually use any GPIOs on the PMIC (which this driver is for)
> as interrupts...
>
> So the best I can do is boot a machine and test there are no
> regressions I guess.
Erm, it does not even compile:
drivers/gpio/gpio-crystalcove.c: In function ‘crystalcove_gpio_probe’:
drivers/gpio/gpio-crystalcove.c:357:10: error: ‘ch’ undeclared (first use in this function); did you mean ‘cg’?
357 | girq = &ch->chip.irq;
| ^~
| cg
drivers/gpio/gpio-crystalcove.c:357:10: note: each undeclared identifier is reported only once for each function it appears in
I've fixed this up locally.
Regards,
Hans
>> + girq->default_type = IRQ_TYPE_NONE;
>> + girq->handler = handle_simple_irq;
>> + girq->threaded = true;
>> retval = request_threaded_irq(irq, NULL, crystalcove_gpio_irq_handler,
>> IRQF_ONESHOT, KBUILD_MODNAME, cg);
>> @@ -370,7 +372,11 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
>> return retval;
>> }
>> - gpiochip_set_nested_irqchip(&cg->chip, &crystalcove_irqchip, irq);
>> + retval = devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg);
>> + if (retval) {
>> + dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
>> + return retval;
>> + }
>> return 0;
>> }
>>
^ permalink raw reply
* Re: [PATCH] gpio: crystalcove: Use irqchip template
From: Hans de Goede @ 2020-07-17 13:59 UTC (permalink / raw)
To: Linus Walleij, linux-gpio
Cc: Bartosz Golaszewski, Andy Shevchenko, Kuppuswamy Sathyanarayanan
In-Reply-To: <20200717112558.15960-1-linus.walleij@linaro.org>
Hi,
On 7/17/20 1:25 PM, Linus Walleij wrote:
> This makes the driver use the irqchip template to assign
> properties to the gpio_irq_chip instead of using the
> explicit calls to gpiochip_irqchip_add_nested() and
> gpiochip_set_nested_irqchip(). The irqchip is instead
> added while adding the gpiochip.
>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Intel folks and Hans: I hope someone can test this, I'm
> a bit uncertain if IRQs could fire before registering
> the chip and if we need a hw_init() in this driver to cope.
I've added this to my personal tree for testing. I will get back
to you when I've either hit an issue, or used it for a while without
issues :)
Hmm, testing this might be tricky, I don't think any boards
actually use any GPIOs on the PMIC (which this driver is for)
as interrupts...
So the best I can do is boot a machine and test there are no
regressions I guess.
Regards,
Hans
> ---
> drivers/gpio/gpio-crystalcove.c | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-crystalcove.c
> index 14d1f4c933b6..424a00ba1c97 100644
> --- a/drivers/gpio/gpio-crystalcove.c
> +++ b/drivers/gpio/gpio-crystalcove.c
> @@ -330,6 +330,7 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
> int retval;
> struct device *dev = pdev->dev.parent;
> struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> + struct gpio_irq_chip *girq;
>
> if (irq < 0)
> return irq;
> @@ -353,14 +354,15 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
> cg->chip.dbg_show = crystalcove_gpio_dbg_show;
> cg->regmap = pmic->regmap;
>
> - retval = devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg);
> - if (retval) {
> - dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
> - return retval;
> - }
> -
> - gpiochip_irqchip_add_nested(&cg->chip, &crystalcove_irqchip, 0,
> - handle_simple_irq, IRQ_TYPE_NONE);
> + girq = &ch->chip.irq;
> + girq->chip = &crystalcove_irqchip;
> + /* This will let us handle the parent IRQ in the driver */
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_simple_irq;
> + girq->threaded = true;
>
> retval = request_threaded_irq(irq, NULL, crystalcove_gpio_irq_handler,
> IRQF_ONESHOT, KBUILD_MODNAME, cg);
> @@ -370,7 +372,11 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
> return retval;
> }
>
> - gpiochip_set_nested_irqchip(&cg->chip, &crystalcove_irqchip, irq);
> + retval = devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg);
> + if (retval) {
> + dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
> + return retval;
> + }
>
> return 0;
> }
>
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Arnd Bergmann @ 2020-07-17 13:54 UTC (permalink / raw)
To: Greg KH
Cc: Geert Uytterhoeven, Linus Walleij, Anson Huang, John Stultz,
Russell King, Shawn Guo, Sascha Hauer, Sascha Hauer,
Fabio Estevam, Catalin Marinas, Will Deacon, Bartosz Golaszewski,
oleksandr.suvorov@toradex.com, Adam Ford, Andreas Kemnade,
hverkuil-cisco@xs4all.nl, Bjorn Andersson, Leo Li, Vinod Koul,
Linux ARM, linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx, Jon Corbet, Olof Johansson, Kevin Hilman
In-Reply-To: <20200717132101.GA2984939@kroah.com>
On Fri, Jul 17, 2020 at 3:21 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Jul 17, 2020 at 02:27:43PM +0200, Geert Uytterhoeven wrote:
> > Hi Greg,
> >
> > On Fri, Jul 17, 2020 at 2:14 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > Android is just finally pushing vendors to get their code upstream,
> > > which is a good thing to see. And building things as a module is an
> > > even better thing as now it is finally allowing arm64 systems to be
> > > built to support more than one specific hardware platform at runtime.
> >
> > Can you please stop spreading this FUD?
>
> For many many SoCs today, this is not true. Their drivers are required
> to be built in and will not work as modules, as we are seeing the
> patches try to fix.
There are two different points here:
a) having drivers as loadable modules: I think everyone agrees this
is a good thing in general. Having more of them makes smaller kernels,
which is good. arm64 is no different from arm32 and powerpc here,
and probably a bit better than x86, which requires all platform specific
core code (PC, numachip, UV, ScaleMP, ...) to be built-in.
b) supporting multiple hardware platforms at runtime: this is totally
unrelated to the platform specific drivers being loadable modules.
arm64 is a little better here than arm32 and powerpc, which need more
than one configuration to support all hardware, about the same as
x86 or s390 and much better than most others that have to chose
a machine at compile time.
> > As I said before, Arm64 kernels have supported more than one specific
> > hardware platform at runtime from the beginning of the arm64 port
> > (assumed the needed platform support has been enabled in the kernel
> > config, of course).
> > Even most arm32 kernels support this, since the introduction of the
> > CONFIG_ARCH_MULTIPLATFORM option. In fact every recently
> > introduced arm32 platform is usually v7, and must conform to this.
> > The sole exceptions are very old platforms, and the v4/v5/v6/v7 split
> > due to architectural issues (the latter still support clusters of
> > platforms in a single kernel).
>
> I think the confusion here is that this really does not work well, if at
> all, on most "high end" SoC chips due to the collection of different
> things all of the vendors ship to their customers. This is the work
> that is trying to be fixed up here.
>
> And look at the driver core work for many driver subsystems to be fixed
> up just to get a single kernel image to work on multiple platforms.
> Just because older ones did it, doesn't mean it actually works today :)
Can you give a specific example? The only problem I'm aware of for
those SoCs is drivers being outside of the mainline kernel. Clearly
having support for loadable modules helps SoC vendors because it
allows them to support a new platform with an existing binary kernel
by shipping third-party driver modules, but for stuff that is already
in mainline, we could in theory support all hardware in a single gigantic
binary kernel with no support for loadable modules at all.
Arnd
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Linus Walleij @ 2020-07-17 13:44 UTC (permalink / raw)
To: Greg KH
Cc: Anson Huang, John Stultz, Russell King, Shawn Guo, Sascha Hauer,
Sascha Hauer, Fabio Estevam, Catalin Marinas, Will Deacon,
Bartosz Golaszewski, oleksandr.suvorov@toradex.com, Adam Ford,
Andreas Kemnade, hverkuil-cisco@xs4all.nl, Bjorn Andersson,
Leo Li, Vinod Koul, Geert Uytterhoeven, Olof Johansson, Linux ARM,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx, Jon Corbet
In-Reply-To: <20200717121436.GA2953399@kroah.com>
On Fri, Jul 17, 2020 at 2:14 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> So moving drivers to modules is good. If a module can be removed, even
> better, but developers should not be lazy and just flat out not try at
> all to make their code unloadable if at all possible.
>
> Does that help?
Yeah it confirms my intuitive maintenance approach: developer submits
modularization patch, I will be a bit inquisitive and "can't you attempt
to make this thing unload too" and if they conclude that that is
an unfathomable effort I will likely merge it anyway as very likely
the kernel looks better after than before provided all build and test
coverage stays the same as well.
Thanks!
Linus Walleij
^ permalink raw reply
* Re: gpiolib gpio_chrdev_release duration is about 30 ms
From: Maxim Kochetkov @ 2020-07-17 13:44 UTC (permalink / raw)
To: Linus Walleij; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski
In-Reply-To: <CACRpkda-pXF71vr5v90yipKubc14tbZW5Ryw1o7rdn4FbWwsTw@mail.gmail.com>
------------------------------------------
#define GPIOS_NUM 8
static char* gpio_names[GPIOS_NUM] = {"IO0", "IO1", "IO2", "IO3", "WR",
"CS", "OE", "ADD_EN"};
................
gpio_init() {
int i;
for (i = 0; i < GPIOS_NUM; ++i) {
syslog(LOG_INFO,"%s %d",__FUNCTION__,__LINE__);
gpio_lines[i] = gpiod_line_find(gpio_names[i]);
syslog(LOG_INFO,"%s %d",__FUNCTION__,__LINE__);
}
}
-------------------------------------------
I used syslog/printk to measure time.
syslog in example shows about 300ms on each gpiod_line_find call.
17.07.2020 16:37, Linus Walleij пишет:
> Hi Maxim,
>
> On Fri, Jul 17, 2020 at 2:56 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
>
>> I'm using libgpiod in userspace.
>> I have 6 gpiochip's on my board.
>> gpiod_line_find takes about 300ms to find GPIO line.
>>
>> gpiod_line_find calls gpiod_foreach_chip
>> then gpiod_chip_iter_next
>> then gpiod_chip_close then close(chip->fd)
>> then we are going to kernel gpiolib gpio_chrdev_release
>> then atomic_notifier_chain_unregister
>> then synchronize_rcu()
>>
>> synchronize_rcu takes about 30 ms (6*30ms=280ms)
>>
>> I tried to remove synchronize_rcu from atomic_notifier_chain_unregister
>> and gpiod_line_find takes about 2ms now.
>
> Interesting! Can you provide some context? Are you just testing because
> curious or do you need to meet a design objective?
>
> Did you use ftrace or similar instrumentation to drill down and find
> where time is spent?
>
> Yours,
> Linus Walleij
>
^ permalink raw reply
* Re: gpiolib gpio_chrdev_release duration is about 30 ms
From: Linus Walleij @ 2020-07-17 13:37 UTC (permalink / raw)
To: Maxim Kochetkov; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski
In-Reply-To: <7eb11c0d-cd11-f873-c336-4ec955a7bdb3@inbox.ru>
Hi Maxim,
On Fri, Jul 17, 2020 at 2:56 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> I'm using libgpiod in userspace.
> I have 6 gpiochip's on my board.
> gpiod_line_find takes about 300ms to find GPIO line.
>
> gpiod_line_find calls gpiod_foreach_chip
> then gpiod_chip_iter_next
> then gpiod_chip_close then close(chip->fd)
> then we are going to kernel gpiolib gpio_chrdev_release
> then atomic_notifier_chain_unregister
> then synchronize_rcu()
>
> synchronize_rcu takes about 30 ms (6*30ms=280ms)
>
> I tried to remove synchronize_rcu from atomic_notifier_chain_unregister
> and gpiod_line_find takes about 2ms now.
Interesting! Can you provide some context? Are you just testing because
curious or do you need to meet a design objective?
Did you use ftrace or similar instrumentation to drill down and find
where time is spent?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Greg KH @ 2020-07-17 13:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linus Walleij, Geert Uytterhoeven, Catalin Marinas,
Bjorn Andersson, oleksandr.suvorov@toradex.com, Fabio Estevam,
Anson Huang, Jon Corbet, Will Deacon, Russell King,
Bartosz Golaszewski, Andreas Kemnade, dl-linux-imx, Sascha Hauer,
open list:GPIO SUBSYSTEM, John Stultz, hverkuil-cisco@xs4all.nl,
Adam Ford, Linux ARM, linux-kernel@vger.kernel.org, Leo Li,
Vinod Koul, Sascha Hauer, Olof Johansson, Shawn Guo
In-Reply-To: <CAK8P3a3Ds4O5yRtGSMbNN8R5dPcdb1HJTY=W5eyToFQ-UhzkBw@mail.gmail.com>
On Fri, Jul 17, 2020 at 03:02:54PM +0200, Arnd Bergmann wrote:
> On Fri, Jul 17, 2020 at 2:16 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Jul 17, 2020 at 02:01:16PM +0200, Linus Walleij wrote:
> > > While I am a big fan of the Android GKI initiative this needs to be aligned
> > > with the Linux core maintainers, so let's ask Greg. I am also paging
> > > John Stultz on this: he is close to this action.
> > >
> > > They both know the Android people very well.
> > >
> > > So there is a rationale like this going on: in order to achieve GKI goals
> > > and have as much as possible of the Linux kernel stashed into loadable
> > > kernel modules, it has been elevated to modus operandi amongst
> > > the developers pushing this change that it is OK to pile up a load of
> > > modules that cannot ever be unloaded.
> >
> > Why can't the module be unloaded? Is it just because they never
> > implement the proper "remove all resources allocated" logic in a remove
> > function, or something else?
>
> For the core kernel parts, it's usually for the lack of tracking of who
> is using the resource provided by the driver, as the subsystems tend
> to be written around x86's "everything is built-in" model.
>
> For instance, a PCIe host bridge might rely on the IOMMU, a
> clock controller, an interrupt controller, a pin controller and a reset
> controller. The host bridge can still be probed at reduced functionality
> if some of these are missing, or it can use deferred probing when
> some others are missing at probe time.
>
> If we want all of drivers to be unloaded again, we need to do one
> of two things:
>
> a) track dependencies, so that removing one of the devices
> underneath leads to everything depending on it to get removed
> as well or will be notified about it going away and can stop using
> it. This is the model used in the network subsystem, where
> any ethernet driver can be unloaded and everything using the
> device gets torn down.
>
> b) use reference counting on the device or (at the minimum)
> try_module_get()/module_put() calls for all such resources
> so as long as the pci host bridge is there, so none of the devices
> it uses will go away when they are still used.
>
> Traditionally, we would have considered the PCIe host bridge to
> be a fundamental part of the system, implying that everything it
> uses is also fundamental, and there was no need to track
> usage at all, just to ensure the probing is done in the right order.
Yeah, ick, for IOMMU and stuff like this, no, load it once and never
unload it makes much more sense.
Just know how to dynamically load the specific driver out of a
collection of them, and all should be fine.
> > > As a minimum requirement I would expect this to be marked by
> > >
> > > struct device_driver {
> > > (...)
> > > /* This module absolutely cannot be unbound */
> > > .suppress_bind_attrs = true;
> > > };
> >
> > No, that's not what bind/unbind is really for. That's a per-subsystem
> > choice as to if you want to allow devices to be added/removed from
> > drivers at runtime. It has nothing to do with module load/unload.
>
> It's a one-way dependency: If we can't allow the device to be
> unbound, then we also should not allow module unloading because
> that forces an unbind.
Ok, then turn that off for the subsystems this does not support, no
objection from me. It's just a fun hack that people use for testing out
drivers on new devices, and for virtual devices.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Greg KH @ 2020-07-17 13:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Anson Huang, John Stultz, Russell King, Shawn Guo,
Sascha Hauer, Sascha Hauer, Fabio Estevam, Catalin Marinas,
Will Deacon, Bartosz Golaszewski, oleksandr.suvorov@toradex.com,
Adam Ford, Andreas Kemnade, hverkuil-cisco@xs4all.nl,
Bjorn Andersson, Leo Li, Vinod Koul, Linux ARM,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx, Jon Corbet, Arnd Bergmann, Olof Johansson,
Kevin Hilman
In-Reply-To: <CAMuHMdWdaQ_jK1T_ErJ36pJbUUS3SFBcqQmyvtufaKha2C76Gg@mail.gmail.com>
On Fri, Jul 17, 2020 at 02:27:43PM +0200, Geert Uytterhoeven wrote:
> Hi Greg,
>
> On Fri, Jul 17, 2020 at 2:14 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > Android is just finally pushing vendors to get their code upstream,
> > which is a good thing to see. And building things as a module is an
> > even better thing as now it is finally allowing arm64 systems to be
> > built to support more than one specific hardware platform at runtime.
>
> Can you please stop spreading this FUD?
For many many SoCs today, this is not true. Their drivers are required
to be built in and will not work as modules, as we are seeing the
patches try to fix.
> As I said before, Arm64 kernels have supported more than one specific
> hardware platform at runtime from the beginning of the arm64 port
> (assumed the needed platform support has been enabled in the kernel
> config, of course).
> Even most arm32 kernels support this, since the introduction of the
> CONFIG_ARCH_MULTIPLATFORM option. In fact every recently
> introduced arm32 platform is usually v7, and must conform to this.
> The sole exceptions are very old platforms, and the v4/v5/v6/v7 split
> due to architectural issues (the latter still support clusters of
> platforms in a single kernel).
I think the confusion here is that this really does not work well, if at
all, on most "high end" SoC chips due to the collection of different
things all of the vendors ship to their customers. This is the work
that is trying to be fixed up here.
And look at the driver core work for many driver subsystems to be fixed
up just to get a single kernel image to work on multiple platforms.
Just because older ones did it, doesn't mean it actually works today :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] irqchip/stm32-exti: map direct event to irq parent
From: Alexandre Torgue @ 2020-07-17 13:11 UTC (permalink / raw)
To: Marc Zyngier
Cc: kernel test robot, Thomas Gleixner, Jason Cooper, kbuild-all,
linux-arm-kernel, linux-kernel, linux-gpio, linux-stm32, marex
In-Reply-To: <87eepaxqot.wl-maz@kernel.org>
On 7/17/20 2:37 PM, Marc Zyngier wrote:
> On Fri, 10 Jul 2020 10:37:47 +0100,
> Alexandre Torgue <alexandre.torgue@st.com> wrote:
>>
>> Hi Marc,
>>
>> On 7/10/20 11:31 AM, Marc Zyngier wrote:
>>> Alexandre,
>>>
>>> On Wed, 08 Jul 2020 05:57:24 +0100,
>>> kernel test robot <lkp@intel.com> wrote:
>>>>
>>>> [1 <text/plain; us-ascii (7bit)>]
>>>> Hi Alexandre,
>>>>
>>>> I love your patch! Perhaps something to improve:
>>>>
>>>> [auto build test WARNING on stm32/stm32-next]
>>>> [also build test WARNING on soc/for-next v5.8-rc4 next-20200707]
>>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>>> And when submitting patch, we suggest to use as documented in
>>>> https://git-scm.com/docs/git-format-patch]
>>>>
>>>> url: https://github.com/0day-ci/linux/commits/Alexandre-Torgue/irqchip-stm32-exti-map-direct-event-to-irq-parent/20200706-161327
>>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
>>>> config: arm-randconfig-s031-20200707 (attached as .config)
>>>> compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
>>>> reproduce:
>>>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>> chmod +x ~/bin/make.cross
>>>> # apt-get install sparse
>>>> # sparse version: v0.6.2-31-gabbfd661-dirty
>>>> # save the attached .config to linux build tree
>>>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm
>>>>
>>>> If you fix the issue, kindly add following tag as appropriate
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>> In file included from include/linux/build_bug.h:5,
>>>> from include/linux/bits.h:23,
>>>> from include/linux/bitops.h:5,
>>>> from drivers/irqchip/irq-stm32-exti.c:8:
>>>> drivers/irqchip/irq-stm32-exti.c: In function 'stm32_exti_h_domain_alloc':
>>>> drivers/irqchip/irq-stm32-exti.c:683:23: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
>>>> 683 | if (desc->irq_parent >= 0) {
>>>> | ^~
>>>> include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
>>>> 58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
>>>> | ^~~~
>>>>>> drivers/irqchip/irq-stm32-exti.c:683:2: note: in expansion of macro 'if'
>>>> 683 | if (desc->irq_parent >= 0) {
>>>
>>> Do you plan to address this? Looks like an actual bug to me.
>>
>> I'll fix it in v2, I was just waiting for other comments before
>> sending a v2. Do you prefer I send a v2 with this fix, and you'll do
>> your review on this v2 ?
>
> I'm certainly not going to review something that has such a glaring
> issue.
Ok, I'll then send v2 quickly.
>
> M.
>
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Arnd Bergmann @ 2020-07-17 13:02 UTC (permalink / raw)
To: Greg KH
Cc: Linus Walleij, Geert Uytterhoeven, Catalin Marinas,
Bjorn Andersson, oleksandr.suvorov@toradex.com, Fabio Estevam,
Anson Huang, Jon Corbet, Will Deacon, Russell King,
Bartosz Golaszewski, Andreas Kemnade, dl-linux-imx, Sascha Hauer,
open list:GPIO SUBSYSTEM, John Stultz, hverkuil-cisco@xs4all.nl,
Adam Ford, Linux ARM, linux-kernel@vger.kernel.org, Leo Li,
Vinod Koul, Sascha Hauer, Olof Johansson, Shawn Guo
In-Reply-To: <20200717121436.GA2953399@kroah.com>
On Fri, Jul 17, 2020 at 2:16 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Jul 17, 2020 at 02:01:16PM +0200, Linus Walleij wrote:
> > While I am a big fan of the Android GKI initiative this needs to be aligned
> > with the Linux core maintainers, so let's ask Greg. I am also paging
> > John Stultz on this: he is close to this action.
> >
> > They both know the Android people very well.
> >
> > So there is a rationale like this going on: in order to achieve GKI goals
> > and have as much as possible of the Linux kernel stashed into loadable
> > kernel modules, it has been elevated to modus operandi amongst
> > the developers pushing this change that it is OK to pile up a load of
> > modules that cannot ever be unloaded.
>
> Why can't the module be unloaded? Is it just because they never
> implement the proper "remove all resources allocated" logic in a remove
> function, or something else?
For the core kernel parts, it's usually for the lack of tracking of who
is using the resource provided by the driver, as the subsystems tend
to be written around x86's "everything is built-in" model.
For instance, a PCIe host bridge might rely on the IOMMU, a
clock controller, an interrupt controller, a pin controller and a reset
controller. The host bridge can still be probed at reduced functionality
if some of these are missing, or it can use deferred probing when
some others are missing at probe time.
If we want all of drivers to be unloaded again, we need to do one
of two things:
a) track dependencies, so that removing one of the devices
underneath leads to everything depending on it to get removed
as well or will be notified about it going away and can stop using
it. This is the model used in the network subsystem, where
any ethernet driver can be unloaded and everything using the
device gets torn down.
b) use reference counting on the device or (at the minimum)
try_module_get()/module_put() calls for all such resources
so as long as the pci host bridge is there, so none of the devices
it uses will go away when they are still used.
Traditionally, we would have considered the PCIe host bridge to
be a fundamental part of the system, implying that everything it
uses is also fundamental, and there was no need to track
usage at all, just to ensure the probing is done in the right order.
> > As a minimum requirement I would expect this to be marked by
> >
> > struct device_driver {
> > (...)
> > /* This module absolutely cannot be unbound */
> > .suppress_bind_attrs = true;
> > };
>
> No, that's not what bind/unbind is really for. That's a per-subsystem
> choice as to if you want to allow devices to be added/removed from
> drivers at runtime. It has nothing to do with module load/unload.
It's a one-way dependency: If we can't allow the device to be
unbound, then we also should not allow module unloading because
that forces an unbind.
Arnd
^ permalink raw reply
* gpiolib gpio_chrdev_release duration is about 30 ms
From: Maxim Kochetkov @ 2020-07-17 12:56 UTC (permalink / raw)
To: linux-gpio; +Cc: bgolaszewski, linus.walleij
Hi.
I'm using libgpiod in userspace.
I have 6 gpiochip's on my board.
gpiod_line_find takes about 300ms to find GPIO line.
gpiod_line_find calls gpiod_foreach_chip
then gpiod_chip_iter_next
then gpiod_chip_close then close(chip->fd)
then we are going to kernel gpiolib gpio_chrdev_release
then atomic_notifier_chain_unregister
then synchronize_rcu()
synchronize_rcu takes about 30 ms (6*30ms=280ms)
I tried to remove synchronize_rcu from atomic_notifier_chain_unregister
and gpiod_line_find takes about 2ms now.
^ permalink raw reply
* Re: [PATCH] irqchip/irq-bcm7038-l1: Allow building on ARM 32-bit
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, linux-kernel,
Florian Fainelli
Cc: marex, linux-arm-kernel, linux-gpio, linux-stm32,
open list:BROADCOM BMIPS MIPS ARCHITECTURE,
open list:BROADCOM BMIPS MIPS ARCHITECTURE
In-Reply-To: <20200709234141.4901-1-f.fainelli@gmail.com>
On Thu, 9 Jul 2020 16:41:41 -0700, Florian Fainelli wrote:
> We need to have a definition for cpu_logical_map[] which on ARM
> platforms is provided by asm/smp_plat.h. This header is not
> automatically included from linux/smp.h and untangling it is a bit
> difficult.
Applied to irq/irqchip-5.9, thanks!
[1/1] irqchip/irq-bcm7038-l1: Allow building on ARM 32-bit
commit: 52b350cbc94721d87f087d1a5800f9531c2d682c
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH 0/6] irqchip: Broadcom STB interrupt controller updates
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, linux-kernel,
Florian Fainelli
Cc: marex, linux-arm-kernel, linux-gpio, linux-stm32,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:BROADCOM BMIPS MIPS ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rob Herring
In-Reply-To: <20200709223016.989-1-f.fainelli@gmail.com>
On Thu, 9 Jul 2020 15:30:10 -0700, Florian Fainelli wrote:
> This patch series contains a number of updates for Broadcom STB L2
> interrupt controllers to enable them as wake-up interrupt controllers,
> and add missing compatible strings that should be matched.
>
> Thanks!
>
> Florian Fainelli (3):
> dt-bindings: interrupt-controller: Document Broadcom STB HIF L2
> dt-bindings: interrupt-controller: Document UPG auxiliary L2
> irqchip/brcmstb-l2: Match UPG_AUX_AON_INTR2 compatible
>
> [...]
Applied to irq/irqchip-5.9, thanks!
[1/6] irqchip/bcm7120-l2: Set controller as wake-up source
commit: f4ccb74569aaf839c2830382e902dd50d564df55
[2/6] irqchip/brcmstb-l2: Set controller as wake-up source
commit: c8d8d6fc478a30f3e8ea5372664dd2a808c4311e
[3/6] dt-bindings: interrupt-controller: Document Broadcom STB HIF L2
commit: 90b06e2dc4d1e8e9311a5275d53f61d90b61efdc
[4/6] irqchip/brcmstb-l2: Match HIF_SPI_INTR2 compatible
commit: 9ac793dc5c97691152818305974299604c67e110
[5/6] dt-bindings: interrupt-controller: Document UPG auxiliary L2
commit: 03a7ac47c14c7ef50742a34b3cfba1a47a578a03
[6/6] irqchip/brcmstb-l2: Match UPG_AUX_AON_INTR2 compatible
commit: 240e176a96187ee84e63626ca0d1aac92da503aa
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH v3 0/8] irqchip: Fix some issues and do some code cleanups about Loongson
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, Tiezhu Yang
Cc: linux-kernel, marex, linux-arm-kernel, linux-gpio, linux-stm32,
Jiaxun Yang
In-Reply-To: <1594087972-21715-1-git-send-email-yangtiezhu@loongson.cn>
On Tue, 7 Jul 2020 10:12:44 +0800, Tiezhu Yang wrote:
> Check the return value of irq_domain_translate_onecell() and
> irq_domain_translate_twocell(), fix potential resource leak
> and dead lock, do some code cleanups about Loongson to make
> it more clean and readable.
>
> v2:
> - In order to avoid git send-email failed, make the related patches
> about Loongson into a new patch series and add "Fixes" tag
> v3:
> - Add a new patch "irqchip/loongson-liointc: Fix potential dead lock"
> - Fix another typo in loongson,liointc.yaml
>
> [...]
Applied to irq/irqchip-5.9, thanks!
[1/8] irqchip/loongson-htpic: Remove redundant kfree operation
commit: f90fafecf4880b9785da85feb9b3e6d85fbf2287
[2/8] irqchip/loongson-htpic: Remove unneeded select of I8259
commit: 85efd6059ae1a99e5e7205f37e910fd41dfa0ade
[3/8] irqchip/loongson-htvec: Fix potential resource leak
commit: 652d54e77a438cf38a5731d8f9983c81e72dc429
[4/8] irqchip/loongson-htvec: Check return value of irq_domain_translate_onecell()
commit: dbec37048d27cee36e103e113b5f9b1852bfe997
[5/8] irqchip/loongson-pch-pic: Check return value of irq_domain_translate_twocell()
commit: 66a535c495f72e1deacc37dfa34acca2a06e3578
[6/8] irqchip/loongson-pch-msi: Remove unneeded variable
commit: b10cbca8f03dd10dc241395a639d488f4144e986
[7/8] irqchip/loongson-liointc: Fix potential dead lock
commit: fa03587cad9bd32aa552377de4f05c50181a35a8
[8/8] dt-bindings: interrupt-controller: Fix typos in loongson,liointc.yaml
commit: 1055df97676a31df0b46942cb8bf66eb37ae318f
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH] irqchip: ativic32: constify irq_domain_ops
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, Masahiro Yamada
Cc: linux-kernel, marex, linux-arm-kernel, linux-gpio, linux-stm32
In-Reply-To: <20200714173857.477422-1-masahiroy@kernel.org>
On Wed, 15 Jul 2020 02:38:57 +0900, Masahiro Yamada wrote:
> This is passed to irq_domain_add_linear(), which accepts a pointer
> to a const structure.
Applied to irq/irqchip-5.9, thanks!
[1/1] irqchip/ativic32: Constify irq_domain_ops
commit: 605a2cf566e130c77fc2cc77fac37fb901fc868a
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH v3 0/3] Allow for qcom-pdc to be loadable as a module
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, John Stultz,
lkml
Cc: marex, linux-arm-kernel, linux-gpio, linux-stm32, Joerg Roedel,
Andy Gross, Todd Kjos, iommu, Linus Walleij, Greg Kroah-Hartman,
Maulik Shah, Lina Iyer, linux-arm-msm, Bjorn Andersson,
Saravana Kannan
In-Reply-To: <20200710231824.60699-1-john.stultz@linaro.org>
On Fri, 10 Jul 2020 23:18:21 +0000, John Stultz wrote:
> This patch series provides exports and config tweaks to allow
> the qcom-pdc driver to be able to be configured as a permement
> modules (particularlly useful for the Android Generic Kernel
> Image efforts).
>
> This was part of a larger patch series, to enable qcom_scm
> driver to be a module as well, but I've split it out as there
> are some outstanding objections I still need to address with
> the follow-on patches, and wanted to see if progress could be
> made on this subset of the series in the meantime.
>
> [...]
Applied to irq/irqchip-5.9, thanks!
[1/3] irqdomain: Export irq_domain_update_bus_token
commit: ce8cefa53c06cd98607125c52c91e74373a893a0
[2/3] genirq: Export irq_chip_retrigger_hierarchy and irq_chip_set_vcpu_affinity_parent
commit: 96703f046c42f8ab15e735953cbfee572c717e2d
[3/3] irqchip/qcom-pdc: Allow QCOM_PDC to be loadable as a permanent module
commit: 5ef7f1bbf9f56c5380c4d876655920cac92008e5
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH -next] irqchip: mips-gic: Make some symbols static
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, Wei Yongjun,
Hulk Robot
Cc: linux-kernel, marex, linux-arm-kernel, linux-gpio, linux-stm32
In-Reply-To: <20200714142245.16124-1-weiyongjun1@huawei.com>
On Tue, 14 Jul 2020 22:22:45 +0800, Wei Yongjun wrote:
> The sparse tool complains as follows:
>
> drivers/irqchip/irq-mips-gic.c:49:1: warning:
> symbol '__pcpu_scope_pcpu_masks' was not declared. Should it be static?
> drivers/irqchip/irq-mips-gic.c:620:6: warning:
> symbol 'gic_ipi_domain_free' was not declared. Should it be static?
> drivers/irqchip/irq-mips-gic.c:634:5: warning:
> symbol 'gic_ipi_domain_match' was not declared. Should it be static?
>
> [...]
Applied to irq/irqchip-5.9, thanks!
[1/1] irqchip/mips-gic: Make local symbols static
commit: 63bf3444359c94d647c2afa79b5e732585469581
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ 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