Re: [PATCH v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support

[Geert Uytterhoeven]

Hi Paolo,

On Mon, 24 Aug 2026 at 17:33, Paolo Pisati <p.pisati@xxxxxxxxx> wrote:
> Driver for the on-board IDE interface on CS-Lab Warp Expansion Card
> (NOTE that idemode=native has to be set in Warp Configuration)
>
> Assisted-by: Claude_Code:claude-opus-5
> Signed-off-by: Paolo Pisati <p.pisati@xxxxxxxxx>

Thanks for the update!

> --- /dev/null
> +++ b/drivers/ata/pata_cswarp.c

> +static unsigned int pata_cswarp_data_xfer(struct ata_queued_cmd *qc,
> +                                         unsigned char *buf,
> +                                         unsigned int buflen, int rw)
> +{
> +       struct ata_device *dev = qc->dev;
> +       struct ata_port *ap = dev->link->ap;
> +       void __iomem *data_addr = ap->ioaddr.data_addr;
> +       unsigned int words = buflen >> 1;
> +       u16 *buf16 = (u16 *)buf;
> +
> +       /* Transfer multiple of 2 bytes */
> +       if (rw == READ)
> +               raw_insw(data_addr, buf16, words);
> +       else
> +               raw_outsw(data_addr, buf16, words);
> +
> +       /* Transfer trailing byte, if any. */
> +       if (unlikely(buflen & 0x01)) {
> +               if (rw == READ)
> +                       buf[buflen - 1] = raw_inw(data_addr) >> 8;
> +               else
> +                       raw_outw(buf[buflen - 1] << 8, data_addr);
> +               words++;
> +       }
> +
> +       return words << 1;

This may be one less than the actual number of bytes
Why not buflen?

> +}

> +static int pata_cswarp_probe(struct zorro_dev *z,
> +                            const struct zorro_device_id *ent)
> +{
> +       static const char board_name[] = "csWarp";
> +       struct ata_host *host;
> +       struct ata_port *ap;
> +       void __iomem *base;
> +       unsigned long board = z->resource.start;
> +
> +       dev_info(&z->dev, "%s IDE controller (board: 0x%lx)\n", board_name,
> +                board);
> +
> +       if (!devm_request_mem_region(&z->dev, board + WARP_OFFSET_ATA, 0x1800,
> +                                    DRV_NAME))
> +               return -ENXIO;
> +
> +       host = ata_host_alloc(&z->dev, 1);
> +       if (!host)
> +               return -ENXIO;
> +
> +       ap = host->ports[0];
> +       base = ioremap(board + WARP_OFFSET_ATA, 0x1800);
> +
> +       ap->ops = &pata_cswarp_ops;
> +
> +       ap->pio_mask = ATA_PIO4;
> +       ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY |
> +               ATA_FLAG_PIO_POLLING;
> +
> +       ap->ioaddr.data_addr            = base;
> +       ap->ioaddr.error_addr           = base + 1 * 4;
> +       ap->ioaddr.feature_addr         = base + 1 * 4;
> +       ap->ioaddr.nsect_addr           = base + 2 * 4;
> +       ap->ioaddr.lbal_addr            = base + 3 * 4;
> +       ap->ioaddr.lbam_addr            = base + 4 * 4;
> +       ap->ioaddr.lbah_addr            = base + 5 * 4;
> +       ap->ioaddr.device_addr          = base + 6 * 4;
> +       ap->ioaddr.status_addr          = base + 7 * 4;
> +       ap->ioaddr.command_addr         = base + 7 * 4;
> +
> +       ap->ioaddr.altstatus_addr       = base + (0x1000 | (6UL << 2));
> +       ap->ioaddr.ctl_addr             = base + (0x1000 | (6UL << 2));
> +
> +       ata_port_desc(ap, "  cmd 0x%lx ctl 0x%lx", (unsigned long)base,
> +                     (unsigned long)ap->ioaddr.ctl_addr);

Both printed addresses are virtual addresses hence not really useful.
If you want to print something, please print board or z->resource
instead.

> +static const struct zorro_device_id pata_cswarp_zorro_tbl[] = {
> +       { ZORRO_PROD_CSLAB_WARP_1260, 0},
> +       { 0 }

Please use named initializers, and drop unneeded zeroes, like Uwe
just did in all existing Zorro drivers:

    { .id = ZORRO_PROD_CSLAB_WARP_1260 },
    { }

> +};

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