On 03/01/2025 01:41, Shree Ramamoorthy wrote:
> 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@xxxxxxxxxxxxxxxx>
>>> ---
>>> 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, ®ulators[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!
What you coudld to is simply return error here and
add the dev_err_probe() in the probe function.
>
>>> +
>>> + 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()
Same here, just return error here and leave the error printing
job for the probe function.
>>
>>> + return error;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static int tps65219_regulator_probe(struct platform_device *pdev)
>>> {
>>> struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
>>
>
--
cheers,
-roger
