From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751827AbdJ3NRi (ORCPT ); Mon, 30 Oct 2017 09:17:38 -0400 Received: from lelnx194.ext.ti.com ([198.47.27.80]:22157 "EHLO lelnx194.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751269AbdJ3NRg (ORCPT ); Mon, 30 Oct 2017 09:17:36 -0400 Date: Mon, 30 Oct 2017 08:17:35 -0500 From: Bin Liu To: Greg Kroah-Hartman CC: Kees Cook , , Subject: Re: [PATCH] usb: musb: Convert timers to use timer_setup() Message-ID: <20171030131735.GA3964@uda0271908> Mail-Followup-To: Bin Liu , Greg Kroah-Hartman , Kees Cook , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org References: <20171024100835.GA69097@beast> <20171027162422.GA65@LTA0271908.dhcp.ti.com> <20171030085215.GB28997@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20171030085215.GB28997@kroah.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Oct 30, 2017 at 09:52:15AM +0100, Greg Kroah-Hartman wrote: > On Fri, Oct 27, 2017 at 11:24:22AM -0500, Bin Liu wrote: > > Hi, > > > > On Tue, Oct 24, 2017 at 03:08:35AM -0700, Kees Cook wrote: > > > In preparation for unconditionally passing the struct timer_list pointer to > > > all timer callbacks, switch to using the new timer_setup() and from_timer() > > > to pass the timer pointer explicitly. > > > > > > Instead of a per-device static timer variable, a spare timer "dev_timer" > > > is added to the musb structure for devices to use for their per-device > > > timer. > > > > > > Cc: Bin Liu > > > Cc: Greg Kroah-Hartman > > > Cc: linux-usb@vger.kernel.org > > > Signed-off-by: Kees Cook > > > --- > > > drivers/usb/musb/am35x.c | 24 +++++++++++------------- > > > drivers/usb/musb/blackfin.c | 13 ++++++------- > > > drivers/usb/musb/blackfin.h | 2 -- > > > drivers/usb/musb/da8xx.c | 26 ++++++++++++-------------- > > > drivers/usb/musb/davinci.c | 20 +++++++++----------- > > > drivers/usb/musb/musb_core.c | 6 +++--- > > > drivers/usb/musb/musb_core.h | 1 + > > > drivers/usb/musb/musb_dsps.c | 7 ++++--- > > > drivers/usb/musb/tusb6010.c | 20 +++++++++----------- > > > 9 files changed, 55 insertions(+), 64 deletions(-) > > > > [snip] > > > > > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > > > index 20f4614178d9..e8573975743d 100644 > > > --- a/drivers/usb/musb/musb_core.h > > > +++ b/drivers/usb/musb/musb_core.h > > > @@ -345,6 +345,7 @@ struct musb { > > > struct list_head pending_list; /* pending work list */ > > > > > > struct timer_list otg_timer; > > > + struct timer_list dev_timer; > > > > Now since dev_timer is added in struct musb for glue drivers, > > > > > struct notifier_block nb; > > > > > > struct dma_controller *dma_controller; > > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > > > index f6b526606ad1..e3d0e626a5d6 100644 > > > --- a/drivers/usb/musb/musb_dsps.c > > > +++ b/drivers/usb/musb/musb_dsps.c > > > @@ -282,9 +282,10 @@ static int dsps_check_status(struct musb *musb, void *unused) > > > return 0; > > > } > > > > > > -static void otg_timer(unsigned long _musb) > > > +static void otg_timer(struct timer_list *t) > > > { > > > - struct musb *musb = (void *)_musb; > > > + struct dsps_glue *glue = from_timer(glue, t, timer); > > > + struct musb *musb = platform_get_drvdata(glue->musb); > > > struct device *dev = musb->controller; > > > unsigned long flags; > > > int err; > > > @@ -480,7 +481,7 @@ static int dsps_musb_init(struct musb *musb) > > > } > > > } > > > > > > - setup_timer(&glue->timer, otg_timer, (unsigned long) musb); > > > + timer_setup(&glue->timer, otg_timer, 0); > > > > this glue->timer is duplicate. It can be removed and use musb->dev_timer > > instead as other glue drivers do. > > I do not understand your review comments at all. Can you do the fixup > here to use timer_setup for the musb timer code? I meant something like diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index f6b526606ad1..8a2abe50f2d4 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -119,7 +119,6 @@ struct dsps_glue { struct platform_device *musb; /* child musb pdev */ const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */ int vbus_irq; /* optional vbus irq */ - struct timer_list timer; /* otg_workaround timer */ unsigned long last_timer; /* last timer data for each instance */ bool sw_babble_enabled; void __iomem *usbss_base; @@ -480,7 +479,7 @@ static int dsps_musb_init(struct musb *musb) } } - setup_timer(&glue->timer, otg_timer, (unsigned long) musb); + timer_setup(&musb->dev_timer, otg_timer, 0); /* Reset the musb */ musb_writel(reg_base, wrp->control, (1 << wrp->reset)); which removes the duplicated timer from dsps_glue, and uses the newly added dev_timer in struct musb. I will send the fixup patch after I tested it. Thanks, -Bin.