On Thu, May 11, 2023 at 4:09 PM Jerome Neanne <jneanne@xxxxxxxxxxxx> wrote: > Add support for TPS65219 PMICs GPIO interface. > > 3 GPIO pins: > - GPIO0 only is IO but input mode reserved for MULTI_DEVICE_ENABLE usage > - GPIO1 and GPIO2 are Output only and referred as GPO1 and GPO2 in spec > > GPIO0 is statically configured as input or output prior to Linux boot. > it is used for MULTI_DEVICE_ENABLE function. > This setting is statically configured by NVM. > GPIO0 can't be used as a generic GPIO (specification Table 8-34). > It's either a GPO when MULTI_DEVICE_EN=0, > or a GPI when MULTI_DEVICE_EN=1. > > Datasheet describes specific usage for non standard GPIO. > Link: https://www.ti.com/lit/ds/symlink/tps65219.pdf > > Co-developed-by: Jonathan Cormier <jcormier@xxxxxxxxxxxxxxxx> > Signed-off-by: Jonathan Cormier <jcormier@xxxxxxxxxxxxxxxx> > Signed-off-by: Jerome Neanne <jneanne@xxxxxxxxxxxx> (...) This overall looks fine. > +static int tps65219_gpio_change_direction(struct gpio_chip *gc, unsigned int offset, > + unsigned int direction) > +{ > + struct tps65219_gpio *gpio = gpiochip_get_data(gc); > + > + /* Documentation is stating that GPIO0 direction must not be changed in Linux: > + * Table 8-34. MFP_1_CONFIG(3): MULTI_DEVICE_ENABLE, > + * Should only be changed in INITIALIZE state (prior to ON Request). > + * Set statically by NVM, changing direction in application can cause a hang. > + * Below can be used for test purpose only: > + */ > + > +#if 0 > + int ret = regmap_update_bits(gpio->tps->regmap, TPS65219_REG_MFP_1_CONFIG, > + TPS65219_GPIO0_DIR_MASK, direction); > + if (ret) > + return ret; > +#endif > + dev_err(gpio->tps->dev, > + "GPIO%d direction set by NVM, change to %u failed, not allowed by specification\n", > + offset, direction); > + return -EOPNOTSUPP; > +} Normally people would complain about #if 0 code. But this is a special case! I definitely want the code to be in there somehow. What about: if (IS_ENABLED(DEBUG))? If someone enables debug with an explicit -DDEBUG to the compiler this could be allowed. Yours, Linus Walleij Yours, Linus Walleij
