Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
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? Well, in fact it does not introduce SoC device different from any others represented inside soc885 node. mk_int_int_mask() is just special way of enabling irq for PCMCIA stuff, in addition to normal pic stuff. I have no idea what you mean here. Care to try again? Emm. Why would I need #address-cells and ranges here? it uses parent bus address space... "#address-cells" is 3 for pcmcia, so not the default value (which is 2), so you need to put it in. The value of this property is not inherited from the parent node. Absence of a "ranges" property means the child bus is *not* direct mapped into the parent bus space. If the mapping you need is 1-1, put in an empty "ranges" property; if not, you have to put the correct mapping in. Segher - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Sun, 6 May 2007 12:04:27 +1000
David Gibson wrote:
> On Sun, May 06, 2007 at 03:04:14AM +0200, Segher Boessenkool wrote:
> > > + [EMAIL PROTECTED] {
> >
> > > + #interrupt-cells = <1>;
> >
> > > + interrupt-parent = ;
> > > + interrupts = ;
> > > + };
> >
> > 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.
>
I have one more patch for this - I think it makes sense to append it to the
series...
Thinking it would be better to do the phandles->labels transition in one step
for all the relevant
stuff in dts as well.
--
Sincerely, Vitaly
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Sun, 6 May 2007 03:04:14 +0200
Segher Boessenkool wrote:
> > + [EMAIL PROTECTED] {
>
> > + #interrupt-cells = <1>;
>
> > + interrupt-parent = ;
> > + interrupts = ;
> > + };
>
> 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?
>
Well, in fact it does not introduce SoC device different from any others
represented inside soc885 node. mk_int_int_mask() is just special
way of enabling irq for PCMCIA stuff, in addition to normal pic stuff.
Emm. Why would I need #address-cells and ranges here? it uses parent bus
address space...
--
Sincerely, Vitaly
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Sun, 6 May 2007 03:04:14 +0200
Segher Boessenkool wrote:
+ [EMAIL PROTECTED] {
+ #interrupt-cells = 1;
+ interrupt-parent = ff00;
+ 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?
Well, in fact it does not introduce SoC device different from any others
represented inside soc885 node. mk_int_int_mask() is just special
way of enabling irq for PCMCIA stuff, in addition to normal pic stuff.
Emm. Why would I need #address-cells and ranges here? it uses parent bus
address space...
--
Sincerely, Vitaly
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Sun, 6 May 2007 12:04:27 +1000
David Gibson wrote:
On Sun, May 06, 2007 at 03:04:14AM +0200, Segher Boessenkool wrote:
+ [EMAIL PROTECTED] {
+ #interrupt-cells = 1;
+ interrupt-parent = ff00;
+ 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.
I have one more patch for this - I think it makes sense to append it to the
series...
Thinking it would be better to do the phandles-labels transition in one step
for all the relevant
stuff in dts as well.
--
Sincerely, Vitaly
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
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? Well, in fact it does not introduce SoC device different from any others represented inside soc885 node. mk_int_int_mask() is just special way of enabling irq for PCMCIA stuff, in addition to normal pic stuff. I have no idea what you mean here. Care to try again? Emm. Why would I need #address-cells and ranges here? it uses parent bus address space... #address-cells is 3 for pcmcia, so not the default value (which is 2), so you need to put it in. The value of this property is not inherited from the parent node. Absence of a ranges property means the child bus is *not* direct mapped into the parent bus space. If the mapping you need is 1-1, put in an empty ranges property; if not, you have to put the correct mapping in. Segher - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Sun, May 06, 2007 at 03:04:14AM +0200, Segher Boessenkool wrote:
> > + [EMAIL PROTECTED] {
>
> > + #interrupt-cells = <1>;
>
> > + interrupt-parent = ;
> > + interrupts = ;
> > + };
>
> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
+ [EMAIL PROTECTED] {
+ #interrupt-cells = <1>;
+ interrupt-parent = ;
+ interrupts = ;
+ };
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Fri, 4 May 2007 12:35:43 -0700 Andrew Morton wrote: > On Fri, 04 May 2007 03:57:51 +0400 > Vitaly Bordug <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Fri, 4 May 2007 12:35:43 -0700 Andrew Morton wrote: On Fri, 04 May 2007 03:57:51 +0400 Vitaly Bordug [EMAIL PROTECTED] 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
+ [EMAIL PROTECTED] {
+ #interrupt-cells = 1;
+ interrupt-parent = ff00;
+ 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Sun, May 06, 2007 at 03:04:14AM +0200, Segher Boessenkool wrote:
+ [EMAIL PROTECTED] {
+ #interrupt-cells = 1;
+ interrupt-parent = ff00;
+ 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Fri, 04 May 2007 03:57:51 +0400
Vitaly Bordug <[EMAIL PROTECTED]> 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.
> +
> [EMAIL PROTECTED] {
> linux,phandle = ;
> #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
> #include
> #include
> +#include
>
> #include
> #include
> @@ -49,6 +50,10 @@
>
> #include "sysdev/mpc8xx_pic.h"
>
> +#ifdef CONFIG_PCMCIA_M8XX
> +struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops;
> +#endif
Please declare this in a header file.
> +/* Some internal interrupt registers use an 8-bit mask for the interrupt
> + * level instead of a number.
> + */
Standard Linux commenting style is:
/*
* Some internal interrupt registers use an 8-bit mask for the interrupt
* level instead of a number.
*/
> +#define mk_int_int_mask(IL) (1 << (7 - (IL/2)))
Insufficiently parenthesised?
static inline functions are preferred.
> +#ifdef CONFIG_PCMCIA_M8XX
> +extern struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops;
Please don't ever put extern declarations in C files. Declare it in a
header, include that header in the C file which contains the definition as
well as within all C files which use the symbol.
> +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 +122,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 +335,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);
Missing a tab.
> + 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;
> +}
Standard Linux layout for switch statements is:
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;
> +}
Ditto.
> +/* first, turn off all power */
> +clrbits32(bcsr_io, 0x0061);
> +
> +/* 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..01e4a40 100644
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
>
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Fri, 04 May 2007 03:57:51 +0400
Vitaly Bordug [EMAIL PROTECTED] 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.
+
[EMAIL PROTECTED] {
linux,phandle = ff00;
#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
Please declare this in a header file.
+/* Some internal interrupt registers use an 8-bit mask for the interrupt
+ * level instead of a number.
+ */
Standard Linux commenting style is:
/*
* Some internal interrupt registers use an 8-bit mask for the interrupt
* level instead of a number.
*/
+#define mk_int_int_mask(IL) (1 (7 - (IL/2)))
Insufficiently parenthesised?
static inline functions are preferred.
+#ifdef CONFIG_PCMCIA_M8XX
+extern struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops;
Please don't ever put extern declarations in C files. Declare it in a
header, include that header in the C file which contains the definition as
well as within all C files which use the symbol.
+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 +122,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 +335,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);
Missing a tab.
+ 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;
+}
Standard Linux layout for switch statements is:
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;
+}
Ditto.
+/* first, turn off all power */
+clrbits32(bcsr_io, 0x0061);
+
+/* 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..01e4a40 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -1028,6 +1028,18 @@ err:
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Friday 04 May 2007, Vitaly Bordug 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. > > Signed-off-by: Vitaly Bordug <[EMAIL PROTECTED]> Acked-by: Arnd Bergmann <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
For example, you could make this compatible = "8xx\0mpc885ads"; "mpc885ads-pcmcia\0mpc8xx-pcmcia" or something like that. Right. I can never remember what goes first... It doesn't really matter all that much; "correct" drivers probe for the most specific thing first, then the next most specific thing they support, etc. It is mostly a convention. The important thing is that you can't just call yourself "8xx", that is way to generic a name. Segher - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Thursday 03 May 2007, Segher Boessenkool wrote: > > For example, you could make this > > > > compatible = "8xx\0mpc885ads"; > > "mpc885ads-pcmcia\0mpc8xx-pcmcia" or something like that. Right. I can never remember what goes first... Arnd <>< - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
+ [EMAIL PROTECTED] {
+ compatible = "8xx";
The compatible property should be a little more specific, imho. Since
there
are differences in how things are done depending on the board, it
would be
good to tell the exact method from the pcmcia node itself.
Just "8xx" isn't good enough -- at a very minimum it
needs to say this is a PCMCIA controller!
For example, you could make this
compatible = "8xx\0mpc885ads";
"mpc885ads-pcmcia\0mpc8xx-pcmcia" or something like that.
Segher
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Thursday 03 May 2007, Vitaly Bordug 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.
Very nice, looks much better now than the previous version.
> diff --git a/arch/powerpc/boot/dts/mpc885ads.dts
> b/arch/powerpc/boot/dts/mpc885ads.dts
> index 110bf61..89d585f 100644
> --- a/arch/powerpc/boot/dts/mpc885ads.dts
> +++ b/arch/powerpc/boot/dts/mpc885ads.dts
> @@ -112,6 +112,18 @@
> compatible = "CPM";
> };
>
> + [EMAIL PROTECTED] {
> + linux,phandle = <0080>;
> + #interrupt-cells = <1>;
> + #size-cells = <2>;
> + compatible = "8xx";
> + device_type = "pcmcia";
> + reg = <80 80>;
> + clock-frequency = <2faf080>;
> + interrupt-parent = ;
> + interrupts = ;
> + };
> +
> [EMAIL PROTECTED] {
> linux,phandle = ;
> #address-cells = <1>;
The compatible property should be a little more specific, imho. Since there
are differences in how things are done depending on the board, it would be
good to tell the exact method from the pcmcia node itself.
For example, you could make this
compatible = "8xx\0mpc885ads";
Your code doesn't currently use this, I like to have the option.
> +#ifdef CONFIG_PCMCIA_M8XX
> +static void pcmcia_hw_setup(int slot, int enable);
> +static int pcmcia_set_voltage(int slot, int vcc, int vpp);
> +
> +struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops = {
> + .hw_ctrl = pcmcia_hw_setup,
> + .voltage_set = pcmcia_set_voltage,
> +};
> +#endif
> +
> void __init mpc885ads_board_setup(void)
> {
> cpm8xx_t *cp;
If you define the global variable in the board code, you can not have multiple
board implementations in a multiplatform kernel.
In order to make that work, you'd need to have the definition of m8xx_pcmcia_ops
in 8xx common code, and then assign the two function pointers dynamically in the
board initialization.
> + if (of_address_to_resource(np, 0, ))
> + return -EINVAL;
> +
> + pcmcia = ioremap(r.start, r.end - r.start + 1);
> + if(pcmcia == NULL)
> + return -EINVAL;
A new of_iomap() function was recently added that allows you
to do these two steps in one.
> -/* 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));
I'm not sure why you need to use the hwirq here, it should not be visible to
device drivers normally.
Is this the same as enable_irq(pcmcia_schlvl) ?
Arnd <><
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Thursday 03 May 2007, Vitaly Bordug 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.
Very nice, looks much better now than the previous version.
diff --git a/arch/powerpc/boot/dts/mpc885ads.dts
b/arch/powerpc/boot/dts/mpc885ads.dts
index 110bf61..89d585f 100644
--- a/arch/powerpc/boot/dts/mpc885ads.dts
+++ b/arch/powerpc/boot/dts/mpc885ads.dts
@@ -112,6 +112,18 @@
compatible = CPM;
};
+ [EMAIL PROTECTED] {
+ linux,phandle = 0080;
+ #interrupt-cells = 1;
+ #size-cells = 2;
+ compatible = 8xx;
+ device_type = pcmcia;
+ reg = 80 80;
+ clock-frequency = 2faf080;
+ interrupt-parent = ff00;
+ interrupts = d 1;
+ };
+
[EMAIL PROTECTED] {
linux,phandle = ff00;
#address-cells = 1;
The compatible property should be a little more specific, imho. Since there
are differences in how things are done depending on the board, it would be
good to tell the exact method from the pcmcia node itself.
For example, you could make this
compatible = 8xx\0mpc885ads;
Your code doesn't currently use this, I like to have the option.
+#ifdef CONFIG_PCMCIA_M8XX
+static void pcmcia_hw_setup(int slot, int enable);
+static int pcmcia_set_voltage(int slot, int vcc, int vpp);
+
+struct mpc8xx_pcmcia_ops m8xx_pcmcia_ops = {
+ .hw_ctrl = pcmcia_hw_setup,
+ .voltage_set = pcmcia_set_voltage,
+};
+#endif
+
void __init mpc885ads_board_setup(void)
{
cpm8xx_t *cp;
If you define the global variable in the board code, you can not have multiple
board implementations in a multiplatform kernel.
In order to make that work, you'd need to have the definition of m8xx_pcmcia_ops
in 8xx common code, and then assign the two function pointers dynamically in the
board initialization.
+ if (of_address_to_resource(np, 0, r))
+ return -EINVAL;
+
+ pcmcia = ioremap(r.start, r.end - r.start + 1);
+ if(pcmcia == NULL)
+ return -EINVAL;
A new of_iomap() function was recently added that allows you
to do these two steps in one.
-/* 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));
I'm not sure why you need to use the hwirq here, it should not be visible to
device drivers normally.
Is this the same as enable_irq(pcmcia_schlvl) ?
Arnd
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
+ [EMAIL PROTECTED] {
+ compatible = 8xx;
The compatible property should be a little more specific, imho. Since
there
are differences in how things are done depending on the board, it
would be
good to tell the exact method from the pcmcia node itself.
Just 8xx isn't good enough -- at a very minimum it
needs to say this is a PCMCIA controller!
For example, you could make this
compatible = 8xx\0mpc885ads;
mpc885ads-pcmcia\0mpc8xx-pcmcia or something like that.
Segher
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Thursday 03 May 2007, Segher Boessenkool wrote: For example, you could make this compatible = 8xx\0mpc885ads; mpc885ads-pcmcia\0mpc8xx-pcmcia or something like that. Right. I can never remember what goes first... Arnd - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
For example, you could make this compatible = 8xx\0mpc885ads; mpc885ads-pcmcia\0mpc8xx-pcmcia or something like that. Right. I can never remember what goes first... It doesn't really matter all that much; correct drivers probe for the most specific thing first, then the next most specific thing they support, etc. It is mostly a convention. The important thing is that you can't just call yourself 8xx, that is way to generic a name. Segher - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [POWERPC] 8xx: mpc885ads pcmcia support
On Friday 04 May 2007, Vitaly Bordug 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. Signed-off-by: Vitaly Bordug [EMAIL PROTECTED] Acked-by: Arnd Bergmann [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
