Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
* Using binary attributes for configuration sysfs entries
@ 2019-03-13  6:32 Jay Aurabind
  2019-03-13  6:56 ` valdis.kletnieks
  2019-03-13 14:25 ` Greg KH
  0 siblings, 2 replies; 7+ messages in thread
From: Jay Aurabind @ 2019-03-13  6:32 UTC (permalink / raw)
  To: kernelnewbies

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]

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

[-- Attachment #2: 0001-staging-pi433-register-sysfs-i-f-for-rx-config.patch --]
[-- Type: text/x-patch, Size: 3340 bytes --]

From 926be3ade2b5769e035bf33ad6fb8ba2a228ce12 Mon Sep 17 00:00:00 2001
From: Aurabindo Jayamohanan <mail@aurabindo.in>
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@aurabindo.in>
---
 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


[-- Attachment #3: Type: text/plain, Size: 170 bytes --]

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-13  6:32 Using binary attributes for configuration sysfs entries Jay Aurabind
2019-03-13  6:56 ` valdis.kletnieks
2019-03-13  9:08   ` Jay Aurabind
2019-03-13 14:25 ` Greg KH
2019-03-14 10:47   ` Jay Aurabind
2019-03-14 14:57     ` Greg KH
2019-03-14 17:41       ` Jay Aurabind

Kernel Newbies archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernelnewbies/0 kernelnewbies/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernelnewbies kernelnewbies/ https://lore.kernel.org/kernelnewbies \
		kernelnewbies@kernelnewbies.org kernelnewbies@archiver.kernel.org
	public-inbox-index kernelnewbies


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernelnewbies.kernelnewbies


AGPL code for this site: git clone https://public-inbox.org/ public-inbox