From: Uwe Kleine-König on
On Mon, Jun 14, 2010 at 11:30:08AM +0200, Lothar Wa�mann wrote:
> Benjamin Herrenschmidt writes:
> > On Mon, 2010-06-14 at 08:39 +0200, Lothar Wa�mann wrote:
> > > All implementations so far use spin_lock_irq_save()!
> >
> > Nothing prevents your implementation to be a tad smarter.
> >
> I vote for consistency, so that device drivers can be kept arch
> independent instead of having to care about implentation details of
> each arch.
Back when I implemented clock support for ns9xxx (unfortunately not in
mainline) I tried with a spinlock first and later switched to a mutex.
IIRC the reason was that on ns9215 enabling the rtc clock took long
(don't remember a number) and successfull enabling was signaled by an
irq. So I would have had to implement irq polling in the clock code.

I think you can find different examples that make both possiblities bad.
All in all I think that a sleeping clock implementation is preferable as
it improves (general) latencies.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K�nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jeremy Kerr on
Hi Ben,

I've just pushed to my clk-common branch, mostly involving the changes we've
discussed so far.

http://kernel.ubuntu.com/git?p=jk/dt/linux-2.6.git;a=commitdiff;h=f2c5141c88aa4c5ba82aee1b5b8abe5e9ff09146

The clk-common-mx51 and clk-common-versatile branches both contain basic ports
of the mx51 and versatile platforms to the new clock infrastructure.

Cheers,


Jeremy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Ben Dooks on
On Mon, Jun 14, 2010 at 11:43:10AM +0200, Uwe Kleine-K?nig wrote:
> On Mon, Jun 14, 2010 at 11:30:08AM +0200, Lothar Wa?mann wrote:
> > Benjamin Herrenschmidt writes:
> > > On Mon, 2010-06-14 at 08:39 +0200, Lothar Wa?mann wrote:
> > > > All implementations so far use spin_lock_irq_save()!
> > >
> > > Nothing prevents your implementation to be a tad smarter.
> > >
> > I vote for consistency, so that device drivers can be kept arch
> > independent instead of having to care about implentation details of
> > each arch.
> Back when I implemented clock support for ns9xxx (unfortunately not in
> mainline) I tried with a spinlock first and later switched to a mutex.
> IIRC the reason was that on ns9215 enabling the rtc clock took long
> (don't remember a number) and successfull enabling was signaled by an
> irq. So I would have had to implement irq polling in the clock code.

Ok, you could have implemented a lock ot update the state, then had
some form of wake-queue to wake up the task once it did.

> I think you can find different examples that make both possiblities bad.
> All in all I think that a sleeping clock implementation is preferable as
> it improves (general) latencies.

It may be that we need to do a bit of work on some of the drivers to
ensure that they don't fully re-set their clocks until they are able
to sleep.

--
Ben (ben(a)fluff.org, http://www.fluff.org/)

'a smiley only costs 4 bytes'
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on
On Wed, 2010-06-16 at 22:16 +0100, Ben Dooks wrote:
> > I think you can find different examples that make both possiblities
> bad.
> > All in all I think that a sleeping clock implementation is
> preferable as
> > it improves (general) latencies.
>
> It may be that we need to do a bit of work on some of the drivers to
> ensure that they don't fully re-set their clocks until they are able
> to sleep.

With the move of the interrupt disable we did upstream not long ago,
most drivers now can sleep at any stage of resume. sysdev's can't so
those are the main issue, but then, I don't like sysdevs :-)

I must say that on powermac I avoid some of these problems by, in
addition to the normal suspend/resume process, having some platform code
that (almost) blindly saves & restores a bunch of ASIC regs on late
suspend/early resume. It ends up being faster and simpler than going
through convolutions of suspend/resume callbacks left and right that
need to happen in the right order.

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Baruch Siach on
Hi Jeremy,

On Mon, Jun 21, 2010 at 01:35:13PM +0800, Jeremy Kerr wrote:
> We currently have 21 definitions of struct clk in the ARM architecture,
> each defined on a per-platform basis. This makes it difficult to define
> platform- (or architecture-) independent clock sources without making
> assumptions about struct clk, and impossible to compile two
> platforms with different struct clks into a single image.
>
> This change is an effort to unify struct clk where possible, by defining
> a common struct clk, containing a set of clock operations. Different
> clock implementations can set their own operations, and have a standard
> interface for generic code. The callback interface is exposed to the
> kernel proper, while the clock implementations only need to be seen by
> the platform internals.
>
> This allows us to share clock code among platforms, and makes it
> possible to dynamically create clock devices in platform-independent
> code.
>
> Platforms can enable the generic struct clock through
> CONFIG_USE_COMMON_STRUCT_CLK. In this case, the clock infrastructure
> consists of a common struct clk:
>
> struct clk {
> const struct clk_ops *ops;
> unsigned int enable_count;
> struct mutex mutex;
> };
>
> And a set of clock operations (defined per type of clock):
>
> struct clk_operations {

That's clk_ops above, and in the code.

> int (*enable)(struct clk *);
> void (*disable)(struct clk *);
> unsigned long (*get_rate)(struct clk *);
> [...]
> };
>
> To define a hardware-specific clock, machine code can "subclass" the
> struct clock into a new struct (adding any device-specific data), and
> provide a set of operations:
>
> struct clk_foo {
> struct clk clk;
> void __iomem *some_register;
> };
>
> struct clk_operations clk_foo_ops = {

Ditto.

> .get_rate = clk_foo_get_rate,
> };
>
> The common clock definitions are based on a development patch from Ben
> Herrenschmidt <benh(a)kernel.crashing.org>.
>
> Signed-off-by: Jeremy Kerr <jeremy.kerr(a)canonical.com>

baruch

--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch(a)tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/