From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754260AbcGHJWc (ORCPT ); Fri, 8 Jul 2016 05:22:32 -0400 Received: from mail-pa0-f67.google.com ([209.85.220.67]:36601 "EHLO mail-pa0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754049AbcGHJW0 (ORCPT ); Fri, 8 Jul 2016 05:22:26 -0400 Date: Fri, 8 Jul 2016 17:14:57 +0800 From: Peter Chen To: Stephen Boyd Cc: linux-usb@vger.kernel.org, Felipe Balbi , Arnd Bergmann , Neil Armstrong , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, Bjorn Andersson , Peter Chen , Greg Kroah-Hartman , Andy Gross , "Ivan T. Ivanov" , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 04/22] usb: chipidea: Only read/write OTGSC from one place Message-ID: <20160708091456.GC20485@shlinux2> References: <20160707222114.1673-1-stephen.boyd@linaro.org> <20160707222114.1673-5-stephen.boyd@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160707222114.1673-5-stephen.boyd@linaro.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 07, 2016 at 03:20:55PM -0700, Stephen Boyd wrote: > With the id and vbus detection done via extcon we need to make > sure we poll the status of OTGSC properly by considering what the > extcon is saying, and not just what the register is saying. Let's > move this hw_wait_reg() function to the only place it's used and > simplify it for polling the OTGSC register. Then we can make > certain we only use the hw_read_otgsc() API to read OTGSC, which > will make sure we properly handle extcon events. > > Cc: Peter Chen > Cc: Greg Kroah-Hartman > Cc: "Ivan T. Ivanov" > Fixes: 3ecb3e09b042 ("usb: chipidea: Use extcon framework for VBUS and ID detect") > Signed-off-by: Stephen Boyd > --- > drivers/usb/chipidea/ci.h | 3 --- > drivers/usb/chipidea/core.c | 32 -------------------------------- > drivers/usb/chipidea/otg.c | 34 ++++++++++++++++++++++++++++++---- > 3 files changed, 30 insertions(+), 39 deletions(-) > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > index cd414559040f..05bc4d631cb9 100644 > --- a/drivers/usb/chipidea/ci.h > +++ b/drivers/usb/chipidea/ci.h > @@ -428,9 +428,6 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode); > > u8 hw_port_test_get(struct ci_hdrc *ci); > > -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask, > - u32 value, unsigned int timeout_ms); > - > void ci_platform_configure(struct ci_hdrc *ci); > > int dbg_create_files(struct ci_hdrc *ci); > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > index 69426e644d17..01390e02ee53 100644 > --- a/drivers/usb/chipidea/core.c > +++ b/drivers/usb/chipidea/core.c > @@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci) > return 0; > } > > -/** > - * hw_wait_reg: wait the register value > - * > - * Sometimes, it needs to wait register value before going on. > - * Eg, when switch to device mode, the vbus value should be lower > - * than OTGSC_BSV before connects to host. > - * > - * @ci: the controller > - * @reg: register index > - * @mask: mast bit > - * @value: the bit value to wait > - * @timeout_ms: timeout in millisecond > - * > - * This function returns an error code if timeout > - */ > -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask, > - u32 value, unsigned int timeout_ms) > -{ > - unsigned long elapse = jiffies + msecs_to_jiffies(timeout_ms); > - > - while (hw_read(ci, reg, mask) != value) { > - if (time_after(jiffies, elapse)) { > - dev_err(ci->dev, "timeout waiting for %08x in %d\n", > - mask, reg); > - return -ETIMEDOUT; > - } > - msleep(20); > - } > - > - return 0; > -} > - > static irqreturn_t ci_irq(int irq, void *data) > { > struct ci_hdrc *ci = data; > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c > index 03b6743461d1..a6fc60934297 100644 > --- a/drivers/usb/chipidea/otg.c > +++ b/drivers/usb/chipidea/otg.c > @@ -104,7 +104,31 @@ void ci_handle_vbus_change(struct ci_hdrc *ci) > usb_gadget_vbus_disconnect(&ci->gadget); > } > > -#define CI_VBUS_STABLE_TIMEOUT_MS 5000 > +/** > + * When we switch to device mode, the vbus value should be lower > + * than OTGSC_BSV before connecting to host. > + * > + * @ci: the controller > + * > + * This function returns an error code if timeout > + */ > +static int hw_wait_otgsc_bsv(struct ci_hdrc *ci) I think the function name should reflect "we wait for vbus lower than bsv" Care to change one? -- Best Regards, Peter Chen