mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@ti.com>
To: Vivek Gautam <gautam.vivek@samsung.com>
Cc: <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-omap@vger.kernel.org>, <gregkh@linuxfoundation.org>,
	<balbi@ti.com>, Doug Anderson <dianders@chromium.org>
Subject: Re: [PATCH RFC] usb: dwc3: Remove dwc3 dependency on gadget.
Date: Thu, 10 Jan 2013 15:02:38 +0200	[thread overview]
Message-ID: <20130110130238.GW28920@arwen.pp.htv.fi> (raw)
In-Reply-To: <1356357513-8892-1-git-send-email-gautam.vivek@samsung.com>

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

Hi,

On Mon, Dec 24, 2012 at 07:28:33PM +0530, Vivek Gautam wrote:
> DWC3 controller curretly depends on CONFIG_USB and CONFIG_USB_GADGET.
> Some hardware may like to use only host feature on dwc3.
> So, removing the dependency of USB_DWC3 on USB_GADGET
> and further modulating the dwc3 core to enable gadget features
> only with USB_GADGET.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> CC: Doug Anderson <dianders@chromium.org>

right, right... Eventually we need to do it, but you're only making
gadget side optional. Host side should be optional too, but then you
need to make sure we don't build dwc3 without gadget and host.

Maybe make a mode selection for Host-only, Peripheral-only, Dual-Role
Device ??

More comments below...

> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index f6a6e07..b70dcf1 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -1,7 +1,7 @@
>  config USB_DWC3
>  	tristate "DesignWare USB3 DRD Core Support"
> -	depends on (USB && USB_GADGET)
> -	select USB_OTG_UTILS
> +	depends on USB
> +	select USB_OTG_UTILS if USB_GADGET

we want USB_OTG_UTILS also on host-only builds because host side,
eventually, will have to learn how to control its PHYs.

>  	select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
>  	help
>  	  Say Y or M here if your system has a Dual Role SuperSpeed
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 4502648..8f469cb 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -5,7 +5,7 @@ obj-$(CONFIG_USB_DWC3)			+= dwc3.o
>  
>  dwc3-y					:= core.o
>  dwc3-y					+= host.o
> -dwc3-y					+= gadget.o ep0.o
> +obj-$(CONFIG_USB_GADGET)               += gadget.o ep0.o

this is wrong. CONFIG_USB_GADGET is tristate, so this should be:

ifneq($(CONFIG_USB_GADGET),)
 dwc3-y	+= gadget.o ep0.o
endif

or something similar

> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 4999563..4dc7ef2 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -865,7 +865,14 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
>  int dwc3_host_init(struct dwc3 *dwc);
>  void dwc3_host_exit(struct dwc3 *dwc);
>  
> +#ifdef CONFIG_USB_GADGET

not taking into account module builds.

>  int dwc3_gadget_init(struct dwc3 *dwc);
>  void dwc3_gadget_exit(struct dwc3 *dwc);
> +#else
> +static inline int dwc3_gadget_init(struct dwc3 *dwc)
> +{ return -EINVAL; }
> +static inline void dwc3_gadget_exit(struct dwc3 *dwc)
> +{ }
> +#endif
>  
>  #endif /* __DRIVERS_USB_DWC3_CORE_H */
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index d4a30f1..553bbaa 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -535,7 +535,8 @@ static ssize_t dwc3_testmode_write(struct file *file,
>  		testmode = 0;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
> -	dwc3_gadget_set_test_mode(dwc, testmode);
> +	if (dwc3_gadget_set_test_mode(dwc, testmode))
> +		dev_dbg(dwc->dev, "host: Invalid request\n");
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
>  	return count;

wrong, if you don't have gadget mode, you just don't create this file.

> @@ -638,7 +639,8 @@ static ssize_t dwc3_link_state_write(struct file *file,
>  		return -EINVAL;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
> -	dwc3_gadget_set_link_state(dwc, state);
> +	if (dwc3_gadget_set_link_state(dwc, state))
> +		dev_dbg(dwc->dev, "host: Invalid request\n");

likewise.

>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
>  	return count;
> diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
> index 99e6d72..28e82db 100644
> --- a/drivers/usb/dwc3/gadget.h
> +++ b/drivers/usb/dwc3/gadget.h
> @@ -105,8 +105,16 @@ static inline void dwc3_gadget_move_request_queued(struct dwc3_request *req)
>  void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>  		int status);
>  
> +#ifdef CONFIG_USB_GADGET

not taking into account CONFIG_USB_GADGET=m

-- 
balbi

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

  reply	other threads:[~2013-01-10 13:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-24 13:58 Vivek Gautam
2013-01-10 13:02 ` Felipe Balbi [this message]
2013-01-11 13:43   ` Vivek Gautam
2013-01-11 13:59     ` Felipe Balbi
2013-01-11 14:28       ` Vivek Gautam
2013-01-14  7:57         ` Felipe Balbi
2013-01-14  9:12           ` Vivek Gautam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130110130238.GW28920@arwen.pp.htv.fi \
    --to=balbi@ti.com \
    --cc=dianders@chromium.org \
    --cc=gautam.vivek@samsung.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®