lkml.org 
[lkml]   [2010]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC,PATCH 1/2] Add a common struct clk
Date
Hi Ben,

> > And a set of clock operations (defined per type of clock):
> >
> > struct clk_operations {
> >
> > int (*enable)(struct clk *);
>
> I'd rather the enable/disable calls where simply a set
> and a bool on/off, very rarelyt is the enable and disable
> operartions different.

I thought about merging these, but decided against it. It does work for the
simple case where we're setting a bit in a register:

static int clk_foo_set_state(struct clk *_clk, int enable)
{
struct clk_foo *clk = to_clk_foo(_clk)
u32 reg;

reg = raw_readl(foo->some_register);
if (enable)
reg |= FOO_ENABLE;
else
reg &= ~FOO_ENABLE;
raw_writel(foo->some_register, reg);

return 0;
}

However, for anything more complex than this - for example, if there's a
parent clock - then we start getting pretty messy:

static int clk_foo_set_state(struct clk *_clk, int enable)
{
struct clk_foo *clk = to_clk_foo(_clk)
u32 reg;

if (enable) {
int ret = clk_enable(clk->parent);
if (ret)
return ret;
}

reg = raw_readl(foo->some_register);
if (enable)
reg |= FOO_ENABLE;
else
reg &= ~FOO_ENABLE;

raw_writel(foo->some_register, reg);

if (!enable)
clk_disable(clk->parent);

return 0;
}

- where most of the function becomes surrounded by "if (enable)" statements.

I'm aware that we can turn this into a conditional call of clk_foo_enable or
clk_foo_disable, but then we're back to square 1. I also think that the simple
case is clearer (if a little more verbose) with separate functions.

Also, enable and disable in the external clock API have different return
types.

> an aside, you might want to just clal these clk_ops to get into the
> spirit of the original naming.

Either is fine with me - looks like 'ops' is more commonly used:

$ git grep -E '^struct \w*operations\s*\{' include/ | wc -l
30

$ git grep -E '^struct \w*ops\s*{' include/ | wc -l
138

Cheers,


Jeremy


\
 
 \ /
  Last update: 2010-06-03 05:23    [W:0.104 / U:0.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site