Hi Michael,
On Sun, Mar 4, 2018 at 12:54 AM, Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
> From: Michael Schmitz <schmitz@xxxxxxxxxx>
>
> New combined SCSI driver for all ESP based Zorro SCSI boards for
> m68k Amiga.
Thanks for your patch!
> --- /dev/null
> +++ b/drivers/scsi/zorro_esp.c
> @@ -0,0 +1,785 @@
> +static struct zorro_driver_data {
> + const char *name;
> + unsigned long offset;
> + unsigned long dma_offset;
> + int absolute;
> + int zorro3; /* offset is absolute address */
zorro3 is unused.
> +} zorro_esp_driver_data[] = {
> + { .name = "CyberStormI", .offset = 0xf400, .dma_offset = 0xf800,
> + .absolute = 0, .zorro3 = 0 },
> + { .name = "CyberStormII", .offset = 0x1ff03, .dma_offset = 0x1ff43,
> + .absolute = 0, .zorro3 = 0 },
> + { .name = "Blizzard 2060", .offset = 0x1ff00, .dma_offset = 0x1ffe0,
> + .absolute = 0, .zorro3 = 0 },
> + { .name = "Blizzard 1230", .offset = 0x8000, .dma_offset = 0x10000,
> + .absolute = 0, .zorro3 = 0 },
> + { .name = "Blizzard 1230II", .offset = 0x10000, .dma_offset = 0x10021,
> + .absolute = 0, .zorro3 = 0 },
> + { .name = "Fastlane", .offset = 0x1000001, .dma_offset = 0x1000041,
> + .absolute = 0, .zorro3 = 1 },
> + { 0 }
I think it's better to not use an array here, but individual structs:
static struct zorro_driver_data cyberstorm_data = ...
static struct zorro_driver_data cyberstorm2_data = ...
...
That makes it easier to review the references from zorro_esp_zorro_tbl[]
below.
> +};
> +
> +static struct zorro_device_id zorro_esp_zorro_tbl[] = {
> + {
> + .id = ZORRO_PROD_PHASE5_BLIZZARD_1220_CYBERSTORM,
> + .driver_data = (unsigned long)&zorro_esp_driver_data[0],
> + },
[...]
> +static unsigned char ctrl_data; /* Keep backup of the stuff written
> + * to ctrl_reg. Always write a copy
> + * to this register when writing to
> + * the hardware register!
> + */
This should be part of the device's zorro_esp_priv.
> +/*
> + * private data used for PIO
> + */
> +struct zorro_esp_priv {
> + struct esp *esp;
> + int error;
> +} zorro_esp_private_data[8];
Dynamic allocation, please.
> +static void zorro_esp_send_pio_cmd(struct esp *esp, u32 addr, u32 esp_count,
> + u32 dma_count, int write, u8 cmd)
> +{
> + struct zorro_esp_priv *zep = ZORRO_ESP_GET_PRIV(esp);
> + u8 __iomem *fifo = esp->regs + ESP_FDATA * 16;
> + u8 phase = esp->sreg & ESP_STAT_PMASK;
> +
> + cmd &= ~ESP_CMD_DMA;
> + zep->error = 0;
> +
> + /* We are passed DMA addresses i.e. physical addresses, but must use
> + * kernel virtual addresses here, so remap to virtual. This is easy
> + * enough for the case of residual bytes of an extended message in
> + * transfer - we know the address must be esp->command_block_dma.
> + * In other cases, hope that phys_to_virt() works ...
> + */
> + if (addr == esp->command_block_dma)
> + addr = (u32) esp->command_block;
> + else
> + addr = (u32) phys_to_virt(addr);
To avoid having a need for phys_to_virt(), you should remember the addresses
passed to/returned from dma_map_*().
if you assign the address to a different variable with the proper
type, you don't
need the cast below....
+ if (write) {
+ u8 *dst = (u8 *)addr;
... here.
> + } else {
The read case doesn't use addr?
> +static void zorro_esp_send_blz1230_dma_cmd(struct esp *esp, u32 addr,
> + u32 esp_count, u32 dma_count, int write, u8 cmd)
> +{
> + struct blz1230_dma_registers *dregs =
> + (struct blz1230_dma_registers *) (esp->dma_regs);
> + u8 phase = esp->sreg & ESP_STAT_PMASK;
> +
> + if (phase == ESP_MIP) {
> + zorro_esp_send_pio_cmd(esp, addr, esp_count,
> + dma_count, write, cmd);
> + return;
> + }
> +
> + BUG_ON(!(cmd & ESP_CMD_DMA));
> +
> + if (write)
> + cache_clear(addr, esp_count);
> + else
> + cache_push(addr, esp_count);
dma_sync_*()
> +static int zorro_esp_init_one(struct zorro_dev *z,
> + const struct zorro_device_id *ent)
> +{
> + struct scsi_host_template *tpnt = &scsi_esp_template;
> + struct Scsi_Host *host;
> + struct esp *esp;
> + struct zorro_driver_data *zdd;
> + struct zorro_esp_priv *zep;
> + unsigned long board, ioaddr, dmaaddr;
> + int err = -ENOMEM;
Initialization not needed.
> +
> + board = zorro_resource_start(z);
> + zdd = (struct zorro_driver_data *)ent->driver_data;
> +
> + pr_info(PFX "%s found at address 0x%lx.\n", zdd->name, board);
> +
> + if (zdd->absolute) {
> + ioaddr = zdd->offset;
> + dmaaddr = zdd->dma_offset;
> + } else {
> + ioaddr = board + zdd->offset;
> + dmaaddr = board + zdd->dma_offset;
> + }
> +
> + if (!zorro_request_device(z, zdd->name)) {
> + pr_err(PFX "cannot reserve region 0x%lx, abort\n",
> + board);
> + return -EBUSY;
> + }
> +
> + /* Fill in the required pieces of hostdata */
> +
> + host = scsi_host_alloc(tpnt, sizeof(struct esp));
> +
> + if (!host) {
> + pr_err(PFX "No host detected; board configuration problem?\n");
> + goto out_free;
> + }
> +
> + host->base = ioaddr;
> + host->this_id = 7;
> +
> + esp = shost_priv(host);
> + esp->host = host;
> + esp->dev = &z->dev;
> +
> + esp->scsi_id = host->this_id;
> + esp->scsi_id_mask = (1 << esp->scsi_id);
> +
> + /* Switch to the correct the DMA routine and clock frequency. */
> + switch (ent->id) {
> + case ZORRO_PROD_PHASE5_BLIZZARD_2060: {
> + zorro_esp_ops.send_dma_cmd = zorro_esp_send_blz2060_dma_cmd;
> + esp->cfreq = 40000000;
> + break;
> + }
> + case ZORRO_PROD_PHASE5_BLIZZARD_1230_IV_1260: {
> + zorro_esp_ops.send_dma_cmd = zorro_esp_send_blz1230_dma_cmd;
> + esp->cfreq = 40000000;
> + break;
> + }
> + case ZORRO_PROD_PHASE5_BLIZZARD_1220_CYBERSTORM: {
> + zorro_esp_ops.send_dma_cmd = zorro_esp_send_cyber_dma_cmd;
> + zorro_esp_ops.irq_pending = cyber_esp_irq_pending;
> + esp->cfreq = 40000000;
> + break;
Store send_dma_cmd and cfreq in zorro_driver_data?
> + }
> + default: {
> + /* Oh noes */
> + pr_err(PFX "Unsupported board!\n");
> + goto fail_free_host;
> + }
> + }
> +
> + esp->ops = &zorro_esp_ops;
> +
> + if (ioaddr > 0xffffff)
> + esp->regs = ioremap_nocache(ioaddr, 0x20);
> + else
> + esp->regs = (void __iomem *)ZTWO_VADDR(ioaddr);
> +
> + if (!esp->regs)
> + goto fail_free_host;
> +
> + /* Let's check whether a Blizzard 12x0 really has SCSI */
> + if ((ent->id == ZORRO_PROD_PHASE5_BLIZZARD_1230_IV_1260) ||
> + (ent->id == ZORRO_PROD_PHASE5_BLIZZARD_1230_II_FASTLANE_Z3_CYBERSCSI_CYBERSTORM060)) {
Add a flag to zorro_driver_data (optional_scsi?) instead of checking
for IDs here.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
