BLK_DEV_IDE_CELLEB dependency fix

PATCH NAK Al Viro 3 participants 10 messages Mar 14, 2007 – Mar 16, 2007 1 message not in the archive

lore

Share & tools

lore

Al Viro

author

2007-03-14 09:16 UTC #0 lore raw

PATCH 10/13 BLK_DEV_IDE_CELLEB dependency fix

It's bool and it depends on IDE => should depend on IDE=y

Signed-off-by Al Viro <[email protected]>

---
 drivers/ide/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/ide/Kconfig +1 -1

index 5d134bb..3f76987 100644
@@ -802,7 +802,7 @@ config BLK_DEV_IDEDMA_PMAC
 
 config BLK_DEV_IDE_CELLEB
 	bool "Toshiba's Cell Reference Set IDE support"
-	depends on PPC_CELLEB
+	depends on PPC_CELLEB && IDE=y
 	help
 	  This driver provides support for the built-in IDE controller on
 	  Toshiba Cell Reference Board.

Signature · 1 line

1.5.0-rc2.GIT

Akira Iguchi

2007-03-15 05:23 UTC #1 lore raw

Quoted 4 lines · Al

It's bool and it depends on IDE => should depend on IDE=y
Signed-off-by Al Viro <[email protected]>

Move to "if BLK_DEV_IDEDMA_PCI" block because it depends on BLK_DEV_IDEDMA_PCI.

Signed-off-by Kou Ishizaki <[email protected]>

Signed-off-by Akira Iguchi <[email protected]>

---

drivers/ide/Kconfig +8 -8

diff -Nrpu -X linux-2.6.21-rc3/Documentation/dontdiff linux-2.6.21-rc3/drivers/ide/Kconfig linux-2.6.21-rc3.mod/drivers/ide/Kconfig
@@ -769,6 +769,14 @@ config BLK_DEV_TC86C001
 	help
 	This driver adds support for Toshiba TC86C001 GOKU-S chip.
 
+config BLK_DEV_IDE_CELLEB
+	bool "Toshiba's Cell Reference Set IDE support"
+	depends on PPC_CELLEB
+	help
+	  This driver provides support for the built-in IDE controller on
+	  Toshiba Cell Reference Board.
+	  If unsure, say Y.
+
 endif
 
 config BLK_DEV_IDE_PMAC

@@ -800,14 +808,6 @@ config BLK_DEV_IDEDMA_PMAC
 	  to transfer data to and from memory.  Saying Y is safe and improves
 	  performance.
 
-config BLK_DEV_IDE_CELLEB
-	bool "Toshiba's Cell Reference Set IDE support"
-	depends on PPC_CELLEB
-	help
-	  This driver provides support for the built-in IDE controller on
-	  Toshiba Cell Reference Board.
-	  If unsure, say Y.
-
 config BLK_DEV_IDE_SWARM
 	tristate "IDE for Sibyte evaluation boards"
 	depends on SIBYTE_SB1xxx_SOC

A message that is not in the archive (sent off-list or not yet received): [email protected]

Al Viro

author

2007-03-15 05:36 UTC #3 lore raw

Quoted 9 lines · Akira Iguchi

Quoted 4 lines · Al
It's bool and it depends on IDE => should depend on IDE=y
Signed-off-by Al Viro <[email protected]>
Move to "if BLK_DEV_IDEDMA_PCI" block because it depends on BLK_DEV_IDEDMA_PCI.

Review

Quoted 8 lines

(quoted diff) +8 -0

+config BLK_DEV_IDE_CELLEB
+	bool "Toshiba's Cell Reference Set IDE support"
+	depends on PPC_CELLEB
+	help
+	  This driver provides support for the built-in IDE controller on
+	  Toshiba Cell Reference Board.
+	  If unsure, say Y.
+

Eh... You still need dependency on IDE=y; otherwise you'll get configs with IDE=m, BLK_DEV_IDE_CELLEB=y and those won't link. BLK_DEV_IDEDMA_PCI is selectable just fine with IDE=m.

It's the same problem as with ps3 fb.

Akira Iguchi

2007-03-15 06:08 UTC #4 lore raw

Quoted 7 lines · Al

Eh... You still need dependency on IDE=y; otherwise you'll get configs with IDE=m, BLK_DEV_IDE_CELLEB=y and those won't link. BLK_DEV_IDEDMA_PCI is selectable just fine with IDE=m.
It's the same problem as with ps3 fb.

I'm sorry I missed this case.
Using some configurations, I found BLK_DEV_IDE=y was better. (I failed to link when IDE=y and BLK_DEV_IDE=m.)

Best regards,
Akira Iguchi

Al Viro

author

2007-03-15 08:46 UTC #5 lore raw

Quoted 6 lines · Akira Iguchi

Quoted 5 lines · Al
Eh... You still need dependency on IDE=y; otherwise you'll get configs with IDE=m, BLK_DEV_IDE_CELLEB=y and those won't link. BLK_DEV_IDEDMA_PCI is selectable just fine with IDE=m.

Quoted 6 lines

Quoted 2 lines
It's the same problem as with ps3 fb.
I'm sorry I missed this case.
Using some configurations, I found BLK_DEV_IDE=y was better. (I failed to link when IDE=y and BLK_DEV_IDE=m.)

Umm... Point taken. After looking at the entire thing... well.

a) BLK_DEV_IDE_PMAC has bogus dependency on IDE=y. If anything, that should've been BLK_DEV_IDE=y; however, it *does* build with ide modular - it doesn't generate a separate module and its initialization is called explicitly from ide one. AFAICS, we can simply drop that dependency.

b) BLK_DEV_IDE_PMAC does *not* build without BLK_DEV_IDEDMA_PMAC. Quoting benh, "I don't see any reason to keep that dma thingy optional anyway".

c) BLK_DEV_MPC8xx_IDE depends on BLK_DEV_IDE=y *and* IDE=y. The latter is obviously redundant. The former... No idea, 8xx is currently b0rken in ARCH=powerpc and I can't be arsed to wade through arch/ppc bitrot. As it is, driver definitely wants ARCH=ppc stuff (__res, for one thing).

Now, BLK_DEV_IDE_CELLEB looks interesting. The nature of breakage is not the same as usual (non-modular driver depends on stuff that might be built modular); what's going on here is funnier. If you get ide-core modular, you'll have BLK_DEV_IDE_CELLEB code *linked* *into* ide-core.ko. Unlike the rest of its ilk, however, it doesn't have its init called directly from ide init. It uses module_init(), which happens to work when it goes into the kernel image (ide-core.o has several initcalls, not a problem), but breaks when it goes into a modular ide-core.ko; there multiple module_init() are fatal.

So AFAICS the minimal fix for that sucker is dependency on BLK_DEV_IDE=y; however, I really wonder if
* it needs to be linked into ide-core (as opposed to being a normal
module of its own)
* alternatively, its init should be called explicitly.

Comments?

Bartlomiej Zolnierkiewicz

2007-03-15 19:41 UTC #6 lore raw

Quoted 14 lines · Al Viro

Quoted 6 lines · Akira Iguchi
Quoted 5 lines · Al
Eh... You still need dependency on IDE=y; otherwise you'll get configs with IDE=m, BLK_DEV_IDE_CELLEB=y and those won't link. BLK_DEV_IDEDMA_PCI is selectable just fine with IDE=m.
Quoted 6 lines
Quoted 2 lines
It's the same problem as with ps3 fb.
I'm sorry I missed this case.
Using some configurations, I found BLK_DEV_IDE=y was better. (I failed to link when IDE=y and BLK_DEV_IDE=m.)

Quoted 6 lines

Umm... Point taken. After looking at the entire thing... well.
a) BLK_DEV_IDE_PMAC has bogus dependency on IDE=y. If anything, that should've been BLK_DEV_IDE=y; however, it *does* build with ide modular - it doesn't generate a separate module and its initialization is called explicitly from ide one. AFAICS, we can simply drop that dependency.

agreed

Quoted 2 lines

b) BLK_DEV_IDE_PMAC does *not* build without BLK_DEV_IDEDMA_PMAC. Quoting benh, "I don't see any reason to keep that dma thingy optional anyway".

ditto

Quoted 2 lines

c) BLK_DEV_MPC8xx_IDE depends on BLK_DEV_IDE=y *and* IDE=y. The latter is obviously redundant. The former... No idea, 8xx is currently b0rken in

ditto

Quoted 13 lines

ARCH=powerpc and I can't be arsed to wade through arch/ppc bitrot. As it is, driver definitely wants ARCH=ppc stuff (__res, for one thing).
Now, BLK_DEV_IDE_CELLEB looks interesting. The nature of breakage is not the same as usual (non-modular driver depends on stuff that might be built modular); what's going on here is funnier. If you get ide-core modular, you'll have BLK_DEV_IDE_CELLEB code *linked* *into* ide-core.ko. Unlike the rest of its ilk, however, it doesn't have its init called directly from ide init. It uses module_init(), which happens to work when it goes into the kernel image (ide-core.o has several initcalls, not a problem), but breaks when it goes into a modular ide-core.ko; there multiple module_init() are fatal.

Quoted 4 lines

So AFAICS the minimal fix for that sucker is dependency on BLK_DEV_IDE=y; however, I really wonder if
* it needs to be linked into ide-core (as opposed to being a normal
module of its own)

AFAICS there are no legacy device ordering issues with scc_pata so it doesn't need to be linked into ide-core but I'll leave the definitive answer to Akira

Quoted 3 lines

* alternatively, its init should be called explicitly.
Comments?

Care to provide patch for non-scc_pata issues that you've discovered? :-)

Thanks,
Bart

Akira Iguchi

2007-03-16 01:30 UTC #7 lore raw

Hi,

Quoted 11 lines

Quoted 5 lines · Bart
Al wrote:
So AFAICS the minimal fix for that sucker is dependency on BLK_DEV_IDE=y; however, I really wonder if
* it needs to be linked into ide-core (as opposed to being a normal
module of its own)
AFAICS there are no legacy device ordering issues with scc_pata so it doesn't need to be linked into ide-core but I'll leave the definitive answer to Akira
Quoted 1 line
* alternatively, its init should be called explicitly.

I don't have the answer why scc_pata is linked into ide-core. Reviewing your comments and codes, I will make the following fixes:
* remove link to ide-core and make normal module
* move from ide/ppc to ide/pci

I will send these patches later.

Best regards,
Akira Iguchi

Akira Iguchi

2007-03-16 01:30 UTC #8 lore raw

In reply to Bartlomiej Zolnierkiewicz

Hi,

Quoted 11 lines

Quoted 5 lines · Bart
Al wrote:
So AFAICS the minimal fix for that sucker is dependency on BLK_DEV_IDE=y; however, I really wonder if
* it needs to be linked into ide-core (as opposed to being a normal
module of its own)
AFAICS there are no legacy device ordering issues with scc_pata so it doesn't need to be linked into ide-core but I'll leave the definitive answer to Akira
Quoted 1 line
* alternatively, its init should be called explicitly.

I will send these patches later.

Best regards,
Akira Iguchi

Akira Iguchi

2007-03-15 06:08 UTC #9 lore raw

In reply to Al Viro

Quoted 7 lines · Al

Eh... You still need dependency on IDE=y; otherwise you'll get configs with IDE=m, BLK_DEV_IDE_CELLEB=y and those won't link. BLK_DEV_IDEDMA_PCI is selectable just fine with IDE=m.
It's the same problem as with ps3 fb.

I'm sorry I missed this case.
Using some configurations, I found BLK_DEV_IDE=y was better. (I failed to link when IDE=y and BLK_DEV_IDE=m.)

Best regards,
Akira Iguchi

Akira Iguchi

2007-03-15 05:53 UTC #10 lore raw

In reply to Al Viro

Quoted 4 lines · Al

It's bool and it depends on IDE => should depend on IDE=y
Signed-off-by Al Viro <[email protected]>

Move to "if BLK_DEV_IDEDMA_PCI" block because it depends on BLK_DEV_IDEDMA_PCI.

Signed-off-by Kou Ishizaki <[email protected]>

Signed-off-by Akira Iguchi <[email protected]>

---

drivers/ide/Kconfig +8 -8

diff -Nrpu -X linux-2.6.21-rc3/Documentation/dontdiff linux-2.6.21-rc3/drivers/ide/Kconfig linux-2.6.21-rc3.mod/drivers/ide/Kconfig
@@ -769,6 +769,14 @@ config BLK_DEV_TC86C001
 	help
 	This driver adds support for Toshiba TC86C001 GOKU-S chip.
 
+config BLK_DEV_IDE_CELLEB
+	bool "Toshiba's Cell Reference Set IDE support"
+	depends on PPC_CELLEB
+	help
+	  This driver provides support for the built-in IDE controller on
+	  Toshiba Cell Reference Board.
+	  If unsure, say Y.
+
 endif
 
 config BLK_DEV_IDE_PMAC

@@ -800,14 +808,6 @@ config BLK_DEV_IDEDMA_PMAC
 	  to transfer data to and from memory.  Saying Y is safe and improves
 	  performance.
 
-config BLK_DEV_IDE_CELLEB
-	bool "Toshiba's Cell Reference Set IDE support"
-	depends on PPC_CELLEB
-	help
-	  This driver provides support for the built-in IDE controller on
-	  Toshiba Cell Reference Board.
-	  If unsure, say Y.
-
 config BLK_DEV_IDE_SWARM
 	tristate "IDE for Sibyte evaluation boards"
 	depends on SIBYTE_SB1xxx_SOC