This controller formats the data to a Wiegand format and bit-bangs
the message on devicetree defined GPIO lines.
Several attributes need to be defined in the devicetree in order
for this driver to work, namely the data-hi-gpios, data-lo-gpios,
pulse-len, frame-gap and interval-len. These attributes are
documented in the devicetree bindings documentation files.
The driver creates a dev file for writing messages on the bus.
It also creates a sysfs file to control the payload length of
messages(in bits). If a message is shorter than the set payload
length, it will be discarded. On the other hand, if a message is
longer, the additional bits will be stripped off.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
---
.../ABI/testing/sysfs-driver-wiegand-gpio | 9 +
MAINTAINERS | 2 +
drivers/wiegand/Kconfig | 8 +
drivers/wiegand/Makefile | 1 +
drivers/wiegand/wiegand-gpio.c | 316 ++++++++++++++++++
5 files changed, 336 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-wiegand-gpio
create mode 100644 drivers/wiegand/wiegand-gpio.c
diff --git a/Documentation/ABI/testing/sysfs-driver-wiegand-gpio b/Documentation/ABI/testing/sysfs-driver-wiegand-gpio
new file mode 100644
index 000000000000..be2246880a83
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-wiegand-gpio
@@ -0,0 +1,9 @@
+What: /sys/devices/platform/.../wiegand-gpio-attributes/payload_len
+Date: January 2023
+Contact: Martin Zaťovič <m.zatovic1@gmail.com>
+Description:
+ Read/set the payload length of messages sent by Wiegand GPIO
+ bit-banging controller in bits. The default value is 26, as
+ that is the most widely-used length of Wiegand messages.
+ Controller will only send messages of at least the set length
+ and it will strip off bits of longer messages.
diff --git a/MAINTAINERS b/MAINTAINERS
index 91e573466d6b..eeeb343ee91c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22522,7 +22522,9 @@ F: include/linux/wiegand.h
WIEGAND GPIO BITBANG DRIVER
M: Martin Zaťovič <m.zatovic1@gmail.com>
S: Maintained
+F: Documentation/ABI/testing/sysfs-driver-wiegand-gpio
F: Documentation/devicetree/bindings/wiegand/wiegand-gpio.yaml
+F: drivers/wiegand/wiegand-gpio.c
WIIMOTE HID DRIVER
M: David Rheinsberg <david.rheinsberg@gmail.com>
diff --git a/drivers/wiegand/Kconfig b/drivers/wiegand/Kconfig
index d6b63250e80b..d3a6c773c767 100644
--- a/drivers/wiegand/Kconfig
+++ b/drivers/wiegand/Kconfig
@@ -18,3 +18,11 @@ config WIEGAND
are initially pulled up. When a bit of value 0 is being transmitted,
the D0 line is pulled down. Similarly, when a bit of value 1 is being
transmitted, the D1 line is pulled down.
+
+config WIEGAND_GPIO
+ tristate "GPIO-based wiegand master (write only)"
+ depends on WIEGAND
+ help
+ This GPIO bitbanging Wiegand controller uses the libgpiod library to
+ utilize GPIO lines for sending Wiegand data. Userspace may access
+ the Wiegand GPIO interface via a dev entry.
diff --git a/drivers/wiegand/Makefile b/drivers/wiegand/Makefile
index d17ecb722c6e..ddf697e21088 100644
--- a/drivers/wiegand/Makefile
+++ b/drivers/wiegand/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_WIEGAND) += wiegand.o
+obj-$(CONFIG_WIEGAND_GPIO) += wiegand-gpio.o
diff --git a/drivers/wiegand/wiegand-gpio.c b/drivers/wiegand/wiegand-gpio.c
new file mode 100644
index 000000000000..e67a30a1c5ae
--- /dev/null
+++ b/drivers/wiegand/wiegand-gpio.c
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/poll.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/wiegand.h>
+
+#define WIEGAND_MAX_PAYLEN_BYTES 256
+
+struct wiegand_gpio {
+ struct device *dev;
+ struct wiegand_controller *ctlr;
+ struct miscdevice misc_dev;
+ struct mutex mutex;
+ struct gpio_desc *gpio_data_hi;
+ struct gpio_desc *gpio_data_lo;
+ struct file_operations fops;
+ u8 data[WIEGAND_MAX_PAYLEN_BYTES];
+};
+
+struct wiegand_gpio_instance {
+ struct wiegand_gpio *dev;
+ unsigned long flags;
+};
+
+static ssize_t store_ulong(u32 *val, const char *buf, size_t size, unsigned long max)
+{
+ int rc;
+ u32 new;
+
+ rc = kstrtou32(buf, 0, &new);
+ if (rc)
+ return rc;
+
+ if (max != 0 && new > max)
+ return -EINVAL;
+
+ *val = new;
+ return size;
+}
+
+/*
+ * Attribute file for setting payload length of Wiegand messages.
+ */
+ssize_t payload_len_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct wiegand_gpio *wiegand_gpio = dev_get_drvdata(dev);
+ struct wiegand_controller *ctlr = wiegand_gpio->ctlr;
+
+ return sysfs_emit(buf, "%u\n", ctlr->payload_len);
+}
+
+ssize_t payload_len_store(struct device *dev, struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ struct wiegand_gpio *wiegand_gpio = dev_get_drvdata(dev);
+ struct wiegand_controller *ctlr = wiegand_gpio->ctlr;
+
+ return store_ulong(&(ctlr->payload_len), buf, count, WIEGAND_MAX_PAYLEN_BYTES * 8);
+}
+DEVICE_ATTR_RW(payload_len);
+
+static struct attribute *wiegand_gpio_attrs[] = {
+ &dev_attr_payload_len.attr,
+ NULL,
+};
+
+ATTRIBUTE_GROUPS(wiegand_gpio);
+
+/*
+ * To send a bit of value 1 following the wiegand protocol, one must set
+ * the wiegand_data_hi to low for the duration of pulse. Similarly to send
+ * a bit of value 0, the wiegand_data_lo is set to low for pulse duration.
+ * This way the two lines are never low at the same time.
+ */
+void wiegand_gpio_send_bit(struct wiegand_gpio *wiegand_gpio, bool value, bool last)
+{
+ u32 pulse_len = wiegand_gpio->ctlr->pulse_len;
+ u32 interval_len = wiegand_gpio->ctlr->interval_len;
+ u32 frame_gap = wiegand_gpio->ctlr->frame_gap;
+ struct gpio_desc *gpio = value ? wiegand_gpio->gpio_data_hi : wiegand_gpio->gpio_data_lo;
+
+ gpiod_set_value_cansleep(gpio, 0);
+ udelay(pulse_len);
+ gpiod_set_value_cansleep(gpio, 1);
+
+ if (last)
+ udelay(frame_gap - pulse_len);
+ else
+ udelay(interval_len - pulse_len);
+}
+
+/* This function is used for writing from file in dev directory */
+static int wiegand_gpio_write_by_bits(struct wiegand_gpio *wiegand_gpio, u16 bitlen)
+{
+ size_t i;
+ bool bit_value, is_last_bit;
+
+ for (i = 0; i < bitlen; i++) {
+ bit_value = ((wiegand_gpio->data[i / 8] >> (7 - (i % 8))) & 0x01);
+ is_last_bit = (i + 1) == bitlen;
+ wiegand_gpio_send_bit(wiegand_gpio, bit_value, is_last_bit);
+ }
+
+ return 0;
+}
+
+static ssize_t wiegand_gpio_get_user_data(struct wiegand_gpio *wiegand_gpio, char __user const *buf,
+ size_t len)
+{
+ size_t rc;
+
+ if (len > WIEGAND_MAX_PAYLEN_BYTES)
+ return -EBADMSG;
+
+ rc = copy_from_user(&wiegand_gpio->data[0], buf, WIEGAND_MAX_PAYLEN_BYTES);
+ if (rc < 0)
+ return rc;
+
+ return len;
+}
+
+static int wiegand_gpio_frelease(struct inode *ino, struct file *filp)
+{
+ struct wiegand_gpio_instance *info = filp->private_data;
+ struct wiegand_gpio *wiegand_gpio = info->dev;
+
+ mutex_lock(&wiegand_gpio->mutex);
+ info->flags = 0;
+ mutex_unlock(&wiegand_gpio->mutex);
+
+ kfree(info);
+
+ return 0;
+}
+
+static ssize_t wiegand_gpio_fwrite(struct file *filp, char __user const *buf, size_t len,
+ loff_t *offset)
+{
+ struct wiegand_gpio_instance *info = filp->private_data;
+ struct wiegand_gpio *wiegand_gpio = info->dev;
+ u32 msg_length = wiegand_gpio->ctlr->payload_len;
+ int rc;
+
+ if (buf == NULL || len == 0 || len * 8 < msg_length)
+ return -EINVAL;
+
+ rc = wiegand_gpio_get_user_data(wiegand_gpio, buf, len);
+ if (rc < 0)
+ return rc;
+
+ wiegand_gpio_write_by_bits(wiegand_gpio, msg_length);
+
+ return len;
+}
+
+static int wiegand_gpio_fopen(struct inode *ino, struct file *filp)
+{
+ int rc;
+ struct wiegand_gpio_instance *info;
+ struct wiegand_gpio *wiegand_gpio = container_of(filp->f_op, struct wiegand_gpio, fops);
+
+ mutex_lock(&wiegand_gpio->mutex);
+
+ if ((filp->f_flags & O_ACCMODE) == O_RDONLY || (filp->f_flags & O_ACCMODE) == O_RDWR) {
+ dev_err(wiegand_gpio->dev, "Device is write only\n");
+ rc = -EIO;
+ goto err;
+ }
+
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ info->dev = wiegand_gpio;
+ info->flags = filp->f_flags;
+ mutex_unlock(&wiegand_gpio->mutex);
+
+ filp->private_data = info;
+
+ return 0;
+err:
+ mutex_unlock(&wiegand_gpio->mutex);
+ return rc;
+}
+
+/* This function is used by device drivers */
+int wiegand_gpio_transfer_message(struct wiegand_device *dev, u8 *message, u8 msg_bitlen)
+{
+ struct wiegand_controller *ctlr = dev->controller;
+ struct wiegand_gpio *wiegand_gpio = wiegand_master_get_devdata(ctlr);
+ u8 msg_bytelength = (msg_bitlen % 8) ? (msg_bitlen / 8) + 1 : (msg_bitlen / 8);
+
+ memcpy(wiegand_gpio->data, message, msg_bytelength);
+ wiegand_gpio_write_by_bits(wiegand_gpio, msg_bitlen);
+
+ return 0;
+}
+
+static int wiegand_gpio_request(struct device *dev, struct wiegand_gpio *wiegand_gpio)
+{
+ /* Get GPIO lines using device tree bindings. */
+ wiegand_gpio->gpio_data_lo = devm_gpiod_get(dev, "data-lo", GPIOD_OUT_HIGH);
+ if (IS_ERR(wiegand_gpio->gpio_data_lo))
+ return PTR_ERR(wiegand_gpio->gpio_data_lo);
+
+ wiegand_gpio->gpio_data_hi = devm_gpiod_get(dev, "data-hi", GPIOD_OUT_HIGH);
+ return PTR_ERR_OR_ZERO(wiegand_gpio->gpio_data_hi);
+}
+
+static int wiegand_gpio_probe(struct platform_device *device)
+{
+ int status;
+ struct wiegand_controller *master;
+ struct wiegand_gpio *wiegand_gpio;
+ struct device *dev = &device->dev;
+
+ master = devm_wiegand_alloc_master(dev, sizeof(*wiegand_gpio));
+ if (!master)
+ return -ENOMEM;
+
+ if (dev->of_node)
+ master->dev.of_node = device->dev.of_node;
+
+ if (status)
+ return status;
+
+ master->transfer_message = &wiegand_gpio_transfer_message;
+ master->payload_len = 26; /* set standard 26-bit format */
+
+ wiegand_gpio = wiegand_master_get_devdata(master);
+ wiegand_gpio->ctlr = master;
+ wiegand_gpio->fops.owner = THIS_MODULE;
+ wiegand_gpio->fops.open = wiegand_gpio_fopen;
+ wiegand_gpio->fops.release = wiegand_gpio_frelease;
+ wiegand_gpio->fops.write = wiegand_gpio_fwrite;
+
+ platform_set_drvdata(device, wiegand_gpio);
+
+ master->bus_num = device->id;
+ wiegand_gpio->dev = dev;
+
+ mutex_init(&wiegand_gpio->mutex);
+
+ status = wiegand_gpio_request(dev, wiegand_gpio);
+ if (status) {
+ dev_err(wiegand_gpio->dev, "failed at requesting GPIOs\n");
+ return status;
+ }
+
+ status = gpiod_direction_output(wiegand_gpio->gpio_data_hi, 1);
+ status |= gpiod_direction_output(wiegand_gpio->gpio_data_lo, 1);
+ if (status) {
+ dev_err(wiegand_gpio->dev, "failed to set GPIOs direction\n");
+ return status;
+ }
+
+ status = devm_wiegand_register_master(dev, master);
+ if (status) {
+ dev_err(wiegand_gpio->dev, "failed to register master\n");
+ return status;
+ }
+
+ wiegand_gpio->misc_dev.name = kasprintf(GFP_KERNEL, "wiegand-gpio%u", master->bus_num);
+ wiegand_gpio->misc_dev.minor = MISC_DYNAMIC_MINOR;
+ wiegand_gpio->misc_dev.fops = &wiegand_gpio->fops;
+
+ status = misc_register(&wiegand_gpio->misc_dev);
+ if (status) {
+ dev_err(wiegand_gpio->dev, "couldn't register misc device\n");
+ return status;
+ }
+ wiegand_gpio->misc_dev.parent = wiegand_gpio->dev;
+
+ device_create_file(dev, &dev_attr_payload_len);
+ dev->groups = wiegand_gpio_groups;
+
+ return status;
+}
+
+static int wiegand_gpio_remove(struct platform_device *device)
+{
+ struct wiegand_gpio *wiegand_gpio = platform_get_drvdata(device);
+
+ misc_deregister(&wiegand_gpio->misc_dev);
+
+ return 0;
+}
+
+static const struct of_device_id wiegand_gpio_dt_idtable[] = {
+ { .compatible = "wiegand-gpio", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, wiegand_gpio_dt_idtable);
+
+static struct platform_driver wiegand_gpio_driver = {
+ .driver = {
+ .name = "wiegand-gpio",
+ .of_match_table = wiegand_gpio_dt_idtable,
+ },
+ .probe = wiegand_gpio_probe,
+ .remove = wiegand_gpio_remove,
+};
+
+module_platform_driver(wiegand_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Wiegand write-only driver realized by GPIOs");
+MODULE_AUTHOR("Martin Zaťovič <m.zatovic1@gmail.com>");
--
2.39.2
On 3/1/23 17:28, Martin Zaťovič wrote:
> This controller formats the data to a Wiegand format and bit-bangs
> the message on devicetree defined GPIO lines.
>
> Several attributes need to be defined in the devicetree in order
> for this driver to work, namely the data-hi-gpios, data-lo-gpios,
> pulse-len, frame-gap and interval-len. These attributes are
> documented in the devicetree bindings documentation files.
>
> The driver creates a dev file for writing messages on the bus.
> It also creates a sysfs file to control the payload length of
> messages(in bits). If a message is shorter than the set payload
> length, it will be discarded. On the other hand, if a message is
> longer, the additional bits will be stripped off.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
> .../ABI/testing/sysfs-driver-wiegand-gpio | 9 +
> MAINTAINERS | 2 +
> drivers/wiegand/Kconfig | 8 +
> drivers/wiegand/Makefile | 1 +
> drivers/wiegand/wiegand-gpio.c | 316 ++++++++++++++++++
> 5 files changed, 336 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-wiegand-gpio
> create mode 100644 drivers/wiegand/wiegand-gpio.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-wiegand-gpio b/Documentation/ABI/testing/sysfs-driver-wiegand-gpio
> new file mode 100644
> index 000000000000..be2246880a83
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-wiegand-gpio
> @@ -0,0 +1,9 @@
> +What: /sys/devices/platform/.../wiegand-gpio-attributes/payload_len
> +Date: January 2023
> +Contact: Martin Zaťovič <m.zatovic1@gmail.com>
> +Description:
> + Read/set the payload length of messages sent by Wiegand GPIO
> + bit-banging controller in bits. The default value is 26, as
> + that is the most widely-used length of Wiegand messages.
> + Controller will only send messages of at least the set length
> + and it will strip off bits of longer messages.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 91e573466d6b..eeeb343ee91c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22522,7 +22522,9 @@ F: include/linux/wiegand.h
> WIEGAND GPIO BITBANG DRIVER
> M: Martin Zaťovič <m.zatovic1@gmail.com>
> S: Maintained
> +F: Documentation/ABI/testing/sysfs-driver-wiegand-gpio
> F: Documentation/devicetree/bindings/wiegand/wiegand-gpio.yaml
> +F: drivers/wiegand/wiegand-gpio.c
>
> WIIMOTE HID DRIVER
> M: David Rheinsberg <david.rheinsberg@gmail.com>
> diff --git a/drivers/wiegand/Kconfig b/drivers/wiegand/Kconfig
> index d6b63250e80b..d3a6c773c767 100644
> --- a/drivers/wiegand/Kconfig
> +++ b/drivers/wiegand/Kconfig
> @@ -18,3 +18,11 @@ config WIEGAND
> are initially pulled up. When a bit of value 0 is being transmitted,
> the D0 line is pulled down. Similarly, when a bit of value 1 is being
> transmitted, the D1 line is pulled down.
> +
> +config WIEGAND_GPIO
> + tristate "GPIO-based wiegand master (write only)"
> + depends on WIEGAND
> + help
> + This GPIO bitbanging Wiegand controller uses the libgpiod library to
> + utilize GPIO lines for sending Wiegand data. Userspace may access
> + the Wiegand GPIO interface via a dev entry.
> diff --git a/drivers/wiegand/Makefile b/drivers/wiegand/Makefile
> index d17ecb722c6e..ddf697e21088 100644
> --- a/drivers/wiegand/Makefile
> +++ b/drivers/wiegand/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_WIEGAND) += wiegand.o
> +obj-$(CONFIG_WIEGAND_GPIO) += wiegand-gpio.o
> diff --git a/drivers/wiegand/wiegand-gpio.c b/drivers/wiegand/wiegand-gpio.c
> new file mode 100644
> index 000000000000..e67a30a1c5ae
> --- /dev/null
> +++ b/drivers/wiegand/wiegand-gpio.c
> @@ -0,0 +1,316 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/poll.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/wiegand.h>
> +
> +#define WIEGAND_MAX_PAYLEN_BYTES 256
> +
> +struct wiegand_gpio {
> + struct device *dev;
> + struct wiegand_controller *ctlr;
> + struct miscdevice misc_dev;
> + struct mutex mutex;
> + struct gpio_desc *gpio_data_hi;
> + struct gpio_desc *gpio_data_lo;
> + struct file_operations fops;
> + u8 data[WIEGAND_MAX_PAYLEN_BYTES];
maybe use bitmap for this?
> +};
> +
> +struct wiegand_gpio_instance {
> + struct wiegand_gpio *dev;
> + unsigned long flags;
> +};
> +
> +static ssize_t store_ulong(u32 *val, const char *buf, size_t size, unsigned long max)
> +{
> + int rc;
> + u32 new;
> +
> + rc = kstrtou32(buf, 0, &new);
> + if (rc)
> + return rc;
> +
> + if (max != 0 && new > max)
> + return -EINVAL;
> +
> + *val = new;
> + return size;
> +}
> +
> +/*
> + * Attribute file for setting payload length of Wiegand messages.
> + */
> +ssize_t payload_len_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct wiegand_gpio *wiegand_gpio = dev_get_drvdata(dev);
> + struct wiegand_controller *ctlr = wiegand_gpio->ctlr;
> +
> + return sysfs_emit(buf, "%u\n", ctlr->payload_len);
> +}
> +
> +ssize_t payload_len_store(struct device *dev, struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + struct wiegand_gpio *wiegand_gpio = dev_get_drvdata(dev);
> + struct wiegand_controller *ctlr = wiegand_gpio->ctlr;
> +
> + return store_ulong(&(ctlr->payload_len), buf, count, WIEGAND_MAX_PAYLEN_BYTES * 8);
> +}
> +DEVICE_ATTR_RW(payload_len);
> +
> +static struct attribute *wiegand_gpio_attrs[] = {
> + &dev_attr_payload_len.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(wiegand_gpio);
> +
> +/*
> + * To send a bit of value 1 following the wiegand protocol, one must set
> + * the wiegand_data_hi to low for the duration of pulse. Similarly to send
> + * a bit of value 0, the wiegand_data_lo is set to low for pulse duration.
> + * This way the two lines are never low at the same time.
> + */
> +void wiegand_gpio_send_bit(struct wiegand_gpio *wiegand_gpio, bool value, bool last)
> +{
> + u32 pulse_len = wiegand_gpio->ctlr->pulse_len;
> + u32 interval_len = wiegand_gpio->ctlr->interval_len;
> + u32 frame_gap = wiegand_gpio->ctlr->frame_gap;
> + struct gpio_desc *gpio = value ? wiegand_gpio->gpio_data_hi : wiegand_gpio->gpio_data_lo;
> +
> + gpiod_set_value_cansleep(gpio, 0);
> + udelay(pulse_len);
> + gpiod_set_value_cansleep(gpio, 1);
> +
> + if (last)
> + udelay(frame_gap - pulse_len);
> + else
> + udelay(interval_len - pulse_len);
> +}
> +
> +/* This function is used for writing from file in dev directory */
> +static int wiegand_gpio_write_by_bits(struct wiegand_gpio *wiegand_gpio, u16 bitlen)
> +{
> + size_t i;
> + bool bit_value, is_last_bit;
> +
> + for (i = 0; i < bitlen; i++) {
> + bit_value = ((wiegand_gpio->data[i / 8] >> (7 - (i % 8))) & 0x01);
> + is_last_bit = (i + 1) == bitlen;
> + wiegand_gpio_send_bit(wiegand_gpio, bit_value, is_last_bit);
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t wiegand_gpio_get_user_data(struct wiegand_gpio *wiegand_gpio, char __user const *buf,
> + size_t len)
> +{
> + size_t rc;
> +
> + if (len > WIEGAND_MAX_PAYLEN_BYTES)
> + return -EBADMSG;
> +
> + rc = copy_from_user(&wiegand_gpio->data[0], buf, WIEGAND_MAX_PAYLEN_BYTES);
> + if (rc < 0)
> + return rc;
> +
> + return len;
> +}
> +
> +static int wiegand_gpio_frelease(struct inode *ino, struct file *filp)
> +{
> + struct wiegand_gpio_instance *info = filp->private_data;
> + struct wiegand_gpio *wiegand_gpio = info->dev;
> +
> + mutex_lock(&wiegand_gpio->mutex);
> + info->flags = 0;
> + mutex_unlock(&wiegand_gpio->mutex);
> +
> + kfree(info);
> +
> + return 0;
> +}
> +
> +static ssize_t wiegand_gpio_fwrite(struct file *filp, char __user const *buf, size_t len,
> + loff_t *offset)
> +{
> + struct wiegand_gpio_instance *info = filp->private_data;
> + struct wiegand_gpio *wiegand_gpio = info->dev;
> + u32 msg_length = wiegand_gpio->ctlr->payload_len;
> + int rc;
> +
> + if (buf == NULL || len == 0 || len * 8 < msg_length)
> + return -EINVAL;
> +
> + rc = wiegand_gpio_get_user_data(wiegand_gpio, buf, len);
> + if (rc < 0)
> + return rc;
> +
> + wiegand_gpio_write_by_bits(wiegand_gpio, msg_length);
> +
> + return len;
> +}
> +
> +static int wiegand_gpio_fopen(struct inode *ino, struct file *filp)
> +{
> + int rc;
> + struct wiegand_gpio_instance *info;
> + struct wiegand_gpio *wiegand_gpio = container_of(filp->f_op, struct wiegand_gpio, fops);
> +
> + mutex_lock(&wiegand_gpio->mutex);
> +
> + if ((filp->f_flags & O_ACCMODE) == O_RDONLY || (filp->f_flags & O_ACCMODE) == O_RDWR) {
> + dev_err(wiegand_gpio->dev, "Device is write only\n");
> + rc = -EIO;
> + goto err;
> + }
> +
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info) {
> + rc = -ENOMEM;
> + goto err;
> + }
> +
> + info->dev = wiegand_gpio;
> + info->flags = filp->f_flags;
> + mutex_unlock(&wiegand_gpio->mutex);
> +
> + filp->private_data = info;
> +
> + return 0;
> +err:
> + mutex_unlock(&wiegand_gpio->mutex);
> + return rc;
> +}
> +
> +/* This function is used by device drivers */
> +int wiegand_gpio_transfer_message(struct wiegand_device *dev, u8 *message, u8 msg_bitlen)
> +{
> + struct wiegand_controller *ctlr = dev->controller;
> + struct wiegand_gpio *wiegand_gpio = wiegand_master_get_devdata(ctlr);
> + u8 msg_bytelength = (msg_bitlen % 8) ? (msg_bitlen / 8) + 1 : (msg_bitlen / 8);
> +
> + memcpy(wiegand_gpio->data, message, msg_bytelength);
> + wiegand_gpio_write_by_bits(wiegand_gpio, msg_bitlen);
> +
> + return 0;
> +}
> +
> +static int wiegand_gpio_request(struct device *dev, struct wiegand_gpio *wiegand_gpio)
> +{
> + /* Get GPIO lines using device tree bindings. */
> + wiegand_gpio->gpio_data_lo = devm_gpiod_get(dev, "data-lo", GPIOD_OUT_HIGH);
> + if (IS_ERR(wiegand_gpio->gpio_data_lo))
> + return PTR_ERR(wiegand_gpio->gpio_data_lo);
> +
> + wiegand_gpio->gpio_data_hi = devm_gpiod_get(dev, "data-hi", GPIOD_OUT_HIGH);
> + return PTR_ERR_OR_ZERO(wiegand_gpio->gpio_data_hi);
> +}
> +
> +static int wiegand_gpio_probe(struct platform_device *device)
> +{
> + int status;
> + struct wiegand_controller *master;
> + struct wiegand_gpio *wiegand_gpio;
> + struct device *dev = &device->dev;
> +
> + master = devm_wiegand_alloc_master(dev, sizeof(*wiegand_gpio));
> + if (!master)
> + return -ENOMEM;
> +
> + if (dev->of_node)
> + master->dev.of_node = device->dev.of_node;
> +
> + if (status)
> + return status;
> +
> + master->transfer_message = &wiegand_gpio_transfer_message;
> + master->payload_len = 26; /* set standard 26-bit format */
> +
> + wiegand_gpio = wiegand_master_get_devdata(master);
> + wiegand_gpio->ctlr = master;
> + wiegand_gpio->fops.owner = THIS_MODULE;
> + wiegand_gpio->fops.open = wiegand_gpio_fopen;
> + wiegand_gpio->fops.release = wiegand_gpio_frelease;
> + wiegand_gpio->fops.write = wiegand_gpio_fwrite;
> +
> + platform_set_drvdata(device, wiegand_gpio);
> +
> + master->bus_num = device->id;
> + wiegand_gpio->dev = dev;
> +
> + mutex_init(&wiegand_gpio->mutex);
> +
> + status = wiegand_gpio_request(dev, wiegand_gpio);
> + if (status) {
> + dev_err(wiegand_gpio->dev, "failed at requesting GPIOs\n");
> + return status;
> + }
> +
> + status = gpiod_direction_output(wiegand_gpio->gpio_data_hi, 1);
> + status |= gpiod_direction_output(wiegand_gpio->gpio_data_lo, 1);
> + if (status) {
> + dev_err(wiegand_gpio->dev, "failed to set GPIOs direction\n");
> + return status;
> + }
> +
> + status = devm_wiegand_register_master(dev, master);
> + if (status) {
> + dev_err(wiegand_gpio->dev, "failed to register master\n");
> + return status;
> + }
> +
> + wiegand_gpio->misc_dev.name = kasprintf(GFP_KERNEL, "wiegand-gpio%u", master->bus_num);
> + wiegand_gpio->misc_dev.minor = MISC_DYNAMIC_MINOR;
> + wiegand_gpio->misc_dev.fops = &wiegand_gpio->fops;
> +
> + status = misc_register(&wiegand_gpio->misc_dev);
> + if (status) {
> + dev_err(wiegand_gpio->dev, "couldn't register misc device\n");
> + return status;
> + }
> + wiegand_gpio->misc_dev.parent = wiegand_gpio->dev;
> +
> + device_create_file(dev, &dev_attr_payload_len);
As far as I understand, Wiegand GPIO bit-banged driver is just one of
the many possible driver implementations. I can imagine having dedicated
hardware to send Wiegand messages. So it seems a little strange to have
chardev API implemented in the bit-banged driver and not in the core, so
it's common for all possible driver implementations.
> + dev->groups = wiegand_gpio_groups;
> +
> + return status;
> +}
> +
> +static int wiegand_gpio_remove(struct platform_device *device)
> +{
> + struct wiegand_gpio *wiegand_gpio = platform_get_drvdata(device);
> +
> + misc_deregister(&wiegand_gpio->misc_dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id wiegand_gpio_dt_idtable[] = {
> + { .compatible = "wiegand-gpio", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, wiegand_gpio_dt_idtable);
> +
> +static struct platform_driver wiegand_gpio_driver = {
> + .driver = {
> + .name = "wiegand-gpio",
> + .of_match_table = wiegand_gpio_dt_idtable,
> + },
> + .probe = wiegand_gpio_probe,
> + .remove = wiegand_gpio_remove,
> +};
> +
> +module_platform_driver(wiegand_gpio_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Wiegand write-only driver realized by GPIOs");
> +MODULE_AUTHOR("Martin Zaťovič <m.zatovic1@gmail.com>");
--
Kind regards,
Evgeny Boger
CTO @ Wiren Board
Hi Martin,
I love your patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.2 next-20230301]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Martin-Za-ovi/dt-bindings-wiegand-add-Wiegand-controller-common-properties/20230301-223030
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230301142835.19614-5-m.zatovic1%40gmail.com
patch subject: [PATCHv3 4/4] wiegand: add Wiegand GPIO bitbanged controller driver
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20230302/202303020937.pxOPT5nt-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/641c36b9878a19ea4977f0e14df22c7475b423df
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Martin-Za-ovi/dt-bindings-wiegand-add-Wiegand-controller-common-properties/20230301-223030
git checkout 641c36b9878a19ea4977f0e14df22c7475b423df
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303020937.pxOPT5nt-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/wiegand/wiegand-gpio.c:50:9: warning: no previous prototype for 'payload_len_show' [-Wmissing-prototypes]
50 | ssize_t payload_len_show(struct device *dev, struct device_attribute *attr, char *buf)
| ^~~~~~~~~~~~~~~~
>> drivers/wiegand/wiegand-gpio.c:58:9: warning: no previous prototype for 'payload_len_store' [-Wmissing-prototypes]
58 | ssize_t payload_len_store(struct device *dev, struct device_attribute *attr, const char *buf,
| ^~~~~~~~~~~~~~~~~
>> drivers/wiegand/wiegand-gpio.c:81:6: warning: no previous prototype for 'wiegand_gpio_send_bit' [-Wmissing-prototypes]
81 | void wiegand_gpio_send_bit(struct wiegand_gpio *wiegand_gpio, bool value, bool last)
| ^~~~~~~~~~~~~~~~~~~~~
>> drivers/wiegand/wiegand-gpio.c:195:5: warning: no previous prototype for 'wiegand_gpio_transfer_message' [-Wmissing-prototypes]
195 | int wiegand_gpio_transfer_message(struct wiegand_device *dev, u8 *message, u8 msg_bitlen)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/payload_len_show +50 drivers/wiegand/wiegand-gpio.c
46
47 /*
48 * Attribute file for setting payload length of Wiegand messages.
49 */
> 50 ssize_t payload_len_show(struct device *dev, struct device_attribute *attr, char *buf)
51 {
52 struct wiegand_gpio *wiegand_gpio = dev_get_drvdata(dev);
53 struct wiegand_controller *ctlr = wiegand_gpio->ctlr;
54
55 return sysfs_emit(buf, "%u\n", ctlr->payload_len);
56 }
57
> 58 ssize_t payload_len_store(struct device *dev, struct device_attribute *attr, const char *buf,
59 size_t count)
60 {
61 struct wiegand_gpio *wiegand_gpio = dev_get_drvdata(dev);
62 struct wiegand_controller *ctlr = wiegand_gpio->ctlr;
63
64 return store_ulong(&(ctlr->payload_len), buf, count, WIEGAND_MAX_PAYLEN_BYTES * 8);
65 }
66 DEVICE_ATTR_RW(payload_len);
67
68 static struct attribute *wiegand_gpio_attrs[] = {
69 &dev_attr_payload_len.attr,
70 NULL,
71 };
72
73 ATTRIBUTE_GROUPS(wiegand_gpio);
74
75 /*
76 * To send a bit of value 1 following the wiegand protocol, one must set
77 * the wiegand_data_hi to low for the duration of pulse. Similarly to send
78 * a bit of value 0, the wiegand_data_lo is set to low for pulse duration.
79 * This way the two lines are never low at the same time.
80 */
> 81 void wiegand_gpio_send_bit(struct wiegand_gpio *wiegand_gpio, bool value, bool last)
82 {
83 u32 pulse_len = wiegand_gpio->ctlr->pulse_len;
84 u32 interval_len = wiegand_gpio->ctlr->interval_len;
85 u32 frame_gap = wiegand_gpio->ctlr->frame_gap;
86 struct gpio_desc *gpio = value ? wiegand_gpio->gpio_data_hi : wiegand_gpio->gpio_data_lo;
87
88 gpiod_set_value_cansleep(gpio, 0);
89 udelay(pulse_len);
90 gpiod_set_value_cansleep(gpio, 1);
91
92 if (last)
93 udelay(frame_gap - pulse_len);
94 else
95 udelay(interval_len - pulse_len);
96 }
97
98 /* This function is used for writing from file in dev directory */
99 static int wiegand_gpio_write_by_bits(struct wiegand_gpio *wiegand_gpio, u16 bitlen)
100 {
101 size_t i;
102 bool bit_value, is_last_bit;
103
104 for (i = 0; i < bitlen; i++) {
105 bit_value = ((wiegand_gpio->data[i / 8] >> (7 - (i % 8))) & 0x01);
106 is_last_bit = (i + 1) == bitlen;
107 wiegand_gpio_send_bit(wiegand_gpio, bit_value, is_last_bit);
108 }
109
110 return 0;
111 }
112
113 static ssize_t wiegand_gpio_get_user_data(struct wiegand_gpio *wiegand_gpio, char __user const *buf,
114 size_t len)
115 {
116 size_t rc;
117
118 if (len > WIEGAND_MAX_PAYLEN_BYTES)
119 return -EBADMSG;
120
121 rc = copy_from_user(&wiegand_gpio->data[0], buf, WIEGAND_MAX_PAYLEN_BYTES);
122 if (rc < 0)
123 return rc;
124
125 return len;
126 }
127
128 static int wiegand_gpio_frelease(struct inode *ino, struct file *filp)
129 {
130 struct wiegand_gpio_instance *info = filp->private_data;
131 struct wiegand_gpio *wiegand_gpio = info->dev;
132
133 mutex_lock(&wiegand_gpio->mutex);
134 info->flags = 0;
135 mutex_unlock(&wiegand_gpio->mutex);
136
137 kfree(info);
138
139 return 0;
140 }
141
142 static ssize_t wiegand_gpio_fwrite(struct file *filp, char __user const *buf, size_t len,
143 loff_t *offset)
144 {
145 struct wiegand_gpio_instance *info = filp->private_data;
146 struct wiegand_gpio *wiegand_gpio = info->dev;
147 u32 msg_length = wiegand_gpio->ctlr->payload_len;
148 int rc;
149
150 if (buf == NULL || len == 0 || len * 8 < msg_length)
151 return -EINVAL;
152
153 rc = wiegand_gpio_get_user_data(wiegand_gpio, buf, len);
154 if (rc < 0)
155 return rc;
156
157 wiegand_gpio_write_by_bits(wiegand_gpio, msg_length);
158
159 return len;
160 }
161
162 static int wiegand_gpio_fopen(struct inode *ino, struct file *filp)
163 {
164 int rc;
165 struct wiegand_gpio_instance *info;
166 struct wiegand_gpio *wiegand_gpio = container_of(filp->f_op, struct wiegand_gpio, fops);
167
168 mutex_lock(&wiegand_gpio->mutex);
169
170 if ((filp->f_flags & O_ACCMODE) == O_RDONLY || (filp->f_flags & O_ACCMODE) == O_RDWR) {
171 dev_err(wiegand_gpio->dev, "Device is write only\n");
172 rc = -EIO;
173 goto err;
174 }
175
176 info = kzalloc(sizeof(*info), GFP_KERNEL);
177 if (!info) {
178 rc = -ENOMEM;
179 goto err;
180 }
181
182 info->dev = wiegand_gpio;
183 info->flags = filp->f_flags;
184 mutex_unlock(&wiegand_gpio->mutex);
185
186 filp->private_data = info;
187
188 return 0;
189 err:
190 mutex_unlock(&wiegand_gpio->mutex);
191 return rc;
192 }
193
194 /* This function is used by device drivers */
> 195 int wiegand_gpio_transfer_message(struct wiegand_device *dev, u8 *message, u8 msg_bitlen)
196 {
197 struct wiegand_controller *ctlr = dev->controller;
198 struct wiegand_gpio *wiegand_gpio = wiegand_master_get_devdata(ctlr);
199 u8 msg_bytelength = (msg_bitlen % 8) ? (msg_bitlen / 8) + 1 : (msg_bitlen / 8);
200
201 memcpy(wiegand_gpio->data, message, msg_bytelength);
202 wiegand_gpio_write_by_bits(wiegand_gpio, msg_bitlen);
203
204 return 0;
205 }
206
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Wed, Mar 01, 2023 at 03:28:35PM +0100, Martin Zaťovič wrote:
> This controller formats the data to a Wiegand format and bit-bangs
> the message on devicetree defined GPIO lines.
>
> Several attributes need to be defined in the devicetree in order
> for this driver to work, namely the data-hi-gpios, data-lo-gpios,
> pulse-len, frame-gap and interval-len. These attributes are
> documented in the devicetree bindings documentation files.
>
> The driver creates a dev file for writing messages on the bus.
> It also creates a sysfs file to control the payload length of
> messages(in bits). If a message is shorter than the set payload
> length, it will be discarded. On the other hand, if a message is
> longer, the additional bits will be stripped off.
...
> +Date: January 2023
Blast from the past?
...
> +config WIEGAND_GPIO
> + tristate "GPIO-based wiegand master (write only)"
> + depends on WIEGAND
> + help
> + This GPIO bitbanging Wiegand controller uses the libgpiod library to
> + utilize GPIO lines for sending Wiegand data. Userspace may access
> + the Wiegand GPIO interface via a dev entry.
What will be the name of the module if M?
...
> +#include <linux/of.h>
No way.
...
> +struct wiegand_gpio {
> + struct device *dev;
> + struct wiegand_controller *ctlr;
> + struct miscdevice misc_dev;
Make it first, same idea as per previous patch comments.
> + struct mutex mutex;
> + struct gpio_desc *gpio_data_hi;
> + struct gpio_desc *gpio_data_lo;
> + struct file_operations fops;
> + u8 data[WIEGAND_MAX_PAYLEN_BYTES];
Have you considered DMA alignment? Is it a problem or not here?
> +};
...
> +static ssize_t store_ulong(u32 *val, const char *buf, size_t size, unsigned long max)
> +{
> + int rc;
> + u32 new;
> + if (max != 0 && new > max)
First part of the conditional is redundant. When you have such a user, you may
add the restriction back.
> + return -EINVAL;
> +
> + *val = new;
> + return size;
> +}
...
> +static struct attribute *wiegand_gpio_attrs[] = {
> + &dev_attr_payload_len.attr,
> + NULL,
No comma for the terminator entry.
> +};
> +
Redundant blank line.
> +ATTRIBUTE_GROUPS(wiegand_gpio);
...
> +void wiegand_gpio_send_bit(struct wiegand_gpio *wiegand_gpio, bool value, bool last)
> +{
> + u32 pulse_len = wiegand_gpio->ctlr->pulse_len;
> + u32 interval_len = wiegand_gpio->ctlr->interval_len;
> + u32 frame_gap = wiegand_gpio->ctlr->frame_gap;
> + struct gpio_desc *gpio = value ? wiegand_gpio->gpio_data_hi : wiegand_gpio->gpio_data_lo;
> +
> + gpiod_set_value_cansleep(gpio, 0);
> + udelay(pulse_len);
> + gpiod_set_value_cansleep(gpio, 1);
> +
> + if (last)
> + udelay(frame_gap - pulse_len);
> + else
> + udelay(interval_len - pulse_len);
This is quite dangerous. You may end up with CPU 100% load for a long time
without any way out. What is the range and why udelay() can't be replaced
with usleep_range() for longer waits?
> +}
...
> +/* This function is used for writing from file in dev directory */
> +static int wiegand_gpio_write_by_bits(struct wiegand_gpio *wiegand_gpio, u16 bitlen)
> +{
> + size_t i;
> + bool bit_value, is_last_bit;
> +
> + for (i = 0; i < bitlen; i++) {
> + bit_value = ((wiegand_gpio->data[i / 8] >> (7 - (i % 8))) & 0x01);
Ah, your buffer should probably be a bitmap.
Also consider bitmap_get_value8().
> + is_last_bit = (i + 1) == bitlen;
> + wiegand_gpio_send_bit(wiegand_gpio, bit_value, is_last_bit);
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t wiegand_gpio_get_user_data(struct wiegand_gpio *wiegand_gpio, char __user const *buf,
> + size_t len)
> +{
> + size_t rc;
> +
> + if (len > WIEGAND_MAX_PAYLEN_BYTES)
> + return -EBADMSG;
> + rc = copy_from_user(&wiegand_gpio->data[0], buf, WIEGAND_MAX_PAYLEN_BYTES);
> + if (rc < 0)
> + return rc;
This is wrong. Homework: read the documentation and existing code to see why
and how to fix.
> + return len;
> +}
...
> +static ssize_t wiegand_gpio_fwrite(struct file *filp, char __user const *buf, size_t len,
> + loff_t *offset)
> +{
> + struct wiegand_gpio_instance *info = filp->private_data;
> + struct wiegand_gpio *wiegand_gpio = info->dev;
> + u32 msg_length = wiegand_gpio->ctlr->payload_len;
> + int rc;
> +
> + if (buf == NULL || len == 0 || len * 8 < msg_length)
!buf
DIV_ROUND_UP(msg_length / 8) > len
less overflow prone.
> + return -EINVAL;
> +
> + rc = wiegand_gpio_get_user_data(wiegand_gpio, buf, len);
> + if (rc < 0)
> + return rc;
> +
> + wiegand_gpio_write_by_bits(wiegand_gpio, msg_length);
> +
> + return len;
> +}
> +static int wiegand_gpio_fopen(struct inode *ino, struct file *filp)
> +{
> + int rc;
> + struct wiegand_gpio_instance *info;
> + struct wiegand_gpio *wiegand_gpio = container_of(filp->f_op, struct wiegand_gpio, fops);
> +
> + mutex_lock(&wiegand_gpio->mutex);
Can it be interrupted by a signal?
> + if ((filp->f_flags & O_ACCMODE) == O_RDONLY || (filp->f_flags & O_ACCMODE) == O_RDWR) {
> + dev_err(wiegand_gpio->dev, "Device is write only\n");
> + rc = -EIO;
> + goto err;
> + }
> +
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info) {
> + rc = -ENOMEM;
> + goto err;
> + }
> +
> + info->dev = wiegand_gpio;
> + info->flags = filp->f_flags;
> + mutex_unlock(&wiegand_gpio->mutex);
> +
> + filp->private_data = info;
> +
> + return 0;
> +err:
> + mutex_unlock(&wiegand_gpio->mutex);
> + return rc;
> +}
...
> + u8 msg_bytelength = (msg_bitlen % 8) ? (msg_bitlen / 8) + 1 : (msg_bitlen / 8);
DIV_ROUND_UP() (you will need math.h)
...
> + if (dev->of_node)
> + master->dev.of_node = device->dev.of_node;
No.
...
> + if (status)
> + return status;
What's this and why is it here?
I'm afraid you haven't compiled this code... :-(
...
> + master->transfer_message = &wiegand_gpio_transfer_message;
> + master->payload_len = 26; /* set standard 26-bit format */
Can you replace master with some of the suggested words?
Or is this a terminology from the specification of the bus?
...
> + status = wiegand_gpio_request(dev, wiegand_gpio);
> + if (status) {
> + dev_err(wiegand_gpio->dev, "failed at requesting GPIOs\n");
> + return status;
return dev_error_probe();
Ditto for the rest.
> + }
...
> + status = gpiod_direction_output(wiegand_gpio->gpio_data_hi, 1);
> + status |= gpiod_direction_output(wiegand_gpio->gpio_data_lo, 1);
Huh?!
...
> + wiegand_gpio->misc_dev.name = kasprintf(GFP_KERNEL, "wiegand-gpio%u", master->bus_num);
No checks?
...
> + dev->groups = wiegand_gpio_groups;
Feels like this can be moved to dev_groups member of the struct driver.
> +
> + return status;
> +}
...
> +static const struct of_device_id wiegand_gpio_dt_idtable[] = {
> + { .compatible = "wiegand-gpio", },
> + {},
No comma for the terminator entry.
> +};
...
> +
Redundant blank line.
> +module_platform_driver(wiegand_gpio_driver);
--
With Best Regards,
Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.