[PATCHv4,4/4] wiegand: add Wiegand GPIO bitbanged controller driver
Commit Message
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 | 189 ++++++++++++++++++
5 files changed, 209 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-wiegand-gpio
create mode 100644 drivers/wiegand/wiegand-gpio.c
Comments
Hi Martin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.4-rc1 next-20230510]
[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/20230511-002708
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230510162243.95820-5-m.zatovic1%40gmail.com
patch subject: [PATCHv4 4/4] wiegand: add Wiegand GPIO bitbanged controller driver
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230511/202305110450.jjNwIYfp-lkp@intel.com/config)
compiler: sparc64-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/3eb47f0de6aecc78d72c144b36ccd97f22d908c5
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/20230511-002708
git checkout 3eb47f0de6aecc78d72c144b36ccd97f22d908c5
# 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=sparc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc 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/202305110450.jjNwIYfp-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/wiegand/wiegand-gpio.c:70:6: warning: no previous prototype for 'wiegand_gpio_send_bit' [-Wmissing-prototypes]
70 | void wiegand_gpio_send_bit(struct wiegand_gpio *wiegand_gpio, bool value, bool last)
| ^~~~~~~~~~~~~~~~~~~~~
>> drivers/wiegand/wiegand-gpio.c:111:5: warning: no previous prototype for 'wiegand_gpio_transfer_message' [-Wmissing-prototypes]
111 | int wiegand_gpio_transfer_message(struct wiegand_controller *ctlr)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/wiegand_gpio_send_bit +70 drivers/wiegand/wiegand-gpio.c
63
64 /*
65 * To send a bit of value 1 following the wiegand protocol, one must set
66 * the wiegand_data_hi to low for the duration of pulse. Similarly to send
67 * a bit of value 0, the wiegand_data_lo is set to low for pulse duration.
68 * This way the two lines are never low at the same time.
69 */
> 70 void wiegand_gpio_send_bit(struct wiegand_gpio *wiegand_gpio, bool value, bool last)
71 {
72 u32 sleep_len;
73 u32 pulse_len = wiegand_gpio->ctlr->pulse_len;
74 u32 interval_len = wiegand_gpio->ctlr->interval_len;
75 u32 frame_gap = wiegand_gpio->ctlr->frame_gap;
76 struct gpio_desc *gpio = value ? wiegand_gpio->data1_gpio : wiegand_gpio->data0_gpio;
77
78 gpiod_set_value_cansleep(gpio, 0);
79 udelay(pulse_len);
80 gpiod_set_value_cansleep(gpio, 1);
81
82 if (last)
83 sleep_len = frame_gap - pulse_len;
84 else
85 sleep_len = interval_len - pulse_len;
86
87 if (sleep_len < 10)
88 udelay(sleep_len);
89 else if (sleep_len < 100)
90 usleep_range(sleep_len - UP_TO_100_USEC_DEVIATION,
91 sleep_len + UP_TO_100_USEC_DEVIATION);
92 else
93 usleep_range(sleep_len - MORE_THAN_100_USEC_DEVIATION,
94 sleep_len + MORE_THAN_100_USEC_DEVIATION);
95 }
96
97 static int wiegand_gpio_write_by_bits(struct wiegand_gpio *wiegand_gpio, u16 bitlen)
98 {
99 size_t i;
100 bool bit_value, is_last_bit;
101
102 for (i = 0; i < bitlen; i++) {
103 bit_value = test_bit(i, wiegand_gpio->ctlr->data_bitmap);
104 is_last_bit = (i + 1) == bitlen;
105 wiegand_gpio_send_bit(wiegand_gpio, bit_value, is_last_bit);
106 }
107
108 return 0;
109 }
110
> 111 int wiegand_gpio_transfer_message(struct wiegand_controller *ctlr)
112 {
113 struct wiegand_gpio *wiegand_gpio = wiegand_primary_get_devdata(ctlr);
114 u8 msg_bitlen = ctlr->payload_len;
115
116 wiegand_gpio_write_by_bits(wiegand_gpio, msg_bitlen);
117
118 return 0;
119 }
120
On Wed, May 10, 2023 at 06:22:43PM +0200, 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: May 2023
Taking into account the estimated release date I think this should be changed
to Aug 2023.
...
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/poll.h>
> +#include <linux/slab.h>
> +#include <linux/wiegand.h>
...
> +#define UP_TO_100_USEC_DEVIATION 1
> +#define MORE_THAN_100_USEC_DEVIATION 3
These require some comments. And maybe better naming (depending on the content
of those comments).
...
> +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 (new > max)
> + return -EINVAL;
ERANGE?
> + *val = new;
> + return size;
> +}
...
> + if (sleep_len < 10)
> + udelay(sleep_len);
> + else if (sleep_len < 100)
> + usleep_range(sleep_len - UP_TO_100_USEC_DEVIATION,
> + sleep_len + UP_TO_100_USEC_DEVIATION);
> + else
> + usleep_range(sleep_len - MORE_THAN_100_USEC_DEVIATION,
> + sleep_len + MORE_THAN_100_USEC_DEVIATION);
NIH fsleep()
...
> +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 = test_bit(i, wiegand_gpio->ctlr->data_bitmap);
> + is_last_bit = (i + 1) == bitlen;
This is idempotent from for-loop, so...
> + wiegand_gpio_send_bit(wiegand_gpio, bit_value, is_last_bit);
> + }
unsigned int i;
bool value;
if (bitlen == 0)
return 0;
for (i = 0; i < bitlen - 1; i++) {
value = test_bit(i, wiegand_gpio->ctlr->data_bitmap);
wiegand_gpio_send_bit(wiegand_gpio, value, false);
}
value = test_bit(bitlen - 1, wiegand_gpio->ctlr->data_bitmap);
wiegand_gpio_send_bit(wiegand_gpio, value, true);
> + return 0;
> +}
...
> +static int wiegand_gpio_request(struct device *dev, struct wiegand_gpio *wiegand_gpio)
> +{
> + wiegand_gpio->data0_gpio = devm_gpiod_get(dev, "data-lo", GPIOD_OUT_HIGH);
> + if (IS_ERR(wiegand_gpio->data0_gpio))
> + return PTR_ERR(wiegand_gpio->data0_gpio);
> +
> + wiegand_gpio->data1_gpio = devm_gpiod_get(dev, "data-hi", GPIOD_OUT_HIGH);
> + return PTR_ERR_OR_ZERO(wiegand_gpio->data1_gpio);
Maybe you can use devm_gpiod_get_array()?
> +}
...
> +static int wiegand_gpio_probe(struct platform_device *device)
> +{
> + int status = 0;
Redundant assignment.
> + struct wiegand_controller *primary;
> + struct wiegand_gpio *wiegand_gpio;
> + struct device *dev = &device->dev;
...
> + primary->payload_len = 26; // set standard 26-bit format
Instead of comment, make a self-explanatory definition?
...
> + status = wiegand_register_controller(primary);
> + if (status)
> + dev_err_probe(wiegand_gpio->dev, status, "failed to register primary\n");
Why out of a sudden it uses this device and not real one?
> + return status;
With above
return dev_err_probe(dev, status, "failed to register primary\n");
return 0;
> +}
...
> +static const struct of_device_id wiegand_gpio_dt_idtable[] = {
> + { .compatible = "wiegand-gpio", },
Inner comma is not needed.
> + {}
> +};
...
> +static struct platform_driver wiegand_gpio_driver = {
> + .driver = {
> + .name = "wiegand-gpio",
> + .of_match_table = wiegand_gpio_dt_idtable,
> + .dev_groups = wiegand_gpio_groups
Leave trailing comma when it's not about termination.
> + },
> + .probe = wiegand_gpio_probe
Ditto.
> +};
new file mode 100644
@@ -0,0 +1,9 @@
+What: /sys/devices/platform/.../wiegand-gpio-attributes/payload_len
+Date: May 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 message length of Wiegand messages.
+ Controller will only send messages of at least the set length
+ and it will strip off bits of longer messages.
@@ -22707,7 +22707,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
WILOCITY WIL6210 WIRELESS DRIVER
L: linux-wireless@vger.kernel.org
@@ -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.
@@ -1 +1,2 @@
obj-$(CONFIG_WIEGAND) += wiegand.o
+obj-$(CONFIG_WIEGAND_GPIO) += wiegand-gpio.o
new file mode 100644
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/wiegand.h>
+
+#define UP_TO_100_USEC_DEVIATION 1
+#define MORE_THAN_100_USEC_DEVIATION 3
+
+struct wiegand_gpio {
+ struct device *dev;
+ struct wiegand_controller *ctlr;
+ struct gpio_desc *data1_gpio;
+ struct gpio_desc *data0_gpio;
+};
+
+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 (new > max)
+ return -EINVAL;
+
+ *val = new;
+ return size;
+}
+
+/*
+ * Attribute file for setting payload length of Wiegand messages.
+ */
+static 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);
+}
+
+static 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);
+}
+static 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 sleep_len;
+ 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->data1_gpio : wiegand_gpio->data0_gpio;
+
+ gpiod_set_value_cansleep(gpio, 0);
+ udelay(pulse_len);
+ gpiod_set_value_cansleep(gpio, 1);
+
+ if (last)
+ sleep_len = frame_gap - pulse_len;
+ else
+ sleep_len = interval_len - pulse_len;
+
+ if (sleep_len < 10)
+ udelay(sleep_len);
+ else if (sleep_len < 100)
+ usleep_range(sleep_len - UP_TO_100_USEC_DEVIATION,
+ sleep_len + UP_TO_100_USEC_DEVIATION);
+ else
+ usleep_range(sleep_len - MORE_THAN_100_USEC_DEVIATION,
+ sleep_len + MORE_THAN_100_USEC_DEVIATION);
+}
+
+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 = test_bit(i, wiegand_gpio->ctlr->data_bitmap);
+ is_last_bit = (i + 1) == bitlen;
+ wiegand_gpio_send_bit(wiegand_gpio, bit_value, is_last_bit);
+ }
+
+ return 0;
+}
+
+int wiegand_gpio_transfer_message(struct wiegand_controller *ctlr)
+{
+ struct wiegand_gpio *wiegand_gpio = wiegand_primary_get_devdata(ctlr);
+ u8 msg_bitlen = ctlr->payload_len;
+
+ wiegand_gpio_write_by_bits(wiegand_gpio, msg_bitlen);
+
+ return 0;
+}
+
+static int wiegand_gpio_request(struct device *dev, struct wiegand_gpio *wiegand_gpio)
+{
+ wiegand_gpio->data0_gpio = devm_gpiod_get(dev, "data-lo", GPIOD_OUT_HIGH);
+ if (IS_ERR(wiegand_gpio->data0_gpio))
+ return PTR_ERR(wiegand_gpio->data0_gpio);
+
+ wiegand_gpio->data1_gpio = devm_gpiod_get(dev, "data-hi", GPIOD_OUT_HIGH);
+ return PTR_ERR_OR_ZERO(wiegand_gpio->data1_gpio);
+}
+
+static int wiegand_gpio_probe(struct platform_device *device)
+{
+ int status = 0;
+ struct wiegand_controller *primary;
+ struct wiegand_gpio *wiegand_gpio;
+ struct device *dev = &device->dev;
+
+ primary = wiegand_alloc_controller(dev, sizeof(*wiegand_gpio), false);
+ if (!primary)
+ return -ENOMEM;
+
+ primary->transfer_message = &wiegand_gpio_transfer_message;
+ primary->payload_len = 26; // set standard 26-bit format
+
+ wiegand_gpio = wiegand_primary_get_devdata(primary);
+ wiegand_gpio->ctlr = primary;
+
+ platform_set_drvdata(device, wiegand_gpio);
+
+ primary->bus_num = device->id;
+ wiegand_gpio->dev = dev;
+
+ status = wiegand_gpio_request(dev, wiegand_gpio);
+ if (status)
+ return dev_err_probe(wiegand_gpio->dev, status, "failed at requesting GPIOs\n");
+
+ status = gpiod_direction_output(wiegand_gpio->data1_gpio, 1);
+ if (status)
+ return dev_err_probe(wiegand_gpio->dev, status, "failed to set GPIOs direction\n");
+
+ status = gpiod_direction_output(wiegand_gpio->data0_gpio, 1);
+ if (status)
+ return dev_err_probe(wiegand_gpio->dev, status, "failed to set GPIOs direction\n");
+
+ status = wiegand_register_controller(primary);
+ if (status)
+ dev_err_probe(wiegand_gpio->dev, status, "failed to register primary\n");
+ return status;
+}
+
+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,
+ .dev_groups = wiegand_gpio_groups
+ },
+ .probe = wiegand_gpio_probe
+};
+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>");