Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1371329 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG"  redefined Peter Chen <hzpeterchen@gmail.com> - 2016-04-05 11:00 +0200
    Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG"  redefined Roger Quadros <rogerq@ti.com> - 2016-04-05 15:00 +0200
      Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG"  redefined Roger Quadros <rogerq@ti.com> - 2016-04-05 15:50 +0200
        Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG"  redefined Peter Chen <hzpeterchen@gmail.com> - 2016-04-06 09:00 +0200
          Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG"  redefined Roger Quadros <rogerq@ti.com> - 2016-04-06 09:20 +0200

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

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

[toc] | [next] | [standalone]

#1371536

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
> 

[toc] | [prev] | [next] | [standalone]

#1371603

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
> 

[toc] | [prev] | [next] | [standalone]

#1372232

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

[toc] | [prev] | [next] | [standalone]

#1372250

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
>>>
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web