On Sun, May 15, 2022 at 01:47:11PM +0300, Yaşar Arabacı wrote:
> Currently this driver uses ioctl for reading/writing per-device and
> per-client configuration. Per-client configuration can be handled by
> usespace and sent to driver with each write() call. Doing so does not
> introduce extra overhead because we copy tx config to fifo for each
> transmit anyway. This way, we don't have to introduce new ioctl's.
>
> This has not been tested as I don't have access to hardware.
>
> Signed-off-by: Yaşar Arabacı <yasar11732@xxxxxxxxx>
> ---
> drivers/staging/pi433/pi433_if.c | 63 ++++++--------------------------
> drivers/staging/pi433/pi433_if.h | 7 +---
> 2 files changed, 13 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
> index 941aaa7eab2e..07cd9054560a 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -109,10 +109,6 @@ struct pi433_device {
>
> struct pi433_instance {
> struct pi433_device *device;
> - struct pi433_tx_cfg tx_cfg;
> -
> - /* control flags */
> - bool tx_cfg_initialized;
> };
>
> /*-------------------------------------------------------------------------*/
> @@ -574,12 +570,6 @@ static int pi433_tx_thread(void *data)
> if (kthread_should_stop())
> return 0;
>
> - /*
> - * get data from fifo in the following order:
> - * - tx_cfg
> - * - size of message
> - * - message
> - */
> retval = kfifo_out(&device->tx_fifo, &tx_cfg, sizeof(tx_cfg));
> if (retval != sizeof(tx_cfg)) {
> dev_dbg(device->dev,
> @@ -588,13 +578,7 @@ static int pi433_tx_thread(void *data)
> continue;
> }
>
> - retval = kfifo_out(&device->tx_fifo, &size, sizeof(size_t));
> - if (retval != sizeof(size_t)) {
> - dev_dbg(device->dev,
> - "reading msg size from fifo failed: got %d, expected %d\n",
> - retval, (unsigned int)sizeof(size_t));
> - continue;
> - }
> + size = tx_cfg.payload_size;
>
> /* use fixed message length, if requested */
> if (tx_cfg.fixed_message_length != 0)
> @@ -811,6 +795,7 @@ pi433_write(struct file *filp, const char __user *buf,
> size_t count, loff_t *f_pos)
> {
> struct pi433_instance *instance;
> + struct pi433_tx_cfg *tx_cfg;
> struct pi433_device *device;
> int retval;
> unsigned int required, available, copied;
> @@ -822,18 +807,16 @@ pi433_write(struct file *filp, const char __user *buf,
> * check, whether internal buffer (tx thread) is big enough
> * for requested size
> */
> - if (count > MAX_MSG_SIZE)
> + if (unlikely(count > MAX_MSG_SIZE))
> return -EMSGSIZE;
Unless you can benchmark the difference, NEVER use likely/unlikely as
the compiler and CPU almost always do a better job than humans in
figuring this stuff out.
Also it's an unrelated change to what you said this commit was going to
do, please don't do that.
thanks,
greg k-h
