* Re: [PATCH 1/4 v1] gpio: Add support for hierarchical IRQ domains
From: Brian Masney @ 2019-07-07 1:46 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio, Bartosz Golaszewski, Thomas Gleixner, Marc Zyngier,
Lina Iyer, Jon Hunter, Sowjanya Komatineni, Bitan Biswas,
linux-tegra, David Daney, Masahiro Yamada, Thierry Reding
In-Reply-To: <20190624132531.6184-1-linus.walleij@linaro.org>
Hi Linus,
On Mon, Jun 24, 2019 at 03:25:28PM +0200, Linus Walleij wrote:
> Hierarchical IRQ domains can be used to stack different IRQ
> controllers on top of each other.
>
> Bring hierarchical IRQ domains into the GPIOLIB core with the
> following basic idea:
I got this working with spmi-gpio with two additional changes. See below
for details. Hopefully I'll have time tomorrow evening (GMT-4) to finish
cleaning up what I have so I can send out my series.
> +static int gpiochip_hierarchy_irq_domain_translate(struct irq_domain *d,
> + struct irq_fwspec *fwspec,
> + unsigned long *hwirq,
> + unsigned int *type)
> +{
> + /* We support standard DT translation */
> + if (is_of_node(fwspec->fwnode) && fwspec->param_count == 2) {
> + return irq_domain_translate_twocell(d, fwspec, hwirq, type);
> + }
> +
> + /* This is for board files and others not using DT */
> + if (is_fwnode_irqchip(fwspec->fwnode)) {
> + int ret;
> +
> + ret = irq_domain_translate_twocell(d, fwspec, hwirq, type);
> + if (ret)
> + return ret;
> + WARN_ON(*type == IRQ_TYPE_NONE);
> + return 0;
> + }
> + return -EINVAL;
> +}
>
> [ snip ]
>
> +static const struct irq_domain_ops gpiochip_hierarchy_domain_ops = {
> + .activate = gpiochip_irq_domain_activate,
> + .deactivate = gpiochip_irq_domain_deactivate,
> + .translate = gpiochip_hierarchy_irq_domain_translate,
> + .alloc = gpiochip_hierarchy_irq_domain_alloc,
> + .free = irq_domain_free_irqs_common,
> +};
spmi and ssbi gpio both need to subtract one from the hwirq in the
translate function.
https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c#L956
I'm going to optionally allow overriding the translate() function
pointer as well.
> +static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
> + unsigned int irq,
> + unsigned int nr_irqs,
> + void *data)
> +{
> + struct gpio_chip *gc = d->host_data;
> + irq_hw_number_t hwirq;
> + unsigned int type = IRQ_TYPE_NONE;
> + struct irq_fwspec *fwspec = data;
> + int ret;
> + int i;
> +
> + chip_info(gc, "called %s\n", __func__);
> +
> + ret = gpiochip_hierarchy_irq_domain_translate(d, fwspec, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + chip_info(gc, "allocate IRQ %d..%d, hwirq %lu..%lu\n",
> + irq, irq + nr_irqs - 1,
> + hwirq, hwirq + nr_irqs - 1);
> +
> + for (i = 0; i < nr_irqs; i++) {
> + struct irq_fwspec parent_fwspec;
> + unsigned int parent_hwirq;
> + unsigned int parent_type;
> + struct gpio_irq_chip *girq = &gc->irq;
> +
> + ret = girq->child_to_parent_hwirq(gc, hwirq, type,
> + &parent_hwirq, &parent_type);
> + if (ret) {
> + chip_err(gc, "can't look up hwirq %lu\n", hwirq);
> + return ret;
> + }
> + chip_info(gc, "found parent hwirq %u\n", parent_hwirq);
> +
> + /*
> + * We set handle_bad_irq because the .set_type() should
> + * always be invoked and set the right type of handler.
> + */
> + irq_domain_set_info(d,
> + irq + i,
> + hwirq + i,
> + gc->irq.chip,
> + gc,
> + handle_bad_irq,
^^^^^^^^^^
In order to get this working, I had to change handle_bad_irq to
handle_level_irq otherwise I get this attempted NULL pointer
dereference:
[ 2.260256] Unable to handle kernel NULL pointer dereference at virtual address 00000018
[ 2.263388] pgd = (ptrval)
[ 2.271624] [00000018] *pgd=00000000
[ 2.274149] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[ 2.277877] Modules linked in:
[ 2.283174] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc7-next-20190701-00017-g58c27736c3cb-dirty #34
[ 2.286043] Hardware name: Generic DT based system
[ 2.295687] PC is at irq_chip_ack_parent+0x8/0x10
[ 2.300540] LR is at __irq_do_set_handler+0x1b4/0x1bc
[ 2.305309] pc : [<c0372af4>] lr : [<c0373f6c>] psr: a0000093
[ 2.310343] sp : de899be0 ip : c100f06c fp : 00000001
[ 2.316420] r10: 0000004e r9 : 00000000 r8 : 00000000
[ 2.321626] r7 : 00000000 r6 : 00000000 r5 : c036eea4 r4 : c4e7a600
[ 2.326839] r3 : 00000000 r2 : 00000000 r1 : c0372aec r0 : c4e7d5c0
[ 2.333438] Flags: NzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
[ 2.339947] Control: 10c5787d Table: 0020406a DAC: 00000051
[ 2.347152] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[ 2.352967] Stack: (0xde899be0 to 0xde89a000)
[ 2.359063] 9be0: 00000000 00000000 c1004c48 c4e7a600 c036eea4 c0373fc0 60000013 921471da
[ 2.363322] 9c00: 0000004e 00000000 c036eea4 00000000 00000000 c0375d44 c4df124c 921471da
[ 2.371480] 9c20: c4df124c c4df124c 0000004e 00000000 c4de8900 c06cf28c c4df124c c036eea4
[ 2.379640] 9c40: 00000000 00000000 c1004c48 c0da5e28 c036eea4 c0da5e48 de899c7c 00000001
[ 2.387800] 9c60: 00000000 000000c1 00000000 00000001 c4e7a600 c036e394 c0d74960 00000000
[ 2.395959] 9c80: 0000004f c0af60b0 00000000 00000000 00000002 0000004e c4df0600 00000001
[ 2.404120] 9ca0: 00000000 ffffffff 00000000 c4de8900 c4e7d5c0 921471da c4e27940 c06cf120
[ 2.412278] 9cc0: 00000001 0000004e 0000004e c4de8900 c0e172b4 00000001 c4e7d5c0 c0376bac
[ 2.420438] 9ce0: 00000000 c0834274 00000000 c1004c48 c4de8900 de899d4c 00000000 de9d9c10
[ 2.428598] 9d00: 00000001 00000000 c4e27940 c03771d4 de899d4c 00000000 00000000 c06ca5b0
[ 2.436759] 9d20: de9d9db8 00000001 00000000 921471da c1004c48 c4df124c c4de8900 00000001
[ 2.444919] 9d40: de9d9c10 c06c90b8 c27065bc def6e270 00000002 00000002 00000000 c10a2374
[ 2.453077] 9d60: c4d6f810 de9d9c10 de899d80 6f697067 de9d0073 a0000013 c1004c48 00000000
[ 2.461237] 9d80: a0000013 c098977c c4e27940 921471da c4e27940 921471da c4d6f810 c4e6ec8c
[ 2.469399] 9da0: c4e2795c 00000000 c27065bc c06c8624 c4e6ec8c c092b1b8 00000001 c27065bc
[ 2.477558] 9dc0: c10c59b8 c4e6ec60 00000000 c4e81000 00000000 def757d4 c4e2795c c4e6ec40
[ 2.485717] 9de0: de9d9c00 c092ab44 c092aac8 c092ab60 00000000 de9d9c10 00000000 c10a2608
[ 2.493876] 9e00: 00000000 c10c59b8 00000000 c10a2608 0000000e c0817c00 c110dcf4 de9d9c10
[ 2.502037] 9e20: c110dcf8 c0815738 c10a2608 de9d9c10 c0f004a4 de9d9c10 c10a2608 c10a2608
[ 2.510197] 9e40: c0815fa4 00000000 c0f56838 c0f56858 c0f004a4 c0815bf4 c0f56858 c0989428
[ 2.518355] 9e60: c0bbf5c8 de9d9c10 00000000 c10a2608 c0815fa4 00000000 c0f56838 c0f56858
[ 2.526515] 9e80: c0f004a4 c0815f9c 00000000 c10a2608 de9d9c10 c0816058 de9dab34 c1004c48
[ 2.534677] 9ea0: c10a2608 c0813908 c1095780 de825858 de9dab34 921471da de82586c c10a2608
[ 2.542835] 9ec0: c4e96780 c1095780 00000000 c0814aa0 c0df2cac c1004c48 ffffe000 c10a2608
[ 2.550994] 9ee0: c1004c48 ffffe000 c0f39074 c0816be4 c10b97c0 c1004c48 ffffe000 c0302f7c
[ 2.559153] 9f00: 00000000 c0f004a4 c10be278 00000000 00000007 c0d59294 c0dd2fe4 00000000
[ 2.567314] 9f20: 00000000 c1004c48 c0d69934 c0d5934c 00002cc0 dffffbf8 dffffc0d 921471da
[ 2.575475] 9f40: 00000000 c10b97c0 dffffa80 921471da c10b97c0 c0f6414c 00000008 c10d2cc0
[ 2.583633] 9f60: c10d2cc0 c0f01204 00000007 00000007 00000000 c0f004a4 000000db 00000000
[ 2.591793] 9f80: c0af5ba8 00000000 c0af5ba8 00000000 00000000 00000000 00000000 00000000
[ 2.599951] 9fa0: 00000000 c0af5bb0 00000000 c03010e8 00000000 00000000 00000000 00000000
[ 2.608111] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 2.616271] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 2.624430] [<c0372af4>] (irq_chip_ack_parent) from [<c0373f6c>] (__irq_do_set_handler+0x1b4/0x1bc)
[ 2.632584] [<c0373f6c>] (__irq_do_set_handler) from [<c0373fc0>] (__irq_set_handler+0x4c/0x78)
[ 2.641441] [<c0373fc0>] (__irq_set_handler) from [<c0375d44>] (irq_domain_set_info+0x38/0x4c)
[ 2.650126] [<c0375d44>] (irq_domain_set_info) from [<c06cf28c>] (gpiochip_hierarchy_irq_domain_alloc+0x16c/0x22c)
[ 2.658808] [<c06cf28c>] (gpiochip_hierarchy_irq_domain_alloc) from [<c0376bac>] (__irq_domain_alloc_irqs+0x12c/0x320)
[ 2.669134] [<c0376bac>] (__irq_domain_alloc_irqs) from [<c03771d4>] (irq_create_fwspec_mapping+0x27c/0x344)
[ 2.679808] [<c03771d4>] (irq_create_fwspec_mapping) from [<c06c90b8>] (gpiochip_to_irq+0x6c/0xa0)
[ 2.689793] [<c06c90b8>] (gpiochip_to_irq) from [<c06c8624>] (gpiod_to_irq+0x48/0x64)
[ 2.698558] [<c06c8624>] (gpiod_to_irq) from [<c092b1b8>] (gpio_keys_probe+0x4b4/0x8e8)
[ 2.706461] [<c092b1b8>] (gpio_keys_probe) from [<c0817c00>] (platform_drv_probe+0x48/0x98)
[ 2.714272] [<c0817c00>] (platform_drv_probe) from [<c0815738>] (really_probe+0x108/0x40c)
[ 2.722599] [<c0815738>] (really_probe) from [<c0815bf4>] (driver_probe_device+0x78/0x1c4)
[ 2.730934] [<c0815bf4>] (driver_probe_device) from [<c0815f9c>] (device_driver_attach+0x58/0x60)
[ 2.739182] [<c0815f9c>] (device_driver_attach) from [<c0816058>] (__driver_attach+0xb4/0x154)
[ 2.748121] [<c0816058>] (__driver_attach) from [<c0813908>] (bus_for_each_dev+0x74/0xb4)
[ 2.756628] [<c0813908>] (bus_for_each_dev) from [<c0814aa0>] (bus_add_driver+0x1c0/0x200)
[ 2.764875] [<c0814aa0>] (bus_add_driver) from [<c0816be4>] (driver_register+0x7c/0x114)
[ 2.773035] [<c0816be4>] (driver_register) from [<c0302f7c>] (do_one_initcall+0x54/0x2a0)
[ 2.781284] [<c0302f7c>] (do_one_initcall) from [<c0f01204>] (kernel_init_freeable+0x2d4/0x36c)
[ 2.789357] [<c0f01204>] (kernel_init_freeable) from [<c0af5bb0>] (kernel_init+0x8/0x110)
[ 2.797860] [<c0af5bb0>] (kernel_init) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
[ 2.806181] Exception stack(0xde899fb0 to 0xde899ff8)
[ 2.813653] 9fa0: 00000000 00000000 00000000 00000000
[ 2.818788] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 2.826943] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 2.835102] Code: 0592301c e12fff13 e5900018 e5903010 (e5933018)
[ 2.841513] ---[ end trace e31675e4bfb4e93f ]---
The parent's irq_chip struct isn't populated yet and the error occurs
here:
void irq_chip_ack_parent(struct irq_data *data)
{
data = data->parent_data;
data->chip->irq_ack(data);
^^^^
We haven't called irq_domain_alloc_irqs_parent() yet, which is fine.
__irq_do_set_handler() has a special check for handle_bad_irq():
https://elixir.bootlin.com/linux/latest/source/kernel/irq/chip.c#L974
I'm not sure what the proper fix is here and not going to dig into this
anymore this evening.
> diff --git a/Documentation/driver-api/gpio/driver.rst b/Documentation/driver-api/gpio/driver.rst
> index 1ce7fcd0f989..3099c7fbefdb 100644
> --- a/Documentation/driver-api/gpio/driver.rst
> +++ b/Documentation/driver-api/gpio/driver.rst
I'm still on linux next-20190701. Does this patch series of yours
require any other patches? I get a merge conflict against driver.rst.
Everything else applies cleanly. I honestly haven't looked in detail
about the conflicts.
Brian
^ permalink raw reply
* Re: [PATCH] gpio: tegra: Fix build error without CONFIG_DEBUG_FS
From: Linus Walleij @ 2019-07-06 22:34 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Yuehaibing, Thierry Reding, Jonathan Hunter, LKML, linux-gpio,
linux-tegra
In-Reply-To: <CAMpxmJU0=w=htiY3CL9GDBU+waBjV0X7yh1UG6ip5BiV3J7nXA@mail.gmail.com>
On Fri, Jul 5, 2019 at 2:59 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
> Wait, nevermind. I think that commit a4de43049a1d ("gpio: tegra:
> Clean-up debugfs initialisation") is wrong and we missed that. Linus
> what do you think about reverting it?
OK I reverted it.
Linus
^ permalink raw reply
* [gpio:devel-gpio-driver-isolation 61/66] arch/arm/plat-orion/gpio.c:306:18: error: implicit declaration of function 'desc_to_gpio'; did you mean 'dev_to_psd'?
From: kbuild test robot @ 2019-07-06 20:41 UTC (permalink / raw)
To: Linus Walleij; +Cc: kbuild-all, linux-gpio
[-- Attachment #1: Type: text/plain, Size: 3475 bytes --]
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/linusw/linux-gpio.git devel-gpio-driver-isolation
head: 6c5bf689e0dca2a882193a202a96222dcba184e9
commit: f92518de7f35387d034f1b688354ff0ab4ad448d [61/66] ARM: plat-orion: Include the right header
config: arm-multi_v5_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout f92518de7f35387d034f1b688354ff0ab4ad448d
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/arm/plat-orion/gpio.c: In function 'orion_gpio_led_blink_set':
>> arch/arm/plat-orion/gpio.c:306:18: error: implicit declaration of function 'desc_to_gpio'; did you mean 'dev_to_psd'? [-Werror=implicit-function-declaration]
unsigned gpio = desc_to_gpio(desc);
^~~~~~~~~~~~
dev_to_psd
>> arch/arm/plat-orion/gpio.c:315:3: error: implicit declaration of function 'gpio_set_value'; did you mean 'pin_is_valid'? [-Werror=implicit-function-declaration]
gpio_set_value(gpio, state);
^~~~~~~~~~~~~~
pin_is_valid
cc1: some warnings being treated as errors
vim +306 arch/arm/plat-orion/gpio.c
ff3e660b Arnaud Patard 2012-04-18 302
c673a2b4 Mika Westerberg 2014-10-31 303 int orion_gpio_led_blink_set(struct gpio_desc *desc, int state,
ff3e660b Arnaud Patard 2012-04-18 304 unsigned long *delay_on, unsigned long *delay_off)
ff3e660b Arnaud Patard 2012-04-18 305 {
c673a2b4 Mika Westerberg 2014-10-31 @306 unsigned gpio = desc_to_gpio(desc);
ff3e660b Arnaud Patard 2012-04-18 307
ff3e660b Arnaud Patard 2012-04-18 308 if (delay_on && delay_off && !*delay_on && !*delay_off)
ff3e660b Arnaud Patard 2012-04-18 309 *delay_on = *delay_off = ORION_BLINK_HALF_PERIOD;
ff3e660b Arnaud Patard 2012-04-18 310
ff3e660b Arnaud Patard 2012-04-18 311 switch (state) {
ff3e660b Arnaud Patard 2012-04-18 312 case GPIO_LED_NO_BLINK_LOW:
ff3e660b Arnaud Patard 2012-04-18 313 case GPIO_LED_NO_BLINK_HIGH:
ff3e660b Arnaud Patard 2012-04-18 314 orion_gpio_set_blink(gpio, 0);
ff3e660b Arnaud Patard 2012-04-18 @315 gpio_set_value(gpio, state);
ff3e660b Arnaud Patard 2012-04-18 316 break;
ff3e660b Arnaud Patard 2012-04-18 317 case GPIO_LED_BLINK:
ff3e660b Arnaud Patard 2012-04-18 318 orion_gpio_set_blink(gpio, 1);
ff3e660b Arnaud Patard 2012-04-18 319 }
ff3e660b Arnaud Patard 2012-04-18 320 return 0;
ff3e660b Arnaud Patard 2012-04-18 321 }
ff3e660b Arnaud Patard 2012-04-18 322 EXPORT_SYMBOL_GPL(orion_gpio_led_blink_set);
ff3e660b Arnaud Patard 2012-04-18 323
07332318 Lennert Buytenhek 2008-10-20 324
:::::: The code at line 306 was first introduced by commit
:::::: c673a2b4008103525a3cf21bedf15ffac37bfef0 leds: leds-gpio: Convert gpio_blink_set() to use GPIO descriptors
:::::: TO: Mika Westerberg <mika.westerberg@linux.intel.com>
:::::: CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32050 bytes --]
^ permalink raw reply
* Re: [PATCH 7/7] gpiolib: Use spinlock_t instead of struct spinlock
From: Linus Walleij @ 2019-07-06 18:15 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-kernel@vger.kernel.org, Thomas Gleixner, Peter Zijlstra,
Bartosz Golaszewski, open list:GPIO SUBSYSTEM
In-Reply-To: <20190704153803.12739-8-bigeasy@linutronix.de>
On Thu, Jul 4, 2019 at 5:38 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> For spinlocks the type spinlock_t should be used instead of "struct
> spinlock".
>
> Use spinlock_t for spinlock's definition.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: linux-gpio@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Patch applied with Bartosz' ACK!
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] gpio: gpiolib-of.c: Add of_node_put() before break
From: Nishka Dasgupta @ 2019-07-06 13:34 UTC (permalink / raw)
To: linus.walleij, bgolaszewski, linux-gpio; +Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a break from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the break.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/gpio/gpiolib-of.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index aec7bd86ae7e..c9325efc1783 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -154,6 +154,7 @@ static void of_gpio_flags_quirks(struct device_node *np,
of_node_full_name(child));
*flags |= OF_GPIO_ACTIVE_LOW;
}
+ of_node_put(child);
break;
}
}
--
2.19.1
^ permalink raw reply related
* Re: [PATCH 39/39] docs: gpio: add sysfs interface to the admin-guide
From: Mauro Carvalho Chehab @ 2019-07-06 11:43 UTC (permalink / raw)
To: Linus Walleij
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
linux-kernel@vger.kernel.org, Jonathan Corbet,
Bartosz Golaszewski, Rafael J. Wysocki, Len Brown, Harry Wei,
Alex Shi, open list:GPIO SUBSYSTEM, ACPI Devel Maling List
In-Reply-To: <CACRpkdbBA612W0x6Y-dwe3E4dhH2ospmn+m2YJ8Sh_Um6XGYhA@mail.gmail.com>
Em Wed, 3 Jul 2019 10:44:38 +0200
Linus Walleij <linus.walleij@linaro.org> escreveu:
> On Fri, Jun 28, 2019 at 2:30 PM Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org> wrote:
>
> > While this is stated as obsoleted, the sysfs interface described
> > there is still valid, and belongs to the admin-guide.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
>
> This doesn't apply to my tree because of dependencies in the
> index
Yeah, this /39 patch series heavily touch the index files.
Better to merge them altogether.
> so I guess it's best if you merge it:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Thanks!
Mauro
>
> Yours,
> Linus Walleij
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH v2] gpiolib: Preserve desc->flags when setting state
From: Linus Walleij @ 2019-07-05 21:55 UTC (permalink / raw)
To: Chris Packham
Cc: Bartosz Golaszewski, Ricardo Ribalda Delgado,
open list:GPIO SUBSYSTEM, linux-kernel@vger.kernel.org
In-Reply-To: <20190704042027.18966-1-chris.packham@alliedtelesis.co.nz>
Hi Chris,
thanks for your patch!
On Thu, Jul 4, 2019 at 6:21 AM Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:
> desc->flags may already have values set by of_gpiochip_add() so make
> sure that this isn't undone when setting the initial direction.
>
> Fixes: 3edfb7bd76bd1cba ("gpiolib: Show correct direction from the beginning")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>
> Notes:
> Changes in v2:
> - add braces to avoid ambiguious else warning
This is almost the solution!
> - if (chip->get_direction && gpiochip_line_is_valid(chip, i))
> - desc->flags = !chip->get_direction(chip, i) ?
> - (1 << FLAG_IS_OUT) : 0;
> - else
> - desc->flags = !chip->direction_input ?
> - (1 << FLAG_IS_OUT) : 0;
> + if (chip->get_direction && gpiochip_line_is_valid(chip, i)) {
> + if (!chip->get_direction(chip, i))
> + set_bit(FLAG_IS_OUT, &desc->flags);
You need to clear_bit() in the reverse case. We just learned we can't
assume anything about the flags here, like just assign them.
> + } else {
> + if (!chip->direction_input)
> + set_bit(FLAG_IS_OUT, &desc->flags);
Same here.
Yours,
Linus Walleij
^ permalink raw reply
* Re: gpio desc flags being lost
From: Linus Walleij @ 2019-07-05 21:53 UTC (permalink / raw)
To: Chris Packham
Cc: Ricardo Ribalda Delgado, Bartosz Golaszewski,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <755abbb5b984414a966367c323f62e59@svr-chch-ex1.atlnz.lc>
On Wed, Jul 3, 2019 at 11:30 PM Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:
> The problem is caused by commit 3edfb7bd76bd1cba ("gpiolib: Show correct
> direction from the beginning"). I'll see if I can whip up a patch to fix it.
Oh. I think:
if (chip->get_direction && gpiochip_line_is_valid(chip, i))
desc->flags = !chip->get_direction(chip, i) ?
(1 << FLAG_IS_OUT) : 0;
else
desc->flags = !chip->direction_input ?
(1 << FLAG_IS_OUT) : 0;
Needs to have desc->flags |= ... &= ~
if (!chip->get_direction(chip, i))
desc->flags |= (1 << FLAG_IS_OUT);
else
desc->flags &= ~(1 << FLAG_IS_OUT);
And the same for direction_input()
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] gpiolib: fix incorrect IRQ requesting of an active-low lineevent
From: Linus Walleij @ 2019-07-05 21:46 UTC (permalink / raw)
To: Michael.Wu
Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
linux-kernel@vger.kernel.org, mvp.kutali
In-Reply-To: <5DB475451BAA174CB158B5E897FC1525920E9FD0@MBS-6F-DAG.vivotek.tw>
On Fri, Jul 5, 2019 at 12:35 PM <Michael.Wu@vatics.com> wrote:
> For example, there is a button which drives level to be low when it is pushed, and drivers level to be high when it is released.
> We want to catch the event when the button is pushed.
>
> In user space we configure a line event with the following code:
>
> req.handleflags = GPIOHANDLE_REQUEST_INPUT;
> req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;
But *THIS* is the case that should have
GPIOHANDLE_REQUEST_ACTIVE_LOW, because you push
the button to activate it (it is inactive when not pushed).
Also this should have GPIOEVENT_REQUEST_RISING_EDGE.
> Run the same logic on another board which the polarity of the button is inverted. The button drives level to be high when it is pushed.
> For the inverted level case, we have to add flag GPIOHANDLE_REQUEST_ACTIVE_LOW:
>
> req.handleflags = GPIOHANDLE_REQUEST_INPUT | GPIOHANDLE_REQUEST_ACTIVE_LOW;
> req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;
This one should not be active low.
And also have GPIOEVENT_REQUEST_RISING_EDGE.
However I agree that the semantic should change as in the
patch, it makes most logical sense.
The reason it looks as it does is because GPIO line values
and interrupts are two separate subsystems inside the kernel
with their own flags (as you've seen).
But you are right, userspace has no idea about that and should
not have to care.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
From: Geert Uytterhoeven @ 2019-07-05 16:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Alexander Graf, Peter Maydell,
Paolo Bonzini, Magnus Damm
Cc: linux-gpio, qemu-devel, linux-renesas-soc, linux-kernel,
Geert Uytterhoeven
GPIO controllers are exported to userspace using /dev/gpiochip*
character devices. Access control to these devices is provided by
standard UNIX file system permissions, on an all-or-nothing basis:
either a GPIO controller is accessible for a user, or it is not.
Currently no mechanism exists to control access to individual GPIOs.
Hence add a virtual GPIO driver to aggregate existing GPIOs (up to 32),
and expose them as a new gpiochip. This is useful for implementing
access control, and assigning a set of GPIOs to a specific user.
Furthermore, it would simplify and harden exporting GPIOs to a virtual
machine, as the VM can just grab the full virtual GPIO controller, and
no longer needs to care about which GPIOs to grab and which not,
reducing the attack surface.
Virtual GPIO controllers are instantiated by writing to the "new_device"
attribute file in sysfs:
$ echo "<gpiochipA> <gpioA1> [<gpioA2> ...]"
"[, <gpiochipB> <gpioB1> [<gpioB2> ...]] ...]"
> /sys/bus/platform/drivers/gpio-virt-agg/new_device
Likewise, virtual GPIO controllers can be destroyed after use:
$ echo gpio-virt-agg.<N> \
> /sys/bus/platform/drivers/gpio-virt-agg/delete_device
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Aggregating GPIOs and exposing them as a new gpiochip was suggested in
response to my proof-of-concept for GPIO virtualization with QEMU[1][2].
Sample session on r8a7791/koelsch:
- Disable the leds node in arch/arm/boot/dts/r8a7791-koelsch.dts
- Create virtual aggregators:
$ echo "e6052000.gpio 19 20" \
> /sys/bus/platform/drivers/gpio-virt-agg/new_device
gpio-virt-agg gpio-virt-agg.0: GPIO 0 => e6052000.gpio/19
gpio-virt-agg gpio-virt-agg.0: GPIO 1 => e6052000.gpio/20
gpiochip_find_base: found new base at 778
gpio gpiochip8: (gpio-virt-agg.0): added GPIO chardev (254:8)
gpiochip_setup_dev: registered GPIOs 778 to 779 on device: gpiochip8 (gpio-virt-agg.0)
$ echo "e6052000.gpio 21, e6050000.gpio 20 21 22" \
> /sys/bus/platform/drivers/gpio-virt-agg/new_device
gpio-virt-agg gpio-virt-agg.1: GPIO 0 => e6052000.gpio/21
gpio-virt-agg gpio-virt-agg.1: GPIO 1 => e6050000.gpio/20
gpio-virt-agg gpio-virt-agg.1: GPIO 2 => e6050000.gpio/21
gpio-virt-agg gpio-virt-agg.1: GPIO 3 => e6050000.gpio/22
gpiochip_find_base: found new base at 774
gpio gpiochip9: (gpio-virt-agg.1): added GPIO chardev (254:9)
gpiochip_setup_dev: registered GPIOs 774 to 777 on device: gpiochip9 (gpio-virt-agg.1)
- Adjust permissions on /dev/gpiochip[89] (optional)
- Control LEDs:
$ gpioset gpiochip8 0=0 1=1 # LED6 OFF, LED7 ON
$ gpioset gpiochip8 0=1 1=0 # LED6 ON, LED7 OFF
$ gpioset gpiochip9 0=0 # LED8 OFF
$ gpioset gpiochip9 0=1 # LED8 ON
- Destroy virtual aggregators:
$ echo gpio-virt-agg.0 \
> /sys/bus/platform/drivers/gpio-virt-agg/delete_device
$ echo gpio-virt-agg.1 \
> /sys/bus/platform/drivers/gpio-virt-agg/delete_device
Thanks for your comments!
References:
- [1] "[PATCH QEMU POC] Add a GPIO backend"
(https://lore.kernel.org/linux-renesas-soc/20181003152521.23144-1-geert+renesas@glider.be/)
- [2] "Getting To Blinky: Virt Edition / Making device pass-through
work on embedded ARM"
(https://fosdem.org/2019/schedule/event/vai_getting_to_blinky/)
---
drivers/gpio/Kconfig | 9 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-virt-agg.c | 390 +++++++++++++++++++++++++++++++++++
3 files changed, 400 insertions(+)
create mode 100644 drivers/gpio/gpio-virt-agg.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f1f02dac324e52b6..8aff4d9626dee110 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1475,3 +1475,12 @@ config GPIO_MOCKUP
it.
endif
+
+config GPIO_VIRT_AGG
+ tristate "GPIO Virtual Aggregator"
+ depends on GPIOLIB
+ help
+ This enabled the GPIO Virtual Aggregator, which provides a way to
+ aggregate existing GPIOs into a new virtual GPIO device.
+ This is useful for assigning a collection of GPIOs to a user, or
+ exported them to a virtual machine.
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 0a494052c1e845ee..32e885b7f3aa4eee 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -152,6 +152,7 @@ obj-$(CONFIG_GPIO_UCB1400) += gpio-ucb1400.o
obj-$(CONFIG_GPIO_UNIPHIER) += gpio-uniphier.o
obj-$(CONFIG_GPIO_VF610) += gpio-vf610.o
obj-$(CONFIG_GPIO_VIPERBOARD) += gpio-viperboard.o
+obj-$(CONFIG_GPIO_VIRT_AGG) += gpio-virt-agg.o
obj-$(CONFIG_GPIO_VR41XX) += gpio-vr41xx.o
obj-$(CONFIG_GPIO_VX855) += gpio-vx855.o
obj-$(CONFIG_GPIO_WHISKEY_COVE) += gpio-wcove.o
diff --git a/drivers/gpio/gpio-virt-agg.c b/drivers/gpio/gpio-virt-agg.c
new file mode 100644
index 0000000000000000..20e5f22beed9d385
--- /dev/null
+++ b/drivers/gpio/gpio-virt-agg.c
@@ -0,0 +1,390 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// GPIO Virtual Aggregator
+//
+// Copyright (C) 2019 Glider bvba
+
+#include <linux/gpio/driver.h>
+#include <linux/idr.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+
+#include "gpiolib.h"
+
+#define DRV_NAME "gpio-virt-agg"
+#define MAX_GPIOS 32
+
+struct gpio_virt_agg_entry {
+ struct platform_device *pdev;
+};
+
+struct gpio_virt_agg_priv {
+ struct gpio_chip chip;
+ struct gpio_desc *desc[MAX_GPIOS];
+};
+
+static DEFINE_MUTEX(gpio_virt_agg_lock); /* protects idr */
+static DEFINE_IDR(gpio_virt_agg_idr);
+
+static int gpio_virt_agg_get_direction(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+
+ return gpiod_get_direction(priv->desc[offset]);
+}
+
+static int gpio_virt_agg_direction_input(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+
+ return gpiod_direction_input(priv->desc[offset]);
+}
+
+static int gpio_virt_agg_direction_output(struct gpio_chip *chip,
+ unsigned int offset, int value)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+
+ return gpiod_direction_output(priv->desc[offset], value);
+}
+
+static int gpio_virt_agg_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+
+ return gpiod_get_value(priv->desc[offset]);
+}
+
+static int gpio_virt_agg_get_multiple(struct gpio_chip *chip,
+ unsigned long *mask, unsigned long *bits)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+ DECLARE_BITMAP(values, MAX_GPIOS) = { 0 };
+ struct gpio_desc *desc[MAX_GPIOS];
+ unsigned int i, j = 0;
+ int ret;
+
+ for_each_set_bit(i, mask, priv->chip.ngpio)
+ desc[j++] = priv->desc[i];
+
+ ret = gpiod_get_array_value(j, desc, NULL, values);
+ if (ret)
+ return ret;
+
+ for_each_set_bit(i, mask, priv->chip.ngpio)
+ __assign_bit(i, bits, test_bit(j++, values));
+
+ return 0;
+}
+
+static void gpio_virt_agg_set(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+
+ gpiod_set_value(priv->desc[offset], value);
+}
+
+static void gpio_virt_agg_set_multiple(struct gpio_chip *chip,
+ unsigned long *mask,
+ unsigned long *bits)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+ DECLARE_BITMAP(values, MAX_GPIOS);
+ struct gpio_desc *desc[MAX_GPIOS];
+ unsigned int i, j = 0;
+
+ for_each_set_bit(i, mask, priv->chip.ngpio) {
+ __assign_bit(j, values, test_bit(i, bits));
+ desc[j++] = priv->desc[i];
+ }
+
+ gpiod_set_array_value(j, desc, NULL, values);
+}
+
+static int gpio_virt_agg_set_config(struct gpio_chip *chip,
+ unsigned int offset, unsigned long config)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+
+ chip = priv->desc[offset]->gdev->chip;
+ if (chip->set_config)
+ return chip->set_config(chip, offset, config);
+
+ // FIXME gpiod_set_transitory() expects success if not implemented
+ return -ENOTSUPP;
+}
+
+static int gpio_virt_agg_init_valid_mask(struct gpio_chip *chip)
+{
+ struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
+ unsigned int i;
+
+ for (i = 0; i < priv->chip.ngpio; i++) {
+ if (gpiochip_line_is_valid(priv->desc[i]->gdev->chip,
+ gpio_chip_hwgpio(priv->desc[i])))
+ set_bit(i, chip->valid_mask);
+ }
+
+ return 0;
+}
+
+static int gpiochip_match_label(struct gpio_chip *chip, void *data)
+{
+ return !strcmp(chip->label, data);
+}
+
+static struct gpio_chip *gpiochip_find_by_label(const char *label)
+{
+ return gpiochip_find((void *)label, gpiochip_match_label);
+}
+
+static ssize_t new_device_store(struct device_driver *driver, const char *buf,
+ size_t count)
+{
+ struct gpio_virt_agg_entry *gva;
+ struct platform_device *pdev;
+ int res, id;
+
+ gva = kzalloc(sizeof(*gva), GFP_KERNEL);
+ if (!gva)
+ return -ENOMEM;
+
+ mutex_lock(&gpio_virt_agg_lock);
+ id = idr_alloc(&gpio_virt_agg_idr, gva, 0, 0, GFP_KERNEL);
+ mutex_unlock(&gpio_virt_agg_lock);
+
+ if (id < 0) {
+ res = id;
+ goto free_gva;
+ }
+
+ /* kernfs guarantees string termination, so count + 1 is safe */
+ pdev = platform_device_register_data(NULL, DRV_NAME, id, buf,
+ count + 1);
+ if (IS_ERR(pdev)) {
+ res = PTR_ERR(pdev);
+ goto remove_idr;
+ }
+
+ gva->pdev = pdev;
+ return count;
+
+remove_idr:
+ mutex_lock(&gpio_virt_agg_lock);
+ idr_remove(&gpio_virt_agg_idr, id);
+ mutex_unlock(&gpio_virt_agg_lock);
+free_gva:
+ kfree(gva);
+ return res;
+}
+
+static DRIVER_ATTR_WO(new_device);
+
+static ssize_t delete_device_store(struct device_driver *driver,
+ const char *buf, size_t count)
+{
+ struct gpio_virt_agg_entry *gva;
+ int id;
+
+ if (strncmp(buf, DRV_NAME ".", strlen(DRV_NAME ".")))
+ return -EINVAL;
+
+ id = simple_strtoul(buf + strlen(DRV_NAME "."), NULL, 10);
+
+ mutex_lock(&gpio_virt_agg_lock);
+ gva = idr_remove(&gpio_virt_agg_idr, id);
+ mutex_unlock(&gpio_virt_agg_lock);
+
+ if (!gva) {
+ pr_info("Cannot find %s.%d\n", DRV_NAME, id);
+ return -ENOENT;
+ }
+
+ platform_device_unregister(gva->pdev);
+ kfree(gva);
+ return count;
+}
+static DRIVER_ATTR_WO(delete_device);
+
+static struct attribute *gpio_virt_agg_attrs[] = {
+ &driver_attr_new_device.attr,
+ &driver_attr_delete_device.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(gpio_virt_agg);
+
+static int gpio_virt_agg_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const char *param = dev_get_platdata(dev);
+ struct gpio_virt_agg_priv *priv;
+ const char *label = NULL;
+ struct gpio_chip *chip;
+ struct gpio_desc *desc;
+ unsigned int offset;
+ int error, i;
+ char *s;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ error = -ENOMEM;
+ goto fail;
+ }
+
+ for (i = 0; i < MAX_GPIOS; i++) {
+ if (*param == '\0' || *param == '\n')
+ break;
+
+ if (*param == ',') {
+ if (label) {
+ devm_kfree(dev, label);
+ label = NULL;
+ }
+ for (param++; *param == ' '; param++) ;
+ }
+
+ if (!label) {
+ s = strchr(param, ' ');
+ if (!s) {
+ dev_info(dev, "Missing gpiochip\n");
+ error = -EINVAL;
+ goto fail;
+ }
+ label = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
+ (int)(s - param), param);
+ if (!label) {
+ error = -ENOMEM;
+ goto fail;
+ }
+
+ chip = gpiochip_find_by_label(label);
+ if (!chip) {
+ dev_info(dev, "Cannot find gpiochip %s\n",
+ label);
+ error = -ENODEV;
+ goto fail;
+ }
+
+ for (param = s + 1; *param == ' '; param++) ;
+ }
+
+ offset = simple_strtoul(param, &s, 10);
+
+ desc = gpiochip_get_desc(chip, offset);
+ if (IS_ERR(desc)) {
+ error = PTR_ERR(desc);
+ dev_info(dev, "Cannot get GPIO %s/%u: %d\n", label,
+ offset, error);
+ goto fail;
+ }
+
+ error = gpiod_request(desc, dev_name(dev));
+ if (error) {
+ dev_info(dev, "Cannot request GPIO %s/%u: %d\n", label,
+ offset, error);
+ goto fail;
+ }
+
+ dev_dbg(dev, "GPIO %u => %s/%u\n", i, label, offset);
+ priv->desc[i] = desc;
+
+ if (gpiod_cansleep(desc))
+ priv->chip.can_sleep = true;
+ if (desc->gdev->chip->set_config)
+ priv->chip.set_config = gpio_virt_agg_set_config;
+ if (desc->gdev->chip->need_valid_mask) {
+ priv->chip.need_valid_mask = true;
+ priv->chip.init_valid_mask =
+ gpio_virt_agg_init_valid_mask;
+ }
+
+ for (param = s; *param == ' '; param++) ;
+ }
+ if (i == MAX_GPIOS)
+ dev_warn(&pdev->dev,
+ "Too many gpios specified, truncating to %u\n",
+ MAX_GPIOS);
+
+ priv->chip.label = dev_name(dev);
+ priv->chip.parent = dev;
+ priv->chip.owner = THIS_MODULE;
+ priv->chip.get_direction = gpio_virt_agg_get_direction;
+ priv->chip.direction_input = gpio_virt_agg_direction_input;
+ priv->chip.direction_output = gpio_virt_agg_direction_output;
+ priv->chip.get = gpio_virt_agg_get;
+ priv->chip.get_multiple = gpio_virt_agg_get_multiple;
+ priv->chip.set = gpio_virt_agg_set;
+ priv->chip.set_multiple = gpio_virt_agg_set_multiple;
+ priv->chip.base = -1;
+ priv->chip.ngpio = i;
+ platform_set_drvdata(pdev, priv);
+
+ error = gpiochip_add_data(&priv->chip, priv);
+ if (error)
+ goto fail;
+
+ return 0;
+
+fail:
+ while (i-- > 0)
+ gpiod_free(priv->desc[i]);
+
+ return error;
+}
+
+static int gpio_virt_agg_remove(struct platform_device *pdev)
+{
+ struct gpio_virt_agg_priv *priv = platform_get_drvdata(pdev);
+ unsigned int i;
+
+ gpiochip_remove(&priv->chip);
+
+ for (i = 0; i < priv->chip.ngpio; i++)
+ gpiod_free(priv->desc[i]);
+
+ return 0;
+}
+
+static struct platform_driver gpio_virt_agg_driver = {
+ .probe = gpio_virt_agg_probe,
+ .remove = gpio_virt_agg_remove,
+ .driver = {
+ .name = DRV_NAME,
+ .groups = gpio_virt_agg_groups,
+ },
+};
+
+static int __init gpio_virt_agg_init(void)
+{
+ return platform_driver_register(&gpio_virt_agg_driver);
+}
+module_init(gpio_virt_agg_init);
+
+static int __exit gpio_virt_agg_idr_remove(int id, void *p, void *data)
+{
+ struct gpio_virt_agg_entry *gva = p;
+
+ platform_device_unregister(gva->pdev);
+ kfree(gva);
+ return 0;
+}
+
+static void __exit gpio_virt_agg_exit(void)
+{
+ mutex_lock(&gpio_virt_agg_lock);
+ idr_for_each(&gpio_virt_agg_idr, gpio_virt_agg_idr_remove, NULL);
+ idr_destroy(&gpio_virt_agg_idr);
+ mutex_unlock(&gpio_virt_agg_lock);
+
+ platform_driver_unregister(&gpio_virt_agg_driver);
+}
+module_exit(gpio_virt_agg_exit);
+
+MODULE_AUTHOR("Geert Uytterhoeven <geert+renesas@glider.be>");
+MODULE_DESCRIPTION("GPIO Virtual Aggregator");
+MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related
* [PATCH] docs/pinctrl: fix compile errors in example code
From: Luca Ceresoli @ 2019-07-05 14:30 UTC (permalink / raw)
To: linux-gpio
Cc: Luca Ceresoli, Linus Walleij, Jonathan Corbet, linux-doc,
linux-kernel
The code in the example does not build for a few trivial errors: type
mismatch in callback, missing semicolon. Fix them to help newcomers using
the example as a starting point.
Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
Documentation/driver-api/pinctl.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/driver-api/pinctl.rst b/Documentation/driver-api/pinctl.rst
index 2bb1bc484278..3d2deaf48841 100644
--- a/Documentation/driver-api/pinctl.rst
+++ b/Documentation/driver-api/pinctl.rst
@@ -638,8 +638,8 @@ group of pins would work something like this::
}
static int foo_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
- unsigned ** const pins,
- unsigned * const num_pins)
+ const unsigned ** pins,
+ unsigned * num_pins)
{
*pins = (unsigned *) foo_groups[selector].pins;
*num_pins = foo_groups[selector].num_pins;
@@ -705,7 +705,7 @@ group of pins would work something like this::
{
u8 regbit = (1 << selector + group);
- writeb((readb(MUX)|regbit), MUX)
+ writeb((readb(MUX)|regbit), MUX);
return 0;
}
--
2.22.0
^ permalink raw reply related
* Re: [PATCH] gpio: tegra: Fix build error without CONFIG_DEBUG_FS
From: Bartosz Golaszewski @ 2019-07-05 12:59 UTC (permalink / raw)
To: Yuehaibing
Cc: Linus Walleij, Thierry Reding, Jonathan Hunter, LKML, linux-gpio,
linux-tegra
In-Reply-To: <CAMpxmJUeg1jVZdCeiRqTZykBZNPGAeQkaNfA7qc1zt+sL9HPjA@mail.gmail.com>
pt., 5 lip 2019 o 14:56 Bartosz Golaszewski
<bgolaszewski@baylibre.com> napisał(a):
>
> pt., 5 lip 2019 o 14:44 Yuehaibing <yuehaibing@huawei.com> napisał(a):
> >
> > On 2019/7/5 20:40, Bartosz Golaszewski wrote:
> > > pt., 5 lip 2019 o 14:34 YueHaibing <yuehaibing@huawei.com> napisał(a):
> > >>
> > >> If CONFIG_DEBUG_FS is not set, building fails:
> > >>
> > >> drivers/gpio/gpio-tegra.c: In function tegra_gpio_probe:
> > >> drivers/gpio/gpio-tegra.c:665:2: error: implicit declaration of function debugfs_create_file;
> > >> did you mean bus_create_file? [-Werror=implicit-function-declaration]
> > >> debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
> > >> ^~~~~~~~~~~~~~~~~~~
> > >> bus_create_file
> > >> drivers/gpio/gpio-tegra.c:666:9: error: tegra_dbg_gpio_fops undeclared (first use in this function);
> > >> did you mean tegra_gpio_pm_ops?
> > >> &tegra_dbg_gpio_fops);
> > >> ^~~~~~~~~~~~~~~~~~~
> > >> tegra_gpio_pm_ops
> > >>
> > >> Reported-by: Hulk Robot <hulkci@huawei.com>
> > >> Fixes: a4de43049a1d ("gpio: tegra: Clean-up debugfs initialisation")
> > >> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > >> ---
> > >> drivers/gpio/gpio-tegra.c | 2 ++
> > >> 1 file changed, 2 insertions(+)
> > >>
> > >> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> > >> index 59b99d8..40fd6bd 100644
> > >> --- a/drivers/gpio/gpio-tegra.c
> > >> +++ b/drivers/gpio/gpio-tegra.c
> > >> @@ -662,8 +662,10 @@ static int tegra_gpio_probe(struct platform_device *pdev)
> > >> }
> > >> }
> > >>
> > >> +#ifdef CONFIG_DEBUG_FS
> > >> debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
> > >> &tegra_dbg_gpio_fops);
> > >> +#endif
> > >>
> > >> return 0;
> > >> }
> > >> --
> > >> 2.7.4
> > >>
> > >>
> > >
> > > Nack, there are inline stubs for all debugfs functions in
> > > ./include/linux/debugfs.h if CONFIG_DEBUG_FS is not selected. Just
> > > move the #include <linux/debugfs.h> to the top of the source file.
> >
> > yes, agree this, but 'tegra_dbg_gpio_fops' is still undeclared.
> >
> > >
> > > Bart
> > >
> > > .
> > >
> >
>
> Can you attach the config you're using?
>
> Bart
Wait, nevermind. I think that commit a4de43049a1d ("gpio: tegra:
Clean-up debugfs initialisation") is wrong and we missed that. Linus
what do you think about reverting it?
Bart
^ permalink raw reply
* Re: [PATCH] gpio: tegra: Fix build error without CONFIG_DEBUG_FS
From: Bartosz Golaszewski @ 2019-07-05 12:56 UTC (permalink / raw)
To: Yuehaibing
Cc: Linus Walleij, Thierry Reding, Jonathan Hunter, LKML, linux-gpio,
linux-tegra
In-Reply-To: <74ffe8ea-e6fb-bd2a-42bd-08392eb27c69@huawei.com>
pt., 5 lip 2019 o 14:44 Yuehaibing <yuehaibing@huawei.com> napisał(a):
>
> On 2019/7/5 20:40, Bartosz Golaszewski wrote:
> > pt., 5 lip 2019 o 14:34 YueHaibing <yuehaibing@huawei.com> napisał(a):
> >>
> >> If CONFIG_DEBUG_FS is not set, building fails:
> >>
> >> drivers/gpio/gpio-tegra.c: In function tegra_gpio_probe:
> >> drivers/gpio/gpio-tegra.c:665:2: error: implicit declaration of function debugfs_create_file;
> >> did you mean bus_create_file? [-Werror=implicit-function-declaration]
> >> debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
> >> ^~~~~~~~~~~~~~~~~~~
> >> bus_create_file
> >> drivers/gpio/gpio-tegra.c:666:9: error: tegra_dbg_gpio_fops undeclared (first use in this function);
> >> did you mean tegra_gpio_pm_ops?
> >> &tegra_dbg_gpio_fops);
> >> ^~~~~~~~~~~~~~~~~~~
> >> tegra_gpio_pm_ops
> >>
> >> Reported-by: Hulk Robot <hulkci@huawei.com>
> >> Fixes: a4de43049a1d ("gpio: tegra: Clean-up debugfs initialisation")
> >> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> >> ---
> >> drivers/gpio/gpio-tegra.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> >> index 59b99d8..40fd6bd 100644
> >> --- a/drivers/gpio/gpio-tegra.c
> >> +++ b/drivers/gpio/gpio-tegra.c
> >> @@ -662,8 +662,10 @@ static int tegra_gpio_probe(struct platform_device *pdev)
> >> }
> >> }
> >>
> >> +#ifdef CONFIG_DEBUG_FS
> >> debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
> >> &tegra_dbg_gpio_fops);
> >> +#endif
> >>
> >> return 0;
> >> }
> >> --
> >> 2.7.4
> >>
> >>
> >
> > Nack, there are inline stubs for all debugfs functions in
> > ./include/linux/debugfs.h if CONFIG_DEBUG_FS is not selected. Just
> > move the #include <linux/debugfs.h> to the top of the source file.
>
> yes, agree this, but 'tegra_dbg_gpio_fops' is still undeclared.
>
> >
> > Bart
> >
> > .
> >
>
Can you attach the config you're using?
Bart
^ permalink raw reply
* Re: [PATCH] gpiolib: fix incorrect IRQ requesting of an active-low lineevent
From: Bartosz Golaszewski @ 2019-07-05 12:50 UTC (permalink / raw)
To: Michael Wu; +Cc: Linus Walleij, linux-gpio, LKML, mvp.kutali
In-Reply-To: <5DB475451BAA174CB158B5E897FC1525920E9FD0@MBS-6F-DAG.vivotek.tw>
pt., 5 lip 2019 o 12:35 <Michael.Wu@vatics.com> napisał(a):
>
> Hi Bartosz,
>
> For example, there is a button which drives level to be low when it is pushed, and drivers level to be high when it is released.
> We want to catch the event when the button is pushed.
>
> In user space we configure a line event with the following code:
>
> req.handleflags = GPIOHANDLE_REQUEST_INPUT;
> req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;
>
> and we hope to get "falling" events by reading the device node:
>
> while (1) {
> read(fd, &dat,sizeof(dat));
> if (dat.id == 0) {
> printf("button pushed\n");
> }
> }
>
> Run the same logic on another board which the polarity of the button is inverted. The button drives level to be high when it is pushed.
> For the inverted level case, we have to add flag GPIOHANDLE_REQUEST_ACTIVE_LOW:
>
> req.handleflags = GPIOHANDLE_REQUEST_INPUT | GPIOHANDLE_REQUEST_ACTIVE_LOW;
> req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;
>
> At the result, there are no any events been caught when the button is pushed.
> By the way, button releasing will emit a "falling" event.
>
>
> Sincerely,
>
> Michael Wu
First: please don't top-post on the mailing list.
Second: have you even built the version you sent? Because I'm getting this:
drivers/gpio/gpiolib.c: In function ‘lineevent_create’:
drivers/gpio/gpiolib.c:963:4: error: ‘IRQ_TRIGGER_RISING’ undeclared
(first use in this function)
IRQ_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
^~~~~~~~~~~~~~~~~~
And third: after fixing the define, this indeed looks like a bug and
I'll need to add a test for that to libgpiod once it's upstream.
Strange we didn't catch it before.
Please send a fixed version and add a Cc tag for stable. Nice catch!
Best regards
Bartosz Golaszewski
^ permalink raw reply
* Re: [PATCH] gpio: tegra: Fix build error without CONFIG_DEBUG_FS
From: Yuehaibing @ 2019-07-05 12:44 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Thierry Reding, Jonathan Hunter, LKML, linux-gpio,
linux-tegra
In-Reply-To: <CAMpxmJVZHJKQ7bbHo=T9R99qguF315bZ=YVRrCdqti2SyzAnDg@mail.gmail.com>
On 2019/7/5 20:40, Bartosz Golaszewski wrote:
> pt., 5 lip 2019 o 14:34 YueHaibing <yuehaibing@huawei.com> napisał(a):
>>
>> If CONFIG_DEBUG_FS is not set, building fails:
>>
>> drivers/gpio/gpio-tegra.c: In function tegra_gpio_probe:
>> drivers/gpio/gpio-tegra.c:665:2: error: implicit declaration of function debugfs_create_file;
>> did you mean bus_create_file? [-Werror=implicit-function-declaration]
>> debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
>> ^~~~~~~~~~~~~~~~~~~
>> bus_create_file
>> drivers/gpio/gpio-tegra.c:666:9: error: tegra_dbg_gpio_fops undeclared (first use in this function);
>> did you mean tegra_gpio_pm_ops?
>> &tegra_dbg_gpio_fops);
>> ^~~~~~~~~~~~~~~~~~~
>> tegra_gpio_pm_ops
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Fixes: a4de43049a1d ("gpio: tegra: Clean-up debugfs initialisation")
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>> drivers/gpio/gpio-tegra.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
>> index 59b99d8..40fd6bd 100644
>> --- a/drivers/gpio/gpio-tegra.c
>> +++ b/drivers/gpio/gpio-tegra.c
>> @@ -662,8 +662,10 @@ static int tegra_gpio_probe(struct platform_device *pdev)
>> }
>> }
>>
>> +#ifdef CONFIG_DEBUG_FS
>> debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
>> &tegra_dbg_gpio_fops);
>> +#endif
>>
>> return 0;
>> }
>> --
>> 2.7.4
>>
>>
>
> Nack, there are inline stubs for all debugfs functions in
> ./include/linux/debugfs.h if CONFIG_DEBUG_FS is not selected. Just
> move the #include <linux/debugfs.h> to the top of the source file.
yes, agree this, but 'tegra_dbg_gpio_fops' is still undeclared.
>
> Bart
>
> .
>
^ permalink raw reply
* Re: [PATCH] gpio: tegra: Fix build error without CONFIG_DEBUG_FS
From: Bartosz Golaszewski @ 2019-07-05 12:40 UTC (permalink / raw)
To: YueHaibing
Cc: Linus Walleij, Thierry Reding, Jonathan Hunter, LKML, linux-gpio,
linux-tegra
In-Reply-To: <20190705123220.54008-1-yuehaibing@huawei.com>
pt., 5 lip 2019 o 14:34 YueHaibing <yuehaibing@huawei.com> napisał(a):
>
> If CONFIG_DEBUG_FS is not set, building fails:
>
> drivers/gpio/gpio-tegra.c: In function tegra_gpio_probe:
> drivers/gpio/gpio-tegra.c:665:2: error: implicit declaration of function debugfs_create_file;
> did you mean bus_create_file? [-Werror=implicit-function-declaration]
> debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
> ^~~~~~~~~~~~~~~~~~~
> bus_create_file
> drivers/gpio/gpio-tegra.c:666:9: error: tegra_dbg_gpio_fops undeclared (first use in this function);
> did you mean tegra_gpio_pm_ops?
> &tegra_dbg_gpio_fops);
> ^~~~~~~~~~~~~~~~~~~
> tegra_gpio_pm_ops
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: a4de43049a1d ("gpio: tegra: Clean-up debugfs initialisation")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> drivers/gpio/gpio-tegra.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> index 59b99d8..40fd6bd 100644
> --- a/drivers/gpio/gpio-tegra.c
> +++ b/drivers/gpio/gpio-tegra.c
> @@ -662,8 +662,10 @@ static int tegra_gpio_probe(struct platform_device *pdev)
> }
> }
>
> +#ifdef CONFIG_DEBUG_FS
> debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
> &tegra_dbg_gpio_fops);
> +#endif
>
> return 0;
> }
> --
> 2.7.4
>
>
Nack, there are inline stubs for all debugfs functions in
./include/linux/debugfs.h if CONFIG_DEBUG_FS is not selected. Just
move the #include <linux/debugfs.h> to the top of the source file.
Bart
^ permalink raw reply
* [PATCH] gpio: tegra: Fix build error without CONFIG_DEBUG_FS
From: YueHaibing @ 2019-07-05 12:32 UTC (permalink / raw)
To: linus.walleij, bgolaszewski, thierry.reding, jonathanh
Cc: linux-kernel, linux-gpio, linux-tegra, YueHaibing
If CONFIG_DEBUG_FS is not set, building fails:
drivers/gpio/gpio-tegra.c: In function tegra_gpio_probe:
drivers/gpio/gpio-tegra.c:665:2: error: implicit declaration of function debugfs_create_file;
did you mean bus_create_file? [-Werror=implicit-function-declaration]
debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
^~~~~~~~~~~~~~~~~~~
bus_create_file
drivers/gpio/gpio-tegra.c:666:9: error: tegra_dbg_gpio_fops undeclared (first use in this function);
did you mean tegra_gpio_pm_ops?
&tegra_dbg_gpio_fops);
^~~~~~~~~~~~~~~~~~~
tegra_gpio_pm_ops
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: a4de43049a1d ("gpio: tegra: Clean-up debugfs initialisation")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/gpio/gpio-tegra.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 59b99d8..40fd6bd 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -662,8 +662,10 @@ static int tegra_gpio_probe(struct platform_device *pdev)
}
}
+#ifdef CONFIG_DEBUG_FS
debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
&tegra_dbg_gpio_fops);
+#endif
return 0;
}
--
2.7.4
^ permalink raw reply related
* RE: [PATCH] gpiolib: fix incorrect IRQ requesting of an active-low lineevent
From: Michael.Wu @ 2019-07-05 10:35 UTC (permalink / raw)
To: bgolaszewski; +Cc: linus.walleij, linux-gpio, linux-kernel, mvp.kutali
In-Reply-To: <CAMpxmJUzaEREeUxCu2BCV12Huv7K=yeUSKntA5RGMfOQbnxaFg@mail.gmail.com>
Hi Bartosz,
For example, there is a button which drives level to be low when it is pushed, and drivers level to be high when it is released.
We want to catch the event when the button is pushed.
In user space we configure a line event with the following code:
req.handleflags = GPIOHANDLE_REQUEST_INPUT;
req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;
and we hope to get "falling" events by reading the device node:
while (1) {
read(fd, &dat,sizeof(dat));
if (dat.id == 0) {
printf("button pushed\n");
}
}
Run the same logic on another board which the polarity of the button is inverted. The button drives level to be high when it is pushed.
For the inverted level case, we have to add flag GPIOHANDLE_REQUEST_ACTIVE_LOW:
req.handleflags = GPIOHANDLE_REQUEST_INPUT | GPIOHANDLE_REQUEST_ACTIVE_LOW;
req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;
At the result, there are no any events been caught when the button is pushed.
By the way, button releasing will emit a "falling" event.
Sincerely,
Michael Wu
-----Original Message-----
From: Bartosz Golaszewski [mailto:bgolaszewski@baylibre.com]
Sent: Friday, July 05, 2019 5:33 PM
To: Michael.Wu(吳忠益)
Cc: Linus Walleij; linux-gpio; LKML; mvp.kutali@gmail.com
Subject: Re: [PATCH] gpiolib: fix incorrect IRQ requesting of an active-low lineevent
pt., 5 lip 2019 o 11:30 Michael Wu <michael.wu@vatics.com> napisał(a):
>
> When a pin is active-low, logical trigger edge should be inverted
> to match the same interrupt opportunity.
>
> For example, a button pushed trigger falling edge in ACTIVE_HIGH
> case; in ACTIVE_LOW case, the button pushed trigger rising edge.
> For user space the IRQ requesting doesn't need to do any
> modification except to configuring GPIOHANDLE_REQUEST_ACTIVE_LOW.
>
> Signed-off-by: Michael Wu <michael.wu@vatics.com>
> ---
> drivers/gpio/gpiolib.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e013d417a936..b98466a05091 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -956,9 +956,11 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
> }
>
> if (eflags & GPIOEVENT_REQUEST_RISING_EDGE)
> - irqflags |= IRQF_TRIGGER_RISING;
> + irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
> + IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
> if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE)
> - irqflags |= IRQF_TRIGGER_FALLING;
> + irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
> + IRQ_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
> irqflags |= IRQF_ONESHOT;
>
> INIT_KFIFO(le->events);
> --
> 2.17.1
>
Is this something that causes a bug in user-space? Any scenario to reproduce it?
Bart
^ permalink raw reply
* [PATCH] gpiolib: fix incorrect IRQ requesting of an active-low lineevent
From: Michael Wu @ 2019-07-05 9:30 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel; +Cc: mvp.kutali
When a pin is active-low, logical trigger edge should be inverted
to match the same interrupt opportunity.
For example, a button pushed trigger falling edge in ACTIVE_HIGH
case; in ACTIVE_LOW case, the button pushed trigger rising edge.
For user space the IRQ requesting doesn't need to do any
modification except to configuring GPIOHANDLE_REQUEST_ACTIVE_LOW.
Signed-off-by: Michael Wu <michael.wu@vatics.com>
---
drivers/gpio/gpiolib.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d417a936..b98466a05091 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -956,9 +956,11 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
}
if (eflags & GPIOEVENT_REQUEST_RISING_EDGE)
- irqflags |= IRQF_TRIGGER_RISING;
+ irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
+ IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE)
- irqflags |= IRQF_TRIGGER_FALLING;
+ irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
+ IRQ_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
irqflags |= IRQF_ONESHOT;
INIT_KFIFO(le->events);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 7/7] gpiolib: Use spinlock_t instead of struct spinlock
From: Bartosz Golaszewski @ 2019-07-05 9:33 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: LKML, Thomas Gleixner, Peter Zijlstra, Linus Walleij, linux-gpio
In-Reply-To: <20190704153803.12739-8-bigeasy@linutronix.de>
czw., 4 lip 2019 o 17:38 Sebastian Andrzej Siewior
<bigeasy@linutronix.de> napisał(a):
>
> For spinlocks the type spinlock_t should be used instead of "struct
> spinlock".
>
> Use spinlock_t for spinlock's definition.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: linux-gpio@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> drivers/gpio/gpiolib.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> index 7a65dad43932c..7c52c2442173e 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -210,7 +210,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
> struct gpio_array *array_info,
> unsigned long *value_bitmap);
>
> -extern struct spinlock gpio_lock;
> +extern spinlock_t gpio_lock;
> extern struct list_head gpio_devices;
>
> struct gpio_desc {
> --
> 2.20.1
>
Thanks for spotting that!
Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
^ permalink raw reply
* Re: [PATCH] gpiolib: fix incorrect IRQ requesting of an active-low lineevent
From: Bartosz Golaszewski @ 2019-07-05 9:33 UTC (permalink / raw)
To: Michael Wu; +Cc: Linus Walleij, linux-gpio, LKML, mvp.kutali
In-Reply-To: <20190705093031.18182-1-michael.wu@vatics.com>
pt., 5 lip 2019 o 11:30 Michael Wu <michael.wu@vatics.com> napisał(a):
>
> When a pin is active-low, logical trigger edge should be inverted
> to match the same interrupt opportunity.
>
> For example, a button pushed trigger falling edge in ACTIVE_HIGH
> case; in ACTIVE_LOW case, the button pushed trigger rising edge.
> For user space the IRQ requesting doesn't need to do any
> modification except to configuring GPIOHANDLE_REQUEST_ACTIVE_LOW.
>
> Signed-off-by: Michael Wu <michael.wu@vatics.com>
> ---
> drivers/gpio/gpiolib.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e013d417a936..b98466a05091 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -956,9 +956,11 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
> }
>
> if (eflags & GPIOEVENT_REQUEST_RISING_EDGE)
> - irqflags |= IRQF_TRIGGER_RISING;
> + irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
> + IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
> if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE)
> - irqflags |= IRQF_TRIGGER_FALLING;
> + irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
> + IRQ_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
> irqflags |= IRQF_ONESHOT;
>
> INIT_KFIFO(le->events);
> --
> 2.17.1
>
Is this something that causes a bug in user-space? Any scenario to reproduce it?
Bart
^ permalink raw reply
* Re: [pinctrl:devel 72/79] drivers/pinctrl/aspeed/pinctrl-aspeed.h:547:28: error: field 'pinmux' has incomplete type
From: Andrew Jeffery @ 2019-07-05 1:44 UTC (permalink / raw)
To: Linus Walleij; +Cc: kbuild-all, linux-gpio, kbuild test robot
In-Reply-To: <201907042014.2WXYWvdg%lkp@intel.com>
Hello Linus,
On Thu, 4 Jul 2019, at 22:11, kbuild test robot wrote:
> tree:
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
> head: eb0a2daa45b83d67b69a620243ed844e9dfa671b
> commit: efa5623981b72f6b5f95933d1c36ed2518c2ee4e [72/79] pinctrl:
> aspeed: Split out pinmux from general pinctrl
> config: arm-allmodconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
> reproduce:
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git checkout efa5623981b72f6b5f95933d1c36ed2518c2ee4e
> # save the attached .config to linux build tree
> GCC_VERSION=7.4.0 make.cross ARCH=arm
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
It looks like in fixing the SPDX churn you dropped the include from this hunk:
```
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
index c5918c4a087c..11cc0eb6666b 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
@@ -1,514 +1,15 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2016 IBM Corp.
- */
+/* Copyright (C) 2016,2019 IBM Corp. */
#ifndef PINCTRL_ASPEED
#define PINCTRL_ASPEED
+#include "pinmux-aspeed.h"
+
```
It's missing here: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/diff/drivers/pinctrl/aspeed/pinctrl-aspeed.h?h=devel&id=efa5623981b72f6b5f95933d1c36ed2518c2ee4e
Should I send a fixup? Or do you want me to re-spin the series on top of
v5.2-rc1 and resend?
Andrew
^ permalink raw reply related
* Re: [PATCH v2] pinctrl: msm8998: Squash TSIF pins together
From: Bjorn Andersson @ 2019-07-04 16:35 UTC (permalink / raw)
To: Marc Gonzalez
Cc: Jonathan Neusch?fer, Jeffrey Hugo, Linus Walleij, MSM, Linux ARM,
gpio, DT, Rob Herring
In-Reply-To: <503b2ae8-ead6-70cd-7b21-ce5f5166a23a@free.fr>
On Thu 04 Jul 01:57 PDT 2019, Marc Gonzalez wrote:
> TSIF is the Transport Stream Interface.
> First, rename tsif1 to tsif0, and tsif2 to tsif1.
> Then squash all 5 tsif0 pins into a single function.
> Same for tsif1.
>
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
Thanks for the respin Marc.
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> Changes from v1:
> - Reword commit message for clarity (hopefully)
> - Drop unrelated change in qcom,msm8998-pinctrl.txt
> - CC DT
> ---
> .../bindings/pinctrl/qcom,msm8998-pinctrl.txt | 5 +-
> drivers/pinctrl/qcom/pinctrl-msm8998.c | 76 +++++--------------
> 2 files changed, 20 insertions(+), 61 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,msm8998-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,msm8998-pinctrl.txt
> index 00174f08ba1d..18e3f3d3e3c5 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,msm8998-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,msm8998-pinctrl.txt
> @@ -124,9 +124,8 @@ to specify in a pin configuration subnode:
> qlink_request, qua_mi2s, sd_card, sd_write, sdc40, sdc41,
> sdc42, sdc43, sdc4_clk, sdc4_cmd, sec_mi2s, sp_cmu,
> spkr_i2s, ssbi1, ssc_irq, ter_mi2s, tgu_ch0, tgu_ch1,
> - tsense_pwm1, tsense_pwm2, tsif1_clk, tsif1_data, tsif1_en,
> - tsif1_error, tsif1_sync, tsif2_clk, tsif2_data, tsif2_en,
> - tsif2_error, tsif2_sync, uim1_clk, uim1_data, uim1_present,
> + tsense_pwm1, tsense_pwm2, tsif0, tsif1,
> + uim1_clk, uim1_data, uim1_present,
> uim1_reset, uim2_clk, uim2_data, uim2_present, uim2_reset,
> uim_batt, usb_phy, vfr_1, vsense_clkout, vsense_data0,
> vsense_data1, vsense_mode, wlan1_adc0, wlan1_adc1,
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm8998.c b/drivers/pinctrl/qcom/pinctrl-msm8998.c
> index 00d7b94bc3f1..a05f41fe2706 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm8998.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm8998.c
> @@ -581,16 +581,8 @@ enum msm8998_functions {
> msm_mux_tgu_ch1,
> msm_mux_tsense_pwm1,
> msm_mux_tsense_pwm2,
> - msm_mux_tsif1_clk,
> - msm_mux_tsif1_data,
> - msm_mux_tsif1_en,
> - msm_mux_tsif1_error,
> - msm_mux_tsif1_sync,
> - msm_mux_tsif2_clk,
> - msm_mux_tsif2_data,
> - msm_mux_tsif2_en,
> - msm_mux_tsif2_error,
> - msm_mux_tsif2_sync,
> + msm_mux_tsif0,
> + msm_mux_tsif1,
> msm_mux_uim1_clk,
> msm_mux_uim1_data,
> msm_mux_uim1_present,
> @@ -692,9 +684,6 @@ static const char * const atest_usb13_groups[] = {
> static const char * const bimc_dte1_groups[] = {
> "gpio8", "gpio10",
> };
> -static const char * const tsif1_sync_groups[] = {
> - "gpio9",
> -};
> static const char * const wlan1_adc0_groups[] = {
> "gpio9",
> };
> @@ -863,9 +852,6 @@ static const char * const lpass_slimbus_groups[] = {
> static const char * const sd_write_groups[] = {
> "gpio40",
> };
> -static const char * const tsif1_error_groups[] = {
> - "gpio40",
> -};
> static const char * const blsp_spi6_groups[] = {
> "gpio41", "gpio42", "gpio43", "gpio44",
> };
> @@ -1048,11 +1034,8 @@ static const char * const blsp_uim2_b_groups[] = {
> static const char * const blsp_i2c5_groups[] = {
> "gpio87", "gpio88",
> };
> -static const char * const tsif1_clk_groups[] = {
> - "gpio89",
> -};
> -static const char * const tsif1_en_groups[] = {
> - "gpio90",
> +static const char * const tsif0_groups[] = {
> + "gpio9", "gpio40", "gpio89", "gpio90", "gpio91",
> };
> static const char * const mdp_vsync0_groups[] = {
> "gpio90",
> @@ -1075,17 +1058,14 @@ static const char * const tgu_ch0_groups[] = {
> static const char * const qdss_cti1_b_groups[] = {
> "gpio90", "gpio91",
> };
> -static const char * const tsif1_data_groups[] = {
> - "gpio91",
> -};
> static const char * const sdc4_cmd_groups[] = {
> "gpio91",
> };
> static const char * const tgu_ch1_groups[] = {
> "gpio91",
> };
> -static const char * const tsif2_error_groups[] = {
> - "gpio92",
> +static const char * const tsif1_groups[] = {
> + "gpio92", "gpio93", "gpio94", "gpio95", "gpio96",
> };
> static const char * const sdc43_groups[] = {
> "gpio92",
> @@ -1093,30 +1073,18 @@ static const char * const sdc43_groups[] = {
> static const char * const vfr_1_groups[] = {
> "gpio92",
> };
> -static const char * const tsif2_clk_groups[] = {
> - "gpio93",
> -};
> static const char * const sdc4_clk_groups[] = {
> "gpio93",
> };
> -static const char * const tsif2_en_groups[] = {
> - "gpio94",
> -};
> static const char * const sdc42_groups[] = {
> "gpio94",
> };
> static const char * const sd_card_groups[] = {
> "gpio95",
> };
> -static const char * const tsif2_data_groups[] = {
> - "gpio95",
> -};
> static const char * const sdc41_groups[] = {
> "gpio95",
> };
> -static const char * const tsif2_sync_groups[] = {
> - "gpio96",
> -};
> static const char * const sdc40_groups[] = {
> "gpio96",
> };
> @@ -1355,16 +1323,8 @@ static const struct msm_function msm8998_functions[] = {
> FUNCTION(tgu_ch1),
> FUNCTION(tsense_pwm1),
> FUNCTION(tsense_pwm2),
> - FUNCTION(tsif1_clk),
> - FUNCTION(tsif1_data),
> - FUNCTION(tsif1_en),
> - FUNCTION(tsif1_error),
> - FUNCTION(tsif1_sync),
> - FUNCTION(tsif2_clk),
> - FUNCTION(tsif2_data),
> - FUNCTION(tsif2_en),
> - FUNCTION(tsif2_error),
> - FUNCTION(tsif2_sync),
> + FUNCTION(tsif0),
> + FUNCTION(tsif1),
> FUNCTION(uim1_clk),
> FUNCTION(uim1_data),
> FUNCTION(uim1_present),
> @@ -1396,7 +1356,7 @@ static const struct msm_pingroup msm8998_groups[] = {
> PINGROUP(6, WEST, blsp_spi8, blsp_uart8_a, blsp_i2c8, _, _, _, _, _, _),
> PINGROUP(7, WEST, blsp_spi8, blsp_uart8_a, blsp_i2c8, ddr_bist, _, atest_tsens2, atest_usb1, _, _),
> PINGROUP(8, EAST, blsp_spi4, blsp_uart1_b, blsp_uim1_b, _, ddr_bist, _, wlan1_adc1, atest_usb13, bimc_dte1),
> - PINGROUP(9, EAST, blsp_spi4, blsp_uart1_b, blsp_uim1_b, tsif1_sync, ddr_bist, _, wlan1_adc0, atest_usb12, bimc_dte0),
> + PINGROUP(9, EAST, blsp_spi4, blsp_uart1_b, blsp_uim1_b, tsif0, ddr_bist, _, wlan1_adc0, atest_usb12, bimc_dte0),
> PINGROUP(10, EAST, mdp_vsync_a, blsp_spi4, blsp_uart1_b, blsp_i2c4, ddr_bist, atest_gpsadc1, wlan2_adc1, atest_usb11, bimc_dte1),
> PINGROUP(11, EAST, mdp_vsync_a, edp_lcd, blsp_spi4, blsp_uart1_b, blsp_i2c4, dbg_out, atest_gpsadc0, wlan2_adc0, atest_usb10),
> PINGROUP(12, EAST, mdp_vsync, m_voc, _, _, _, _, _, _, _),
> @@ -1427,7 +1387,7 @@ static const struct msm_pingroup msm8998_groups[] = {
> PINGROUP(37, NORTH, agera_pll, _, _, _, _, _, _, _, _),
> PINGROUP(38, WEST, usb_phy, _, _, _, _, _, _, _, _),
> PINGROUP(39, WEST, lpass_slimbus, _, _, _, _, _, _, _, _),
> - PINGROUP(40, EAST, sd_write, tsif1_error, _, _, _, _, _, _, _),
> + PINGROUP(40, EAST, sd_write, tsif0, _, _, _, _, _, _, _),
> PINGROUP(41, EAST, blsp_spi6, blsp_uart3_b, blsp_uim3_b, _, qdss, _, _, _, _),
> PINGROUP(42, EAST, blsp_spi6, blsp_uart3_b, blsp_uim3_b, _, qdss, _, _, _, _),
> PINGROUP(43, EAST, blsp_spi6, blsp_uart3_b, blsp_i2c6, _, qdss, _, _, _, _),
> @@ -1476,14 +1436,14 @@ static const struct msm_pingroup msm8998_groups[] = {
> PINGROUP(86, EAST, blsp_spi5, blsp_uart2_b, blsp_uim2_b, _, _, _, _, _, _),
> PINGROUP(87, EAST, blsp_spi5, blsp_uart2_b, blsp_i2c5, _, _, _, _, _, _),
> PINGROUP(88, EAST, blsp_spi5, blsp_uart2_b, blsp_i2c5, _, _, _, _, _, _),
> - PINGROUP(89, EAST, tsif1_clk, phase_flag, _, _, _, _, _, _, _),
> - PINGROUP(90, EAST, tsif1_en, mdp_vsync0, mdp_vsync1, mdp_vsync2, mdp_vsync3, blsp1_spi, tgu_ch0, qdss_cti1_b, _),
> - PINGROUP(91, EAST, tsif1_data, sdc4_cmd, tgu_ch1, phase_flag, qdss_cti1_b, _, _, _, _),
> - PINGROUP(92, EAST, tsif2_error, sdc43, vfr_1, phase_flag, _, _, _, _, _),
> - PINGROUP(93, EAST, tsif2_clk, sdc4_clk, _, qdss, _, _, _, _, _),
> - PINGROUP(94, EAST, tsif2_en, sdc42, _, _, _, _, _, _, _),
> - PINGROUP(95, EAST, tsif2_data, sdc41, _, _, _, _, _, _, _),
> - PINGROUP(96, EAST, tsif2_sync, sdc40, phase_flag, _, _, _, _, _, _),
> + PINGROUP(89, EAST, tsif0, phase_flag, _, _, _, _, _, _, _),
> + PINGROUP(90, EAST, tsif0, mdp_vsync0, mdp_vsync1, mdp_vsync2, mdp_vsync3, blsp1_spi, tgu_ch0, qdss_cti1_b, _),
> + PINGROUP(91, EAST, tsif0, sdc4_cmd, tgu_ch1, phase_flag, qdss_cti1_b, _, _, _, _),
> + PINGROUP(92, EAST, tsif1, sdc43, vfr_1, phase_flag, _, _, _, _, _),
> + PINGROUP(93, EAST, tsif1, sdc4_clk, _, qdss, _, _, _, _, _),
> + PINGROUP(94, EAST, tsif1, sdc42, _, _, _, _, _, _, _),
> + PINGROUP(95, EAST, tsif1, sdc41, _, _, _, _, _, _, _),
> + PINGROUP(96, EAST, tsif1, sdc40, phase_flag, _, _, _, _, _, _),
> PINGROUP(97, WEST, _, mdp_vsync_b, ldo_en, _, _, _, _, _, _),
> PINGROUP(98, WEST, _, mdp_vsync_b, ldo_update, _, _, _, _, _, _),
> PINGROUP(99, WEST, _, _, _, _, _, _, _, _, _),
> --
> 2.17.1
^ permalink raw reply
* [PATCH 7/7] gpiolib: Use spinlock_t instead of struct spinlock
From: Sebastian Andrzej Siewior @ 2019-07-04 15:38 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, Peter Zijlstra, Sebastian Andrzej Siewior, Linus Walleij,
Bartosz Golaszewski, linux-gpio
In-Reply-To: <20190704153803.12739-1-bigeasy@linutronix.de>
For spinlocks the type spinlock_t should be used instead of "struct
spinlock".
Use spinlock_t for spinlock's definition.
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc: linux-gpio@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/gpio/gpiolib.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 7a65dad43932c..7c52c2442173e 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -210,7 +210,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
struct gpio_array *array_info,
unsigned long *value_bitmap);
-extern struct spinlock gpio_lock;
+extern spinlock_t gpio_lock;
extern struct list_head gpio_devices;
struct gpio_desc {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v1] pinctrl: intel: Drop double check for data in intel_pinctrl_probe_by_uid()
From: Mika Westerberg @ 2019-07-04 13:13 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-gpio, Linus Walleij
In-Reply-To: <20190704130239.24111-1-andriy.shevchenko@linux.intel.com>
On Thu, Jul 04, 2019 at 04:02:39PM +0300, Andy Shevchenko wrote:
> There is no need to duplicate the check which is done in the common
> intel_pinctrl_probe().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.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