linux-kernel - Re: [PATCH v3 1/8] i2c: riic: Use dev_err_probe in probe and riic_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+V-a8v0_9Y5qsDGtwLL76-LY5u8Tf4odbDvU9sbVE73BZ=ZHQ@mail.gmail.com>
Date: Tue, 31 Dec 2024 14:20:28 +0000
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Chris Brandt <chris.brandt@...esas.com>, Andi Shyti <andi.shyti@...nel.org>, 
	Geert Uytterhoeven <geert+renesas@...der.be>, Wolfram Sang <wsa+renesas@...g-engineering.com>, 
	Philipp Zabel <p.zabel@...gutronix.de>, linux-renesas-soc@...r.kernel.org, 
	linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Biju Das <biju.das.jz@...renesas.com>, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH v3 1/8] i2c: riic: Use dev_err_probe in probe and
 riic_init_hw functions

Hi Andy,

Thank you for the review.

On Sat, Dec 28, 2024 at 11:35 PM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
>
> Fri, Dec 27, 2024 at 11:51:47AM +0000, Prabhakar kirjoitti:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > 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