Hello Linus, On Fri, 11 Jan 2019 11:05:03 +0100, Linus Walleij wrote: > > The flag is simply propagated all the way to the core GPIO subsystem, > > where it is used to call the gpio_chip ->set_config callback with the > > appropriate existing PIN_CONFIG_BIAS_* values. > > > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxx> > > I'm overall happy with this approach. Thanks for this review and feedback, very useful. > > > +/* Bit 4 express pull up */ > > +#define GPIO_PULL_UP 16 > > + > > +/* Bit 5 express pull down */ > > +#define GPIO_PULL_DOWN 32 > > Please use > #define GPIO_PULL_UP BIT(5) > #define GPIO_PULL_DOWN BIT(6) Here, I did exactly like below: keep the existing approach used in the file. Today it has: /* Bit 0 express polarity */ #define GPIO_ACTIVE_HIGH 0 #define GPIO_ACTIVE_LOW 1 /* Bit 1 express single-endedness */ #define GPIO_PUSH_PULL 0 #define GPIO_SINGLE_ENDED 2 /* Bit 2 express Open drain or open source */ #define GPIO_LINE_OPEN_SOURCE 0 #define GPIO_LINE_OPEN_DRAIN 4 [...] /* Bit 3 express GPIO suspend/resume and reset persistence */ #define GPIO_PERSISTENT 0 #define GPIO_TRANSITORY 8 So I kept the same logic and used 16 and 32 to define the GPIO_PULL_UP/GPIO_PULL_DOWN values instead of BIT(x). BIT(x) would have been more logical. However, we are in the dt-bindings includes here, and apparently, there is no definition for the BIT() macro in those headers. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
