Hello all,
It's been a while and here comes the sixth version of the patches.
What was done:
- adjust module dependecies and what it selects in Kconfig
- use managed device resource call when adding gpio chip
- remove redundant code
- cosmetic changes
Thanks,
Mauri
rangediff v5 -> v6:
1: a1bb23f56378 ! 1: 87348f391671 dt-bindings: gpio-cascade: add documentation
@@ Commit message
the cascaded line.
Signed-off-by: Mauri Sandberg <[email protected]>
+ Reviewed-by: Rob Herring <[email protected]>
---
+ v5 -> v6:
+ - added Reviewed-by tag by Rob
v4 -> v5:
- renamed gpio-mux-input -> gpio-cascade
- changed vague term 'pin' to 'upstream line'
2: e0be9df088a9 ! 2: a8e608181bc7 gpio: gpio-cascade: add generic GPIO cascade
@@ Metadata
## Commit message ##
gpio: gpio-cascade: add generic GPIO cascade
- Adds support for a building cascades of GPIO lines. That is, it allows
+ Adds support for building cascades of GPIO lines. That is, it allows
setups when there is one upstream line and multiple cascaded lines, out
of which one can be chosen at a time. The status of the upstream line
- can be conveyd to the selected cascaded line or, vice versa, the status
+ can be conveyed to the selected cascaded line or, vice versa, the status
of the cascaded line can be conveyed to the upstream line.
- A gpio-mux is being used to select, which cascaded GPIO line is being
+ A multiplexer is being used to select, which cascaded GPIO line is being
used at any given time.
At the moment only input direction is supported. In future it should be
@@ Commit message
Signed-off-by: Mauri Sandberg <[email protected]>
---
+ v5 -> v6:
+ - In Kconfig, remove dependency to OF_GPIO and select only MULTIPLEXER
+ - refactor code preferring one-liners
+ - clean up prints, removing them from success-path.
+ - don't explicitly set gpio_chip.of_node as it's done in the GPIO library
+ - use devm_gpiochip_add_data instead of gpiochip_add
v4 -> v5:
- renamed gpio-mux-input -> gpio-cascade. refactored code accordingly
here and there and changed to use new bindings and compatible string
@@ drivers/gpio/Kconfig: config GPIO_VIRTIO
+
+config GPIO_CASCADE
+ tristate "General GPIO cascade"
-+ depends on OF_GPIO
+ select MULTIPLEXER
-+ select MUX_GPIO
+ help
+ Say yes here to enable support for generic GPIO cascade.
+
@@ drivers/gpio/gpio-cascade.c (new)
+
+static int gpio_cascade_get_value(struct gpio_chip *gc, unsigned int offset)
+{
-+ struct gpio_cascade *cas;
++ struct gpio_cascade *cas = chip_to_cascade(gc);
+ int ret;
+
-+ cas = chip_to_cascade(gc);
+ ret = mux_control_select(cas->mux_control, offset);
+ if (ret)
+ return ret;
@@ drivers/gpio/gpio-cascade.c (new)
+
+static int gpio_cascade_probe(struct platform_device *pdev)
+{
-+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct gpio_cascade *cas;
+ struct mux_control *mc;
@@ drivers/gpio/gpio-cascade.c (new)
+ struct gpio_chip *gc;
+ int err;
+
-+ cas = devm_kzalloc(dev, sizeof(struct gpio_cascade), GFP_KERNEL);
-+ if (cas == NULL)
++ cas = devm_kzalloc(dev, sizeof(*cas), GFP_KERNEL);
++ if (!cas)
+ return -ENOMEM;
+
+ mc = devm_mux_control_get(dev, NULL);
-+ if (IS_ERR(mc)) {
-+ err = (int) PTR_ERR(mc);
-+ if (err != -EPROBE_DEFER)
-+ dev_err(dev, "unable to get mux-control: %d\n", err);
-+ return err;
-+ }
++ if (IS_ERR(mc))
++ return dev_err_probe(dev,
++ PTR_ERR(mc),
++ "unable to get mux-control\n");
+
+ cas->mux_control = mc;
+ upstream = devm_gpiod_get(dev, "upstream", GPIOD_IN);
+ if (IS_ERR(upstream)) {
-+ err = (int) PTR_ERR(upstream);
-+ dev_err(dev, "unable to claim upstream GPIO line: %d\n", err);
-+ return err;
++ dev_err(dev, "unable to claim upstream GPIO line\n");
++ return -ENODEV;
+ }
+
+ cas->upstream_line = upstream;
@@ drivers/gpio/gpio-cascade.c (new)
+ gc->label = dev_name(cas->parent);
+ gc->parent = cas->parent;
+ gc->owner = THIS_MODULE;
-+ gc->of_node = np;
+
-+ err = gpiochip_add(&cas->gpio_chip);
++ err = devm_gpiochip_add_data(dev, &cas->gpio_chip, NULL);
+ if (err) {
-+ dev_err(dev, "unable to add gpio chip, err=%d\n", err);
++ dev_err(dev, "unable to add gpio chip\n");
+ return err;
+ }
+
+ platform_set_drvdata(pdev, cas);
-+ dev_info(dev, "registered %u cascaded GPIO lines\n", gc->ngpio);
+ return 0;
+}
+
+static const struct of_device_id gpio_cascade_id[] = {
-+ {
-+ .compatible = "gpio-cascade",
-+ .data = NULL,
-+ },
++ { .compatible = "gpio-cascade" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, gpio_cascade_id);
Mauri Sandberg (2):
dt-bindings: gpio-cascade: add documentation
gpio: gpio-cascade: add generic GPIO cascade
.../bindings/gpio/gpio-cascade.yaml | 103 ++++++++++++++
drivers/gpio/Kconfig | 12 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-cascade.c | 129 ++++++++++++++++++
4 files changed, 245 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-cascade.yaml
create mode 100644 drivers/gpio/gpio-cascade.c
base-commit: f4a20dfac88c06c9b529a41ff4cf9acba8f3fdff
--
2.25.1
Adds support for building cascades of GPIO lines. That is, it allows
setups when there is one upstream line and multiple cascaded lines, out
of which one can be chosen at a time. The status of the upstream line
can be conveyed to the selected cascaded line or, vice versa, the status
of the cascaded line can be conveyed to the upstream line.
A multiplexer is being used to select, which cascaded GPIO line is being
used at any given time.
At the moment only input direction is supported. In future it should be
possible to add support for output direction, too.
Signed-off-by: Mauri Sandberg <[email protected]>
---
v5 -> v6:
- In Kconfig, remove dependency to OF_GPIO and select only MULTIPLEXER
- refactor code preferring one-liners
- clean up prints, removing them from success-path.
- don't explicitly set gpio_chip.of_node as it's done in the GPIO library
- use devm_gpiochip_add_data instead of gpiochip_add
v4 -> v5:
- renamed gpio-mux-input -> gpio-cascade. refactored code accordingly
here and there and changed to use new bindings and compatible string
- ambigious and vague 'pin' was rename to 'upstream_line'
- dropped Tested-by and Reviewed-by due to changes in bindings
- dropped Reported-by suggested by an automatic bot as it was not really
appropriate to begin with
- functionally it's the same as v4
v3 -> v4:
- Changed author email
- Included Tested-by and Reviewed-by from Drew
v2 -> v3:
- use managed device resources
- update Kconfig description
v1 -> v2:
- removed .owner from platform_driver as per test bot's instruction
- added MODULE_AUTHOR, MODULE_DESCRIPTION, MODULE_LICENSE
- added gpio_mux_input_get_direction as it's recommended for all chips
- removed because this is input only chip: gpio_mux_input_set_value
- removed because they are not needed for input/output only chips:
gpio_mux_input_direction_input
gpio_mux_input_direction_output
- fixed typo in an error message
- added info message about successful registration
- removed can_sleep flag as this does not sleep while getting GPIO value
like I2C or SPI do
- Updated description in Kconfig
---
drivers/gpio/Kconfig | 12 ++++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-cascade.c | 129 ++++++++++++++++++++++++++++++++++++
3 files changed, 142 insertions(+)
create mode 100644 drivers/gpio/gpio-cascade.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 37a6f77c86fe..25096fb150f6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1694,4 +1694,16 @@ config GPIO_VIRTIO
endmenu
+comment "Other GPIO expanders"
+
+config GPIO_CASCADE
+ tristate "General GPIO cascade"
+ select MULTIPLEXER
+ help
+ Say yes here to enable support for generic GPIO cascade.
+
+ This allows building one-to-many cascades of GPIO lines using
+ different types of multiplexers readily available. At the
+ moment only input lines are supported.
+
endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 71ee9fc2ff83..e8945456e7ea 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o
obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o
obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o
obj-$(CONFIG_GPIO_CADENCE) += gpio-cadence.o
+obj-$(CONFIG_GPIO_CASCADE) += gpio-cascade.o
obj-$(CONFIG_GPIO_CLPS711X) += gpio-clps711x.o
obj-$(CONFIG_GPIO_SNPS_CREG) += gpio-creg-snps.o
obj-$(CONFIG_GPIO_CRYSTAL_COVE) += gpio-crystalcove.o
diff --git a/drivers/gpio/gpio-cascade.c b/drivers/gpio/gpio-cascade.c
new file mode 100644
index 000000000000..151ece25c64e
--- /dev/null
+++ b/drivers/gpio/gpio-cascade.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * A generic GPIO cascade driver
+ *
+ * Copyright (C) 2021 Mauri Sandberg <[email protected]>
+ *
+ * This allows building cascades of GPIO lines in a manner illustrated
+ * below:
+ *
+ * /|---- Cascaded GPIO line 0
+ * Upstream | |---- Cascaded GPIO line 1
+ * GPIO line ----+ | .
+ * | | .
+ * \|---- Cascaded GPIO line n
+ *
+ * A gpio-mux is being used to select, which cascaded line is being
+ * addressed at any given time.
+ *
+ * At the moment only input mode is supported due to lack of means for
+ * testing output functionality. At least theoretically output should be
+ * possible with an open drain constructions.
+ */
+
+#include <linux/module.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mux/consumer.h>
+
+struct gpio_cascade {
+ struct device *parent;
+ struct gpio_chip gpio_chip;
+ struct mux_control *mux_control;
+ struct gpio_desc *upstream_line;
+};
+
+static struct gpio_cascade *chip_to_cascade(struct gpio_chip *gc)
+{
+ return container_of(gc, struct gpio_cascade, gpio_chip);
+}
+
+static int gpio_cascade_get_direction(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ return GPIO_LINE_DIRECTION_IN;
+}
+
+static int gpio_cascade_get_value(struct gpio_chip *gc, unsigned int offset)
+{
+ struct gpio_cascade *cas = chip_to_cascade(gc);
+ int ret;
+
+ ret = mux_control_select(cas->mux_control, offset);
+ if (ret)
+ return ret;
+
+ ret = gpiod_get_value(cas->upstream_line);
+ mux_control_deselect(cas->mux_control);
+ return ret;
+}
+
+static int gpio_cascade_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct gpio_cascade *cas;
+ struct mux_control *mc;
+ struct gpio_desc *upstream;
+ struct gpio_chip *gc;
+ int err;
+
+ cas = devm_kzalloc(dev, sizeof(*cas), GFP_KERNEL);
+ if (!cas)
+ return -ENOMEM;
+
+ mc = devm_mux_control_get(dev, NULL);
+ if (IS_ERR(mc))
+ return dev_err_probe(dev,
+ PTR_ERR(mc),
+ "unable to get mux-control\n");
+
+ cas->mux_control = mc;
+ upstream = devm_gpiod_get(dev, "upstream", GPIOD_IN);
+ if (IS_ERR(upstream)) {
+ dev_err(dev, "unable to claim upstream GPIO line\n");
+ return -ENODEV;
+ }
+
+ cas->upstream_line = upstream;
+ cas->parent = dev;
+
+ gc = &cas->gpio_chip;
+ gc->get = gpio_cascade_get_value;
+ gc->get_direction = gpio_cascade_get_direction;
+
+ gc->base = -1;
+ gc->ngpio = mux_control_states(mc);
+ gc->label = dev_name(cas->parent);
+ gc->parent = cas->parent;
+ gc->owner = THIS_MODULE;
+
+ err = devm_gpiochip_add_data(dev, &cas->gpio_chip, NULL);
+ if (err) {
+ dev_err(dev, "unable to add gpio chip\n");
+ return err;
+ }
+
+ platform_set_drvdata(pdev, cas);
+ return 0;
+}
+
+static const struct of_device_id gpio_cascade_id[] = {
+ { .compatible = "gpio-cascade" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, gpio_cascade_id);
+
+static struct platform_driver gpio_cascade_driver = {
+ .driver = {
+ .name = "gpio-cascade",
+ .of_match_table = gpio_cascade_id,
+ },
+ .probe = gpio_cascade_probe,
+};
+module_platform_driver(gpio_cascade_driver);
+
+MODULE_AUTHOR("Mauri Sandberg <[email protected]>");
+MODULE_DESCRIPTION("Generic GPIO cascade");
+MODULE_LICENSE("GPL");
--
2.25.1
Add documentation for a general GPIO cascade. It allows building
one-to-many cascades of GPIO lines using multiplexer to choose
the cascaded line.
Signed-off-by: Mauri Sandberg <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
v5 -> v6:
- added Reviewed-by tag by Rob
v4 -> v5:
- renamed gpio-mux-input -> gpio-cascade
- changed vague term 'pin' to 'upstream line'
- added more verbose description for the module
- added missing 'mux-controls' entry
- dropped Tested-by and Reviewed-by due to changes in bindings
v3 -> v4:
- Changed author email
- Included Tested-by and Reviewed-by from Drew
v2 -> v3: added a complete example on dual 4-way multiplexer
v1 -> v2: added a little bit more text in the binding documenation
---
.../bindings/gpio/gpio-cascade.yaml | 103 ++++++++++++++++++
1 file changed, 103 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-cascade.yaml
diff --git a/Documentation/devicetree/bindings/gpio/gpio-cascade.yaml b/Documentation/devicetree/bindings/gpio/gpio-cascade.yaml
new file mode 100644
index 000000000000..46248f8e2ba5
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-cascade.yaml
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-cascade.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic GPIO cascade
+
+maintainers:
+ - Mauri Sandberg <[email protected]>
+
+description: |
+ A generic GPIO cascade
+
+ This hardware construction cascades (multiplexes) several GPIO lines from
+ one-to-many using a software controlled multiplexer. The most common use
+ case is probably reading several inputs by switching the multiplexer over
+ several input lines, which in practice works well since input lines has
+ high impedance.
+
+ Constructions with multiplexed outputs are also possible using open drain
+ electronics.
+
+ The number of cascaded GPIO lines is limited by the technology used to
+ switch over the cascaded lines. There are readily available dual/triple
+ 4-to-1 multiplexers, for example, and others.
+
+ Illustration (pins used to drive the multiplexer are omitted for clarity)
+
+ /|---- Cascaded GPIO line 0
+ Upstream | |---- Cascaded GPIO line 1
+ GPIO line ----+ | .
+ | | .
+ \|---- Cascaded GPIO line n
+
+properties:
+ compatible:
+ enum:
+ - gpio-cascade
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ mux-controls:
+ minItems: 1
+ maxItems: 1
+ description: |
+ The mux controller that will be driving the GPIO cascade.
+
+ upstream-gpios:
+ description: |
+ The GPIO line used as the upstream line will convey the status to/from
+ cascaded GPIO lines. In an input mode, by using this line, it is
+ possible to read the status from selected cascaded GPIO line.
+
+ In an output mode the status of the upstream GPIO will be conveyed to
+ the selected cascaded GPIO line.
+
+required:
+ - compatible
+ - gpio-controller
+ - "#gpio-cells"
+ - mux-controls
+ - upstream-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ mux: mux-controller {
+ compatible = "gpio-mux";
+ #mux-control-cells = <0>;
+
+ mux-gpios = <&gpio 9 GPIO_ACTIVE_HIGH>,
+ <&gpio 11 GPIO_ACTIVE_HIGH>;
+ };
+
+ gpio2: key-mux1 {
+ compatible = "gpio-cascade";
+ mux-controls = <&mux>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ // GPIOs used by this node, upstream pin
+ upstream-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>;
+ };
+
+ gpio3: key-mux2 {
+ compatible = "gpio-cascade";
+ mux-controls = <&mux>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ // GPIOs used by this node, upstream pin
+ upstream-gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
+ };
+
+...
--
2.25.1
On Tue, Oct 19, 2021 at 4:00 PM Mauri Sandberg <[email protected]> wrote:
>
> Adds support for building cascades of GPIO lines. That is, it allows
> setups when there is one upstream line and multiple cascaded lines, out
> of which one can be chosen at a time. The status of the upstream line
> can be conveyed to the selected cascaded line or, vice versa, the status
> of the cascaded line can be conveyed to the upstream line.
>
> A multiplexer is being used to select, which cascaded GPIO line is being
> used at any given time.
>
> At the moment only input direction is supported. In future it should be
> possible to add support for output direction, too.
Thanks for an update! My comments below.
...
> +config GPIO_CASCADE
> + tristate "General GPIO cascade"
> + select MULTIPLEXER
> + help
> + Say yes here to enable support for generic GPIO cascade.
> +
> + This allows building one-to-many cascades of GPIO lines using
> + different types of multiplexers readily available. At the
> + moment only input lines are supported.
Care to mention what will be the module name in the case of being
built as a module?
(Hint: there are plenty of existing examples in the kernel)
...
> +#include <linux/module.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
I would move this group...
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/mux/consumer.h>
> +
...to be somewhere here to explicitly show that this is the GPIO
subsystem related driver.
...
> + mc = devm_mux_control_get(dev, NULL);
> + if (IS_ERR(mc))
> + return dev_err_probe(dev,
> + PTR_ERR(mc),
> + "unable to get mux-control\n");
Why not one line?
...
> + upstream = devm_gpiod_get(dev, "upstream", GPIOD_IN);
> + if (IS_ERR(upstream)) {
> + dev_err(dev, "unable to claim upstream GPIO line\n");
> + return -ENODEV;
Why shadowing error code? What happens if it's deferred?
Hint: use dev_err_probe() here as well.
> + }
...
> + err = devm_gpiochip_add_data(dev, &cas->gpio_chip, NULL);
> + if (err) {
> + dev_err(dev, "unable to add gpio chip\n");
> + return err;
> + }
> +
> + platform_set_drvdata(pdev, cas);
> + return 0;
I would rather do
platform_set_drvdata(pdev, cas);
return devm_gpiochip_add_data(dev, &cas->gpio_chip, NULL);
--
With Best Regards,
Andy Shevchenko