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

Peter Chen
2016-04-06T06:53:22Z

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