| Message ID | 20241226215412.395822-7-s-ramamoorthy@ti.com (mailing list archive) |
|---|---|
| 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);
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() > >> + 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); >> >
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@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! 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); >> >
On 26/12/2024 23:54, Shree Ramamoorthy wrote: > 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(+) > > 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; reverse xmas tree? > + > + 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; here too. > + > + for (i = 0; i < arr_size; ++i) { > + irq_type = &irq_types[i]; > + unnecessary new line. > + 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; > + here too > + 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); This patch by itself will complain during build as there are no users for these functions. Could you please squash patches 6 and 7?
Hi, On 1/4/2025 12:45 PM, Roger Quadros wrote: > > On 26/12/2024 23:54, Shree Ramamoorthy wrote: >> 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(+) >> >> 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; > reverse xmas tree? Applied reverse xmas tree style to this file & will review other files as well for this. >> + >> + 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; > here too. > >> + >> + for (i = 0; i < arr_size; ++i) { >> + irq_type = &irq_types[i]; >> + > unnecessary new line. > >> + 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; >> + > here too Removed both new lines. >> + 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); > This patch by itself will complain during build as there are no users for > these functions. > Could you please squash patches 6 and 7? I kept patch 6 and 7 separate as the diff was hard to read & the git diff options did not resolve this. Is there a way to keep these 2 patches separate for user readability and avoid the build error? Or just squash them to prevent build errors knowing the diff will be hard to read? Thank you for your help!
On 1/6/25 4:02 PM, Shree Ramamoorthy wrote: > Hi, > > On 1/4/2025 12:45 PM, Roger Quadros wrote: >> >> On 26/12/2024 23:54, Shree Ramamoorthy wrote: >>> 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(+) >>> >>> 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; >> reverse xmas tree? > > Applied reverse xmas tree style to this file & will review other files as well for this. > >>> + >>> + 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; >> here too. >> >>> + >>> + for (i = 0; i < arr_size; ++i) { >>> + irq_type = &irq_types[i]; >>> + >> unnecessary new line. >> >>> + 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; >>> + >> here too > > Removed both new lines. > >>> + 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); >> This patch by itself will complain during build as there are no users for >> these functions. >> Could you please squash patches 6 and 7? > > I kept patch 6 and 7 separate as the diff was hard to read & > the git diff options did not resolve this. Is there a way to keep these 2 patches > separate for user readability and avoid the build error? Or just squash them to > prevent build errors knowing the diff will be hard to read? Thank you for your help! > > Instead of splitting the adding and the using of the functions, could you split tps65219_register_regulators() and tps65219_request_irqs() into their own patches? Each patch should add and also make use of the added function. Andrew
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(+)