Re: [PATCH] b44-ssb: Fix the SSB dependency hell

[Adrian Bunk <bunk@stusta.de>]

On Sat, Aug 11, 2007 at 02:57:36AM +0200, Johannes Berg wrote:
> On Sat, 2007-08-11 at 02:43 +0200, Adrian Bunk wrote:
> 
> > -ENOMENUCONFIGPATCH
> 
> Has anybody decided how it could possibly even look like anyhow? It
> should be fixed, but nobody has a plan.

The simplest idea would be that an option select'ing some other option 
inherits the dependencies of the latter. That should fix all problems 
select currently has.

There are for sure some problems hidden that will show up during 
implementation, but since noone is implementing it we'll never know...

> > That's horrible - you shouldn't force the user to manually enable three 
> > options.
> 
> Well, akpm says: "select is broken. do not ever use it"

select works fine if you understand the pitfalls.

Kconfig is a _user interface_, and making using it easy for the user 
is therefore important.

Whenever there's a mistake in a help text or something confusing
users will run into problems with the kernel they compiled.

> > config SSB_PCIHOST_POSSIBLE
> [...]
> 
> > 	depends on SSB_PCIHOST_POSSIBLE
> > 	select SSB
> > 	select SSB_PCIHOST
> 
> That would, indeed, be possible. But it's ... ugly ... you've now
> effectively pushed the information on what *SSB* depends on into each
> SSB *user* instead of SSB itself...

No, this information is still in drivers/ssb/Kconfig.

This would replace:

config B44
	depends on SSB_PCIHOST

with:

config B44
	depends on SSB_PCIHOST_POSSIBLE
	select SSB_PCIHOST

That's an easily understandable pattern without redundant information
about the required dependencies of SSB_PCIHOST.

> > Is there any extremely good reason why options like SSB or SSB_PCIHOST 
> > have to be user visible? 
> 
> Yes. Embedded systems like the small Linksys routers come with SSB as
> the system bus. No PCI/PCIHOST.

OK.

> > And according to the kconfig help text, we should remove the B44_PCI 
> > option and enable the code unconditionally?
> > (Or what was the person writing this help text smoking^Wthinking when
> >  writing it?)
> 
> Same reason. They have a b44 core there but no pci.

OK, that's understandable.

But the kconfig user currently only gets:

config B44_PCI
        bool "Broadcom 440x PCI device support"
	...
	help
          Support for Broadcom 440x PCI devices.

          Say Y, unless you know what you are doing.
          If you say N here I will _not_ listen to your
          bugreports!

An example how to make it better:

config USB_OHCI_HCD_SSB
        bool "OHCI support for the Broadcom SSB OHCI core (embedded systems only)"
	...
        help
          Support for the Sonics Silicon Backplane (SSB) attached
          Broadcom USB OHCI core.

          This device is only present in some embedded devices with
          Broadcom based SSB bus.

          If unsure, say N.

That's the difference between a silly sounding help text ("I will _not_ 
listen to your bugreports!") and a help text that gives the kconfig user 
all required information ("only present in some embedded devices").


And I'm not yet convinced B44_PCI is really worth bothering the user 
with - what about automatically enabling PCI support in the driver if 
PCI support is enabled in the kernel?


> johannes

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