LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Brownell <david-b@pacbell.net>
To: hskinnemoen@atmel.com
Cc: nicolas.ferre@rfo.atmel.com, linux@maxim.org.za,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org
Subject: Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library
Date: Sun, 24 Feb 2008 17:03:10 -0800	[thread overview]
Message-ID: <20080225010310.B59E98E4B8@adsl-69-226-248-13.dsl.pltn13.pacbell.net> (raw)
In-Reply-To: <20080225012637.1535c95b@siona>

> > > > 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 */
> > > > +#define        ATMEL_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_head	node;
> > > };
> > 
> > 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
>

  reply	other threads:[~2008-02-25  1:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-23  1:23 David Brownell
2008-02-24 17:45 ` Haavard Skinnemoen
2008-02-24 22:55   ` David Brownell
2008-02-25  0:26     ` Haavard Skinnemoen
2008-02-25  1:03       ` David Brownell [this message]
2008-02-25 11:43         ` Haavard Skinnemoen
2008-02-25 18:06           ` David Brownell
2008-02-26  9:08             ` Haavard Skinnemoen
2008-02-26  9:17               ` David Brownell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080225010310.B59E98E4B8@adsl-69-226-248-13.dsl.pltn13.pacbell.net \
    --to=david-b@pacbell.net \
    --cc=akpm@linux-foundation.org \
    --cc=hskinnemoen@atmel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@maxim.org.za \
    --cc=nicolas.ferre@rfo.atmel.com \
    --subject='Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).