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] [day] [month] [year] [list]
Date:   Thu, 15 Mar 2018 15:55:09 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Maciej Purski <m.purski@...sung.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Robin Murphy <robin.murphy@....com>,
        dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
        linux-samsung-soc@...r.kernel.org
Cc:     David Airlie <airlied@...ux.ie>,
        Michael Turquette <mturquette@...libre.com>,
        Kamil Debski <kamil@...as.org>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Sylwester Nawrocki <s.nawrocki@...sung.com>,
        Thibault Saunier <thibault.saunier@....samsung.com>,
        Joonyoung Shim <jy0922.shim@...sung.com>,
        Russell King <linux@...linux.org.uk>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Javier Martinez Canillas <javier@....samsung.com>,
        Kukjin Kim <kgene@...nel.org>,
        Hoegeun Kwon <hoegeun.kwon@...sung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Inki Dae <inki.dae@...sung.com>,
        Jeongtae Park <jtp.park@...sung.com>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>,
        Andrzej Pietrasiewicz <andrzej.p@...sung.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Seung-Woo Kim <sw0312.kim@...sung.com>,
        Hans Verkuil <hansverk@...co.com>,
        Kyungmin Park <kyungmin.park@...sung.com>
Subject: Re: [PATCH 1/8] clk: Add clk_bulk_alloc functions

Quoting Marek Szyprowski (2018-02-20 01:36:03)
> Hi Robin,
> 
> On 2018-02-19 17:29, Robin Murphy wrote:
> >
> > Seeing how every subsequent patch ends up with the roughly this same 
> > stanza:
> >
> >     x = devm_clk_bulk_alloc(dev, num, names);
> >     if (IS_ERR(x)
> >         return PTR_ERR(x);
> >     ret = devm_clk_bulk_get(dev, x, num);
> >
> > I wonder if it might be better to simply implement:
> >
> >     int devm_clk_bulk_alloc_get(dev, &x, num, names)
> >
> > that does the whole lot in one go, and let drivers that want to do 
> > more fiddly things continue to open-code the allocation.
> >
> > But perhaps that's an abstraction too far... I'm not all that familiar 
> > with the lie of the land here.
> 
> Hmmm. This patchset clearly shows, that it would be much simpler if we
> get rid of clk_bulk_data structure at all and let clk_bulk_* functions
> to operate on struct clk *array[]. Typically the list of clock names
> is already in some kind of array (taken from driver data or statically
> embedded into driver), so there is little benefit from duplicating it
> in clk_bulk_data. Sadly, I missed clk_bulk_* api discussion and maybe
> there are other benefits from this approach.
> 
> If not, I suggest simplifying clk_bulk_* api by dropping clk_bulk_data
> structure and switching to clock ptr array:
> 
> int clk_bulk_get(struct device *dev, int num_clock, struct clk *clocks[],
>                   const char *clk_names[]);
> int clk_bulk_prepare(int num_clks, struct clk *clks[]);
> int clk_bulk_enable(int num_clks, struct clk *clks[]);
> ...
> 

If you have an array of pointers to names of clks then we can put the
struct clk pointers adjacent to them at the same time. I suppose the
problem there is that some drivers want to mark that array of pointers
to names as const. And then we can't update the clk pointers next to
them.

This is the same design that regulators has done so that's why it's
written like this for clks. Honestly, we're talking about a handful of
pointers here so I'm not sure it really matters much. Just duplicate the
pointer and be done if you want to mark the array of names as const or
have your const 'setup' structure point to the bulk_data array that you
define statically non-const somewhere globally.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ