lkml.org 
[lkml]   [2003]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: Complete support PC-9800 for 2.5.64-ac4 (11/11) SCSI
From
Date
On Sat, 2003-03-15 at 19:15, Osamu Tomita wrote:
> This is the patch to support NEC PC-9800 subarchitecture
> against 2.5.64-ac4. (11/11)
>
> SCSI host adapter support.
> - BIOS parameter change for PC98.
> - Add pc980155 driver for old PC98.
> - wd33c93.c update error handler for eh_*.
> - wd33c93.h register to int for PIO mode.

I suppose the first thing to point out is that it would be helpful if
you could send this to linux-scsi@vger.kernel.org. Although most SCSI
people also read linux-kernel, it is easy to lose things in the noise.

> diff -Nru linux/drivers/scsi/sd.c linux98/drivers/scsi/sd.c
> --- linux/drivers/scsi/sd.c 2003-03-05 12:29:32.000000000 +0900
> +++ linux98/drivers/scsi/sd.c 2003-03-08 11:13:28.000000000 +0900
> @@ -485,6 +485,15 @@
> else
> scsicam_bios_param(bdev, sdkp->capacity, diskinfo);
>
> +#ifdef CONFIG_X86_PC9800
> + {
> + extern int pc98_bios_param(struct scsi_device *,
> + struct block_device *,
> + sector_t, int *);
> + pc98_bios_param(sdp, bdev, sdkp->capacity, diskinfo);
> + }
> +#endif
> +
> if (put_user(diskinfo[0], &loc->heads))
> return -EFAULT;
> if (put_user(diskinfo[1], &loc->sectors))

You already have the pc98_bios_param as part of your local driver. Why
do you need this in addition?

[...]
> static inline uchar
> read_aux_stat(const wd33c93_regs regs)
> {
> - return inb(*regs.SASR);
> + return inb(regs.SASR);
> }
>
> static inline void
> write_wd33c93(const wd33c93_regs regs, uchar reg_num, uchar value)
> {
> - outb(reg_num, *regs.SASR);
> - outb(value, *regs.SCMD);
> + outb(reg_num, regs.SASR);
> + outb(value, regs.SCMD);
> }
>
[...]
> /* This is what the 3393 chip looks like to us */
> typedef struct {
> +#ifdef CONFIG_WD33C93_PIO
> + unsigned int SASR;
> + unsigned int SCMD;
> +#else
> volatile unsigned char *SASR;
> volatile unsigned char *SCMD;
> +#endif
> } wd33c93_regs;
>

This really doesn't look right. For non PIO (which is all drivers apart
from yours), they expect to dereference SASR to get the port number (as
an unsigned char). If you remove the dereference, don't they all break?

Perhaps the better thing to do is to make your driver use an unsigned
int *, so the dereference works in all cases.

James

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:34    [from the cache]
©2003-2011 Jasper Spaans