From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752674AbdJ3IwE (ORCPT ); Mon, 30 Oct 2017 04:52:04 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:39692 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752600AbdJ3IwD (ORCPT ); Mon, 30 Oct 2017 04:52:03 -0400 Date: Mon, 30 Oct 2017 09:52:15 +0100 From: Greg Kroah-Hartman To: Bin Liu , Kees Cook , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] usb: musb: Convert timers to use timer_setup() Message-ID: <20171030085215.GB28997@kroah.com> References: <20171024100835.GA69097@beast> <20171027162422.GA65@LTA0271908.dhcp.ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171027162422.GA65@LTA0271908.dhcp.ti.com> User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? thanks, greg k-h