wt., 12 maj 2020 o 16:41 Michael Walle [off-list ref] napisał(a):

quoted

quoted

+
+MODULE_AUTHOR("Michael Walle [off-list ref]");
+MODULE_DESCRIPTION("GPIO generic regmap driver core");
+MODULE_LICENSE("GPL");

diff --git a/include/linux/gpio-regmap.h b/include/linux/gpio-regmap.h
new file mode 100644
index 000000000000..a868cbcde6e9
--- /dev/null
+++ b/include/linux/gpio-regmap.h

@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _LINUX_GPIO_REGMAP_H
+#define _LINUX_GPIO_REGMAP_H
+
+struct gpio_regmap;
+
+#define GPIO_REGMAP_ADDR_ZERO ((unsigned long)(-1))
+#define GPIO_REGMAP_ADDR(addr) ((addr) ? : GPIO_REGMAP_ADDR_ZERO)
+

What if the addr is actually 0?

Then the driver has to set GPIO_REGMAP_ADDR_ZERO or use the convenience
macro GPIO_REGMAP_ADDR.

So you can have

   struct gpio_regmap_config config = { 0 };
   config.reg_dat_base = 0x10;
   config.reg_dir_out_base = 0x20;

or

   config.reg_dat_base = GPIO_REGMAP_ADDR_ZERO;

or if you can't be sure if the RHS value might be zero:

   config.reg_dat_base = GPIO_REGMAP_ADDR(reg);

quoted

Maybe drop GPIO_REGMAP_ADDR and require users to set unused registers
to GPIO_REGMAP_ADDR_ZERO?

Thats bad because:
  * you'd have to set plenty of unused base registers for a simple driver
  * if there will be additional properties in the future, you have to
touch
    all other drivers, because they are initialized as 0 (ie. valid reg
0).

quoted

quoted

+/**
+ * struct gpio_regmap_config - Description of a generic regmap
gpio_chip.
+ *
+ * @parent:            The parent device
+ * @regmap:            The regmap used to access the registers
+ *                     given, the name of the device is used
+ * @label:             (Optional) Descriptive name for GPIO
controller.
+ *                     If not given, the name of the device is used.
+ * @ngpio:             Number of GPIOs
+ * @reg_dat_base:      (Optional) (in) register base address
+ * @reg_set_base:      (Optional) set register base address
+ * @reg_clr_base:      (Optional) clear register base address
+ * @reg_dir_in_base:   (Optional) out setting register base address
+ * @reg_dir_out_base:  (Optional) in setting register base address

The two above are inverted I think?

good catch.

quoted

Also: why the limitation of only supporting one at a time?

they should be exclusive, either you have a register where you set the
output bits to one, or the input bits. Maybe this need a bit more
context
above. in gpio-mmio.c you can set both and both are used in
set_direction(), but only one is read in get_direction().

That being said, I have no strong opinion wether they should be
exclusive
or not, besides the symmetry of set_/get_direction().

-michael

Sorry for the late response, your comments make sense to me. Are you
going to submit a v4 before the v5.8 merge window?

Bart