Hello all,
Thank you for your review comments so far. Here comes new attempt at the gpio
multiplexer driver. There are quite a lot of changes if you look at the diff
based on the v4. Functionality is still the same, though. Only renaming,
refactoring and some more documentation.
What was done
- rename 'gpio-mux-input' -> 'gpio-cascade' in sources
- bindings were changed to reflect the renaming
- refactoring here and there, renaming variables
- documentation was made simpler and more verbose
Drew: I chose to drop the Reviewed-by and Tested-by tags because, even though
mostly renaming, bindings were changed too and that affects functionality. If
you have the time, could you please have another look?
Linus: I hope the changes to the documentation were what you had in mind.
Rob: I fixed a couple of errors that you bot detected but one is due to missing
schema file (gpio-mux) and adding that is not in the scope of this work.
Andy: I dropped the Reported-by tag the bot had instructed me to add.
Thanks,
Mauri
diff between v4 and v5
1: 496558967cd8 ! 1: 83defaf33475 dt-bindings: gpio-mux-input: add documentation
@@ Metadata
Author: Mauri Sandberg <[email protected]>
## Commit message ##
- dt-bindings: gpio-mux-input: add documentation
+ dt-bindings: gpio-cascade: add documentation
- Add documentation for a general GPIO multiplexer.
+ 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]>
- Tested-by: Drew Fustini <[email protected]>
- Reviewed-by: Drew Fustini <[email protected]>
---
+ 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
- ## Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml (new) ##
+ ## Documentation/devicetree/bindings/gpio/gpio-cascade.yaml (new) ##
@@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
-+$id: http://devicetree.org/schemas/gpio/gpio-mux-input.yaml#
++$id: http://devicetree.org/schemas/gpio/gpio-cascade.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
-+title: Generic GPIO input multiplexer
++title: Generic GPIO cascade
+
+maintainers:
+ - Mauri Sandberg <[email protected]>
+
+description: |
-+ A generic GPIO based input multiplexer
++ A generic GPIO cascade
+
-+ This driver uses a mux-controller to drive the multiplexer and has a single
-+ output pin for reading the inputs to the mux.
++ 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.
+
-+ For GPIO consumer documentation see gpio.txt.
++ 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-mux-input
++ - gpio-cascade
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
-+ pin-gpios:
++ mux-controls:
++ minItems: 1
++ maxItems: 1
+ description: |
-+ The GPIO pin used as the output from the multiplexer
++ 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"
-+ - pin-gpios
++ - mux-controls
++ - upstream-gpios
+
+additionalProperties: false
+
@@ Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml (new)
+ };
+
+ gpio2: key-mux1 {
-+ compatible = "gpio-mux-input";
++ compatible = "gpio-cascade";
+ mux-controls = <&mux>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+
-+ // GPIOs used by this node, mux pin
-+ pin-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>;
++ // GPIOs used by this node, upstream pin
++ upstream-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>;
+ };
+
+ gpio3: key-mux2 {
-+ compatible = "gpio-mux-input";
++ compatible = "gpio-cascade";
+ mux-controls = <&mux>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+
-+ // GPIOs used by this node, mux pin
-+ pin-gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
++ // GPIOs used by this node, upstream pin
++ upstream-gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
+ };
+
+...
2: 782e009ba54b ! 2: d1063611d6ac gpio: gpio-mux-input: add generic gpio input multiplexer
@@ Metadata
Author: Mauri Sandberg <[email protected]>
## Commit message ##
- gpio: gpio-mux-input: add generic gpio input multiplexer
+ gpio: gpio-cascade: add generic GPIO cascade
- Adds support for a generic GPIO multiplexer. To drive the multiplexer a
- mux-controller is needed. The output pin of the multiplexer is a GPIO
- pin.
+ Adds support for a 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
+ 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
+ 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.
- Reported-by: kernel test robot <[email protected]>
Signed-off-by: Mauri Sandberg <[email protected]>
- Tested-by: Drew Fustini <[email protected]>
- Reviewed-by: Drew Fustini <[email protected]>
---
+ 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
@@ drivers/gpio/Kconfig: config GPIO_MOCKUP
+comment "Other GPIO expanders"
+
-+config GPIO_MUX_INPUT
-+ tristate "General GPIO input multiplexer"
++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 input multiplexer.
++ Say yes here to enable support for generic GPIO cascade.
+
-+ This driver uses a mux-controller to drive the multiplexer and has a
-+ single output pin for reading the inputs to the mux. The driver can
-+ be used in situations when GPIO pins are used to select what
-+ multiplexer pin should be used for reading input and the output pin
-+ of the multiplexer is connected to a GPIO input pin.
++ 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
## drivers/gpio/Makefile ##
-@@ drivers/gpio/Makefile: obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
- obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
- obj-$(CONFIG_GPIO_MSC313) += gpio-msc313.o
- obj-$(CONFIG_GPIO_MT7621) += gpio-mt7621.o
-+obj-$(CONFIG_GPIO_MUX_INPUT) += gpio-mux-input.o
- obj-$(CONFIG_GPIO_MVEBU) += gpio-mvebu.o
- obj-$(CONFIG_GPIO_MXC) += gpio-mxc.o
- obj-$(CONFIG_GPIO_MXS) += gpio-mxs.o
+@@ drivers/gpio/Makefile: 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
- ## drivers/gpio/gpio-mux-input.c (new) ##
+ ## drivers/gpio/gpio-cascade.c (new) ##
@@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
-+ * A generic GPIO input multiplexer driver
++ * 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>
@@ drivers/gpio/gpio-mux-input.c (new)
+#include <linux/platform_device.h>
+#include <linux/mux/consumer.h>
+
-+struct gpio_mux_input {
++struct gpio_cascade {
+ struct device *parent;
+ struct gpio_chip gpio_chip;
+ struct mux_control *mux_control;
-+ struct gpio_desc *mux_pin;
++ struct gpio_desc *upstream_line;
+};
+
-+static struct gpio_mux_input *gpio_to_mux(struct gpio_chip *gc)
++static struct gpio_cascade *chip_to_cascade(struct gpio_chip *gc)
+{
-+ return container_of(gc, struct gpio_mux_input, gpio_chip);
++ return container_of(gc, struct gpio_cascade, gpio_chip);
+}
+
-+static int gpio_mux_input_get_direction(struct gpio_chip *gc,
++static int gpio_cascade_get_direction(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ return GPIO_LINE_DIRECTION_IN;
+}
+
-+static int gpio_mux_input_get_value(struct gpio_chip *gc, unsigned int offset)
++static int gpio_cascade_get_value(struct gpio_chip *gc, unsigned int offset)
+{
-+ struct gpio_mux_input *mux;
++ struct gpio_cascade *cas;
+ int ret;
+
-+ mux = gpio_to_mux(gc);
-+ ret = mux_control_select(mux->mux_control, offset);
++ cas = chip_to_cascade(gc);
++ ret = mux_control_select(cas->mux_control, offset);
+ if (ret)
+ return ret;
+
-+ ret = gpiod_get_value(mux->mux_pin);
-+ mux_control_deselect(mux->mux_control);
++ ret = gpiod_get_value(cas->upstream_line);
++ mux_control_deselect(cas->mux_control);
+ return ret;
+}
+
-+static int gpio_mux_input_probe(struct platform_device *pdev)
++static int gpio_cascade_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
-+ struct gpio_mux_input *mux;
++ struct gpio_cascade *cas;
+ struct mux_control *mc;
-+ struct gpio_desc *pin;
++ struct gpio_desc *upstream;
+ struct gpio_chip *gc;
+ int err;
+
-+ mux = devm_kzalloc(dev, sizeof(struct gpio_mux_input), GFP_KERNEL);
-+ if (mux == NULL)
++ cas = devm_kzalloc(dev, sizeof(struct gpio_cascade), GFP_KERNEL);
++ if (cas == NULL)
+ return -ENOMEM;
+
+ mc = devm_mux_control_get(dev, NULL);
@@ drivers/gpio/gpio-mux-input.c (new)
+ return err;
+ }
+
-+ mux->mux_control = mc;
-+ pin = devm_gpiod_get(dev, "pin", GPIOD_IN);
-+ if (IS_ERR(pin)) {
-+ err = (int) PTR_ERR(pin);
-+ dev_err(dev, "unable to claim output pin: %d\n", err);
++ 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;
+ }
+
-+ mux->mux_pin = pin;
-+ mux->parent = dev;
++ cas->upstream_line = upstream;
++ cas->parent = dev;
+
-+ gc = &mux->gpio_chip;
-+ gc->get = gpio_mux_input_get_value;
-+ gc->get_direction = gpio_mux_input_get_direction;
++ 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(mux->parent);
-+ gc->parent = mux->parent;
++ gc->label = dev_name(cas->parent);
++ gc->parent = cas->parent;
+ gc->owner = THIS_MODULE;
+ gc->of_node = np;
+
-+ err = gpiochip_add(&mux->gpio_chip);
++ err = gpiochip_add(&cas->gpio_chip);
+ if (err) {
+ dev_err(dev, "unable to add gpio chip, err=%d\n", err);
+ return err;
+ }
+
-+ platform_set_drvdata(pdev, mux);
-+ dev_info(dev, "registered %u input GPIOs\n", gc->ngpio);
++ 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_mux_input_id[] = {
++static const struct of_device_id gpio_cascade_id[] = {
+ {
-+ .compatible = "gpio-mux-input",
++ .compatible = "gpio-cascade",
+ .data = NULL,
+ },
+ { /* sentinel */ }
+};
-+MODULE_DEVICE_TABLE(of, gpio_mux_input_id);
++MODULE_DEVICE_TABLE(of, gpio_cascade_id);
+
-+static struct platform_driver gpio_mux_input_driver = {
++static struct platform_driver gpio_cascade_driver = {
+ .driver = {
-+ .name = "gpio-mux-input",
-+ .of_match_table = gpio_mux_input_id,
++ .name = "gpio-cascade",
++ .of_match_table = gpio_cascade_id,
+ },
-+ .probe = gpio_mux_input_probe,
++ .probe = gpio_cascade_probe,
+};
-+module_platform_driver(gpio_mux_input_driver);
++module_platform_driver(gpio_cascade_driver);
+
+MODULE_AUTHOR("Mauri Sandberg <[email protected]>");
-+MODULE_DESCRIPTION("Generic GPIO input multiplexer");
++MODULE_DESCRIPTION("Generic GPIO cascade");
+MODULE_LICENSE("GPL");
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 | 14 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-cascade.c | 139 ++++++++++++++++++
4 files changed, 257 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-cascade.yaml
create mode 100644 drivers/gpio/gpio-cascade.c
base-commit: 6d49b3a0f351925b5ea5047166c112b7590b918a
--
2.25.1
Adds support for a 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
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
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]>
---
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 | 14 ++++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-cascade.c | 139 ++++++++++++++++++++++++++++++++++++
3 files changed, 154 insertions(+)
create mode 100644 drivers/gpio/gpio-cascade.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ae2721967191..d8bdb6d87b1f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1669,4 +1669,18 @@ config GPIO_MOCKUP
endmenu
+comment "Other GPIO expanders"
+
+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.
+
+ 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 32a32659866a..94a9d6e43bc2 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..91053ac489c2
--- /dev/null
+++ b/drivers/gpio/gpio-cascade.c
@@ -0,0 +1,139 @@
+// 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;
+ int ret;
+
+ cas = chip_to_cascade(gc);
+ 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_node *np = pdev->dev.of_node;
+ 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(struct gpio_cascade), GFP_KERNEL);
+ if (cas == NULL)
+ 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;
+ }
+
+ 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;
+ }
+
+ 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;
+ gc->of_node = np;
+
+ err = gpiochip_add(&cas->gpio_chip);
+ if (err) {
+ dev_err(dev, "unable to add gpio chip, err=%d\n", err);
+ 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,
+ },
+ { /* 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
On Mon, Jun 21, 2021 at 8:25 PM Mauri Sandberg <[email protected]> wrote:
>
> Adds support for a building cascades of GPIO lines. That is, it allows
for building
> 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
conveyed
> 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
> 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.
Since in parallel there is a discussion about the virtio-gpio
interface, how will this work with it?
> +// 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.
> + */
...
> +static int gpio_cascade_get_value(struct gpio_chip *gc, unsigned int offset)
> +{
> + struct gpio_cascade *cas;
> + int ret;
> + cas = chip_to_cascade(gc);
Doing this in the definition block above will save a LOC.
> + 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;
> +}
...
> + struct device_node *np = pdev->dev.of_node;
Nope, see below.
...
> + cas = devm_kzalloc(dev, sizeof(struct gpio_cascade), GFP_KERNEL);
sizeof(*cas)
> + if (cas == NULL)
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;
Oh là là! No, the explicit castings are bad. besides the fact that all
above can be replaced by
return dev_err_probe(...);
> + }
> +
> + 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);
No castings. Use proper printf() specifiers.
> + return err;
> + }
...
> + gc->of_node = np;
This should be guarded by CONFIG_OF_GPIO.
And no need to use the np temporary variable for one use like this.
...
> + err = gpiochip_add(&cas->gpio_chip);
Why not the devm variant?
> + if (err) {
> + dev_err(dev, "unable to add gpio chip, err=%d\n", err);
> + return err;
> + }
...
> + dev_info(dev, "registered %u cascaded GPIO lines\n", gc->ngpio);
No, we don't pollute logs when everything is fine.
...
> +static const struct of_device_id gpio_cascade_id[] = {
> + {
> + .compatible = "gpio-cascade",
> + .data = NULL,
Redundant.
> + },
All above may consume only a single LOC.
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, gpio_cascade_id);
--
With Best Regards,
Andy Shevchenko
On 21.06.21 19:43, Andy Shevchenko wrote:
> Since in parallel there is a discussion about the virtio-gpio
> interface, how will this work with it?
Haven't really understood what this is actually doing. A multiplexer
where only external line is connected to some actual gpio at a time ?
Or does it merge multiple inputs into one (eg. logical OR) ?
Is that about real hardware mux chips or just a software only ?
What is the actual use case ?
For now, I don't see any relation to virtio-gpio. Correct me if I'm
wrong.
--mtx
--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 21.6.2021 20.43, Andy Shevchenko wrote:
>> + gc->of_node = np;
>
> This should be guarded by CONFIG_OF_GPIO.
In the Kconfig I have a dependency to OF_GPIO. Is the guarding still
necessary? Or should the guard be added and dependency removed? Or have
them both?
On Fri, Oct 15, 2021 at 3:56 PM Mauri Sandberg <[email protected]> wrote:
> On 21.6.2021 20.43, Andy Shevchenko wrote:
>
> >> + gc->of_node = np;
> >
> > This should be guarded by CONFIG_OF_GPIO.
The above is actually done by the GPIO library nowadays.
> In the Kconfig I have a dependency to OF_GPIO. Is the guarding still
> necessary? Or should the guard be added and dependency removed? Or have
> them both?
For this kind ("generic") driver the OF_GPIO dependency is simply
wrong. You shouldn't have it.
--
With Best Regards,
Andy Shevchenko