[PATCH] Staging: pi433: Don't use ioctl for per-client configuration

[Yaşar Arabacı]

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;
 
-	/*
-	 * check if tx_cfg has been initialized otherwise we won't be able to
-	 * config the RF trasmitter correctly due to invalid settings
-	 */
-	if (!instance->tx_cfg_initialized) {
-		dev_notice_once(device->dev,
-				"write: failed due to unconfigured tx_cfg (see PI433_IOC_WR_TX_CFG)\n");
-		return -EINVAL;
-	}
+	if (unlikely(count < sizeof(struct pi433_tx_cfg)))
+		return -EMSGSIZE;
+
+	tx_cfg = (struct pi433_tx_cfg *)buf;
+
+	if (unlikely(count != sizeof(struct pi433_tx_cfg)) + tx_cfg->payload_size)
+		return -EMSGSIZE;
 
 	/*
 	 * write the following sequence into fifo:
@@ -843,24 +826,14 @@ pi433_write(struct file *filp, const char __user *buf,
 	 */
 	mutex_lock(&device->tx_fifo_lock);
 
-	required = sizeof(instance->tx_cfg) + sizeof(size_t) + count;
 	available = kfifo_avail(&device->tx_fifo);
-	if (required > available) {
+	if (count > available) {
 		dev_dbg(device->dev, "write to fifo failed: %d bytes required but %d available\n",
 			required, available);
 		mutex_unlock(&device->tx_fifo_lock);
 		return -EAGAIN;
 	}
 
-	retval = kfifo_in(&device->tx_fifo, &instance->tx_cfg,
-			  sizeof(instance->tx_cfg));
-	if (retval != sizeof(instance->tx_cfg))
-		goto abort;
-
-	retval = kfifo_in(&device->tx_fifo, &count, sizeof(size_t));
-	if (retval != sizeof(size_t))
-		goto abort;
-
 	retval = kfifo_from_user(&device->tx_fifo, buf, count, &copied);
 	if (retval || copied != count)
 		goto abort;
@@ -884,7 +857,6 @@ static long pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct pi433_instance	*instance;
 	struct pi433_device	*device;
-	struct pi433_tx_cfg	tx_cfg;
 	void __user *argp = (void __user *)arg;
 
 	/* Check type and command number */
@@ -898,19 +870,6 @@ static long pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return -ESHUTDOWN;
 
 	switch (cmd) {
-	case PI433_IOC_RD_TX_CFG:
-		if (copy_to_user(argp, &instance->tx_cfg,
-				 sizeof(struct pi433_tx_cfg)))
-			return -EFAULT;
-		break;
-	case PI433_IOC_WR_TX_CFG:
-		if (copy_from_user(&tx_cfg, argp, sizeof(struct pi433_tx_cfg)))
-			return -EFAULT;
-		mutex_lock(&device->tx_fifo_lock);
-		memcpy(&instance->tx_cfg, &tx_cfg, sizeof(struct pi433_tx_cfg));
-		instance->tx_cfg_initialized = true;
-		mutex_unlock(&device->tx_fifo_lock);
-		break;
 	case PI433_IOC_RD_RX_CFG:
 		if (copy_to_user(argp, &device->rx_cfg,
 				 sizeof(struct pi433_rx_cfg)))
diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index 25ee0b77a32c..5dd2a3c6e661 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -75,6 +75,8 @@ struct pi433_tx_cfg {
 
 	__u8			sync_pattern[8];
 	__u8			address_byte;
+	__u32			payload_size;
+	__u8			payload[];
 };
 
 /**
@@ -135,11 +137,6 @@ struct pi433_rx_cfg {
 
 #define PI433_IOC_MAGIC	'r'
 
-#define PI433_IOC_RD_TX_CFG                                             \
-	_IOR(PI433_IOC_MAGIC, PI433_TX_CFG_IOCTL_NR, char[sizeof(struct pi433_tx_cfg)])
-#define PI433_IOC_WR_TX_CFG                                             \
-	_IOW(PI433_IOC_MAGIC, PI433_TX_CFG_IOCTL_NR, char[sizeof(struct pi433_tx_cfg)])
-
 #define PI433_IOC_RD_RX_CFG                                             \
 	_IOR(PI433_IOC_MAGIC, PI433_RX_CFG_IOCTL_NR, char[sizeof(struct pi433_rx_cfg)])
 #define PI433_IOC_WR_RX_CFG                                             \
-- 
2.36.1