1. Patchew
  2. linux
  3. [PATCH v2 0/3] Add TI TPS65215 PMIC GPIO Support
  4. View patch

[PATCH v2 3/3] gpio tps65215: Add support for varying gpio/offset values

Shree Ramamoorthy posted 3 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH v2 3/3] gpio tps65215: Add support for varying gpio/offset values
Posted by Shree Ramamoorthy 1 year, 1 month ago 
Add device-specific structs to select the different PMIC .npgio and .offset
values. With the chip_data struct values selected based on the match data,
having a separate GPIO0_OFFSET macro is no longer needed.

Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com>
---
 drivers/gpio/gpio-tps65219.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-tps65219.c b/drivers/gpio/gpio-tps65219.c
index 70a4410c473a..14286dd5fdfb 100644
--- a/drivers/gpio/gpio-tps65219.c
+++ b/drivers/gpio/gpio-tps65219.c
@@ -13,7 +13,6 @@
 #include <linux/regmap.h>
 
 #define TPS65219_GPIO0_DIR_MASK		BIT(3)
-#define TPS65219_GPIO0_OFFSET		2
 #define TPS6521X_GPIO0_IDX			0
 
 struct tps65219_gpio {
@@ -21,6 +20,11 @@ struct tps65219_gpio {
 	struct tps65219 *tps;
 };
 
+struct tps65219_chip_data {
+	int ngpio;
+	int offset;
+};
+
 static int tps65219_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
 	struct tps65219_gpio *gpio = gpiochip_get_data(gc);
@@ -71,7 +75,7 @@ static void tps65219_gpio_set(struct gpio_chip *gc, unsigned int offset, int val
 	struct device *dev = gpio->tps->dev;
 	int v, mask, bit;
 
-	bit = (offset == TPS6521X_GPIO0_IDX) ? TPS65219_GPIO0_OFFSET : offset - 1;
+	bit = (offset == TPS6521X_GPIO0_IDX) ? (gpio->gpio_chip.ngpio - 1) : offset - 1;
 
 	mask = BIT(bit);
 	v = value ? mask : 0;
@@ -148,14 +152,29 @@ static const struct gpio_chip tps65219_template_chip = {
 	.get			= tps65219_gpio_get,
 	.set			= tps65219_gpio_set,
 	.base			= -1,
-	.ngpio			= 3,
 	.can_sleep		= true,
 };
 
+static const struct tps65219_chip_data chip_info_table[] = {
+	[TPS65215] = {
+		.ngpio = 2,
+		.offset = 1,
+	},
+	[TPS65219] = {
+		.ngpio = 3,
+		.offset = 2,
+	},
+};
+
 static int tps65219_gpio_probe(struct platform_device *pdev)
 {
 	struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
 	struct tps65219_gpio *gpio;
+	const struct tps65219_chip_data *pmic;
+
+	enum pmic_id chip = platform_get_device_id(pdev)->driver_data;
+
+	pmic = &chip_info_table[chip];
 
 	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
 	if (!gpio)
@@ -164,6 +183,8 @@ static int tps65219_gpio_probe(struct platform_device *pdev)
 	gpio->tps = tps;
 	gpio->gpio_chip = tps65219_template_chip;
 	gpio->gpio_chip.label = dev_name(&pdev->dev);
+	gpio->gpio_chip.ngpio =  pmic->ngpio;
+	gpio->gpio_chip.offset = pmic->offset;
 	gpio->gpio_chip.parent = tps->dev;
 
 	return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio_chip, gpio);
-- 
2.34.1
Re: [PATCH v2 3/3] gpio tps65215: Add support for varying gpio/offset values
Posted by Roger Quadros 1 year, 1 month ago 

On 04/01/2025 00:54, Shree Ramamoorthy wrote:
> Add device-specific structs to select the different PMIC .npgio and .offset
> values. With the chip_data struct values selected based on the match data,
> having a separate GPIO0_OFFSET macro is no longer needed.
> 
> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com>
> ---
>  drivers/gpio/gpio-tps65219.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-tps65219.c b/drivers/gpio/gpio-tps65219.c
> index 70a4410c473a..14286dd5fdfb 100644
> --- a/drivers/gpio/gpio-tps65219.c
> +++ b/drivers/gpio/gpio-tps65219.c
> @@ -13,7 +13,6 @@
>  #include <linux/regmap.h>
>  
>  #define TPS65219_GPIO0_DIR_MASK		BIT(3)
> -#define TPS65219_GPIO0_OFFSET		2
>  #define TPS6521X_GPIO0_IDX			0
>  
>  struct tps65219_gpio {
> @@ -21,6 +20,11 @@ struct tps65219_gpio {
>  	struct tps65219 *tps;
>  };
>  
> +struct tps65219_chip_data {
> +	int ngpio;
> +	int offset;
> +};
> +
>  static int tps65219_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>  {
>  	struct tps65219_gpio *gpio = gpiochip_get_data(gc);
> @@ -71,7 +75,7 @@ static void tps65219_gpio_set(struct gpio_chip *gc, unsigned int offset, int val
>  	struct device *dev = gpio->tps->dev;
>  	int v, mask, bit;
>  
> -	bit = (offset == TPS6521X_GPIO0_IDX) ? TPS65219_GPIO0_OFFSET : offset - 1;
> +	bit = (offset == TPS6521X_GPIO0_IDX) ? (gpio->gpio_chip.ngpio - 1) : offset - 1;

shouldn't this be
	bit = (offset == TPS6521X_GPIO0_IDX) ? (gpio->gpio_chip.offset - 1) : offset - 1;

>  
>  	mask = BIT(bit);
>  	v = value ? mask : 0;
> @@ -148,14 +152,29 @@ static const struct gpio_chip tps65219_template_chip = {
>  	.get			= tps65219_gpio_get,
>  	.set			= tps65219_gpio_set,
>  	.base			= -1,
> -	.ngpio			= 3,
>  	.can_sleep		= true,
>  };
>  
> +static const struct tps65219_chip_data chip_info_table[] = {
> +	[TPS65215] = {
> +		.ngpio = 2,
> +		.offset = 1,
> +	},
> +	[TPS65219] = {
> +		.ngpio = 3,
> +		.offset = 2,
> +	},
> +};
> +
>  static int tps65219_gpio_probe(struct platform_device *pdev)
>  {
>  	struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
>  	struct tps65219_gpio *gpio;
> +	const struct tps65219_chip_data *pmic;
> +
> +	enum pmic_id chip = platform_get_device_id(pdev)->driver_data;
> +

unnecessary newline?

> +	pmic = &chip_info_table[chip];
>

here too?
  
>  	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
>  	if (!gpio)
> @@ -164,6 +183,8 @@ static int tps65219_gpio_probe(struct platform_device *pdev)
>  	gpio->tps = tps;
>  	gpio->gpio_chip = tps65219_template_chip;
>  	gpio->gpio_chip.label = dev_name(&pdev->dev);
> +	gpio->gpio_chip.ngpio =  pmic->ngpio;
> +	gpio->gpio_chip.offset = pmic->offset;
>  	gpio->gpio_chip.parent = tps->dev;
>  
>  	return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio_chip, gpio);

-- 
cheers,
-roger
Re: [PATCH v2 3/3] gpio tps65215: Add support for varying gpio/offset values
Posted by Shree Ramamoorthy 1 year, 1 month ago 
Hi,

On 1/4/2025 12:27 PM, Roger Quadros wrote:
>
> On 04/01/2025 00:54, Shree Ramamoorthy wrote:
>> Add device-specific structs to select the different PMIC .npgio and .offset
>> values. With the chip_data struct values selected based on the match data,
>> having a separate GPIO0_OFFSET macro is no longer needed.
>>
>> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com>
>> ---
>>  drivers/gpio/gpio-tps65219.c | 27 ++++++++++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-tps65219.c b/drivers/gpio/gpio-tps65219.c
>> index 70a4410c473a..14286dd5fdfb 100644
>> --- a/drivers/gpio/gpio-tps65219.c
>> +++ b/drivers/gpio/gpio-tps65219.c
>> @@ -13,7 +13,6 @@
>>  #include <linux/regmap.h>
>>  
>>  #define TPS65219_GPIO0_DIR_MASK		BIT(3)
>> -#define TPS65219_GPIO0_OFFSET		2
>>  #define TPS6521X_GPIO0_IDX			0
>>  
>>  struct tps65219_gpio {
>> @@ -21,6 +20,11 @@ struct tps65219_gpio {
>>  	struct tps65219 *tps;
>>  };
>>  
>> +struct tps65219_chip_data {
>> +	int ngpio;
>> +	int offset;
>> +};
>> +
>>  static int tps65219_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>>  {
>>  	struct tps65219_gpio *gpio = gpiochip_get_data(gc);
>> @@ -71,7 +75,7 @@ static void tps65219_gpio_set(struct gpio_chip *gc, unsigned int offset, int val
>>  	struct device *dev = gpio->tps->dev;
>>  	int v, mask, bit;
>>  
>> -	bit = (offset == TPS6521X_GPIO0_IDX) ? TPS65219_GPIO0_OFFSET : offset - 1;
>> +	bit = (offset == TPS6521X_GPIO0_IDX) ? (gpio->gpio_chip.ngpio - 1) : offset - 1;
> shouldn't this be
> 	bit = (offset == TPS6521X_GPIO0_IDX) ? (gpio->gpio_chip.offset - 1) : offset - 1;

Thank you for catching this! I have applied to the change for the next version sent out.

>>  
>>  	mask = BIT(bit);
>>  	v = value ? mask : 0;
>> @@ -148,14 +152,29 @@ static const struct gpio_chip tps65219_template_chip = {
>>  	.get			= tps65219_gpio_get,
>>  	.set			= tps65219_gpio_set,
>>  	.base			= -1,
>> -	.ngpio			= 3,
>>  	.can_sleep		= true,
>>  };
>>  
>> +static const struct tps65219_chip_data chip_info_table[] = {
>> +	[TPS65215] = {
>> +		.ngpio = 2,
>> +		.offset = 1,
>> +	},
>> +	[TPS65219] = {
>> +		.ngpio = 3,
>> +		.offset = 2,
>> +	},
>> +};
>> +
>>  static int tps65219_gpio_probe(struct platform_device *pdev)
>>  {
>>  	struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
>>  	struct tps65219_gpio *gpio;
>> +	const struct tps65219_chip_data *pmic;
>> +
>> +	enum pmic_id chip = platform_get_device_id(pdev)->driver_data;
>> +
> unnecessary newline?
>
>> +	pmic = &chip_info_table[chip];
>>
> here too?

I have reorganized the above section to have the variable declarations separate
from the chunk of code assigning values. Thanks for reviewing!

>>  	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
>>  	if (!gpio)
>> @@ -164,6 +183,8 @@ static int tps65219_gpio_probe(struct platform_device *pdev)
>>  	gpio->tps = tps;
>>  	gpio->gpio_chip = tps65219_template_chip;
>>  	gpio->gpio_chip.label = dev_name(&pdev->dev);
>> +	gpio->gpio_chip.ngpio =  pmic->ngpio;
>> +	gpio->gpio_chip.offset = pmic->offset;
>>  	gpio->gpio_chip.parent = tps->dev;
>>  
>>  	return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio_chip, gpio);

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.