| Message ID | 20230818071405.28581-3-schmitzmic@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Q40 IDE fixes | expand |
On Fri, 18 Aug 2023, Michael Schmitz wrote: > Some users of pata_falcon on Q40 have IDE disks in default > IDE little endian byte order, whereas legacy disks use > host-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 2 and 3. > > Default setting is no byte swapping, i.e. compatibility with > the native Falcon or Q40 operating system disk format. > > Cc: William R Sowerbutts <will@sowerbutts.com> > Cc: Finn Thain <fthain@linux-m68k.org> > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > > --- > > Changes since v1: > > Damien Le Moal: > - change patch title > - drop swap_data flag > > Finn Thain: > - drop allocation of ap->private struct, use field as bitmask > > Changes since RFC v4: > > Geert Uytterhoeven: > - don't shift static module parameter for drive 3/4 bitmask > - simplify bit mask calculation to always use pdev->id > > Finn Thain: > - correct bit numbers for drive 3/4 > > Changes since RFC v3: > > - split off this byte swap handling into separate patch > > - add hint regarding third and fourth drive on Q40 > > Finn Thain: > - rename module parameter to 'data_swab' to better reflect its use > > William Sowerbutts: > - correct IDE drive number used in data swap conditional > --- > drivers/ata/pata_falcon.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c > index 346259e3bbc8..27443cb757de 100644 > --- 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_swap_mask; > + > +module_param_named(data_swab, pata_falcon_swap_mask, 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), > }; > @@ -46,11 +51,12 @@ static unsigned int pata_falcon_data_xfer(struct ata_queued_cmd *qc, > void __iomem *data_addr = ap->ioaddr.data_addr; > unsigned int words = buflen >> 1; > struct scsi_cmnd *cmd = qc->scsicmd; > + int dev_id = dev->devno; Is that variable really needed? > bool swap = 1; > > if (dev->class == ATA_DEV_ATA && cmd && > !blk_rq_is_passthrough(scsi_cmd_to_rq(cmd))) > - swap = 0; > + swap = (uintptr_t)ap->private_data & BIT(dev_id); > > /* Transfer multiple of 2 bytes */ > if (rw == READ) { > @@ -199,6 +205,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 *)(uintptr_t)(pata_falcon_swap_mask >> (2 * pdev->id)); > + My compiler doesn't need that extra type cast in there... > irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > if (irq_res && irq_res->start > 0) { > irq = irq_res->start; >
Hi Finn, On Fri, Aug 18, 2023 at 9:43 AM Finn Thain <fthain@linux-m68k.org> wrote: > On Fri, 18 Aug 2023, Michael Schmitz wrote: > > Some users of pata_falcon on Q40 have IDE disks in default > > IDE little endian byte order, whereas legacy disks use > > host-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 2 and 3. > > > > Default setting is no byte swapping, i.e. compatibility with > > the native Falcon or Q40 operating system disk format. > > > > Cc: William R Sowerbutts <will@sowerbutts.com> > > Cc: Finn Thain <fthain@linux-m68k.org> > > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > > --- a/drivers/ata/pata_falcon.c > > +++ b/drivers/ata/pata_falcon.c > > @@ -199,6 +205,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 *)(uintptr_t)(pata_falcon_swap_mask >> (2 * pdev->id)); > > + > > My compiler doesn't need that extra type cast in there... Because it's a 32-bit compiler ;-) With a 64-bit compiler, you would get warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] Alternatively, you can change pata_falcon_swap_mask from int to long. > > > 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
Hi Finn, Am 18.08.2023 um 19:43 schrieb Finn Thain: > > On Fri, 18 Aug 2023, Michael Schmitz wrote: > >> Some users of pata_falcon on Q40 have IDE disks in default >> IDE little endian byte order, whereas legacy disks use >> host-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 2 and 3. >> >> Default setting is no byte swapping, i.e. compatibility with >> the native Falcon or Q40 operating system disk format. >> >> Cc: William R Sowerbutts <will@sowerbutts.com> >> Cc: Finn Thain <fthain@linux-m68k.org> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org> >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> >> >> --- >> >> Changes since v1: >> >> Damien Le Moal: >> - change patch title >> - drop swap_data flag >> >> Finn Thain: >> - drop allocation of ap->private struct, use field as bitmask >> >> Changes since RFC v4: >> >> Geert Uytterhoeven: >> - don't shift static module parameter for drive 3/4 bitmask >> - simplify bit mask calculation to always use pdev->id >> >> Finn Thain: >> - correct bit numbers for drive 3/4 >> >> Changes since RFC v3: >> >> - split off this byte swap handling into separate patch >> >> - add hint regarding third and fourth drive on Q40 >> >> Finn Thain: >> - rename module parameter to 'data_swab' to better reflect its use >> >> William Sowerbutts: >> - correct IDE drive number used in data swap conditional >> --- >> drivers/ata/pata_falcon.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c >> index 346259e3bbc8..27443cb757de 100644 >> --- 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_swap_mask; >> + >> +module_param_named(data_swab, pata_falcon_swap_mask, 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), >> }; >> @@ -46,11 +51,12 @@ static unsigned int pata_falcon_data_xfer(struct ata_queued_cmd *qc, >> void __iomem *data_addr = ap->ioaddr.data_addr; >> unsigned int words = buflen >> 1; >> struct scsi_cmnd *cmd = qc->scsicmd; >> + int dev_id = dev->devno; > > Is that variable really needed? Not really, no. > >> bool swap = 1; >> >> if (dev->class == ATA_DEV_ATA && cmd && >> !blk_rq_is_passthrough(scsi_cmd_to_rq(cmd))) >> - swap = 0; >> + swap = (uintptr_t)ap->private_data & BIT(dev_id); >> >> /* Transfer multiple of 2 bytes */ >> if (rw == READ) { >> @@ -199,6 +205,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 *)(uintptr_t)(pata_falcon_swap_mask >> (2 * pdev->id)); >> + > > My compiler doesn't need that extra type cast in there... Geert's suggestion for 64 bit safety... Cheers, Michael > >> irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> if (irq_res && irq_res->start > 0) { >> irq = irq_res->start; >>
On Fri, 18 Aug 2023, Geert Uytterhoeven wrote: > On Fri, Aug 18, 2023 at 9:43 AM Finn Thain <fthain@linux-m68k.org> wrote: > > On Fri, 18 Aug 2023, Michael Schmitz wrote: > > > --- a/drivers/ata/pata_falcon.c > > > +++ b/drivers/ata/pata_falcon.c > > > @@ -199,6 +205,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 *)(uintptr_t)(pata_falcon_swap_mask >> (2 * pdev->id)); > > > + > > > > My compiler doesn't need that extra type cast in there... > > Because it's a 32-bit compiler ;-) > With a 64-bit compiler, you would get > > warning: cast to pointer from integer of different size > [-Wint-to-pointer-cast] > > Alternatively, you can change pata_falcon_swap_mask from int to long. > I see. Thanks for clarifying that for me.
diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c index 346259e3bbc8..27443cb757de 100644 --- 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_swap_mask; + +module_param_named(data_swab, pata_falcon_swap_mask, 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), }; @@ -46,11 +51,12 @@ static unsigned int pata_falcon_data_xfer(struct ata_queued_cmd *qc, void __iomem *data_addr = ap->ioaddr.data_addr; unsigned int words = buflen >> 1; struct scsi_cmnd *cmd = qc->scsicmd; + int dev_id = dev->devno; bool swap = 1; if (dev->class == ATA_DEV_ATA && cmd && !blk_rq_is_passthrough(scsi_cmd_to_rq(cmd))) - swap = 0; + swap = (uintptr_t)ap->private_data & BIT(dev_id); /* Transfer multiple of 2 bytes */ if (rw == READ) { @@ -199,6 +205,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 *)(uintptr_t)(pata_falcon_swap_mask >> (2 * pdev->id)); + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (irq_res && irq_res->start > 0) { irq = irq_res->start;
Some users of pata_falcon on Q40 have IDE disks in default IDE little endian byte order, whereas legacy disks use host-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 2 and 3. Default setting is no byte swapping, i.e. compatibility with the native Falcon or Q40 operating system disk format. Cc: William R Sowerbutts <will@sowerbutts.com> Cc: Finn Thain <fthain@linux-m68k.org> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> --- Changes since v1: Damien Le Moal: - change patch title - drop swap_data flag Finn Thain: - drop allocation of ap->private struct, use field as bitmask Changes since RFC v4: Geert Uytterhoeven: - don't shift static module parameter for drive 3/4 bitmask - simplify bit mask calculation to always use pdev->id Finn Thain: - correct bit numbers for drive 3/4 Changes since RFC v3: - split off this byte swap handling into separate patch - add hint regarding third and fourth drive on Q40 Finn Thain: - rename module parameter to 'data_swab' to better reflect its use William Sowerbutts: - correct IDE drive number used in data swap conditional --- drivers/ata/pata_falcon.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)