[PATCH v4 0/4] Simplifications on Kconfigs

[PATCH v4 0/4] Simplifications on Kconfigs

[Felipe Contreras]

Hi,

These have been discussed quite extensively before, but I never took the time
to split them:

http://article.gmane.org/gmane.linux.kernel/1050608
http://article.gmane.org/gmane.linux.usb.general/38230

Some are simple, and people OK'ed them, some others are a bit controversial.
However, something needs to be done.

At some point in time Linus complained about the huge amount of changes in the
ARM configs, which were huge at the time, and he even threated to remove them
all. The switch to simplified defconfigs (using 'make savedefconfig') greatly
alleviated the situation, however, the idea was that kconfig files should
continue to be improved until the defconfigs are so small, that they are not
needed at all any more. We are pretty far from that point.

So... the following patches remove the need to manually specify some
configurations in some platforms, like TWL4030_CORE, REGULATOR_TWL4030, and
TWL4030_USB. Note however that these are only *defaults*; the user can always
disable them for whatever reason, and they are not *dependencies*; the user can
still enable them in other architectures (like x86).

The final patch although it looks like it's adding a dependency, it's not; the
dependency is already there, except in the form selects. Well, almost, because
right now you can build USB_MUSB_HDRC if TWL4030_USB=n and NOP_USB_XCEIV=n in
some architectures (like MACH_OMAP_RX51), and that's actually bad, because that
has no possibility of working. Fortunately the dependencies can be built in x86
(or any other architecture), so USB_MUSB_HDRC can be built as well. IOW; this
is just shuffling the code so that at least OMAP2+ machines have sensible
defaults when USB_MUSB_HDRC is on.

One of the results is that this simple defconfig works (or at least selects the
right USB stuff):

---
CONFIG_ARCH_OMAP=y
CONFIG_ARCH_OMAP2PLUS_TYPICAL=n

CONFIG_USB=y
CONFIG_USB_GADGET=y
CONFIG_USB_MUSB_OTG=y
CONFIG_USB_MUSB_HDRC=y
CONFIG_USB_ETH=y
CONFIG_USB_ETH_EEM=y

# Should be selected automatically
# CONFIG_TWL4030_USB=y

# TODO USB_MUSB_OTG should select this
CONFIG_USB_GADGET_MUSB_HDRC=y
---

Cheers.

Felipe Contreras (4):
  mfd: simplify twl4030_power config
  mfd: simplify twl4030_core config
  regulator: simplify twl4030 config
  usb: simplify hdrc configuration

 drivers/mfd/Kconfig       |    3 ++-
 drivers/regulator/Kconfig |    1 +
 drivers/usb/musb/Kconfig  |    4 +---
 drivers/usb/otg/Kconfig   |    3 +++
 4 files changed, 7 insertions(+), 4 deletions(-)

-- 
1.7.8.rc1.14.g248db

[PATCH v4 1/4] mfd: simplify twl4030_power config

[Felipe Contreras]

It doesn't depend on ARM, at least not to build.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 drivers/mfd/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index f1391c2..c4b6f7d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -223,7 +223,7 @@ config TWL4030_MADC
 
 config TWL4030_POWER
 	bool "Support power resources on TWL4030 family chips"
-	depends on TWL4030_CORE && ARM
+	depends on TWL4030_CORE
 	help
 	  Say yes here if you want to use the power resources on the
 	  TWL4030 family chips.  Most of these resources are regulators,
-- 
1.7.8.rc1.14.g248db

[PATCH v4 2/4] mfd: simplify twl4030_core config

[Felipe Contreras]

The documentation says it all "These multi-function chips are found on
many OMAP2 and OMAP3 boards", so lets enable it by default.

This simplifies the defconfig from:

 CONFIG_ARCH_OMAP=y
 CONFIG_TWL4030_CORE=y

To:

 CONFIG_ARCH_OMAP=y

Of course, you can always do:

 CONFIG_ARCH_OMAP=y
 CONFIG_TWL4030_CORE=n

If you want, to reduce the size of the kernel. This is just the
*default*.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 drivers/mfd/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c4b6f7d..057abd0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -200,6 +200,7 @@ config MENELAUS
 config TWL4030_CORE
 	bool "Texas Instruments TWL4030/TWL5030/TWL6030/TPS659x0 Support"
 	depends on I2C=y && GENERIC_HARDIRQS
+	default y if ARCH_OMAP3 || ARCH_OMAP4
 	help
 	  Say yes here if you have TWL4030 / TWL6030 family chip on your board.
 	  This core driver provides register access and IRQ handling
-- 
1.7.8.rc1.14.g248db

[PATCH v4 3/4] regulator: simplify twl4030 config

[Felipe Contreras]

If we have TWL4030_CORE and REGULATOR, we *obviously* want
REGULATOR_TWL4030 by default. But can be disabled if somebody wants for
some strange reason.

This decreases the defconfig from:

 CONFIG_TWL4030_CORE=y
 CONFIG_REGULATOR=y
 CONFIG_REGULATOR_TWL4030=y

To:

 CONFIG_TWL4030_CORE=y
 CONFIG_REGULATOR=y

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 drivers/regulator/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 9713b1b..e980b2f 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -138,6 +138,7 @@ config REGULATOR_MAX8998
 config REGULATOR_TWL4030
 	bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 PMIC"
 	depends on TWL4030_CORE
+	default y
 	help
 	  This driver supports the voltage regulators provided by
 	  this family of companion chips.
-- 
1.7.8.rc1.14.g248db

Re: [PATCH v4 3/4] regulator: simplify twl4030 config

[Mark Brown]

On Tue, Dec 20, 2011 at 03:15:16AM +0200, Felipe Contreras wrote:
> If we have TWL4030_CORE and REGULATOR, we *obviously* want
> REGULATOR_TWL4030 by default. But can be disabled if somebody wants for
> some strange reason.

No, this is the wrong way to go about this - why are you making this
change for this one driver only?  Exactly the same argument would apply
to all the other MFD regulators and to all the other MFD subdrivers in
the various subsystems.

Re: [PATCH v4 3/4] regulator: simplify twl4030 config

[Felipe Contreras]

On Tue, Dec 20, 2011 at 3:19 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Dec 20, 2011 at 03:15:16AM +0200, Felipe Contreras wrote:
>> If we have TWL4030_CORE and REGULATOR, we *obviously* want
>> REGULATOR_TWL4030 by default. But can be disabled if somebody wants for
>> some strange reason.
>
> No, this is the wrong way to go about this - why are you making this
> change for this one driver only?

Because that's the only one I care about. I can try to do the same for
all the others if you wish.

> Exactly the same argument would apply
> to all the other MFD regulators and to all the other MFD subdrivers in
> the various subsystems.

Yes, the ones that closely related to certain architectures or chips.

And this is already done:

REGULATOR_AB3100:
	depends on AB3100_CORE
	default y if AB3100_CORE

(if AB3100_CORE is redundant BTW)

REGULATOR_TPS6105X:
	depends on TPS6105X
	default y if TPS6105X

ditto

And in mfd:

config TPS65010
	default y if MACH_OMAP_H2 || MACH_OMAP_H3 || MACH_OMAP_OSK

config ABX500_CORE
	default y if ARCH_U300 || ARCH_U8500

config AB3100_CORE
	default y if ARCH_U300

config AB3100_OTP
	default y if AB3100_CORE

These 3 seem to be linked, but the same logic applies; most people
should say 'y', so make it the default.

-- 
Felipe Contreras

Re: [PATCH v4 3/4] regulator: simplify twl4030 config

[Mark Brown]

On Tue, Dec 20, 2011 at 03:36:59AM +0200, Felipe Contreras wrote:
> On Tue, Dec 20, 2011 at 3:19 AM, Mark Brown

> > No, this is the wrong way to go about this - why are you making this
> > change for this one driver only?

> Because that's the only one I care about. I can try to do the same for
> all the others if you wish.

Yes, of course.  Half the point in having subsystems and standard ways
of doing things is so that when we look at things we've got a reasonable
idea as to what they're supposed to be doing and people looking at the
code can figure out what's going on.   We don't want people to have to
play guessing games about this stuff.

> > Exactly the same argument would apply
> > to all the other MFD regulators and to all the other MFD subdrivers in
> > the various subsystems.

> Yes, the ones that closely related to certain architectures or chips.

Obviously all MFDs are closely releated to chips...

> And this is already done:
> 
> REGULATOR_AB3100:
> 	depends on AB3100_CORE
> 	default y if AB3100_CORE

> (if AB3100_CORE is redundant BTW)

> REGULATOR_TPS6105X:
> 	depends on TPS6105X
> 	default y if TPS6105X

So it's done by a small proportion of drivers...

Re: [PATCH v4 3/4] regulator: simplify twl4030 config

[Felipe Contreras]

On Tue, Dec 20, 2011 at 3:53 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Dec 20, 2011 at 03:36:59AM +0200, Felipe Contreras wrote:
>> On Tue, Dec 20, 2011 at 3:19 AM, Mark Brown
>
>> > No, this is the wrong way to go about this - why are you making this
>> > change for this one driver only?
>
>> Because that's the only one I care about. I can try to do the same for
>> all the others if you wish.
>
> Yes, of course.  Half the point in having subsystems and standard ways
> of doing things is so that when we look at things we've got a reasonable
> idea as to what they're supposed to be doing and people looking at the
> code can figure out what's going on.   We don't want people to have to
> play guessing games about this stuff.

Of course... *ideally*, but we are not there right now.

>> > Exactly the same argument would apply
>> > to all the other MFD regulators and to all the other MFD subdrivers in
>> > the various subsystems.
>
>> Yes, the ones that closely related to certain architectures or chips.
>
> Obviously all MFDs are closely releated to chips...
>
>> And this is already done:
>>
>> REGULATOR_AB3100:
>>       depends on AB3100_CORE
>>       default y if AB3100_CORE
>
>> (if AB3100_CORE is redundant BTW)
>
>> REGULATOR_TPS6105X:
>>       depends on TPS6105X
>>       default y if TPS6105X
>
> So it's done by a small proportion of drivers...

Yes, so the only question is; is this desirable or not? Personally, I
don't see why a normal person would want to enable REGULATOR and
TWL4030_CORE but not REGULATOR_TWL4030; what's the point of enabling
REGULATOR if it's not going to do anything? Anyway, these people can
still choose REGULATOR_TWL4030=n, but it *certainly* doesn't seem to
be the normal use-case. And that's the whole point of 'default y' to
begin with; to enable by default what most people should say 'y' to.

-- 
Felipe Contreras

Re: [PATCH v4 3/4] regulator: simplify twl4030 config

[Mark Brown]

On Tue, Dec 20, 2011 at 04:04:14AM +0200, Felipe Contreras wrote:
> On Tue, Dec 20, 2011 at 3:53 AM, Mark Brown

> > Yes, of course.  Half the point in having subsystems and standard ways
> > of doing things is so that when we look at things we've got a reasonable
> > idea as to what they're supposed to be doing and people looking at the
> > code can figure out what's going on.   We don't want people to have to
> > play guessing games about this stuff.

> Of course... *ideally*, but we are not there right now.

We pretty much were, but this patch has changelog which says "I've got a
cool new way of doing things for this one driver!" which clearly applies
to other drivers.  I wouldn't mind a patch which does this for all MFDs
(though I really do think it should be applied to most subsystems) but
doing it for one driver isn't great.

Re: [PATCH v4 3/4] regulator: simplify twl4030 config

[Mark Brown]

On Tue, Dec 20, 2011 at 03:15:16AM +0200, Felipe Contreras wrote:
> If we have TWL4030_CORE and REGULATOR, we *obviously* want
> REGULATOR_TWL4030 by default. But can be disabled if somebody wants for
> some strange reason.

Oh, and I just noticed that this is v4 of some 4 part series I've never
heard of before of which I only seem to have patch 3.  Seems like
something's going wrong here...

Re: [PATCH v4 3/4] regulator: simplify twl4030 config

[Felipe Contreras]

On Tue, Dec 20, 2011 at 3:25 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Dec 20, 2011 at 03:15:16AM +0200, Felipe Contreras wrote:
>> If we have TWL4030_CORE and REGULATOR, we *obviously* want
>> REGULATOR_TWL4030 by default. But can be disabled if somebody wants for
>> some strange reason.
>
> Oh, and I just noticed that this is v4 of some 4 part series I've never
> heard of before of which I only seem to have patch 3.  Seems like
> something's going wrong here...

You have commented on v3:
http://thread.gmane.org/gmane.linux.usb.general/38230

I just used 'git send-email' with --cc-cmd
./scripts/get_maintainer.pl, so blame git send-email. Anyway, I don't
think you are interested in other parts of the patch series, but if
you are, they are in LKML.

-- 
Felipe Contreras

[PATCH v4 4/4] usb: simplify hdrc configuration

[Felipe Contreras]

HDRC requires either NOP_USB_XCEIV or TWLX030_USB, and it selects the
right one based on architectures or board.

However, many OMAP2+ boards are excluded from this selection and it's
possible to have a configuration with no transceiver, so HDRC doesn't
work.

What we need is a 'soft' dependency, and this can be achieved by
transforming the 'selects' to 'default'. This way we can have sane
defaults for all OMAP2+ boards, and the ones that need NOP can just
specify that manually.

This simplifies the config from:

     CONFIG_USB=y
     CONFIG_TWL4030_CORE=y
     CONFIG_TWL4030_USB=y

To:

     CONFIG_USB=y
     CONFIG_TWL4030_CORE=y

And so on.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 drivers/usb/musb/Kconfig |    4 +---
 drivers/usb/otg/Kconfig  |    3 +++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 07a0346..b60d3a5 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -7,9 +7,7 @@
 config USB_MUSB_HDRC
 	depends on USB && USB_GADGET
 	depends on (ARM || (BF54x && !BF544) || (BF52x && !BF522 && !BF523))
-	select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN)
-	select TWL4030_USB if MACH_OMAP_3430SDP
-	select TWL6030_USB if MACH_OMAP_4430SDP || MACH_OMAP4_PANDA
+	depends on (NOP_USB_XCEIV || TWL4030_USB || TWL6030_USB)
 	select USB_OTG_UTILS
 	select USB_GADGET_DUALSPEED
 	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
diff --git a/drivers/usb/otg/Kconfig b/drivers/usb/otg/Kconfig
index c66481a..5beb8b7 100644
--- a/drivers/usb/otg/Kconfig
+++ b/drivers/usb/otg/Kconfig
@@ -60,6 +60,7 @@ config TWL4030_USB
 	tristate "TWL4030 USB Transceiver Driver"
 	depends on TWL4030_CORE && REGULATOR_TWL4030
 	select USB_OTG_UTILS
+	default y if ARCH_OMAP3
 	help
 	  Enable this to support the USB OTG transceiver on TWL4030
 	  family chips (including the TWL5030 and TPS659x0 devices).
@@ -70,6 +71,7 @@ config TWL6030_USB
 	tristate "TWL6030 USB Transceiver Driver"
 	depends on TWL4030_CORE
 	select USB_OTG_UTILS
+	default y if ARCH_OMAP4
 	help
 	  Enable this to support the USB OTG transceiver on TWL6030
 	  family chips. This TWL6030 transceiver has the VBUS and ID GND
@@ -81,6 +83,7 @@ config TWL6030_USB
 config NOP_USB_XCEIV
 	tristate "NOP USB Transceiver Driver"
 	select USB_OTG_UTILS
+	default y if ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN
 	help
 	 this driver is to be used by all the usb transceiver which are either
 	 built-in with usb ip or which are autonomous and doesn't require any
-- 
1.7.8.rc1.14.g248db

Re: [PATCH v4 4/4] usb: simplify hdrc configuration

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]

Hi,

On Tue, Dec 20, 2011 at 03:15:17AM +0200, Felipe Contreras wrote:
> HDRC requires either NOP_USB_XCEIV or TWLX030_USB, and it selects the

that's not entirely true. Like any USB Link controller, MUSB needs a PHY
to work, but can be any off-the-shelf PHY. Nokia has used isp1704 and
isp1707 for instance. I guess Nokia also used tusb1211 and none of them
are listed there.

I would rather remove all the dependencies because the person compiling
the kernel should know that a Link needs a PHY. Maybe add a note
somewhere stating that you should enable the PHY driver for your
platform ?

> right one based on architectures or board.
> 
> However, many OMAP2+ boards are excluded from this selection and it's
> possible to have a configuration with no transceiver, so HDRC doesn't
> work.
> 
> What we need is a 'soft' dependency, and this can be achieved by
> transforming the 'selects' to 'default'. This way we can have sane
> defaults for all OMAP2+ boards, and the ones that need NOP can just
> specify that manually.
> 
> This simplifies the config from:
> 
>      CONFIG_USB=y
>      CONFIG_TWL4030_CORE=y
>      CONFIG_TWL4030_USB=y
> 
> To:
> 
>      CONFIG_USB=y
>      CONFIG_TWL4030_CORE=y
> 
> And so on.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  drivers/usb/musb/Kconfig |    4 +---
>  drivers/usb/otg/Kconfig  |    3 +++
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> index 07a0346..b60d3a5 100644
> --- a/drivers/usb/musb/Kconfig
> +++ b/drivers/usb/musb/Kconfig
> @@ -7,9 +7,7 @@
>  config USB_MUSB_HDRC
>  	depends on USB && USB_GADGET
>  	depends on (ARM || (BF54x && !BF544) || (BF52x && !BF522 && !BF523))
> -	select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN)
> -	select TWL4030_USB if MACH_OMAP_3430SDP
> -	select TWL6030_USB if MACH_OMAP_4430SDP || MACH_OMAP4_PANDA
> +	depends on (NOP_USB_XCEIV || TWL4030_USB || TWL6030_USB)

on commit log, you said you were converting select to default, but here
you're converting select to depend. I have just added a patch allowing
musb to compile on all ARCHes and cannot take this step backwards.
Sorry. Any chance to resping this patch in another way ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4 4/4] usb: simplify hdrc configuration

[Felipe Contreras]

On Tue, Dec 20, 2011 at 1:04 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Dec 20, 2011 at 03:15:17AM +0200, Felipe Contreras wrote:
>> HDRC requires either NOP_USB_XCEIV or TWLX030_USB, and it selects the
>
> that's not entirely true. Like any USB Link controller, MUSB needs a PHY
> to work, but can be any off-the-shelf PHY. Nokia has used isp1704 and
> isp1707 for instance. I guess Nokia also used tusb1211 and none of them
> are listed there.

Then have a CONFIG_MUSB_PHY menu, or whatever.

> I would rather remove all the dependencies because the person compiling
> the kernel should know that a Link needs a PHY.

Are you serious? Most likely the person compiling the kernel doesn't
even know what PHY really means, and just wants the USB to f*cking
work.

> Maybe add a note
> somewhere stating that you should enable the PHY driver for your
> platform ?

You can say the same of pretty much all the dependencies in the
kernel: oh, just add a note saying that this depends on that. And how
would they know what a PHY driver is?

You already know the dependency, it only needs to be written down:
depends on CONFIG_MUSB_PHY. And then you have a menu for
CONFIG_MUSB_PHY and select the correct transceiver.

Right now it's pretty much impossible for a normal person to figure
out a working configuration on his/her own without just enabling
everything.

-- 
Felipe Contreras