[RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

[RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

[Martin Zaťovič]

This patch documents the devicetree entry for enabling Wiegand
bus driver. The drivers that will use Wiegand bus driver shall
create a sub-node of the documented node.

Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
---
Hello again,

this is the second round of RFC patches in an attempt to add Wiegand
driver to linux kernel. Thank you for all the issues you have pointed
out in the first round. I have tried to fix all of them and I have
also implemented a Wiegand bus driver, that is now used by the GPIO
driver itself - as suggested by Linus.

Any advice you have for me regarding the patches will be appreciated!

With regards,
Martin Zaťovič
---
 .../devicetree/bindings/bus/wiegand.yaml      | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/wiegand.yaml

diff --git a/Documentation/devicetree/bindings/bus/wiegand.yaml b/Documentation/devicetree/bindings/bus/wiegand.yaml
new file mode 100644
index 000000000000..1ed863ab925c
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/wiegand.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/wiegand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Wiegand Bus
+
+maintainers:
+  - Martin Zaťovič <m.zatovic1@gmail.com>
+
+description: |
+  Wiegand interface is a wiring standard popularized in the 1980s. To this day
+  many card readers, fingerprint readers, sensors, etc. use Wiegand interface
+  particularly for access control applications. It utilizes two wires to
+  transmit the data - D0 and D1.
+
+  Both data lines are initially pulled up. To send a bit of value 1, the D1
+  line is set low. Similarly to send a bit of value 0, the D0 line is set low.
+
+properties:
+  $nodename:
+    pattern: "^wiegand(@[0-9a-f]+)?$"
+
+  compatible:
+    contains:
+      const: wiegand
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    wiegand {
+        compatible = "wiegand";
+
+        wiegand-gpio {
+            compatible = "wiegand,wiegand-gpio";
+            pinctrl-names = "default";
+            pinctrl-0 = <&pinctrl_uart2_wiegand>;
+            data-hi-gpios = <&gpio 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+            data-lo-gpios = <&gpio 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+        };
+    };
+
+...
-- 
2.37.3

[RFCv2 PATCH 2/4] bus: add Wiegand bus driver

[Martin Zaťovič]

The Wiegand bus driver spawns devices and matches them with
drivers.

Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
---
The bus driver currently assumes that any new Wiegand driver will
have a matching entry in the devicetree. It is currently sufficient
as I will only be implementing the GPIO driver. If someone
implements a Wiegand driver that will not use devicetree, he will
also have to edit this bus driver, in order to match properly. Is
that a correct approach?
---
 drivers/bus/Kconfig     |   6 +
 drivers/bus/Makefile    |   2 +
 drivers/bus/wiegand.c   | 339 ++++++++++++++++++++++++++++++++++++++++
 include/linux/wiegand.h | 110 +++++++++++++
 4 files changed, 457 insertions(+)
 create mode 100644 drivers/bus/wiegand.c
 create mode 100644 include/linux/wiegand.h

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 7bfe998f3514..9675f5a13ffb 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -241,6 +241,12 @@ config VEXPRESS_CONFIG
 	  Platform configuration infrastructure for the ARM Ltd.
 	  Versatile Express.
 
+config WIEGAND
+    tristate "Wiegand bus"
+    help
+      Wiegand Protocol is a low level 2-wire serial protocol. This
+      enables the support of the bus.
+
 config DA8XX_MSTPRI
 	bool "TI da8xx master peripheral priority driver"
 	depends on ARCH_DAVINCI_DA8XX
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index d90eed189a65..4a9fa6314e54 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -36,6 +36,8 @@ obj-$(CONFIG_TI_SYSC)		+= ti-sysc.o
 obj-$(CONFIG_TS_NBUS)		+= ts-nbus.o
 obj-$(CONFIG_UNIPHIER_SYSTEM_BUS)	+= uniphier-system-bus.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
+obj-$(CONFIG_WIEGAND)		+= wiegand.o
+
 
 obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
 
diff --git a/drivers/bus/wiegand.c b/drivers/bus/wiegand.c
new file mode 100644
index 000000000000..c76026d0c3b1
--- /dev/null
+++ b/drivers/bus/wiegand.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/slab.h>
+#include <linux/wiegand.h>
+
+static const struct {
+	const char *name;
+	const char *desc;
+} wiegand_module_table[] = {
+	{ NULL,  NULL },
+	{ "wiegand-gpio", "Wiegand GPIO write-only master" },
+};
+
+int wiegand_gpio_calc_parity8(u8 v)
+{
+	v = (v >> 4) ^ (v & ((1 << 4)-1));
+	v = (v >> 2) ^ (v & ((1 << 2)-1));
+	v = (v >> 1) ^ (v & ((1 << 1)-1));
+	return v;
+}
+EXPORT_SYMBOL_GPL(wiegand_gpio_calc_parity8);
+
+void wiegand_gpio_add_parity_to_data(unsigned char *tmp, u8 *data,
+						enum wiegand_format fmt)
+{
+	switch (fmt) {
+	case WIEGAND_V26:
+		data[0] = (tmp[0] >> 1) | (wiegand_gpio_calc_parity8(
+						tmp[0] ^ (tmp[1] & 0xf0)) << 7);
+		data[1] = (tmp[0] << 7) | (tmp[1] >> 1);
+		data[2] = (tmp[1] << 7) | (tmp[2] >> 1);
+		data[3] = (tmp[2] << 7) | (!wiegand_gpio_calc_parity8(
+						(tmp[1] & 0x0f) ^ tmp[2]) << 6);
+		break;
+	case WIEGAND_V36:
+		tmp[4] &= 0xc0;
+
+		data[0] = (tmp[0] >> 1) | (wiegand_gpio_calc_parity8(
+				tmp[0] ^ tmp[1] ^ (tmp[2] & 0x80)) << 7);
+		data[1] = (tmp[0] << 7) | (tmp[1] >> 1);
+		data[2] = (tmp[1] << 7) | (tmp[2] >> 1);
+		data[3] = (tmp[2] << 7) | (tmp[3] >> 1);
+		data[4] = (tmp[3] << 7) | (tmp[4] >> 1) |
+			(!wiegand_gpio_calc_parity8(
+				(tmp[2] & 0x7f) ^ tmp[3] ^ tmp[4]) << 4);
+		break;
+	case WIEGAND_V37:
+		tmp[4] &= 0xe0;
+
+		data[0] = (tmp[0] >> 1) | (wiegand_gpio_calc_parity8(
+				tmp[0] ^ tmp[1] ^ (tmp[2] & 0xc0)) << 7);
+		data[1] = (tmp[0] << 7) | (tmp[1] >> 1);
+		data[2] = (tmp[1] << 7) | (tmp[2] >> 1);
+		data[3] = (tmp[2] << 7) | (tmp[3] >> 1);
+		data[4] = (tmp[3] << 7) | (tmp[4] >> 1) |
+				(!wiegand_gpio_calc_parity8(
+				(tmp[2] & 0x7f) ^ tmp[3] ^ tmp[4]) << 3);
+		break;
+	default:
+		WARN_ON(fmt != WIEGAND_V37 &&
+			fmt != WIEGAND_V36 &&
+			fmt != WIEGAND_V26);
+	}
+}
+EXPORT_SYMBOL_GPL(wiegand_gpio_add_parity_to_data);
+
+static inline bool wiegand_module_known(unsigned int id)
+{
+	return id >= WIEGAND_MODULE_FIRST && id <= WIEGAND_MODULE_LAST;
+}
+
+static inline const char *wiegand_module_name(unsigned int id)
+{
+	if (wiegand_module_known(id))
+		return wiegand_module_table[id].name;
+	else
+		return "unknown";
+}
+
+static int wiegand_match(struct device *dev, struct device_driver *drv)
+{
+	struct wiegand_device *wdev = to_wiegand_device(dev);
+	struct wiegand_driver *wdrv = to_wiegand_driver(drv);
+	const enum wiegand_module_id *t;
+
+	if (of_driver_match_device(dev, drv))
+		return 1;
+
+	if (!wdrv->id_table)
+		return 0;
+
+	for (t = wdrv->id_table; *t; ++t)
+		if (*t == wdev->id)
+			return 1;
+
+	return 0;
+}
+
+static struct bus_type wiegand_bus_type = {
+	.name		= "wiegand",
+	.match		= wiegand_match,
+};
+
+int __wiegand_register_driver(struct module *owner,
+			     struct wiegand_driver *wdrv)
+{
+	wdrv->driver.owner = owner;
+	wdrv->driver.bus = &wiegand_bus_type;
+	return driver_register(&wdrv->driver);
+}
+EXPORT_SYMBOL_GPL(__wiegand_register_driver);
+
+static int wiegand_dev_check(struct device *dev, void *data)
+{
+	struct wiegand_device *wdev = to_wiegand_device(dev);
+	struct wiegand_device *new_dev = data;
+
+	if (wdev->wiegand == new_dev->wiegand && wdev->id == new_dev->id &&
+	    wdev->idx == new_dev->idx)
+		return -EBUSY;
+	return 0;
+}
+
+static void wiegand_dev_release(struct device *dev)
+{
+	struct wiegand_device *wdev = to_wiegand_device(dev);
+
+	put_device(wdev->wiegand->dev);
+	kfree(wdev);
+}
+
+static struct wiegand_device *
+wiegand_alloc_device(struct wiegand *wiegand)
+{
+	struct wiegand_device *dev;
+
+	if (!get_device(wiegand->dev))
+		return NULL;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
+		put_device(wiegand->dev);
+		return NULL;
+	}
+
+	dev->wiegand = wiegand;
+	dev->dev.parent = wiegand->dev;
+	dev->dev.bus = &wiegand_bus_type;
+	dev->dev.release = wiegand_dev_release;
+
+	device_initialize(&dev->dev);
+
+	return dev;
+}
+
+static int wiegand_add_device(struct wiegand_device *dev)
+{
+	static DEFINE_MUTEX(add_mutex);
+	int ret;
+
+	if (dev->idx >= WIEGAND_MAX_MODULES)
+		return -EINVAL;
+
+	dev_set_name(&dev->dev, "wiegand-%s.%u", wiegand_module_name(dev->id),
+		     dev->idx);
+
+	mutex_lock(&add_mutex);
+
+	ret = bus_for_each_dev(&wiegand_bus_type, NULL, dev,
+			       wiegand_dev_check);
+	if (ret)
+		goto done;
+
+	ret = device_add(&dev->dev);
+	if (ret < 0)
+		dev_err(dev->wiegand->dev, "Can't add %s, status %d\n",
+			dev_name(dev->wiegand->dev), ret);
+
+done:
+	mutex_unlock(&add_mutex);
+	return ret;
+}
+
+static int __unregister(struct device *dev, void *null)
+{
+	if (dev->of_node)
+		of_node_put(dev->of_node);
+
+	device_unregister(dev);
+
+	return 0;
+}
+
+static struct wiegand_device *
+of_register_wiegand_device(struct wiegand *wiegand, struct device_node *nc)
+{
+	struct wiegand_device *dev;
+	const char *val;
+	int ret;
+
+	dev = wiegand_alloc_device(wiegand);
+	if (!dev) {
+		dev_err(wiegand->dev,
+			"Wiegand device alloc error for %pOF\n", nc);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ret = of_property_read_string(nc, "compatible", &val);
+	if (ret) {
+		dev_err(wiegand->dev, "%pOF has no valid 'compatible' property (%d)\n",
+			nc, ret);
+		goto err_put;
+	}
+
+	if (strcmp(val, "wiegand,wiegand-gpio") == 0) {
+		dev->idx = 0;
+		dev->id = WIEGAND_MODULE_GPIO;
+		wiegand->modules[0] = dev->id;
+		wiegand->count++;
+	}
+
+	of_node_get(nc);
+	dev->dev.of_node = nc;
+
+	ret = wiegand_add_device(dev);
+	if (ret) {
+		dev_err(wiegand->dev,
+			"Wiegand device register error for %pOF\n", nc);
+		of_node_put(nc);
+		goto err_put;
+	}
+
+	return dev;
+
+err_put:
+	put_device(&dev->dev);
+	return ERR_PTR(ret);
+}
+
+static void of_register_wiegand_devices(struct wiegand *wiegand)
+{
+	struct wiegand_device *dev;
+	struct device_node *nc;
+
+	if (!wiegand->dev->of_node)
+		return;
+
+	for_each_available_child_of_node(wiegand->dev->of_node, nc) {
+		dev = of_register_wiegand_device(wiegand, nc);
+		if (IS_ERR(dev)) {
+			dev_warn(wiegand->dev,
+				"Failed to create Wiegand device for %pOF\n",
+				nc);
+		}
+	}
+}
+
+static int wiegand_probe(struct platform_device *pdev)
+{
+	struct wiegand *wiegand;
+
+	wiegand = devm_kzalloc(&pdev->dev, sizeof(struct wiegand),
+			      GFP_KERNEL);
+	if (!wiegand)
+		return -ENOMEM;
+
+	wiegand->dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, wiegand);
+
+	mutex_init(&wiegand->lock);
+
+	of_register_wiegand_devices(wiegand);
+
+	return 0;
+}
+
+static int wiegand_remove(struct platform_device *pdev)
+{
+	struct wiegand *wiegand = dev_get_drvdata(&pdev->dev);
+
+	device_for_each_child(&pdev->dev, NULL, __unregister);
+
+	mutex_destroy(&wiegand->lock);
+
+	return 0;
+}
+
+static const struct of_device_id wiegand_dt_ids[] = {
+	{ .compatible = "wiegand" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, wiegand_dt_ids);
+
+static struct platform_driver wiegand_driver = {
+	.driver = {
+		.name		= "wiegand",
+		.of_match_table = wiegand_dt_ids,
+	},
+	.probe		= wiegand_probe,
+	.remove		= wiegand_remove,
+};
+
+static int __init wiegand_init(void)
+{
+	int ret;
+
+	ret = bus_register(&wiegand_bus_type);
+	if (ret < 0) {
+		pr_err("Wiegand bus registration failed: %d\n", ret);
+		goto error;
+	}
+
+	ret = platform_driver_register(&wiegand_driver);
+	if (ret < 0) {
+		pr_err("Wiegand driver registration failed: %d\n", ret);
+		goto error_bus;
+	}
+
+	return 0;
+
+error_bus:
+	bus_unregister(&wiegand_bus_type);
+error:
+	return ret;
+}
+postcore_initcall_sync(wiegand_init);
+
+static void __exit wiegand_exit(void)
+{
+	platform_driver_unregister(&wiegand_driver);
+	bus_unregister(&wiegand_bus_type);
+}
+module_exit(wiegand_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Wiegand bus driver");
+MODULE_AUTHOR("Martin Zaťovič <m.zatovic1@gmail.com>");
diff --git a/include/linux/wiegand.h b/include/linux/wiegand.h
new file mode 100644
index 000000000000..98d8671f042e
--- /dev/null
+++ b/include/linux/wiegand.h
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef H_LINUX_INCLUDE_LINUX_WIEGAND_H
+#define H_LINUX_INCLUDE_LINUX_WIEGAND_H
+
+#include <linux/device.h>
+#include <linux/mutex.h>
+
+#define WIEGAND_MAX_PAYLEN_BYTES 256
+#define WIEGAND_MAX_MODULES 1
+
+/* The used wiegand format; when data does not end at octet boundaries, the
+ * lower bits of the last octet will be ignored and only the upper ones will be
+ * used.
+ */
+enum wiegand_format {
+	WIEGAND_V26 = 26,
+	WIEGAND_V36 = 36,
+	WIEGAND_V37 = 37,
+	WIEGAND_CUSTOM = 0,
+};
+
+enum wiegand_module_id {
+	WIEGAND_MODULE_FIRST = 0x01,
+	WIEGAND_MODULE_GPIO = 0x01,
+	WIEGAND_MODULE_LAST = 0x01,
+};
+
+enum wiegand_paylen {
+	WIEGAND_V26_PAYLEN = 24,
+	WIEGAND_V36_PAYLEN = 34,
+	WIEGAND_V37_PAYLEN = 35,
+};
+
+extern struct bus_type wiegand_type;
+
+struct wiegand {
+	struct device *dev;
+	struct mutex lock;
+	int count;
+	u8 modules[WIEGAND_MAX_MODULES];
+};
+
+struct wiegand_driver {
+	const enum wiegand_module_id *id_table;
+	struct device_driver driver;
+};
+
+struct wiegand_device {
+	struct device dev;
+	struct wiegand *wiegand;
+	enum wiegand_module_id id;
+	unsigned int idx;
+};
+
+extern int __wiegand_register_driver(struct module *owner,
+				    struct wiegand_driver *mdrv);
+
+static inline void wiegand_unregister_driver(struct wiegand_driver *mdrv)
+{
+	if (mdrv)
+		driver_unregister(&mdrv->driver);
+}
+
+#define wiegand_register_driver(driver) \
+	__wiegand_register_driver(THIS_MODULE, driver)
+
+#define module_wiegand_driver(__wiegand_driver) \
+	module_driver(__wiegand_driver, wiegand_register_driver, \
+			wiegand_unregister_driver)
+
+static inline struct wiegand_driver *
+to_wiegand_driver(struct device_driver *drv)
+{
+	if (!drv)
+		return NULL;
+	return container_of(drv, struct wiegand_driver, driver);
+}
+
+static inline struct wiegand_device *
+to_wiegand_device(struct device *dev)
+{
+	if (!dev)
+		return NULL;
+	return container_of(dev, struct wiegand_device, dev);
+}
+
+/**
+ * struct wiegand_setup - Wiegand communication attributes
+ * @pulse_len: length of the low pulse in usec; defaults to 50us
+ * @interval_len: length of a whole bit (both the pulse and the high phase) in
+ *	usec; defaults to 2000us
+ * @frame_gap: length of the last bit of a frame (both the pulse and the high
+ *	phase) in usec; defaults to interval_len
+ * @format: the used wiegand format
+ * @payload_len: payload of wiegand message in bits
+ */
+struct wiegand_setup {
+	unsigned long		pulse_len;
+	unsigned long		interval_len;
+	unsigned long		frame_gap;
+	enum wiegand_format	format;
+	unsigned long		payload_len;
+};
+
+extern int wiegand_gpio_calc_parity8(u8 v);
+extern void wiegand_gpio_add_parity_to_data(unsigned char *tmp, u8 *data,
+						enum wiegand_format fmt);
+
+#endif	/* H_LINUX_INCLUDE_LINUX_WIEGAND_H */
-- 
2.37.3

[RFCv2 PATCH 3/4] dt-bindings: gpio: add Wiegand GPIO driver dt documentation

[Martin Zaťovič]

The Wiegand GPIO driver uses two GPIO lines to transmit data -
data-hi and data-lo. These lines need to be defined in the
devicetree, otherwise the driver will not probe successfully.

Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
---
 .../bindings/gpio/gpio-wiegand.yaml           | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml

diff --git a/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml b/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
new file mode 100644
index 000000000000..3b235667ae17
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-wiegand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Wiegand GPIO controller
+
+description: |
+  Wiegand GPIO controller running under Wiegand bus.
+
+maintainers:
+  - Martin Zaťovič <m.zatovic1@gmail.com>
+
+properties:
+  $nodename:
+    pattern: "^wiegand-gpio@[0-9a-f]+$"
+
+  compatible:
+    const: wiegand,wiegand-gpio
+
+  data-hi-gpios:
+    description: GPIO spec for data-hi line to use
+    maxItems: 1
+
+  data-lo-gpios:
+    description: GPIO spec for data-lo line to use
+    maxItems: 1
+
+required:
+  - compatible
+  - data-hi-gpios
+  - data-lo-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    wiegand {
+        compatible = "wiegand";
+
+        wiegand-gpio {
+            compatible = "wiegand,wiegand-gpio";
+            pinctrl-names = "default";
+            pinctrl-0 = <&pinctrl_uart2_wiegand>;
+            data-hi-gpios = <&gpio2 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+            data-lo-gpios = <&gpio2 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+        };
+    };
+
+...
-- 
2.37.3

[RFCv2 PATCH 4/4] gpio: add Wiegand GPIO driver

[Martin Zaťovič]

Wiegand GPIO driver uses GPIO lines defined in the devicetree to
transmit data following the Wiegand protocol.

Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
---
 .../ABI/testing/sysfs-driver-gpio-wiegand     |  40 ++
 MAINTAINERS                                   |   9 +
 drivers/gpio/Kconfig                          |   7 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-wiegand.c                   | 492 ++++++++++++++++++
 5 files changed, 549 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-gpio-wiegand
 create mode 100644 drivers/gpio/gpio-wiegand.c

diff --git a/Documentation/ABI/testing/sysfs-driver-gpio-wiegand b/Documentation/ABI/testing/sysfs-driver-gpio-wiegand
new file mode 100644
index 000000000000..efcce06968ef
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-gpio-wiegand
@@ -0,0 +1,40 @@
+What:		/sys/devices/platform/.../wiegand-gpio-attributes/format
+Date:		October 2022
+Contact:	Martin Zaťovič <m.zatovic1@gmail.com>
+Description:    Sets a format of Wiegand communication. Currently supported
+        formats are 26, 36 and 37 bits. These formats automatically add
+        checksums to the first and last bits of a message. To set a custom
+        format, 0 needs to be written here. Custom format writes an amount of
+        bits defined by payload_len(see below) and it does not add checksums so
+        the user is responsible for that.
+Users:		any user space application which wants to communicate using Wiegand
+
+What:		/sys/devices/platform/.../wiegand-gpio-attributes/payload_len
+Date:		October 2022
+Contact:	Martin Zaťovič <sampaio.ime@gmail.com>
+Description:    Depends on format attribute - using one of the standard formats
+        the payload_len attribute file becomes read-only and it contains the
+        number of bits of a message without the checksum bits(e.g. 24 for
+        26-bit format). Using a custom format makes this file writable for
+        configuring the Wiegand message length in bits.
+Users:		any user space application which wants to communicate using Wiegand
+
+What: /sys/devices/platform/.../wiegand-gpio-attributes/pulse_len
+Date:       October 2022
+Contact:    Martin Zaťovič
+Description:    Length of the low pulse in usecs; defaults to 50us.
+Users:		any user space application which wants to communicate using Wiegand
+
+What: /sys/devices/platform/.../wiegand-gpio-attributes/interval_len
+Date:       October 2022
+Contact:    Martin Zaťovič
+Description:    Length of the whole bit(both the pulse and the high phase) in
+        usecs; defaults to 2000us
+Users:		any user space application which wants to communicate using Wiegand
+
+What: /sys/devices/platform/.../wiegand-gpio-attributes/frame_gap
+Date:       October 2022
+Contact:    Martin Zaťovič
+Description:    Length of the last bit of a frame(both the pulse and the high
+        phase) in usecs; defaults to interval_len
+Users:		any user space application which wants to communicate using Wiegand
diff --git a/MAINTAINERS b/MAINTAINERS
index 8656ab794786..43b21bd5333a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21960,6 +21960,15 @@ L:	linux-input@vger.kernel.org
 S:	Maintained
 F:	drivers/hid/hid-wiimote*
 
+WIEGAND DRIVER
+M:	Martin Zaťovič <m.zatovic1@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/bus/wiegand.yaml
+F:	Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
+F:	drivers/bus/wiegand.c
+F:	drivers/gpio/gpio-wiegand.c
+F:	include/linux/wiegand.h
+
 WILOCITY WIL6210 WIRELESS DRIVER
 L:	linux-wireless@vger.kernel.org
 S:	Orphan
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0642f579196f..3cd57e28df19 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -703,6 +703,13 @@ config GPIO_VX855
 	  Additional drivers must be enabled in order to use the
 	  functionality of the device.
 
+config GPIO_WIEGAND
+    tristate "Wiegand GPIO support"
+    depends on WIEGAND
+    help
+      This enables the GPIO module for Wiegand protocol communication.
+      The Wiegand bus driver has to be enabled first.
+
 config GPIO_WCD934X
 	tristate "Qualcomm Technologies Inc WCD9340/WCD9341 GPIO controller driver"
 	depends on MFD_WCD934X && OF_GPIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a0985d30f51b..09024e291cc4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -173,6 +173,7 @@ obj-$(CONFIG_GPIO_VISCONTI)		+= gpio-visconti.o
 obj-$(CONFIG_GPIO_VX855)		+= gpio-vx855.o
 obj-$(CONFIG_GPIO_WCD934X)		+= gpio-wcd934x.o
 obj-$(CONFIG_GPIO_WHISKEY_COVE)		+= gpio-wcove.o
+obj-$(CONFIG_GPIO_WIEGAND)		+= gpio-wiegand.o
 obj-$(CONFIG_GPIO_WINBOND)		+= gpio-winbond.o
 obj-$(CONFIG_GPIO_WM831X)		+= gpio-wm831x.o
 obj-$(CONFIG_GPIO_WM8350)		+= gpio-wm8350.o
diff --git a/drivers/gpio/gpio-wiegand.c b/drivers/gpio/gpio-wiegand.c
new file mode 100644
index 000000000000..b45e39010fd6
--- /dev/null
+++ b/drivers/gpio/gpio-wiegand.c
@@ -0,0 +1,492 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/poll.h>
+#include <linux/miscdevice.h>
+#include <linux/of.h>
+#include <linux/gpio/consumer.h>
+#include <linux/wiegand.h>
+
+struct wiegand_gpio_device *wiegand_gpio_glob;
+
+struct wiegand_gpio_device {
+	struct device *dev;
+	struct miscdevice *misc_dev;
+	struct mutex mutex;
+	struct gpio_desc *gpio_data_hi;
+	struct gpio_desc *gpio_data_lo;
+	struct wiegand_setup setup;
+	u8 data[WIEGAND_MAX_PAYLEN_BYTES];
+};
+
+struct wiegand_gpio_instance {
+	struct wiegand_gpio_device *dev;
+	unsigned long flags;
+};
+
+static const struct wiegand_setup WIEGAND_SETUP = {
+	.pulse_len	= 50,
+	.interval_len	= 2000,
+	.frame_gap	= 2000,
+	.format		= WIEGAND_V26,
+	.payload_len	= 24,
+};
+
+/*
+ * 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 ar the same time.
+ */
+void wiegand_gpio_send_bit(struct wiegand_gpio_device *wiegand_gpio,
+				bool value, bool last)
+{
+	struct wiegand_setup *setup = &wiegand_gpio->setup;
+	struct gpio_desc *gpio = value ? wiegand_gpio->gpio_data_hi
+				: wiegand_gpio->gpio_data_lo;
+
+	gpiod_set_value_cansleep(gpio, 0);
+	udelay(setup->pulse_len);
+	gpiod_set_value_cansleep(gpio, 1);
+
+	if (last)
+		udelay(setup->frame_gap - setup->pulse_len);
+	else
+		udelay(setup->interval_len - setup->pulse_len);
+}
+
+/* This function is used for custom format */
+static int wiegand_gpio_write_by_bits(struct wiegand_gpio_device *wiegand_gpio,
+				size_t len)
+{
+	size_t i, bitlen;
+	bool bit_value, is_last_bit;
+	struct wiegand_setup *setup = &wiegand_gpio->setup;
+
+	bitlen = setup->format ? setup->payload_len + 2 : setup->payload_len;
+
+	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, (bool)bit_value,
+				is_last_bit);
+	}
+
+	return 0;
+}
+
+static unsigned int wiegand_gpio_calc_bytes(unsigned int bits)
+{
+	if (bits % 8 != 0)
+		return (bits / 8) + 1;
+	else
+		return bits / 8;
+}
+
+static unsigned int wiegand_gpio_get_payload_size(
+						unsigned long payload_len_bits,
+						enum wiegand_format fmt)
+{
+	unsigned int ret;
+
+	if (fmt == WIEGAND_CUSTOM)
+		ret = wiegand_gpio_calc_bytes(payload_len_bits);
+	else
+		/* add 2 for parity bits */
+		ret = wiegand_gpio_calc_bytes(payload_len_bits + 2);
+
+	return ret;
+}
+
+static ssize_t wiegand_gpio_get_user_data(
+				struct wiegand_gpio_device *wiegand_gpio,
+				char __user const *buf, size_t len)
+{
+	size_t rc;
+	size_t num_copy;
+	unsigned char tmp[WIEGAND_MAX_PAYLEN_BYTES];
+	struct wiegand_setup *setup = &wiegand_gpio->setup;
+
+	num_copy = wiegand_gpio_get_payload_size(setup->payload_len,
+								setup->format);
+
+	if (setup->format == WIEGAND_CUSTOM) {
+		rc = copy_from_user(&wiegand_gpio->data[0], buf, num_copy);
+		if (rc < 0)
+			return rc;
+	} else {
+		rc = copy_from_user(tmp, buf, num_copy);
+		if (rc < 0)
+			return rc;
+		wiegand_gpio_add_parity_to_data(tmp, wiegand_gpio->data,
+								setup->format);
+	}
+	return num_copy;
+}
+
+static int wiegand_gpio_release(struct inode *ino, struct file *filp)
+{
+	struct wiegand_gpio_instance *info = filp->private_data;
+	struct wiegand_gpio_device *wiegand_gpio = info->dev;
+
+	mutex_lock(&wiegand_gpio->mutex);
+	mutex_unlock(&wiegand_gpio->mutex);
+
+	kfree(info);
+
+	return 0;
+}
+
+static ssize_t wiegand_gpio_write(struct file *filp,
+		char __user const *buf, size_t len, loff_t *offset)
+{
+	struct wiegand_gpio_instance *info = filp->private_data;
+	struct wiegand_gpio_device *wiegand_gpio = info->dev;
+	int rc;
+
+	if (len * 8 < wiegand_gpio->setup.payload_len)
+		return -EBADMSG;
+	if (buf == NULL || len == 0)
+		return -EINVAL;
+
+	wiegand_gpio_get_user_data(wiegand_gpio, buf, len);
+	rc = wiegand_gpio_write_by_bits(wiegand_gpio, len);
+
+	return len;
+}
+
+static int wiegand_gpio_open(struct inode *ino, struct file *filp)
+{
+	struct wiegand_gpio_device *wiegand_gpio;
+	struct wiegand_gpio_instance *info;
+	int rc;
+
+	wiegand_gpio = wiegand_gpio_glob;
+
+	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;
+	}
+
+	gpiod_direction_output(wiegand_gpio->gpio_data_hi, 1);
+	gpiod_direction_output(wiegand_gpio->gpio_data_lo, 1);
+
+	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;
+}
+
+const struct file_operations wiegand_gpio_fops = {
+	.owner		= THIS_MODULE,
+	.open		= wiegand_gpio_open,
+	.release	= wiegand_gpio_release,
+	.write		= wiegand_gpio_write,
+};
+
+static struct miscdevice wiegand_misc_device = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "wiegand-gpio",
+	.fops = &wiegand_gpio_fops,
+};
+
+static ssize_t store_ulong(unsigned long *val, const char *buf,
+				size_t size, unsigned long max)
+{
+	int ret;
+	unsigned long new;
+
+	ret = kstrtoul(buf, 0, &new);
+	if (ret)
+		return ret;
+	if (max != 0 && new > max)
+		return -EINVAL;
+
+	*val = new;
+	return size;
+}
+
+ssize_t pulse_len_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	return sysfs_emit(buf, "%lu\n", device->setup.pulse_len);
+}
+
+ssize_t pulse_len_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	return store_ulong(&(device->setup.pulse_len), buf, count, 0);
+}
+
+ssize_t interval_len_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	return sysfs_emit(buf, "%lu\n", device->setup.pulse_len);
+}
+
+ssize_t interval_len_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	return store_ulong(&(device->setup.interval_len), buf, count, 0);
+}
+
+ssize_t frame_gap_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	return sysfs_emit(buf, "%lu\n", device->setup.frame_gap);
+}
+
+ssize_t frame_gap_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	return store_ulong(&(device->setup.frame_gap), buf, count, 0);
+}
+
+ssize_t format_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	return sysfs_emit(buf, "%u\n", device->setup.format);
+}
+
+/*
+ * To set a particular format, the number of bits the driver is supposed to
+ * transmit is written to the format sysfs file. For standard formats, the
+ * allowed inputs are 26, 36 and 37. To set a custom format, 0 is passed.
+ */
+ssize_t format_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	int ret;
+	unsigned long new;
+	enum wiegand_format *val;
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	val = &(device->setup.format);
+
+	ret = kstrtoul(buf, 0, &new);
+	if (ret)
+		return ret;
+
+	switch (new) {
+	case 0:
+		*val = WIEGAND_CUSTOM;
+		break;
+	case 26:
+		*val = WIEGAND_V26;
+		device->setup.payload_len = WIEGAND_V26_PAYLEN;
+		break;
+	case 36:
+		*val = WIEGAND_V36;
+		device->setup.payload_len = WIEGAND_V36_PAYLEN;
+		break;
+	case 37:
+		*val = WIEGAND_V37;
+		device->setup.payload_len = WIEGAND_V37_PAYLEN;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+/*
+ * Using a custom format, the payload_len sysfs file configures the size of
+ * messages payload in bits. When one of the standard formats is used, this
+ * file is read-only and contains the size of the message in bits without the
+ * parity bits.
+ */
+ssize_t payload_len_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	return sysfs_emit(buf, "%lu\n", device->setup.pulse_len);
+}
+
+ssize_t payload_len_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct wiegand_gpio_device *device = dev_get_drvdata(dev);
+
+	if (!device)
+		return -ENODEV;
+
+	/* standard formats use fixed payload size */
+	if (device->setup.format != WIEGAND_CUSTOM)
+		return -EPERM;
+
+	return store_ulong(&(device->setup.payload_len), buf, count,
+					WIEGAND_MAX_PAYLEN_BYTES * 8);
+}
+
+DEVICE_ATTR_RW(pulse_len);
+DEVICE_ATTR_RW(interval_len);
+DEVICE_ATTR_RW(frame_gap);
+DEVICE_ATTR_RW(format);
+DEVICE_ATTR_RW(payload_len);
+
+static struct attribute *wiegand_attrs[] = {
+	&dev_attr_pulse_len.attr,
+	&dev_attr_interval_len.attr,
+	&dev_attr_frame_gap.attr,
+	&dev_attr_format.attr,
+	&dev_attr_payload_len.attr,
+	NULL,
+};
+
+static struct attribute_group wiegand_group = {
+	.name = "wiegand-gpio-attributes",
+	.attrs = wiegand_attrs,
+};
+
+static int wiegand_gpio_dev_probe(struct device *device)
+{
+	int rc;
+	struct wiegand_gpio_device *wiegand_gpio;
+
+	wiegand_gpio = devm_kzalloc(device, sizeof(*wiegand_gpio), GFP_KERNEL);
+	if (!wiegand_gpio)
+		return -ENOMEM;
+
+	wiegand_gpio->dev = device;
+	wiegand_gpio->misc_dev = &wiegand_misc_device;
+
+	wiegand_gpio_glob = wiegand_gpio;
+
+	/* Get GPIO lines using device tree bindings. */
+	wiegand_gpio->gpio_data_lo = devm_gpiod_get(wiegand_gpio->dev,
+			"data-lo", GPIOD_OUT_HIGH);
+	if (IS_ERR(wiegand_gpio->gpio_data_lo)) {
+		return dev_err_probe(wiegand_gpio->dev,
+				PTR_ERR(wiegand_gpio->gpio_data_lo),
+				"Can't get data-lo line.");
+	}
+	wiegand_gpio->gpio_data_hi = devm_gpiod_get(wiegand_gpio->dev,
+			"data-hi", GPIOD_OUT_HIGH);
+	if (IS_ERR(wiegand_gpio->gpio_data_hi)) {
+		return dev_err_probe(wiegand_gpio->dev,
+				PTR_ERR(wiegand_gpio->gpio_data_hi),
+				"Can't get data-hi line.");
+	}
+
+	rc = sysfs_create_group(&wiegand_gpio->dev->kobj, &wiegand_group);
+	if (rc < 0) {
+		dev_err(wiegand_gpio->dev, "Couldn't register sysfs group\n");
+		return rc;
+	}
+
+	mutex_init(&wiegand_gpio->mutex);
+
+	misc_register(wiegand_gpio->misc_dev);
+	wiegand_gpio->misc_dev->parent = wiegand_gpio->dev;
+
+	memcpy(&wiegand_gpio->setup, &WIEGAND_SETUP,
+			sizeof(struct wiegand_setup));
+
+	dev_set_drvdata(device, wiegand_gpio);
+
+	return 0;
+}
+
+static int wiegand_gpio_dev_remove(struct device *device)
+{
+	struct wiegand_gpio_device *wiegand_gpio = dev_get_drvdata(device);
+
+	sysfs_remove_group(&wiegand_gpio->dev->kobj, &wiegand_group);
+
+	misc_deregister(&wiegand_misc_device);
+	kfree(wiegand_gpio);
+
+	return 0;
+}
+
+static const struct of_device_id wiegand_gpio_dt_idtable[] = {
+	{ .compatible = "wiegand,wiegand-gpio", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, wiegand_gpio_dt_idtable);
+
+static const enum wiegand_module_id wiegand_gpio_module_table[] = {
+	WIEGAND_MODULE_GPIO,
+	0,
+};
+
+static struct wiegand_driver wiegand_gpio_driver = {
+	.driver = {
+		.name	= "wiegand-gpio",
+		.probe		= wiegand_gpio_dev_probe,
+		.remove		= wiegand_gpio_dev_remove,
+		.of_match_table = wiegand_gpio_dt_idtable,
+	},
+	.id_table = wiegand_gpio_module_table,
+};
+
+module_wiegand_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.37.3

Re: [RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

[Rob Herring]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2392 bytes --]

On Wed, 05 Oct 2022 16:57:43 +0200, Martin Zaťovič wrote:
> This patch documents the devicetree entry for enabling Wiegand
> bus driver. The drivers that will use Wiegand bus driver shall
> create a sub-node of the documented node.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
> Hello again,
> 
> this is the second round of RFC patches in an attempt to add Wiegand
> driver to linux kernel. Thank you for all the issues you have pointed
> out in the first round. I have tried to fix all of them and I have
> also implemented a Wiegand bus driver, that is now used by the GPIO
> driver itself - as suggested by Linus.
> 
> Any advice you have for me regarding the patches will be appreciated!
> 
> With regards,
> Martin Zaťovič
> ---
>  .../devicetree/bindings/bus/wiegand.yaml      | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/wiegand.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/wiegand.example.dtb: wiegand: 'wiegand-gpio' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/wiegand.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/wiegand.example.dtb: wiegand: wiegand-gpio: {'compatible': ['wiegand,wiegand-gpio'], 'pinctrl-names': ['default'], 'pinctrl-0': [[4294967295]], 'data-hi-gpios': [[4294967295, 7, 6]], 'data-lo-gpios': [[4294967295, 6, 6]]} is not of type 'array'
	From schema: /usr/local/lib/python3.10/dist-packages/dtschema/schemas/gpio/gpio-consumer.yaml
Documentation/devicetree/bindings/bus/wiegand.example.dtb:0:0: /example-0/wiegand/wiegand-gpio: failed to match any schema with compatible: ['wiegand,wiegand-gpio']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [RFCv2 PATCH 3/4] dt-bindings: gpio: add Wiegand GPIO driver dt documentation

[Rob Herring]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1960 bytes --]

On Wed, 05 Oct 2022 16:57:45 +0200, Martin Zaťovič wrote:
> The Wiegand GPIO driver uses two GPIO lines to transmit data -
> data-hi and data-lo. These lines need to be defined in the
> devicetree, otherwise the driver will not probe successfully.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
>  .../bindings/gpio/gpio-wiegand.yaml           | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/gpio-wiegand.example.dtb: wiegand: wiegand-gpio: {'compatible': ['wiegand,wiegand-gpio'], 'pinctrl-names': ['default'], 'pinctrl-0': [[4294967295]], 'data-hi-gpios': [[4294967295, 7, 6]], 'data-lo-gpios': [[4294967295, 6, 6]]} is not of type 'array'
	From schema: /usr/local/lib/python3.10/dist-packages/dtschema/schemas/gpio/gpio-consumer.yaml
Documentation/devicetree/bindings/gpio/gpio-wiegand.example.dtb:0:0: /example-0/wiegand: failed to match any schema with compatible: ['wiegand']
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/gpio-wiegand.example.dtb: wiegand-gpio: $nodename:0: 'wiegand-gpio' does not match '^wiegand-gpio@[0-9a-f]+$'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [RFCv2 PATCH 3/4] dt-bindings: gpio: add Wiegand GPIO driver dt documentation

[Krzysztof Kozlowski]

On 05/10/2022 16:57, Martin Zaťovič wrote:
> The Wiegand GPIO driver uses two GPIO lines to transmit data -
> data-hi and data-lo. These lines need to be defined in the
> devicetree, otherwise the driver will not probe successfully.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
>  .../bindings/gpio/gpio-wiegand.yaml           | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml b/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
> new file mode 100644
> index 000000000000..3b235667ae17
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-wiegand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wiegand GPIO controller
> +
> +description: |
> +  Wiegand GPIO controller running under Wiegand bus.

GPIO controllers need "gpio-controller" property, so this seems to be
something else.

> +
> +maintainers:
> +  - Martin Zaťovič <m.zatovic1@gmail.com>
> +
> +properties:
> +  $nodename:
> +    pattern: "^wiegand-gpio@[0-9a-f]+$"

No need to enforce node name, unless this is for a class of devices. But
then why "gpio" not just "wiegand"?

> +
> +  compatible:
> +    const: wiegand,wiegand-gpio
> +
> +  data-hi-gpios:
> +    description: GPIO spec for data-hi line to use
> +    maxItems: 1
> +
> +  data-lo-gpios:
> +    description: GPIO spec for data-lo line to use
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - data-hi-gpios
> +  - data-lo-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    wiegand {
> +        compatible = "wiegand";
> +
> +        wiegand-gpio {

I have troubles understanding this. The "wiegand" node is the bus,
right? Then what is "wiegand-gpio"? GPIO controller? Then why it is not
marked as GPIO controller? What GPIOs does it control?

> +            compatible = "wiegand,wiegand-gpio";
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&pinctrl_uart2_wiegand>;
> +            data-hi-gpios = <&gpio2 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +            data-lo-gpios = <&gpio2 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;

Aren't these properties of the bus, not the device?

> +        };
> +    };
> +
> +...

Best regards,
Krzysztof

Re: [RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

[Krzysztof Kozlowski]

On 05/10/2022 16:57, Martin Zaťovič wrote:
> This patch documents the devicetree entry for enabling Wiegand
> bus driver. The drivers that will use Wiegand bus driver shall
> create a sub-node of the documented node.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
> Hello again,
> 
> this is the second round of RFC patches in an attempt to add Wiegand
> driver to linux kernel. Thank you for all the issues you have pointed
> out in the first round. I have tried to fix all of them and I have
> also implemented a Wiegand bus driver, that is now used by the GPIO
> driver itself - as suggested by Linus.
> 
> Any advice you have for me regarding the patches will be appreciated!
> 
> With regards,
> Martin Zaťovič
> ---
>  .../devicetree/bindings/bus/wiegand.yaml      | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/wiegand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/bus/wiegand.yaml b/Documentation/devicetree/bindings/bus/wiegand.yaml
> new file mode 100644
> index 000000000000..1ed863ab925c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/wiegand.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/wiegand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wiegand Bus
> +
> +maintainers:
> +  - Martin Zaťovič <m.zatovic1@gmail.com>
> +
> +description: |
> +  Wiegand interface is a wiring standard popularized in the 1980s. To this day
> +  many card readers, fingerprint readers, sensors, etc. use Wiegand interface
> +  particularly for access control applications. It utilizes two wires to
> +  transmit the data - D0 and D1.
> +
> +  Both data lines are initially pulled up. To send a bit of value 1, the D1
> +  line is set low. Similarly to send a bit of value 0, the D0 line is set low.
> +
> +properties:
> +  $nodename:
> +    pattern: "^wiegand(@[0-9a-f]+)?$"
> +
> +  compatible:
> +    contains:
> +      const: wiegand
> +

If the bus uses two wires, shouldn't you describe them here? Otherwise
what wires are you using?

> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    wiegand {
> +        compatible = "wiegand";
> +
> +        wiegand-gpio {
> +            compatible = "wiegand,wiegand-gpio";
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&pinctrl_uart2_wiegand>;
> +            data-hi-gpios = <&gpio 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +            data-lo-gpios = <&gpio 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +        };
> +    };
> +
> +...

Best regards,
Krzysztof

Re: [RFCv2 PATCH 2/4] bus: add Wiegand bus driver

[Krzysztof Kozlowski]

On 05/10/2022 16:57, Martin Zaťovič wrote:
> The Wiegand bus driver spawns devices and matches them with
> drivers.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
> The bus driver currently assumes that any new Wiegand driver will
> have a matching entry in the devicetree. It is currently sufficient
> as I will only be implementing the GPIO driver. If someone
> implements a Wiegand driver that will not use devicetree, he will
> also have to edit this bus driver, in order to match properly. Is
> that a correct approach?

(...)

> +static struct wiegand_device *
> +of_register_wiegand_device(struct wiegand *wiegand, struct device_node *nc)
> +{
> +	struct wiegand_device *dev;
> +	const char *val;
> +	int ret;
> +
> +	dev = wiegand_alloc_device(wiegand);
> +	if (!dev) {
> +		dev_err(wiegand->dev,
> +			"Wiegand device alloc error for %pOF\n", nc);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	ret = of_property_read_string(nc, "compatible", &val);
> +	if (ret) {
> +		dev_err(wiegand->dev, "%pOF has no valid 'compatible' property (%d)\n",
> +			nc, ret);
> +		goto err_put;
> +	}
> +
> +	if (strcmp(val, "wiegand,wiegand-gpio") == 0) {

This does not look right. Bus can have any device attached, so limiting
some bus behavior to a specific device is not really scalable.

Anyway device and node matching should not be with strcmp but rather
of_*_match().

Best regards,
Krzysztof

Re: [RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

[Krzysztof Kozlowski]

On 05/10/2022 16:57, Martin Zaťovič wrote:
> This patch documents the devicetree entry for enabling Wiegand
> bus driver. The drivers that will use Wiegand bus driver shall
> create a sub-node of the documented node.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
> Hello again,
> 

Missing cover letter and changelog.

Best regards,
Krzysztof

Re: [RFCv2 PATCH 3/4] dt-bindings: gpio: add Wiegand GPIO driver dt documentation

[Krzysztof Kozlowski]

On 06/10/2022 10:23, Krzysztof Kozlowski wrote:
> On 05/10/2022 16:57, Martin Zaťovič wrote:
>> The Wiegand GPIO driver uses two GPIO lines to transmit data -
>> data-hi and data-lo. These lines need to be defined in the
>> devicetree, otherwise the driver will not probe successfully.
>>
>> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
>> ---
>>  .../bindings/gpio/gpio-wiegand.yaml           | 53 +++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml b/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
>> new file mode 100644
>> index 000000000000..3b235667ae17
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-wiegand.yaml
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/gpio/gpio-wiegand.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Wiegand GPIO controller
>> +
>> +description: |
>> +  Wiegand GPIO controller running under Wiegand bus.
> 
> GPIO controllers need "gpio-controller" property, so this seems to be
> something else.
> 
>> +
>> +maintainers:
>> +  - Martin Zaťovič <m.zatovic1@gmail.com>
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^wiegand-gpio@[0-9a-f]+$"
> 
> No need to enforce node name, unless this is for a class of devices. But
> then why "gpio" not just "wiegand"?
> 
>> +
>> +  compatible:
>> +    const: wiegand,wiegand-gpio
>> +
>> +  data-hi-gpios:
>> +    description: GPIO spec for data-hi line to use
>> +    maxItems: 1
>> +
>> +  data-lo-gpios:
>> +    description: GPIO spec for data-lo line to use
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - data-hi-gpios
>> +  - data-lo-gpios
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    wiegand {
>> +        compatible = "wiegand";
>> +
>> +        wiegand-gpio {
> 
> I have troubles understanding this. The "wiegand" node is the bus,
> right? Then what is "wiegand-gpio"? GPIO controller? Then why it is not
> marked as GPIO controller? What GPIOs does it control?
> 
>> +            compatible = "wiegand,wiegand-gpio";
>> +            pinctrl-names = "default";
>> +            pinctrl-0 = <&pinctrl_uart2_wiegand>;
>> +            data-hi-gpios = <&gpio2 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
>> +            data-lo-gpios = <&gpio2 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> 
> Aren't these properties of the bus, not the device?

So this looks like specific implementation of Wiegand bus - a Wiegand
bus controller. If it is correct, it should not be represented as child
of a bus... because this is a bus. IOW, just like SPI or I2C controllers
(why is this one different?), the bus is defined by SPI controller:

wiegand {
	compatible = "wiegand,wiegand-gpio";
	data-hi-gpios = <&gpio2 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
	data-lo-gpios = <&gpio2 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;

	child-device, e.g. some-card {
		compatible = "foo,bar";
		// more properties of the device
	};
};

Best regards,
Krzysztof

Re: [RFCv2 PATCH 4/4] gpio: add Wiegand GPIO driver

[Bartosz Golaszewski]

On Wed, Oct 5, 2022 at 4:58 PM Martin Zaťovič <m.zatovic1@gmail.com> wrote:
>
> Wiegand GPIO driver uses GPIO lines defined in the devicetree to
> transmit data following the Wiegand protocol.
>
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---

[snip]

> +
> +DEVICE_ATTR_RW(pulse_len);
> +DEVICE_ATTR_RW(interval_len);
> +DEVICE_ATTR_RW(frame_gap);
> +DEVICE_ATTR_RW(format);
> +DEVICE_ATTR_RW(payload_len);
> +

We don't really allow GPIO drivers to define all kinds of custom
device attributes. Also: this driver does not register a GPIO provider
- rather it's a GPIO consumer.

For what you're trying to achieve: have you tried using libgpiod and
controlling the lines from user-space? If that's too slow, then I'd
say this driver should go somewhere else. Maybe you'd need a whole new
protocol sub-system for that. In any case - this subsystem is not the
right place.

Bartosz

[snip]

Re: [RFCv2 PATCH 2/4] bus: add Wiegand bus driver

[Linus Walleij]

On Wed, Oct 5, 2022 at 4:58 PM Martin Zaťovič <m.zatovic1@gmail.com> wrote:

> The Wiegand bus driver spawns devices and matches them with
> drivers.
>
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>

Overall this is a vast improvement over the first patches!

I might not have time to look closer right now, but I see you got
a lot of comments from Krzysztof et al so you have some stuff to
work on.

Looking forward to the first non-RFC patch series!

Yours,
Linus Walleij