Re: [PATCH] gpio MIPS/OCTEON: Add a driver for OCTEON's on-chip GPIOpins.

From: David Daney
Date: Thu Jun 20 2013 - 14:27:19 EST


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.

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.

David Daney

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/