Re: [PATCH v3 1/8] i2c: riic: Use dev_err_probe in probe and riic_init_hw functions

[Lad, Prabhakar]

Hi Andy,

Thank you for the review.

On Sat, Dec 28, 2024 at 11:35 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> Fri, Dec 27, 2024 at 11:51:47AM +0000, Prabhakar kirjoitti:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Refactor error handling in the riic_i2c_probe() and riic_init_hw()
> > functions by replacing multiple dev_err() calls with dev_err_probe().
> >
> > Additionally, update the riic_init_hw() function to use a local `dev`
> > pointer instead of `riic->adapter.dev` for dev_err_probe(), as the I2C
> > adapter is not initialized at this stage.
>
> ...
>
> > +     if (brl > (0x1F + 3))
> > +             return dev_err_probe(dev, -EINVAL, "invalid speed (%lu). Too slow.\n",
> > +                                  (unsigned long)t->bus_freq_hz);
>
> There is nothing special about bus_freq_hz. Why casting?
>
Ok, I'll update it like below:

    if (brl > (0x1F + 3))
        return dev_err_probe(dev, -EINVAL, "invalid speed (%uHz). Too slow.\n",
                     t->bus_freq_hz);

> ...
>
> >               ret = devm_request_irq(dev, ret, riic_irqs[i].isr,
>
> I hate code doing
>
>                 ret = foo(ret);
>
> >                                      0, riic_irqs[i].name, riic);
>
> > +             if (ret)
> > +                     return dev_err_probe(dev, ret, "failed to request irq %s\n",
> > +                                          riic_irqs[i].name);
>
> While this following the original code, with the above change (introducing a
> separate variable for IRQ) this might also print it.
>
Ok, I'll create a new patch for this and have something like below:


    for (i = 0; i < ARRAY_SIZE(riic_irqs); i++) {
        int irq = platform_get_irq(pdev, riic_irqs[i].res_num);

        ret = devm_request_irq(dev, irq, riic_irqs[i].isr,
                       0, riic_irqs[i].name, riic);
        if (ret)
            return ret;
    }

Cheers,
Prabhakar