* [PATCH] usb: musb: Convert timers to use timer_setup()
@ 2017-10-24 10:08 Kees Cook
2017-10-27 16:24 ` Bin Liu
0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2017-10-24 10:08 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Bin Liu, linux-usb, linux-kernel
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 <b-liu@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
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(-)
diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index 02fbb4fe3745..2cf990e85bb2 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -133,11 +133,9 @@ static void am35x_musb_set_vbus(struct musb *musb, int is_on)
#define POLL_SECONDS 2
-static struct timer_list otg_workaround;
-
-static void otg_timer(unsigned long _musb)
+static void otg_timer(struct timer_list *t)
{
- struct musb *musb = (void *)_musb;
+ struct musb *musb = from_timer(musb, t, dev_timer);
void __iomem *mregs = musb->mregs;
u8 devctl;
unsigned long flags;
@@ -173,7 +171,7 @@ static void otg_timer(unsigned long _musb)
case OTG_STATE_B_IDLE:
devctl = musb_readb(mregs, MUSB_DEVCTL);
if (devctl & MUSB_DEVCTL_BDEVICE)
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
else
musb->xceiv->otg->state = OTG_STATE_A_IDLE;
break;
@@ -195,12 +193,12 @@ static void am35x_musb_try_idle(struct musb *musb, unsigned long timeout)
musb->xceiv->otg->state == OTG_STATE_A_WAIT_BCON)) {
dev_dbg(musb->controller, "%s active, deleting timer\n",
usb_otg_state_string(musb->xceiv->otg->state));
- del_timer(&otg_workaround);
+ del_timer(&musb->dev_timer);
last_timer = jiffies;
return;
}
- if (time_after(last_timer, timeout) && timer_pending(&otg_workaround)) {
+ if (time_after(last_timer, timeout) && timer_pending(&musb->dev_timer)) {
dev_dbg(musb->controller, "Longer idle timer already pending, ignoring...\n");
return;
}
@@ -209,7 +207,7 @@ static void am35x_musb_try_idle(struct musb *musb, unsigned long timeout)
dev_dbg(musb->controller, "%s inactive, starting idle timer for %u ms\n",
usb_otg_state_string(musb->xceiv->otg->state),
jiffies_to_msecs(timeout - jiffies));
- mod_timer(&otg_workaround, timeout);
+ mod_timer(&musb->dev_timer, timeout);
}
static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
@@ -278,14 +276,14 @@ static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
*/
musb->int_usb &= ~MUSB_INTR_VBUSERROR;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VFALL;
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
- del_timer(&otg_workaround);
+ del_timer(&musb->dev_timer);
} else {
musb->is_active = 0;
MUSB_DEV_MODE(musb);
@@ -324,7 +322,7 @@ static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
/* Poll for ID change */
if (musb->xceiv->otg->state == OTG_STATE_B_IDLE)
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
spin_unlock_irqrestore(&musb->lock, flags);
@@ -365,7 +363,7 @@ static int am35x_musb_init(struct musb *musb)
if (IS_ERR_OR_NULL(musb->xceiv))
return -EPROBE_DEFER;
- setup_timer(&otg_workaround, otg_timer, (unsigned long) musb);
+ timer_setup(&musb->dev_timer, otg_timer, 0);
/* Reset the musb */
if (data->reset)
@@ -395,7 +393,7 @@ static int am35x_musb_exit(struct musb *musb)
struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
struct omap_musb_board_data *data = plat->board_data;
- del_timer_sync(&otg_workaround);
+ del_timer_sync(&musb->dev_timer);
/* Shutdown the on-chip PHY and its PLL. */
if (data->set_phy_power)
diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
index 4418574a36a1..7c580df75dc1 100644
--- a/drivers/usb/musb/blackfin.c
+++ b/drivers/usb/musb/blackfin.c
@@ -223,7 +223,7 @@ static irqreturn_t blackfin_interrupt(int irq, void *__hci)
if ((musb->xceiv->otg->state == OTG_STATE_B_IDLE
|| musb->xceiv->otg->state == OTG_STATE_A_WAIT_BCON) ||
(musb->int_usb & MUSB_INTR_DISCONNECT && is_host_active(musb))) {
- mod_timer(&musb_conn_timer, jiffies + TIMER_DELAY);
+ mod_timer(&musb->dev_timer, jiffies + TIMER_DELAY);
musb->a_wait_bcon = TIMER_DELAY;
}
@@ -232,9 +232,9 @@ static irqreturn_t blackfin_interrupt(int irq, void *__hci)
return retval;
}
-static void musb_conn_timer_handler(unsigned long _musb)
+static void musb_conn_timer_handler(struct timer_list *t)
{
- struct musb *musb = (void *)_musb;
+ struct musb *musb = from_timer(musb, t, dev_timer);
unsigned long flags;
u16 val;
static u8 toggle;
@@ -266,7 +266,7 @@ static void musb_conn_timer_handler(unsigned long _musb)
musb_writeb(musb->mregs, MUSB_INTRUSB, val);
musb->xceiv->otg->state = OTG_STATE_B_IDLE;
}
- mod_timer(&musb_conn_timer, jiffies + TIMER_DELAY);
+ mod_timer(&musb->dev_timer, jiffies + TIMER_DELAY);
break;
case OTG_STATE_B_IDLE:
/*
@@ -310,7 +310,7 @@ static void musb_conn_timer_handler(unsigned long _musb)
* shortening it, if accelerating A-plug detection
* is needed in OTG mode.
*/
- mod_timer(&musb_conn_timer, jiffies + TIMER_DELAY / 4);
+ mod_timer(&musb->dev_timer, jiffies + TIMER_DELAY / 4);
}
break;
default:
@@ -445,8 +445,7 @@ static int bfin_musb_init(struct musb *musb)
bfin_musb_reg_init(musb);
- setup_timer(&musb_conn_timer, musb_conn_timer_handler,
- (unsigned long) musb);
+ timer_setup(&musb->dev_timer, musb_conn_timer_handler, 0);
musb->xceiv->set_power = bfin_musb_set_power;
diff --git a/drivers/usb/musb/blackfin.h b/drivers/usb/musb/blackfin.h
index c84dae546dc6..231f2d23b8a1 100644
--- a/drivers/usb/musb/blackfin.h
+++ b/drivers/usb/musb/blackfin.h
@@ -82,6 +82,4 @@ static void dump_fifo_data(u8 *buf, u16 len)
/* Almost 1 second */
#define TIMER_DELAY (1 * HZ)
-static struct timer_list musb_conn_timer;
-
#endif /* __MUSB_BLACKFIN_H__ */
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index df88123274ca..a0bc6e7372e0 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -138,11 +138,9 @@ static void da8xx_musb_set_vbus(struct musb *musb, int is_on)
#define POLL_SECONDS 2
-static struct timer_list otg_workaround;
-
-static void otg_timer(unsigned long _musb)
+static void otg_timer(struct timer_list *t)
{
- struct musb *musb = (void *)_musb;
+ struct musb *musb = from_timer(musb, t, dev_timer);
void __iomem *mregs = musb->mregs;
u8 devctl;
unsigned long flags;
@@ -178,7 +176,7 @@ static void otg_timer(unsigned long _musb)
* VBUSERR got reported during enumeration" cases.
*/
if (devctl & MUSB_DEVCTL_VBUS) {
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
break;
}
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
@@ -201,7 +199,7 @@ static void otg_timer(unsigned long _musb)
musb_writeb(mregs, MUSB_DEVCTL, devctl | MUSB_DEVCTL_SESSION);
devctl = musb_readb(mregs, MUSB_DEVCTL);
if (devctl & MUSB_DEVCTL_BDEVICE)
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
else
musb->xceiv->otg->state = OTG_STATE_A_IDLE;
break;
@@ -223,12 +221,12 @@ static void da8xx_musb_try_idle(struct musb *musb, unsigned long timeout)
musb->xceiv->otg->state == OTG_STATE_A_WAIT_BCON)) {
dev_dbg(musb->controller, "%s active, deleting timer\n",
usb_otg_state_string(musb->xceiv->otg->state));
- del_timer(&otg_workaround);
+ del_timer(&musb->dev_timer);
last_timer = jiffies;
return;
}
- if (time_after(last_timer, timeout) && timer_pending(&otg_workaround)) {
+ if (time_after(last_timer, timeout) && timer_pending(&musb->dev_timer)) {
dev_dbg(musb->controller, "Longer idle timer already pending, ignoring...\n");
return;
}
@@ -237,7 +235,7 @@ static void da8xx_musb_try_idle(struct musb *musb, unsigned long timeout)
dev_dbg(musb->controller, "%s inactive, starting idle timer for %u ms\n",
usb_otg_state_string(musb->xceiv->otg->state),
jiffies_to_msecs(timeout - jiffies));
- mod_timer(&otg_workaround, timeout);
+ mod_timer(&musb->dev_timer, timeout);
}
static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
@@ -297,14 +295,14 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
*/
musb->int_usb &= ~MUSB_INTR_VBUSERROR;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VFALL;
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
- del_timer(&otg_workaround);
+ del_timer(&musb->dev_timer);
} else {
musb->is_active = 0;
MUSB_DEV_MODE(musb);
@@ -331,7 +329,7 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
/* Poll for ID change */
if (musb->xceiv->otg->state == OTG_STATE_B_IDLE)
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
spin_unlock_irqrestore(&musb->lock, flags);
@@ -393,7 +391,7 @@ static int da8xx_musb_init(struct musb *musb)
goto fail;
}
- setup_timer(&otg_workaround, otg_timer, (unsigned long)musb);
+ timer_setup(&musb->dev_timer, otg_timer, 0);
/* Reset the controller */
musb_writel(reg_base, DA8XX_USB_CTRL_REG, DA8XX_SOFT_RESET_MASK);
@@ -431,7 +429,7 @@ static int da8xx_musb_exit(struct musb *musb)
{
struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
- del_timer_sync(&otg_workaround);
+ del_timer_sync(&musb->dev_timer);
phy_power_off(glue->phy);
phy_exit(glue->phy);
diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 52b491d3d5d8..3a7048e84e1c 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -199,11 +199,9 @@ static void davinci_musb_set_vbus(struct musb *musb, int is_on)
#define POLL_SECONDS 2
-static struct timer_list otg_workaround;
-
-static void otg_timer(unsigned long _musb)
+static void otg_timer(struct timer_list *t)
{
- struct musb *musb = (void *)_musb;
+ struct musb *musb = from_timer(musb, t, dev_timer);
void __iomem *mregs = musb->mregs;
u8 devctl;
unsigned long flags;
@@ -224,7 +222,7 @@ static void otg_timer(unsigned long _musb)
* VBUSERR got reported during enumeration" cases.
*/
if (devctl & MUSB_DEVCTL_VBUS) {
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
break;
}
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
@@ -248,7 +246,7 @@ static void otg_timer(unsigned long _musb)
devctl | MUSB_DEVCTL_SESSION);
devctl = musb_readb(mregs, MUSB_DEVCTL);
if (devctl & MUSB_DEVCTL_BDEVICE)
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
else
musb->xceiv->otg->state = OTG_STATE_A_IDLE;
break;
@@ -325,14 +323,14 @@ static irqreturn_t davinci_musb_interrupt(int irq, void *__hci)
*/
musb->int_usb &= ~MUSB_INTR_VBUSERROR;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VFALL;
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
- del_timer(&otg_workaround);
+ del_timer(&musb->dev_timer);
} else {
musb->is_active = 0;
MUSB_DEV_MODE(musb);
@@ -361,7 +359,7 @@ static irqreturn_t davinci_musb_interrupt(int irq, void *__hci)
/* poll for ID change */
if (musb->xceiv->otg->state == OTG_STATE_B_IDLE)
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);
spin_unlock_irqrestore(&musb->lock, flags);
@@ -393,7 +391,7 @@ static int davinci_musb_init(struct musb *musb)
if (revision == 0)
goto fail;
- setup_timer(&otg_workaround, otg_timer, (unsigned long) musb);
+ timer_setup(&musb->dev_timer, otg_timer, 0);
davinci_musb_source_power(musb, 0, 1);
@@ -443,7 +441,7 @@ static int davinci_musb_init(struct musb *musb)
static int davinci_musb_exit(struct musb *musb)
{
- del_timer_sync(&otg_workaround);
+ del_timer_sync(&musb->dev_timer);
/* force VBUS off */
if (cpu_is_davinci_dm355()) {
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index ff5a1a8989d5..0f34cf30cdf5 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -485,9 +485,9 @@ void musb_load_testpacket(struct musb *musb)
/*
* Handles OTG hnp timeouts, such as b_ase0_brst
*/
-static void musb_otg_timer_func(unsigned long data)
+static void musb_otg_timer_func(struct timer_list *t)
{
- struct musb *musb = (struct musb *)data;
+ struct musb *musb = from_timer(musb, t, otg_timer);
unsigned long flags;
spin_lock_irqsave(&musb->lock, flags);
@@ -2330,7 +2330,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
if (status < 0)
goto fail3;
- setup_timer(&musb->otg_timer, musb_otg_timer_func, (unsigned long) musb);
+ timer_setup(&musb->otg_timer, musb_otg_timer_func, 0);
/* attach to the IRQ */
if (request_irq(nIrq, musb->isr, IRQF_SHARED, dev_name(dev), musb)) {
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;
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);
/* Reset the musb */
musb_writel(reg_base, wrp->control, (1 << wrp->reset));
diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
index 4eb640c54f2c..f8fce7df654f 100644
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -452,11 +452,9 @@ static int tusb_musb_vbus_status(struct musb *musb)
return ret;
}
-static struct timer_list musb_idle_timer;
-
-static void musb_do_idle(unsigned long _musb)
+static void musb_do_idle(struct timer_list *t)
{
- struct musb *musb = (void *)_musb;
+ struct musb *musb = from_timer(musb, t, dev_timer);
unsigned long flags;
spin_lock_irqsave(&musb->lock, flags);
@@ -523,13 +521,13 @@ static void tusb_musb_try_idle(struct musb *musb, unsigned long timeout)
&& (musb->xceiv->otg->state == OTG_STATE_A_WAIT_BCON))) {
dev_dbg(musb->controller, "%s active, deleting timer\n",
usb_otg_state_string(musb->xceiv->otg->state));
- del_timer(&musb_idle_timer);
+ del_timer(&musb->dev_timer);
last_timer = jiffies;
return;
}
if (time_after(last_timer, timeout)) {
- if (!timer_pending(&musb_idle_timer))
+ if (!timer_pending(&musb->dev_timer))
last_timer = timeout;
else {
dev_dbg(musb->controller, "Longer idle timer already pending, ignoring\n");
@@ -541,7 +539,7 @@ static void tusb_musb_try_idle(struct musb *musb, unsigned long timeout)
dev_dbg(musb->controller, "%s inactive, for idle timer for %lu ms\n",
usb_otg_state_string(musb->xceiv->otg->state),
(unsigned long)jiffies_to_msecs(timeout - jiffies));
- mod_timer(&musb_idle_timer, timeout);
+ mod_timer(&musb->dev_timer, timeout);
}
/* ticks of 60 MHz clock */
@@ -873,7 +871,7 @@ static irqreturn_t tusb_musb_interrupt(int irq, void *__hci)
}
if (int_src & TUSB_INT_SRC_USB_IP_CONN)
- del_timer(&musb_idle_timer);
+ del_timer(&musb->dev_timer);
/* OTG state change reports (annoyingly) not issued by Mentor core */
if (int_src & (TUSB_INT_SRC_VBUS_SENSE_CHNG
@@ -982,7 +980,7 @@ static void tusb_musb_disable(struct musb *musb)
musb_writel(tbase, TUSB_DMA_INT_MASK, 0x7fffffff);
musb_writel(tbase, TUSB_GPIO_INT_MASK, 0x1ff);
- del_timer(&musb_idle_timer);
+ del_timer(&musb->dev_timer);
if (is_dma_capable() && !dma_off) {
printk(KERN_WARNING "%s %s: dma still active\n",
@@ -1142,7 +1140,7 @@ static int tusb_musb_init(struct musb *musb)
musb->xceiv->set_power = tusb_draw_power;
the_musb = musb;
- setup_timer(&musb_idle_timer, musb_do_idle, (unsigned long) musb);
+ timer_setup(&musb->dev_timer, musb_do_idle, 0);
done:
if (ret < 0) {
@@ -1156,7 +1154,7 @@ static int tusb_musb_init(struct musb *musb)
static int tusb_musb_exit(struct musb *musb)
{
- del_timer_sync(&musb_idle_timer);
+ del_timer_sync(&musb->dev_timer);
the_musb = NULL;
if (musb->board_set_power)
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: musb: Convert timers to use timer_setup()
2017-10-24 10:08 [PATCH] usb: musb: Convert timers to use timer_setup() Kees Cook
@ 2017-10-27 16:24 ` Bin Liu
2017-10-30 8:52 ` Greg Kroah-Hartman
0 siblings, 1 reply; 7+ messages in thread
From: Bin Liu @ 2017-10-27 16:24 UTC (permalink / raw)
To: Kees Cook; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
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 <b-liu@ti.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> 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.
Regards,
-Bin.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: musb: Convert timers to use timer_setup()
2017-10-27 16:24 ` Bin Liu
@ 2017-10-30 8:52 ` Greg Kroah-Hartman
2017-10-30 13:17 ` Bin Liu
2017-10-30 16:33 ` [PATCH] usb: musb: Convert timers to use timer_setup() Bin Liu
0 siblings, 2 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2017-10-30 8:52 UTC (permalink / raw)
To: Bin Liu, Kees Cook, linux-usb, linux-kernel
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 <b-liu@ti.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: linux-usb@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: musb: Convert timers to use timer_setup()
2017-10-30 8:52 ` Greg Kroah-Hartman
@ 2017-10-30 13:17 ` Bin Liu
[not found] ` <1509380996-1271-1-git-send-email-b-liu@ti.com>
2017-10-30 16:33 ` [PATCH] usb: musb: Convert timers to use timer_setup() Bin Liu
1 sibling, 1 reply; 7+ messages in thread
From: Bin Liu @ 2017-10-30 13:17 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Kees Cook, linux-usb, linux-kernel
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 <b-liu@ti.com>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: linux-usb@vger.kernel.org
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > ---
> > > 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.
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: musb: Convert timers to use timer_setup()
2017-10-30 8:52 ` Greg Kroah-Hartman
2017-10-30 13:17 ` Bin Liu
@ 2017-10-30 16:33 ` Bin Liu
1 sibling, 0 replies; 7+ messages in thread
From: Bin Liu @ 2017-10-30 16:33 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Kees Cook, linux-usb, linux-kernel
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 <b-liu@ti.com>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: linux-usb@vger.kernel.org
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > ---
> > > 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?
The fixup patch was sent. Here is my Acked-by for this patch.
Acked-by: Bin Liu <b-liu@ti.com>
Regards,
-Bin.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: musb: dsps: remove the duplicated timer
[not found] ` <1509380996-1271-1-git-send-email-b-liu@ti.com>
@ 2017-10-30 21:29 ` Kees Cook
2017-10-30 21:44 ` Greg Kroah-Hartman
0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2017-10-30 21:29 UTC (permalink / raw)
To: Bin Liu, Greg Kroah-Hartman; +Cc: linux-usb, LKML
On Mon, Oct 30, 2017 at 9:29 AM, Bin Liu <b-liu@ti.com> wrote:
> Now struct musb has the timer (dev_timer) for glue drivers, so let's
> remove the duplicated timer defined in dsps glue driver, and use
> dev_timer defined in struct musb.
>
> Signed-off-by: Bin Liu <b-liu@ti.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Thanks for this; it wasn't clear to me if both timers needed to exist
(i.e. glue operated independently from the musb object), so I didn't
make this change myself.
Greg, it probably makes sense to just take both patches. Does that work for you?
-Kees
> ---
>
> This is the fixup patch for [1].
>
> [1] https://marc.info/?l=linux-usb&m=150936947225302&w=2
>
> drivers/usb/musb/musb_dsps.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index e3d0e626a5d6..4d95fe714f27 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;
> @@ -149,6 +148,7 @@ struct dsps_glue {
>
> static void dsps_mod_timer(struct dsps_glue *glue, int wait_ms)
> {
> + struct musb *musb = platform_get_drvdata(glue->musb);
> int wait;
>
> if (wait_ms < 0)
> @@ -156,7 +156,7 @@ static void dsps_mod_timer(struct dsps_glue *glue, int wait_ms)
> else
> wait = msecs_to_jiffies(wait_ms);
>
> - mod_timer(&glue->timer, jiffies + wait);
> + mod_timer(&musb->dev_timer, jiffies + wait);
> }
>
> /*
> @@ -216,7 +216,7 @@ static void dsps_musb_disable(struct musb *musb)
> musb_writel(reg_base, wrp->coreintr_clear, wrp->usb_bitmap);
> musb_writel(reg_base, wrp->epintr_clear,
> wrp->txep_bitmap | wrp->rxep_bitmap);
> - del_timer_sync(&glue->timer);
> + del_timer_sync(&musb->dev_timer);
> }
>
> /* Caller must take musb->lock */
> @@ -230,7 +230,7 @@ static int dsps_check_status(struct musb *musb, void *unused)
> int skip_session = 0;
>
> if (glue->vbus_irq)
> - del_timer(&glue->timer);
> + del_timer(&musb->dev_timer);
>
> /*
> * We poll because DSPS IP's won't expose several OTG-critical
> @@ -284,8 +284,7 @@ static int dsps_check_status(struct musb *musb, void *unused)
>
> static void otg_timer(struct timer_list *t)
> {
> - struct dsps_glue *glue = from_timer(glue, t, timer);
> - struct musb *musb = platform_get_drvdata(glue->musb);
> + struct musb *musb = from_timer(musb, t, dev_timer);
> struct device *dev = musb->controller;
> unsigned long flags;
> int err;
> @@ -481,7 +480,7 @@ static int dsps_musb_init(struct musb *musb)
> }
> }
>
> - timer_setup(&glue->timer, otg_timer, 0);
> + timer_setup(&musb->dev_timer, otg_timer, 0);
>
> /* Reset the musb */
> musb_writel(reg_base, wrp->control, (1 << wrp->reset));
> @@ -516,7 +515,7 @@ static int dsps_musb_exit(struct musb *musb)
> struct device *dev = musb->controller;
> struct dsps_glue *glue = dev_get_drvdata(dev->parent);
>
> - del_timer_sync(&glue->timer);
> + del_timer_sync(&musb->dev_timer);
> usb_phy_shutdown(musb->xceiv);
> phy_power_off(musb->phy);
> phy_exit(musb->phy);
> @@ -1028,7 +1027,7 @@ static int dsps_suspend(struct device *dev)
> return ret;
> }
>
> - del_timer_sync(&glue->timer);
> + del_timer_sync(&musb->dev_timer);
>
> mbase = musb->ctrl_base;
> glue->context.control = musb_readl(mbase, wrp->control);
> --
> 1.9.1
>
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: musb: dsps: remove the duplicated timer
2017-10-30 21:29 ` [PATCH] usb: musb: dsps: remove the duplicated timer Kees Cook
@ 2017-10-30 21:44 ` Greg Kroah-Hartman
0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2017-10-30 21:44 UTC (permalink / raw)
To: Kees Cook; +Cc: Bin Liu, linux-usb, LKML
On Mon, Oct 30, 2017 at 02:29:54PM -0700, Kees Cook wrote:
> On Mon, Oct 30, 2017 at 9:29 AM, Bin Liu <b-liu@ti.com> wrote:
> > Now struct musb has the timer (dev_timer) for glue drivers, so let's
> > remove the duplicated timer defined in dsps glue driver, and use
> > dev_timer defined in struct musb.
> >
> > Signed-off-by: Bin Liu <b-liu@ti.com>
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> Thanks for this; it wasn't clear to me if both timers needed to exist
> (i.e. glue operated independently from the musb object), so I didn't
> make this change myself.
>
> Greg, it probably makes sense to just take both patches. Does that work for you?
Yes, I can do that, thanks.
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-30 21:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-24 10:08 [PATCH] usb: musb: Convert timers to use timer_setup() Kees Cook
2017-10-27 16:24 ` Bin Liu
2017-10-30 8:52 ` Greg Kroah-Hartman
2017-10-30 13:17 ` Bin Liu
[not found] ` <1509380996-1271-1-git-send-email-b-liu@ti.com>
2017-10-30 21:29 ` [PATCH] usb: musb: dsps: remove the duplicated timer Kees Cook
2017-10-30 21:44 ` Greg Kroah-Hartman
2017-10-30 16:33 ` [PATCH] usb: musb: Convert timers to use timer_setup() Bin Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.