* Re: [RFC] SW connection between DVB Transport Stream demuxer and I2C-based frontend
From: Marc Gonzalez @ 2019-07-10 13:52 UTC (permalink / raw)
To: Peter Rosin, I2C, linux-media, GPIO
Cc: Mauro Carvalho Chehab, Jonathan Neuschäfer, Brad Love,
Antti Palosaari, Olli Salonen, Bjorn Andersson, Linus Walleij,
Jeffrey Hugo, Wolfram Sang, Simon Horman, Peter Korsgaard,
Linux ARM
In-Reply-To: <7d47a978-5307-a2c8-acc2-f29ce7567bd5@axentia.se>
On 08/07/2019 21:58, Peter Rosin wrote:
> On 2019-07-08 13:08, Marc Gonzalez wrote:
>
>> PROBLEM #2
>>
>> The tuner (si2157) is not on the i2c5 bus, instead it is on a private
>> i2c bus *behind* si2168, which routes requests to the proper client.
>> For the time being, I don't know how to model this relationship in DT.
>> (TODO: check i2c_slave_cb_t slave_cb in struct i2c_client)
>> I have initialized si2157 in the si2168 driver, but this doesn't feel
>> right. (Though it seems all(?) users pair 2168 with 2157.)
>>
>>
>> diff --git a/arch/arm64/boot/dts/qcom/apq8098-batfish.dts b/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
>> index 29d59ecad138..9353e62375a7 100644
>> --- a/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
>> +++ b/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
>> @@ -30,6 +30,28 @@
>> status = "ok";
>> };
>>
>> +&blsp1_i2c5 {
>> + status = "ok";
>> + clock-frequency = <100000>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&i2c5_default>;
>> +
>> + dvb_demod: si2168@64 {
>> + compatible = "silabs,si2168";
>> + reg = <0x64>;
>> + reset-gpios = <&tlmm 84 GPIO_ACTIVE_LOW>;
>
>
> In principle, I think you should be able to add something like this here:
>
> i2c-gate {
> #address-cells = <1>;
> #size-cells = <0>;
>
> tuner@60 {
> compatible = "silabs,si2157";
> reg = <0x60>;
> /* whatever else is needed */
> };
> };
>
> But in practice, I don't know if the si2157 driver understands that or
> if there is anything else that gets in the way. Totally untested...
>
> The i2c-gate child node is examined when you call i2c_mux_add_adapter
> if you have provided the correct struct device in the second argument
> when you called i2c_mux_alloc (I think that is already the case). You
> also need to set the I2C_MUX_GATE flag in the i2c_mux_alloc call, but
> I do not see a problem with that since I think only one child adapter
> is added. If it is a problem to add the I2C_MUX_GATE flag, then you
> can just adjust the above i2c-gate node according to the bindings in
> Documentation/devicetree/bindings/i2c/i2c-mux.txt. I.e. add this
> instead of the above i2c-gate node:
>
> i2c-mux {
> #address-cells = <1>;
> #size-cells = <0>;
>
> i2c@0 {
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0>;
>
> tuner@60 {
> compatible = "silabs,si2157";
> reg = <0x60>;
> /* whatever else is needed */
> };
> };
> };
>
> But it feels *right* to add the I2C_MUX_GATE flag, because a gate is
> what you have. I think?
Thanks, Peter!
Your solution works great. It's refreshing to have stuff work
out-of-the-box!
Problem #2 is now taken care of. And Brad proposed an elegant
solution (IMO) for Problem #1.
I'll send an updated RFC v2 tomorrow. (My patch series is a
complete mess right now.)
Regards.
^ permalink raw reply
* Re: [PATCH] gpio: don't WARN() on NULL descs if gpiolib is disabled
From: Enrico Weigelt, metux IT consult @ 2019-07-10 13:09 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: open list:GPIO SUBSYSTEM, linux-kernel@vger.kernel.org,
Bartosz Golaszewski, Claus H . Stovgaard
In-Reply-To: <CACRpkdb5xKHZja0mkd-wZJ+YHZpGJaDrkA0dv60MNYKXFcPK4w@mail.gmail.com>
On 09.07.19 15:30, Linus Walleij wrote:
Hi,
> I remember I had this discussion in the past, and I made a large> refactoring to make it possible for drivers that need gpiod_*> calls
to simply do:> > select GPIOLIB> > in Kconfig.
Would that allow enabling gpio consumers or drivers selectable w/o
having the gpio subsystem enabled first ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* WARNING in gpio_to_desc
From: syzbot @ 2019-07-10 11:07 UTC (permalink / raw)
To: andreyknvl, bgolaszewski, linus.walleij, linux-gpio, linux-kernel,
linux-usb, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 7829a896 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=136a97d8600000
kernel config: https://syzkaller.appspot.com/x/.config?x=f6d4561982f71f63
dashboard link: https://syzkaller.appspot.com/bug?extid=cf35b76f35e068a1107f
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15660838600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15ff38cfa00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+cf35b76f35e068a1107f@syzkaller.appspotmail.com
usb 1-1: config 0 interface 60 altsetting 0 bulk endpoint 0x2 has invalid
maxpacket 0
usb 1-1: New USB device found, idVendor=1286, idProduct=2046,
bcdDevice=33.1a
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
usb 1-1: NFC: intf 000000003523355a id 0000000011a20f39
nfcmrvl 1-1:0.60: NFC: failed to request reset_n io
------------[ cut here ]------------
invalid GPIO -22
WARNING: CPU: 1 PID: 21 at drivers/gpio/gpiolib.c:124 gpio_to_desc
drivers/gpio/gpiolib.c:124 [inline]
WARNING: CPU: 1 PID: 21 at drivers/gpio/gpiolib.c:124
gpio_to_desc+0x152/0x1f0 drivers/gpio/gpiolib.c:106
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.2.0-rc6+ #13
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
panic+0x292/0x6c9 kernel/panic.c:219
__warn.cold+0x20/0x4b kernel/panic.c:576
report_bug+0x262/0x2a0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:986
RIP: 0010:gpio_to_desc drivers/gpio/gpiolib.c:124 [inline]
RIP: 0010:gpio_to_desc+0x152/0x1f0 drivers/gpio/gpiolib.c:106
Code: 00 00 77 13 e8 cf e0 57 ff 4c 89 e8 5b 5d 41 5c 41 5d 41 5e 41 5f c3
e8 bc e0 57 ff 89 ee 48 c7 c7 80 24 db 85 e8 59 a5 2d ff <0f> 0b eb d6 e8
a5 e0 57 ff 48 c7 c7 80 d6 08 87 4c 89 f6 e8 f6 23
RSP: 0018:ffff8881d9eff138 EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff8881d1605180 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8127ef3d RDI: ffffed103b3dfe19
RBP: 00000000ffffffea R08: ffff8881d9e36000 R09: ffffed103b663ed7
R10: ffffed103b663ed6 R11: ffff8881db31f6b7 R12: ffff8881d1605180
R13: 0000000000000000 R14: 0000000000000293 R15: 0000000000000000
__gpio_set_value include/asm-generic/gpio.h:104 [inline]
gpio_set_value include/linux/gpio.h:71 [inline]
nfcmrvl_chip_halt+0x4e/0x70 drivers/nfc/nfcmrvl/main.c:259
nfcmrvl_nci_register_dev+0x2d4/0x378 drivers/nfc/nfcmrvl/main.c:176
nfcmrvl_probe+0x4e9/0x5e0 drivers/nfc/nfcmrvl/usb.c:344
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x660 drivers/base/dd.c:509
driver_probe_device+0x104/0x210 drivers/base/dd.c:670
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:777
bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:843
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2111
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x660 drivers/base/dd.c:509
driver_probe_device+0x104/0x210 drivers/base/dd.c:670
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:777
bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:843
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2111
usb_new_device.cold+0x8c1/0x1016 drivers/usb/core/hub.c:2534
hub_port_connect drivers/usb/core/hub.c:5089 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
port_event drivers/usb/core/hub.c:5350 [inline]
hub_event+0x1ada/0x3590 drivers/usb/core/hub.c:5432
process_one_work+0x905/0x1570 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x30b/0x410 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
From: Geert Uytterhoeven @ 2019-07-10 10:21 UTC (permalink / raw)
To: Phil Reid
Cc: Geert Uytterhoeven, Linus Walleij, Bartosz Golaszewski,
Alexander Graf, Peter Maydell, Paolo Bonzini, Magnus Damm,
open list:GPIO SUBSYSTEM, QEMU Developers, Linux-Renesas,
Linux Kernel Mailing List
In-Reply-To: <8500a069-9e29-d6ad-e5e4-22d5a3eead59@electromag.com.au>
Hi Phil,
On Wed, Jul 10, 2019 at 4:00 AM Phil Reid <preid@electromag.com.au> wrote:
> On 6/07/2019 00:05, Geert Uytterhoeven wrote:
> > 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
> >
>
> Nice.
> This provides similar functionality to the "gpio inverter" driver currently on the list.
> Other than being just a buffer.
Indeed, both drivers forward GPIO calls, but the gpio inverter modifies
some parameters passed.
The way the drivers obtain references to GPIOs is different, though: the
inverter driver obtains a fixed description from DT, while the virtual
aggregator receives the description at runtime, from sysfs.
But perhaps both drivers could share some code?
> Would it be possible to do the lookup via line names?
Doesn't the fact that a GPIO has a line name means that it is in use, and
thus cannot be aggregated and exported to another user?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/2] gpio: em: remove the gpiochip before removing the irq domain
From: Geert Uytterhoeven @ 2019-07-10 10:13 UTC (permalink / raw)
To: Phil Reid
Cc: Bartosz Golaszewski, Linus Walleij, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, Bartosz Golaszewski, stable
In-Reply-To: <510f14c9-fc3b-734c-53ff-cbf4a7579e32@electromag.com.au>
Hi Phil,
On Wed, Jul 10, 2019 at 11:37 AM Phil Reid <preid@electromag.com.au> wrote:
> On 10/07/2019 17:08, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > In commit 8764c4ca5049 ("gpio: em: use the managed version of
> > gpiochip_add_data()") we implicitly altered the ordering of resource
> > freeing: since gpiochip_remove() calls gpiochip_irqchip_remove()
> > internally, we now can potentially use the irq_domain after it was
> > destroyed in the remove() callback (as devm resources are freed after
> > remove() has returned).
> >
> > Use devm_add_action() to keep the ordering right and entirely kill
> > the remove() callback in the driver.
> >
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Fixes: 8764c4ca5049 ("gpio: em: use the managed version of gpiochip_add_data()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > --- a/drivers/gpio/gpio-em.c
> > +++ b/drivers/gpio/gpio-em.c
> > @@ -333,39 +340,32 @@ static int em_gio_probe(struct platform_device *pdev)
> > return -ENXIO;
> > }
> >
> > + ret = devm_add_action(&pdev->dev,
> > + em_gio_irq_domain_remove, p->irq_domain);
>
> Could devm_add_action_or_reset be used?
Thank you very much for bringing this function to my attention!
I was just wondering if devm_add_action() should call the action on
failure, as this is what most callers seem to do anyway.
>
> > + if (ret) {
> > + irq_domain_remove(p->irq_domain);
> > + return ret;
> > + }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/2] gpio: em: remove the gpiochip before removing the irq domain
From: Bartosz Golaszewski @ 2019-07-10 9:47 UTC (permalink / raw)
To: Phil Reid
Cc: Bartosz Golaszewski, Linus Walleij, Geert Uytterhoeven,
linux-gpio, LKML, Stable # 4 . 20+
In-Reply-To: <510f14c9-fc3b-734c-53ff-cbf4a7579e32@electromag.com.au>
śr., 10 lip 2019 o 11:37 Phil Reid <preid@electromag.com.au> napisał(a):
>
> G'day Bartosz,
>
> One comment below
>
> On 10/07/2019 17:08, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > In commit 8764c4ca5049 ("gpio: em: use the managed version of
> > gpiochip_add_data()") we implicitly altered the ordering of resource
> > freeing: since gpiochip_remove() calls gpiochip_irqchip_remove()
> > internally, we now can potentially use the irq_domain after it was
> > destroyed in the remove() callback (as devm resources are freed after
> > remove() has returned).
> >
> > Use devm_add_action() to keep the ordering right and entirely kill
> > the remove() callback in the driver.
> >
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Fixes: 8764c4ca5049 ("gpio: em: use the managed version of gpiochip_add_data()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> > drivers/gpio/gpio-em.c | 35 +++++++++++++++++------------------
> > 1 file changed, 17 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-em.c b/drivers/gpio/gpio-em.c
> > index b6af705a4e5f..c88028ac66f2 100644
> > --- a/drivers/gpio/gpio-em.c
> > +++ b/drivers/gpio/gpio-em.c
> > @@ -259,6 +259,13 @@ static const struct irq_domain_ops em_gio_irq_domain_ops = {
> > .xlate = irq_domain_xlate_twocell,
> > };
> >
> > +static void em_gio_irq_domain_remove(void *data)
> > +{
> > + struct irq_domain *domain = data;
> > +
> > + irq_domain_remove(domain);
> > +}
> > +
> > static int em_gio_probe(struct platform_device *pdev)
> > {
> > struct em_gio_priv *p;
> > @@ -333,39 +340,32 @@ static int em_gio_probe(struct platform_device *pdev)
> > return -ENXIO;
> > }
> >
> > + ret = devm_add_action(&pdev->dev,
> > + em_gio_irq_domain_remove, p->irq_domain);
>
> Could devm_add_action_or_reset be used?
>
Of course it could and it should. :)
I'll resend tomorrow to not spam the mailing list.
Thanks,
Bart
> > + if (ret) {
> > + irq_domain_remove(p->irq_domain);
> > + return ret;
> > + }
> > +
> > if (devm_request_irq(&pdev->dev, irq[0]->start,
> > em_gio_irq_handler, 0, name, p)) {
> > dev_err(&pdev->dev, "failed to request low IRQ\n");
> > - ret = -ENOENT;
> > - goto err1;
> > + return -ENOENT;
> > }
> >
> > if (devm_request_irq(&pdev->dev, irq[1]->start,
> > em_gio_irq_handler, 0, name, p)) {
> > dev_err(&pdev->dev, "failed to request high IRQ\n");
> > - ret = -ENOENT;
> > - goto err1;
> > + return -ENOENT;
> > }
> >
> > ret = devm_gpiochip_add_data(&pdev->dev, gpio_chip, p);
> > if (ret) {
> > dev_err(&pdev->dev, "failed to add GPIO controller\n");
> > - goto err1;
> > + return ret;
> > }
> >
> > return 0;
> > -
> > -err1:
> > - irq_domain_remove(p->irq_domain);
> > - return ret;
> > -}
> > -
> > -static int em_gio_remove(struct platform_device *pdev)
> > -{
> > - struct em_gio_priv *p = platform_get_drvdata(pdev);
> > -
> > - irq_domain_remove(p->irq_domain);
> > - return 0;
> > }
> >
> > static const struct of_device_id em_gio_dt_ids[] = {
> > @@ -376,7 +376,6 @@ MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
> >
> > static struct platform_driver em_gio_device_driver = {
> > .probe = em_gio_probe,
> > - .remove = em_gio_remove,
> > .driver = {
> > .name = "em_gio",
> > .of_match_table = em_gio_dt_ids,
> >
>
>
> --
> Regards
> Phil Reid
>
^ permalink raw reply
* Re: [PATCH 1/2] gpio: em: remove the gpiochip before removing the irq domain
From: Phil Reid @ 2019-07-10 9:37 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij, Geert Uytterhoeven
Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, stable
In-Reply-To: <20190710090852.9239-1-brgl@bgdev.pl>
G'day Bartosz,
One comment below
On 10/07/2019 17:08, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> In commit 8764c4ca5049 ("gpio: em: use the managed version of
> gpiochip_add_data()") we implicitly altered the ordering of resource
> freeing: since gpiochip_remove() calls gpiochip_irqchip_remove()
> internally, we now can potentially use the irq_domain after it was
> destroyed in the remove() callback (as devm resources are freed after
> remove() has returned).
>
> Use devm_add_action() to keep the ordering right and entirely kill
> the remove() callback in the driver.
>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Fixes: 8764c4ca5049 ("gpio: em: use the managed version of gpiochip_add_data()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/gpio/gpio-em.c | 35 +++++++++++++++++------------------
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpio/gpio-em.c b/drivers/gpio/gpio-em.c
> index b6af705a4e5f..c88028ac66f2 100644
> --- a/drivers/gpio/gpio-em.c
> +++ b/drivers/gpio/gpio-em.c
> @@ -259,6 +259,13 @@ static const struct irq_domain_ops em_gio_irq_domain_ops = {
> .xlate = irq_domain_xlate_twocell,
> };
>
> +static void em_gio_irq_domain_remove(void *data)
> +{
> + struct irq_domain *domain = data;
> +
> + irq_domain_remove(domain);
> +}
> +
> static int em_gio_probe(struct platform_device *pdev)
> {
> struct em_gio_priv *p;
> @@ -333,39 +340,32 @@ static int em_gio_probe(struct platform_device *pdev)
> return -ENXIO;
> }
>
> + ret = devm_add_action(&pdev->dev,
> + em_gio_irq_domain_remove, p->irq_domain);
Could devm_add_action_or_reset be used?
> + if (ret) {
> + irq_domain_remove(p->irq_domain);
> + return ret;
> + }
> +
> if (devm_request_irq(&pdev->dev, irq[0]->start,
> em_gio_irq_handler, 0, name, p)) {
> dev_err(&pdev->dev, "failed to request low IRQ\n");
> - ret = -ENOENT;
> - goto err1;
> + return -ENOENT;
> }
>
> if (devm_request_irq(&pdev->dev, irq[1]->start,
> em_gio_irq_handler, 0, name, p)) {
> dev_err(&pdev->dev, "failed to request high IRQ\n");
> - ret = -ENOENT;
> - goto err1;
> + return -ENOENT;
> }
>
> ret = devm_gpiochip_add_data(&pdev->dev, gpio_chip, p);
> if (ret) {
> dev_err(&pdev->dev, "failed to add GPIO controller\n");
> - goto err1;
> + return ret;
> }
>
> return 0;
> -
> -err1:
> - irq_domain_remove(p->irq_domain);
> - return ret;
> -}
> -
> -static int em_gio_remove(struct platform_device *pdev)
> -{
> - struct em_gio_priv *p = platform_get_drvdata(pdev);
> -
> - irq_domain_remove(p->irq_domain);
> - return 0;
> }
>
> static const struct of_device_id em_gio_dt_ids[] = {
> @@ -376,7 +376,6 @@ MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
>
> static struct platform_driver em_gio_device_driver = {
> .probe = em_gio_probe,
> - .remove = em_gio_remove,
> .driver = {
> .name = "em_gio",
> .of_match_table = em_gio_dt_ids,
>
--
Regards
Phil Reid
^ permalink raw reply
* Re: [PATCH] pinctrl: aspeed: Strip moved macros and structs from private header
From: Linus Walleij @ 2019-07-10 9:21 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: open list:GPIO SUBSYSTEM
In-Reply-To: <20190710032216.4088-1-andrew@aj.id.au>
On Wed, Jul 10, 2019 at 5:22 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> Further cleanup from the SPDX fixup fallout for the recent ASPEED
> series. aspeed_g4_defconfig, aspeed_g5_defconfig and multi_v5_defconfig
> now compile. Smoke tested the g4 and g5 kernels under QEMU's
> palmetto-bmc and romulus-bmc machines respectively.
>
> Fixes: 35d8510ea3ad ("pinctrl: aspeed: Fix missed include")
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>
> Sorry Linus, I guess I should have just sent the fixup patch rather than
> waiting for your response about which way to handle the break. That way I would
> have actually fixed the problem rather than just eyeballing it and making
> half-baked suggestions.
>
> The content stripped in this patch was moved to aspeed-pinmux.h, so with
> 35d8510ea3ad ("pinctrl: aspeed: Fix missed include") we hit
> duplicate-declaration problems. Trivially resolved by removing the moved
> content.
I applied it and now it compiles so I'm happy :)
There was inevitably going to be some fallout from the SPDX stuff
in this merge window, but tglx has me convinced that it is worth
the effort and merge mess, so we just hash it out and carry on.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 2/2] gpio: em: use a helper variable for &pdev->dev
From: Bartosz Golaszewski @ 2019-07-10 9:08 UTC (permalink / raw)
To: Linus Walleij, Geert Uytterhoeven
Cc: linux-gpio, linux-kernel, Bartosz Golaszewski
In-Reply-To: <20190710090852.9239-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Instead of always dereferencing &pdev->dev, just assign a helper local
variable of type struct device * and use it where applicable.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/gpio/gpio-em.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-em.c b/drivers/gpio/gpio-em.c
index c88028ac66f2..e3aa6fe3a320 100644
--- a/drivers/gpio/gpio-em.c
+++ b/drivers/gpio/gpio-em.c
@@ -272,11 +272,12 @@ static int em_gio_probe(struct platform_device *pdev)
struct resource *io[2], *irq[2];
struct gpio_chip *gpio_chip;
struct irq_chip *irq_chip;
- const char *name = dev_name(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ const char *name = dev_name(dev);
unsigned int ngpios;
int ret;
- p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
+ p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
if (!p)
return -ENOMEM;
@@ -290,27 +291,27 @@ static int em_gio_probe(struct platform_device *pdev)
irq[1] = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
if (!io[0] || !io[1] || !irq[0] || !irq[1]) {
- dev_err(&pdev->dev, "missing IRQ or IOMEM\n");
+ dev_err(dev, "missing IRQ or IOMEM\n");
return -EINVAL;
}
- p->base0 = devm_ioremap_nocache(&pdev->dev, io[0]->start,
+ p->base0 = devm_ioremap_nocache(dev, io[0]->start,
resource_size(io[0]));
if (!p->base0)
return -ENOMEM;
- p->base1 = devm_ioremap_nocache(&pdev->dev, io[1]->start,
+ p->base1 = devm_ioremap_nocache(dev, io[1]->start,
resource_size(io[1]));
if (!p->base1)
return -ENOMEM;
- if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios)) {
- dev_err(&pdev->dev, "Missing ngpios OF property\n");
+ if (of_property_read_u32(dev->of_node, "ngpios", &ngpios)) {
+ dev_err(dev, "Missing ngpios OF property\n");
return -EINVAL;
}
gpio_chip = &p->gpio_chip;
- gpio_chip->of_node = pdev->dev.of_node;
+ gpio_chip->of_node = dev->of_node;
gpio_chip->direction_input = em_gio_direction_input;
gpio_chip->get = em_gio_get;
gpio_chip->direction_output = em_gio_direction_output;
@@ -319,7 +320,7 @@ static int em_gio_probe(struct platform_device *pdev)
gpio_chip->request = em_gio_request;
gpio_chip->free = em_gio_free;
gpio_chip->label = name;
- gpio_chip->parent = &pdev->dev;
+ gpio_chip->parent = dev;
gpio_chip->owner = THIS_MODULE;
gpio_chip->base = -1;
gpio_chip->ngpio = ngpios;
@@ -333,35 +334,35 @@ static int em_gio_probe(struct platform_device *pdev)
irq_chip->irq_release_resources = em_gio_irq_relres;
irq_chip->flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND;
- p->irq_domain = irq_domain_add_simple(pdev->dev.of_node, ngpios, 0,
+ p->irq_domain = irq_domain_add_simple(dev->of_node, ngpios, 0,
&em_gio_irq_domain_ops, p);
if (!p->irq_domain) {
- dev_err(&pdev->dev, "cannot initialize irq domain\n");
+ dev_err(dev, "cannot initialize irq domain\n");
return -ENXIO;
}
- ret = devm_add_action(&pdev->dev,
+ ret = devm_add_action(dev,
em_gio_irq_domain_remove, p->irq_domain);
if (ret) {
irq_domain_remove(p->irq_domain);
return ret;
}
- if (devm_request_irq(&pdev->dev, irq[0]->start,
+ if (devm_request_irq(dev, irq[0]->start,
em_gio_irq_handler, 0, name, p)) {
- dev_err(&pdev->dev, "failed to request low IRQ\n");
+ dev_err(dev, "failed to request low IRQ\n");
return -ENOENT;
}
- if (devm_request_irq(&pdev->dev, irq[1]->start,
+ if (devm_request_irq(dev, irq[1]->start,
em_gio_irq_handler, 0, name, p)) {
- dev_err(&pdev->dev, "failed to request high IRQ\n");
+ dev_err(dev, "failed to request high IRQ\n");
return -ENOENT;
}
- ret = devm_gpiochip_add_data(&pdev->dev, gpio_chip, p);
+ ret = devm_gpiochip_add_data(dev, gpio_chip, p);
if (ret) {
- dev_err(&pdev->dev, "failed to add GPIO controller\n");
+ dev_err(dev, "failed to add GPIO controller\n");
return ret;
}
--
2.21.0
^ permalink raw reply related
* [PATCH 1/2] gpio: em: remove the gpiochip before removing the irq domain
From: Bartosz Golaszewski @ 2019-07-10 9:08 UTC (permalink / raw)
To: Linus Walleij, Geert Uytterhoeven
Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, stable
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
In commit 8764c4ca5049 ("gpio: em: use the managed version of
gpiochip_add_data()") we implicitly altered the ordering of resource
freeing: since gpiochip_remove() calls gpiochip_irqchip_remove()
internally, we now can potentially use the irq_domain after it was
destroyed in the remove() callback (as devm resources are freed after
remove() has returned).
Use devm_add_action() to keep the ordering right and entirely kill
the remove() callback in the driver.
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: 8764c4ca5049 ("gpio: em: use the managed version of gpiochip_add_data()")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/gpio/gpio-em.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-em.c b/drivers/gpio/gpio-em.c
index b6af705a4e5f..c88028ac66f2 100644
--- a/drivers/gpio/gpio-em.c
+++ b/drivers/gpio/gpio-em.c
@@ -259,6 +259,13 @@ static const struct irq_domain_ops em_gio_irq_domain_ops = {
.xlate = irq_domain_xlate_twocell,
};
+static void em_gio_irq_domain_remove(void *data)
+{
+ struct irq_domain *domain = data;
+
+ irq_domain_remove(domain);
+}
+
static int em_gio_probe(struct platform_device *pdev)
{
struct em_gio_priv *p;
@@ -333,39 +340,32 @@ static int em_gio_probe(struct platform_device *pdev)
return -ENXIO;
}
+ ret = devm_add_action(&pdev->dev,
+ em_gio_irq_domain_remove, p->irq_domain);
+ if (ret) {
+ irq_domain_remove(p->irq_domain);
+ return ret;
+ }
+
if (devm_request_irq(&pdev->dev, irq[0]->start,
em_gio_irq_handler, 0, name, p)) {
dev_err(&pdev->dev, "failed to request low IRQ\n");
- ret = -ENOENT;
- goto err1;
+ return -ENOENT;
}
if (devm_request_irq(&pdev->dev, irq[1]->start,
em_gio_irq_handler, 0, name, p)) {
dev_err(&pdev->dev, "failed to request high IRQ\n");
- ret = -ENOENT;
- goto err1;
+ return -ENOENT;
}
ret = devm_gpiochip_add_data(&pdev->dev, gpio_chip, p);
if (ret) {
dev_err(&pdev->dev, "failed to add GPIO controller\n");
- goto err1;
+ return ret;
}
return 0;
-
-err1:
- irq_domain_remove(p->irq_domain);
- return ret;
-}
-
-static int em_gio_remove(struct platform_device *pdev)
-{
- struct em_gio_priv *p = platform_get_drvdata(pdev);
-
- irq_domain_remove(p->irq_domain);
- return 0;
}
static const struct of_device_id em_gio_dt_ids[] = {
@@ -376,7 +376,6 @@ MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
static struct platform_driver em_gio_device_driver = {
.probe = em_gio_probe,
- .remove = em_gio_remove,
.driver = {
.name = "em_gio",
.of_match_table = em_gio_dt_ids,
--
2.21.0
^ permalink raw reply related
* Re: [PATCH V4 2/2] gpio: inverter: document the inverter bindings
From: Harish Jenny K N @ 2019-07-10 8:28 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Mark Rutland, devicetree,
open list:GPIO SUBSYSTEM, Balasubramani Vivekanandan
In-Reply-To: <CAL_JsqLQvjtnfUsZ2RP4eozvdwMLzNxtgmT+XFaxW4xzoFjL=w@mail.gmail.com>
Hi,
On 09/07/19 9:38 PM, Rob Herring wrote:
> On Mon, Jul 8, 2019 at 11:25 PM Harish Jenny K N
> <harish_kandiga@mentor.com> wrote:
>> Hi Rob,
>>
>>
>> On 09/07/19 4:06 AM, Rob Herring wrote:
>>> On Fri, Jun 28, 2019 at 3:31 AM Harish Jenny K N
>>> <harish_kandiga@mentor.com> wrote:
>>>> Document the device tree binding for the inverter gpio
>>>> controller to configure the polarity of the gpio pins
>>>> used by the consumers.
>>>>
>>>> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
>>>> ---
>>>> .../devicetree/bindings/gpio/gpio-inverter.txt | 29 ++++++++++++++++++++++
>>>> 1 file changed, 29 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-inverter.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-inverter.txt b/Documentation/devicetree/bindings/gpio/gpio-inverter.txt
>>>> new file mode 100644
>>>> index 0000000..8bb6b2e
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-inverter.txt
>>>> @@ -0,0 +1,29 @@
>>>> +GPIO-INVERTER
>>>> +======
>>>> +This binding defines the gpio-inverter. The gpio-inverter is a driver that
>>>> +allows to properly describe the gpio polarities on the hardware.
>>> I don't understand. Please explain this in terms of the hardware, not a driver.
>>
>> gpio inverters can be used on different hardware to alter the polarity of gpio chips.
>> The polarity of pins can change from hardware to hardware with the use of inverters.
> Yes, I know what an inverter is.
>
>> This device tree binding models gpio inverters in the device tree to properly describe the hardware.
> We already define the active state of GPIOs in the consumers. If
> there's an inverter in the middle, the consumer active state is simply
> inverted. I don't agree that that is a hack as Linus said without some
> reasoning why an inverter needs to be modeled in DT. Anything about
> what 'userspace' needs is not a reason. That's a Linux thing that has
> little to do with hardware description.
Yes we are talking about the hardware level inversions here. The usecase is for those without the gpio consumer driver. The usecase started with the concept of allowing an abstraction of the underlying hardware for the userland controlling program such that this program does not care whether the GPIO lines are inverted or not physically. In other words, a single userland controlling program can work unmodified across a variety of hardware platforms with the device tree mapping the logical to physical relationship of the GPIO hardware.
I totally understand anything about what 'userspace' needs is not a reason, but this is not restricted to userspace alone as kernel drivers may need this just as much. Also we are just modelling/describing the hardware state in the device tree.
Just to mention that Linus Walleij had proposed this inverter model to describe the hardware and the gpio inverter driver is developed based on comments/review from him.
Also my sincere request to Linus Walleij to please let his opinion know on this.
Thanks,
Best Regards,
Harish Jenny K N
^ permalink raw reply
* [PATCH] pinctrl: aspeed: Strip moved macros and structs from private header
From: Andrew Jeffery @ 2019-07-10 3:22 UTC (permalink / raw)
To: linux-gpio; +Cc: Andrew Jeffery, linus.walleij
Further cleanup from the SPDX fixup fallout for the recent ASPEED
series. aspeed_g4_defconfig, aspeed_g5_defconfig and multi_v5_defconfig
now compile. Smoke tested the g4 and g5 kernels under QEMU's
palmetto-bmc and romulus-bmc machines respectively.
Fixes: 35d8510ea3ad ("pinctrl: aspeed: Fix missed include")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
Sorry Linus, I guess I should have just sent the fixup patch rather than
waiting for your response about which way to handle the break. That way I would
have actually fixed the problem rather than just eyeballing it and making
half-baked suggestions.
The content stripped in this patch was moved to aspeed-pinmux.h, so with
35d8510ea3ad ("pinctrl: aspeed: Fix missed include") we hit
duplicate-declaration problems. Trivially resolved by removing the moved
content.
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 498 ------------------------
1 file changed, 498 deletions(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
index b7790395aead..7fcfc5004b44 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
@@ -18,504 +18,6 @@
#include "pinmux-aspeed.h"
-/*
- * The ASPEED SoCs provide typically more than 200 pins for GPIO and other
- * functions. The SoC function enabled on a pin is determined on a priority
- * basis where a given pin can provide a number of different signal types.
- *
- * The signal active on a pin is described by both a priority level and
- * compound logical expressions involving multiple operators, registers and
- * bits. Some difficulty arises as the pin's function bit masks for each
- * priority level are frequently not the same (i.e. cannot just flip a bit to
- * change from a high to low priority signal), or even in the same register.
- * Further, not all signals can be unmuxed, as some expressions depend on
- * values in the hardware strapping register (which is treated as read-only).
- *
- * SoC Multi-function Pin Expression Examples
- * ------------------------------------------
- *
- * Here are some sample mux configurations from the AST2400 and AST2500
- * datasheets to illustrate the corner cases, roughly in order of least to most
- * corner. The signal priorities are in decending order from P0 (highest).
- *
- * D6 is a pin with a single function (beside GPIO); a high priority signal
- * that participates in one function:
- *
- * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- * D6 GPIOA0 MAC1LINK SCU80[0]=1 GPIOA0
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- *
- * C5 is a multi-signal pin (high and low priority signals). Here we touch
- * different registers for the different functions that enable each signal:
- *
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- * C5 GPIOA4 SCL9 SCU90[22]=1 TIMER5 SCU80[4]=1 GPIOA4
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- *
- * E19 is a single-signal pin with two functions that influence the active
- * signal. In this case both bits have the same meaning - enable a dedicated
- * LPC reset pin. However it's not always the case that the bits in the
- * OR-relationship have the same meaning.
- *
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- * E19 GPIOB4 LPCRST# SCU80[12]=1 | Strap[14]=1 GPIOB4
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- *
- * For example, pin B19 has a low-priority signal that's enabled by two
- * distinct SoC functions: A specific SIOPBI bit in register SCUA4, and an ACPI
- * bit in the STRAP register. The ACPI bit configures signals on pins in
- * addition to B19. Both of the low priority functions as well as the high
- * priority function must be disabled for GPIOF1 to be used.
- *
- * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
- * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
- * B19 GPIOF1 NDCD4 SCU80[25]=1 SIOPBI# SCUA4[12]=1 | Strap[19]=0 GPIOF1
- * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
- *
- * For pin E18, the SoC ANDs the expected state of three bits to determine the
- * pin's active signal:
- *
- * * SCU3C[3]: Enable external SOC reset function
- * * SCU80[15]: Enable SPICS1# or EXTRST# function pin
- * * SCU90[31]: Select SPI interface CS# output
- *
- * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
- * E18 GPIOB7 EXTRST# SCU3C[3]=1 & SCU80[15]=1 & SCU90[31]=0 SPICS1# SCU3C[3]=1 & SCU80[15]=1 & SCU90[31]=1 GPIOB7
- * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
- *
- * (Bits SCU3C[3] and SCU80[15] appear to only be used in the expressions for
- * selecting the signals on pin E18)
- *
- * Pin T5 is a multi-signal pin with a more complex configuration:
- *
- * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- * T5 GPIOL1 VPIDE SCU90[5:4]!=0 & SCU84[17]=1 NDCD1 SCU84[17]=1 GPIOL1
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- *
- * The high priority signal configuration is best thought of in terms of its
- * exploded form, with reference to the SCU90[5:4] bits:
- *
- * * SCU90[5:4]=00: disable
- * * SCU90[5:4]=01: 18 bits (R6/G6/B6) video mode.
- * * SCU90[5:4]=10: 24 bits (R8/G8/B8) video mode.
- * * SCU90[5:4]=11: 30 bits (R10/G10/B10) video mode.
- *
- * Re-writing:
- *
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- * T5 GPIOL1 VPIDE (SCU90[5:4]=1 & SCU84[17]=1) NDCD1 SCU84[17]=1 GPIOL1
- * | (SCU90[5:4]=2 & SCU84[17]=1)
- * | (SCU90[5:4]=3 & SCU84[17]=1)
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- *
- * For reference the SCU84[17] bit configure the "UART1 NDCD1 or Video VPIDE
- * function pin", where the signal itself is determined by whether SCU94[5:4]
- * is disabled or in one of the 18, 24 or 30bit video modes.
- *
- * Other video-input-related pins require an explicit state in SCU90[5:4], e.g.
- * W1 and U5:
- *
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- * W1 GPIOL6 VPIB0 SCU90[5:4]=3 & SCU84[22]=1 TXD1 SCU84[22]=1 GPIOL6
- * U5 GPIOL7 VPIB1 SCU90[5:4]=3 & SCU84[23]=1 RXD1 SCU84[23]=1 GPIOL7
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- *
- * The examples of T5 and W1 are particularly fertile, as they also demonstrate
- * that despite operating as part of the video input bus each signal needs to
- * be enabled individually via it's own SCU84 (in the cases of T5 and W1)
- * register bit. This is a little crazy if the bus doesn't have optional
- * signals, but is used to decent effect with some of the UARTs where not all
- * signals are required. However, this isn't done consistently - UART1 is
- * enabled on a per-pin basis, and by contrast, all signals for UART6 are
- * enabled by a single bit.
- *
- * Further, the high and low priority signals listed in the table above share
- * a configuration bit. The VPI signals should operate in concert in a single
- * function, but the UART signals should retain the ability to be configured
- * independently. This pushes the implementation down the path of tagging a
- * signal's expressions with the function they participate in, rather than
- * defining masks affecting multiple signals per function. The latter approach
- * fails in this instance where applying the configuration for the UART pin of
- * interest will stomp on the state of other UART signals when disabling the
- * VPI functions on the current pin.
- *
- * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
- * -----+------------+-----------+---------------------------+-----------+---------------+------------
- * A12 RGMII1TXCK GPIOT0 SCUA0[0]=1 RMII1TXEN Strap[6]=0 RGMII1TXCK
- * B12 RGMII1TXCTL GPIOT1 SCUA0[1]=1 – Strap[6]=0 RGMII1TXCTL
- * -----+------------+-----------+---------------------------+-----------+---------------+------------
- *
- * A12 demonstrates that the "Other" signal isn't always GPIO - in this case
- * GPIOT0 is a high-priority signal and RGMII1TXCK is Other. Thus, GPIO
- * should be treated like any other signal type with full function expression
- * requirements, and not assumed to be the default case. Separately, GPIOT0 and
- * GPIOT1's signal descriptor bits are distinct, therefore we must iterate all
- * pins in the function's group to disable the higher-priority signals such
- * that the signal for the function of interest is correctly enabled.
- *
- * Finally, three priority levels aren't always enough; the AST2500 brings with
- * it 18 pins of five priority levels, however the 18 pins only use three of
- * the five priority levels.
- *
- * Ultimately the requirement to control pins in the examples above drive the
- * design:
- *
- * * Pins provide signals according to functions activated in the mux
- * configuration
- *
- * * Pins provide up to five signal types in a priority order
- *
- * * For priorities levels defined on a pin, each priority provides one signal
- *
- * * Enabling lower priority signals requires higher priority signals be
- * disabled
- *
- * * A function represents a set of signals; functions are distinct if their
- * sets of signals are not equal
- *
- * * Signals participate in one or more functions
- *
- * * A function is described by an expression of one or more signal
- * descriptors, which compare bit values in a register
- *
- * * A signal expression is the smallest set of signal descriptors whose
- * comparisons must evaluate 'true' for a signal to be enabled on a pin.
- *
- * * A function's signal is active on a pin if evaluating all signal
- * descriptors in the pin's signal expression for the function yields a 'true'
- * result
- *
- * * A signal at a given priority on a given pin is active if any of the
- * functions in which the signal participates are active, and no higher
- * priority signal on the pin is active
- *
- * * GPIO is configured per-pin
- *
- * And so:
- *
- * * To disable a signal, any function(s) activating the signal must be
- * disabled
- *
- * * Each pin must know the signal expressions of functions in which it
- * participates, for the purpose of enabling the Other function. This is done
- * by deactivating all functions that activate higher priority signals on the
- * pin.
- *
- * As a concrete example:
- *
- * * T5 provides three signals types: VPIDE, NDCD1 and GPIO
- *
- * * The VPIDE signal participates in 3 functions: VPI18, VPI24 and VPI30
- *
- * * The NDCD1 signal participates in just its own NDCD1 function
- *
- * * VPIDE is high priority, NDCD1 is low priority, and GPIOL1 is the least
- * prioritised
- *
- * * The prerequisit for activating the NDCD1 signal is that the VPI18, VPI24
- * and VPI30 functions all be disabled
- *
- * * Similarly, all of VPI18, VPI24, VPI30 and NDCD1 functions must be disabled
- * to provide GPIOL6
- *
- * Considerations
- * --------------
- *
- * If pinctrl allows us to allocate a pin we can configure a function without
- * concern for the function of already allocated pins, if pin groups are
- * created with respect to the SoC functions in which they participate. This is
- * intuitive, but it did not feel obvious from the bit/pin relationships.
- *
- * Conversely, failing to allocate all pins in a group indicates some bits (as
- * well as pins) required for the group's configuration will already be in use,
- * likely in a way that's inconsistent with the requirements of the failed
- * group.
- */
-
-#define ASPEED_IP_SCU 0
-#define ASPEED_IP_GFX 1
-#define ASPEED_IP_LPC 2
-#define ASPEED_NR_PINMUX_IPS 3
-
-/*
- * The "Multi-function Pins Mapping and Control" table in the SoC datasheet
- * references registers by the device/offset mnemonic. The register macros
- * below are named the same way to ease transcription and verification (as
- * opposed to naming them e.g. PINMUX_CTRL_[0-9]). Further, signal expressions
- * reference registers beyond those dedicated to pinmux, such as the system
- * reset control and MAC clock configuration registers. The AST2500 goes a step
- * further and references registers in the graphics IP block.
- */
-#define SCU2C 0x2C /* Misc. Control Register */
-#define SCU3C 0x3C /* System Reset Control/Status Register */
-#define SCU48 0x48 /* MAC Interface Clock Delay Setting */
-#define HW_STRAP1 0x70 /* AST2400 strapping is 33 bits, is split */
-#define HW_REVISION_ID 0x7C /* Silicon revision ID register */
-#define SCU80 0x80 /* Multi-function Pin Control #1 */
-#define SCU84 0x84 /* Multi-function Pin Control #2 */
-#define SCU88 0x88 /* Multi-function Pin Control #3 */
-#define SCU8C 0x8C /* Multi-function Pin Control #4 */
-#define SCU90 0x90 /* Multi-function Pin Control #5 */
-#define SCU94 0x94 /* Multi-function Pin Control #6 */
-#define SCUA0 0xA0 /* Multi-function Pin Control #7 */
-#define SCUA4 0xA4 /* Multi-function Pin Control #8 */
-#define SCUA8 0xA8 /* Multi-function Pin Control #9 */
-#define SCUAC 0xAC /* Multi-function Pin Control #10 */
-#define HW_STRAP2 0xD0 /* Strapping */
-
- /**
- * A signal descriptor, which describes the register, bits and the
- * enable/disable values that should be compared or written.
- *
- * @ip: The IP block identifier, used as an index into the regmap array in
- * struct aspeed_pinctrl_data
- * @reg: The register offset with respect to the base address of the IP block
- * @mask: The mask to apply to the register. The lowest set bit of the mask is
- * used to derive the shift value.
- * @enable: The value that enables the function. Value should be in the LSBs,
- * not at the position of the mask.
- * @disable: The value that disables the function. Value should be in the
- * LSBs, not at the position of the mask.
- */
-struct aspeed_sig_desc {
- unsigned int ip;
- unsigned int reg;
- u32 mask;
- u32 enable;
- u32 disable;
-};
-
-/**
- * Describes a signal expression. The expression is evaluated by ANDing the
- * evaluation of the descriptors.
- *
- * @signal: The signal name for the priority level on the pin. If the signal
- * type is GPIO, then the signal name must begin with the string
- * "GPIO", e.g. GPIOA0, GPIOT4 etc.
- * @function: The name of the function the signal participates in for the
- * associated expression
- * @ndescs: The number of signal descriptors in the expression
- * @descs: Pointer to an array of signal descriptors that comprise the
- * function expression
- */
-struct aspeed_sig_expr {
- const char *signal;
- const char *function;
- int ndescs;
- const struct aspeed_sig_desc *descs;
-};
-
-/**
- * A struct capturing the list of expressions enabling signals at each priority
- * for a given pin. The signal configuration for a priority level is evaluated
- * by ORing the evaluation of the signal expressions in the respective
- * priority's list.
- *
- * @name: A name for the pin
- * @prios: A pointer to an array of expression list pointers
- *
- */
-struct aspeed_pin_desc {
- const char *name;
- const struct aspeed_sig_expr ***prios;
-};
-
-/* Macro hell */
-
-#define SIG_DESC_IP_BIT(ip, reg, idx, val) \
- { ip, reg, BIT_MASK(idx), val, (((val) + 1) & 1) }
-
-/**
- * Short-hand macro for describing an SCU descriptor enabled by the state of
- * one bit. The disable value is derived.
- *
- * @reg: The signal's associated register, offset from base
- * @idx: The signal's bit index in the register
- * @val: The value (0 or 1) that enables the function
- */
-#define SIG_DESC_BIT(reg, idx, val) \
- SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, val)
-
-#define SIG_DESC_IP_SET(ip, reg, idx) SIG_DESC_IP_BIT(ip, reg, idx, 1)
-
-/**
- * A further short-hand macro expanding to an SCU descriptor enabled by a set
- * bit.
- *
- * @reg: The register, offset from base
- * @idx: The bit index in the register
- */
-#define SIG_DESC_SET(reg, idx) SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, 1)
-
-#define SIG_DESC_LIST_SYM(sig, func) sig_descs_ ## sig ## _ ## func
-#define SIG_DESC_LIST_DECL(sig, func, ...) \
- static const struct aspeed_sig_desc SIG_DESC_LIST_SYM(sig, func)[] = \
- { __VA_ARGS__ }
-
-#define SIG_EXPR_SYM(sig, func) sig_expr_ ## sig ## _ ## func
-#define SIG_EXPR_DECL_(sig, func) \
- static const struct aspeed_sig_expr SIG_EXPR_SYM(sig, func) = \
- { \
- .signal = #sig, \
- .function = #func, \
- .ndescs = ARRAY_SIZE(SIG_DESC_LIST_SYM(sig, func)), \
- .descs = &(SIG_DESC_LIST_SYM(sig, func))[0], \
- }
-
-/**
- * Declare a signal expression.
- *
- * @sig: A macro symbol name for the signal (is subjected to stringification
- * and token pasting)
- * @func: The function in which the signal is participating
- * @...: Signal descriptors that define the signal expression
- *
- * For example, the following declares the ROMD8 signal for the ROM16 function:
- *
- * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
- *
- * And with multiple signal descriptors:
- *
- * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
- * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
- */
-#define SIG_EXPR_DECL(sig, func, ...) \
- SIG_DESC_LIST_DECL(sig, func, __VA_ARGS__); \
- SIG_EXPR_DECL_(sig, func)
-
-/**
- * Declare a pointer to a signal expression
- *
- * @sig: The macro symbol name for the signal (subjected to token pasting)
- * @func: The macro symbol name for the function (subjected to token pasting)
- */
-#define SIG_EXPR_PTR(sig, func) (&SIG_EXPR_SYM(sig, func))
-
-#define SIG_EXPR_LIST_SYM(sig) sig_exprs_ ## sig
-
-/**
- * Declare a signal expression list for reference in a struct aspeed_pin_prio.
- *
- * @sig: A macro symbol name for the signal (is subjected to token pasting)
- * @...: Signal expression structure pointers (use SIG_EXPR_PTR())
- *
- * For example, the 16-bit ROM bus can be enabled by one of two possible signal
- * expressions:
- *
- * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
- * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
- * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
- * SIG_EXPR_LIST_DECL(ROMD8, SIG_EXPR_PTR(ROMD8, ROM16),
- * SIG_EXPR_PTR(ROMD8, ROM16S));
- */
-#define SIG_EXPR_LIST_DECL(sig, ...) \
- static const struct aspeed_sig_expr *SIG_EXPR_LIST_SYM(sig)[] = \
- { __VA_ARGS__, NULL }
-
-/**
- * A short-hand macro for declaring a function expression and an expression
- * list with a single function.
- *
- * @func: A macro symbol name for the function (is subjected to token pasting)
- * @...: Function descriptors that define the function expression
- *
- * For example, signal NCTS6 participates in its own function with one group:
- *
- * SIG_EXPR_LIST_DECL_SINGLE(NCTS6, NCTS6, SIG_DESC_SET(SCU90, 7));
- */
-#define SIG_EXPR_LIST_DECL_SINGLE(sig, func, ...) \
- SIG_DESC_LIST_DECL(sig, func, __VA_ARGS__); \
- SIG_EXPR_DECL_(sig, func); \
- SIG_EXPR_LIST_DECL(sig, SIG_EXPR_PTR(sig, func))
-
-#define SIG_EXPR_LIST_DECL_DUAL(sig, f0, f1) \
- SIG_EXPR_LIST_DECL(sig, SIG_EXPR_PTR(sig, f0), SIG_EXPR_PTR(sig, f1))
-
-#define SIG_EXPR_LIST_PTR(sig) (&SIG_EXPR_LIST_SYM(sig)[0])
-
-#define PIN_EXPRS_SYM(pin) pin_exprs_ ## pin
-#define PIN_EXPRS_PTR(pin) (&PIN_EXPRS_SYM(pin)[0])
-#define PIN_SYM(pin) pin_ ## pin
-
-#define MS_PIN_DECL_(pin, ...) \
- static const struct aspeed_sig_expr **PIN_EXPRS_SYM(pin)[] = \
- { __VA_ARGS__, NULL }; \
- static const struct aspeed_pin_desc PIN_SYM(pin) = \
- { #pin, PIN_EXPRS_PTR(pin) }
-
-/**
- * Declare a multi-signal pin
- *
- * @pin: The pin number
- * @other: Macro name for "other" functionality (subjected to stringification)
- * @high: Macro name for the highest priority signal functions
- * @low: Macro name for the low signal functions
- *
- * For example:
- *
- * #define A8 56
- * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
- * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
- * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
- * SIG_EXPR_LIST_DECL(ROMD8, SIG_EXPR_PTR(ROMD8, ROM16),
- * SIG_EXPR_PTR(ROMD8, ROM16S));
- * SIG_EXPR_LIST_DECL_SINGLE(NCTS6, NCTS6, SIG_DESC_SET(SCU90, 7));
- * MS_PIN_DECL(A8, GPIOH0, ROMD8, NCTS6);
- */
-#define MS_PIN_DECL(pin, other, high, low) \
- SIG_EXPR_LIST_DECL_SINGLE(other, other); \
- MS_PIN_DECL_(pin, \
- SIG_EXPR_LIST_PTR(high), \
- SIG_EXPR_LIST_PTR(low), \
- SIG_EXPR_LIST_PTR(other))
-
-#define PIN_GROUP_SYM(func) pins_ ## func
-#define FUNC_GROUP_SYM(func) groups_ ## func
-#define FUNC_GROUP_DECL(func, ...) \
- static const int PIN_GROUP_SYM(func)[] = { __VA_ARGS__ }; \
- static const char *FUNC_GROUP_SYM(func)[] = { #func }
-
-/**
- * Declare a single signal pin
- *
- * @pin: The pin number
- * @other: Macro name for "other" functionality (subjected to stringification)
- * @sig: Macro name for the signal (subjected to stringification)
- *
- * For example:
- *
- * #define E3 80
- * SIG_EXPR_LIST_DECL_SINGLE(SCL5, I2C5, I2C5_DESC);
- * SS_PIN_DECL(E3, GPIOK0, SCL5);
- */
-#define SS_PIN_DECL(pin, other, sig) \
- SIG_EXPR_LIST_DECL_SINGLE(other, other); \
- MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(sig), SIG_EXPR_LIST_PTR(other))
-
-/**
- * Single signal, single function pin declaration
- *
- * @pin: The pin number
- * @other: Macro name for "other" functionality (subjected to stringification)
- * @sig: Macro name for the signal (subjected to stringification)
- * @...: Signal descriptors that define the function expression
- *
- * For example:
- *
- * SSSF_PIN_DECL(A4, GPIOA2, TIMER3, SIG_DESC_SET(SCU80, 2));
- */
-#define SSSF_PIN_DECL(pin, other, sig, ...) \
- SIG_EXPR_LIST_DECL_SINGLE(sig, sig, __VA_ARGS__); \
- SIG_EXPR_LIST_DECL_SINGLE(other, other); \
- MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(sig), SIG_EXPR_LIST_PTR(other)); \
- FUNC_GROUP_DECL(sig, pin)
-
-#define GPIO_PIN_DECL(pin, gpio) \
- SIG_EXPR_LIST_DECL_SINGLE(gpio, gpio); \
- MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(gpio))
-
/**
* @param The pinconf parameter type
* @pins The pin range this config struct covers, [low, high]
--
2.20.1
^ permalink raw reply related
* Re: [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
From: Phil Reid @ 2019-07-10 2:00 UTC (permalink / raw)
To: Geert Uytterhoeven, Linus Walleij, Bartosz Golaszewski,
Alexander Graf, Peter Maydell, Paolo Bonzini, Magnus Damm
Cc: linux-gpio, qemu-devel, linux-renesas-soc, linux-kernel
In-Reply-To: <20190705160536.12047-1-geert+renesas@glider.be>
G'day Geert,
On 6/07/2019 00:05, Geert Uytterhoeven wrote:
> 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
>
Nice.
This provides similar functionality to the "gpio inverter" driver currently on the list.
Other than being just a buffer.
Would it be possible to do the lookup via line names?
> 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");
>
--
Regards
Phil Reid
ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au
3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@electromag.com.au
^ permalink raw reply
* Re: [RFC] SW connection between DVB Transport Stream demuxer and I2C-based frontend
From: Jonathan Neuschäfer @ 2019-07-09 23:21 UTC (permalink / raw)
To: Peter Rosin
Cc: Marc Gonzalez, I2C, linux-media, GPIO, Mauro Carvalho Chehab,
Jonathan Neuschäfer, Brad Love, Antti Palosaari,
Olli Salonen, Bjorn Andersson, Linus Walleij, Jeffrey Hugo,
Wolfram Sang, Simon Horman, Peter Korsgaard, Linux ARM
In-Reply-To: <7d47a978-5307-a2c8-acc2-f29ce7567bd5@axentia.se>
[-- Attachment #1: Type: text/plain, Size: 944 bytes --]
On Mon, Jul 08, 2019 at 07:58:00PM +0000, Peter Rosin wrote:
> On 2019-07-08 13:08, Marc Gonzalez wrote:
[...]
> > + dvb_demod: si2168@64 {
> > + compatible = "silabs,si2168";
> > + reg = <0x64>;
> > + reset-gpios = <&tlmm 84 GPIO_ACTIVE_LOW>;
>
>
> In principle, I think you should be able to add something like this here:
>
> i2c-gate {
> #address-cells = <1>;
> #size-cells = <0>;
>
> tuner@60 {
> compatible = "silabs,si2157";
> reg = <0x60>;
> /* whatever else is needed */
> };
> };
>
> But in practice, I don't know if the si2157 driver understands that or
> if there is anything else that gets in the way. Totally untested...
So far, both drivers don't deal with devicetree at all, and there are no
bindings, so we have the chance to write the best bindings possible.
An i2c-gate subnode on the si2168 node looks good to me, FWIW.
Greetings,
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 7/9] dt-bindings: vendor-prefixes: add Sipeed
From: Rob Herring @ 2019-07-09 22:57 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Maxime Ripard, Chen-Yu Tsai, Linus Walleij, devicetree,
linux-arm-kernel, linux-kernel, linux-clk, linux-gpio,
linux-sunxi, Icenowy Zheng
In-Reply-To: <20190623043801.14040-8-icenowy@aosc.io>
On Sun, 23 Jun 2019 12:37:59 +0800, Icenowy Zheng wrote:
> Shenzhen Sipeed Technology Co., Ltd. is a company focused on development
> kits, which also contains rebranded Lichee Pi series.
>
> Add its vendor prefix binding.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> Changes in v3:
> - Rebased because of the addition of sinlinx and sinovoip.
>
> Patch introduced in v2.
>
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Applied, thanks.
Rob
^ permalink raw reply
* Re: [PATCH v3 4/9] clk: sunxi-ng: v3s: add Allwinner V3 support
From: Rob Herring @ 2019-07-09 22:56 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Maxime Ripard, Chen-Yu Tsai, Linus Walleij, devicetree,
linux-arm-kernel, linux-kernel, linux-clk, linux-gpio,
linux-sunxi
In-Reply-To: <20190623043801.14040-5-icenowy@aosc.io>
On Sun, Jun 23, 2019 at 12:37:56PM +0800, Icenowy Zheng wrote:
> Allwinner V3 has the same main die with V3s, but with more pins wired.
> There's a I2S bus on V3 that is not available on V3s.
>
> Add the V3-only peripheral's clocks and reset to the V3s CCU driver,
> bound to a new V3 compatible string. The driver name is not changed
> because it's part of the device tree binding (the header file name).
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> No changes in v3/v2.
>
> drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 225 +++++++++++++++++++++-
> drivers/clk/sunxi-ng/ccu-sun8i-v3s.h | 2 +-
> include/dt-bindings/clock/sun8i-v3s-ccu.h | 4 +
> include/dt-bindings/reset/sun8i-v3s-ccu.h | 3 +
Acked-by: Rob Herring <robh@kernel.org>
> 4 files changed, 231 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [PATCH] gpio: em: use the managed version of gpiochip_add_data()
From: Bartosz Golaszewski @ 2019-07-09 19:26 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
Linux Kernel Mailing List
In-Reply-To: <CAMuHMdV=eVJKVENkLUi1pj7MY8RGwUGZEt=MG4fdfvToZZquNQ@mail.gmail.com>
wt., 9 lip 2019 o 20:49 Geert Uytterhoeven <geert@linux-m68k.org> napisał(a):
>
> Hi Bartosz,
>
> On Tue, May 28, 2019 at 5:46 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Use the managed variant of gpiochip_add_data() and remove the call to
> > gpiochip_remove().
> >
> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> > drivers/gpio/gpio-em.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-em.c b/drivers/gpio/gpio-em.c
> > index 40f8c38bec1c..299101d25fa8 100644
> > --- a/drivers/gpio/gpio-em.c
> > +++ b/drivers/gpio/gpio-em.c
> > @@ -359,7 +359,7 @@ static int em_gio_probe(struct platform_device *pdev)
> > goto err1;
> > }
> >
> > - ret = gpiochip_add_data(gpio_chip, p);
> > + ret = devm_gpiochip_add_data(&pdev->dev, gpio_chip, p);
> > if (ret) {
> > dev_err(&pdev->dev, "failed to add GPIO controller\n");
> > goto err1;
> > @@ -376,8 +376,6 @@ static int em_gio_remove(struct platform_device *pdev)
> > {
> > struct em_gio_priv *p = platform_get_drvdata(pdev);
> >
> > - gpiochip_remove(&p->gpio_chip);
> > -
> > irq_domain_remove(p->irq_domain);
>
> On a second thought, is it safe to call irq_domain_remove() before
> gpiochip_remove() (which calls gpiochip_irqchip_remove())?
>
Good call. I think the most elegant solution here would be to use
devm_add_action() to keep the ordering right. I'll send a follow-up
tomorrow morning.
Bart
> > return 0;
>
> > }
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] gpio: em: use the managed version of gpiochip_add_data()
From: Geert Uytterhoeven @ 2019-07-09 18:49 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
Linux Kernel Mailing List
In-Reply-To: <20190528154601.7597-1-brgl@bgdev.pl>
Hi Bartosz,
On Tue, May 28, 2019 at 5:46 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Use the managed variant of gpiochip_add_data() and remove the call to
> gpiochip_remove().
>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/gpio/gpio-em.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-em.c b/drivers/gpio/gpio-em.c
> index 40f8c38bec1c..299101d25fa8 100644
> --- a/drivers/gpio/gpio-em.c
> +++ b/drivers/gpio/gpio-em.c
> @@ -359,7 +359,7 @@ static int em_gio_probe(struct platform_device *pdev)
> goto err1;
> }
>
> - ret = gpiochip_add_data(gpio_chip, p);
> + ret = devm_gpiochip_add_data(&pdev->dev, gpio_chip, p);
> if (ret) {
> dev_err(&pdev->dev, "failed to add GPIO controller\n");
> goto err1;
> @@ -376,8 +376,6 @@ static int em_gio_remove(struct platform_device *pdev)
> {
> struct em_gio_priv *p = platform_get_drvdata(pdev);
>
> - gpiochip_remove(&p->gpio_chip);
> -
> irq_domain_remove(p->irq_domain);
On a second thought, is it safe to call irq_domain_remove() before
gpiochip_remove() (which calls gpiochip_irqchip_remove())?
> return 0;
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [GIT PULL] bulk GPIO changes for v5.3
From: pr-tracker-bot @ 2019-07-09 18:05 UTC (permalink / raw)
To: Linus Walleij
Cc: Linus Torvalds, open list:GPIO SUBSYSTEM, linux-kernel,
Bartosz Golaszewski
In-Reply-To: <CACRpkdYAJNVj98VjpmhY+suKfjH+WA4KWOvoHNAQvD60hzStbQ@mail.gmail.com>
The pull request you sent on Mon, 8 Jul 2019 09:26:55 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git tags/gpio-v5.3-1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/2ec98f567888501df0140c858af5f5ea10216a6f
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [PATCH v1 01/22] docs: Documentation/*.txt: rename all ReST files to *.rst
From: Rob Herring @ 2019-07-09 17:02 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, linux-fbdev, linux-ia64, kvm, linux-sh,
linux-pci, dri-devel, platform-driver-x86, kernel-hardening,
sparclinux, linux-arch, linux-s390, Jonathan Corbet, x86,
linux-security-module, devicetree, linux-watchdog,
Mauro Carvalho Chehab, linux-block, linux-gpio,
openipmi-developer, linux-arm-kernel, linaro-mm-sig, linux-parisc,
linux-mm, netdev, linux-wireless, linux-kernel, iommu,
linux-crypto
In-Reply-To: <6b6b6db8d6de9b66223dd6d4b43eb60ead4c71d7.1560891322.git.mchehab+samsung@kernel.org>
On Tue, Jun 18, 2019 at 06:05:25PM -0300, Mauro Carvalho Chehab wrote:
> Those files are actually at ReST format. Ok, currently, they
> don't belong to any place yet at the organized book series,
> but we don't want patches to break them as ReST files. So,
> rename them and add a :orphan: in order to shut up warning
> messages like those:
>
> ...
> Documentation/svga.rst: WARNING: document isn't included in any toctree
> Documentation/switchtec.rst: WARNING: document isn't included in any toctree
> ...
>
> Later patches will move them to a better place and remove the
> :orphan: markup.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>
> I had to remove the long list of maintainers got by
> getpatch.pl, as it was too long. I opted to keep only the
> mailing lists.
>
> Documentation/ABI/removed/sysfs-class-rfkill | 2 +-
> Documentation/ABI/stable/sysfs-class-rfkill | 2 +-
> Documentation/ABI/stable/sysfs-devices-node | 2 +-
> Documentation/ABI/testing/procfs-diskstats | 2 +-
> Documentation/ABI/testing/sysfs-block | 2 +-
> .../ABI/testing/sysfs-class-switchtec | 2 +-
> .../ABI/testing/sysfs-devices-system-cpu | 4 +-
> .../{DMA-API-HOWTO.txt => DMA-API-HOWTO.rst} | 2 +
> Documentation/{DMA-API.txt => DMA-API.rst} | 8 ++-
> .../{DMA-ISA-LPC.txt => DMA-ISA-LPC.rst} | 4 +-
> ...{DMA-attributes.txt => DMA-attributes.rst} | 2 +
> Documentation/{IPMI.txt => IPMI.rst} | 2 +
> .../{IRQ-affinity.txt => IRQ-affinity.rst} | 2 +
> .../{IRQ-domain.txt => IRQ-domain.rst} | 2 +
> Documentation/{IRQ.txt => IRQ.rst} | 2 +
> .../{Intel-IOMMU.txt => Intel-IOMMU.rst} | 2 +
> Documentation/PCI/pci.rst | 8 +--
> Documentation/{SAK.txt => SAK.rst} | 3 +-
> Documentation/{SM501.txt => SM501.rst} | 2 +
> Documentation/admin-guide/hw-vuln/l1tf.rst | 2 +-
> .../admin-guide/kernel-parameters.txt | 4 +-
> .../{atomic_bitops.txt => atomic_bitops.rst} | 3 +-
> Documentation/block/biodoc.txt | 2 +-
> .../{bt8xxgpio.txt => bt8xxgpio.rst} | 3 +-
> Documentation/{btmrvl.txt => btmrvl.rst} | 2 +
> ...-mapping.txt => bus-virt-phys-mapping.rst} | 54 +++++++++---------
> ...g-warn-once.txt => clearing-warn-once.rst} | 2 +
> Documentation/{cpu-load.txt => cpu-load.rst} | 2 +
> .../{cputopology.txt => cputopology.rst} | 2 +
> Documentation/{crc32.txt => crc32.rst} | 2 +
> Documentation/{dcdbas.txt => dcdbas.rst} | 2 +
> ...ging-modules.txt => debugging-modules.rst} | 2 +
> ...hci1394.txt => debugging-via-ohci1394.rst} | 2 +
> Documentation/{dell_rbu.txt => dell_rbu.rst} | 3 +-
> Documentation/device-mapper/statistics.rst | 4 +-
> .../devicetree/bindings/phy/phy-bindings.txt | 2 +-
Acked-by: Rob Herring <robh@kernel.org>
> Documentation/{digsig.txt => digsig.rst} | 2 +
> Documentation/driver-api/usb/dma.rst | 6 +-
> Documentation/driver-model/device.rst | 2 +-
> Documentation/{efi-stub.txt => efi-stub.rst} | 2 +
> Documentation/{eisa.txt => eisa.rst} | 2 +
> Documentation/fb/vesafb.rst | 2 +-
> Documentation/filesystems/sysfs.txt | 2 +-
> ...ex-requeue-pi.txt => futex-requeue-pi.rst} | 2 +
> .../{gcc-plugins.txt => gcc-plugins.rst} | 2 +
> Documentation/gpu/drm-mm.rst | 2 +-
> Documentation/{highuid.txt => highuid.rst} | 4 +-
> .../{hw_random.txt => hw_random.rst} | 2 +
> .../{hwspinlock.txt => hwspinlock.rst} | 2 +
> Documentation/ia64/irq-redir.rst | 2 +-
> .../{intel_txt.txt => intel_txt.rst} | 2 +
> .../{io-mapping.txt => io-mapping.rst} | 2 +
> .../{io_ordering.txt => io_ordering.rst} | 2 +
> Documentation/{iostats.txt => iostats.rst} | 2 +
> ...flags-tracing.txt => irqflags-tracing.rst} | 3 +-
> Documentation/{isa.txt => isa.rst} | 2 +
> Documentation/{isapnp.txt => isapnp.rst} | 2 +
> ...hreads.txt => kernel-per-CPU-kthreads.rst} | 4 +-
> Documentation/{kobject.txt => kobject.rst} | 6 +-
> Documentation/{kprobes.txt => kprobes.rst} | 3 +-
> Documentation/{kref.txt => kref.rst} | 2 +
> Documentation/laptops/thinkpad-acpi.rst | 6 +-
> Documentation/{ldm.txt => ldm.rst} | 5 +-
> Documentation/locking/rt-mutex.rst | 2 +-
> ...kup-watchdogs.txt => lockup-watchdogs.rst} | 2 +
> Documentation/{lsm.txt => lsm.rst} | 2 +
> Documentation/{lzo.txt => lzo.rst} | 2 +
> Documentation/{mailbox.txt => mailbox.rst} | 2 +
> Documentation/memory-barriers.txt | 6 +-
> ...hameleon-bus.txt => men-chameleon-bus.rst} | 2 +
> Documentation/networking/scaling.rst | 4 +-
> .../{nommu-mmap.txt => nommu-mmap.rst} | 2 +
> Documentation/{ntb.txt => ntb.rst} | 2 +
> Documentation/{numastat.txt => numastat.rst} | 3 +-
> Documentation/{padata.txt => padata.rst} | 2 +
> ...port-lowlevel.txt => parport-lowlevel.rst} | 2 +
> ...-semaphore.txt => percpu-rw-semaphore.rst} | 2 +
> Documentation/{phy.txt => phy.rst} | 2 +
> Documentation/{pi-futex.txt => pi-futex.rst} | 2 +
> Documentation/{pnp.txt => pnp.rst} | 13 +++--
> ...reempt-locking.txt => preempt-locking.rst} | 4 +-
> Documentation/{pwm.txt => pwm.rst} | 2 +
> Documentation/{rbtree.txt => rbtree.rst} | 54 +++++++++---------
> .../{remoteproc.txt => remoteproc.rst} | 4 +-
> Documentation/{rfkill.txt => rfkill.rst} | 2 +
> ...ust-futex-ABI.txt => robust-futex-ABI.rst} | 2 +
> ...{robust-futexes.txt => robust-futexes.rst} | 2 +
> Documentation/{rpmsg.txt => rpmsg.rst} | 2 +
> Documentation/{rtc.txt => rtc.rst} | 8 ++-
> Documentation/s390/vfio-ccw.rst | 6 +-
> Documentation/{sgi-ioc4.txt => sgi-ioc4.rst} | 2 +
> Documentation/{siphash.txt => siphash.rst} | 2 +
> .../{smsc_ece1099.txt => smsc_ece1099.rst} | 2 +
> .../{speculation.txt => speculation.rst} | 2 +
> .../{static-keys.txt => static-keys.rst} | 2 +
> Documentation/{svga.txt => svga.rst} | 2 +
> .../{switchtec.txt => switchtec.rst} | 4 +-
> .../{sync_file.txt => sync_file.rst} | 2 +
> Documentation/sysctl/kernel.txt | 4 +-
> Documentation/sysctl/vm.txt | 2 +-
> Documentation/{tee.txt => tee.rst} | 2 +
> .../{this_cpu_ops.txt => this_cpu_ops.rst} | 2 +
> Documentation/trace/kprobetrace.rst | 2 +-
> .../translations/ko_KR/memory-barriers.txt | 6 +-
> Documentation/translations/zh_CN/IRQ.txt | 4 +-
> .../translations/zh_CN/filesystems/sysfs.txt | 2 +-
> .../translations/zh_CN/io_ordering.txt | 4 +-
> ...access.txt => unaligned-memory-access.rst} | 2 +
> ...ed-device.txt => vfio-mediated-device.rst} | 4 +-
> Documentation/{vfio.txt => vfio.rst} | 2 +
> .../{video-output.txt => video-output.rst} | 3 +-
> Documentation/watchdog/hpwdt.rst | 2 +-
> Documentation/x86/topology.rst | 2 +-
> Documentation/{xillybus.txt => xillybus.rst} | 2 +
> Documentation/{xz.txt => xz.rst} | 2 +
> Documentation/{zorro.txt => zorro.rst} | 7 ++-
> MAINTAINERS | 56 +++++++++----------
> arch/Kconfig | 4 +-
> arch/arm/Kconfig | 2 +-
> arch/ia64/hp/common/sba_iommu.c | 12 ++--
> arch/ia64/sn/pci/pci_dma.c | 4 +-
> arch/parisc/Kconfig | 2 +-
> arch/parisc/kernel/pci-dma.c | 2 +-
> arch/sh/Kconfig | 2 +-
> arch/sparc/Kconfig | 2 +-
> arch/unicore32/include/asm/io.h | 2 +-
> arch/x86/Kconfig | 4 +-
> arch/x86/include/asm/dma-mapping.h | 4 +-
> arch/x86/kernel/amd_gart_64.c | 2 +-
> block/partitions/Kconfig | 2 +-
> drivers/base/core.c | 2 +-
> drivers/char/Kconfig | 4 +-
> drivers/char/hw_random/core.c | 2 +-
> drivers/char/ipmi/Kconfig | 2 +-
> drivers/char/ipmi/ipmi_si_hotmod.c | 2 +-
> drivers/char/ipmi/ipmi_si_intf.c | 2 +-
> drivers/dma-buf/Kconfig | 2 +-
> drivers/gpio/Kconfig | 2 +-
> drivers/parisc/sba_iommu.c | 16 +++---
> drivers/pci/switch/Kconfig | 2 +-
> drivers/platform/x86/Kconfig | 4 +-
> drivers/platform/x86/dcdbas.c | 2 +-
> drivers/platform/x86/dell_rbu.c | 2 +-
> drivers/pnp/isapnp/Kconfig | 2 +-
> drivers/vfio/Kconfig | 2 +-
> drivers/vfio/mdev/Kconfig | 2 +-
> include/asm-generic/bitops/atomic.h | 2 +-
> include/linux/dma-mapping.h | 2 +-
> include/linux/hw_random.h | 2 +-
> include/linux/io-mapping.h | 2 +-
> include/linux/jump_label.h | 2 +-
> include/linux/kobject.h | 2 +-
> include/linux/kobject_ns.h | 2 +-
> include/linux/rbtree.h | 2 +-
> include/linux/rbtree_augmented.h | 2 +-
> include/media/videobuf-dma-sg.h | 2 +-
> init/Kconfig | 2 +-
> kernel/dma/debug.c | 2 +-
> kernel/padata.c | 2 +-
> lib/Kconfig | 2 +-
> lib/Kconfig.debug | 2 +-
> lib/crc32.c | 2 +-
> lib/kobject.c | 4 +-
> lib/lzo/lzo1x_decompress_safe.c | 2 +-
> lib/xz/Kconfig | 2 +-
> mm/Kconfig | 2 +-
> mm/nommu.c | 2 +-
> samples/kprobes/kprobe_example.c | 2 +-
> samples/kprobes/kretprobe_example.c | 2 +-
> scripts/gcc-plugins/Kconfig | 2 +-
> security/Kconfig | 2 +-
> tools/include/linux/rbtree.h | 2 +-
> tools/include/linux/rbtree_augmented.h | 2 +-
> 173 files changed, 397 insertions(+), 242 deletions(-)
> rename Documentation/{DMA-API-HOWTO.txt => DMA-API-HOWTO.rst} (99%)
> rename Documentation/{DMA-API.txt => DMA-API.rst} (99%)
> rename Documentation/{DMA-ISA-LPC.txt => DMA-ISA-LPC.rst} (98%)
> rename Documentation/{DMA-attributes.txt => DMA-attributes.rst} (99%)
> rename Documentation/{IPMI.txt => IPMI.rst} (99%)
> rename Documentation/{IRQ-affinity.txt => IRQ-affinity.rst} (99%)
> rename Documentation/{IRQ-domain.txt => IRQ-domain.rst} (99%)
> rename Documentation/{IRQ.txt => IRQ.rst} (99%)
> rename Documentation/{Intel-IOMMU.txt => Intel-IOMMU.rst} (99%)
> rename Documentation/{SAK.txt => SAK.rst} (99%)
> rename Documentation/{SM501.txt => SM501.rst} (99%)
> rename Documentation/{atomic_bitops.txt => atomic_bitops.rst} (99%)
> rename Documentation/{bt8xxgpio.txt => bt8xxgpio.rst} (99%)
> rename Documentation/{btmrvl.txt => btmrvl.rst} (99%)
> rename Documentation/{bus-virt-phys-mapping.txt => bus-virt-phys-mapping.rst} (93%)
> rename Documentation/{clearing-warn-once.txt => clearing-warn-once.rst} (96%)
> rename Documentation/{cpu-load.txt => cpu-load.rst} (99%)
> rename Documentation/{cputopology.txt => cputopology.rst} (99%)
> rename Documentation/{crc32.txt => crc32.rst} (99%)
> rename Documentation/{dcdbas.txt => dcdbas.rst} (99%)
> rename Documentation/{debugging-modules.txt => debugging-modules.rst} (98%)
> rename Documentation/{debugging-via-ohci1394.txt => debugging-via-ohci1394.rst} (99%)
> rename Documentation/{dell_rbu.txt => dell_rbu.rst} (99%)
> rename Documentation/{digsig.txt => digsig.rst} (99%)
> rename Documentation/{efi-stub.txt => efi-stub.rst} (99%)
> rename Documentation/{eisa.txt => eisa.rst} (99%)
> rename Documentation/{futex-requeue-pi.txt => futex-requeue-pi.rst} (99%)
> rename Documentation/{gcc-plugins.txt => gcc-plugins.rst} (99%)
> rename Documentation/{highuid.txt => highuid.rst} (99%)
> rename Documentation/{hw_random.txt => hw_random.rst} (99%)
> rename Documentation/{hwspinlock.txt => hwspinlock.rst} (99%)
> rename Documentation/{intel_txt.txt => intel_txt.rst} (99%)
> rename Documentation/{io-mapping.txt => io-mapping.rst} (99%)
> rename Documentation/{io_ordering.txt => io_ordering.rst} (99%)
> rename Documentation/{iostats.txt => iostats.rst} (99%)
> rename Documentation/{irqflags-tracing.txt => irqflags-tracing.rst} (99%)
> rename Documentation/{isa.txt => isa.rst} (99%)
> rename Documentation/{isapnp.txt => isapnp.rst} (98%)
> rename Documentation/{kernel-per-CPU-kthreads.txt => kernel-per-CPU-kthreads.rst} (99%)
> rename Documentation/{kobject.txt => kobject.rst} (99%)
> rename Documentation/{kprobes.txt => kprobes.rst} (99%)
> rename Documentation/{kref.txt => kref.rst} (99%)
> rename Documentation/{ldm.txt => ldm.rst} (98%)
> rename Documentation/{lockup-watchdogs.txt => lockup-watchdogs.rst} (99%)
> rename Documentation/{lsm.txt => lsm.rst} (99%)
> rename Documentation/{lzo.txt => lzo.rst} (99%)
> rename Documentation/{mailbox.txt => mailbox.rst} (99%)
> rename Documentation/{men-chameleon-bus.txt => men-chameleon-bus.rst} (99%)
> rename Documentation/{nommu-mmap.txt => nommu-mmap.rst} (99%)
> rename Documentation/{ntb.txt => ntb.rst} (99%)
> rename Documentation/{numastat.txt => numastat.rst} (99%)
> rename Documentation/{padata.txt => padata.rst} (99%)
> rename Documentation/{parport-lowlevel.txt => parport-lowlevel.rst} (99%)
> rename Documentation/{percpu-rw-semaphore.txt => percpu-rw-semaphore.rst} (99%)
> rename Documentation/{phy.txt => phy.rst} (99%)
> rename Documentation/{pi-futex.txt => pi-futex.rst} (99%)
> rename Documentation/{pnp.txt => pnp.rst} (98%)
> rename Documentation/{preempt-locking.txt => preempt-locking.rst} (99%)
> rename Documentation/{pwm.txt => pwm.rst} (99%)
> rename Documentation/{rbtree.txt => rbtree.rst} (94%)
> rename Documentation/{remoteproc.txt => remoteproc.rst} (99%)
> rename Documentation/{rfkill.txt => rfkill.rst} (99%)
> rename Documentation/{robust-futex-ABI.txt => robust-futex-ABI.rst} (99%)
> rename Documentation/{robust-futexes.txt => robust-futexes.rst} (99%)
> rename Documentation/{rpmsg.txt => rpmsg.rst} (99%)
> rename Documentation/{rtc.txt => rtc.rst} (99%)
> rename Documentation/{sgi-ioc4.txt => sgi-ioc4.rst} (99%)
> rename Documentation/{siphash.txt => siphash.rst} (99%)
> rename Documentation/{smsc_ece1099.txt => smsc_ece1099.rst} (99%)
> rename Documentation/{speculation.txt => speculation.rst} (99%)
> rename Documentation/{static-keys.txt => static-keys.rst} (99%)
> rename Documentation/{svga.txt => svga.rst} (99%)
> rename Documentation/{switchtec.txt => switchtec.rst} (98%)
> rename Documentation/{sync_file.txt => sync_file.rst} (99%)
> rename Documentation/{tee.txt => tee.rst} (99%)
> rename Documentation/{this_cpu_ops.txt => this_cpu_ops.rst} (99%)
> rename Documentation/{unaligned-memory-access.txt => unaligned-memory-access.rst} (99%)
> rename Documentation/{vfio-mediated-device.txt => vfio-mediated-device.rst} (99%)
> rename Documentation/{vfio.txt => vfio.rst} (99%)
> rename Documentation/{video-output.txt => video-output.rst} (99%)
> rename Documentation/{xillybus.txt => xillybus.rst} (99%)
> rename Documentation/{xz.txt => xz.rst} (99%)
> rename Documentation/{zorro.txt => zorro.rst} (99%)
^ permalink raw reply
* Re: [PATCH V4 2/2] gpio: inverter: document the inverter bindings
From: Rob Herring @ 2019-07-09 16:08 UTC (permalink / raw)
To: Harish Jenny K N
Cc: Linus Walleij, Bartosz Golaszewski, Mark Rutland, devicetree,
open list:GPIO SUBSYSTEM, Balasubramani Vivekanandan
In-Reply-To: <06c95f15-d577-e43d-e046-ee222f86c406@mentor.com>
On Mon, Jul 8, 2019 at 11:25 PM Harish Jenny K N
<harish_kandiga@mentor.com> wrote:
>
> Hi Rob,
>
>
> On 09/07/19 4:06 AM, Rob Herring wrote:
> > On Fri, Jun 28, 2019 at 3:31 AM Harish Jenny K N
> > <harish_kandiga@mentor.com> wrote:
> >> Document the device tree binding for the inverter gpio
> >> controller to configure the polarity of the gpio pins
> >> used by the consumers.
> >>
> >> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> >> ---
> >> .../devicetree/bindings/gpio/gpio-inverter.txt | 29 ++++++++++++++++++++++
> >> 1 file changed, 29 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-inverter.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/gpio/gpio-inverter.txt b/Documentation/devicetree/bindings/gpio/gpio-inverter.txt
> >> new file mode 100644
> >> index 0000000..8bb6b2e
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/gpio/gpio-inverter.txt
> >> @@ -0,0 +1,29 @@
> >> +GPIO-INVERTER
> >> +======
> >> +This binding defines the gpio-inverter. The gpio-inverter is a driver that
> >> +allows to properly describe the gpio polarities on the hardware.
> > I don't understand. Please explain this in terms of the hardware, not a driver.
>
>
> gpio inverters can be used on different hardware to alter the polarity of gpio chips.
> The polarity of pins can change from hardware to hardware with the use of inverters.
Yes, I know what an inverter is.
> This device tree binding models gpio inverters in the device tree to properly describe the hardware.
We already define the active state of GPIOs in the consumers. If
there's an inverter in the middle, the consumer active state is simply
inverted. I don't agree that that is a hack as Linus said without some
reasoning why an inverter needs to be modeled in DT. Anything about
what 'userspace' needs is not a reason. That's a Linux thing that has
little to do with hardware description.
Rob
^ permalink raw reply
* Re: [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
From: Geert Uytterhoeven @ 2019-07-09 15:59 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Geert Uytterhoeven, Linus Walleij, Alexander Graf, Peter Maydell,
Paolo Bonzini, Magnus Damm, linux-gpio, QEMU Developers,
Linux-Renesas, LKML
In-Reply-To: <CAMpxmJUF1s1zyXVtoUGfbV7Yk+heua4rNjY=DrX=jr-v8UfNxA@mail.gmail.com>
Hi Bartosz,
On Tue, Jul 9, 2019 at 4:59 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
> pon., 8 lip 2019 o 12:24 Geert Uytterhoeven <geert@linux-m68k.org> napisał(a):
> > On Mon, Jul 8, 2019 at 11:45 AM Bartosz Golaszewski
> > <bgolaszewski@baylibre.com> wrote:
> > > pt., 5 lip 2019 o 18:05 Geert Uytterhoeven <geert+renesas@glider.be> napisał(a):
> > > > 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
> Am I doing it right? I'm trying to create a device and am only getting this:
>
> # echo gpiochip2 23 > new_device
> [ 707.507039] gpio-virt-agg gpio-virt-agg.0: Cannot find gpiochip gpiochip2
>
> gpiochip2 *does* exist in the system.
Please try the name of the platform device instead.
I.e. for my koelsch (R-Car M2-W), it needs "e6052000.gpio" instead
of "gpiochip2".
Probably the driver should match on both.
> I see. I'll try to review it more thoroughly once I get to play with
> it. So far I'm stuck on creating the virtual chip.
Thanks, good luck!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
From: Bartosz Golaszewski @ 2019-07-09 14:58 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Linus Walleij, Alexander Graf, Peter Maydell,
Paolo Bonzini, Magnus Damm, linux-gpio, QEMU Developers,
Linux-Renesas, LKML
In-Reply-To: <CAMuHMdWdb0dcS8Nvk-Poz2dT7nuHjFhqpsRPZZnSKsc3VffcRA@mail.gmail.com>
pon., 8 lip 2019 o 12:24 Geert Uytterhoeven <geert@linux-m68k.org> napisał(a):
>
> Hi Bartosz,
>
> On Mon, Jul 8, 2019 at 11:45 AM Bartosz Golaszewski
> <bgolaszewski@baylibre.com> wrote:
> > pt., 5 lip 2019 o 18:05 Geert Uytterhoeven <geert+renesas@glider.be> napisał(a):
> > > 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>
>
> > I like the general idea and the interface looks mostly fine. Since
> > this is new ABI I think it needs to be documented as well.
>
> Sure.
>
> > I'm having trouble building this module:
> >
> > CALL scripts/atomic/check-atomics.sh
> > CALL scripts/checksyscalls.sh
> > CHK include/generated/compile.h
> > Kernel: arch/arm/boot/Image is ready
> > Building modules, stage 2.
> > MODPOST 235 modules
> > ERROR: "gpiod_request" [drivers/gpio/gpio-virt-agg.ko] undefined!
> > ERROR: "gpiochip_get_desc" [drivers/gpio/gpio-virt-agg.ko] undefined!
> > ERROR: "gpiod_free" [drivers/gpio/gpio-virt-agg.ko] undefined!
> > scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> > make[1]: *** [__modpost] Error 1
> > Makefile:1287: recipe for target 'modules' failed
> > make: *** [modules] Error 2
> > make: *** Waiting for unfinished jobs....
> >
> > I'm not sure what the problem is.
>
> Oops. As this is an RFC, I didn't bother trying to build this driver as
> a module, only builtin. Apparently the 3 symbols above are not yet
> exported using EXPORT_SYMBOL_GPL().
>
Am I doing it right? I'm trying to create a device and am only getting this:
# echo gpiochip2 23 > new_device
[ 707.507039] gpio-virt-agg gpio-virt-agg.0: Cannot find gpiochip gpiochip2
gpiochip2 *does* exist in the system.
> > > --- /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
> >
> > Do we really need this limit? I see it simplifies the code, but maybe
> > we can allocate the relevant arrays dynamically and not limit users?
>
> Sure. That limit can be lifted.
>
> > > +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
>
> BTW, do you have a comment about this FIXME?
>
Ha! Interesting. I'll give it a thought and respond elsewhere as it's
a different subject.
> > > + return -ENOTSUPP;
> > > +}
>
> > > +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;
> >
> > This 'i' here is reported as possibly not initialized:
> >
> > drivers/gpio/gpio-virt-agg.c: In function ‘gpio_virt_agg_probe’:
> > drivers/gpio/gpio-virt-agg.c:230:13: warning: ‘i’ may be used
> > uninitialized in this function [-Wmaybe-uninitialized]
> > int error, i;
> > ^
>
> Oops, should be preinitialized to zero. WIll fix.
>
> > > +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]);
>
> Perhaps I should use gpiod_put() instead, which is exported to modules?
>
> > > +
> > > + return 0;
> > > +}
> >
> > You shouldn't need this function at all. It's up to users to free descriptors.
>
> This frees the upstream descriptors, not the descriptors used by users
> of the virtual gpiochip. Shouldn't they be freed, as they are no longer
> in use?
>
> Note that .probe() doesn't use devm_gpiochip_add_data(), as the upstream
> descriptors need to be freed after the call to gpiochip_remove().
>
> Thanks!
I see. I'll try to review it more thoroughly once I get to play with
it. So far I'm stuck on creating the virtual chip.
Bart
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* [pinctrl:for-next 80/80] drivers/pinctrl/aspeed/pinctrl-aspeed.c:83:34: error: passing argument 2 of 'aspeed_sig_expr_eval' from incompatible pointer type
From: kbuild test robot @ 2019-07-09 14:44 UTC (permalink / raw)
To: Linus Walleij; +Cc: kbuild-all, linux-gpio
[-- Attachment #1: Type: text/plain, Size: 13474 bytes --]
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git for-next
head: 35d8510ea3ada735ad0564cddffbc12434a4ec3d
commit: 35d8510ea3ada735ad0564cddffbc12434a4ec3d [80/80] pinctrl: aspeed: Fix missed include
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 35d8510ea3ada735ad0564cddffbc12434a4ec3d
# 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 >>):
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:0:
drivers/pinctrl/aspeed/pinctrl-aspeed.h:282:8: error: redefinition of 'struct aspeed_sig_desc'
struct aspeed_sig_desc {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:
drivers/pinctrl/aspeed/pinmux-aspeed.h:441:8: note: originally defined here
struct aspeed_sig_desc {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:0:
drivers/pinctrl/aspeed/pinctrl-aspeed.h:303:8: error: redefinition of 'struct aspeed_sig_expr'
struct aspeed_sig_expr {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:
drivers/pinctrl/aspeed/pinmux-aspeed.h:462:8: note: originally defined here
struct aspeed_sig_expr {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:0:
drivers/pinctrl/aspeed/pinctrl-aspeed.h:320:8: error: redefinition of 'struct aspeed_pin_desc'
struct aspeed_pin_desc {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:
drivers/pinctrl/aspeed/pinmux-aspeed.h:479:8: note: originally defined here
struct aspeed_pin_desc {
^~~~~~~~~~~~~~~
drivers/pinctrl/aspeed/pinctrl-aspeed.c: In function 'aspeed_sig_expr_enable':
>> drivers/pinctrl/aspeed/pinctrl-aspeed.c:83:34: error: passing argument 2 of 'aspeed_sig_expr_eval' from incompatible pointer type [-Werror=incompatible-pointer-types]
ret = aspeed_sig_expr_eval(ctx, expr, true);
^~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:
drivers/pinctrl/aspeed/pinmux-aspeed.h:724:5: note: expected 'const struct aspeed_sig_expr *' but argument is of type 'const struct aspeed_sig_expr *'
int aspeed_sig_expr_eval(const struct aspeed_pinmux_data *ctx,
^~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/aspeed/pinctrl-aspeed.c:88:35: error: passing argument 2 of 'aspeed_sig_expr_set' from incompatible pointer type [-Werror=incompatible-pointer-types]
return aspeed_sig_expr_set(ctx, expr, true);
^~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:
drivers/pinctrl/aspeed/pinmux-aspeed.h:728:19: note: expected 'const struct aspeed_sig_expr *' but argument is of type 'const struct aspeed_sig_expr *'
static inline int aspeed_sig_expr_set(const struct aspeed_pinmux_data *ctx,
^~~~~~~~~~~~~~~~~~~
drivers/pinctrl/aspeed/pinctrl-aspeed.c: In function 'aspeed_sig_expr_disable':
drivers/pinctrl/aspeed/pinctrl-aspeed.c:98:34: error: passing argument 2 of 'aspeed_sig_expr_eval' from incompatible pointer type [-Werror=incompatible-pointer-types]
ret = aspeed_sig_expr_eval(ctx, expr, true);
^~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:
drivers/pinctrl/aspeed/pinmux-aspeed.h:724:5: note: expected 'const struct aspeed_sig_expr *' but argument is of type 'const struct aspeed_sig_expr *'
int aspeed_sig_expr_eval(const struct aspeed_pinmux_data *ctx,
^~~~~~~~~~~~~~~~~~~~
drivers/pinctrl/aspeed/pinctrl-aspeed.c:103:35: error: passing argument 2 of 'aspeed_sig_expr_set' from incompatible pointer type [-Werror=incompatible-pointer-types]
return aspeed_sig_expr_set(ctx, expr, false);
^~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed.c:15:
drivers/pinctrl/aspeed/pinmux-aspeed.h:728:19: note: expected 'const struct aspeed_sig_expr *' but argument is of type 'const struct aspeed_sig_expr *'
static inline int aspeed_sig_expr_set(const struct aspeed_pinmux_data *ctx,
^~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:26:0:
drivers/pinctrl/aspeed/pinctrl-aspeed.h:282:8: error: redefinition of 'struct aspeed_sig_desc'
struct aspeed_sig_desc {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:26:
drivers/pinctrl/aspeed/pinmux-aspeed.h:441:8: note: originally defined here
struct aspeed_sig_desc {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:26:0:
drivers/pinctrl/aspeed/pinctrl-aspeed.h:303:8: error: redefinition of 'struct aspeed_sig_expr'
struct aspeed_sig_expr {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:26:
drivers/pinctrl/aspeed/pinmux-aspeed.h:462:8: note: originally defined here
struct aspeed_sig_expr {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:26:0:
drivers/pinctrl/aspeed/pinctrl-aspeed.h:320:8: error: redefinition of 'struct aspeed_pin_desc'
struct aspeed_pin_desc {
^~~~~~~~~~~~~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:26:
drivers/pinctrl/aspeed/pinmux-aspeed.h:479:8: note: originally defined here
struct aspeed_pin_desc {
^~~~~~~~~~~~~~~
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c: In function 'aspeed_g5_sig_expr_set':
>> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:2582:34: error: passing argument 2 of 'aspeed_sig_expr_eval' from incompatible pointer type [-Werror=incompatible-pointer-types]
ret = aspeed_sig_expr_eval(ctx, expr, enable);
^~~~
In file included from drivers/pinctrl/aspeed/pinctrl-aspeed.h:19:0,
from drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:26:
drivers/pinctrl/aspeed/pinmux-aspeed.h:724:5: note: expected 'const struct aspeed_sig_expr *' but argument is of type 'const struct aspeed_sig_expr *'
int aspeed_sig_expr_eval(const struct aspeed_pinmux_data *ctx,
^~~~~~~~~~~~~~~~~~~~
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c: At top level:
>> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:2593:9: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.set = aspeed_g5_sig_expr_set,
^~~~~~~~~~~~~~~~~~~~~~
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c:2593:9: note: (near initialization for 'aspeed_g5_ops.set')
cc1: some warnings being treated as errors
vim +/aspeed_sig_expr_eval +83 drivers/pinctrl/aspeed/pinctrl-aspeed.c
4d3d0e427 Andrew Jeffery 2016-08-30 @15 #include "pinctrl-aspeed.h"
4d3d0e427 Andrew Jeffery 2016-08-30 16
4d3d0e427 Andrew Jeffery 2016-08-30 17 int aspeed_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
4d3d0e427 Andrew Jeffery 2016-08-30 18 {
4d3d0e427 Andrew Jeffery 2016-08-30 19 struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
4d3d0e427 Andrew Jeffery 2016-08-30 20
efa562398 Andrew Jeffery 2019-06-28 21 return pdata->pinmux.ngroups;
4d3d0e427 Andrew Jeffery 2016-08-30 22 }
4d3d0e427 Andrew Jeffery 2016-08-30 23
4d3d0e427 Andrew Jeffery 2016-08-30 24 const char *aspeed_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
4d3d0e427 Andrew Jeffery 2016-08-30 25 unsigned int group)
4d3d0e427 Andrew Jeffery 2016-08-30 26 {
4d3d0e427 Andrew Jeffery 2016-08-30 27 struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
4d3d0e427 Andrew Jeffery 2016-08-30 28
efa562398 Andrew Jeffery 2019-06-28 29 return pdata->pinmux.groups[group].name;
4d3d0e427 Andrew Jeffery 2016-08-30 30 }
4d3d0e427 Andrew Jeffery 2016-08-30 31
4d3d0e427 Andrew Jeffery 2016-08-30 32 int aspeed_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
4d3d0e427 Andrew Jeffery 2016-08-30 33 unsigned int group, const unsigned int **pins,
4d3d0e427 Andrew Jeffery 2016-08-30 34 unsigned int *npins)
4d3d0e427 Andrew Jeffery 2016-08-30 35 {
4d3d0e427 Andrew Jeffery 2016-08-30 36 struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
4d3d0e427 Andrew Jeffery 2016-08-30 37
efa562398 Andrew Jeffery 2019-06-28 38 *pins = &pdata->pinmux.groups[group].pins[0];
efa562398 Andrew Jeffery 2019-06-28 39 *npins = pdata->pinmux.groups[group].npins;
4d3d0e427 Andrew Jeffery 2016-08-30 40
4d3d0e427 Andrew Jeffery 2016-08-30 41 return 0;
4d3d0e427 Andrew Jeffery 2016-08-30 42 }
4d3d0e427 Andrew Jeffery 2016-08-30 43
4d3d0e427 Andrew Jeffery 2016-08-30 44 void aspeed_pinctrl_pin_dbg_show(struct pinctrl_dev *pctldev,
4d3d0e427 Andrew Jeffery 2016-08-30 45 struct seq_file *s, unsigned int offset)
4d3d0e427 Andrew Jeffery 2016-08-30 46 {
4d3d0e427 Andrew Jeffery 2016-08-30 47 seq_printf(s, " %s", dev_name(pctldev->dev));
4d3d0e427 Andrew Jeffery 2016-08-30 48 }
4d3d0e427 Andrew Jeffery 2016-08-30 49
4d3d0e427 Andrew Jeffery 2016-08-30 50 int aspeed_pinmux_get_fn_count(struct pinctrl_dev *pctldev)
4d3d0e427 Andrew Jeffery 2016-08-30 51 {
4d3d0e427 Andrew Jeffery 2016-08-30 52 struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
4d3d0e427 Andrew Jeffery 2016-08-30 53
efa562398 Andrew Jeffery 2019-06-28 54 return pdata->pinmux.nfunctions;
4d3d0e427 Andrew Jeffery 2016-08-30 55 }
4d3d0e427 Andrew Jeffery 2016-08-30 56
4d3d0e427 Andrew Jeffery 2016-08-30 57 const char *aspeed_pinmux_get_fn_name(struct pinctrl_dev *pctldev,
4d3d0e427 Andrew Jeffery 2016-08-30 58 unsigned int function)
4d3d0e427 Andrew Jeffery 2016-08-30 59 {
4d3d0e427 Andrew Jeffery 2016-08-30 60 struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
4d3d0e427 Andrew Jeffery 2016-08-30 61
efa562398 Andrew Jeffery 2019-06-28 62 return pdata->pinmux.functions[function].name;
4d3d0e427 Andrew Jeffery 2016-08-30 63 }
4d3d0e427 Andrew Jeffery 2016-08-30 64
4d3d0e427 Andrew Jeffery 2016-08-30 65 int aspeed_pinmux_get_fn_groups(struct pinctrl_dev *pctldev,
4d3d0e427 Andrew Jeffery 2016-08-30 66 unsigned int function,
4d3d0e427 Andrew Jeffery 2016-08-30 67 const char * const **groups,
4d3d0e427 Andrew Jeffery 2016-08-30 68 unsigned int * const num_groups)
4d3d0e427 Andrew Jeffery 2016-08-30 69 {
4d3d0e427 Andrew Jeffery 2016-08-30 70 struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
4d3d0e427 Andrew Jeffery 2016-08-30 71
efa562398 Andrew Jeffery 2019-06-28 72 *groups = pdata->pinmux.functions[function].groups;
efa562398 Andrew Jeffery 2019-06-28 73 *num_groups = pdata->pinmux.functions[function].ngroups;
4d3d0e427 Andrew Jeffery 2016-08-30 74
4d3d0e427 Andrew Jeffery 2016-08-30 75 return 0;
4d3d0e427 Andrew Jeffery 2016-08-30 76 }
4d3d0e427 Andrew Jeffery 2016-08-30 77
efa562398 Andrew Jeffery 2019-06-28 78 static int aspeed_sig_expr_enable(const struct aspeed_pinmux_data *ctx,
efa562398 Andrew Jeffery 2019-06-28 79 const struct aspeed_sig_expr *expr)
4d3d0e427 Andrew Jeffery 2016-08-30 80 {
7d29ed88a Andrew Jeffery 2016-12-20 81 int ret;
4d3d0e427 Andrew Jeffery 2016-08-30 82
efa562398 Andrew Jeffery 2019-06-28 @83 ret = aspeed_sig_expr_eval(ctx, expr, true);
7d29ed88a Andrew Jeffery 2016-12-20 84 if (ret < 0)
7d29ed88a Andrew Jeffery 2016-12-20 85 return ret;
7d29ed88a Andrew Jeffery 2016-12-20 86
7d29ed88a Andrew Jeffery 2016-12-20 87 if (!ret)
efa562398 Andrew Jeffery 2019-06-28 @88 return aspeed_sig_expr_set(ctx, expr, true);
7d29ed88a Andrew Jeffery 2016-12-20 89
7d29ed88a Andrew Jeffery 2016-12-20 90 return 0;
4d3d0e427 Andrew Jeffery 2016-08-30 91 }
4d3d0e427 Andrew Jeffery 2016-08-30 92
:::::: The code at line 83 was first introduced by commit
:::::: efa5623981b72f6b5f95933d1c36ed2518c2ee4e pinctrl: aspeed: Split out pinmux from general pinctrl
:::::: TO: Andrew Jeffery <andrew@aj.id.au>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>
---
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: 70930 bytes --]
^ permalink raw reply
* Re: [PATCH] gpio: don't WARN() on NULL descs if gpiolib is disabled
From: Bartosz Golaszewski @ 2019-07-09 14:20 UTC (permalink / raw)
To: Linus Walleij
Cc: open list:GPIO SUBSYSTEM, linux-kernel@vger.kernel.org,
Bartosz Golaszewski, Claus H . Stovgaard
In-Reply-To: <CACRpkdb5xKHZja0mkd-wZJ+YHZpGJaDrkA0dv60MNYKXFcPK4w@mail.gmail.com>
wt., 9 lip 2019 o 15:30 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> Hi Bartosz,
>
> On Mon, Jul 8, 2019 at 10:25 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > If gpiolib is disabled, we use the inline stubs from gpio/consumer.h
> > instead of regular definitions of GPIO API. The stubs for 'optional'
> > variants of gpiod_get routines return NULL in this case as if the
> > relevant GPIO wasn't found. This is correct so far.
> >
> > Calling other (non-gpio_get) stubs from this header triggers a warning
> > because the GPIO descriptor couldn't have been requested. The warning
> > however is unconditional (WARN_ON(1)) and is emitted even if the passed
> > descriptor pointer is NULL.
> >
> > We don't want to force the users of 'optional' gpio_get to check the
> > returned pointer before calling e.g. gpiod_set_value() so let's only
> > WARN on non-NULL descriptors.
> >
> > Reported-by: Claus H. Stovgaard <cst@phaseone.com>
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> I remember I had this discussion in the past, and I made a large
> refactoring to make it possible for drivers that need gpiod_*
> calls to simply do:
>
> select GPIOLIB
>
> in Kconfig.
>
> This should solve also this problem I think.
>
> However I do realize that there may be situations where people
> simply want to make GPIO support entirely optional without
> having to e.g. create custom stubs and encapsulate things
> inside if IS_ENABLED(CONFIG_GPIOLIB).
>
In this case the board doesn't provide any GPIO controller at all so
there's simply no need to select GPIOLIB - it would only add bloat.
> I was thinking something like this in the stubs:
>
> gpiod_get[_index]() {
> return POISON;
> }
>
> gpiod_get[_index]_optional() {
> return NULL;
> }
This is already being done.
>
> This way all gpiod_get() and optional calls are properly
> handled and the semantic that only _optional calls
> can return NULL is preserved. (Your patch would
> violate this.)
>
Maybe I'm missing something, but I don't quite see how my patch
violates this behavior. :(
> Then other stubs can do:
>
> gpiod_set_value() {
> WARN_ON(desc);
> }
>
> As in your patch, and all will be smooth provided the
> _optional calls have been used to obtain the desc.
>
> Yours,
> Linus Walleij
Bart
^ 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