2005-01-06 01:30:46

by Kyle Moffett

[permalink] [raw]
Subject: [PATCH] [2.6] Clean up SL811 headers



Cheers,
Kyle Moffett

-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCM/CS/IT/U d- s++: a18 C++++>$ UB/L/X/*++++(+)>$ P+++(++++)>$
L++++(+++) E W++(+) N+++(++) o? K? w--- O? M++ V? PS+() PE+(-) Y+
PGP+++ t+(+++) 5 X R? tv-(--) b++++(++) DI+ D+ G e->++++$ h!*()>++$ r
!y?(-)
------END GEEK CODE BLOCK------


Attachments:
cleanup-sl811-headers.patch (2.22 kB)

2005-01-06 09:12:29

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] [2.6] Clean up SL811 headers

On Wed, Jan 05, 2005 at 08:30:25PM -0500, Kyle Moffett wrote:
> The USB SL811 host has a structure stuck in linux/usb_sl811.h As this
> structure is kernel private and only used by a single piece of code,
> this
> patch moves it to the header file from which it is used, deleting the
> old one.
>
> Signed-off-by: Kyle Moffett <[email protected]>

No. It's platform data. That means that platforms, (eg, arch/arm/mach-pxa)
are expected to supply it to the SL811 driver.

linux/usb_sl811.h is the correct location for it. The platforms which
use it are not merged just yet, but the SL811 driver is a recent merge
in preparation of this happening.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-01-06 09:34:15

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH] [2.6] Clean up SL811 headers

Kyle Moffett said:
> The USB SL811 host has a structure stuck in linux/usb_sl811.h As this
> structure is kernel private and only used by a single piece of code,

That's very wrong ... the structure in question is the interface between
the board-specific setup and the driver (as it says in the comment). A
better way to put the situation is that http://www.kernel.org doesn't currently
bundle support for a board that wires that chip into its platform_bus.


> this patch moves it to the header file from which it is used, deleting the
> old one.

Here's a thought experiment for you ... do that for EVERY driver
on the platform_bus, and see how quickly you get a big mess.
Board support packages on 2.4 tended to take that approach;
good riddance, IMO.

When you take that approach, then adding support for a new
board means ** CHANGING EVERY DRIVER USED WITH THAT BOARD **
to know how the chip is set up on that particular board.
IRQs, addressing, CPU and GPIO pin multiplexing, support
logic ... yeech!

Better to have a single arch/XYZ/mach-ZYX/board-ABC.c file that
just sets up the relevant platform devices, and the platform_data
those drivers use. The board support packages should be small,
mostly using standard drivers ... not be invasive monsters.

- Dave