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.
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]>
---
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 | 16 +++++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-mux-input.c | 124 ++++++++++++++++++++++++++++++++++
3 files changed, 141 insertions(+)
create mode 100644 drivers/gpio/gpio-mux-input.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1dd0ec6727fd..8a41a283ba42 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1657,4 +1657,20 @@ config GPIO_MOCKUP
endmenu
+comment "Other GPIO expanders"
+
+config GPIO_MUX_INPUT
+ tristate "General GPIO input multiplexer"
+ depends on OF_GPIO
+ select MULTIPLEXER
+ select MUX_GPIO
+ help
+ Say yes here to enable support for generic GPIO input multiplexer.
+
+ 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.
+
endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d7c81e1611a4..ff2b530d8ef4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -104,6 +104,7 @@ 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
diff --git a/drivers/gpio/gpio-mux-input.c b/drivers/gpio/gpio-mux-input.c
new file mode 100644
index 000000000000..e0739640541e
--- /dev/null
+++ b/drivers/gpio/gpio-mux-input.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * A generic GPIO input multiplexer driver
+ *
+ * Copyright (C) 2021 Mauri Sandberg <[email protected]>
+ *
+ */
+
+#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_mux_input {
+ struct device *parent;
+ struct gpio_chip gpio_chip;
+ struct mux_control *mux_control;
+ struct gpio_desc *mux_pin;
+};
+
+static struct gpio_mux_input *gpio_to_mux(struct gpio_chip *gc)
+{
+ return container_of(gc, struct gpio_mux_input, gpio_chip);
+}
+
+static int gpio_mux_input_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)
+{
+ struct gpio_mux_input *mux;
+ int ret;
+
+ mux = gpio_to_mux(gc);
+ ret = mux_control_select(mux->mux_control, offset);
+ if (ret)
+ return ret;
+
+ ret = gpiod_get_value(mux->mux_pin);
+ mux_control_deselect(mux->mux_control);
+ return ret;
+}
+
+static int gpio_mux_input_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct gpio_mux_input *mux;
+ struct mux_control *mc;
+ struct gpio_desc *pin;
+ struct gpio_chip *gc;
+ int err;
+
+ mux = devm_kzalloc(dev, sizeof(struct gpio_mux_input), GFP_KERNEL);
+ if (mux == 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;
+ }
+
+ 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);
+ return err;
+ }
+
+ mux->mux_pin = pin;
+ mux->parent = dev;
+
+ gc = &mux->gpio_chip;
+ gc->get = gpio_mux_input_get_value;
+ gc->get_direction = gpio_mux_input_get_direction;
+
+ gc->base = -1;
+ gc->ngpio = mux_control_states(mc);
+ gc->label = dev_name(mux->parent);
+ gc->parent = mux->parent;
+ gc->owner = THIS_MODULE;
+ gc->of_node = np;
+
+ err = gpiochip_add(&mux->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);
+ return 0;
+}
+
+static const struct of_device_id gpio_mux_input_id[] = {
+ {
+ .compatible = "gpio-mux-input",
+ .data = NULL,
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, gpio_mux_input_id);
+
+static struct platform_driver gpio_mux_input_driver = {
+ .driver = {
+ .name = "gpio-mux-input",
+ .of_match_table = gpio_mux_input_id,
+ },
+ .probe = gpio_mux_input_probe,
+};
+module_platform_driver(gpio_mux_input_driver);
+
+MODULE_AUTHOR("Mauri Sandberg <[email protected]>");
+MODULE_DESCRIPTION("Generic GPIO input multiplexer");
+MODULE_LICENSE("GPL");
--
2.25.1
On Sun, May 30, 2021 at 7:16 PM Mauri Sandberg <[email protected]> wrote:
>
> 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.
>
> Reported-by: kernel test robot <[email protected]>
Is it a fix? Shall we add the Fixes tag?
> Signed-off-by: Mauri Sandberg <[email protected]>
> Tested-by: Drew Fustini <[email protected]>
> Reviewed-by: Drew Fustini <[email protected]>
--
With Best Regards,
Andy Shevchenko
On 30.5.2021 21.09, Andy Shevchenko wrote:
> On Sun, May 30, 2021 at 7:16 PM Mauri Sandberg <[email protected]> wrote:
>> 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.
>>
>> Reported-by: kernel test robot <[email protected]>
> Is it a fix? Shall we add the Fixes tag?
In the v1 a build bot complained about .owner along these lines:
--- snip ----
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
cocci warnings: (new ones prefixed by >>)
>> drivers/gpio/gpio-mux-input.c:138:3-8: No need to set .owner here.
The core will do it.
Please review and possibly fold the followup patch.
--- snip ---
I removed the .owner attribute in v2 as requested but wasn't really sure
whether it was "appropriate"
to add the tag so I put it there anyhow. Technically, this does not fix
any previous commit.
>> Signed-off-by: Mauri Sandberg <[email protected]>
>> Tested-by: Drew Fustini <[email protected]>
>> Reviewed-by: Drew Fustini <[email protected]>
>
On Sun, May 30, 2021 at 10:02 PM Mauri Sandberg <[email protected]> wrote:
> On 30.5.2021 21.09, Andy Shevchenko wrote:
> > On Sun, May 30, 2021 at 7:16 PM Mauri Sandberg <[email protected]> wrote:
> >> Reported-by: kernel test robot <[email protected]>
> > Is it a fix? Shall we add the Fixes tag?
>
> In the v1 a build bot complained about .owner along these lines:
>
> --- snip ----
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
>
> cocci warnings: (new ones prefixed by >>)
> >> drivers/gpio/gpio-mux-input.c:138:3-8: No need to set .owner here.
> The core will do it.
>
> Please review and possibly fold the followup patch.
> --- snip ---
>
> I removed the .owner attribute in v2 as requested but wasn't really sure
> whether it was "appropriate"
> to add the tag so I put it there anyhow. Technically, this does not fix
> any previous commit.
For this kind of thing you may attribute the reporter(s) by mentioning
them in the comment lines / cover letter.
--
With Best Regards,
Andy Shevchenko
On 30.5.2021 22.38, Andy Shevchenko wrote:
> On Sun, May 30, 2021 at 10:02 PM Mauri Sandberg <[email protected]> wrote:
>> On 30.5.2021 21.09, Andy Shevchenko wrote:
>>> On Sun, May 30, 2021 at 7:16 PM Mauri Sandberg <[email protected]> wrote:
>>>> Reported-by: kernel test robot <[email protected]>
>>> Is it a fix? Shall we add the Fixes tag?
>> In the v1 a build bot complained about .owner along these lines:
>>
>> --- snip ----
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <[email protected]>
>>
>>
>> cocci warnings: (new ones prefixed by >>)
>> >> drivers/gpio/gpio-mux-input.c:138:3-8: No need to set .owner here.
>> The core will do it.
>>
>> Please review and possibly fold the followup patch.
>> --- snip ---
>>
>> I removed the .owner attribute in v2 as requested but wasn't really sure
>> whether it was "appropriate"
>> to add the tag so I put it there anyhow. Technically, this does not fix
>> any previous commit.
> For this kind of thing you may attribute the reporter(s) by mentioning
> them in the comment lines / cover letter.
It's there in the patch version notes so the 'Reported-by' was
unnecessary. Should it be removed?
That is, is there a tool sitting somwhere that tries to match reports
and their fixes?
On Sun, May 30, 2021 at 6:16 PM Mauri Sandberg <[email protected]> wrote:
> 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.
>
> 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]>
The commit message and part of the driver becomes hard to
read and understand because the word pin is overused.
Switch to talk about "gpio lines" rather than pins.
Draw a simple ASCII image like this:
/|---- Cascaded GPIO line 0
|M|---- Cascaded GPIO line 1
GPIO line ----+U| .
|X| .
\|---- Cascaded GPIO line n
Maybe also as illustration in the driver and in the bindings.
Make things easy to understand.
Explain exactly why only input lines can be multiplexed.
I'm not sure it should be restricted to just input
in theory, but since that is all we can test, restrict it to
input in practice.
> +config GPIO_MUX_INPUT
> + tristate "General GPIO input multiplexer"
Rename it just GPIO_MUX
"General GPIO multiplexer"
Then clarify in the help description that it currently can only
handle input lines.
> + depends on OF_GPIO
> + select MULTIPLEXER
> + select MUX_GPIO
> + help
> + Say yes here to enable support for generic GPIO input multiplexer.
> +
> + 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.
Input output etc, this gets very hard to understand.
Switch terminology from "pin" to "GPIO lines", (or "GPIO rails").
Use the word "routing" as the GPIO line is routed through the
multiplexer. Maybe spell out multiplexer for clarity.
Explain why, for electrical reasons, output lines are harder
to multiplex like this, as the output will not maintain
state. Notice that "using open drain constructions, output
multiplexing may be possible, but it is currently not implemented."
> +static int gpio_mux_input_get_direction(struct gpio_chip *gc,
> + unsigned int offset)
> +{
> + return GPIO_LINE_DIRECTION_IN;
> +}
Explain why this is a restriction with a comment in the code.
Add comment that in the future we might be able to handle
also output.
> +static int gpio_mux_input_get_value(struct gpio_chip *gc, unsigned int offset)
This looks very nice!
We might have to extend this driver at some point.
Intuitively I'd say it takes some time and then someone
comes along and say "actually we have done this
for output as well, using some open drain and stuff"
but this is a good starting point anyway we need no
big upfront designs.
Yours,
Linus Walleij