linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Add support for TI TPS65219 PMIC GPIO interface.
@ 2023-02-24 11:38 Jerome Neanne
  2023-02-24 11:38 ` [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC Jerome Neanne
  2023-02-24 11:38 ` [PATCH 2/2] mfd: tps65219: Add gpio cell instance Jerome Neanne
  0 siblings, 2 replies; 9+ messages in thread
From: Jerome Neanne @ 2023-02-24 11:38 UTC (permalink / raw)
  To: linus.walleij, brgl, tony, lee
  Cc: linux-kernel, linux-gpio, linux-omap, Jerome Neanne

GPIO interface consist in 3 pins:
Two GPIOS are output only: GPO1, GPO2.

GPIO0 is used for multi device support:
- The input-functionality is only used in multi-PMIC configuration
- In single-PMIC, it can be used as an output

The configuration is static and flashed in NVM in factory.
Description tps65219.pdf chapter 7.3.13

Linux must not change MULTI_DEVICE_ENABLE bit at run time.

This was done for test purpose only to check input/output
correct behavior on EVM board (no access to different NVM config).

Tested on k3-am62x-lp-sk board. This board MULTI_DEVICE_ENABLE=0

Despite the register bits are out of order,
driver is remapping in natural order:
GPIO0 is gpiochip line 0
GPO1/2 are gpiochip line 1/2

 Initial version by Jon Cormier on TI Mainline.
 Ported upstream by Jerome Neanne


Link: https://www.ti.com/lit/ds/symlink/tps65219.pdf

Jerome Neanne (2):
  gpio: tps65219: add GPIO support for TPS65219 PMIC
  mfd: tps65219: Add gpio cell instance

 MAINTAINERS                  |   1 +
 drivers/gpio/Kconfig         |  13 +++
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-tps65219.c | 167 +++++++++++++++++++++++++++++++++++
 drivers/mfd/tps65219.c       |   7 +-
 5 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpio/gpio-tps65219.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC
  2023-02-24 11:38 [PATCH 0/2] Add support for TI TPS65219 PMIC GPIO interface Jerome Neanne
@ 2023-02-24 11:38 ` Jerome Neanne
  2023-02-24 12:16   ` andy.shevchenko
  2023-02-24 11:38 ` [PATCH 2/2] mfd: tps65219: Add gpio cell instance Jerome Neanne
  1 sibling, 1 reply; 9+ messages in thread
From: Jerome Neanne @ 2023-02-24 11:38 UTC (permalink / raw)
  To: linus.walleij, brgl, tony, lee
  Cc: linux-kernel, linux-gpio, linux-omap, Jerome Neanne,
	Jonathan Cormier

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.

Link: https://www.ti.com/lit/ds/symlink/tps65219.pdf
Signed-off-by: Jonathan Cormier <jcormier@criticallink.com>
Signed-off-by: Jerome Neanne <jneanne@baylibre.com>
---
 MAINTAINERS                  |   1 +
 drivers/gpio/Kconfig         |  13 +++
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-tps65219.c | 167 +++++++++++++++++++++++++++++++++++
 4 files changed, 182 insertions(+)
 create mode 100644 drivers/gpio/gpio-tps65219.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 468f39a37b33..76ed548cb926 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15330,6 +15330,7 @@ F:	arch/arm/configs/omap2plus_defconfig
 F:	arch/arm/mach-omap2/
 F:	arch/arm/plat-omap/
 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 ec7cfd4f52b1..a1ccc357e1ac 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1385,6 +1385,19 @@ 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
+	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.
+
 config GPIO_TPS6586X
 	bool "TPS6586X GPIO"
 	depends on MFD_TPS6586X
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 010587025fc8..c0f570678db0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -156,6 +156,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..af8d3f54fc2f
--- /dev/null
+++ b/drivers/gpio/gpio-tps65219.c
@@ -0,0 +1,167 @@
+// 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/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/gpio/driver.h>
+#include <linux/mfd/tps65219.h>
+
+#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;
+	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;
+	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);
+
+	if (offset != TPS65219_GPIO0_IDX)
+		regmap_update_bits(gpio->tps->regmap,
+				   TPS65219_REG_GENERAL_CONFIG,
+				   BIT(offset - 1), value ? BIT(offset - 1) : 0);
+	else
+		regmap_update_bits(gpio->tps->regmap,
+				   TPS65219_REG_GENERAL_CONFIG,
+				   BIT(TPS65219_GPIO0_OFFSET),
+				   value ? BIT(TPS65219_GPIO0_OFFSET) : 0);
+}
+
+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:
+	 * ret = regmap_update_bits(gpio->tps->regmap, TPS65219_REG_MFP_1_CONFIG,
+	 *			 TPS65219_GPIO0_DIR_MASK, direction);
+	 */
+	dev_err(gpio->tps->dev,
+		"GPIO%d direction set by NVM, change to %u failed, not allowed by specification\n",
+		 offset, direction);
+	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;
+	}
+	if (tps65219_gpio_get_direction(gc, offset) == TPS65219_GPIO_DIR_IN)
+		return 0;
+	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;
+	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;
+	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@criticallink.com>");
+MODULE_DESCRIPTION("TPS65219 GPIO driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] mfd: tps65219: Add gpio cell instance
  2023-02-24 11:38 [PATCH 0/2] Add support for TI TPS65219 PMIC GPIO interface Jerome Neanne
  2023-02-24 11:38 ` [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC Jerome Neanne
@ 2023-02-24 11:38 ` Jerome Neanne
  2023-02-24 12:17   ` andy.shevchenko
  1 sibling, 1 reply; 9+ messages in thread
From: Jerome Neanne @ 2023-02-24 11:38 UTC (permalink / raw)
  To: linus.walleij, brgl, tony, lee
  Cc: linux-kernel, linux-gpio, linux-omap, Jerome Neanne,
	Jonathan Cormier

tps65219 PMIC GPIOs are exposed in a standard way:
gpiodetect
gpiochip0 [tps65219-gpio] (3 lines)

Signed-off-by: Jonathan Cormier <jcormier@criticallink.com>
Signed-off-by: Jerome Neanne <jneanne@baylibre.com>
---
 drivers/mfd/tps65219.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/tps65219.c b/drivers/mfd/tps65219.c
index c134f3f6e202..5ce62e480ff9 100644
--- a/drivers/mfd/tps65219.c
+++ b/drivers/mfd/tps65219.c
@@ -115,7 +115,7 @@ static const struct mfd_cell tps65219_cells[] = {
 		.resources = tps65219_regulator_resources,
 		.num_resources = ARRAY_SIZE(tps65219_regulator_resources),
 	},
-	{ .name = "tps65219-gpios", },
+	{ .name = "tps65219-gpio", },
 };
 
 static const struct mfd_cell tps65219_pwrbutton_cell = {
@@ -267,6 +267,11 @@ static int tps65219_probe(struct i2c_client *client)
 		return ret;
 	}
 
+	if (ret) {
+		dev_err(tps->dev, "Failed to add gpio: %d\n", ret);
+		return ret;
+	}
+
 	pwr_button = of_property_read_bool(tps->dev->of_node, "ti,power-button");
 	if (pwr_button) {
 		ret = devm_mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC
  2023-02-24 11:38 ` [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC Jerome Neanne
@ 2023-02-24 12:16   ` andy.shevchenko
  2023-02-24 12:18     ` andy.shevchenko
  2023-02-27 19:20     ` Jon Cormier
  0 siblings, 2 replies; 9+ messages in thread
From: andy.shevchenko @ 2023-02-24 12:16 UTC (permalink / raw)
  To: Jerome Neanne
  Cc: linus.walleij, brgl, tony, lee, linux-kernel, linux-gpio,
	linux-omap, Jonathan Cormier

Fri, Feb 24, 2023 at 12:38:36PM +0100, Jerome Neanne kirjoitti:
> 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.

Linux

> 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.

Something wrong with the indentation.

> Link: https://www.ti.com/lit/ds/symlink/tps65219.pdf

Can it be Datasheet tag?

> Signed-off-by: Jonathan Cormier <jcormier@criticallink.com>
> Signed-off-by: Jerome Neanne <jneanne@baylibre.com>

Not sure how to interpet this along with the From line.

...

> +config GPIO_TPS65219
> +	tristate "TPS65219 GPIO"
> +	depends on 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.

Similar issues as with commit message. Also if chosen as M, how will it be
called?

> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/mfd/tps65219.h>
> +
> +#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 void tps65219_gpio_set(struct gpio_chip *gc, unsigned int offset,
> +			      int value)
> +{
> +	struct tps65219_gpio *gpio = gpiochip_get_data(gc);
> +
> +	if (offset != TPS65219_GPIO0_IDX)
> +		regmap_update_bits(gpio->tps->regmap,
> +				   TPS65219_REG_GENERAL_CONFIG,
> +				   BIT(offset - 1), value ? BIT(offset - 1) : 0);
> +	else
> +		regmap_update_bits(gpio->tps->regmap,
> +				   TPS65219_REG_GENERAL_CONFIG,
> +				   BIT(TPS65219_GPIO0_OFFSET),
> +				   value ? BIT(TPS65219_GPIO0_OFFSET) : 0);

With tenporary variables for value and mask this can be simplified.

	bit = (offset == _IDX) ? GPIO0_OFFSET : offset - 1;

	mask = BIT(bit);
	v = value ? mask : 0;

	regmap_update_bits(..., mask, v);

> +}

...

> +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:

You may rather put code into #if 0 ... #endif, so it will be easier to get the
test. Also, probably you would need to take care about error handling properly.

> +	 * ret = regmap_update_bits(gpio->tps->regmap, TPS65219_REG_MFP_1_CONFIG,
> +	 *			 TPS65219_GPIO0_DIR_MASK, direction);
> +	 */
> +	dev_err(gpio->tps->dev,
> +		"GPIO%d direction set by NVM, change to %u failed, not allowed by specification\n",
> +		 offset, direction);
> +	return -EOPNOTSUPP;
> +}

...

> +static struct platform_driver tps65219_gpio_driver = {
> +	.driver = {.name = "tps65219-gpio",},

Can you write it as

	.driver = {
		...
	},
?

> +	.probe = tps65219_gpio_probe,
> +};

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] mfd: tps65219: Add gpio cell instance
  2023-02-24 11:38 ` [PATCH 2/2] mfd: tps65219: Add gpio cell instance Jerome Neanne
@ 2023-02-24 12:17   ` andy.shevchenko
  0 siblings, 0 replies; 9+ messages in thread
From: andy.shevchenko @ 2023-02-24 12:17 UTC (permalink / raw)
  To: Jerome Neanne
  Cc: linus.walleij, brgl, tony, lee, linux-kernel, linux-gpio,
	linux-omap, Jonathan Cormier

Fri, Feb 24, 2023 at 12:38:37PM +0100, Jerome Neanne kirjoitti:
> tps65219 PMIC GPIOs are exposed in a standard way:
> gpiodetect
> gpiochip0 [tps65219-gpio] (3 lines)
> 
> Signed-off-by: Jonathan Cormier <jcormier@criticallink.com>
> Signed-off-by: Jerome Neanne <jneanne@baylibre.com>

Same issues with the commit message as per previous patch.

...

> @@ -267,6 +267,11 @@ static int tps65219_probe(struct i2c_client *client)
>  		return ret;
>  	}
>  
> +	if (ret) {
> +		dev_err(tps->dev, "Failed to add gpio: %d\n", ret);
> +		return ret;
> +	}

Isn't it a dead code?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC
  2023-02-24 12:16   ` andy.shevchenko
@ 2023-02-24 12:18     ` andy.shevchenko
  2023-02-27 19:20     ` Jon Cormier
  1 sibling, 0 replies; 9+ messages in thread
From: andy.shevchenko @ 2023-02-24 12:18 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: Jerome Neanne, linus.walleij, brgl, tony, lee, linux-kernel,
	linux-gpio, linux-omap, Jonathan Cormier

Fri, Feb 24, 2023 at 02:16:06PM +0200, andy.shevchenko@gmail.com kirjoitti:
> Fri, Feb 24, 2023 at 12:38:36PM +0100, Jerome Neanne kirjoitti:

...

> > +#include <linux/of.h>

Forgot to add that there is no user of this header.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC
  2023-02-24 12:16   ` andy.shevchenko
  2023-02-24 12:18     ` andy.shevchenko
@ 2023-02-27 19:20     ` Jon Cormier
  2023-02-27 19:51       ` Andy Shevchenko
  1 sibling, 1 reply; 9+ messages in thread
From: Jon Cormier @ 2023-02-27 19:20 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: Jerome Neanne, linus.walleij, brgl, tony, lee, linux-kernel,
	linux-gpio, linux-omap

On Fri, Feb 24, 2023 at 7:16 AM <andy.shevchenko@gmail.com> wrote:
>
> Fri, Feb 24, 2023 at 12:38:36PM +0100, Jerome Neanne kirjoitti:
> > 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.
>
> Linux
>
> > 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.
>
> Something wrong with the indentation.
>
> > Link: https://www.ti.com/lit/ds/symlink/tps65219.pdf
>
> Can it be Datasheet tag?
>
> > Signed-off-by: Jonathan Cormier <jcormier@criticallink.com>
> > Signed-off-by: Jerome Neanne <jneanne@baylibre.com>
>
> Not sure how to interpet this along with the From line.
Are two sign-offs not allowed/expected?  I wrote the initial
implementation of this driver and Jerome updated it and is handling
submitting it since he did the rest of the TPS65219 drivers.
>
> ...
>
> > +config GPIO_TPS65219
> > +     tristate "TPS65219 GPIO"
> > +     depends on 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.
>
> Similar issues as with commit message. Also if chosen as M, how will it be
> called?
Based on INPUT_TPS65219_PWRBUTTON, this should have:

To compile this driver as a module, choose M here. The module will
be called tps65219-gpio.


--
Jonathan Cormier
Software Engineer

Voice:  315.425.4045 x222



http://www.CriticalLink.com
6712 Brooklawn Parkway, Syracuse, NY 13211

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC
  2023-02-27 19:20     ` Jon Cormier
@ 2023-02-27 19:51       ` Andy Shevchenko
  2023-02-28 11:18         ` jerome Neanne
  0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2023-02-27 19:51 UTC (permalink / raw)
  To: Jon Cormier
  Cc: Jerome Neanne, linus.walleij, brgl, tony, lee, linux-kernel,
	linux-gpio, linux-omap

On Mon, Feb 27, 2023 at 9:20 PM Jon Cormier <jcormier@criticallink.com> wrote:
> On Fri, Feb 24, 2023 at 7:16 AM <andy.shevchenko@gmail.com> wrote:
> > Fri, Feb 24, 2023 at 12:38:36PM +0100, Jerome Neanne kirjoitti:

...

> > > Signed-off-by: Jonathan Cormier <jcormier@criticallink.com>
> > > Signed-off-by: Jerome Neanne <jneanne@baylibre.com>
> >
> > Not sure how to interpet this along with the From line.
> Are two sign-offs not allowed/expected?  I wrote the initial
> implementation of this driver and Jerome updated it and is handling
> submitting it since he did the rest of the TPS65219 drivers.

1. Submitter's SoB must be the last SoB in the chain.
2. Developers also need to be marked with Co-developed-by.


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC
  2023-02-27 19:51       ` Andy Shevchenko
@ 2023-02-28 11:18         ` jerome Neanne
  0 siblings, 0 replies; 9+ messages in thread
From: jerome Neanne @ 2023-02-28 11:18 UTC (permalink / raw)
  To: Andy Shevchenko, Jon Cormier
  Cc: linus.walleij, brgl, tony, lee, linux-kernel, linux-gpio,
	linux-omap



On 27/02/2023 20:51, Andy Shevchenko wrote:
>>>> Signed-off-by: Jonathan Cormier<jcormier@criticallink.com>
>>>> Signed-off-by: Jerome Neanne<jneanne@baylibre.com>
>>> Not sure how to interpet this along with the From line.
>> Are two sign-offs not allowed/expected?  I wrote the initial
>> implementation of this driver and Jerome updated it and is handling
>> submitting it since he did the rest of the TPS65219 drivers.
> 1. Submitter's SoB must be the last SoB in the chain.
> 2. Developers also need to be marked with Co-developed-by.
Got it! My mistake. I'll fix following your instructions.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-02-28 11:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-24 11:38 [PATCH 0/2] Add support for TI TPS65219 PMIC GPIO interface Jerome Neanne
2023-02-24 11:38 ` [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC Jerome Neanne
2023-02-24 12:16   ` andy.shevchenko
2023-02-24 12:18     ` andy.shevchenko
2023-02-27 19:20     ` Jon Cormier
2023-02-27 19:51       ` Andy Shevchenko
2023-02-28 11:18         ` jerome Neanne
2023-02-24 11:38 ` [PATCH 2/2] mfd: tps65219: Add gpio cell instance Jerome Neanne
2023-02-24 12:17   ` andy.shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).