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> > --- > MAINTAINERS | 1 + > drivers/gpio/Kconfig | 17 +++++ > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-tps65219.c | 173 +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 192 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index c0cde28c62c6..d912b7465e84 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15398,6 +15398,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git > F: arch/arm/configs/omap2plus_defconfig > F: arch/arm/mach-omap2/ > F: drivers/bus/ti-sysc.c > +F: drivers/gpio/gpio-tps65219.c > F: drivers/i2c/busses/i2c-omap.c > F: drivers/irqchip/irq-omap-intc.c > F: drivers/mfd/*omap*.c > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 5521f060d58e..f4e37881d01a 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -1440,6 +1440,23 @@ config GPIO_TPS65218 > Select this option to enable GPIO driver for the TPS65218 > chip family. > > +config GPIO_TPS65219 > + tristate "TPS65219 GPIO" > + depends on MFD_TPS65219 > + default MFD_TPS65219 > + help > + Select this option to enable GPIO driver for the TPS65219 > + chip family. > + 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. > + It's either a GPO when MULTI_DEVICE_EN=0, > + or a GPI when MULTI_DEVICE_EN=1. > + > + This driver can also be built as a module. If so, the > + module will be called gpio_tps65219. > + > config GPIO_TPS6586X > bool "TPS6586X GPIO" > depends on MFD_TPS6586X > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 20036af3acb1..7843b16f5d59 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -160,6 +160,7 @@ obj-$(CONFIG_GPIO_TN48M_CPLD) += gpio-tn48m.o > obj-$(CONFIG_GPIO_TPIC2810) += gpio-tpic2810.o > obj-$(CONFIG_GPIO_TPS65086) += gpio-tps65086.o > obj-$(CONFIG_GPIO_TPS65218) += gpio-tps65218.o > +obj-$(CONFIG_GPIO_TPS65219) += gpio-tps65219.o > obj-$(CONFIG_GPIO_TPS6586X) += gpio-tps6586x.o > obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o > obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o > diff --git a/drivers/gpio/gpio-tps65219.c b/drivers/gpio/gpio-tps65219.c > new file mode 100644 > index 000000000000..42bbd142e4b6 > --- /dev/null > +++ b/drivers/gpio/gpio-tps65219.c > @@ -0,0 +1,173 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * GPIO driver for TI TPS65219 PMICs > + * > + * Copyright (C) 2022 Texas Instruments Incorporated - http://www.ti.com/ > + */ > + > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#include <linux/gpio/driver.h> > +#include <linux/mfd/tps65219.h> > + Please keep all includes together and ordered alphabetically. I see you probably wanted to split them thematically but we don't really do this. > +#define TPS65219_GPIO0_DIR_MASK BIT(3) > +#define TPS65219_GPIO0_OFFSET 2 > +#define TPS65219_GPIO0_IDX 0 > +#define TPS65219_GPIO_DIR_IN 1 > +#define TPS65219_GPIO_DIR_OUT 0 > + > +struct tps65219_gpio { > + struct gpio_chip gpio_chip; > + struct tps65219 *tps; > +}; > + > +static int tps65219_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) > +{ > + struct tps65219_gpio *gpio = gpiochip_get_data(gc); > + int ret, val; > + > + if (offset != TPS65219_GPIO0_IDX) > + return GPIO_LINE_DIRECTION_OUT; > + > + ret = regmap_read(gpio->tps->regmap, TPS65219_REG_MFP_1_CONFIG, &val); > + if (ret) > + return ret; > + > + return !!(val & TPS65219_GPIO0_DIR_MASK); > +} > + > +static int tps65219_gpio_get(struct gpio_chip *gc, unsigned int offset) > +{ > + struct tps65219_gpio *gpio = gpiochip_get_data(gc); > + int ret, val; > + > + if (offset != TPS65219_GPIO0_IDX) { > + dev_err(gpio->tps->dev, > + "GPIO%d is output only, cannot get\n", > + offset); > + return -EOPNOTSUPP; > + } > + > + ret = regmap_read(gpio->tps->regmap, TPS65219_REG_MFP_CTRL, &val); > + if (ret) > + return ret; Add newline here. > + dev_warn(gpio->tps->dev, > + "GPIO%d = %d, used for MULTI_DEVICE_ENABLE, not as standard GPIO\n", > + offset, !!(val & BIT(TPS65219_MFP_GPIO_STATUS_MASK))); > + > + /* depends on NVM config return error if dir output else the GPIO0 status bit */ > + if (tps65219_gpio_get_direction(gc, offset) == TPS65219_GPIO_DIR_OUT) > + return -EOPNOTSUPP; Add newline here. > + return !!(val & BIT(TPS65219_MFP_GPIO_STATUS_MASK)); > +} > + > +static void tps65219_gpio_set(struct gpio_chip *gc, unsigned int offset, > + int value) > +{ > + struct tps65219_gpio *gpio = gpiochip_get_data(gc); > + int v, mask, bit; > + > + bit = (offset == TPS65219_GPIO0_IDX) ? TPS65219_GPIO0_OFFSET : offset - 1; > + > + mask = BIT(bit); > + v = value ? mask : 0; > + > + regmap_update_bits(gpio->tps->regmap, > + TPS65219_REG_GENERAL_CONFIG, > + mask, v); This can fail - I suggest emitting an error message as regmap won't do it. > +} > + > +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 Agree with Linus here on enabling it for DEBUG. And a newline here. > + dev_err(gpio->tps->dev, > + "GPIO%d direction set by NVM, change to %u failed, not allowed by specification\n", > + offset, direction); And before return. > + return -EOPNOTSUPP; > +} > + > +static int tps65219_gpio_direction_input(struct gpio_chip *gc, unsigned int offset) > +{ > + struct tps65219_gpio *gpio = gpiochip_get_data(gc); > + > + if (offset != TPS65219_GPIO0_IDX) { > + dev_err(gpio->tps->dev, > + "GPIO%d is output only, cannot change to input\n", > + offset); > + return -EOPNOTSUPP; > + } Newline here. > + if (tps65219_gpio_get_direction(gc, offset) == TPS65219_GPIO_DIR_IN) > + return 0; and here. > + return tps65219_gpio_change_direction(gc, offset, TPS65219_GPIO_DIR_IN); > +} > + > +static int tps65219_gpio_direction_output(struct gpio_chip *gc, unsigned int offset, > + int value) > +{ > + tps65219_gpio_set(gc, offset, value); > + if (offset != TPS65219_GPIO0_IDX) > + return 0; > + > + if (tps65219_gpio_get_direction(gc, offset) == TPS65219_GPIO_DIR_OUT) > + return 0; and here. > + return tps65219_gpio_change_direction(gc, offset, TPS65219_GPIO_DIR_OUT); > +} > + > +static const struct gpio_chip tps65219_gpio_chip = { > + .label = "tps65219-gpio", > + .owner = THIS_MODULE, > + .get_direction = tps65219_gpio_get_direction, > + .direction_input = tps65219_gpio_direction_input, > + .direction_output = tps65219_gpio_direction_output, > + .get = tps65219_gpio_get, > + .set = tps65219_gpio_set, > + .base = -1, > + .ngpio = 3, > + .can_sleep = true, > +}; > + > +static int tps65219_gpio_probe(struct platform_device *pdev) > +{ > + struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent); > + struct tps65219_gpio *gpio; > + > + gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL); > + if (!gpio) > + return -ENOMEM; > + > + gpio->tps = tps; > + gpio->gpio_chip = tps65219_gpio_chip; Aren't you getting any warnings here about dropping the 'const' from the global structure? > + gpio->gpio_chip.parent = tps->dev; > + > + return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio_chip, gpio); > +} > + > +static struct platform_driver tps65219_gpio_driver = { > + .driver = { > + .name = "tps65219-gpio", > + }, > + .probe = tps65219_gpio_probe, > +}; > +module_platform_driver(tps65219_gpio_driver); > + > +MODULE_ALIAS("platform:tps65219-gpio"); > +MODULE_AUTHOR("Jonathan Cormier <jcormier@xxxxxxxxxxxxxxxx>"); > +MODULE_DESCRIPTION("TPS65219 GPIO driver"); > +MODULE_LICENSE("GPL"); > Otherwise looks good, just a couple nits. Bart > -- > 2.34.1 >
