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

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

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, &regulators[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!