Re: Two remain problems at chipidea driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Mar 20, 2013 at 04:26:02PM +0200, Alexander Shishkin wrote:
> >> >> dr_cap is what the device can actually do (host, peripheral, etc). Tells
> >> >> us which roles to initialize and wether we can access OTGSC on this
> >> >> device.
> >> >> dr_mode is what function of the device we'll be using on this particular
> >> >> board.
> >> >
> >> > Sorry, I don't get why the driver needs to know what the chipidea can do
> >> > in theory (dr_cap). IMHO it should be sufficient to tell the driver what
> >> > that exact hardware it runs on can do (dr_mode). What the hardware can
> >> > do depends on the actual chipidea implementation used in that SoC and
> >> > the board the SoC is soldered on.
> >> 
> >> Again, see the discussion above.
> >> 
> >> In real world products (that is, phones and tablets as opposed to jolly
> >> fun development boards), vendors will want to limit the usb
> >> functionality to peripheral only or host only or whatever, because the
> >> middleware stack can only do one thing or because they don't want to go
> >> through with otg certification or you name it. Meanwhile, the controller
> >
> > that's not entirely true. A manufacturer can decide to skip OTG
> > certification but still support Dual Role. Look at the whole Android
> > Accessory Kit, for example.
> 
> Sure, I was just making an example of how device capabilities can differ
> from device's intended function.
> 
> >> and the whole device can still support otg. And we need to know that if
> >> we're to try to detect vbus session, because that is done via OTGSC
> >> which is only available in otg configurations.
> >
> > well, if it's only available in OTG configurations, then you make the
> > same assumption in driver. If driver was compiled with OTG, you check
> > OTGSC; otherwise don't.
> 
> I'd kind of like to support different configurations in runtime and have
> as few compilation options as possible. Of course, if it means extra
> spaghetti, there's a trade off right there.

right, that's what I did with drivers/usb/dwc3/, it helped cut down
ifdeferry to a minimum. But when chromebook with Exynos5 showed up, we
_had_ to allow manufacturers to ship the notebook without the peripheral
side, since they'd never, ever use it. Since the code was already
prepared for that, it was pretty simple and there's no ifdef hell
anywhere. Below you will find original commit. The main idea is that, if
you want a distro-like kernel, then you always build with everything
(DRD), but if you're building a real product, as you said, you may not
want to ship both modes unless you're really going to use them.

commit 388e5c51135f817f01177c42261f1116a6d7f2ad
Author: Vivek Gautam <gautam.vivek@xxxxxxxxxxx>
Date:   Tue Jan 15 16:09:21 2013 +0530

    usb: dwc3: remove dwc3 dependency on host AND gadget.
    
    DWC3 controller curretly depends on USB && USB_GADGET.
    Some hardware may like to use only host feature on dwc3,
    or only gadget feature.
    
    So, removing this dependency of USB_DWC3 on USB and USB_GADGET.
    Adding the mode of operaiton of DWC3 also here
    HOST/GADGET/DUAL_ROLE based on which features are enabled.
    
    [ balbi@xxxxxx :
    	. make sure we have default modes for all possible Kernel
    		configurations.
    	. Remove the config -> menuconfig change as it's unnecessary
    	. switch over to IS_ENABLED() ]
    
    CC: Doug Anderson <dianders@xxxxxxxxxxxx>
    Signed-off-by: Vivek Gautam <gautam.vivek@xxxxxxxxxxx>
    Signed-off-by: Felipe Balbi <balbi@xxxxxx>

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index f6a6e07..77e3f40 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,6 @@
 config USB_DWC3
 	tristate "DesignWare USB3 DRD Core Support"
-	depends on (USB && USB_GADGET)
+	depends on (USB || USB_GADGET)
 	select USB_OTG_UTILS
 	select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
 	help
@@ -12,6 +12,35 @@ config USB_DWC3
 
 if USB_DWC3
 
+choice
+	bool "DWC3 Mode Selection"
+	default USB_DWC3_DUAL_ROLE if (USB && USB_GADGET)
+	default USB_DWC3_HOST if (USB && !USB_GADGET)
+	default USB_DWC3_GADGET if (!USB && USB_GADGET)
+
+config USB_DWC3_HOST
+	bool "Host only mode"
+	depends on USB
+	help
+	  Select this when you want to use DWC3 in host mode only,
+	  thereby the gadget feature will be regressed.
+
+config USB_DWC3_GADGET
+	bool "Gadget only mode"
+	depends on USB_GADGET
+	help
+	  Select this when you want to use DWC3 in gadget mode only,
+	  thereby the host feature will be regressed.
+
+config USB_DWC3_DUAL_ROLE
+	bool "Dual Role mode"
+	depends on (USB && USB_GADGET)
+	help
+	  This is the default mode of working of DWC3 controller where
+	  both host and gadget features are enabled.
+
+endchoice
+
 config USB_DWC3_DEBUG
 	bool "Enable Debugging Messages"
 	help
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 4502648..0c7ac925 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -4,8 +4,14 @@ ccflags-$(CONFIG_USB_DWC3_VERBOSE)	+= -DVERBOSE_DEBUG
 obj-$(CONFIG_USB_DWC3)			+= dwc3.o
 
 dwc3-y					:= core.o
-dwc3-y					+= host.o
-dwc3-y					+= gadget.o ep0.o
+
+ifneq ($(filter y,$(CONFIG_USB_DWC3_HOST) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
+	dwc3-y				+= host.o
+endif
+
+ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
+	dwc3-y				+= gadget.o ep0.o
+endif
 
 ifneq ($(CONFIG_DEBUG_FS),)
 	dwc3-y				+= debugfs.o
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index f02b3e0..b417506 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -864,10 +864,24 @@ union dwc3_event {
 void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
 int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
 
+#if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
 int dwc3_host_init(struct dwc3 *dwc);
 void dwc3_host_exit(struct dwc3 *dwc);
-
+#else
+static inline int dwc3_host_init(struct dwc3 *dwc)
+{ return 0; }
+static inline void dwc3_host_exit(struct dwc3 *dwc)
+{ }
+#endif
+
+#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
 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 0; }
+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 aff8fd3e..4a752e7 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -661,6 +661,7 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
 		goto err1;
 	}
 
+#if IS_ENABLED(CONFIG_USB_DWC3_GADGET)
 	file = debugfs_create_file("mode", S_IRUGO | S_IWUSR, root,
 			dwc, &dwc3_mode_fops);
 	if (!file) {
@@ -681,6 +682,7 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
 		ret = -ENOMEM;
 		goto err1;
 	}
+#endif
 
 	return 0;
 

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux