linux-kernel - Re: [PATCH 0/5] Clocklib: generic clocks framework

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]

Date:	Fri, 25 Apr 2008 15:46:51 -0700
From:	David Brownell <david-b@...bell.net>
To:	Dmitry <dbaryshkov@...il.com>
Cc:	"Paul Walmsley" <paul@...an.com>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org,
	"Haavard Skinnemoen" <haavard.skinnemoen@...el.com>,
	"Russell King" <rmk+lkml@....linux.org.uk>,
	"Paul Mundt" <lethal@...ux-sh.org>,
	"pHilipp Zabel" <philipp.zabel@...il.com>,
	"Pavel Machek" <pavel@....cz>, tony@...mide.com,
	hiroshi.DOYU@...ia.com
Subject: Re: [PATCH 0/5] Clocklib: generic clocks framework

On Monday 21 April 2008, Dmitry wrote:
> >  - I don't think that I understand the clk_functions part of your code.
> >   Is this a shorthand to construct aliases to other struct clks?
>
> Yes, that's one of usages for it. E.g. current AT91 code has same
> functionality named at91_clock_associate.

As in:

	at91_clock_associate("usart0_clk", &pdev0->dev, "usart");
	at91_clock_associate("usart1_clk", &pdev1->dev, "usart");
	at91_clock_associate("usart2_clk", &pdev2->dev, "usart");

That essentially maps a device and logical clock name to a specific
clock (those "usartX_clk" identifiers are global), so that drivers
can clk_get(dev, "usart") and get the right clock.  It decouples
clock and device declarations, which should help when integrating
external clocks too.

The assumption that clk_get(NULL, "usartX_clk") works is mostly
just a convenience, although it's probably a fair assumption for
many other SOC platforms.  Conceptually that first parameter is
a clock, not a name.

> > +struct clk_function {
> > +       const char *parent;
> > +       void *priv;
> > +       struct clk clk;
> > +};

That doesn't really seem attuned to the task of associating
logical clock names to devices.  Wouldn't it be better to
have something directly addressing that core mechanism?

> Also once we get to multiple chips providers/users, we'll see,
> that the clock simply can't have just one record in the clocks tree.

I don't follow.  Why not?  If a clock has multiple records, I'd
expect its refcounts would easily get borked.  I think I don't
like your notion of a "wrapper clock".

> It's provided by some
> pin (provider_name) and then consumed by several devices (several
> consumer_name + consumer_device pairs). That is it.

There would still be just one clock though.  It shouldn't matter
how clk_get(this_dev, "c1") and clk_get(that_dev, "c2") find it.
Hashtable, linked list, divination, ... so long as it works.

Those other records should just hold {dev, name} ==> clock mappings;
they wouldn't be records for the clock itself.  But those "functions"
do not seem to record just mappings.

- Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/