LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 6/7] Holly DTS
From: Josh Boyer @ 2007-05-05 18:12 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <e1d22829fe3f12509018791541073706@kernel.crashing.org>

On Sat, 2007-05-05 at 19:21 +0200, Segher Boessenkool wrote:
> > +/ {
> > +	model = "ppc750-tsi109";
> 
> "model" should be something really specific; typically
> a (unique!) model number.  This means you can't use the
> same device tree for Holly and Hickory (but there are
> more reasons for that; see below).
> 
> > +	compatible = "ppc750-tsi";
> 
> The needs to be more specific as well; "ibm,holly"
> or something.

Why?  Holly and Hickory share the same memory map and devices.  The only
thing that differs from what I can tell is the actual CPU itself.

> 
> > +		PowerPC,750@0 {
> 
> Needs to be PowerPC,750CL etc.
> 
> > +			name = "PowerPC,750";
> 
> You don't need to specify "name" in modern DTS.

Ok.

> > +  	tsi109@c0000000 {
> > +		device_type = "tsi-bridge";
> 
> Don't put a "device_type" here, it is useless
> (and undefined).  There are more like this, but
> perhaps Linux (wrongly) probes on "device_type"
> for those, so the kernel would need updating
> first.

It's not useless.  The TSI code probes by device_type all over the
place.  Do you have an example of how to probe for something like this
without using device_type?

> > +		bus-frequency = <0>;
> 
> 0?

Can likely go.

> 
> > +		ethernet@6200 {
> > +			model = "TSI-ETH";
> 
> "model" should be an exact model name.  If you don't
> have one, just leave out the "model" property.

OK.

> 
> > +			address = [ 00 0d 60 f4 3f 04 ];
> 
> "local-mac-address" instead.  And you don't want to
> hardcode one specific address like this; just keep
> it open so if the bootloader fails to fill it in you
> notice.  If the bootloader cannot do that, just leave
> out this property completely (not compliant, but neither
> is filling in the wrong value).

Ok.

> 
> > +		serial@7808 {
> > +			reg-shift = <0>;
> 
> This is the default value so you can leave it out.

Ok.

> 
> > +	  	MPIC: pic@7400 {
> > +			built-in;
> 
> Why say this for MPIC only, not for most other nodes?
> What binding defines "built-in", anyway?

http://playground.sun.com/1275/bindings/chrp/chrp1_8a.ps

Page 34.

> 
> > +				/*--------------------------------------------+
> > +				| AD19.
> > +				+--------------------------------------------*/
> > +				1800 0 0 1 &RT0 26 0
> > +				1800 0 0 2 &RT0 27 0
> > +				1800 0 0 3 &RT0 24 0
> > +				1800 0 0 4 &RT0 25 0
> > +
> > +				2000 0 0 1 &RT0 27 0
> > +				2000 0 0 2 &RT0 24 0
> > +				2000 0 0 3 &RT0 25 0
> > +				2000 0 0 4 &RT0 26 0
> > +				>;
> 
> You forgot to say "AD20" ;-)

Heh, ok.

> 
> > +	chosen {
> > +		linux,platform = <0>;
> > +		linux,initrd-start = <0>;
> > +		linux,initrd-end = <0>;
> 
> Do you need to set those zero properties?

Perhaps not.  I'll look at that.

josh

^ permalink raw reply

* Re: [PATCH 5/6] Support for the Ebony 440GP reference board in arch/powerpc
From: Milton Miller @ 2007-05-05 18:43 UTC (permalink / raw)
  To: Olof Johansson; +Cc: ppcdev, David Gibson
In-Reply-To: <20070504143645.GA10645@lixom.net>

Olof wrote:

> > +static void ebony_exit(void)
> > +{
> > +     unsigned long tmp;
> > +
> > +     asm volatile (
> > +             "mfspr  %0,%1\n"
> > +             "oris   %0,%0,%2@h\n"
> > +             "mtspr  %1,%0"
> > +             : "=&r"(tmp) : "i"(SPRN_DBCR0), "i"(DBCR0_RST_SYSTEM)
>
> You don't have to pass in the constants here, you can specify them in
> the asm. Makes it a little more readable.

How?   CPP doesn't apply inside the strings.

Did you mean because these defines were immediately above:

> > +> +#define SPRN_DBCR0           0x134
> > +#define   DBCR0_RST_SYSTEM   0x30000000

could just be comments in the asm?

That works until someone moves them to an include file.

milton

^ permalink raw reply

* Re: [PATCH 5/6] Support for the Ebony 440GP reference board in arch/powerpc
From: Olof Johansson @ 2007-05-05 19:02 UTC (permalink / raw)
  To: Milton Miller; +Cc: ppcdev, David Gibson
In-Reply-To: <00bf8e702554e7903d92f618ee236528@bga.com>

On Sat, May 05, 2007 at 01:43:01PM -0500, Milton Miller wrote:
> Olof wrote:
> 
> > > +static void ebony_exit(void)
> > > +{
> > > +     unsigned long tmp;
> > > +
> > > +     asm volatile (
> > > +             "mfspr  %0,%1\n"
> > > +             "oris   %0,%0,%2@h\n"
> > > +             "mtspr  %1,%0"
> > > +             : "=&r"(tmp) : "i"(SPRN_DBCR0), "i"(DBCR0_RST_SYSTEM)
> >
> > You don't have to pass in the constants here, you can specify them in
> > the asm. Makes it a little more readable.
> 
> How?   CPP doesn't apply inside the strings.
> 
> Did you mean because these defines were immediately above:
> 
> > > +> +#define SPRN_DBCR0           0x134
> > > +#define   DBCR0_RST_SYSTEM   0x30000000
> 
> could just be comments in the asm?
> 
> That works until someone moves them to an include file.

Hm, I thought i had done that in the past myself. I guess not. Nevermind
then.


-Olof

^ permalink raw reply

* Re: [PATCH v2 6/7] Holly DTS
From: Olof Johansson @ 2007-05-05 19:13 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <e1d22829fe3f12509018791541073706@kernel.crashing.org>

On Sat, May 05, 2007 at 07:21:33PM +0200, Segher Boessenkool wrote:

> > +		PowerPC,750@0 {
> 
> Needs to be PowerPC,750CL etc.

Why _needs_? Apple never used anything else than PowerPC,G5, no matter
what generation chip.

> > +  	tsi109@c0000000 {
> > +		device_type = "tsi-bridge";
> 
> Don't put a "device_type" here, it is useless
> (and undefined).  There are more like this, but
> perhaps Linux (wrongly) probes on "device_type"
> for those, so the kernel would need updating
> first.

Josh is just using the precedence that the other platform gives, the
driver is keying off of device_type. While that should be fixed, that's
a separate issue. I'm sure they'd welcome a driver patch from you.

(Does anyone know if there's an actual firmware out there with the
tsi-bridge device_type set, and thus just changing DTS's won't help? We'd
still need to be able to boot/probe the device on those firmwares.)

> > +		bus-frequency = <0>;
> 
> 0?

Filled in by bootloader/wrapper? Other dts'es are similar. Could be nice
to have a comment next to it if that's the case.


-Olof

^ permalink raw reply

* Re: [PATCH v2 6/7] Holly DTS
From: Josh Boyer @ 2007-05-05 19:46 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20070505191306.GB23524@lixom.net>

On Sat, 2007-05-05 at 14:13 -0500, Olof Johansson wrote:
> > > +  	tsi109@c0000000 {
> > > +		device_type = "tsi-bridge";
> > 
> > Don't put a "device_type" here, it is useless
> > (and undefined).  There are more like this, but
> > perhaps Linux (wrongly) probes on "device_type"
> > for those, so the kernel would need updating
> > first.
> 
> Josh is just using the precedence that the other platform gives, the
> driver is keying off of device_type. While that should be fixed, that's
> a separate issue. I'm sure they'd welcome a driver patch from you.

Right.  I'm not saying it _has_ to be device_type or anything.  It's
simply what the current in-tree functions key off of.  And if that needs
to be fixed, that's fine too.  I just don't think it's a prerequisite
for getting the Holly platform patches merged.

> 
> (Does anyone know if there's an actual firmware out there with the
> tsi-bridge device_type set, and thus just changing DTS's won't help? We'd
> still need to be able to boot/probe the device on those firmwares.)

I'm not sure.  I know PIBS doesn't _currently_ do anything with the DT
on this board, but I think work is on-going there.  As for the FSL
board, I have no idea.  Perhaps U-Boot is doing something there?

> 
> > > +		bus-frequency = <0>;
> > 
> > 0?
> 
> Filled in by bootloader/wrapper? Other dts'es are similar. Could be nice
> to have a comment next to it if that's the case.

No, it's just leftover.  It can be pulled.

josh

^ permalink raw reply

* Re: powerpc_flash_init(), wtf!?
From: Segher Boessenkool @ 2007-05-05 20:19 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <463CC0A4.5040407@ru.mvista.com>

>> But if the flash really is on an external bus controlled by a bus
>> controller on the SoC, it sounds like it should go under that bus
>> bridge.  In which case the SoC would need another range in its ranges
>> property.
>
>    Erm, how multiple memory ranges are supposed to work? Aren't the 
> addresses in the "reg" property of subnodes relative to the "ranges" 
> property?

If the address space of a child bus is a subset (perhaps
after some mapping) of the address space of the parent bus,
the "ranges" property in the parent node describes that
subset (and mapping).  If the address spaces are identical,
this is expressed by an empty "ranges" property.  If there
is no such mapping (indirect register access, for example),
this is expressed by the absence of a "ranges" property.

There can be multiple ranges in a "ranges" property, that's
why it's called "ranges" and not "range".  A common example
is the "ranges" property in PCI bus nodes (if those have
both a memory and legacy I/O range; for host bridges, this
means that both those ranges are direct mapped).

I hope this answered your question, it was a bit vague.


Segher

^ permalink raw reply

* Re: [PATCH 5/6] Support for the Ebony 440GP reference board in arch/powerpc
From: Segher Boessenkool @ 2007-05-05 20:37 UTC (permalink / raw)
  To: Milton Miller; +Cc: Olof Johansson, ppcdev, David Gibson
In-Reply-To: <00bf8e702554e7903d92f618ee236528@bga.com>

>>> +static void ebony_exit(void)
>>> +{
>>> +     unsigned long tmp;
>>> +
>>> +     asm volatile (
>>> +             "mfspr  %0,%1\n"
>>> +             "oris   %0,%0,%2@h\n"
>>> +             "mtspr  %1,%0"
>>> +             : "=&r"(tmp) : "i"(SPRN_DBCR0), "i"(DBCR0_RST_SYSTEM)
>>
>> You don't have to pass in the constants here, you can specify them in
>> the asm. Makes it a little more readable.
>
> How?   CPP doesn't apply inside the strings.

So put it outside the strings:

+     asm volatile (
+             "mfspr  %0," #SPRN_DBCR0 "\n\t"
+             "oris   %0,%0," #DBCR0_RST_SYSTEM "@h\n\t"
+             "mtspr  " #SPRN_DBCR0 ",%0"
+             : "=&r"(tmp));


Segher

^ permalink raw reply

* Re: [PATCH v2 6/7] Holly DTS
From: Segher Boessenkool @ 2007-05-05 20:44 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <1178388746.3393.47.camel@zod.rchland.ibm.com>

>>> +/ {
>>> +	model = "ppc750-tsi109";
>>
>> "model" should be something really specific; typically
>> a (unique!) model number.  This means you can't use the
>> same device tree for Holly and Hickory (but there are
>> more reasons for that; see below).
>>
>>> +	compatible = "ppc750-tsi";
>>
>> The needs to be more specific as well; "ibm,holly"
>> or something.
>
> Why?  Holly and Hickory share the same memory map and devices.  The 
> only
> thing that differs from what I can tell is the actual CPU itself.

And the model number I suspect.

"ppc750-tsi" really isn't good enough as a "compatible"
property, there are many many more boards with some 750
and some TSI bridge.

>>> +  	tsi109@c0000000 {
>>> +		device_type = "tsi-bridge";
>>
>> Don't put a "device_type" here, it is useless
>> (and undefined).  There are more like this, but
>> perhaps Linux (wrongly) probes on "device_type"
>> for those, so the kernel would need updating
>> first.
>
> It's not useless.  The TSI code probes by device_type all over the
> place.  Do you have an example of how to probe for something like this
> without using device_type?

Sure: use "compatible" instead.

This problem is all over the place, don't worry
about it too much.  It would be good if new ports
could stop adding to the madness though ;-)

>>> +	  	MPIC: pic@7400 {
>>> +			built-in;
>>
>> Why say this for MPIC only, not for most other nodes?
>> What binding defines "built-in", anyway?
>
> http://playground.sun.com/1275/bindings/chrp/chrp1_8a.ps

But this isn't a CHRP compatible board, that binding
doesn't apply.


Segher

^ permalink raw reply

* Re: [PATCH v2 6/7] Holly DTS
From: Segher Boessenkool @ 2007-05-05 20:49 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20070505191306.GB23524@lixom.net>

>>> +		PowerPC,750@0 {
>>
>> Needs to be PowerPC,750CL etc.
>
> Why _needs_?

Because that's what the PowerPC binding says:

	Open Firmware standard property. The value of the
	is property shall be of the form: =93PowerPC,<name>=94,
	where <name> is the name of the processor chip which
	may be displayed to the user.


> Apple never used anything else than PowerPC,G5, no matter
> what generation chip.

I don't care what Apple did in their device trees,
they made many mistakes.

>>> +  	tsi109@c0000000 {
>>> +		device_type =3D "tsi-bridge";
>>
>> Don't put a "device_type" here, it is useless
>> (and undefined).  There are more like this, but
>> perhaps Linux (wrongly) probes on "device_type"
>> for those, so the kernel would need updating
>> first.
>
> Josh is just using the precedence that the other platform gives, the
> driver is keying off of device_type. While that should be fixed, =
that's
> a separate issue.

Yes I know, I tried to explain that already, but
seems I failed :-)

> I'm sure they'd welcome a driver patch from you.

It's on my todo list.

>>> +		bus-frequency =3D <0>;
>>
>> 0?
>
> Filled in by bootloader/wrapper?

I don't think it is?

> Other dts'es are similar. Could be nice
> to have a comment next to it if that's the case.

Yeah.  There's also this ?? syntax now I believe.


Segher

^ permalink raw reply

* Re: [PATCH v2 6/7] Holly DTS
From: Segher Boessenkool @ 2007-05-05 20:51 UTC (permalink / raw)
  To: Josh Boyer; +Cc: Olof Johansson, linuxppc-dev
In-Reply-To: <1178394364.3393.54.camel@zod.rchland.ibm.com>

> Right.  I'm not saying it _has_ to be device_type or anything.  It's
> simply what the current in-tree functions key off of.  And if that 
> needs
> to be fixed, that's fine too.  I just don't think it's a prerequisite
> for getting the Holly platform patches merged.

Sure, I didn't say "NAK" or anything :-)


Segher

^ permalink raw reply

* Re: [PATCH v2 6/7] Holly DTS
From: Josh Boyer @ 2007-05-05 21:36 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <76e38f13b63a2cf4c793299e73671db5@kernel.crashing.org>

On Sat, 2007-05-05 at 22:44 +0200, Segher Boessenkool wrote:
> >>> +/ {
> >>> +	model = "ppc750-tsi109";
> >>
> >> "model" should be something really specific; typically
> >> a (unique!) model number.  This means you can't use the
> >> same device tree for Holly and Hickory (but there are
> >> more reasons for that; see below).
> >>
> >>> +	compatible = "ppc750-tsi";
> >>
> >> The needs to be more specific as well; "ibm,holly"
> >> or something.
> >
> > Why?  Holly and Hickory share the same memory map and devices.  The 
> > only
> > thing that differs from what I can tell is the actual CPU itself.
> 
> And the model number I suspect.
> 
> "ppc750-tsi" really isn't good enough as a "compatible"
> property, there are many many more boards with some 750
> and some TSI bridge.

Ok.  It really doesn't matter one way or another to me, since it's
something I'm defining anyway.  "ibm,holly" and/or "ibm,hickory" or
"ibm,ppc750-tsi" would likely work.

Actually, I might muck around with the PVR in the bootwrapper and poke
the correct model and compatible nodes in there.  That might work out
well enough.

> >>> +  	tsi109@c0000000 {
> >>> +		device_type = "tsi-bridge";
> >>
> >> Don't put a "device_type" here, it is useless
> >> (and undefined).  There are more like this, but
> >> perhaps Linux (wrongly) probes on "device_type"
> >> for those, so the kernel would need updating
> >> first.
> >
> > It's not useless.  The TSI code probes by device_type all over the
> > place.  Do you have an example of how to probe for something like this
> > without using device_type?
> 
> Sure: use "compatible" instead.
> 
> This problem is all over the place, don't worry
> about it too much.  It would be good if new ports
> could stop adding to the madness though ;-)

For now, I'm inclined to leave it as is to minimize the amount of things
needed to actually get Holly merged.  However, I'll start looking at
doing it via compatible shortly.  Hopefully someone will be able to test
out the mpc7448hpc2 board as I go to make sure things are breaking
there.

> >>> +	  	MPIC: pic@7400 {
> >>> +			built-in;
> >>
> >> Why say this for MPIC only, not for most other nodes?
> >> What binding defines "built-in", anyway?
> >
> > http://playground.sun.com/1275/bindings/chrp/chrp1_8a.ps
> 
> But this isn't a CHRP compatible board, that binding
> doesn't apply.

I'm not pretending it is.  You just asked which binding defined it and I
was proud of myself for actually finding one ;).

It doesn't actually do anything in this case, so it can be removed.

josh

^ permalink raw reply

* Re: virtex_devices.c
From: Grant Likely @ 2007-05-05 22:16 UTC (permalink / raw)
  To: David H. Lynch Jr., linuxppc-embedded
In-Reply-To: <463C379D.7020804@dlasys.net>

On 5/5/07, David H. Lynch Jr. <dhlii@dlasys.net> wrote:
>    I am in the process of migrating my BSP code - which is loosely
> based on your earlier xilinx_mlxxx code to your newer code.
>    I am pulling your code from your virtex-dev branch of your git tree.
>
>
>    The first big obstacle I have encountered is the assumption that any
> early serial device will be an 8250.
>    When I altered your earlier code to deal with other early serial
> devices (my BSP supports both the UartLite and a pseudo serial device
> used when the Pico card is hosted called the Keyhole).
>
>     I made 2 significant changes:
>           I completely eliminated the use of plat_serial8250 in the
> platform data, and used uart_port as the structure for platform data.
>     It had everything I needed for 8250's and could be messaged for most
> anything else I had to deal with.
>     I explicitly set the type field in the structure so that I could use
> it do decide the approriate init routine to call.
>
>     I do not like this - as adding additional potential early serial
> devices means creating an ever increasing set of tests.
>     But it was better that only supporting the 8250.

I think I understand what you're describing, but I'd need to see your
code snippits to be sure.  Can you send me the relevant patches?

>     At some point if I get inspired I may take a wack at crafting
> generic approach using an early serial mini driver.
>
>     But for the moment my choices are:
>             add a few more 8250 specific #ifdefs to kill off most of the
> conflicting code in virtex_devices.c
>     and adapt my own code that is already in my BSP. This is the
> simplest and does the least damage to your code.

Considering the fact that the whole purpose of virtex_devices.c is to
massage the mess of #defines that is xparameters.h in to something
more parsable, then extra #if defined() test may not be a big deal.

However, I need clarification.  Are you talking about early serial
port support, console support or regular serial devices?  For example;
on one of my boards here I've got both 16550 and uartlite devices on
the same board and it works fine for serial port access.  I've also
got another design with only uartlite and another with only 16550.
Each of these scenarios should work with the code that is now in
mainline (but there are some issues still).

plat_serial8250_port is used because it is the structure used by the
platform bus to connect the 8250 driver to 8250 devices.  This is of
course specific to the 8250 driver.  The Uartlite ports are simple
enough that they don't need a *_platform_data structure; instead the
resources table in a virtex_platform_devices[] entry is sufficient to
describe the device to the platform bus.  You should be able to do the
same thing with your keyhole device registration.

Now, early serial is still a problem.  When using zImages, you must
make sure that only one of CONFIG_SERIAL_UARTLITE_CONSOLE and
CONFIG_SERIAL_8250_CONSOLE is set.  If they are both set, then it will
not compile.  (But this is just a zImage boot wrapper issue.  The
kernel proper should work fine).  Also, early serial in the kernel
proper is not implemented in driver which is in mainline, but console
and regular serial access is supported.

To add early serial support, I think virtex_early_serial_map() should
be reworked to scan the whole virtex_platform_devices table looking
for either "uartlite", "serial8250" or "keyhole" devices.

>              Or restructure your code in virtex_devices so that it uses
> uart_port as the pdata format and has support to
>     initialize devices other than the 8250. Which I think is the better
> way to go, but before I go stomping all over your
>     serial code I thought I was ask your thoughts ?

Go ahead, stomp away.  Worst that can happen is that I disagree.  :)
I certainly won't be offended.

>
>              Is there some compelling reason to use plat_serial8250 as
> the structure in which the port data is stored  as opposed to
>     something more generic ?

plat_serial8250 isn't supposed to be generic.  The generic bit is
supposed to be the virtex_platform_devices table.  BTW, my personal
opinion is that plat_serial8250 is poorly implemented because it maps
multiple 8250 devices to a single platform_device.  Instead, it should
be a 1-1 mapping (like all the other devices in the
virtex_platform_devices table).  ie. Don't try to describe all serial
devices, regardless of type, in a single platform_device record.

>              Do you have an objection to my making the changes needed to
> switch to the uart_port struct as the initialization data ?

It doesn't smell right; but I'm not sure I fully understand what
you're suggesting.  Show me patches.

>
>              Do you have any objection to my adding additional choices
> such as UartLite to the virtex_devices serial initialization ?

Absolutely not.  This is a good thing.

>     Do you have a different direction you would prefer to see things go
> - aside from moving to device trees and/or implimenting
>     a more generic early serial driver,  both of which I don't think I
> want to try at this instant ?

As I described above.

Now, all this being said; we do need to migrate to arch/powerpc
relatively soon.  If the changes are not too complex or if they ease
our migration to arch/powerpc, then I'm all for it.  If you're
undertaking a major development effort of stuff that's arch/ppc only;
then I recommend against it.  :-)

Oh, and I'd *really* like to see pico support hit mainline.  Easiest
way to do this is to break of chunks and tackle them one at a time
which is easier than trying to get the whole patchset in at once.
Keyhole driver sounds like a good place to start.

Cheers,
g.


-- 
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [PATCH 3/13] powerpc: Add bootwrapper support for Marvell/mv64x60 hostbridge
From: Paul Mackerras @ 2007-05-05 23:27 UTC (permalink / raw)
  To: Mark A. Greer; +Cc: linuxppc-dev
In-Reply-To: <20070503184402.GA5600@mag.az.mvista.com>

Mark A. Greer writes:

> Actually, none of the config code should be needed by the bootwrapper
> itself because the bootwrapper mpsc and i2c drivers use PIO (so accessing
> memory isn't an issue).  The "get info" type routines are still needed.

Talking of i2c, why does the bootwrapper need an i2c driver at all?
You don't tell me in the patch description for your patch 5/13...

> Its pretty much 1:1.  Whatever is in the bootwrapper (including
> #define's) isn't needed in the kernel.

OK, good.

> I expect that the setup will be done the same for all of the boards
> that need it.  Not all of the boards will need it, though.  It depends
> on their firmware.  The config code is filling the gap between what the
> firmware should have done and what the kernel drivers require (i.e.,
> allowing the ctlrs to access system memory and pci mem/io space).

Is it possible that different kernels will want things set up
differently?  Or is there really only one sensible setup?

> Paul, I'm a little perplexed by you wanting this code pushed back into
> the kernel.  A while back there was a definite desire to push stuff like
> this out of the kernel (mainly by Ben).  I tend to agree with Ben on
> this.  Either way, it would good to have everyone pushing in the same
> direction. :)

One of these days you guys will write me nice informative patch
descriptions that explain the rationale and design decisions behind
the changes you're proposing, without me nagging. :) I'm not so much
pushing back as trying to get you to tell me why the code is here and
not somewhere else, and what the obvious alternatives might be and why
we aren't doing them.  Or better still, put that information into a
new set of patch descriptions for me...

BTW, the descriptions on your string of 13 were completely empty for
many of them, and for the remainder they just said what had been
changed.  Please be more informative.  Yes, it does require thinking
about things from the point of view of someone who doesn't have all
the context in their head, as you do, which can take a bit of mental
effort.  But in 2 or 3 years time it might be you that doesn't have
the context in mind any more and will really appreciate a more verbose
description. :)

Paul.

^ permalink raw reply

* Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
From: Vitaly Bordug @ 2007-05-05 23:27 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linuxppc-dev, linux-pcmcia, linux-kernel
In-Reply-To: <20070504123543.5e3fd908.akpm@linux-foundation.org>

On Fri, 4 May 2007 12:35:43 -0700
Andrew Morton wrote:

> On Fri, 04 May 2007 03:57:51 +0400
> Vitaly Bordug <vitb@kernel.crashing.org> wrote:
> 
> > 
> > Adds support for PowerQuicc on-chip PCMCIA. The driver is
> > implemented as of_device, so only arch/powerpc stuff is capable to
> > use it, which now implies only mpc885ads reference board.
> > 
> > To cope with the code that should be hooked inside driver, but is
> > really board specific (like set_voltage), global structure
> > mpc8xx_pcmcia_ops holds necessary function pointers that are filled
> > in the BSP code.
> > 
> 
> argh.
> 
> akpm:/home/akpm> grep '^.*        ' x | wc -l 
> 72
> 
> please, Linux uses hard-tabs, not
> spacespacespacespacespacespacespacespace everywhere.
>

Whoops. That must've survived being copypasted from the original m8xx_pcmcia.c.
That reminds me to do Lindent on the affected sources but that is subject for another patch.
Sorry for the hassle. 

Apparently all the issues were correct, and I'll follow-up with the reworked patch. Thanks for looking at it.

-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: [PATCH 1/4] Add support for 750CL Holly board
From: Arnd Bergmann @ 2007-05-06  0:09 UTC (permalink / raw)
  To: Josh Boyer; +Cc: Olof Johansson, linuxppc-dev
In-Reply-To: <1178377874.3393.17.camel@zod.rchland.ibm.com>

On Saturday 05 May 2007, Josh Boyer wrote:
> > 
> > It would be very easy to take tsi108_setup_pci take the config base as
> > a function paramter and pass it in from the board setup code, and store
> > it in a static variable to be used for the tsi108_clear_pci_cfg_error
> > function.
> > 
> > That'd easily make the code multiplatform friendly without having to
> > add it to the device tree. Very few other platforms have the config base
> > address in there, if any. It shouldn't be a requirement that Holly has it.
> 
> Yeah, after thinking about it, that's what I plan on doing actually.
> 

Yes, sounds fair enough. The more interesting bits to clean up in the
driver are to convert it to an of_platform_driver and to use phylib, and
then autodetecting this will be trivial, and until then hardcoding in the
board setup seems a good solution.

	Arnd <><

^ permalink raw reply

* Re: [PATCH 5/6] Support for the Ebony 440GP reference board in arch/powerpc
From: David Gibson @ 2007-05-06  0:43 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Olof Johansson, ppcdev, Milton Miller
In-Reply-To: <1a1a143ae0292ea0a6006a26fb95a446@kernel.crashing.org>

On Sat, May 05, 2007 at 10:37:37PM +0200, Segher Boessenkool wrote:
> >>> +static void ebony_exit(void)
> >>> +{
> >>> +     unsigned long tmp;
> >>> +
> >>> +     asm volatile (
> >>> +             "mfspr  %0,%1\n"
> >>> +             "oris   %0,%0,%2@h\n"
> >>> +             "mtspr  %1,%0"
> >>> +             : "=&r"(tmp) : "i"(SPRN_DBCR0), "i"(DBCR0_RST_SYSTEM)
> >>
> >> You don't have to pass in the constants here, you can specify them in
> >> the asm. Makes it a little more readable.
> >
> > How?   CPP doesn't apply inside the strings.
> 
> So put it outside the strings:
> 
> +     asm volatile (
> +             "mfspr  %0," #SPRN_DBCR0 "\n\t"
> +             "oris   %0,%0," #DBCR0_RST_SYSTEM "@h\n\t"
> +             "mtspr  " #SPRN_DBCR0 ",%0"
> +             : "=&r"(tmp));

Um, I think stringify(SPRN_DBCR0) is needed there, not just a bare #.
At which point it's not entirely clear to be that just using the "i"
constraint isn't the simplest option after all.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
From: Vitaly Bordug @ 2007-05-06  0:47 UTC (permalink / raw)
  To: linux-pcmcia; +Cc: linuxppc-dev, linux-kernel


Adds support for PowerQuicc on-chip PCMCIA. The driver is implemented as
of_device, so only arch/powerpc stuff is capable to use it, which now
implies only mpc885ads reference board.

To cope with the code that should be hooked inside driver, but is really
board specific (like set_voltage), global structure mpc8xx_pcmcia_ops
holds necessary function pointers that are filled in the BSP code.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>

---

 arch/powerpc/boot/dts/mpc885ads.dts          |   12 +
 arch/powerpc/platforms/8xx/m8xx_setup.c      |    5 
 arch/powerpc/platforms/8xx/mpc885ads.h       |    8 +
 arch/powerpc/platforms/8xx/mpc885ads_setup.c |   76 ++++++
 arch/powerpc/sysdev/fsl_soc.c                |   13 +
 arch/powerpc/sysdev/mpc8xx_pic.h             |    2 
 drivers/pcmcia/Kconfig                       |    1 
 drivers/pcmcia/m8xx_pcmcia.c                 |  352 ++++++++++++--------------
 include/asm-powerpc/mpc8xx.h                 |    4 
 include/linux/fsl_devices.h                  |    5 
 10 files changed, 292 insertions(+), 186 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc885ads.dts b/arch/powerpc/boot/dts/mpc885ads.dts
index 110bf61..0f3e8cf 100644
--- a/arch/powerpc/boot/dts/mpc885ads.dts
+++ b/arch/powerpc/boot/dts/mpc885ads.dts
@@ -112,6 +112,18 @@
 			compatible = "CPM";
 		};
 
+		pcmcia@0080 {
+			linux,phandle = <0080>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			compatible = "fsl,pq-pcmcia";
+			device_type = "pcmcia";
+			reg = <80 80>;
+			clock-frequency = <2faf080>;
+			interrupt-parent = <ff000000>;
+			interrupts = <d 1>;
+		};
+
 		cpm@ff000000 {
 			linux,phandle = <ff000000>;
 			#address-cells = <1>;
diff --git a/arch/powerpc/platforms/8xx/m8xx_setup.c b/arch/powerpc/platforms/8xx/m8xx_setup.c
index 0901dba..f169355 100644
--- a/arch/powerpc/platforms/8xx/m8xx_setup.c
+++ b/arch/powerpc/platforms/8xx/m8xx_setup.c
@@ -32,6 +32,7 @@
 #include <linux/root_dev.h>
 #include <linux/time.h>
 #include <linux/rtc.h>
+#include <linux/fsl_devices.h>
 
 #include <asm/mmu.h>
 #include <asm/reg.h>
@@ -49,6 +50,10 @@
 
 #include "sysdev/mpc8xx_pic.h"
 
+#ifdef CONFIG_PCMCIA_M8XX
+struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops;
+#endif
+
 void m8xx_calibrate_decr(void);
 extern void m8xx_wdt_handler_install(bd_t *bp);
 extern int cpm_pic_init(void);
diff --git a/arch/powerpc/platforms/8xx/mpc885ads.h b/arch/powerpc/platforms/8xx/mpc885ads.h
index 7c31aec..932b59a 100644
--- a/arch/powerpc/platforms/8xx/mpc885ads.h
+++ b/arch/powerpc/platforms/8xx/mpc885ads.h
@@ -91,5 +91,13 @@
 #define SICR_ENET_MASK	((uint)0x00ff0000)
 #define SICR_ENET_CLKRT	((uint)0x002c0000)
 
+/* 
+ * Some internal interrupt registers use an 8-bit mask for the interrupt
+ * level instead of a number.
+ */
+static inline uint mk_int_int_mask(uint mask) {
+	return (1 << (7 - (mask/2)));
+}
+
 #endif /* __ASM_MPC885ADS_H__ */
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/platforms/8xx/mpc885ads_setup.c b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
index a57b577..4e76e1c 100644
--- a/arch/powerpc/platforms/8xx/mpc885ads_setup.c
+++ b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
@@ -22,6 +22,7 @@
 
 #include <linux/fs_enet_pd.h>
 #include <linux/fs_uart_pd.h>
+#include <linux/fsl_devices.h>
 #include <linux/mii.h>
 
 #include <asm/delay.h>
@@ -51,6 +52,11 @@ static void init_smc1_uart_ioports(struct fs_uart_platform_info* fpi);
 static void init_smc2_uart_ioports(struct fs_uart_platform_info* fpi);
 static void init_scc3_ioports(struct fs_platform_info* ptr);
 
+#ifdef CONFIG_PCMCIA_M8XX
+static void pcmcia_hw_setup(int slot, int enable);
+static int pcmcia_set_voltage(int slot, int vcc, int vpp);
+#endif
+
 void __init mpc885ads_board_setup(void)
 {
 	cpm8xx_t *cp;
@@ -115,6 +121,12 @@ void __init mpc885ads_board_setup(void)
 	immr_unmap(io_port);
 
 #endif
+
+#ifdef CONFIG_PCMCIA_M8XX
+	/*Set up board specific hook-ups*/
+	m8xx_pcmcia_ops.hw_ctrl = pcmcia_hw_setup;
+	m8xx_pcmcia_ops.voltage_set = pcmcia_set_voltage;
+#endif
 }
 
 
@@ -322,6 +334,70 @@ void init_smc_ioports(struct fs_uart_platform_info *data)
 	}
 }
 
+#ifdef CONFIG_PCMCIA_M8XX
+static void pcmcia_hw_setup(int slot, int enable)
+{
+	unsigned *bcsr_io;
+
+	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
+	if (enable)
+		clrbits32(bcsr_io, BCSR1_PCCEN);
+	else
+		setbits32(bcsr_io, BCSR1_PCCEN);
+
+	iounmap(bcsr_io);
+}
+
+static int pcmcia_set_voltage(int slot, int vcc, int vpp)
+{
+	u32 reg = 0;
+	unsigned *bcsr_io;
+
+	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
+
+	switch(vcc) {
+	case 0:
+		break;
+	case 33:
+		reg |= BCSR1_PCCVCC0;
+		break;
+	case 50:
+		reg |= BCSR1_PCCVCC1;
+		break;
+	default:
+		return 1;
+	}
+
+	switch(vpp) {
+	case 0:
+       	break;
+	case 33:
+	case 50:
+       	if(vcc == vpp)
+			reg |= BCSR1_PCCVPP1;
+		else
+			return 1;
+		break;
+	case 120:
+	if ((vcc == 33) || (vcc == 50))
+		reg |= BCSR1_PCCVPP0;
+	else
+		return 1;
+	default:
+		return 1;
+	}
+
+	/* first, turn off all power */
+	clrbits32(bcsr_io, 0x00610000);
+
+	/* enable new powersettings */
+	setbits32(bcsr_io, reg);
+
+	iounmap(bcsr_io);
+	return 0;
+}
+#endif
+
 int platform_device_skip(const char *model, int id)
 {
 #ifdef CONFIG_MPC8xx_SECOND_ETH_SCC3
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 8a123c7..880e45f 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -1028,6 +1028,19 @@ err:
 
 arch_initcall(fs_enet_of_init);
 
+static int __init fsl_pcmcia_of_init(void)
+{
+	struct device_node *np = NULL;
+	/*
+	 * Register all the devices which type is "pcmcia"
+	 */
+	while ((np = of_find_compatible_node(np, 
+			"pcmcia", "fsl,pq-pcmcia")) != NULL)
+			    of_platform_device_create(np, "m8xx-pcmcia", NULL);
+	return 0;
+}
+
+arch_initcall(fsl_pcmcia_of_init);
 
 static const char *smc_regs = "regs";
 static const char *smc_pram = "pram";
diff --git a/arch/powerpc/sysdev/mpc8xx_pic.h b/arch/powerpc/sysdev/mpc8xx_pic.h
index afa2ee6..07be061 100644
--- a/arch/powerpc/sysdev/mpc8xx_pic.h
+++ b/arch/powerpc/sysdev/mpc8xx_pic.h
@@ -4,7 +4,7 @@
 #include <linux/irq.h>
 #include <linux/interrupt.h>
 
-extern struct hw_interrupt_type mpc8xx_pic;
+/*extern struct hw_interrupt_type mpc8xx_pic;*/
 
 int mpc8xx_pic_init(void);
 unsigned int mpc8xx_get_irq(void);
diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
index 35f8864..c3fd55d 100644
--- a/drivers/pcmcia/Kconfig
+++ b/drivers/pcmcia/Kconfig
@@ -183,6 +183,7 @@ config PCMCIA_M8XX
         tristate "MPC8xx PCMCIA support"
         depends on PCMCIA && PPC && 8xx 
         select PCCARD_IODYN
+	select PCCARD_NONSTATIC
         help
         Say Y here to include support for PowerPC 8xx series PCMCIA
         controller.
diff --git a/drivers/pcmcia/m8xx_pcmcia.c b/drivers/pcmcia/m8xx_pcmcia.c
index 9721ed7..88a4316 100644
--- a/drivers/pcmcia/m8xx_pcmcia.c
+++ b/drivers/pcmcia/m8xx_pcmcia.c
@@ -10,7 +10,7 @@
  * Further fixes, v2.6 kernel port
  *     <marcelo.tosatti@cyclades.com>
  * 
- * Some fixes, additions (C) 2005 Montavista Software, Inc. 
+ * Some fixes, additions (C) 2005-2007 Montavista Software, Inc. 
  *     <vbordug@ru.mvista.com>
  *
  * "The ExCA standard specifies that socket controllers should provide
@@ -40,10 +40,6 @@
 #include <linux/fcntl.h>
 #include <linux/string.h>
 
-#include <asm/io.h>
-#include <asm/bitops.h>
-#include <asm/system.h>
-
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/slab.h>
@@ -51,11 +47,18 @@
 #include <linux/ioport.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
-#include <linux/platform_device.h>
+#include <linux/fsl_devices.h>
 
+#include <asm/io.h>
+#include <asm/bitops.h>
+#include <asm/system.h>
+#include <asm/time.h>
 #include <asm/mpc8xx.h>
 #include <asm/8xx_immap.h>
 #include <asm/irq.h>
+#include <asm/fs_pd.h>
+#include <asm/of_device.h>
+#include <asm/of_platform.h>
 
 #include <pcmcia/version.h>
 #include <pcmcia/cs_types.h>
@@ -146,27 +149,17 @@ MODULE_LICENSE("Dual MPL/GPL");
 #define PCMCIA_MEM_WIN_BASE 0xe0000000 /* base address for memory window 0   */
 #define PCMCIA_MEM_WIN_SIZE 0x04000000 /* each memory window is 64 MByte     */
 #define PCMCIA_IO_WIN_BASE  _IO_BASE   /* base address for io window 0       */
-
-#define PCMCIA_SCHLVL PCMCIA_INTERRUPT /* Status Change Interrupt Level      */
-
 /* ------------------------------------------------------------------------- */
 
-/* 2.4.x and newer has this always in HZ */
-#define M8XX_BUSFREQ ((((bd_t *)&(__res))->bi_busfreq))
-
-static int pcmcia_schlvl = PCMCIA_SCHLVL;
+static int pcmcia_schlvl;
 
 static DEFINE_SPINLOCK(events_lock);
 
-
 #define PCMCIA_SOCKET_KEY_5V 1
 #define PCMCIA_SOCKET_KEY_LV 2
 
 /* look up table for pgcrx registers */
-static u32 *m8xx_pgcrx[2] = {
-	&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pgcra,
-	&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pgcrb
-};
+static u32 *m8xx_pgcrx[2];
 
 /*
  * This structure is used to address each window in the PCMCIA controller.
@@ -228,11 +221,16 @@ struct event_table {
 	u32 eventbit;
 };
 
+static const char driver_name[] = "m8xx-pcmcia";
+
 struct socket_info {
 	void	(*handler)(void *info, u32 events);
 	void	*info;
 
 	u32 slot;
+	pcmconf8xx_t *pcmcia;
+	u32 bus_freq;
+	int hwirq;
 
 	socket_state_t state;
 	struct pccard_mem_map mem_win[PCMCIA_MEM_WIN_NO];
@@ -408,78 +406,20 @@ static void hardware_disable(int slot)
 #if defined(CONFIG_MPC885ADS)
 
 #define PCMCIA_BOARD_MSG "MPC885ADS"
-
-static int voltage_set(int slot, int vcc, int vpp)
-{
-	u32 reg = 0;
-	unsigned *bcsr_io;
-
-	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
-
-	switch(vcc) {
-		case 0:
-			break;
-		case 33:
-			reg |= BCSR1_PCCVCC0;
-			break;
-		case 50:
-			reg |= BCSR1_PCCVCC1;
-			break;
-		default:
-			goto out_unmap;
-	}
-
-	switch(vpp) {
-		case 0:
-			break;
-		case 33:
-		case 50:
-			if(vcc == vpp)
-				reg |= BCSR1_PCCVPP1;
-			else
-				goto out_unmap;
-			break;
-		case 120:
-			if ((vcc == 33) || (vcc == 50))
-				reg |= BCSR1_PCCVPP0;
-			else
-				goto out_unmap;
-		default:
-			goto out_unmap;
-	}
-
-	/* first, turn off all power */
-	out_be32(bcsr_io, in_be32(bcsr_io) & ~(BCSR1_PCCVCC_MASK | BCSR1_PCCVPP_MASK));
-
-	/* enable new powersettings */
-	out_be32(bcsr_io, in_be32(bcsr_io) | reg);
-
-	iounmap(bcsr_io);
-	return 0;
-
-out_unmap:
-	iounmap(bcsr_io);
-	return 1;
-}
-
 #define socket_get(_slot_) PCMCIA_SOCKET_KEY_5V
 
-static void hardware_enable(int slot)
+static inline void hardware_enable(int slot)
 {
-	unsigned *bcsr_io;
-
-	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
-	out_be32(bcsr_io, in_be32(bcsr_io) & ~BCSR1_PCCEN);
-	iounmap(bcsr_io);
+	 m8xx_pcmcia_ops.hw_ctrl(slot, 1);
 }
 
-static void hardware_disable(int slot)
+static inline void hardware_disable(int slot)
 {
-	unsigned *bcsr_io;
+	m8xx_pcmcia_ops.hw_ctrl(slot, 0);
+}
 
-	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
-	out_be32(bcsr_io, in_be32(bcsr_io) |  BCSR1_PCCEN);
-	iounmap(bcsr_io);
+static inline int voltage_set(int slot, int vcc, int vpp)
+	return m8xx_pcmcia_ops.voltage_set(slot, vcc, vpp);
 }
 
 #endif
@@ -604,48 +544,6 @@ static int voltage_set(int slot, int vcc, int vpp)
 
 #endif /* CONFIG_PRxK */
 
-static void m8xx_shutdown(void)
-{
-	u32 m, i;
-	struct pcmcia_win *w;
-
-	for(i = 0; i < PCMCIA_SOCKETS_NO; i++){
-		w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
-
-		out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr, M8XX_PCMCIA_MASK(i));
-		out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per, in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per) & ~M8XX_PCMCIA_MASK(i));
-
-		/* turn off interrupt and disable CxOE */
-		out_be32(M8XX_PGCRX(i), M8XX_PGCRX_CXOE);
-
-		/* turn off memory windows */
-		for(m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
-			out_be32(&w->or, 0); /* set to not valid */
-			w++;
-		}
-
-		/* turn off voltage */
-		voltage_set(i, 0, 0);
-
-		/* disable external hardware */
-		hardware_disable(i);
-	}
-
-	free_irq(pcmcia_schlvl, NULL);
-}
-
-static struct device_driver m8xx_driver = {
-        .name = "m8xx-pcmcia",
-        .bus = &platform_bus_type,
-        .suspend = pcmcia_socket_dev_suspend,
-        .resume = pcmcia_socket_dev_resume,
-};
-
-static struct platform_device m8xx_device = {
-        .name = "m8xx-pcmcia",
-        .id = 0,
-};
-
 static u32 pending_events[PCMCIA_SOCKETS_NO];
 static DEFINE_SPINLOCK(pending_event_lock);
 
@@ -654,13 +552,14 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 	struct socket_info *s;
 	struct event_table *e;
 	unsigned int i, events, pscr, pipr, per;
+	pcmconf8xx_t	*pcmcia = socket[0].pcmcia;
 
 	dprintk("Interrupt!\n");
 	/* get interrupt sources */
 
-	pscr = in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr);
-	pipr = in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pipr);
-	per = in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per);
+	pscr = in_be32(&pcmcia->pcmc_pscr);
+	pipr = in_be32(&pcmcia->pcmc_pipr);
+	per = in_be32(&pcmcia->pcmc_per);
 
 	for(i = 0; i < PCMCIA_SOCKETS_NO; i++) {
 		s = &socket[i];
@@ -724,7 +623,7 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 			per &= ~M8XX_PCMCIA_RDY_L(0);
 			per &= ~M8XX_PCMCIA_RDY_L(1);
 
-			out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per, per);
+			out_be32(&pcmcia->pcmc_per, per);
 
 			if (events)
 				pcmcia_parse_events(&socket[i].socket, events);
@@ -732,7 +631,7 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 	}
 
 	/* clear the interrupt sources */
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr, pscr);
+	out_be32(&pcmcia->pcmc_pscr, pscr);
 
 	dprintk("Interrupt done.\n");
 
@@ -753,7 +652,7 @@ static u32 m8xx_get_graycode(u32 size)
 	return k;
 }
 
-static u32 m8xx_get_speed(u32 ns, u32 is_io)
+static u32 m8xx_get_speed(u32 ns, u32 is_io, u32 bus_freq)
 {
 	u32 reg, clocks, psst, psl, psht;
 
@@ -781,7 +680,7 @@ static u32 m8xx_get_speed(u32 ns, u32 is_io)
 
 #define ADJ 180 /* 80 % longer accesstime - to be sure */
 
-	clocks = ((M8XX_BUSFREQ / 1000) * ns) / 1000;
+	clocks = ((bus_freq / 1000) * ns) / 1000;
 	clocks = (clocks * ADJ) / (100*1000);
 	if(clocks >= PCMCIA_BMT_LIMIT) {
 		printk( "Max access time limit reached\n");
@@ -806,8 +705,9 @@ static int m8xx_get_status(struct pcmcia_socket *sock, unsigned int *value)
 	int lsock = container_of(sock, struct socket_info, socket)->slot;
 	struct socket_info *s = &socket[lsock];
 	unsigned int pipr, reg;
+	pcmconf8xx_t *pcmcia = s->pcmcia;
 
-	pipr = in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pipr);
+	pipr = in_be32(&pcmcia->pcmc_pipr);
 
 	*value  = ((pipr & (M8XX_PCMCIA_CD1(lsock)
 			    | M8XX_PCMCIA_CD2(lsock))) == 0) ? SS_DETECT : 0;
@@ -918,6 +818,7 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	struct event_table *e;
 	unsigned int reg;
 	unsigned long flags;
+	pcmconf8xx_t *pcmcia = socket[0].pcmcia;
 
 	dprintk( "SetSocket(%d, flags %#3.3x, Vcc %d, Vpp %d, "
 	      "io_irq %d, csc_mask %#2.2x)\n", lsock, state->flags,
@@ -927,6 +828,7 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	if(voltage_set(lsock, state->Vcc, state->Vpp))
 		return -EINVAL;
 
+
 	/* Take care of reset... */
 	if(state->flags & SS_RESET)
 		out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) |  M8XX_PGCRX_CXRESET); /* active high */
@@ -982,7 +884,8 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 		 * If io_irq is non-zero we should enable irq.
 		 */
 		if(state->io_irq) {
-			out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) | mk_int_int_mask(state->io_irq) << 24);
+			out_be32(M8XX_PGCRX(lsock),
+				 in_be32(M8XX_PGCRX(lsock)) | mk_int_int_mask(s->hwirq) << 24);
 			/*
 			 * Strange thing here:
 			 * The manual does not tell us which interrupt
@@ -1027,7 +930,7 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	 * Writing ones will clear the bits.
 	 */
 
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr, reg);
+	out_be32(&pcmcia->pcmc_pscr, reg);
 
 	/*
 	 * Write the mask.
@@ -1036,15 +939,8 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	 * Ones will enable the interrupt.
 	 */
 
-	/*
-	  reg |= ((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per
-	  & M8XX_PCMCIA_MASK(lsock);
-	*/
-
-	reg |= in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per) &
-		(M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
-
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per, reg);
+	reg |= in_be32(&pcmcia->pcmc_per) & (M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
+	out_be32(&pcmcia->pcmc_per, reg);
 
 	spin_unlock_irqrestore(&events_lock, flags);
 
@@ -1062,6 +958,8 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 	struct socket_info *s = &socket[lsock];
 	struct pcmcia_win *w;
 	unsigned int reg, winnr;
+	pcmconf8xx_t *pcmcia = s->pcmcia;
+
 
 #define M8XX_SIZE (io->stop - io->start + 1)
 #define M8XX_BASE (PCMCIA_IO_WIN_BASE + io->start)
@@ -1086,7 +984,7 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 
 		/* setup registers */
 
-		w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
+		w = (void *) &pcmcia->pcmc_pbr0;
 		w += winnr;
 
 		out_be32(&w->or, 0); /* turn off window first */
@@ -1095,12 +993,13 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 		reg <<= 27;
   		reg |= M8XX_PCMCIA_POR_IO |(lsock << 2);
 
-		reg |= m8xx_get_speed(io->speed, 1);
+		reg |= m8xx_get_speed(io->speed, 1, s->bus_freq);
 
 		if(io->flags & MAP_WRPROT)
 			reg |= M8XX_PCMCIA_POR_WRPROT;
 
-		if(io->flags & (MAP_16BIT | MAP_AUTOSZ))
+		/*if(io->flags & (MAP_16BIT | MAP_AUTOSZ))*/
+		if(io->flags & MAP_16BIT)
 			reg |= M8XX_PCMCIA_POR_16BIT;
 
 		if(io->flags & MAP_ACTIVE)
@@ -1117,7 +1016,7 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 
 		/* setup registers */
 
-		w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
+		w = (void *) &pcmcia->pcmc_pbr0;
 		w += winnr;
 
 		out_be32(&w->or, 0); /* turn off window */
@@ -1144,6 +1043,7 @@ static int m8xx_set_mem_map(struct pcmcia_socket *sock, struct pccard_mem_map *m
 	struct pcmcia_win *w;
 	struct pccard_mem_map *old;
 	unsigned int reg, winnr;
+	pcmconf8xx_t *pcmcia = s->pcmcia;
 
 	dprintk( "SetMemMap(%d, %d, %#2.2x, %d ns, "
 	      "%#5.5lx, %#5.5x)\n", lsock, mem->map, mem->flags,
@@ -1166,12 +1066,12 @@ static int m8xx_set_mem_map(struct pcmcia_socket *sock, struct pccard_mem_map *m
 
 	/* Setup the window in the pcmcia controller */
 
-	w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
+	w = (void *) &pcmcia->pcmc_pbr0;
 	w += winnr;
 
 	reg |= lsock << 2;
 
-	reg |= m8xx_get_speed(mem->speed, 0);
+	reg |= m8xx_get_speed(mem->speed, 0, s->bus_freq);
 
 	if(mem->flags & MAP_ATTRIB)
 		reg |=  M8XX_PCMCIA_POR_ATTRMEM;
@@ -1236,60 +1136,69 @@ static int m8xx_sock_init(struct pcmcia_socket *sock)
 
 }
 
-static int m8xx_suspend(struct pcmcia_socket *sock)
+static int m8xx_sock_suspend(struct pcmcia_socket *sock)
 {
 	return m8xx_set_socket(sock, &dead_socket);
 }
 
 static struct pccard_operations m8xx_services = {
 	.init	= m8xx_sock_init,
-	.suspend = m8xx_suspend,
+	.suspend = m8xx_sock_suspend,
 	.get_status = m8xx_get_status,
 	.set_socket = m8xx_set_socket,
 	.set_io_map = m8xx_set_io_map,
 	.set_mem_map = m8xx_set_mem_map,
 };
 
-static int __init m8xx_init(void)
+static int __init m8xx_probe(struct of_device *ofdev, const struct of_device_id *match)
 {
 	struct pcmcia_win *w;
-	unsigned int i,m;
+	unsigned int i, m, hwirq;
+	pcmconf8xx_t *pcmcia;
+	int status;
+	struct device_node *np = ofdev->node;
 
 	pcmcia_info("%s\n", version);
 
-	if (driver_register(&m8xx_driver))
-		return -1;
+	pcmcia = of_iomap(np, 0);
+	if(pcmcia == NULL)
+		return -EINVAL;
+
+	pcmcia_schlvl = irq_of_parse_and_map(np, 0);
+	hwirq  = irq_map[pcmcia_schlvl].hwirq;
+	if (pcmcia_schlvl < 0)
+		return -EINVAL;
+
+	m8xx_pgcrx[0] = &pcmcia->pcmc_pgcra;
+	m8xx_pgcrx[1] = &pcmcia->pcmc_pgcrb;
+
 
 	pcmcia_info(PCMCIA_BOARD_MSG " using " PCMCIA_SLOT_MSG
-		    " with IRQ %u.\n", pcmcia_schlvl);
+		    " with IRQ %u  (%d). \n", pcmcia_schlvl, hwirq);
 
 	/* Configure Status change interrupt */
 
-	if(request_irq(pcmcia_schlvl, m8xx_interrupt, 0,
-			  "m8xx_pcmcia", NULL)) {
+	if(request_irq(pcmcia_schlvl, m8xx_interrupt, IRQF_SHARED,
+			  driver_name, socket)) {
 		pcmcia_error("Cannot allocate IRQ %u for SCHLVL!\n",
 			     pcmcia_schlvl);
 		return -1;
 	}
 
-	w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
-
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr,
-		M8XX_PCMCIA_MASK(0)| M8XX_PCMCIA_MASK(1));
+	w = (void *) &pcmcia->pcmc_pbr0;
 
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per,
-		in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per) &
-		~(M8XX_PCMCIA_MASK(0)| M8XX_PCMCIA_MASK(1)));
+	out_be32(&pcmcia->pcmc_pscr, M8XX_PCMCIA_MASK(0)| M8XX_PCMCIA_MASK(1));
+	clrbits32(&pcmcia->pcmc_per, M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
 
-/* connect interrupt and disable CxOE */
+	/* connect interrupt and disable CxOE */
 
-	out_be32(M8XX_PGCRX(0), M8XX_PGCRX_CXOE | (mk_int_int_mask(pcmcia_schlvl) << 16));
-	out_be32(M8XX_PGCRX(1), M8XX_PGCRX_CXOE | (mk_int_int_mask(pcmcia_schlvl) << 16));
+	out_be32(M8XX_PGCRX(0), M8XX_PGCRX_CXOE | (mk_int_int_mask(hwirq) << 16));
+	out_be32(M8XX_PGCRX(1), M8XX_PGCRX_CXOE | (mk_int_int_mask(hwirq) << 16));
 
-/* intialize the fixed memory windows */
+	/* intialize the fixed memory windows */
 
 	for(i = 0; i < PCMCIA_SOCKETS_NO; i++){
-		for(m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
+		for (m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
 			out_be32(&w->br, PCMCIA_MEM_WIN_BASE +
 				(PCMCIA_MEM_WIN_SIZE
 				 * (m + i * PCMCIA_MEM_WIN_NO)));
@@ -1300,16 +1209,14 @@ static int __init m8xx_init(void)
 		}
 	}
 
-/* turn off voltage */
+	/* turn off voltage */
 	voltage_set(0, 0, 0);
 	voltage_set(1, 0, 0);
 
-/* Enable external hardware */
+	/* Enable external hardware */
 	hardware_enable(0);
 	hardware_enable(1);
 
-	platform_device_register(&m8xx_device);
-
 	for (i = 0 ; i < PCMCIA_SOCKETS_NO; i++) {
 		socket[i].slot = i;
 		socket[i].socket.owner = THIS_MODULE;
@@ -1317,30 +1224,105 @@ static int __init m8xx_init(void)
 		socket[i].socket.irq_mask = 0x000;
 		socket[i].socket.map_size = 0x1000;
 		socket[i].socket.io_offset = 0;
-		socket[i].socket.pci_irq = i  ? 7 : 9;
+		socket[i].socket.pci_irq = pcmcia_schlvl;
 		socket[i].socket.ops = &m8xx_services;
-		socket[i].socket.resource_ops = &pccard_iodyn_ops;
+		socket[i].socket.resource_ops = &pccard_nonstatic_ops;
 		socket[i].socket.cb_dev = NULL;
-		socket[i].socket.dev.parent = &m8xx_device.dev;
+		socket[i].socket.dev.parent = &ofdev->dev;
+		socket[i].pcmcia = pcmcia;
+		socket[i].bus_freq = ppc_proc_freq;
+		socket[i].hwirq = hwirq;
+
+
 	}
 
-	for (i = 0; i < PCMCIA_SOCKETS_NO; i++)
-		pcmcia_register_socket(&socket[i].socket);
+	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
+		status = pcmcia_register_socket(&socket[i].socket);
+		if (status < 0)
+			pcmcia_error("Socket register failed\n");
+	}
 
 	return 0;
 }
 
-static void __exit m8xx_exit(void)
+static int m8xx_remove(struct of_device* ofdev)
 {
-	int i;
+	u32 m, i;
+	struct pcmcia_win *w;
+	pcmconf8xx_t *pcmcia = socket[0].pcmcia;
+
+	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
+		w = (void *) &pcmcia->pcmc_pbr0;
+
+		out_be32(&pcmcia->pcmc_pscr, M8XX_PCMCIA_MASK(i));
+		out_be32(&pcmcia->pcmc_per,
+			in_be32(&pcmcia->pcmc_per) & ~M8XX_PCMCIA_MASK(i));
 
+		/* turn off interrupt and disable CxOE */
+		out_be32(M8XX_PGCRX(i), M8XX_PGCRX_CXOE);
+
+		/* turn off memory windows */
+		for (m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
+			out_be32(&w->or, 0); /* set to not valid */
+			w++;
+		}
+
+		/* turn off voltage */
+		voltage_set(i, 0, 0);
+
+		/* disable external hardware */
+		hardware_disable(i);
+	}
 	for (i = 0; i < PCMCIA_SOCKETS_NO; i++)
 		pcmcia_unregister_socket(&socket[i].socket);
 
-	m8xx_shutdown();
+	free_irq(pcmcia_schlvl, NULL);
 
-	platform_device_unregister(&m8xx_device);
-	driver_unregister(&m8xx_driver);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int m8xx_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	return pcmcia_socket_dev_suspend(&pdev->dev, state);
+}
+
+static int m8xx_resume(struct platform_device *pdev)
+{
+	return pcmcia_socket_dev_resume(&pdev->dev);
+}
+#else
+#define m8xx_suspend NULL
+#define m8xx_resume NULL
+#endif
+
+static struct of_device_id m8xx_pcmcia_match[] = {
+	{
+		.type = "pcmcia",
+		.compatible = "fsl,pq-pcmcia",
+	},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, m8xx_pcmcia_match);
+
+static struct of_platform_driver m8xx_pcmcia_driver = {
+	.name		= (char *) driver_name,
+	.match_table	= m8xx_pcmcia_match,
+	.probe		= m8xx_probe,
+	.remove		= m8xx_remove,
+	.suspend	= m8xx_suspend,
+	.resume		= m8xx_resume,
+};
+
+static int __init m8xx_init(void)
+{
+	return of_register_platform_driver(&m8xx_pcmcia_driver);
+}
+
+static void __exit m8xx_exit(void)
+{
+	of_unregister_platform_driver(&m8xx_pcmcia_driver);
 }
 
 module_init(m8xx_init);
diff --git a/include/asm-powerpc/mpc8xx.h b/include/asm-powerpc/mpc8xx.h
index 5803711..c1a6efc 100644
--- a/include/asm-powerpc/mpc8xx.h
+++ b/include/asm-powerpc/mpc8xx.h
@@ -23,6 +23,10 @@
 #include <platforms/8xx/mpc885ads.h>
 #endif
 
+#ifdef CONFIG_PCMCIA_M8XX 
+extern struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops;
+#endif 
+
 #endif /* CONFIG_8xx */
 #endif /* __CONFIG_8xx_DEFS */
 #endif /* __KERNEL__ */
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index 73710d6..12e631f 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -120,5 +120,10 @@ struct fsl_spi_platform_data {
 	u32	sysclk;
 };
 
+struct mpc8xx_pcmcia_ops {
+	void(*hw_ctrl)(int slot, int enable);
+	int(*voltage_set)(int slot, int vcc, int vpp);
+};
+
 #endif /* _FSL_DEVICE_H_ */
 #endif /* __KERNEL__ */

^ permalink raw reply related

* [PATCH 0/2] of_device 8xx PCMCIA support && cleanup
From: Vitaly Bordug @ 2007-05-06  1:26 UTC (permalink / raw)
  To: linux-pcmcia; +Cc: linuxppc-dev, linux-kernel

Current series features the reworked  8xx of_device PCMCIA patch
based on feedback from Andrew + indentation & whitespace fixes
in drivers/pcmcia/m8xx_pcmcia.c

Andrew,
Please disregard previous patch since it contains a typo. Indentation 
patch is not strictly necessary but is useful too, imho.

--
-Vitaly

^ permalink raw reply

* [PATCH 1/2] [POWERPC] 8xx: mpc885ads pcmcia support
From: Vitaly Bordug @ 2007-05-06  1:26 UTC (permalink / raw)
  To: linux-pcmcia; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20070506011004.12695.41514.stgit@localhost.localdomain>


Adds support for PowerQuicc on-chip PCMCIA. The driver is implemented as
of_device, so only arch/powerpc stuff is capable to use it, which now
implies only mpc885ads reference board.

To cope with the code that should be hooked inside driver, but is really
board specific (like set_voltage), global structure mpc8xx_pcmcia_ops
holds necessary function pointers that are filled in the BSP code.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>

---

 arch/powerpc/boot/dts/mpc885ads.dts          |   12 +
 arch/powerpc/platforms/8xx/m8xx_setup.c      |    5 
 arch/powerpc/platforms/8xx/mpc885ads.h       |    8 +
 arch/powerpc/platforms/8xx/mpc885ads_setup.c |   76 ++++++
 arch/powerpc/sysdev/fsl_soc.c                |   13 +
 arch/powerpc/sysdev/mpc8xx_pic.h             |    2 
 drivers/pcmcia/Kconfig                       |    1 
 drivers/pcmcia/m8xx_pcmcia.c                 |  351 ++++++++++++--------------
 include/asm-powerpc/mpc8xx.h                 |    4 
 include/linux/fsl_devices.h                  |    5 
 10 files changed, 292 insertions(+), 185 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc885ads.dts b/arch/powerpc/boot/dts/mpc885ads.dts
index 110bf61..0f3e8cf 100644
--- a/arch/powerpc/boot/dts/mpc885ads.dts
+++ b/arch/powerpc/boot/dts/mpc885ads.dts
@@ -112,6 +112,18 @@
 			compatible = "CPM";
 		};
 
+		pcmcia@0080 {
+			linux,phandle = <0080>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			compatible = "fsl,pq-pcmcia";
+			device_type = "pcmcia";
+			reg = <80 80>;
+			clock-frequency = <2faf080>;
+			interrupt-parent = <ff000000>;
+			interrupts = <d 1>;
+		};
+
 		cpm@ff000000 {
 			linux,phandle = <ff000000>;
 			#address-cells = <1>;
diff --git a/arch/powerpc/platforms/8xx/m8xx_setup.c b/arch/powerpc/platforms/8xx/m8xx_setup.c
index 0901dba..f169355 100644
--- a/arch/powerpc/platforms/8xx/m8xx_setup.c
+++ b/arch/powerpc/platforms/8xx/m8xx_setup.c
@@ -32,6 +32,7 @@
 #include <linux/root_dev.h>
 #include <linux/time.h>
 #include <linux/rtc.h>
+#include <linux/fsl_devices.h>
 
 #include <asm/mmu.h>
 #include <asm/reg.h>
@@ -49,6 +50,10 @@
 
 #include "sysdev/mpc8xx_pic.h"
 
+#ifdef CONFIG_PCMCIA_M8XX
+struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops;
+#endif
+
 void m8xx_calibrate_decr(void);
 extern void m8xx_wdt_handler_install(bd_t *bp);
 extern int cpm_pic_init(void);
diff --git a/arch/powerpc/platforms/8xx/mpc885ads.h b/arch/powerpc/platforms/8xx/mpc885ads.h
index 7c31aec..932b59a 100644
--- a/arch/powerpc/platforms/8xx/mpc885ads.h
+++ b/arch/powerpc/platforms/8xx/mpc885ads.h
@@ -91,5 +91,13 @@
 #define SICR_ENET_MASK	((uint)0x00ff0000)
 #define SICR_ENET_CLKRT	((uint)0x002c0000)
 
+/* 
+ * Some internal interrupt registers use an 8-bit mask for the interrupt
+ * level instead of a number.
+ */
+static inline uint mk_int_int_mask(uint mask) {
+	return (1 << (7 - (mask/2)));
+}
+
 #endif /* __ASM_MPC885ADS_H__ */
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/platforms/8xx/mpc885ads_setup.c b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
index a57b577..4e76e1c 100644
--- a/arch/powerpc/platforms/8xx/mpc885ads_setup.c
+++ b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
@@ -22,6 +22,7 @@
 
 #include <linux/fs_enet_pd.h>
 #include <linux/fs_uart_pd.h>
+#include <linux/fsl_devices.h>
 #include <linux/mii.h>
 
 #include <asm/delay.h>
@@ -51,6 +52,11 @@ static void init_smc1_uart_ioports(struct fs_uart_platform_info* fpi);
 static void init_smc2_uart_ioports(struct fs_uart_platform_info* fpi);
 static void init_scc3_ioports(struct fs_platform_info* ptr);
 
+#ifdef CONFIG_PCMCIA_M8XX
+static void pcmcia_hw_setup(int slot, int enable);
+static int pcmcia_set_voltage(int slot, int vcc, int vpp);
+#endif
+
 void __init mpc885ads_board_setup(void)
 {
 	cpm8xx_t *cp;
@@ -115,6 +121,12 @@ void __init mpc885ads_board_setup(void)
 	immr_unmap(io_port);
 
 #endif
+
+#ifdef CONFIG_PCMCIA_M8XX
+	/*Set up board specific hook-ups*/
+	m8xx_pcmcia_ops.hw_ctrl = pcmcia_hw_setup;
+	m8xx_pcmcia_ops.voltage_set = pcmcia_set_voltage;
+#endif
 }
 
 
@@ -322,6 +334,70 @@ void init_smc_ioports(struct fs_uart_platform_info *data)
 	}
 }
 
+#ifdef CONFIG_PCMCIA_M8XX
+static void pcmcia_hw_setup(int slot, int enable)
+{
+	unsigned *bcsr_io;
+
+	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
+	if (enable)
+		clrbits32(bcsr_io, BCSR1_PCCEN);
+	else
+		setbits32(bcsr_io, BCSR1_PCCEN);
+
+	iounmap(bcsr_io);
+}
+
+static int pcmcia_set_voltage(int slot, int vcc, int vpp)
+{
+	u32 reg = 0;
+	unsigned *bcsr_io;
+
+	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
+
+	switch(vcc) {
+	case 0:
+		break;
+	case 33:
+		reg |= BCSR1_PCCVCC0;
+		break;
+	case 50:
+		reg |= BCSR1_PCCVCC1;
+		break;
+	default:
+		return 1;
+	}
+
+	switch(vpp) {
+	case 0:
+       	break;
+	case 33:
+	case 50:
+       	if(vcc == vpp)
+			reg |= BCSR1_PCCVPP1;
+		else
+			return 1;
+		break;
+	case 120:
+	if ((vcc == 33) || (vcc == 50))
+		reg |= BCSR1_PCCVPP0;
+	else
+		return 1;
+	default:
+		return 1;
+	}
+
+	/* first, turn off all power */
+	clrbits32(bcsr_io, 0x00610000);
+
+	/* enable new powersettings */
+	setbits32(bcsr_io, reg);
+
+	iounmap(bcsr_io);
+	return 0;
+}
+#endif
+
 int platform_device_skip(const char *model, int id)
 {
 #ifdef CONFIG_MPC8xx_SECOND_ETH_SCC3
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 8a123c7..880e45f 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -1028,6 +1028,19 @@ err:
 
 arch_initcall(fs_enet_of_init);
 
+static int __init fsl_pcmcia_of_init(void)
+{
+	struct device_node *np = NULL;
+	/*
+	 * Register all the devices which type is "pcmcia"
+	 */
+	while ((np = of_find_compatible_node(np, 
+			"pcmcia", "fsl,pq-pcmcia")) != NULL)
+			    of_platform_device_create(np, "m8xx-pcmcia", NULL);
+	return 0;
+}
+
+arch_initcall(fsl_pcmcia_of_init);
 
 static const char *smc_regs = "regs";
 static const char *smc_pram = "pram";
diff --git a/arch/powerpc/sysdev/mpc8xx_pic.h b/arch/powerpc/sysdev/mpc8xx_pic.h
index afa2ee6..07be061 100644
--- a/arch/powerpc/sysdev/mpc8xx_pic.h
+++ b/arch/powerpc/sysdev/mpc8xx_pic.h
@@ -4,7 +4,7 @@
 #include <linux/irq.h>
 #include <linux/interrupt.h>
 
-extern struct hw_interrupt_type mpc8xx_pic;
+/*extern struct hw_interrupt_type mpc8xx_pic;*/
 
 int mpc8xx_pic_init(void);
 unsigned int mpc8xx_get_irq(void);
diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
index 35f8864..c3fd55d 100644
--- a/drivers/pcmcia/Kconfig
+++ b/drivers/pcmcia/Kconfig
@@ -183,6 +183,7 @@ config PCMCIA_M8XX
         tristate "MPC8xx PCMCIA support"
         depends on PCMCIA && PPC && 8xx 
         select PCCARD_IODYN
+	select PCCARD_NONSTATIC
         help
         Say Y here to include support for PowerPC 8xx series PCMCIA
         controller.
diff --git a/drivers/pcmcia/m8xx_pcmcia.c b/drivers/pcmcia/m8xx_pcmcia.c
index 9721ed7..40f6e20 100644
--- a/drivers/pcmcia/m8xx_pcmcia.c
+++ b/drivers/pcmcia/m8xx_pcmcia.c
@@ -10,7 +10,7 @@
  * Further fixes, v2.6 kernel port
  *     <marcelo.tosatti@cyclades.com>
  * 
- * Some fixes, additions (C) 2005 Montavista Software, Inc. 
+ * Some fixes, additions (C) 2005-2007 Montavista Software, Inc. 
  *     <vbordug@ru.mvista.com>
  *
  * "The ExCA standard specifies that socket controllers should provide
@@ -40,10 +40,6 @@
 #include <linux/fcntl.h>
 #include <linux/string.h>
 
-#include <asm/io.h>
-#include <asm/bitops.h>
-#include <asm/system.h>
-
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/slab.h>
@@ -51,11 +47,18 @@
 #include <linux/ioport.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
-#include <linux/platform_device.h>
+#include <linux/fsl_devices.h>
 
+#include <asm/io.h>
+#include <asm/bitops.h>
+#include <asm/system.h>
+#include <asm/time.h>
 #include <asm/mpc8xx.h>
 #include <asm/8xx_immap.h>
 #include <asm/irq.h>
+#include <asm/fs_pd.h>
+#include <asm/of_device.h>
+#include <asm/of_platform.h>
 
 #include <pcmcia/version.h>
 #include <pcmcia/cs_types.h>
@@ -146,27 +149,17 @@ MODULE_LICENSE("Dual MPL/GPL");
 #define PCMCIA_MEM_WIN_BASE 0xe0000000 /* base address for memory window 0   */
 #define PCMCIA_MEM_WIN_SIZE 0x04000000 /* each memory window is 64 MByte     */
 #define PCMCIA_IO_WIN_BASE  _IO_BASE   /* base address for io window 0       */
-
-#define PCMCIA_SCHLVL PCMCIA_INTERRUPT /* Status Change Interrupt Level      */
-
 /* ------------------------------------------------------------------------- */
 
-/* 2.4.x and newer has this always in HZ */
-#define M8XX_BUSFREQ ((((bd_t *)&(__res))->bi_busfreq))
-
-static int pcmcia_schlvl = PCMCIA_SCHLVL;
+static int pcmcia_schlvl;
 
 static DEFINE_SPINLOCK(events_lock);
 
-
 #define PCMCIA_SOCKET_KEY_5V 1
 #define PCMCIA_SOCKET_KEY_LV 2
 
 /* look up table for pgcrx registers */
-static u32 *m8xx_pgcrx[2] = {
-	&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pgcra,
-	&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pgcrb
-};
+static u32 *m8xx_pgcrx[2];
 
 /*
  * This structure is used to address each window in the PCMCIA controller.
@@ -228,11 +221,16 @@ struct event_table {
 	u32 eventbit;
 };
 
+static const char driver_name[] = "m8xx-pcmcia";
+
 struct socket_info {
 	void	(*handler)(void *info, u32 events);
 	void	*info;
 
 	u32 slot;
+	pcmconf8xx_t *pcmcia;
+	u32 bus_freq;
+	int hwirq;
 
 	socket_state_t state;
 	struct pccard_mem_map mem_win[PCMCIA_MEM_WIN_NO];
@@ -408,78 +406,21 @@ static void hardware_disable(int slot)
 #if defined(CONFIG_MPC885ADS)
 
 #define PCMCIA_BOARD_MSG "MPC885ADS"
+#define socket_get(_slot_) PCMCIA_SOCKET_KEY_5V
 
-static int voltage_set(int slot, int vcc, int vpp)
+static inline void hardware_enable(int slot)
 {
-	u32 reg = 0;
-	unsigned *bcsr_io;
-
-	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
-
-	switch(vcc) {
-		case 0:
-			break;
-		case 33:
-			reg |= BCSR1_PCCVCC0;
-			break;
-		case 50:
-			reg |= BCSR1_PCCVCC1;
-			break;
-		default:
-			goto out_unmap;
-	}
-
-	switch(vpp) {
-		case 0:
-			break;
-		case 33:
-		case 50:
-			if(vcc == vpp)
-				reg |= BCSR1_PCCVPP1;
-			else
-				goto out_unmap;
-			break;
-		case 120:
-			if ((vcc == 33) || (vcc == 50))
-				reg |= BCSR1_PCCVPP0;
-			else
-				goto out_unmap;
-		default:
-			goto out_unmap;
-	}
-
-	/* first, turn off all power */
-	out_be32(bcsr_io, in_be32(bcsr_io) & ~(BCSR1_PCCVCC_MASK | BCSR1_PCCVPP_MASK));
-
-	/* enable new powersettings */
-	out_be32(bcsr_io, in_be32(bcsr_io) | reg);
-
-	iounmap(bcsr_io);
-	return 0;
-
-out_unmap:
-	iounmap(bcsr_io);
-	return 1;
+	 m8xx_pcmcia_ops.hw_ctrl(slot, 1);
 }
 
-#define socket_get(_slot_) PCMCIA_SOCKET_KEY_5V
-
-static void hardware_enable(int slot)
+static inline void hardware_disable(int slot)
 {
-	unsigned *bcsr_io;
-
-	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
-	out_be32(bcsr_io, in_be32(bcsr_io) & ~BCSR1_PCCEN);
-	iounmap(bcsr_io);
+	m8xx_pcmcia_ops.hw_ctrl(slot, 0);
 }
 
-static void hardware_disable(int slot)
+static inline int voltage_set(int slot, int vcc, int vpp)
 {
-	unsigned *bcsr_io;
-
-	bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
-	out_be32(bcsr_io, in_be32(bcsr_io) |  BCSR1_PCCEN);
-	iounmap(bcsr_io);
+	return m8xx_pcmcia_ops.voltage_set(slot, vcc, vpp);
 }
 
 #endif
@@ -604,48 +545,6 @@ static int voltage_set(int slot, int vcc, int vpp)
 
 #endif /* CONFIG_PRxK */
 
-static void m8xx_shutdown(void)
-{
-	u32 m, i;
-	struct pcmcia_win *w;
-
-	for(i = 0; i < PCMCIA_SOCKETS_NO; i++){
-		w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
-
-		out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr, M8XX_PCMCIA_MASK(i));
-		out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per, in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per) & ~M8XX_PCMCIA_MASK(i));
-
-		/* turn off interrupt and disable CxOE */
-		out_be32(M8XX_PGCRX(i), M8XX_PGCRX_CXOE);
-
-		/* turn off memory windows */
-		for(m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
-			out_be32(&w->or, 0); /* set to not valid */
-			w++;
-		}
-
-		/* turn off voltage */
-		voltage_set(i, 0, 0);
-
-		/* disable external hardware */
-		hardware_disable(i);
-	}
-
-	free_irq(pcmcia_schlvl, NULL);
-}
-
-static struct device_driver m8xx_driver = {
-        .name = "m8xx-pcmcia",
-        .bus = &platform_bus_type,
-        .suspend = pcmcia_socket_dev_suspend,
-        .resume = pcmcia_socket_dev_resume,
-};
-
-static struct platform_device m8xx_device = {
-        .name = "m8xx-pcmcia",
-        .id = 0,
-};
-
 static u32 pending_events[PCMCIA_SOCKETS_NO];
 static DEFINE_SPINLOCK(pending_event_lock);
 
@@ -654,13 +553,14 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 	struct socket_info *s;
 	struct event_table *e;
 	unsigned int i, events, pscr, pipr, per;
+	pcmconf8xx_t	*pcmcia = socket[0].pcmcia;
 
 	dprintk("Interrupt!\n");
 	/* get interrupt sources */
 
-	pscr = in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr);
-	pipr = in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pipr);
-	per = in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per);
+	pscr = in_be32(&pcmcia->pcmc_pscr);
+	pipr = in_be32(&pcmcia->pcmc_pipr);
+	per = in_be32(&pcmcia->pcmc_per);
 
 	for(i = 0; i < PCMCIA_SOCKETS_NO; i++) {
 		s = &socket[i];
@@ -724,7 +624,7 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 			per &= ~M8XX_PCMCIA_RDY_L(0);
 			per &= ~M8XX_PCMCIA_RDY_L(1);
 
-			out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per, per);
+			out_be32(&pcmcia->pcmc_per, per);
 
 			if (events)
 				pcmcia_parse_events(&socket[i].socket, events);
@@ -732,7 +632,7 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 	}
 
 	/* clear the interrupt sources */
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr, pscr);
+	out_be32(&pcmcia->pcmc_pscr, pscr);
 
 	dprintk("Interrupt done.\n");
 
@@ -753,7 +653,7 @@ static u32 m8xx_get_graycode(u32 size)
 	return k;
 }
 
-static u32 m8xx_get_speed(u32 ns, u32 is_io)
+static u32 m8xx_get_speed(u32 ns, u32 is_io, u32 bus_freq)
 {
 	u32 reg, clocks, psst, psl, psht;
 
@@ -781,7 +681,7 @@ static u32 m8xx_get_speed(u32 ns, u32 is_io)
 
 #define ADJ 180 /* 80 % longer accesstime - to be sure */
 
-	clocks = ((M8XX_BUSFREQ / 1000) * ns) / 1000;
+	clocks = ((bus_freq / 1000) * ns) / 1000;
 	clocks = (clocks * ADJ) / (100*1000);
 	if(clocks >= PCMCIA_BMT_LIMIT) {
 		printk( "Max access time limit reached\n");
@@ -806,8 +706,9 @@ static int m8xx_get_status(struct pcmcia_socket *sock, unsigned int *value)
 	int lsock = container_of(sock, struct socket_info, socket)->slot;
 	struct socket_info *s = &socket[lsock];
 	unsigned int pipr, reg;
+	pcmconf8xx_t *pcmcia = s->pcmcia;
 
-	pipr = in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pipr);
+	pipr = in_be32(&pcmcia->pcmc_pipr);
 
 	*value  = ((pipr & (M8XX_PCMCIA_CD1(lsock)
 			    | M8XX_PCMCIA_CD2(lsock))) == 0) ? SS_DETECT : 0;
@@ -918,6 +819,7 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	struct event_table *e;
 	unsigned int reg;
 	unsigned long flags;
+	pcmconf8xx_t *pcmcia = socket[0].pcmcia;
 
 	dprintk( "SetSocket(%d, flags %#3.3x, Vcc %d, Vpp %d, "
 	      "io_irq %d, csc_mask %#2.2x)\n", lsock, state->flags,
@@ -927,6 +829,7 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	if(voltage_set(lsock, state->Vcc, state->Vpp))
 		return -EINVAL;
 
+
 	/* Take care of reset... */
 	if(state->flags & SS_RESET)
 		out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) |  M8XX_PGCRX_CXRESET); /* active high */
@@ -982,7 +885,8 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 		 * If io_irq is non-zero we should enable irq.
 		 */
 		if(state->io_irq) {
-			out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) | mk_int_int_mask(state->io_irq) << 24);
+			out_be32(M8XX_PGCRX(lsock),
+				 in_be32(M8XX_PGCRX(lsock)) | mk_int_int_mask(s->hwirq) << 24);
 			/*
 			 * Strange thing here:
 			 * The manual does not tell us which interrupt
@@ -1027,7 +931,7 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	 * Writing ones will clear the bits.
 	 */
 
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr, reg);
+	out_be32(&pcmcia->pcmc_pscr, reg);
 
 	/*
 	 * Write the mask.
@@ -1036,15 +940,8 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	 * Ones will enable the interrupt.
 	 */
 
-	/*
-	  reg |= ((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per
-	  & M8XX_PCMCIA_MASK(lsock);
-	*/
-
-	reg |= in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per) &
-		(M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
-
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per, reg);
+	reg |= in_be32(&pcmcia->pcmc_per) & (M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
+	out_be32(&pcmcia->pcmc_per, reg);
 
 	spin_unlock_irqrestore(&events_lock, flags);
 
@@ -1062,6 +959,8 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 	struct socket_info *s = &socket[lsock];
 	struct pcmcia_win *w;
 	unsigned int reg, winnr;
+	pcmconf8xx_t *pcmcia = s->pcmcia;
+
 
 #define M8XX_SIZE (io->stop - io->start + 1)
 #define M8XX_BASE (PCMCIA_IO_WIN_BASE + io->start)
@@ -1086,7 +985,7 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 
 		/* setup registers */
 
-		w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
+		w = (void *) &pcmcia->pcmc_pbr0;
 		w += winnr;
 
 		out_be32(&w->or, 0); /* turn off window first */
@@ -1095,12 +994,13 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 		reg <<= 27;
   		reg |= M8XX_PCMCIA_POR_IO |(lsock << 2);
 
-		reg |= m8xx_get_speed(io->speed, 1);
+		reg |= m8xx_get_speed(io->speed, 1, s->bus_freq);
 
 		if(io->flags & MAP_WRPROT)
 			reg |= M8XX_PCMCIA_POR_WRPROT;
 
-		if(io->flags & (MAP_16BIT | MAP_AUTOSZ))
+		/*if(io->flags & (MAP_16BIT | MAP_AUTOSZ))*/
+		if(io->flags & MAP_16BIT)
 			reg |= M8XX_PCMCIA_POR_16BIT;
 
 		if(io->flags & MAP_ACTIVE)
@@ -1117,7 +1017,7 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 
 		/* setup registers */
 
-		w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
+		w = (void *) &pcmcia->pcmc_pbr0;
 		w += winnr;
 
 		out_be32(&w->or, 0); /* turn off window */
@@ -1144,6 +1044,7 @@ static int m8xx_set_mem_map(struct pcmcia_socket *sock, struct pccard_mem_map *m
 	struct pcmcia_win *w;
 	struct pccard_mem_map *old;
 	unsigned int reg, winnr;
+	pcmconf8xx_t *pcmcia = s->pcmcia;
 
 	dprintk( "SetMemMap(%d, %d, %#2.2x, %d ns, "
 	      "%#5.5lx, %#5.5x)\n", lsock, mem->map, mem->flags,
@@ -1166,12 +1067,12 @@ static int m8xx_set_mem_map(struct pcmcia_socket *sock, struct pccard_mem_map *m
 
 	/* Setup the window in the pcmcia controller */
 
-	w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
+	w = (void *) &pcmcia->pcmc_pbr0;
 	w += winnr;
 
 	reg |= lsock << 2;
 
-	reg |= m8xx_get_speed(mem->speed, 0);
+	reg |= m8xx_get_speed(mem->speed, 0, s->bus_freq);
 
 	if(mem->flags & MAP_ATTRIB)
 		reg |=  M8XX_PCMCIA_POR_ATTRMEM;
@@ -1236,60 +1137,69 @@ static int m8xx_sock_init(struct pcmcia_socket *sock)
 
 }
 
-static int m8xx_suspend(struct pcmcia_socket *sock)
+static int m8xx_sock_suspend(struct pcmcia_socket *sock)
 {
 	return m8xx_set_socket(sock, &dead_socket);
 }
 
 static struct pccard_operations m8xx_services = {
 	.init	= m8xx_sock_init,
-	.suspend = m8xx_suspend,
+	.suspend = m8xx_sock_suspend,
 	.get_status = m8xx_get_status,
 	.set_socket = m8xx_set_socket,
 	.set_io_map = m8xx_set_io_map,
 	.set_mem_map = m8xx_set_mem_map,
 };
 
-static int __init m8xx_init(void)
+static int __init m8xx_probe(struct of_device *ofdev, const struct of_device_id *match)
 {
 	struct pcmcia_win *w;
-	unsigned int i,m;
+	unsigned int i, m, hwirq;
+	pcmconf8xx_t *pcmcia;
+	int status;
+	struct device_node *np = ofdev->node;
 
 	pcmcia_info("%s\n", version);
 
-	if (driver_register(&m8xx_driver))
-		return -1;
+	pcmcia = of_iomap(np, 0);
+	if(pcmcia == NULL)
+		return -EINVAL;
+
+	pcmcia_schlvl = irq_of_parse_and_map(np, 0);
+	hwirq  = irq_map[pcmcia_schlvl].hwirq;
+	if (pcmcia_schlvl < 0)
+		return -EINVAL;
+
+	m8xx_pgcrx[0] = &pcmcia->pcmc_pgcra;
+	m8xx_pgcrx[1] = &pcmcia->pcmc_pgcrb;
+
 
 	pcmcia_info(PCMCIA_BOARD_MSG " using " PCMCIA_SLOT_MSG
-		    " with IRQ %u.\n", pcmcia_schlvl);
+		    " with IRQ %u  (%d). \n", pcmcia_schlvl, hwirq);
 
 	/* Configure Status change interrupt */
 
-	if(request_irq(pcmcia_schlvl, m8xx_interrupt, 0,
-			  "m8xx_pcmcia", NULL)) {
+	if(request_irq(pcmcia_schlvl, m8xx_interrupt, IRQF_SHARED,
+			  driver_name, socket)) {
 		pcmcia_error("Cannot allocate IRQ %u for SCHLVL!\n",
 			     pcmcia_schlvl);
 		return -1;
 	}
 
-	w = (void *) &((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pbr0;
-
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_pscr,
-		M8XX_PCMCIA_MASK(0)| M8XX_PCMCIA_MASK(1));
+	w = (void *) &pcmcia->pcmc_pbr0;
 
-	out_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per,
-		in_be32(&((immap_t *)IMAP_ADDR)->im_pcmcia.pcmc_per) &
-		~(M8XX_PCMCIA_MASK(0)| M8XX_PCMCIA_MASK(1)));
+	out_be32(&pcmcia->pcmc_pscr, M8XX_PCMCIA_MASK(0)| M8XX_PCMCIA_MASK(1));
+	clrbits32(&pcmcia->pcmc_per, M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
 
-/* connect interrupt and disable CxOE */
+	/* connect interrupt and disable CxOE */
 
-	out_be32(M8XX_PGCRX(0), M8XX_PGCRX_CXOE | (mk_int_int_mask(pcmcia_schlvl) << 16));
-	out_be32(M8XX_PGCRX(1), M8XX_PGCRX_CXOE | (mk_int_int_mask(pcmcia_schlvl) << 16));
+	out_be32(M8XX_PGCRX(0), M8XX_PGCRX_CXOE | (mk_int_int_mask(hwirq) << 16));
+	out_be32(M8XX_PGCRX(1), M8XX_PGCRX_CXOE | (mk_int_int_mask(hwirq) << 16));
 
-/* intialize the fixed memory windows */
+	/* intialize the fixed memory windows */
 
 	for(i = 0; i < PCMCIA_SOCKETS_NO; i++){
-		for(m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
+		for (m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
 			out_be32(&w->br, PCMCIA_MEM_WIN_BASE +
 				(PCMCIA_MEM_WIN_SIZE
 				 * (m + i * PCMCIA_MEM_WIN_NO)));
@@ -1300,16 +1210,14 @@ static int __init m8xx_init(void)
 		}
 	}
 
-/* turn off voltage */
+	/* turn off voltage */
 	voltage_set(0, 0, 0);
 	voltage_set(1, 0, 0);
 
-/* Enable external hardware */
+	/* Enable external hardware */
 	hardware_enable(0);
 	hardware_enable(1);
 
-	platform_device_register(&m8xx_device);
-
 	for (i = 0 ; i < PCMCIA_SOCKETS_NO; i++) {
 		socket[i].slot = i;
 		socket[i].socket.owner = THIS_MODULE;
@@ -1317,30 +1225,105 @@ static int __init m8xx_init(void)
 		socket[i].socket.irq_mask = 0x000;
 		socket[i].socket.map_size = 0x1000;
 		socket[i].socket.io_offset = 0;
-		socket[i].socket.pci_irq = i  ? 7 : 9;
+		socket[i].socket.pci_irq = pcmcia_schlvl;
 		socket[i].socket.ops = &m8xx_services;
-		socket[i].socket.resource_ops = &pccard_iodyn_ops;
+		socket[i].socket.resource_ops = &pccard_nonstatic_ops;
 		socket[i].socket.cb_dev = NULL;
-		socket[i].socket.dev.parent = &m8xx_device.dev;
+		socket[i].socket.dev.parent = &ofdev->dev;
+		socket[i].pcmcia = pcmcia;
+		socket[i].bus_freq = ppc_proc_freq;
+		socket[i].hwirq = hwirq;
+
+
 	}
 
-	for (i = 0; i < PCMCIA_SOCKETS_NO; i++)
-		pcmcia_register_socket(&socket[i].socket);
+	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
+		status = pcmcia_register_socket(&socket[i].socket);
+		if (status < 0)
+			pcmcia_error("Socket register failed\n");
+	}
 
 	return 0;
 }
 
-static void __exit m8xx_exit(void)
+static int m8xx_remove(struct of_device* ofdev)
 {
-	int i;
+	u32 m, i;
+	struct pcmcia_win *w;
+	pcmconf8xx_t *pcmcia = socket[0].pcmcia;
+
+	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
+		w = (void *) &pcmcia->pcmc_pbr0;
+
+		out_be32(&pcmcia->pcmc_pscr, M8XX_PCMCIA_MASK(i));
+		out_be32(&pcmcia->pcmc_per,
+			in_be32(&pcmcia->pcmc_per) & ~M8XX_PCMCIA_MASK(i));
 
+		/* turn off interrupt and disable CxOE */
+		out_be32(M8XX_PGCRX(i), M8XX_PGCRX_CXOE);
+
+		/* turn off memory windows */
+		for (m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
+			out_be32(&w->or, 0); /* set to not valid */
+			w++;
+		}
+
+		/* turn off voltage */
+		voltage_set(i, 0, 0);
+
+		/* disable external hardware */
+		hardware_disable(i);
+	}
 	for (i = 0; i < PCMCIA_SOCKETS_NO; i++)
 		pcmcia_unregister_socket(&socket[i].socket);
 
-	m8xx_shutdown();
+	free_irq(pcmcia_schlvl, NULL);
 
-	platform_device_unregister(&m8xx_device);
-	driver_unregister(&m8xx_driver);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int m8xx_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	return pcmcia_socket_dev_suspend(&pdev->dev, state);
+}
+
+static int m8xx_resume(struct platform_device *pdev)
+{
+	return pcmcia_socket_dev_resume(&pdev->dev);
+}
+#else
+#define m8xx_suspend NULL
+#define m8xx_resume NULL
+#endif
+
+static struct of_device_id m8xx_pcmcia_match[] = {
+	{
+		.type = "pcmcia",
+		.compatible = "fsl,pq-pcmcia",
+	},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, m8xx_pcmcia_match);
+
+static struct of_platform_driver m8xx_pcmcia_driver = {
+	.name		= (char *) driver_name,
+	.match_table	= m8xx_pcmcia_match,
+	.probe		= m8xx_probe,
+	.remove		= m8xx_remove,
+	.suspend	= m8xx_suspend,
+	.resume		= m8xx_resume,
+};
+
+static int __init m8xx_init(void)
+{
+	return of_register_platform_driver(&m8xx_pcmcia_driver);
+}
+
+static void __exit m8xx_exit(void)
+{
+	of_unregister_platform_driver(&m8xx_pcmcia_driver);
 }
 
 module_init(m8xx_init);
diff --git a/include/asm-powerpc/mpc8xx.h b/include/asm-powerpc/mpc8xx.h
index 5803711..c1a6efc 100644
--- a/include/asm-powerpc/mpc8xx.h
+++ b/include/asm-powerpc/mpc8xx.h
@@ -23,6 +23,10 @@
 #include <platforms/8xx/mpc885ads.h>
 #endif
 
+#ifdef CONFIG_PCMCIA_M8XX 
+extern struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops;
+#endif 
+
 #endif /* CONFIG_8xx */
 #endif /* __CONFIG_8xx_DEFS */
 #endif /* __KERNEL__ */
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index 73710d6..12e631f 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -120,5 +120,10 @@ struct fsl_spi_platform_data {
 	u32	sysclk;
 };
 
+struct mpc8xx_pcmcia_ops {
+	void(*hw_ctrl)(int slot, int enable);
+	int(*voltage_set)(int slot, int vcc, int vpp);
+};
+
 #endif /* _FSL_DEVICE_H_ */
 #endif /* __KERNEL__ */

^ permalink raw reply related

* [PATCH 2/2] [POWERPC] 8xx: fix whitespace and indentation
From: Vitaly Bordug @ 2007-05-06  1:26 UTC (permalink / raw)
  To: linux-pcmcia; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20070506011004.12695.41514.stgit@localhost.localdomain>


Rolling forward PCMCIA driver, it was discovered that the indentation
in existing one is very odd. This patch is just result of Lindent run ontop
of culprit files.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>

---

 drivers/pcmcia/m8xx_pcmcia.c |  548 +++++++++++++++++++++---------------------
 1 files changed, 278 insertions(+), 270 deletions(-)

diff --git a/drivers/pcmcia/m8xx_pcmcia.c b/drivers/pcmcia/m8xx_pcmcia.c
index 40f6e20..79e14ab 100644
--- a/drivers/pcmcia/m8xx_pcmcia.c
+++ b/drivers/pcmcia/m8xx_pcmcia.c
@@ -113,7 +113,7 @@ MODULE_LICENSE("Dual MPL/GPL");
 #define CONFIG_PCMCIA_SLOT_B
 #endif
 
-#endif /* !defined(CONFIG_PCMCIA_SLOT_A) && !defined(CONFIG_PCMCIA_SLOT_B) */
+#endif				/* !defined(CONFIG_PCMCIA_SLOT_A) && !defined(CONFIG_PCMCIA_SLOT_B) */
 
 #if defined(CONFIG_PCMCIA_SLOT_A) && defined(CONFIG_PCMCIA_SLOT_B)
 
@@ -146,9 +146,9 @@ MODULE_LICENSE("Dual MPL/GPL");
 
 /* ------------------------------------------------------------------------- */
 
-#define PCMCIA_MEM_WIN_BASE 0xe0000000 /* base address for memory window 0   */
-#define PCMCIA_MEM_WIN_SIZE 0x04000000 /* each memory window is 64 MByte     */
-#define PCMCIA_IO_WIN_BASE  _IO_BASE   /* base address for io window 0       */
+#define PCMCIA_MEM_WIN_BASE 0xe0000000	/* base address for memory window 0   */
+#define PCMCIA_MEM_WIN_SIZE 0x04000000	/* each memory window is 64 MByte     */
+#define PCMCIA_IO_WIN_BASE  _IO_BASE	/* base address for io window 0       */
 /* ------------------------------------------------------------------------- */
 
 static int pcmcia_schlvl;
@@ -169,8 +169,8 @@ static u32 *m8xx_pgcrx[2];
  */
 
 struct pcmcia_win {
-	u32	br;
-	u32	or;
+	u32 br;
+	u32 or;
 };
 
 /*
@@ -214,7 +214,7 @@ struct pcmcia_win {
 
 /* we keep one lookup table per socket to check flags */
 
-#define PCMCIA_EVENTS_MAX 5  /* 4 max at a time + termination */
+#define PCMCIA_EVENTS_MAX 5	/* 4 max at a time + termination */
 
 struct event_table {
 	u32 regbit;
@@ -224,8 +224,8 @@ struct event_table {
 static const char driver_name[] = "m8xx-pcmcia";
 
 struct socket_info {
-	void	(*handler)(void *info, u32 events);
-	void	*info;
+	void (*handler) (void *info, u32 events);
+	void *info;
 
 	u32 slot;
 	pcmconf8xx_t *pcmcia;
@@ -234,7 +234,7 @@ struct socket_info {
 
 	socket_state_t state;
 	struct pccard_mem_map mem_win[PCMCIA_MEM_WIN_NO];
-	struct pccard_io_map  io_win[PCMCIA_IO_WIN_NO];
+	struct pccard_io_map io_win[PCMCIA_IO_WIN_NO];
 	struct event_table events[PCMCIA_EVENTS_MAX];
 	struct pcmcia_socket socket;
 };
@@ -248,8 +248,7 @@ static struct socket_info socket[PCMCIA_SOCKETS_NO];
 
 #define M8XX_SIZES_NO 32
 
-static const u32 m8xx_size_to_gray[M8XX_SIZES_NO] =
-{
+static const u32 m8xx_size_to_gray[M8XX_SIZES_NO] = {
 	0x00000001, 0x00000002, 0x00000008, 0x00000004,
 	0x00000080, 0x00000040, 0x00000010, 0x00000020,
 	0x00008000, 0x00004000, 0x00001000, 0x00002000,
@@ -265,7 +264,7 @@ static const u32 m8xx_size_to_gray[M8XX_SIZES_NO] =
 
 static irqreturn_t m8xx_interrupt(int irq, void *dev);
 
-#define PCMCIA_BMT_LIMIT (15*4)  /* Bus Monitor Timeout value */
+#define PCMCIA_BMT_LIMIT (15*4)	/* Bus Monitor Timeout value */
 
 /* ------------------------------------------------------------------------- */
 /* board specific stuff:                                                     */
@@ -289,8 +288,9 @@ static int voltage_set(int slot, int vcc, int vpp)
 {
 	u32 reg = 0;
 
-	switch(vcc) {
-	case 0: break;
+	switch (vcc) {
+	case 0:
+		break;
 	case 33:
 		reg |= BCSR1_PCVCTL4;
 		break;
@@ -301,11 +301,12 @@ static int voltage_set(int slot, int vcc, int vpp)
 		return 1;
 	}
 
-	switch(vpp) {
-	case 0: break;
+	switch (vpp) {
+	case 0:
+		break;
 	case 33:
 	case 50:
-		if(vcc == vpp)
+		if (vcc == vpp)
 			reg |= BCSR1_PCVCTL6;
 		else
 			return 1;
@@ -316,25 +317,29 @@ static int voltage_set(int slot, int vcc, int vpp)
 		return 1;
 	}
 
-	if(!((vcc == 50) || (vcc == 0)))
+	if (!((vcc == 50) || (vcc == 0)))
 		return 1;
 
 	/* first, turn off all power */
 
-	out_be32(((u32 *)RPX_CSR_ADDR), in_be32(((u32 *)RPX_CSR_ADDR)) & ~(BCSR1_PCVCTL4 | BCSR1_PCVCTL5 | BCSR1_PCVCTL6 | BCSR1_PCVCTL7));
+	out_be32(((u32 *) RPX_CSR_ADDR),
+		 in_be32(((u32 *) RPX_CSR_ADDR)) & ~(BCSR1_PCVCTL4 |
+						     BCSR1_PCVCTL5 |
+						     BCSR1_PCVCTL6 |
+						     BCSR1_PCVCTL7));
 
 	/* enable new powersettings */
 
-	out_be32(((u32 *)RPX_CSR_ADDR), in_be32(((u32 *)RPX_CSR_ADDR)) | reg);
+	out_be32(((u32 *) RPX_CSR_ADDR), in_be32(((u32 *) RPX_CSR_ADDR)) | reg);
 
 	return 0;
 }
 
 #define socket_get(_slot_) PCMCIA_SOCKET_KEY_5V
-#define hardware_enable(_slot_)  /* No hardware to enable */
-#define hardware_disable(_slot_) /* No hardware to disable */
+#define hardware_enable(_slot_)	/* No hardware to enable */
+#define hardware_disable(_slot_)	/* No hardware to disable */
 
-#endif /* CONFIG_RPXCLASSIC */
+#endif				/* CONFIG_RPXCLASSIC */
 
 /* FADS Boards from Motorola                                               */
 
@@ -346,43 +351,45 @@ static int voltage_set(int slot, int vcc, int vpp)
 {
 	u32 reg = 0;
 
-	switch(vcc) {
-		case 0:
-			break;
-		case 33:
-			reg |= BCSR1_PCCVCC0;
-			break;
-		case 50:
-			reg |= BCSR1_PCCVCC1;
-			break;
-		default:
-			return 1;
+	switch (vcc) {
+	case 0:
+		break;
+	case 33:
+		reg |= BCSR1_PCCVCC0;
+		break;
+	case 50:
+		reg |= BCSR1_PCCVCC1;
+		break;
+	default:
+		return 1;
 	}
 
-	switch(vpp) {
-		case 0:
-			break;
-		case 33:
-		case 50:
-			if(vcc == vpp)
-				reg |= BCSR1_PCCVPP1;
-			else
-				return 1;
-			break;
-		case 120:
-			if ((vcc == 33) || (vcc == 50))
-				reg |= BCSR1_PCCVPP0;
-			else
-				return 1;
-		default:
+	switch (vpp) {
+	case 0:
+		break;
+	case 33:
+	case 50:
+		if (vcc == vpp)
+			reg |= BCSR1_PCCVPP1;
+		else
 			return 1;
+		break;
+	case 120:
+		if ((vcc == 33) || (vcc == 50))
+			reg |= BCSR1_PCCVPP0;
+		else
+			return 1;
+	default:
+		return 1;
 	}
 
 	/* first, turn off all power */
-	out_be32((u32 *)BCSR1, in_be32((u32 *)BCSR1) & ~(BCSR1_PCCVCC_MASK | BCSR1_PCCVPP_MASK));
+	out_be32((u32 *) BCSR1,
+		 in_be32((u32 *) BCSR1) & ~(BCSR1_PCCVCC_MASK |
+					    BCSR1_PCCVPP_MASK));
 
 	/* enable new powersettings */
-	out_be32((u32 *)BCSR1, in_be32((u32 *)BCSR1) | reg);
+	out_be32((u32 *) BCSR1, in_be32((u32 *) BCSR1) | reg);
 
 	return 0;
 }
@@ -391,12 +398,12 @@ static int voltage_set(int slot, int vcc, int vpp)
 
 static void hardware_enable(int slot)
 {
-	out_be32((u32 *)BCSR1, in_be32((u32 *)BCSR1) & ~BCSR1_PCCEN);
+	out_be32((u32 *) BCSR1, in_be32((u32 *) BCSR1) & ~BCSR1_PCCEN);
 }
 
 static void hardware_disable(int slot)
 {
-	out_be32((u32 *)BCSR1, in_be32((u32 *)BCSR1) |  BCSR1_PCCEN);
+	out_be32((u32 *) BCSR1, in_be32((u32 *) BCSR1) | BCSR1_PCCEN);
 }
 
 #endif
@@ -410,7 +417,7 @@ static void hardware_disable(int slot)
 
 static inline void hardware_enable(int slot)
 {
-	 m8xx_pcmcia_ops.hw_ctrl(slot, 1);
+	m8xx_pcmcia_ops.hw_ctrl(slot, 1);
 }
 
 static inline void hardware_disable(int slot)
@@ -436,52 +443,53 @@ static int voltage_set(int slot, int vcc, int vpp)
 {
 	u8 reg = 0;
 
-	switch(vcc) {
-		case 0:
-			break;
-		case 33:
-			reg |= CSR2_VCC_33;
-			break;
-		case 50:
-			reg |= CSR2_VCC_50;
-			break;
-		default:
-			return 1;
+	switch (vcc) {
+	case 0:
+		break;
+	case 33:
+		reg |= CSR2_VCC_33;
+		break;
+	case 50:
+		reg |= CSR2_VCC_50;
+		break;
+	default:
+		return 1;
 	}
 
-	switch(vpp) {
-		case 0:
-			break;
-		case 33:
-		case 50:
-			if(vcc == vpp)
-				reg |= CSR2_VPP_VCC;
-			else
-				return 1;
-			break;
-		case 120:
-			if ((vcc == 33) || (vcc == 50))
-				reg |= CSR2_VPP_12;
-			else
-				return 1;
-		default:
+	switch (vpp) {
+	case 0:
+		break;
+	case 33:
+	case 50:
+		if (vcc == vpp)
+			reg |= CSR2_VPP_VCC;
+		else
+			return 1;
+		break;
+	case 120:
+		if ((vcc == 33) || (vcc == 50))
+			reg |= CSR2_VPP_12;
+		else
 			return 1;
+	default:
+		return 1;
 	}
 
 	/* first, turn off all power */
-	out_8((u8 *)MBX_CSR2_ADDR, in_8((u8 *)MBX_CSR2_ADDR) & ~(CSR2_VCC_MASK | CSR2_VPP_MASK));
+	out_8((u8 *) MBX_CSR2_ADDR,
+	      in_8((u8 *) MBX_CSR2_ADDR) & ~(CSR2_VCC_MASK | CSR2_VPP_MASK));
 
 	/* enable new powersettings */
-	out_8((u8 *)MBX_CSR2_ADDR, in_8((u8 *)MBX_CSR2_ADDR) | reg);
+	out_8((u8 *) MBX_CSR2_ADDR, in_8((u8 *) MBX_CSR2_ADDR) | reg);
 
 	return 0;
 }
 
 #define socket_get(_slot_) PCMCIA_SOCKET_KEY_5V
-#define hardware_enable(_slot_)  /* No hardware to enable */
-#define hardware_disable(_slot_) /* No hardware to disable */
+#define hardware_enable(_slot_)	/* No hardware to enable */
+#define hardware_disable(_slot_)	/* No hardware to disable */
 
-#endif /* CONFIG_MBX */
+#endif				/* CONFIG_MBX */
 
 #if defined(CONFIG_PRxK)
 #include <asm/cpld.h>
@@ -495,43 +503,46 @@ static int voltage_set(int slot, int vcc, int vpp)
 	u8 regread;
 	cpld_regs *ccpld = get_cpld();
 
-	switch(vcc) {
-		case 0:
-			break;
-		case 33:
-			reg |= PCMCIA_VCC_33;
-			break;
-		case 50:
-			reg |= PCMCIA_VCC_50;
-			break;
-		default:
-			return 1;
+	switch (vcc) {
+	case 0:
+		break;
+	case 33:
+		reg |= PCMCIA_VCC_33;
+		break;
+	case 50:
+		reg |= PCMCIA_VCC_50;
+		break;
+	default:
+		return 1;
 	}
 
-	switch(vpp) {
-		case 0:
-			break;
-		case 33:
-		case 50:
-			if(vcc == vpp)
-				reg |= PCMCIA_VPP_VCC;
-			else
-				return 1;
-			break;
-		case 120:
-			if ((vcc == 33) || (vcc == 50))
-				reg |= PCMCIA_VPP_12;
-			else
-				return 1;
-		default:
+	switch (vpp) {
+	case 0:
+		break;
+	case 33:
+	case 50:
+		if (vcc == vpp)
+			reg |= PCMCIA_VPP_VCC;
+		else
 			return 1;
+		break;
+	case 120:
+		if ((vcc == 33) || (vcc == 50))
+			reg |= PCMCIA_VPP_12;
+		else
+			return 1;
+	default:
+		return 1;
 	}
 
 	reg = reg >> (slot << 2);
 	regread = in_8(&ccpld->fpga_pc_ctl);
-	if (reg != (regread & ((PCMCIA_VCC_MASK | PCMCIA_VPP_MASK) >> (slot << 2)))) {
+	if (reg !=
+	    (regread & ((PCMCIA_VCC_MASK | PCMCIA_VPP_MASK) >> (slot << 2)))) {
 		/* enable new powersettings */
-		regread = regread & ~((PCMCIA_VCC_MASK | PCMCIA_VPP_MASK) >> (slot << 2));
+		regread =
+		    regread & ~((PCMCIA_VCC_MASK | PCMCIA_VPP_MASK) >>
+				(slot << 2));
 		out_8(&ccpld->fpga_pc_ctl, reg | regread);
 		msleep(100);
 	}
@@ -540,10 +551,10 @@ static int voltage_set(int slot, int vcc, int vpp)
 }
 
 #define socket_get(_slot_) PCMCIA_SOCKET_KEY_LV
-#define hardware_enable(_slot_)  /* No hardware to enable */
-#define hardware_disable(_slot_) /* No hardware to disable */
+#define hardware_enable(_slot_)	/* No hardware to enable */
+#define hardware_disable(_slot_)	/* No hardware to disable */
 
-#endif /* CONFIG_PRxK */
+#endif				/* CONFIG_PRxK */
 
 static u32 pending_events[PCMCIA_SOCKETS_NO];
 static DEFINE_SPINLOCK(pending_event_lock);
@@ -553,7 +564,7 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 	struct socket_info *s;
 	struct event_table *e;
 	unsigned int i, events, pscr, pipr, per;
-	pcmconf8xx_t	*pcmcia = socket[0].pcmcia;
+	pcmconf8xx_t *pcmcia = socket[0].pcmcia;
 
 	dprintk("Interrupt!\n");
 	/* get interrupt sources */
@@ -562,16 +573,16 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 	pipr = in_be32(&pcmcia->pcmc_pipr);
 	per = in_be32(&pcmcia->pcmc_per);
 
-	for(i = 0; i < PCMCIA_SOCKETS_NO; i++) {
+	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
 		s = &socket[i];
 		e = &s->events[0];
 		events = 0;
 
-		while(e->regbit) {
-			if(pscr & e->regbit)
+		while (e->regbit) {
+			if (pscr & e->regbit)
 				events |= e->eventbit;
 
-				e++;
+			e++;
 		}
 
 		/*
@@ -579,13 +590,11 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 		 * not too nice done,
 		 * we depend on that CD2 is the bit to the left of CD1...
 		 */
-		if(events & SS_DETECT)
-			if(((pipr & M8XX_PCMCIA_CD2(i)) >> 1) ^
-				(pipr & M8XX_PCMCIA_CD1(i)))
-			{
+		if (events & SS_DETECT)
+			if (((pipr & M8XX_PCMCIA_CD2(i)) >> 1) ^
+			    (pipr & M8XX_PCMCIA_CD1(i))) {
 				events &= ~SS_DETECT;
 			}
-
 #ifdef PCMCIA_GLITCHY_CD
 		/*
 		 * I've experienced CD problems with my ADS board.
@@ -593,24 +602,23 @@ static irqreturn_t m8xx_interrupt(int irq, void *dev)
 		 * real change of Card detection.
 		 */
 
-		if((events & SS_DETECT) &&
-		   ((pipr &
-		     (M8XX_PCMCIA_CD2(i) | M8XX_PCMCIA_CD1(i))) == 0) &&
-		   (s->state.Vcc | s->state.Vpp)) {
+		if ((events & SS_DETECT) &&
+		    ((pipr &
+		      (M8XX_PCMCIA_CD2(i) | M8XX_PCMCIA_CD1(i))) == 0) &&
+		    (s->state.Vcc | s->state.Vpp)) {
 			events &= ~SS_DETECT;
 			/*printk( "CD glitch workaround - CD = 0x%08x!\n",
-				(pipr & (M8XX_PCMCIA_CD2(i)
-					 | M8XX_PCMCIA_CD1(i))));*/
+			   (pipr & (M8XX_PCMCIA_CD2(i)
+			   | M8XX_PCMCIA_CD1(i)))); */
 		}
 #endif
 
 		/* call the handler */
 
 		dprintk("slot %u: events = 0x%02x, pscr = 0x%08x, "
-			"pipr = 0x%08x\n",
-			i, events, pscr, pipr);
+			"pipr = 0x%08x\n", i, events, pscr, pipr);
 
-		if(events) {
+		if (events) {
 			spin_lock(&pending_event_lock);
 			pending_events[i] |= events;
 			spin_unlock(&pending_event_lock);
@@ -643,11 +651,11 @@ static u32 m8xx_get_graycode(u32 size)
 {
 	u32 k;
 
-	for(k = 0; k < M8XX_SIZES_NO; k++)
-		if(m8xx_size_to_gray[k] == size)
+	for (k = 0; k < M8XX_SIZES_NO; k++)
+		if (m8xx_size_to_gray[k] == size)
 			break;
 
-	if((k == M8XX_SIZES_NO) || (m8xx_size_to_gray[k] == -1))
+	if ((k == M8XX_SIZES_NO) || (m8xx_size_to_gray[k] == -1))
 		k = -1;
 
 	return k;
@@ -657,7 +665,7 @@ static u32 m8xx_get_speed(u32 ns, u32 is_io, u32 bus_freq)
 {
 	u32 reg, clocks, psst, psl, psht;
 
-	if(!ns) {
+	if (!ns) {
 
 		/*
 		 * We get called with IO maps setup to 0ns
@@ -665,10 +673,10 @@ static u32 m8xx_get_speed(u32 ns, u32 is_io, u32 bus_freq)
 		 * They should be 255ns.
 		 */
 
-		if(is_io)
+		if (is_io)
 			ns = 255;
 		else
-			ns = 100;  /* fast memory if 0 */
+			ns = 100;	/* fast memory if 0 */
 	}
 
 	/*
@@ -679,23 +687,23 @@ static u32 m8xx_get_speed(u32 ns, u32 is_io, u32 bus_freq)
 
 /* how we want to adjust the timing - in percent */
 
-#define ADJ 180 /* 80 % longer accesstime - to be sure */
+#define ADJ 180			/* 80 % longer accesstime - to be sure */
 
 	clocks = ((bus_freq / 1000) * ns) / 1000;
-	clocks = (clocks * ADJ) / (100*1000);
-	if(clocks >= PCMCIA_BMT_LIMIT) {
-		printk( "Max access time limit reached\n");
-		clocks = PCMCIA_BMT_LIMIT-1;
+	clocks = (clocks * ADJ) / (100 * 1000);
+	if (clocks >= PCMCIA_BMT_LIMIT) {
+		printk("Max access time limit reached\n");
+		clocks = PCMCIA_BMT_LIMIT - 1;
 	}
 
-	psst = clocks / 7;          /* setup time */
-	psht = clocks / 7;          /* hold time */
-	psl  = (clocks * 5) / 7;    /* strobe length */
+	psst = clocks / 7;	/* setup time */
+	psht = clocks / 7;	/* hold time */
+	psl = (clocks * 5) / 7;	/* strobe length */
 
 	psst += clocks - (psst + psht + psl);
 
-	reg =  psst << 12;
-	reg |= psl  << 7;
+	reg = psst << 12;
+	reg |= psl << 7;
 	reg |= psht << 16;
 
 	return reg;
@@ -710,8 +718,8 @@ static int m8xx_get_status(struct pcmcia_socket *sock, unsigned int *value)
 
 	pipr = in_be32(&pcmcia->pcmc_pipr);
 
-	*value  = ((pipr & (M8XX_PCMCIA_CD1(lsock)
-			    | M8XX_PCMCIA_CD2(lsock))) == 0) ? SS_DETECT : 0;
+	*value = ((pipr & (M8XX_PCMCIA_CD1(lsock)
+			   | M8XX_PCMCIA_CD2(lsock))) == 0) ? SS_DETECT : 0;
 	*value |= (pipr & M8XX_PCMCIA_WP(lsock)) ? SS_WRPROT : 0;
 
 	if (s->state.flags & SS_IOCARD)
@@ -795,16 +803,16 @@ static int m8xx_get_status(struct pcmcia_socket *sock, unsigned int *value)
 	/* read out VS1 and VS2 */
 
 	reg = (pipr & M8XX_PCMCIA_VS_MASK(lsock))
-		>> M8XX_PCMCIA_VS_SHIFT(lsock);
+	    >> M8XX_PCMCIA_VS_SHIFT(lsock);
 
-	if(socket_get(lsock) == PCMCIA_SOCKET_KEY_LV) {
-		switch(reg) {
+	if (socket_get(lsock) == PCMCIA_SOCKET_KEY_LV) {
+		switch (reg) {
 		case 1:
 			*value |= SS_3VCARD;
-			break; /* GND, NC - 3.3V only */
+			break;	/* GND, NC - 3.3V only */
 		case 2:
 			*value |= SS_XVCARD;
-			break; /* NC. GND - x.xV only */
+			break;	/* NC. GND - x.xV only */
 		};
 	}
 
@@ -812,7 +820,7 @@ static int m8xx_get_status(struct pcmcia_socket *sock, unsigned int *value)
 	return 0;
 }
 
-static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
+static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t * state)
 {
 	int lsock = container_of(sock, struct socket_info, socket)->slot;
 	struct socket_info *s = &socket[lsock];
@@ -821,20 +829,20 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	unsigned long flags;
 	pcmconf8xx_t *pcmcia = socket[0].pcmcia;
 
-	dprintk( "SetSocket(%d, flags %#3.3x, Vcc %d, Vpp %d, "
-	      "io_irq %d, csc_mask %#2.2x)\n", lsock, state->flags,
-	      state->Vcc, state->Vpp, state->io_irq, state->csc_mask);
+	dprintk("SetSocket(%d, flags %#3.3x, Vcc %d, Vpp %d, "
+		"io_irq %d, csc_mask %#2.2x)\n", lsock, state->flags,
+		state->Vcc, state->Vpp, state->io_irq, state->csc_mask);
 
 	/* First, set voltage - bail out if invalid */
-	if(voltage_set(lsock, state->Vcc, state->Vpp))
+	if (voltage_set(lsock, state->Vcc, state->Vpp))
 		return -EINVAL;
 
-
 	/* Take care of reset... */
-	if(state->flags & SS_RESET)
-		out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) |  M8XX_PGCRX_CXRESET); /* active high */
+	if (state->flags & SS_RESET)
+		out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) | M8XX_PGCRX_CXRESET);	/* active high */
 	else
-		out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) & ~M8XX_PGCRX_CXRESET);
+		out_be32(M8XX_PGCRX(lsock),
+			 in_be32(M8XX_PGCRX(lsock)) & ~M8XX_PGCRX_CXRESET);
 
 	/* ... and output enable. */
 
@@ -846,10 +854,11 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	   no pullups are present -> the cards act wierd.
 	   So right now the buffers are enabled if the power is on. */
 
-	if(state->Vcc || state->Vpp)
-		out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) & ~M8XX_PGCRX_CXOE); /* active low */
+	if (state->Vcc || state->Vpp)
+		out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) & ~M8XX_PGCRX_CXOE);	/* active low */
 	else
-		out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) | M8XX_PGCRX_CXOE);
+		out_be32(M8XX_PGCRX(lsock),
+			 in_be32(M8XX_PGCRX(lsock)) | M8XX_PGCRX_CXOE);
 
 	/*
 	 * We'd better turn off interrupts before
@@ -866,17 +875,17 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	e = &s->events[0];
 	reg = 0;
 
-	if(state->csc_mask & SS_DETECT) {
+	if (state->csc_mask & SS_DETECT) {
 		e->eventbit = SS_DETECT;
 		reg |= e->regbit = (M8XX_PCMCIA_CD2(lsock)
 				    | M8XX_PCMCIA_CD1(lsock));
 		e++;
 	}
-	if(state->flags & SS_IOCARD) {
+	if (state->flags & SS_IOCARD) {
 		/*
 		 * I/O card
 		 */
-		if(state->csc_mask & SS_STSCHG) {
+		if (state->csc_mask & SS_STSCHG) {
 			e->eventbit = SS_STSCHG;
 			reg |= e->regbit = M8XX_PCMCIA_BVD1(lsock);
 			e++;
@@ -884,9 +893,10 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 		/*
 		 * If io_irq is non-zero we should enable irq.
 		 */
-		if(state->io_irq) {
+		if (state->io_irq) {
 			out_be32(M8XX_PGCRX(lsock),
-				 in_be32(M8XX_PGCRX(lsock)) | mk_int_int_mask(s->hwirq) << 24);
+				 in_be32(M8XX_PGCRX(lsock)) |
+				 mk_int_int_mask(s->hwirq) << 24);
 			/*
 			 * Strange thing here:
 			 * The manual does not tell us which interrupt
@@ -897,33 +907,32 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 			 * have to be cleared in PSCR in the interrupt handler.
 			 */
 			reg |= M8XX_PCMCIA_RDY_L(lsock);
-		}
-		else
-			out_be32(M8XX_PGCRX(lsock), in_be32(M8XX_PGCRX(lsock)) & 0x00ffffff);
-	}
-	else {
+		} else
+			out_be32(M8XX_PGCRX(lsock),
+				 in_be32(M8XX_PGCRX(lsock)) & 0x00ffffff);
+	} else {
 		/*
 		 * Memory card
 		 */
-		if(state->csc_mask & SS_BATDEAD) {
+		if (state->csc_mask & SS_BATDEAD) {
 			e->eventbit = SS_BATDEAD;
 			reg |= e->regbit = M8XX_PCMCIA_BVD1(lsock);
 			e++;
 		}
-		if(state->csc_mask & SS_BATWARN) {
+		if (state->csc_mask & SS_BATWARN) {
 			e->eventbit = SS_BATWARN;
 			reg |= e->regbit = M8XX_PCMCIA_BVD2(lsock);
 			e++;
 		}
 		/* What should I trigger on - low/high,raise,fall? */
-		if(state->csc_mask & SS_READY) {
+		if (state->csc_mask & SS_READY) {
 			e->eventbit = SS_READY;
-			reg |= e->regbit = 0; //??
+			reg |= e->regbit = 0;	//??
 			e++;
 		}
 	}
 
-	e->regbit = 0;  /* terminate list */
+	e->regbit = 0;		/* terminate list */
 
 	/*
 	 * Clear the status changed .
@@ -940,7 +949,9 @@ static int m8xx_set_socket(struct pcmcia_socket *sock, socket_state_t *state)
 	 * Ones will enable the interrupt.
 	 */
 
-	reg |= in_be32(&pcmcia->pcmc_per) & (M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
+	reg |=
+	    in_be32(&pcmcia->
+		    pcmc_per) & (M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
 	out_be32(&pcmcia->pcmc_per, reg);
 
 	spin_unlock_irqrestore(&events_lock, flags);
@@ -961,67 +972,66 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 	unsigned int reg, winnr;
 	pcmconf8xx_t *pcmcia = s->pcmcia;
 
-
 #define M8XX_SIZE (io->stop - io->start + 1)
 #define M8XX_BASE (PCMCIA_IO_WIN_BASE + io->start)
 
-	dprintk( "SetIOMap(%d, %d, %#2.2x, %d ns, "
-	      "%#4.4x-%#4.4x)\n", lsock, io->map, io->flags,
-	      io->speed, io->start, io->stop);
+	dprintk("SetIOMap(%d, %d, %#2.2x, %d ns, "
+		"%#4.4x-%#4.4x)\n", lsock, io->map, io->flags,
+		io->speed, io->start, io->stop);
 
 	if ((io->map >= PCMCIA_IO_WIN_NO) || (io->start > 0xffff)
 	    || (io->stop > 0xffff) || (io->stop < io->start))
 		return -EINVAL;
 
-	if((reg = m8xx_get_graycode(M8XX_SIZE)) == -1)
+	if ((reg = m8xx_get_graycode(M8XX_SIZE)) == -1)
 		return -EINVAL;
 
-	if(io->flags & MAP_ACTIVE) {
+	if (io->flags & MAP_ACTIVE) {
 
-		dprintk( "io->flags & MAP_ACTIVE\n");
+		dprintk("io->flags & MAP_ACTIVE\n");
 
 		winnr = (PCMCIA_MEM_WIN_NO * PCMCIA_SOCKETS_NO)
-			+ (lsock * PCMCIA_IO_WIN_NO) + io->map;
+		    + (lsock * PCMCIA_IO_WIN_NO) + io->map;
 
 		/* setup registers */
 
-		w = (void *) &pcmcia->pcmc_pbr0;
+		w = (void *)&pcmcia->pcmc_pbr0;
 		w += winnr;
 
-		out_be32(&w->or, 0); /* turn off window first */
+		out_be32(&w->or, 0);	/* turn off window first */
 		out_be32(&w->br, M8XX_BASE);
 
 		reg <<= 27;
-  		reg |= M8XX_PCMCIA_POR_IO |(lsock << 2);
+		reg |= M8XX_PCMCIA_POR_IO | (lsock << 2);
 
 		reg |= m8xx_get_speed(io->speed, 1, s->bus_freq);
 
-		if(io->flags & MAP_WRPROT)
+		if (io->flags & MAP_WRPROT)
 			reg |= M8XX_PCMCIA_POR_WRPROT;
 
-		/*if(io->flags & (MAP_16BIT | MAP_AUTOSZ))*/
-		if(io->flags & MAP_16BIT)
+		/*if(io->flags & (MAP_16BIT | MAP_AUTOSZ)) */
+		if (io->flags & MAP_16BIT)
 			reg |= M8XX_PCMCIA_POR_16BIT;
 
-		if(io->flags & MAP_ACTIVE)
+		if (io->flags & MAP_ACTIVE)
 			reg |= M8XX_PCMCIA_POR_VALID;
 
 		out_be32(&w->or, reg);
 
 		dprintk("Socket %u: Mapped io window %u at %#8.8x, "
-		      "OR = %#8.8x.\n", lsock, io->map, w->br, w->or);
+			"OR = %#8.8x.\n", lsock, io->map, w->br, w->or);
 	} else {
 		/* shutdown IO window */
 		winnr = (PCMCIA_MEM_WIN_NO * PCMCIA_SOCKETS_NO)
-			+ (lsock * PCMCIA_IO_WIN_NO) + io->map;
+		    + (lsock * PCMCIA_IO_WIN_NO) + io->map;
 
 		/* setup registers */
 
-		w = (void *) &pcmcia->pcmc_pbr0;
+		w = (void *)&pcmcia->pcmc_pbr0;
 		w += winnr;
 
-		out_be32(&w->or, 0); /* turn off window */
-		out_be32(&w->br, 0); /* turn off base address */
+		out_be32(&w->or, 0);	/* turn off window */
+		out_be32(&w->br, 0);	/* turn off base address */
 
 		dprintk("Socket %u: Unmapped io window %u at %#8.8x, "
 			"OR = %#8.8x.\n", lsock, io->map, w->br, w->or);
@@ -1029,15 +1039,14 @@ static int m8xx_set_io_map(struct pcmcia_socket *sock, struct pccard_io_map *io)
 
 	/* copy the struct and modify the copy */
 	s->io_win[io->map] = *io;
-	s->io_win[io->map].flags &= (MAP_WRPROT
-				     | MAP_16BIT
-				     | MAP_ACTIVE);
+	s->io_win[io->map].flags &= (MAP_WRPROT | MAP_16BIT | MAP_ACTIVE);
 	dprintk("SetIOMap exit\n");
 
 	return 0;
 }
 
-static int m8xx_set_mem_map(struct pcmcia_socket *sock, struct pccard_mem_map *mem)
+static int m8xx_set_mem_map(struct pcmcia_socket *sock,
+			    struct pccard_mem_map *mem)
 {
 	int lsock = container_of(sock, struct socket_info, socket)->slot;
 	struct socket_info *s = &socket[lsock];
@@ -1046,19 +1055,19 @@ static int m8xx_set_mem_map(struct pcmcia_socket *sock, struct pccard_mem_map *m
 	unsigned int reg, winnr;
 	pcmconf8xx_t *pcmcia = s->pcmcia;
 
-	dprintk( "SetMemMap(%d, %d, %#2.2x, %d ns, "
-	      "%#5.5lx, %#5.5x)\n", lsock, mem->map, mem->flags,
-	      mem->speed, mem->static_start, mem->card_start);
+	dprintk("SetMemMap(%d, %d, %#2.2x, %d ns, "
+		"%#5.5lx, %#5.5x)\n", lsock, mem->map, mem->flags,
+		mem->speed, mem->static_start, mem->card_start);
 
 	if ((mem->map >= PCMCIA_MEM_WIN_NO)
-//	    || ((mem->s) >= PCMCIA_MEM_WIN_SIZE)
+//          || ((mem->s) >= PCMCIA_MEM_WIN_SIZE)
 	    || (mem->card_start >= 0x04000000)
-	    || (mem->static_start & 0xfff)                /* 4KByte resolution */
-	    || (mem->card_start & 0xfff))
+	    || (mem->static_start & 0xfff)	/* 4KByte resolution */
+	    ||(mem->card_start & 0xfff))
 		return -EINVAL;
 
-	if((reg = m8xx_get_graycode(PCMCIA_MEM_WIN_SIZE)) == -1) {
-		printk( "Cannot set size to 0x%08x.\n", PCMCIA_MEM_WIN_SIZE);
+	if ((reg = m8xx_get_graycode(PCMCIA_MEM_WIN_SIZE)) == -1) {
+		printk("Cannot set size to 0x%08x.\n", PCMCIA_MEM_WIN_SIZE);
 		return -EINVAL;
 	}
 	reg <<= 27;
@@ -1067,50 +1076,47 @@ static int m8xx_set_mem_map(struct pcmcia_socket *sock, struct pccard_mem_map *m
 
 	/* Setup the window in the pcmcia controller */
 
-	w = (void *) &pcmcia->pcmc_pbr0;
+	w = (void *)&pcmcia->pcmc_pbr0;
 	w += winnr;
 
 	reg |= lsock << 2;
 
 	reg |= m8xx_get_speed(mem->speed, 0, s->bus_freq);
 
-	if(mem->flags & MAP_ATTRIB)
-		reg |=  M8XX_PCMCIA_POR_ATTRMEM;
+	if (mem->flags & MAP_ATTRIB)
+		reg |= M8XX_PCMCIA_POR_ATTRMEM;
 
-	if(mem->flags & MAP_WRPROT)
+	if (mem->flags & MAP_WRPROT)
 		reg |= M8XX_PCMCIA_POR_WRPROT;
 
-	if(mem->flags & MAP_16BIT)
+	if (mem->flags & MAP_16BIT)
 		reg |= M8XX_PCMCIA_POR_16BIT;
 
-	if(mem->flags & MAP_ACTIVE)
+	if (mem->flags & MAP_ACTIVE)
 		reg |= M8XX_PCMCIA_POR_VALID;
 
 	out_be32(&w->or, reg);
 
 	dprintk("Socket %u: Mapped memory window %u at %#8.8x, "
-	      "OR = %#8.8x.\n", lsock, mem->map, w->br, w->or);
+		"OR = %#8.8x.\n", lsock, mem->map, w->br, w->or);
 
-	if(mem->flags & MAP_ACTIVE) {
+	if (mem->flags & MAP_ACTIVE) {
 		/* get the new base address */
 		mem->static_start = PCMCIA_MEM_WIN_BASE +
-			(PCMCIA_MEM_WIN_SIZE * winnr)
-			+ mem->card_start;
+		    (PCMCIA_MEM_WIN_SIZE * winnr)
+		    + mem->card_start;
 	}
 
 	dprintk("SetMemMap(%d, %d, %#2.2x, %d ns, "
-	      "%#5.5lx, %#5.5x)\n", lsock, mem->map, mem->flags,
-	      mem->speed, mem->static_start, mem->card_start);
+		"%#5.5lx, %#5.5x)\n", lsock, mem->map, mem->flags,
+		mem->speed, mem->static_start, mem->card_start);
 
 	/* copy the struct and modify the copy */
 
 	old = &s->mem_win[mem->map];
 
 	*old = *mem;
-	old->flags &= (MAP_ATTRIB
-		       | MAP_WRPROT
-		       | MAP_16BIT
-		       | MAP_ACTIVE);
+	old->flags &= (MAP_ATTRIB | MAP_WRPROT | MAP_16BIT | MAP_ACTIVE);
 
 	return 0;
 }
@@ -1121,7 +1127,7 @@ static int m8xx_sock_init(struct pcmcia_socket *sock)
 	pccard_io_map io = { 0, 0, 0, 0, 1 };
 	pccard_mem_map mem = { 0, 0, 0, 0, 0, 0 };
 
-	dprintk( "sock_init(%d)\n", s);
+	dprintk("sock_init(%d)\n", s);
 
 	m8xx_set_socket(sock, &dead_socket);
 	for (i = 0; i < PCMCIA_IO_WIN_NO; i++) {
@@ -1143,7 +1149,7 @@ static int m8xx_sock_suspend(struct pcmcia_socket *sock)
 }
 
 static struct pccard_operations m8xx_services = {
-	.init	= m8xx_sock_init,
+	.init = m8xx_sock_init,
 	.suspend = m8xx_sock_suspend,
 	.get_status = m8xx_get_status,
 	.set_socket = m8xx_set_socket,
@@ -1151,7 +1157,8 @@ static struct pccard_operations m8xx_services = {
 	.set_mem_map = m8xx_set_mem_map,
 };
 
-static int __init m8xx_probe(struct of_device *ofdev, const struct of_device_id *match)
+static int __init m8xx_probe(struct of_device *ofdev,
+			     const struct of_device_id *match)
 {
 	struct pcmcia_win *w;
 	unsigned int i, m, hwirq;
@@ -1162,49 +1169,50 @@ static int __init m8xx_probe(struct of_device *ofdev, const struct of_device_id
 	pcmcia_info("%s\n", version);
 
 	pcmcia = of_iomap(np, 0);
-	if(pcmcia == NULL)
+	if (pcmcia == NULL)
 		return -EINVAL;
 
 	pcmcia_schlvl = irq_of_parse_and_map(np, 0);
-	hwirq  = irq_map[pcmcia_schlvl].hwirq;
+	hwirq = irq_map[pcmcia_schlvl].hwirq;
 	if (pcmcia_schlvl < 0)
 		return -EINVAL;
 
 	m8xx_pgcrx[0] = &pcmcia->pcmc_pgcra;
 	m8xx_pgcrx[1] = &pcmcia->pcmc_pgcrb;
 
-
 	pcmcia_info(PCMCIA_BOARD_MSG " using " PCMCIA_SLOT_MSG
 		    " with IRQ %u  (%d). \n", pcmcia_schlvl, hwirq);
 
 	/* Configure Status change interrupt */
 
-	if(request_irq(pcmcia_schlvl, m8xx_interrupt, IRQF_SHARED,
-			  driver_name, socket)) {
+	if (request_irq(pcmcia_schlvl, m8xx_interrupt, IRQF_SHARED,
+			driver_name, socket)) {
 		pcmcia_error("Cannot allocate IRQ %u for SCHLVL!\n",
 			     pcmcia_schlvl);
 		return -1;
 	}
 
-	w = (void *) &pcmcia->pcmc_pbr0;
+	w = (void *)&pcmcia->pcmc_pbr0;
 
-	out_be32(&pcmcia->pcmc_pscr, M8XX_PCMCIA_MASK(0)| M8XX_PCMCIA_MASK(1));
+	out_be32(&pcmcia->pcmc_pscr, M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
 	clrbits32(&pcmcia->pcmc_per, M8XX_PCMCIA_MASK(0) | M8XX_PCMCIA_MASK(1));
 
 	/* connect interrupt and disable CxOE */
 
-	out_be32(M8XX_PGCRX(0), M8XX_PGCRX_CXOE | (mk_int_int_mask(hwirq) << 16));
-	out_be32(M8XX_PGCRX(1), M8XX_PGCRX_CXOE | (mk_int_int_mask(hwirq) << 16));
+	out_be32(M8XX_PGCRX(0),
+		 M8XX_PGCRX_CXOE | (mk_int_int_mask(hwirq) << 16));
+	out_be32(M8XX_PGCRX(1),
+		 M8XX_PGCRX_CXOE | (mk_int_int_mask(hwirq) << 16));
 
 	/* intialize the fixed memory windows */
 
-	for(i = 0; i < PCMCIA_SOCKETS_NO; i++){
+	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
 		for (m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
 			out_be32(&w->br, PCMCIA_MEM_WIN_BASE +
-				(PCMCIA_MEM_WIN_SIZE
-				 * (m + i * PCMCIA_MEM_WIN_NO)));
+				 (PCMCIA_MEM_WIN_SIZE
+				  * (m + i * PCMCIA_MEM_WIN_NO)));
 
-			out_be32(&w->or, 0);  /* set to not valid */
+			out_be32(&w->or, 0);	/* set to not valid */
 
 			w++;
 		}
@@ -1218,10 +1226,11 @@ static int __init m8xx_probe(struct of_device *ofdev, const struct of_device_id
 	hardware_enable(0);
 	hardware_enable(1);
 
-	for (i = 0 ; i < PCMCIA_SOCKETS_NO; i++) {
+	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
 		socket[i].slot = i;
 		socket[i].socket.owner = THIS_MODULE;
-		socket[i].socket.features = SS_CAP_PCCARD | SS_CAP_MEM_ALIGN | SS_CAP_STATIC_MAP;
+		socket[i].socket.features =
+		    SS_CAP_PCCARD | SS_CAP_MEM_ALIGN | SS_CAP_STATIC_MAP;
 		socket[i].socket.irq_mask = 0x000;
 		socket[i].socket.map_size = 0x1000;
 		socket[i].socket.io_offset = 0;
@@ -1234,7 +1243,6 @@ static int __init m8xx_probe(struct of_device *ofdev, const struct of_device_id
 		socket[i].bus_freq = ppc_proc_freq;
 		socket[i].hwirq = hwirq;
 
-
 	}
 
 	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
@@ -1246,25 +1254,25 @@ static int __init m8xx_probe(struct of_device *ofdev, const struct of_device_id
 	return 0;
 }
 
-static int m8xx_remove(struct of_device* ofdev)
+static int m8xx_remove(struct of_device *ofdev)
 {
 	u32 m, i;
 	struct pcmcia_win *w;
 	pcmconf8xx_t *pcmcia = socket[0].pcmcia;
 
 	for (i = 0; i < PCMCIA_SOCKETS_NO; i++) {
-		w = (void *) &pcmcia->pcmc_pbr0;
+		w = (void *)&pcmcia->pcmc_pbr0;
 
 		out_be32(&pcmcia->pcmc_pscr, M8XX_PCMCIA_MASK(i));
 		out_be32(&pcmcia->pcmc_per,
-			in_be32(&pcmcia->pcmc_per) & ~M8XX_PCMCIA_MASK(i));
+			 in_be32(&pcmcia->pcmc_per) & ~M8XX_PCMCIA_MASK(i));
 
 		/* turn off interrupt and disable CxOE */
 		out_be32(M8XX_PGCRX(i), M8XX_PGCRX_CXOE);
 
 		/* turn off memory windows */
 		for (m = 0; m < PCMCIA_MEM_WIN_NO; m++) {
-			out_be32(&w->or, 0); /* set to not valid */
+			out_be32(&w->or, 0);	/* set to not valid */
 			w++;
 		}
 
@@ -1299,21 +1307,21 @@ static int m8xx_resume(struct platform_device *pdev)
 
 static struct of_device_id m8xx_pcmcia_match[] = {
 	{
-		.type = "pcmcia",
-		.compatible = "fsl,pq-pcmcia",
-	},
+	 .type = "pcmcia",
+	 .compatible = "fsl,pq-pcmcia",
+	 },
 	{},
 };
 
 MODULE_DEVICE_TABLE(of, m8xx_pcmcia_match);
 
 static struct of_platform_driver m8xx_pcmcia_driver = {
-	.name		= (char *) driver_name,
-	.match_table	= m8xx_pcmcia_match,
-	.probe		= m8xx_probe,
-	.remove		= m8xx_remove,
-	.suspend	= m8xx_suspend,
-	.resume		= m8xx_resume,
+	.name = (char *)driver_name,
+	.match_table = m8xx_pcmcia_match,
+	.probe = m8xx_probe,
+	.remove = m8xx_remove,
+	.suspend = m8xx_suspend,
+	.resume = m8xx_resume,
 };
 
 static int __init m8xx_init(void)

^ permalink raw reply related

* Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
From: Segher Boessenkool @ 2007-05-06  1:04 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-dev, linux-pcmcia, linux-kernel
In-Reply-To: <20070506004737.31233.52611.stgit@localhost.localdomain>

> +		pcmcia@0080 {

> +			#interrupt-cells = <1>;

> +			interrupt-parent = <ff000000>;
> +			interrupts = <d 1>;
> +		};

Since this node's children's interrupt representation
is different from the node's parent's, you need an
interrupt-map in here.  You also forgot "#address-cells"
and I think you need "ranges" too?


Segher

^ permalink raw reply

* Re: [PATCH 5/6] Support for the Ebony 440GP reference board in arch/powerpc
From: Segher Boessenkool @ 2007-05-06  1:58 UTC (permalink / raw)
  To: David Gibson; +Cc: Olof Johansson, ppcdev, Milton Miller
In-Reply-To: <20070506004319.GH11303@localhost.localdomain>

>>> How?   CPP doesn't apply inside the strings.
>>
>> So put it outside the strings:
>>
>> +     asm volatile (
>> +             "mfspr  %0," #SPRN_DBCR0 "\n\t"
>> +             "oris   %0,%0," #DBCR0_RST_SYSTEM "@h\n\t"
>> +             "mtspr  " #SPRN_DBCR0 ",%0"
>> +             : "=&r"(tmp));
>
> Um, I think stringify(SPRN_DBCR0) is needed there, not just a bare #.

Yes of course.  Someone is paying attention ;-) (*)

> At which point it's not entirely clear to be that just using the "i"
> constraint isn't the simplest option after all.

Sure.  Or even hardcoding the values (with comments),
which would show why a bare "oris" is correct at all.


Segher




(*) ... just not me

^ permalink raw reply

* Re: [PATCH v2 6/7] Holly DTS
From: Segher Boessenkool @ 2007-05-06  0:47 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <1178400972.3393.64.camel@zod.rchland.ibm.com>

>>>>> +	compatible = "ppc750-tsi";
>>>>
>>>> The needs to be more specific as well; "ibm,holly"
>>>> or something.
>>>
>>> Why?  Holly and Hickory share the same memory map and devices.  The
>>> only
>>> thing that differs from what I can tell is the actual CPU itself.
>>
>> And the model number I suspect.
>>
>> "ppc750-tsi" really isn't good enough as a "compatible"
>> property, there are many many more boards with some 750
>> and some TSI bridge.
>
> Ok.  It really doesn't matter one way or another to me, since it's
> something I'm defining anyway.  "ibm,holly" and/or "ibm,hickory" or
> "ibm,ppc750-tsi" would likely work.

That last one still has the same problem, just now
only for IBM boards.  Maybe go fancy and do
"ibm,ppc750-tsi-holly" or something.  Your decision,
just make it specific enough.

> Actually, I might muck around with the PVR in the bootwrapper and poke
> the correct model and compatible nodes in there.  That might work out
> well enough.

Yeah, that should work great.

>>> It's not useless.  The TSI code probes by device_type all over the
>>> place.  Do you have an example of how to probe for something like 
>>> this
>>> without using device_type?
>>
>> Sure: use "compatible" instead.
>>
>> This problem is all over the place, don't worry
>> about it too much.  It would be good if new ports
>> could stop adding to the madness though ;-)
>
> For now, I'm inclined to leave it as is to minimize the amount of 
> things
> needed to actually get Holly merged.  However, I'll start looking at
> doing it via compatible shortly.

Just make sure your "compatible" property is good
to go; then when the kernel starts doing the right
thing, your tree still works.

>>>>> +	  	MPIC: pic@7400 {
>>>>> +			built-in;
>>>>
>>>> Why say this for MPIC only, not for most other nodes?
>>>> What binding defines "built-in", anyway?
>>>
>>> http://playground.sun.com/1275/bindings/chrp/chrp1_8a.ps
>>
>> But this isn't a CHRP compatible board, that binding
>> doesn't apply.
>
> I'm not pretending it is.  You just asked which binding defined it and 
> I
> was proud of myself for actually finding one ;).

Heh sure :-)

> It doesn't actually do anything in this case, so it can be removed.

Exactly.

Cheers,


Segher

^ permalink raw reply

* Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
From: David Gibson @ 2007-05-06  2:04 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linux-pcmcia, linux-kernel, linuxppc-dev
In-Reply-To: <67c6bf6b7d37f5e133398ea83edeac78@kernel.crashing.org>

On Sun, May 06, 2007 at 03:04:14AM +0200, Segher Boessenkool wrote:
> > +		pcmcia@0080 {
> 
> > +			#interrupt-cells = <1>;
> 
> > +			interrupt-parent = <ff000000>;
> > +			interrupts = <d 1>;
> > +		};
> 
> Since this node's children's interrupt representation
> is different from the node's parent's, you need an
> interrupt-map in here.  You also forgot "#address-cells"
> and I think you need "ranges" too?

And we should use a reference, instead of an implicit phandle for the
interrupt-parent.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH] Fix interrupt distribution in ppc970
From: Milton Miller @ 2007-05-06  6:52 UTC (permalink / raw)
  To: mohan; +Cc: kexec, fastboot, ppcdev, Paul Mackerras, Anton Blanchard
In-Reply-To: <20070503144721.GA28460@in.ibm.com>

On May 3, 2007, at 9:47 AM, Mohan Kumar M wrote:
> On Thu, Apr 26, 2007 at 09:42:50AM -0500, Milton Miller wrote:
>> Yes.   The whole point of
>>> -static int get_irq_server(unsigned int virq)
>>> +static int get_irq_server(unsigned int virq, unsigned int
>>> strict_check)
>> was to factor out the common code in this function.
>
> Milton,
>
> How about this patch?

Getting closer, still not right.

>
> Index: linux-2.6.21.1/arch/powerpc/platforms/pseries/xics.c
> ===================================================================
> --- linux-2.6.21.1.orig/arch/powerpc/platforms/pseries/xics.c
> +++ linux-2.6.21.1/arch/powerpc/platforms/pseries/xics.c
> @@ -156,9 +156,9 @@ static inline void lpar_qirr_info(int n_
>
>
>  #ifdef CONFIG_SMP
> -static int get_irq_server(unsigned int virq)
> +static int get_irq_server(unsigned int virq, unsigned int 
> strict_check)
>  {
> -	unsigned int server;
> +	int server;
>  	/* For the moment only implement delivery to all cpus or one cpu */
>  	cpumask_t cpumask = irq_desc[virq].affinity;
>  	cpumask_t tmp = CPU_MASK_NONE;
> @@ -166,22 +166,28 @@ static int get_irq_server(unsigned int v
>  	if (!distribute_irqs)
>  		return default_server;
>
> -	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> -		server = default_distrib_server;
> -	} else {
> +	if (!cpus_equal(cpumask, CPU_MASK_ALL)) {
>  		cpus_and(tmp, cpu_online_map, cpumask);
>
> -		if (cpus_empty(tmp))
> -			server = default_distrib_server;
> +		server = first_cpu(tmp);
> +
> +		if (server < NR_CPUS)
> +			return get_hard_smp_processor_id(server);
> +		else {
> +			if (strict_check)
> +				return -1;


> +			else
> +				return default_distrib_server;
> +		}
> +	} else {

Take out the above 4 lines, so that the return always has cpu_online
vs cpu_present factored in, by falling through from specific mask to
default server selection.

> +		if (cpus_equal(cpu_online_map, cpu_present_map))
> +			return default_distrib_server;
>  		else
> -			server = get_hard_smp_processor_id(first_cpu(tmp));
> +			return default_server;
>  	}

[This matching brace will go and indent will change.]

> -
> -	return server;
> -
>  }
...

> @@ -398,8 +404,7 @@ static void xics_set_affinity(unsigned i
>  	unsigned int irq;
>  	int status;
>  	int xics_status[2];
> -	unsigned long newmask;
> -	cpumask_t tmp = CPU_MASK_NONE;
> +	int irq_server;
>
>  	irq = (unsigned int)irq_map[virq].hwirq;
>  	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
> @@ -413,18 +418,28 @@ static void xics_set_affinity(unsigned i
>  		return;
>  	}
>
> -	/* For the moment only implement delivery to all cpus or one cpu */
> -	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> -		newmask = default_distrib_server;
> -	} else {
> -		cpus_and(tmp, cpu_online_map, cpumask);
> -		if (cpus_empty(tmp))
> +	/* Get current irq_server for the given irq */
> +	irq_server = get_irq_server(irq, 1);
> +	if (irq_server == -1) {
> +		printk(KERN_ERR "xics_set_affinity: Invalid cpumask\n");

WARNING or NOTICE would be fine.  The interrupt number should be 
printed.
and maybe "No online cpus in <cpumask> for irq %d" would be better (I
think there is a print cpumask helper).

> +		return;
> +	}
> +
> +	/* For the moment only implement delivery to all cpus or one cpu.
> +	 * Compare the irq_server with the new cpumask. If the irq_server
> +	 * is specified in cpumask, do the required rtas_call, otherwise
> +	 * return by printing an error message
> +	 */
> +	if (!cpus_equal(cpumask, CPU_MASK_ALL)) {
> +		if (!cpu_isset(irq_server, cpumask)) {
> +			printk(KERN_ERR "xics_set_affinity: Invalid "
> +							"cpumask\n");
>  			return;

I don't understand what you are trying to do here.  We already
chose the mask, and printed an error if we got -1 due to strict.
I think this can just be dropped?

> -		newmask = get_hard_smp_processor_id(first_cpu(tmp));
> +		}
>  	}
>
>  	status = rtas_call(ibm_set_xive, 3, 1, NULL,
> -				irq, newmask, xics_status[1]);
> +				irq, irq_server, xics_status[1]);
>
>  	if (status) {
>  		printk(KERN_ERR "xics_set_affinity: irq=%u ibm,set-xive "


milton

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox