Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-26 Thread David Brownell
On Tuesday 26 February 2008, Haavard Skinnemoen wrote:
> Ok. Let's drop the clock references...
> 
> > > and it will always need a pointer through which to access the
> > > registers, so the mid-layer might as well do those things.
> > 
> > True about doing the ioremap.
> 
> ...and keep the regs pointer, and possibly add the iomem resource.
> 
> Ok?

I'd be OK with clocks *and* irqs, but don't feel religious at all.


> Btw, I'm not saying that you're the one who has to make these changes.
> Once we agree, I can send a patch to do the things we agreed on.

I'll ask you to do that then, yes.

- Dave



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


Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-26 Thread Haavard Skinnemoen
On Mon, 25 Feb 2008 10:06:44 -0800
David Brownell <[EMAIL PROTECTED]> wrote:

> > > > Which reminds me...you were talking about a patch that adds oneshot
> > > > support for the count/compare clocksource and more cleanups, but I
> > > > don't think I've seen it...?
> > > 
> > > I avoid sending non-working patches, and hadn't made time to
> > > work on that issue recently.
> >
> > I was thinking that I could perhaps help you get it working...
> 
> OK, if you want I'll send it.

That would be great. Otherwise I fear your work might be lost...

> > > > I've never really seen the point of indenting those defines at all.
> > > 
> > > Without them, it's harder to discern the logical structure of
> > > all the various bitfields and their contents.
> >
> > I prefer to separate the registers from the bitfields and the other
> > stuff. That way, no indentation is necessary.
> 
> But that way it's no longer clear which symbols apply to which
> registers.  It's not "needed" because that information became
> harder to find ... as in ,
> which ISTR isn't even complete in its bitfield definitions.

Ok, I get your point.

> There's one clear need for tclib:  to hand out timers from the (small)
> pool of hardware entities.  Each one can be used for different purposes
> by different drivers.
> 
> The *current* tclib addresses only that minimal need.
> 
> You're the one who wants to add more to it; I was just pointing out that
> you were being inconsistent.

And I was trying to point out that _you_ were being inconsistent ;-)

> > Of course the driver should be responsible for calling clk_enable() and
> > clk_disable(). Otherwise, power management will be tricky. And since
> > the driver may need to make a decision about which interrupts to
> > request, it might as well call platform_get_irq() directly.
> 
> You're being inconsistent here.  It has to make the same kinds of
> decision about which clocks to enable/disable.  So why should it
> have to platform_get_irq() but not clk_get()?
> 
> > On the other hand, the driver will _always_ need a reference to each
> > clock,
> 
> No; it doesn't need references to clocks for unused TC channels.

Ok. Let's drop the clock references...

> > and it will always need a pointer through which to access the
> > registers, so the mid-layer might as well do those things.
> 
> True about doing the ioremap.

...and keep the regs pointer, and possibly add the iomem resource.

Ok?

Btw, I'm not saying that you're the one who has to make these changes.
Once we agree, I can send a patch to do the things we agreed on.

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


Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-25 Thread David Brownell
> > > Which reminds me...you were talking about a patch that adds oneshot
> > > support for the count/compare clocksource and more cleanups, but I
> > > don't think I've seen it...?
> > 
> > I avoid sending non-working patches, and hadn't made time to
> > work on that issue recently.
>
> I was thinking that I could perhaps help you get it working...

OK, if you want I'll send it.


> > > I've never really seen the point of indenting those defines at all.
> > 
> > Without them, it's harder to discern the logical structure of
> > all the various bitfields and their contents.
>
> I prefer to separate the registers from the bitfields and the other
> stuff. That way, no indentation is necessary.

But that way it's no longer clear which symbols apply to which
registers.  It's not "needed" because that information became
harder to find ... as in ,
which ISTR isn't even complete in its bitfield definitions.


> > > I thought about that, but while the driver can safely call clk_enable()
> > > on the same clock several times, I'm not sure if it's such a great idea
> > > to call request_irq() on the same interrupt several times. So the
> > > driver probably needs to know how many irqs there really are and might
> > > as well use platform_get_irq() to find out.
> > 
> > I thought the whole point of passing the clocks was to avoid needing
> > to ask for them!!  If trying one or three platform_get_irq() calls is
> > OK, then surely trying one or three clk_get() calls is also OK...
>
> If you want to go down that path, surely reserving the iomem resource
> is fine too? Why don't we just kill the whole tclib layer, the driver
> can certainly do everything itself?

There's one clear need for tclib:  to hand out timers from the (small)
pool of hardware entities.  Each one can be used for different purposes
by different drivers.

The *current* tclib addresses only that minimal need.

You're the one who wants to add more to it; I was just pointing out that
you were being inconsistent.


> Of course the driver should be responsible for calling clk_enable() and
> clk_disable(). Otherwise, power management will be tricky. And since
> the driver may need to make a decision about which interrupts to
> request, it might as well call platform_get_irq() directly.

You're being inconsistent here.  It has to make the same kinds of
decision about which clocks to enable/disable.  So why should it
have to platform_get_irq() but not clk_get()?


> On the other hand, the driver will _always_ need a reference to each
> clock,

No; it doesn't need references to clocks for unused TC channels.


>   and it will always need a pointer through which to access the
> registers, so the mid-layer might as well do those things.

True about doing the ioremap.

- Dave

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


Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-25 Thread Haavard Skinnemoen
On Sun, 24 Feb 2008 17:03:10 -0800
David Brownell <[EMAIL PROTECTED]> wrote:

> > Which reminds me...you were talking about a patch that adds oneshot
> > support for the count/compare clocksource and more cleanups, but I
> > don't think I've seen it...?
> 
> I avoid sending non-working patches, and hadn't made time to
> work on that issue recently.

I was thinking that I could perhaps help you get it working...

> > But I was thinking about Linus' suggestions that we exploit the
> > distributed nature of git and do cross-tree merges to synchronize
> > changes to common code.
> >
> > Setting up a separate git tree would allow the changes to go into the
> > arm tree without littering it with possibly unstable avr32 changes as
> > well, and it would allow me to merge them and put more stuff on top.
> 
> Doing that with ARM patches is Russell's call; he hasn't been too
> keen on merging from non-Linus GIT trees when that came up before.

Fine with me either way.

> > I've never really seen the point of indenting those defines at all.
> 
> Without them, it's harder to discern the logical structure of
> all the various bitfields and their contents.

I prefer to separate the registers from the bitfields and the other
stuff. That way, no indentation is necessary.

> > I thought about that, but while the driver can safely call clk_enable()
> > on the same clock several times, I'm not sure if it's such a great idea
> > to call request_irq() on the same interrupt several times. So the
> > driver probably needs to know how many irqs there really are and might
> > as well use platform_get_irq() to find out.
> 
> I thought the whole point of passing the clocks was to avoid needing
> to ask for them!!  If trying one or three platform_get_irq() calls is
> OK, then surely trying one or three clk_get() calls is also OK...

If you want to go down that path, surely reserving the iomem resource
is fine too? Why don't we just kill the whole tclib layer, the driver
can certainly do everything itself?

Of course the driver should be responsible for calling clk_enable() and
clk_disable(). Otherwise, power management will be tricky. And since
the driver may need to make a decision about which interrupts to
request, it might as well call platform_get_irq() directly.

On the other hand, the driver will _always_ need a reference to each
clock, and it will always need a pointer through which to access the
registers, so the mid-layer might as well do those things.

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


Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-24 Thread David Brownell
> > > > Note that this won't be usable until the AT91 and AT32 platforms
> > > > incorporate patches to configure the relevant platform devices.
> > > > Those changes are probably 2.6.26 material.
> > 
> > More specifically (and all those patches are available now):
> > 
> >  - AT91 needs clocksource/clockevent support for the SAM9 PIT timer;
> >  - AVR32 needs more extensive clocksource/clockevent updates;
>
> Which reminds me...you were talking about a patch that adds oneshot
> support for the count/compare clocksource and more cleanups, but I
> don't think I've seen it...?

I avoid sending non-working patches, and hadn't made time to
work on that issue recently.


> Yes, getting the fundamental stuff into mainline now would help a lot.

Or at least, towards the front of the merge queue, ahead of
the various platform-specific patches.


> But I was thinking about Linus' suggestions that we exploit the
> distributed nature of git and do cross-tree merges to synchronize
> changes to common code.
>
> Setting up a separate git tree would allow the changes to go into the
> arm tree without littering it with possibly unstable avr32 changes as
> well, and it would allow me to merge them and put more stuff on top.

Doing that with ARM patches is Russell's call; he hasn't been too
keen on merging from non-Linus GIT trees when that came up before.


> > > > +#define ATMEL_TC_BMR   0xc4/* TC Block Mode Register */
> > > > +#define ATMEL_TC_TC0XC0S   (3 << 0)/* external clock 0 
> > > > source */
> > > > +#defineATMEL_TC_TC0XC0S_TCLK0  (0 << 0)
> > >
> > > Hmm. Indentation using spaces? I didn't know you were into that sort of
> > > thing ;-)
> > 
> > It's way better than indenting off the right margin of the page!
>
> I've never really seen the point of indenting those defines at all.

Without them, it's harder to discern the logical structure of
all the various bitfields and their contents.


> > > Anyway, my main issue is that I think we should add a data structure
> > > with information about each device, similar to struct ssc_device in the
> > > atmel-ssc driver. How about something like this?
> > >
> > > struct atmel_tc_block {
> > >   void __iomem*regs;  /* non-NULL when busy */
> > >   struct platform_device  *pdev;
> > >   struct clk  *clk[3];
> > >   struct list_headnode;
> > > };
> > 
> > And per-channel IRQs too...
>
> I thought about that, but while the driver can safely call clk_enable()
> on the same clock several times, I'm not sure if it's such a great idea
> to call request_irq() on the same interrupt several times. So the
> driver probably needs to know how many irqs there really are and might
> as well use platform_get_irq() to find out.

I thought the whole point of passing the clocks was to avoid needing
to ask for them!!  If trying one or three platform_get_irq() calls is
OK, then surely trying one or three clk_get() calls is also OK...

- Dave


> > Well, if you want to take these patches off my hands and then be
> > responsible for merging them upstream, I won't object.  :)
>
> I can do that.
>
> It's getting late around here...I'll reply to your other mail tomorrow.
>
> Haavard
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-24 Thread Haavard Skinnemoen
On Sun, 24 Feb 2008 14:55:27 -0800
David Brownell <[EMAIL PROTECTED]> wrote:

> On Sun, 24 Feb 2008 18:45:54 +0100
> Haavard Skinnemoen <[EMAIL PROTECTED]> wrote:
> >
> > On Fri, 22 Feb 2008 17:23:23 -0800
> > David Brownell <[EMAIL PROTECTED]> wrote:
> >
> > > Note that this won't be usable until the AT91 and AT32 platforms
> > > incorporate patches to configure the relevant platform devices.
> > > Those changes are probably 2.6.26 material.
> 
> More specifically (and all those patches are available now):
> 
>  - AT91 needs clocksource/clockevent support for the SAM9 PIT timer;
>  - AVR32 needs more extensive clocksource/clockevent updates;

Which reminds me...you were talking about a patch that adds oneshot
support for the count/compare clocksource and more cleanups, but I
don't think I've seen it...?

I've probably messed things up enough that it won't apply cleanly
anymore, but if you just send me what you have, I'll rebase and fold it
as necessary.

>  - Both also need platform device setup;

Right. I think I've got both those patches.

> Merging these two patches before those is a perfectly safe NOP.

Right. I just want to get everything properly queued up before the
merge window opens.

> > I see the following possibilities:
> >   * I switch avr32 over to use the new code after it has been merged
> > into mainline. This means the new code won't be tested on avr32
> > until near the end of the next merge window -- not good.
> 
> Well, "more widely" tested.  We've both observed it to work; basically
> the same code has worked on AT91 for about a year now, too.

Sure, but a bit more exposure won't hurt.

> And that's why I suggest merging these parts, now that they're known to
> work on both architectures.  The arch-specific bits can follow at their
> own pace, neither one blocking the other.
> 
> >   * I forward the patches that switch avr32 over to Andrew so that they
> > can be included in -mm right after the tclib support. Not a bad
> > option, but I'm planning to do more AVR32 PM work before 2.6.26, so
> > there might be conflicts.
> 
> Conflicts there would be the easy part to deal with.  They're all
> specific to AVR32, and you can merge those in any convenient order.

Assuming the tclib stuff comes first, yes. I'm not worried about me
having to deal with conflicts, I'm worried about Andrew or Stephen
having to fix up my mess.

> >   * I take the whole thing through my avr32 git tree.
> 
> Doesn't work as smoothly for the AT91 side ... but Andrew Victor has
> very limited time any more for such stuff (these AT91 patches will
> go through him first then Russell) so I'm not sure any added delays
> there could hurt much.

Right. I was just mentioning it as one option.

> >   * I (or someone else) set up a new git tree for tclib + AT91 and
> > AVR32 platform support and ask Stephen to pull it into the -next
> > tree. Or, once it's stable, rmk and I can both pull it into our
> > respective trees. But that obviously means no rebasing and funny
> > games from that point on...
> >
> > I think the last option is the best one. What do you think?
> 
> In effect, I've had that last option as a series of quilt patches,
> and posting these two is the first step in that merge sequence...
> heck, they could merge right now into 2.6.25-rc3 and that would let
> the two arch series merge at their own paces.  (AVR32 direct from
> you, AT91 very indirectly through Andrew then Russell.)

Yes, getting the fundamental stuff into mainline now would help a lot.
But I was thinking about Linus' suggestions that we exploit the
distributed nature of git and do cross-tree merges to synchronize
changes to common code.

Setting up a separate git tree would allow the changes to go into the
arm tree without littering it with possibly unstable avr32 changes as
well, and it would allow me to merge them and put more stuff on top.
And if the patches are ready for mainline, there should be no need to
fix up the history of the "tclib" tree later, so it should be perfectly
safe to merge into several trees (assuming those trees don't rebase and
make them appear as different commits.)

> My personal priority is to get these patches into the merge queue(s)
> so that they're no longer sitting on my disks and so that when I
> want those platforms to have HRT and/or NO_HZ, it's already there. 

Which goes nicely along with my priority of reducing the overall power
consumption on avr32 :)

> > > +/* we "know" that there will be either one or two TC blocks */
> > > +static struct platform_device *blocks[2];
> >
> > You seem to know more about future Atmel chips than I do :-P
> 
> I only "know" about currently announced ones.  ;)

Yeah, I didn't actually check how many TC blocks are planned on other
chips. I just want this to be a bit future-proof.

> > I'm not a huge fan of such static limitations.
> 
> Me either, but at this point doing more than that seems to me
> to land in that "overengineering" category

Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-24 Thread David Brownell
On Sun, 24 Feb 2008 18:45:54 +0100
Haavard Skinnemoen <[EMAIL PROTECTED]> wrote:
>
> On Fri, 22 Feb 2008 17:23:23 -0800
> David Brownell <[EMAIL PROTECTED]> wrote:
>
> > Create  based on  and the
> > at91sam9263 and at32ap7000 datasheets.  Most AT91 and AT32 SOCs have one
> > or two of these TC blocks, which include three 16-bit timers that can be
> > interconnected in various ways.
> > 
> > These TC blocks can be used for external interfacing (such as PWM and
> > measurement), or used as somewhat quirky sixteen-bit timers.
> > 
> > Signed-off-by: David Brownell <[EMAIL PROTECTED]>
> > ---
>
> Sorry for the delay...some late comments coming up.

Happens.  :)


> > Note that this won't be usable until the AT91 and AT32 platforms
> > incorporate patches to configure the relevant platform devices.
> > Those changes are probably 2.6.26 material.

More specifically (and all those patches are available now):

 - AT91 needs clocksource/clockevent support for the SAM9 PIT timer;
 - AVR32 needs more extensive clocksource/clockevent updates;
 - Both also need platform device setup;

Merging these two patches before those is a perfectly safe NOP.


> Right...and there are a few funny dependencies we need to watch out
> for. AVR32 currently uses one of the TC blocks as a system timer. That
> code needs to be ripped out, but that will cause a fourfold increase in
> the power consumption of the AP7000 CPU.

Because the AVR32 count/compare clocksource (architectural registers)
precludes using the "idle" state in the idle loop ... it counts CPU
cycles, which don't happen in the "idle" state.


>   So I want to keep the window
> between ripping out the old code and using the new code as small as
> possible.

Right.


> I see the following possibilities:
>   * I switch avr32 over to use the new code after it has been merged
> into mainline. This means the new code won't be tested on avr32
> until near the end of the next merge window -- not good.

Well, "more widely" tested.  We've both observed it to work; basically
the same code has worked on AT91 for about a year now, too.

And that's why I suggest merging these parts, now that they're known to
work on both architectures.  The arch-specific bits can follow at their
own pace, neither one blocking the other.


>   * I forward the patches that switch avr32 over to Andrew so that they
> can be included in -mm right after the tclib support. Not a bad
> option, but I'm planning to do more AVR32 PM work before 2.6.26, so
> there might be conflicts.

Conflicts there would be the easy part to deal with.  They're all
specific to AVR32, and you can merge those in any convenient order.


>   * I take the whole thing through my avr32 git tree.

Doesn't work as smoothly for the AT91 side ... but Andrew Victor has
very limited time any more for such stuff (these AT91 patches will
go through him first then Russell) so I'm not sure any added delays
there could hurt much.


>   * I (or someone else) set up a new git tree for tclib + AT91 and
> AVR32 platform support and ask Stephen to pull it into the -next
> tree. Or, once it's stable, rmk and I can both pull it into our
> respective trees. But that obviously means no rebasing and funny
> games from that point on...
>
> I think the last option is the best one. What do you think?

In effect, I've had that last option as a series of quilt patches,
and posting these two is the first step in that merge sequence...
heck, they could merge right now into 2.6.25-rc3 and that would let
the two arch series merge at their own paces.  (AVR32 direct from
you, AT91 very indirectly through Andrew then Russell.)


My personal priority is to get these patches into the merge queue(s)
so that they're no longer sitting on my disks and so that when I
want those platforms to have HRT and/or NO_HZ, it's already there. 


> > +#if defined(CONFIG_AVR32)
> > +/* AVR32 has these divide PBB */
> > +const u8 atmel_tc_divisors[5] = { 0, 4, 8, 16, 32, };
> > +EXPORT_SYMBOL(atmel_tc_divisors);
> > +
> > +#elif defined(CONFIG_ARCH_AT91)
> > +/* AT91 has these divide MCK */
> > +const u8 atmel_tc_divisors[5] = { 2, 8, 32, 128, 0, };
> > +EXPORT_SYMBOL(atmel_tc_divisors);
> > +
> > +#endif
> > +
> > +/* we "know" that there will be either one or two TC blocks */
> > +static struct platform_device *blocks[2];
>
> You seem to know more about future Atmel chips than I do :-P

I only "know" about currently announced ones.  ;)


> I'm not a huge fan of such static limitations.

Me either, but at this point doing more than that seems to me
to land in that "overengineering" category...


> > +/**
> > + * atmel_tc_alloc - allocate a specified TC block
> > + * @block: which block to allocate
> > + * @iomem: used to return its IO memory resource
> > + *
> > + * Caller allocates a block.  If it is available, its I/O space is 
> > requested
> > + * and returned through the iomem pointer, and the device node for the 
> > block
> > + * is returne

Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-24 Thread Haavard Skinnemoen
On Fri, 22 Feb 2008 17:23:23 -0800
David Brownell <[EMAIL PROTECTED]> wrote:

> Create  based on  and the
> at91sam9263 and at32ap7000 datasheets.  Most AT91 and AT32 SOCs have one
> or two of these TC blocks, which include three 16-bit timers that can be
> interconnected in various ways.
> 
> These TC blocks can be used for external interfacing (such as PWM and
> measurement), or used as somewhat quirky sixteen-bit timers.
> 
> Signed-off-by: David Brownell <[EMAIL PROTECTED]>
> ---

Sorry for the delay...some late comments coming up.

> Note that this won't be usable until the AT91 and AT32 platforms
> incorporate patches to configure the relevant platform devices.
> Those changes are probably 2.6.26 material.

Right...and there are a few funny dependencies we need to watch out
for. AVR32 currently uses one of the TC blocks as a system timer. That
code needs to be ripped out, but that will cause a fourfold increase in
the power consumption of the AP7000 CPU. So I want to keep the window
between ripping out the old code and using the new code as small as
possible.

I see the following possibilities:
  * I switch avr32 over to use the new code after it has been merged
into mainline. This means the new code won't be tested on avr32
until near the end of the next merge window -- not good.
  * I forward the patches that switch avr32 over to Andrew so that they
can be included in -mm right after the tclib support. Not a bad
option, but I'm planning to do more AVR32 PM work before 2.6.26, so
there might be conflicts.
  * I take the whole thing through my avr32 git tree.
  * I (or someone else) set up a new git tree for tclib + AT91 and
AVR32 platform support and ask Stephen to pull it into the -next
tree. Or, once it's stable, rmk and I can both pull it into our
respective trees. But that obviously means no rebasing and funny
games from that point on...

I think the last option is the best one. What do you think?

> +#if defined(CONFIG_AVR32)
> +/* AVR32 has these divide PBB */
> +const u8 atmel_tc_divisors[5] = { 0, 4, 8, 16, 32, };
> +EXPORT_SYMBOL(atmel_tc_divisors);
> +
> +#elif defined(CONFIG_ARCH_AT91)
> +/* AT91 has these divide MCK */
> +const u8 atmel_tc_divisors[5] = { 2, 8, 32, 128, 0, };
> +EXPORT_SYMBOL(atmel_tc_divisors);
> +
> +#endif
> +
> +/* we "know" that there will be either one or two TC blocks */
> +static struct platform_device *blocks[2];

You seem to know more about future Atmel chips than I do :-P

I'm not a huge fan of such static limitations. Is there any way we can
turn it into a list? That probably means defining a struct atmel_tc
around each platform_device, but that might be nice to have for other
reasons too.

> +/**
> + * atmel_tc_alloc - allocate a specified TC block
> + * @block: which block to allocate
> + * @iomem: used to return its IO memory resource
> + *
> + * Caller allocates a block.  If it is available, its I/O space is requested
> + * and returned through the iomem pointer, and the device node for the block
> + * is returned.  When it is not available, NULL is returned.

Any reason why it can't ioremap() the registers as well? Most drivers
probably want that.

> + * On some platforms, each TC channel has its own clocks and IRQs.  Drivers
> + * should clk_get() and clk_enable() "t0_clk", "t1_clk, and "t2_clk".
> + * In the same vein, they should platform_get_irq() for irqs 0, 1, and 2.
> + * On other platforms, only irq 0 and "t0_clk" will be available; drivers
> + * should handle with both configurations.

Sounds complicated. I think it would be better if tclib did all the
clk_get() calls and stored each clock into the atmel_tc struct so that
the driver can simply clk_enable() each channel (which may point to the
same clock.)

> +struct platform_device *atmel_tc_alloc(unsigned block, struct resource 
> **iomem)
> +{
> + struct platform_device  *tc;
> + struct resource *r;
> +
> + if (block >= ARRAY_SIZE(blocks) || !iomem)
> + return NULL;
> +
> + tc = blocks[block];
> + if (tc) {
> + r = platform_get_resource(tc, IORESOURCE_MEM, 0);
> + if (r)
> + r = request_mem_region(r->start, 256, NULL);

Shouldn't you use the real resource size instead of some magic number?

> +static struct platform_driver tc_driver = {
> + .driver.name= "atmel_tcb",

Wow, I didn't know that was possible...

> +#define ATMEL_TC_BMR 0xc4/* TC Block Mode Register */
> +#define ATMEL_TC_TC0XC0S (3 << 0)/* external clock 0 source */
> +#defineATMEL_TC_TC0XC0S_TCLK0(0 << 0)

Hmm. Indentation using spaces? I didn't know you were into that sort of
thing ;-)

Anyway, my main issue is that I think we should add a data structure
with information about each device, similar to struct ssc_device in the
atmel-ssc driver. How about something like this?

struct atmel_tc_block {
void __iomem*regs;  /* non-NULL when busy */
  

[patch 2.6.25-rc2-git 1/2] atmel_tc library

2008-02-22 Thread David Brownell
Create  based on  and the
at91sam9263 and at32ap7000 datasheets.  Most AT91 and AT32 SOCs have one
or two of these TC blocks, which include three 16-bit timers that can be
interconnected in various ways.

These TC blocks can be used for external interfacing (such as PWM and
measurement), or used as somewhat quirky sixteen-bit timers.

Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
Note that this won't be usable until the AT91 and AT32 platforms
incorporate patches to configure the relevant platform devices.
Those changes are probably 2.6.26 material.

 drivers/misc/Kconfig   |8 +
 drivers/misc/Makefile  |1 
 drivers/misc/atmel_tclib.c |  107 +
 include/linux/atmel_tc.h   |  221 +
 4 files changed, 337 insertions(+)

--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -22,6 +22,14 @@ config ATMEL_PWM
  purposes including software controlled power-efficent backlights
  on LCD displays, motor control, and waveform generation.
 
+config ATMEL_TCLIB
+   bool "Atmel AT32/AT91 Timer/Counter Library"
+   depends on (AVR32 || ARCH_AT91)
+   help
+ Select this if you want a library to allocate the Timer/Counter
+ blocks found on many Atmel processors.  This facilitates using
+ these blocks by different drivers despite processor differences.
+
 config IBM_ASM
tristate "Device driver for IBM RSA service processor"
depends on X86 && PCI && INPUT && EXPERIMENTAL
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_ACER_WMI) += acer-wmi.o
 obj-$(CONFIG_ASUS_LAPTOP) += asus-laptop.o
 obj-$(CONFIG_ATMEL_PWM)+= atmel_pwm.o
 obj-$(CONFIG_ATMEL_SSC)+= atmel-ssc.o
+obj-$(CONFIG_ATMEL_TCLIB)  += atmel_tclib.o
 obj-$(CONFIG_TC1100_WMI)   += tc1100-wmi.o
 obj-$(CONFIG_LKDTM)+= lkdtm.o
 obj-$(CONFIG_TIFM_CORE)+= tifm_core.o
--- /dev/null
+++ b/drivers/misc/atmel_tclib.c
@@ -0,0 +1,107 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+/*
+ * This is a thin library to solve the problem of how to portably allocate
+ * one of the TC blocks.  For simplicity, it doesn't currently expect to
+ * share individual timers between different drivers.
+ */
+
+#if defined(CONFIG_AVR32)
+/* AVR32 has these divide PBB */
+const u8 atmel_tc_divisors[5] = { 0, 4, 8, 16, 32, };
+EXPORT_SYMBOL(atmel_tc_divisors);
+
+#elif defined(CONFIG_ARCH_AT91)
+/* AT91 has these divide MCK */
+const u8 atmel_tc_divisors[5] = { 2, 8, 32, 128, 0, };
+EXPORT_SYMBOL(atmel_tc_divisors);
+
+#endif
+
+/* we "know" that there will be either one or two TC blocks */
+static struct platform_device *blocks[2];
+
+
+/**
+ * atmel_tc_alloc - allocate a specified TC block
+ * @block: which block to allocate
+ * @iomem: used to return its IO memory resource
+ *
+ * Caller allocates a block.  If it is available, its I/O space is requested
+ * and returned through the iomem pointer, and the device node for the block
+ * is returned.  When it is not available, NULL is returned.
+ *
+ * On some platforms, each TC channel has its own clocks and IRQs.  Drivers
+ * should clk_get() and clk_enable() "t0_clk", "t1_clk, and "t2_clk".
+ * In the same vein, they should platform_get_irq() for irqs 0, 1, and 2.
+ * On other platforms, only irq 0 and "t0_clk" will be available; drivers
+ * should handle with both configurations.
+ */
+struct platform_device *atmel_tc_alloc(unsigned block, struct resource **iomem)
+{
+   struct platform_device  *tc;
+   struct resource *r;
+
+   if (block >= ARRAY_SIZE(blocks) || !iomem)
+   return NULL;
+
+   tc = blocks[block];
+   if (tc) {
+   r = platform_get_resource(tc, IORESOURCE_MEM, 0);
+   if (r)
+   r = request_mem_region(r->start, 256, NULL);
+   *iomem = r;
+   if (!r)
+   tc = NULL;
+   }
+
+   return tc;
+}
+EXPORT_SYMBOL_GPL(atmel_tc_alloc);
+
+/**
+ * atmel_tc_free - release a specified TC block
+ * @block: which block to release
+ *
+ * This reverses the effect of atmel_tc_alloc(), invalidating the resource
+ * returned by that routine and making the TC available to other drivers.
+ */
+void atmel_tc_free(struct platform_device *tc)
+{
+   if (tc->id >= 0 && tc->id < ARRAY_SIZE(blocks)) {
+   struct resource *r;
+
+   r = platform_get_resource(tc, IORESOURCE_MEM, 0);
+   release_mem_region(r->start, 256);
+   }
+}
+EXPORT_SYMBOL_GPL(atmel_tc_free);
+
+static int __init tc_probe(struct platform_device *pdev)
+{
+   static char __initdata e2big[] =
+   KERN_ERR "tclib: can't record TC block %d\n";
+
+   if (pdev->id < 0 || pdev->id >= ARRAY_SIZE(blocks)) {
+   printk(e2big, pdev->id);
+   return -ENFILE;
+   }
+   blocks[pdev->id