[PATCH v2 0/3] usb: generic Kconfig fixes

[PATCH v2 0/3] usb: generic Kconfig fixes

[Felipe Contreras]

This gets rid of some warnings, makes defconfigs smarter by having more proper
dependencies, and general cleanups.

BTW, this is the reason USB_GADGET_MUSB_HDRC sometimes is not selected:
http://article.gmane.org/gmane.linux.kbuild.devel/5480

v2: comments by Felipe Balbi

Felipe Contreras (3):
  usb: fix Kconfig warning
  usb: trivial Kconfig cleanups
  drivers: cleanup Kconfig stuff

 drivers/mfd/Kconfig        |    3 ++-
 drivers/regulator/Kconfig  |    1 +
 drivers/usb/core/Kconfig   |    3 ---
 drivers/usb/gadget/Kconfig |   16 ++--------------
 drivers/usb/musb/Kconfig   |   23 ++++++-----------------
 drivers/usb/otg/Kconfig    |    2 ++
 6 files changed, 13 insertions(+), 35 deletions(-)

-- 
1.7.3.1.2.g7fe2b

[PATCH v2 1/3] usb: fix Kconfig warning

[Felipe Contreras]

warning: (USB_MUSB_HDRC_HCD && USB_SUPPORT && USB_MUSB_HDRC &&
(USB_MUSB_HOST || USB_MUSB_OTG) && USB_GADGET_MUSB_HDRC || USB_MUSB_OTG
&& <choice> && USB && USB_GADGET && PM && EXPERIMENTAL) selects USB_OTG
which has unmet direct dependencies (USB_GADGET_OMAP && ARCH_OMAP_OTG &&
USB_OHCI_HCD)

This doesn't seem to happen on 2.6.36-rc8, but still doesn't make sense
to keep this duplicated config that is already defined in usb/core.

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

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index cd27f9b..19a6eb6 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -209,17 +209,6 @@ config USB_OMAP
 	default USB_GADGET
 	select USB_GADGET_SELECTED
 
-config USB_OTG
-	boolean "OTG Support"
-	depends on USB_GADGET_OMAP && ARCH_OMAP_OTG && USB_OHCI_HCD
-	help
-	   The most notable feature of USB OTG is support for a
-	   "Dual-Role" device, which can act as either a device
-	   or a host.  The initial role choice can be changed
-	   later, when two dual-role devices talk to each other.
-
-	   Select this only if your OMAP board has a Mini-AB connector.
-
 config USB_GADGET_PXA25X
 	boolean "PXA 25x or IXP 4xx"
 	depends on (ARCH_PXA && PXA25x) || ARCH_IXP4XX
-- 
1.7.3.1.2.g7fe2b

Re: [PATCH v2 1/3] usb: fix Kconfig warning

[Felipe Balbi]

On Tue, Oct 19, 2010 at 05:03:26AM -0500, Felipe Contreras wrote:
>warning: (USB_MUSB_HDRC_HCD && USB_SUPPORT && USB_MUSB_HDRC &&
>(USB_MUSB_HOST || USB_MUSB_OTG) && USB_GADGET_MUSB_HDRC || USB_MUSB_OTG
>&& <choice> && USB && USB_GADGET && PM && EXPERIMENTAL) selects USB_OTG
>which has unmet direct dependencies (USB_GADGET_OMAP && ARCH_OMAP_OTG &&
>USB_OHCI_HCD)
>
>This doesn't seem to happen on 2.6.36-rc8, but still doesn't make sense
>to keep this duplicated config that is already defined in usb/core.
>
>Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

Since this is related to MUSB, Greg to do you want me to queue for -rc0
cycle and re-send for you ?

-- 
balbi

Re: [PATCH v2 1/3] usb: fix Kconfig warning

[Greg KH]

On Tue, Oct 19, 2010 at 01:28:09PM +0300, Felipe Balbi wrote:
> On Tue, Oct 19, 2010 at 05:03:26AM -0500, Felipe Contreras wrote:
> >warning: (USB_MUSB_HDRC_HCD && USB_SUPPORT && USB_MUSB_HDRC &&
> >(USB_MUSB_HOST || USB_MUSB_OTG) && USB_GADGET_MUSB_HDRC || USB_MUSB_OTG
> >&& <choice> && USB && USB_GADGET && PM && EXPERIMENTAL) selects USB_OTG
> >which has unmet direct dependencies (USB_GADGET_OMAP && ARCH_OMAP_OTG &&
> >USB_OHCI_HCD)
> >
> >This doesn't seem to happen on 2.6.36-rc8, but still doesn't make sense
> >to keep this duplicated config that is already defined in usb/core.
> >
> >Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> 
> Since this is related to MUSB, Greg to do you want me to queue for -rc0
> cycle and re-send for you ?

If you ack this patch, I'll be glad to take it as-is.

thanks,

greg k-h

Re: [PATCH v2 1/3] usb: fix Kconfig warning

[Felipe Balbi]

On Tue, Oct 19, 2010 at 05:03:26AM -0500, Felipe Contreras wrote:
>warning: (USB_MUSB_HDRC_HCD && USB_SUPPORT && USB_MUSB_HDRC &&
>(USB_MUSB_HOST || USB_MUSB_OTG) && USB_GADGET_MUSB_HDRC || USB_MUSB_OTG
>&& <choice> && USB && USB_GADGET && PM && EXPERIMENTAL) selects USB_OTG
>which has unmet direct dependencies (USB_GADGET_OMAP && ARCH_OMAP_OTG &&
>USB_OHCI_HCD)
>
>This doesn't seem to happen on 2.6.36-rc8, but still doesn't make sense
>to keep this duplicated config that is already defined in usb/core.
>

Acked-by: Felipe Balbi <balbi@ti.com>

>Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>---
> drivers/usb/gadget/Kconfig |   11 -----------
> 1 files changed, 0 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
>index cd27f9b..19a6eb6 100644
>--- a/drivers/usb/gadget/Kconfig
>+++ b/drivers/usb/gadget/Kconfig
>@@ -209,17 +209,6 @@ config USB_OMAP
> 	default USB_GADGET
> 	select USB_GADGET_SELECTED
>
>-config USB_OTG
>-	boolean "OTG Support"
>-	depends on USB_GADGET_OMAP && ARCH_OMAP_OTG && USB_OHCI_HCD
>-	help
>-	   The most notable feature of USB OTG is support for a
>-	   "Dual-Role" device, which can act as either a device
>-	   or a host.  The initial role choice can be changed
>-	   later, when two dual-role devices talk to each other.
>-
>-	   Select this only if your OMAP board has a Mini-AB connector.
>-
> config USB_GADGET_PXA25X
> 	boolean "PXA 25x or IXP 4xx"
> 	depends on (ARCH_PXA && PXA25x) || ARCH_IXP4XX
>-- 
>1.7.3.1.2.g7fe2b

-- 
balbi

[PATCH v2 2/3] usb: trivial Kconfig cleanups

[Felipe Contreras]

Note: USB_GADGET_MUSB_HDRC is already defined in usb/gadget.

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

diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index 9eed5b5..8f57a26 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -110,8 +110,6 @@ config USB_OTG
 	bool
 	depends on USB && EXPERIMENTAL
 	depends on USB_SUSPEND
-	default n
-
 
 config USB_OTG_WHITELIST
 	bool "Rely on OTG Targeted Peripherals List"
@@ -139,4 +137,3 @@ config USB_OTG_BLACKLIST_HUB
 	  external hubs.  OTG hosts are allowed to reduce hardware
 	  and software costs by not supporting external hubs.  So
 	  are "Embedded Hosts" that don't offer OTG support.
-
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 19a6eb6..c530bc6 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -125,7 +125,6 @@ choice
 config USB_GADGET_AT91
 	boolean "Atmel AT91 USB Device Port"
 	depends on ARCH_AT91 && !ARCH_AT91SAM9RL && !ARCH_AT91CAP9 && !ARCH_AT91SAM9G45
-	select USB_GADGET_SELECTED
 	help
 	   Many Atmel AT91 processors (such as the AT91RM2000) have a
 	   full speed USB Device Port with support for five configurable
@@ -139,6 +138,7 @@ config USB_AT91
 	tristate
 	depends on USB_GADGET_AT91
 	default USB_GADGET
+	select USB_GADGET_SELECTED
 
 config USB_GADGET_ATMEL_USBA
 	boolean "Atmel USBA"
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index cfd38ed..20895c4 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -8,12 +8,12 @@ comment "Enable Host or Gadget support to see Inventra options"
 
 # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller
 config USB_MUSB_HDRC
+	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
 	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 USB_OTG_UTILS
-	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
 	help
 	  Say Y here if your system has a dual role high speed USB
 	  controller based on the Mentor Graphics silicon IP.  Then
@@ -79,8 +79,8 @@ choice
 # use USB_MUSB_HDRC_HCD not USB_MUSB_HOST to #ifdef host side support;
 # OTG needs both roles, not just USB_MUSB_HOST.
 config USB_MUSB_HOST
-	depends on USB
 	bool "USB Host"
+	depends on USB
 	help
 	  Say Y here if your system supports the USB host role.
 	  If it has a USB "A" (rectangular), "Mini-A" (uncommon),
@@ -90,8 +90,8 @@ config USB_MUSB_HOST
 # use USB_GADGET_MUSB_HDRC not USB_MUSB_PERIPHERAL to #ifdef peripheral
 # side support ... OTG needs both roles
 config USB_MUSB_PERIPHERAL
-	depends on USB_GADGET
 	bool "USB Peripheral (gadget stack)"
+	depends on USB_GADGET
 	select USB_GADGET_MUSB_HDRC
 	help
 	  Say Y here if your system supports the USB peripheral role.
@@ -100,8 +100,8 @@ config USB_MUSB_PERIPHERAL
 	  (With a "Mini-AB" connector, you should enable USB OTG.)
 
 config USB_MUSB_OTG
-	depends on USB && USB_GADGET && PM && EXPERIMENTAL
 	bool "Both host and peripheral:  USB OTG (On The Go) Device"
+	depends on USB && USB_GADGET && PM && EXPERIMENTAL
 	select USB_GADGET_MUSB_HDRC
 	select USB_OTG
 	help
@@ -125,14 +125,6 @@ config USB_MUSB_OTG
 
 endchoice
 
-# enable peripheral support (including with OTG)
-config USB_GADGET_MUSB_HDRC
-	bool
-	depends on USB_MUSB_HDRC && (USB_MUSB_PERIPHERAL || USB_MUSB_OTG)
-#	default y
-#	select USB_GADGET_DUALSPEED
-#	select USB_GADGET_SELECTED
-
 # enables host support (including with OTG)
 config USB_MUSB_HDRC_HCD
 	bool
@@ -140,7 +132,6 @@ config USB_MUSB_HDRC_HCD
 	select USB_OTG if USB_GADGET_MUSB_HDRC
 	default y
 
-
 config MUSB_PIO_ONLY
 	bool 'Disable DMA (always use PIO)'
 	depends on USB_MUSB_HDRC
-- 
1.7.3.1.2.g7fe2b

Re: [PATCH v2 2/3] usb: trivial Kconfig cleanups

[Greg KH]

On Tue, Oct 19, 2010 at 01:03:27PM +0300, Felipe Contreras wrote:
> Note: USB_GADGET_MUSB_HDRC is already defined in usb/gadget.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

Please describe _what_ trivial cleanups you did, and why you did them.
Care to resend?

thanks,

greg k-h

Re: [PATCH v2 2/3] usb: trivial Kconfig cleanups

[Felipe Contreras]

On Tue, Oct 19, 2010 at 6:51 PM, Greg KH <greg@kroah.com> wrote:
> On Tue, Oct 19, 2010 at 01:03:27PM +0300, Felipe Contreras wrote:
>> Note: USB_GADGET_MUSB_HDRC is already defined in usb/gadget.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> Please describe _what_ trivial cleanups you did, and why you did them.
> Care to resend?

How about:
Shuffle the code a bit so the description is at the top, remove
duplicated configs (USB_GADGET_MUSB_HDRC),  'default n' is redundant,
and USB_GADGET_SELECTED should go after USB_GADGET (on the appropriate
config).

-- 
Felipe Contreras

Re: [PATCH v2 2/3] usb: trivial Kconfig cleanups

[Greg KH]

On Tue, Oct 19, 2010 at 08:15:19PM +0300, Felipe Contreras wrote:
> On Tue, Oct 19, 2010 at 6:51 PM, Greg KH <greg@kroah.com> wrote:
> > On Tue, Oct 19, 2010 at 01:03:27PM +0300, Felipe Contreras wrote:
> >> Note: USB_GADGET_MUSB_HDRC is already defined in usb/gadget.
> >>
> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >
> > Please describe _what_ trivial cleanups you did, and why you did them.
> > Care to resend?
> 
> How about:
> Shuffle the code a bit so the description is at the top, remove
> duplicated configs (USB_GADGET_MUSB_HDRC),  'default n' is redundant,
> and USB_GADGET_SELECTED should go after USB_GADGET (on the appropriate
> config).

Sounds good, but you need to resend it with this information in it, not
much I can do with it in an independant email.

thanks,

greg k-h

Re: [PATCH v2 2/3] usb: trivial Kconfig cleanups

[Felipe Contreras]

On Tue, Oct 19, 2010 at 8:50 PM, Greg KH <greg@kroah.com> wrote:
> On Tue, Oct 19, 2010 at 08:15:19PM +0300, Felipe Contreras wrote:
>> On Tue, Oct 19, 2010 at 6:51 PM, Greg KH <greg@kroah.com> wrote:
>> > On Tue, Oct 19, 2010 at 01:03:27PM +0300, Felipe Contreras wrote:
>> >> Note: USB_GADGET_MUSB_HDRC is already defined in usb/gadget.
>> >>
>> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> >
>> > Please describe _what_ trivial cleanups you did, and why you did them.
>> > Care to resend?
>>
>> How about:
>> Shuffle the code a bit so the description is at the top, remove
>> duplicated configs (USB_GADGET_MUSB_HDRC),  'default n' is redundant,
>> and USB_GADGET_SELECTED should go after USB_GADGET (on the appropriate
>> config).
>
> Sounds good, but you need to resend it with this information in it, not
> much I can do with it in an independant email.

I know that, I just wanted to see if it was ok before resending so I
don't resend another time.

-- 
Felipe Contreras

Re: [PATCH v2 2/3] usb: trivial Kconfig cleanups

[Felipe Balbi]

On Tue, Oct 19, 2010 at 05:03:27AM -0500, Felipe Contreras wrote:
>Note: USB_GADGET_MUSB_HDRC is already defined in usb/gadget.
>
>Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>---
> drivers/usb/core/Kconfig   |    3 ---
> drivers/usb/gadget/Kconfig |    2 +-
> drivers/usb/musb/Kconfig   |   17 ++++-------------

for drivers/usb/musb/Kconfig:

Acked-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

[PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

Add missing dependencies, and remove redundant ones, and select sensible
defaults.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 drivers/mfd/Kconfig        |    3 ++-
 drivers/regulator/Kconfig  |    1 +
 drivers/usb/gadget/Kconfig |    3 +--
 drivers/usb/musb/Kconfig   |    6 ++----
 drivers/usb/otg/Kconfig    |    2 ++
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index db51ea1..0bbaf01 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -156,6 +156,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
@@ -169,7 +170,7 @@ config TWL4030_CORE
 
 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,
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 172951b..af0439d 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -111,6 +111,7 @@ config REGULATOR_MAX8998
 config REGULATOR_TWL4030
 	bool "TI TWL4030/TWL5030/TWL6030/TPS695x0 PMIC"
 	depends on TWL4030_CORE
+	default y
 	help
 	  This driver supports the voltage regulators provided by
 	  this family of companion chips.
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index c530bc6..2aac57e 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -111,7 +111,6 @@ config	USB_GADGET_SELECTED
 #
 choice
 	prompt "USB Peripheral Controller"
-	depends on USB_GADGET
 	help
 	   A USB device uses a controller to talk to its host.
 	   Systems should have only one such upstream link.
@@ -191,7 +190,6 @@ config USB_GADGET_OMAP
 	boolean "OMAP USB Device Controller"
 	depends on ARCH_OMAP
 	select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3 || MACH_OMAP_H4_OTG
-	select USB_OTG_UTILS if ARCH_OMAP
 	help
 	   Many Texas Instruments OMAP processors have flexible full
 	   speed USB device controllers, with support for up to 30
@@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
 config USB_G_NOKIA
 	tristate "Nokia composite gadget"
 	depends on PHONET
+	depends on USB_GADGET_MUSB_HDRC
 	help
 	  The Nokia composite gadget provides support for acm, obex
 	  and phonet in only one composite gadget driver.
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 20895c4..a3f01f6 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -11,8 +11,7 @@ config USB_MUSB_HDRC
 	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
 	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
+	depends on (NOP_USB_XCEIV || TWL4030_USB)
 	select USB_OTG_UTILS
 	help
 	  Say Y here if your system has a dual role high speed USB
@@ -59,8 +58,7 @@ comment "Blackfin high speed USB Support"
 
 config USB_TUSB6010
 	boolean "TUSB 6010 support"
-	depends on USB_MUSB_HDRC && !USB_MUSB_SOC
-	select NOP_USB_XCEIV
+	depends on USB_MUSB_HDRC && !USB_MUSB_SOC && NOP_USB_XCEIV
 	default y
 	help
 	  The TUSB 6010 chip, from Texas Instruments, connects a discrete
diff --git a/drivers/usb/otg/Kconfig b/drivers/usb/otg/Kconfig
index 3b12895..2e5fd2d 100644
--- a/drivers/usb/otg/Kconfig
+++ b/drivers/usb/otg/Kconfig
@@ -53,6 +53,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).
@@ -62,6 +63,7 @@ config TWL4030_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.3.1.2.g7fe2b

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

Hi,

On Tue, Oct 19, 2010 at 05:03:28AM -0500, Felipe Contreras wrote:
>+	depends on (NOP_USB_XCEIV || TWL4030_USB)
>+	default y if ARCH_OMAP3
>+	default y if ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN

it could be only me, but I see these kind of dependencies tend to
disaster. They can all grow to infinity. Soon, we will have another twl
family of devices, or we will have another MACH_* which will need
nop_xceiv, etc etc.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Tue, Oct 19, 2010 at 1:32 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Oct 19, 2010 at 05:03:28AM -0500, Felipe Contreras wrote:
>>
>> +       depends on (NOP_USB_XCEIV || TWL4030_USB)
>> +       default y if ARCH_OMAP3
>> +       default y if ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN
>
> it could be only me, but I see these kind of dependencies tend to
> disaster. They can all grow to infinity. Soon, we will have another twl
> family of devices, or we will have another MACH_* which will need
> nop_xceiv, etc etc.

The USB_MUSB_HDRC dependency on NOP_USB_XCEIV || TWL4030_USB is
already there, I'm just making it explicit. The rest are not
dependencies, just sensible defaults, and they are already in
drivers/usb/musb/Kconfig, I'm just moving them.

If you have a better approach to avoid manually selecting TWL4030_USB
on defconfigs, and making sure USB_MUSB_HDRC works, please share it.

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

Hi,

On Tue, Oct 19, 2010 at 05:59:58AM -0500, Felipe Contreras wrote:
>The USB_MUSB_HDRC dependency on NOP_USB_XCEIV || TWL4030_USB is
>already there, I'm just making it explicit. The rest are not
>dependencies, just sensible defaults, and they are already in
>drivers/usb/musb/Kconfig, I'm just moving them.

might be true, but what will happen is that other developers will just
"follow what's in the tree" so they'll keep on adding more and more
"sesible defaults" to this line.

Linus had some really good comments about Kconfig usage some time ago
and I'm sure he wouldn't like us adding "sensible defaults" all over the
place.

>If you have a better approach to avoid manually selecting TWL4030_USB
>on defconfigs, and making sure USB_MUSB_HDRC works, please share it.

that's the thing. Currently, I don't think it should be automatic as we
can only have one transceiver structure. Look into
drivers/usb/otg/otg.c, it's just a static global pointer on that file,
which prevents us from having a working solution if you enable more than
one transceiver.

Ideally there would be a class or something similar to allow us to
register N transceiver and use the correct one in runtime, that would
make distro-makers' life easier. It'll probably help Linaro also at some
point, but as of today, enabling more than one transceiver will never
work.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Tue, Oct 19, 2010 at 2:16 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Oct 19, 2010 at 05:59:58AM -0500, Felipe Contreras wrote:
>>
>> The USB_MUSB_HDRC dependency on NOP_USB_XCEIV || TWL4030_USB is
>> already there, I'm just making it explicit. The rest are not
>> dependencies, just sensible defaults, and they are already in
>> drivers/usb/musb/Kconfig, I'm just moving them.
>
> might be true, but what will happen is that other developers will just
> "follow what's in the tree" so they'll keep on adding more and more
> "sesible defaults" to this line.

That is actually the intention.

> Linus had some really good comments about Kconfig usage some time ago
> and I'm sure he wouldn't like us adding "sensible defaults" all over the
> place.

It's the other way around. He wanted to get rid of defconfigs
completely by moving the complexity to Kconfigs:
http://article.gmane.org/gmane.linux.kernel/994720
http://article.gmane.org/gmane.linux.kernel/994706
http://article.gmane.org/gmane.linux.kernel/994718

You seem to be pushing for the exact opposite; keep the complexity on
the defconfigs and the burden on the user creating configs.

>> If you have a better approach to avoid manually selecting TWL4030_USB
>> on defconfigs, and making sure USB_MUSB_HDRC works, please share it.
>
> that's the thing. Currently, I don't think it should be automatic as we
> can only have one transceiver structure. Look into
> drivers/usb/otg/otg.c, it's just a static global pointer on that file,
> which prevents us from having a working solution if you enable more than
> one transceiver.
>
> Ideally there would be a class or something similar to allow us to
> register N transceiver and use the correct one in runtime, that would
> make distro-makers' life easier. It'll probably help Linaro also at some
> point, but as of today, enabling more than one transceiver will never
> work.

Ok. But for now 'default y if ARCH_OMAP3' for TWL4030_USB doesn't
hurt, nor increases the total size of Kconfigs, and solves a real
problem.

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

Hi,

On Tue, Oct 19, 2010 at 06:36:32AM -0500, Felipe Contreras wrote:
>That is actually the intention.

so it's a bit messed up one :-p

>It's the other way around. He wanted to get rid of defconfigs
>completely by moving the complexity to Kconfigs:

Yes, but he was suggesting to add things like Kconfig.rx51 and that
would be selecting correct drivers to get rx51 working, he was
definitely not suggesting to add sensible defaults all over the tree.

>http://article.gmane.org/gmane.linux.kernel/994720
>http://article.gmane.org/gmane.linux.kernel/994706
>http://article.gmane.org/gmane.linux.kernel/994718
>
>You seem to be pushing for the exact opposite; keep the complexity on
>the defconfigs and the burden on the user creating configs.

no, you got me wrong. We do need to get rid of defconfigs, but we have
to do so while keeping the resulting binary working and as of today, if
you enable more than one transceiver driver, it won't work.

>Ok. But for now 'default y if ARCH_OMAP3' for TWL4030_USB doesn't
>hurt, nor increases the total size of Kconfigs, and solves a real
>problem.

I don't see it that way. I see that there will be boards with one too
many transceivers enabled and the resulting binary won't work, but I'll
leave that for Greg to decide.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Tue, Oct 19, 2010 at 2:52 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Oct 19, 2010 at 06:36:32AM -0500, Felipe Contreras wrote:
>>
>> That is actually the intention.
>
> so it's a bit messed up one :-p

Why? It makes configurations simpler.

>> It's the other way around. He wanted to get rid of defconfigs
>> completely by moving the complexity to Kconfigs:
>
> Yes, but he was suggesting to add things like Kconfig.rx51 and that
> would be selecting correct drivers to get rx51 working, he was
> definitely not suggesting to add sensible defaults all over the tree.

There are two parts, one part is to fix the Kconfigs, and another was
to come up with a way to replace the horrible defconfigs. Part two is
mostly fixed thanks to 'make savedefconfig', since the defconfigs are
not so horrible any more, but part one is a continued effort.

For part two Linus had the idea to use Kconfigs instead of defconfigs,
but they would mostly look very similar to the current ones generated
by 'make savedefconfig'.

I don't know what makes you think Linus didn't want part one to be
done. He wanted the Kconfig.rx51 to be human readable, and you are
proposing that TWL4030_USB should be manually added there... I don't
think TWL4030_USB says anything to human beings.

See the last link:
---
It's often tedious for other cases too ("I just want to enable a
particular driver, what do I need to do so?")
---

>> http://article.gmane.org/gmane.linux.kernel/994720
>> http://article.gmane.org/gmane.linux.kernel/994706
>> http://article.gmane.org/gmane.linux.kernel/994718
>>
>> You seem to be pushing for the exact opposite; keep the complexity on
>> the defconfigs and the burden on the user creating configs.
>
> no, you got me wrong. We do need to get rid of defconfigs, but we have
> to do so while keeping the resulting binary working and as of today, if
> you enable more than one transceiver driver, it won't work.

Yes, but how do you get rid of the defconfig while also having a
working USB_G_NOKIA? Somehow a transceiver should be automatically
selected.

>> Ok. But for now 'default y if ARCH_OMAP3' for TWL4030_USB doesn't
>> hurt, nor increases the total size of Kconfigs, and solves a real
>> problem.
>
> I don't see it that way. I see that there will be boards with one too
> many transceivers enabled and the resulting binary won't work, but I'll
> leave that for Greg to decide.

That is no change from the current situation; you can still have two
transceivers. My patch is not changing that.

The issue you are pointing out is a separate one, and I can write a
patch for that... basically:

config NOP_USB_XCEIV
  depends !TWL4030_USB
config TWL4030_USB
  depends !NOP_USB_XCEIV

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

Hi,

(already at home so using personal email)

On Tue, 19 Oct 2010 17:32:07 +0300, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> There are two parts, one part is to fix the Kconfigs, and another was
> to come up with a way to replace the horrible defconfigs. Part two is
> mostly fixed thanks to 'make savedefconfig', since the defconfigs are
> not so horrible any more, but part one is a continued effort.
> 
> For part two Linus had the idea to use Kconfigs instead of defconfigs,
> but they would mostly look very similar to the current ones generated
> by 'make savedefconfig'.
> 
> I don't know what makes you think Linus didn't want part one to be
> done. He wanted the Kconfig.rx51 to be human readable, and you are
> proposing that TWL4030_USB should be manually added there... I don't
> think TWL4030_USB says anything to human beings.

I don't read anywhere Linus saying we should clutter drivers/*/Kconfig
with defaults. I think you're inverting things, it's much better
to select TWL4030_USB when you enable MACH_OMAP_RX51 then default y
if MACH_OMAP_RX51.

> Yes, but how do you get rid of the defconfig while also having a
> working USB_G_NOKIA? Somehow a transceiver should be automatically
> selected.

and you do that selection when the board is enabled otherwise the
drivers Kconfig will be cluttered with default Y if blabla.

But like I said before, it's Greg's ball. Personally, I think
you're taking the wrong approach.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Tue, Oct 19, 2010 at 6:05 PM, Felipe Balbi <me@felipebalbi.com> wrote:
> On Tue, 19 Oct 2010 17:32:07 +0300, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> There are two parts, one part is to fix the Kconfigs, and another was
>> to come up with a way to replace the horrible defconfigs. Part two is
>> mostly fixed thanks to 'make savedefconfig', since the defconfigs are
>> not so horrible any more, but part one is a continued effort.
>>
>> For part two Linus had the idea to use Kconfigs instead of defconfigs,
>> but they would mostly look very similar to the current ones generated
>> by 'make savedefconfig'.
>>
>> I don't know what makes you think Linus didn't want part one to be
>> done. He wanted the Kconfig.rx51 to be human readable, and you are
>> proposing that TWL4030_USB should be manually added there... I don't
>> think TWL4030_USB says anything to human beings.
>
> I don't read anywhere Linus saying we should clutter drivers/*/Kconfig
> with defaults. I think you're inverting things, it's much better
> to select TWL4030_USB when you enable MACH_OMAP_RX51 then default y
> if MACH_OMAP_RX51.

So say, if you have 100 board defconfigs with OMAP3 chips and TWL4030,
each and every one of them should select both USB_MUSB_HDRC and
TWL4030_USB?

And you are saying if somebody is doing 'make menuconfig', that person
should somehow know that _obviously_ he needs TWL4030_USB in order to
use USB_MUSB_HDRC, or USB_G_NOKIA?

That's precisely the opposite to:
---
It's often tedious for other cases too ("I just want to enable a
particular driver, what do I need to do so?")
---
---
In other words, you _can_ encode the information that is in the
xyz_defconfig files by doing it in Kconfig.xyz files instead. But you do
it in a human-readable manner. And the hierarchical thing is absolutely
required for that - otherwise you'd end up with just another form of the
current xyz_defconfig.
---

>> Yes, but how do you get rid of the defconfig while also having a
>> working USB_G_NOKIA? Somehow a transceiver should be automatically
>> selected.
>
> and you do that selection when the board is enabled otherwise the
> drivers Kconfig will be cluttered with default Y if blabla.

If you can imagine a hierarchy like:

 drivers/usb/otg/Kconfig
 arch/arm/mach-omap2/Kconfig
 ^ arch/arm/mach-omap2/boards/Kconfig.n900

You would want TWL4030_USB to be up in the hierarchy, so that you
don't have to add one extra line to each and every board file.

But in fact, you don't want TWL4030_USB on the Kconfig because not
everyone might want/need USB support. That should be selected by the
user, and asking the user to somehow know that he needs both
TWL4030_USB and USB_MUSB_HDRC is way too much to ask in my opinion.

The idea would be to keep Kconfig.n900 so minimal that it's not needed at all.

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Tony Lindgren]

* Felipe Contreras <felipe.contreras@gmail.com> [101019 13:43]:
> On Tue, Oct 19, 2010 at 6:05 PM, Felipe Balbi <me@felipebalbi.com> wrote:
> > On Tue, 19 Oct 2010 17:32:07 +0300, Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> >> There are two parts, one part is to fix the Kconfigs, and another was
> >> to come up with a way to replace the horrible defconfigs. Part two is
> >> mostly fixed thanks to 'make savedefconfig', since the defconfigs are
> >> not so horrible any more, but part one is a continued effort.
> >>
> >> For part two Linus had the idea to use Kconfigs instead of defconfigs,
> >> but they would mostly look very similar to the current ones generated
> >> by 'make savedefconfig'.
> >>
> >> I don't know what makes you think Linus didn't want part one to be
> >> done. He wanted the Kconfig.rx51 to be human readable, and you are
> >> proposing that TWL4030_USB should be manually added there... I don't
> >> think TWL4030_USB says anything to human beings.
> >
> > I don't read anywhere Linus saying we should clutter drivers/*/Kconfig
> > with defaults. I think you're inverting things, it's much better
> > to select TWL4030_USB when you enable MACH_OMAP_RX51 then default y
> > if MACH_OMAP_RX51.
> 
> So say, if you have 100 board defconfigs with OMAP3 chips and TWL4030,
> each and every one of them should select both USB_MUSB_HDRC and
> TWL4030_USB?
> 
> And you are saying if somebody is doing 'make menuconfig', that person
> should somehow know that _obviously_ he needs TWL4030_USB in order to
> use USB_MUSB_HDRC, or USB_G_NOKIA?

Hey we're already dealing with stuff like this :)

See ARCH_OMAP2PLUS_TYPICAL in arch/arm/mach-omap2/Kconfig:

config ARCH_OMAP2PLUS_TYPICAL
        bool "Typical OMAP configuration"
        default y
        select AEABI
        select REGULATOR
        select PM
        select PM_RUNTIME
        select VFP
        select NEON if ARCH_OMAP3 || ARCH_OMAP4
        select SERIAL_OMAP
        select SERIAL_OMAP_CONSOLE
        select I2C
        select I2C_OMAP
        select MFD
        select MENELAUS if ARCH_OMAP2
        select TWL4030_CORE if ARCH_OMAP3 || ARCH_OMAP4
        select TWL4030_POWER if ARCH_OMAP3 || ARCH_OMAP4
        help
          Compile a kernel suitable for booting most boards

This actually produces a booting omap2+ kernel with no .config
at all. I've tested it with:

$ echo CONFIG_ARCH_OMAP=y > .config
$ yes "" | ARCH=arm make oldconfig

So if you have something that's needed on almost every omap,
you can select it there.

Regards,

Tony

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Felipe Contreras <felipe.contreras@gmail.com> [101019 13:43]:
>> On Tue, Oct 19, 2010 at 6:05 PM, Felipe Balbi <me@felipebalbi.com> wrote:
>> > On Tue, 19 Oct 2010 17:32:07 +0300, Felipe Contreras
>> > <felipe.contreras@gmail.com> wrote:
>> >> There are two parts, one part is to fix the Kconfigs, and another was
>> >> to come up with a way to replace the horrible defconfigs. Part two is
>> >> mostly fixed thanks to 'make savedefconfig', since the defconfigs are
>> >> not so horrible any more, but part one is a continued effort.
>> >>
>> >> For part two Linus had the idea to use Kconfigs instead of defconfigs,
>> >> but they would mostly look very similar to the current ones generated
>> >> by 'make savedefconfig'.
>> >>
>> >> I don't know what makes you think Linus didn't want part one to be
>> >> done. He wanted the Kconfig.rx51 to be human readable, and you are
>> >> proposing that TWL4030_USB should be manually added there... I don't
>> >> think TWL4030_USB says anything to human beings.
>> >
>> > I don't read anywhere Linus saying we should clutter drivers/*/Kconfig
>> > with defaults. I think you're inverting things, it's much better
>> > to select TWL4030_USB when you enable MACH_OMAP_RX51 then default y
>> > if MACH_OMAP_RX51.
>> 
>> So say, if you have 100 board defconfigs with OMAP3 chips and TWL4030,
>> each and every one of them should select both USB_MUSB_HDRC and
>> TWL4030_USB?
>> 
>> And you are saying if somebody is doing 'make menuconfig', that person
>> should somehow know that _obviously_ he needs TWL4030_USB in order to
>> use USB_MUSB_HDRC, or USB_G_NOKIA?
>
> Hey we're already dealing with stuff like this :)
>
> See ARCH_OMAP2PLUS_TYPICAL in arch/arm/mach-omap2/Kconfig:
>
> config ARCH_OMAP2PLUS_TYPICAL
>         bool "Typical OMAP configuration"
>         default y
>         select AEABI
>         select REGULATOR
>         select PM
>         select PM_RUNTIME
>         select VFP
>         select NEON if ARCH_OMAP3 || ARCH_OMAP4
>         select SERIAL_OMAP
>         select SERIAL_OMAP_CONSOLE
>         select I2C
>         select I2C_OMAP
>         select MFD
>         select MENELAUS if ARCH_OMAP2
>         select TWL4030_CORE if ARCH_OMAP3 || ARCH_OMAP4
>         select TWL4030_POWER if ARCH_OMAP3 || ARCH_OMAP4
>         help
>           Compile a kernel suitable for booting most boards
>
> This actually produces a booting omap2+ kernel with no .config
> at all. I've tested it with:
>
> $ echo CONFIG_ARCH_OMAP=y > .config
> $ yes "" | ARCH=arm make oldconfig
>
> So if you have something that's needed on almost every omap,
> you can select it there.

One thing I don't like about this is that there is currently no way to
'select' as a module.

In order to get to a minimal kernel, in theory, we don't need regulator,
i2c, MFD, or twl4030* built in.  They should all be built as modules.
Yes, there are some assumptions made today that some of these things are
built in, and then are used from init code, but that is broken.

If someone is looking for a good omap-kernel-janitors project, I would
suggest getting to a *really* minimal OMAP2+ kernel with everything
possible built as modules to be a very good project.

Kevin

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Tony Lindgren]

* Kevin Hilman <khilman@deeprootsystems.com> [101019 17:04]:
> Tony Lindgren <tony@atomide.com> writes:
> >
> > This actually produces a booting omap2+ kernel with no .config
> > at all. I've tested it with:
> >
> > $ echo CONFIG_ARCH_OMAP=y > .config
> > $ yes "" | ARCH=arm make oldconfig
> >
> > So if you have something that's needed on almost every omap,
> > you can select it there.
> 
> One thing I don't like about this is that there is currently no way to
> 'select' as a module.

I think that would take two passes to do currently for the depenencies..
But we can certainly select all the omap drivers as modules in
omap2plus_defconfig.

> In order to get to a minimal kernel, in theory, we don't need regulator,
> i2c, MFD, or twl4030* built in.  They should all be built as modules.
> Yes, there are some assumptions made today that some of these things are
> built in, and then are used from init code, but that is broken.

I think the only thing keeping us from doing that is that most of
the boards don't use initramfs. Without initramfs these are needed
to mount root on MMC.
 
> If someone is looking for a good omap-kernel-janitors project, I would
> suggest getting to a *really* minimal OMAP2+ kernel with everything
> possible built as modules to be a very good project.

Agreed. The way to do that is to set everything possible as modules
in omap2plus_defconfig, and move to kernel + initramfs based booting
like most distros do.

Regards,

Tony

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

Hi,

On Tue, Oct 19, 2010 at 07:13:07PM -0500, Kevin Hilman wrote:
>One thing I don't like about this is that there is currently no way to
>'select' as a module.

Yeah, I remember suggesting to add "module" to Kconfig language which
would do the job of selecting as module.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[felipe.contreras]

On Wed, Oct 20, 2010 at 3:13 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Tony Lindgren <tony@atomide.com> writes:
>> So if you have something that's needed on almost every omap,
>> you can select it there.
>
> One thing I don't like about this is that there is currently no way to
> 'select' as a module.
>
> In order to get to a minimal kernel, in theory, we don't need regulator,
> i2c, MFD, or twl4030* built in.  They should all be built as modules.
> Yes, there are some assumptions made today that some of these things are
> built in, and then are used from init code, but that is broken.
>
> If someone is looking for a good omap-kernel-janitors project, I would
> suggest getting to a *really* minimal OMAP2+ kernel with everything
> possible built as modules to be a very good project.

Yes, one use-case would be to select things as modules by default only
if ARCH_OMAP2PLUS_TYPICAL is selected.

But not everyone needs many of these things, I expect to be able to do
ARCH_OMAP2PLUS_TYPICAL=n, and manually enable USB stuff
USB_MUSB_HDRC=y, without having to know the dependencies myself.

That was the idea: simplify the defconfigs so that they are truly
minimal (they boot), and then people can add whatever they need (like
USB).

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Roger Quadros]

Hi Felipe,

On 10/19/2010 01:03 PM, ext Felipe Contreras wrote:
> Add missing dependencies, and remove redundant ones, and select sensible
> defaults.
>
> Signed-off-by: Felipe Contreras<felipe.contreras@gmail.com>
> ---
>   drivers/mfd/Kconfig        |    3 ++-
>   drivers/regulator/Kconfig  |    1 +
>   drivers/usb/gadget/Kconfig |    3 +--
>   drivers/usb/musb/Kconfig   |    6 ++----
>   drivers/usb/otg/Kconfig    |    2 ++
>   5 files changed, 8 insertions(+), 7 deletions(-)
>

> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index c530bc6..2aac57e 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -111,7 +111,6 @@ config	USB_GADGET_SELECTED
>   #
>   choice
>   	prompt "USB Peripheral Controller"
> -	depends on USB_GADGET
>   	help
>   	   A USB device uses a controller to talk to its host.
>   	   Systems should have only one such upstream link.
> @@ -191,7 +190,6 @@ config USB_GADGET_OMAP
>   	boolean "OMAP USB Device Controller"
>   	depends on ARCH_OMAP
>   	select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3 || MACH_OMAP_H4_OTG
> -	select USB_OTG_UTILS if ARCH_OMAP
>   	help
>   	   Many Texas Instruments OMAP processors have flexible full
>   	   speed USB device controllers, with support for up to 30
> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>   config USB_G_NOKIA
>   	tristate "Nokia composite gadget"
>   	depends on PHONET
> +	depends on USB_GADGET_MUSB_HDRC

This is wrong. Is there a build problem or run-time problem without this?

-- 
regards,
-roger

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

Hi Roger,

On Tue, Oct 19, 2010 at 4:40 PM, Roger Quadros <roger.quadros@nokia.com> wrote:
>> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>>  config USB_G_NOKIA
>>        tristate "Nokia composite gadget"
>>        depends on PHONET
>> +       depends on USB_GADGET_MUSB_HDRC
>
> This is wrong. Is there a build problem or run-time problem without this?

Try:
CONFIG_USB_G_NOKIA=y
CONFIG_USB_GADGET_MUSB_HDRC=n

At least g_nokia doesn't seem to work correctly on my N900.

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Roger Quadros]

On 10/19/2010 05:33 PM, ext Felipe Contreras wrote:
> Hi Roger,
>
> On Tue, Oct 19, 2010 at 4:40 PM, Roger Quadros<roger.quadros@nokia.com>  wrote:
>>> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>>>   config USB_G_NOKIA
>>>         tristate "Nokia composite gadget"
>>>         depends on PHONET
>>> +       depends on USB_GADGET_MUSB_HDRC
>>
>> This is wrong. Is there a build problem or run-time problem without this?
>
> Try:
> CONFIG_USB_G_NOKIA=y
> CONFIG_USB_GADGET_MUSB_HDRC=n
>
> At least g_nokia doesn't seem to work correctly on my N900.
>
It won't work on N900 because N900 has MUSB USB gadget controller and it needs 
to be selected somehow while building for N900.

You should set CONFIG_USB_GADGET_MUSB_HDRC to y somewhere else while building 
for N900 and not here because g_nokia is not MUSB specific.

-- 
regards,
-roger

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

On Wed, Oct 20, 2010 at 02:46:49AM -0500, Roger Quadros wrote:
>On 10/19/2010 05:33 PM, ext Felipe Contreras wrote:
>> Hi Roger,
>>
>> On Tue, Oct 19, 2010 at 4:40 PM, Roger Quadros<roger.quadros@nokia.com>  wrote:
>>>> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>>>>   config USB_G_NOKIA
>>>>         tristate "Nokia composite gadget"
>>>>         depends on PHONET
>>>> +       depends on USB_GADGET_MUSB_HDRC
>>>
>>> This is wrong. Is there a build problem or run-time problem without this?
>>
>> Try:
>> CONFIG_USB_G_NOKIA=y
>> CONFIG_USB_GADGET_MUSB_HDRC=n
>>
>> At least g_nokia doesn't seem to work correctly on my N900.
>>
>It won't work on N900 because N900 has MUSB USB gadget controller and
>it needs to be selected somehow while building for N900.
>
>You should set CONFIG_USB_GADGET_MUSB_HDRC to y somewhere else while
>building for N900 and not here because g_nokia is not MUSB specific.

It's the same thing I've been trying to say. No gadget driver is
controller-specific. Even though this one is called g_nokia.ko it
doesn't only Nokia can use it, neither does it mean nokia will always
use MUSB.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[felipe.contreras]

On Wed, Oct 20, 2010 at 10:46 AM, Roger Quadros <roger.quadros@nokia.com> wrote:
> On 10/19/2010 05:33 PM, ext Felipe Contreras wrote:
>> On Tue, Oct 19, 2010 at 4:40 PM, Roger Quadros<roger.quadros@nokia.com>
>>  wrote:
>>>>
>>>> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>>>>  config USB_G_NOKIA
>>>>        tristate "Nokia composite gadget"
>>>>        depends on PHONET
>>>> +       depends on USB_GADGET_MUSB_HDRC
>>>
>>> This is wrong. Is there a build problem or run-time problem without this?
>>
>> Try:
>> CONFIG_USB_G_NOKIA=y
>> CONFIG_USB_GADGET_MUSB_HDRC=n
>>
>> At least g_nokia doesn't seem to work correctly on my N900.
>>
> It won't work on N900 because N900 has MUSB USB gadget controller and it
> needs to be selected somehow while building for N900.
>
> You should set CONFIG_USB_GADGET_MUSB_HDRC to y somewhere else while
> building for N900 and not here because g_nokia is not MUSB specific.

How do you expect people to know that? The Kconfig should define what
USB_G_NOKIA needs to work, so that people can enable those things
without go Googling hunting for a workable defconfig. Right now people
can build g_nokia modules that don't work, and that shouldn't be
allowed by the Kconfig.

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Roger Quadros]

On 10/20/2010 11:53 AM, ext felipe.contreras@gmail.com wrote:
> On Wed, Oct 20, 2010 at 10:46 AM, Roger Quadros<roger.quadros@nokia.com>  wrote:
>> On 10/19/2010 05:33 PM, ext Felipe Contreras wrote:
>>> On Tue, Oct 19, 2010 at 4:40 PM, Roger Quadros<roger.quadros@nokia.com>
>>>   wrote:
>>>>>
>>>>> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>>>>>   config USB_G_NOKIA
>>>>>         tristate "Nokia composite gadget"
>>>>>         depends on PHONET
>>>>> +       depends on USB_GADGET_MUSB_HDRC
>>>>
>>>> This is wrong. Is there a build problem or run-time problem without this?
>>>
>>> Try:
>>> CONFIG_USB_G_NOKIA=y
>>> CONFIG_USB_GADGET_MUSB_HDRC=n
>>>
>>> At least g_nokia doesn't seem to work correctly on my N900.
>>>
>> It won't work on N900 because N900 has MUSB USB gadget controller and it
>> needs to be selected somehow while building for N900.
>>
>> You should set CONFIG_USB_GADGET_MUSB_HDRC to y somewhere else while
>> building for N900 and not here because g_nokia is not MUSB specific.
>
> How do you expect people to know that? The Kconfig should define what
> USB_G_NOKIA needs to work, so that people can enable those things
> without go Googling hunting for a workable defconfig. Right now people
> can build g_nokia modules that don't work, and that shouldn't be
> allowed by the Kconfig.
>

USB_G_NOKIA just needs a USB gadget controller to work. The gadget controller 
used for the board should come from the board's Kconfig which will ideally be 
supplied by the board's vendor.

For example, can't USB_GADGET_MUSB_HDRC be selected in MACH_NOKIA_RX51 in 
arch/arm/mach-omap2/Kconfig? or is there a better place to put it?

-- 
regards,
-roger

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Wed, Oct 20, 2010 at 12:14 PM, Roger Quadros <roger.quadros@nokia.com> wrote:
> On 10/20/2010 11:53 AM, ext felipe.contreras@gmail.com wrote:
>>
>> On Wed, Oct 20, 2010 at 10:46 AM, Roger Quadros<roger.quadros@nokia.com>
>>  wrote:
>>>
>>> On 10/19/2010 05:33 PM, ext Felipe Contreras wrote:
>>>>
>>>> On Tue, Oct 19, 2010 at 4:40 PM, Roger Quadros<roger.quadros@nokia.com>
>>>>  wrote:
>>>>>>
>>>>>> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>>>>>>  config USB_G_NOKIA
>>>>>>        tristate "Nokia composite gadget"
>>>>>>        depends on PHONET
>>>>>> +       depends on USB_GADGET_MUSB_HDRC
>>>>>
>>>>> This is wrong. Is there a build problem or run-time problem without
>>>>> this?
>>>>
>>>> Try:
>>>> CONFIG_USB_G_NOKIA=y
>>>> CONFIG_USB_GADGET_MUSB_HDRC=n
>>>>
>>>> At least g_nokia doesn't seem to work correctly on my N900.
>>>>
>>> It won't work on N900 because N900 has MUSB USB gadget controller and it
>>> needs to be selected somehow while building for N900.
>>>
>>> You should set CONFIG_USB_GADGET_MUSB_HDRC to y somewhere else while
>>> building for N900 and not here because g_nokia is not MUSB specific.
>>
>> How do you expect people to know that? The Kconfig should define what
>> USB_G_NOKIA needs to work, so that people can enable those things
>> without go Googling hunting for a workable defconfig. Right now people
>> can build g_nokia modules that don't work, and that shouldn't be
>> allowed by the Kconfig.
>>
>
> USB_G_NOKIA just needs a USB gadget controller to work. The gadget
> controller used for the board should come from the board's Kconfig which
> will ideally be supplied by the board's vendor.

Ok, but USB_OMAP is not supposed to work on the N900? (I tried and it didn't)

> For example, can't USB_GADGET_MUSB_HDRC be selected in MACH_NOKIA_RX51 in
> arch/arm/mach-omap2/Kconfig? or is there a better place to put it?

I don't think so, because people might not want USB at all. The ideal
case would be for USB_GADGET_MUSB_HDRC to be selected automatically
when the user selects USB and USB_GADGET, but that's not happening
because USB_GADGET_OMAP is selected first (all the dependencies are
met).

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Roger Quadros]

On 10/20/2010 12:23 PM, ext Felipe Contreras wrote:
> On Wed, Oct 20, 2010 at 12:14 PM, Roger Quadros<roger.quadros@nokia.com>  wrote:
>> On 10/20/2010 11:53 AM, ext felipe.contreras@gmail.com wrote:
>>>
>>> On Wed, Oct 20, 2010 at 10:46 AM, Roger Quadros<roger.quadros@nokia.com>
>>>   wrote:
>>>>
>>>> On 10/19/2010 05:33 PM, ext Felipe Contreras wrote:
>>>>>
>>>>> On Tue, Oct 19, 2010 at 4:40 PM, Roger Quadros<roger.quadros@nokia.com>
>>>>>   wrote:
>>>>>>>
>>>>>>> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>>>>>>>   config USB_G_NOKIA
>>>>>>>         tristate "Nokia composite gadget"
>>>>>>>         depends on PHONET
>>>>>>> +       depends on USB_GADGET_MUSB_HDRC
>>>>>>
>>>>>> This is wrong. Is there a build problem or run-time problem without
>>>>>> this?
>>>>>
>>>>> Try:
>>>>> CONFIG_USB_G_NOKIA=y
>>>>> CONFIG_USB_GADGET_MUSB_HDRC=n
>>>>>
>>>>> At least g_nokia doesn't seem to work correctly on my N900.
>>>>>
>>>> It won't work on N900 because N900 has MUSB USB gadget controller and it
>>>> needs to be selected somehow while building for N900.
>>>>
>>>> You should set CONFIG_USB_GADGET_MUSB_HDRC to y somewhere else while
>>>> building for N900 and not here because g_nokia is not MUSB specific.
>>>
>>> How do you expect people to know that? The Kconfig should define what
>>> USB_G_NOKIA needs to work, so that people can enable those things
>>> without go Googling hunting for a workable defconfig. Right now people
>>> can build g_nokia modules that don't work, and that shouldn't be
>>> allowed by the Kconfig.
>>>
>>
>> USB_G_NOKIA just needs a USB gadget controller to work. The gadget
>> controller used for the board should come from the board's Kconfig which
>> will ideally be supplied by the board's vendor.
>
> Ok, but USB_OMAP is not supposed to work on the N900? (I tried and it didn't)

No it won't work. USB_OMAP was for older OMAP's. MUSB is on OMAP3 and later. But 
MUSB is not limited to OMAP SoC.

>
>> For example, can't USB_GADGET_MUSB_HDRC be selected in MACH_NOKIA_RX51 in
>> arch/arm/mach-omap2/Kconfig? or is there a better place to put it?
>
> I don't think so, because people might not want USB at all. The ideal
> case would be for USB_GADGET_MUSB_HDRC to be selected automatically
> when the user selects USB and USB_GADGET, but that's not happening
> because USB_GADGET_OMAP is selected first (all the dependencies are
> met).
>
Yes you are right. In think with the current setup (i.e. controller selection at 
kconfig and limitation to one controller per config) we will never be able to 
satisfy all users.

We might need to move to some kind of gadget controller framework and runtime 
controller selection to solve this problem.

-- 
regards,
-roger

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Wed, Oct 20, 2010 at 3:33 PM, Roger Quadros <roger.quadros@nokia.com> wrote:
> On 10/20/2010 12:23 PM, ext Felipe Contreras wrote:
>> On Wed, Oct 20, 2010 at 12:14 PM, Roger Quadros<roger.quadros@nokia.com>
>>  wrote:
>>> USB_G_NOKIA just needs a USB gadget controller to work. The gadget
>>> controller used for the board should come from the board's Kconfig which
>>> will ideally be supplied by the board's vendor.
>>
>> Ok, but USB_OMAP is not supposed to work on the N900? (I tried and it
>> didn't)
>
> No it won't work. USB_OMAP was for older OMAP's. MUSB is on OMAP3 and later.
> But MUSB is not limited to OMAP SoC.

config USB_GADGET_OMAP
  depends ARCH_OMAP

So how about s/ARCH_OMAP/ARCH_OMAP1/

Then USB_GADGET_MUSB_HDRC would be selected by default on OMAP2+.

>>> For example, can't USB_GADGET_MUSB_HDRC be selected in MACH_NOKIA_RX51 in
>>> arch/arm/mach-omap2/Kconfig? or is there a better place to put it?
>>
>> I don't think so, because people might not want USB at all. The ideal
>> case would be for USB_GADGET_MUSB_HDRC to be selected automatically
>> when the user selects USB and USB_GADGET, but that's not happening
>> because USB_GADGET_OMAP is selected first (all the dependencies are
>> met).
>>
> Yes you are right. In think with the current setup (i.e. controller
> selection at kconfig and limitation to one controller per config) we will
> never be able to satisfy all users.
>
> We might need to move to some kind of gadget controller framework and
> runtime controller selection to solve this problem.

Definitely, for multi-omap. But for OMAP1/OMAP2+ setting ARCH_OMAP1 as
I just suggested would do the trick.

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Roger Quadros]

Hi,

On 10/20/2010 05:38 PM, ext Felipe Contreras wrote:
> On Wed, Oct 20, 2010 at 3:33 PM, Roger Quadros<roger.quadros@nokia.com>  wrote:
>> On 10/20/2010 12:23 PM, ext Felipe Contreras wrote:
>>> On Wed, Oct 20, 2010 at 12:14 PM, Roger Quadros<roger.quadros@nokia.com>
>>>   wrote:
>>>> USB_G_NOKIA just needs a USB gadget controller to work. The gadget
>>>> controller used for the board should come from the board's Kconfig which
>>>> will ideally be supplied by the board's vendor.
>>>
>>> Ok, but USB_OMAP is not supposed to work on the N900? (I tried and it
>>> didn't)
>>
>> No it won't work. USB_OMAP was for older OMAP's. MUSB is on OMAP3 and later.
>> But MUSB is not limited to OMAP SoC.
>
> config USB_GADGET_OMAP
>    depends ARCH_OMAP
>
> So how about s/ARCH_OMAP/ARCH_OMAP1/
>
> Then USB_GADGET_MUSB_HDRC would be selected by default on OMAP2+.
>

But I'm not sure that omap_udc is used only for OMAP1 architecture.
Dave/Tony should confirm this.


-- 
regards,
-roger

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

Hi,

On Thu, Oct 21, 2010 at 09:07:59AM -0500, Roger Quadros wrote:
>But I'm not sure that omap_udc is used only for OMAP1 architecture.
>Dave/Tony should confirm this.

Maybe the old omap2420_h4 board still uses it, but from 2430 I think the
ip isn't even there anymore.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [101021 08:32]:
> Hi,
> 
> On Thu, Oct 21, 2010 at 09:07:59AM -0500, Roger Quadros wrote:
> >But I'm not sure that omap_udc is used only for OMAP1 architecture.
> >Dave/Tony should confirm this.
> 
> Maybe the old omap2420_h4 board still uses it, but from 2430 I think the
> ip isn't even there anymore.

I don't think it's currently working on 2420. But still, the drivers
should be platform independent. All the drivers that might be possible
should at least build and not mess up things for other platforms.

Regards,

Tony

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Roger Quadros]

On 10/20/2010 12:23 PM, ext Felipe Contreras wrote:
> On Wed, Oct 20, 2010 at 12:14 PM, Roger Quadros<roger.quadros@nokia.com>  wrote:
>> On 10/20/2010 11:53 AM, ext felipe.contreras@gmail.com wrote:
>>>
>>> On Wed, Oct 20, 2010 at 10:46 AM, Roger Quadros<roger.quadros@nokia.com>
>>>   wrote:
>>>>
>>>> On 10/19/2010 05:33 PM, ext Felipe Contreras wrote:
>>>>>
>>>>> On Tue, Oct 19, 2010 at 4:40 PM, Roger Quadros<roger.quadros@nokia.com>
>>>>>   wrote:
>>>>>>>
>>>>>>> @@ -843,6 +841,7 @@ config USB_CDC_COMPOSITE
>>>>>>>   config USB_G_NOKIA
>>>>>>>         tristate "Nokia composite gadget"
>>>>>>>         depends on PHONET
>>>>>>> +       depends on USB_GADGET_MUSB_HDRC
>>>>>>
>>>>>> This is wrong. Is there a build problem or run-time problem without
>>>>>> this?
>>>>>
>>>>> Try:
>>>>> CONFIG_USB_G_NOKIA=y
>>>>> CONFIG_USB_GADGET_MUSB_HDRC=n
>>>>>
>>>>> At least g_nokia doesn't seem to work correctly on my N900.
>>>>>
>>>> It won't work on N900 because N900 has MUSB USB gadget controller and it
>>>> needs to be selected somehow while building for N900.
>>>>
>>>> You should set CONFIG_USB_GADGET_MUSB_HDRC to y somewhere else while
>>>> building for N900 and not here because g_nokia is not MUSB specific.
>>>
>>> How do you expect people to know that? The Kconfig should define what
>>> USB_G_NOKIA needs to work, so that people can enable those things
>>> without go Googling hunting for a workable defconfig. Right now people
>>> can build g_nokia modules that don't work, and that shouldn't be
>>> allowed by the Kconfig.
>>>
>>
>> USB_G_NOKIA just needs a USB gadget controller to work. The gadget
>> controller used for the board should come from the board's Kconfig which
>> will ideally be supplied by the board's vendor.
>
> Ok, but USB_OMAP is not supposed to work on the N900? (I tried and it didn't)

No it won't work. USB_OMAP was for older OMAP's. MUSB is on OMAP3 and later. But 
MUSB is not limited to OMAP SoC.

>
>> For example, can't USB_GADGET_MUSB_HDRC be selected in MACH_NOKIA_RX51 in
>> arch/arm/mach-omap2/Kconfig? or is there a better place to put it?
>
> I don't think so, because people might not want USB at all. The ideal
> case would be for USB_GADGET_MUSB_HDRC to be selected automatically
> when the user selects USB and USB_GADGET, but that's not happening
> because USB_GADGET_OMAP is selected first (all the dependencies are
> met).
>
Yes you are right. I think with the current setup (i.e. controller selection at 
kconfig and limitation to one controller per config) we will never be able to 
satisfy all users.

We might need to move to some kind of gadget controller framework and run time 
controller selection to solve this problem.

-- 
regards,
-roger

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

On Wed, Oct 20, 2010 at 04:23:13AM -0500, Felipe Contreras wrote:
>Ok, but USB_OMAP is not supposed to work on the N900? (I tried and it didn't)

ooooh, that's old. That's from the omap1. It's the old omap_udc.c

>I don't think so, because people might not want USB at all. The ideal
>case would be for USB_GADGET_MUSB_HDRC to be selected automatically
>when the user selects USB and USB_GADGET, but that's not happening
>because USB_GADGET_OMAP is selected first (all the dependencies are
>met).

And how is that different from:

default y if ARCH_OMAP3 ? If you don't want USB at all, you'll disable
it by hand, the default is on. Select on board Kconfig.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Wed, Oct 20, 2010 at 3:54 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Oct 20, 2010 at 04:23:13AM -0500, Felipe Contreras wrote:
>>
>> Ok, but USB_OMAP is not supposed to work on the N900? (I tried and it
>> didn't)
>
> ooooh, that's old. That's from the omap1. It's the old omap_udc.c

Ok, then the dependency should be ARCH_OMAP1, not ARCH_OMAP.

>> I don't think so, because people might not want USB at all. The ideal
>> case would be for USB_GADGET_MUSB_HDRC to be selected automatically
>> when the user selects USB and USB_GADGET, but that's not happening
>> because USB_GADGET_OMAP is selected first (all the dependencies are
>> met).
>
> And how is that different from:
>
> default y if ARCH_OMAP3 ? If you don't want USB at all, you'll disable
> it by hand, the default is on. Select on board Kconfig.

TWL4030_USB is inside drivers/usb/Kconfig and drivers/usb/otg/Kconfig,
which means in order to achieve the same on the board config you would
have to do:

default y if USB_SUPPORT && (USB || USB_GADGET) && ARCH_OMAP3

Seems to be more convoluted to me.

Besides, I think TWL4030_USB and the full description of it doesn't
give any hints to the people reading drivers/usb/otg/Kconfig what is
it used for. 'default y if ARCH_OMAP3' achieves that.

Plus, there is no big change:

config USB_MUSB_HDRC
-	select TWL4030_USB if MACH_OMAP_3430SDP

config TWL4030_USB
+	default y if ARCH_OMAP3

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

hi,

On Wed, Oct 20, 2010 at 09:52:06AM -0500, Felipe Contreras wrote:
>TWL4030_USB is inside drivers/usb/Kconfig and drivers/usb/otg/Kconfig,
>which means in order to achieve the same on the board config you would
>have to do:
>
>default y if USB_SUPPORT && (USB || USB_GADGET) && ARCH_OMAP3
>
>Seems to be more convoluted to me.
>
>Besides, I think TWL4030_USB and the full description of it doesn't
>give any hints to the people reading drivers/usb/otg/Kconfig what is
>it used for. 'default y if ARCH_OMAP3' achieves that.

no it doesn't. No user will 'cat drivers/usb/otg/Kconfig | grep -A 10
TWL4030' and check the 'default y if ARCH_OMAP3

if you think help is not enough, fix help.

>Plus, there is no big change:
>
>config USB_MUSB_HDRC
>-	select TWL4030_USB if MACH_OMAP_3430SDP
>
>config TWL4030_USB
>+	default y if ARCH_OMAP3
>

what's the problem with:

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index ab784bf..3af9580 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -215,6 +215,7 @@ config MACH_NOKIA_RX51
         depends on ARCH_OMAP3
         default y
         select OMAP_PACKAGE_CBB
+       select TWL4030_USB
  
  config MACH_OMAP_ZOOM2
         bool "OMAP3 Zoom2 board"

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Wed, Oct 20, 2010 at 6:15 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Oct 20, 2010 at 09:52:06AM -0500, Felipe Contreras wrote:
>>
>> TWL4030_USB is inside drivers/usb/Kconfig and drivers/usb/otg/Kconfig,
>> which means in order to achieve the same on the board config you would
>> have to do:
>>
>> default y if USB_SUPPORT && (USB || USB_GADGET) && ARCH_OMAP3
>>
>> Seems to be more convoluted to me.
>>
>> Besides, I think TWL4030_USB and the full description of it doesn't
>> give any hints to the people reading drivers/usb/otg/Kconfig what is
>> it used for. 'default y if ARCH_OMAP3' achieves that.
>
> no it doesn't. No user will 'cat drivers/usb/otg/Kconfig | grep -A 10
> TWL4030' and check the 'default y if ARCH_OMAP3
>
> if you think help is not enough, fix help.

Users would not need to care... if they enable USB and USB_GADGET, it
will be selected automatically for OMAP3, which is what users would
expect.

I was talking more about developers.

How should the help be fixed?
"Select y if you are using an OMAP3 system that has TWL4030"

>> Plus, there is no big change:
>>
>> config USB_MUSB_HDRC
>> -       select TWL4030_USB if MACH_OMAP_3430SDP
>>
>> config TWL4030_USB
>> +       default y if ARCH_OMAP3
>>
>
> what's the problem with:
>
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index ab784bf..3af9580 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -215,6 +215,7 @@ config MACH_NOKIA_RX51
>        depends on ARCH_OMAP3
>        default y
>        select OMAP_PACKAGE_CBB
> +       select TWL4030_USB
>  config MACH_OMAP_ZOOM2
>        bool "OMAP3 Zoom2 board"
>

1) Do you think all the OMAP3 boards should add that?
2) What if you don't want to use USB?

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

Hi,

On Wed, Oct 20, 2010 at 11:22:17AM -0500, Felipe Contreras wrote:
>Users would not need to care... if they enable USB and USB_GADGET, it
>will be selected automatically for OMAP3, which is what users would
>expect.

wouldn't a user expect USB to be enabled if the board _has_ a USB
connector ?

>I was talking more about developers.
>
>How should the help be fixed?
>"Select y if you are using an OMAP3 system that has TWL4030"

If you have the TWL4030 family of PMIC chips and want to use USB, you
probably need this. If unsure, say Y.

>1) Do you think all the OMAP3 boards should add that?

why not ?

>2) What if you don't want to use USB?

what if you don't want to use USB and we use your choice of:

default y if ARCH_OMAP3 ??

what's the big difference ? The default will be "USB enabled" and if you
don't want, just drop it. The thing is that in most cases we want to
support what is wired on the board. RX51 has a USB micro-B connector, so
by default we want USB peripheral enabled and host disabled. If a user
(or developer for that matter) wants to play with the unsupported host
mode on RX51, he will enable that part by himself and pray for it to
work. If, on the other hand, a camera developer doesn't need USB at all
and decides to save a few bytes on the binary, he will make sure to
disable it.

The thing is that there's no way to make everybody happy, so we stick to
what the board is wired to do.

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Wed, Oct 20, 2010 at 8:54 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Oct 20, 2010 at 11:22:17AM -0500, Felipe Contreras wrote:
>>
>> Users would not need to care... if they enable USB and USB_GADGET, it
>> will be selected automatically for OMAP3, which is what users would
>> expect.
>
> wouldn't a user expect USB to be enabled if the board _has_ a USB
> connector ?

Not if it's not going to be used.

>> I was talking more about developers.
>>
>> How should the help be fixed?
>> "Select y if you are using an OMAP3 system that has TWL4030"
>
> If you have the TWL4030 family of PMIC chips and want to use USB, you
> probably need this. If unsure, say Y.

If you see TWL4030_USB, it's because you have enabled USB_SUPPORT, so
you want to use USB. And that help still doesn't say anything to most
people.

>> 1) Do you think all the OMAP3 boards should add that?
>
> why not ?

You prefer a patch that adds one line per board rather than a patch
that adds one line in total... That is exactly the opposite of Linus's
complaint regarding big ARM changesets.

>> 2) What if you don't want to use USB?
>
> what if you don't want to use USB and we use your choice of:
>
> default y if ARCH_OMAP3 ??
>
> what's the big difference ? The default will be "USB enabled" and if you
> don't want, just drop it. The thing is that in most cases we want to
> support what is wired on the board.

For "most cases" there is 'default'.

> RX51 has a USB micro-B connector, so
> by default we want USB peripheral enabled and host disabled. If a user
> (or developer for that matter) wants to play with the unsupported host
> mode on RX51, he will enable that part by himself and pray for it to
> work. If, on the other hand, a camera developer doesn't need USB at all
> and decides to save a few bytes on the binary, he will make sure to
> disable it.

Your patch would not allow such camera developer to disable USB
through the kernel configuration.

> The thing is that there's no way to make everybody happy, so we stick to
> what the board is wired to do.

I don't understand, currently TWL4030_USB is not selected in any way,
you are saying it should always be enabled (which I disagree with),
but that's not currently happening. If anything, my patch is changing
things to go into the direction you want, not the opposite.

My patch shuffles _one_ line that:
1) Allows clueless OMAP3 users to have TWL4030_USB enabled by default
_if_ they enable USB and USB_GADGET regardless of which board is being
used
2) Still allow people to build kernels without USB, nor TWL4030_USB
3) Simplify defconfgs by removing one line
4) Don't need extra changes in the board configurations
5) Doesn't increase the total lines in Kconfigs

Compared to the current situation 1) would actually change things into
the direction you advocated to: TWL4030_USB is enabled by default.

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

hi,

On Thu, Oct 21, 2010 at 04:04:27AM -0500, Felipe Contreras wrote:
>> wouldn't a user expect USB to be enabled if the board _has_ a USB
>> connector ?
>
>Not if it's not going to be used.

and how would you know before hand if it's going to be used or not ?

>>> 1) Do you think all the OMAP3 boards should add that?
>>
>> why not ?
>
>You prefer a patch that adds one line per board rather than a patch
>that adds one line in total... That is exactly the opposite of Linus's
>complaint regarding big ARM changesets.

The thing is that what you are proposing will not scale when we have too
many boards with different transceivers. Then we will end up with things
like:

config USB_ARCH_HAS_OHCI
	boolean
	# ARM:
	default y if SA1111
	default y if ARCH_OMAP
	default y if ARCH_LH7A404
	default y if ARCH_S3C2410
	default y if PXA27x
	default y if PXA3xx
	default y if ARCH_EP93XX
	default y if ARCH_AT91
	default y if ARCH_PNX4008 && I2C
	default y if MFD_TC6393XB
	default y if ARCH_W90X900
	default y if ARCH_DAVINCI_DA8XX
	# PPC:
	default y if STB03xxx
	default y if PPC_MPC52xx
	# MIPS:
	default y if MIPS_ALCHEMY
	default y if MACH_JZ4740
	# SH:
	default y if CPU_SUBTYPE_SH7720
	default y if CPU_SUBTYPE_SH7721
	default y if CPU_SUBTYPE_SH7763
	default y if CPU_SUBTYPE_SH7786
	# more:
	default PCI

which to me would be much better that e.g. SA1111 does a select
USB_ARCH_HAS_OHCI like it's done for HAVE_CLK

-- 
balbi

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

Hi,

On Thu, Oct 21, 2010 at 1:24 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Oct 21, 2010 at 04:04:27AM -0500, Felipe Contreras wrote:
>>>
>>> wouldn't a user expect USB to be enabled if the board _has_ a USB
>>> connector ?
>>
>> Not if it's not going to be used.
>
> and how would you know before hand if it's going to be used or not ?

You don't, that's what Kconfig is for: so the user can dynamically
select what he wants or not on the kernel. And in order for it to be
useful, dependencies have to be correct.

>>>> 1) Do you think all the OMAP3 boards should add that?
>>>
>>> why not ?
>>
>> You prefer a patch that adds one line per board rather than a patch
>> that adds one line in total... That is exactly the opposite of Linus's
>> complaint regarding big ARM changesets.
>
> The thing is that what you are proposing will not scale when we have too
> many boards with different transceivers. Then we will end up with things
> like:
>
> config USB_ARCH_HAS_OHCI

[...]

We are not talking about USB_ARCH_HAS_OHCI, we are talking about
TWL4030_USB, there's nothing to scale there.

-- 
Felipe Contreras

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Greg KH]

On Tue, Oct 19, 2010 at 01:03:28PM +0300, Felipe Contreras wrote:
> Add missing dependencies, and remove redundant ones, and select sensible
> defaults.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  drivers/mfd/Kconfig        |    3 ++-
>  drivers/regulator/Kconfig  |    1 +
>  drivers/usb/gadget/Kconfig |    3 +--
>  drivers/usb/musb/Kconfig   |    6 ++----
>  drivers/usb/otg/Kconfig    |    2 ++
>  5 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index db51ea1..0bbaf01 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -156,6 +156,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

Sorry, but unless the driver is somehow required to keep the board from
going up in a puff of smoke, a default should NEVER be y.

Because of that, I can't take this patch.

greg k-h

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Anand Gadiyar]

On 10/19/2010 9:22 PM, Greg KH wrote:
> On Tue, Oct 19, 2010 at 01:03:28PM +0300, Felipe Contreras wrote:
>> Add missing dependencies, and remove redundant ones, and select sensible
>> defaults.
>>
>> Signed-off-by: Felipe Contreras<felipe.contreras@gmail.com>
>> ---
>>   drivers/mfd/Kconfig        |    3 ++-
>>   drivers/regulator/Kconfig  |    1 +
>>   drivers/usb/gadget/Kconfig |    3 +--
>>   drivers/usb/musb/Kconfig   |    6 ++----
>>   drivers/usb/otg/Kconfig    |    2 ++
>>   5 files changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index db51ea1..0bbaf01 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -156,6 +156,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
>
> Sorry, but unless the driver is somehow required to keep the board from
> going up in a puff of smoke, a default should NEVER be y.
>
> Because of that, I can't take this patch.
>

This is the driver for the Power-management IC on the board. Without 
this IC being correctly configured, I'm guessing a lot of things will 
fail. Not so sure if anything will go up in smoke (sounds unlikely), or 
if the kernel will fail to boot - I'll check quickly - but for this one 
specific instance, I think it's reasonable to accept a default y.

Kevin/Tony can probably confirm.

Greg,

For other instances of unnecessary "default y", do you think they should 
be removed?

- Anand

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Greg KH]

On Tue, Oct 19, 2010 at 09:42:17PM +0530, Anand Gadiyar wrote:
> On 10/19/2010 9:22 PM, Greg KH wrote:
> >On Tue, Oct 19, 2010 at 01:03:28PM +0300, Felipe Contreras wrote:
> >>Add missing dependencies, and remove redundant ones, and select sensible
> >>defaults.
> >>
> >>Signed-off-by: Felipe Contreras<felipe.contreras@gmail.com>
> >>---
> >>  drivers/mfd/Kconfig        |    3 ++-
> >>  drivers/regulator/Kconfig  |    1 +
> >>  drivers/usb/gadget/Kconfig |    3 +--
> >>  drivers/usb/musb/Kconfig   |    6 ++----
> >>  drivers/usb/otg/Kconfig    |    2 ++
> >>  5 files changed, 8 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >>index db51ea1..0bbaf01 100644
> >>--- a/drivers/mfd/Kconfig
> >>+++ b/drivers/mfd/Kconfig
> >>@@ -156,6 +156,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
> >
> >Sorry, but unless the driver is somehow required to keep the board from
> >going up in a puff of smoke, a default should NEVER be y.
> >
> >Because of that, I can't take this patch.
> >
> 
> This is the driver for the Power-management IC on the board. Without
> this IC being correctly configured, I'm guessing a lot of things
> will fail. Not so sure if anything will go up in smoke (sounds
> unlikely), or if the kernel will fail to boot - I'll check quickly -
> but for this one specific instance, I think it's reasonable to
> accept a default y.

Sorry, but no, I'm not going to take it for the reasons stated above.

> Kevin/Tony can probably confirm.
> 
> Greg,
> 
> For other instances of unnecessary "default y", do you think they
> should be removed?

I wouldn't worry about trivial things like that, the damage is already
done.

thanks,

greg k-h

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Kevin Hilman]

Anand Gadiyar <gadiyar@ti.com> writes:

> On 10/19/2010 9:22 PM, Greg KH wrote:
>> On Tue, Oct 19, 2010 at 01:03:28PM +0300, Felipe Contreras wrote:
>>> Add missing dependencies, and remove redundant ones, and select sensible
>>> defaults.
>>>
>>> Signed-off-by: Felipe Contreras<felipe.contreras@gmail.com>
>>> ---
>>>   drivers/mfd/Kconfig        |    3 ++-
>>>   drivers/regulator/Kconfig  |    1 +
>>>   drivers/usb/gadget/Kconfig |    3 +--
>>>   drivers/usb/musb/Kconfig   |    6 ++----
>>>   drivers/usb/otg/Kconfig    |    2 ++
>>>   5 files changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>> index db51ea1..0bbaf01 100644
>>> --- a/drivers/mfd/Kconfig
>>> +++ b/drivers/mfd/Kconfig
>>> @@ -156,6 +156,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
>>
>> Sorry, but unless the driver is somehow required to keep the board from
>> going up in a puff of smoke, a default should NEVER be y.
>>
>> Because of that, I can't take this patch.
>>
>
> This is the driver for the Power-management IC on the board. Without
> this IC being correctly configured, I'm guessing a lot of things will
> fail. Not so sure if anything will go up in smoke (sounds unlikely),
> or if the kernel will fail to boot - I'll check quickly - but for this
> one specific instance, I think it's reasonable to accept a default y.
>
> Kevin/Tony can probably confirm.

I don't thing the 'default y' should be done at this level.  Instead,
the boards/SoCs that have the hard requirement should do the 'select'

Kevin