Re: [PATCH v1 6/7] regulator: tps65215: Define probe() helper functions

[Shree Ramamoorthy <s-ramamoorthy@ti.com>]

Hi,

On 1/1/25 5:01 AM, Christophe JAILLET wrote:
> Le 26/12/2024 à 22:54, Shree Ramamoorthy a écrit :
>> Factor register_regulators() and request_irqs() out into smaller 
>> functions.
>> These 2 helper functions are used in the next restructure probe() 
>> patch to
>> go through the common (overlapping) regulators and irqs first, then the
>> device-specific structs identifed in the chip_data struct.
>>
>> Signed-off-by: Shree Ramamoorthy 
>> <s-ramamoorthy-l0cyMroinI0@public.gmane.org>
>> ---
>>   drivers/regulator/tps65219-regulator.c | 64 ++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/drivers/regulator/tps65219-regulator.c 
>> b/drivers/regulator/tps65219-regulator.c
>> index 13f0e68d8e85..8469ee89802c 100644
>> --- a/drivers/regulator/tps65219-regulator.c
>> +++ b/drivers/regulator/tps65219-regulator.c
>> @@ -346,6 +346,70 @@ static struct chip_data chip_info_table[] = {
>>       },
>>   };
>>   +static int tps65219_register_regulators(const struct 
>> regulator_desc *regulators,
>> +                    struct tps65219 *tps,
>> +                    struct device *dev,
>> +                    struct regulator_config config,
>> +                    unsigned int arr_size)
>> +{
>> +    int i;
>> +    struct regulator_dev *rdev;
>> +
>> +    config.driver_data = tps;
>> +    config.dev = tps->dev;
>> +    config.regmap = tps->regmap;
>> +
>> +    for (i = 0; i < arr_size; i++) {
>> +        rdev = devm_regulator_register(dev, &regulators[i],
>> +                        &config);
>> +        if (IS_ERR(rdev)) {
>> +            dev_err(tps->dev,
>> +                "Failed to register %s regulator\n",
>> +                regulators[i].name);
>
> This will be called from probe in 7/7.
> So this could be return dev_err_probe()
>
I left these as dev_err(), since dev_err_probe() is used when there is a chance
-EPROBE_DEFER is returned. For both functions using dev_err() here, -ENOMEM is returned.
Should I still switch these 2 instances to dev_err_probe()?

Thank you for your help!

>> +
>> +            return PTR_ERR(rdev);
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int tps65219_request_irqs(struct tps65219_regulator_irq_type 
>> *irq_types,
>> +                 struct tps65219 *tps, struct platform_device *pdev,
>> +                 struct tps65219_regulator_irq_data *irq_data,
>> +                 unsigned int arr_size)
>> +{
>> +    int i;
>> +    int irq;
>> +    int error;
>> +    struct tps65219_regulator_irq_type *irq_type;
>> +
>> +    for (i = 0; i < arr_size; ++i) {
>> +        irq_type = &irq_types[i];
>> +
>> +        irq = platform_get_irq_byname(pdev, irq_type->irq_name);
>> +        if (irq < 0)
>> +            return -EINVAL;
>> +
>> +        irq_data[i].dev = tps->dev;
>> +        irq_data[i].type = irq_type;
>> +
>> +        error = devm_request_threaded_irq(tps->dev, irq, NULL,
>> +                          tps65219_regulator_irq_handler,
>> +                          IRQF_ONESHOT,
>> +                          irq_type->irq_name,
>> +                          &irq_data[i]);
>> +        if (error) {
>> +            dev_err(tps->dev,
>> +                "Failed to request %s IRQ %d: %d\n",
>> +                irq_type->irq_name, irq, error);
>
> This will be called from probe in 7/7.
> So this could be return dev_err_probe()
>
>> +            return error;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int tps65219_regulator_probe(struct platform_device *pdev)
>>   {
>>       struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
>

-- 
Best,
Shree Ramamoorthy
PMIC Software Engineer