Re: [patch 2.6.25-rc2-git 2/2] atmel_tc clocksource/clockevent code
On Mon, 25 Feb 2008 09:51:16 -0800
David Brownell <[EMAIL PROTECTED]> wrote:
> > > > > +static cycle_t tc_get_cycles(void)
> > > > > +{
> > > > > + unsigned long flags;
> > > > > + u32 lower, upper;
> > > > > +
> > > > > + raw_local_irq_save(flags);
> > > >
> > > > Why do you need to use the raw version?
> > >
> > > This is part of the system timer code, and it should never be a
> > > preemption point. Plus I didn't want to waste any instruction
> > > cycles in code that runs before e.g. the DBGU IRQ handler would
> > > get called... observably, such extra cycles *do* hurt.
> >
> > I don't understand what you mean by preemption point, but I guess the
> > non-raw version may consume some extra cycles when lockdep is enabled.
>
> A preemption point is where CONFIG_PREEMPT kicks in task switching
> logic; lockdep is different.
I know, but I dont' see how local_irq_save/restore has anything to do
with it, raw or not. There would be absolutely no point checking for
preemption on local_irq_restore() since no one would have been able to
set the TIF_NEED_RESCHED flag while interrupts were disabled...
raw_local_irq_save/restore is only different from
local_irq_save/restore when lockdep is enabled. That's why I don't
understand why you're talking about preemption.
> > If we really expect using TC as a clocksource but not as a clockevent
> > is going to be a common usage, perhaps we should move the decision into
> > Kconfig?
>
> Maybe, but I already spent lots more time on this than I wanted. :(
I'm not asking you to do it. I'm asking if it would be a good thing to
do. I said that I can take these patches off your back if you want, but
I want to make sure I don't do anything with them that you disagree
with.
> Another way to address that rm9200 issue would be to just rate
> the TC clockevent source lower than the one based on the system
> timer, so it's set up but never enabled ... and remember "t2_clk",
> calling clk_enable() only when that clockevent device is active.
>
> That would address one half of the suspend/resume equation too,
> ensuring that clock is disabled during suspend...
Yes, we could probably do that. Which means we can just remove all the
ifdeffery?
> The other half being: how to clk_disable(t0_clk) during system
> suspend? (And t1_clk on some systems.) There's currently no
> clocksource.set_mode() call; evidently there's an assumption that
> such counters cost no power, so they can always be left running.
Yes...that would be a clocksource core issue I guess. Better leave that
for later...
> > > > I don't think it is safe to assume that one clock per channel always
> > > > means one irq per channel as well...
> > >
> > > On current chips, that's how it works.
> >
> > Indeed. I just don't see any fundamental reason why it has to work that
> > way, which is why I don't think we should depend on it.
>
> AT91 chips share identifiers between clocks and interrupts; that's
> fundamental, yes?
>
> If some future chip works differently, that's a good time to change
> things. Otherwise I see little point in caring about such issues.
Agreed.
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
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
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 include/asm-avr32/arch-at32ap/time.h, 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 2/2] atmel_tc clocksource/clockevent code
On Mon, 25 Feb 2008 09:51:16 -0800
David Brownell [EMAIL PROTECTED] wrote:
+static cycle_t tc_get_cycles(void)
+{
+ unsigned long flags;
+ u32 lower, upper;
+
+ raw_local_irq_save(flags);
Why do you need to use the raw version?
This is part of the system timer code, and it should never be a
preemption point. Plus I didn't want to waste any instruction
cycles in code that runs before e.g. the DBGU IRQ handler would
get called... observably, such extra cycles *do* hurt.
I don't understand what you mean by preemption point, but I guess the
non-raw version may consume some extra cycles when lockdep is enabled.
A preemption point is where CONFIG_PREEMPT kicks in task switching
logic; lockdep is different.
I know, but I dont' see how local_irq_save/restore has anything to do
with it, raw or not. There would be absolutely no point checking for
preemption on local_irq_restore() since no one would have been able to
set the TIF_NEED_RESCHED flag while interrupts were disabled...
raw_local_irq_save/restore is only different from
local_irq_save/restore when lockdep is enabled. That's why I don't
understand why you're talking about preemption.
If we really expect using TC as a clocksource but not as a clockevent
is going to be a common usage, perhaps we should move the decision into
Kconfig?
Maybe, but I already spent lots more time on this than I wanted. :(
I'm not asking you to do it. I'm asking if it would be a good thing to
do. I said that I can take these patches off your back if you want, but
I want to make sure I don't do anything with them that you disagree
with.
Another way to address that rm9200 issue would be to just rate
the TC clockevent source lower than the one based on the system
timer, so it's set up but never enabled ... and remember t2_clk,
calling clk_enable() only when that clockevent device is active.
That would address one half of the suspend/resume equation too,
ensuring that clock is disabled during suspend...
Yes, we could probably do that. Which means we can just remove all the
ifdeffery?
The other half being: how to clk_disable(t0_clk) during system
suspend? (And t1_clk on some systems.) There's currently no
clocksource.set_mode() call; evidently there's an assumption that
such counters cost no power, so they can always be left running.
Yes...that would be a clocksource core issue I guess. Better leave that
for later...
I don't think it is safe to assume that one clock per channel always
means one irq per channel as well...
On current chips, that's how it works.
Indeed. I just don't see any fundamental reason why it has to work that
way, which is why I don't think we should depend on it.
AT91 chips share identifiers between clocks and interrupts; that's
fundamental, yes?
If some future chip works differently, that's a good time to change
things. Otherwise I see little point in caring about such issues.
Agreed.
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
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 2/2] atmel_tc clocksource/clockevent code
On Sun, 24 Feb 2008 15:42:51 -0800
David Brownell <[EMAIL PROTECTED]> wrote:
> > On Fri, 22 Feb 2008 17:28:37 -0800
> > David Brownell <[EMAIL PROTECTED]> wrote:
> >
> > > +static cycle_t tc_get_cycles(void)
> > > +{
> > > + unsigned long flags;
> > > + u32 lower, upper;
> > > +
> > > + raw_local_irq_save(flags);
> >
> > Why do you need to use the raw version?
>
> This is part of the system timer code, and it should never be a
> preemption point. Plus I didn't want to waste any instruction
> cycles in code that runs before e.g. the DBGU IRQ handler would
> get called... observably, such extra cycles *do* hurt.
I don't understand what you mean by preemption point, but I guess the
non-raw version may consume some extra cycles when lockdep is enabled.
> > > +#ifdef CONFIG_GENERIC_CLOCKEVENTS
> > > +#define USE_TC_CLKEVT
> > > +#endif
> > > +
> > > +#ifdef CONFIG_ARCH_AT91RM9200
> > > +/* The AT91rm9200 system timer is a oneshot-capable 32k timer that's
> > > + * always running ... configuring a TC channel to work the same way
> > > + * would just waste some power.
> > > + */
> > > +#undef USE_TC_CLKEVT
> > > +#endif
> > > +
> > > +#ifdef USE_TC_CLKEVT
> >
> > Can't you just use #ifndef CONFIG_ARCH_AT91RM9200 and avoid the whole
> > ifdef/define/undef dance above?
>
> I can't know that some other platform won't have a better system
> timer solution, and didn't want to assume that only that single
> venerable chip had such a solution ... it's kind of puzzling to
> me (software guy) that none of the newest Atmel SOCs have better
> timer infrastructure than they do. ;)
Heh.
If we really expect using TC as a clocksource but not as a clockevent
is going to be a common usage, perhaps we should move the decision into
Kconfig?
Besides, I don't really see the difference between having a big #ifdef
expression around the whole clockevent block and having a big #ifdef
expression around the definition of USE_TC_CLKEVT except that the
latter is more code...
> > > + case CLOCK_EVT_MODE_ONESHOT:
> > > + /* slow clock, count up to RC, then irq and stop */
> >
> > Hmm. Do you really want to stop it? Won't you get better accuracy if
> > you let it run and program the next event as (prev_event + delta)?
>
> No, ONESHOT means "stop after the IRQ I asked for". And when
> tc_next_event() is asked to trigger that IRQ, it's relative to
> the instant it's requested, not relative to the last one that
> was requested (which may not have triggered yet, or may have
> been quite some time ago).
Right. Sounds like it might introduce some inaccuracy, but I guess it's
the clocksource's job to keep track of the actual time at the time of
the event.
> > > +static struct irqaction tc_irqaction = {
> > > + .name = "tc_clkevt",
> > > + .flags = IRQF_TIMER | IRQF_DISABLED,
> > > + .handler= ch2_irq,
> > > +};
> >
> > I don't think you need to define this statically. You can call
> > request_irq() at arch_initcall() time.
>
> That could be done, yes ... and using request_irq()/free_irq()
> would also let this be linked as a module, not just statically.
>
> On the other hand, doing it this way doesn't hurt either does it?
> Unless a modular build is important.
No, it doesn't hurt. Maybe we should keep it static so that we have the
option of initializing it earlier if we need to.
> > I don't think it is safe to assume that one clock per channel always
> > means one irq per channel as well...
>
> On current chips, that's how it works.
Indeed. I just don't see any fundamental reason why it has to work that
way, which is why I don't think we should depend on it.
> > What's wrong with
> >
> > irq = platform_get_irq(pdev, 2);
> > if (irq < 0)
> > irq = platform_get_irq(pdev, 0);
>
> Nothing much, except that I took the more concise path. Got patch?
Not until we reach a conclusion about the tclib core.
> > How about we make tcb_clksrc_init() global and call it from the
> > platform code with whatever TCB the platform thinks is appropriate?
>
> That could work too, but if it goes that way then there's no
> point in changes to support a modular build (e.g. the irqaction
> thing you noted above).
True.
> > Perhaps remove the prompt from ATMEL_TCB_CLKSRC and have the platform
> > select it as well? I certainly want to force this stuff on on the
> > AP7000...otherwise we'll just get lots of complaints that the thing is
> > using 4x more power than it's supposed to...
>
> Well, "force" seems the wrong approach to me. That should be a
> board-specific choice. The ap700x power budget may not be the
> most important system design consideration, so making such a
> decision at the platform ("ap700x") level seems wrong.
>
> Suppose someone has to build an AVR32 based system that uses both
> TCB modules to hook up to external hardware, so that neither one
> is available for use as a "system timer"?
Good point. Let's keep it as it is and let the board "select" it
Re: [patch 2.6.25-rc2-git 2/2] atmel_tc clocksource/clockevent code
On Sun, 24 Feb 2008 15:42:51 -0800
David Brownell [EMAIL PROTECTED] wrote:
On Fri, 22 Feb 2008 17:28:37 -0800
David Brownell [EMAIL PROTECTED] wrote:
+static cycle_t tc_get_cycles(void)
+{
+ unsigned long flags;
+ u32 lower, upper;
+
+ raw_local_irq_save(flags);
Why do you need to use the raw version?
This is part of the system timer code, and it should never be a
preemption point. Plus I didn't want to waste any instruction
cycles in code that runs before e.g. the DBGU IRQ handler would
get called... observably, such extra cycles *do* hurt.
I don't understand what you mean by preemption point, but I guess the
non-raw version may consume some extra cycles when lockdep is enabled.
+#ifdef CONFIG_GENERIC_CLOCKEVENTS
+#define USE_TC_CLKEVT
+#endif
+
+#ifdef CONFIG_ARCH_AT91RM9200
+/* The AT91rm9200 system timer is a oneshot-capable 32k timer that's
+ * always running ... configuring a TC channel to work the same way
+ * would just waste some power.
+ */
+#undef USE_TC_CLKEVT
+#endif
+
+#ifdef USE_TC_CLKEVT
Can't you just use #ifndef CONFIG_ARCH_AT91RM9200 and avoid the whole
ifdef/define/undef dance above?
I can't know that some other platform won't have a better system
timer solution, and didn't want to assume that only that single
venerable chip had such a solution ... it's kind of puzzling to
me (software guy) that none of the newest Atmel SOCs have better
timer infrastructure than they do. ;)
Heh.
If we really expect using TC as a clocksource but not as a clockevent
is going to be a common usage, perhaps we should move the decision into
Kconfig?
Besides, I don't really see the difference between having a big #ifdef
expression around the whole clockevent block and having a big #ifdef
expression around the definition of USE_TC_CLKEVT except that the
latter is more code...
+ case CLOCK_EVT_MODE_ONESHOT:
+ /* slow clock, count up to RC, then irq and stop */
Hmm. Do you really want to stop it? Won't you get better accuracy if
you let it run and program the next event as (prev_event + delta)?
No, ONESHOT means stop after the IRQ I asked for. And when
tc_next_event() is asked to trigger that IRQ, it's relative to
the instant it's requested, not relative to the last one that
was requested (which may not have triggered yet, or may have
been quite some time ago).
Right. Sounds like it might introduce some inaccuracy, but I guess it's
the clocksource's job to keep track of the actual time at the time of
the event.
+static struct irqaction tc_irqaction = {
+ .name = tc_clkevt,
+ .flags = IRQF_TIMER | IRQF_DISABLED,
+ .handler= ch2_irq,
+};
I don't think you need to define this statically. You can call
request_irq() at arch_initcall() time.
That could be done, yes ... and using request_irq()/free_irq()
would also let this be linked as a module, not just statically.
On the other hand, doing it this way doesn't hurt either does it?
Unless a modular build is important.
No, it doesn't hurt. Maybe we should keep it static so that we have the
option of initializing it earlier if we need to.
I don't think it is safe to assume that one clock per channel always
means one irq per channel as well...
On current chips, that's how it works.
Indeed. I just don't see any fundamental reason why it has to work that
way, which is why I don't think we should depend on it.
What's wrong with
irq = platform_get_irq(pdev, 2);
if (irq 0)
irq = platform_get_irq(pdev, 0);
Nothing much, except that I took the more concise path. Got patch?
Not until we reach a conclusion about the tclib core.
How about we make tcb_clksrc_init() global and call it from the
platform code with whatever TCB the platform thinks is appropriate?
That could work too, but if it goes that way then there's no
point in changes to support a modular build (e.g. the irqaction
thing you noted above).
True.
Perhaps remove the prompt from ATMEL_TCB_CLKSRC and have the platform
select it as well? I certainly want to force this stuff on on the
AP7000...otherwise we'll just get lots of complaints that the thing is
using 4x more power than it's supposed to...
Well, force seems the wrong approach to me. That should be a
board-specific choice. The ap700x power budget may not be the
most important system design consideration, so making such a
decision at the platform (ap700x) level seems wrong.
Suppose someone has to build an AVR32 based system that uses both
TCB modules to hook up to external hardware, so that neither one
is available for use as a system timer?
Good point. Let's keep it as it is and let the board select it
through its defconfig.
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
Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library
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
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 >
Re: [patch 2.6.25-rc2-git 2/2] atmel_tc clocksource/clockevent code
Again, sorry for the delay...it really sucks that I haven't been able
to look at this stuff closely until now. Hopefully a late review is
better than no review.
On Fri, 22 Feb 2008 17:28:37 -0800
David Brownell <[EMAIL PROTECTED]> wrote:
> +static cycle_t tc_get_cycles(void)
> +{
> + unsigned long flags;
> + u32 lower, upper;
> +
> + raw_local_irq_save(flags);
Why do you need to use the raw version?
> +retry:
> + upper = __raw_readl(tcaddr + ATMEL_TC_REG(1, CV));
> + lower = __raw_readl(tcaddr + ATMEL_TC_REG(0, CV));
> + if (upper != __raw_readl(tcaddr + ATMEL_TC_REG(1, CV)))
> + goto retry;
Did you just open-code a do/while loop using goto? ;-)
> +#ifdef CONFIG_GENERIC_CLOCKEVENTS
> +#define USE_TC_CLKEVT
> +#endif
> +
> +#ifdef CONFIG_ARCH_AT91RM9200
> +/* The AT91rm9200 system timer is a oneshot-capable 32k timer that's
> + * always running ... configuring a TC channel to work the same way
> + * would just waste some power.
> + */
> +#undef USE_TC_CLKEVT
> +#endif
> +
> +#ifdef USE_TC_CLKEVT
Can't you just use #ifndef CONFIG_ARCH_AT91RM9200 and avoid the whole
ifdef/define/undef dance above?
> + case CLOCK_EVT_MODE_ONESHOT:
> + /* slow clock, count up to RC, then irq and stop */
Hmm. Do you really want to stop it? Won't you get better accuracy if
you let it run and program the next event as (prev_event + delta)?
> +static struct irqaction tc_irqaction = {
> + .name = "tc_clkevt",
> + .flags = IRQF_TIMER | IRQF_DISABLED,
> + .handler= ch2_irq,
> +};
I don't think you need to define this statically. You can call
request_irq() at arch_initcall() time.
> +static void __init setup_clkevents(struct platform_device *pdev,
> + struct clk *t0_clk, int clk32k_divisor_idx)
> +{
> + struct clk *t2_clk = clk_get(>dev, "t2_clk");
> + int irq;
> +
> + /* SOCs have either one IRQ and one clock for the whole
> + * TC block; or one each per channel. We use channel 2.
> + */
> + if (IS_ERR(t2_clk)) {
> + t2_clk = t0_clk;
> + irq = platform_get_irq(pdev, 0);
> + } else {
> + irq = platform_get_irq(pdev, 2);
> + clk_enable(t2_clk);
> + }
I don't think it is safe to assume that one clock per channel always
means one irq per channel as well...
What's wrong with
irq = platform_get_irq(pdev, 2);
if (irq < 0)
irq = platform_get_irq(pdev, 0);
?
> +config ATMEL_TCB_CLKSRC
> + bool "TC Block Clocksource"
> + depends on ATMEL_TCLIB && GENERIC_TIME
> + default y
> + help
> + Select this to get a high precision clocksource based on a
> + TC block with a 5+ MHz base clock rate. Two timer channels
> + are combined to make a single 32-bit timer.
> +
> + When GENERIC_CLOCKEVENTS is defined, the third timer channel
> + may be used as a clock event device supporting oneshot mode
> + (delays of up to two seconds) based on the 32 KiHz clock.
> +
> +config ATMEL_TCB_CLKSRC_BLOCK
> + int
> + depends on ATMEL_TCB_CLKSRC
> + prompt "TC Block" if ARCH_AT91RM9200 || ARCH_AT91SAM9260 ||
> CPU_AT32AP700X
> + default 0
> + range 0 1
Hmm...I don't like the direction this is going...
How about we make tcb_clksrc_init() global and call it from the
platform code with whatever TCB the platform thinks is appropriate?
Perhaps remove the prompt from ATMEL_TCB_CLKSRC and have the platform
select it as well? I certainly want to force this stuff on on the
AP7000...otherwise we'll just get lots of complaints that the thing is
using 4x more power than it's supposed to...
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
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 */
Re: [patch 2.6.25-rc2-git 1/2] atmel_tc library
On Fri, 22 Feb 2008 17:23:23 -0800
David Brownell [EMAIL PROTECTED] wrote:
Create linux/atmel_tc.h based on asm-arm/arch-at91/at91-tc.h 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 */
struct platform_device *pdev;
Re: [patch 2.6.25-rc2-git 2/2] atmel_tc clocksource/clockevent code
Again, sorry for the delay...it really sucks that I haven't been able
to look at this stuff closely until now. Hopefully a late review is
better than no review.
On Fri, 22 Feb 2008 17:28:37 -0800
David Brownell [EMAIL PROTECTED] wrote:
+static cycle_t tc_get_cycles(void)
+{
+ unsigned long flags;
+ u32 lower, upper;
+
+ raw_local_irq_save(flags);
Why do you need to use the raw version?
+retry:
+ upper = __raw_readl(tcaddr + ATMEL_TC_REG(1, CV));
+ lower = __raw_readl(tcaddr + ATMEL_TC_REG(0, CV));
+ if (upper != __raw_readl(tcaddr + ATMEL_TC_REG(1, CV)))
+ goto retry;
Did you just open-code a do/while loop using goto? ;-)
+#ifdef CONFIG_GENERIC_CLOCKEVENTS
+#define USE_TC_CLKEVT
+#endif
+
+#ifdef CONFIG_ARCH_AT91RM9200
+/* The AT91rm9200 system timer is a oneshot-capable 32k timer that's
+ * always running ... configuring a TC channel to work the same way
+ * would just waste some power.
+ */
+#undef USE_TC_CLKEVT
+#endif
+
+#ifdef USE_TC_CLKEVT
Can't you just use #ifndef CONFIG_ARCH_AT91RM9200 and avoid the whole
ifdef/define/undef dance above?
+ case CLOCK_EVT_MODE_ONESHOT:
+ /* slow clock, count up to RC, then irq and stop */
Hmm. Do you really want to stop it? Won't you get better accuracy if
you let it run and program the next event as (prev_event + delta)?
+static struct irqaction tc_irqaction = {
+ .name = tc_clkevt,
+ .flags = IRQF_TIMER | IRQF_DISABLED,
+ .handler= ch2_irq,
+};
I don't think you need to define this statically. You can call
request_irq() at arch_initcall() time.
+static void __init setup_clkevents(struct platform_device *pdev,
+ struct clk *t0_clk, int clk32k_divisor_idx)
+{
+ struct clk *t2_clk = clk_get(pdev-dev, t2_clk);
+ int irq;
+
+ /* SOCs have either one IRQ and one clock for the whole
+ * TC block; or one each per channel. We use channel 2.
+ */
+ if (IS_ERR(t2_clk)) {
+ t2_clk = t0_clk;
+ irq = platform_get_irq(pdev, 0);
+ } else {
+ irq = platform_get_irq(pdev, 2);
+ clk_enable(t2_clk);
+ }
I don't think it is safe to assume that one clock per channel always
means one irq per channel as well...
What's wrong with
irq = platform_get_irq(pdev, 2);
if (irq 0)
irq = platform_get_irq(pdev, 0);
?
+config ATMEL_TCB_CLKSRC
+ bool TC Block Clocksource
+ depends on ATMEL_TCLIB GENERIC_TIME
+ default y
+ help
+ Select this to get a high precision clocksource based on a
+ TC block with a 5+ MHz base clock rate. Two timer channels
+ are combined to make a single 32-bit timer.
+
+ When GENERIC_CLOCKEVENTS is defined, the third timer channel
+ may be used as a clock event device supporting oneshot mode
+ (delays of up to two seconds) based on the 32 KiHz clock.
+
+config ATMEL_TCB_CLKSRC_BLOCK
+ int
+ depends on ATMEL_TCB_CLKSRC
+ prompt TC Block if ARCH_AT91RM9200 || ARCH_AT91SAM9260 ||
CPU_AT32AP700X
+ default 0
+ range 0 1
Hmm...I don't like the direction this is going...
How about we make tcb_clksrc_init() global and call it from the
platform code with whatever TCB the platform thinks is appropriate?
Perhaps remove the prompt from ATMEL_TCB_CLKSRC and have the platform
select it as well? I certainly want to force this stuff on on the
AP7000...otherwise we'll just get lots of complaints that the thing is
using 4x more power than it's supposed to...
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
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... I don't entirely agree. Most drivers I've dealt with can handle an arbitrary number of devices. I don't see
Re: [PATCH] atmel_spi: Fix clock polarity
On Sat, 23 Feb 2008 00:05:07 -0800 Andrew Morton <[EMAIL PROTECTED]> wrote: > I had it queued for 2.6.26 which I guess was wrong. I'll bump it into > 2.6.25. Thanks! > Is it needed in 2.6.24.x? I think so. The last time that code was changed was before 2.6.23 AFAICT, so perhaps 2.6.23.x as well. I'll have a closer look and pass it on to the stable team when it hits mainline. 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] macb: Fix speed setting
On Sat, 23 Feb 2008 00:03:23 -0800 Andrew Morton <[EMAIL PROTECTED]> wrote: > On Thu, 21 Feb 2008 17:41:05 +0100 Haavard Skinnemoen <[EMAIL PROTECTED]> > wrote: > > > From: Atsushi Nemoto <[EMAIL PROTECTED]> > > > > Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by > > conversion to generic PHY layer in kernel 2.6.23. > > So.. shouldn't we fix it in 2.6.23.x and 2.6.24.x? Absolutely. I'll send it to the stable team after it hits mainline. 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] macb: Fix speed setting
On Sat, 23 Feb 2008 00:03:23 -0800 Andrew Morton [EMAIL PROTECTED] wrote: On Thu, 21 Feb 2008 17:41:05 +0100 Haavard Skinnemoen [EMAIL PROTECTED] wrote: From: Atsushi Nemoto [EMAIL PROTECTED] Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by conversion to generic PHY layer in kernel 2.6.23. So.. shouldn't we fix it in 2.6.23.x and 2.6.24.x? Absolutely. I'll send it to the stable team after it hits mainline. 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] atmel_spi: Fix clock polarity
On Sat, 23 Feb 2008 00:05:07 -0800 Andrew Morton [EMAIL PROTECTED] wrote: I had it queued for 2.6.26 which I guess was wrong. I'll bump it into 2.6.25. Thanks! Is it needed in 2.6.24.x? I think so. The last time that code was changed was before 2.6.23 AFAICT, so perhaps 2.6.23.x as well. I'll have a closer look and pass it on to the stable team when it hits mainline. 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] macb: Fix speed setting
On Thu, 21 Feb 2008 17:41:05 +0100 Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > From: Atsushi Nemoto <[EMAIL PROTECTED]> > > Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by > conversion to generic PHY layer in kernel 2.6.23. > > Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]> > Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]> > --- Btw, in case it wasn't clear, I want this to go into your "fixes" queue and merged before 2.6.25 since it fixes a bug that prevents the driver from working on 10Mbps networks. I'll send the patch to the stable team after it hits mainline. Thanks! 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] macb: Fix speed setting
On Thu, 21 Feb 2008 17:41:05 +0100 Haavard Skinnemoen [EMAIL PROTECTED] wrote: From: Atsushi Nemoto [EMAIL PROTECTED] Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by conversion to generic PHY layer in kernel 2.6.23. Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED] Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED] --- Btw, in case it wasn't clear, I want this to go into your fixes queue and merged before 2.6.25 since it fixes a bug that prevents the driver from working on 10Mbps networks. I'll send the patch to the stable team after it hits mainline. Thanks! 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: MMC card detection
On Thu, 21 Feb 2008 19:46:20 +0100 Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > In order to fix this problem, I think I need a way to tell the MMC core > that the card really is gone and that there's no point trying to > communicate with it. Is there any way I can do that? Never mind, I figured it out. I can simply fail the command right away with -ENOMEDIUM if I know the card isn't present. Seems to work like a charm. 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/
MMC card detection
Hi Pierre, I've been trying to debug some card detection problems in the atmel-mci driver. Sometimes, when I remove a card, the event doesn't seem to get detected properly, and the MMC core thinks the card is still there. When I re-insert the card, the MMC core thinks the card is gone. I've tried to add a debouncing timer to avoid glitches on the card detect pin. This helps a bit, but it does not eliminate the problem altogether. It seems like the problem only occurs if the card is removed very slowly. If I increase the debouncing or detection delay, I have to remove the card more slowly to trigger the problem. I think the real problem is that the card detection interrupt triggers while there is still electrical contact with the card. So when the MMC core tries to send a SEND_STATUS command to check if it's still there, the card will respond even though it's about to be removed. Since there will be no more interrupts as the card is completely removed, the MMC core will never notice that the card is gone. When the card is reinserted, the MMC core will try to send a SEND_STATUS command again. This time, the card won't respond because it is in the "idle" state, and the MMC core will think the card is gone. In order to fix this problem, I think I need a way to tell the MMC core that the card really is gone and that there's no point trying to communicate with it. Is there any way I can do that? 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/
[PATCH] atmel_serial: Fix interrupt handler return value
We should only return IRQ_HANDLED when we actually found something to handle. This is important since the USART interrupt handler may be shared with the timer interrupt on some chips. Pointed-out-by: michael <[EMAIL PROTECTED]> Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]> --- drivers/serial/atmel_serial.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c index fad245b..d57bf3e 100644 --- a/drivers/serial/atmel_serial.c +++ b/drivers/serial/atmel_serial.c @@ -549,7 +549,7 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id) atmel_handle_transmit(port, pending); } while (pass_counter++ < ATMEL_ISR_PASS_LIMIT); - return IRQ_HANDLED; + return pass_counter ? IRQ_HANDLED : IRQ_NONE; } /* -- 1.5.4.1 -- 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/
[PATCH] macb: Fix speed setting
From: Atsushi Nemoto <[EMAIL PROTECTED]> Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by conversion to generic PHY layer in kernel 2.6.23. Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]> Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]> --- drivers/net/macb.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 81bf005..1d210ed 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -148,7 +148,7 @@ static void macb_handle_link_change(struct net_device *dev) if (phydev->duplex) reg |= MACB_BIT(FD); - if (phydev->speed) + if (phydev->speed == SPEED_100) reg |= MACB_BIT(SPD); macb_writel(bp, NCFGR, reg); -- 1.5.4.1 -- 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] macb: Fix speed setting
On Fri, 22 Feb 2008 00:07:31 +0900 (JST) Atsushi Nemoto <[EMAIL PROTECTED]> wrote: > > I'm willing to take your word for it, but some documentation would be > > really nice... > > Well, simple grepping drivers/net/phy enlighten us ;) Yeah, the patch certainly looks correct as far as I can tell. > Anyway, this patch fixes problem with 10Mbps on AT91SAM9260 board. Thanks. I'll pass it 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] macb: Fix speed setting
On Thu, 21 Feb 2008 22:50:54 +0900 (JST) Atsushi Nemoto <[EMAIL PROTECTED]> wrote: > Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by > conversion to generic PHY layer in kernel 2.6.23. > > Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]> > --- > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > index 81bf005..1d210ed 100644 > --- a/drivers/net/macb.c > +++ b/drivers/net/macb.c > @@ -148,7 +148,7 @@ static void macb_handle_link_change(struct net_device > *dev) > > if (phydev->duplex) > reg |= MACB_BIT(FD); > - if (phydev->speed) > + if (phydev->speed == SPEED_100) I have to admit that even after looking through include/linux/phy.h and include/linux/mii.h, I don't have the faintest idea what values we can expect to find in the "speed" field of phydev. The comment above struct phy_device says that it is used "like in mii_if_info", but struct mii_if_info doesn't have a "speed" field... I'm willing to take your word for it, but some documentation would be really nice... 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] macb: Fix speed setting
On Thu, 21 Feb 2008 22:50:54 +0900 (JST) Atsushi Nemoto [EMAIL PROTECTED] wrote: Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by conversion to generic PHY layer in kernel 2.6.23. Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED] --- diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 81bf005..1d210ed 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -148,7 +148,7 @@ static void macb_handle_link_change(struct net_device *dev) if (phydev-duplex) reg |= MACB_BIT(FD); - if (phydev-speed) + if (phydev-speed == SPEED_100) I have to admit that even after looking through include/linux/phy.h and include/linux/mii.h, I don't have the faintest idea what values we can expect to find in the speed field of phydev. The comment above struct phy_device says that it is used like in mii_if_info, but struct mii_if_info doesn't have a speed field... I'm willing to take your word for it, but some documentation would be really nice... 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/
[PATCH] atmel_serial: Fix interrupt handler return value
We should only return IRQ_HANDLED when we actually found something to handle. This is important since the USART interrupt handler may be shared with the timer interrupt on some chips. Pointed-out-by: michael [EMAIL PROTECTED] Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED] --- drivers/serial/atmel_serial.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c index fad245b..d57bf3e 100644 --- a/drivers/serial/atmel_serial.c +++ b/drivers/serial/atmel_serial.c @@ -549,7 +549,7 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id) atmel_handle_transmit(port, pending); } while (pass_counter++ ATMEL_ISR_PASS_LIMIT); - return IRQ_HANDLED; + return pass_counter ? IRQ_HANDLED : IRQ_NONE; } /* -- 1.5.4.1 -- 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] macb: Fix speed setting
On Fri, 22 Feb 2008 00:07:31 +0900 (JST) Atsushi Nemoto [EMAIL PROTECTED] wrote: I'm willing to take your word for it, but some documentation would be really nice... Well, simple grepping drivers/net/phy enlighten us ;) Yeah, the patch certainly looks correct as far as I can tell. Anyway, this patch fixes problem with 10Mbps on AT91SAM9260 board. Thanks. I'll pass it 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/
[PATCH] macb: Fix speed setting
From: Atsushi Nemoto [EMAIL PROTECTED] Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by conversion to generic PHY layer in kernel 2.6.23. Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED] Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED] --- drivers/net/macb.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 81bf005..1d210ed 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -148,7 +148,7 @@ static void macb_handle_link_change(struct net_device *dev) if (phydev-duplex) reg |= MACB_BIT(FD); - if (phydev-speed) + if (phydev-speed == SPEED_100) reg |= MACB_BIT(SPD); macb_writel(bp, NCFGR, reg); -- 1.5.4.1 -- 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/
MMC card detection
Hi Pierre, I've been trying to debug some card detection problems in the atmel-mci driver. Sometimes, when I remove a card, the event doesn't seem to get detected properly, and the MMC core thinks the card is still there. When I re-insert the card, the MMC core thinks the card is gone. I've tried to add a debouncing timer to avoid glitches on the card detect pin. This helps a bit, but it does not eliminate the problem altogether. It seems like the problem only occurs if the card is removed very slowly. If I increase the debouncing or detection delay, I have to remove the card more slowly to trigger the problem. I think the real problem is that the card detection interrupt triggers while there is still electrical contact with the card. So when the MMC core tries to send a SEND_STATUS command to check if it's still there, the card will respond even though it's about to be removed. Since there will be no more interrupts as the card is completely removed, the MMC core will never notice that the card is gone. When the card is reinserted, the MMC core will try to send a SEND_STATUS command again. This time, the card won't respond because it is in the idle state, and the MMC core will think the card is gone. In order to fix this problem, I think I need a way to tell the MMC core that the card really is gone and that there's no point trying to communicate with it. Is there any way I can do that? 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: MMC card detection
On Thu, 21 Feb 2008 19:46:20 +0100 Haavard Skinnemoen [EMAIL PROTECTED] wrote: In order to fix this problem, I think I need a way to tell the MMC core that the card really is gone and that there's no point trying to communicate with it. Is there any way I can do that? Never mind, I figured it out. I can simply fail the command right away with -ENOMEDIUM if I know the card isn't present. Seems to work like a charm. 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: atmel_spi clock polarity
On Wed, 20 Feb 2008 14:21:09 +0900 (JST) Atsushi Nemoto <[EMAIL PROTECTED]> wrote: > On Mon, 18 Feb 2008 15:31:58 +0100, Haavard Skinnemoen <[EMAIL PROTECTED]> > wrote: > > > Anyway, I will try your patch in a few days. > > > > Ok, thanks. If it works, that would be great, but given your > > description above I'm not sure if I dare hope for it. > > Unfortunately it did not work. The clock state did not change by > writing MR register. Ok, thanks for testing. In that case, I think the best fix is to let NPCS0 stay selected permanently in MR and overwrite CSR0 with to the new slave's settings before asserting CS. But that's a more complicated change, and I don't know how it will affect the AT91RM9200 special cases. So I suggest we merge your patch for 2.6.25, and try to optimize it for 2.6.26. David, do you want me to pass on the patch with my signoff or just ask Andrew to add my Acked-by to the patch already in mm? 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: atmel_spi clock polarity
On Wed, 20 Feb 2008 14:21:09 +0900 (JST) Atsushi Nemoto [EMAIL PROTECTED] wrote: On Mon, 18 Feb 2008 15:31:58 +0100, Haavard Skinnemoen [EMAIL PROTECTED] wrote: Anyway, I will try your patch in a few days. Ok, thanks. If it works, that would be great, but given your description above I'm not sure if I dare hope for it. Unfortunately it did not work. The clock state did not change by writing MR register. Ok, thanks for testing. In that case, I think the best fix is to let NPCS0 stay selected permanently in MR and overwrite CSR0 with to the new slave's settings before asserting CS. But that's a more complicated change, and I don't know how it will affect the AT91RM9200 special cases. So I suggest we merge your patch for 2.6.25, and try to optimize it for 2.6.26. David, do you want me to pass on the patch with my signoff or just ask Andrew to add my Acked-by to the patch already in mm? 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: [spi-devel-general] atmel_spi clock polarity
On Mon, 18 Feb 2008 11:57:56 -0800 David Brownell <[EMAIL PROTECTED]> wrote: > On Monday 18 February 2008, Atsushi Nemoto wrote: > > IIRC the clock state follows > > CSRn.CPOL just before the real transfer. > > No ... clock state should be valid *before* chipselect goes > active. So I'm thinking the patch from Haavard is likely > the right change. Hopefully it is... > > CLK __|~|___|~~~|___|~~~|___|~~~|___ > > ... and at T1 CPOL is changed?? That's wrong. There should > never be a partial clock period while a chipselect is active. > While it's inactive, sure -- no chip should care. ...but what I'm afraid of is that since we're using GPIO chipselects, the controller may _think_ that no chip is selected and change the clock polarity just before it would pull the chipselect low if we were using automatic chipselects... If that's the case, it would be a bit strange if it actually helped to initialize all the CSRn registers, but it would also offer us a nice workaround... Atsushi, do you by any chance have debugging enabled? That would explain the long delay from CS activation to the change of clock polarity. Compared to printk() the DMA setup takes almost no time at all. If you can confirm that my patch helps, I think that's the one we want in mainline. 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: atmel_spi clock polarity
On Mon, 18 Feb 2008 23:12:43 +0900 (JST) Atsushi Nemoto <[EMAIL PROTECTED]> wrote: > T0-T1 was relatively longer then T1-T2. I suppose T1 is not the > point of updating MR register, but the point of starting DMA transfer. Aw. I see. > Anyway, I will try your patch in a few days. Ok, thanks. If it works, that would be great, but given your description above I'm not sure if I dare hope for it. Hmm...I suppose we could just use CSR0 for all transfers and not bother updating MR. That might actually be cheaper than doing it "the right way"...and allow us to support an arbitrary number of chipselects instead of just four. But I guess the AT91RM9200 won't be too happy about that... 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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Fri, 15 Feb 2008 09:12:35 -0800 "Nelson, Shannon" <[EMAIL PROTECTED]> wrote: > I'll jump in here briefly - I'm okay with the direction this is going, > but I want to be protective of ioatdma performance. As used in struct > ioat_desc_sw, the cookie and ack elements end up very close to the end > of a cache line and I'd like them to not get pushed out across the > boundry. I don't think this proposal changes the layout, I'm just > bringing up my concern. Sure, performance is very important, and it's good to see that you're critical about the changes I'm proposing. That said, the memory layout doesn't change at all with this patch -- the fields that didn't go into the generic dma descriptor were at the end of the struct to begin with. I can add a comment saying that cookie and ack must always come first. Any other fields that we need to be careful about? 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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Sat, 16 Feb 2008 13:06:54 -0700
"Dan Williams" <[EMAIL PROTECTED]> wrote:
> I like the direction of the patch, i.e. splitting out separate
> functionality into separate structs. However, I do not want to break
> the model of clients sourcing the operations and drivers sinking them
> which dma_slave_descriptor appears to do. How about adding a
> scatterlist pointer and an 'unmap_type' to the common descriptor?
> Where unmap_type selects between, page, single, sg, or no-unmap.
> Drivers already know the length and direction.
But there are currently no operations available for submitting
scatterlists as a single descriptor -- the client iterates over the
scatterlist and submits one descriptor for each entry. So there's no
way you can associated a scatterlist with a single descriptor. You can
perhaps attach it to the last one, but that may get you into trouble if
the transfer is terminated early for some reason.
I don't think the pure source/sink model is very realistic -- clients
can't just submit DMA transfers and then forget about them. They must
at the very least check the status and take appropriate action if there
was an error. They must also notify the driver that the descriptor can
be reused (although I guess if a client doesn't care about the result
it can do this immediately after submission, but I really think clients
_should_ care about the result.)
And since they need to do some sort of cleanup anyway, they might as
well unmap the buffers (or call some dma_memcpy_finish() type of
function that does it for them.)
The clients certainly know the length and direction too, but they don't
necessarily know the physical address since the mapping is done by a
"middle layer". I guess that's the main problem with the model I'm
proposing.
How about we add a kind of "address cookie" struct like this (feel free
to suggest better names):
struct dma_buf_addr {
void *cpu_addr;
dma_addr_t dma_addr;
};
The client initializes the cpu_addr part and passes the struct to
dma_async_memcpy_buf_to_buf() or whatever, the middle layer sets the
dma_addr after mapping the buffer (or page), and the client passes the
same struct to dma_finish_memcpy_buf_to_buf(). Which will then be able
to unmap both buffers appropriately.
This will also eliminate the hack in crypto/async_tx/async_xor.c and
make HIGHMEM64G work again.
Scatterlists currently don't have any middle-layer support, so we can
ignore them for now and let the client take the full responsibility of
mapping and unmapping the buffers. If we were to add some middle-layer
functions for dealing with scatterlists, I don't think they would need
any special treatment -- the dma address is kept in the struct
scatterlist array passed by the client, so it would work pretty much
the same way without any special treatment.
Alternatively, we could convert the whole API to use scatterlists. But
that's probably overdoing it.
Btw, this discussion is a bit off-topic for the patch in question, but
it's an issue that needs to be resolved.
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: atmel_spi clock polarity
On Sat, 16 Feb 2008 22:32:52 +0900 (JST) Atsushi Nemoto <[EMAIL PROTECTED]> wrote: > Here is my quick workaround for this problem. It makes all CSRn.CPOL > match for the transfer before activating chipselect. I'm not quite > sure my analysis is correct, and there might be better solution. > Could you give me any comments? I'm not sure if I fully understand what problem you're seeing. Is the clock state wrong when the chip select is activated? If so, does the patch below help? Haavard diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c index 293b7ca..4f19b82 100644 --- a/drivers/spi/atmel_spi.c +++ b/drivers/spi/atmel_spi.c @@ -90,6 +90,7 @@ static void cs_activate(struct atmel_spi *as, struct spi_device *spi) mr = spi_readl(as, MR); mr = SPI_BFINS(PCS, ~(1 << spi->chip_select), mr); + spi_writel(as, MR, mr); dev_dbg(>dev, "activate %u%s, mr %08x\n", gpio, active ? " (high)" : "", @@ -97,7 +98,6 @@ static void cs_activate(struct atmel_spi *as, struct spi_device *spi) if (!(cpu_is_at91rm9200() && spi->chip_select == 0)) gpio_set_value(gpio, active); - spi_writel(as, MR, mr); } static void cs_deactivate(struct atmel_spi *as, struct spi_device *spi) -- 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: atmel_spi clock polarity
On Sat, 16 Feb 2008 22:32:52 +0900 (JST) Atsushi Nemoto [EMAIL PROTECTED] wrote: Here is my quick workaround for this problem. It makes all CSRn.CPOL match for the transfer before activating chipselect. I'm not quite sure my analysis is correct, and there might be better solution. Could you give me any comments? I'm not sure if I fully understand what problem you're seeing. Is the clock state wrong when the chip select is activated? If so, does the patch below help? Haavard diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c index 293b7ca..4f19b82 100644 --- a/drivers/spi/atmel_spi.c +++ b/drivers/spi/atmel_spi.c @@ -90,6 +90,7 @@ static void cs_activate(struct atmel_spi *as, struct spi_device *spi) mr = spi_readl(as, MR); mr = SPI_BFINS(PCS, ~(1 spi-chip_select), mr); + spi_writel(as, MR, mr); dev_dbg(spi-dev, activate %u%s, mr %08x\n, gpio, active ? (high) : , @@ -97,7 +98,6 @@ static void cs_activate(struct atmel_spi *as, struct spi_device *spi) if (!(cpu_is_at91rm9200() spi-chip_select == 0)) gpio_set_value(gpio, active); - spi_writel(as, MR, mr); } static void cs_deactivate(struct atmel_spi *as, struct spi_device *spi) -- 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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Sat, 16 Feb 2008 13:06:54 -0700
Dan Williams [EMAIL PROTECTED] wrote:
I like the direction of the patch, i.e. splitting out separate
functionality into separate structs. However, I do not want to break
the model of clients sourcing the operations and drivers sinking them
which dma_slave_descriptor appears to do. How about adding a
scatterlist pointer and an 'unmap_type' to the common descriptor?
Where unmap_type selects between, page, single, sg, or no-unmap.
Drivers already know the length and direction.
But there are currently no operations available for submitting
scatterlists as a single descriptor -- the client iterates over the
scatterlist and submits one descriptor for each entry. So there's no
way you can associated a scatterlist with a single descriptor. You can
perhaps attach it to the last one, but that may get you into trouble if
the transfer is terminated early for some reason.
I don't think the pure source/sink model is very realistic -- clients
can't just submit DMA transfers and then forget about them. They must
at the very least check the status and take appropriate action if there
was an error. They must also notify the driver that the descriptor can
be reused (although I guess if a client doesn't care about the result
it can do this immediately after submission, but I really think clients
_should_ care about the result.)
And since they need to do some sort of cleanup anyway, they might as
well unmap the buffers (or call some dma_memcpy_finish() type of
function that does it for them.)
The clients certainly know the length and direction too, but they don't
necessarily know the physical address since the mapping is done by a
middle layer. I guess that's the main problem with the model I'm
proposing.
How about we add a kind of address cookie struct like this (feel free
to suggest better names):
struct dma_buf_addr {
void *cpu_addr;
dma_addr_t dma_addr;
};
The client initializes the cpu_addr part and passes the struct to
dma_async_memcpy_buf_to_buf() or whatever, the middle layer sets the
dma_addr after mapping the buffer (or page), and the client passes the
same struct to dma_finish_memcpy_buf_to_buf(). Which will then be able
to unmap both buffers appropriately.
This will also eliminate the hack in crypto/async_tx/async_xor.c and
make HIGHMEM64G work again.
Scatterlists currently don't have any middle-layer support, so we can
ignore them for now and let the client take the full responsibility of
mapping and unmapping the buffers. If we were to add some middle-layer
functions for dealing with scatterlists, I don't think they would need
any special treatment -- the dma address is kept in the struct
scatterlist array passed by the client, so it would work pretty much
the same way without any special treatment.
Alternatively, we could convert the whole API to use scatterlists. But
that's probably overdoing it.
Btw, this discussion is a bit off-topic for the patch in question, but
it's an issue that needs to be resolved.
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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Fri, 15 Feb 2008 09:12:35 -0800 Nelson, Shannon [EMAIL PROTECTED] wrote: I'll jump in here briefly - I'm okay with the direction this is going, but I want to be protective of ioatdma performance. As used in struct ioat_desc_sw, the cookie and ack elements end up very close to the end of a cache line and I'd like them to not get pushed out across the boundry. I don't think this proposal changes the layout, I'm just bringing up my concern. Sure, performance is very important, and it's good to see that you're critical about the changes I'm proposing. That said, the memory layout doesn't change at all with this patch -- the fields that didn't go into the generic dma descriptor were at the end of the struct to begin with. I can add a comment saying that cookie and ack must always come first. Any other fields that we need to be careful about? 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: atmel_spi clock polarity
On Mon, 18 Feb 2008 23:12:43 +0900 (JST) Atsushi Nemoto [EMAIL PROTECTED] wrote: T0-T1 was relatively longer then T1-T2. I suppose T1 is not the point of updating MR register, but the point of starting DMA transfer. Aw. I see. Anyway, I will try your patch in a few days. Ok, thanks. If it works, that would be great, but given your description above I'm not sure if I dare hope for it. Hmm...I suppose we could just use CSR0 for all transfers and not bother updating MR. That might actually be cheaper than doing it the right way...and allow us to support an arbitrary number of chipselects instead of just four. But I guess the AT91RM9200 won't be too happy about that... 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: [spi-devel-general] atmel_spi clock polarity
On Mon, 18 Feb 2008 11:57:56 -0800 David Brownell [EMAIL PROTECTED] wrote: On Monday 18 February 2008, Atsushi Nemoto wrote: IIRC the clock state follows CSRn.CPOL just before the real transfer. No ... clock state should be valid *before* chipselect goes active. So I'm thinking the patch from Haavard is likely the right change. Hopefully it is... CLK __|~|___|~~~|___|~~~|___|~~~|___ ... and at T1 CPOL is changed?? That's wrong. There should never be a partial clock period while a chipselect is active. While it's inactive, sure -- no chip should care. ...but what I'm afraid of is that since we're using GPIO chipselects, the controller may _think_ that no chip is selected and change the clock polarity just before it would pull the chipselect low if we were using automatic chipselects... If that's the case, it would be a bit strange if it actually helped to initialize all the CSRn registers, but it would also offer us a nice workaround... Atsushi, do you by any chance have debugging enabled? That would explain the long delay from CS activation to the change of clock polarity. Compared to printk() the DMA setup takes almost no time at all. If you can confirm that my patch helps, I think that's the one we want in mainline. 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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Wed, 13 Feb 2008 20:24:02 +0100
Haavard Skinnemoen <[EMAIL PROTECTED]> wrote:
> But looking at your latest patch series, I guess we can use the new
> "next" field instead. It's not like we really need the full
> capabilities of list_head.
On second thought, if we do this, we would be using the "next" field in
an undocumented way. It is currently documented as follows:
* @next: at completion submit this descriptor
But that's not how we're going to use it when doing slave transfers:
We're using it to keep track of all the descriptors that have already
been submitted.
I think it would actually be better to go the other way and have the
async_tx API extend the descriptor as well for its private fields. That
way, we get the pointer we need, but we can document it differently.
So how about we do something along the lines of the patch below? We
need to update all the users too of course, but apart from making the
dma_slave_descriptor struct smaller, I don't think it will change the
actual memory layout at all.
Haavard
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9bb3407..abaf324 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -267,8 +267,7 @@ struct dma_client {
typedef void (*dma_async_tx_callback)(void *dma_async_param);
/**
- * struct dma_async_tx_descriptor - async transaction descriptor
- * ---dma generic offload fields---
+ * struct dma_descriptor - generic dma offload descriptor
* @cookie: tracking cookie for this transaction, set to -EBUSY if
* this tx is sitting on a dependency list
* @ack: the descriptor can not be reused until the client acknowledges
@@ -280,12 +279,8 @@ typedef void (*dma_async_tx_callback)(void
*dma_async_param);
* @tx_submit: set the prepared descriptor(s) to be executed by the engine
* @callback: routine to call after this operation is complete
* @callback_param: general parameter to pass to the callback routine
- * ---async_tx api specific fields---
- * @next: at completion submit this descriptor
- * @parent: pointer to the next level up in the dependency chain
- * @lock: protect the parent and next pointers
*/
-struct dma_async_tx_descriptor {
+struct dma_descriptor {
dma_cookie_t cookie;
int ack;
dma_addr_t phys;
@@ -294,6 +289,17 @@ struct dma_async_tx_descriptor {
dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
dma_async_tx_callback callback;
void *callback_param;
+};
+
+/**
+ * struct dma_async_tx_descriptor - async transaction descriptor
+ * @dma: generic dma descriptor
+ * @next: at completion submit this descriptor
+ * @parent: pointer to the next level up in the dependency chain
+ * @lock: protect the parent and next pointers
+ */
+struct dma_async_tx_descriptor {
+ struct dma_descriptor dma;
struct dma_async_tx_descriptor *next;
struct dma_async_tx_descriptor *parent;
spinlock_t lock;
@@ -301,13 +307,13 @@ struct dma_async_tx_descriptor {
/**
* struct dma_slave_descriptor - extended DMA descriptor for slave DMA
- * @async_tx: async transaction descriptor
- * @client_node: for use by the client, for example when operating on
- * scatterlists.
+ * @dma: generic dma descriptor
+ * @next: for use by the client, for example to keep track of
+ * submitted descriptors when dealing with scatterlists.
*/
struct dma_slave_descriptor {
- struct dma_async_tx_descriptor txd;
- struct list_head client_node;
+ struct dma_descriptor dma;
+ struct dma_slave_descriptor *next;
};
/**
--
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 0/4] async_tx: fix dependency handling and related cleanups
On Wed, 13 Feb 2008 00:02:52 -0700 Dan Williams <[EMAIL PROTECTED]> wrote: > Dan Williams (4): > iop-adma: remove the workaround for missed interrupts on iop3xx > async_tx: kill ->device_dependency_added > async_tx: fix multiple dependency submission > async_tx: checkpatch says s/__FUNCTION__/__func__/g All of the above changes are fine with me. I'll rebase my dmaslave patches on top of them; that will probably allow me to get rid of the dma_slave_descriptor struct. 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 0/4] async_tx: fix dependency handling and related cleanups
On Wed, 13 Feb 2008 00:02:52 -0700 Dan Williams [EMAIL PROTECTED] wrote: Dan Williams (4): iop-adma: remove the workaround for missed interrupts on iop3xx async_tx: kill -device_dependency_added async_tx: fix multiple dependency submission async_tx: checkpatch says s/__FUNCTION__/__func__/g All of the above changes are fine with me. I'll rebase my dmaslave patches on top of them; that will probably allow me to get rid of the dma_slave_descriptor struct. 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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Wed, 13 Feb 2008 20:24:02 +0100
Haavard Skinnemoen [EMAIL PROTECTED] wrote:
But looking at your latest patch series, I guess we can use the new
next field instead. It's not like we really need the full
capabilities of list_head.
On second thought, if we do this, we would be using the next field in
an undocumented way. It is currently documented as follows:
* @next: at completion submit this descriptor
But that's not how we're going to use it when doing slave transfers:
We're using it to keep track of all the descriptors that have already
been submitted.
I think it would actually be better to go the other way and have the
async_tx API extend the descriptor as well for its private fields. That
way, we get the pointer we need, but we can document it differently.
So how about we do something along the lines of the patch below? We
need to update all the users too of course, but apart from making the
dma_slave_descriptor struct smaller, I don't think it will change the
actual memory layout at all.
Haavard
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9bb3407..abaf324 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -267,8 +267,7 @@ struct dma_client {
typedef void (*dma_async_tx_callback)(void *dma_async_param);
/**
- * struct dma_async_tx_descriptor - async transaction descriptor
- * ---dma generic offload fields---
+ * struct dma_descriptor - generic dma offload descriptor
* @cookie: tracking cookie for this transaction, set to -EBUSY if
* this tx is sitting on a dependency list
* @ack: the descriptor can not be reused until the client acknowledges
@@ -280,12 +279,8 @@ typedef void (*dma_async_tx_callback)(void
*dma_async_param);
* @tx_submit: set the prepared descriptor(s) to be executed by the engine
* @callback: routine to call after this operation is complete
* @callback_param: general parameter to pass to the callback routine
- * ---async_tx api specific fields---
- * @next: at completion submit this descriptor
- * @parent: pointer to the next level up in the dependency chain
- * @lock: protect the parent and next pointers
*/
-struct dma_async_tx_descriptor {
+struct dma_descriptor {
dma_cookie_t cookie;
int ack;
dma_addr_t phys;
@@ -294,6 +289,17 @@ struct dma_async_tx_descriptor {
dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
dma_async_tx_callback callback;
void *callback_param;
+};
+
+/**
+ * struct dma_async_tx_descriptor - async transaction descriptor
+ * @dma: generic dma descriptor
+ * @next: at completion submit this descriptor
+ * @parent: pointer to the next level up in the dependency chain
+ * @lock: protect the parent and next pointers
+ */
+struct dma_async_tx_descriptor {
+ struct dma_descriptor dma;
struct dma_async_tx_descriptor *next;
struct dma_async_tx_descriptor *parent;
spinlock_t lock;
@@ -301,13 +307,13 @@ struct dma_async_tx_descriptor {
/**
* struct dma_slave_descriptor - extended DMA descriptor for slave DMA
- * @async_tx: async transaction descriptor
- * @client_node: for use by the client, for example when operating on
- * scatterlists.
+ * @dma: generic dma descriptor
+ * @next: for use by the client, for example to keep track of
+ * submitted descriptors when dealing with scatterlists.
*/
struct dma_slave_descriptor {
- struct dma_async_tx_descriptor txd;
- struct list_head client_node;
+ struct dma_descriptor dma;
+ struct dma_slave_descriptor *next;
};
/**
--
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: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Thu, 14 Feb 2008 11:34:03 -0700 "Dan Williams" <[EMAIL PROTECTED]> wrote: > On Thu, Feb 14, 2008 at 1:36 AM, Haavard Skinnemoen > <[EMAIL PROTECTED]> wrote: > > It's ok to use PIO for small and/or odd transfers like "read 2 bytes > > from this SDIO register", something that the mmc-block driver would > > never ask us to do. Using PIO for huge block data transfers will really > > hurt. > > Except your testing has already shown that running out of descriptors > rarely, if ever happens. It just becomes an exercise in tuning the > pool size, and letting this simple fall back mechanism be a safety net > for the corner cases. True...it's just that you normally expect _better_ performance when increasing the size of the transfer. I'm just afraid that someone might start tuning things elsewhere, only to find that crossing a certain threshold gives a drastic reduction in performance. That would be completely counter-intuitive. > > I think I'll try triggering the mmc tasklet from the DMA callback for > > now. Scheduling a tasklet from another tasklet should work just fine, > > right? > > Yeah that should work because you are no longer under the channel lock. Right. And if we insert the interrupt descriptor somewhere in the middle instead of at the end, we should be able to submit a new batch of descriptors before the previous batch is exhausted, thus maintaining maximum throughput without any pauses. I'll probably keep it simple at first though. Gotta save some optimizations for later ;) 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/
MMC core debugfs support (was Re: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers)
[removing lots of people from Cc]
On Wed, 13 Feb 2008 19:30:51 +0100
Pierre Ossman <[EMAIL PROTECTED]> wrote:
> > +static int req_dbg_open(struct inode *inode, struct file *file)
> > +{
>
> And this should go into the core.
I've started working on this, but I've run into a problem: The mmc core
structures don't seem to keep any references to the current request. So
I don't really have any information to put into the 'req' file after
moving it into the core.
Any ideas on how to solve this?
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: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Wed, 13 Feb 2008 16:55:54 -0700 "Dan Williams" <[EMAIL PROTECTED]> wrote: > Well, the other two possibilities are: > > 1/ Spin/sleep until a descriptor shows up Won't work since the transfer hasn't been started yet, so it will spin indefinitely. I guess we could return, send the command and use a waitqueue to wait for the callback. But this will make error handling seriously yucky. > 2/ Fall back to PIO for a few transfers Which means killing performance for large transfers. Not really an option. It's ok to use PIO for small and/or odd transfers like "read 2 bytes from this SDIO register", something that the mmc-block driver would never ask us to do. Using PIO for huge block data transfers will really hurt. > Descriptor availability is improved if the code interleaves allocation > and submission. Currently it looks like we wait until all descriptors > for the scatterlist are allocated before we start submitting. No, none of the descriptors will appear before the command has been sent, the card has responded, and a full block of data has been transferred. I suppose we could send the command earlier, but I don't think it will help a lot and it will complicate error handling. There may be room for improvement though. The current scheme of splitting DMA preparation and submission was initially used because older versions of the controller would instantly fail with an overrun or underrun if the data wasn't available immediately when the command had been sent. Sending the command earlier and doing interleaved allocation and submission might improve performance a bit. I think I'll try triggering the mmc tasklet from the DMA callback for now. Scheduling a tasklet from another tasklet should work just fine, right? 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: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Wed, 13 Feb 2008 16:55:54 -0700 Dan Williams [EMAIL PROTECTED] wrote: Well, the other two possibilities are: 1/ Spin/sleep until a descriptor shows up Won't work since the transfer hasn't been started yet, so it will spin indefinitely. I guess we could return, send the command and use a waitqueue to wait for the callback. But this will make error handling seriously yucky. 2/ Fall back to PIO for a few transfers Which means killing performance for large transfers. Not really an option. It's ok to use PIO for small and/or odd transfers like read 2 bytes from this SDIO register, something that the mmc-block driver would never ask us to do. Using PIO for huge block data transfers will really hurt. Descriptor availability is improved if the code interleaves allocation and submission. Currently it looks like we wait until all descriptors for the scatterlist are allocated before we start submitting. No, none of the descriptors will appear before the command has been sent, the card has responded, and a full block of data has been transferred. I suppose we could send the command earlier, but I don't think it will help a lot and it will complicate error handling. There may be room for improvement though. The current scheme of splitting DMA preparation and submission was initially used because older versions of the controller would instantly fail with an overrun or underrun if the data wasn't available immediately when the command had been sent. Sending the command earlier and doing interleaved allocation and submission might improve performance a bit. I think I'll try triggering the mmc tasklet from the DMA callback for now. Scheduling a tasklet from another tasklet should work just fine, right? 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/
MMC core debugfs support (was Re: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers)
[removing lots of people from Cc]
On Wed, 13 Feb 2008 19:30:51 +0100
Pierre Ossman [EMAIL PROTECTED] wrote:
+static int req_dbg_open(struct inode *inode, struct file *file)
+{
And this should go into the core.
I've started working on this, but I've run into a problem: The mmc core
structures don't seem to keep any references to the current request. So
I don't really have any information to put into the 'req' file after
moving it into the core.
Any ideas on how to solve this?
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: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Thu, 14 Feb 2008 11:34:03 -0700 Dan Williams [EMAIL PROTECTED] wrote: On Thu, Feb 14, 2008 at 1:36 AM, Haavard Skinnemoen [EMAIL PROTECTED] wrote: It's ok to use PIO for small and/or odd transfers like read 2 bytes from this SDIO register, something that the mmc-block driver would never ask us to do. Using PIO for huge block data transfers will really hurt. Except your testing has already shown that running out of descriptors rarely, if ever happens. It just becomes an exercise in tuning the pool size, and letting this simple fall back mechanism be a safety net for the corner cases. True...it's just that you normally expect _better_ performance when increasing the size of the transfer. I'm just afraid that someone might start tuning things elsewhere, only to find that crossing a certain threshold gives a drastic reduction in performance. That would be completely counter-intuitive. I think I'll try triggering the mmc tasklet from the DMA callback for now. Scheduling a tasklet from another tasklet should work just fine, right? Yeah that should work because you are no longer under the channel lock. Right. And if we insert the interrupt descriptor somewhere in the middle instead of at the end, we should be able to submit a new batch of descriptors before the previous batch is exhausted, thus maintaining maximum throughput without any pauses. I'll probably keep it simple at first though. Gotta save some optimizations for later ;) 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: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Wed, 13 Feb 2008 12:11:58 -0700 "Dan Williams" <[EMAIL PROTECTED]> wrote: > > + desc = chan->device->device_prep_slave(chan, > > + sg_dma_address(sg), direction, > > + DMA_SLAVE_WIDTH_32BIT, > > + sg_dma_len(sg), dma_flags); > > + desc->txd.callback = NULL; > > + list_add_tail(>client_node, > > + >dma.data_descs); > > + } > > Need to handle device_prep_slave returning NULL? You're right, we definitely need to handle that. Which probably means we need to prepare an interrupt descriptor first that we can throw in when we're unable to obtain more descriptors, and submit the rest from the callback. Except we're not allowed to submit anything from the callback. Ouch. How can we solve that? Set up a work queue and submit it from there? Trigger a different tasklet? In any case, I guess I need to implement support for interrupt descriptors in the dw_dmac driver. 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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Wed, 13 Feb 2008 12:07:26 -0700
"Dan Williams" <[EMAIL PROTECTED]> wrote:
> > +struct dma_slave_descriptor {
> > + struct dma_async_tx_descriptor txd;
> > + struct list_head client_node;
> > +};
>
> Can you explain a bit why client_node is needed? I do not think we
> need dma_slave_descriptor if dma_unmap data / control is added to
> dma_async_tx_descriptor. Hmm?
Well, it's perhaps not needed for slave transfers as such. But the MMC
driver (and I suspect quite a few other users of the slave interface)
deals with scatterlists, so it needs a way to keep track of all the
descriptors it submits. Hence the list node.
But looking at your latest patch series, I guess we can use the new
"next" field instead. It's not like we really need the full
capabilities of list_head.
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: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Wed, 13 Feb 2008 19:30:51 +0100 Pierre Ossman <[EMAIL PROTECTED]> wrote: > I think this was meant to go away. > And this should go into the core. > I also pointed this out. mmc_remove_host() will synchronize this for > you. Right. Sorry. I focused so much on getting the driver to work correctly that I totally forgot about these things. I'll fix it up before the next round (which I hope can be labeled "PATCH" instead of "RFC".) Btw, this isn't the latest version of the driver...but I'm afraid the latest one suffers from these omissions too, except for the last one which I did actually remove. 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: BUG: 2.6.25-rc1: iptables postrouting setup causes oops
On Wed, 13 Feb 2008 08:42:15 -0800 "Andrew G. Morgan" <[EMAIL PROTECTED]> wrote: > However, it is a warning and, for any existing app that doesn't care > about newly added capabilities, the warning is benign. Ok, I see. Thanks for explaining. 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: Taint kernel after WARN_ON(condition) v2
On Wed, 13 Feb 2008 22:27:40 +0800 Nur Hussein <[EMAIL PROTECTED]> wrote: > This does not work on architectures where WARN_ON has its own definition. > These archs are: > 1. s390 > 2. superh > 3. avr32 > 4. parisc Hmm. Relying on the generic code in lib/bug.c qualifies as "own definition" these days? I think the patch below should take care of all four...unless I've misunderstood something. Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]> diff --git a/lib/bug.c b/lib/bug.c index 530f38f..0d67419 100644 --- a/lib/bug.c +++ b/lib/bug.c @@ -35,6 +35,7 @@ Jeremy Fitzhardinge <[EMAIL PROTECTED]> 2006 */ +#include #include #include #include @@ -149,6 +150,7 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) (void *)bugaddr); show_regs(regs); + add_taint(TAINT_WARN); return BUG_TRAP_TYPE_WARN; } -- 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: Tests of undefined CONFIG variables.
On Wed, 13 Feb 2008 05:54:12 -0500 (EST) "Robert P. J. Day" <[EMAIL PROTECTED]> wrote: > i've also updated the list of what i call "badref" CONFIG variables > -- that is, tests of CONFIG_ variables that appear to be undefined > anywhere in a Kconfig file (which typically represents a meaningless > test, naturally). I've fixed BOARD_ATSTK1002_SW2_CUSTOM, thanks. It was a real bug. As for AP7000_8_BIT_SMC, it is sort-of used. It is part of a choice menu where the other two choices are used like this: #if defined(CONFIG_AP700X_32_BIT_SMC) ... #elif defined(CONFIG_AP700X_16_BIT_SMC) ... #else ... #endif and the #else block covers the only remaining choice, AP700X_8_BIT_SMC. I can fix it by turning the #else above into an #elif I guess. 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: BUG: 2.6.25-rc1: iptables postrouting setup causes oops
On Wed, 13 Feb 2008 10:10:24 +0100
Haavard Skinnemoen <[EMAIL PROTECTED]> wrote:
> > > ip_tables: (C) 2000-2006 Netfilter Core Team
> > > nf_conntrack version 0.5.0 (1024 buckets, 4096 max)
> > > Unable to handle kernel paging request at virtual address d76a7138
> > > ptbr = 91d3b000 pgd = e5f3 pte = 00014370
>
> Hmm. It actually found something in the pte? Looks like a swap
> entry...but that doesn't make sense at that virtual address. Userspace
> is below 0x8000.
(...)
> > If so, the bug could be almost anywhere - in slab, or in some random piece
> > of code which scribbles on slab's data structures.
>
> Yes, it looks like memory corruption, especially since the page table
> appears to be corrupted as well. But I'll have a look and see if the
> code that dumps the pte is doing something bogus...
Yes, that code is indeed buggy. The below patch should fix it, although
the page tables probably won't contain anything interesting, and it
could still be a memory corruption issue. And it definitely won't fix
the real issue.
I have a couple of patches that will eliminate the need for this fixup
(and probably improve performance as well), but they are probably 2.6.26
material.
Haavard
diff --git a/arch/avr32/mm/fault.c b/arch/avr32/mm/fault.c
index 6560cb1..ce4e429 100644
--- a/arch/avr32/mm/fault.c
+++ b/arch/avr32/mm/fault.c
@@ -189,6 +189,8 @@ no_context:
page = sysreg_read(PTBR);
printk(KERN_ALERT "ptbr = %08lx", page);
+ if (address >= TASK_SIZE)
+ page = (unsigned long)swapper_pg_dir;
if (page) {
page = ((unsigned long *)page)[address >> 22];
printk(" pgd = %08lx", page);
--
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: BUG: 2.6.25-rc1: iptables postrouting setup causes oops
On Wed, 13 Feb 2008 00:48:29 -0800 Andrew Morton <[EMAIL PROTECTED]> wrote: > On Tue, 12 Feb 2008 22:46:01 +1100 Ben Nizette <[EMAIL PROTECTED]> wrote: > > > > > On an AVR32, root over NFS, config attached, running (from a startup > > script): > > > > iptables -t nat -A POSTROUTING -o eth0 -j MASQUERADE > > > > Results in (dmesg extract including a bit of context for good measure): > > -8< > > VFS: Mounted root (nfs filesystem). > > Freeing init memory: 72K (9000 - 90012000) > > eth0: no IPv6 routers present > > warning: `dnsmasq' uses 32-bit capabilities (legacy support in use) Hmm. What does that mean? What size do capabilities normally have? > > ip_tables: (C) 2000-2006 Netfilter Core Team > > nf_conntrack version 0.5.0 (1024 buckets, 4096 max) > > Unable to handle kernel paging request at virtual address d76a7138 > > ptbr = 91d3b000 pgd = e5f3 pte = 00014370 Hmm. It actually found something in the pte? Looks like a swap entry...but that doesn't make sense at that virtual address. Userspace is below 0x8000. > > Oops: Kernel access of bad area, sig: 11 [#1] > > FRAME_POINTER chip: 0x01f:0x1e82 rev 2 > > Modules linked in: nf_conntrack_ipv4(+) nf_conntrack ip_tables > > PC is at kmem_cache_alloc+0x2c/0x54 > > LR is at nf_conntrack_l4proto_register+0x34/0x9c [nf_conntrack] > > I take it that the above means that the crash is in kmem_cache_alloc()? That's correct. > If so, the bug could be almost anywhere - in slab, or in some random piece > of code which scribbles on slab's data structures. Yes, it looks like memory corruption, especially since the page table appears to be corrupted as well. But I'll have a look and see if the code that dumps the pte is doing something bogus... > > Perfectly repeatable. > > If my theory is correct, changing pretty much anything in the kernel config > might just make it go away. But still, it would be most valuable if you > could try running a bisection search, as described in > http://www.kernel.org/doc/local/git-quick.html, thanks. Yes, that would be very valuable. 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: [RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users
On Tue, 12 Feb 2008 15:27:29 -0700 "Dan Williams" <[EMAIL PROTECTED]> wrote: > > > Or just let the subsystem always be available. > > > > It used to be always available, but then it was changed. Assuming there > > was a reason for this change, I guess we don't want to change it back. > > > > Adrian had concerns about users enabling NET_DMA when the hardware > capability is relatively rare. So, +1 for HAVE_DMA_DEVICE Why not introduce a new hidden symbol, e.g. DMA_ENGINE_DRIVER, have all the drivers select it and let NET_DMA depend on it? Wait a minute...the Kconfig already does this. What was the problem again? 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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Wed, 13 Feb 2008 00:21:41 -0700
"Dan Williams" <[EMAIL PROTECTED]> wrote:
> On Feb 12, 2008 9:43 AM, Haavard Skinnemoen <[EMAIL PROTECTED]> wrote:
> [..]
> > +enum dma_slave_direction {
> > + DMA_SLAVE_TO_MEMORY,
> > + DMA_SLAVE_FROM_MEMORY,
> > +};
>
> Just reuse enum dma_data_direction from the dma-mapping api.
Hmm...ok. That will add two "directions" that are a bit useless in this
context (DMA_BIDIRECTIONAL and DMA_NONE.) But I guess it's still worth
it since we can pass the direction on unchanged when syncing the
buffers.
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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Wed, 13 Feb 2008 00:21:41 -0700
Dan Williams [EMAIL PROTECTED] wrote:
On Feb 12, 2008 9:43 AM, Haavard Skinnemoen [EMAIL PROTECTED] wrote:
[..]
+enum dma_slave_direction {
+ DMA_SLAVE_TO_MEMORY,
+ DMA_SLAVE_FROM_MEMORY,
+};
Just reuse enum dma_data_direction from the dma-mapping api.
Hmm...ok. That will add two directions that are a bit useless in this
context (DMA_BIDIRECTIONAL and DMA_NONE.) But I guess it's still worth
it since we can pass the direction on unchanged when syncing the
buffers.
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: [RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users
On Tue, 12 Feb 2008 15:27:29 -0700 Dan Williams [EMAIL PROTECTED] wrote: Or just let the subsystem always be available. It used to be always available, but then it was changed. Assuming there was a reason for this change, I guess we don't want to change it back. Adrian had concerns about users enabling NET_DMA when the hardware capability is relatively rare. So, +1 for HAVE_DMA_DEVICE Why not introduce a new hidden symbol, e.g. DMA_ENGINE_DRIVER, have all the drivers select it and let NET_DMA depend on it? Wait a minute...the Kconfig already does this. What was the problem again? 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: BUG: 2.6.25-rc1: iptables postrouting setup causes oops
On Wed, 13 Feb 2008 00:48:29 -0800 Andrew Morton [EMAIL PROTECTED] wrote: On Tue, 12 Feb 2008 22:46:01 +1100 Ben Nizette [EMAIL PROTECTED] wrote: On an AVR32, root over NFS, config attached, running (from a startup script): iptables -t nat -A POSTROUTING -o eth0 -j MASQUERADE Results in (dmesg extract including a bit of context for good measure): -8 VFS: Mounted root (nfs filesystem). Freeing init memory: 72K (9000 - 90012000) eth0: no IPv6 routers present warning: `dnsmasq' uses 32-bit capabilities (legacy support in use) Hmm. What does that mean? What size do capabilities normally have? ip_tables: (C) 2000-2006 Netfilter Core Team nf_conntrack version 0.5.0 (1024 buckets, 4096 max) Unable to handle kernel paging request at virtual address d76a7138 ptbr = 91d3b000 pgd = e5f3 pte = 00014370 Hmm. It actually found something in the pte? Looks like a swap entry...but that doesn't make sense at that virtual address. Userspace is below 0x8000. Oops: Kernel access of bad area, sig: 11 [#1] FRAME_POINTER chip: 0x01f:0x1e82 rev 2 Modules linked in: nf_conntrack_ipv4(+) nf_conntrack ip_tables PC is at kmem_cache_alloc+0x2c/0x54 LR is at nf_conntrack_l4proto_register+0x34/0x9c [nf_conntrack] I take it that the above means that the crash is in kmem_cache_alloc()? That's correct. If so, the bug could be almost anywhere - in slab, or in some random piece of code which scribbles on slab's data structures. Yes, it looks like memory corruption, especially since the page table appears to be corrupted as well. But I'll have a look and see if the code that dumps the pte is doing something bogus... Perfectly repeatable. If my theory is correct, changing pretty much anything in the kernel config might just make it go away. But still, it would be most valuable if you could try running a bisection search, as described in http://www.kernel.org/doc/local/git-quick.html, thanks. Yes, that would be very valuable. 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: BUG: 2.6.25-rc1: iptables postrouting setup causes oops
On Wed, 13 Feb 2008 10:10:24 +0100
Haavard Skinnemoen [EMAIL PROTECTED] wrote:
ip_tables: (C) 2000-2006 Netfilter Core Team
nf_conntrack version 0.5.0 (1024 buckets, 4096 max)
Unable to handle kernel paging request at virtual address d76a7138
ptbr = 91d3b000 pgd = e5f3 pte = 00014370
Hmm. It actually found something in the pte? Looks like a swap
entry...but that doesn't make sense at that virtual address. Userspace
is below 0x8000.
(...)
If so, the bug could be almost anywhere - in slab, or in some random piece
of code which scribbles on slab's data structures.
Yes, it looks like memory corruption, especially since the page table
appears to be corrupted as well. But I'll have a look and see if the
code that dumps the pte is doing something bogus...
Yes, that code is indeed buggy. The below patch should fix it, although
the page tables probably won't contain anything interesting, and it
could still be a memory corruption issue. And it definitely won't fix
the real issue.
I have a couple of patches that will eliminate the need for this fixup
(and probably improve performance as well), but they are probably 2.6.26
material.
Haavard
diff --git a/arch/avr32/mm/fault.c b/arch/avr32/mm/fault.c
index 6560cb1..ce4e429 100644
--- a/arch/avr32/mm/fault.c
+++ b/arch/avr32/mm/fault.c
@@ -189,6 +189,8 @@ no_context:
page = sysreg_read(PTBR);
printk(KERN_ALERT ptbr = %08lx, page);
+ if (address = TASK_SIZE)
+ page = (unsigned long)swapper_pg_dir;
if (page) {
page = ((unsigned long *)page)[address 22];
printk( pgd = %08lx, page);
--
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: Tests of undefined CONFIG variables.
On Wed, 13 Feb 2008 05:54:12 -0500 (EST) Robert P. J. Day [EMAIL PROTECTED] wrote: i've also updated the list of what i call badref CONFIG variables -- that is, tests of CONFIG_ variables that appear to be undefined anywhere in a Kconfig file (which typically represents a meaningless test, naturally). I've fixed BOARD_ATSTK1002_SW2_CUSTOM, thanks. It was a real bug. As for AP7000_8_BIT_SMC, it is sort-of used. It is part of a choice menu where the other two choices are used like this: #if defined(CONFIG_AP700X_32_BIT_SMC) ... #elif defined(CONFIG_AP700X_16_BIT_SMC) ... #else ... #endif and the #else block covers the only remaining choice, AP700X_8_BIT_SMC. I can fix it by turning the #else above into an #elif I guess. 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: Taint kernel after WARN_ON(condition) v2
On Wed, 13 Feb 2008 22:27:40 +0800 Nur Hussein [EMAIL PROTECTED] wrote: This does not work on architectures where WARN_ON has its own definition. These archs are: 1. s390 2. superh 3. avr32 4. parisc Hmm. Relying on the generic code in lib/bug.c qualifies as own definition these days? I think the patch below should take care of all four...unless I've misunderstood something. Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED] diff --git a/lib/bug.c b/lib/bug.c index 530f38f..0d67419 100644 --- a/lib/bug.c +++ b/lib/bug.c @@ -35,6 +35,7 @@ Jeremy Fitzhardinge [EMAIL PROTECTED] 2006 */ +#include linux/kernel.h #include linux/list.h #include linux/module.h #include linux/bug.h @@ -149,6 +150,7 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) (void *)bugaddr); show_regs(regs); + add_taint(TAINT_WARN); return BUG_TRAP_TYPE_WARN; } -- 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: BUG: 2.6.25-rc1: iptables postrouting setup causes oops
On Wed, 13 Feb 2008 08:42:15 -0800 Andrew G. Morgan [EMAIL PROTECTED] wrote: However, it is a warning and, for any existing app that doesn't care about newly added capabilities, the warning is benign. Ok, I see. Thanks for explaining. 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: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Wed, 13 Feb 2008 19:30:51 +0100 Pierre Ossman [EMAIL PROTECTED] wrote: I think this was meant to go away. And this should go into the core. I also pointed this out. mmc_remove_host() will synchronize this for you. Right. Sorry. I focused so much on getting the driver to work correctly that I totally forgot about these things. I'll fix it up before the next round (which I hope can be labeled PATCH instead of RFC.) Btw, this isn't the latest version of the driver...but I'm afraid the latest one suffers from these omissions too, except for the last one which I did actually remove. 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: [RFC v3 4/7] dmaengine: Add slave DMA interface
On Wed, 13 Feb 2008 12:07:26 -0700
Dan Williams [EMAIL PROTECTED] wrote:
+struct dma_slave_descriptor {
+ struct dma_async_tx_descriptor txd;
+ struct list_head client_node;
+};
Can you explain a bit why client_node is needed? I do not think we
need dma_slave_descriptor if dma_unmap data / control is added to
dma_async_tx_descriptor. Hmm?
Well, it's perhaps not needed for slave transfers as such. But the MMC
driver (and I suspect quite a few other users of the slave interface)
deals with scatterlists, so it needs a way to keep track of all the
descriptors it submits. Hence the list node.
But looking at your latest patch series, I guess we can use the new
next field instead. It's not like we really need the full
capabilities of list_head.
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: [RFC v2 5/5] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Wed, 13 Feb 2008 12:11:58 -0700 Dan Williams [EMAIL PROTECTED] wrote: + desc = chan-device-device_prep_slave(chan, + sg_dma_address(sg), direction, + DMA_SLAVE_WIDTH_32BIT, + sg_dma_len(sg), dma_flags); + desc-txd.callback = NULL; + list_add_tail(desc-client_node, + host-dma.data_descs); + } Need to handle device_prep_slave returning NULL? You're right, we definitely need to handle that. Which probably means we need to prepare an interrupt descriptor first that we can throw in when we're unable to obtain more descriptors, and submit the rest from the callback. Except we're not allowed to submit anything from the callback. Ouch. How can we solve that? Set up a work queue and submit it from there? Trigger a different tasklet? In any case, I guess I need to implement support for interrupt descriptors in the dw_dmac driver. 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: [RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users
On Tue, 12 Feb 2008 14:43:30 -0600 Olof Johansson <[EMAIL PROTECTED]> wrote: > > - depends on (PCI && X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX > > + depends on (PCI && X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX > > || AVR32 > > This is a slippery slope. Things should be the other way around instead, > create a HAVE_DMA_DEVICE, select it in the relevant platform code and > make DMADEVICES depend on that. Agree. I'll cook up a patch tomorrow to make it use the new HAVE_* scheme. > Or just let the subsystem always be available. It used to be always available, but then it was changed. Assuming there was a reason for this change, I guess we don't want to change it back. 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/
[RFC v3 6/7] dmaengine: Driver for the Synopsys DesignWare DMA controller
This adds a driver for the Synopsys DesignWare DMA controller (aka
DMACA on AVR32 systems.) This DMA controller can be found integrated
on the AT32AP7000 chip and is primarily meant for peripheral DMA
transfer, but can also be used for memory-to-memory transfers.
This patch is based on a driver from David Brownell which was based on
an older version of the DMA Engine framework. It also implements the
proposed extensions to the DMA Engine API for slave DMA operations.
The dmatest client shows no problems, but the performance is not as
good as it should be yet. However, DMA slave transfer performance is
quite acceptable, so while I think the memcpy performance can be
improved, it's not very high priority right now.
Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]>
Changes since v2:
* Dequeue all pending transfers in terminate_all()
* Rename dw_dmac.h -> dw_dmac_regs.h
* Define and use controller-specific dma_slave data
* Fix up a few outdated comments
* Define hardware registers as structs (doesn't generate better
code, unfortunately, but it looks nicer.)
* Get number of channels from platform_data instead of hardcoding it
based on CONFIG_WHATEVER_CPU.
* Give slave clients exclusive access to the channel
---
arch/avr32/mach-at32ap/at32ap700x.c| 26 +-
drivers/dma/Kconfig|9 +
drivers/dma/Makefile |1 +
drivers/dma/dw_dmac.c | 1170
drivers/dma/dw_dmac_regs.h | 249 ++
include/asm-avr32/arch-at32ap/at32ap700x.h | 16 +
include/linux/dw_dmac.h| 62 ++
7 files changed, 1520 insertions(+), 13 deletions(-)
create mode 100644 drivers/dma/dw_dmac.c
create mode 100644 drivers/dma/dw_dmac_regs.h
create mode 100644 include/linux/dw_dmac.h
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c
b/arch/avr32/mach-at32ap/at32ap700x.c
index 7678fee..038e17b 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -451,6 +451,17 @@ static void __init genclk_init_parent(struct clk *clk)
clk->parent = parent;
}
+static struct dw_dma_platform_data dw_dmac0_data = {
+ .nr_channels= 3,
+};
+
+static struct resource dw_dmac0_resource[] = {
+ PBMEM(0xff20),
+ IRQ(2),
+};
+DEFINE_DEV_DATA(dw_dmac, 0);
+DEV_CLK(hclk, dw_dmac0, hsb, 10);
+
/*
* System peripherals
* */
@@ -557,17 +568,6 @@ static struct clk pico_clk = {
.users = 1,
};
-static struct resource dmaca0_resource[] = {
- {
- .start = 0xff20,
- .end= 0xff20,
- .flags = IORESOURCE_MEM,
- },
- IRQ(2),
-};
-DEFINE_DEV(dmaca, 0);
-DEV_CLK(hclk, dmaca0, hsb, 10);
-
/*
* HMATRIX
* */
@@ -667,7 +667,7 @@ void __init at32_add_system_devices(void)
platform_device_register(_eic0_device);
platform_device_register(_device);
platform_device_register(_device);
- platform_device_register(_device);
+ platform_device_register(_dmac0_device);
platform_device_register(_systc0_device);
@@ -1687,7 +1687,7 @@ struct clk *at32_clock_list[] = {
_mck,
_hclk,
_pclk,
- _hclk,
+ _dmac0_hclk,
_clk,
_mck,
_mck,
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 1a727c1..76582db 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -37,6 +37,15 @@ config INTEL_IOP_ADMA
help
Enable support for the Intel(R) IOP Series RAID engines.
+config DW_DMAC
+ tristate "Synopsys DesignWare AHB DMA support"
+ depends on AVR32
+ select DMA_ENGINE
+ default y if CPU_AT32AP7000
+ help
+ Support the Synopsys DesignWare AHB DMA controller. This
+ can be integrated in chips such as the Atmel AT32ap7000.
+
config DMA_ENGINE
bool
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index cecfb60..0f3b24f 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
obj-$(CONFIG_NET_DMA) += iovlock.o
obj-$(CONFIG_INTEL_IOATDMA) += ioatdma.o
+obj-$(CONFIG_DW_DMAC) += dw_dmac.o
ioatdma-objs := ioat.o ioat_dma.o ioat_dca.o
obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
obj-$(CONFIG_DMATEST) += dmatest.o
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
new file mode 100644
index 000..9d63c2f
--- /dev/null
+++ b/drivers/dma/dw_dmac.c
@@ -0,0 +1,1170 @@
+/*
+ * Driver for the Synopsys DesignWare DMA Controller (aka DMACA on
+ * AVR32 systems.)
+ *
+ * Copyright (C) 2007 Atmel Corporation
+ *
+ * This pr
[RFC v3 7/7] Atmel MCI: Driver for Atmel on-chip MMC controllers
This is a driver for the MMC controller on the AP7000 chips from
Atmel. It should in theory work on AT91 systems too with some
tweaking, but since the DMA interface is quite different, it's not
entirely clear if it's worth it.
This driver has been around for a while in BSPs and kernel sources
provided by Atmel, but this particular version uses the generic DMA
Engine framework (with the slave extensions) instead of an
avr32-only DMA controller framework.
This driver can also use PIO transfers when no DMA channels are
available, and for transfers where using DMA may be difficult or
impractical for some reason (e.g. the DMA setup overhead is usually
not worth it for very short transfers.)
The driver has been tested using mmc-block and ext3fs on several SD,
SDHC and MMC+ cards. Reads and writes work fine, with read transfer
rates up to 3.4MB/s with lots of debug options enabled.
The driver has also been tested using the mmc_test module on the same
cards, with somewhat less convincing results. In particular, badly
aligned multiblock writes seem to confuse some of the cards.
Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]>
Changes since v2:
* Reset the controller after each transfer since we're violating the
spec sometimes. This is very cheap, so we don't try to be clever.
* Turn off the MMC clock when no requests are pending.
* Implement support for PIO transfers (i.e. not using DMA.)
* Rename atmel-mci.h -> atmel-mci-regs.h
* Use controller-specific data passed from the platform code to set
up DMA slave transfers. These parameters include including physical
DMA device, peripheral handshake IDs, channel priorities, etc.
* Fix several card removal bugs
---
arch/avr32/boards/atngw100/setup.c |7 +
arch/avr32/boards/atstk1000/atstk1002.c |3 +
arch/avr32/mach-at32ap/at32ap700x.c | 47 +-
drivers/mmc/host/Kconfig| 10 +
drivers/mmc/host/Makefile |1 +
drivers/mmc/host/atmel-mci-regs.h | 194 +
drivers/mmc/host/atmel-mci.c| 1400 +++
include/asm-avr32/arch-at32ap/board.h |6 +-
include/asm-avr32/atmel-mci.h | 12 +
9 files changed, 1674 insertions(+), 6 deletions(-)
create mode 100644 drivers/mmc/host/atmel-mci-regs.h
create mode 100644 drivers/mmc/host/atmel-mci.c
create mode 100644 include/asm-avr32/atmel-mci.h
diff --git a/arch/avr32/boards/atngw100/setup.c
b/arch/avr32/boards/atngw100/setup.c
index a398be2..96833bf 100644
--- a/arch/avr32/boards/atngw100/setup.c
+++ b/arch/avr32/boards/atngw100/setup.c
@@ -17,6 +17,7 @@
#include
#include
+#include
#include
#include
@@ -42,6 +43,11 @@ static struct spi_board_info spi0_board_info[] __initdata = {
},
};
+static struct mci_platform_data __initdata mci0_data = {
+ .detect_pin = GPIO_PIN_PC(25),
+ .wp_pin = GPIO_PIN_PE(0),
+};
+
/*
* The next two functions should go away as the boot loader is
* supposed to initialize the macb address registers with a valid
@@ -157,6 +163,7 @@ static int __init atngw100_init(void)
set_hw_addr(at32_add_device_eth(1, _data[1]));
at32_add_device_spi(0, spi0_board_info, ARRAY_SIZE(spi0_board_info));
+ at32_add_device_mci(0, _data);
at32_add_device_usba(0, NULL);
for (i = 0; i < ARRAY_SIZE(ngw_leds); i++) {
diff --git a/arch/avr32/boards/atstk1000/atstk1002.c
b/arch/avr32/boards/atstk1000/atstk1002.c
index 000eb42..8b92cd6 100644
--- a/arch/avr32/boards/atstk1000/atstk1002.c
+++ b/arch/avr32/boards/atstk1000/atstk1002.c
@@ -228,6 +228,9 @@ static int __init atstk1002_init(void)
#ifdef CONFIG_BOARD_ATSTK100X_SPI1
at32_add_device_spi(1, spi1_board_info, ARRAY_SIZE(spi1_board_info));
#endif
+#ifndef CONFIG_BOARD_ATSTK1002_SW2_CUSTOM
+ at32_add_device_mci(0, NULL);
+#endif
#ifdef CONFIG_BOARD_ATSTK1002_SW5_CUSTOM
set_hw_addr(at32_add_device_eth(1, _data[1]));
#else
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c
b/arch/avr32/mach-at32ap/at32ap700x.c
index 038e17b..0a0a499 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -6,12 +6,14 @@
* published by the Free Software Foundation.
*/
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
@@ -1032,20 +1034,48 @@ static struct clk atmel_mci0_pclk = {
.index = 9,
};
-struct platform_device *__init at32_add_device_mci(unsigned int id)
+struct platform_device *__init
+at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
{
- struct platform_device *pdev;
+ struct mci_platform_data_data;
+ struct platform_device *pdev;
+ struct dw_dma_slave *dws;
if (id != 0)
return NULL;
pdev = platform_device_alloc("atmel_mci", id);
if (!pdev)
- return NULL
[RFC v3 2/7] dmaengine: Add dma_client parameter to device_alloc_chan_resources
A DMA controller capable of doing slave transfers may need to know a
few things about the slave when preparing the channel. We don't want
to add this information to struct dma_channel since the channel hasn't
yet been bound to a client at this point.
Instead, pass a reference to the client requesting the channel to the
driver's device_alloc_chan_resources hook so that it can pick the
necessary information from the dma_client struct by itself.
Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]>
---
drivers/dma/dmaengine.c |3 ++-
drivers/dma/ioat_dma.c|5 +++--
drivers/dma/iop-adma.c|7 ---
include/linux/dmaengine.h |3 ++-
4 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 7c7cb4b..9d11f06 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -174,7 +174,8 @@ static void dma_client_chan_alloc(struct dma_client *client)
if (!dma_chan_satisfies_mask(chan, client->cap_mask))
continue;
- desc = chan->device->device_alloc_chan_resources(chan);
+ desc = chan->device->device_alloc_chan_resources(
+ chan, client);
if (desc >= 0) {
ack = client->event_callback(client,
chan,
diff --git a/drivers/dma/ioat_dma.c b/drivers/dma/ioat_dma.c
index dff38ac..3e82bfb 100644
--- a/drivers/dma/ioat_dma.c
+++ b/drivers/dma/ioat_dma.c
@@ -452,7 +452,8 @@ static void ioat2_dma_massage_chan_desc(struct
ioat_dma_chan *ioat_chan)
* ioat_dma_alloc_chan_resources - returns the number of allocated descriptors
* @chan: the channel to be filled out
*/
-static int ioat_dma_alloc_chan_resources(struct dma_chan *chan)
+static int ioat_dma_alloc_chan_resources(struct dma_chan *chan,
+struct dma_client *client)
{
struct ioat_dma_chan *ioat_chan = to_ioat_chan(chan);
struct ioat_desc_sw *desc;
@@ -1058,7 +1059,7 @@ static int ioat_dma_self_test(struct ioatdma_device
*device)
dma_chan = container_of(device->common.channels.next,
struct dma_chan,
device_node);
- if (device->common.device_alloc_chan_resources(dma_chan) < 1) {
+ if (device->common.device_alloc_chan_resources(dma_chan, NULL) < 1) {
dev_err(>pdev->dev,
"selftest cannot allocate chan resource\n");
err = -ENODEV;
diff --git a/drivers/dma/iop-adma.c b/drivers/dma/iop-adma.c
index 3986d54..aacda12 100644
--- a/drivers/dma/iop-adma.c
+++ b/drivers/dma/iop-adma.c
@@ -447,7 +447,8 @@ static void iop_chan_start_null_memcpy(struct iop_adma_chan
*iop_chan);
static void iop_chan_start_null_xor(struct iop_adma_chan *iop_chan);
/* returns the number of allocated descriptors */
-static int iop_adma_alloc_chan_resources(struct dma_chan *chan)
+static int iop_adma_alloc_chan_resources(struct dma_chan *chan,
+struct dma_client *client)
{
char *hw_desc;
int idx;
@@ -844,7 +845,7 @@ static int __devinit iop_adma_memcpy_self_test(struct
iop_adma_device *device)
dma_chan = container_of(device->common.channels.next,
struct dma_chan,
device_node);
- if (iop_adma_alloc_chan_resources(dma_chan) < 1) {
+ if (iop_adma_alloc_chan_resources(dma_chan, NULL) < 1) {
err = -ENODEV;
goto out;
}
@@ -942,7 +943,7 @@ iop_adma_xor_zero_sum_self_test(struct iop_adma_device
*device)
dma_chan = container_of(device->common.channels.next,
struct dma_chan,
device_node);
- if (iop_adma_alloc_chan_resources(dma_chan) < 1) {
+ if (iop_adma_alloc_chan_resources(dma_chan, NULL) < 1) {
err = -ENODEV;
goto out;
}
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index acbb364..9cd7ed9 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -278,7 +278,8 @@ struct dma_device {
int dev_id;
struct device *dev;
- int (*device_alloc_chan_resources)(struct dma_chan *chan);
+ int (*device_alloc_chan_resources)(struct dma_chan *chan,
+ struct dma_client *client);
void (*device_free_chan_resources)(struct dma_chan *chan);
struct dma_async_tx_descriptor *(*device_prep_dma_memcpy)(
--
1.5.3.8
--
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/
[RFC v3 4/7] dmaengine: Add slave DMA interface
This patch adds the necessary interfaces to the DMA Engine framework
to use functionality found on most embedded DMA controllers: DMA from
and to I/O registers with hardware handshaking.
In this context, hardware hanshaking means that the peripheral that
owns the I/O registers in question is able to tell the DMA controller
when more data is available for reading, or when there is room for
more data to be written. This usually happens internally on the chip,
but these signals may also be exported outside the chip for things
like IDE DMA, etc.
A new struct dma_slave is introduced. This contains information that
the DMA engine driver needs to set up slave transfers to and from a
slave device. Most engines supporting DMA slave transfers will want to
extend this structure with controller-specific parameters. This
additional information is usually passed from the platform/board code
through the client driver.
A "slave" pointer is added to the dma_client struct. This must point
to a valid dma_slave structure iff the DMA_SLAVE capability is
requested. The DMA engine driver may use this information in its
device_alloc_chan_resources hook to configure the DMA controller for
slave transfers from and to the given slave device.
A new struct dma_slave_descriptor is added. This extends the standard
dma_async_tx_descriptor with a few members that are needed for doing
slave DMA from/to peripherals.
A new operation for creating such descriptors is added to struct
dma_device. Another new operation for terminating all pending
transfers is added as well. The latter is needed because there may be
errors outside the scope of the DMA Engine framework that may require
DMA operations to be terminated prematurely.
DMA Engine drivers may extend the dma_device, dma_chan and/or
dma_slave_descriptor structures to allow controller-specific
operations. The client driver can detect such extensions by looking at
the DMA Engine's struct device, or it can request a specific DMA
Engine device by setting the dma_dev field in struct dma_slave.
Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]>
dmaslave interface changes since v2:
* Add a dma_dev field to struct dma_slave. If set, the client can
only be bound to the DMA controller that corresponds to this
device. This allows controller-specific extensions of the
dma_slave structure; if the device matches, the controller may
safely assume its extensions are present.
* Move reg_width into struct dma_slave as there are currently no
users that need to be able to set the width on a per-transfer
basis.
dmaslave interface changes since v1:
* Drop the set_direction and set_width descriptor hooks. Pass the
direction and width to the prep function instead.
* Declare a dma_slave struct with fixed information about a slave,
i.e. register addresses, handshake interfaces and such.
* Add pointer to a dma_slave struct to dma_client. Can be NULL if
the DMA_SLAVE capability isn't requested.
* Drop the set_slave device hook since the alloc_chan_resources hook
now has enough information to set up the channel for slave
transfers.
---
drivers/dma/dmaengine.c | 16 +-
include/linux/dmaengine.h | 74 -
2 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 3076d80..b689140 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -159,7 +159,12 @@ static void dma_client_chan_alloc(struct dma_client
*client)
enum dma_state_client ack;
/* Find a channel */
- list_for_each_entry(device, _device_list, global_node)
+ list_for_each_entry(device, _device_list, global_node) {
+ /* Does the client require a specific DMA controller? */
+ if (client->slave && client->slave->dma_dev
+ && client->slave->dma_dev != device->dev)
+ continue;
+
list_for_each_entry(chan, >channels, device_node) {
if (!dma_chan_satisfies_mask(chan, client->cap_mask))
continue;
@@ -180,6 +185,7 @@ static void dma_client_chan_alloc(struct dma_client *client)
return;
}
}
+ }
}
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
@@ -276,6 +282,10 @@ static void dma_clients_notify_removed(struct dma_chan
*chan)
*/
void dma_async_client_register(struct dma_client *client)
{
+ /* validate client data */
+ BUG_ON(dma_has_cap(DMA_SLAVE, client->cap_mask) &&
+ !client->slave);
+
mutex_lock(_list_mutex);
list_add_tail(>global_node, _client_list);
mutex_unlock(_list_mutex);
@@ -350,6 +360,10 @@ int dma_async_device_register(struct dma_device
[RFC v3 3/7] dmaengine: Add dma_chan_is_in_use() function
This moves the code checking if a DMA channel is in use from
show_in_use() into an inline helper function, dma_is_in_use(). DMA
controllers can use this in order to give clients exclusive access to
channels (usually necessary when setting up slave DMA.)
I have to admit that I don't really understand the channel refcounting
logic at all... dma_chan_get() simply increments a per-cpu value. How
can we be sure that whatever CPU calls dma_chan_is_in_use() sees the
same value?
Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]>
---
drivers/dma/dmaengine.c | 12 +---
include/linux/dmaengine.h | 17 +
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 9d11f06..3076d80 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -105,17 +105,7 @@ static ssize_t show_bytes_transferred(struct device *dev,
struct device_attribut
static ssize_t show_in_use(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct dma_chan *chan = to_dma_chan(dev);
- int in_use = 0;
-
- if (unlikely(chan->slow_ref) &&
- atomic_read(>refcount.refcount) > 1)
- in_use = 1;
- else {
- if (local_read(&(per_cpu_ptr(chan->local,
- get_cpu())->refcount)) > 0)
- in_use = 1;
- put_cpu();
- }
+ int in_use = dma_chan_is_in_use(chan);
return sprintf(buf, "%d\n", in_use);
}
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9cd7ed9..5d9a3a2 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -175,6 +175,23 @@ static inline void dma_chan_put(struct dma_chan *chan)
}
}
+static inline bool dma_chan_is_in_use(struct dma_chan *chan)
+{
+ bool in_use = false;
+
+ if (unlikely(chan->slow_ref) &&
+ atomic_read(>refcount.refcount) > 1)
+ in_use = true;
+ else {
+ if (local_read(&(per_cpu_ptr(chan->local,
+ get_cpu())->refcount)) > 0)
+ in_use = true;
+ put_cpu();
+ }
+
+ return in_use;
+}
+
/*
* typedef dma_event_callback - function pointer to a DMA event callback
* For each channel added to the system this routine is called for each client.
--
1.5.3.8
--
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/
[RFC v3 1/7] dmaengine: Couple DMA channels to their physical DMA device
Set the 'parent' field of channel class devices to point to the physical DMA device initialized by the DMA engine driver. This allows drivers to use chan->dev.parent for syncing DMA buffers and adds a 'device' symlink to the real device in /sys/class/dma/dmaXchanY. Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]> --- drivers/dma/dmaengine.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c index 2996523..7c7cb4b 100644 --- a/drivers/dma/dmaengine.c +++ b/drivers/dma/dmaengine.c @@ -379,7 +379,7 @@ int dma_async_device_register(struct dma_device *device) chan->chan_id = chancnt++; chan->dev.class = _devclass; - chan->dev.parent = NULL; + chan->dev.parent = device->dev; snprintf(chan->dev.bus_id, BUS_ID_SIZE, "dma%dchan%d", device->dev_id, chan->chan_id); -- 1.5.3.8 -- 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/
[RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users
Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]> --- drivers/dma/Kconfig |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 893a3f8..1a727c1 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -4,7 +4,7 @@ menuconfig DMADEVICES bool "DMA Engine support" - depends on (PCI && X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX + depends on (PCI && X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX || AVR32 depends on !HIGHMEM64G help DMA engines can do asynchronous data transfers without -- 1.5.3.8 -- 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: [RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users
On Tue, 12 Feb 2008 14:43:30 -0600 Olof Johansson [EMAIL PROTECTED] wrote: - depends on (PCI X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX + depends on (PCI X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX || AVR32 This is a slippery slope. Things should be the other way around instead, create a HAVE_DMA_DEVICE, select it in the relevant platform code and make DMADEVICES depend on that. Agree. I'll cook up a patch tomorrow to make it use the new HAVE_* scheme. Or just let the subsystem always be available. It used to be always available, but then it was changed. Assuming there was a reason for this change, I guess we don't want to change it back. 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/
[RFC v3 6/7] dmaengine: Driver for the Synopsys DesignWare DMA controller
This adds a driver for the Synopsys DesignWare DMA controller (aka
DMACA on AVR32 systems.) This DMA controller can be found integrated
on the AT32AP7000 chip and is primarily meant for peripheral DMA
transfer, but can also be used for memory-to-memory transfers.
This patch is based on a driver from David Brownell which was based on
an older version of the DMA Engine framework. It also implements the
proposed extensions to the DMA Engine API for slave DMA operations.
The dmatest client shows no problems, but the performance is not as
good as it should be yet. However, DMA slave transfer performance is
quite acceptable, so while I think the memcpy performance can be
improved, it's not very high priority right now.
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED]
Changes since v2:
* Dequeue all pending transfers in terminate_all()
* Rename dw_dmac.h - dw_dmac_regs.h
* Define and use controller-specific dma_slave data
* Fix up a few outdated comments
* Define hardware registers as structs (doesn't generate better
code, unfortunately, but it looks nicer.)
* Get number of channels from platform_data instead of hardcoding it
based on CONFIG_WHATEVER_CPU.
* Give slave clients exclusive access to the channel
---
arch/avr32/mach-at32ap/at32ap700x.c| 26 +-
drivers/dma/Kconfig|9 +
drivers/dma/Makefile |1 +
drivers/dma/dw_dmac.c | 1170
drivers/dma/dw_dmac_regs.h | 249 ++
include/asm-avr32/arch-at32ap/at32ap700x.h | 16 +
include/linux/dw_dmac.h| 62 ++
7 files changed, 1520 insertions(+), 13 deletions(-)
create mode 100644 drivers/dma/dw_dmac.c
create mode 100644 drivers/dma/dw_dmac_regs.h
create mode 100644 include/linux/dw_dmac.h
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c
b/arch/avr32/mach-at32ap/at32ap700x.c
index 7678fee..038e17b 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -451,6 +451,17 @@ static void __init genclk_init_parent(struct clk *clk)
clk-parent = parent;
}
+static struct dw_dma_platform_data dw_dmac0_data = {
+ .nr_channels= 3,
+};
+
+static struct resource dw_dmac0_resource[] = {
+ PBMEM(0xff20),
+ IRQ(2),
+};
+DEFINE_DEV_DATA(dw_dmac, 0);
+DEV_CLK(hclk, dw_dmac0, hsb, 10);
+
/*
* System peripherals
* */
@@ -557,17 +568,6 @@ static struct clk pico_clk = {
.users = 1,
};
-static struct resource dmaca0_resource[] = {
- {
- .start = 0xff20,
- .end= 0xff20,
- .flags = IORESOURCE_MEM,
- },
- IRQ(2),
-};
-DEFINE_DEV(dmaca, 0);
-DEV_CLK(hclk, dmaca0, hsb, 10);
-
/*
* HMATRIX
* */
@@ -667,7 +667,7 @@ void __init at32_add_system_devices(void)
platform_device_register(at32_eic0_device);
platform_device_register(smc0_device);
platform_device_register(pdc_device);
- platform_device_register(dmaca0_device);
+ platform_device_register(dw_dmac0_device);
platform_device_register(at32_systc0_device);
@@ -1687,7 +1687,7 @@ struct clk *at32_clock_list[] = {
smc0_mck,
pdc_hclk,
pdc_pclk,
- dmaca0_hclk,
+ dw_dmac0_hclk,
pico_clk,
pio0_mck,
pio1_mck,
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 1a727c1..76582db 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -37,6 +37,15 @@ config INTEL_IOP_ADMA
help
Enable support for the Intel(R) IOP Series RAID engines.
+config DW_DMAC
+ tristate Synopsys DesignWare AHB DMA support
+ depends on AVR32
+ select DMA_ENGINE
+ default y if CPU_AT32AP7000
+ help
+ Support the Synopsys DesignWare AHB DMA controller. This
+ can be integrated in chips such as the Atmel AT32ap7000.
+
config DMA_ENGINE
bool
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index cecfb60..0f3b24f 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
obj-$(CONFIG_NET_DMA) += iovlock.o
obj-$(CONFIG_INTEL_IOATDMA) += ioatdma.o
+obj-$(CONFIG_DW_DMAC) += dw_dmac.o
ioatdma-objs := ioat.o ioat_dma.o ioat_dca.o
obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
obj-$(CONFIG_DMATEST) += dmatest.o
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
new file mode 100644
index 000..9d63c2f
--- /dev/null
+++ b/drivers/dma/dw_dmac.c
@@ -0,0 +1,1170 @@
+/*
+ * Driver for the Synopsys DesignWare DMA Controller (aka DMACA on
+ * AVR32 systems.)
+ *
+ * Copyright (C) 2007 Atmel
[RFC v3 7/7] Atmel MCI: Driver for Atmel on-chip MMC controllers
This is a driver for the MMC controller on the AP7000 chips from
Atmel. It should in theory work on AT91 systems too with some
tweaking, but since the DMA interface is quite different, it's not
entirely clear if it's worth it.
This driver has been around for a while in BSPs and kernel sources
provided by Atmel, but this particular version uses the generic DMA
Engine framework (with the slave extensions) instead of an
avr32-only DMA controller framework.
This driver can also use PIO transfers when no DMA channels are
available, and for transfers where using DMA may be difficult or
impractical for some reason (e.g. the DMA setup overhead is usually
not worth it for very short transfers.)
The driver has been tested using mmc-block and ext3fs on several SD,
SDHC and MMC+ cards. Reads and writes work fine, with read transfer
rates up to 3.4MB/s with lots of debug options enabled.
The driver has also been tested using the mmc_test module on the same
cards, with somewhat less convincing results. In particular, badly
aligned multiblock writes seem to confuse some of the cards.
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED]
Changes since v2:
* Reset the controller after each transfer since we're violating the
spec sometimes. This is very cheap, so we don't try to be clever.
* Turn off the MMC clock when no requests are pending.
* Implement support for PIO transfers (i.e. not using DMA.)
* Rename atmel-mci.h - atmel-mci-regs.h
* Use controller-specific data passed from the platform code to set
up DMA slave transfers. These parameters include including physical
DMA device, peripheral handshake IDs, channel priorities, etc.
* Fix several card removal bugs
---
arch/avr32/boards/atngw100/setup.c |7 +
arch/avr32/boards/atstk1000/atstk1002.c |3 +
arch/avr32/mach-at32ap/at32ap700x.c | 47 +-
drivers/mmc/host/Kconfig| 10 +
drivers/mmc/host/Makefile |1 +
drivers/mmc/host/atmel-mci-regs.h | 194 +
drivers/mmc/host/atmel-mci.c| 1400 +++
include/asm-avr32/arch-at32ap/board.h |6 +-
include/asm-avr32/atmel-mci.h | 12 +
9 files changed, 1674 insertions(+), 6 deletions(-)
create mode 100644 drivers/mmc/host/atmel-mci-regs.h
create mode 100644 drivers/mmc/host/atmel-mci.c
create mode 100644 include/asm-avr32/atmel-mci.h
diff --git a/arch/avr32/boards/atngw100/setup.c
b/arch/avr32/boards/atngw100/setup.c
index a398be2..96833bf 100644
--- a/arch/avr32/boards/atngw100/setup.c
+++ b/arch/avr32/boards/atngw100/setup.c
@@ -17,6 +17,7 @@
#include linux/leds.h
#include linux/spi/spi.h
+#include asm/atmel-mci.h
#include asm/io.h
#include asm/setup.h
@@ -42,6 +43,11 @@ static struct spi_board_info spi0_board_info[] __initdata = {
},
};
+static struct mci_platform_data __initdata mci0_data = {
+ .detect_pin = GPIO_PIN_PC(25),
+ .wp_pin = GPIO_PIN_PE(0),
+};
+
/*
* The next two functions should go away as the boot loader is
* supposed to initialize the macb address registers with a valid
@@ -157,6 +163,7 @@ static int __init atngw100_init(void)
set_hw_addr(at32_add_device_eth(1, eth_data[1]));
at32_add_device_spi(0, spi0_board_info, ARRAY_SIZE(spi0_board_info));
+ at32_add_device_mci(0, mci0_data);
at32_add_device_usba(0, NULL);
for (i = 0; i ARRAY_SIZE(ngw_leds); i++) {
diff --git a/arch/avr32/boards/atstk1000/atstk1002.c
b/arch/avr32/boards/atstk1000/atstk1002.c
index 000eb42..8b92cd6 100644
--- a/arch/avr32/boards/atstk1000/atstk1002.c
+++ b/arch/avr32/boards/atstk1000/atstk1002.c
@@ -228,6 +228,9 @@ static int __init atstk1002_init(void)
#ifdef CONFIG_BOARD_ATSTK100X_SPI1
at32_add_device_spi(1, spi1_board_info, ARRAY_SIZE(spi1_board_info));
#endif
+#ifndef CONFIG_BOARD_ATSTK1002_SW2_CUSTOM
+ at32_add_device_mci(0, NULL);
+#endif
#ifdef CONFIG_BOARD_ATSTK1002_SW5_CUSTOM
set_hw_addr(at32_add_device_eth(1, eth_data[1]));
#else
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c
b/arch/avr32/mach-at32ap/at32ap700x.c
index 038e17b..0a0a499 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -6,12 +6,14 @@
* published by the Free Software Foundation.
*/
#include linux/clk.h
+#include linux/dw_dmac.h
#include linux/fb.h
#include linux/init.h
#include linux/platform_device.h
#include linux/dma-mapping.h
#include linux/spi/spi.h
+#include asm/atmel-mci.h
#include asm/io.h
#include asm/irq.h
@@ -1032,20 +1034,48 @@ static struct clk atmel_mci0_pclk = {
.index = 9,
};
-struct platform_device *__init at32_add_device_mci(unsigned int id)
+struct platform_device *__init
+at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
{
- struct platform_device *pdev;
+ struct mci_platform_data_data;
+ struct platform_device *pdev;
+ struct
[RFC v3 4/7] dmaengine: Add slave DMA interface
This patch adds the necessary interfaces to the DMA Engine framework
to use functionality found on most embedded DMA controllers: DMA from
and to I/O registers with hardware handshaking.
In this context, hardware hanshaking means that the peripheral that
owns the I/O registers in question is able to tell the DMA controller
when more data is available for reading, or when there is room for
more data to be written. This usually happens internally on the chip,
but these signals may also be exported outside the chip for things
like IDE DMA, etc.
A new struct dma_slave is introduced. This contains information that
the DMA engine driver needs to set up slave transfers to and from a
slave device. Most engines supporting DMA slave transfers will want to
extend this structure with controller-specific parameters. This
additional information is usually passed from the platform/board code
through the client driver.
A slave pointer is added to the dma_client struct. This must point
to a valid dma_slave structure iff the DMA_SLAVE capability is
requested. The DMA engine driver may use this information in its
device_alloc_chan_resources hook to configure the DMA controller for
slave transfers from and to the given slave device.
A new struct dma_slave_descriptor is added. This extends the standard
dma_async_tx_descriptor with a few members that are needed for doing
slave DMA from/to peripherals.
A new operation for creating such descriptors is added to struct
dma_device. Another new operation for terminating all pending
transfers is added as well. The latter is needed because there may be
errors outside the scope of the DMA Engine framework that may require
DMA operations to be terminated prematurely.
DMA Engine drivers may extend the dma_device, dma_chan and/or
dma_slave_descriptor structures to allow controller-specific
operations. The client driver can detect such extensions by looking at
the DMA Engine's struct device, or it can request a specific DMA
Engine device by setting the dma_dev field in struct dma_slave.
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED]
dmaslave interface changes since v2:
* Add a dma_dev field to struct dma_slave. If set, the client can
only be bound to the DMA controller that corresponds to this
device. This allows controller-specific extensions of the
dma_slave structure; if the device matches, the controller may
safely assume its extensions are present.
* Move reg_width into struct dma_slave as there are currently no
users that need to be able to set the width on a per-transfer
basis.
dmaslave interface changes since v1:
* Drop the set_direction and set_width descriptor hooks. Pass the
direction and width to the prep function instead.
* Declare a dma_slave struct with fixed information about a slave,
i.e. register addresses, handshake interfaces and such.
* Add pointer to a dma_slave struct to dma_client. Can be NULL if
the DMA_SLAVE capability isn't requested.
* Drop the set_slave device hook since the alloc_chan_resources hook
now has enough information to set up the channel for slave
transfers.
---
drivers/dma/dmaengine.c | 16 +-
include/linux/dmaengine.h | 74 -
2 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 3076d80..b689140 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -159,7 +159,12 @@ static void dma_client_chan_alloc(struct dma_client
*client)
enum dma_state_client ack;
/* Find a channel */
- list_for_each_entry(device, dma_device_list, global_node)
+ list_for_each_entry(device, dma_device_list, global_node) {
+ /* Does the client require a specific DMA controller? */
+ if (client-slave client-slave-dma_dev
+client-slave-dma_dev != device-dev)
+ continue;
+
list_for_each_entry(chan, device-channels, device_node) {
if (!dma_chan_satisfies_mask(chan, client-cap_mask))
continue;
@@ -180,6 +185,7 @@ static void dma_client_chan_alloc(struct dma_client *client)
return;
}
}
+ }
}
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
@@ -276,6 +282,10 @@ static void dma_clients_notify_removed(struct dma_chan
*chan)
*/
void dma_async_client_register(struct dma_client *client)
{
+ /* validate client data */
+ BUG_ON(dma_has_cap(DMA_SLAVE, client-cap_mask)
+ !client-slave);
+
mutex_lock(dma_list_mutex);
list_add_tail(client-global_node, dma_client_list);
mutex_unlock(dma_list_mutex);
@@ -350,6 +360,10 @@ int dma_async_device_register(struct dma_device *device)
!device-device_prep_dma_memset
[RFC v3 3/7] dmaengine: Add dma_chan_is_in_use() function
This moves the code checking if a DMA channel is in use from
show_in_use() into an inline helper function, dma_is_in_use(). DMA
controllers can use this in order to give clients exclusive access to
channels (usually necessary when setting up slave DMA.)
I have to admit that I don't really understand the channel refcounting
logic at all... dma_chan_get() simply increments a per-cpu value. How
can we be sure that whatever CPU calls dma_chan_is_in_use() sees the
same value?
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED]
---
drivers/dma/dmaengine.c | 12 +---
include/linux/dmaengine.h | 17 +
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 9d11f06..3076d80 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -105,17 +105,7 @@ static ssize_t show_bytes_transferred(struct device *dev,
struct device_attribut
static ssize_t show_in_use(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct dma_chan *chan = to_dma_chan(dev);
- int in_use = 0;
-
- if (unlikely(chan-slow_ref)
- atomic_read(chan-refcount.refcount) 1)
- in_use = 1;
- else {
- if (local_read((per_cpu_ptr(chan-local,
- get_cpu())-refcount)) 0)
- in_use = 1;
- put_cpu();
- }
+ int in_use = dma_chan_is_in_use(chan);
return sprintf(buf, %d\n, in_use);
}
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9cd7ed9..5d9a3a2 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -175,6 +175,23 @@ static inline void dma_chan_put(struct dma_chan *chan)
}
}
+static inline bool dma_chan_is_in_use(struct dma_chan *chan)
+{
+ bool in_use = false;
+
+ if (unlikely(chan-slow_ref)
+ atomic_read(chan-refcount.refcount) 1)
+ in_use = true;
+ else {
+ if (local_read((per_cpu_ptr(chan-local,
+ get_cpu())-refcount)) 0)
+ in_use = true;
+ put_cpu();
+ }
+
+ return in_use;
+}
+
/*
* typedef dma_event_callback - function pointer to a DMA event callback
* For each channel added to the system this routine is called for each client.
--
1.5.3.8
--
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/
[RFC v3 1/7] dmaengine: Couple DMA channels to their physical DMA device
Set the 'parent' field of channel class devices to point to the physical DMA device initialized by the DMA engine driver. This allows drivers to use chan-dev.parent for syncing DMA buffers and adds a 'device' symlink to the real device in /sys/class/dma/dmaXchanY. Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED] --- drivers/dma/dmaengine.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c index 2996523..7c7cb4b 100644 --- a/drivers/dma/dmaengine.c +++ b/drivers/dma/dmaengine.c @@ -379,7 +379,7 @@ int dma_async_device_register(struct dma_device *device) chan-chan_id = chancnt++; chan-dev.class = dma_devclass; - chan-dev.parent = NULL; + chan-dev.parent = device-dev; snprintf(chan-dev.bus_id, BUS_ID_SIZE, dma%dchan%d, device-dev_id, chan-chan_id); -- 1.5.3.8 -- 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/
[RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED] --- drivers/dma/Kconfig |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 893a3f8..1a727c1 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -4,7 +4,7 @@ menuconfig DMADEVICES bool DMA Engine support - depends on (PCI X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX + depends on (PCI X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX || AVR32 depends on !HIGHMEM64G help DMA engines can do asynchronous data transfers without -- 1.5.3.8 -- 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/
[RFC v3 2/7] dmaengine: Add dma_client parameter to device_alloc_chan_resources
A DMA controller capable of doing slave transfers may need to know a
few things about the slave when preparing the channel. We don't want
to add this information to struct dma_channel since the channel hasn't
yet been bound to a client at this point.
Instead, pass a reference to the client requesting the channel to the
driver's device_alloc_chan_resources hook so that it can pick the
necessary information from the dma_client struct by itself.
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED]
---
drivers/dma/dmaengine.c |3 ++-
drivers/dma/ioat_dma.c|5 +++--
drivers/dma/iop-adma.c|7 ---
include/linux/dmaengine.h |3 ++-
4 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 7c7cb4b..9d11f06 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -174,7 +174,8 @@ static void dma_client_chan_alloc(struct dma_client *client)
if (!dma_chan_satisfies_mask(chan, client-cap_mask))
continue;
- desc = chan-device-device_alloc_chan_resources(chan);
+ desc = chan-device-device_alloc_chan_resources(
+ chan, client);
if (desc = 0) {
ack = client-event_callback(client,
chan,
diff --git a/drivers/dma/ioat_dma.c b/drivers/dma/ioat_dma.c
index dff38ac..3e82bfb 100644
--- a/drivers/dma/ioat_dma.c
+++ b/drivers/dma/ioat_dma.c
@@ -452,7 +452,8 @@ static void ioat2_dma_massage_chan_desc(struct
ioat_dma_chan *ioat_chan)
* ioat_dma_alloc_chan_resources - returns the number of allocated descriptors
* @chan: the channel to be filled out
*/
-static int ioat_dma_alloc_chan_resources(struct dma_chan *chan)
+static int ioat_dma_alloc_chan_resources(struct dma_chan *chan,
+struct dma_client *client)
{
struct ioat_dma_chan *ioat_chan = to_ioat_chan(chan);
struct ioat_desc_sw *desc;
@@ -1058,7 +1059,7 @@ static int ioat_dma_self_test(struct ioatdma_device
*device)
dma_chan = container_of(device-common.channels.next,
struct dma_chan,
device_node);
- if (device-common.device_alloc_chan_resources(dma_chan) 1) {
+ if (device-common.device_alloc_chan_resources(dma_chan, NULL) 1) {
dev_err(device-pdev-dev,
selftest cannot allocate chan resource\n);
err = -ENODEV;
diff --git a/drivers/dma/iop-adma.c b/drivers/dma/iop-adma.c
index 3986d54..aacda12 100644
--- a/drivers/dma/iop-adma.c
+++ b/drivers/dma/iop-adma.c
@@ -447,7 +447,8 @@ static void iop_chan_start_null_memcpy(struct iop_adma_chan
*iop_chan);
static void iop_chan_start_null_xor(struct iop_adma_chan *iop_chan);
/* returns the number of allocated descriptors */
-static int iop_adma_alloc_chan_resources(struct dma_chan *chan)
+static int iop_adma_alloc_chan_resources(struct dma_chan *chan,
+struct dma_client *client)
{
char *hw_desc;
int idx;
@@ -844,7 +845,7 @@ static int __devinit iop_adma_memcpy_self_test(struct
iop_adma_device *device)
dma_chan = container_of(device-common.channels.next,
struct dma_chan,
device_node);
- if (iop_adma_alloc_chan_resources(dma_chan) 1) {
+ if (iop_adma_alloc_chan_resources(dma_chan, NULL) 1) {
err = -ENODEV;
goto out;
}
@@ -942,7 +943,7 @@ iop_adma_xor_zero_sum_self_test(struct iop_adma_device
*device)
dma_chan = container_of(device-common.channels.next,
struct dma_chan,
device_node);
- if (iop_adma_alloc_chan_resources(dma_chan) 1) {
+ if (iop_adma_alloc_chan_resources(dma_chan, NULL) 1) {
err = -ENODEV;
goto out;
}
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index acbb364..9cd7ed9 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -278,7 +278,8 @@ struct dma_device {
int dev_id;
struct device *dev;
- int (*device_alloc_chan_resources)(struct dma_chan *chan);
+ int (*device_alloc_chan_resources)(struct dma_chan *chan,
+ struct dma_client *client);
void (*device_free_chan_resources)(struct dma_chan *chan);
struct dma_async_tx_descriptor *(*device_prep_dma_memcpy)(
--
1.5.3.8
--
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] AVR32: pass i2c board info through at32_add_device_twi
On Thu, 07 Feb 2008 15:28:57 +1100 Ben Nizette <[EMAIL PROTECTED]> wrote: > New-style I2C drivers require that motherboard-mounted I2C devices are > registered with the I2C core, typically at arch_initcall time. This can be > done nice and neat by passing the struct i2c_board_info[] through > at32_add_device_twi just like we do for the SPI board info. > > While we've got the hood up, remove a duplicate declaration of > at32_add_device_twi() in board.h. > > Signed-Off-By: Ben Nizette <[EMAIL PROTECTED]> Applied, thanks. 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] AVR32: pass i2c board info through at32_add_device_twi
On Thu, 07 Feb 2008 15:28:57 +1100 Ben Nizette [EMAIL PROTECTED] wrote: New-style I2C drivers require that motherboard-mounted I2C devices are registered with the I2C core, typically at arch_initcall time. This can be done nice and neat by passing the struct i2c_board_info[] through at32_add_device_twi just like we do for the SPI board info. While we've got the hood up, remove a duplicate declaration of at32_add_device_twi() in board.h. Signed-Off-By: Ben Nizette [EMAIL PROTECTED] Applied, thanks. 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: [RFC v2 2/5] dmaengine: Add slave DMA interface
On Wed, 6 Feb 2008 14:08:35 -0700 "Dan Williams" <[EMAIL PROTECTED]> wrote: > On Jan 30, 2008 5:26 AM, Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > [..] > > Right. I'll add a "unsigned int engine_type" field so that engine > > drivers can go ahead and extend the standard dma_device structure. > > Maybe we should add a "void *platform_data" field to the dma_slave > > struct as well so that platforms can pass arbitrary platform-specific > > information to the DMA controller driver? > > > > I think we can get away with not adding an engine_type field: > 1/ For a given platform there will usually only be one driver active. > For example I have an architecture (IOP) specific dma_copy_to_user > implementation that can safely assume it is talking to the iop-adma > driver since ioat_dma and others are precluded by the Kconfig. > 2/ If there was a situation where two dma drivers were active in a > system you could tell them apart by comparing the function pointers, > i.e. dma_device1->device_prep_dma_memcpy != > dma_device2->device_prep_dma_memcpy. What would you be comparing them against? Perhaps you could pass a struct device * from the platform code, which can be compared against "dev" in struct dma_device? Or you could check dma_device->dev->name perhaps. In any case, I agree we probably don't need the engine_type field. 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: [RFC v2 0/5] dmaengine: Slave DMA interface and example users
On Wed, 6 Feb 2008 11:46:43 -0700 "Dan Williams" <[EMAIL PROTECTED]> wrote: > > The client must somehow know when the transfer is complete -- after > > all, it has to call async_tx_ack() at some point. So additional > > callbacks shouldn't be needed. > > > > The 'ack' only signifies that the client is done with this descriptor, > it tells the api "this descriptor can be freed/reused, no dependent > operations will be submitted against it". This can and does happen > before the operation actually completes. Hmm...ok. But at some point, the client must know that the buffer is completely filled with valid data so that it can call some kind of operation_foo_finish() function to do the necessary unmapping... > > This requires three additional fields in the dma_async_tx_descriptor > > structure, but in many cases the driver needs these fields in its own > > private descriptor wrapper anyway. > > > > I agree this should be moved up to the common descriptor. The unmap > routines are fairly symmetric, so it may not be that bad to also have > an "unmap type" that the cleanup routines could key off of, one of the > options being "do not unmap" for clients that know what they are > doing. I'd prefer that all clients know what they are doing ;-) 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: [RFC v2 0/5] dmaengine: Slave DMA interface and example users
On Wed, 6 Feb 2008 11:46:43 -0700 Dan Williams [EMAIL PROTECTED] wrote: The client must somehow know when the transfer is complete -- after all, it has to call async_tx_ack() at some point. So additional callbacks shouldn't be needed. The 'ack' only signifies that the client is done with this descriptor, it tells the api this descriptor can be freed/reused, no dependent operations will be submitted against it. This can and does happen before the operation actually completes. Hmm...ok. But at some point, the client must know that the buffer is completely filled with valid data so that it can call some kind of operation_foo_finish() function to do the necessary unmapping... This requires three additional fields in the dma_async_tx_descriptor structure, but in many cases the driver needs these fields in its own private descriptor wrapper anyway. I agree this should be moved up to the common descriptor. The unmap routines are fairly symmetric, so it may not be that bad to also have an unmap type that the cleanup routines could key off of, one of the options being do not unmap for clients that know what they are doing. I'd prefer that all clients know what they are doing ;-) 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: [RFC v2 2/5] dmaengine: Add slave DMA interface
On Wed, 6 Feb 2008 14:08:35 -0700 Dan Williams [EMAIL PROTECTED] wrote: On Jan 30, 2008 5:26 AM, Haavard Skinnemoen [EMAIL PROTECTED] wrote: [..] Right. I'll add a unsigned int engine_type field so that engine drivers can go ahead and extend the standard dma_device structure. Maybe we should add a void *platform_data field to the dma_slave struct as well so that platforms can pass arbitrary platform-specific information to the DMA controller driver? I think we can get away with not adding an engine_type field: 1/ For a given platform there will usually only be one driver active. For example I have an architecture (IOP) specific dma_copy_to_user implementation that can safely assume it is talking to the iop-adma driver since ioat_dma and others are precluded by the Kconfig. 2/ If there was a situation where two dma drivers were active in a system you could tell them apart by comparing the function pointers, i.e. dma_device1-device_prep_dma_memcpy != dma_device2-device_prep_dma_memcpy. What would you be comparing them against? Perhaps you could pass a struct device * from the platform code, which can be compared against dev in struct dma_device? Or you could check dma_device-dev-name perhaps. In any case, I agree we probably don't need the engine_type field. 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 -mm v4 6/9] atmel_serial: Split the interrupt handler
On Wed, 06 Feb 2008 14:41:09 +0100 michael <[EMAIL PROTECTED]> wrote: > I refer to this part of documentation: > > "The USART behavior when hardware handshaking is enabled is the same as > the behavior in > standard synchronous or asynchronous mode, except that the receiver > drives the RTS pin as > described below and the level on the CTS pin modifies the behavior of > the transmitter as > described below. Using this mode requires using the PDC channel for > reception. The transmitter > can handle hardware handshaking in any case." Oh. I guess the answer is no, then. 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 -mm v4 6/9] atmel_serial: Split the interrupt handler
On Tue, 05 Feb 2008 13:29:35 +0100 michael <[EMAIL PROTECTED]> wrote: > Just one question: > Receiving with hardware handshake works without PDC? I don't know...I haven't tried. These patches shouldn't change anything though. 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 -mm v4 6/9] atmel_serial: Split the interrupt handler
On Tue, 05 Feb 2008 13:29:35 +0100 michael [EMAIL PROTECTED] wrote: Just one question: Receiving with hardware handshake works without PDC? I don't know...I haven't tried. These patches shouldn't change anything though. 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 -mm v4 6/9] atmel_serial: Split the interrupt handler
On Wed, 06 Feb 2008 14:41:09 +0100 michael [EMAIL PROTECTED] wrote: I refer to this part of documentation: The USART behavior when hardware handshaking is enabled is the same as the behavior in standard synchronous or asynchronous mode, except that the receiver drives the RTS pin as described below and the level on the CTS pin modifies the behavior of the transmitter as described below. Using this mode requires using the PDC channel for reception. The transmitter can handle hardware handshaking in any case. Oh. I guess the answer is no, then. 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/
