linux-kernel - Re: [PATCH 2/5] gpiolib / ACPI: convert to gpiod interfaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131008084508.GI3521@intel.com>
Date:	Tue, 8 Oct 2013 11:45:08 +0300
From:	Mika Westerberg <mika.westerberg@...ux.intel.com>
To:	Alexandre Courbot <gnurou@...il.com>
Cc:	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	rjw@...ysocki.net, Linus Walleij <linus.walleij@...aro.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	Mathias Nyman <mathias.nyman@...ux.intel.com>,
	Alexandre Courbot <acourbot@...dia.com>,
	Rob Landley <rob@...dley.net>, linux-acpi@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/5] gpiolib / ACPI: convert to gpiod interfaces

On Mon, Oct 07, 2013 at 09:54:13PM -0700, Alexandre Courbot wrote:
> > +struct gpio_desc *acpi_get_gpiod(char *path, int pin)
> >  {
> >         struct gpio_chip *chip;
> >         acpi_handle handle;
> > @@ -48,18 +49,15 @@ int acpi_get_gpio(char *path, int pin)
> >
> >         status = acpi_get_handle(NULL, path, &handle);
> >         if (ACPI_FAILURE(status))
> > -               return -ENODEV;
> > +               return ERR_PTR(-ENODEV);
> >
> >         chip = gpiochip_find(handle, acpi_gpiochip_find);
> >         if (!chip)
> > -               return -ENODEV;
> > +               return ERR_PTR(-ENODEV);
> >
> > -       if (!gpio_is_valid(chip->base + pin))
> > -               return -EINVAL;
> > -
> > -       return chip->base + pin;
> > +       return gpio_to_desc(chip->base + pin);
> 
> I think you want to avoid using gpio_to_desc(). It is just a
> convenience function provided to ease the transition for consumers
> that still need to rely on GPIO numbers for some reason. The same
> applies to desc_to_gpio(), although the usage you are making of it
> (implemented fallbacks for the integer space) is the one that is
> intended.
> 
> The last line could be changed to "return &chip->desc[pin];" after
> checking that 0 <= pin <= chip->ngpio.

I tried that but it doesn't compile anymore :-(

drivers/gpio/gpiolib-acpi.c: In function ‘acpi_get_gpiod’:
drivers/gpio/gpiolib-acpi.c:61:2: error: invalid use of undefined type ‘struct gpio_desc’
drivers/gpio/gpiolib-acpi.c:61:20: error: dereferencing pointer to incomplete type

Since struct gpio_desc is internal to gpiolib.c we can't dereference it
outside.

The DT version also uses gpio_to_desc().

> This minor issue apart, I really like the fact you are moving this
> under the gpiolib APIs (also appreciate the testing of the new API
> this patchset provides!).
> 
> I also wonder whether you could also avoid exporting acpi_get_gpiod*
> (which allow GPIO consumers to shortcut gpiolib) by implementing
> acpi_get_gpio* into the C file (maybe even using gpiod_get()). I see a
> "char *path" parameter though, so maybe that would not be possible at
> the moment.

Good point, I'll check if we can do that.

> Once the gpio_to_desc() issue mentioned above is fixed I think I can give my
> 
> Reviewed-by: Alexandre Courbot <acourbot@...dia.com>

Thanks!

> 
> on patches 2-5.
> 
> Thanks,
> Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/