From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Linus Torvalds <torvalds@transmeta.com>
Cc: Paul Mackerras <paulus@samba.org>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
Linux Kernel Development <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] M68k IDE updates
Date: Mon, 14 Apr 2003 21:54:58 +0200 (MEST) [thread overview]
Message-ID: <Pine.GSO.4.21.0304142148210.25092-100000@vervain.sonytel.be> (raw)
In-Reply-To: <Pine.LNX.4.44.0304141038430.19302-100000@home.transmeta.com>
On Mon, 14 Apr 2003, Linus Torvalds wrote:
> On Mon, 14 Apr 2003, Paul Mackerras wrote:
> > Since __ide_mm_insw doesn't get told whether it is transferring normal
> > sector data or drive ID data, it can't necessarily do the right thing
> > in both situations.
>
> Can we please then just separate the two functions out into "fetch sector
> data" and "fetch drive ID"? And NOT playing with another frigging broken
> passed-down flag that people get wrong and isn't obvious what it does
> anyway? It's a lot easier to do
>
> /* On sane architectures, data and ID are accessed the same */
> #define ide_fetch_sector_data(...) __ide_fetch_data(..)
> #define ide_fetch_id_data(...) __ide_fetch_data(..)
>
> than it is to carry a flag around and having to remember to get it right
> in every place this is used.
>
> It's more efficient too, but the _clarity_ and lack of dynamic flags is a
> hell of a lot more important.
>
> And stupid architectures that may have to re-implement (and possible
> duplicate) the ID fetch code only have themselves to blame. Although it
> might easily be as simple as
>
> /*
> * The PCI bus is wired up the wrong way, we need to byteswap
> * the ID results after they come back
> */
> static inline xxx ide_fetch_id_data(...)
> {
> __ide_fetch_data(..)
> bswap_id_data(..)
> }
>
> and please keep this in some m68k-specific file instead of forcing
> _everybody_ to know about the braindamage.
I think the least-intrusive solution is something like this:
--- linux-2.5/drivers/ide/ide-iops.c.orig Mon Apr 14 21:43:30 2003
+++ linux-2.5/drivers/ide/ide-iops.c Mon Apr 14 21:44:53 2003
@@ -423,8 +423,7 @@
*/
void ide_fix_driveid (struct hd_driveid *id)
{
-#ifndef __LITTLE_ENDIAN
-# ifdef __BIG_ENDIAN
+ if (ide_driveid_needs_swapping(id)) {
int i;
u16 *stringcast;
@@ -512,10 +511,7 @@
for (i = 0; i < 48; i++)
id->words206_254[i] = __le16_to_cpu(id->words206_254[i]);
id->integrity_word = __le16_to_cpu(id->integrity_word);
-# else
-# error "Please fix <asm/byteorder.h>"
-# endif
-#endif
+ }
}
EXPORT_SYMBOL(ide_fix_driveid);
Where ide_driveid_needs_swapping() is #define'd to return 0 (never swap), 1
(always swap), or whatever architecture-specific logic you need.
We can even have defaults in <linux/ide.h>
#ifndef ide_driveid_needs_swapping
# ifdef __LITTLE_ENDIAN
# define ide_driveid_needs_swapping(id) 0
# else
# ifdef __BIG_ENDIAN
# define ide_driveid_needs_swapping(id) 1
# else
# error "Please fix <asm/byteorder.h>"
# endif
# endif
#endif
Sounds sufficiently sane?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
next prev parent reply other threads:[~2003-04-14 19:43 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-04-13 13:06 [PATCH] M68k IDE updates Geert Uytterhoeven
2003-04-13 14:10 ` Alan Cox
2003-04-13 23:43 ` Paul Mackerras
2003-04-14 8:39 ` Geert Uytterhoeven
2003-04-14 9:19 ` Benjamin Herrenschmidt
2003-04-14 9:24 ` Geert Uytterhoeven
2003-04-14 12:19 ` Alan Cox
2003-04-14 12:21 ` Alan Cox
2003-04-14 13:44 ` Geert Uytterhoeven
2003-04-14 16:03 ` Alan Cox
2003-04-15 4:45 ` Jamie Lokier
2003-04-15 8:11 ` Geert Uytterhoeven
2003-04-15 9:23 ` Jörn Engel
2003-04-15 9:52 ` Geert Uytterhoeven
2003-04-14 12:48 ` Alan Cox
2003-04-14 12:48 ` Alan Cox
2003-04-14 17:44 ` Linus Torvalds
2003-04-14 19:54 ` Geert Uytterhoeven [this message]
2003-04-14 22:31 ` Paul Mackerras
2003-04-15 8:14 ` Geert Uytterhoeven
2003-04-21 16:55 ` Geert Uytterhoeven
2003-04-22 14:49 ` Alan Cox
2003-04-22 13:55 Mudama, Eric
[not found] <Pine.GSO.4.21.0304221802570.16017-100000@vervain.sonytel.be>
2003-04-23 11:27 ` Richard Zidlicky
2003-04-23 11:04 ` Alan Cox
2003-04-23 20:19 ` John Bradford
2003-04-24 9:47 ` Richard Zidlicky
2003-04-24 11:26 ` Geert Uytterhoeven
2003-04-24 13:14 ` Richard Zidlicky
2003-04-24 14:11 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.GSO.4.21.0304142148210.25092-100000@vervain.sonytel.be \
--to=geert@linux-m68k.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=paulus@samba.org \
--cc=torvalds@transmeta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).