Re: [PATCH] Stop ARM boards crashing when CUPS is loaded - 2.6.35-rc5

[Milton Miller <miltonm@bga.com>]

On Fri, 16 Jul 2010 about 10:42:23 -0600 Lee Jones wrote:
>On 16/07/10 17:23, Milton Miller wrote:
> > On Fri Jul 16 2010 about 05:33:06 EST,  Lee Jones wrote:
> > > > The best solution is probably for the parport code to go through a
> > > > modernisation cycle like the serial code did, essentially using
> > > > platform devices to pass the base addresses. This would make the
> > > > driver more portable, and eliminates this problem entirely (because
> > > > platforms which don't have parports won't register the platform device(s)
> > > > necessary for parport to even probe illegal addresses.)
> > >
> > > This sounds brilliant - when are you going to start? </kidding>
> > 
> > It has a long time ago ...
> > 
> > drivers/parport/parport_pc.c calls parport_pc_find_nonpci_ports,
> > which is in asm/parport.h
>
>I'm not entirely sure what you're trying to say here?
>
>How does that help with the platformisation of the driver?

I was reading quickly, but my point was the code already defers to the
architecture the method of finding the ports to scan, which is the
important part.

I just looked at the 8250 code and it abuses the platform device model.
Instead of a platform device for each port, it has multiple port
descriptions set via platform_data in one or a a few device instances.
It then only uses this information to fill in its internal array of all
ports, which drives the actual registration with the serial core.  It also
registers a platform device to to get suspend and resume hooks, but now has
to scan its list of ports for all the instances driven from this "device".
Yech.  Please don't use this as an example of a modern driver.


>
> > > In all seriousness, do you think anyone is likely to undertake this
> > > work anytime soon? I am seeing this problem in a distribution which
> > > is due for release in October. I have no problem implementing a config
> > > change in the meantime, but as you say, a more _correct_ and portable
> > > solution should be sought.
> > 
> > Why not replace the arm asm/parport.h with asm-generic/parport.h which
> > already has a check for CONFIG_ISA, which appears to only be selected
> > on a few ARM platforms?
>
>static int __devinit parport_pc_find_isa_ports(int autoirq, int autodma);
>static int __devinit parport_pc_find_nonpci_ports(int autoirq, int autodma)
>{
>#ifdef CONFIG_ISA
>	return parport_pc_find_isa_ports(autoirq, autodma);
>#else
>	return 0;
>#endif
>}
>
>That's perfect! 
>
>This would work a treat.
>
>Surely this #ifdef should be in all the parport.h files which call
>parport_pc_find_isa_ports?

No, as CONFIG_ISA is supposed to be ISA slots, and other architectures
may frequently have 8250 ports at the pc legacy port numbers without
ISA slots.

milton