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@xxxxxx>
>> ---
>> 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);
