Using binary attributes for configuration sysfs entries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Before I send this patch to actual mailing list, I'd appreciate if
someone could tell me if this is a bad idea!

The driver in staging for pi433 (a wireless transceiver) uses IOCTLs
at the moment. I wish to add a sysfs interface to it that control the
various transmission and reception parameters. In the ioctl interface,
it uses two structs that have about 40 parameters in total.

For the corresponding sysfs interface, since there are a lot of
parameters, would it be justified to use the same binary format though
sysfs_create_binary_file() ? The rationale is that it would be easier
to simply pack all the config options in the struct and send it in
once rather than individually write 40 files. This is what the
attached patch follows. Interface is added only for reception
parameters as of now.

Any others suggestions welcome!


-- 

Thanks and Regards,
Jay
From 926be3ade2b5769e035bf33ad6fb8ba2a228ce12 Mon Sep 17 00:00:00 2001
From: Aurabindo Jayamohanan <mail@xxxxxxxxxxxx>
Date: Mon, 11 Mar 2019 16:12:48 +0530
Subject: [PATCH] staging: pi433: register sysfs i/f for rx config

Exposes a binary sysfs file for setting reception configuration
options for pi433. A binary interface is used since it would be
counterintuitive to manually set a dozen options individually.

Signed-off-by: Aurabindo Jayamohanan <mail@xxxxxxxxxxxx>
---
 drivers/staging/pi433/pi433_if.c | 50 ++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index b2314636dc89..7f429b112a0a 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -25,6 +25,7 @@
 #include <linux/uaccess.h>
 #include <linux/fs.h>
 #include <linux/device.h>
+#include <linux/sysfs.h>
 #include <linux/cdev.h>
 #include <linux/err.h>
 #include <linux/kfifo.h>
@@ -1089,6 +1090,40 @@ static void pi433_free_minor(struct pi433_device *dev)
 	mutex_unlock(&minor_lock);
 }
 
+static ssize_t pi433_rx_cfg_read(struct file *filp, struct kobject *kobj,
+				struct bin_attribute *bin_attr, char *buf,
+				loff_t off,
+				size_t count)
+{
+	struct pi433_device *pdev = bin_attr->private;
+	struct pi433_rx_cfg *src = pdev->rx_cfg;
+
+	if (count != bin_attr->size) {
+		return -EIO;
+	}
+
+	memcpy((void *)buf, (void *)src, bin_attr->size);
+
+	return bin_attr->size;
+}
+
+static ssize_t pi433_rx_cfg_write(struct file *filp, struct kobject *kobj,
+				struct bin_attribute *bin_attr, char *buf,
+				loff_t off,
+				size_t count)
+{
+	struct pi433_device *pdev = bin_attr->private;
+	struct pi433_rx_cfg *dst = pdev->rx_cfg;
+
+	if (count != bin_attr->size) {
+		return -EIO;
+	}
+
+	memcpy((void *)dst, (void *)buf, bin_attr->size);
+
+	return bin_attr->size;
+}
+
 /*-------------------------------------------------------------------------*/
 
 static const struct file_operations pi433_fops = {
@@ -1107,6 +1142,8 @@ static const struct file_operations pi433_fops = {
 	.llseek =	no_llseek,
 };
 
+BIN_ATTR_RW(pi433_rx_cfg, sizeof(struct pi433_rx_cfg));
+
 /*-------------------------------------------------------------------------*/
 
 static int pi433_probe(struct spi_device *spi)
@@ -1243,6 +1280,15 @@ static int pi433_probe(struct spi_device *spi)
 		goto send_thread_failed;
 	}
 
+	/* create binary sysfs sentry for rx_cfg*/
+	bin_attr_pi433_rx_cfg.private = device;
+	retval = sysfs_create_bin_file(&device->dev->kobj,
+					&bin_attr_pi433_rx_cfg);
+	if (retval) {
+		dev_err(device->dev, "creation rx_cfg sysfs attribute failed");
+		goto sysfs_attr_failed;
+	}
+
 	/* create cdev */
 	device->cdev = cdev_alloc();
 	if (!device->cdev) {
@@ -1265,6 +1311,8 @@ static int pi433_probe(struct spi_device *spi)
 del_cdev:
 	cdev_del(device->cdev);
 cdev_failed:
+	sysfs_remove_bin_file(&device->dev->kobj, &bin_attr_pi433_rx_cfg);
+sysfs_attr_failed:
 	kthread_stop(device->tx_task_struct);
 send_thread_failed:
 	device_destroy(pi433_class, device->devt);
@@ -1284,6 +1332,8 @@ static int pi433_remove(struct spi_device *spi)
 {
 	struct pi433_device	*device = spi_get_drvdata(spi);
 
+	sysfs_remove_bin_file(&device->dev->kobj, &bin_attr_pi433_rx_cfg);
+
 	/* free GPIOs */
 	free_gpio(device);
 
-- 
2.19.1

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]

  Powered by Linux