Re: [RFC PATCH 78/71] ncr5380: Add support for HP 53C400A-based cards (C2502)

Finn Thain
2015-12-06T03:39:26Z

On Sat, 5 Dec 2015, Ondrej Zary wrote:

> HP C2502 cards (based on 53C400A chips) use different magic numbers for
> software-based I/O address configuration than other cards. The
> configuration is also extended to allow setting the IRQ.
>
> Move the configuration to a new function magic_configure() and move
> magic the magic numbers into an array. Add new magic numbers for these
> HP cards and hp_53c400a module parameter to use them.
>
> Tested with HP C2502 and DTCT-436P.
>
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> ---
> drivers/scsi/g_NCR5380.c | 69 +++++++++++++++++++++++++++++++++++-----------
> drivers/scsi/g_NCR5380.h | 1 +
> 2 files changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
> index 38579b0..52110e0 100644
> --- a/drivers/scsi/g_NCR5380.c
> +++ b/drivers/scsi/g_NCR5380.c
> @@ -80,6 +80,7 @@ static int ncr_5380;
> static int ncr_53c400;
> static int ncr_53c400a;
> static int dtc_3181e;
> +static int hp_53c400a;
>
> static struct override {
> NCR5380_map_type NCR5380_map_name;
> @@ -225,6 +226,26 @@ static int __init do_DTC3181E_setup(char *str)
>
> #endif
>
> +#ifndef SCSI_G_NCR5380_MEM
> +static void magic_configure(int idx, u8 irq, u8 magic[])
> +{
> + u8 cfg = 0;
> +
> + outb(magic[0], 0x779);
> + outb(magic[1], 0x379);
> + outb(magic[2], 0x379);
> + outb(magic[3], 0x379);
> + outb(magic[4], 0x379);
> +
> + /* allowed IRQs for HP 53C400A */
> + if (irq != 2 && irq != 3 && irq != 4 && irq != 5 && irq != 7)
> + irq = 0;
> + if (idx >= 0 && idx <= 7)
> + cfg = 0x80 | idx | (irq << 4);
> + outb(cfg, 0x379);
> +}
> +#endif
> +
> /**
> * generic_NCR5380_detect - look for NCR5380 controllers
> * @tpnt: the scsi template
> @@ -241,8 +262,9 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
> static int current_override;
> int count;
> unsigned int *ports;
> + u8 *magic;
> #ifndef SCSI_G_NCR5380_MEM
> - int i;
> + int i, port_idx;
> unsigned long region_size = 16;
> #endif
> static unsigned int __initdata ncr_53c400a_ports[] = {
> @@ -251,6 +273,12 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
> static unsigned int __initdata dtc_3181e_ports[] = {
> 0x220, 0x240, 0x280, 0x2a0, 0x2c0, 0x300, 0x320, 0x340, 0
> };
> + static u8 ncr_53c400a_magic[] __initdata = {
> + 0x59, 0xb9, 0xc5, 0xae, 0xa6
> + };
> + static u8 hp_53c400a_magic[] __initdata = {
> + 0x0f, 0x22, 0xf0, 0x20, 0x80
> + };

Magic numbers that are clearly labeled as such tells me that no-one can
say what they actually mean... is that right?

> int flags;
> struct Scsi_Host *instance;
> struct NCR5380_hostdata *hostdata;
> @@ -273,6 +301,8 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
> overrides[0].board = BOARD_NCR53C400A;
> else if (dtc_3181e)
> overrides[0].board = BOARD_DTC3181E;
> + else if (hp_53c400a)
> + overrides[0].board = BOARD_HP53C400A;
> #ifndef SCSI_G_NCR5380_MEM
> if (!current_override && isapnp_present()) {
> struct pnp_dev *dev = NULL;
> @@ -323,13 +353,21 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
> flags = FLAG_NO_PSEUDO_DMA;
> #endif
> break;
> + case BOARD_HP53C400A:
> + flags = FLAG_NO_DMA_FIXUP;
> + ports = ncr_53c400a_ports;
> + magic = hp_53c400a_magic;

Nice! A new board.

> + overrides[current_override].board = BOARD_NCR53C400A;

Do you rewrite overrides[current_override].board in order to avoid another
term in a long conditional expression? Don't you find that confusing? Why
not consistently use switch statements when we have to test
overrides[current_override].board?

> + break;
> case BOARD_NCR53C400A:
> flags = FLAG_NO_DMA_FIXUP;
> ports = ncr_53c400a_ports;
> + magic = ncr_53c400a_magic;
> break;
> case BOARD_DTC3181E:
> flags = FLAG_NO_DMA_FIXUP;
> ports = dtc_3181e_ports;
> + magic = ncr_53c400a_magic;
> break;
> }
>
> @@ -338,12 +376,7 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
> /* wakeup sequence for the NCR53C400A and DTC3181E */
>
> /* Disable the adapter and look for a free io port */
> - outb(0x59, 0x779);
> - outb(0xb9, 0x379);
> - outb(0xc5, 0x379);
> - outb(0xae, 0x379);
> - outb(0xa6, 0x379);
> - outb(0x00, 0x379);
> + magic_configure(-1, 0, magic);
>
> if (overrides[current_override].NCR5380_map_name != PORT_AUTO)
> for (i = 0; ports[i]; i++) {
> @@ -362,17 +395,14 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
> }
> if (ports[i]) {
> /* At this point we have our region reserved */
> - outb(0x59, 0x779);
> - outb(0xb9, 0x379);
> - outb(0xc5, 0x379);
> - outb(0xae, 0x379);
> - outb(0xa6, 0x379);
> - outb(0x80 | i, 0x379); /* set io port to be used */
> + magic_configure(i, 0, magic); /* no IRQ yet */
> outb(0xc0, ports[i] + 9);
> - if (inb(ports[i] + 9) != 0x80)
> + if (inb(ports[i] + 9) != 0x80) {
> continue;
> - else
> + } else {
> overrides[current_override].NCR5380_map_name = ports[i];
> + port_idx = i;
> + }
> } else
> continue;
> }
> @@ -464,12 +494,18 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
> if (instance->irq == 255)
> instance->irq = NO_IRQ;
>
> - if (instance->irq != NO_IRQ)
> + if (instance->irq != NO_IRQ) {
> +#ifndef SCSI_G_NCR5380_MEM
> + /* set IRQ for HP 53C400A */
> + if (magic == hp_53c400a_magic)
> + magic_configure(port_idx, instance->irq, magic);
> +#endif

This would be better done earlier. We need to determine and assign the
instance->irq value before NCR5380_init() in order to fix the irq number
printed in the log messages when the driver starts. But that should
probably be a separate patch...

> if (request_irq(instance->irq, generic_NCR5380_intr,
> 0, "NCR5380", instance)) {
> printk(KERN_WARNING "scsi%d : IRQ%d not free, interrupts disabled\n", instance->host_no, instance->irq);
> instance->irq = NO_IRQ;
> }
> + }
>
> if (instance->irq == NO_IRQ) {
> printk(KERN_INFO "scsi%d : interrupts not enabled. for better interactive performance,\n", instance->host_no);
> @@ -763,6 +799,7 @@ module_param(ncr_5380, int, 0);
> module_param(ncr_53c400, int, 0);
> module_param(ncr_53c400a, int, 0);
> module_param(dtc_3181e, int, 0);
> +module_param(hp_53c400a, int, 0);
> MODULE_LICENSE("GPL");
>
> #if !defined(SCSI_G_NCR5380_MEM) && defined(MODULE)
> diff --git a/drivers/scsi/g_NCR5380.h b/drivers/scsi/g_NCR5380.h
> index b3936aa..369ab9e 100644
> --- a/drivers/scsi/g_NCR5380.h
> +++ b/drivers/scsi/g_NCR5380.h
> @@ -89,6 +89,7 @@
> #define BOARD_NCR53C400 1
> #define BOARD_NCR53C400A 2
> #define BOARD_DTC3181E 3
> +#define BOARD_HP53C400A 4
>
> #endif /* GENERIC_NCR5380_H */
>
>

--