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

[Roger Quadros <rogerq@kernel.org>]

On 06/01/2025 19:30, Andrew Davis wrote:
> 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.

Thanks!

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

Agreed.

> 
> Andrew
> 
>>>   MODULE_LICENSE("GPL");
>>

-- 
cheers,
-roger