* [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure
2014-10-28 23:25 [PATCHv6 0/8] usb: dwc2: Add support for dual-role dinguyen
@ 2014-10-28 23:25 ` dinguyen
2014-10-30 13:54 ` Felipe Balbi
2014-10-31 2:47 ` Kever Yang
2014-10-28 23:25 ` [PATCHv6 2/8] usb: dwc2: Move gadget probe function into platform code dinguyen
` (6 subsequent siblings)
7 siblings, 2 replies; 32+ messages in thread
From: dinguyen @ 2014-10-28 23:25 UTC (permalink / raw)
To: paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel, Dinh Nguyen
From: Dinh Nguyen <dinguyen@opensource.altera.com>
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 <dinguyen@opensource.altera.com>
Signed-off-by: Paul Zimmerman <paulz@synopsys.com>
---
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 | 145 +++++++++++++++++++++---------------------
2 files changed, 154 insertions(+), 147 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 55c90c5..96c283d 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 701d3e1..78b3d1a 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;
@@ -2593,7 +2594,7 @@ error:
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;
@@ -2658,7 +2659,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);
@@ -2684,7 +2685,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;
@@ -2748,7 +2749,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;
@@ -2777,7 +2778,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);
@@ -2800,7 +2801,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);
@@ -2818,7 +2819,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 */
@@ -2868,7 +2869,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) {
@@ -2923,7 +2924,7 @@ err:
*/
static int s3c_hsotg_udc_stop(struct usb_gadget *gadget)
{
- 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;
}
@@ -3572,7 +3573,7 @@ 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;
}
@@ -3602,7 +3603,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);
s3c_hsotg_delete_debug(hsotg);
@@ -3613,7 +3614,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;
@@ -3642,7 +3643,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
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure
2014-10-28 23:25 ` [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure dinguyen
@ 2014-10-30 13:54 ` Felipe Balbi
2014-10-31 14:55 ` Dinh Nguyen
2014-10-31 2:47 ` Kever Yang
1 sibling, 1 reply; 32+ messages in thread
From: Felipe Balbi @ 2014-10-30 13:54 UTC (permalink / raw)
To: dinguyen
Cc: paulz, balbi, dinh.linux, swarren, b.zolnierkie, matthijs,
r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders, kever.yang,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 11213 bytes --]
On Tue, Oct 28, 2014 at 06:25:42PM -0500, dinguyen@opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> 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 <dinguyen@opensource.altera.com>
> Signed-off-by: Paul Zimmerman <paulz@synopsys.com>
> ---
> 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 | 145 +++++++++++++++++++++---------------------
> 2 files changed, 154 insertions(+), 147 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 55c90c5..96c283d 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;
this shouldn't be limited to gadget.
> + struct usb_phy *uphy;
this shouldn't be limited to gadget.
> + struct s3c_hsotg_plat *plat;
> +
> + int irq;
this shouldn't be limited to gadget.
> + struct clk *clk;
this shouldn't be limited to gadget.
> +
> + struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
this shouldn't be limited to gadget.
> + u32 phyif;
> + int fifo_mem;
> + unsigned int dedicated_fifos:1;
> + unsigned char num_of_eps;
> + u32 fifo_map;
> +
> + struct dentry *debug_root;
this shouldn't be limited to gadget.
> + struct dentry *debug_file;
this shouldn't be limited to gadget.
> + struct dentry *debug_fifo;
this shouldn't be limited to gadget.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure
2014-10-30 13:54 ` Felipe Balbi
@ 2014-10-31 14:55 ` Dinh Nguyen
0 siblings, 0 replies; 32+ messages in thread
From: Dinh Nguyen @ 2014-10-31 14:55 UTC (permalink / raw)
To: balbi
Cc: paulz, dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga,
jg1.han, sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/30/2014 08:54 AM, Felipe Balbi wrote:
> On Tue, Oct 28, 2014 at 06:25:42PM -0500,
> dinguyen@opensource.altera.com wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> 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 <dinguyen@opensource.altera.com>
>> Signed-off-by: Paul Zimmerman <paulz@synopsys.com> --- 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 | 145
>> +++++++++++++++++++++--------------------- 2 files changed, 154
>> insertions(+), 147 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>> index 55c90c5..96c283d 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;
>
> this shouldn't be limited to gadget.
>
>> + struct usb_phy *uphy;
>
> this shouldn't be limited to gadget.
>
>> + struct s3c_hsotg_plat *plat; + + int irq;
>
> this shouldn't be limited to gadget.
>
>> + struct clk *clk;
>
> this shouldn't be limited to gadget.
>
>> + + struct regulator_bulk_data
>> supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
>
> this shouldn't be limited to gadget.
>
>> + u32 phyif; + int fifo_mem; + unsigned int dedicated_fifos:1; +
>> unsigned char num_of_eps; + u32 fifo_map; + + struct dentry
>> *debug_root;
>
> this shouldn't be limited to gadget.
>
>> + struct dentry *debug_file;
>
> this shouldn't be limited to gadget.
>
>> + struct dentry *debug_fifo;
>
> this shouldn't be limited to gadget.
>
All fixed..
thanks,
Dinh
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQIcBAEBAgAGBQJUU6LrAAoJEBmUBAuBoyj024wP/j7x+PW8rV8Aqzhkg4d1zSKE
BE7/Axir8uPVTZfIOFBouHz6h0APztgW+by7DZdXCE8K6gUDAfzruxLRHNjctT6E
KRj6A+i7qYnF/0+kc9b2InWYfS8NcOxths7r70RhJz7jlCphpaqlfhlicMSc0Ury
Z/EeOYRpbwltDHGSBSkcPaw0yL4h/BQIRm1rRaUj+10cliSYwW5Re6mfiWVV+0pl
WA56MM5xf7N9PO1Q+TPyStLyUTLyb3jiNwCOWfUCIkpHwJC9AIgbuBZXPMrSGOoL
cMNoLHkgfY4HPFgzGlGxIgbHdkMec6A78FzSgz04P5+Uzh/MGKyrecordMbpQAPS
DCGGWOsPrujAbWhgrus+cfxznIV3+4vhP+uC387RgLsYaJLRpAEpYN5aTABRU8u2
uEsbfYIhRGaGDQqTR0mzcKMXy9xoUMLNLkpiaqlGTs2Et0f1ZkElIn/dpefx417u
Ocmfq7ab0cXgHnYNjdk9yE9JchgjUG5sn+INKbm/lLmXNyN3VHNZnBO42v1d78Gw
sylySISc1aUJGXkuRJDPM4oC4jyPrerLB4BQbF0bQqaakz+U83+I+wZ7e9IOldO9
bPk5I7VZO8F8Ve0oP1WvuVZGeCnhrTd2isfx/orQ4J3uQFBD9hddGyaroDPtRIkA
zoNVOSXVySi9EiPYT12u
=382E
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure
2014-10-28 23:25 ` [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure dinguyen
2014-10-30 13:54 ` Felipe Balbi
@ 2014-10-31 2:47 ` Kever Yang
2014-10-31 13:48 ` Felipe Balbi
1 sibling, 1 reply; 32+ messages in thread
From: Kever Yang @ 2014-10-31 2:47 UTC (permalink / raw)
To: dinguyen, paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, linux-usb, linux-kernel
Hi Dinh
On 10/29/2014 07:25 AM, dinguyen@opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> 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.
Are we still need so much s3c prefix? Can we replace all the s3c into dwc2?
- Kever
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure
2014-10-31 2:47 ` Kever Yang
@ 2014-10-31 13:48 ` Felipe Balbi
0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2014-10-31 13:48 UTC (permalink / raw)
To: Kever Yang
Cc: dinguyen, paulz, balbi, dinh.linux, swarren, b.zolnierkie,
matthijs, r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 872 bytes --]
On Fri, Oct 31, 2014 at 10:47:16AM +0800, Kever Yang wrote:
> Hi Dinh
> On 10/29/2014 07:25 AM, dinguyen@opensource.altera.com wrote:
> >From: Dinh Nguyen <dinguyen@opensource.altera.com>
> >
> >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.
> Are we still need so much s3c prefix? Can we replace all the s3c into dwc2?
separate patch, one which you can send if it bothers you so much.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCHv6 2/8] usb: dwc2: Move gadget probe function into platform code
2014-10-28 23:25 [PATCHv6 0/8] usb: dwc2: Add support for dual-role dinguyen
2014-10-28 23:25 ` [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure dinguyen
@ 2014-10-28 23:25 ` dinguyen
2014-10-30 13:57 ` Felipe Balbi
2014-10-28 23:25 ` [PATCHv6 3/8] usb: dwc2: Initialize the USB core for peripheral mode dinguyen
` (5 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: dinguyen @ 2014-10-28 23:25 UTC (permalink / raw)
To: paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel, Dinh Nguyen
From: Dinh Nguyen <dinguyen@opensource.altera.com>
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 <dinguyen@opensource.altera.com>
Acked-by: Paul Zimmerman <paulz@synopsys.com>
---
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 | 99 ++++++++++----------------------------------
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, 92 insertions(+), 93 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 96c283d..de2b194 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 78b3d1a..38ec1cc 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);
@@ -3573,11 +3544,11 @@ 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;
}
- ret = usb_add_gadget_udc(&pdev->dev, &hsotg->gadget);
+ ret = usb_add_gadget_udc(dev, &hsotg->gadget);
if (ret)
goto err_ep_mem;
@@ -3596,25 +3567,24 @@ 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);
clk_disable_unprepare(hsotg->clk);
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;
@@ -3640,10 +3610,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;
@@ -3664,31 +3634,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 <ben@simtec.co.uk>");
-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 = ¶ms_bcm2835 },
{ .compatible = "rockchip,rk3066-usb", .data = ¶ms_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
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCHv6 2/8] usb: dwc2: Move gadget probe function into platform code
2014-10-28 23:25 ` [PATCHv6 2/8] usb: dwc2: Move gadget probe function into platform code dinguyen
@ 2014-10-30 13:57 ` Felipe Balbi
2014-10-31 14:59 ` Dinh Nguyen
0 siblings, 1 reply; 32+ messages in thread
From: Felipe Balbi @ 2014-10-30 13:57 UTC (permalink / raw)
To: dinguyen
Cc: paulz, balbi, dinh.linux, swarren, b.zolnierkie, matthijs,
r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders, kever.yang,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]
On Tue, Oct 28, 2014 at 06:25:43PM -0500, dinguyen@opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> 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
s/resources is/resources are
> 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().
this period character in the middle of the sentence doesn't make sense,
perhaps a comma is what you want ? The sentence can be improved too:
"Since the host workqueue will not get initialized if the drier is
configured for peripheral mode only, we add a 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).
updating to dev_pm_ops should really be a separate patch.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv6 2/8] usb: dwc2: Move gadget probe function into platform code
2014-10-30 13:57 ` Felipe Balbi
@ 2014-10-31 14:59 ` Dinh Nguyen
0 siblings, 0 replies; 32+ messages in thread
From: Dinh Nguyen @ 2014-10-31 14:59 UTC (permalink / raw)
To: balbi
Cc: paulz, dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga,
jg1.han, sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/30/2014 08:57 AM, Felipe Balbi wrote:
> On Tue, Oct 28, 2014 at 06:25:43PM -0500,
> dinguyen@opensource.altera.com wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> 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
>
> s/resources is/resources are
>
>> 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().
>
> this period character in the middle of the sentence doesn't make
> sense, perhaps a comma is what you want ? The sentence can be
> improved too:
>
> "Since the host workqueue will not get initialized if the drier is
> configured for peripheral mode only, we add a 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).
>
> updating to dev_pm_ops should really be a separate patch.
>
All fixed...
Thanks,
Dinh
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQIcBAEBAgAGBQJUU6PAAAoJEBmUBAuBoyj0KJUP/RWpP9YkctpmZQgrb2xq0JaG
eLCpOnKVYaTBCAhydJOKBBx0odYyxORlxTWbJCbZPvPhrZ7NDTINqEVWJOqYmtqe
Xl8aEPYm+ckcfTliONK3yIP/MA49he/qbGOmqXFwFMbcAvKFN/kpq05cjzZ6T8bO
hILDO2Yy2HVQHonq3uKppmTB9DSnCNfCR0Cuum2fpOzmVLY/X46EM5UD2e+XmVgo
dsINAGe40FwFIoLZzAradn12MrmCjOM6MPpkkGZ3SMGjKXY7yQX38c9WbcCvsm6g
y1/5fDqdRlsq+Weoo9N3H03LUX8MlGCpZ7dgLrABXMEjdySj3eVTj4bHgPxRjRY/
+SfI2YR8RyEcHj2UTqgMUp4JmRl2CNiB9fsvZHMQQm2MTeFkcUpZnSUXWHB9Vtwv
s1I5nMOSoT2NDBg6QS9a1T1s1gdSywOnDBd+/SK7mf4QkQumvf2Nvu6avQB1Rxxm
sqpGWztuhuycg332CXc9W0EBXcg5t8SRyk9SFgksH3MezS85gAQxZtXyv44NuCUM
ciTru8tKh3ncszws2NmX9yyQgC4fM3kHDytOwDXFt23f3xHyjg+2m3O1TLErAwig
yqj2MfYTfqfYVrs/hOvOpEbUgUSy5MQY/EOMIsI6aGfB2Nba72wrYQP79ugb84u9
Ftd1XH47lvXX/R8VOV2x
=ZAkf
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCHv6 3/8] usb: dwc2: Initialize the USB core for peripheral mode
2014-10-28 23:25 [PATCHv6 0/8] usb: dwc2: Add support for dual-role dinguyen
2014-10-28 23:25 ` [PATCHv6 1/8] usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure dinguyen
2014-10-28 23:25 ` [PATCHv6 2/8] usb: dwc2: Move gadget probe function into platform code dinguyen
@ 2014-10-28 23:25 ` dinguyen
2014-10-28 23:25 ` [PATCHv6 4/8] usb: dwc2: Update common interrupt handler to call gadget interrupt handler dinguyen
` (4 subsequent siblings)
7 siblings, 0 replies; 32+ messages in thread
From: dinguyen @ 2014-10-28 23:25 UTC (permalink / raw)
To: paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel, Dinh Nguyen
From: Dinh Nguyen <dinguyen@opensource.altera.com>
Initialize the USB driver to peripheral mode when a B-Device connector
is attached.
Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Acked-by: Paul Zimmerman <paulz@synopsys.com>
---
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 de2b194..80d29c7 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 38ec1cc..19d1b03 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
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCHv6 4/8] usb: dwc2: Update common interrupt handler to call gadget interrupt handler
2014-10-28 23:25 [PATCHv6 0/8] usb: dwc2: Add support for dual-role dinguyen
` (2 preceding siblings ...)
2014-10-28 23:25 ` [PATCHv6 3/8] usb: dwc2: Initialize the USB core for peripheral mode dinguyen
@ 2014-10-28 23:25 ` dinguyen
2014-10-30 14:00 ` Felipe Balbi
2014-10-28 23:25 ` [PATCHv6 5/8] usb: dwc2: Add call_gadget functions for perpheral mode interrupts dinguyen
` (3 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: dinguyen @ 2014-10-28 23:25 UTC (permalink / raw)
To: paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel, Dinh Nguyen
From: Dinh Nguyen <dinguyen@opensource.altera.com>
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 <dinguyen@opensource.altera.com>
Acked-by: Paul Zimmerman <paulz@synopsys.com>
---
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 80d29c7..ec70862 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 19d1b03..202f8cc 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
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCHv6 4/8] usb: dwc2: Update common interrupt handler to call gadget interrupt handler
2014-10-28 23:25 ` [PATCHv6 4/8] usb: dwc2: Update common interrupt handler to call gadget interrupt handler dinguyen
@ 2014-10-30 14:00 ` Felipe Balbi
2014-10-31 15:00 ` Dinh Nguyen
2014-10-31 20:12 ` Paul Zimmerman
0 siblings, 2 replies; 32+ messages in thread
From: Felipe Balbi @ 2014-10-30 14:00 UTC (permalink / raw)
To: dinguyen
Cc: paulz, balbi, dinh.linux, swarren, b.zolnierkie, matthijs,
r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders, kever.yang,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4013 bytes --]
Hi,
On Tue, Oct 28, 2014 at 06:25:45PM -0500, dinguyen@opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> 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 <dinguyen@opensource.altera.com>
> Acked-by: Paul Zimmerman <paulz@synopsys.com>
> ---
> 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 80d29c7..ec70862 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 19d1b03..202f8cc 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)
why ? It would've been a lot easier to just make the IRQ line shared.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 4/8] usb: dwc2: Update common interrupt handler to call gadget interrupt handler
2014-10-30 14:00 ` Felipe Balbi
@ 2014-10-31 15:00 ` Dinh Nguyen
2014-10-31 20:12 ` Paul Zimmerman
1 sibling, 0 replies; 32+ messages in thread
From: Dinh Nguyen @ 2014-10-31 15:00 UTC (permalink / raw)
To: balbi
Cc: paulz, dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga,
jg1.han, sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/30/2014 09:00 AM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Oct 28, 2014 at 06:25:45PM -0500,
> dinguyen@opensource.altera.com wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> 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 <dinguyen@opensource.altera.com>
>> Acked-by: Paul Zimmerman <paulz@synopsys.com> --- 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 80d29c7..ec70862 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
>> 19d1b03..202f8cc 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)
>
> why ? It would've been a lot easier to just make the IRQ line
> shared.
>
Reworked to use IRQF_SHARED...
Thanks,
Dinh
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQIcBAEBAgAGBQJUU6P5AAoJEBmUBAuBoyj0vWwP/1Qq0Gt9U1MF6LHJU853+P+I
jx6xVVanPSuK8yTWprh7M/endv+UkVwwt+2sGZ0MMzLDH1T7Kf6FdqhjlNqvFeRK
qK5LUkpMKvMJKcw6tQNGsjB75X93GRkWLxuZEQ9BpGjfy1fct6Wic/kv66dUVCCI
DevfZZFPGdSmiichVup0sO/cfTJ84R8xyQVuF77LvZ/Wbm606ypV4PTMEjKcFhmr
5s6BeOINm7p1NMuoDaWbmjjOHAfTwy06W3p81LMsWH5yfVCq4TgJSwMbdo8s/i/Y
po/1Nu0x5lwwao1J2OZA7b30i1vbtztCzVPtFcO5quN9rOlsaXRNBGLf9jjHP1OO
PzRSsh1EbAinc4Ohbnii/bdx2g7lWP2ktf5qqoeVocgZkrKA/k5w/QK1qL1z3Onl
ZkylCf5VMd06eOkeGbdce8UFhbceNLw0ryqAVN0bsxiD0u78aH6KvhYhBo29p/1R
VMlKPrsYVDp9xjsp24S/u7o4NvNx8VeIfaKA5JN0Hnzncy03ByaO/o5iMPLb231l
/yCoVyt2A9q7LQs/HeyCZYOFAPpG8ZbTsZrucKB9rRWXizDIrpIfJdcH3mIp71ab
fhhf8sE1kDMrqADLTd1UL96DKCJnU2N8qJLSll6uWNi95J6OogIkj+H4YFPXXWiM
+wUVqVAyYzPKsCaMRNTF
=5+gL
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 32+ messages in thread* RE: [PATCHv6 4/8] usb: dwc2: Update common interrupt handler to call gadget interrupt handler
2014-10-30 14:00 ` Felipe Balbi
2014-10-31 15:00 ` Dinh Nguyen
@ 2014-10-31 20:12 ` Paul Zimmerman
1 sibling, 0 replies; 32+ messages in thread
From: Paul Zimmerman @ 2014-10-31 20:12 UTC (permalink / raw)
To: balbi@ti.com, dinguyen@opensource.altera.com
Cc: dinh.linux@gmail.com, swarren@wwwdotorg.org,
b.zolnierkie@samsung.com, matthijs@stdin.nl,
r.baldyga@samsung.com, jg1.han@samsung.com,
sachin.kamat@linaro.org, ben-linux@fluff.org,
dianders@chromium.org, kever.yang@rock-chips.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> From: Felipe Balbi [mailto:balbi@ti.com]
> Sent: Thursday, October 30, 2014 7:01 AM
>
> On Tue, Oct 28, 2014 at 06:25:45PM -0500, dinguyen@opensource.altera.com wrote:
> > From: Dinh Nguyen <dinguyen@opensource.altera.com>
> >
> > 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 <dinguyen@opensource.altera.com>
> > Acked-by: Paul Zimmerman <paulz@synopsys.com>
> > ---
> > 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 80d29c7..ec70862 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 19d1b03..202f8cc 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)
>
> why ? It would've been a lot easier to just make the IRQ line shared.
Actually, I would prefer to keep the common interrupt handler. Reason
being, the HSOTG core can spew a *lot* of interrupts when in host mode,
and the driver already struggles to keep up with them on some platforms.
We see this problem especially on the Raspberry Pi.
So I think adding the overhead of an additional interrupt handling
path is a bad idea. Unless the peripheral-mode interrupt handler can
somehow be disabled when the controller is in host mode.
Maybe we could delay registering the gadget interrupt handler until
the controller switches to peripheral mode, and unregister it when it
switches back to host mode? But that seems pretty "odd" to me.
Felipe, what do you think?
--
Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCHv6 5/8] usb: dwc2: Add call_gadget functions for perpheral mode interrupts
2014-10-28 23:25 [PATCHv6 0/8] usb: dwc2: Add support for dual-role dinguyen
` (3 preceding siblings ...)
2014-10-28 23:25 ` [PATCHv6 4/8] usb: dwc2: Update common interrupt handler to call gadget interrupt handler dinguyen
@ 2014-10-28 23:25 ` dinguyen
2014-10-30 14:01 ` Felipe Balbi
2014-10-28 23:25 ` [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node dinguyen
` (2 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: dinguyen @ 2014-10-28 23:25 UTC (permalink / raw)
To: paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel, Dinh Nguyen
From: Dinh Nguyen <dinguyen@opensource.altera.com>
Update the dwc2 wakeup and suspend interrupt functions to use call_gadget
when the IP is in peripheral mode.
Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Acked-by: Paul Zimmerman <paulz@synopsys.com>
---
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
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCHv6 5/8] usb: dwc2: Add call_gadget functions for perpheral mode interrupts
2014-10-28 23:25 ` [PATCHv6 5/8] usb: dwc2: Add call_gadget functions for perpheral mode interrupts dinguyen
@ 2014-10-30 14:01 ` Felipe Balbi
2014-10-31 15:01 ` Dinh Nguyen
0 siblings, 1 reply; 32+ messages in thread
From: Felipe Balbi @ 2014-10-30 14:01 UTC (permalink / raw)
To: dinguyen
Cc: paulz, balbi, dinh.linux, swarren, b.zolnierkie, matthijs,
r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders, kever.yang,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 355 bytes --]
On Tue, Oct 28, 2014 at 06:25:46PM -0500, dinguyen@opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> Update the dwc2 wakeup and suspend interrupt functions to use call_gadget
> when the IP is in peripheral mode.
it seems like you're actually fixing a bug here. Those calls weren't
there before.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv6 5/8] usb: dwc2: Add call_gadget functions for perpheral mode interrupts
2014-10-30 14:01 ` Felipe Balbi
@ 2014-10-31 15:01 ` Dinh Nguyen
0 siblings, 0 replies; 32+ messages in thread
From: Dinh Nguyen @ 2014-10-31 15:01 UTC (permalink / raw)
To: balbi
Cc: paulz, dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga,
jg1.han, sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/30/2014 09:01 AM, Felipe Balbi wrote:
> On Tue, Oct 28, 2014 at 06:25:46PM -0500,
> dinguyen@opensource.altera.com wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> Update the dwc2 wakeup and suspend interrupt functions to use
>> call_gadget when the IP is in peripheral mode.
>
> it seems like you're actually fixing a bug here. Those calls
> weren't there before.
>
This patch can be sent separately from this series.
Thanks,
Dinh
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQIcBAEBAgAGBQJUU6QzAAoJEBmUBAuBoyj0XpIQAKLQxsksyfEBAOWNwapO1Grx
jMK9+QpqZuGwo5Gha905iAO1Q/XP5tHvkLBzGyeEpNhEqBWXTTAvu1i2EToSiNId
JTtae/eJA5xfxoljULqL3Vuy1SfOt+EdBHoMdn/vQQfBp9kUiqfJNgZeUsDgvbBK
7xNfdocMQjqPPbseNGL3193aasOFyZdc9sI+ZR3XjeXjNPgzwJzfB9eMZH+zpRKU
nVcWY5UBkw05UnvDNwc4e/+VNpBRj4NLBBLGPleUJ1KXyunUet5r4oCLYLasZaFn
zk6VvovQMUDk9DhnK2d8w6oosmSA/eD8zKn3K3++LV/XGwb5rHP+zC5jpAkbjN/o
4ZudegACKNUiwvo0tfbgtI5no/VzlaArSM6n3WT6Fz5cDveibZbU7z78AqIqRkwF
Ai4+bn510OCgfEEu2rVRXl21r/TAt9r3ujh1mPPUwJ+nWdoxjmm7+0MyKNSeB/Ab
MmVZfQaELLGTOMHH0T0/yr6XX8Bc4LxGtbjVQRlmW7mYfafnzhE+aWAeLnyF4V9r
cA0YE09jOQoXh8WU6k18SKbapF8HDaXSLBcTWnXBR93TQu4U05qx5iB8hEZ7S9NT
z3uQdfgjk8LjhaYWTtSsT29E6oDbvho8h5TF48KiwN4Ud0wagqJq1W3vlGhrgKUd
wy94/mOPpifATM0S4KUr
=qCZw
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-28 23:25 [PATCHv6 0/8] usb: dwc2: Add support for dual-role dinguyen
` (4 preceding siblings ...)
2014-10-28 23:25 ` [PATCHv6 5/8] usb: dwc2: Add call_gadget functions for perpheral mode interrupts dinguyen
@ 2014-10-28 23:25 ` dinguyen
2014-10-29 1:30 ` Paul Zimmerman
` (2 more replies)
2014-10-28 23:25 ` [PATCHv6 7/8] usb: dwc2: Update Kconfig to support dual-role dinguyen
2014-10-28 23:25 ` [PATCHv6 8/8] usb: dwc2: move usb_disabled() call to host driver only dinguyen
7 siblings, 3 replies; 32+ messages in thread
From: dinguyen @ 2014-10-28 23:25 UTC (permalink / raw)
To: paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel, Dinh Nguyen
From: Dinh Nguyen <dinguyen@opensource.altera.com>
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 <dinguyen@opensource.altera.com>
---
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 ec70862..48120c8 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
^ permalink raw reply related [flat|nested] 32+ messages in thread* RE: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-28 23:25 ` [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node dinguyen
@ 2014-10-29 1:30 ` Paul Zimmerman
2014-10-30 14:04 ` Felipe Balbi
2014-10-31 2:38 ` Kever Yang
2 siblings, 0 replies; 32+ messages in thread
From: Paul Zimmerman @ 2014-10-29 1:30 UTC (permalink / raw)
To: dinguyen@opensource.altera.com, balbi@ti.com
Cc: dinh.linux@gmail.com, swarren@wwwdotorg.org,
b.zolnierkie@samsung.com, matthijs@stdin.nl,
r.baldyga@samsung.com, jg1.han@samsung.com,
sachin.kamat@linaro.org, ben-linux@fluff.org,
dianders@chromium.org, kever.yang@rock-chips.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> From: dinguyen@opensource.altera.com [mailto:dinguyen@opensource.altera.com]
> Sent: Tuesday, October 28, 2014 4:26 PM
>
> 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 <dinguyen@opensource.altera.com>
> ---
> 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 ec70862..48120c8 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;
> }
Hi Felipe,
I don't feel qualified to ack/nak this patch, since I don't fully
understand the clock subsystem. Can you review this one and see if it
looks OK?
--
Paul
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-28 23:25 ` [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node dinguyen
2014-10-29 1:30 ` Paul Zimmerman
@ 2014-10-30 14:04 ` Felipe Balbi
2014-10-31 15:20 ` Dinh Nguyen
2014-10-31 2:38 ` Kever Yang
2 siblings, 1 reply; 32+ messages in thread
From: Felipe Balbi @ 2014-10-30 14:04 UTC (permalink / raw)
To: dinguyen
Cc: paulz, balbi, dinh.linux, swarren, b.zolnierkie, matthijs,
r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders, kever.yang,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4637 bytes --]
Hi,
On Tue, Oct 28, 2014 at 06:25:47PM -0500, dinguyen@opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> 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 <dinguyen@opensource.altera.com>
> ---
> 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 ec70862..48120c8 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);
instead of exposing the clock detail to the entire driver, add IS_ERR()
checks to resume and suspend instead. In fact, NULL is a valid clock, so
you might as well:
clk = clk_get(foo, bar);
if (IS_ERR(clk))
dwc->clk = NULL;
else
dwc->clk = clk;
Then you don't need any IS_ERR() checks sprinkled around the driver.
> @@ -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);
wait a minute, if there is no clock we don't call the gadget interrupt
handler ? Why ? Who will disable the IRQ line ?
> 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;
> + }
no here... this should have been taken care by dwc2_gadget_init()
itself.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-30 14:04 ` Felipe Balbi
@ 2014-10-31 15:20 ` Dinh Nguyen
2014-10-31 17:42 ` Felipe Balbi
0 siblings, 1 reply; 32+ messages in thread
From: Dinh Nguyen @ 2014-10-31 15:20 UTC (permalink / raw)
To: balbi
Cc: paulz, dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga,
jg1.han, sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel
On 10/30/2014 09:04 AM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Oct 28, 2014 at 06:25:47PM -0500, dinguyen@opensource.altera.com wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> 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 <dinguyen@opensource.altera.com>
>> ---
>> 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 ec70862..48120c8 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);
>
> instead of exposing the clock detail to the entire driver, add IS_ERR()
> checks to resume and suspend instead. In fact, NULL is a valid clock, so
> you might as well:
>
> clk = clk_get(foo, bar);
> if (IS_ERR(clk))
> dwc->clk = NULL;
> else
> dwc->clk = clk;
>
> Then you don't need any IS_ERR() checks sprinkled around the driver.
But we would still need to check for the clock before accessing gadget
functionality right?
if (dwc2->clk)
call_gadget();
>
>> @@ -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);
>
> wait a minute, if there is no clock we don't call the gadget interrupt
> handler ? Why ? Who will disable the IRQ line ?
This portion is no longer needed when I use shared IRQ lines.
>
>> 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;
>> + }
>
> no here... this should have been taken care by dwc2_gadget_init()
> itself.
Fixed..
Thanks,
Dinh
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-31 15:20 ` Dinh Nguyen
@ 2014-10-31 17:42 ` Felipe Balbi
2014-10-31 19:31 ` Dinh Nguyen
0 siblings, 1 reply; 32+ messages in thread
From: Felipe Balbi @ 2014-10-31 17:42 UTC (permalink / raw)
To: Dinh Nguyen
Cc: balbi, paulz, dinh.linux, swarren, b.zolnierkie, matthijs,
r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders, kever.yang,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2874 bytes --]
Hi,
On Fri, Oct 31, 2014 at 10:20:06AM -0500, Dinh Nguyen wrote:
> >> @@ -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);
> >
> > instead of exposing the clock detail to the entire driver, add IS_ERR()
> > checks to resume and suspend instead. In fact, NULL is a valid clock, so
> > you might as well:
> >
> > clk = clk_get(foo, bar);
> > if (IS_ERR(clk))
> > dwc->clk = NULL;
> > else
> > dwc->clk = clk;
> >
> > Then you don't need any IS_ERR() checks sprinkled around the driver.
>
> But we would still need to check for the clock before accessing gadget
> functionality right?
>
> if (dwc2->clk)
> call_gadget();
Read my comment again. "NULL is a valid clock". Look at what
clk_enable() does when a NULL pointer is passed:
static int __clk_enable(struct clk *clk)
{
int ret = 0;
if (!clk)
return 0;
if (WARN_ON(clk->prepare_count == 0))
return -ESHUTDOWN;
if (clk->enable_count == 0) {
ret = __clk_enable(clk->parent);
if (ret)
return ret;
if (clk->ops->enable) {
ret = clk->ops->enable(clk->hw);
if (ret) {
__clk_disable(clk->parent);
return ret;
}
}
}
clk->enable_count++;
return 0;
}
int clk_enable(struct clk *clk)
{
unsigned long flags;
int ret;
flags = clk_enable_lock();
ret = __clk_enable(clk);
clk_enable_unlock(flags);
return ret;
}
EXPORT_SYMBOL_GPL(clk_enable);
> >> @@ -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);
> >
> > wait a minute, if there is no clock we don't call the gadget interrupt
> > handler ? Why ? Who will disable the IRQ line ?
>
> This portion is no static int __clk_enable(struct clk *clk)
huh ? What I mean is that this has the potential of leaving that IRQ
line enabled. Imagine you don't have a clock and s3c_hsotg_irq() isn't
called, then a peripheral IRQ fires, since the handler isn't called, who
will clear the interrupt ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-31 17:42 ` Felipe Balbi
@ 2014-10-31 19:31 ` Dinh Nguyen
2014-10-31 19:56 ` Dinh Nguyen
2014-11-03 15:25 ` Felipe Balbi
0 siblings, 2 replies; 32+ messages in thread
From: Dinh Nguyen @ 2014-10-31 19:31 UTC (permalink / raw)
To: balbi
Cc: paulz, dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga,
jg1.han, sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel
On 10/31/2014 12:42 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Oct 31, 2014 at 10:20:06AM -0500, Dinh Nguyen wrote:
>>>> @@ -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);
>>>
>>> instead of exposing the clock detail to the entire driver, add IS_ERR()
>>> checks to resume and suspend instead. In fact, NULL is a valid clock, so
>>> you might as well:
>>>
>>> clk = clk_get(foo, bar);
>>> if (IS_ERR(clk))
>>> dwc->clk = NULL;
>>> else
>>> dwc->clk = clk;
>>>
>>> Then you don't need any IS_ERR() checks sprinkled around the driver.
>>
>> But we would still need to check for the clock before accessing gadget
>> functionality right?
>>
>> if (dwc2->clk)
>> call_gadget();
>
> Read my comment again. "NULL is a valid clock". Look at what
> clk_enable() does when a NULL pointer is passed:
>
> static int __clk_enable(struct clk *clk)
> {
> int ret = 0;
>
> if (!clk)
> return 0;
>
> if (WARN_ON(clk->prepare_count == 0))
> return -ESHUTDOWN;
>
> if (clk->enable_count == 0) {
> ret = __clk_enable(clk->parent);
>
> if (ret)
> return ret;
>
> if (clk->ops->enable) {
> ret = clk->ops->enable(clk->hw);
> if (ret) {
> __clk_disable(clk->parent);
> return ret;
> }
> }
> }
>
> clk->enable_count++;
> return 0;
> }
>
> int clk_enable(struct clk *clk)
> {
> unsigned long flags;
> int ret;
>
> flags = clk_enable_lock();
> ret = __clk_enable(clk);
> clk_enable_unlock(flags);
>
> return ret;
> }
> EXPORT_SYMBOL_GPL(clk_enable);
Ah yes, thanks for the explanation. So if clk=NULL, it just return 0.
But what I'm saying is that if the driver is configured for dual-role
mode, and no clock is specified, then the driver should not be accessing
any gadget functionality.
So as the patch series stands right now, if I swap out an A connector to
a B-connector, then I get a connect_id_status change interrupt. The
status would show a device and I would initialize the gadget portion of
the driver.
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 44c609f..96810f7 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 (hsotg->clk)
+ s3c_hsotg_core_init(hsotg);
So if I don't have a valid clock, I'll be accessing the peripheral
portion of the IP.
But I guess not having the check for the valid clock here should be fine
as I don't see a case where there can be 2 different clocks for host and
peripheral?
>
>>>> @@ -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);
>>>
>>> wait a minute, if there is no clock we don't call the gadget interrupt
>>> handler ? Why ? Who will disable the IRQ line ?
>>
>> This portion is no static int __clk_enable(struct clk *clk)
>
> huh ? What I mean is that this has the potential of leaving that IRQ
> line enabled. Imagine you don't have a clock and s3c_hsotg_irq() isn't
> called, then a peripheral IRQ fires, since the handler isn't called, who
> will clear the interrupt ?
>
Yes, right. This portion of the code is no longer needed when I switched
to use a shared IRQ.
Dinh
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-31 19:31 ` Dinh Nguyen
@ 2014-10-31 19:56 ` Dinh Nguyen
2014-11-03 15:25 ` Felipe Balbi
1 sibling, 0 replies; 32+ messages in thread
From: Dinh Nguyen @ 2014-10-31 19:56 UTC (permalink / raw)
To: balbi
Cc: paulz, dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga,
jg1.han, sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel
On 10/31/2014 02:31 PM, Dinh Nguyen wrote:
> On 10/31/2014 12:42 PM, Felipe Balbi wrote:
>> Hi,
>>
>> On Fri, Oct 31, 2014 at 10:20:06AM -0500, Dinh Nguyen wrote:
>>>>> @@ -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);
>>>>
>>>> instead of exposing the clock detail to the entire driver, add IS_ERR()
>>>> checks to resume and suspend instead. In fact, NULL is a valid clock, so
>>>> you might as well:
>>>>
>>>> clk = clk_get(foo, bar);
>>>> if (IS_ERR(clk))
>>>> dwc->clk = NULL;
>>>> else
>>>> dwc->clk = clk;
>>>>
>>>> Then you don't need any IS_ERR() checks sprinkled around the driver.
>>>
>>> But we would still need to check for the clock before accessing gadget
>>> functionality right?
>>>
>>> if (dwc2->clk)
>>> call_gadget();
>>
>> Read my comment again. "NULL is a valid clock". Look at what
>> clk_enable() does when a NULL pointer is passed:
>>
>> static int __clk_enable(struct clk *clk)
>> {
>> int ret = 0;
>>
>> if (!clk)
>> return 0;
>>
>> if (WARN_ON(clk->prepare_count == 0))
>> return -ESHUTDOWN;
>>
>> if (clk->enable_count == 0) {
>> ret = __clk_enable(clk->parent);
>>
>> if (ret)
>> return ret;
>>
>> if (clk->ops->enable) {
>> ret = clk->ops->enable(clk->hw);
>> if (ret) {
>> __clk_disable(clk->parent);
>> return ret;
>> }
>> }
>> }
>>
>> clk->enable_count++;
>> return 0;
>> }
>>
>> int clk_enable(struct clk *clk)
>> {
>> unsigned long flags;
>> int ret;
>>
>> flags = clk_enable_lock();
>> ret = __clk_enable(clk);
>> clk_enable_unlock(flags);
>>
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(clk_enable);
>
> Ah yes, thanks for the explanation. So if clk=NULL, it just return 0.
> But what I'm saying is that if the driver is configured for dual-role
> mode, and no clock is specified, then the driver should not be accessing
> any gadget functionality.
>
> So as the patch series stands right now, if I swap out an A connector to
> a B-connector, then I get a connect_id_status change interrupt. The
> status would show a device and I would initialize the gadget portion of
> the driver.
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 44c609f..96810f7 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 (hsotg->clk)
> + s3c_hsotg_core_init(hsotg);
>
> So if I don't have a valid clock, I'll be accessing the peripheral
> portion of the IP.
>
> But I guess not having the check for the valid clock here should be fine
> as I don't see a case where there can be 2 different clocks for host and
> peripheral?
>
Ah...nevermind. I don't need to check for clocks at all because in
dwc2_gadget_init(), the clock node check comes before
usb_add_gadget_udc(). Thus without a clock node, gadget functionality is
disabled already.
Thanks,
Dinh
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-31 19:31 ` Dinh Nguyen
2014-10-31 19:56 ` Dinh Nguyen
@ 2014-11-03 15:25 ` Felipe Balbi
1 sibling, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2014-11-03 15:25 UTC (permalink / raw)
To: Dinh Nguyen
Cc: balbi, paulz, dinh.linux, swarren, b.zolnierkie, matthijs,
r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders, kever.yang,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5369 bytes --]
Hi,
On Fri, Oct 31, 2014 at 02:31:31PM -0500, Dinh Nguyen wrote:
> On 10/31/2014 12:42 PM, Felipe Balbi wrote:
> > Hi,
> >
> > On Fri, Oct 31, 2014 at 10:20:06AM -0500, Dinh Nguyen wrote:
> >>>> @@ -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);
> >>>
> >>> instead of exposing the clock detail to the entire driver, add IS_ERR()
> >>> checks to resume and suspend instead. In fact, NULL is a valid clock, so
> >>> you might as well:
> >>>
> >>> clk = clk_get(foo, bar);
> >>> if (IS_ERR(clk))
> >>> dwc->clk = NULL;
> >>> else
> >>> dwc->clk = clk;
> >>>
> >>> Then you don't need any IS_ERR() checks sprinkled around the driver.
> >>
> >> But we would still need to check for the clock before accessing gadget
> >> functionality right?
> >>
> >> if (dwc2->clk)
> >> call_gadget();
> >
> > Read my comment again. "NULL is a valid clock". Look at what
> > clk_enable() does when a NULL pointer is passed:
> >
> > static int __clk_enable(struct clk *clk)
> > {
> > int ret = 0;
> >
> > if (!clk)
> > return 0;
> >
> > if (WARN_ON(clk->prepare_count == 0))
> > return -ESHUTDOWN;
> >
> > if (clk->enable_count == 0) {
> > ret = __clk_enable(clk->parent);
> >
> > if (ret)
> > return ret;
> >
> > if (clk->ops->enable) {
> > ret = clk->ops->enable(clk->hw);
> > if (ret) {
> > __clk_disable(clk->parent);
> > return ret;
> > }
> > }
> > }
> >
> > clk->enable_count++;
> > return 0;
> > }
> >
> > int clk_enable(struct clk *clk)
> > {
> > unsigned long flags;
> > int ret;
> >
> > flags = clk_enable_lock();
> > ret = __clk_enable(clk);
> > clk_enable_unlock(flags);
> >
> > return ret;
> > }
> > EXPORT_SYMBOL_GPL(clk_enable);
>
> Ah yes, thanks for the explanation. So if clk=NULL, it just return 0.
> But what I'm saying is that if the driver is configured for dual-role
> mode, and no clock is specified, then the driver should not be accessing
> any gadget functionality.
why ? Why only for gadget and why can't it work on platforms without
clk? What if Paul wants to run the gadget side on his HAPS-5x platform
configured as a PCIe card ?
You haven't explained why gadget has this hard-dependency on clk and why
*only* gadget has it. This sounds really, really wrong. Why can host
side run without clk but gadget can't ?
Moreover, if you really want to prevent people from using gadget
without clock, fail dwc2_gadget_init() and have the core fallback to
host-only, then print a warning message.
> So as the patch series stands right now, if I swap out an A connector to
> a B-connector, then I get a connect_id_status change interrupt. The
> status would show a device and I would initialize the gadget portion of
> the driver.
that's fine, but why the hard-dependency on clk ?
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 44c609f..96810f7 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 (hsotg->clk)
> + s3c_hsotg_core_init(hsotg);
>
> So if I don't have a valid clock, I'll be accessing the peripheral
> portion of the IP.
so what ?
> But I guess not having the check for the valid clock here should be fine
> as I don't see a case where there can be 2 different clocks for host and
> peripheral?
probably the same thing.
> >>>> @@ -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);
> >>>
> >>> wait a minute, if there is no clock we don't call the gadget interrupt
> >>> handler ? Why ? Who will disable the IRQ line ?
> >>
> >> This portion is no static int __clk_enable(struct clk *clk)
> >
> > huh ? What I mean is that this has the potential of leaving that IRQ
> > line enabled. Imagine you don't have a clock and s3c_hsotg_irq() isn't
> > called, then a peripheral IRQ fires, since the handler isn't called, who
> > will clear the interrupt ?
> >
>
> Yes, right. This portion of the code is no longer needed when I switched
> to use a shared IRQ.
k
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-28 23:25 ` [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node dinguyen
2014-10-29 1:30 ` Paul Zimmerman
2014-10-30 14:04 ` Felipe Balbi
@ 2014-10-31 2:38 ` Kever Yang
2014-10-31 13:49 ` Felipe Balbi
2 siblings, 1 reply; 32+ messages in thread
From: Kever Yang @ 2014-10-31 2:38 UTC (permalink / raw)
To: dinguyen, paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, linux-usb, linux-kernel
Hi Dinh,
On 10/29/2014 07:25 AM, dinguyen@opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> 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.
We have to add clock management for hcd, and I think it is better to
do it before more Socs use this driver, isn't it?
I have do something in my RFC patches, but I think I still do it in a
wrong way.
Can we just handle all the clock thing in platform?
Balbi suggested in my patch that we can "hide" clk_enable()/disable() under
->runtime_resume()/->runtime_suspend() and linux driver model.
Can this be in platform driver?
- Kever
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node
2014-10-31 2:38 ` Kever Yang
@ 2014-10-31 13:49 ` Felipe Balbi
0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2014-10-31 13:49 UTC (permalink / raw)
To: Kever Yang
Cc: dinguyen, paulz, balbi, dinh.linux, swarren, b.zolnierkie,
matthijs, r.baldyga, jg1.han, sachin.kamat, ben-linux, dianders,
linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]
On Fri, Oct 31, 2014 at 10:38:28AM +0800, Kever Yang wrote:
> Hi Dinh,
>
> On 10/29/2014 07:25 AM, dinguyen@opensource.altera.com wrote:
> >From: Dinh Nguyen <dinguyen@opensource.altera.com>
> >
> >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.
> We have to add clock management for hcd, and I think it is better to
> do it before more Socs use this driver, isn't it?
> I have do something in my RFC patches, but I think I still do it in a wrong
> way.
> Can we just handle all the clock thing in platform?
>
> Balbi suggested in my patch that we can "hide" clk_enable()/disable() under
> ->runtime_resume()/->runtime_suspend() and linux driver model.
> Can this be in platform driver?
it can and it probably should. Implement
->runtime_resume()/->runtime_suspend()/runtime_idle() in platform.c and
call pm_runtime_enable()/get()/put()/mark_last_busy()/autosuspend()
properly.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCHv6 7/8] usb: dwc2: Update Kconfig to support dual-role
2014-10-28 23:25 [PATCHv6 0/8] usb: dwc2: Add support for dual-role dinguyen
` (5 preceding siblings ...)
2014-10-28 23:25 ` [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there isn't a clock node dinguyen
@ 2014-10-28 23:25 ` dinguyen
2014-10-28 23:25 ` [PATCHv6 8/8] usb: dwc2: move usb_disabled() call to host driver only dinguyen
7 siblings, 0 replies; 32+ messages in thread
From: dinguyen @ 2014-10-28 23:25 UTC (permalink / raw)
To: paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel, Dinh Nguyen
From: Dinh Nguyen <dinguyen@opensource.altera.com>
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 <dinguyen@opensource.altera.com>
Acked-by: Paul Zimmerman <paulz@synopsys.com>
---
v6: Correct notes for USB_DWC2_DUAL_ROLE and USB_DWC2_PERIPHERAL
Remove extra default for USB_DWC2_PLATFORM and make it a condition for
building dwc2_platform.ko. In addition USB_DWC2_PLATFORM and USB_DWC2_PCI
are now tristate.
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 | 66 ++++++++++++++++++++++++++++-------------------
drivers/usb/dwc2/Makefile | 32 +++++++++++------------
2 files changed, 55 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index 4d02718..b323c4c 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 || USB_GADGET
help
Say Y here if your system has a Dual Role Hi-Speed USB
@@ -10,49 +10,61 @@ 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 an additional module named 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.
-config USB_DWC2_PLATFORM
- bool "DWC2 Platform"
- depends on USB_DWC2_HOST
- default USB_DWC2_HOST
+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 to be enabled.
+
+config USB_DWC2_DUAL_ROLE
+ bool "Dual Role mode"
+ depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
help
- The Designware USB2.0 platform interface module for
- controllers directly connected to the CPU. This is only
- used for host mode.
+ 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 to be enabled.
+endchoice
+
+config USB_DWC2_PLATFORM
+ tristate "DWC2 Platform"
+ default USB_DWC2_HOST || USB_DWC2_PERIPHERAL
+ help
+ The Designware USB2.0 platform interface module for
+ controllers directly connected to the CPU.
config USB_DWC2_PCI
- bool "DWC2 PCI"
+ tristate "DWC2 PCI"
depends on USB_DWC2_HOST && PCI
default USB_DWC2_HOST
help
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..8f75267 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_PLATFORM) += dwc2_platform.o
+dwc2_platform-y := platform.o
--
2.0.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCHv6 8/8] usb: dwc2: move usb_disabled() call to host driver only
2014-10-28 23:25 [PATCHv6 0/8] usb: dwc2: Add support for dual-role dinguyen
` (6 preceding siblings ...)
2014-10-28 23:25 ` [PATCHv6 7/8] usb: dwc2: Update Kconfig to support dual-role dinguyen
@ 2014-10-28 23:25 ` dinguyen
2014-10-29 1:26 ` Paul Zimmerman
7 siblings, 1 reply; 32+ messages in thread
From: dinguyen @ 2014-10-28 23:25 UTC (permalink / raw)
To: paulz, balbi
Cc: dinh.linux, swarren, b.zolnierkie, matthijs, r.baldyga, jg1.han,
sachin.kamat, ben-linux, dianders, kever.yang, linux-usb,
linux-kernel, Dinh Nguyen
From: Dinh Nguyen <dinguyen@opensource.altera.com>
Now that platform.c will get built for both Host and Gadget, if we leave the
usb_disabled() call in platform.c, it results in the following build error
when (!USB && USB_GADGET) condition is met.
ERROR: "usb_disabled" [drivers/usb/dwc2/dwc2_platform.ko] undefined!
Since usb_disabled() is mostly used to disable USB host functionality, move
the call the host portion for the DWC2 driver.
Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
---
drivers/usb/dwc2/hcd.c | 3 +++
drivers/usb/dwc2/platform.c | 3 ---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index fa49c72..b741997 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2780,6 +2780,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
int i, num_channels;
int retval;
+ if (usb_disabled())
+ return -ENODEV;
+
dev_dbg(hsotg->dev, "DWC OTG HCD INIT\n");
/* Detect config values from hardware */
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 77c8417..123cf54 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -157,9 +157,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
int retval;
int irq;
- if (usb_disabled())
- return -ENODEV;
-
match = of_match_device(dwc2_of_match_table, &dev->dev);
if (match && match->data) {
params = match->data;
--
2.0.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* RE: [PATCHv6 8/8] usb: dwc2: move usb_disabled() call to host driver only
2014-10-28 23:25 ` [PATCHv6 8/8] usb: dwc2: move usb_disabled() call to host driver only dinguyen
@ 2014-10-29 1:26 ` Paul Zimmerman
2014-10-29 13:35 ` Dinh Nguyen
0 siblings, 1 reply; 32+ messages in thread
From: Paul Zimmerman @ 2014-10-29 1:26 UTC (permalink / raw)
To: dinguyen@opensource.altera.com, balbi@ti.com
Cc: dinh.linux@gmail.com, swarren@wwwdotorg.org,
b.zolnierkie@samsung.com, matthijs@stdin.nl,
r.baldyga@samsung.com, jg1.han@samsung.com,
sachin.kamat@linaro.org, ben-linux@fluff.org,
dianders@chromium.org, kever.yang@rock-chips.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> From: dinguyen@opensource.altera.com [mailto:dinguyen@opensource.altera.com]
> Sent: Tuesday, October 28, 2014 4:26 PM
>
> Now that platform.c will get built for both Host and Gadget, if we leave the
> usb_disabled() call in platform.c, it results in the following build error
> when (!USB && USB_GADGET) condition is met.
>
> ERROR: "usb_disabled" [drivers/usb/dwc2/dwc2_platform.ko] undefined!
>
> Since usb_disabled() is mostly used to disable USB host functionality, move
> the call the host portion for the DWC2 driver.
>
> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> ---
> drivers/usb/dwc2/hcd.c | 3 +++
> drivers/usb/dwc2/platform.c | 3 ---
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index fa49c72..b741997 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -2780,6 +2780,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> int i, num_channels;
> int retval;
>
> + if (usb_disabled())
> + return -ENODEV;
> +
> dev_dbg(hsotg->dev, "DWC OTG HCD INIT\n");
>
> /* Detect config values from hardware */
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 77c8417..123cf54 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -157,9 +157,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
> int retval;
> int irq;
>
> - if (usb_disabled())
> - return -ENODEV;
> -
> match = of_match_device(dwc2_of_match_table, &dev->dev);
> if (match && match->data) {
> params = match->data;
I'm confused. You are saying the build is broken until patch 8/8 is
applied? As always, that is not acceptable. You need to fix the
breakage at the point where it was introduced, not leave it broken
until the last patch in the series.
--
Paul
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 8/8] usb: dwc2: move usb_disabled() call to host driver only
2014-10-29 1:26 ` Paul Zimmerman
@ 2014-10-29 13:35 ` Dinh Nguyen
2014-10-30 14:07 ` Felipe Balbi
0 siblings, 1 reply; 32+ messages in thread
From: Dinh Nguyen @ 2014-10-29 13:35 UTC (permalink / raw)
To: Paul Zimmerman, balbi@ti.com
Cc: dinh.linux@gmail.com, swarren@wwwdotorg.org,
b.zolnierkie@samsung.com, matthijs@stdin.nl,
r.baldyga@samsung.com, jg1.han@samsung.com,
sachin.kamat@linaro.org, ben-linux@fluff.org,
dianders@chromium.org, kever.yang@rock-chips.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
On 10/28/14, 8:26 PM, Paul Zimmerman wrote:
>> From: dinguyen@opensource.altera.com [mailto:dinguyen@opensource.altera.com]
>> Sent: Tuesday, October 28, 2014 4:26 PM
>>
>> Now that platform.c will get built for both Host and Gadget, if we leave the
>> usb_disabled() call in platform.c, it results in the following build error
>> when (!USB && USB_GADGET) condition is met.
>>
>> ERROR: "usb_disabled" [drivers/usb/dwc2/dwc2_platform.ko] undefined!
>>
>> Since usb_disabled() is mostly used to disable USB host functionality, move
>> the call the host portion for the DWC2 driver.
>>
>> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
>> ---
>> drivers/usb/dwc2/hcd.c | 3 +++
>> drivers/usb/dwc2/platform.c | 3 ---
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index fa49c72..b741997 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -2780,6 +2780,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
>> int i, num_channels;
>> int retval;
>>
>> + if (usb_disabled())
>> + return -ENODEV;
>> +
>> dev_dbg(hsotg->dev, "DWC OTG HCD INIT\n");
>>
>> /* Detect config values from hardware */
>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>> index 77c8417..123cf54 100644
>> --- a/drivers/usb/dwc2/platform.c
>> +++ b/drivers/usb/dwc2/platform.c
>> @@ -157,9 +157,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
>> int retval;
>> int irq;
>>
>> - if (usb_disabled())
>> - return -ENODEV;
>> -
>> match = of_match_device(dwc2_of_match_table, &dev->dev);
>> if (match && match->data) {
>> params = match->data;
>
> I'm confused. You are saying the build is broken until patch 8/8 is
> applied? As always, that is not acceptable. You need to fix the
> breakage at the point where it was introduced, not leave it broken
> until the last patch in the series.
>
The build gets broken when patch 7/8 of is applied. That is the patch
that finally allows platform.c to get built for host and gadget. I can
fold this patch into patch 7/8.
Dinh
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCHv6 8/8] usb: dwc2: move usb_disabled() call to host driver only
2014-10-29 13:35 ` Dinh Nguyen
@ 2014-10-30 14:07 ` Felipe Balbi
0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2014-10-30 14:07 UTC (permalink / raw)
To: Dinh Nguyen
Cc: Paul Zimmerman, balbi@ti.com, dinh.linux@gmail.com,
swarren@wwwdotorg.org, b.zolnierkie@samsung.com,
matthijs@stdin.nl, r.baldyga@samsung.com, jg1.han@samsung.com,
sachin.kamat@linaro.org, ben-linux@fluff.org,
dianders@chromium.org, kever.yang@rock-chips.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2471 bytes --]
On Wed, Oct 29, 2014 at 08:35:24AM -0500, Dinh Nguyen wrote:
>
>
> On 10/28/14, 8:26 PM, Paul Zimmerman wrote:
> >> From: dinguyen@opensource.altera.com [mailto:dinguyen@opensource.altera.com]
> >> Sent: Tuesday, October 28, 2014 4:26 PM
> >>
> >> Now that platform.c will get built for both Host and Gadget, if we leave the
> >> usb_disabled() call in platform.c, it results in the following build error
> >> when (!USB && USB_GADGET) condition is met.
> >>
> >> ERROR: "usb_disabled" [drivers/usb/dwc2/dwc2_platform.ko] undefined!
> >>
> >> Since usb_disabled() is mostly used to disable USB host functionality, move
> >> the call the host portion for the DWC2 driver.
> >>
> >> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> >> ---
> >> drivers/usb/dwc2/hcd.c | 3 +++
> >> drivers/usb/dwc2/platform.c | 3 ---
> >> 2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> >> index fa49c72..b741997 100644
> >> --- a/drivers/usb/dwc2/hcd.c
> >> +++ b/drivers/usb/dwc2/hcd.c
> >> @@ -2780,6 +2780,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> >> int i, num_channels;
> >> int retval;
> >>
> >> + if (usb_disabled())
> >> + return -ENODEV;
> >> +
> >> dev_dbg(hsotg->dev, "DWC OTG HCD INIT\n");
> >>
> >> /* Detect config values from hardware */
> >> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> >> index 77c8417..123cf54 100644
> >> --- a/drivers/usb/dwc2/platform.c
> >> +++ b/drivers/usb/dwc2/platform.c
> >> @@ -157,9 +157,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
> >> int retval;
> >> int irq;
> >>
> >> - if (usb_disabled())
> >> - return -ENODEV;
> >> -
> >> match = of_match_device(dwc2_of_match_table, &dev->dev);
> >> if (match && match->data) {
> >> params = match->data;
> >
> > I'm confused. You are saying the build is broken until patch 8/8 is
> > applied? As always, that is not acceptable. You need to fix the
> > breakage at the point where it was introduced, not leave it broken
> > until the last patch in the series.
> >
>
> The build gets broken when patch 7/8 of is applied. That is the patch
> that finally allows platform.c to get built for host and gadget. I can
> fold this patch into patch 7/8.
then you invert things, make $subject patch 7 (or even patch 1) and
patch 7 becomes patch 8.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread