lkml.org 
[lkml]   [2015]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] GPIO: Add GPIO support for the ACCES 104-IDIO-16
From
Date
On 10/05/2015 04:29 AM, Linus Walleij wrote:
>> +struct a_104_idio_16_gpio {
>> + struct gpio_chip chip;
>> + spinlock_t lock;
>> + unsigned base;
>
> Isn't this void __iomem *base?

The 'base' member is used to hold the I/O port base address passed to the
inb/outb functions for port-mapped I/O operations. Since the addresses are
not dereferenced, I don't believe an __iomem pointer would be correct.

>> +static const unsigned A_104_IDIO_16_EXTENT = 8;
>
> Looks like it could be a #define A_104_IDIO_16_EXTENT 8

I used a const variable for the benefit of type-safety; I assumed the
compiler would optimize it. What is the advantage of a #define constant?

>> +static void __exit a_104_idio_16_exit(void)
>> +{
>> + pr_info("104-idio-16: Exiting 104-idio-16 module\n");
>> +
>> + gpiochip_remove(&gp.chip);
>
> Where is that &gp.chip? Not in this file. Nor should you use any globals.
>

I agree that using a global data structure isn't good practice, but I'm not
sure how else to expose the gpio_chip structure in the respective module
_init and _exit functions since they have void parameter lists. Would it be
more appropriate to use the platform device API in this situation to avoid
the global data structure?

>> +static int a_104_idio_16_gpio_get(struct gpio_chip *chip, unsigned offset)
>> +{
>> + struct a_104_idio_16_gpio *a104i16gp = to_a104i16gp(chip);
>> + const unsigned BIT_MASK = 1U << (offset-16);
>> +
>> + if (offset < 16)
>> + return 0;
>
> Always return 0, why? Is that really correct?

GPIO 0-15 are output-only. The kerneldoc for 'struct gpio_chip' states that
for output signals the get function should return the value actually sensed,
or zero. Since I cannot sense the output signals, I return zero in these cases.
Is this behavior correct?

- William



\
 
 \ /
  Last update: 2015-10-06 02:01    [W:0.099 / U:0.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site