Re: [PATCH RFC v3 2/2] m68k/q40: add data_swab option for pata_falcon to byte-swap disk data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 17, 2023 at 12:15 PM Finn Thain <fthain@xxxxxxxxxxxxxx> wrote:
On Thu, 17 Aug 2023, Michael Schmitz wrote:
Some users of pata_falcon on Q40 have IDE disks in classic
little endian byte order, whereas legacy disks use native
big-endian byte order as on the Atari Falcon.

Add module parameter 'data_swab' to allow connecting drives
with non-native data byte order.  Drives selected by the
data_swap bit mask will have their user data byte-swapped to
host byte order, i.e. 'pata_falcon.data_swab=2' will byte-swap
all user data on drive B, leaving data on drive A in native
byte order. On Q40, drives on a second IDE interface may be
added to the bit mask as bits 3 and 4.

Many would say that's off by one, as it's popular to number the LSB as bit
zero.


Default setting is no byte swapping, i.e. compatibility with
the native Falcon or Q40 operating system disk format.

Cc: William R Sowerbutts <will@xxxxxxxxxxxxxx>
Cc: Finn Thain <fthain@xxxxxxxxxxxxxx>
Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Signed-off-by: Michael Schmitz <schmitzmic@xxxxxxxxx>

--- a/drivers/ata/pata_falcon.c
+++ b/drivers/ata/pata_falcon.c

@@ -165,10 +178,20 @@ static int __init pata_falcon_init_one(struct platform_device *pdev)
      ap->pio_mask = ATA_PIO4;
      ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY;

+     priv = devm_kzalloc(&pdev->dev,
+             sizeof(struct pata_falcon_priv), GFP_KERNEL);
+     if (!priv)
+             return -ENOMEM;
+
+     ap->private_data = priv;
+
      /* N.B. this assumes data_addr will be used for word-sized I/O only */
      ap->ioaddr.data_addr = (void __iomem *)base_mem_res->start;

      if (base_res) {         /* only Q40 has IO resources */
+             if (pdev->id)
+                     pata_falcon_swap_mask >>= 2;

Although this driver uses module_platform_driver_probe(), and thus
does not support unbind/rebind, shifting a global variable is still
fragile, and depends on probe order (what if the second interface is
probed first?).

+             priv->swap_mask = pata_falcon_swap_mask;

priv->swap_mask = pata_falcon_swap_mask >> (2 * pdev->id);

--- a/drivers/ata/pata_falcon.c
+++ b/drivers/ata/pata_falcon.c
@@ -33,6 +33,11 @@
 #define DRV_NAME "pata_falcon"
 #define DRV_VERSION "0.1.0"

+static int pata_falcon_data_swab;
+
+module_param_named(data_swab, pata_falcon_data_swab, int, 0444);
+MODULE_PARM_DESC(data_swab, "Data byte swap enable/disable bitmap (0x1==drive1, 0x2==drive2, 0x4==drive3, 0x8==drive4, default: 0)");
+
 static const struct scsi_host_template pata_falcon_sht = {
        ATA_PIO_SHT(DRV_NAME),
 };
@@ -50,7 +55,7 @@ static unsigned int pata_falcon_data_xfer(struct ata_queued_cmd *qc,

        if (dev->class == ATA_DEV_ATA && cmd &&
            !blk_rq_is_passthrough(scsi_cmd_to_rq(cmd)))
-               swap = 0;
+               swap = (int)ap->private_data & BIT(dev->devno);

"(uintptr_t)", to make it 64-bit clean.

Yeah, we don't support COMPILE_TEST=y yet for PATA_FALCON.
BTW, looks like we don't need any of the following anymore:

    #include <asm/setup.h>
    #include <asm/atarihw.h>
    #include <asm/atariints.h>
    #include <asm/atari_stdma.h>

That leaves us with <asm/ide.h>, (which does not exist on most
architectures, and seems to be mostly obsolete), for the (indirectly
included) definitions of raw_{in,out}sw_{,swapw}()....


        /* Transfer multiple of 2 bytes */
        if (rw == READ) {
@@ -199,6 +204,8 @@ static int __init pata_falcon_init_one(struct platform_device *pdev)
        ap->ioaddr.altstatus_addr       = ctl_base + io_offset;
        ap->ioaddr.ctl_addr             = ctl_base + io_offset;

+       ap->private_data = (void *)((pata_falcon_data_swab >> (2 * pdev->id)) & 3);

"(void *)(uintptr_t)", to make it 64-bit clean.

+
        irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
        if (irq_res && irq_res->start > 0) {
                irq = irq_res->start;

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



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux