Re: [PATCH v5 09/11] regulator: tps6594-regulator: Add TI TPS65224 PMIC regulators

[Dan Carpenter]

On Thu, Mar 28, 2024 at 06:10:14PM +0530, Bhargav Raviprakash wrote:
>  static irqreturn_t tps6594_regulator_irq_handler(int irq, void *data)
>  {
>  	struct tps6594_regulator_irq_data *irq_data = data;
> @@ -369,17 +513,23 @@ static irqreturn_t tps6594_regulator_irq_handler(int irq, void *data)
>  static int tps6594_request_reg_irqs(struct platform_device *pdev,
              ^^^^^^^
This function is not beautiful.  I think since you're changing it from
being tps6594 specific, maybe you want to rename a bunch of stuff.

>  				    struct regulator_dev *rdev,
>  				    struct tps6594_regulator_irq_data *irq_data,
> -				    struct tps6594_regulator_irq_type *tps6594_regs_irq_types,
> +				    struct tps6594_regulator_irq_type *regs_irq_types,
>  				    int *irq_idx)
>  {
>  	struct tps6594_regulator_irq_type *irq_type;
>  	struct tps6594 *tps = dev_get_drvdata(pdev->dev.parent);
> -	int j;
> +	size_t j;
>  	int irq;
>  	int error;
> +	size_t interrupt_cnt;
> +
> +	if (tps->chip_id == TPS6594)
> +		interrupt_cnt = ARRAY_SIZE(tps6594_buck1_irq_types);
> +	else
> +		interrupt_cnt = ARRAY_SIZE(tps65224_buck1_irq_types);

Either 1) pass both the array and the size or 2) just use tps->chip_id
to determine both the array and the arrays_size.  Passing the array and
then determining which array was passed by looking at the type is ugly.

regards,
dan carpenter


>  
> -	for (j = 0; j < REGS_INT_NB; j++) {
> -		irq_type = &tps6594_regs_irq_types[j];
> +	for (j = 0; j < interrupt_cnt; j++) {
> +		irq_type = &regs_irq_types[j];
>  		irq = platform_get_irq_byname(pdev, irq_type->irq_name);
>  		if (irq < 0)
>  			return -EINVAL;