linux-kernel - Re: [PATCH v3 2/3] iio: adc: Add driver for Renesas RZ/G2L A/D converter

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: <145309b88353d4127c659dfabd374252cb2afc48.camel@pengutronix.de>
Date:   Tue, 27 Jul 2021 10:13:25 +0200
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     "Lad, Prabhakar" <prabhakar.csengg@...il.com>
Cc:     Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Rob Herring <robh+dt@...nel.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Magnus Damm <magnus.damm@...il.com>,
        Alexandru Ardelean <aardelean@...iqon.com>,
        linux-iio <linux-iio@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Biju Das <biju.das.jz@...renesas.com>
Subject: Re: [PATCH v3 2/3] iio: adc: Add driver for Renesas RZ/G2L A/D
 converter

On Tue, 2021-07-27 at 09:02 +0100, Lad, Prabhakar wrote:
[...]
> > > +     ret = devm_add_action_or_reset(&pdev->dev,
> > > +                                    rzg2l_adc_reset_assert, adc->adrstn);
> > > +     if (ret) {
> > > +             dev_err(&pdev->dev, "failed to register adrstn assert devm action, %d\n",
> > > +                     ret);
> > > +             return ret;
> > > +     }
> > 
> > This is the wrong way around. Installing devres actions should be done
> > after the thing they are supposed to revert in case of error. You should
> > move this down below the reset_control_deassert(adc->adrstn).
> > 
> Ouch my understanding was, there won't be any harm in asserting the
> reset line. Agree with will move this below
> reset_control_deassert(adc->adrstn).

You are probably right, but it's still better do it correctly. Just
imagine one of the reset lines turns out to be shared later, or somebody
else will look at this driver for inspiration.

regards
Philipp