Hi Paolo,
On Tue, 25 Mar 2025 at 16:56, 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)
>
> Signed-off-by: Paolo Pisati <p.pisati@xxxxxxxxx>
Thanks for your patch!
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -1025,6 +1025,17 @@ config PATA_GAYLE
>
> If unsure, say N.
>
> +config PATA_CSWARP
> + tristate "Amiga CS Warp PATA support"
> + depends on M68K && AMIGA && ZORRO
> + help
> + This option enables support for the on-board IDE interface on
> + CS-Lab Warp Expansion Card (NOTE that idemode=native has to be
> + set in Warp Configuration)
> +
> + If unsure, say N.
> +
> +
Please drop one blank line.
> config PATA_BUDDHA
> tristate "Buddha/Catweasel/X-Surf PATA support"
> depends on ZORRO
> index 000000000000..e0a0db6f7cfd
> --- /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;
> + volatile 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)) {
> + unsigned char pad[2] = { };
> +
> + /* Point buf to the tail of buffer */
> + buf16 += words;
> +
> + if (rw == READ) {
> + *pad = raw_inw(data_addr);
> + *buf16 = pad[0];
This looks wrong to me: as *pad is the same as pad[0], you could just
eliminate the pad[] intermediate...
Moreover, this writes one byte too much into the buffer.
> + } else {
> + pad[0] = *buf16;
> + raw_outw(*pad, data_addr);
> + }
Comparing to what v3 did, I think you want:
if (rw == READ)
buf[buflen-1] = raw_inw(data_addr) >> 8;
else
raw_outw(buf[buflen-1] << 8, data_addr);
As all Amigas are big-endian, I think there is no need to implement
this using some endian conversion function.
> + }
> +
> + return buflen;
> +}
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
