lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 6 Apr 2016 10:13:52 +0300
From:	Roger Quadros <rogerq@...com>
To:	Peter Chen <hzpeterchen@...il.com>
CC:	<Peter.Chen@....com>, <balbi@...nel.org>,
	<gregkh@...uxfoundation.org>, <linux-usb@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 3/3] usb: otg-fsm: Prevent build warning "VDBG"
 redefined

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@...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@...r.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@...r.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ