Re: [PATCH] 2.6.10 Altix : ioc4 serial driver support

[Patrick Gefre <pfg@sgi.com>]

Christoph Hellwig wrote:

> So both claim the same PCI ID?  In this case you need to creat a small
> shim driver that exports a pseudo-bus to the serial and ide driver using
> the driver model.  You must never return an error from ->probe if you
> actually use that particular device.
> 

Has this been done before ? Any example I can use ??


> 
> +/* defining this will force the driver to run in polled mode */
> +//#define POLLING_FOR_CHARACTERS
> 
> Again, what's the need for these conditionals?
> 

Most of these are compile options to use/not use particular things/"features". Some were
used during debugging. It's a small thing, I'll delete.


> 
> +/* a table to keep the card names as passed to request_region */
> +static struct {
> +	char c_name[20];
> +} Table_o_cards[IOC4_NUM_CARDS];
> 
> Completely superflous.  Just pass "ioc4_serial" as argument to request_region.
> 

WHAT ?!?!?!?  Then I get nice output that actually identifies each card when I have >1. 8^) Not a 
big thing, I'll delete.


> +
> +	switch (type) {
> +	case IOC4_SIO_INTR_TYPE:
> +		switch (which) {
> +		case IOC4_W_IES:
> +			writel(val, (void *)&mem->sio_ies_ro);
> 
> The second argumnet to writeX (and readX) is actually void __iomem *,
> but to see the difference you need to run sparse (from sparse.bkbits.net)
> over the driver.  Please store all I/O addresses in void __iomem * pointers
> in your structures and avoid the cast here and in all the other places.
> 

So then I'd have to declare the end elements as:
void __iomem foo;

They are 32 bit values, so it's OK to assume that void __iomem is 32bits ?

FWIW I did run sparse and it didn't complain about the readX/writeX.....


> no need to cast the return value from kmalloc (dito for the other places)
> 

Why is that ? Seems if kmalloc returns a void * and the left side is not, a casting is appropriate ?

-- Pat