I tested this patch
[ 1.540000] atari-falcon-ide atari-falcon-ide.0: Atari Falcon and Q40/Q60 PATA controller
[ 1.570000] scsi host0: pata_falcon
[ 1.580000] ata1: PATA max PIO4 cmd 0x1f0 ctl 0x3f6 irq 14
[ 1.590000] atari-falcon-ide atari-falcon-ide.1: Atari Falcon and Q40/Q60 PATA controller
[ 1.630000] scsi host1: pata_falcon
[ 1.640000] ata2: PATA max PIO4 cmd 0x170 ctl 0x376 irq 15
[ 1.840000] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 1.840000] Oops: 00000000
[ 1.840000] Modules linked in:
[ 1.840000] PC: [<00211a82>] pata_falcon_data_xfer+0x2e/0x220
[ 1.840000] SR: 2700 SP: (ptrval) a2: 00853070
[ 1.840000] d0: 00915ea0 d1: 000010a0 d2: 000000b0 d3: 00000ea0
[ 1.840000] d4: 00000160 d5: 00000000 a0: 0091549a a1: 00915d60
[ 1.840000] Process kworker/0:1 (pid: 14, task=(ptrval))
[ 1.840000] Frame format=7 eff addr=00859e64 ssw=0505 faddr=00000000
[ 1.840000] wb 1 stat/addr/data: 0005 00306b70 004666b4
[ 1.840000] wb 2 stat/addr/data: 0005 00032934 00466b22
[ 1.840000] wb 3 stat/addr/data: 0005 00859e44 00000000
[ 1.840000] push data: 004666b4 00032a56 00464982 00000003
[ 1.840000] Stack from 00859e44:
[ 1.840000] 00000000 00000ea0 00915658 00000001 00000000 004caff4 00914000 002109b0
[ 1.840000] 0020ff74 00000000 002109ec 0091549a 00915ea0 00000160 00000000 004caff4
[ 1.840000] 0091549a 00210ae8 0091549a 004caff4 00000ea0 00000160 00915458 00000008
[ 1.840000] 0091549a 00914000 00915550 00211072 0091549a 00915458 00000008 0091549a
[ 1.840000] 00914000 00211564 0091549a 0091549a 00211084 0031bcfa 00203eb4 00000000
[ 1.840000] 007ca5e0 0091405e 00914000 00915550 0020ff74 0000a7c6 0091549a 0091405e
[ 1.840000] Call Trace: [<002109b0>] ata_pio_xfer+0x0/0x8c
[ 1.840000] [<0020ff74>] ata_sff_busy_wait+0x0/0x42
[ 1.840000] [<002109ec>] ata_pio_xfer+0x3c/0x8c
[ 1.840000] [<00210ae8>] ata_pio_sector+0xac/0x112
[ 1.840000] [<00211072>] ata_pio_sectors+0x94/0xa6
[ 1.840000] [<00211564>] ata_sff_hsm_move+0x4e0/0x56e
[ 1.840000] [<00211084>] ata_sff_hsm_move+0x0/0x56e
[ 1.840000] [<0031bcfa>] warn_slowpath_fmt+0x0/0x62
[ 1.840000] [<00203eb4>] ata_msleep+0x0/0xa2
[ 1.840000] [<0020ff74>] ata_sff_busy_wait+0x0/0x42
[ 1.840000] [<0000a7c6>] ATANSM+0x62/0x70
[ 1.840000] [<002116ea>] ata_sff_pio_task+0xf8/0x104
[ 1.840000] [<00015cbc>] kernel_thread+0x0/0x68
[ 1.840000] [<00024f36>] process_one_work+0x12a/0x1b2
[ 1.840000] [<00022fc2>] worker_clr_flags+0x0/0x72
[ 1.840000] [<00025406>] worker_thread+0x230/0x292
[ 1.840000] [<00015cbc>] kernel_thread+0x0/0x68
[ 1.840000] [<000295a4>] kthread_exit+0x0/0x14
[ 1.840000] [<000251d6>] worker_thread+0x0/0x292
[ 1.840000] [<00029668>] kthread+0x96/0xa0
[ 1.840000] [<000295d2>] kthread+0x0/0xa0
[ 1.840000] [<00002898>] ret_from_kernel_thread+0xc/0x14
[ 1.840000]
[ 1.840000] Code: 286d 0024 2c6d 24e8 2404 e28a 2a68 0008 <2055> 2068 00ac 2228 00fc 7601 c684 7001 b0a9 0108 6742 7007 c082 4a85 6700 0090
[ 1.840000] Disabling lock debugging due to kernel taint
[ 1.840000] note: kworker/0:1[14] exited with irqs disabled
This line is the problem:
+ int dev_id = cmd->device->sdev_target->id;
I changed this to 'dev_id = dev->devno' instead, which fixed it.
I confirmed that the byte swapping on/off feature works.
Will
On Wed, Aug 16, 2023 at 10:32:12AM +1200, Michael Schmitz wrote:
With commit 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver
with pata_falcon and falconide"), the Q40 IDE driver was
replaced by pata_falcon.c (and the later obsoleted
falconide.c).
Both IO and memory resources were defined for the Q40 IDE
platform device, but definition of the IDE register addresses
was modeled after the Falcon case, both in use of the memory
resources and in including register scale and byte vs. word
offset in the address.
This was correct for the Falcon case, which does not apply
any address translation to the register addresses. In the
Q40 case, all of device base address, byte access offset
and register scaling is included in the platform specific
ISA access translation (in asm/mm_io.h).
As a consequence, such address translation gets applied
twice, and register addresses are mangled.
Use the device base address from the platform IO resource,
and use standard register offsets from that base in order
to calculate register addresses (the IO address translation
will then apply the correct ISA window base and scaling).
Encode PIO_OFFSET into IO port addresses for all registers
except the data transfer register. Encode the MMIO offset
there (pata_falcon_data_xfer() directly uses raw IO with
no address translation).
Add module parameter 'data_swap' to allow connecting drives
with non-native data byte order. Drives selected by the
data_swap bit mask will have their user data swapped to host
byte order, i.e. 'pata_falcon.data_swap=2' will swap all user
data on drive B, leaving data on drive A in native order.
Reported-by: William R Sowerbutts <will@xxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@xxxxxxxxxxxxxx
Link: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@xxxxxxxxxxxxxx
Fixes: 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver with pata_falcon and falconide")
Cc: Finn Thain <fthain@xxxxxxxxxxxxxx>
Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Signed-off-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
---
Changes from v2:
- add driver parameter 'data_swap' as bit mask for drives to swap
Changes from v1:
Finn Thain:
- take care to supply IO address suitable for ioread8/iowrite8
- use MMIO address for data transfer
---
drivers/ata/pata_falcon.c | 90 ++++++++++++++++++++++++++++++---------
1 file changed, 69 insertions(+), 21 deletions(-)
diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c
index 996516e64f13..e6038eca39d6 100644
--- a/drivers/ata/pata_falcon.c
+++ b/drivers/ata/pata_falcon.c
@@ -33,6 +33,16 @@
#define DRV_NAME "pata_falcon"
#define DRV_VERSION "0.1.0"
+static int pata_falcon_swap_mask = 0;
+
+module_param_named(data_swap, pata_falcon_swap_mask, int, 0444);
+MODULE_PARM_DESC(data_swap, "Data byte swap enable/disable (0x1==drive1, 0x2==drive2, default==0)");
+
+struct pata_falcon_priv {
+ unsigned int swap_mask;
+ bool swap_data;
+};
+
static const struct scsi_host_template pata_falcon_sht = {
ATA_PIO_SHT(DRV_NAME),
};
@@ -44,13 +54,15 @@ static unsigned int pata_falcon_data_xfer(struct ata_queued_cmd *qc,
struct ata_device *dev = qc->dev;
struct ata_port *ap = dev->link->ap;
void __iomem *data_addr = ap->ioaddr.data_addr;
+ struct pata_falcon_priv *priv = ap->private_data;
unsigned int words = buflen >> 1;
struct scsi_cmnd *cmd = qc->scsicmd;
+ int dev_id = cmd->device->sdev_target->id;
bool swap = 1;
if (dev->class == ATA_DEV_ATA && cmd &&
!blk_rq_is_passthrough(scsi_cmd_to_rq(cmd)))
- swap = 0;
+ swap = priv->swap_data && (priv->swap_mask & 1<<dev_id);
/* Transfer multiple of 2 bytes */
if (rw == READ) {
@@ -123,6 +135,7 @@ static int __init pata_falcon_init_one(struct platform_device *pdev)
struct resource *base_res, *ctl_res, *irq_res;
struct ata_host *host;
struct ata_port *ap;
+ struct pata_falcon_priv *priv;
void __iomem *base;
int irq = 0;
@@ -165,26 +178,61 @@ 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;
- base = (void __iomem *)base_mem_res->start;
- /* N.B. this assumes data_addr will be used for word-sized I/O only */
- ap->ioaddr.data_addr = base + 0 + 0 * 4;
- ap->ioaddr.error_addr = base + 1 + 1 * 4;
- ap->ioaddr.feature_addr = base + 1 + 1 * 4;
- ap->ioaddr.nsect_addr = base + 1 + 2 * 4;
- ap->ioaddr.lbal_addr = base + 1 + 3 * 4;
- ap->ioaddr.lbam_addr = base + 1 + 4 * 4;
- ap->ioaddr.lbah_addr = base + 1 + 5 * 4;
- ap->ioaddr.device_addr = base + 1 + 6 * 4;
- ap->ioaddr.status_addr = base + 1 + 7 * 4;
- ap->ioaddr.command_addr = base + 1 + 7 * 4;
-
- base = (void __iomem *)ctl_mem_res->start;
- ap->ioaddr.altstatus_addr = base + 1;
- ap->ioaddr.ctl_addr = base + 1;
-
- ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx",
- (unsigned long)base_mem_res->start,
- (unsigned long)ctl_mem_res->start);
+ priv = devm_kzalloc(&pdev->dev,
+ sizeof(struct pata_falcon_priv), GFP_KERNEL);
+
+ if (!priv)
+ return -ENOMEM;
+
+ ap->private_data = priv;
+
+ priv->swap_mask = pata_falcon_swap_mask;
+ if (priv->swap_mask)
+ priv->swap_data = 1;
+
+ if (MACH_IS_Q40) {
+ base = (void __iomem *)base_mem_res->start;
+ ap->ioaddr.data_addr = base + 0;
+ base = (void __iomem *)base_res->start;
+ ap->ioaddr.error_addr = base + 0x10000 + 1;
+ ap->ioaddr.feature_addr = base + 0x10000 + 1;
+ ap->ioaddr.nsect_addr = base + 0x10000 + 2;
+ ap->ioaddr.lbal_addr = base + 0x10000 + 3;
+ ap->ioaddr.lbam_addr = base + 0x10000 + 4;
+ ap->ioaddr.lbah_addr = base + 0x10000 + 5;
+ ap->ioaddr.device_addr = base + 0x10000 + 6;
+ ap->ioaddr.status_addr = base + 0x10000 + 7;
+ ap->ioaddr.command_addr = base + 0x10000 + 7;
+
+ base = (void __iomem *)ctl_res->start;
+ ap->ioaddr.altstatus_addr = base + 0x10000;
+ ap->ioaddr.ctl_addr = base + 0x10000;
+
+ ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx",
+ (unsigned long)base_res->start,
+ (unsigned long)ctl_res->start);
+ } else {
+ base = (void __iomem *)base_mem_res->start;
+ /* N.B. this assumes data_addr will be used for word-sized I/O only */
+ ap->ioaddr.data_addr = base + 0 + 0 * 4;
+ ap->ioaddr.error_addr = base + 1 + 1 * 4;
+ ap->ioaddr.feature_addr = base + 1 + 1 * 4;
+ ap->ioaddr.nsect_addr = base + 1 + 2 * 4;
+ ap->ioaddr.lbal_addr = base + 1 + 3 * 4;
+ ap->ioaddr.lbam_addr = base + 1 + 4 * 4;
+ ap->ioaddr.lbah_addr = base + 1 + 5 * 4;
+ ap->ioaddr.device_addr = base + 1 + 6 * 4;
+ ap->ioaddr.status_addr = base + 1 + 7 * 4;
+ ap->ioaddr.command_addr = base + 1 + 7 * 4;
+
+ base = (void __iomem *)ctl_mem_res->start;
+ ap->ioaddr.altstatus_addr = base + 1;
+ ap->ioaddr.ctl_addr = base + 1;
+
+ ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx",
+ (unsigned long)base_mem_res->start,
+ (unsigned long)ctl_mem_res->start);
+ }
irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (irq_res && irq_res->start > 0) {
--
2.17.1
_________________________________________________________________________
William R Sowerbutts will@xxxxxxxxxxxxxx
"Carpe post meridiem" http://sowerbutts.com
main(){char*s=">#=0> ^#X@#@^7=",c=0,m;for(;c<15;c++)for
(m=-1;m<7;putchar(m++/6&c%3/2?10:s[c]-31&1<<m?42:32));}
