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
| ||
|
Date: Mon, 18 Oct 2021 19:02:43 +0200 From: Emil Renner Berthing <kernel@...il.dk> To: Andy Shevchenko <andy.shevchenko@...il.com> Cc: linux-riscv <linux-riscv@...ts.infradead.org>, devicetree <devicetree@...r.kernel.org>, linux-clk <linux-clk@...r.kernel.org>, "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>, "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>, Palmer Dabbelt <palmer@...belt.com>, Paul Walmsley <paul.walmsley@...ive.com>, Rob Herring <robh+dt@...nel.org>, Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Marc Zyngier <maz@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>, Linus Walleij <linus.walleij@...aro.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Daniel Lezcano <daniel.lezcano@...aro.org>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Jiri Slaby <jirislaby@...nel.org>, Maximilian Luz <luzmaximilian@...il.com>, Sagar Kadam <sagar.kadam@...ive.com>, Drew Fustini <drew@...gleboard.org>, Geert Uytterhoeven <geert@...ux-m68k.org>, Anup Patel <anup.patel@....com>, Atish Patra <atish.patra@....com>, Matteo Croce <mcroce@...rosoft.com>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Huan Feng <huan.feng@...rfivetech.com> Subject: Re: [PATCH v1 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs On Mon, 18 Oct 2021 at 18:29, Andy Shevchenko <andy.shevchenko@...il.com> wrote: > On Mon, Oct 18, 2021 at 7:23 PM Andy Shevchenko > <andy.shevchenko@...il.com> wrote: > > On Mon, Oct 18, 2021 at 6:56 PM Emil Renner Berthing <kernel@...il.dk> wrote: > > > On Mon, 18 Oct 2021 at 17:48, Andy Shevchenko <andy.shevchenko@...il.com> wrote: > > > > On Mon, Oct 18, 2021 at 6:35 PM Emil Renner Berthing <kernel@...il.dk> wrote: > > > > > On Tue, 12 Oct 2021 at 19:03, Andy Shevchenko <andy.shevchenko@...il.com> wrote: > > > > > > On Tue, Oct 12, 2021 at 4:43 PM Emil Renner Berthing <kernel@...il.dk> wrote: > > ... > > > > > > > > + ret = clk_prepare_enable(clk); > > > > > > > + if (ret) { > > > > > > > > > > > > > + reset_control_deassert(rst); > > > > > > > > > > > > Use devm_add_action_or_reset(). > > > > > > > > > > I don't see how that is better. > > > > > > > > Pity. The rule of thumb is to either try to use devm_*() everywhere in > > > > the probe, or don't use it at all. Above is the more-or-less standard > > > > pattern where devn_add_action_or_reset() is being used in the entire > > > > kernel. > > > > > > > > > Then I'd first need to call that and > > > > > check for errors, but just on the line below enabling the clock the > > > > > reset line is deasserted anyway, so then the action isn't needed any > > > > > longer. So that 3 lines of code for devm_add_action_or_reset + > > > > > lingering unneeded action or code to remove it again vs. just the line > > > > > above. > > > > > > > > Then don't use devm_*() at all. What's the point? > > > > > > I'm confused. So you wan't an unneeded action to linger because the > > > probe function temporarily asserts reset for 3 lines of code? > > > > I;m talking about clk_prepare_enable(). > > Having a second look I found even problematic error paths because of > mixing devm_*() with non-devm_*() calls, which only assures me that > your ->probe() error path is broken and should be revisited. So do you want to expand on that now or should I send v2 first?
Powered by blists - more mailing lists