[2.6 patch] select FW_LOADER -> depends HOTPLUG

[2.6 patch] select FW_LOADER -> depends HOTPLUG

[Adrian Bunk]

The contract is easy:
If you select something, you have to ensure the depenencies of the 
selected option are met.

This is simple.

And most people get it wrong.

The patch below adds dependencies on HOTPLUG for all options that select 
FW_LOADER.


diffstat output:
 drivers/bluetooth/Kconfig           |    4 ++--
 drivers/media/dvb/ttpci/Kconfig     |    2 +-
 drivers/media/dvb/ttusb-dec/Kconfig |    2 +-
 drivers/net/wireless/Kconfig        |    5 ++---
 drivers/scsi/Kconfig                |    2 +-
 5 files changed, 7 insertions(+), 8 deletions(-)


Signed-off-by: Adrian Bunk <bunk@fs.tum.de>

--- linux-2.6.8-rc3-mm2-full-3.4/drivers/bluetooth/Kconfig.old	2004-08-09 21:48:55.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/bluetooth/Kconfig	2004-08-09 21:49:18.000000000 +0200
@@ -65,7 +65,7 @@
 
 config BT_HCIBCM203X
 	tristate "HCI BCM203x USB driver"
-	depends on USB
+	depends on USB && HOTPLUG
 	select FW_LOADER
 	help
 	  Bluetooth HCI BCM203x USB driver.
@@ -77,7 +77,7 @@
 
 config BT_HCIBFUSB
 	tristate "HCI BlueFRITZ! USB driver"
-	depends on USB
+	depends on USB && HOTPLUG
 	select FW_LOADER
 	help
 	  Bluetooth HCI BlueFRITZ! USB driver.
--- linux-2.6.8-rc3-mm2-full-3.4/drivers/scsi/Kconfig.old	2004-08-09 21:48:20.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/scsi/Kconfig	2004-08-09 21:48:32.000000000 +0200
@@ -1019,7 +1019,7 @@
 
 config SCSI_IPR
 	tristate "IBM Power Linux RAID adapter support"
-	depends on PCI && SCSI && PPC
+	depends on PCI && SCSI && PPC && HOTPLUG
 	select FW_LOADER
 	---help---
 	  This driver supports the IBM Power Linux family RAID adapters.
--- linux-2.6.8-rc3-mm2-full-3.4/drivers/media/dvb/ttusb-dec/Kconfig.old	2004-08-09 21:47:21.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/media/dvb/ttusb-dec/Kconfig	2004-08-09 21:47:52.000000000 +0200
@@ -1,6 +1,6 @@
 config DVB_TTUSB_DEC
 	tristate "Technotrend/Hauppauge USB DEC devices"
-	depends on DVB_CORE && USB
+	depends on DVB_CORE && USB && HOTPLUG
 	select FW_LOADER
 	select CRC32
 	help
--- linux-2.6.8-rc3-mm2-full-3.4/drivers/media/dvb/ttpci/Kconfig.old	2004-08-09 21:45:32.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/media/dvb/ttpci/Kconfig	2004-08-09 21:46:15.000000000 +0200
@@ -1,6 +1,6 @@
 config DVB_AV7110
 	tristate "AV7110 cards"
-	depends on DVB_CORE
+	depends on DVB_CORE && HOTPLUG
 	select FW_LOADER
 	select VIDEO_DEV
 	select VIDEO_SAA7146_VV
--- linux-2.6.8-rc3-mm2-full-3.4/drivers/net/wireless/Kconfig.old	2004-08-09 21:43:19.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/net/wireless/Kconfig	2004-08-09 21:44:10.000000000 +0200
@@ -223,7 +223,7 @@
 
 config ATMEL
       tristate "Atmel at76c50x chipset  802.11b support"
-      depends on NET_RADIO && EXPERIMENTAL
+      depends on NET_RADIO && HOTPLUG && EXPERIMENTAL
       select FW_LOADER
       select CRC32
        ---help---
@@ -293,8 +293,6 @@
 config PCMCIA_ATMEL
 	tristate "Atmel at76c502/at76c504 PCMCIA cards"
 	depends on NET_RADIO && ATMEL && PCMCIA
-	select FW_LOADER
-	select CRC32
 	---help---
 	  Enable support for PCMCIA cards containing the
 	  Atmel at76c502 and at76c504 chips.
@@ -309,6 +307,7 @@
 
 comment "Prism GT/Duette 802.11(a/b/g) PCI/Cardbus support"
 	depends on NET_RADIO && PCI
+
 config PRISM54
 	tristate 'Intersil Prism GT/Duette/Indigo PCI/Cardbus' 
 	depends on PCI && NET_RADIO && EXPERIMENTAL && HOTPLUG

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Adrian Bunk]

On Mon, Aug 09, 2004 at 09:56:56PM +0200, Adrian Bunk wrote:
> 
> The contract is easy:
> If you select something, you have to ensure the depenencies of the 
> selected option are met.
> 
> This is simple.
> 
> And most people get it wrong.
> 
> The patch below adds dependencies on HOTPLUG for all options that select 
> FW_LOADER.
>...

I missed one more option (an option that selects FW_LOADER was itself 
selected...).

Updated patch below.

diffstat output:
 drivers/bluetooth/Kconfig           |    4 ++--
 drivers/media/dvb/ttpci/Kconfig     |    4 ++--
 drivers/media/dvb/ttusb-dec/Kconfig |    2 +-
 drivers/net/wireless/Kconfig        |    5 ++---
 drivers/scsi/Kconfig                |    2 +-
 5 files changed, 8 insertions(+), 9 deletions(-)


Signed-off-by: Adrian Bunk <bunk@fs.tum.de>

--- linux-2.6.8-rc3-mm2-full-3.4/drivers/bluetooth/Kconfig.old	2004-08-09 21:48:55.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/bluetooth/Kconfig	2004-08-09 21:49:18.000000000 +0200
@@ -65,7 +65,7 @@
 
 config BT_HCIBCM203X
 	tristate "HCI BCM203x USB driver"
-	depends on USB
+	depends on USB && HOTPLUG
 	select FW_LOADER
 	help
 	  Bluetooth HCI BCM203x USB driver.
@@ -77,7 +77,7 @@
 
 config BT_HCIBFUSB
 	tristate "HCI BlueFRITZ! USB driver"
-	depends on USB
+	depends on USB && HOTPLUG
 	select FW_LOADER
 	help
 	  Bluetooth HCI BlueFRITZ! USB driver.
--- linux-2.6.8-rc3-mm2-full-3.4/drivers/scsi/Kconfig.old	2004-08-09 21:48:20.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/scsi/Kconfig	2004-08-09 21:48:32.000000000 +0200
@@ -1019,7 +1019,7 @@
 
 config SCSI_IPR
 	tristate "IBM Power Linux RAID adapter support"
-	depends on PCI && SCSI && PPC
+	depends on PCI && SCSI && PPC && HOTPLUG
 	select FW_LOADER
 	---help---
 	  This driver supports the IBM Power Linux family RAID adapters.
--- linux-2.6.8-rc3-mm2-full-3.4/drivers/media/dvb/ttusb-dec/Kconfig.old	2004-08-09 21:47:21.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/media/dvb/ttusb-dec/Kconfig	2004-08-09 21:47:52.000000000 +0200
@@ -1,6 +1,6 @@
 config DVB_TTUSB_DEC
 	tristate "Technotrend/Hauppauge USB DEC devices"
-	depends on DVB_CORE && USB
+	depends on DVB_CORE && USB && HOTPLUG
 	select FW_LOADER
 	select CRC32
 	help
--- linux-2.6.8-rc3-mm2-full-3.4/drivers/net/wireless/Kconfig.old	2004-08-09 21:43:19.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/net/wireless/Kconfig	2004-08-09 21:44:10.000000000 +0200
@@ -223,7 +223,7 @@
 
 config ATMEL
       tristate "Atmel at76c50x chipset  802.11b support"
-      depends on NET_RADIO && EXPERIMENTAL
+      depends on NET_RADIO && HOTPLUG && EXPERIMENTAL
       select FW_LOADER
       select CRC32
        ---help---
@@ -293,8 +293,6 @@
 config PCMCIA_ATMEL
 	tristate "Atmel at76c502/at76c504 PCMCIA cards"
 	depends on NET_RADIO && ATMEL && PCMCIA
-	select FW_LOADER
-	select CRC32
 	---help---
 	  Enable support for PCMCIA cards containing the
 	  Atmel at76c502 and at76c504 chips.
@@ -309,6 +307,7 @@
 
 comment "Prism GT/Duette 802.11(a/b/g) PCI/Cardbus support"
 	depends on NET_RADIO && PCI
+
 config PRISM54
 	tristate 'Intersil Prism GT/Duette/Indigo PCI/Cardbus' 
 	depends on PCI && NET_RADIO && EXPERIMENTAL && HOTPLUG
--- linux-2.6.8-rc3-mm2-full-3.4/drivers/media/dvb/ttpci/Kconfig.old	2004-08-09 21:45:32.000000000 +0200
+++ linux-2.6.8-rc3-mm2-full-3.4/drivers/media/dvb/ttpci/Kconfig	2004-08-09 22:00:32.000000000 +0200
@@ -1,6 +1,6 @@
 config DVB_AV7110
 	tristate "AV7110 cards"
-	depends on DVB_CORE
+	depends on DVB_CORE && HOTPLUG
 	select FW_LOADER
 	select VIDEO_DEV
 	select VIDEO_SAA7146_VV
@@ -91,7 +91,7 @@
 
 config DVB_BUDGET_PATCH
 	tristate "AV7110 cards with Budget Patch"
-	depends on DVB_BUDGET
+	depends on DVB_BUDGET && HOTPLUG
 	select DVB_AV7110
 	help
 	  Support for Budget Patch (full TS) modification on 

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Marcelo Tosatti]

On Mon, Aug 09, 2004 at 09:56:56PM +0200, Adrian Bunk wrote:
> 
> The contract is easy:
> If you select something, you have to ensure the depenencies of the 
> selected option are met.
> 
> This is simple.
> 
> And most people get it wrong.
> 
> The patch below adds dependencies on HOTPLUG for all options that select 
> FW_LOADER.

BTW last time I tried to turn CONFIG_FW_LOADER off on 2.6.8-rc3 I was not able
to do so... Is that expected?

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Adrian Bunk]

On Mon, Aug 09, 2004 at 05:26:03PM -0300, Marcelo Tosatti wrote:
> 
> BTW last time I tried to turn CONFIG_FW_LOADER off on 2.6.8-rc3 I was not able
> to do so... Is that expected?

Yes:
  cd linux-2.6.7
  find . -name Kconfig\* | xargs grep select | grep FW_LOADER

AFAIK there's currently no way implemented to easyly disable an option 
that is selected by other options.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Sam Ravnborg]

On Mon, Aug 09, 2004 at 09:56:56PM +0200, Adrian Bunk wrote:
> 
> The contract is easy:
> If you select something, you have to ensure the depenencies of the 
> selected option are met.
> 
> This is simple.
> 
> And most people get it wrong.

No - kconfig gets it wrong.
When selecting a config option kconfig shall secure that
'depends on' are evaluated also for the selected symbol.

Otherwise we will end up in a mintenace nightmare. Just think of how many
places to fix if we add and additional depends on to FW_LOADER.

Roman Zippel added in to:.

	Sam

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Roman Zippel]

Hi,

On Mon, 9 Aug 2004, Sam Ravnborg wrote:

> No - kconfig gets it wrong.
> When selecting a config option kconfig shall secure that
> 'depends on' are evaluated also for the selected symbol.

Which dependencies? select was more intended to select symbols without a 
prompt (it's dependency would be simply 'n'). The selected symbol can also 
have multiple prompts, how should these dependencies be merged?
The current select is intentionally simple, so the calculation is 
straightforward. Anything more complex I have to completely rethink the 
behaviour between depencies and selects, e.g. something like this:

	A ---selects----> C ---selects----> D
	B --depends on-->   --depends on--> E

If you want to change the behaviour of select how will it change the 
behaviour of the other dependencies and selects?
I have some ideas for that, but I didn't had the time yet to sit down and 
completely think it through and a few other changes are more important.

bye, Roman

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Adrian Bunk]

On Tue, Aug 10, 2004 at 02:24:47AM +0200, Roman Zippel wrote:
> Hi,
> 
> On Mon, 9 Aug 2004, Sam Ravnborg wrote:
> 
> > No - kconfig gets it wrong.
> > When selecting a config option kconfig shall secure that
> > 'depends on' are evaluated also for the selected symbol.
> 
> Which dependencies? select was more intended to select symbols without a 
> prompt (it's dependency would be simply 'n'). The selected symbol can also 
> have multiple prompts, how should these dependencies be merged?
> The current select is intentionally simple, so the calculation is 
> straightforward. Anything more complex I have to completely rethink the 
> behaviour between depencies and selects, e.g. something like this:
> 
> 	A ---selects----> C ---selects----> D
> 	B --depends on-->   --depends on--> E
> 
> If you want to change the behaviour of select how will it change the 
> behaviour of the other dependencies and selects?
>...

The current usage of select in the kernel covers many select's of 
symbols that actually have a prompt.

I assume Sam thinks in the direction to let a symbol inherit the 
dependencies off all symbols it selects.

E.g. in

config A
	depends on B

config C
	select A


C should be treated as if it would depend on B.


> bye, Roman

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Sam Ravnborg]

On Tue, Aug 10, 2004 at 10:44:11AM +0200, Adrian Bunk wrote:
> 
> I assume Sam thinks in the direction to let a symbol inherit the 
> dependencies off all symbols it selects.
> 
> E.g. in
> 
> config A
> 	depends on B
> 
> config C
> 	select A
        depends on Z

  config Z
        depends on Y
> 
> 
> C should be treated as if it would depend on B.

Correct. But at the same time I miss some functionality to
tell me what a given symbol:
1) depends on
2) selects

It would be nice in menuconfig to see what config symbol
that has dependencies and/or side effects. 

[*] PCI support
Could look like:
[*]d PCI support
The space for the 'd' tag could be misused for other purposes later.


The pressing 'd' would give me the following output:

C "The C prompt"
-> depends on
   CONFIG_Z "Prompt for Z"
   -> depends on
      CONFIG_Y "Prompt for Y"
-> Selects
   CONFIG_A "Promtp for A"
   -> depends on
      CONFIG_B "Prompt for B"

Or something similar.
The idea is to give user an idea of dependencies (both ways) - recursive.
Using both CONFIG symbol and prompt will give the user an idea where to
locate it.

Something like this is on my wish list.

	Sam

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Marcelo Tosatti]

On Tue, Aug 10, 2004 at 11:16:57PM +0200, Sam Ravnborg wrote:
> On Tue, Aug 10, 2004 at 10:44:11AM +0200, Adrian Bunk wrote:
> > 
> > I assume Sam thinks in the direction to let a symbol inherit the 
> > dependencies off all symbols it selects.
> > 
> > E.g. in
> > 
> > config A
> > 	depends on B
> > 
> > config C
> > 	select A
>         depends on Z
> 
>   config Z
>         depends on Y
> > 
> > 
> > C should be treated as if it would depend on B.
> 
> Correct. But at the same time I miss some functionality to
> tell me what a given symbol:
> 1) depends on
> 2) selects
> 
> It would be nice in menuconfig to see what config symbol
> that has dependencies and/or side effects. 

Definately agreed. Not all users want/can go and grep the source 
tree looking for dependencies of a given 
symbol.

Showing the dependencies on menuconfig/xconfig is a user friendly 
feature. 

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Randy.Dunlap]

On Wed, 11 Aug 2004 08:15:25 -0300 Marcelo Tosatti wrote:

| On Tue, Aug 10, 2004 at 11:16:57PM +0200, Sam Ravnborg wrote:
| > On Tue, Aug 10, 2004 at 10:44:11AM +0200, Adrian Bunk wrote:
| > > 
| > > I assume Sam thinks in the direction to let a symbol inherit the 
| > > dependencies off all symbols it selects.
| > > 
| > > E.g. in
| > > 
| > > config A
| > > 	depends on B
| > > 
| > > config C
| > > 	select A
| >         depends on Z
| > 
| >   config Z
| >         depends on Y
| > > 
| > > 
| > > C should be treated as if it would depend on B.
| > 
| > Correct. But at the same time I miss some functionality to
| > tell me what a given symbol:
| > 1) depends on
| > 2) selects
| > 
| > It would be nice in menuconfig to see what config symbol
| > that has dependencies and/or side effects. 
| 
| Definately agreed. Not all users want/can go and grep the source 
| tree looking for dependencies of a given 
| symbol.
| 
| Showing the dependencies on menuconfig/xconfig is a user friendly 
| feature. 

xconfig already has a way to do this.  Set all Options (5 of them,
although fewer would probably work).  menuconfig doesn't do this.

What would be even more helpful would be a way to tell *config to
jump from <current location, where a symbol is used> to
<where a symbol is defined>.

--
~Randy

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Roman Zippel]

On Tue, 10 Aug 2004, Sam Ravnborg wrote:

> On Tue, Aug 10, 2004 at 10:44:11AM +0200, Adrian Bunk wrote:
> > 
> > I assume Sam thinks in the direction to let a symbol inherit the 
> > dependencies off all symbols it selects.
> > 
> > E.g. in
> > 
> > config A
> > 	depends on B
> > 
> > config C
> > 	select A
>         depends on Z
> 
>   config Z
>         depends on Y
> > 
> > 
> > C should be treated as if it would depend on B.

There are two problems:
1) If A has no prompt it's not visible and so it's dependency is 'n', this 
means a number of symbols wouldn't be visible anymore.
2) It would change the bahaviour of symbols, which already do multiple 
selects (e.g. CONFIG_INET_AH), the select of CRYPTO would be useless, as 
it would only become visible, when CRYPTO is enabled. This means such 
selects wouldn't be possible anymore.

This really needs a different (but similiar) mechanism, what I have in 
mind is something like this:

config A
	autoselect

config B
	depends on A

For the visibility calculation A is set to y and A is automatically 
selected if any symbol, which depends on A, is enabled.

> Correct. But at the same time I miss some functionality to
> tell me what a given symbol:
> 1) depends on
> 2) selects
> 
> It would be nice in menuconfig to see what config symbol
> that has dependencies and/or side effects. 

xconfig has something like this, if you enable 'Debug Info', although it 
rather dumps the internal representation.
Adding something like this to menuconfig, would mean hacking lxdialog, 
which is rather at the bottom of the list of things I want to do. :)

bye, Roman

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Adrian Bunk]

On Thu, Aug 12, 2004 at 01:05:47AM +0200, Roman Zippel wrote:
> 
> 
> On Tue, 10 Aug 2004, Sam Ravnborg wrote:
> 
> > On Tue, Aug 10, 2004 at 10:44:11AM +0200, Adrian Bunk wrote:
> > > 
> > > I assume Sam thinks in the direction to let a symbol inherit the 
> > > dependencies off all symbols it selects.
> > > 
> > > E.g. in
> > > 
> > > config A
> > > 	depends on B
> > > 
> > > config C
> > > 	select A
> >         depends on Z
> > 
> >   config Z
> >         depends on Y
> > > 
> > > 
> > > C should be treated as if it would depend on B.
> 
> There are two problems:
> 1) If A has no prompt it's not visible and so it's dependency is 'n', this 
> means a number of symbols wouldn't be visible anymore.
> 2) It would change the bahaviour of symbols, which already do multiple 
> selects (e.g. CONFIG_INET_AH), the select of CRYPTO would be useless, as 
> it would only become visible, when CRYPTO is enabled. This means such 
> selects wouldn't be possible anymore.
> 
> This really needs a different (but similiar) mechanism, what I have in 
> mind is something like this:
> 
> config A
> 	autoselect
> 
> config B
> 	depends on A
> 
> For the visibility calculation A is set to y and A is automatically 
> selected if any symbol, which depends on A, is enabled.
>...

How do you want to handle the following?

<--  snip  -->

config FW_LOADER
      tristate "Hotplug firmware loading support"
      depends on HOTPLUG

config ATMEL
      tristate "Atmel at76c50x chipset  802.11b support"
      depends on NET_RADIO && EXPERIMENTAL
      select FW_LOADER
      select CRC32

<--  snip  -->


We've been biten that often by cases like a "select I2C_ALGOBIT" without 
a dependency on or a select of I2C and such cases are real issues that 
need a proper handling.


> bye, Roman

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] select FW_LOADER -> depends HOTPLUG

[Roman Zippel]

Hi,

On Thu, 12 Aug 2004, Adrian Bunk wrote:

> <--  snip  -->
> 
> config FW_LOADER
>       tristate "Hotplug firmware loading support"
>       depends on HOTPLUG
> 
> config ATMEL
>       tristate "Atmel at76c50x chipset  802.11b support"
>       depends on NET_RADIO && EXPERIMENTAL
>       select FW_LOADER
>       select CRC32
> 
> <--  snip  -->

The basic idea is to mark FW_LOADER as automatically selected and ATMEL 
would simply depend on it and as soon as ATMEL is selected FW_LOADER is 
selected. It'll work similiar to select, but it will respect dependencies 
(since it uses normal depencencies), it will also be easier to force 
deselection of FW_LOADER by (temporarily) disabling autoselect for it.

bye, Roman

menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Sam Ravnborg]

On Thu, Aug 12, 2004 at 01:05:47AM +0200, Roman Zippel wrote:
 
> > It would be nice in menuconfig to see what config symbol
> > that has dependencies and/or side effects. 
> 
> xconfig has something like this, if you enable 'Debug Info', although it 
> rather dumps the internal representation.
> Adding something like this to menuconfig, would mean hacking lxdialog, 
> which is rather at the bottom of the list of things I want to do. :)

Did a quick hack on this.
When choosing help on "HCI BlueFRITZ! USB driver" menuconfig now displays:

-------------------------------------------------
Depends on (select to enable this option):
BT & USB
Selects (will be enabled by this option): 
FW_LOADER

CONFIG_BT_HCIBFUSB

Bluetooth HCI BlueFRITZ! USB driver.
....
-------------------------------------------------

To simplify things I just malloc'ed 'enough' memory for the help screen.
Just a quick hack, but something to play with - and no lxdialog hacking :-)

	Sam

===== scripts/kconfig/expr.c 1.5 vs edited =====
--- 1.5/scripts/kconfig/expr.c	2004-03-19 07:04:54 +01:00
+++ edited/scripts/kconfig/expr.c	2004-08-14 09:20:04 +02:00
@@ -1087,3 +1087,42 @@
 {
 	expr_print(e, expr_print_file_helper, out, E_NONE);
 }
+
+void expr_get_dep_txt(struct expr *e, char *t)
+{
+	if (!e)
+		return;
+
+	switch (e->type) {
+	case E_SYMBOL:
+		strcat(t, e->left.sym->name);
+		break;
+	case E_AND:
+		expr_get_dep_txt(e->left.expr, t);
+		strcat(t, " & ");
+		expr_get_dep_txt(e->right.expr, t);
+		break;
+	case E_OR:
+		expr_get_dep_txt(e->left.expr, t);
+		strcat(t, " | ");
+		expr_get_dep_txt(e->right.expr, t);
+		break;
+	case E_NOT:
+		strcat(t, " !");
+		expr_get_dep_txt(e->left.expr, t);
+		break;
+	case E_EQUAL:
+		expr_get_dep_txt(e->left.expr, t);
+		strcat(t, " == ");
+		expr_get_dep_txt(e->right.expr, t);
+		break;
+	case E_UNEQUAL:
+		expr_get_dep_txt(e->left.expr, t);
+		strcat(t, " != ");
+		expr_get_dep_txt(e->right.expr, t);
+		break;
+	default:
+		printf("expr_calc_value: %d?\n", e->type);
+		break;
+	}
+}
===== scripts/kconfig/expr.h 1.11 vs edited =====
--- 1.11/scripts/kconfig/expr.h	2004-03-19 07:04:54 +01:00
+++ edited/scripts/kconfig/expr.h	2004-08-14 09:18:15 +02:00
@@ -174,6 +174,7 @@
 struct expr *expr_trans_compare(struct expr *e, enum expr_type type, struct symbol *sym);
 
 void expr_fprint(struct expr *e, FILE *out);
+void expr_get_dep_txt(struct expr *e, char *t);
 
 static inline int expr_is_yes(struct expr *e)
 {
===== scripts/kconfig/lkc_proto.h 1.3 vs edited =====
--- 1.3/scripts/kconfig/lkc_proto.h	2003-05-28 22:46:41 +02:00
+++ edited/scripts/kconfig/lkc_proto.h	2004-08-14 09:01:55 +02:00
@@ -9,6 +9,8 @@
 
 P(menu_is_visible,bool,(struct menu *menu));
 P(menu_get_prompt,const char *,(struct menu *menu));
+P(menu_get_dep_txt,void,(struct menu *menu, char *t));
+P(menu_get_select_txt,void,(struct menu *menu, char *t));
 P(menu_get_root_menu,struct menu *,(struct menu *menu));
 P(menu_get_parent_menu,struct menu *,(struct menu *menu));
 
===== scripts/kconfig/mconf.c 1.12 vs edited =====
--- 1.12/scripts/kconfig/mconf.c	2004-07-14 17:47:52 +02:00
+++ edited/scripts/kconfig/mconf.c	2004-08-14 09:23:13 +02:00
@@ -572,19 +572,29 @@
 static void show_help(struct menu *menu)
 {
 	const char *help;
-	char *helptext;
+	char *helptext = malloc(20000); /* Enough.. */
 	struct symbol *sym = menu->sym;
 
+	*helptext = '\0';
+	strcat(helptext, "Depends on (select to enable this option):\n");
+	menu_get_dep_txt(menu, helptext);
+	strcat(helptext, "\nSelects (will be enabled by this option):\n");
+	menu_get_select_txt(menu, helptext);
+	
 	help = sym->help;
 	if (!help)
 		help = nohelp_text;
 	if (sym->name) {
-		helptext = malloc(strlen(sym->name) + strlen(help) + 16);
-		sprintf(helptext, "CONFIG_%s:\n\n%s", sym->name, help);
-		show_helptext(menu_get_prompt(menu), helptext);
-		free(helptext);
-	} else
-		show_helptext(menu_get_prompt(menu), help);
+		strcat(helptext, "\n\nCONFIG_");
+		strcat(helptext, sym->name);
+		strcat(helptext, "\n\n");
+		strcat(helptext, help);
+	} else {
+		strcat(helptext, "\n\n");
+		strcat(helptext, help);
+	}
+	show_helptext(menu_get_prompt(menu), helptext);
+	free(helptext);
 }
 
 static void show_readme(void)
===== scripts/kconfig/menu.c 1.12 vs edited =====
--- 1.12/scripts/kconfig/menu.c	2004-02-04 06:31:11 +01:00
+++ edited/scripts/kconfig/menu.c	2004-08-14 09:37:28 +02:00
@@ -372,6 +372,26 @@
 	return NULL;
 }
 
+void menu_get_dep_txt(struct menu *menu, char *t)
+{
+	if (menu->dep)
+		expr_get_dep_txt(menu->dep, t);
+	if (menu->sym && menu->sym->dep)
+		expr_get_dep_txt(menu->sym->dep, t);
+}
+
+void menu_get_select_txt(struct menu *menu, char *t)
+{
+	if (menu->sym)
+	{
+		struct property *prop;
+		for_all_properties(menu->sym, prop, P_SELECT)
+		{
+			expr_get_dep_txt(prop->expr, t);
+		}
+	}
+}
+
 struct menu *menu_get_root_menu(struct menu *menu)
 {
 	return &rootmenu;

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Randy.Dunlap]

On Sat, 14 Aug 2004 09:49:53 +0200 Sam Ravnborg wrote:

| On Thu, Aug 12, 2004 at 01:05:47AM +0200, Roman Zippel wrote:
|  
| > > It would be nice in menuconfig to see what config symbol
| > > that has dependencies and/or side effects. 
| > 
| > xconfig has something like this, if you enable 'Debug Info', although it 
| > rather dumps the internal representation.
| > Adding something like this to menuconfig, would mean hacking lxdialog, 
| > which is rather at the bottom of the list of things I want to do. :)
| 
| Did a quick hack on this.
| When choosing help on "HCI BlueFRITZ! USB driver" menuconfig now displays:
| 
| -------------------------------------------------
| Depends on (select to enable this option):
| BT & USB
| Selects (will be enabled by this option): 
| FW_LOADER
| 
| CONFIG_BT_HCIBFUSB
| 
| Bluetooth HCI BlueFRITZ! USB driver.
| ....
| -------------------------------------------------
| 
| To simplify things I just malloc'ed 'enough' memory for the help screen.
| Just a quick hack, but something to play with - and no lxdialog hacking :-)

Hi Sam,
I started on this yesterday also, but you are ahead of me.

Looking pretty good, but for menu items in the EMBEDDED menu,
it misses entries like this one:

config FUTEX
	bool "Enable futex support" if EMBEDDED

It doesn't list any Depends data, but it actually depends on EMBEDDED.

I'll look at it more later.  Got other stuff to do now.

--
~Randy

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Adrian Bunk]

On Sat, Aug 14, 2004 at 09:49:53AM +0200, Sam Ravnborg wrote:
> On Thu, Aug 12, 2004 at 01:05:47AM +0200, Roman Zippel wrote:
>  
> > > It would be nice in menuconfig to see what config symbol
> > > that has dependencies and/or side effects. 
> > 
> > xconfig has something like this, if you enable 'Debug Info', although it 
> > rather dumps the internal representation.
> > Adding something like this to menuconfig, would mean hacking lxdialog, 
> > which is rather at the bottom of the list of things I want to do. :)
> 
> Did a quick hack on this.
> When choosing help on "HCI BlueFRITZ! USB driver" menuconfig now displays:
> 
> -------------------------------------------------
> Depends on (select to enable this option):
> BT & USB
> Selects (will be enabled by this option): 
> FW_LOADER
>...
 
Unless I misunderstood Roman, FW_LOADER should be no longer selected.

But if the dependencies of BT_HCIBFUSB were
  depends on BT && USB && FW_LOADER
the dependency information was useless since the option itself isn't 
shown for FW_LOADER=n.

And even if the option was shown:
How should an average sysadmin configuring his own kernel know where on 
earth the FW_LOADER option is he has to enable?

> 	Sam
>...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Sam Ravnborg]

On Sat, Aug 14, 2004 at 11:05:24PM +0200, Adrian Bunk wrote:
> 
> And even if the option was shown:
> How should an average sysadmin configuring his own kernel know where on 
> earth the FW_LOADER option is he has to enable?

If I ever find time to look into it then my plan is to give
the user the possibility to see _all_ menus + symbols with a prompt.
And for all symbols with a promt and with dependency information
to let the user enable a pop up window giving the possibility
to select all "Depends on" symbols.



So consider:
config FOO
	depends on BAR

config BAR
	select BAZ

config BAZ


Then when pressing 'd' on the FOO symbol (which is marked
as invisible) the user will be prompted with:

CONFIG_FOO

Depends on:
CONFIG_BAR: [ ] "Prompt for BAR"

...

	Sam

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Adrian Bunk]

On Sun, Aug 15, 2004 at 12:37:49AM +0200, Sam Ravnborg wrote:
> 
> If I ever find time to look into it then my plan is to give
> the user the possibility to see _all_ menus + symbols with a prompt.
> 
> And for all symbols with a promt and with dependency information
> to let the user enable a pop up window giving the possibility
> to select all "Depends on" symbols.
> 
> 
> 
> So consider:
> config FOO
> 	depends on BAR
> 
> config BAR
> 	select BAZ
> 
> config BAZ
> 
> 
> Then when pressing 'd' on the FOO symbol (which is marked
> as invisible) the user will be prompted with:
> 
> CONFIG_FOO
> 
> Depends on:
> CONFIG_BAR: [ ] "Prompt for BAR"


Assuming I'm configuring a kernel for i386, what should I see when 
pressing 'd' on the following driver?


config OAKNET
        tristate "National DP83902AV (Oak ethernet) support"
        depends on NET_ETHERNET && PPC && BROKEN
        select CRC32



> ...
> 
> 	Sam

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Sam Ravnborg]

On Sun, Aug 15, 2004 at 10:21:11PM +0200, Adrian Bunk wrote:
> > CONFIG_FOO
> > 
> > Depends on:
> > CONFIG_BAR: [ ] "Prompt for BAR"
> 
> 
> Assuming I'm configuring a kernel for i386, what should I see when 
> pressing 'd' on the following driver?
> 
> 
> config OAKNET
>         tristate "National DP83902AV (Oak ethernet) support"
>         depends on NET_ETHERNET && PPC && BROKEN
>         select CRC32
> 
What about:
Depends on:
  [ ] "Prompt for NET_ETHERNET"	(CONFIG_NET_ETHERNET)
  --- "Promt ..." (CONFIG_PPC)
  --- " Prompt ... " (CONFIG_BROKEN)

Selects
  "Prompt for CRC32)" (CONFIG_CRC32)

That would make sense to me at least.
If I ever look into it I will touch upon the corner cases for sure.

	Sam

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Adrian Bunk]

On Sun, Aug 15, 2004 at 10:32:45PM +0200, Sam Ravnborg wrote:
> On Sun, Aug 15, 2004 at 10:21:11PM +0200, Adrian Bunk wrote:
> > > CONFIG_FOO
> > > 
> > > Depends on:
> > > CONFIG_BAR: [ ] "Prompt for BAR"
> > 
> > 
> > Assuming I'm configuring a kernel for i386, what should I see when 
> > pressing 'd' on the following driver?
> > 
> > 
> > config OAKNET
> >         tristate "National DP83902AV (Oak ethernet) support"
> >         depends on NET_ETHERNET && PPC && BROKEN
> >         select CRC32
> > 
> What about:
> Depends on:
>   [ ] "Prompt for NET_ETHERNET"	(CONFIG_NET_ETHERNET)
>   --- "Promt ..." (CONFIG_PPC)
>   --- " Prompt ... " (CONFIG_BROKEN)

CONFIG_BROKEN is available.

And in other cases, e.g. I2C_ALGOBIT might be a dependency, but depends 
itself on I2C which might be disabled.

> Selects
>   "Prompt for CRC32)" (CONFIG_CRC32)
> 
> That would make sense to me at least.

Showing all !i386 drivers on i386 will make it harder for people to find 
the drivers they actually need.

> If I ever look into it I will touch upon the corner cases for sure.

Thanks.

BTW:
"make oldconfig" will need a similar treatment before we can start 
changing selects to depends.

> 	Sam

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Roman Zippel]

Hi,

On Sat, 14 Aug 2004, Adrian Bunk wrote:

> Unless I misunderstood Roman, FW_LOADER should be no longer selected.

Use it if you really need to, but just don't use it as convenient 
replacement for depends.

bye, Roman

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Adrian Bunk]

On Sun, Aug 15, 2004 at 07:35:53PM +0200, Roman Zippel wrote:
> Hi,
> 
> On Sat, 14 Aug 2004, Adrian Bunk wrote:
> 
> > Unless I misunderstood Roman, FW_LOADER should be no longer selected.
> 
> Use it if you really need to, but just don't use it as convenient 
> replacement for depends.

And what's the correct handling of dependencies the selected symbol has?

FW_LOADER depends on HOTPLUG, and this was the issue that started the 
whole thread.

> bye, Roman

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Roman Zippel]

Hi,

On Sun, 15 Aug 2004, Adrian Bunk wrote:

> And what's the correct handling of dependencies the selected symbol has?
> 
> FW_LOADER depends on HOTPLUG, and this was the issue that started the 
> whole thread.

The use of select is already a crotch here, so there's no real correct 
handling. There are a few possibilities:
- if you select FW_LOADER, you have to select HOTPLUG too
- if you select FW_LOADER, you have to depend on HOTPLUG
- FW_LOADER itself can select HOTPLUG

bye, Roman

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Adrian Bunk]

On Mon, Aug 16, 2004 at 12:47:05AM +0200, Roman Zippel wrote:
> Hi,
> 
> On Sun, 15 Aug 2004, Adrian Bunk wrote:
> 
> > And what's the correct handling of dependencies the selected symbol has?
> > 
> > FW_LOADER depends on HOTPLUG, and this was the issue that started the 
> > whole thread.
> 
> The use of select is already a crotch here, so there's no real correct 
> handling. There are a few possibilities:
> - if you select FW_LOADER, you have to select HOTPLUG too
> - if you select FW_LOADER, you have to depend on HOTPLUG
> - FW_LOADER itself can select HOTPLUG

Solution 2 is what my patch tried.

Thinking about them, I'd prefer solution 3. But with solution 1 or 3, 
I'm sure people like Russell King will scream since this will make it 
non-trivial to de-select HOTPLUG.

> bye, Roman

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Russell King]

On Mon, Aug 16, 2004 at 09:57:33PM +0200, Adrian Bunk wrote:
> On Mon, Aug 16, 2004 at 12:47:05AM +0200, Roman Zippel wrote:
> > The use of select is already a crotch here, so there's no real correct 
> > handling. There are a few possibilities:
> > - if you select FW_LOADER, you have to select HOTPLUG too
> > - if you select FW_LOADER, you have to depend on HOTPLUG
> > - FW_LOADER itself can select HOTPLUG
> 
> Solution 2 is what my patch tried.
> 
> Thinking about them, I'd prefer solution 3. But with solution 1 or 3, 
> I'm sure people like Russell King will scream since this will make it 
> non-trivial to de-select HOTPLUG.

Let me make my position over the use of "select" clear: I do not
oppose its appropriate use, where that is defined as selecting
another configuration option for which the user has no visibility.

In the above case, it _may_ make sense (I haven't looked deeply
into it yet) to:

- make _all_ drivers which need FW_LOADER select it
- make _all_ drivers which currently depend on HOTPLUG select it
- make FW_LOADER select HOTPLUG
- remove user questions for FW_LOADER and HOTPLUG

That means that FW_LOADER and HOTPLUG are automatically selected
whenever the configuration requires them and are automatically
deselected when it doesn't need them, and you don't have to worry
about whether you can disable them now or after finding the
thousand and one configuration symbols which need to be turned off
first.

However, keeping the option user-visible _and_ using select is
problematical to say the least.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Adrian Bunk]

On Mon, Aug 16, 2004 at 09:07:30PM +0100, Russell King wrote:
> 
> Let me make my position over the use of "select" clear: I do not
> oppose its appropriate use, where that is defined as selecting
> another configuration option for which the user has no visibility.
> 
> In the above case, it _may_ make sense (I haven't looked deeply
> into it yet) to:
> 
> - make _all_ drivers which need FW_LOADER select it
> - make _all_ drivers which currently depend on HOTPLUG select it
> - make FW_LOADER select HOTPLUG
> - remove user questions for FW_LOADER and HOTPLUG
> 
> That means that FW_LOADER and HOTPLUG are automatically selected
> whenever the configuration requires them and are automatically
> deselected when it doesn't need them, and you don't have to worry
> about whether you can disable them now or after finding the
> thousand and one configuration symbols which need to be turned off
> first.
> 
> However, keeping the option user-visible _and_ using select is
> problematical to say the least.

Currently, I see no good alternative to e.g. USB_STORAGE selecting SCSI.
Enhanchements to _all_ kernel configuration tools are required before 
this can be changed.

But it still leaves the problem that modules not included in the kernel  
might require the functionality provided by such an option.
Even CRC32 is user-visible.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Roman Zippel]

Hi,

On Mon, 16 Aug 2004, Adrian Bunk wrote:

> But it still leaves the problem that modules not included in the kernel  
> might require the functionality provided by such an option.
> Even CRC32 is user-visible.

config CRC32
	tristate "CRC32 functions" if SHOW_ME_EVERYTHING
	default m

bye, Roman

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Roman Zippel]

Hi,

On Sat, 14 Aug 2004, Sam Ravnborg wrote:

> +void expr_get_dep_txt(struct expr *e, char *t)

What's wrong with expr_print()?

> +	if (menu->sym && menu->sym->dep)
> +		expr_get_dep_txt(menu->sym->dep, t);

The dep field of the symbol isn't used currently (it's left from the 
converter).
Did you already look at the similiar code in qconf.cc?

bye, Roman

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Sam Ravnborg]

On Sun, Aug 15, 2004 at 07:28:28PM +0200, Roman Zippel wrote:
> Hi,
> 
> On Sat, 14 Aug 2004, Sam Ravnborg wrote:
> 
> > +void expr_get_dep_txt(struct expr *e, char *t)
> 
> What's wrong with expr_print()?
Nothing ;-)
Originally I had in mind expanding string storage using realloc as required.
But my hack became a bit simpler than that, so I could have used it.

> 
> > +	if (menu->sym && menu->sym->dep)
> > +		expr_get_dep_txt(menu->sym->dep, t);
> 
> The dep field of the symbol isn't used currently (it's left from the 
> converter).
Should we delete it then?

> Did you already look at the similiar code in qconf.cc?
No.
If I revisit this then I will do so, and use expr_print().

	Sam

Re: menuconfig displays dependencies [Was: select FW_LOADER -> depends HOTPLUG]

[Roman Zippel]

Hi,

On Sun, 15 Aug 2004, Sam Ravnborg wrote:

> > What's wrong with expr_print()?
> Nothing ;-)
> Originally I had in mind expanding string storage using realloc as required.
> But my hack became a bit simpler than that, so I could have used it.

I already thought about changing the argument to expr_print into a 
structure to e.g. add another function to get more control about how to 
print a symbol, do realloc would be easier then as well.

bye, Roman