Re: [PATCH v2 07/11] usb: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes

2013-04-10 Thread Peter Korsgaard
> "Daniel" == Daniel Mack  writes:

 Daniel> This makes building the actual object files optional to the selected
 Daniel> mode, which saves users who know which kind of USB mode support they
 Daniel> need some binary size.

 Daniel> Unimplemented functions are stubbed out with static inline functions.

 Daniel> Signed-off-by: Daniel Mack 
 Daniel> ---
 Daniel>  drivers/usb/musb/Kconfig   | 29 +
 Daniel>  drivers/usb/musb/Makefile  | 10 --
 Daniel>  drivers/usb/musb/musb_gadget.h | 21 +
 Daniel>  drivers/usb/musb/musb_host.h   | 29 +++--
 Daniel>  4 files changed, 85 insertions(+), 4 deletions(-)

 Daniel> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
 Daniel> index 47442d3..aab1568 100644
 Daniel> --- a/drivers/usb/musb/Kconfig
 Daniel> +++ b/drivers/usb/musb/Kconfig
 Daniel> @@ -28,6 +28,35 @@ config USB_MUSB_HDRC
 Daniel>  if USB_MUSB_HDRC
 
 Daniel>  choice
 Daniel> +  bool "MUSB Mode Selection"
 Daniel> +  default USB_MUSB_DUAL_ROLE if (USB && USB_GADGET)
 Daniel> +  default USB_MUSB_HOST if (USB && !USB_GADGET)
 Daniel> +  default USB_MUSB_GADGET if (!USB && USB_GADGET)
 Daniel> +
 Daniel> +config USB_MUSB_HOST
 Daniel> +  bool "Host only mode"
 Daniel> +  depends on USB
 Daniel> +  help
 Daniel> +Select this when you want to use MUSB in host mode only,
 Daniel> +thereby the gadget feature will be regressed.
 Daniel> +
 Daniel> +config USB_MUSB_GADGET
 Daniel> +  bool "Gadget only mode"
 Daniel> +  depends on USB_GADGET
 Daniel> +  help
 Daniel> +Select this when you want to use MUSB in gadget mode only,
 Daniel> +thereby the host feature will be regressed.
 Daniel> +
 Daniel> +config USB_MUSB_DUAL_ROLE
 Daniel> +  bool "Dual Role mode"
 Daniel> +  depends on (USB && USB_GADGET)
 Daniel> +  help
 Daniel> +This is the default mode of working of MUSB controller where
 Daniel> +both host and gadget features are enabled.
 Daniel> +
 Daniel> +endchoice
 Daniel> +
 Daniel> +choice
 Daniel>prompt "Platform Glue Layer"
 
 Daniel>  config USB_MUSB_DAVINCI
 Daniel> diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
 Daniel> index 3b85871..6b13a53 100644
 Daniel> --- a/drivers/usb/musb/Makefile
 Daniel> +++ b/drivers/usb/musb/Makefile
 Daniel> @@ -6,10 +6,16 @@ obj-$(CONFIG_USB_MUSB_HDRC) += musb_hdrc.o
 
 Daniel>  musb_hdrc-y := musb_core.o
 
 Daniel> -musb_hdrc-y   += musb_gadget_ep0.o 
musb_gadget.o
 Daniel> -musb_hdrc-y   += musb_virthub.o 
musb_host.o
 Daniel>  musb_hdrc-$(CONFIG_DEBUG_FS)  += musb_debugfs.o
 
 Daniel> +ifneq ($(filter y,$(CONFIG_USB_MUSB_HOST) 
$(CONFIG_USB_MUSB_DUAL_ROLE)),)
 Daniel> +  musb_hdrc-y += musb_virthub.o 
musb_host.o
 Daniel> +endif
 Daniel> +
 Daniel> +ifneq ($(filter y,$(CONFIG_USB_MUSB_GADGET) 
$(CONFIG_USB_MUSB_DUAL_ROLE)),)
 Daniel> +  musb_hdrc-y += musb_gadget_ep0.o 
musb_gadget.o
 Daniel> +endif

I believe unselected option values simply expand to the empty strings,
so these can just be:

musb_hdrc-$(CONFIG_USB_MUSB_HOST)$(CONFIG_USB_MUSB_DUAL_ROLE) += musb_virthub.o 
musb_host.o

musb_hdrc-$(CONFIG_USB_MUSB_GADGET)$(CONFIG_USB_MUSB_DUAL_ROLE) += 
musb_gadget_ep0.o musb_gadget.o

Other than that, it looks good:

Acked-by: Peter Korsgaard 

-- 
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 07/11] usb: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes

2013-04-08 Thread Daniel Mack
Hi Ravi,

On 08.04.2013 14:48, B, Ravi wrote:
>> On 08.04.2013 12:42, B, Ravi wrote:
>>>> Subject: [PATCH v2 07/11] usb: musb: add Kconfig options for 
>>>> HOST,
>> GAGDET
>>>> or DUAL_ROLE modes
>>>> 
>>>> This makes building the actual object files optional to the 
>>>> selected mode, which saves users who know which kind of USB 
>>>> mode support they need some binary size.
>>>> 
>>>> Unimplemented functions are stubbed out with static inline 
>>>> functions.
>>>> 
>>>> Signed-off-by: Daniel Mack  --- 
>>>> drivers/usb/musb/Kconfig   | 29 
>>>> + drivers/usb/musb/Makefile  |
>>>>  10 -- drivers/usb/musb/musb_gadget.h | 21 
>>>> + drivers/usb/musb/musb_host.h   | 29 
>>>> +++-- 4 files changed, 85 
>>>> insertions(+), 4 deletions(-)
>>>> 
>>>> diff --git a/drivers/usb/musb/Kconfig 
>>>> b/drivers/usb/musb/Kconfig index 47442d3..aab1568 100644 --- 
>>>> a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ 
>>>> -28,6 +28,35 @@ config USB_MUSB_HDRC if USB_MUSB_HDRC
>>>> 
>>>> choice +   bool "MUSB Mode Selection" +default 
>>>> USB_MUSB_DUAL_ROLE if (USB && USB_GADGET) +default 
>>>> USB_MUSB_HOST if (USB && !USB_GADGET) +default USB_MUSB_GADGET
>>>> if (!USB && USB_GADGET) + +config USB_MUSB_HOST +  bool "Host
>>>> only mode" +   depends on USB +help +Select this when you
>>>> want to use MUSB in host mode only, +thereby the gadget
>>>> feature will be regressed. + +config USB_MUSB_GADGET + bool
>>>> "Gadget only mode" +   depends on USB_GADGET + help + Select this
>>>> when you want to use MUSB in gadget mode only, + thereby the
>>>> host feature will be regressed. + +config USB_MUSB_DUAL_ROLE +
>>>> bool "Dual Role mode" +depends on (USB && USB_GADGET) +help +
>>>> This is the default mode of working of MUSB controller where +
>>>> both host and gadget features are enabled. + +endchoice
>>> 
>>> How do you cater for multi-instance support? Where one controller
>>> to
>> force as host and other as device (similar to am33x beagle 
>> configuration).
>>> In general for all combination possible like  >>  device>
>>   etc.
>> 
>> The actual mode chosen for an instance is passed in via pdata/DT. 
>> The Kconfig options only exist in order to stub out code that 
>> certainly isn't needed for a particular platform. IOW, to reduce 
>> the binary size.
>> 
>> So the beagleboard will have to select the DUAL_ROLE config 
>> parameters, where other board (like mine) can go for the HOST only
>>  option.
>> 
>> If a user selects a mode as runtime parameter that is stubbed out 
>> by the chosen config, it will simply don't do anything. Just as 
>> with a driver that is referenced from DT but not compiled in. I 
>> don't think we should be over-smart here and mess with the 
>> configured platform data. If the platform data is wrong, it has to
>>  be fixed anyway. Felipe, do you agree?
>> 
>> 
> 
> I understand, this is for binary code reduction. But in multi 
> instance case force host/device config option shall be selected only
>  when both controller need to configured as host only or device only.
>  like  or . In any other case dual-role 
> need to be chosen.

Yes, agreed.

> You can explain in help section of this Kconfig option.

Ok, I'll wait for more comments on this series and then add this to a
respin.


Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 07/11] usb: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes

2013-04-08 Thread B, Ravi
Hi Daniel

> 
> On 08.04.2013 12:42, B, Ravi wrote:
> >> Subject: [PATCH v2 07/11] usb: musb: add Kconfig options for HOST,
> GAGDET
> >> or DUAL_ROLE modes
> >>
> >> This makes building the actual object files optional to the selected
> >> mode, which saves users who know which kind of USB mode support they
> >> need some binary size.
> >>
> >> Unimplemented functions are stubbed out with static inline functions.
> >>
> >> Signed-off-by: Daniel Mack 
> >> ---
> >>  drivers/usb/musb/Kconfig   | 29 +
> >>  drivers/usb/musb/Makefile  | 10 --
> >>  drivers/usb/musb/musb_gadget.h | 21 +
> >>  drivers/usb/musb/musb_host.h   | 29 +++--
> >>  4 files changed, 85 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> >> index 47442d3..aab1568 100644
> >> --- a/drivers/usb/musb/Kconfig
> >> +++ b/drivers/usb/musb/Kconfig
> >> @@ -28,6 +28,35 @@ config USB_MUSB_HDRC
> >>  if USB_MUSB_HDRC
> >>
> >>  choice
> >> +  bool "MUSB Mode Selection"
> >> +  default USB_MUSB_DUAL_ROLE if (USB && USB_GADGET)
> >> +  default USB_MUSB_HOST if (USB && !USB_GADGET)
> >> +  default USB_MUSB_GADGET if (!USB && USB_GADGET)
> >> +
> >> +config USB_MUSB_HOST
> >> +  bool "Host only mode"
> >> +  depends on USB
> >> +  help
> >> +Select this when you want to use MUSB in host mode only,
> >> +thereby the gadget feature will be regressed.
> >> +
> >> +config USB_MUSB_GADGET
> >> +  bool "Gadget only mode"
> >> +  depends on USB_GADGET
> >> +  help
> >> +Select this when you want to use MUSB in gadget mode only,
> >> +thereby the host feature will be regressed.
> >> +
> >> +config USB_MUSB_DUAL_ROLE
> >> +  bool "Dual Role mode"
> >> +  depends on (USB && USB_GADGET)
> >> +  help
> >> +This is the default mode of working of MUSB controller where
> >> +both host and gadget features are enabled.
> >> +
> >> +endchoice
> >
> > How do you cater for multi-instance support? Where one controller to
> force as host and other as device (similar to am33x beagle configuration).
> > In general for all combination possible like  
>   etc.
> 
> The actual mode chosen for an instance is passed in via pdata/DT. The
> Kconfig options only exist in order to stub out code that certainly
> isn't needed for a particular platform. IOW, to reduce the binary size.
> 
> So the beagleboard will have to select the DUAL_ROLE config parameters,
> where other board (like mine) can go for the HOST only option.
> 
> If a user selects a mode as runtime parameter that is stubbed out by the
> chosen config, it will simply don't do anything. Just as with a driver
> that is referenced from DT but not compiled in. I don't think we should
> be over-smart here and mess with the configured platform data. If the
> platform data is wrong, it has to be fixed anyway. Felipe, do you agree?
> 
> 

I understand, this is for binary code reduction. But in multi instance case 
force host/device config option shall be selected only when both controller 
need to configured as host only or device only. like  or . In any other case dual-role need to be chosen.
You can explain in help section of this Kconfig option.

--
Ravi B
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 07/11] usb: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes

2013-04-08 Thread Daniel Mack
Hi Ravi,

On 08.04.2013 12:42, B, Ravi wrote:
>> Subject: [PATCH v2 07/11] usb: musb: add Kconfig options for HOST, GAGDET
>> or DUAL_ROLE modes
>>
>> This makes building the actual object files optional to the selected
>> mode, which saves users who know which kind of USB mode support they
>> need some binary size.
>>
>> Unimplemented functions are stubbed out with static inline functions.
>>
>> Signed-off-by: Daniel Mack 
>> ---
>>  drivers/usb/musb/Kconfig   | 29 +
>>  drivers/usb/musb/Makefile  | 10 --
>>  drivers/usb/musb/musb_gadget.h | 21 +
>>  drivers/usb/musb/musb_host.h   | 29 +++--
>>  4 files changed, 85 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>> index 47442d3..aab1568 100644
>> --- a/drivers/usb/musb/Kconfig
>> +++ b/drivers/usb/musb/Kconfig
>> @@ -28,6 +28,35 @@ config USB_MUSB_HDRC
>>  if USB_MUSB_HDRC
>>
>>  choice
>> +bool "MUSB Mode Selection"
>> +default USB_MUSB_DUAL_ROLE if (USB && USB_GADGET)
>> +default USB_MUSB_HOST if (USB && !USB_GADGET)
>> +default USB_MUSB_GADGET if (!USB && USB_GADGET)
>> +
>> +config USB_MUSB_HOST
>> +bool "Host only mode"
>> +depends on USB
>> +help
>> +  Select this when you want to use MUSB in host mode only,
>> +  thereby the gadget feature will be regressed.
>> +
>> +config USB_MUSB_GADGET
>> +bool "Gadget only mode"
>> +depends on USB_GADGET
>> +help
>> +  Select this when you want to use MUSB in gadget mode only,
>> +  thereby the host feature will be regressed.
>> +
>> +config USB_MUSB_DUAL_ROLE
>> +bool "Dual Role mode"
>> +depends on (USB && USB_GADGET)
>> +help
>> +  This is the default mode of working of MUSB controller where
>> +  both host and gadget features are enabled.
>> +
>> +endchoice
> 
> How do you cater for multi-instance support? Where one controller to force as 
> host and other as device (similar to am33x beagle configuration).
> In general for all combination possible like   
>   etc.

The actual mode chosen for an instance is passed in via pdata/DT. The
Kconfig options only exist in order to stub out code that certainly
isn't needed for a particular platform. IOW, to reduce the binary size.

So the beagleboard will have to select the DUAL_ROLE config parameters,
where other board (like mine) can go for the HOST only option.

If a user selects a mode as runtime parameter that is stubbed out by the
chosen config, it will simply don't do anything. Just as with a driver
that is referenced from DT but not compiled in. I don't think we should
be over-smart here and mess with the configured platform data. If the
platform data is wrong, it has to be fixed anyway. Felipe, do you agree?


Thanks,
Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 07/11] usb: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes

2013-04-08 Thread B, Ravi
Daniel

> Subject: [PATCH v2 07/11] usb: musb: add Kconfig options for HOST, GAGDET
> or DUAL_ROLE modes
> 
> This makes building the actual object files optional to the selected
> mode, which saves users who know which kind of USB mode support they
> need some binary size.
> 
> Unimplemented functions are stubbed out with static inline functions.
> 
> Signed-off-by: Daniel Mack 
> ---
>  drivers/usb/musb/Kconfig   | 29 +
>  drivers/usb/musb/Makefile  | 10 --
>  drivers/usb/musb/musb_gadget.h | 21 +
>  drivers/usb/musb/musb_host.h   | 29 +++--
>  4 files changed, 85 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> index 47442d3..aab1568 100644
> --- a/drivers/usb/musb/Kconfig
> +++ b/drivers/usb/musb/Kconfig
> @@ -28,6 +28,35 @@ config USB_MUSB_HDRC
>  if USB_MUSB_HDRC
> 
>  choice
> + bool "MUSB Mode Selection"
> + default USB_MUSB_DUAL_ROLE if (USB && USB_GADGET)
> + default USB_MUSB_HOST if (USB && !USB_GADGET)
> + default USB_MUSB_GADGET if (!USB && USB_GADGET)
> +
> +config USB_MUSB_HOST
> + bool "Host only mode"
> + depends on USB
> + help
> +   Select this when you want to use MUSB in host mode only,
> +   thereby the gadget feature will be regressed.
> +
> +config USB_MUSB_GADGET
> + bool "Gadget only mode"
> + depends on USB_GADGET
> + help
> +   Select this when you want to use MUSB in gadget mode only,
> +   thereby the host feature will be regressed.
> +
> +config USB_MUSB_DUAL_ROLE
> + bool "Dual Role mode"
> + depends on (USB && USB_GADGET)
> + help
> +   This is the default mode of working of MUSB controller where
> +   both host and gadget features are enabled.
> +
> +endchoice

How do you cater for multi-instance support? Where one controller to force as 
host and other as device (similar to am33x beagle configuration).
In general for all combination possible like   
  etc.

--
Ravi B
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 07/11] usb: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes

2013-04-05 Thread Daniel Mack
This makes building the actual object files optional to the selected
mode, which saves users who know which kind of USB mode support they
need some binary size.

Unimplemented functions are stubbed out with static inline functions.

Signed-off-by: Daniel Mack 
---
 drivers/usb/musb/Kconfig   | 29 +
 drivers/usb/musb/Makefile  | 10 --
 drivers/usb/musb/musb_gadget.h | 21 +
 drivers/usb/musb/musb_host.h   | 29 +++--
 4 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 47442d3..aab1568 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -28,6 +28,35 @@ config USB_MUSB_HDRC
 if USB_MUSB_HDRC
 
 choice
+   bool "MUSB Mode Selection"
+   default USB_MUSB_DUAL_ROLE if (USB && USB_GADGET)
+   default USB_MUSB_HOST if (USB && !USB_GADGET)
+   default USB_MUSB_GADGET if (!USB && USB_GADGET)
+
+config USB_MUSB_HOST
+   bool "Host only mode"
+   depends on USB
+   help
+ Select this when you want to use MUSB in host mode only,
+ thereby the gadget feature will be regressed.
+
+config USB_MUSB_GADGET
+   bool "Gadget only mode"
+   depends on USB_GADGET
+   help
+ Select this when you want to use MUSB in gadget mode only,
+ thereby the host feature will be regressed.
+
+config USB_MUSB_DUAL_ROLE
+   bool "Dual Role mode"
+   depends on (USB && USB_GADGET)
+   help
+ This is the default mode of working of MUSB controller where
+ both host and gadget features are enabled.
+
+endchoice
+
+choice
prompt "Platform Glue Layer"
 
 config USB_MUSB_DAVINCI
diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
index 3b85871..6b13a53 100644
--- a/drivers/usb/musb/Makefile
+++ b/drivers/usb/musb/Makefile
@@ -6,10 +6,16 @@ obj-$(CONFIG_USB_MUSB_HDRC) += musb_hdrc.o
 
 musb_hdrc-y := musb_core.o
 
-musb_hdrc-y+= musb_gadget_ep0.o 
musb_gadget.o
-musb_hdrc-y+= musb_virthub.o musb_host.o
 musb_hdrc-$(CONFIG_DEBUG_FS)   += musb_debugfs.o
 
+ifneq ($(filter y,$(CONFIG_USB_MUSB_HOST) $(CONFIG_USB_MUSB_DUAL_ROLE)),)
+   musb_hdrc-y += musb_virthub.o musb_host.o
+endif
+
+ifneq ($(filter y,$(CONFIG_USB_MUSB_GADGET) $(CONFIG_USB_MUSB_DUAL_ROLE)),)
+   musb_hdrc-y += musb_gadget_ep0.o 
musb_gadget.o
+endif
+
 # Hardware Glue Layer
 obj-$(CONFIG_USB_MUSB_OMAP2PLUS)   += omap2430.o
 obj-$(CONFIG_USB_MUSB_AM35X)   += am35x.o
diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
index 75f821c..0314dfc 100644
--- a/drivers/usb/musb/musb_gadget.h
+++ b/drivers/usb/musb/musb_gadget.h
@@ -37,6 +37,7 @@
 
 #include 
 
+#if IS_ENABLED(CONFIG_USB_MUSB_GADGET) || IS_ENABLED(CONFIG_USB_MUSB_DUAL_ROLE)
 extern irqreturn_t musb_g_ep0_irq(struct musb *);
 extern void musb_g_tx(struct musb *, u8);
 extern void musb_g_rx(struct musb *, u8);
@@ -48,6 +49,26 @@ extern void musb_g_disconnect(struct musb *);
 extern void musb_gadget_cleanup(struct musb *);
 extern int musb_gadget_setup(struct musb *);
 
+#else
+static inline irqreturn_t musb_g_ep0_irq(struct musb *musb)
+{
+   return 0;
+}
+
+static inline void musb_g_tx(struct musb *musb, u8 epnum)  {}
+static inline void musb_g_rx(struct musb *musb, u8 epnum)  {}
+static inline void musb_g_reset(struct musb *musb) {}
+static inline void musb_g_suspend(struct musb *musb)   {}
+static inline void musb_g_resume(struct musb *musb){}
+static inline void musb_g_wakeup(struct musb *musb){}
+static inline void musb_g_disconnect(struct musb *musb){}
+static inline void musb_gadget_cleanup(struct musb *musb)  {}
+static inline int musb_gadget_setup(struct musb *musb)
+{
+   return 0;
+}
+#endif
+
 enum buffer_map_state {
UN_MAPPED = 0,
PRE_MAPPED,
diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h
index e47035e..1ce6e4e 100644
--- a/drivers/usb/musb/musb_host.h
+++ b/drivers/usb/musb/musb_host.h
@@ -39,8 +39,6 @@
 
 #define musb_to_hcd(MUSB) ((MUSB)->hcd)
 
-extern struct musb *hcd_to_musb(struct usb_hcd *);
-
 /* stored in "usb_host_endpoint.hcpriv" for scheduled endpoints */
 struct musb_qh {
struct usb_host_endpoint *hep;  /* usbcore info */
@@ -78,6 +76,9 @@ static inline struct musb_qh *first_qh(struct list_head *q)
return list_entry(q->next, struct musb_qh, ring);
 }
 
+
+#if IS_ENABLED(CONFIG_USB_MUSB_HOST) || IS_ENABLED(CONFIG_USB_MUSB_DUAL_ROLE)
+extern struct musb *hcd_to_musb(struct usb_hcd *);
 extern irqreturn_t musb_h_ep0_irq(struct musb *);
 extern int musb_host_alloc(struct musb *);
 extern void musb_host_tx(struct musb *, u8);
@@ -90,6 +91,30 @@ extern void musb_host_rx(st