[PATCH v6 0/3] usb: otg-fsm: Add documentation and some trivial cleanups

[PATCH v6 0/3] usb: otg-fsm: Add documentation and some trivial cleanups

[Roger Quadros]

Hi,

Add documentation for struct otg_fsm with some trivial cleanups.
All patches have been Acked by otg-fsm maintainer (Peter Chen).

cheers,
-roger

Roger Quadros (3):
  usb: otg-fsm: Add documentation for struct otg_fsm
  usb: otg-fsm: support multiple instances
  usb: otg-fsm: Prevent build warning "VDBG" redefined

 drivers/usb/chipidea/otg_fsm.c   |   1 +
 drivers/usb/common/usb-otg-fsm.c |  20 +++----
 drivers/usb/phy/phy-fsl-usb.c    |   1 +
 include/linux/usb/otg-fsm.h      | 110 +++++++++++++++++++++++++++++++--------
 4 files changed, 100 insertions(+), 32 deletions(-)

-- 
2.5.0

[PATCH v6 1/3] usb: otg-fsm: Add documentation for struct otg_fsm

[Roger Quadros]

struct otg_fsm is the interface to the OTG state machine.

Document the input, output and internal state variables.
Definations are taken from Table 7-2 and Table 7-4 of
the USB OTG & EH Specification Rev.2.0

Re-arrange some of the members as per use case for more
clarity.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Peter Chen <peter.chen@freescale.com>
---
 include/linux/usb/otg-fsm.h | 90 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 83 insertions(+), 7 deletions(-)

diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 24198e1..8eec0c2 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -72,37 +72,113 @@ enum otg_fsm_timer {
 	NUM_OTG_FSM_TIMERS,
 };
 
-/* OTG state machine according to the OTG spec */
+/**
+ * struct otg_fsm - OTG state machine according to the OTG spec
+ *
+ * OTG hardware Inputs
+ *
+ *	Common inputs for A and B device
+ * @id:		TRUE for B-device, FALSE for A-device.
+ * @adp_change: TRUE when current ADP measurement (n) value, compared to the
+ *		ADP measurement taken at n-2, differs by more than CADP_THR
+ * @power_up:	TRUE when the OTG device first powers up its USB system and
+ *		ADP measurement taken if ADP capable
+ *
+ *	A-Device state inputs
+ * @a_srp_det:	TRUE if the A-device detects SRP
+ * @a_vbus_vld:	TRUE when VBUS voltage is in regulation
+ * @b_conn:	TRUE if the A-device detects connection from the B-device
+ * @a_bus_resume: TRUE when the B-device detects that the A-device is signaling
+ *		  a resume (K state)
+ *	B-Device state inputs
+ * @a_bus_suspend: TRUE when the B-device detects that the A-device has put the
+ *		bus into suspend
+ * @a_conn:	TRUE if the B-device detects a connection from the A-device
+ * @b_se0_srp:	TRUE when the line has been at SE0 for more than the minimum
+ *		time before generating SRP
+ * @b_ssend_srp: TRUE when the VBUS has been below VOTG_SESS_VLD for more than
+ *		 the minimum time before generating SRP
+ * @b_sess_vld:	TRUE when the B-device detects that the voltage on VBUS is
+ *		above VOTG_SESS_VLD
+ * @test_device: TRUE when the B-device switches to B-Host and detects an OTG
+ *		test device. This must be set by host/hub driver
+ *
+ *	Application inputs (A-Device)
+ * @a_bus_drop:	TRUE when A-device application needs to power down the bus
+ * @a_bus_req:	TRUE when A-device application wants to use the bus.
+ *		FALSE to suspend the bus
+ *
+ *	Application inputs (B-Device)
+ * @b_bus_req:	TRUE during the time that the Application running on the
+ *		B-device wants to use the bus
+ *
+ *	Auxilary inputs (OTG v1.3 only. Obsolete now.)
+ * @a_sess_vld:	TRUE if the A-device detects that VBUS is above VA_SESS_VLD
+ * @b_bus_suspend: TRUE when the A-device detects that the B-device has put
+ *		the bus into suspend
+ * @b_bus_resume: TRUE when the A-device detects that the B-device is signaling
+ *		 resume on the bus
+ *
+ * OTG Output status. Read only for users. Updated by OTG FSM helpers defined
+ * in this file
+ *
+ *	Outputs for Both A and B device
+ * @drv_vbus:	TRUE when A-device is driving VBUS
+ * @loc_conn:	TRUE when the local device has signaled that it is connected
+ *		to the bus
+ * @loc_sof:	TRUE when the local device is generating activity on the bus
+ * @adp_prb:	TRUE when the local device is in the process of doing
+ *		ADP probing
+ *
+ *	Outputs for B-device state
+ * @adp_sns:	TRUE when the B-device is in the process of carrying out
+ *		ADP sensing
+ * @data_pulse: TRUE when the B-device is performing data line pulsing
+ *
+ * Internal Variables
+ *
+ * a_set_b_hnp_en: TRUE when the A-device has successfully set the
+ *		b_hnp_enable bit in the B-device.
+ *		   Unused as OTG fsm uses otg->host->b_hnp_enable instead
+ * b_srp_done:	TRUE when the B-device has completed initiating SRP
+ * b_hnp_enable: TRUE when the B-device has accepted the
+ *		SetFeature(b_hnp_enable) B-device.
+ *		Unused as OTG fsm uses otg->gadget->b_hnp_enable instead
+ * a_clr_err:	Asserted (by application ?) to clear a_vbus_err due to an
+ *		overcurrent condition and causes the A-device to transition
+ *		to a_wait_vfall
+ */
 struct otg_fsm {
 	/* Input */
 	int id;
 	int adp_change;
 	int power_up;
-	int test_device;
-	int a_bus_drop;
-	int a_bus_req;
 	int a_srp_det;
 	int a_vbus_vld;
 	int b_conn;
 	int a_bus_resume;
 	int a_bus_suspend;
 	int a_conn;
-	int b_bus_req;
 	int b_se0_srp;
 	int b_ssend_srp;
 	int b_sess_vld;
+	int test_device;
+	int a_bus_drop;
+	int a_bus_req;
+	int b_bus_req;
+
 	/* Auxilary inputs */
 	int a_sess_vld;
 	int b_bus_resume;
 	int b_bus_suspend;
 
 	/* Output */
-	int data_pulse;
 	int drv_vbus;
 	int loc_conn;
 	int loc_sof;
 	int adp_prb;
 	int adp_sns;
+	int data_pulse;
 
 	/* Internal variables */
 	int a_set_b_hnp_en;
@@ -110,7 +186,7 @@ struct otg_fsm {
 	int b_hnp_enable;
 	int a_clr_err;
 
-	/* Informative variables */
+	/* Informative variables. All unused as of now */
 	int a_bus_drop_inf;
 	int a_bus_req_inf;
 	int a_clr_err_inf;
-- 
2.5.0

[PATCH v6 2/3] usb: otg-fsm: support multiple instances

[Roger Quadros]

Move the state_changed variable into struct otg_fsm
so that we can support multiple instances.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/common/usb-otg-fsm.c | 10 ++++------
 include/linux/usb/otg-fsm.h      |  1 +
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 504708f..9059b7d 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -61,8 +61,6 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
 	return 0;
 }
 
-static int state_changed;
-
 /* Called when leaving a state.  Do state clean up jobs here */
 static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state old_state)
 {
@@ -208,7 +206,6 @@ static void otg_start_hnp_polling(struct otg_fsm *fsm)
 /* Called when entering a state */
 static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
 {
-	state_changed = 1;
 	if (fsm->otg->state == new_state)
 		return 0;
 	VDBG("Set state: %s\n", usb_otg_state_string(new_state));
@@ -324,6 +321,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
 	}
 
 	fsm->otg->state = new_state;
+	fsm->state_changed = 1;
 	return 0;
 }
 
@@ -335,7 +333,7 @@ int otg_statemachine(struct otg_fsm *fsm)
 	mutex_lock(&fsm->lock);
 
 	state = fsm->otg->state;
-	state_changed = 0;
+	fsm->state_changed = 0;
 	/* State machine state change judgement */
 
 	switch (state) {
@@ -448,7 +446,7 @@ int otg_statemachine(struct otg_fsm *fsm)
 	}
 	mutex_unlock(&fsm->lock);
 
-	VDBG("quit statemachine, changed = %d\n", state_changed);
-	return state_changed;
+	VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
+	return fsm->state_changed;
 }
 EXPORT_SYMBOL_GPL(otg_statemachine);
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 8eec0c2..7a03505 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -210,6 +210,7 @@ struct otg_fsm {
 	struct mutex lock;
 	u8 *host_req_flag;
 	struct delayed_work hnp_polling_work;
+	bool state_changed;
 };
 
 struct otg_fsm_ops {
-- 
2.5.0

[PATCH v6 3/3] usb: otg-fsm: Prevent build warning "VDBG" redefined

[Roger Quadros]

If usb/otg-fsm.h and usb/composite.h are included together
then it results in the build warning [1].

Prevent that by using dev_vdbg() instead.

Also get rid of MPC_LOC which doesn't seem to be used
by anyone.

[1] - warning fixed by this patch:

   In file included from drivers/usb/dwc3/core.h:33,
                    from drivers/usb/dwc3/ep0.c:33:
   include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
   In file included from drivers/usb/dwc3/ep0.c:31:
   include/linux/usb/composite.h:615:1: warning: this is the location
	of the previous definition

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/otg_fsm.c   |  1 +
 drivers/usb/common/usb-otg-fsm.c | 12 +++++++-----
 drivers/usb/phy/phy-fsl-usb.c    |  1 +
 include/linux/usb/otg-fsm.h      | 19 ++++---------------
 4 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index de8e22e..5f169b3 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -805,6 +805,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
 	ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
 	ci->fsm.otg->state = OTG_STATE_UNDEFINED;
 	ci->fsm.ops = &ci_otg_ops;
+	ci->fsm.dev = ci->dev;
 	ci->gadget.hnp_polling_support = 1;
 	ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
 	if (!ci->fsm.host_req_flag)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 9059b7d..c5a61fe 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
 	int ret = 0;
 
 	if (fsm->protocol != protocol) {
-		VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
-			fsm->protocol, protocol);
+		dev_vdbg(fsm->dev,
+			 "Changing role fsm->protocol= %d; new protocol= %d\n",
+			 fsm->protocol, protocol);
 		/* stop old protocol */
 		if (fsm->protocol == PROTO_HOST)
 			ret = otg_start_host(fsm, 0);
@@ -208,7 +209,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
 {
 	if (fsm->otg->state == new_state)
 		return 0;
-	VDBG("Set state: %s\n", usb_otg_state_string(new_state));
+	dev_vdbg(fsm->dev, "Set state: %s\n", usb_otg_state_string(new_state));
 	otg_leave_state(fsm, fsm->otg->state);
 	switch (new_state) {
 	case OTG_STATE_B_IDLE:
@@ -338,7 +339,7 @@ int otg_statemachine(struct otg_fsm *fsm)
 
 	switch (state) {
 	case OTG_STATE_UNDEFINED:
-		VDBG("fsm->id = %d\n", fsm->id);
+		dev_vdbg(fsm->dev, "fsm->id = %d\n", fsm->id);
 		if (fsm->id)
 			otg_set_state(fsm, OTG_STATE_B_IDLE);
 		else
@@ -446,7 +447,8 @@ int otg_statemachine(struct otg_fsm *fsm)
 	}
 	mutex_unlock(&fsm->lock);
 
-	VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
+	dev_vdbg(fsm->dev, "quit statemachine, changed = %d\n",
+		 fsm->state_changed);
 	return fsm->state_changed;
 }
 EXPORT_SYMBOL_GPL(otg_statemachine);
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..ee3f2c2 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -817,6 +817,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
 
 	/* Set OTG state machine operations */
 	fsl_otg_tc->fsm.ops = &fsl_otg_ops;
+	fsl_otg_tc->fsm.dev = &pdev->dev;
 
 	/* initialize the otg structure */
 	fsl_otg_tc->phy.label = DRIVER_DESC;
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 7a03505..47b8392 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -18,24 +18,10 @@
 #ifndef __LINUX_USB_OTG_FSM_H
 #define __LINUX_USB_OTG_FSM_H
 
+#include <linux/device.h>
 #include <linux/mutex.h>
 #include <linux/errno.h>
 
-#undef VERBOSE
-
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
-				 __func__, ## args)
-#else
-#define VDBG(stuff...)	do {} while (0)
-#endif
-
-#ifdef VERBOSE
-#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
-#else
-#define MPC_LOC do {} while (0)
-#endif
-
 #define PROTO_UNDEF	(0)
 #define PROTO_HOST	(1)
 #define PROTO_GADGET	(2)
@@ -211,6 +197,9 @@ struct otg_fsm {
 	u8 *host_req_flag;
 	struct delayed_work hnp_polling_work;
 	bool state_changed;
+
+	/* for debug prints */
+	struct device *dev;
 };
 
 struct otg_fsm_ops {
-- 
2.5.0

[PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG" redefined

[Roger Quadros]

If usb/otg-fsm.h and usb/composite.h are included together
then it results in the build warning [1].

Prevent that by using dev_vdbg() instead.

Also get rid of MPC_LOC which doesn't seem to be used
by anyone.

[1] - warning fixed by this patch:

   In file included from drivers/usb/dwc3/core.h:33,
                    from drivers/usb/dwc3/ep0.c:33:
   include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
   In file included from drivers/usb/dwc3/ep0.c:31:
   include/linux/usb/composite.h:615:1: warning: this is the location
	of the previous definition

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
v7: define VDBG locally in phy-fsl-usb.c

 drivers/usb/chipidea/otg_fsm.c   |  1 +
 drivers/usb/common/usb-otg-fsm.c | 12 +++++++-----
 drivers/usb/phy/phy-fsl-usb.c    |  8 ++++++++
 include/linux/usb/otg-fsm.h      | 19 ++++---------------
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index de8e22e..5f169b3 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -805,6 +805,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
 	ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
 	ci->fsm.otg->state = OTG_STATE_UNDEFINED;
 	ci->fsm.ops = &ci_otg_ops;
+	ci->fsm.dev = ci->dev;
 	ci->gadget.hnp_polling_support = 1;
 	ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
 	if (!ci->fsm.host_req_flag)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 9059b7d..c5a61fe 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
 	int ret = 0;
 
 	if (fsm->protocol != protocol) {
-		VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
-			fsm->protocol, protocol);
+		dev_vdbg(fsm->dev,
+			 "Changing role fsm->protocol= %d; new protocol= %d\n",
+			 fsm->protocol, protocol);
 		/* stop old protocol */
 		if (fsm->protocol == PROTO_HOST)
 			ret = otg_start_host(fsm, 0);
@@ -208,7 +209,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
 {
 	if (fsm->otg->state == new_state)
 		return 0;
-	VDBG("Set state: %s\n", usb_otg_state_string(new_state));
+	dev_vdbg(fsm->dev, "Set state: %s\n", usb_otg_state_string(new_state));
 	otg_leave_state(fsm, fsm->otg->state);
 	switch (new_state) {
 	case OTG_STATE_B_IDLE:
@@ -338,7 +339,7 @@ int otg_statemachine(struct otg_fsm *fsm)
 
 	switch (state) {
 	case OTG_STATE_UNDEFINED:
-		VDBG("fsm->id = %d\n", fsm->id);
+		dev_vdbg(fsm->dev, "fsm->id = %d\n", fsm->id);
 		if (fsm->id)
 			otg_set_state(fsm, OTG_STATE_B_IDLE);
 		else
@@ -446,7 +447,8 @@ int otg_statemachine(struct otg_fsm *fsm)
 	}
 	mutex_unlock(&fsm->lock);
 
-	VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
+	dev_vdbg(fsm->dev, "quit statemachine, changed = %d\n",
+		 fsm->state_changed);
 	return fsm->state_changed;
 }
 EXPORT_SYMBOL_GPL(otg_statemachine);
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..c57ef5c 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -44,6 +44,13 @@
 
 #include "phy-fsl-usb.h"
 
+#ifdef VERBOSE
+#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
+				    __func__, ## args)
+#else
+#define VDBG(stuff...)	do {} while (0)
+#endif
+
 #define DRIVER_VERSION "Rev. 1.55"
 #define DRIVER_AUTHOR "Jerry Huang/Li Yang"
 #define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
@@ -817,6 +824,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
 
 	/* Set OTG state machine operations */
 	fsl_otg_tc->fsm.ops = &fsl_otg_ops;
+	fsl_otg_tc->fsm.dev = &pdev->dev;
 
 	/* initialize the otg structure */
 	fsl_otg_tc->phy.label = DRIVER_DESC;
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 7a03505..47b8392 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -18,24 +18,10 @@
 #ifndef __LINUX_USB_OTG_FSM_H
 #define __LINUX_USB_OTG_FSM_H
 
+#include <linux/device.h>
 #include <linux/mutex.h>
 #include <linux/errno.h>
 
-#undef VERBOSE
-
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
-				 __func__, ## args)
-#else
-#define VDBG(stuff...)	do {} while (0)
-#endif
-
-#ifdef VERBOSE
-#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
-#else
-#define MPC_LOC do {} while (0)
-#endif
-
 #define PROTO_UNDEF	(0)
 #define PROTO_HOST	(1)
 #define PROTO_GADGET	(2)
@@ -211,6 +197,9 @@ struct otg_fsm {
 	u8 *host_req_flag;
 	struct delayed_work hnp_polling_work;
 	bool state_changed;
+
+	/* for debug prints */
+	struct device *dev;
 };
 
 struct otg_fsm_ops {
-- 
2.5.0

Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG" redefined

[Peter Chen]

On Thu, Mar 31, 2016 at 12:41:19PM +0300, Roger Quadros wrote:
> If usb/otg-fsm.h and usb/composite.h are included together
> then it results in the build warning [1].
> 
> Prevent that by using dev_vdbg() instead.
> 

After considering it more, I think it may not be a good solution
that we delete VDBG at one header file, but keep it at another
one. In future, we may add VDBG at another file, and cause the
same problem. In fact, I find VDBG is defined at several files
in USB folder (and only at USB folder), I plan to replace them
with standard one (dev_vdbg) together.

> Also get rid of MPC_LOC which doesn't seem to be used
> by anyone.
> 

If you want, you can only delete MPC_LOC at this patch.

Peter

> [1] - warning fixed by this patch:
> 
>    In file included from drivers/usb/dwc3/core.h:33,
>                     from drivers/usb/dwc3/ep0.c:33:
>    include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
>    In file included from drivers/usb/dwc3/ep0.c:31:
>    include/linux/usb/composite.h:615:1: warning: this is the location
> 	of the previous definition
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> v7: define VDBG locally in phy-fsl-usb.c
> 
>  drivers/usb/chipidea/otg_fsm.c   |  1 +
>  drivers/usb/common/usb-otg-fsm.c | 12 +++++++-----
>  drivers/usb/phy/phy-fsl-usb.c    |  8 ++++++++
>  include/linux/usb/otg-fsm.h      | 19 ++++---------------
>  4 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index de8e22e..5f169b3 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -805,6 +805,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
>  	ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
>  	ci->fsm.otg->state = OTG_STATE_UNDEFINED;
>  	ci->fsm.ops = &ci_otg_ops;
> +	ci->fsm.dev = ci->dev;
>  	ci->gadget.hnp_polling_support = 1;
>  	ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
>  	if (!ci->fsm.host_req_flag)
> diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
> index 9059b7d..c5a61fe 100644
> --- a/drivers/usb/common/usb-otg-fsm.c
> +++ b/drivers/usb/common/usb-otg-fsm.c
> @@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
>  	int ret = 0;
>  
>  	if (fsm->protocol != protocol) {
> -		VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
> -			fsm->protocol, protocol);
> +		dev_vdbg(fsm->dev,
> +			 "Changing role fsm->protocol= %d; new protocol= %d\n",
> +			 fsm->protocol, protocol);
>  		/* stop old protocol */
>  		if (fsm->protocol == PROTO_HOST)
>  			ret = otg_start_host(fsm, 0);
> @@ -208,7 +209,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
>  {
>  	if (fsm->otg->state == new_state)
>  		return 0;
> -	VDBG("Set state: %s\n", usb_otg_state_string(new_state));
> +	dev_vdbg(fsm->dev, "Set state: %s\n", usb_otg_state_string(new_state));
>  	otg_leave_state(fsm, fsm->otg->state);
>  	switch (new_state) {
>  	case OTG_STATE_B_IDLE:
> @@ -338,7 +339,7 @@ int otg_statemachine(struct otg_fsm *fsm)
>  
>  	switch (state) {
>  	case OTG_STATE_UNDEFINED:
> -		VDBG("fsm->id = %d\n", fsm->id);
> +		dev_vdbg(fsm->dev, "fsm->id = %d\n", fsm->id);
>  		if (fsm->id)
>  			otg_set_state(fsm, OTG_STATE_B_IDLE);
>  		else
> @@ -446,7 +447,8 @@ int otg_statemachine(struct otg_fsm *fsm)
>  	}
>  	mutex_unlock(&fsm->lock);
>  
> -	VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
> +	dev_vdbg(fsm->dev, "quit statemachine, changed = %d\n",
> +		 fsm->state_changed);
>  	return fsm->state_changed;
>  }
>  EXPORT_SYMBOL_GPL(otg_statemachine);
> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
> index 94eb292..c57ef5c 100644
> --- a/drivers/usb/phy/phy-fsl-usb.c
> +++ b/drivers/usb/phy/phy-fsl-usb.c
> @@ -44,6 +44,13 @@
>  
>  #include "phy-fsl-usb.h"
>  
> +#ifdef VERBOSE
> +#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
> +				    __func__, ## args)
> +#else
> +#define VDBG(stuff...)	do {} while (0)
> +#endif
> +
>  #define DRIVER_VERSION "Rev. 1.55"
>  #define DRIVER_AUTHOR "Jerry Huang/Li Yang"
>  #define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
> @@ -817,6 +824,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
>  
>  	/* Set OTG state machine operations */
>  	fsl_otg_tc->fsm.ops = &fsl_otg_ops;
> +	fsl_otg_tc->fsm.dev = &pdev->dev;
>  
>  	/* initialize the otg structure */
>  	fsl_otg_tc->phy.label = DRIVER_DESC;
> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
> index 7a03505..47b8392 100644
> --- a/include/linux/usb/otg-fsm.h
> +++ b/include/linux/usb/otg-fsm.h
> @@ -18,24 +18,10 @@
>  #ifndef __LINUX_USB_OTG_FSM_H
>  #define __LINUX_USB_OTG_FSM_H
>  
> +#include <linux/device.h>
>  #include <linux/mutex.h>
>  #include <linux/errno.h>
>  
> -#undef VERBOSE
> -
> -#ifdef VERBOSE
> -#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
> -				 __func__, ## args)
> -#else
> -#define VDBG(stuff...)	do {} while (0)
> -#endif
> -
> -#ifdef VERBOSE
> -#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
> -#else
> -#define MPC_LOC do {} while (0)
> -#endif
> -
>  #define PROTO_UNDEF	(0)
>  #define PROTO_HOST	(1)
>  #define PROTO_GADGET	(2)
> @@ -211,6 +197,9 @@ struct otg_fsm {
>  	u8 *host_req_flag;
>  	struct delayed_work hnp_polling_work;
>  	bool state_changed;
> +
> +	/* for debug prints */
> +	struct device *dev;
>  };
>  
>  struct otg_fsm_ops {
> -- 
> 2.5.0
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG" redefined

[Roger Quadros]

Peter,

On 05/04/16 11:52, Peter Chen wrote:
> On Thu, Mar 31, 2016 at 12:41:19PM +0300, Roger Quadros wrote:
>> If usb/otg-fsm.h and usb/composite.h are included together
>> then it results in the build warning [1].
>>
>> Prevent that by using dev_vdbg() instead.
>>
> 
> After considering it more, I think it may not be a good solution
> that we delete VDBG at one header file, but keep it at another
> one. In future, we may add VDBG at another file, and cause the
> same problem. In fact, I find VDBG is defined at several files
> in USB folder (and only at USB folder), I plan to replace them
> with standard one (dev_vdbg) together.

OK, please ignore this patch then.

cheers,
-roger

> 
>> Also get rid of MPC_LOC which doesn't seem to be used
>> by anyone.
>>
> 
> If you want, you can only delete MPC_LOC at this patch.
> 
> Peter
> 
>> [1] - warning fixed by this patch:
>>
>>    In file included from drivers/usb/dwc3/core.h:33,
>>                     from drivers/usb/dwc3/ep0.c:33:
>>    include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
>>    In file included from drivers/usb/dwc3/ep0.c:31:
>>    include/linux/usb/composite.h:615:1: warning: this is the location
>> 	of the previous definition
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> v7: define VDBG locally in phy-fsl-usb.c
>>
>>  drivers/usb/chipidea/otg_fsm.c   |  1 +
>>  drivers/usb/common/usb-otg-fsm.c | 12 +++++++-----
>>  drivers/usb/phy/phy-fsl-usb.c    |  8 ++++++++
>>  include/linux/usb/otg-fsm.h      | 19 ++++---------------
>>  4 files changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
>> index de8e22e..5f169b3 100644
>> --- a/drivers/usb/chipidea/otg_fsm.c
>> +++ b/drivers/usb/chipidea/otg_fsm.c
>> @@ -805,6 +805,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
>>  	ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
>>  	ci->fsm.otg->state = OTG_STATE_UNDEFINED;
>>  	ci->fsm.ops = &ci_otg_ops;
>> +	ci->fsm.dev = ci->dev;
>>  	ci->gadget.hnp_polling_support = 1;
>>  	ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
>>  	if (!ci->fsm.host_req_flag)
>> diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
>> index 9059b7d..c5a61fe 100644
>> --- a/drivers/usb/common/usb-otg-fsm.c
>> +++ b/drivers/usb/common/usb-otg-fsm.c
>> @@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
>>  	int ret = 0;
>>  
>>  	if (fsm->protocol != protocol) {
>> -		VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
>> -			fsm->protocol, protocol);
>> +		dev_vdbg(fsm->dev,
>> +			 "Changing role fsm->protocol= %d; new protocol= %d\n",
>> +			 fsm->protocol, protocol);
>>  		/* stop old protocol */
>>  		if (fsm->protocol == PROTO_HOST)
>>  			ret = otg_start_host(fsm, 0);
>> @@ -208,7 +209,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
>>  {
>>  	if (fsm->otg->state == new_state)
>>  		return 0;
>> -	VDBG("Set state: %s\n", usb_otg_state_string(new_state));
>> +	dev_vdbg(fsm->dev, "Set state: %s\n", usb_otg_state_string(new_state));
>>  	otg_leave_state(fsm, fsm->otg->state);
>>  	switch (new_state) {
>>  	case OTG_STATE_B_IDLE:
>> @@ -338,7 +339,7 @@ int otg_statemachine(struct otg_fsm *fsm)
>>  
>>  	switch (state) {
>>  	case OTG_STATE_UNDEFINED:
>> -		VDBG("fsm->id = %d\n", fsm->id);
>> +		dev_vdbg(fsm->dev, "fsm->id = %d\n", fsm->id);
>>  		if (fsm->id)
>>  			otg_set_state(fsm, OTG_STATE_B_IDLE);
>>  		else
>> @@ -446,7 +447,8 @@ int otg_statemachine(struct otg_fsm *fsm)
>>  	}
>>  	mutex_unlock(&fsm->lock);
>>  
>> -	VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
>> +	dev_vdbg(fsm->dev, "quit statemachine, changed = %d\n",
>> +		 fsm->state_changed);
>>  	return fsm->state_changed;
>>  }
>>  EXPORT_SYMBOL_GPL(otg_statemachine);
>> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
>> index 94eb292..c57ef5c 100644
>> --- a/drivers/usb/phy/phy-fsl-usb.c
>> +++ b/drivers/usb/phy/phy-fsl-usb.c
>> @@ -44,6 +44,13 @@
>>  
>>  #include "phy-fsl-usb.h"
>>  
>> +#ifdef VERBOSE
>> +#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
>> +				    __func__, ## args)
>> +#else
>> +#define VDBG(stuff...)	do {} while (0)
>> +#endif
>> +
>>  #define DRIVER_VERSION "Rev. 1.55"
>>  #define DRIVER_AUTHOR "Jerry Huang/Li Yang"
>>  #define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
>> @@ -817,6 +824,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
>>  
>>  	/* Set OTG state machine operations */
>>  	fsl_otg_tc->fsm.ops = &fsl_otg_ops;
>> +	fsl_otg_tc->fsm.dev = &pdev->dev;
>>  
>>  	/* initialize the otg structure */
>>  	fsl_otg_tc->phy.label = DRIVER_DESC;
>> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
>> index 7a03505..47b8392 100644
>> --- a/include/linux/usb/otg-fsm.h
>> +++ b/include/linux/usb/otg-fsm.h
>> @@ -18,24 +18,10 @@
>>  #ifndef __LINUX_USB_OTG_FSM_H
>>  #define __LINUX_USB_OTG_FSM_H
>>  
>> +#include <linux/device.h>
>>  #include <linux/mutex.h>
>>  #include <linux/errno.h>
>>  
>> -#undef VERBOSE
>> -
>> -#ifdef VERBOSE
>> -#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
>> -				 __func__, ## args)
>> -#else
>> -#define VDBG(stuff...)	do {} while (0)
>> -#endif
>> -
>> -#ifdef VERBOSE
>> -#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
>> -#else
>> -#define MPC_LOC do {} while (0)
>> -#endif
>> -
>>  #define PROTO_UNDEF	(0)
>>  #define PROTO_HOST	(1)
>>  #define PROTO_GADGET	(2)
>> @@ -211,6 +197,9 @@ struct otg_fsm {
>>  	u8 *host_req_flag;
>>  	struct delayed_work hnp_polling_work;
>>  	bool state_changed;
>> +
>> +	/* for debug prints */
>> +	struct device *dev;
>>  };
>>  
>>  struct otg_fsm_ops {
>> -- 
>> 2.5.0
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG" redefined

[Roger Quadros]

Peter,

On 05/04/16 15:52, Roger Quadros wrote:
> Peter,
> 
> On 05/04/16 11:52, Peter Chen wrote:
>> On Thu, Mar 31, 2016 at 12:41:19PM +0300, Roger Quadros wrote:
>>> If usb/otg-fsm.h and usb/composite.h are included together
>>> then it results in the build warning [1].
>>>
>>> Prevent that by using dev_vdbg() instead.
>>>
>>
>> After considering it more, I think it may not be a good solution
>> that we delete VDBG at one header file, but keep it at another
>> one. In future, we may add VDBG at another file, and cause the
>> same problem. In fact, I find VDBG is defined at several files
>> in USB folder (and only at USB folder), I plan to replace them
>> with standard one (dev_vdbg) together.
> 
> OK, please ignore this patch then.

On second thoughts can you please retain this patch and post the
VDBG removal from composite.h cleanup separately?

I'm sending a revised usb-otg patchset today which depends on this series
and will break without this patch.

cheers,
-roger

> 
>>
>>> Also get rid of MPC_LOC which doesn't seem to be used
>>> by anyone.
>>>
>>
>> If you want, you can only delete MPC_LOC at this patch.
>>
>> Peter
>>
>>> [1] - warning fixed by this patch:
>>>
>>>    In file included from drivers/usb/dwc3/core.h:33,
>>>                     from drivers/usb/dwc3/ep0.c:33:
>>>    include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
>>>    In file included from drivers/usb/dwc3/ep0.c:31:
>>>    include/linux/usb/composite.h:615:1: warning: this is the location
>>> 	of the previous definition
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>> v7: define VDBG locally in phy-fsl-usb.c
>>>
>>>  drivers/usb/chipidea/otg_fsm.c   |  1 +
>>>  drivers/usb/common/usb-otg-fsm.c | 12 +++++++-----
>>>  drivers/usb/phy/phy-fsl-usb.c    |  8 ++++++++
>>>  include/linux/usb/otg-fsm.h      | 19 ++++---------------
>>>  4 files changed, 20 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
>>> index de8e22e..5f169b3 100644
>>> --- a/drivers/usb/chipidea/otg_fsm.c
>>> +++ b/drivers/usb/chipidea/otg_fsm.c
>>> @@ -805,6 +805,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
>>>  	ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
>>>  	ci->fsm.otg->state = OTG_STATE_UNDEFINED;
>>>  	ci->fsm.ops = &ci_otg_ops;
>>> +	ci->fsm.dev = ci->dev;
>>>  	ci->gadget.hnp_polling_support = 1;
>>>  	ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
>>>  	if (!ci->fsm.host_req_flag)
>>> diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
>>> index 9059b7d..c5a61fe 100644
>>> --- a/drivers/usb/common/usb-otg-fsm.c
>>> +++ b/drivers/usb/common/usb-otg-fsm.c
>>> @@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
>>>  	int ret = 0;
>>>  
>>>  	if (fsm->protocol != protocol) {
>>> -		VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
>>> -			fsm->protocol, protocol);
>>> +		dev_vdbg(fsm->dev,
>>> +			 "Changing role fsm->protocol= %d; new protocol= %d\n",
>>> +			 fsm->protocol, protocol);
>>>  		/* stop old protocol */
>>>  		if (fsm->protocol == PROTO_HOST)
>>>  			ret = otg_start_host(fsm, 0);
>>> @@ -208,7 +209,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
>>>  {
>>>  	if (fsm->otg->state == new_state)
>>>  		return 0;
>>> -	VDBG("Set state: %s\n", usb_otg_state_string(new_state));
>>> +	dev_vdbg(fsm->dev, "Set state: %s\n", usb_otg_state_string(new_state));
>>>  	otg_leave_state(fsm, fsm->otg->state);
>>>  	switch (new_state) {
>>>  	case OTG_STATE_B_IDLE:
>>> @@ -338,7 +339,7 @@ int otg_statemachine(struct otg_fsm *fsm)
>>>  
>>>  	switch (state) {
>>>  	case OTG_STATE_UNDEFINED:
>>> -		VDBG("fsm->id = %d\n", fsm->id);
>>> +		dev_vdbg(fsm->dev, "fsm->id = %d\n", fsm->id);
>>>  		if (fsm->id)
>>>  			otg_set_state(fsm, OTG_STATE_B_IDLE);
>>>  		else
>>> @@ -446,7 +447,8 @@ int otg_statemachine(struct otg_fsm *fsm)
>>>  	}
>>>  	mutex_unlock(&fsm->lock);
>>>  
>>> -	VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
>>> +	dev_vdbg(fsm->dev, "quit statemachine, changed = %d\n",
>>> +		 fsm->state_changed);
>>>  	return fsm->state_changed;
>>>  }
>>>  EXPORT_SYMBOL_GPL(otg_statemachine);
>>> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
>>> index 94eb292..c57ef5c 100644
>>> --- a/drivers/usb/phy/phy-fsl-usb.c
>>> +++ b/drivers/usb/phy/phy-fsl-usb.c
>>> @@ -44,6 +44,13 @@
>>>  
>>>  #include "phy-fsl-usb.h"
>>>  
>>> +#ifdef VERBOSE
>>> +#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
>>> +				    __func__, ## args)
>>> +#else
>>> +#define VDBG(stuff...)	do {} while (0)
>>> +#endif
>>> +
>>>  #define DRIVER_VERSION "Rev. 1.55"
>>>  #define DRIVER_AUTHOR "Jerry Huang/Li Yang"
>>>  #define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
>>> @@ -817,6 +824,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
>>>  
>>>  	/* Set OTG state machine operations */
>>>  	fsl_otg_tc->fsm.ops = &fsl_otg_ops;
>>> +	fsl_otg_tc->fsm.dev = &pdev->dev;
>>>  
>>>  	/* initialize the otg structure */
>>>  	fsl_otg_tc->phy.label = DRIVER_DESC;
>>> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
>>> index 7a03505..47b8392 100644
>>> --- a/include/linux/usb/otg-fsm.h
>>> +++ b/include/linux/usb/otg-fsm.h
>>> @@ -18,24 +18,10 @@
>>>  #ifndef __LINUX_USB_OTG_FSM_H
>>>  #define __LINUX_USB_OTG_FSM_H
>>>  
>>> +#include <linux/device.h>
>>>  #include <linux/mutex.h>
>>>  #include <linux/errno.h>
>>>  
>>> -#undef VERBOSE
>>> -
>>> -#ifdef VERBOSE
>>> -#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
>>> -				 __func__, ## args)
>>> -#else
>>> -#define VDBG(stuff...)	do {} while (0)
>>> -#endif
>>> -
>>> -#ifdef VERBOSE
>>> -#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
>>> -#else
>>> -#define MPC_LOC do {} while (0)
>>> -#endif
>>> -
>>>  #define PROTO_UNDEF	(0)
>>>  #define PROTO_HOST	(1)
>>>  #define PROTO_GADGET	(2)
>>> @@ -211,6 +197,9 @@ struct otg_fsm {
>>>  	u8 *host_req_flag;
>>>  	struct delayed_work hnp_polling_work;
>>>  	bool state_changed;
>>> +
>>> +	/* for debug prints */
>>> +	struct device *dev;
>>>  };
>>>  
>>>  struct otg_fsm_ops {
>>> -- 
>>> 2.5.0
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG" redefined

[Peter Chen]

On Tue, Apr 05, 2016 at 04:48:19PM +0300, Roger Quadros wrote:
> Peter,
> 
> On 05/04/16 15:52, Roger Quadros wrote:
> > Peter,
> > 
> > On 05/04/16 11:52, Peter Chen wrote:
> >> On Thu, Mar 31, 2016 at 12:41:19PM +0300, Roger Quadros wrote:
> >>> If usb/otg-fsm.h and usb/composite.h are included together
> >>> then it results in the build warning [1].
> >>>
> >>> Prevent that by using dev_vdbg() instead.
> >>>
> >>
> >> After considering it more, I think it may not be a good solution
> >> that we delete VDBG at one header file, but keep it at another
> >> one. In future, we may add VDBG at another file, and cause the
> >> same problem. In fact, I find VDBG is defined at several files
> >> in USB folder (and only at USB folder), I plan to replace them
> >> with standard one (dev_vdbg) together.
> > 
> > OK, please ignore this patch then.
> 
> On second thoughts can you please retain this patch and post the
> VDBG removal from composite.h cleanup separately?
> 

I find the struct usb_otg has a struct device pointer, and you changes
all fsm stuffs under struct usb_otg (like otg.fsm) in your later patches,
then, would you please refine this patch that just using otg->dev for
print and move VDBG to phy-fsl-usb.c, of course, you need to move this
patch in that patch series.

Peter

> I'm sending a revised usb-otg patchset today which depends on this series
> and will break without this patch.
> 
> cheers,
> -roger
> 
> > 
> >>
> >>> Also get rid of MPC_LOC which doesn't seem to be used
> >>> by anyone.
> >>>
> >>
> >> If you want, you can only delete MPC_LOC at this patch.
> >>
> >> Peter
> >>
> >>> [1] - warning fixed by this patch:
> >>>
> >>>    In file included from drivers/usb/dwc3/core.h:33,
> >>>                     from drivers/usb/dwc3/ep0.c:33:
> >>>    include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
> >>>    In file included from drivers/usb/dwc3/ep0.c:31:
> >>>    include/linux/usb/composite.h:615:1: warning: this is the location
> >>> 	of the previous definition
> >>>
> >>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >>> ---
> >>> v7: define VDBG locally in phy-fsl-usb.c
> >>>
> >>>  drivers/usb/chipidea/otg_fsm.c   |  1 +
> >>>  drivers/usb/common/usb-otg-fsm.c | 12 +++++++-----
> >>>  drivers/usb/phy/phy-fsl-usb.c    |  8 ++++++++
> >>>  include/linux/usb/otg-fsm.h      | 19 ++++---------------
> >>>  4 files changed, 20 insertions(+), 20 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> >>> index de8e22e..5f169b3 100644
> >>> --- a/drivers/usb/chipidea/otg_fsm.c
> >>> +++ b/drivers/usb/chipidea/otg_fsm.c
> >>> @@ -805,6 +805,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
> >>>  	ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
> >>>  	ci->fsm.otg->state = OTG_STATE_UNDEFINED;
> >>>  	ci->fsm.ops = &ci_otg_ops;
> >>> +	ci->fsm.dev = ci->dev;
> >>>  	ci->gadget.hnp_polling_support = 1;
> >>>  	ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
> >>>  	if (!ci->fsm.host_req_flag)
> >>> diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
> >>> index 9059b7d..c5a61fe 100644
> >>> --- a/drivers/usb/common/usb-otg-fsm.c
> >>> +++ b/drivers/usb/common/usb-otg-fsm.c
> >>> @@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
> >>>  	int ret = 0;
> >>>  
> >>>  	if (fsm->protocol != protocol) {
> >>> -		VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
> >>> -			fsm->protocol, protocol);
> >>> +		dev_vdbg(fsm->dev,
> >>> +			 "Changing role fsm->protocol= %d; new protocol= %d\n",
> >>> +			 fsm->protocol, protocol);
> >>>  		/* stop old protocol */
> >>>  		if (fsm->protocol == PROTO_HOST)
> >>>  			ret = otg_start_host(fsm, 0);
> >>> @@ -208,7 +209,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
> >>>  {
> >>>  	if (fsm->otg->state == new_state)
> >>>  		return 0;
> >>> -	VDBG("Set state: %s\n", usb_otg_state_string(new_state));
> >>> +	dev_vdbg(fsm->dev, "Set state: %s\n", usb_otg_state_string(new_state));
> >>>  	otg_leave_state(fsm, fsm->otg->state);
> >>>  	switch (new_state) {
> >>>  	case OTG_STATE_B_IDLE:
> >>> @@ -338,7 +339,7 @@ int otg_statemachine(struct otg_fsm *fsm)
> >>>  
> >>>  	switch (state) {
> >>>  	case OTG_STATE_UNDEFINED:
> >>> -		VDBG("fsm->id = %d\n", fsm->id);
> >>> +		dev_vdbg(fsm->dev, "fsm->id = %d\n", fsm->id);
> >>>  		if (fsm->id)
> >>>  			otg_set_state(fsm, OTG_STATE_B_IDLE);
> >>>  		else
> >>> @@ -446,7 +447,8 @@ int otg_statemachine(struct otg_fsm *fsm)
> >>>  	}
> >>>  	mutex_unlock(&fsm->lock);
> >>>  
> >>> -	VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
> >>> +	dev_vdbg(fsm->dev, "quit statemachine, changed = %d\n",
> >>> +		 fsm->state_changed);
> >>>  	return fsm->state_changed;
> >>>  }
> >>>  EXPORT_SYMBOL_GPL(otg_statemachine);
> >>> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
> >>> index 94eb292..c57ef5c 100644
> >>> --- a/drivers/usb/phy/phy-fsl-usb.c
> >>> +++ b/drivers/usb/phy/phy-fsl-usb.c
> >>> @@ -44,6 +44,13 @@
> >>>  
> >>>  #include "phy-fsl-usb.h"
> >>>  
> >>> +#ifdef VERBOSE
> >>> +#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
> >>> +				    __func__, ## args)
> >>> +#else
> >>> +#define VDBG(stuff...)	do {} while (0)
> >>> +#endif
> >>> +
> >>>  #define DRIVER_VERSION "Rev. 1.55"
> >>>  #define DRIVER_AUTHOR "Jerry Huang/Li Yang"
> >>>  #define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
> >>> @@ -817,6 +824,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
> >>>  
> >>>  	/* Set OTG state machine operations */
> >>>  	fsl_otg_tc->fsm.ops = &fsl_otg_ops;
> >>> +	fsl_otg_tc->fsm.dev = &pdev->dev;
> >>>  
> >>>  	/* initialize the otg structure */
> >>>  	fsl_otg_tc->phy.label = DRIVER_DESC;
> >>> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
> >>> index 7a03505..47b8392 100644
> >>> --- a/include/linux/usb/otg-fsm.h
> >>> +++ b/include/linux/usb/otg-fsm.h
> >>> @@ -18,24 +18,10 @@
> >>>  #ifndef __LINUX_USB_OTG_FSM_H
> >>>  #define __LINUX_USB_OTG_FSM_H
> >>>  
> >>> +#include <linux/device.h>
> >>>  #include <linux/mutex.h>
> >>>  #include <linux/errno.h>
> >>>  
> >>> -#undef VERBOSE
> >>> -
> >>> -#ifdef VERBOSE
> >>> -#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
> >>> -				 __func__, ## args)
> >>> -#else
> >>> -#define VDBG(stuff...)	do {} while (0)
> >>> -#endif
> >>> -
> >>> -#ifdef VERBOSE
> >>> -#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
> >>> -#else
> >>> -#define MPC_LOC do {} while (0)
> >>> -#endif
> >>> -
> >>>  #define PROTO_UNDEF	(0)
> >>>  #define PROTO_HOST	(1)
> >>>  #define PROTO_GADGET	(2)
> >>> @@ -211,6 +197,9 @@ struct otg_fsm {
> >>>  	u8 *host_req_flag;
> >>>  	struct delayed_work hnp_polling_work;
> >>>  	bool state_changed;
> >>> +
> >>> +	/* for debug prints */
> >>> +	struct device *dev;
> >>>  };
> >>>  
> >>>  struct otg_fsm_ops {
> >>> -- 
> >>> 2.5.0
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

-- 

Best Regards,
Peter Chen

Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG" redefined

[Roger Quadros]

On 06/04/16 09:46, Peter Chen wrote:
> On Tue, Apr 05, 2016 at 04:48:19PM +0300, Roger Quadros wrote:
>> Peter,
>>
>> On 05/04/16 15:52, Roger Quadros wrote:
>>> Peter,
>>>
>>> On 05/04/16 11:52, Peter Chen wrote:
>>>> On Thu, Mar 31, 2016 at 12:41:19PM +0300, Roger Quadros wrote:
>>>>> If usb/otg-fsm.h and usb/composite.h are included together
>>>>> then it results in the build warning [1].
>>>>>
>>>>> Prevent that by using dev_vdbg() instead.
>>>>>
>>>>
>>>> After considering it more, I think it may not be a good solution
>>>> that we delete VDBG at one header file, but keep it at another
>>>> one. In future, we may add VDBG at another file, and cause the
>>>> same problem. In fact, I find VDBG is defined at several files
>>>> in USB folder (and only at USB folder), I plan to replace them
>>>> with standard one (dev_vdbg) together.
>>>
>>> OK, please ignore this patch then.
>>
>> On second thoughts can you please retain this patch and post the
>> VDBG removal from composite.h cleanup separately?
>>
> 
> I find the struct usb_otg has a struct device pointer, and you changes
> all fsm stuffs under struct usb_otg (like otg.fsm) in your later patches,
> then, would you please refine this patch that just using otg->dev for
> print and move VDBG to phy-fsl-usb.c, of course, you need to move this
> patch in that patch series.

OK. I'll rework this patch and include it in the otg series.

cheers,
-roger

> 
> Peter
> 
>> I'm sending a revised usb-otg patchset today which depends on this series
>> and will break without this patch.
>>
>> cheers,
>> -roger
>>
>>>
>>>>
>>>>> Also get rid of MPC_LOC which doesn't seem to be used
>>>>> by anyone.
>>>>>
>>>>
>>>> If you want, you can only delete MPC_LOC at this patch.
>>>>
>>>> Peter
>>>>
>>>>> [1] - warning fixed by this patch:
>>>>>
>>>>>    In file included from drivers/usb/dwc3/core.h:33,
>>>>>                     from drivers/usb/dwc3/ep0.c:33:
>>>>>    include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
>>>>>    In file included from drivers/usb/dwc3/ep0.c:31:
>>>>>    include/linux/usb/composite.h:615:1: warning: this is the location
>>>>> 	of the previous definition
>>>>>
>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>> ---
>>>>> v7: define VDBG locally in phy-fsl-usb.c
>>>>>
>>>>>  drivers/usb/chipidea/otg_fsm.c   |  1 +
>>>>>  drivers/usb/common/usb-otg-fsm.c | 12 +++++++-----
>>>>>  drivers/usb/phy/phy-fsl-usb.c    |  8 ++++++++
>>>>>  include/linux/usb/otg-fsm.h      | 19 ++++---------------
>>>>>  4 files changed, 20 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
>>>>> index de8e22e..5f169b3 100644
>>>>> --- a/drivers/usb/chipidea/otg_fsm.c
>>>>> +++ b/drivers/usb/chipidea/otg_fsm.c
>>>>> @@ -805,6 +805,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
>>>>>  	ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
>>>>>  	ci->fsm.otg->state = OTG_STATE_UNDEFINED;
>>>>>  	ci->fsm.ops = &ci_otg_ops;
>>>>> +	ci->fsm.dev = ci->dev;
>>>>>  	ci->gadget.hnp_polling_support = 1;
>>>>>  	ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
>>>>>  	if (!ci->fsm.host_req_flag)
>>>>> diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
>>>>> index 9059b7d..c5a61fe 100644
>>>>> --- a/drivers/usb/common/usb-otg-fsm.c
>>>>> +++ b/drivers/usb/common/usb-otg-fsm.c
>>>>> @@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
>>>>>  	int ret = 0;
>>>>>  
>>>>>  	if (fsm->protocol != protocol) {
>>>>> -		VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
>>>>> -			fsm->protocol, protocol);
>>>>> +		dev_vdbg(fsm->dev,
>>>>> +			 "Changing role fsm->protocol= %d; new protocol= %d\n",
>>>>> +			 fsm->protocol, protocol);
>>>>>  		/* stop old protocol */
>>>>>  		if (fsm->protocol == PROTO_HOST)
>>>>>  			ret = otg_start_host(fsm, 0);
>>>>> @@ -208,7 +209,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
>>>>>  {
>>>>>  	if (fsm->otg->state == new_state)
>>>>>  		return 0;
>>>>> -	VDBG("Set state: %s\n", usb_otg_state_string(new_state));
>>>>> +	dev_vdbg(fsm->dev, "Set state: %s\n", usb_otg_state_string(new_state));
>>>>>  	otg_leave_state(fsm, fsm->otg->state);
>>>>>  	switch (new_state) {
>>>>>  	case OTG_STATE_B_IDLE:
>>>>> @@ -338,7 +339,7 @@ int otg_statemachine(struct otg_fsm *fsm)
>>>>>  
>>>>>  	switch (state) {
>>>>>  	case OTG_STATE_UNDEFINED:
>>>>> -		VDBG("fsm->id = %d\n", fsm->id);
>>>>> +		dev_vdbg(fsm->dev, "fsm->id = %d\n", fsm->id);
>>>>>  		if (fsm->id)
>>>>>  			otg_set_state(fsm, OTG_STATE_B_IDLE);
>>>>>  		else
>>>>> @@ -446,7 +447,8 @@ int otg_statemachine(struct otg_fsm *fsm)
>>>>>  	}
>>>>>  	mutex_unlock(&fsm->lock);
>>>>>  
>>>>> -	VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
>>>>> +	dev_vdbg(fsm->dev, "quit statemachine, changed = %d\n",
>>>>> +		 fsm->state_changed);
>>>>>  	return fsm->state_changed;
>>>>>  }
>>>>>  EXPORT_SYMBOL_GPL(otg_statemachine);
>>>>> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
>>>>> index 94eb292..c57ef5c 100644
>>>>> --- a/drivers/usb/phy/phy-fsl-usb.c
>>>>> +++ b/drivers/usb/phy/phy-fsl-usb.c
>>>>> @@ -44,6 +44,13 @@
>>>>>  
>>>>>  #include "phy-fsl-usb.h"
>>>>>  
>>>>> +#ifdef VERBOSE
>>>>> +#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
>>>>> +				    __func__, ## args)
>>>>> +#else
>>>>> +#define VDBG(stuff...)	do {} while (0)
>>>>> +#endif
>>>>> +
>>>>>  #define DRIVER_VERSION "Rev. 1.55"
>>>>>  #define DRIVER_AUTHOR "Jerry Huang/Li Yang"
>>>>>  #define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
>>>>> @@ -817,6 +824,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
>>>>>  
>>>>>  	/* Set OTG state machine operations */
>>>>>  	fsl_otg_tc->fsm.ops = &fsl_otg_ops;
>>>>> +	fsl_otg_tc->fsm.dev = &pdev->dev;
>>>>>  
>>>>>  	/* initialize the otg structure */
>>>>>  	fsl_otg_tc->phy.label = DRIVER_DESC;
>>>>> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
>>>>> index 7a03505..47b8392 100644
>>>>> --- a/include/linux/usb/otg-fsm.h
>>>>> +++ b/include/linux/usb/otg-fsm.h
>>>>> @@ -18,24 +18,10 @@
>>>>>  #ifndef __LINUX_USB_OTG_FSM_H
>>>>>  #define __LINUX_USB_OTG_FSM_H
>>>>>  
>>>>> +#include <linux/device.h>
>>>>>  #include <linux/mutex.h>
>>>>>  #include <linux/errno.h>
>>>>>  
>>>>> -#undef VERBOSE
>>>>> -
>>>>> -#ifdef VERBOSE
>>>>> -#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
>>>>> -				 __func__, ## args)
>>>>> -#else
>>>>> -#define VDBG(stuff...)	do {} while (0)
>>>>> -#endif
>>>>> -
>>>>> -#ifdef VERBOSE
>>>>> -#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
>>>>> -#else
>>>>> -#define MPC_LOC do {} while (0)
>>>>> -#endif
>>>>> -
>>>>>  #define PROTO_UNDEF	(0)
>>>>>  #define PROTO_HOST	(1)
>>>>>  #define PROTO_GADGET	(2)
>>>>> @@ -211,6 +197,9 @@ struct otg_fsm {
>>>>>  	u8 *host_req_flag;
>>>>>  	struct delayed_work hnp_polling_work;
>>>>>  	bool state_changed;
>>>>> +
>>>>> +	/* for debug prints */
>>>>> +	struct device *dev;
>>>>>  };
>>>>>  
>>>>>  struct otg_fsm_ops {
>>>>> -- 
>>>>> 2.5.0
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
> 

Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG" redefined

[Peter Chen]

On Wed, Apr 06, 2016 at 10:13:52AM +0300, Roger Quadros wrote:
> On 06/04/16 09:46, Peter Chen wrote:
> > On Tue, Apr 05, 2016 at 04:48:19PM +0300, Roger Quadros wrote:
> >> Peter,
> >>
> >> On 05/04/16 15:52, Roger Quadros wrote:
> >>> Peter,
> >>>
> >>> On 05/04/16 11:52, Peter Chen wrote:
> >>>> On Thu, Mar 31, 2016 at 12:41:19PM +0300, Roger Quadros wrote:
> >>>>> If usb/otg-fsm.h and usb/composite.h are included together
> >>>>> then it results in the build warning [1].
> >>>>>
> >>>>> Prevent that by using dev_vdbg() instead.
> >>>>>
> >>>>
> >>>> After considering it more, I think it may not be a good solution
> >>>> that we delete VDBG at one header file, but keep it at another
> >>>> one. In future, we may add VDBG at another file, and cause the
> >>>> same problem. In fact, I find VDBG is defined at several files
> >>>> in USB folder (and only at USB folder), I plan to replace them
> >>>> with standard one (dev_vdbg) together.
> >>>
> >>> OK, please ignore this patch then.
> >>
> >> On second thoughts can you please retain this patch and post the
> >> VDBG removal from composite.h cleanup separately?
> >>
> > 
> > I find the struct usb_otg has a struct device pointer, and you changes
> > all fsm stuffs under struct usb_otg (like otg.fsm) in your later patches,
> > then, would you please refine this patch that just using otg->dev for
> > print and move VDBG to phy-fsl-usb.c, of course, you need to move this
> > patch in that patch series.
> 
> OK. I'll rework this patch and include it in the otg series.
> 

I have already queued your 1st and 2nd patch at my tree.

-- 

Best Regards,
Peter Chen

Re: [PATCH v6 0/3] usb: otg-fsm: Add documentation and some trivial cleanups

[Peter Chen]

On Wed, Mar 30, 2016 at 12:56:27PM +0300, Roger Quadros wrote:
> Hi,
> 
> Add documentation for struct otg_fsm with some trivial cleanups.
> All patches have been Acked by otg-fsm maintainer (Peter Chen).
> 

I will queue them, thanks.

-- 
Best Regards,
Peter Chen

Re: [PATCH v6 0/3] usb: otg-fsm: Add documentation and some trivial cleanups

[Roger Quadros]

Peter,

On 31/03/16 10:08, Peter Chen wrote:
> On Wed, Mar 30, 2016 at 12:56:27PM +0300, Roger Quadros wrote:
>> Hi,
>>
>> Add documentation for struct otg_fsm with some trivial cleanups.
>> All patches have been Acked by otg-fsm maintainer (Peter Chen).
>>
> 
> I will queue them, thanks.
> 
Patch 3 might cause a build failure for drivers/usb/phy/phy-fsl-usb.c
since it relies on VDBG that was defined in include/linux/usb/otg-fsm.h

I will send a revised patch 3 that locally defines VDBG for phy-fsl-usb.c

cheers,
-roger