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 = ®s_irq_types[j];
> irq = platform_get_irq_byname(pdev, irq_type->irq_name);
> if (irq < 0)
> return -EINVAL;
