* [PATCH] IDE: correct partially initialised hw structures
@ 2002-10-27 20:09 Peter Denison
2002-11-03 21:09 ` Andre Hedrick
0 siblings, 1 reply; 4+ messages in thread
From: Peter Denison @ 2002-10-27 20:09 UTC (permalink / raw)
To: Andre Hedrick, Alan Cox; +Cc: linux-kernel
Summary: Initialise all parts of hw_regs_t structures before passing them
to ide_register_hw
The hw structure (specifically the hw->chipset field) held uninitialised
data. This (before the initialisation order fixup recently posted) meant
that no chipset could ever get selected by an idex=<chipset> commandline
(silently!).
Only occurs on non-PCI platforms. All ARM platforms have already been
fixed - though slightly differently.
I'm not sure how valid a lot of the code I patched actually is. Does it
make sense on e.g. ia64 or x86_64 to specify CONFIG_PCI = n? Or even to
have the concept of default interfaces? The interfaces should get probed
anyway, and the ide_init_hwif_ports() part is called from init_hwif_data()
already. The only thing these arch-specific routines do extra is calling
ide_register_hw, which probably wants to happen from the probes, not here.
Applies: 2.5.44 (and probably others)
--- linux/include/asm-ia64/ide.h.old 2002-10-06 10:11:35.000000000 +0100
+++ linux/include/asm-ia64/ide.h 2002-10-27 18:47:26.000000000 +0000
@@ -82,6 +82,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for(index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-x86_64/ide.h.old 2002-10-19 12:43:40.000000000 +0100
+++ linux/include/asm-x86_64/ide.h 2002-10-27 18:47:46.000000000 +0000
@@ -69,6 +69,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for(index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-ppc/ide.h.old 2002-10-06 10:11:33.000000000 +0100
+++ linux/include/asm-ppc/ide.h 2002-10-27 18:48:20.000000000 +0000
@@ -89,6 +89,8 @@
int index;
ide_ioreg_t base;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for (index = 0; index < MAX_HWIFS; index++) {
base = ide_default_io_base(index);
if (base == 0)
--- linux/include/asm-mips/ide.h.old 2002-08-25 13:26:53.000000000 +0100
+++ linux/include/asm-mips/ide.h 2002-10-27 19:37:39.000000000 +0000
@@ -55,6 +55,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for(index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-parisc/ide.h.old 2002-08-25 13:26:53.000000000 +0100
+++ linux/include/asm-parisc/ide.h 2002-10-27 18:48:54.000000000 +0000
@@ -71,6 +71,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for(index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-i386/ide.h.old 2002-10-06 10:02:03.000000000 +0100
+++ linux/include/asm-i386/ide.h 2002-10-19 00:17:05.000000000 +0100
@@ -68,6 +68,8 @@
#ifndef CONFIG_PCI
hw_regs_t hw;
int index;
+
+ memset(&hw, 0, sizeof(hw_regs_t));
for(index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
--- linux/include/asm-sparc64/ide.h.old 2002-10-06 10:02:03.000000000 +0100
+++ linux/include/asm-sparc64/ide.h 2002-10-27 18:50:10.000000000 +0000
@@ -59,6 +59,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for (index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-alpha/ide.h.old 2002-10-06 10:11:37.000000000 +0100
+++ linux/include/asm-alpha/ide.h 2002-10-27 18:50:58.000000000 +0000
@@ -72,6 +72,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for (index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-sparc/ide.h.old 2002-10-06 10:02:03.000000000 +0100
+++ linux/include/asm-sparc/ide.h 2002-10-27 18:52:12.000000000 +0000
@@ -63,6 +63,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for (index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-cris/ide.h.old 2002-08-25 13:26:53.000000000 +0100
+++ linux/include/asm-cris/ide.h 2002-10-27 18:52:48.000000000 +0000
@@ -79,6 +79,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for(index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-mips64/ide.h.old 2002-08-25 13:26:53.000000000 +0100
+++ linux/include/asm-mips64/ide.h 2002-10-27 19:38:04.000000000 +0000
@@ -58,6 +58,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for(index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--- linux/include/asm-sh/ide.h.old 2002-08-25 13:26:53.000000000 +0100
+++ linux/include/asm-sh/ide.h 2002-10-27 18:53:46.000000000 +0000
@@ -97,6 +97,8 @@
hw_regs_t hw;
int index;
+ memset(&hw, 0, sizeof(hw_regs_t));
+
for(index = 0; index < MAX_HWIFS; index++) {
ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
hw.irq = ide_default_irq(ide_default_io_base(index));
--
Peter Denison <peterd at marshadder dot uklinux dot net>
Please use the address above only for personal mail, not copied to any lists
that are gatewayed to news or web pages unless the addresses are removed.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] IDE: correct partially initialised hw structures
2002-10-27 20:09 [PATCH] IDE: correct partially initialised hw structures Peter Denison
@ 2002-11-03 21:09 ` Andre Hedrick
2002-11-07 19:04 ` Peter Denison
0 siblings, 1 reply; 4+ messages in thread
From: Andre Hedrick @ 2002-11-03 21:09 UTC (permalink / raw)
To: Peter Denison; +Cc: Alan Cox, linux-kernel
No major issues here; however does it not stomp on the preloaded defaults?
On Sun, 27 Oct 2002, Peter Denison wrote:
> Summary: Initialise all parts of hw_regs_t structures before passing them
> to ide_register_hw
>
> The hw structure (specifically the hw->chipset field) held uninitialised
> data. This (before the initialisation order fixup recently posted) meant
> that no chipset could ever get selected by an idex=<chipset> commandline
> (silently!).
>
> Only occurs on non-PCI platforms. All ARM platforms have already been
> fixed - though slightly differently.
>
> I'm not sure how valid a lot of the code I patched actually is. Does it
> make sense on e.g. ia64 or x86_64 to specify CONFIG_PCI = n? Or even to
> have the concept of default interfaces? The interfaces should get probed
> anyway, and the ide_init_hwif_ports() part is called from init_hwif_data()
> already. The only thing these arch-specific routines do extra is calling
> ide_register_hw, which probably wants to happen from the probes, not here.
>
> Applies: 2.5.44 (and probably others)
>
> --- linux/include/asm-ia64/ide.h.old 2002-10-06 10:11:35.000000000 +0100
> +++ linux/include/asm-ia64/ide.h 2002-10-27 18:47:26.000000000 +0000
> @@ -82,6 +82,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for(index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-x86_64/ide.h.old 2002-10-19 12:43:40.000000000 +0100
> +++ linux/include/asm-x86_64/ide.h 2002-10-27 18:47:46.000000000 +0000
> @@ -69,6 +69,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for(index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-ppc/ide.h.old 2002-10-06 10:11:33.000000000 +0100
> +++ linux/include/asm-ppc/ide.h 2002-10-27 18:48:20.000000000 +0000
> @@ -89,6 +89,8 @@
> int index;
> ide_ioreg_t base;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for (index = 0; index < MAX_HWIFS; index++) {
> base = ide_default_io_base(index);
> if (base == 0)
> --- linux/include/asm-mips/ide.h.old 2002-08-25 13:26:53.000000000 +0100
> +++ linux/include/asm-mips/ide.h 2002-10-27 19:37:39.000000000 +0000
> @@ -55,6 +55,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for(index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-parisc/ide.h.old 2002-08-25 13:26:53.000000000 +0100
> +++ linux/include/asm-parisc/ide.h 2002-10-27 18:48:54.000000000 +0000
> @@ -71,6 +71,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for(index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-i386/ide.h.old 2002-10-06 10:02:03.000000000 +0100
> +++ linux/include/asm-i386/ide.h 2002-10-19 00:17:05.000000000 +0100
> @@ -68,6 +68,8 @@
> #ifndef CONFIG_PCI
> hw_regs_t hw;
> int index;
> +
> + memset(&hw, 0, sizeof(hw_regs_t));
>
> for(index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> --- linux/include/asm-sparc64/ide.h.old 2002-10-06 10:02:03.000000000 +0100
> +++ linux/include/asm-sparc64/ide.h 2002-10-27 18:50:10.000000000 +0000
> @@ -59,6 +59,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for (index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-alpha/ide.h.old 2002-10-06 10:11:37.000000000 +0100
> +++ linux/include/asm-alpha/ide.h 2002-10-27 18:50:58.000000000 +0000
> @@ -72,6 +72,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for (index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-sparc/ide.h.old 2002-10-06 10:02:03.000000000 +0100
> +++ linux/include/asm-sparc/ide.h 2002-10-27 18:52:12.000000000 +0000
> @@ -63,6 +63,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for (index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-cris/ide.h.old 2002-08-25 13:26:53.000000000 +0100
> +++ linux/include/asm-cris/ide.h 2002-10-27 18:52:48.000000000 +0000
> @@ -79,6 +79,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for(index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-mips64/ide.h.old 2002-08-25 13:26:53.000000000 +0100
> +++ linux/include/asm-mips64/ide.h 2002-10-27 19:38:04.000000000 +0000
> @@ -58,6 +58,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for(index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
> --- linux/include/asm-sh/ide.h.old 2002-08-25 13:26:53.000000000 +0100
> +++ linux/include/asm-sh/ide.h 2002-10-27 18:53:46.000000000 +0000
> @@ -97,6 +97,8 @@
> hw_regs_t hw;
> int index;
>
> + memset(&hw, 0, sizeof(hw_regs_t));
> +
> for(index = 0; index < MAX_HWIFS; index++) {
> ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> hw.irq = ide_default_irq(ide_default_io_base(index));
>
> --
> Peter Denison <peterd at marshadder dot uklinux dot net>
> Please use the address above only for personal mail, not copied to any lists
> that are gatewayed to news or web pages unless the addresses are removed.
>
Andre Hedrick
LAD Storage Consulting Group
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] IDE: correct partially initialised hw structures
2002-11-03 21:09 ` Andre Hedrick
@ 2002-11-07 19:04 ` Peter Denison
2002-11-12 14:28 ` Alan Cox
0 siblings, 1 reply; 4+ messages in thread
From: Peter Denison @ 2002-11-07 19:04 UTC (permalink / raw)
To: Andre Hedrick; +Cc: Alan Cox, linux-kernel
On Sun, 3 Nov 2002, Andre Hedrick wrote:
> No major issues here; however does it not stomp on the preloaded defaults?
Well, the patch cleans up the fact that preloaded defaults are stomped on
by uninitialised data. Now they are stomped on by 0 !! Gradual
improvement.
Actually, if the patch that moves ide_init_default_hwifs() is applied then
maybe this isn't strictly necessary. The whole area needs cleaning up, but
I was loathe to start doing major changes. I've done large parts of the
call tree for the ide driver, and I _still_ don't fully understand what
should be called when. Just as a sample, there's:
ide_init_data
ide_init_default_hwifs
ide_init_hwif_ports
init_hwif_data
ideprobe_init
probe_hwif_init
hwif_init
etc.
with little or no explanation as to what they do, but more crucially, when
they should be called. You can work out what they each do, but an idea of
the initialisation architecture would really help sort out if there are
branches that can now be got rid of.
> On Sun, 27 Oct 2002, Peter Denison wrote:
>
> > Summary: Initialise all parts of hw_regs_t structures before passing them
> > to ide_register_hw
> >
> > The hw structure (specifically the hw->chipset field) held uninitialised
> > data. This (before the initialisation order fixup recently posted) meant
> > that no chipset could ever get selected by an idex=<chipset> commandline
> > (silently!).
> >
> > Only occurs on non-PCI platforms. All ARM platforms have already been
> > fixed - though slightly differently.
--
Peter Denison <peterd at marshadder dot uklinux dot net>
Please use the address above only for personal mail, not copied to any lists
that are gatewayed to news or web pages unless the addresses are removed.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] IDE: correct partially initialised hw structures
2002-11-07 19:04 ` Peter Denison
@ 2002-11-12 14:28 ` Alan Cox
0 siblings, 0 replies; 4+ messages in thread
From: Alan Cox @ 2002-11-12 14:28 UTC (permalink / raw)
To: Peter Denison; +Cc: Andre Hedrick, Alan Cox, Linux Kernel Mailing List
On Thu, 2002-11-07 at 19:04, Peter Denison wrote:
> Actually, if the patch that moves ide_init_default_hwifs() is applied then
> maybe this isn't strictly necessary. The whole area needs cleaning up, but
> I was loathe to start doing major changes. I've done large parts of the
> call tree for the ide driver, and I _still_ don't fully understand what
> should be called when. Just as a sample, there's:
> ide_init_data
> ide_init_default_hwifs
> ide_init_hwif_ports
> init_hwif_data
> ideprobe_init
> probe_hwif_init
> hwif_init
> etc.
Can you leave that bit alone for the moment. I'm currently splitting
ide.c into ide.c and ide-io.c so that all the crap is in one place ready
for extermination
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2002-11-12 13:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-10-27 20:09 [PATCH] IDE: correct partially initialised hw structures Peter Denison
2002-11-03 21:09 ` Andre Hedrick
2002-11-07 19:04 ` Peter Denison
2002-11-12 14:28 ` Alan Cox
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).