On 24/10/17 09:25, 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.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: "Bryan O'Donoghue" <[email protected]>
> Cc: Johan Hovold <[email protected]>
> Cc: Alex Elder <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
> ---
> drivers/staging/greybus/loopback.c | 14 ++++----------
> drivers/staging/greybus/operation.c | 7 +++----
> 2 files changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index 08e255884206..045aaf81113a 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -572,16 +572,11 @@ static void gb_loopback_async_operation_work(struct work_struct *work)
> gb_loopback_async_operation_put(op_async);
> }
>
> -static void gb_loopback_async_operation_timeout(unsigned long data)
> +static void gb_loopback_async_operation_timeout(struct timer_list *t)
> {
> - struct gb_loopback_async_operation *op_async;
> - u16 id = data;
> + struct gb_loopback_async_operation *op_async =
> + from_timer(op_async, t, timer);
>
> - op_async = gb_loopback_operation_find(id);
> - if (!op_async) {
> - pr_err("operation %d not found - time out ?\n", id);
> - return;
> - }
Hi Kees, you need to add
gb_loopback_async_operation_get(op_async); when dropping the
gb_loopback_operation_find() call here.
> schedule_work(&op_async->work);
> }
>
> @@ -631,8 +626,7 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
> if (ret)
> goto error;
>
> - setup_timer(&op_async->timer, gb_loopback_async_operation_timeout,
> - (unsigned long)operation->id);
> + timer_setup(&op_async->timer, gb_loopback_async_operation_timeout, 0);
> op_async->timer.expires = jiffies + gb->jiffy_timeout;
> add_timer(&op_async->timer);
>
> diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
> index 3023012808d9..ee4ba3f23bef 100644
> --- a/drivers/staging/greybus/operation.c
> +++ b/drivers/staging/greybus/operation.c
> @@ -294,9 +294,9 @@ static void gb_operation_work(struct work_struct *work)
> gb_operation_put(operation);
> }
>
> -static void gb_operation_timeout(unsigned long arg)
> +static void gb_operation_timeout(struct timer_list *t)
> {
> - struct gb_operation *operation = (void *)arg;
> + struct gb_operation *operation = from_timer(operation, t, timer);
>
> if (gb_operation_result_set(operation, -ETIMEDOUT)) {
> /*
> @@ -541,8 +541,7 @@ gb_operation_create_common(struct gb_connection *connection, u8 type,
> goto err_request;
> }
>
> - setup_timer(&operation->timer, gb_operation_timeout,
> - (unsigned long)operation);
> + timer_setup(&operation->timer, gb_operation_timeout, 0);
> }
>
> operation->flags = op_flags;
>
Other than that, the rest looks good to me and you can add my
Reviewed-by: Bryan O'Donoghue <[email protected]>
From 1582126742408023281@xxx Tue Oct 24 08:27:19 +0000 2017
X-GM-THRID: 1582126742408023281
X-Gmail-Labels: Inbox,Category Forums