2014-10-20 18:56:30

by Dinh Nguyen

[permalink] [raw]
Subject: [PATCHv5 0/7] usb: dwc2: Add support for dual-role

From: Dinh Nguyen <[email protected]>

Hello,

This is version 5 of the patch series that combines the dwc2 gadget and host
driver into a single dual role driver. Here are the main differences from V4:

- Squashed 5 patches from V4 into patch 2. Patchset is now only 7 patches.
- Makefile moved to be the last patch in the series.
- When building for kernel modules, dwc2.ko will get built for all modes(host,
gadget, and dual-role). dwc2_platform.ko and dwc2_pci.ko will get built
for platform SOC and PCI.

For v5, the series is rebased on top of v3.18-rc1.

As usual, tested on SOCFPGA(host, gadget, and dual-role) and on Rpi-B
(host mode only).

I have pushed this series to a git repo to make it more convenient for people
to test/review.

git://git.rocketboards.org/linux-socfpga-next.git dwc2_dual_role_v5

Thanks,

Dinh Nguyen (7):
usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure
usb: dwc2: Move gadget probe function into platform code
usb: dwc2: Initialize the USB core for peripheral mode
usb: dwc2: Update common interrupt handler to call gadget interrupt
handler
usb: dwc2: Add call_gadget functions for perpheral mode interrupts
usb: dwc2: gadget: Do not fail probe if there isn't a clock node
usb: dwc2: Update Kconfig to support dual-role

drivers/usb/dwc2/Kconfig | 61 ++++++----
drivers/usb/dwc2/Makefile | 32 ++---
drivers/usb/dwc2/core.c | 10 --
drivers/usb/dwc2/core.h | 192 +++++++++++++++++------------
drivers/usb/dwc2/core_intr.c | 16 ++-
drivers/usb/dwc2/gadget.c | 283 +++++++++++++------------------------------
drivers/usb/dwc2/hcd.c | 3 +-
drivers/usb/dwc2/hcd.h | 10 --
drivers/usb/dwc2/pci.c | 7 ++
drivers/usb/dwc2/platform.c | 52 ++++++++
10 files changed, 331 insertions(+), 335 deletions(-)

--
2.0.3


2014-10-20 18:56:38

by Dinh Nguyen

[permalink] [raw]
Subject: [PATCHv5 1/7] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure

From: Dinh Nguyen <[email protected]>

Adds the gadget data structure and appropriate data structure pointers
to the common dwc2_hsotg data structure. To keep the driver data
dereference code looking clean, the gadget variable declares are only available
for peripheral and dual-role mode. This is needed so that the dwc2_hsotg data
structure can be used by the hcd and gadget drivers.

Updates gadget.c to use the dwc2_hsotg data structure and gadget pointers
that have been moved into the common dwc2_hsotg structure.

Signed-off-by: Dinh Nguyen <[email protected]>
Signed-off-by: Paul Zimmerman <[email protected]>
---
v5: Keep the changes to mininum and maintain hcd and gadget driver to build
and work separately. Use IS_ENABLED() instead of #if defined
v3: Updated with paulz's suggestion to avoid double pointers.
v2: Left the function parameter name as 'hsotg' and just changed its type.
---
drivers/usb/dwc2/core.h | 156 ++++++++++++++++++++++++----------------------
drivers/usb/dwc2/gadget.c | 147 +++++++++++++++++++++----------------------
2 files changed, 155 insertions(+), 148 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index bf015ab..5412f57 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -84,7 +84,7 @@ static const char * const s3c_hsotg_supply_names[] = {
*/
#define EP0_MPS_LIMIT 64

-struct s3c_hsotg;
+struct dwc2_hsotg;
struct s3c_hsotg_req;

/**
@@ -130,7 +130,7 @@ struct s3c_hsotg_req;
struct s3c_hsotg_ep {
struct usb_ep ep;
struct list_head queue;
- struct s3c_hsotg *parent;
+ struct dwc2_hsotg *parent;
struct s3c_hsotg_req *req;
struct dentry *debugfs;

@@ -155,67 +155,6 @@ struct s3c_hsotg_ep {
};

/**
- * struct s3c_hsotg - driver state.
- * @dev: The parent device supplied to the probe function
- * @driver: USB gadget driver
- * @phy: The otg phy transceiver structure for phy control.
- * @uphy: The otg phy transceiver structure for old USB phy control.
- * @plat: The platform specific configuration data. This can be removed once
- * all SoCs support usb transceiver.
- * @regs: The memory area mapped for accessing registers.
- * @irq: The IRQ number we are using
- * @supplies: Definition of USB power supplies
- * @phyif: PHY interface width
- * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
- * @num_of_eps: Number of available EPs (excluding EP0)
- * @debug_root: root directrory for debugfs.
- * @debug_file: main status file for debugfs.
- * @debug_fifo: FIFO status file for debugfs.
- * @ep0_reply: Request used for ep0 reply.
- * @ep0_buff: Buffer for EP0 reply data, if needed.
- * @ctrl_buff: Buffer for EP0 control requests.
- * @ctrl_req: Request for EP0 control packets.
- * @setup: NAK management for EP0 SETUP
- * @last_rst: Time of last reset
- * @eps: The endpoints being supplied to the gadget framework
- */
-struct s3c_hsotg {
- struct device *dev;
- struct usb_gadget_driver *driver;
- struct phy *phy;
- struct usb_phy *uphy;
- struct s3c_hsotg_plat *plat;
-
- spinlock_t lock;
-
- void __iomem *regs;
- int irq;
- struct clk *clk;
-
- struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
-
- u32 phyif;
- int fifo_mem;
- unsigned int dedicated_fifos:1;
- unsigned char num_of_eps;
- u32 fifo_map;
-
- struct dentry *debug_root;
- struct dentry *debug_file;
- struct dentry *debug_fifo;
-
- struct usb_request *ep0_reply;
- struct usb_request *ctrl_req;
- u8 ep0_buff[8];
- u8 ctrl_buff[8];
-
- struct usb_gadget gadget;
- unsigned int setup;
- unsigned long last_rst;
- struct s3c_hsotg_ep *eps;
-};
-
-/**
* struct s3c_hsotg_req - data transfer request
* @req: The USB gadget request
* @queue: The list of requests for the endpoint this is queued for.
@@ -229,6 +168,7 @@ struct s3c_hsotg_req {
unsigned char mapped;
};

+#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
#define call_gadget(_hs, _entry) \
do { \
if ((_hs)->gadget.speed != USB_SPEED_UNKNOWN && \
@@ -238,6 +178,9 @@ do { \
spin_lock(&_hs->lock); \
} \
} while (0)
+#else
+#define call_gadget(_hs, _entry) do {} while (0)
+#endif

struct dwc2_hsotg;
struct dwc2_host_chan;
@@ -495,11 +438,13 @@ struct dwc2_hw_params {
* struct dwc2_hsotg - Holds the state of the driver, including the non-periodic
* and periodic schedules
*
+ * These are common for both host and peripheral modes:
+ *
* @dev: The struct device pointer
* @regs: Pointer to controller regs
- * @core_params: Parameters that define how the core should be configured
* @hw_params: Parameters that were autodetected from the
* hardware registers
+ * @core_params: Parameters that define how the core should be configured
* @op_state: The operational State, during transitions (a_host=>
* a_peripheral and b_device=>b_host) this may not match
* the core, but allows the software to determine
@@ -508,6 +453,8 @@ struct dwc2_hw_params {
* - USB_DR_MODE_PERIPHERAL
* - USB_DR_MODE_HOST
* - USB_DR_MODE_OTG
+ * @lock: Spinlock that protects all the driver data structures
+ * @priv: Stores a pointer to the struct usb_hcd
* @queuing_high_bandwidth: True if multiple packets of a high-bandwidth
* transfer are in process of being queued
* @srp_success: Stores status of SRP request in the case of a FS PHY
@@ -517,6 +464,9 @@ struct dwc2_hw_params {
* interrupt
* @wkp_timer: Timer object for handling Wakeup Detected interrupt
* @lx_state: Lx state of connected device
+ *
+ * These are for host mode:
+ *
* @flags: Flags for handling root port state changes
* @non_periodic_sched_inactive: Inactive QHs in the non-periodic schedule.
* Transfers associated with these QHs are not currently
@@ -585,11 +535,31 @@ struct dwc2_hw_params {
* @status_buf_dma: DMA address for status_buf
* @start_work: Delayed work for handling host A-cable connection
* @reset_work: Delayed work for handling a port reset
- * @lock: Spinlock that protects all the driver data structures
- * @priv: Stores a pointer to the struct usb_hcd
* @otg_port: OTG port number
* @frame_list: Frame list
* @frame_list_dma: Frame list DMA address
+ *
+ * These are for peripheral mode:
+ *
+ * @driver: USB gadget driver
+ * @phy: The otg phy transceiver structure for phy control.
+ * @uphy: The otg phy transceiver structure for old USB phy control.
+ * @plat: The platform specific configuration data. This can be removed once
+ * all SoCs support usb transceiver.
+ * @supplies: Definition of USB power supplies
+ * @phyif: PHY interface width
+ * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
+ * @num_of_eps: Number of available EPs (excluding EP0)
+ * @debug_root: Root directrory for debugfs.
+ * @debug_file: Main status file for debugfs.
+ * @debug_fifo: FIFO status file for debugfs.
+ * @ep0_reply: Request used for ep0 reply.
+ * @ep0_buff: Buffer for EP0 reply data, if needed.
+ * @ctrl_buff: Buffer for EP0 control requests.
+ * @ctrl_req: Request for EP0 control packets.
+ * @setup: NAK management for EP0 SETUP
+ * @last_rst: Time of last reset
+ * @eps: The endpoints being supplied to the gadget framework
*/
struct dwc2_hsotg {
struct device *dev;
@@ -601,6 +571,9 @@ struct dwc2_hsotg {
enum usb_otg_state op_state;
enum usb_dr_mode dr_mode;

+ spinlock_t lock;
+ void *priv;
+
unsigned int queuing_high_bandwidth:1;
unsigned int srp_success:1;

@@ -609,6 +582,14 @@ struct dwc2_hsotg {
struct timer_list wkp_timer;
enum dwc2_lx_state lx_state;

+ /* DWC OTG HW Release versions */
+#define DWC2_CORE_REV_2_71a 0x4f54271a
+#define DWC2_CORE_REV_2_90a 0x4f54290a
+#define DWC2_CORE_REV_2_92a 0x4f54292a
+#define DWC2_CORE_REV_2_94a 0x4f54294a
+#define DWC2_CORE_REV_3_00a 0x4f54300a
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
union dwc2_hcd_internal_flags {
u32 d32;
struct {
@@ -655,19 +636,10 @@ struct dwc2_hsotg {

struct delayed_work start_work;
struct delayed_work reset_work;
- spinlock_t lock;
- void *priv;
u8 otg_port;
u32 *frame_list;
dma_addr_t frame_list_dma;

- /* DWC OTG HW Release versions */
-#define DWC2_CORE_REV_2_71a 0x4f54271a
-#define DWC2_CORE_REV_2_90a 0x4f54290a
-#define DWC2_CORE_REV_2_92a 0x4f54292a
-#define DWC2_CORE_REV_2_94a 0x4f54294a
-#define DWC2_CORE_REV_3_00a 0x4f54300a
-
#ifdef DEBUG
u32 frrem_samples;
u64 frrem_accum;
@@ -686,6 +658,40 @@ struct dwc2_hsotg {
u32 hfnum_other_samples_b;
u64 hfnum_other_frrem_accum_b;
#endif
+#endif /* CONFIG_USB_DWC2_HOST || CONFIG_USB_DWC2_DUAL_ROLE */
+
+#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
+ /* Gadget structures */
+ struct usb_gadget_driver *driver;
+ struct phy *phy;
+ struct usb_phy *uphy;
+ struct s3c_hsotg_plat *plat;
+
+ int irq;
+ struct clk *clk;
+
+ struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
+
+ u32 phyif;
+ int fifo_mem;
+ unsigned int dedicated_fifos:1;
+ unsigned char num_of_eps;
+ u32 fifo_map;
+
+ struct dentry *debug_root;
+ struct dentry *debug_file;
+ struct dentry *debug_fifo;
+
+ struct usb_request *ep0_reply;
+ struct usb_request *ctrl_req;
+ u8 ep0_buff[8];
+ u8 ctrl_buff[8];
+
+ struct usb_gadget gadget;
+ unsigned int setup;
+ unsigned long last_rst;
+ struct s3c_hsotg_ep *eps;
+#endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */
};

/* Reasons for halting a host channel */
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7b5856f..6611ea3 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -36,6 +36,7 @@
#include <linux/platform_data/s3c-hsotg.h>

#include "core.h"
+#include "hw.h"

/* conversion functions */
static inline struct s3c_hsotg_req *our_req(struct usb_request *req)
@@ -48,9 +49,9 @@ static inline struct s3c_hsotg_ep *our_ep(struct usb_ep *ep)
return container_of(ep, struct s3c_hsotg_ep, ep);
}

-static inline struct s3c_hsotg *to_hsotg(struct usb_gadget *gadget)
+static inline struct dwc2_hsotg *to_hsotg(struct usb_gadget *gadget)
{
- return container_of(gadget, struct s3c_hsotg, gadget);
+ return container_of(gadget, struct dwc2_hsotg, gadget);
}

static inline void __orr32(void __iomem *ptr, u32 val)
@@ -64,7 +65,7 @@ static inline void __bic32(void __iomem *ptr, u32 val)
}

/* forward decleration of functions */
-static void s3c_hsotg_dump(struct s3c_hsotg *hsotg);
+static void s3c_hsotg_dump(struct dwc2_hsotg *hsotg);

/**
* using_dma - return the DMA status of the driver.
@@ -85,7 +86,7 @@ static void s3c_hsotg_dump(struct s3c_hsotg *hsotg);
*
* Until this issue is sorted out, we always return 'false'.
*/
-static inline bool using_dma(struct s3c_hsotg *hsotg)
+static inline bool using_dma(struct dwc2_hsotg *hsotg)
{
return false; /* support is not complete */
}
@@ -95,7 +96,7 @@ static inline bool using_dma(struct s3c_hsotg *hsotg)
* @hsotg: The device state
* @ints: A bitmask of the interrupts to enable
*/
-static void s3c_hsotg_en_gsint(struct s3c_hsotg *hsotg, u32 ints)
+static void s3c_hsotg_en_gsint(struct dwc2_hsotg *hsotg, u32 ints)
{
u32 gsintmsk = readl(hsotg->regs + GINTMSK);
u32 new_gsintmsk;
@@ -113,7 +114,7 @@ static void s3c_hsotg_en_gsint(struct s3c_hsotg *hsotg, u32 ints)
* @hsotg: The device state
* @ints: A bitmask of the interrupts to enable
*/
-static void s3c_hsotg_disable_gsint(struct s3c_hsotg *hsotg, u32 ints)
+static void s3c_hsotg_disable_gsint(struct dwc2_hsotg *hsotg, u32 ints)
{
u32 gsintmsk = readl(hsotg->regs + GINTMSK);
u32 new_gsintmsk;
@@ -134,7 +135,7 @@ static void s3c_hsotg_disable_gsint(struct s3c_hsotg *hsotg, u32 ints)
* Set or clear the mask for an individual endpoint's interrupt
* request.
*/
-static void s3c_hsotg_ctrl_epint(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_ctrl_epint(struct dwc2_hsotg *hsotg,
unsigned int ep, unsigned int dir_in,
unsigned int en)
{
@@ -159,7 +160,7 @@ static void s3c_hsotg_ctrl_epint(struct s3c_hsotg *hsotg,
* s3c_hsotg_init_fifo - initialise non-periodic FIFOs
* @hsotg: The device instance.
*/
-static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
{
unsigned int ep;
unsigned int addr;
@@ -283,7 +284,7 @@ static inline int is_ep_periodic(struct s3c_hsotg_ep *hs_ep)
* This is the reverse of s3c_hsotg_map_dma(), called for the completion
* of a request to ensure the buffer is ready for access by the caller.
*/
-static void s3c_hsotg_unmap_dma(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_unmap_dma(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *hs_ep,
struct s3c_hsotg_req *hs_req)
{
@@ -312,7 +313,7 @@ static void s3c_hsotg_unmap_dma(struct s3c_hsotg *hsotg,
*
* This routine is only needed for PIO
*/
-static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
+static int s3c_hsotg_write_fifo(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *hs_ep,
struct s3c_hsotg_req *hs_req)
{
@@ -517,7 +518,7 @@ static unsigned get_ep_limit(struct s3c_hsotg_ep *hs_ep)
* Start the given request running by setting the endpoint registers
* appropriately, and writing any data to the FIFOs.
*/
-static void s3c_hsotg_start_req(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_start_req(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *hs_ep,
struct s3c_hsotg_req *hs_req,
bool continuing)
@@ -707,7 +708,7 @@ static void s3c_hsotg_start_req(struct s3c_hsotg *hsotg,
* DMA memory, then we map the memory and mark our request to allow us to
* cleanup on completion.
*/
-static int s3c_hsotg_map_dma(struct s3c_hsotg *hsotg,
+static int s3c_hsotg_map_dma(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *hs_ep,
struct usb_request *req)
{
@@ -736,7 +737,7 @@ static int s3c_hsotg_ep_queue(struct usb_ep *ep, struct usb_request *req,
{
struct s3c_hsotg_req *hs_req = our_req(req);
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hs = hs_ep->parent;
+ struct dwc2_hsotg *hs = hs_ep->parent;
bool first;

dev_dbg(hs->dev, "%s: req %p: %d@%p, noi=%d, zero=%d, snok=%d\n",
@@ -768,7 +769,7 @@ static int s3c_hsotg_ep_queue_lock(struct usb_ep *ep, struct usb_request *req,
gfp_t gfp_flags)
{
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hs = hs_ep->parent;
+ struct dwc2_hsotg *hs = hs_ep->parent;
unsigned long flags = 0;
int ret = 0;

@@ -799,7 +800,7 @@ static void s3c_hsotg_complete_oursetup(struct usb_ep *ep,
struct usb_request *req)
{
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hsotg = hs_ep->parent;
+ struct dwc2_hsotg *hsotg = hs_ep->parent;

dev_dbg(hsotg->dev, "%s: ep %p, req %p\n", __func__, ep, req);

@@ -814,7 +815,7 @@ static void s3c_hsotg_complete_oursetup(struct usb_ep *ep,
* Convert the given wIndex into a pointer to an driver endpoint
* structure, or return NULL if it is not a valid endpoint.
*/
-static struct s3c_hsotg_ep *ep_from_windex(struct s3c_hsotg *hsotg,
+static struct s3c_hsotg_ep *ep_from_windex(struct dwc2_hsotg *hsotg,
u32 windex)
{
struct s3c_hsotg_ep *ep = &hsotg->eps[windex & 0x7F];
@@ -843,7 +844,7 @@ static struct s3c_hsotg_ep *ep_from_windex(struct s3c_hsotg *hsotg,
* Create a request and queue it on the given endpoint. This is useful as
* an internal method of sending replies to certain control requests, etc.
*/
-static int s3c_hsotg_send_reply(struct s3c_hsotg *hsotg,
+static int s3c_hsotg_send_reply(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *ep,
void *buff,
int length)
@@ -884,7 +885,7 @@ static int s3c_hsotg_send_reply(struct s3c_hsotg *hsotg,
* @hsotg: The device state
* @ctrl: USB control request
*/
-static int s3c_hsotg_process_req_status(struct s3c_hsotg *hsotg,
+static int s3c_hsotg_process_req_status(struct dwc2_hsotg *hsotg,
struct usb_ctrlrequest *ctrl)
{
struct s3c_hsotg_ep *ep0 = &hsotg->eps[0];
@@ -955,7 +956,7 @@ static struct s3c_hsotg_req *get_ep_head(struct s3c_hsotg_ep *hs_ep)
* @hsotg: The device state
* @ctrl: USB control request
*/
-static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
+static int s3c_hsotg_process_req_feature(struct dwc2_hsotg *hsotg,
struct usb_ctrlrequest *ctrl)
{
struct s3c_hsotg_ep *ep0 = &hsotg->eps[0];
@@ -1028,8 +1029,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
return 1;
}

-static void s3c_hsotg_enqueue_setup(struct s3c_hsotg *hsotg);
-static void s3c_hsotg_disconnect(struct s3c_hsotg *hsotg);
+static void s3c_hsotg_enqueue_setup(struct dwc2_hsotg *hsotg);
+static void s3c_hsotg_disconnect(struct dwc2_hsotg *hsotg);

/**
* s3c_hsotg_stall_ep0 - stall ep0
@@ -1037,7 +1038,7 @@ static void s3c_hsotg_disconnect(struct s3c_hsotg *hsotg);
*
* Set stall for ep0 as response for setup request.
*/
-static void s3c_hsotg_stall_ep0(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_stall_ep0(struct dwc2_hsotg *hsotg)
{
struct s3c_hsotg_ep *ep0 = &hsotg->eps[0];
u32 reg;
@@ -1076,7 +1077,7 @@ static void s3c_hsotg_stall_ep0(struct s3c_hsotg *hsotg)
* needs to work out what to do next (and whether to pass it on to the
* gadget driver).
*/
-static void s3c_hsotg_process_control(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_process_control(struct dwc2_hsotg *hsotg,
struct usb_ctrlrequest *ctrl)
{
struct s3c_hsotg_ep *ep0 = &hsotg->eps[0];
@@ -1161,7 +1162,7 @@ static void s3c_hsotg_complete_setup(struct usb_ep *ep,
struct usb_request *req)
{
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hsotg = hs_ep->parent;
+ struct dwc2_hsotg *hsotg = hs_ep->parent;

if (req->status < 0) {
dev_dbg(hsotg->dev, "%s: failed %d\n", __func__, req->status);
@@ -1183,7 +1184,7 @@ static void s3c_hsotg_complete_setup(struct usb_ep *ep,
* Enqueue a request on EP0 if necessary to received any SETUP packets
* received from the host.
*/
-static void s3c_hsotg_enqueue_setup(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_enqueue_setup(struct dwc2_hsotg *hsotg)
{
struct usb_request *req = hsotg->ctrl_req;
struct s3c_hsotg_req *hs_req = our_req(req);
@@ -1226,7 +1227,7 @@ static void s3c_hsotg_enqueue_setup(struct s3c_hsotg *hsotg)
*
* Note, expects the ep to already be locked as appropriate.
*/
-static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_complete_request(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *hs_ep,
struct s3c_hsotg_req *hs_req,
int result)
@@ -1291,7 +1292,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
* endpoint, so sort out whether we need to read the data into a request
* that has been made for that endpoint.
*/
-static void s3c_hsotg_rx_data(struct s3c_hsotg *hsotg, int ep_idx, int size)
+static void s3c_hsotg_rx_data(struct dwc2_hsotg *hsotg, int ep_idx, int size)
{
struct s3c_hsotg_ep *hs_ep = &hsotg->eps[ep_idx];
struct s3c_hsotg_req *hs_req = hs_ep->req;
@@ -1356,7 +1357,7 @@ static void s3c_hsotg_rx_data(struct s3c_hsotg *hsotg, int ep_idx, int size)
* currently believed that we do not need to wait for any space in
* the TxFIFO.
*/
-static void s3c_hsotg_send_zlp(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_send_zlp(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_req *req)
{
u32 ctrl;
@@ -1398,7 +1399,7 @@ static void s3c_hsotg_send_zlp(struct s3c_hsotg *hsotg,
* transfer for an OUT endpoint has been completed, either by a short
* packet or by the finish of a transfer.
*/
-static void s3c_hsotg_handle_outdone(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_handle_outdone(struct dwc2_hsotg *hsotg,
int epnum, bool was_setup)
{
u32 epsize = readl(hsotg->regs + DOEPTSIZ(epnum));
@@ -1471,7 +1472,7 @@ static void s3c_hsotg_handle_outdone(struct s3c_hsotg *hsotg,
*
* Return the current frame number
*/
-static u32 s3c_hsotg_read_frameno(struct s3c_hsotg *hsotg)
+static u32 s3c_hsotg_read_frameno(struct dwc2_hsotg *hsotg)
{
u32 dsts;

@@ -1498,7 +1499,7 @@ static u32 s3c_hsotg_read_frameno(struct s3c_hsotg *hsotg)
* as the actual data should be sent to the memory directly and we turn
* on the completion interrupts to get notifications of transfer completion.
*/
-static void s3c_hsotg_handle_rx(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_handle_rx(struct dwc2_hsotg *hsotg)
{
u32 grxstsr = readl(hsotg->regs + GRXSTSP);
u32 epnum, status, size;
@@ -1590,7 +1591,7 @@ static u32 s3c_hsotg_ep0_mps(unsigned int mps)
* Configure the maximum packet size for the given endpoint, updating
* the hardware control registers to reflect this.
*/
-static void s3c_hsotg_set_ep_maxpacket(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_set_ep_maxpacket(struct dwc2_hsotg *hsotg,
unsigned int ep, unsigned int mps)
{
struct s3c_hsotg_ep *hs_ep = &hsotg->eps[ep];
@@ -1645,7 +1646,7 @@ bad_mps:
* @hsotg: The driver state
* @idx: The index for the endpoint (0..15)
*/
-static void s3c_hsotg_txfifo_flush(struct s3c_hsotg *hsotg, unsigned int idx)
+static void s3c_hsotg_txfifo_flush(struct dwc2_hsotg *hsotg, unsigned int idx)
{
int timeout;
int val;
@@ -1681,7 +1682,7 @@ static void s3c_hsotg_txfifo_flush(struct s3c_hsotg *hsotg, unsigned int idx)
* Check to see if there is a request that has data to send, and if so
* make an attempt to write data into the FIFO.
*/
-static int s3c_hsotg_trytx(struct s3c_hsotg *hsotg,
+static int s3c_hsotg_trytx(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *hs_ep)
{
struct s3c_hsotg_req *hs_req = hs_ep->req;
@@ -1714,7 +1715,7 @@ static int s3c_hsotg_trytx(struct s3c_hsotg *hsotg,
* An IN transfer has been completed, update the transfer's state and then
* call the relevant completion routines.
*/
-static void s3c_hsotg_complete_in(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_complete_in(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *hs_ep)
{
struct s3c_hsotg_req *hs_req = hs_ep->req;
@@ -1791,7 +1792,7 @@ static void s3c_hsotg_complete_in(struct s3c_hsotg *hsotg,
*
* Process and clear any interrupt pending for an individual endpoint
*/
-static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
+static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx,
int dir_in)
{
struct s3c_hsotg_ep *hs_ep = &hsotg->eps[idx];
@@ -1916,7 +1917,7 @@ static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
* Handle updating the device settings after the enumeration phase has
* been completed.
*/
-static void s3c_hsotg_irq_enumdone(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_irq_enumdone(struct dwc2_hsotg *hsotg)
{
u32 dsts = readl(hsotg->regs + DSTS);
int ep0_mps = 0, ep_mps = 8;
@@ -1993,7 +1994,7 @@ static void s3c_hsotg_irq_enumdone(struct s3c_hsotg *hsotg)
* Go through the requests on the given endpoint and mark them
* completed with the given result code.
*/
-static void kill_all_requests(struct s3c_hsotg *hsotg,
+static void kill_all_requests(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *ep,
int result, bool force)
{
@@ -2027,7 +2028,7 @@ static void kill_all_requests(struct s3c_hsotg *hsotg,
* transactions and signal the gadget driver that this
* has happened.
*/
-static void s3c_hsotg_disconnect(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_disconnect(struct dwc2_hsotg *hsotg)
{
unsigned ep;

@@ -2042,7 +2043,7 @@ static void s3c_hsotg_disconnect(struct s3c_hsotg *hsotg)
* @hsotg: The device state:
* @periodic: True if this is a periodic FIFO interrupt
*/
-static void s3c_hsotg_irq_fifoempty(struct s3c_hsotg *hsotg, bool periodic)
+static void s3c_hsotg_irq_fifoempty(struct dwc2_hsotg *hsotg, bool periodic)
{
struct s3c_hsotg_ep *ep;
int epno, ret;
@@ -2076,7 +2077,7 @@ static void s3c_hsotg_irq_fifoempty(struct s3c_hsotg *hsotg, bool periodic)
*
* Issue a soft reset to the core, and await the core finishing it.
*/
-static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)
+static int s3c_hsotg_corereset(struct dwc2_hsotg *hsotg)
{
int timeout;
u32 grstctl;
@@ -2124,7 +2125,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)
*
* Issue a soft reset to the core, and await the core finishing it.
*/
-static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_core_init(struct dwc2_hsotg *hsotg)
{
s3c_hsotg_corereset(hsotg);

@@ -2258,7 +2259,7 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
*/
static irqreturn_t s3c_hsotg_irq(int irq, void *pw)
{
- struct s3c_hsotg *hsotg = pw;
+ struct dwc2_hsotg *hsotg = pw;
int retry_count = 8;
u32 gintsts;
u32 gintmsk;
@@ -2450,7 +2451,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
const struct usb_endpoint_descriptor *desc)
{
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hsotg = hs_ep->parent;
+ struct dwc2_hsotg *hsotg = hs_ep->parent;
unsigned long flags;
int index = hs_ep->index;
u32 epctrl_reg;
@@ -2590,7 +2591,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
static int s3c_hsotg_ep_disable(struct usb_ep *ep)
{
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hsotg = hs_ep->parent;
+ struct dwc2_hsotg *hsotg = hs_ep->parent;
int dir_in = hs_ep->dir_in;
int index = hs_ep->index;
unsigned long flags;
@@ -2655,7 +2656,7 @@ static int s3c_hsotg_ep_dequeue(struct usb_ep *ep, struct usb_request *req)
{
struct s3c_hsotg_req *hs_req = our_req(req);
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hs = hs_ep->parent;
+ struct dwc2_hsotg *hs = hs_ep->parent;
unsigned long flags;

dev_dbg(hs->dev, "ep_dequeue(%p,%p)\n", ep, req);
@@ -2681,7 +2682,7 @@ static int s3c_hsotg_ep_dequeue(struct usb_ep *ep, struct usb_request *req)
static int s3c_hsotg_ep_sethalt(struct usb_ep *ep, int value)
{
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hs = hs_ep->parent;
+ struct dwc2_hsotg *hs = hs_ep->parent;
int index = hs_ep->index;
u32 epreg;
u32 epctl;
@@ -2745,7 +2746,7 @@ static int s3c_hsotg_ep_sethalt(struct usb_ep *ep, int value)
static int s3c_hsotg_ep_sethalt_lock(struct usb_ep *ep, int value)
{
struct s3c_hsotg_ep *hs_ep = our_ep(ep);
- struct s3c_hsotg *hs = hs_ep->parent;
+ struct dwc2_hsotg *hs = hs_ep->parent;
unsigned long flags = 0;
int ret = 0;

@@ -2774,7 +2775,7 @@ static struct usb_ep_ops s3c_hsotg_ep_ops = {
* A wrapper for platform code responsible for controlling
* low-level USB code
*/
-static void s3c_hsotg_phy_enable(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_phy_enable(struct dwc2_hsotg *hsotg)
{
struct platform_device *pdev = to_platform_device(hsotg->dev);

@@ -2797,7 +2798,7 @@ static void s3c_hsotg_phy_enable(struct s3c_hsotg *hsotg)
* A wrapper for platform code responsible for controlling
* low-level USB code
*/
-static void s3c_hsotg_phy_disable(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_phy_disable(struct dwc2_hsotg *hsotg)
{
struct platform_device *pdev = to_platform_device(hsotg->dev);

@@ -2815,7 +2816,7 @@ static void s3c_hsotg_phy_disable(struct s3c_hsotg *hsotg)
* s3c_hsotg_init - initalize the usb core
* @hsotg: The driver state
*/
-static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_init(struct dwc2_hsotg *hsotg)
{
/* unmask subset of endpoint interrupts */

@@ -2865,7 +2866,7 @@ static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
struct usb_gadget_driver *driver)
{
- struct s3c_hsotg *hsotg = to_hsotg(gadget);
+ struct dwc2_hsotg *hsotg = to_hsotg(gadget);
int ret;

if (!hsotg) {
@@ -2921,7 +2922,7 @@ err:
static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
struct usb_gadget_driver *driver)
{
- struct s3c_hsotg *hsotg = to_hsotg(gadget);
+ struct dwc2_hsotg *hsotg = to_hsotg(gadget);
unsigned long flags = 0;
int ep;

@@ -2941,7 +2942,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,

spin_unlock_irqrestore(&hsotg->lock, flags);

- regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
+ regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
+ hsotg->supplies);

clk_disable(hsotg->clk);

@@ -2968,7 +2970,7 @@ static int s3c_hsotg_gadget_getframe(struct usb_gadget *gadget)
*/
static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
{
- struct s3c_hsotg *hsotg = to_hsotg(gadget);
+ struct dwc2_hsotg *hsotg = to_hsotg(gadget);
unsigned long flags = 0;

dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on);
@@ -3006,7 +3008,7 @@ static const struct usb_gadget_ops s3c_hsotg_gadget_ops = {
* creation) to give to the gadget driver. Setup the endpoint name, any
* direction information and other state that may be required.
*/
-static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
+static void s3c_hsotg_initep(struct dwc2_hsotg *hsotg,
struct s3c_hsotg_ep *hs_ep,
int epnum)
{
@@ -3055,7 +3057,7 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
*
* Read the USB core HW configuration registers
*/
-static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_hw_cfg(struct dwc2_hsotg *hsotg)
{
u32 cfg2, cfg3, cfg4;
/* check hardware configuration */
@@ -3079,7 +3081,7 @@ static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
* s3c_hsotg_dump - dump state of the udc
* @param: The device state
*/
-static void s3c_hsotg_dump(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_dump(struct dwc2_hsotg *hsotg)
{
#ifdef DEBUG
struct device *dev = hsotg->dev;
@@ -3138,7 +3140,7 @@ static void s3c_hsotg_dump(struct s3c_hsotg *hsotg)
*/
static int state_show(struct seq_file *seq, void *v)
{
- struct s3c_hsotg *hsotg = seq->private;
+ struct dwc2_hsotg *hsotg = seq->private;
void __iomem *regs = hsotg->regs;
int idx;

@@ -3208,7 +3210,7 @@ static const struct file_operations state_fops = {
*/
static int fifo_show(struct seq_file *seq, void *v)
{
- struct s3c_hsotg *hsotg = seq->private;
+ struct dwc2_hsotg *hsotg = seq->private;
void __iomem *regs = hsotg->regs;
u32 val;
int idx;
@@ -3264,7 +3266,7 @@ static const char *decode_direction(int is_in)
static int ep_show(struct seq_file *seq, void *v)
{
struct s3c_hsotg_ep *ep = seq->private;
- struct s3c_hsotg *hsotg = ep->parent;
+ struct dwc2_hsotg *hsotg = ep->parent;
struct s3c_hsotg_req *req;
void __iomem *regs = hsotg->regs;
int index = ep->index;
@@ -3341,7 +3343,7 @@ static const struct file_operations ep_fops = {
* with the same name as the device itself, in case we end up
* with multiple blocks in future systems.
*/
-static void s3c_hsotg_create_debug(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_create_debug(struct dwc2_hsotg *hsotg)
{
struct dentry *root;
unsigned epidx;
@@ -3387,7 +3389,7 @@ static void s3c_hsotg_create_debug(struct s3c_hsotg *hsotg)
*
* Cleanup (remove) the debugfs files for use on module exit.
*/
-static void s3c_hsotg_delete_debug(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_delete_debug(struct dwc2_hsotg *hsotg)
{
unsigned epidx;

@@ -3405,7 +3407,6 @@ static void s3c_hsotg_delete_debug(struct s3c_hsotg *hsotg)
* s3c_hsotg_probe - probe function for hsotg driver
* @pdev: The platform information for the driver
*/
-
static int s3c_hsotg_probe(struct platform_device *pdev)
{
struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
@@ -3413,13 +3414,13 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
struct usb_phy *uphy;
struct device *dev = &pdev->dev;
struct s3c_hsotg_ep *eps;
- struct s3c_hsotg *hsotg;
+ struct dwc2_hsotg *hsotg;
struct resource *res;
int epnum;
int ret;
int i;

- hsotg = devm_kzalloc(&pdev->dev, sizeof(struct s3c_hsotg), GFP_KERNEL);
+ hsotg = devm_kzalloc(&pdev->dev, sizeof(struct dwc2_hsotg), GFP_KERNEL);
if (!hsotg)
return -ENOMEM;

@@ -3500,7 +3501,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
- dev_err(dev, "failed to request supplies: %d\n", ret);
+ dev_err(hsotg->dev, "failed to request supplies: %d\n", ret);
goto err_clk;
}

@@ -3508,7 +3509,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
hsotg->supplies);

if (ret) {
- dev_err(hsotg->dev, "failed to enable supplies: %d\n", ret);
+ dev_err(dev, "failed to enable supplies: %d\n", ret);
goto err_supplies;
}

@@ -3571,13 +3572,13 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
- dev_err(hsotg->dev, "failed to disable supplies: %d\n", ret);
+ dev_err(&pdev->dev, "failed to disable supplies: %d\n", ret);
goto err_ep_mem;
}

s3c_hsotg_phy_disable(hsotg);

- ret = usb_add_gadget_udc(&pdev->dev, &hsotg->gadget);
+ ret = usb_add_gadget_udc(dev, &hsotg->gadget);
if (ret)
goto err_ep_mem;

@@ -3603,7 +3604,7 @@ err_clk:
*/
static int s3c_hsotg_remove(struct platform_device *pdev)
{
- struct s3c_hsotg *hsotg = platform_get_drvdata(pdev);
+ struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);

usb_del_gadget_udc(&hsotg->gadget);

@@ -3621,7 +3622,7 @@ static int s3c_hsotg_remove(struct platform_device *pdev)

static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
{
- struct s3c_hsotg *hsotg = platform_get_drvdata(pdev);
+ struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
unsigned long flags;
int ret = 0;

@@ -3650,7 +3651,7 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)

static int s3c_hsotg_resume(struct platform_device *pdev)
{
- struct s3c_hsotg *hsotg = platform_get_drvdata(pdev);
+ struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
unsigned long flags;
int ret = 0;

--
2.0.3

2014-10-20 18:56:47

by Dinh Nguyen

[permalink] [raw]
Subject: [PATCHv5 2/7] usb: dwc2: Move gadget probe function into platform code

From: Dinh Nguyen <[email protected]>

This patch will aggregate the probing of gadget/hcd driver into platform.c.
The gadget probe funtion is converted into gadget_init that is now only
responsible for gadget only initialization. All the gadget resources is now
handled by platform.c

Since the host workqueue will not get initialized if the driver is configured
for peripheral mode only. Thus we need to check for wq_otg before calling
queue_work().

Also, we move spin_lock_init to common location for both host and gadget that
is either in platform.c or pci.c.

We also ove suspend/resume code to common platform code, and update it to use
the new PM API (struct dev_pm_ops).

Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.

Signed-off-by: Dinh Nguyen <[email protected]>
Acked-by: Paul Zimmerman <[email protected]>
---
v5: Reworked by squashing the following commits into this one:
* [PATCHv4 02/12] usb: dwc2: move "samsung,s3c6400-hsotg" into common platform
* [PATCHv4 03/12] usb: dwc2: Update the gadget driver to use common dwc2_hsotg
structure
* [PATCHv4 09/12] usb: dwc2: initialize the spin_lock for both host and gadget
* [PATCHv4 10/12] usb: dwc2: Add suspend/resume for gadget
* [PATCHv4 11/12] usb: dwc2: check that the host work queue is valid
Also use IS_ENABLED instead of #if defined
---
drivers/usb/dwc2/core.h | 34 +++++++++++++++-
drivers/usb/dwc2/core_intr.c | 8 ++--
drivers/usb/dwc2/gadget.c | 97 +++++++++-----------------------------------
drivers/usb/dwc2/hcd.c | 1 -
drivers/usb/dwc2/hcd.h | 10 -----
drivers/usb/dwc2/pci.c | 1 +
drivers/usb/dwc2/platform.c | 32 +++++++++++++++
7 files changed, 91 insertions(+), 92 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 5412f57..b21aace 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -667,7 +667,6 @@ struct dwc2_hsotg {
struct usb_phy *uphy;
struct s3c_hsotg_plat *plat;

- int irq;
struct clk *clk;

struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
@@ -961,4 +960,37 @@ extern void dwc2_dump_global_registers(struct dwc2_hsotg *hsotg);
*/
extern u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg);

+/* Gadget defines */
+#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
+extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
+extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
+extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
+extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
+#else
+static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
+{ return 0; }
+#endif
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
+extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
+#else
+static inline void dwc2_set_all_params(struct dwc2_core_params *params, int value) {}
+static inline int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
+ const struct dwc2_core_params *params)
+{ return 0; }
+#endif
+
#endif /* __DWC2_CORE_H__ */
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index c93918b..b176c2f 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -287,9 +287,11 @@ static void dwc2_handle_conn_id_status_change_intr(struct dwc2_hsotg *hsotg)
* Release lock before scheduling workq as it holds spinlock during
* scheduling.
*/
- spin_unlock(&hsotg->lock);
- queue_work(hsotg->wq_otg, &hsotg->wf_otg);
- spin_lock(&hsotg->lock);
+ if (hsotg->wq_otg) {
+ spin_unlock(&hsotg->lock);
+ queue_work(hsotg->wq_otg, &hsotg->wf_otg);
+ spin_lock(&hsotg->lock);
+ }

/* Clear interrupt */
writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6611ea3..c407d33 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3404,26 +3404,21 @@ static void s3c_hsotg_delete_debug(struct dwc2_hsotg *hsotg)
}

/**
- * s3c_hsotg_probe - probe function for hsotg driver
- * @pdev: The platform information for the driver
+ * dwc2_gadget_init - init function for gadget
+ * @dwc2: The data structure for the DWC2 driver.
+ * @irq: The IRQ number for the controller.
*/
-static int s3c_hsotg_probe(struct platform_device *pdev)
+int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{
- struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
+ struct device *dev = hsotg->dev;
+ struct s3c_hsotg_plat *plat = dev->platform_data;
struct phy *phy;
struct usb_phy *uphy;
- struct device *dev = &pdev->dev;
struct s3c_hsotg_ep *eps;
- struct dwc2_hsotg *hsotg;
- struct resource *res;
int epnum;
int ret;
int i;

- hsotg = devm_kzalloc(&pdev->dev, sizeof(struct dwc2_hsotg), GFP_KERNEL);
- if (!hsotg)
- return -ENOMEM;
-
/* Set default UTMI width */
hsotg->phyif = GUSBCFG_PHYIF16;

@@ -3431,14 +3426,14 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
* Attempt to find a generic PHY, then look for an old style
* USB PHY, finally fall back to pdata
*/
- phy = devm_phy_get(&pdev->dev, "usb2-phy");
+ phy = devm_phy_get(dev, "usb2-phy");
if (IS_ERR(phy)) {
uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
if (IS_ERR(uphy)) {
/* Fallback for pdata */
- plat = dev_get_platdata(&pdev->dev);
+ plat = dev_get_platdata(dev);
if (!plat) {
- dev_err(&pdev->dev,
+ dev_err(dev,
"no platform data or transceiver defined\n");
return -EPROBE_DEFER;
}
@@ -3455,36 +3450,12 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
hsotg->phyif = GUSBCFG_PHYIF8;
}

- hsotg->dev = dev;
-
- hsotg->clk = devm_clk_get(&pdev->dev, "otg");
+ hsotg->clk = devm_clk_get(dev, "otg");
if (IS_ERR(hsotg->clk)) {
dev_err(dev, "cannot get otg clock\n");
return PTR_ERR(hsotg->clk);
}

- platform_set_drvdata(pdev, hsotg);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
- hsotg->regs = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(hsotg->regs)) {
- ret = PTR_ERR(hsotg->regs);
- goto err_clk;
- }
-
- ret = platform_get_irq(pdev, 0);
- if (ret < 0) {
- dev_err(dev, "cannot find IRQ\n");
- goto err_clk;
- }
-
- spin_lock_init(&hsotg->lock);
-
- hsotg->irq = ret;
-
- dev_info(dev, "regs %p, irq %d\n", hsotg->regs, hsotg->irq);
-
hsotg->gadget.max_speed = USB_SPEED_HIGH;
hsotg->gadget.ops = &s3c_hsotg_gadget_ops;
hsotg->gadget.name = dev_name(dev);
@@ -3501,7 +3472,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
- dev_err(hsotg->dev, "failed to request supplies: %d\n", ret);
+ dev_err(dev, "failed to request supplies: %d\n", ret);
goto err_clk;
}

@@ -3520,7 +3491,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_hw_cfg(hsotg);
s3c_hsotg_init(hsotg);

- ret = devm_request_irq(&pdev->dev, hsotg->irq, s3c_hsotg_irq, 0,
+ ret = devm_request_irq(dev, irq, s3c_hsotg_irq, 0,
dev_name(dev), hsotg);
if (ret < 0) {
s3c_hsotg_phy_disable(hsotg);
@@ -3572,7 +3543,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
- dev_err(&pdev->dev, "failed to disable supplies: %d\n", ret);
+ dev_err(dev, "failed to disable supplies: %d\n", ret);
goto err_ep_mem;
}

@@ -3597,15 +3568,14 @@ err_clk:

return ret;
}
+EXPORT_SYMBOL_GPL(dwc2_gadget_init);

/**
* s3c_hsotg_remove - remove function for hsotg driver
* @pdev: The platform information for the driver
*/
-static int s3c_hsotg_remove(struct platform_device *pdev)
+int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
-
usb_del_gadget_udc(&hsotg->gadget);

s3c_hsotg_delete_debug(hsotg);
@@ -3619,10 +3589,10 @@ static int s3c_hsotg_remove(struct platform_device *pdev)

return 0;
}
+EXPORT_SYMBOL_GPL(s3c_hsotg_remove);

-static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
+int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
unsigned long flags;
int ret = 0;

@@ -3648,10 +3618,10 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)

return ret;
}
+EXPORT_SYMBOL_GPL(s3c_hsotg_suspend);

-static int s3c_hsotg_resume(struct platform_device *pdev)
+int s3c_hsotg_resume(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
unsigned long flags;
int ret = 0;

@@ -3672,31 +3642,4 @@ static int s3c_hsotg_resume(struct platform_device *pdev)

return ret;
}
-
-#ifdef CONFIG_OF
-static const struct of_device_id s3c_hsotg_of_ids[] = {
- { .compatible = "samsung,s3c6400-hsotg", },
- { .compatible = "snps,dwc2", },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids);
-#endif
-
-static struct platform_driver s3c_hsotg_driver = {
- .driver = {
- .name = "s3c-hsotg",
- .owner = THIS_MODULE,
- .of_match_table = of_match_ptr(s3c_hsotg_of_ids),
- },
- .probe = s3c_hsotg_probe,
- .remove = s3c_hsotg_remove,
- .suspend = s3c_hsotg_suspend,
- .resume = s3c_hsotg_resume,
-};
-
-module_platform_driver(s3c_hsotg_driver);
-
-MODULE_DESCRIPTION("Samsung S3C USB High-speed/OtG device");
-MODULE_AUTHOR("Ben Dooks <[email protected]>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:s3c-hsotg");
+EXPORT_SYMBOL_GPL(s3c_hsotg_resume);
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 0a0e6f0..4a3cce0 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2839,7 +2839,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,

hcd->has_tt = 1;

- spin_lock_init(&hsotg->lock);
((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg;
hsotg->priv = hcd;

diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index a12bb15..e69a843 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -668,9 +668,6 @@ extern irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg);
*/
extern void dwc2_hcd_stop(struct dwc2_hsotg *hsotg);

-extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
-extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
-
/**
* dwc2_hcd_is_b_host() - Returns 1 if core currently is acting as B host,
* and 0 otherwise
@@ -680,13 +677,6 @@ extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
extern int dwc2_hcd_is_b_host(struct dwc2_hsotg *hsotg);

/**
- * dwc2_hcd_get_frame_number() - Returns current frame number
- *
- * @hsotg: The DWC2 HCD
- */
-extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
-
-/**
* dwc2_hcd_dump_state() - Dumps hsotg state
*
* @hsotg: The DWC2 HCD
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index c291fca..6d33ecf 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -141,6 +141,7 @@ static int dwc2_driver_probe(struct pci_dev *dev,

pci_set_master(dev);

+ spin_lock_init(&hsotg->lock);
retval = dwc2_hcd_init(hsotg, dev->irq, &dwc2_module_params);
if (retval) {
pci_disable_device(dev);
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 121dbda..5783ed0 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -121,6 +121,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);

dwc2_hcd_remove(hsotg);
+ s3c_hsotg_remove(hsotg);

return 0;
}
@@ -129,6 +130,7 @@ static const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "brcm,bcm2835-usb", .data = &params_bcm2835 },
{ .compatible = "rockchip,rk3066-usb", .data = &params_rk3066 },
{ .compatible = "snps,dwc2", .data = NULL },
+ { .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
@@ -204,6 +206,10 @@ static int dwc2_driver_probe(struct platform_device *dev)

hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);

+ spin_lock_init(&hsotg->lock);
+ retval = dwc2_gadget_init(hsotg, irq);
+ if (retval)
+ return retval;
retval = dwc2_hcd_init(hsotg, irq, params);
if (retval)
return retval;
@@ -213,10 +219,36 @@ static int dwc2_driver_probe(struct platform_device *dev)
return retval;
}

+static int dwc2_suspend(struct device *dev)
+{
+ struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
+ int ret = 0;
+
+ if (dwc2_is_device_mode(dwc2))
+ ret = s3c_hsotg_suspend(dwc2);
+ return ret;
+}
+
+static int dwc2_resume(struct device *dev)
+{
+ struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
+ int ret = 0;
+
+ if (dwc2_is_device_mode(dwc2))
+ ret = s3c_hsotg_resume(dwc2);
+
+ return ret;
+}
+
+static const struct dev_pm_ops dwc2_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(dwc2_suspend, dwc2_resume)
+};
+
static struct platform_driver dwc2_platform_driver = {
.driver = {
.name = dwc2_driver_name,
.of_match_table = dwc2_of_match_table,
+ .pm = &dwc2_dev_pm_ops,
},
.probe = dwc2_driver_probe,
.remove = dwc2_driver_remove,
--
2.0.3

2014-10-20 18:56:51

by Dinh Nguyen

[permalink] [raw]
Subject: [PATCHv5 3/7] usb: dwc2: Initialize the USB core for peripheral mode

From: Dinh Nguyen <[email protected]>

Initialize the USB driver to peripheral mode when a B-Device connector
is attached.

Signed-off-by: Dinh Nguyen <[email protected]>
Acked-by: Paul Zimmerman <[email protected]>
---
v5: move the export of s3c_hsotg_core_init into this patch
---
drivers/usb/dwc2/core.h | 2 ++
drivers/usb/dwc2/gadget.c | 2 +-
drivers/usb/dwc2/hcd.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index b21aace..de1f357 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -966,6 +966,7 @@ extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
+extern void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2);
#else
static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
{ return 0; }
@@ -975,6 +976,7 @@ static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
{ return 0; }
static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{ return 0; }
+static inline void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2) {}
#endif

#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index c407d33..6793ca8 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2125,7 +2125,7 @@ static int s3c_hsotg_corereset(struct dwc2_hsotg *hsotg)
*
* Issue a soft reset to the core, and await the core finishing it.
*/
-static void s3c_hsotg_core_init(struct dwc2_hsotg *hsotg)
+void s3c_hsotg_core_init(struct dwc2_hsotg *hsotg)
{
s3c_hsotg_corereset(hsotg);

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 4a3cce0..44c609f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1371,6 +1371,7 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
hsotg->op_state = OTG_STATE_B_PERIPHERAL;
dwc2_core_init(hsotg, false, -1);
dwc2_enable_global_interrupts(hsotg);
+ s3c_hsotg_core_init(hsotg);
} else {
/* A-Device connector (Host Mode) */
dev_dbg(hsotg->dev, "connId A\n");
--
2.0.3

2014-10-20 18:56:59

by Dinh Nguyen

[permalink] [raw]
Subject: [PATCHv5 4/7] usb: dwc2: Update common interrupt handler to call gadget interrupt handler

From: Dinh Nguyen <[email protected]>

Make dwc2_handle_common_intr call the gadget interrupt function when operating
in peripheral mode. Remove the spinlock functions in s3c_hsotg_irq as
dwc2_handle_common_intr() already has the spinlocks.

Move the registeration of the IRQ to common code for platform and PCI.

Remove duplicate interrupt conditions that was in gadget, as those are handled
by dwc2 common interrupt handler.

Signed-off-by: Dinh Nguyen <[email protected]>
Acked-by: Paul Zimmerman <[email protected]>
---
v5: remove individual devm_request_irq from gadget and hcd, and place a
single devm_request_irq in platform and pci.
v2: Keep interrupt handler for host and peripheral modes separate
---
drivers/usb/dwc2/core.c | 10 --------
drivers/usb/dwc2/core.h | 3 +++
drivers/usb/dwc2/core_intr.c | 3 +++
drivers/usb/dwc2/gadget.c | 57 ++------------------------------------------
drivers/usb/dwc2/pci.c | 6 +++++
drivers/usb/dwc2/platform.c | 9 +++++++
6 files changed, 23 insertions(+), 65 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index d926945..7605850b 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -458,16 +458,6 @@ int dwc2_core_init(struct dwc2_hsotg *hsotg, bool select_phy, int irq)
/* Clear the SRP success bit for FS-I2c */
hsotg->srp_success = 0;

- if (irq >= 0) {
- dev_dbg(hsotg->dev, "registering common handler for irq%d\n",
- irq);
- retval = devm_request_irq(hsotg->dev, irq,
- dwc2_handle_common_intr, IRQF_SHARED,
- dev_name(hsotg->dev), hsotg);
- if (retval)
- return retval;
- }
-
/* Enable common interrupts */
dwc2_enable_common_interrupts(hsotg);

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index de1f357..ba488ec 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -967,6 +967,7 @@ extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
extern void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2);
+irqreturn_t s3c_hsotg_irq(int irq, void *pw);
#else
static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
{ return 0; }
@@ -977,6 +978,8 @@ static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{ return 0; }
static inline void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2) {}
+static inline irqreturn_t s3c_hsotg_irq(int irq, void *pw)
+{ return IRQ_HANDLED; }
#endif

#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index b176c2f..b0c14e0 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -474,6 +474,9 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)

spin_lock(&hsotg->lock);

+ if (dwc2_is_device_mode(hsotg))
+ retval = s3c_hsotg_irq(irq, dev);
+
gintsts = dwc2_read_common_intr(hsotg);
if (gintsts & ~GINTSTS_PRTINT)
retval = IRQ_HANDLED;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6793ca8..6ffbfc2 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2257,14 +2257,13 @@ void s3c_hsotg_core_init(struct dwc2_hsotg *hsotg)
* @irq: The IRQ number triggered
* @pw: The pw value when registered the handler.
*/
-static irqreturn_t s3c_hsotg_irq(int irq, void *pw)
+irqreturn_t s3c_hsotg_irq(int irq, void *pw)
{
struct dwc2_hsotg *hsotg = pw;
int retry_count = 8;
u32 gintsts;
u32 gintmsk;

- spin_lock(&hsotg->lock);
irq_retry:
gintsts = readl(hsotg->regs + GINTSTS);
gintmsk = readl(hsotg->regs + GINTMSK);
@@ -2274,33 +2273,12 @@ irq_retry:

gintsts &= gintmsk;

- if (gintsts & GINTSTS_OTGINT) {
- u32 otgint = readl(hsotg->regs + GOTGINT);
-
- dev_info(hsotg->dev, "OTGInt: %08x\n", otgint);
-
- writel(otgint, hsotg->regs + GOTGINT);
- }
-
- if (gintsts & GINTSTS_SESSREQINT) {
- dev_dbg(hsotg->dev, "%s: SessReqInt\n", __func__);
- writel(GINTSTS_SESSREQINT, hsotg->regs + GINTSTS);
- }
-
if (gintsts & GINTSTS_ENUMDONE) {
writel(GINTSTS_ENUMDONE, hsotg->regs + GINTSTS);

s3c_hsotg_irq_enumdone(hsotg);
}

- if (gintsts & GINTSTS_CONIDSTSCHNG) {
- dev_dbg(hsotg->dev, "ConIDStsChg (DSTS=0x%08x, GOTCTL=%08x)\n",
- readl(hsotg->regs + DSTS),
- readl(hsotg->regs + GOTGCTL));
-
- writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
- }
-
if (gintsts & (GINTSTS_OEPINT | GINTSTS_IEPINT)) {
u32 daint = readl(hsotg->regs + DAINT);
u32 daintmsk = readl(hsotg->regs + DAINTMSK);
@@ -2381,25 +2359,6 @@ irq_retry:
s3c_hsotg_handle_rx(hsotg);
}

- if (gintsts & GINTSTS_MODEMIS) {
- dev_warn(hsotg->dev, "warning, mode mismatch triggered\n");
- writel(GINTSTS_MODEMIS, hsotg->regs + GINTSTS);
- }
-
- if (gintsts & GINTSTS_USBSUSP) {
- dev_info(hsotg->dev, "GINTSTS_USBSusp\n");
- writel(GINTSTS_USBSUSP, hsotg->regs + GINTSTS);
-
- call_gadget(hsotg, suspend);
- }
-
- if (gintsts & GINTSTS_WKUPINT) {
- dev_info(hsotg->dev, "GINTSTS_WkUpIn\n");
- writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
-
- call_gadget(hsotg, resume);
- }
-
if (gintsts & GINTSTS_ERLYSUSP) {
dev_dbg(hsotg->dev, "GINTSTS_ErlySusp\n");
writel(GINTSTS_ERLYSUSP, hsotg->regs + GINTSTS);
@@ -2435,10 +2394,9 @@ irq_retry:
if (gintsts & IRQ_RETRY_MASK && --retry_count > 0)
goto irq_retry;

- spin_unlock(&hsotg->lock);
-
return IRQ_HANDLED;
}
+EXPORT_SYMBOL(s3c_hsotg_irq);

/**
* s3c_hsotg_ep_enable - enable the given endpoint
@@ -3491,17 +3449,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
s3c_hsotg_hw_cfg(hsotg);
s3c_hsotg_init(hsotg);

- ret = devm_request_irq(dev, irq, s3c_hsotg_irq, 0,
- dev_name(dev), hsotg);
- if (ret < 0) {
- s3c_hsotg_phy_disable(hsotg);
- clk_disable_unprepare(hsotg->clk);
- regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
- hsotg->supplies);
- dev_err(dev, "cannot claim IRQ\n");
- goto err_clk;
- }
-
/* hsotg->num_of_eps holds number of EPs other than ep0 */

if (hsotg->num_of_eps == 0) {
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index 6d33ecf..a4e724b 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -141,6 +141,12 @@ static int dwc2_driver_probe(struct pci_dev *dev,

pci_set_master(dev);

+ retval = devm_request_irq(hsotg->dev, dev->irq,
+ dwc2_handle_common_intr, IRQF_SHARED,
+ dev_name(hsotg->dev), hsotg);
+ if (retval)
+ return retval;
+
spin_lock_init(&hsotg->lock);
retval = dwc2_hcd_init(hsotg, dev->irq, &dwc2_module_params);
if (retval) {
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 5783ed0..72f32f7 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -196,6 +196,15 @@ static int dwc2_driver_probe(struct platform_device *dev)
return irq;
}

+ dev_dbg(hsotg->dev, "registering common handler for irq%d\n",
+ irq);
+
+ retval = devm_request_irq(hsotg->dev, irq,
+ dwc2_handle_common_intr, IRQF_SHARED,
+ dev_name(hsotg->dev), hsotg);
+ if (retval)
+ return retval;
+
res = platform_get_resource(dev, IORESOURCE_MEM, 0);
hsotg->regs = devm_ioremap_resource(&dev->dev, res);
if (IS_ERR(hsotg->regs))
--
2.0.3

2014-10-20 18:57:08

by Dinh Nguyen

[permalink] [raw]
Subject: [PATCHv5 5/7] usb: dwc2: Add call_gadget functions for perpheral mode interrupts

From: Dinh Nguyen <[email protected]>

Update the dwc2 wakeup and suspend interrupt functions to use call_gadget
when the IP is in peripheral mode.

Signed-off-by: Dinh Nguyen <[email protected]>
Acked-by: Paul Zimmerman <[email protected]>
---
drivers/usb/dwc2/core_intr.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index b0c14e0..1240875 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -339,6 +339,7 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
}
/* Change to L0 state */
hsotg->lx_state = DWC2_L0;
+ call_gadget(hsotg, resume);
} else {
if (hsotg->lx_state != DWC2_L1) {
u32 pcgcctl = readl(hsotg->regs + PCGCTL);
@@ -399,6 +400,7 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg)
"DSTS.Suspend Status=%d HWCFG4.Power Optimize=%d\n",
!!(dsts & DSTS_SUSPSTS),
hsotg->hw_params.power_optimized);
+ call_gadget(hsotg, suspend);
} else {
if (hsotg->op_state == OTG_STATE_A_PERIPHERAL) {
dev_dbg(hsotg->dev, "a_peripheral->a_host\n");
--
2.0.3

2014-10-20 18:57:16

by Dinh Nguyen

[permalink] [raw]
Subject: [PATCHv5 6/7] usb: dwc2: gadget: Do not fail probe if there isn't a clock node

From: Dinh Nguyen <[email protected]>

Since the dwc2 hcd driver is currently not looking for a clock node during
init, we should not completely fail if there isn't a clock provided.
For dual-role mode, we will only fail init for a non-clock node error. We
then update the HCD to only call gadget funtions if there is a proper clock
node.

Signed-off-by: Dinh Nguyen <[email protected]>
---
v5: reworked to not access gadget functions from the hcd.
---
drivers/usb/dwc2/core.h | 3 +--
drivers/usb/dwc2/core_intr.c | 9 ++++++---
drivers/usb/dwc2/hcd.c | 3 ++-
drivers/usb/dwc2/platform.c | 19 +++++++++++++++----
4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index ba488ec..8794c08 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -660,6 +660,7 @@ struct dwc2_hsotg {
#endif
#endif /* CONFIG_USB_DWC2_HOST || CONFIG_USB_DWC2_DUAL_ROLE */

+ struct clk *clk;
#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
/* Gadget structures */
struct usb_gadget_driver *driver;
@@ -667,8 +668,6 @@ struct dwc2_hsotg {
struct usb_phy *uphy;
struct s3c_hsotg_plat *plat;

- struct clk *clk;
-
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];

u32 phyif;
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 1240875..1608037 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -339,7 +339,8 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
}
/* Change to L0 state */
hsotg->lx_state = DWC2_L0;
- call_gadget(hsotg, resume);
+ if (!IS_ERR(hsotg->clk))
+ call_gadget(hsotg, resume);
} else {
if (hsotg->lx_state != DWC2_L1) {
u32 pcgcctl = readl(hsotg->regs + PCGCTL);
@@ -400,7 +401,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg)
"DSTS.Suspend Status=%d HWCFG4.Power Optimize=%d\n",
!!(dsts & DSTS_SUSPSTS),
hsotg->hw_params.power_optimized);
- call_gadget(hsotg, suspend);
+ if (!IS_ERR(hsotg->clk))
+ call_gadget(hsotg, suspend);
} else {
if (hsotg->op_state == OTG_STATE_A_PERIPHERAL) {
dev_dbg(hsotg->dev, "a_peripheral->a_host\n");
@@ -477,7 +479,8 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
spin_lock(&hsotg->lock);

if (dwc2_is_device_mode(hsotg))
- retval = s3c_hsotg_irq(irq, dev);
+ if (!IS_ERR(hsotg->clk))
+ retval = s3c_hsotg_irq(irq, dev);

gintsts = dwc2_read_common_intr(hsotg);
if (gintsts & ~GINTSTS_PRTINT)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 44c609f..fa49c72 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1371,7 +1371,8 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
hsotg->op_state = OTG_STATE_B_PERIPHERAL;
dwc2_core_init(hsotg, false, -1);
dwc2_enable_global_interrupts(hsotg);
- s3c_hsotg_core_init(hsotg);
+ if (!IS_ERR(hsotg->clk))
+ s3c_hsotg_core_init(hsotg);
} else {
/* A-Device connector (Host Mode) */
dev_dbg(hsotg->dev, "connId A\n");
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 72f32f7..77c8417 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -217,8 +217,17 @@ static int dwc2_driver_probe(struct platform_device *dev)

spin_lock_init(&hsotg->lock);
retval = dwc2_gadget_init(hsotg, irq);
- if (retval)
- return retval;
+ if (retval) {
+ /*
+ * We will not fail the driver initialization for dual-role
+ * if no clock node is supplied. However, all gadget
+ * functionality will be disabled if a clock node is not
+ * provided. Host functionality will continue.
+ * TO-DO: make clock node a requirement for the HCD.
+ */
+ if (!IS_ERR(hsotg->clk))
+ return retval;
+ }
retval = dwc2_hcd_init(hsotg, irq, params);
if (retval)
return retval;
@@ -234,7 +243,8 @@ static int dwc2_suspend(struct device *dev)
int ret = 0;

if (dwc2_is_device_mode(dwc2))
- ret = s3c_hsotg_suspend(dwc2);
+ if (!IS_ERR(dwc2->clk))
+ ret = s3c_hsotg_suspend(dwc2);
return ret;
}

@@ -244,7 +254,8 @@ static int dwc2_resume(struct device *dev)
int ret = 0;

if (dwc2_is_device_mode(dwc2))
- ret = s3c_hsotg_resume(dwc2);
+ if (!IS_ERR(dwc2->clk))
+ ret = s3c_hsotg_resume(dwc2);

return ret;
}
--
2.0.3

2014-10-20 18:57:22

by Dinh Nguyen

[permalink] [raw]
Subject: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

From: Dinh Nguyen <[email protected]>

Update DWC2 kconfig and makefile to support dual-role mode. The platform
file will always get compiled for the case where the controller is directly
connected to the CPU. So for loadable modules, dwc2.ko is built for host,
peripheral, and dual-role mode. The PCI bus interface will be called
dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.

Signed-off-by: Dinh Nguyen <[email protected]>
Acked-by: Paul Zimmerman <[email protected]>
---
v5: dwc2.ko for all modes along with dwc2_platform.ko and dwc2_pci.ko will
get built for kernel modules.
v3: Add USB_GADGET=y and USB_GADGET=USB_DWC2 for peripheral and dual-role
config options.
v2: Remove reference to dwc2_gadget
---
drivers/usb/dwc2/Kconfig | 61 ++++++++++++++++++++++++++++-------------------
drivers/usb/dwc2/Makefile | 32 ++++++++++++-------------
2 files changed, 53 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index f93807b..1ea702e 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -1,5 +1,5 @@
config USB_DWC2
- bool "DesignWare USB2 DRD Core Support"
+ tristate "DesignWare USB2 DRD Core Support"
depends on USB
help
Say Y here if your system has a Dual Role Hi-Speed USB
@@ -10,31 +10,53 @@ config USB_DWC2
bus interface module (if you have a PCI bus system) will be
called dwc2_pci.ko, and the platform interface module (for
controllers directly connected to the CPU) will be called
- dwc2_platform.ko. For gadget mode, there will be a single
- module called dwc2_gadget.ko.
-
- NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
- host and gadget drivers are still currently separate drivers.
- There are plans to merge the dwc2_gadget driver with the dwc2
- host driver in the near future to create a dual-role driver.
+ dwc2_platform.ko. For all modes(host, gadget and dual-role), there
+ will be a single module called dwc2.ko.

if USB_DWC2

+choice
+ bool "DWC2 Mode Selection"
+ default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
+ default USB_DWC2_HOST if (USB && !USB_GADGET)
+ default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
+
config USB_DWC2_HOST
- tristate "Host only mode"
+ bool "Host only mode"
depends on USB
help
The Designware USB2.0 high-speed host controller
- integrated into many SoCs.
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Host-only mode.
+
+comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
+
+config USB_DWC2_PERIPHERAL
+ bool "Gadget only mode"
+ depends on USB_GADGET=y || USB_GADGET=USB_DWC2
+ help
+ The Designware USB2.0 high-speed gadget controller
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Peripheral-only mode. This option requires
+ USB_GADGET=y.
+
+config USB_DWC2_DUAL_ROLE
+ bool "Dual Role mode"
+ depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
+ help
+ Select this option if you want the driver to work in a dual-role
+ mode. In this mode both host and gadget features are enabled, and
+ the role will be determined by the cable that gets plugged-in. This
+ option requires USB_GADGET=y.
+endchoice

config USB_DWC2_PLATFORM
bool "DWC2 Platform"
- depends on USB_DWC2_HOST
default USB_DWC2_HOST
- help
- The Designware USB2.0 platform interface module for
- controllers directly connected to the CPU. This is only
- used for host mode.
+ default y
+ help
+ The Designware USB2.0 platform interface module for
+ controllers directly connected to the CPU.

config USB_DWC2_PCI
bool "DWC2 PCI"
@@ -44,15 +66,6 @@ config USB_DWC2_PCI
The Designware USB2.0 PCI interface module for controllers
connected to a PCI bus. This is only used for host mode.

-comment "Gadget mode requires USB Gadget support to be enabled"
-
-config USB_DWC2_PERIPHERAL
- tristate "Gadget only mode"
- depends on USB_GADGET
- help
- The Designware USB2.0 high-speed gadget controller
- integrated into many SoCs.
-
config USB_DWC2_DEBUG
bool "Enable Debugging Messages"
help
diff --git a/drivers/usb/dwc2/Makefile b/drivers/usb/dwc2/Makefile
index b73d2a5..2175d93 100644
--- a/drivers/usb/dwc2/Makefile
+++ b/drivers/usb/dwc2/Makefile
@@ -1,28 +1,28 @@
ccflags-$(CONFIG_USB_DWC2_DEBUG) += -DDEBUG
ccflags-$(CONFIG_USB_DWC2_VERBOSE) += -DVERBOSE_DEBUG

-obj-$(CONFIG_USB_DWC2_HOST) += dwc2.o
+obj-$(CONFIG_USB_DWC2) += dwc2.o
dwc2-y := core.o core_intr.o
-dwc2-y += hcd.o hcd_intr.o
-dwc2-y += hcd_queue.o hcd_ddma.o
+
+ifneq ($(filter y,$(CONFIG_USB_DWC2_HOST) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
+ dwc2-y += hcd.o hcd_intr.o
+ dwc2-y += hcd_queue.o hcd_ddma.o
+endif
+
+ifneq ($(filter y,$(CONFIG_USB_DWC2_PERIPHERAL) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
+ dwc2-y += gadget.o
+endif

# NOTE: The previous s3c-hsotg peripheral mode only driver has been moved to
# this location and renamed gadget.c. When building for dynamically linked
-# modules, dwc2_gadget.ko will get built for peripheral mode. For host mode,
-# the core module will be dwc2.ko, the PCI bus interface module will called
-# dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
-# At present the host and gadget driver will be separate drivers, but there
-# are plans in the near future to create a dual-role driver.
+# modules, dwc2.ko will get built for host mode, peripheral mode, and dual-role
+# mode. The PCI bus interface module will called dwc2_pci.ko and the platform
+# interface module will be called dwc2_platform.ko.

ifneq ($(CONFIG_USB_DWC2_PCI),)
- obj-$(CONFIG_USB_DWC2_HOST) += dwc2_pci.o
+ obj-$(CONFIG_USB_DWC2) += dwc2_pci.o
dwc2_pci-y := pci.o
endif

-ifneq ($(CONFIG_USB_DWC2_PLATFORM),)
- obj-$(CONFIG_USB_DWC2_HOST) += dwc2_platform.o
- dwc2_platform-y := platform.o
-endif
-
-obj-$(CONFIG_USB_DWC2_PERIPHERAL) += dwc2_gadget.o
-dwc2_gadget-y := gadget.o
+obj-$(CONFIG_USB_DWC2) += dwc2_platform.o
+dwc2_platform-y := platform.o
--
2.0.3

2014-10-20 19:42:38

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

[email protected] schreef op ma 20-10-2014 om 13:52
[-0500]:
> From: Dinh Nguyen <[email protected]>
>
> Update DWC2 kconfig and makefile to support dual-role mode. The platform
> file will always get compiled for the case where the controller is directly
> connected to the CPU. So for loadable modules, dwc2.ko is built for host,
> peripheral, and dual-role mode. The PCI bus interface will be called
> dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
>
> Signed-off-by: Dinh Nguyen <[email protected]>
> Acked-by: Paul Zimmerman <[email protected]>
> ---
> v5: dwc2.ko for all modes along with dwc2_platform.ko and dwc2_pci.ko will
> get built for kernel modules.
> v3: Add USB_GADGET=y and USB_GADGET=USB_DWC2 for peripheral and dual-role
> config options.
> v2: Remove reference to dwc2_gadget
> ---
> drivers/usb/dwc2/Kconfig | 61 ++++++++++++++++++++++++++++-------------------
> drivers/usb/dwc2/Makefile | 32 ++++++++++++-------------
> 2 files changed, 53 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
> index f93807b..1ea702e 100644
> --- a/drivers/usb/dwc2/Kconfig
> +++ b/drivers/usb/dwc2/Kconfig
> @@ -1,5 +1,5 @@
> config USB_DWC2
> - bool "DesignWare USB2 DRD Core Support"
> + tristate "DesignWare USB2 DRD Core Support"
> depends on USB

(Side note: drivers/usb/dwc2/Kconfig is sourced (in drivers/usb/Kconfig)
even if USB is _not_ set. But USB_DCW2 still depends on USB. Why is
that?)

> help
> Say Y here if your system has a Dual Role Hi-Speed USB
> @@ -10,31 +10,53 @@ config USB_DWC2
> bus interface module (if you have a PCI bus system) will be
> called dwc2_pci.ko, and the platform interface module (for
> controllers directly connected to the CPU) will be called
> - dwc2_platform.ko. For gadget mode, there will be a single
> - module called dwc2_gadget.ko.
> -
> - NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
> - host and gadget drivers are still currently separate drivers.
> - There are plans to merge the dwc2_gadget driver with the dwc2
> - host driver in the near future to create a dual-role driver.
> + dwc2_platform.ko. For all modes(host, gadget and dual-role), there
> + will be a single module called dwc2.ko.
>
> if USB_DWC2
>
> +choice
> + bool "DWC2 Mode Selection"
> + default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
> + default USB_DWC2_HOST if (USB && !USB_GADGET)
> + default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
> +

Juggling kconfig tristate logic is still rather hard for me, but don't
the above three "if" rules all evaluate to non-zero if both USB and
USB_GADGET are 'm'? If that's correct perhaps USB_DWC2_DUAL_ROLE should
be the last default (assuming the last default wins, that is).

Besides, will the default USB_DWC2_PERIPHERAL ever trigger as !USB can't
be true at this point, can it?

> config USB_DWC2_HOST
> - tristate "Host only mode"
> + bool "Host only mode"
> depends on USB
> help
> The Designware USB2.0 high-speed host controller
> - integrated into many SoCs.
> + integrated into many SoCs. Select this option if you want the
> + driver to operate in Host-only mode.
> +
> +comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
> +
> +config USB_DWC2_PERIPHERAL
> + bool "Gadget only mode"
> + depends on USB_GADGET=y || USB_GADGET=USB_DWC2
> + help
> + The Designware USB2.0 high-speed gadget controller
> + integrated into many SoCs. Select this option if you want the
> + driver to operate in Peripheral-only mode. This option requires
> + USB_GADGET=y.
> +
> +config USB_DWC2_DUAL_ROLE
> + bool "Dual Role mode"
> + depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
> + help
> + Select this option if you want the driver to work in a dual-role
> + mode. In this mode both host and gadget features are enabled, and
> + the role will be determined by the cable that gets plugged-in. This
> + option requires USB_GADGET=y.
> +endchoice

(I wonder how the dependencies of these three config entries interact
with the three defaults of this choice. Since we're dealing with three
options here this requires a piece of paper, a pencil, and clear mind to
figure out. Maybe I'll try that in a few days...)

> config USB_DWC2_PLATFORM
> bool "DWC2 Platform"
> - depends on USB_DWC2_HOST
> default USB_DWC2_HOST
> - help
> - The Designware USB2.0 platform interface module for
> - controllers directly connected to the CPU. This is only
> - used for host mode.
> + default y

You now have to default lines. Which one wins?

> + help
> + The Designware USB2.0 platform interface module for
> + controllers directly connected to the CPU.
>
> config USB_DWC2_PCI
> bool "DWC2 PCI"
> @@ -44,15 +66,6 @@ config USB_DWC2_PCI
> The Designware USB2.0 PCI interface module for controllers
> connected to a PCI bus. This is only used for host mode.
>
> -comment "Gadget mode requires USB Gadget support to be enabled"
> -
> -config USB_DWC2_PERIPHERAL
> - tristate "Gadget only mode"
> - depends on USB_GADGET
> - help
> - The Designware USB2.0 high-speed gadget controller
> - integrated into many SoCs.
> -
> config USB_DWC2_DEBUG
> bool "Enable Debugging Messages"
> help
> diff --git a/drivers/usb/dwc2/Makefile b/drivers/usb/dwc2/Makefile
> index b73d2a5..2175d93 100644


Paul Bolle

2014-10-21 20:51:28

by Dinh Nguyen

[permalink] [raw]
Subject: Re: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

On 10/20/2014 02:42 PM, Paul Bolle wrote:
> [email protected] schreef op ma 20-10-2014 om 13:52
> [-0500]:
>> From: Dinh Nguyen <[email protected]>
>>
>> Update DWC2 kconfig and makefile to support dual-role mode. The platform
>> file will always get compiled for the case where the controller is directly
>> connected to the CPU. So for loadable modules, dwc2.ko is built for host,
>> peripheral, and dual-role mode. The PCI bus interface will be called
>> dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
>>
>> Signed-off-by: Dinh Nguyen <[email protected]>
>> Acked-by: Paul Zimmerman <[email protected]>
>> ---
>> v5: dwc2.ko for all modes along with dwc2_platform.ko and dwc2_pci.ko will
>> get built for kernel modules.
>> v3: Add USB_GADGET=y and USB_GADGET=USB_DWC2 for peripheral and dual-role
>> config options.
>> v2: Remove reference to dwc2_gadget
>> ---
>> drivers/usb/dwc2/Kconfig | 61 ++++++++++++++++++++++++++++-------------------
>> drivers/usb/dwc2/Makefile | 32 ++++++++++++-------------
>> 2 files changed, 53 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
>> index f93807b..1ea702e 100644
>> --- a/drivers/usb/dwc2/Kconfig
>> +++ b/drivers/usb/dwc2/Kconfig
>> @@ -1,5 +1,5 @@
>> config USB_DWC2
>> - bool "DesignWare USB2 DRD Core Support"
>> + tristate "DesignWare USB2 DRD Core Support"
>> depends on USB
>
> (Side note: drivers/usb/dwc2/Kconfig is sourced (in drivers/usb/Kconfig)
> even if USB is _not_ set. But USB_DCW2 still depends on USB. Why is
> that?)

Because USB is for Host-Side support. DWC2 driver should only get built
for when USB is enabled. I think you're getting USB and USB_SUPPORT
mixed up.

In addition, thanks to your comment, I realized that DWC2 should also be
available to build when USB_GADGET is enabled. A patch has been sent:

http://marc.info/?l=linux-usb&m=141392377124818&w=2

>
>> help
>> Say Y here if your system has a Dual Role Hi-Speed USB
>> @@ -10,31 +10,53 @@ config USB_DWC2
>> bus interface module (if you have a PCI bus system) will be
>> called dwc2_pci.ko, and the platform interface module (for
>> controllers directly connected to the CPU) will be called
>> - dwc2_platform.ko. For gadget mode, there will be a single
>> - module called dwc2_gadget.ko.
>> -
>> - NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
>> - host and gadget drivers are still currently separate drivers.
>> - There are plans to merge the dwc2_gadget driver with the dwc2
>> - host driver in the near future to create a dual-role driver.
>> + dwc2_platform.ko. For all modes(host, gadget and dual-role), there
>> + will be a single module called dwc2.ko.
>>
>> if USB_DWC2
>>
>> +choice
>> + bool "DWC2 Mode Selection"
>> + default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
>> + default USB_DWC2_HOST if (USB && !USB_GADGET)
>> + default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
>> +
>
> Juggling kconfig tristate logic is still rather hard for me, but don't
> the above three "if" rules all evaluate to non-zero if both USB and
> USB_GADGET are 'm'? If that's correct perhaps USB_DWC2_DUAL_ROLE should
> be the last default (assuming the last default wins, that is).

As the way it is, the functionality is correct. As this is the same as
DWC3's Kconfig, perhaps Felipe can comment if something doesn't seem
correct.

>
> Besides, will the default USB_DWC2_PERIPHERAL ever trigger as !USB can't
> be true at this point, can it?

Yes it can. USB is for Host-side support, so you can have a condition
where you just want to build for GADGET and !USB.

>
>> config USB_DWC2_HOST
>> - tristate "Host only mode"
>> + bool "Host only mode"
>> depends on USB
>> help
>> The Designware USB2.0 high-speed host controller
>> - integrated into many SoCs.
>> + integrated into many SoCs. Select this option if you want the
>> + driver to operate in Host-only mode.
>> +
>> +comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
>> +
>> +config USB_DWC2_PERIPHERAL
>> + bool "Gadget only mode"
>> + depends on USB_GADGET=y || USB_GADGET=USB_DWC2
>> + help
>> + The Designware USB2.0 high-speed gadget controller
>> + integrated into many SoCs. Select this option if you want the
>> + driver to operate in Peripheral-only mode. This option requires
>> + USB_GADGET=y.
>> +
>> +config USB_DWC2_DUAL_ROLE
>> + bool "Dual Role mode"
>> + depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
>> + help
>> + Select this option if you want the driver to work in a dual-role
>> + mode. In this mode both host and gadget features are enabled, and
>> + the role will be determined by the cable that gets plugged-in. This
>> + option requires USB_GADGET=y.
>> +endchoice
>
> (I wonder how the dependencies of these three config entries interact
> with the three defaults of this choice. Since we're dealing with three
> options here this requires a piece of paper, a pencil, and clear mind to
> figure out. Maybe I'll try that in a few days...)
>
>> config USB_DWC2_PLATFORM
>> bool "DWC2 Platform"
>> - depends on USB_DWC2_HOST
>> default USB_DWC2_HOST
>> - help
>> - The Designware USB2.0 platform interface module for
>> - controllers directly connected to the CPU. This is only
>> - used for host mode.
>> + default y
>
> You now have to default lines. Which one wins?

Yes, "default y" should be removed.


Thanks,
Dinh

Subject: Re: [PATCHv5 2/7] usb: dwc2: Move gadget probe function into platform code


Hi,

On Monday, October 20, 2014 01:52:01 PM [email protected] wrote:
> From: Dinh Nguyen <[email protected]>
>
> This patch will aggregate the probing of gadget/hcd driver into platform.c.
> The gadget probe funtion is converted into gadget_init that is now only
> responsible for gadget only initialization. All the gadget resources is now
> handled by platform.c
>
> Since the host workqueue will not get initialized if the driver is configured
> for peripheral mode only. Thus we need to check for wq_otg before calling
> queue_work().
>
> Also, we move spin_lock_init to common location for both host and gadget that
> is either in platform.c or pci.c.
>
> We also ove suspend/resume code to common platform code, and update it to use
> the new PM API (struct dev_pm_ops).
>
> Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.

This patch seems to break bisectability. It moves all the gadget probing
to platform.c but Kconfig/Makefile are not updated (platform.c will be
compiled only for CONFIG_USB_DWC2_PLATFORM=y which in turn depends on
CONFIG_USB_DWC2_HOST). IMO patch #7 should be merged into this one (#2).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Signed-off-by: Dinh Nguyen <[email protected]>
> Acked-by: Paul Zimmerman <[email protected]>
> ---
> v5: Reworked by squashing the following commits into this one:
> * [PATCHv4 02/12] usb: dwc2: move "samsung,s3c6400-hsotg" into common platform
> * [PATCHv4 03/12] usb: dwc2: Update the gadget driver to use common dwc2_hsotg
> structure
> * [PATCHv4 09/12] usb: dwc2: initialize the spin_lock for both host and gadget
> * [PATCHv4 10/12] usb: dwc2: Add suspend/resume for gadget
> * [PATCHv4 11/12] usb: dwc2: check that the host work queue is valid
> Also use IS_ENABLED instead of #if defined
> ---
> drivers/usb/dwc2/core.h | 34 +++++++++++++++-
> drivers/usb/dwc2/core_intr.c | 8 ++--
> drivers/usb/dwc2/gadget.c | 97 +++++++++-----------------------------------
> drivers/usb/dwc2/hcd.c | 1 -
> drivers/usb/dwc2/hcd.h | 10 -----
> drivers/usb/dwc2/pci.c | 1 +
> drivers/usb/dwc2/platform.c | 32 +++++++++++++++
> 7 files changed, 91 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 5412f57..b21aace 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -667,7 +667,6 @@ struct dwc2_hsotg {
> struct usb_phy *uphy;
> struct s3c_hsotg_plat *plat;
>
> - int irq;
> struct clk *clk;
>
> struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
> @@ -961,4 +960,37 @@ extern void dwc2_dump_global_registers(struct dwc2_hsotg *hsotg);
> */
> extern u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg);
>
> +/* Gadget defines */
> +#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
> +extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
> +extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
> +extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
> +extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
> +#else
> +static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
> +{ return 0; }
> +static inline int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2)
> +{ return 0; }
> +static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
> +{ return 0; }
> +static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> +{ return 0; }
> +#endif
> +
> +#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
> +extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
> +extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
> +extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
> +#else
> +static inline void dwc2_set_all_params(struct dwc2_core_params *params, int value) {}
> +static inline int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg)
> +{ return 0; }
> +static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg) {}
> +static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
> +static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
> +static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> + const struct dwc2_core_params *params)
> +{ return 0; }
> +#endif
> +
> #endif /* __DWC2_CORE_H__ */
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index c93918b..b176c2f 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -287,9 +287,11 @@ static void dwc2_handle_conn_id_status_change_intr(struct dwc2_hsotg *hsotg)
> * Release lock before scheduling workq as it holds spinlock during
> * scheduling.
> */
> - spin_unlock(&hsotg->lock);
> - queue_work(hsotg->wq_otg, &hsotg->wf_otg);
> - spin_lock(&hsotg->lock);
> + if (hsotg->wq_otg) {
> + spin_unlock(&hsotg->lock);
> + queue_work(hsotg->wq_otg, &hsotg->wf_otg);
> + spin_lock(&hsotg->lock);
> + }
>
> /* Clear interrupt */
> writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 6611ea3..c407d33 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3404,26 +3404,21 @@ static void s3c_hsotg_delete_debug(struct dwc2_hsotg *hsotg)
> }
>
> /**
> - * s3c_hsotg_probe - probe function for hsotg driver
> - * @pdev: The platform information for the driver
> + * dwc2_gadget_init - init function for gadget
> + * @dwc2: The data structure for the DWC2 driver.
> + * @irq: The IRQ number for the controller.
> */
> -static int s3c_hsotg_probe(struct platform_device *pdev)
> +int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> {
> - struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
> + struct device *dev = hsotg->dev;
> + struct s3c_hsotg_plat *plat = dev->platform_data;
> struct phy *phy;
> struct usb_phy *uphy;
> - struct device *dev = &pdev->dev;
> struct s3c_hsotg_ep *eps;
> - struct dwc2_hsotg *hsotg;
> - struct resource *res;
> int epnum;
> int ret;
> int i;
>
> - hsotg = devm_kzalloc(&pdev->dev, sizeof(struct dwc2_hsotg), GFP_KERNEL);
> - if (!hsotg)
> - return -ENOMEM;
> -
> /* Set default UTMI width */
> hsotg->phyif = GUSBCFG_PHYIF16;
>
> @@ -3431,14 +3426,14 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> * Attempt to find a generic PHY, then look for an old style
> * USB PHY, finally fall back to pdata
> */
> - phy = devm_phy_get(&pdev->dev, "usb2-phy");
> + phy = devm_phy_get(dev, "usb2-phy");
> if (IS_ERR(phy)) {
> uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> if (IS_ERR(uphy)) {
> /* Fallback for pdata */
> - plat = dev_get_platdata(&pdev->dev);
> + plat = dev_get_platdata(dev);
> if (!plat) {
> - dev_err(&pdev->dev,
> + dev_err(dev,
> "no platform data or transceiver defined\n");
> return -EPROBE_DEFER;
> }
> @@ -3455,36 +3450,12 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> hsotg->phyif = GUSBCFG_PHYIF8;
> }
>
> - hsotg->dev = dev;
> -
> - hsotg->clk = devm_clk_get(&pdev->dev, "otg");
> + hsotg->clk = devm_clk_get(dev, "otg");
> if (IS_ERR(hsotg->clk)) {
> dev_err(dev, "cannot get otg clock\n");
> return PTR_ERR(hsotg->clk);
> }
>
> - platform_set_drvdata(pdev, hsotg);
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> - hsotg->regs = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(hsotg->regs)) {
> - ret = PTR_ERR(hsotg->regs);
> - goto err_clk;
> - }
> -
> - ret = platform_get_irq(pdev, 0);
> - if (ret < 0) {
> - dev_err(dev, "cannot find IRQ\n");
> - goto err_clk;
> - }
> -
> - spin_lock_init(&hsotg->lock);
> -
> - hsotg->irq = ret;
> -
> - dev_info(dev, "regs %p, irq %d\n", hsotg->regs, hsotg->irq);
> -
> hsotg->gadget.max_speed = USB_SPEED_HIGH;
> hsotg->gadget.ops = &s3c_hsotg_gadget_ops;
> hsotg->gadget.name = dev_name(dev);
> @@ -3501,7 +3472,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(hsotg->supplies),
> hsotg->supplies);
> if (ret) {
> - dev_err(hsotg->dev, "failed to request supplies: %d\n", ret);
> + dev_err(dev, "failed to request supplies: %d\n", ret);
> goto err_clk;
> }
>
> @@ -3520,7 +3491,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> s3c_hsotg_hw_cfg(hsotg);
> s3c_hsotg_init(hsotg);
>
> - ret = devm_request_irq(&pdev->dev, hsotg->irq, s3c_hsotg_irq, 0,
> + ret = devm_request_irq(dev, irq, s3c_hsotg_irq, 0,
> dev_name(dev), hsotg);
> if (ret < 0) {
> s3c_hsotg_phy_disable(hsotg);
> @@ -3572,7 +3543,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
> hsotg->supplies);
> if (ret) {
> - dev_err(&pdev->dev, "failed to disable supplies: %d\n", ret);
> + dev_err(dev, "failed to disable supplies: %d\n", ret);
> goto err_ep_mem;
> }
>
> @@ -3597,15 +3568,14 @@ err_clk:
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(dwc2_gadget_init);
>
> /**
> * s3c_hsotg_remove - remove function for hsotg driver
> * @pdev: The platform information for the driver
> */
> -static int s3c_hsotg_remove(struct platform_device *pdev)
> +int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
> {
> - struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
> -
> usb_del_gadget_udc(&hsotg->gadget);
>
> s3c_hsotg_delete_debug(hsotg);
> @@ -3619,10 +3589,10 @@ static int s3c_hsotg_remove(struct platform_device *pdev)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(s3c_hsotg_remove);
>
> -static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
> +int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
> {
> - struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
> unsigned long flags;
> int ret = 0;
>
> @@ -3648,10 +3618,10 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(s3c_hsotg_suspend);
>
> -static int s3c_hsotg_resume(struct platform_device *pdev)
> +int s3c_hsotg_resume(struct dwc2_hsotg *hsotg)
> {
> - struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
> unsigned long flags;
> int ret = 0;
>
> @@ -3672,31 +3642,4 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
>
> return ret;
> }
> -
> -#ifdef CONFIG_OF
> -static const struct of_device_id s3c_hsotg_of_ids[] = {
> - { .compatible = "samsung,s3c6400-hsotg", },
> - { .compatible = "snps,dwc2", },
> - { /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids);
> -#endif
> -
> -static struct platform_driver s3c_hsotg_driver = {
> - .driver = {
> - .name = "s3c-hsotg",
> - .owner = THIS_MODULE,
> - .of_match_table = of_match_ptr(s3c_hsotg_of_ids),
> - },
> - .probe = s3c_hsotg_probe,
> - .remove = s3c_hsotg_remove,
> - .suspend = s3c_hsotg_suspend,
> - .resume = s3c_hsotg_resume,
> -};
> -
> -module_platform_driver(s3c_hsotg_driver);
> -
> -MODULE_DESCRIPTION("Samsung S3C USB High-speed/OtG device");
> -MODULE_AUTHOR("Ben Dooks <[email protected]>");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:s3c-hsotg");
> +EXPORT_SYMBOL_GPL(s3c_hsotg_resume);
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 0a0e6f0..4a3cce0 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -2839,7 +2839,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
>
> hcd->has_tt = 1;
>
> - spin_lock_init(&hsotg->lock);
> ((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg;
> hsotg->priv = hcd;
>
> diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
> index a12bb15..e69a843 100644
> --- a/drivers/usb/dwc2/hcd.h
> +++ b/drivers/usb/dwc2/hcd.h
> @@ -668,9 +668,6 @@ extern irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg);
> */
> extern void dwc2_hcd_stop(struct dwc2_hsotg *hsotg);
>
> -extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
> -extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
> -
> /**
> * dwc2_hcd_is_b_host() - Returns 1 if core currently is acting as B host,
> * and 0 otherwise
> @@ -680,13 +677,6 @@ extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
> extern int dwc2_hcd_is_b_host(struct dwc2_hsotg *hsotg);
>
> /**
> - * dwc2_hcd_get_frame_number() - Returns current frame number
> - *
> - * @hsotg: The DWC2 HCD
> - */
> -extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
> -
> -/**
> * dwc2_hcd_dump_state() - Dumps hsotg state
> *
> * @hsotg: The DWC2 HCD
> diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
> index c291fca..6d33ecf 100644
> --- a/drivers/usb/dwc2/pci.c
> +++ b/drivers/usb/dwc2/pci.c
> @@ -141,6 +141,7 @@ static int dwc2_driver_probe(struct pci_dev *dev,
>
> pci_set_master(dev);
>
> + spin_lock_init(&hsotg->lock);
> retval = dwc2_hcd_init(hsotg, dev->irq, &dwc2_module_params);
> if (retval) {
> pci_disable_device(dev);
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 121dbda..5783ed0 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -121,6 +121,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
> struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
>
> dwc2_hcd_remove(hsotg);
> + s3c_hsotg_remove(hsotg);
>
> return 0;
> }
> @@ -129,6 +130,7 @@ static const struct of_device_id dwc2_of_match_table[] = {
> { .compatible = "brcm,bcm2835-usb", .data = &params_bcm2835 },
> { .compatible = "rockchip,rk3066-usb", .data = &params_rk3066 },
> { .compatible = "snps,dwc2", .data = NULL },
> + { .compatible = "samsung,s3c6400-hsotg", .data = NULL},
> {},
> };
> MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
> @@ -204,6 +206,10 @@ static int dwc2_driver_probe(struct platform_device *dev)
>
> hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
>
> + spin_lock_init(&hsotg->lock);
> + retval = dwc2_gadget_init(hsotg, irq);
> + if (retval)
> + return retval;
> retval = dwc2_hcd_init(hsotg, irq, params);
> if (retval)
> return retval;
> @@ -213,10 +219,36 @@ static int dwc2_driver_probe(struct platform_device *dev)
> return retval;
> }
>
> +static int dwc2_suspend(struct device *dev)
> +{
> + struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + if (dwc2_is_device_mode(dwc2))
> + ret = s3c_hsotg_suspend(dwc2);
> + return ret;
> +}
> +
> +static int dwc2_resume(struct device *dev)
> +{
> + struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + if (dwc2_is_device_mode(dwc2))
> + ret = s3c_hsotg_resume(dwc2);
> +
> + return ret;
> +}
> +
> +static const struct dev_pm_ops dwc2_dev_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(dwc2_suspend, dwc2_resume)
> +};
> +
> static struct platform_driver dwc2_platform_driver = {
> .driver = {
> .name = dwc2_driver_name,
> .of_match_table = dwc2_of_match_table,
> + .pm = &dwc2_dev_pm_ops,
> },
> .probe = dwc2_driver_probe,
> .remove = dwc2_driver_remove,

Subject: Re: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role


Hi,

On Monday, October 20, 2014 01:52:06 PM [email protected] wrote:
> From: Dinh Nguyen <[email protected]>
>
> Update DWC2 kconfig and makefile to support dual-role mode. The platform
> file will always get compiled for the case where the controller is directly
> connected to the CPU. So for loadable modules, dwc2.ko is built for host,
> peripheral, and dual-role mode. The PCI bus interface will be called
> dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
>
> Signed-off-by: Dinh Nguyen <[email protected]>
> Acked-by: Paul Zimmerman <[email protected]>
> ---
> v5: dwc2.ko for all modes along with dwc2_platform.ko and dwc2_pci.ko will
> get built for kernel modules.
> v3: Add USB_GADGET=y and USB_GADGET=USB_DWC2 for peripheral and dual-role
> config options.
> v2: Remove reference to dwc2_gadget
> ---
> drivers/usb/dwc2/Kconfig | 61 ++++++++++++++++++++++++++++-------------------
> drivers/usb/dwc2/Makefile | 32 ++++++++++++-------------
> 2 files changed, 53 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
> index f93807b..1ea702e 100644
> --- a/drivers/usb/dwc2/Kconfig
> +++ b/drivers/usb/dwc2/Kconfig
> @@ -1,5 +1,5 @@
> config USB_DWC2
> - bool "DesignWare USB2 DRD Core Support"
> + tristate "DesignWare USB2 DRD Core Support"
> depends on USB
> help
> Say Y here if your system has a Dual Role Hi-Speed USB
> @@ -10,31 +10,53 @@ config USB_DWC2
> bus interface module (if you have a PCI bus system) will be
> called dwc2_pci.ko, and the platform interface module (for
> controllers directly connected to the CPU) will be called
> - dwc2_platform.ko. For gadget mode, there will be a single
> - module called dwc2_gadget.ko.
> -
> - NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
> - host and gadget drivers are still currently separate drivers.
> - There are plans to merge the dwc2_gadget driver with the dwc2
> - host driver in the near future to create a dual-role driver.
> + dwc2_platform.ko. For all modes(host, gadget and dual-role), there
> + will be a single module called dwc2.ko.
>
> if USB_DWC2
>
> +choice
> + bool "DWC2 Mode Selection"
> + default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
> + default USB_DWC2_HOST if (USB && !USB_GADGET)
> + default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
> +
> config USB_DWC2_HOST
> - tristate "Host only mode"
> + bool "Host only mode"
> depends on USB
> help
> The Designware USB2.0 high-speed host controller
> - integrated into many SoCs.
> + integrated into many SoCs. Select this option if you want the
> + driver to operate in Host-only mode.
> +
> +comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
> +
> +config USB_DWC2_PERIPHERAL
> + bool "Gadget only mode"
> + depends on USB_GADGET=y || USB_GADGET=USB_DWC2
> + help
> + The Designware USB2.0 high-speed gadget controller
> + integrated into many SoCs. Select this option if you want the
> + driver to operate in Peripheral-only mode. This option requires
> + USB_GADGET=y.
> +
> +config USB_DWC2_DUAL_ROLE
> + bool "Dual Role mode"
> + depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
> + help
> + Select this option if you want the driver to work in a dual-role
> + mode. In this mode both host and gadget features are enabled, and
> + the role will be determined by the cable that gets plugged-in. This
> + option requires USB_GADGET=y.
> +endchoice
>
> config USB_DWC2_PLATFORM
> bool "DWC2 Platform"
> - depends on USB_DWC2_HOST
> default USB_DWC2_HOST

It should be "default USB_DWC2_HOST || USB_DWC2_PERIPHERAL" because
USB_DWC2_PLATFORM replaces current USB_DWC2_PERIPHERAL functionality.

Additionaly USB_DWC2_PLATFORM should be changed to tristate
(USB_DWC2_PERIPHERAL was a tristeate before your changes).

BTW It is a bit late but it would be great if you could split your
patchset on two. First one merging gadget functionality into
core/platform code and the second one adding USB_DWC2_DUAL_ROLE
functionality.

> - help
> - The Designware USB2.0 platform interface module for
> - controllers directly connected to the CPU. This is only
> - used for host mode.
> + default y
> + help
> + The Designware USB2.0 platform interface module for
> + controllers directly connected to the CPU.
>
> config USB_DWC2_PCI
> bool "DWC2 PCI"
> @@ -44,15 +66,6 @@ config USB_DWC2_PCI
> The Designware USB2.0 PCI interface module for controllers
> connected to a PCI bus. This is only used for host mode.
>
> -comment "Gadget mode requires USB Gadget support to be enabled"
> -
> -config USB_DWC2_PERIPHERAL
> - tristate "Gadget only mode"
> - depends on USB_GADGET
> - help
> - The Designware USB2.0 high-speed gadget controller
> - integrated into many SoCs.
> -
> config USB_DWC2_DEBUG
> bool "Enable Debugging Messages"
> help
> diff --git a/drivers/usb/dwc2/Makefile b/drivers/usb/dwc2/Makefile
> index b73d2a5..2175d93 100644
> --- a/drivers/usb/dwc2/Makefile
> +++ b/drivers/usb/dwc2/Makefile
> @@ -1,28 +1,28 @@
> ccflags-$(CONFIG_USB_DWC2_DEBUG) += -DDEBUG
> ccflags-$(CONFIG_USB_DWC2_VERBOSE) += -DVERBOSE_DEBUG
>
> -obj-$(CONFIG_USB_DWC2_HOST) += dwc2.o
> +obj-$(CONFIG_USB_DWC2) += dwc2.o
> dwc2-y := core.o core_intr.o
> -dwc2-y += hcd.o hcd_intr.o
> -dwc2-y += hcd_queue.o hcd_ddma.o
> +
> +ifneq ($(filter y,$(CONFIG_USB_DWC2_HOST) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
> + dwc2-y += hcd.o hcd_intr.o
> + dwc2-y += hcd_queue.o hcd_ddma.o
> +endif
> +
> +ifneq ($(filter y,$(CONFIG_USB_DWC2_PERIPHERAL) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
> + dwc2-y += gadget.o
> +endif
>
> # NOTE: The previous s3c-hsotg peripheral mode only driver has been moved to
> # this location and renamed gadget.c. When building for dynamically linked
> -# modules, dwc2_gadget.ko will get built for peripheral mode. For host mode,
> -# the core module will be dwc2.ko, the PCI bus interface module will called
> -# dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
> -# At present the host and gadget driver will be separate drivers, but there
> -# are plans in the near future to create a dual-role driver.
> +# modules, dwc2.ko will get built for host mode, peripheral mode, and dual-role
> +# mode. The PCI bus interface module will called dwc2_pci.ko and the platform
> +# interface module will be called dwc2_platform.ko.
>
> ifneq ($(CONFIG_USB_DWC2_PCI),)
> - obj-$(CONFIG_USB_DWC2_HOST) += dwc2_pci.o
> + obj-$(CONFIG_USB_DWC2) += dwc2_pci.o
> dwc2_pci-y := pci.o
> endif
>
> -ifneq ($(CONFIG_USB_DWC2_PLATFORM),)
> - obj-$(CONFIG_USB_DWC2_HOST) += dwc2_platform.o
> - dwc2_platform-y := platform.o
> -endif
> -
> -obj-$(CONFIG_USB_DWC2_PERIPHERAL) += dwc2_gadget.o
> -dwc2_gadget-y := gadget.o
> +obj-$(CONFIG_USB_DWC2) += dwc2_platform.o

Shouldn't it use CONFIG_USB_DWC2_PLATFORM instead of CONFIG_USB_DWC2?

> +dwc2_platform-y := platform.o

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Subject: Re: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

On Wednesday, October 22, 2014 02:25:46 PM Bartlomiej Zolnierkiewicz wrote:

> BTW It is a bit late but it would be great if you could split your
> patchset on two. First one merging gadget functionality into
> core/platform code and the second one adding USB_DWC2_DUAL_ROLE
> functionality.

On the second thought I think that the dual-role is needed to preserve
existing functionality (available through separate gadget and host
drivers) so please scrap the above comment.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

2014-10-22 18:45:31

by Paul Zimmerman

[permalink] [raw]
Subject: RE: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

> From: Dinh Nguyen [mailto:[email protected]]
> Sent: Tuesday, October 21, 2014 1:48 PM
>
> diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
> index f93807b..1ea702e 100644
> --- a/drivers/usb/dwc2/Kconfig
> +++ b/drivers/usb/dwc2/Kconfig
> @@ -1,5 +1,5 @@
> config USB_DWC2
> - bool "DesignWare USB2 DRD Core Support"
> + tristate "DesignWare USB2 DRD Core Support"
> depends on USB
> help
> Say Y here if your system has a Dual Role Hi-Speed USB
> @@ -10,31 +10,53 @@ config USB_DWC2
> bus interface module (if you have a PCI bus system) will be
> called dwc2_pci.ko, and the platform interface module (for
> controllers directly connected to the CPU) will be called
> - dwc2_platform.ko. For gadget mode, there will be a single
> - module called dwc2_gadget.ko.
> -
> - NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
> - host and gadget drivers are still currently separate drivers.
> - There are plans to merge the dwc2_gadget driver with the dwc2
> - host driver in the near future to create a dual-role driver.
> + dwc2_platform.ko. For all modes(host, gadget and dual-role), there
> + will be a single module called dwc2.ko.

Maybe "For all modes (host, gadget and dual-role), there will be an
additional module named dwc2.ko." That would be clearer.

> if USB_DWC2
>
> +choice
> + bool "DWC2 Mode Selection"
> + default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
> + default USB_DWC2_HOST if (USB && !USB_GADGET)
> + default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
> +
> config USB_DWC2_HOST
> - tristate "Host only mode"
> + bool "Host only mode"
> depends on USB
> help
> The Designware USB2.0 high-speed host controller
> - integrated into many SoCs.
> + integrated into many SoCs. Select this option if you want the
> + driver to operate in Host-only mode.
> +
> +comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
> +
> +config USB_DWC2_PERIPHERAL
> + bool "Gadget only mode"
> + depends on USB_GADGET=y || USB_GADGET=USB_DWC2
> + help
> + The Designware USB2.0 high-speed gadget controller
> + integrated into many SoCs. Select this option if you want the
> + driver to operate in Peripheral-only mode. This option requires
> + USB_GADGET=y.

Shouldn't this be "This option requires USB_GADGET to be enabled"? It
doesn't have to be built-in.

> +config USB_DWC2_DUAL_ROLE
> + bool "Dual Role mode"
> + depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
> + help
> + Select this option if you want the driver to work in a dual-role
> + mode. In this mode both host and gadget features are enabled, and
> + the role will be determined by the cable that gets plugged-in. This
> + option requires USB_GADGET=y.

Ditto.

Once you fix these, plus the extraneous "default y" that Paul Bolle
pointed out, you can add my acked-by.

--
Paul

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-22 20:27:13

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

On Tue, 2014-10-21 at 15:47 -0500, Dinh Nguyen wrote:
> On 10/20/2014 02:42 PM, Paul Bolle wrote:
> > (Side note: drivers/usb/dwc2/Kconfig is sourced (in drivers/usb/Kconfig)
> > even if USB is _not_ set. But USB_DCW2 still depends on USB. Why is
> > that?)
>
> Because USB is for Host-Side support. DWC2 driver should only get built
> for when USB is enabled. I think you're getting USB and USB_SUPPORT
> mixed up.

No, I don't think I did. Because in drivers/usb/Kconfig we see
if USB

[...]

endif

[...]

source "drivers/usb/dwc2/Kconfig"

[...]

> In addition, thanks to your comment, I realized that DWC2 should also be
> available to build when USB_GADGET is enabled. A patch has been sent:
>
> http://marc.info/?l=linux-usb&m=141392377124818&w=2

I haven't checked, but doesn't this mean USB_DWC2 could just depend on
USB_SUPPORT?


Paul Bolle

2014-10-22 20:54:05

by Paul Zimmerman

[permalink] [raw]
Subject: RE: [PATCHv5 2/7] usb: dwc2: Move gadget probe function into platform code

> From: Bartlomiej Zolnierkiewicz [mailto:[email protected]]
> Sent: Wednesday, October 22, 2014 4:16 AM
>
> On Monday, October 20, 2014 01:52:01 PM [email protected] wrote:
> > From: Dinh Nguyen <[email protected]>
> >
> > This patch will aggregate the probing of gadget/hcd driver into platform.c.
> > The gadget probe funtion is converted into gadget_init that is now only
> > responsible for gadget only initialization. All the gadget resources is now
> > handled by platform.c
> >
> > Since the host workqueue will not get initialized if the driver is configured
> > for peripheral mode only. Thus we need to check for wq_otg before calling
> > queue_work().
> >
> > Also, we move spin_lock_init to common location for both host and gadget that
> > is either in platform.c or pci.c.
> >
> > We also ove suspend/resume code to common platform code, and update it to use
> > the new PM API (struct dev_pm_ops).
> >
> > Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.
>
> This patch seems to break bisectability. It moves all the gadget probing
> to platform.c but Kconfig/Makefile are not updated (platform.c will be
> compiled only for CONFIG_USB_DWC2_PLATFORM=y which in turn depends on
> CONFIG_USB_DWC2_HOST). IMO patch #7 should be merged into this one (#2).

It doesn't break the compile, I already tested it. It does break the
operation of the driver until patch #7 is applied, but I think that's
OK in the middle of a patch series. I think it's a bit much to expect
the driver to keep working at each step of a patch series like this.

--
Paul

2014-10-23 15:09:46

by Dinh Nguyen

[permalink] [raw]
Subject: Re: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

On 10/22/2014 03:27 PM, Paul Bolle wrote:
> On Tue, 2014-10-21 at 15:47 -0500, Dinh Nguyen wrote:
>> On 10/20/2014 02:42 PM, Paul Bolle wrote:
>>> (Side note: drivers/usb/dwc2/Kconfig is sourced (in drivers/usb/Kconfig)
>>> even if USB is _not_ set. But USB_DCW2 still depends on USB. Why is
>>> that?)
>>
>> Because USB is for Host-Side support. DWC2 driver should only get built
>> for when USB is enabled. I think you're getting USB and USB_SUPPORT
>> mixed up.
>
> No, I don't think I did. Because in drivers/usb/Kconfig we see
> if USB
>
> [...]
>
> endif
>
> [...]
>
> source "drivers/usb/dwc2/Kconfig"
>
> [...]

Well, CONFIG_USB enables the host stack that is needed by the DWC2
driver, without CONFIG_USB, the DWC2 driver will not be able to build.

>
>> In addition, thanks to your comment, I realized that DWC2 should also be
>> available to build when USB_GADGET is enabled. A patch has been sent:
>>
>> http://marc.info/?l=linux-usb&m=141392377124818&w=2
>
> I haven't checked, but doesn't this mean USB_DWC2 could just depend on
> USB_SUPPORT?
>
I don't think so because USB_SUPPORT will not select USB or USB_GAGDGET,
and the DWC2 driver will need either 1 or both for it to build correctly.

Dinh

2014-10-23 17:10:30

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

On Thu, 2014-10-23 at 10:05 -0500, Dinh Nguyen wrote:
> On 10/22/2014 03:27 PM, Paul Bolle wrote:
> > On Tue, 2014-10-21 at 15:47 -0500, Dinh Nguyen wrote:
> Well, CONFIG_USB enables the host stack that is needed by the DWC2
> driver, without CONFIG_USB, the DWC2 driver will not be able to build.
>
> >> In addition, thanks to your comment, I realized that DWC2 should also be
> >> available to build when USB_GADGET is enabled. A patch has been sent:
> >>
> >> http://marc.info/?l=linux-usb&m=141392377124818&w=2

Doesn't that patch contradict your statement? It allows USB_DWC2 to be
set even if USB is not set.

> > I haven't checked, but doesn't this mean USB_DWC2 could just depend on
> > USB_SUPPORT?
> >
> I don't think so because USB_SUPPORT will not select USB or USB_GAGDGET,
> and the DWC2 driver will need either 1 or both for it to build correctly.

My comment was not well thought through. It was a waste of your time.


Paul Bolle

2014-10-23 18:28:10

by Paul Zimmerman

[permalink] [raw]
Subject: RE: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

> From: Bartlomiej Zolnierkiewicz [mailto:[email protected]]
> Sent: Wednesday, October 22, 2014 5:26 AM
>
> On Monday, October 20, 2014 01:52:06 PM [email protected] wrote:
> > From: Dinh Nguyen <[email protected]>
> >
> > config USB_DWC2_PLATFORM
> > bool "DWC2 Platform"
> > - depends on USB_DWC2_HOST
> > default USB_DWC2_HOST
>
> It should be "default USB_DWC2_HOST || USB_DWC2_PERIPHERAL" because
> USB_DWC2_PLATFORM replaces current USB_DWC2_PERIPHERAL functionality.
>
> Additionaly USB_DWC2_PLATFORM should be changed to tristate
> (USB_DWC2_PERIPHERAL was a tristeate before your changes).

Dinh, I think this is a good point. Is there any reason why
USB_DWC2_PLATFORM (and USB_DWC2_PCI for that matter) can't be
tristate? That's what DWC3 does.

--
Paul

2014-10-23 18:30:32

by Dinh Nguyen

[permalink] [raw]
Subject: Re: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role

On 10/23/2014 01:28 PM, Paul Zimmerman wrote:
>> From: Bartlomiej Zolnierkiewicz [mailto:[email protected]]
>> Sent: Wednesday, October 22, 2014 5:26 AM
>>
>> On Monday, October 20, 2014 01:52:06 PM [email protected] wrote:
>>> From: Dinh Nguyen <[email protected]>
>>>
>>> config USB_DWC2_PLATFORM
>>> bool "DWC2 Platform"
>>> - depends on USB_DWC2_HOST
>>> default USB_DWC2_HOST
>>
>> It should be "default USB_DWC2_HOST || USB_DWC2_PERIPHERAL" because
>> USB_DWC2_PLATFORM replaces current USB_DWC2_PERIPHERAL functionality.
>>
>> Additionaly USB_DWC2_PLATFORM should be changed to tristate
>> (USB_DWC2_PERIPHERAL was a tristeate before your changes).
>
> Dinh, I think this is a good point. Is there any reason why
> USB_DWC2_PLATFORM (and USB_DWC2_PCI for that matter) can't be
> tristate? That's what DWC3 does.
>

Yes, in my v6, I have made this change.

Thanks,
Dinh

2014-10-23 19:11:10

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCHv5 2/7] usb: dwc2: Move gadget probe function into platform code

On Wed, Oct 22, 2014 at 08:54:00PM +0000, Paul Zimmerman wrote:
> > From: Bartlomiej Zolnierkiewicz [mailto:[email protected]]
> > Sent: Wednesday, October 22, 2014 4:16 AM
> >
> > On Monday, October 20, 2014 01:52:01 PM [email protected] wrote:
> > > From: Dinh Nguyen <[email protected]>
> > >
> > > This patch will aggregate the probing of gadget/hcd driver into platform.c.
> > > The gadget probe funtion is converted into gadget_init that is now only
> > > responsible for gadget only initialization. All the gadget resources is now
> > > handled by platform.c
> > >
> > > Since the host workqueue will not get initialized if the driver is configured
> > > for peripheral mode only. Thus we need to check for wq_otg before calling
> > > queue_work().
> > >
> > > Also, we move spin_lock_init to common location for both host and gadget that
> > > is either in platform.c or pci.c.
> > >
> > > We also ove suspend/resume code to common platform code, and update it to use
> > > the new PM API (struct dev_pm_ops).
> > >
> > > Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.
> >
> > This patch seems to break bisectability. It moves all the gadget probing
> > to platform.c but Kconfig/Makefile are not updated (platform.c will be
> > compiled only for CONFIG_USB_DWC2_PLATFORM=y which in turn depends on
> > CONFIG_USB_DWC2_HOST). IMO patch #7 should be merged into this one (#2).
>
> It doesn't break the compile, I already tested it. It does break the
> operation of the driver until patch #7 is applied, but I think that's
> OK in the middle of a patch series. I think it's a bit much to expect
> the driver to keep working at each step of a patch series like this.

It's your driver and, at the end of the day, your headache; but the
very day you need to run a git bisect and you end up in the middle of
one of these commits, you'll regret this statement :-)

There's usually a way to make sure things continue to work even if it
means duplicating some code until the conversion is completed, or adding
temporary flags which get added and removed within the same series, etc.

my 2 cents

--
balbi


Attachments:
(No filename) (2.12 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments