* Add ->atapi_{in,out}put_bytes and ->ata_{in,out}put_data methods to
falconide and q40ide host drivers (->ata_* methods are implemented on
top of ->atapi_* methods so they also do byte-swapping now).
* Cleanup atapi_{in,out}put_bytes().
Cc: Geert Uytterhoeven <[email protected]>
Cc: Michael Schmitz <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
PS one of future patches will merge ->atapi_ and ->ata_ methods for IDE
drivers/ide/ide-iops.c | 14 --------------
drivers/ide/legacy/falconide.c | 30 ++++++++++++++++++++++++++++++
drivers/ide/legacy/q40ide.c | 28 ++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 14 deletions(-)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -248,13 +248,6 @@ static void atapi_input_bytes(ide_drive_
ide_hwif_t *hwif = HWIF(drive);
++bytecount;
-#if defined(CONFIG_ATARI) || defined(CONFIG_Q40)
- if (MACH_IS_ATARI || MACH_IS_Q40) {
- /* Atari has a byte-swapped IDE interface */
- insw_swapw(hwif->io_ports.data_addr, buffer, bytecount / 2);
- return;
- }
-#endif /* CONFIG_ATARI || CONFIG_Q40 */
hwif->ata_input_data(drive, buffer, bytecount / 4);
if ((bytecount & 0x03) >= 2)
hwif->INSW(hwif->io_ports.data_addr,
@@ -266,13 +259,6 @@ static void atapi_output_bytes(ide_drive
ide_hwif_t *hwif = HWIF(drive);
++bytecount;
-#if defined(CONFIG_ATARI) || defined(CONFIG_Q40)
- if (MACH_IS_ATARI || MACH_IS_Q40) {
- /* Atari has a byte-swapped IDE interface */
- outsw_swapw(hwif->io_ports.data_addr, buffer, bytecount / 2);
- return;
- }
-#endif /* CONFIG_ATARI || CONFIG_Q40 */
hwif->ata_output_data(drive, buffer, bytecount / 4);
if ((bytecount & 0x03) >= 2)
hwif->OUTSW(hwif->io_ports.data_addr,
Index: b/drivers/ide/legacy/falconide.c
===================================================================
--- a/drivers/ide/legacy/falconide.c
+++ b/drivers/ide/legacy/falconide.c
@@ -44,6 +44,30 @@
int falconide_intr_lock;
EXPORT_SYMBOL(falconide_intr_lock);
+static void falconide_atapi_input_bytes(ide_drive_t *drive, void *buf,
+ unsigned int len)
+{
+ insw_swapw(drive->hwif->io_ports.data_addr, buf, (len + 1) / 2);
+}
+
+static void falconide_atapi_output_bytes(ide_drive_t *drive, void *buf,
+ unsigned int len)
+{
+ outsw_swapw(drive->hwif->io_ports.data_addr, buf, (len + 1) / 2);
+}
+
+static void falconide_ata_input_data(ide_drive_t *drive, void *buf,
+ unsigned int wcount)
+{
+ falconide_atapi_input_bytes(drive, buf, wcount * 4);
+}
+
+static void falconide_ata_output_data(ide_drive_t *drive, void *buf,
+ unsigned int wcount)
+{
+ falconide_atapi_output_bytes(drive, buf, wcount * 4);
+}
+
static void __init falconide_setup_ports(hw_regs_t *hw)
{
int i;
@@ -89,6 +113,12 @@ static int __init falconide_init(void)
ide_init_port_hw(hwif, &hw);
+ /* Atari has a byte-swapped IDE interface */
+ hwif->atapi_input_bytes = falconide_atapi_input_bytes;
+ hwif->atapi_output_bytes = falconide_atapi_output_bytes;
+ hwif->ata_input_data = falconide_ata_input_data;
+ hwif->ata_output_data = falconide_ata_output_data;
+
ide_get_lock(NULL, NULL);
ide_device_add(idx, NULL);
ide_release_lock();
Index: b/drivers/ide/legacy/q40ide.c
===================================================================
--- a/drivers/ide/legacy/q40ide.c
+++ b/drivers/ide/legacy/q40ide.c
@@ -90,7 +90,29 @@ void q40_ide_setup_ports ( hw_regs_t *hw
hw->ack_intr = ack_intr;
}
+static void q40ide_atapi_input_bytes(ide_drive_t *drive, void *buf,
+ unsigned int len)
+{
+ insw_swapw(drive->hwif->io_ports.data_addr, buf, (len + 1) / 2);
+}
+static void q40ide_atapi_output_bytes(ide_drive_t *drive, void *buf,
+ unsigned int len)
+{
+ outsw_swapw(drive->hwif->io_ports.data_addr, buf, (len + 1) / 2);
+}
+
+static void q40ide_ata_input_data(ide_drive_t *drive, void *buf,
+ unsigned int wcount)
+{
+ q40ide_atapi_input_bytes(drive, buf, wcount * 4);
+}
+
+static void q40ide_ata_output_data(ide_drive_t *drive, void *buf,
+ unsigned int wcount)
+{
+ q40ide_atapi_output_bytes(drive, buf, wcount * 4);
+}
/*
* the static array is needed to have the name reported in /proc/ioports,
@@ -141,6 +163,12 @@ static int __init q40ide_init(void)
if (hwif) {
ide_init_port_hw(hwif, &hw);
+ /* Q40 has a byte-swapped IDE interface */
+ hwif->atapi_input_bytes = q40ide_atapi_input_bytes;
+ hwif->atapi_output_bytes = q40ide_atapi_output_bytes;
+ hwif->ata_input_data = q40ide_ata_input_data;
+ hwif->ata_output_data = q40ide_ata_output_data;
+
idx[i] = hwif->index;
}
}
Hi Bart,
On Sun, 30 Mar 2008, Bartlomiej Zolnierkiewicz wrote:
> * Add ->atapi_{in,out}put_bytes and ->ata_{in,out}put_data methods to
> falconide and q40ide host drivers (->ata_* methods are implemented on
> top of ->atapi_* methods so they also do byte-swapping now).
>
> * Cleanup atapi_{in,out}put_bytes().
Thanks!
One remaining issue (for which the fix has never been submitted upstream so
far) with Atari and Q40 is that due to the byteswapped interface, the driveid
is also byteswapped, so it has to be unswapped again in ide_fix_driveid().
Here's a very old and unclean but working patch:
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -284,6 +284,23 @@ void ide_fix_driveid (struct hd_driveid
int i;
u16 *stringcast;
+#ifdef __mc68000__
+ if (!MACH_IS_AMIGA && !MACH_IS_MAC && !MACH_IS_Q40 && !MACH_IS_ATARI)
+ return;
+
+#ifdef M68K_IDE_SWAPW
+ if (M68K_IDE_SWAPW) { /* fix bus byteorder first */
+ u_char *p = (u_char *)id;
+ u_char t;
+ for (i = 0; i < 512; i += 2) {
+ t = p[i];
+ p[i] = p[i+1];
+ p[i+1] = t;
+ }
+ }
+#endif
+#endif /* __mc68000__ */
+
id->config = __le16_to_cpu(id->config);
id->cyls = __le16_to_cpu(id->cyls);
id->reserved2 = __le16_to_cpu(id->reserved2);
Note that include/asm-m68k/ide.h has
#define M68K_IDE_SWAPW (MACH_IS_Q40 || MACH_IS_ATARI)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi,
On Sunday 30 March 2008, Geert Uytterhoeven wrote:
> Hi Bart,
>
> On Sun, 30 Mar 2008, Bartlomiej Zolnierkiewicz wrote:
> > * Add ->atapi_{in,out}put_bytes and ->ata_{in,out}put_data methods to
> > falconide and q40ide host drivers (->ata_* methods are implemented on
> > top of ->atapi_* methods so they also do byte-swapping now).
> >
> > * Cleanup atapi_{in,out}put_bytes().
>
> Thanks!
>
> One remaining issue (for which the fix has never been submitted upstream so
> far) with Atari and Q40 is that due to the byteswapped interface, the driveid
> is also byteswapped, so it has to be unswapped again in ide_fix_driveid().
My patch causes unswapping for _all_ data coming from the device so I wonder
whether the ide_fix_driveid() fix is still needed?
[ I now recall some discussion that we shouldn't un-swap fs requests because
of how the things were done in the past fs itself is stored byte-swapped on
the disk - if this is the case I will recast the patch to pass rq to
->ata_*put_data in ide_pio_sector() and check rq->cmd_type == REQ_TYPE_FS
in falconide/q40ide_*put_data() to decide whether to unswap data or not ]
Thanks,
Bart
> Here's a very old and unclean but working patch:
>
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -284,6 +284,23 @@ void ide_fix_driveid (struct hd_driveid
> int i;
> u16 *stringcast;
>
> +#ifdef __mc68000__
> + if (!MACH_IS_AMIGA && !MACH_IS_MAC && !MACH_IS_Q40 && !MACH_IS_ATARI)
> + return;
> +
> +#ifdef M68K_IDE_SWAPW
> + if (M68K_IDE_SWAPW) { /* fix bus byteorder first */
> + u_char *p = (u_char *)id;
> + u_char t;
> + for (i = 0; i < 512; i += 2) {
> + t = p[i];
> + p[i] = p[i+1];
> + p[i+1] = t;
> + }
> + }
> +#endif
> +#endif /* __mc68000__ */
> +
> id->config = __le16_to_cpu(id->config);
> id->cyls = __le16_to_cpu(id->cyls);
> id->reserved2 = __le16_to_cpu(id->reserved2);
>
> Note that include/asm-m68k/ide.h has
>
> #define M68K_IDE_SWAPW (MACH_IS_Q40 || MACH_IS_ATARI)
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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
On Sun, 30 Mar 2008, Bartlomiej Zolnierkiewicz wrote:
> On Sunday 30 March 2008, Geert Uytterhoeven wrote:
> > On Sun, 30 Mar 2008, Bartlomiej Zolnierkiewicz wrote:
> > > * Add ->atapi_{in,out}put_bytes and ->ata_{in,out}put_data methods to
> > > falconide and q40ide host drivers (->ata_* methods are implemented on
> > > top of ->atapi_* methods so they also do byte-swapping now).
> > >
> > > * Cleanup atapi_{in,out}put_bytes().
> >
> > Thanks!
> >
> > One remaining issue (for which the fix has never been submitted upstream so
> > far) with Atari and Q40 is that due to the byteswapped interface, the driveid
> > is also byteswapped, so it has to be unswapped again in ide_fix_driveid().
>
> My patch causes unswapping for _all_ data coming from the device so I wonder
> whether the ide_fix_driveid() fix is still needed?
I'll give it a try on Aranym...
> [ I now recall some discussion that we shouldn't un-swap fs requests because
> of how the things were done in the past fs itself is stored byte-swapped on
> the disk - if this is the case I will recast the patch to pass rq to
> ->ata_*put_data in ide_pio_sector() and check rq->cmd_type == REQ_TYPE_FS
> in falconide/q40ide_*put_data() to decide whether to unswap data or not ]
Yes, the data on the disk is stored byte-swapped.
So it's only the drive ID and packet commands that should be swapped.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Mon, 31 Mar 2008, Geert Uytterhoeven wrote:
> On Sun, 30 Mar 2008, Bartlomiej Zolnierkiewicz wrote:
> > On Sunday 30 March 2008, Geert Uytterhoeven wrote:
> > > On Sun, 30 Mar 2008, Bartlomiej Zolnierkiewicz wrote:
> > > > * Add ->atapi_{in,out}put_bytes and ->ata_{in,out}put_data methods to
> > > > falconide and q40ide host drivers (->ata_* methods are implemented on
> > > > top of ->atapi_* methods so they also do byte-swapping now).
> > > >
> > > > * Cleanup atapi_{in,out}put_bytes().
> > >
> > > Thanks!
> > >
> > > One remaining issue (for which the fix has never been submitted upstream so
> > > far) with Atari and Q40 is that due to the byteswapped interface, the driveid
> > > is also byteswapped, so it has to be unswapped again in ide_fix_driveid().
> >
> > My patch causes unswapping for _all_ data coming from the device so I wonder
> > whether the ide_fix_driveid() fix is still needed?
>
> I'll give it a try on Aranym...
Oops, just applying this series of 5 patches is not sufficient, I get
lots of rejects...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
> Yes, the data on the disk is stored byte-swapped.
> So it's only the drive ID and packet commands that should be swapped.
If you are storing the data on disk byte swapped then reverse the logic
in the driver so you don't need hacks for un-swapping commands and write
a bytesewap device mapper layer in the right place. Then you can even
move disks around.
It's ugly because the solution is currently in the wrong place (for good
historical reasons this is true)
Alan
Hi,
On Monday 31. March 2008, Alan Cox wrote:
> > Yes, the data on the disk is stored byte-swapped.
> > So it's only the drive ID and packet commands that should be swapped.
>
> If you are storing the data on disk byte swapped then reverse the logic
> in the driver so you don't need hacks for un-swapping commands and write
> a bytesewap device mapper layer in the right place. Then you can even
> move disks around.
That would require an additional data copy and a double byteswap on machines
which are not that fast in first place...
bye, Roman
> That would require an additional data copy and a double byteswap on machines
> which are not that fast in first place...
So use the blitter ;)
At this point the future is not Commode Amiga or Atari TT and we risk
mess in core code for obscure platforms ?
Alan
Hi,
On Mon, 31 Mar 2008, Alan Cox wrote:
> > That would require an additional data copy and a double byteswap on machines
> > which are not that fast in first place...
>
> So use the blitter ;)
>
> At this point the future is not Commode Amiga or Atari TT and we risk
> mess in core code for obscure platforms ?
We have worse mess and a _single_ simple hook which helps a small platform
and doesn't hurt others doesn't sound that bad.
bye, Roman