linux-kernel - Re: [PATCH] clk: Mark fwnodes when their clock provider is added

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>] [day] [month] [year] [list]

Date:   Mon, 05 Apr 2021 13:04:40 +0200
From:   Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
To:     Stephen Boyd <sboyd@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Saravana Kannan <saravanak@...gle.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Tudor Ambarus <tudor.ambarus@...rochip.com>,
        Jonathan Corbet <corbet@....net>,
        Frank Rowand <frowand.list@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Kevin Hilman <khilman@...nel.org>,
        Len Brown <len.brown@...el.com>, Len Brown <lenb@...nel.org>,
        Marc Zyngier <maz@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Pavel Machek <pavel@....cz>,
        "Rafael J.Wysocki" <rafael@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        DOCUMENTATION <linux-doc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM list <linux-pm@...r.kernel.org>,
        linux-clk <linux-clk@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS "
         "<devicetree@...r.ke rnel.org>, ACPI Devel Maling List "
         "<linux-acpi@...r.kernel.org>, Android Kernel Team "
         "<kernel-team@...roid.com>, linux-rpi-kernel" 
        <linux-rpi-kernel@...ts.infradead.org>
Subject: Re: [PATCH] clk: Mark fwnodes when their clock provider is added

On Wed, 2021-03-31 at 12:25 -0700, Stephen Boyd wrote:
> Quoting Geert Uytterhoeven (2021-03-31 00:05:00)
> > On Wed, Mar 31, 2021 at 4:22 AM Stephen Boyd <sboyd@...nel.org> wrote:
> > > > > Does it have any use?
> > > > 
> > > > of_clk_del_provider() removes the first provider found with node == NULL.
> > > > If there are two drivers calling of_clk_add_hw_provider(), and one of
> > > > hem calls of_clk_del_provider() later, the wrong provider may be
> > > > removed from the list.
> > > > 
> > > 
> > > So you're saying we shouldn't add a NULL device node pointer to the list
> > > so that this can't happen? That doesn't mean returning an error from
> > > of_clk_add_hw_provider() would be useful though.
> > > of_clk_add_hw_provider() can return 0 if np == NULL and
> > > of_clk_del_provider() can return early if np == NULL too.
> > 
> > I don't know if I grasp all meanings of the above.
> > 
> > The main question is if it is valid for a driver to call
> > of_clk_add_hw_provider()
> > with np == NULL.
> >   - If yes, should that register the provider?
> 
> No it should not register the provider. That would be bad as you pointed
> out.
> 
> >       - If yes, how to handle two drivers calling of_clk_add_hw_provider()
> >         with np = NULL, as their unregistration order is not guaranteed to
> >         be correct.
> > 
> > If no, is that something to ignore (0), or a bug (error)?
> 
> This is my question above. Is there a use to having
> of_clk_add_hw_provider() return an error value when np == NULL? I doubt
> it.
> 
> Returning 0 would reduce the if conditions in driver code in this case
> and be consistent with the CONFIG_OF=n inline stub that returns 0 when
> CONFIG_OF is disabled. The only case an error would be returned is if we
> couldn't allocate memory or if the assigned clocks code failed. Seems
> sane to me. The downside is that drivers would maybe register clkdev
> lookups when they don't need to and waste some memory. I'm fine with
> that until we have some sort of non-DT based clk provider lookup
> mechanism that could unify the two methods.

What about devm_of_clk_add_hw_provider() users, do we care that a seemingly
empty managed resource will be created?

Regards,
Nicolas


Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)