[PATCH] init. mca_bus_type even if !MCA_bus

[PATCH] init. mca_bus_type even if !MCA_bus

[Randy.Dunlap]

// Linux 2.6.6
// need to call mca_system_init() to register MCA bus struct,
// otherwise find_mca_adapter() oopses with a NULL ptr.
// Fixes this oops reported last week:
//	http://marc.theaimsgroup.com/?l=linux-kernel&m=108455738606747&w=2
// Thanks to James Bottomley for pointing this out.

diffstat:=
 arch/i386/kernel/mca.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)


diff -Naurp ./arch/i386/kernel/mca.c~check_mca_bus ./arch/i386/kernel/mca.c
--- ./arch/i386/kernel/mca.c~check_mca_bus	2004-05-09 19:33:05.000000000 -0700
+++ ./arch/i386/kernel/mca.c	2004-05-17 14:50:32.000000000 -0700
@@ -258,16 +258,16 @@ static int __init mca_init(void)
 
 	/* Make sure the MCA bus is present */
 
-	if(!MCA_bus)
-		return -ENODEV;
-
-	printk(KERN_INFO "Micro Channel bus detected.\n");
-
-	if(mca_system_init()) {
+	if (mca_system_init()) {
 		printk(KERN_ERR "MCA bus system initialisation failed\n");
 		return -ENODEV;
 	}
 
+	if (!MCA_bus)
+		return -ENODEV;
+
+	printk(KERN_INFO "Micro Channel bus detected.\n");
+
 	/* All MCA systems have at least a primary bus */
 	bus = mca_attach_bus(MCA_PRIMARY_BUS);
 	if (!bus)


--
~Randy

Re: [PATCH] init. mca_bus_type even if !MCA_bus

[Randy.Dunlap]

On Mon, 17 May 2004 15:14:12 -0700 Andrew Morton wrote:

| "Randy.Dunlap" <rddunlap@osdl.org> wrote:
| >
| > -	if(mca_system_init()) {
| > +	if (mca_system_init()) {
| >  		printk(KERN_ERR "MCA bus system initialisation failed\n");
| >  		return -ENODEV;
| >  	}
| >  
| > +	if (!MCA_bus)
| > +		return -ENODEV;
| 
| Why is it appropriate to register the MCA bus type when there is no
| MCA bus present?

Mostly because it was selected with CONFIG_MCA=y.

Another option (I think, need to test) is to check !MCA_bus
in drivers/mca/mca-legacy.c::find_mca_adapter(), so that
mca_bus_type isn't used when it shouldn't be.

Do you prefer that approach?

--
~Randy

Re: [PATCH] init. mca_bus_type even if !MCA_bus

[Andrew Morton]

"Randy.Dunlap" <rddunlap@osdl.org> wrote:
>
> -	if(mca_system_init()) {
> +	if (mca_system_init()) {
>  		printk(KERN_ERR "MCA bus system initialisation failed\n");
>  		return -ENODEV;
>  	}
>  
> +	if (!MCA_bus)
> +		return -ENODEV;

Why is it appropriate to register the MCA bus type when there is no
MCA bus present?

Re: [PATCH] init. mca_bus_type even if !MCA_bus

[James Bottomley]

On Mon, 2004-05-17 at 17:14, Andrew Morton wrote:
> Why is it appropriate to register the MCA bus type when there is no
> MCA bus present?

The legacy bus functions all have a bus_for_each_dev in them.  This
can't execute correctly unless the bus is registered.  So either a check
for MCA_bus has to be added to each of them, or we register the bus but
attach no devices, so the loop exits without doing anything.

James

Re: [PATCH] init. mca_bus_type even if !MCA_bus

[Andrew Morton]

"Randy.Dunlap" <rddunlap@osdl.org> wrote:
>
> On Mon, 17 May 2004 15:14:12 -0700 Andrew Morton wrote:
> 
> | "Randy.Dunlap" <rddunlap@osdl.org> wrote:
> | >
> | > -	if(mca_system_init()) {
> | > +	if (mca_system_init()) {
> | >  		printk(KERN_ERR "MCA bus system initialisation failed\n");
> | >  		return -ENODEV;
> | >  	}
> | >  
> | > +	if (!MCA_bus)
> | > +		return -ENODEV;
> | 
> | Why is it appropriate to register the MCA bus type when there is no
> | MCA bus present?
> 
> Mostly because it was selected with CONFIG_MCA=y.
> 
> Another option (I think, need to test) is to check !MCA_bus
> in drivers/mca/mca-legacy.c::find_mca_adapter(), so that
> mca_bus_type isn't used when it shouldn't be.
> 
> Do you prefer that approach?

well my question really was a question, rather than a reimplementation
suggestion.  If it _is_ appropriate that mca_bus_type be registered on a
platform which is discovered to have no MCA hardware then fine.

Greg? James?  Any insights?

Re: [PATCH] init. mca_bus_type even if !MCA_bus

[Andrew Morton]

James Bottomley <James.Bottomley@SteelEye.com> wrote:
>
> On Mon, 2004-05-17 at 17:14, Andrew Morton wrote:
> > Why is it appropriate to register the MCA bus type when there is no
> > MCA bus present?
> 
> The legacy bus functions all have a bus_for_each_dev in them.  This
> can't execute correctly unless the bus is registered.  So either a check
> for MCA_bus has to be added to each of them, or we register the bus but
> attach no devices, so the loop exits without doing anything.
> 

Fair enough.  Let's run with Randy's patch and see what else explodes ;)

Re: [PATCH] init. mca_bus_type even if !MCA_bus

[Greg KH]

On Mon, May 17, 2004 at 03:52:22PM -0700, Andrew Morton wrote:
> "Randy.Dunlap" <rddunlap@osdl.org> wrote:
> >
> > On Mon, 17 May 2004 15:14:12 -0700 Andrew Morton wrote:
> > 
> > | "Randy.Dunlap" <rddunlap@osdl.org> wrote:
> > | >
> > | > -	if(mca_system_init()) {
> > | > +	if (mca_system_init()) {
> > | >  		printk(KERN_ERR "MCA bus system initialisation failed\n");
> > | >  		return -ENODEV;
> > | >  	}
> > | >  
> > | > +	if (!MCA_bus)
> > | > +		return -ENODEV;
> > | 
> > | Why is it appropriate to register the MCA bus type when there is no
> > | MCA bus present?
> > 
> > Mostly because it was selected with CONFIG_MCA=y.
> > 
> > Another option (I think, need to test) is to check !MCA_bus
> > in drivers/mca/mca-legacy.c::find_mca_adapter(), so that
> > mca_bus_type isn't used when it shouldn't be.
> > 
> > Do you prefer that approach?
> 
> well my question really was a question, rather than a reimplementation
> suggestion.  If it _is_ appropriate that mca_bus_type be registered on a
> platform which is discovered to have no MCA hardware then fine.
> 
> Greg? James?  Any insights?

Ick, James is the one to answer here, as I remember this being pretty
messy at times due to the way the MCA code currently is...

thanks,

greg k-h

Re: [PATCH] init. mca_bus_type even if !MCA_bus

[James Bottomley]

On Mon, 2004-05-17 at 17:52, Andrew Morton wrote:
> well my question really was a question, rather than a reimplementation
> suggestion.  If it _is_ appropriate that mca_bus_type be registered on a
> platform which is discovered to have no MCA hardware then fine.
> 
> Greg? James?  Any insights?

If the use compiles with CONFIG_MCA it makes sense to register the bus. 
It's also certainly easier than having to check for MCA_bus being set in
all of the routines.

James