2021-10-21 08:41:54

by Cai,Huoqing

[permalink] [raw]
Subject: [PATCH] iio: adc: ina2xx: Make use of the helper macro kthread_run()

Repalce kthread_create/wake_up_process() with kthread_run()
to simplify the code.

Signed-off-by: Cai Huoqing <[email protected]>
---
drivers/iio/adc/ina2xx-adc.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index a4b2ff9e0dd5..360d7a00f60d 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -842,15 +842,14 @@ static int ina2xx_buffer_enable(struct iio_dev *indio_dev)
dev_dbg(&indio_dev->dev, "Async readout mode: %d\n",
chip->allow_async_readout);

- task = kthread_create(ina2xx_capture_thread, (void *)indio_dev,
- "%s:%d-%uus", indio_dev->name,
- iio_device_id(indio_dev),
- sampling_us);
+ task = kthread_run(ina2xx_capture_thread, (void *)indio_dev,
+ "%s:%d-%uus", indio_dev->name,
+ iio_device_id(indio_dev),
+ sampling_us);
if (IS_ERR(task))
return PTR_ERR(task);

get_task_struct(task);
- wake_up_process(task);
chip->task = task;

return 0;
--
2.25.1


2021-10-21 08:50:43

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH] iio: adc: ina2xx: Make use of the helper macro kthread_run()

On 10/21/21 10:39 AM, Cai Huoqing wrote:
> Repalce kthread_create/wake_up_process() with kthread_run()
> to simplify the code.
>
> Signed-off-by: Cai Huoqing <[email protected]>

Hi,

Thanks for the patch, this looks good!

Reviewed-by: Lars-Peter Clausen <[email protected]>

> ---
> [...]
> if (IS_ERR(task))
> return PTR_ERR(task);
>
> get_task_struct(task);

This is unrelated to this patch. But I wonder do we really need the
get_task_struct()? The driver calls put_task_struct() right after
kthread_stop().

kthread_create()/kthread_run() and kthread_stop() already do reference
counting of the task, so we are essentially just double reference
counting. Maybe you can send another patch to cleanup the
get_task_struct()/put_task_struct() in this driver.

> - wake_up_process(task);
> chip->task = task;
>
> return 0;