Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
From: Jay Aurabind <jay.aurabind@gmail.com>
To: kernelnewbies <kernelnewbies@kernelnewbies.org>
Subject: Using binary attributes for configuration sysfs entries
Date: Wed, 13 Mar 2019 12:02:02 +0530
Message-ID: <CAOsEZojbwaYZOOT95QsrLBXpPKpFgeLh41pqL6sFij_-8tVg+g@mail.gmail.com> (raw)

[-- 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

             reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13  6:32 Jay Aurabind [this message]
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

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOsEZojbwaYZOOT95QsrLBXpPKpFgeLh41pqL6sFij_-8tVg+g@mail.gmail.com \
    --to=jay.aurabind@gmail.com \
    --cc=kernelnewbies@kernelnewbies.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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