On 06/20/2013 11:43 AM, Joe Perches wrote:
On Thu, 2013-06-20 at 11:27 -0700, David Daney wrote:
On 06/20/2013 11:18 AM, Joe Perches wrote:
On Thu, 2013-06-20 at 11:10 -0700, David Daney wrote:
Sorry for not responding earlier, but my e-mail system seems to have
malfunctioned with respect to this message...
[]
On 06/17/2013 01:51 AM, Linus Walleij wrote:
+static int octeon_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ struct octeon_gpio *gpio = container_of(chip, struct octeon_gpio, chip);
+ u64 read_bits = cvmx_read_csr(gpio->register_base + RX_DAT);
+
+ return ((1ull << offset) & read_bits) != 0;
A common idiom we use for this is:
return !!(read_bits & (1ull << offset));
I hate that idiom, but if its use is a condition of accepting the patch,
I will change it.
Or use an even more common idiom and change the
function to return bool and let the compiler do it.
... but it is part of the gpiochip system interface, so it would have to
be done kernel wide.
Not really. It's a local static function.
... which we generate a pointer to, and then assign that pointer to a
variable with a type defined in the gpiochip system interface. So If we
do what you suggest, the result is:
CC drivers/gpio/gpio-octeon.o
drivers/gpio/gpio-octeon.c: In function 'octeon_gpio_probe':
drivers/gpio/gpio-octeon.c:113:12: warning: assignment from incompatible
pointer type [enabled by default]
Really I don't like the idea of GPIO lines having Boolean truth values
associated with them. Some represent things that are active-high and
others active-low. Converting the pin voltage being above or below a
given threshold to something other than zero or one would in my opinion
be confusing.
No worries, just offering options. Your code, your choice.
