| Message ID | 20241226215412.395822-7-s-ramamoorthy@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add TI TPS65215 PMIC Regulator Support | expand |
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, ®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() > + > + 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);
Le 03/01/2025 à 00:41, Shree Ramamoorthy a écrit : > 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, ®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! > >>> + >>> + 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() Up to you to choose one or the other. The other advantages of using dev_err_probe() are: - log the error code in a human readable format - combined with return, it usually saves a few LoC, because some { } can be removed most of the time. CJ >> >>> + return error; >>> + } >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static int tps65219_regulator_probe(struct platform_device *pdev) >>> { >>> struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent); >> >
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); + + 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); + return error; + } + } + + return 0; +} + static int tps65219_regulator_probe(struct platform_device *pdev) { struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
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@ti.com> --- drivers/regulator/tps65219-regulator.c | 64 ++++++++++++++++++++++++++ 1 file changed, 64 insertions(+)