* Re: [RFC] Clock binding
From: Benjamin Herrenschmidt @ 2009-08-28 2:43 UTC (permalink / raw)
To: Mitch Bradley; +Cc: linuxppc-dev list, devicetree-discuss
In-Reply-To: <4A974295.9020504@firmworks.com>
On Thu, 2009-08-27 at 16:36 -1000, Mitch Bradley wrote:
> The idea of a wiki as a registration authority is a good one, but I'm
> not volunteering to maintain it :-)
here goes my hope :-)
Do we have wiki's we could use on power.org or should we aim for a
community place ? Anybody has suggestions ? I wouldn't even try to
maintain a web site myself, that would be irresponsible of anybody to
let me do so !
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC] Clock binding
From: Mitch Bradley @ 2009-08-28 2:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list, devicetree-discuss
In-Reply-To: <1251420686.20467.96.camel@pasglop>
The idea of a wiki as a registration authority is a good one, but I'm
not volunteering to maintain it :-)
^ permalink raw reply
* Re: [RFC] Clock binding
From: Benjamin Herrenschmidt @ 2009-08-28 0:51 UTC (permalink / raw)
To: Mitch Bradley; +Cc: linuxppc-dev list, devicetree-discuss
In-Reply-To: <4A970C85.8090703@firmworks.com>
> I agree in general. It has long been a convention of mine to follow the
> vendor's names as exactly as possible. But that often presents
> difficulties. Many of them have been touched on in our previous
> discussion but I'll list some here just to emphasize the problem we face:
>
> a) Inconsistent naming within a vendor's documentation set - datasheet
> spells it one way, programmer's manual another, appnotes/porting guide
> still another, reference schematic spells it two different ways (pin
> name on part versus net name of signal wire).
Right. I should emphasis that what the device-tree should contain is the
pin name on part and -not- the net name of the wire. Of course, bogus
datasheet and inconsistent spelling will still hurt, nothing much we can
do about it, other than having the first to write a driver become a
de-facto "guide" to other implementors :-)
Maybe we could have a Wiki where the first time a given chip is used for
a binding, we put down the names used to encourage people to use the
same. To some extent it's going to be the responsibility of the person
writing the .dts to pick up names that will work with existing drivers.
In fact that problem is no different than picking up "compatible" names,
and that Wiki thing might help in both cases.
> b) Sometimes the name is abbreviated and sometimes spelled out. SMBALRT
> vs. SMbus Alert.
Right. Still better than a magic number where you simply have no
reference whatsoever to what it is.
> c) Different tools (CAD programs, word processors) have different
> conventions leading to existence of ambiguously-representable name
> components - particular cases in point are overbars for active low
> signals and embedded spaces/underscores/hyphens in names.
Yes, that can be an issue, though overbar is less common for clocks.
> d) Compatible part from different vendors leads to confusion about which
> vendor's names are canonical. Or leading vendor goes out of business or
> gets bought.
Sure, same problem with "compatible" properties. There's no silver
bullet here. But we could try the Wiki approach to provide an unofficial
"reference" of canonical names for common devices. For -very- common
ones such as 8250-compatible UARTs, a full blown binding is probably the
way to go.
> These problems are getting worse rapidly as more devices are being
> sourced from Asia, where the linguistic connection to the Roman alphabet
> is tenuous.
True.
> You need a Pope to decide what is canonical.
Perhaps. But we can try our best with something like a wiki in the
meantime and see how it works out.
Cheers,
Ben.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH] rtc: Set wakeup capability for I2C and SPI RTC drivers
From: Anton Vorontsov @ 2009-08-28 0:30 UTC (permalink / raw)
To: David Brownell
Cc: David Brownell, linux-kernel, linuxppc-dev, Ben Dooks,
Jean Delvare, Andrew Morton, linux-pm
In-Reply-To: <20090827231925.GA1131@oksana.dev.rtsoft.ru>
On Fri, Aug 28, 2009 at 03:19:25AM +0400, Anton Vorontsov wrote:
[...]
> > That is why platform code should device_init_wakeup() and
> > drivers should check device_can_wakeup(dev) ...
>
> They should (and do) check may_wakeup() (i.e. should_wakeup) before
> suspending, not can_wakeup().
>
> static int ds1374_suspend(struct i2c_client *client, pm_message_t state)
> {
> if (client->irq >= 0 && device_may_wakeup(&client->dev))
> enable_irq_wake(client->irq);
> return 0;
> }
>
> (quite funny, they issue enable_irq_wake(), assuming that otherwise
> IRQ line won't trigger CPU wakeup. But in reality, there are interrupt
> controllers that you can't control in that regard: any IRQ activity
> will always resume CPU. And so 'echo disable > /sys/.../wakeup' won't
> guarantee anything. Unreliable, nasty? Could be.)
BTW, of course we can fix this by masking interrupts before
suspending, but nobody actually do this (but should, I think).
And if RTC's IRQ is wired to power switch you're in trouble
without any way to fix this.
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH] rtc: Set wakeup capability for I2C and SPI RTC drivers
From: Anton Vorontsov @ 2009-08-27 23:19 UTC (permalink / raw)
To: David Brownell
Cc: David Brownell, linux-kernel, linuxppc-dev, Ben Dooks,
Jean Delvare, Andrew Morton, linux-pm
In-Reply-To: <200908271452.37358.david-b@pacbell.net>
On Thu, Aug 27, 2009 at 02:52:36PM -0700, David Brownell wrote:
[...]
> > I believe that it's not platform code responsibility to allow or
> > disallow wakeups, instead, drivers themselves should set the capability
> > if a device can trigger wakeups.
>
> Drivers can't generally know if that's possible though.
> They need to be told that it is, by platform code.
Um? Of course they know, DS1374 device driver knows that DS1374
chips can issue wakeups, what it doesn't know is if that wakeup
event will trigger CPU resume/power-up.
And even platform code doesn't know that. For example, on PowerPC
CPUs there could be several 'suspend' states, some allow wakeup
on particular IRQs, some don't. In some cases wakeup event will be
ignored, in other cases it will take effect. The suspend mode
depends on user's decision ('echo <mode> > /sys/power/state').
> Most devices can't issue wakeup events.
The device drivers I modified know that devices can issue wakeup
events.
> > That's what drivers/base/power/sysfs.c says:
> >
> > * It is the responsibility of device drivers to enable (or disable)
> > * wakeup signaling as part of changing device power states, respecting
> > * the policy choices provided through the driver model.
> >
> > I2C and SPI RTC devices send wakeup events via interrupt lines, so we
> > should set the wakeup capability if IRQ is routed.
>
> Re-read the quoted sentence. You're saying that policy
> choices should be hard-wired into the driver; contrary
> to that quote. (In this case the choice is one that
> platform code must report, and which the hardware
> designer made: if the device can issue wakeup events.)
The patch doesn't hard-wire the policy, quite the contrary: it
removes the "can't do wakeup" policy.
I'm just adding device_set_wakeup_capable() calls, telling everybody
that the devices can issue wakeup events (and they truly can).
Yes, it could be that the IRQ line is wired not to a CPU, but
to some power switch, and so nobody pass any IRQs to the drivers.
In that case platform-specific I2C_CLIENT_WAKE may be useful.
> > Ideally we should also check irq for wakeup capability before setting
> > device's capability, i.e.
> >
> > if (can_irq_wake(irq))
> > device_set_wakeup_capable(&client->dev, 1);
> >
> > But there is no can_irq_wake() call exist, and it is not that trivial
> > to implement it for all interrupts controllers and complex/cascaded
> > setups.
>
> That is why platform code should device_init_wakeup() and
> drivers should check device_can_wakeup(dev) ...
They should (and do) check may_wakeup() (i.e. should_wakeup) before
suspending, not can_wakeup().
static int ds1374_suspend(struct i2c_client *client, pm_message_t state)
{
if (client->irq >= 0 && device_may_wakeup(&client->dev))
enable_irq_wake(client->irq);
return 0;
}
(quite funny, they issue enable_irq_wake(), assuming that otherwise
IRQ line won't trigger CPU wakeup. But in reality, there are interrupt
controllers that you can't control in that regard: any IRQ activity
will always resume CPU. And so 'echo disable > /sys/.../wakeup' won't
guarantee anything. Unreliable, nasty? Could be.)
> > drivers/base/power/sysfs.c also covers these cases:
> >
> > * Devices may not be able to generate wakeup events from all power
> > * states. Also, the events may be ignored in some configurations;
> > * for example, they might need help from other devices that aren't
> > * active
> >
> > So there is no guarantee that wakeup will actually work,
>
> Yes there is ... it's only **exceptional** cases where it can't
> work. Your patch would make it routine that those flags be
> unreliable; pretty nasty.
It's not exceptional at all, you really can't tell if device's wakeup
event will take any effect. It depends on so many factors that it's
virtually impossible to account them all. And hiding wakeup capability
only because you aren't sure _is_ policy hard-wiring, no?
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [RFC] Clock binding
From: Mitch Bradley @ 2009-08-27 22:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list, devicetree-discuss
In-Reply-To: <1251411532.20467.74.camel@pasglop>
>
>> > One advantage of indices is that they avoid endless arguments about the
>> > exact name (and spelling) of things.
>>
>
> Right, though in that case, nobody gets to have to decide on the name,
> it comes from the chip manufacturer pin naming or data sheet.
>
>
I agree in general. It has long been a convention of mine to follow the
vendor's names as exactly as possible. But that often presents
difficulties. Many of them have been touched on in our previous
discussion but I'll list some here just to emphasize the problem we face:
a) Inconsistent naming within a vendor's documentation set - datasheet
spells it one way, programmer's manual another, appnotes/porting guide
still another, reference schematic spells it two different ways (pin
name on part versus net name of signal wire).
b) Sometimes the name is abbreviated and sometimes spelled out. SMBALRT
vs. SMbus Alert.
c) Different tools (CAD programs, word processors) have different
conventions leading to existence of ambiguously-representable name
components - particular cases in point are overbars for active low
signals and embedded spaces/underscores/hyphens in names.
d) Compatible part from different vendors leads to confusion about which
vendor's names are canonical. Or leading vendor goes out of business or
gets bought.
These problems are getting worse rapidly as more devices are being
sourced from Asia, where the linguistic connection to the Roman alphabet
is tenuous.
You need a Pope to decide what is canonical.
^ permalink raw reply
* Re: [RFC] Clock binding
From: Mitch Bradley @ 2009-08-27 22:28 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list, devicetree-discuss
In-Reply-To: <1251411532.20467.74.camel@pasglop>
>
>
>> > Open Firmware often avoids indexed structures. Cases in point include
>> > the use of named properties instead of fixed structures and named
>> > methods instead of function pointer arrays. Open Firmware's use of
>> > arrays for reg properties seems like the right choice for that
>> > particular case, but shouldn't be construed to suggest that arrays are
>> > good for everything.
>>
>
> Well, the "reg" property is fine for the common cases of devices with
> one IO (or MMIO) range, no confusion possible, or PCI since it encodes
> the BAR number. For other cases, especially random embedded stuff that
> maps several regions of memory, it's a bit harder since we go back to
> the need of having "somebody" define which region is which.
>
>
>
Indeed. You choose based on the common case and eventually there will
be some case that stretches the boundaries.
I suppose that, if the problem became severe enough, one could invent a
new "reg-names" property, a list of strings naming the reg entries.
^ permalink raw reply
* Re: [RFC] Clock binding
From: Benjamin Herrenschmidt @ 2009-08-27 22:18 UTC (permalink / raw)
To: Mitch Bradley; +Cc: linuxppc-dev list, devicetree-discuss
In-Reply-To: <4A96F69A.5050003@firmworks.com>
On Thu, 2009-08-27 at 11:11 -1000, Mitch Bradley wrote:
> I refrained from commenting as I didn't want to get involved in an
> endless argument about "goodness".
Oh well, I asked for it, didn't it ? :-)
> Indexed arrays are appropriate for some cases and names are better for
> others. Names are especially good for large spaces that are sparse or
> weakly-structured. The same is true for subroutine arguments. It's
> nice to be able to write setup_uart(baud=115200, flow_control=rts_cts),
> but you would go crazy (or become a COBOL programmer) if you had to name
> every argument to every subroutine.
Agreed.
> Open Firmware often avoids indexed structures. Cases in point include
> the use of named properties instead of fixed structures and named
> methods instead of function pointer arrays. Open Firmware's use of
> arrays for reg properties seems like the right choice for that
> particular case, but shouldn't be construed to suggest that arrays are
> good for everything.
Well, the "reg" property is fine for the common cases of devices with
one IO (or MMIO) range, no confusion possible, or PCI since it encodes
the BAR number. For other cases, especially random embedded stuff that
maps several regions of memory, it's a bit harder since we go back to
the need of having "somebody" define which region is which.
> One problem you run into with names is the "registration authority"
> problem.
Right.
> Who maintains the list of valid names to avoid collisions and
> to ensure consistent spelling? It's a solvable problem, but one that
> must be considered. Of course, a related problem exists with indices -
> what is the meaning of index value "N", and how do you manage the
> addition of new fields and deletion of others? Names are easier to
> manage in some cases and indices easier in others.
Maybe to some extent. I tend to prefer names whenever possible, but I
agree that the "central authority" problem is always around.
> In the particular case of a clock binding, I don't have enough
> experience with the problem details to have formed a strong opinion.
I'm in a similar situation, hence my request for comment. From my
observations though, clock inputs are generally named on a given chip,
IO core, SoC, etc... hence, I would suggest using a case insensitive
matching with a recommendation to use the name of the clock pin or clock
input signal in the case of IP cores as per the spec of the chip. This
sounds better than an arbitrary number. So at least for the naming of
clock inputs on a device, I do have a warm feeling with using names.
It has the added advantage from my selfish point of view which is that I
have to deal in Linux with an existing name based API and existing
drivers using it :-) Though the common case for 1 clock per device is to
pass NULL, which makes the problem go away.
In the same idea of simplifying the easy cases, you'll note that I made
it legit to have a clock provider simply have a "clock-frequency"
property and as such "fit" within existing practices for fixed frequency
non-programable clock sources.
Now, where I'm less comfortable is the naming on the clock provider
side. IE. My initial proposal was calling for binding named inputs to
indexed outputs on the provider. My new proposal makes that index a
simple convention in a given tree by having the provider also map those
to names since I didn't like the discrepancy here.
I suppose we could go roughly with the same idea, which is that the name
is case insensitive and matched on the part manufacturer's output
pin/signal name.
In any case, I'm sure what we do won't be absolutely perfect but it's
that or doing nothing and at this stage I believe we are in a situation
where even an imperfect solution will be greatly beneficial. In any
case, driver and architecture code can always work around problems such
as a device-tree providing a different name than what the driver
expects.
> Are there well-known clock names that will be used in common code that
> is shared among different vendors (e.g. "primary-clock")? If so, the
> binding should "preregister" a list of common names.
Well... I was more thinking about the signal name on the part. I want to
avoid using global clock names. IE. I want to avoid having to put in the
picture the name of the clock wires in the SoC or chip (SYSCLK,
PCICLK, ...) because typically those do vary from board to board and
vendor to vendor. But the name of the clock input on a device chip and
the name of the clock output on a PLL chip tend to be defined by the
datasheet of the said chip :-) Of course there are going to be
interesting variations such as compatible chips from different
manufacturers but, mostly we should cope better than magic indices.
> Will implementors of new hardware have to scratch their heads to decide
> what to name their clock inputs? The binding should offer some guidance
> about good name choices and spelling rules, to avoid an eventuall mess
> as new people come on board and pull names out of the air, with
> different conventions for capitalization and punctuation and abbreviation.
Should they ? IE. If the names come from the part datasheet, and we make
the matching case insensitive, I don't see that much need of the chip
vendors or board designers caring that much about rules for naming the
clocks.
> One advantage of indices is that they avoid endless arguments about the
> exact name (and spelling) of things.
Right, though in that case, nobody gets to have to decide on the name,
it comes from the chip manufacturer pin naming or data sheet.
> In my current project, there are
> several different hardware manuals for the same that must all be
> consulted to get the full picture, and they often use different names or
> inconsistent spellings for the same thing. It makes finding things very
> challenging.
Agreed about the general problem area.
> With a name-based interface, it pays to keep in mind that lots of people
> will ultimately be involved. Many of them will be new so they won't know
> the conventions well, and few will be careful and precise in their use
> of language (i.e. names). Provide a lot of guidance about how to choose
> a set of names.
And the easier the problem appears to be, the more people without a clue
about what they are talking about will feel compelled to intervene in
the discussion :-)
> Suppose there were a conventional set of names like "clock0", "clock1",
> ..., essentially boiling down to verbosely-spelled indices. I expect
> that a lot of people would choose to use it just to avoid having to
> thing of better names and having "bike shed" arguments with coworkers.
>
> So there you have it - my incoherent rambling commentary with no
> particular conclusion.
Nah, it's interesting, it also forces me to write down my thought with
better details.
Cheers,
Ben.
> > Cheers,
> > Ben.
> >
> >
> >> > I'm cooking up a patch that replace our current primitive implementation
> >> > in arch/powerpc/kernel/clock.c with something along the lines of what I
> >> > described. However, I want a bit more churn here on the device-tree
> >> > related bits.
> >> >
> >> > So, basically, the goal here is to define a binding so that we can link
> >> > a device clock inputs to a clock provider clock outputs.
> >> >
> >> > In general, in a system, there's actually 3 "names" involved. The clock
> >> > provider output name, the clock signal name, and the clock input name on
> >> > the device. However, I want to avoid involving the clock signal name as
> >> > it's a "global" name and it will just end up being a mess if we start
> >> > exposing that.
> >> >
> >> > So basically, it boils down to a device having some clock inputs,
> >> > referenced by names, that need to be linked to another node which is a
> >> > clock provider, which has outputs, references either by number or names,
> >> > see discussion below.
> >> >
> >> > First, why names, and not numbers ? IE. It's the OF "tradition" for
> >> > resources to just be an array, like interrupts, or address ranges in
> >> > "reg" properties, and one has to know what the Nth interrupt correspond
> >> > too.
> >> >
> >> > My answer here is that maybe the tradition but it's crap :-) Names are
> >> > much better in the long run, besides it makes it easier to represent if
> >> > not all inputs have been wired. Also, to some extent, things like PCI do
> >> > encode a "name" with "reg" or "assigned-addresses" properties as part of
> >> > the config space offset in the top part of the address, and that has
> >> > proved very useful.
> >> >
> >> > Thus I think using names is the way to go, and we should even generalize
> >> > that and add a new "interrupt-names" property to name the members of an
> >> > "interrupts" :-)
> >> >
> >> > So back to the subject at hand. That leaves us with having to populate
> >> > the driver with some kind of map (I call it clock-map). Ideally, if
> >> > everything is named, which is the best approach imho, that map would
> >> > bind a list of:
> >> >
> >> > - clock input name
> >> > - clock provider phandle
> >> > - clock output name on provider
> >> >
> >> > However, it's a bit nasty to mix strings and numbers (phandles) in a
> >> > single property. It's possible, but would likely lead to the phandle not
> >> > being aligned and tools such as lsprop to fail miserably to display
> >> > those properties in any kind of readable form.
> >> >
> >> > My earlier emails proposed an approach like this:
> >> >
> >> > - clock input names go into a "clock-names" property
> >> > (which I suggest naming instead "clock-input-names" btw)
> >> >
> >> > - the map goes into a "clock-map" property and for each input
> >> > provides a phandle and a one cell numerical ID that identifies
> >> > the clock on the source.
> >> >
> >> > However, I really dislike that numerical clock ID. Magic numbers suck.
> >> > It should be a string. But I don't want to add a 3rd property in there.
> >> >
> >> > Hence my idea below. It's not perfect but it's the less sucky i've come
> >> > up with so far. And then we can do some small refinements.
> >> >
> >> > * Device has:
> >> >
> >> > - "clock-input-names" as above
> >> > - "clock-map" contains list of phandle,index
> >> >
> >> > * Clock source has:
> >> >
> >> > - "clock-output-names" list of strings
> >> >
> >> > The "index" in the clock map thus would reference the
> >> > "clock-output-names" array in the clock provider. That means that the
> >> > "magic number" here is entirely local to a given device-tree, doesn't
> >> > leak into driver code, which continues using names.
> >> >
> >> > In addition, we can even have some smooth "upgrade" path from existing
> >> > "clock-frequency" properties by assuming that if "clock-output-names" is
> >> > absent, but "clock-frequency" exist, then index 0 references a fixed
> >> > frequency clock source without a driver. This could be generally handy
> >> > anyway to represent crystals of fixed bus clocks without having to write
> >> > a clock source driver for them.
> >> >
> >> > Any comments ?
> >> >
> >> > I'll post a patch, maybe later today, implementing the above (I may or
> >> > may not have time to also convert the existing 512x code to it, we'll
> >> > see).
> >> >
> >> > Cheers,
> >> > Ben.
> >> >
> >> >
> >> >
> >> >
> >> >
> >> > _______________________________________________
> >> > devicetree-discuss mailing list
> >> > devicetree-discuss@lists.ozlabs.org
> >> > https://lists.ozlabs.org/listinfo/devicetree-discuss
> >>
> >
> >
^ permalink raw reply
* Re: [PATCH] rtc: Set wakeup capability for I2C and SPI RTC drivers
From: David Brownell @ 2009-08-27 21:52 UTC (permalink / raw)
To: Anton Vorontsov
Cc: linux-kernel, linuxppc-dev, Ben Dooks, Jean Delvare,
Andrew Morton, linux-pm
In-Reply-To: <20090827182207.GA7358@oksana.dev.rtsoft.ru>
NAK; see details below
On Thursday 27 August 2009, Anton Vorontsov wrote:
> RTC core won't allow wakeup alarms to be set if RTC devices' parent
> (i.e. i2c_client or spi_device) isn't wakeup capable.
Quite rightly so ... being wakeup-capable is config-specific.
> For I2C devices there is I2C_CLIENT_WAKE flag exists that we can pass
> via board info, and if set, I2C core will initialize wakeup capability.
> For SPI devices there is no such flag at all.
So why not add it for SPI? If it's an issue, it's not
unique to RTC devices.
> I believe that it's not platform code responsibility to allow or
> disallow wakeups, instead, drivers themselves should set the capability
> if a device can trigger wakeups.
Drivers can't generally know if that's possible though.
They need to be told that it is, by platform code.
Most devices can't issue wakeup events.
> That's what drivers/base/power/sysfs.c says:
>
> * It is the responsibility of device drivers to enable (or disable)
> * wakeup signaling as part of changing device power states, respecting
> * the policy choices provided through the driver model.
>
> I2C and SPI RTC devices send wakeup events via interrupt lines, so we
> should set the wakeup capability if IRQ is routed.
Re-read the quoted sentence. You're saying that policy
choices should be hard-wired into the driver; contrary
to that quote. (In this case the choice is one that
platform code must report, and which the hardware
designer made: if the device can issue wakeup events.)
> Ideally we should also check irq for wakeup capability before setting
> device's capability, i.e.
>
> if (can_irq_wake(irq))
> device_set_wakeup_capable(&client->dev, 1);
>
> But there is no can_irq_wake() call exist, and it is not that trivial
> to implement it for all interrupts controllers and complex/cascaded
> setups.
That is why platform code should device_init_wakeup() and
drivers should check device_can_wakeup(dev) ...
> drivers/base/power/sysfs.c also covers these cases:
>
> * Devices may not be able to generate wakeup events from all power
> * states. Also, the events may be ignored in some configurations;
> * for example, they might need help from other devices that aren't
> * active
>
> So there is no guarantee that wakeup will actually work,
Yes there is ... it's only **exceptional** cases where it can't
work. Your patch would make it routine that those flags be
unreliable; pretty nasty.
^ permalink raw reply
* Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From: Benjamin Herrenschmidt @ 2009-08-27 21:28 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
Paul Mackerras, arun, Ingo Molnar, linuxppc-dev, Balbir Singh
In-Reply-To: <1251377607.18584.96.camel@twins>
On Thu, 2009-08-27 at 14:53 +0200, Peter Zijlstra wrote:
> I'm not quite seeing how this makes anything any better. Not we have 3
> function pointers, where 1 should suffice.
There's also the question of us having different "idle" vs.
"power_save", the former being the entire idle loop, the later being the
part that does put the processor into power.
At what level are we trying to change the loop here ?
There are some requirements of things to do in our idle loop that really
don't have their place in generic drivers/* code.
Ben.
> /me wonders what's wrong with something like:
>
> struct idle_func_desc {
> int power;
> int latency;
> void (*idle)(void);
> struct list_head list;
> };
>
> static void spin_idle(void)
> {
> for (;;)
> cpu_relax();
> }
>
> static idle_func_desc default_idle_func = {
> power = 0, /* doesn't safe any power */
> latency = INT_MAX, /* has max latency */
> idle = spin_idle,
> list = INIT_LIST_HEAD(default_idle_func.list),
> };
>
> void (*idle_func)(void);
> static struct list_head idle_func_list;
>
> static void pick_idle_func(void)
> {
> struct idle_func_desc *desc, *idle = &default_idle_desc;
>
> list_for_each_entry(desc, &idle_func_list, list) {
> if (desc->power < idle->power)
> continue;
> if (desc->latency > target_latency);
> continue;
> idle = desc;
> }
>
> pm_idle = idle->idle;
> }
>
> void register_idle_func(struct idle_func_desc *desc)
> {
> WARN_ON_ONCE(!list_empty(&desc->list));
>
> list_add_tail(&idle_func_list, &desc->list);
> pick_idle_func();
> }
>
> void unregister_idle_func(struct idle_func_desc *desc)
> {
> WARN_ON_ONCE(list_empty(&desc->list));
>
> list_del_init(&desc->list);
> if (idle_func == desc->idle)
> pick_idle_func();
> }
^ permalink raw reply
* Re: [RFC] Clock binding
From: Mitch Bradley @ 2009-08-27 21:11 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list, devicetree-discuss
In-Reply-To: <1251346157.20467.22.camel@pasglop>
>
> On Tue, 2009-08-18 at 14:21 +1000, Benjamin Herrenschmidt wrote:
>
>> > So here's a followup to my discussion about the clock API.
>>
>
> Really nobody has a comment here ? :-) Not even Mitch ?
>
I refrained from commenting as I didn't want to get involved in an
endless argument about "goodness".
Indexed arrays are appropriate for some cases and names are better for
others. Names are especially good for large spaces that are sparse or
weakly-structured. The same is true for subroutine arguments. It's
nice to be able to write setup_uart(baud=115200, flow_control=rts_cts),
but you would go crazy (or become a COBOL programmer) if you had to name
every argument to every subroutine.
Open Firmware often avoids indexed structures. Cases in point include
the use of named properties instead of fixed structures and named
methods instead of function pointer arrays. Open Firmware's use of
arrays for reg properties seems like the right choice for that
particular case, but shouldn't be construed to suggest that arrays are
good for everything.
One problem you run into with names is the "registration authority"
problem. Who maintains the list of valid names to avoid collisions and
to ensure consistent spelling? It's a solvable problem, but one that
must be considered. Of course, a related problem exists with indices -
what is the meaning of index value "N", and how do you manage the
addition of new fields and deletion of others? Names are easier to
manage in some cases and indices easier in others.
In the particular case of a clock binding, I don't have enough
experience with the problem details to have formed a strong opinion.
Are there well-known clock names that will be used in common code that
is shared among different vendors (e.g. "primary-clock")? If so, the
binding should "preregister" a list of common names.
Will implementors of new hardware have to scratch their heads to decide
what to name their clock inputs? The binding should offer some guidance
about good name choices and spelling rules, to avoid an eventuall mess
as new people come on board and pull names out of the air, with
different conventions for capitalization and punctuation and abbreviation.
One advantage of indices is that they avoid endless arguments about the
exact name (and spelling) of things. In my current project, there are
several different hardware manuals for the same that must all be
consulted to get the full picture, and they often use different names or
inconsistent spellings for the same thing. It makes finding things very
challenging.
With a name-based interface, it pays to keep in mind that lots of people
will ultimately be involved. Many of them will be new so they won't know
the conventions well, and few will be careful and precise in their use
of language (i.e. names). Provide a lot of guidance about how to choose
a set of names.
Suppose there were a conventional set of names like "clock0", "clock1",
..., essentially boiling down to verbosely-spelled indices. I expect
that a lot of people would choose to use it just to avoid having to
thing of better names and having "bike shed" arguments with coworkers.
So there you have it - my incoherent rambling commentary with no
particular conclusion.
> Cheers,
> Ben.
>
>
>> > I'm cooking up a patch that replace our current primitive implementation
>> > in arch/powerpc/kernel/clock.c with something along the lines of what I
>> > described. However, I want a bit more churn here on the device-tree
>> > related bits.
>> >
>> > So, basically, the goal here is to define a binding so that we can link
>> > a device clock inputs to a clock provider clock outputs.
>> >
>> > In general, in a system, there's actually 3 "names" involved. The clock
>> > provider output name, the clock signal name, and the clock input name on
>> > the device. However, I want to avoid involving the clock signal name as
>> > it's a "global" name and it will just end up being a mess if we start
>> > exposing that.
>> >
>> > So basically, it boils down to a device having some clock inputs,
>> > referenced by names, that need to be linked to another node which is a
>> > clock provider, which has outputs, references either by number or names,
>> > see discussion below.
>> >
>> > First, why names, and not numbers ? IE. It's the OF "tradition" for
>> > resources to just be an array, like interrupts, or address ranges in
>> > "reg" properties, and one has to know what the Nth interrupt correspond
>> > too.
>> >
>> > My answer here is that maybe the tradition but it's crap :-) Names are
>> > much better in the long run, besides it makes it easier to represent if
>> > not all inputs have been wired. Also, to some extent, things like PCI do
>> > encode a "name" with "reg" or "assigned-addresses" properties as part of
>> > the config space offset in the top part of the address, and that has
>> > proved very useful.
>> >
>> > Thus I think using names is the way to go, and we should even generalize
>> > that and add a new "interrupt-names" property to name the members of an
>> > "interrupts" :-)
>> >
>> > So back to the subject at hand. That leaves us with having to populate
>> > the driver with some kind of map (I call it clock-map). Ideally, if
>> > everything is named, which is the best approach imho, that map would
>> > bind a list of:
>> >
>> > - clock input name
>> > - clock provider phandle
>> > - clock output name on provider
>> >
>> > However, it's a bit nasty to mix strings and numbers (phandles) in a
>> > single property. It's possible, but would likely lead to the phandle not
>> > being aligned and tools such as lsprop to fail miserably to display
>> > those properties in any kind of readable form.
>> >
>> > My earlier emails proposed an approach like this:
>> >
>> > - clock input names go into a "clock-names" property
>> > (which I suggest naming instead "clock-input-names" btw)
>> >
>> > - the map goes into a "clock-map" property and for each input
>> > provides a phandle and a one cell numerical ID that identifies
>> > the clock on the source.
>> >
>> > However, I really dislike that numerical clock ID. Magic numbers suck.
>> > It should be a string. But I don't want to add a 3rd property in there.
>> >
>> > Hence my idea below. It's not perfect but it's the less sucky i've come
>> > up with so far. And then we can do some small refinements.
>> >
>> > * Device has:
>> >
>> > - "clock-input-names" as above
>> > - "clock-map" contains list of phandle,index
>> >
>> > * Clock source has:
>> >
>> > - "clock-output-names" list of strings
>> >
>> > The "index" in the clock map thus would reference the
>> > "clock-output-names" array in the clock provider. That means that the
>> > "magic number" here is entirely local to a given device-tree, doesn't
>> > leak into driver code, which continues using names.
>> >
>> > In addition, we can even have some smooth "upgrade" path from existing
>> > "clock-frequency" properties by assuming that if "clock-output-names" is
>> > absent, but "clock-frequency" exist, then index 0 references a fixed
>> > frequency clock source without a driver. This could be generally handy
>> > anyway to represent crystals of fixed bus clocks without having to write
>> > a clock source driver for them.
>> >
>> > Any comments ?
>> >
>> > I'll post a patch, maybe later today, implementing the above (I may or
>> > may not have time to also convert the existing 512x code to it, we'll
>> > see).
>> >
>> > Cheers,
>> > Ben.
>> >
>> >
>> >
>> >
>> >
>> > _______________________________________________
>> > devicetree-discuss mailing list
>> > devicetree-discuss@lists.ozlabs.org
>> > https://lists.ozlabs.org/listinfo/devicetree-discuss
>>
>
>
^ permalink raw reply
* Re: [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2009-08-27 20:55 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev list
In-Reply-To: <20090827110545.GJ5387@hansolo.jdub.homelinux.org>
On Thu, 2009-08-27 at 07:05 -0400, Josh Boyer wrote:
> On Thu, Aug 27, 2009 at 01:33:48PM +1000, Benjamin Herrenschmidt wrote:
> >Hi Linus !
> >
> >Here are a couple of last minute patches for 2.6.31. One is a regression fix
> >(afaik) where a PS3 driver gets incorrectly loaded on other platforms and
> >crashes, along with a PS3 defconfig update.
> >The following changes since commit f415c413f458837bd0c27086b79aca889f9435e4:
> > Linus Torvalds (1):
> > Merge git://git.kernel.org/.../davem/net-2.6
> >
> >are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
> >
> >Geert Uytterhoeven (1):
> > powerpc/ps3: Add missing check for PS3 to rtc-ps3 platform device registration
> >
> >Geoff Levand (1):
> > powerpc/ps3: Update ps3_defconfig
>
> Hm. No 44x flush_icache_range patch?
Well, the ps3 stuff is a regression that will crash the machine on boot
with some of the default configs... the 44x icache problem is not a
regression and will cause some obscure ltrace behaviour issues. So the
later is less high priority and I prefer having it in .32 and then send
back to -stable.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] fix book E watchdog to take WDIOC_SETTIMEOUT arg in seconds
From: Wim Van Sebroeck @ 2009-08-27 20:27 UTC (permalink / raw)
To: Josh Boyer; +Cc: Linux kernel, linuxppc-dev
In-Reply-To: <20090827120841.GA25303@zod.rchland.ibm.com>
Hi Josh,
> >Now we only need someone that can look at the CONFIG_4xx cases still :-)
>
> It seems the FSL watchdog is much more flexible than the one found in 4xx
> cores. On 4xx, you basically have 4 static choices that represent specific
> times determined by the clock frequency. I'm concerned that the lack of
> granularity here will result in less than desirable behavior.
>
> For example, with a 400MHz clock you would get choices of roughly:
>
> 5.2 ms
> 83.9 ms
> 1.34 s
> 21.47 s
>
> Personally, I consider the first two options basically unusable. Considering
> the second two, if a user were to say "Set the timeout for 2 seconds" they
> would then get a timeout of 21 seconds with the framework Chris' patch has
> set up. That doesn't really seem to be ideal to me.
Hmm, my opinion: in that case we should use a timer that triggers the watchdog until userspaces times out (like we do for other watchdogs allready). Maybe we should split this driver. I have the same issue with the Freescale i.mx driver that is under review also.
Kind regards,
Wim.
^ permalink raw reply
* Re: [PATCH 1/5] powerpc/qe: Increase MAX_QE_RISC to 4
From: Timur Tabi @ 2009-08-27 20:00 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20090827173011.GA739@oksana.dev.rtsoft.ru>
Anton Vorontsov wrote:
> MPC8569 CPUs have four QE RISCs, so we need to increase MAX_QE_RISC
> constant, otherwise qe_upload_firmware() fails at sanity checking.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
Acked-by: Timur Tabi <timur@freescale.com>
Anton, I'll review the rest of your patches next week. I'm too busy to do it now.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* [PATCH] rtc: Set wakeup capability for I2C and SPI RTC drivers
From: Anton Vorontsov @ 2009-08-27 18:22 UTC (permalink / raw)
To: Andrew Morton
Cc: David Brownell, linux-kernel, linuxppc-dev, Ben Dooks,
Jean Delvare, linux-pm
RTC core won't allow wakeup alarms to be set if RTC devices' parent
(i.e. i2c_client or spi_device) isn't wakeup capable.
For I2C devices there is I2C_CLIENT_WAKE flag exists that we can pass
via board info, and if set, I2C core will initialize wakeup capability.
For SPI devices there is no such flag at all.
I believe that it's not platform code responsibility to allow or
disallow wakeups, instead, drivers themselves should set the capability
if a device can trigger wakeups.
That's what drivers/base/power/sysfs.c says:
* It is the responsibility of device drivers to enable (or disable)
* wakeup signaling as part of changing device power states, respecting
* the policy choices provided through the driver model.
I2C and SPI RTC devices send wakeup events via interrupt lines, so we
should set the wakeup capability if IRQ is routed.
Ideally we should also check irq for wakeup capability before setting
device's capability, i.e.
if (can_irq_wake(irq))
device_set_wakeup_capable(&client->dev, 1);
But there is no can_irq_wake() call exist, and it is not that trivial
to implement it for all interrupts controllers and complex/cascaded
setups.
drivers/base/power/sysfs.c also covers these cases:
* Devices may not be able to generate wakeup events from all power
* states. Also, the events may be ignored in some configurations;
* for example, they might need help from other devices that aren't
* active
So there is no guarantee that wakeup will actually work, and so I think
there is no point in being pedantic wrt checking IRQ wakeup capability.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/rtc/rtc-ds1305.c | 2 ++
drivers/rtc/rtc-ds1307.c | 2 ++
drivers/rtc/rtc-ds1374.c | 2 ++
3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/rtc/rtc-ds1305.c b/drivers/rtc/rtc-ds1305.c
index 2736b11..e256c03 100644
--- a/drivers/rtc/rtc-ds1305.c
+++ b/drivers/rtc/rtc-ds1305.c
@@ -780,6 +780,8 @@ static int __devinit ds1305_probe(struct spi_device *spi)
spi->irq, status);
goto fail1;
}
+
+ device_set_wakeup_capable(&spi->dev, 1);
}
/* export NVRAM */
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 47a93c0..87097d4 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -881,6 +881,8 @@ read_rtc:
"unable to request IRQ!\n");
goto exit_irq;
}
+
+ device_set_wakeup_capable(&client->dev, 1);
set_bit(HAS_ALARM, &ds1307->flags);
dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
}
diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
index 713f7bf..5317bbc 100644
--- a/drivers/rtc/rtc-ds1374.c
+++ b/drivers/rtc/rtc-ds1374.c
@@ -383,6 +383,8 @@ static int ds1374_probe(struct i2c_client *client,
dev_err(&client->dev, "unable to request IRQ\n");
goto out_free;
}
+
+ device_set_wakeup_capable(&client->dev, 1);
}
ds1374->rtc = rtc_device_register(client->name, &client->dev,
--
1.6.3.3
^ permalink raw reply related
* [PATCH 4/5] ucc_geth: Remove UGETH_MAGIC_PACKET Kconfig symbol and code
From: Anton Vorontsov @ 2009-08-27 17:35 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linuxppc-dev, Scott Wood, Timur Tabi
This patch removes currently unused UGETH_MAGIC_PACKET Kconfig symbol
and code, i.e. magic_packet_detection_{enable,disable} functions.
The two functions each contain just two steps that we'll place into
suspend/resume code path under CONFIG_PM.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/Kconfig | 4 ----
drivers/net/ucc_geth.c | 32 --------------------------------
2 files changed, 0 insertions(+), 36 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 5f6509a..d171131 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2367,10 +2367,6 @@ config UCC_GETH
This driver supports the Gigabit Ethernet mode of the QUICC Engine,
which is available on some Freescale SOCs.
-config UGETH_MAGIC_PACKET
- bool "Magic Packet detection support"
- depends on UCC_GETH
-
config UGETH_TX_ON_DEMAND
bool "Transmit on Demand support"
depends on UCC_GETH
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 488b591..d4e51ee 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -437,38 +437,6 @@ static void hw_add_addr_in_hash(struct ucc_geth_private *ugeth,
QE_CR_PROTOCOL_ETHERNET, 0);
}
-#ifdef CONFIG_UGETH_MAGIC_PACKET
-static void magic_packet_detection_enable(struct ucc_geth_private *ugeth)
-{
- struct ucc_fast_private *uccf;
- struct ucc_geth __iomem *ug_regs;
-
- uccf = ugeth->uccf;
- ug_regs = ugeth->ug_regs;
-
- /* Enable interrupts for magic packet detection */
- setbits32(uccf->p_uccm, UCC_GETH_UCCE_MPD);
-
- /* Enable magic packet detection */
- setbits32(&ug_regs->maccfg2, MACCFG2_MPE);
-}
-
-static void magic_packet_detection_disable(struct ucc_geth_private *ugeth)
-{
- struct ucc_fast_private *uccf;
- struct ucc_geth __iomem *ug_regs;
-
- uccf = ugeth->uccf;
- ug_regs = ugeth->ug_regs;
-
- /* Disable interrupts for magic packet detection */
- clrbits32(uccf->p_uccm, UCC_GETH_UCCE_MPD);
-
- /* Disable magic packet detection */
- clrbits32(&ug_regs->maccfg2, MACCFG2_MPE);
-}
-#endif /* MAGIC_PACKET */
-
static inline int compare_addr(u8 **addr1, u8 **addr2)
{
return memcmp(addr1, addr2, ENET_NUM_OCTETS_PER_ADDRESS);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 5/5] ucc_geth: Implement suspend/resume and Wake-On-LAN support
From: Anton Vorontsov @ 2009-08-27 17:35 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linuxppc-dev, Scott Wood, Timur Tabi
This patch implements suspend/resume and WOL support for UCC Ethernet
driver.
We support two wake up events: wake on PHY/link changes and wake
on magic packet.
In some CPUs (like MPC8569) QE shuts down during sleep, so magic packet
detection is unusable, and also on resume we should fully reinitialize
UCC structures.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/ucc_geth.c | 85 ++++++++++++++++++++++++++++++++++++++++
drivers/net/ucc_geth.h | 1 +
drivers/net/ucc_geth_ethtool.c | 40 +++++++++++++++++++
3 files changed, 126 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index d4e51ee..33ed69e 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3481,6 +3481,10 @@ static int ucc_geth_open(struct net_device *dev)
napi_enable(&ugeth->napi);
netif_start_queue(dev);
+ device_set_wakeup_capable(&dev->dev,
+ qe_alive_during_sleep() || ugeth->phydev->irq);
+ device_set_wakeup_enable(&dev->dev, ugeth->wol_en);
+
return err;
err:
@@ -3545,6 +3549,85 @@ static void ucc_geth_timeout(struct net_device *dev)
schedule_work(&ugeth->timeout_work);
}
+
+#ifdef CONFIG_PM
+
+static int ucc_geth_suspend(struct of_device *ofdev, pm_message_t state)
+{
+ struct net_device *ndev = dev_get_drvdata(&ofdev->dev);
+ struct ucc_geth_private *ugeth = netdev_priv(ndev);
+
+ if (!netif_running(ndev))
+ return 0;
+
+ napi_disable(&ugeth->napi);
+
+ /*
+ * Disable the controller, otherwise we'll wakeup on any network
+ * activity.
+ */
+ ugeth_disable(ugeth, COMM_DIR_RX_AND_TX);
+
+ if (ugeth->wol_en & WAKE_MAGIC) {
+ setbits32(ugeth->uccf->p_uccm, UCC_GETH_UCCE_MPD);
+ setbits32(&ugeth->ug_regs->maccfg2, MACCFG2_MPE);
+ ucc_fast_enable(ugeth->uccf, COMM_DIR_RX_AND_TX);
+ } else if (!(ugeth->wol_en & WAKE_PHY)) {
+ phy_stop(ugeth->phydev);
+ }
+
+ return 0;
+}
+
+static int ucc_geth_resume(struct of_device *ofdev)
+{
+ struct net_device *ndev = dev_get_drvdata(&ofdev->dev);
+ struct ucc_geth_private *ugeth = netdev_priv(ndev);
+ int err;
+
+ if (!netif_running(ndev))
+ return 0;
+
+ if (qe_alive_during_sleep()) {
+ if (ugeth->wol_en & WAKE_MAGIC) {
+ ucc_fast_disable(ugeth->uccf, COMM_DIR_RX_AND_TX);
+ clrbits32(&ugeth->ug_regs->maccfg2, MACCFG2_MPE);
+ clrbits32(ugeth->uccf->p_uccm, UCC_GETH_UCCE_MPD);
+ }
+ ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
+ } else {
+ /*
+ * Full reinitialization is required if QE shuts down
+ * during sleep.
+ */
+ ucc_geth_memclean(ugeth);
+
+ err = ucc_geth_init_mac(ugeth);
+ if (err) {
+ ugeth_err("%s: Cannot initialize MAC, aborting.",
+ ndev->name);
+ return err;
+ }
+ }
+
+ ugeth->oldlink = 0;
+ ugeth->oldspeed = 0;
+ ugeth->oldduplex = -1;
+
+ phy_stop(ugeth->phydev);
+ phy_start(ugeth->phydev);
+
+ napi_enable(&ugeth->napi);
+ netif_start_queue(ndev);
+
+ return 0;
+}
+
+#else
+#define ucc_geth_suspend NULL
+#define ucc_geth_resume NULL
+#endif
+
static phy_interface_t to_phy_interface(const char *phy_connection_type)
{
if (strcasecmp(phy_connection_type, "mii") == 0)
@@ -3836,6 +3919,8 @@ static struct of_platform_driver ucc_geth_driver = {
.match_table = ucc_geth_match,
.probe = ucc_geth_probe,
.remove = ucc_geth_remove,
+ .suspend = ucc_geth_suspend,
+ .resume = ucc_geth_resume,
};
static int __init ucc_geth_init(void)
diff --git a/drivers/net/ucc_geth.h b/drivers/net/ucc_geth.h
index 195ab26..fee97ee 100644
--- a/drivers/net/ucc_geth.h
+++ b/drivers/net/ucc_geth.h
@@ -1220,6 +1220,7 @@ struct ucc_geth_private {
int oldspeed;
int oldduplex;
int oldlink;
+ int wol_en;
struct device_node *node;
};
diff --git a/drivers/net/ucc_geth_ethtool.c b/drivers/net/ucc_geth_ethtool.c
index 304128f..7075f26 100644
--- a/drivers/net/ucc_geth_ethtool.c
+++ b/drivers/net/ucc_geth_ethtool.c
@@ -359,6 +359,44 @@ uec_get_drvinfo(struct net_device *netdev,
drvinfo->regdump_len = uec_get_regs_len(netdev);
}
+#ifdef CONFIG_PM
+
+static void uec_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+ struct ucc_geth_private *ugeth = netdev_priv(netdev);
+ struct phy_device *phydev = ugeth->phydev;
+
+ if (phydev && phydev->irq)
+ wol->supported |= WAKE_PHY;
+ if (qe_alive_during_sleep())
+ wol->supported |= WAKE_MAGIC;
+
+ wol->wolopts = ugeth->wol_en;
+}
+
+static int uec_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+ struct ucc_geth_private *ugeth = netdev_priv(netdev);
+ struct phy_device *phydev = ugeth->phydev;
+
+ if (wol->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
+ return -EINVAL;
+ else if (wol->wolopts & WAKE_PHY && (!phydev || !phydev->irq))
+ return -EINVAL;
+ else if (wol->wolopts & WAKE_MAGIC && !qe_alive_during_sleep())
+ return -EINVAL;
+
+ ugeth->wol_en = wol->wolopts;
+ device_set_wakeup_enable(&netdev->dev, ugeth->wol_en);
+
+ return 0;
+}
+
+#else
+#define uec_get_wol NULL
+#define uec_set_wol NULL
+#endif /* CONFIG_PM */
+
static const struct ethtool_ops uec_ethtool_ops = {
.get_settings = uec_get_settings,
.set_settings = uec_set_settings,
@@ -377,6 +415,8 @@ static const struct ethtool_ops uec_ethtool_ops = {
.get_sset_count = uec_get_sset_count,
.get_strings = uec_get_strings,
.get_ethtool_stats = uec_get_ethtool_stats,
+ .get_wol = uec_get_wol,
+ .set_wol = uec_set_wol,
};
void uec_set_ethtool_ops(struct net_device *netdev)
--
1.6.3.3
^ permalink raw reply related
* [PATCH 3/5] ucc_geth: Factor out MAC initialization steps into a call
From: Anton Vorontsov @ 2009-08-27 17:35 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linuxppc-dev, Scott Wood, Timur Tabi
This patch factors out MAC initialization into ucc_geth_init_mac()
function that we'll use for suspend/resume.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/ucc_geth.c | 87 ++++++++++++++++++++++++++++-------------------
1 files changed, 52 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 3b957e6..488b591 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3413,46 +3413,25 @@ static int ucc_geth_set_mac_addr(struct net_device *dev, void *p)
return 0;
}
-/* Called when something needs to use the ethernet device */
-/* Returns 0 for success. */
-static int ucc_geth_open(struct net_device *dev)
+static int ucc_geth_init_mac(struct ucc_geth_private *ugeth)
{
- struct ucc_geth_private *ugeth = netdev_priv(dev);
+ struct net_device *dev = ugeth->ndev;
int err;
- ugeth_vdbg("%s: IN", __func__);
-
- /* Test station address */
- if (dev->dev_addr[0] & ENET_GROUP_ADDR) {
- if (netif_msg_ifup(ugeth))
- ugeth_err("%s: Multicast address used for station address"
- " - is this what you wanted?", __func__);
- return -EINVAL;
- }
-
- err = init_phy(dev);
- if (err) {
- if (netif_msg_ifup(ugeth))
- ugeth_err("%s: Cannot initialize PHY, aborting.",
- dev->name);
- return err;
- }
-
err = ucc_struct_init(ugeth);
if (err) {
if (netif_msg_ifup(ugeth))
- ugeth_err("%s: Cannot configure internal struct, aborting.", dev->name);
- goto out_err_stop;
+ ugeth_err("%s: Cannot configure internal struct, "
+ "aborting.", dev->name);
+ goto err;
}
- napi_enable(&ugeth->napi);
-
err = ucc_geth_startup(ugeth);
if (err) {
if (netif_msg_ifup(ugeth))
ugeth_err("%s: Cannot configure net device, aborting.",
dev->name);
- goto out_err;
+ goto err;
}
err = adjust_enet_interface(ugeth);
@@ -3460,7 +3439,7 @@ static int ucc_geth_open(struct net_device *dev)
if (netif_msg_ifup(ugeth))
ugeth_err("%s: Cannot configure net device, aborting.",
dev->name);
- goto out_err;
+ goto err;
}
/* Set MACSTNADDR1, MACSTNADDR2 */
@@ -3474,13 +3453,51 @@ static int ucc_geth_open(struct net_device *dev)
&ugeth->ug_regs->macstnaddr1,
&ugeth->ug_regs->macstnaddr2);
- phy_start(ugeth->phydev);
-
err = ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
if (err) {
if (netif_msg_ifup(ugeth))
ugeth_err("%s: Cannot enable net device, aborting.", dev->name);
- goto out_err;
+ goto err;
+ }
+
+ return 0;
+err:
+ ucc_geth_stop(ugeth);
+ return err;
+}
+
+/* Called when something needs to use the ethernet device */
+/* Returns 0 for success. */
+static int ucc_geth_open(struct net_device *dev)
+{
+ struct ucc_geth_private *ugeth = netdev_priv(dev);
+ int err;
+
+ ugeth_vdbg("%s: IN", __func__);
+
+ /* Test station address */
+ if (dev->dev_addr[0] & ENET_GROUP_ADDR) {
+ if (netif_msg_ifup(ugeth))
+ ugeth_err("%s: Multicast address used for station "
+ "address - is this what you wanted?",
+ __func__);
+ return -EINVAL;
+ }
+
+ err = init_phy(dev);
+ if (err) {
+ if (netif_msg_ifup(ugeth))
+ ugeth_err("%s: Cannot initialize PHY, aborting.",
+ dev->name);
+ return err;
+ }
+
+ err = ucc_geth_init_mac(ugeth);
+ if (err) {
+ if (netif_msg_ifup(ugeth))
+ ugeth_err("%s: Cannot initialize MAC, aborting.",
+ dev->name);
+ goto err;
}
err = request_irq(ugeth->ug_info->uf_info.irq, ucc_geth_irq_handler,
@@ -3489,16 +3506,16 @@ static int ucc_geth_open(struct net_device *dev)
if (netif_msg_ifup(ugeth))
ugeth_err("%s: Cannot get IRQ for net device, aborting.",
dev->name);
- goto out_err;
+ goto err;
}
+ phy_start(ugeth->phydev);
+ napi_enable(&ugeth->napi);
netif_start_queue(dev);
return err;
-out_err:
- napi_disable(&ugeth->napi);
-out_err_stop:
+err:
ucc_geth_stop(ugeth);
return err;
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH 2/5] powerpc/qe: Implement qe_alive_during_sleep() helper function
From: Anton Vorontsov @ 2009-08-27 17:35 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linuxppc-dev, Scott Wood, Timur Tabi
In some CPUs (i.e. MPC8569) QE shuts down completely during sleep,
drivers may want to know that to reinitialize registers and buffer
descriptors.
This patch implements qe_alive_during_sleep() helper function, so far
it just checks if MPC8569-compatible power management controller is
present, which is a sign that QE turns off during sleep.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/include/asm/qe.h | 1 +
arch/powerpc/sysdev/qe_lib/qe.c | 13 +++++++++++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/qe.h b/arch/powerpc/include/asm/qe.h
index e8232bb..20c0b07 100644
--- a/arch/powerpc/include/asm/qe.h
+++ b/arch/powerpc/include/asm/qe.h
@@ -163,6 +163,7 @@ int qe_get_snum(void);
void qe_put_snum(u8 snum);
unsigned int qe_get_num_of_risc(void);
unsigned int qe_get_num_of_snums(void);
+int qe_alive_during_sleep(void);
/* we actually use cpm_muram implementation, define this for convenience */
#define qe_muram_init cpm_muram_init
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index f485d5a..a765f9c 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -65,6 +65,19 @@ static unsigned int qe_num_of_snum;
static phys_addr_t qebase = -1;
+int qe_alive_during_sleep(void)
+{
+ static int ret = -1;
+
+ if (ret != -1)
+ return ret;
+
+ ret = !of_find_compatible_node(NULL, NULL, "fsl,mpc8569-pmc");
+
+ return ret;
+}
+EXPORT_SYMBOL(qe_alive_during_sleep);
+
phys_addr_t get_qe_base(void)
{
struct device_node *qe;
--
1.6.3.3
^ permalink raw reply related
* [PATCH 1/5] ucc_geth: Fix NULL pointer dereference in uec_get_ethtool_stats()
From: Anton Vorontsov @ 2009-08-27 17:35 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linuxppc-dev, Scott Wood, Timur Tabi
In commit 3e73fc9a12679a546284d597c1f19165792d0b83 ("ucc_geth: Fix IO
memory (un)mapping code") I fixed ug_regs IO memory leak by properly
freeing the allocated memory. But ethtool_stats() callback doesn't
check for ug_regs being NULL, and that causes following oops if
'ethtool -S' is executed on a closed eth device:
Unable to handle kernel paging request for data at address 0x00000180
Faulting instruction address: 0xc0208228
Oops: Kernel access of bad area, sig: 11 [#1]
...
NIP [c0208228] uec_get_ethtool_stats+0x38/0x140
LR [c02559a0] ethtool_get_stats+0xf8/0x23c
Call Trace:
[ef87bcd0] [c025597c] ethtool_get_stats+0xd4/0x23c (unreliable)
[ef87bd00] [c025706c] dev_ethtool+0xfe8/0x11bc
[ef87be00] [c0252b5c] dev_ioctl+0x454/0x6a8
...
---[ end trace 77fff1162a9586b0 ]---
Segmentation fault
This patch fixes the issue.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/ucc_geth_ethtool.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ucc_geth_ethtool.c b/drivers/net/ucc_geth_ethtool.c
index 61fe80d..304128f 100644
--- a/drivers/net/ucc_geth_ethtool.c
+++ b/drivers/net/ucc_geth_ethtool.c
@@ -319,9 +319,13 @@ static void uec_get_ethtool_stats(struct net_device *netdev,
int i, j = 0;
if (stats_mode & UCC_GETH_STATISTICS_GATHERING_MODE_HARDWARE) {
- base = (u32 __iomem *)&ugeth->ug_regs->tx64;
+ if (ugeth->ug_regs)
+ base = (u32 __iomem *)&ugeth->ug_regs->tx64;
+ else
+ base = NULL;
+
for (i = 0; i < UEC_HW_STATS_LEN; i++)
- data[j++] = in_be32(&base[i]);
+ data[j++] = base ? in_be32(&base[i]) : 0;
}
if (stats_mode & UCC_GETH_STATISTICS_GATHERING_MODE_FIRMWARE_TX) {
base = (u32 __iomem *)ugeth->p_tx_fw_statistics_pram;
--
1.6.3.3
^ permalink raw reply related
* [PATCH 5/5] powerpc/85xx: Add power management support for MPC85xxMDS boards
From: Anton Vorontsov @ 2009-08-27 17:30 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
- Add power management controller nodes;
- Add interrupts for RTC nodes, the RTC interrupt may be used as a
wakeup source;
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/boot/dts/mpc8568mds.dts | 15 +++++++++++++--
arch/powerpc/boot/dts/mpc8569mds.dts | 13 ++++++++++++-
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 1 +
3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8568mds.dts b/arch/powerpc/boot/dts/mpc8568mds.dts
index 00c2bbd..4d18ca3 100644
--- a/arch/powerpc/boot/dts/mpc8568mds.dts
+++ b/arch/powerpc/boot/dts/mpc8568mds.dts
@@ -107,6 +107,8 @@
rtc@68 {
compatible = "dallas,ds1374";
reg = <0x68>;
+ interrupts = <3 1>;
+ interrupt-parent = <&mpic>;
};
};
@@ -252,10 +254,19 @@
interrupt-parent = <&mpic>;
};
- global-utilities@e0000 { //global utilities block
- compatible = "fsl,mpc8548-guts";
+ global-utilities@e0000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,mpc8568-guts", "fsl,mpc8548-guts";
reg = <0xe0000 0x1000>;
+ ranges = <0 0xe0000 0x1000>;
fsl,has-rstcr;
+
+ pmc: power@80 {
+ compatible = "fsl,mpc8568-pmc",
+ "fsl,mpc8548-pmc";
+ reg = <0x80 0x10>;
+ };
};
serial1: serial@4600 {
diff --git a/arch/powerpc/boot/dts/mpc8569mds.dts b/arch/powerpc/boot/dts/mpc8569mds.dts
index 880f896..88d96b8 100644
--- a/arch/powerpc/boot/dts/mpc8569mds.dts
+++ b/arch/powerpc/boot/dts/mpc8569mds.dts
@@ -171,6 +171,8 @@
rtc@68 {
compatible = "dallas,ds1374";
reg = <0x68>;
+ interrupts = <3 1>;
+ interrupt-parent = <&mpic>;
};
};
@@ -304,9 +306,18 @@
};
global-utilities@e0000 {
- compatible = "fsl,mpc8569-guts";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,mpc8569-guts", "fsl,mpc8548-guts";
reg = <0xe0000 0x1000>;
+ ranges = <0 0xe0000 0x1000>;
fsl,has-rstcr;
+
+ pmc: power@80 {
+ compatible = "fsl,mpc8569-pmc",
+ "fsl,mpc8548-pmc";
+ reg = <0x80 0x10>;
+ };
};
par_io@e0100 {
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 20a61d0..995ddad 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -300,6 +300,7 @@ static struct of_device_id mpc85xx_ids[] = {
{ .compatible = "fsl,qe", },
{ .compatible = "gianfar", },
{ .compatible = "fsl,rapidio-delta", },
+ { .compatible = "fsl,mpc8548-guts", },
{},
};
--
1.6.3.3
^ permalink raw reply related
* [PATCH 4/5] powerpc/85xx: Add suspend/resume support
From: Anton Vorontsov @ 2009-08-27 17:30 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
This patch adds suspend/resume support for MPC8540-compatible and
MPC8569 CPUs.
MPC8540-compatible PMCs are trivial: we just write SLP bit into PM
control and status register.
MPC8569 is a bit trickier, QE turns off during suspend and so on
resume we must reload QE microcode firmware and reset QE.
So far we don't support Deep Sleep mode as found in newer MPC85xx
CPUs (i.e. MPC8536). It can be relatively easy implemented though,
and for it we reserve 'mem' suspend type.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/Kconfig | 2 +-
arch/powerpc/platforms/85xx/Makefile | 1 +
arch/powerpc/platforms/85xx/suspend.c | 115 +++++++++++++++++++++++++++++++++
3 files changed, 117 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/suspend.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d00131c..46ebfe6 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -212,7 +212,7 @@ config ARCH_HIBERNATION_POSSIBLE
config ARCH_SUSPEND_POSSIBLE
def_bool y
- depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx
+ depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || PPC_85xx
config PPC_DCR_NATIVE
bool
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index 835733f..cd1ad6e 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -2,6 +2,7 @@
# Makefile for the PowerPC 85xx linux kernel.
#
obj-$(CONFIG_SMP) += smp.o
+obj-$(CONFIG_SUSPEND) += suspend.o
obj-$(CONFIG_MPC8540_ADS) += mpc85xx_ads.o
obj-$(CONFIG_MPC8560_ADS) += mpc85xx_ads.o
diff --git a/arch/powerpc/platforms/85xx/suspend.c b/arch/powerpc/platforms/85xx/suspend.c
new file mode 100644
index 0000000..d4ca5e2
--- /dev/null
+++ b/arch/powerpc/platforms/85xx/suspend.c
@@ -0,0 +1,115 @@
+/*
+ * Suspend/resume support
+ *
+ * Copyright © 2009 MontaVista Software, Inc.
+ *
+ * Author: Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/suspend.h>
+#include <linux/device.h>
+#include <linux/of_platform.h>
+#include <linux/firmware.h>
+#include <asm/qe.h>
+
+struct pmc_regs {
+ __be32 pmcsr;
+#define PMCSR_SLP (1 << 17)
+};
+
+struct pmc_data {
+ unsigned int flags;
+#define PMC_NEED_QE_RELOAD (1 << 0)
+
+ const char *fw_name;
+};
+
+static struct device *pmc_dev;
+static struct pmc_regs __iomem *pmc_regs;
+static const struct pmc_data *pmc_data;
+static struct qe_firmware *pmc_qefw;
+
+static int pmc_suspend_enter(suspend_state_t state)
+{
+ out_be32(&pmc_regs->pmcsr, PMCSR_SLP);
+
+ if (pmc_qefw) {
+ int ret;
+
+ ret = qe_upload_firmware(pmc_qefw);
+ if (ret)
+ dev_err(pmc_dev, "could not upload firmware\n");
+
+ qe_reset();
+ }
+ return 0;
+}
+
+static int pmc_suspend_valid(suspend_state_t state)
+{
+ if (state != PM_SUSPEND_STANDBY)
+ return 0;
+
+ if (pmc_data && pmc_data->flags & PMC_NEED_QE_RELOAD && !pmc_qefw) {
+ const struct firmware *fw;
+ int ret;
+
+ ret = request_firmware(&fw, pmc_data->fw_name, pmc_dev);
+ if (ret) {
+ dev_err(pmc_dev, "could not request firmware %s\n",
+ pmc_data->fw_name);
+ return 0;
+ }
+
+ pmc_qefw = (struct qe_firmware *)fw->data;
+ }
+
+ return 1;
+}
+
+static struct platform_suspend_ops pmc_suspend_ops = {
+ .valid = pmc_suspend_valid,
+ .enter = pmc_suspend_enter,
+};
+
+static int pmc_probe(struct of_device *ofdev, const struct of_device_id *id)
+{
+ pmc_regs = of_iomap(ofdev->node, 0);
+ if (!pmc_regs)
+ return -ENOMEM;
+
+ pmc_dev = &ofdev->dev;
+ pmc_data = id->data;
+ suspend_set_ops(&pmc_suspend_ops);
+ return 0;
+}
+
+static struct pmc_data mpc8569_pmc_data = {
+ .flags = PMC_NEED_QE_RELOAD,
+ .fw_name = "fsl_qe_ucode_8569.bin",
+};
+
+static const struct of_device_id pmc_ids[] = {
+ { .compatible = "fsl,mpc8569-pmc", .data = &mpc8569_pmc_data, },
+ { .compatible = "fsl,mpc8548-pmc", },
+ { },
+};
+
+static struct of_platform_driver pmc_driver = {
+ .driver.name = "mpc85xx-pmc",
+ .match_table = pmc_ids,
+ .probe = pmc_probe,
+};
+
+static int pmc_init(void)
+{
+ return of_register_platform_driver(&pmc_driver);
+}
+device_initcall(pmc_init);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 2/5] powerpc/qe: Make qe_reset() code path safe for repeated invocation
From: Anton Vorontsov @ 2009-08-27 17:30 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
For MPC8569 CPUs we'll need to reset QE after each suspend, so make
qe_reset() code path suitable for repeated invocation, that is:
- Don't initialize rheap structures if already initialized;
- Don't allocate muram for SDMA if already allocated, just reinitialize
registers with previously allocated muram offset;
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/sysdev/cpm_common.c | 3 +++
arch/powerpc/sysdev/qe_lib/qe.c | 10 ++++++----
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c
index e4b6d66..ddfca52 100644
--- a/arch/powerpc/sysdev/cpm_common.c
+++ b/arch/powerpc/sysdev/cpm_common.c
@@ -81,6 +81,9 @@ int __init cpm_muram_init(void)
int i = 0;
int ret = 0;
+ if (muram_pbase)
+ return 0;
+
spin_lock_init(&cpm_muram_lock);
/* initialize the info header */
rh_init(&cpm_muram_info, 1,
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index b06564f..f485d5a 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -317,16 +317,18 @@ EXPORT_SYMBOL(qe_put_snum);
static int qe_sdma_init(void)
{
struct sdma __iomem *sdma = &qe_immr->sdma;
- unsigned long sdma_buf_offset;
+ static unsigned long sdma_buf_offset;
if (!sdma)
return -ENODEV;
/* allocate 2 internal temporary buffers (512 bytes size each) for
* the SDMA */
- sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
- if (IS_ERR_VALUE(sdma_buf_offset))
- return -ENOMEM;
+ if (!sdma_buf_offset) {
+ sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
+ if (IS_ERR_VALUE(sdma_buf_offset))
+ return -ENOMEM;
+ }
out_be32(&sdma->sdebcr, (u32) sdma_buf_offset & QE_SDEBCR_BA_MASK);
out_be32(&sdma->sdmr, (QE_SDMR_GLB_1_MSK |
--
1.6.3.3
^ permalink raw reply related
* [PATCH 3/5] powerpc/qe: Add qe_upload_firmware() stub for non-QE builds
From: Anton Vorontsov @ 2009-08-27 17:30 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
This is needed to avoid #ifdefs in MPC85xx suspend/resume code.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/include/asm/qe.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/qe.h b/arch/powerpc/include/asm/qe.h
index 791c67a..e8232bb 100644
--- a/arch/powerpc/include/asm/qe.h
+++ b/arch/powerpc/include/asm/qe.h
@@ -218,8 +218,15 @@ struct qe_firmware_info {
u64 extended_modes; /* Extended modes */
};
+#ifdef CONFIG_QUICC_ENGINE
/* Upload a firmware to the QE */
int qe_upload_firmware(const struct qe_firmware *firmware);
+#else
+static inline int qe_upload_firmware(const struct qe_firmware *firmware)
+{
+ return -ENOSYS;
+}
+#endif /* CONFIG_QUICC_ENGINE */
/* Obtain information on the uploaded firmware */
struct qe_firmware_info *qe_get_firmware_info(void);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 1/5] powerpc/qe: Increase MAX_QE_RISC to 4
From: Anton Vorontsov @ 2009-08-27 17:30 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
MPC8569 CPUs have four QE RISCs, so we need to increase MAX_QE_RISC
constant, otherwise qe_upload_firmware() fails at sanity checking.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/sysdev/qe_lib/qe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index 237e365..b06564f 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -336,7 +336,7 @@ static int qe_sdma_init(void)
}
/* The maximum number of RISCs we support */
-#define MAX_QE_RISC 2
+#define MAX_QE_RISC 4
/* Firmware information stored here for qe_get_firmware_info() */
static struct qe_firmware_info qe_firmware_info;
--
1.6.3.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox