Re: [PATCH v2 1/3] gpio: tps65215: Add TPS65215 to platform_device_id table

[Andrew Davis <afd@ti.com>]

On 1/4/25 12:21 PM, Roger Quadros wrote:
> 
> 
> On 04/01/2025 00:54, Shree Ramamoorthy wrote:
>> Add platform_device_id struct and use the platform_get_device_id() output
>> to match which PMIC device is in use. With new name options, the gpio_chip
>> .label field is now assigned to the platform_device name match.
>>
>> Remove MODULE_ALIAS since it is now generated by MODULE_DEVICE_TABLE.
>>
>> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com>
>> ---
>>   drivers/gpio/gpio-tps65219.c | 17 ++++++++++++-----
>>   1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-tps65219.c b/drivers/gpio/gpio-tps65219.c
>> index 526640c39a11..7e03be0c7c92 100644
>> --- a/drivers/gpio/gpio-tps65219.c
>> +++ b/drivers/gpio/gpio-tps65219.c
>> @@ -1,8 +1,8 @@
>>   // SPDX-License-Identifier: GPL-2.0
>>   /*
>> - * GPIO driver for TI TPS65219 PMICs
>> + * GPIO driver for TI TPS65215/TPS65219 PMICs
>>    *
>> - * Copyright (C) 2022 Texas Instruments Incorporated - http://www.ti.com/
>> + * Copyright (C) 2024 Texas Instruments Incorporated - http://www.ti.com/
>>    */
>>   
>>   #include <linux/bits.h>
>> @@ -141,7 +141,6 @@ static int tps65219_gpio_direction_output(struct gpio_chip *gc, unsigned int off
>>   }
>>   
>>   static const struct gpio_chip tps65219_template_chip = {
>> -	.label			= "tps65219-gpio",
>>   	.owner			= THIS_MODULE,
>>   	.get_direction		= tps65219_gpio_get_direction,
>>   	.direction_input	= tps65219_gpio_direction_input,
>> @@ -164,20 +163,28 @@ 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.parent = tps->dev;
>>   
>>   	return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio_chip, gpio);
>>   }
>>   
>> +static const struct platform_device_id tps6521x_gpio_id_table[] = {
>> +	{ "tps65215-gpio", TPS65215 },
>> +	{ "tps65219-gpio", TPS65219 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(platform, tps6521x_gpio_id_table);
>> +
>>   static struct platform_driver tps65219_gpio_driver = {
>>   	.driver = {
>>   		.name = "tps65219-gpio",
>>   	},
>>   	.probe = tps65219_gpio_probe,
>> +	.id_table = tps6521x_gpio_id_table,
>>   };
>>   module_platform_driver(tps65219_gpio_driver);
>>   
>> -MODULE_ALIAS("platform:tps65219-gpio");
> 
> Why do you drop the MODULE_ALIAS?
> You can add multiple MODULE_ALIASES if needed.
> 

The new MODULE_DEVICE_TABLE() above causes all the needed
module aliases to be made for us automatically.

>>   MODULE_AUTHOR("Jonathan Cormier <jcormier@criticallink.com>");
>> -MODULE_DESCRIPTION("TPS65219 GPIO driver");
>> +MODULE_DESCRIPTION("TPS65215/TPS65219 GPIO driver");
> 
> "TPS6521x GPIO driver"?
> 
> I also see a product named TPS65216.
> By any chance can that be also supported by this driver?
> 

That is kinda the issue with "x" in the name, TPS65216 might
need a different driver, in which case the x here would mislead
folks into thinking this driver covers the whole family.

Andrew

>>   MODULE_LICENSE("GPL");
>