| Message ID | 20210325122832.119147-3-sandberg@mailfence.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: add generic gpio input multiplexer | expand |
On Thu, Mar 25, 2021 at 02:28:32PM +0200, Mauri Sandberg wrote: > Suppport for a general GPIO multiplexer. To drive the multiplexer a > mux-controller is needed. The output pin of the multiplexer is a GPIO > pin > > Signed-off-by: Mauri Sandberg <sandberg@mailfence.com> Thanks for posting the RFC so we can take a look at the code and discuss how it works. > --- > drivers/gpio/Kconfig | 11 +++ > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-mux-input.c | 143 ++++++++++++++++++++++++++++++++++ > 3 files changed, 155 insertions(+) > create mode 100644 drivers/gpio/gpio-mux-input.c > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index c70f46e80a3b..41062d8f7d93 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -1641,4 +1641,15 @@ config GPIO_MOCKUP > > endmenu > > +comment "Other GPIO expanders" > + > +config GPIO_MUX_INPUT > + tristate "General GPIO input multiplexer" > + select MULTIPLEXER > + select MUX_GPIO > + depends on OF_GPIO > + help > + Say yes here to enable support for generic GPIO input multiplexer. This > + needs a multiplexer controller to drive the select pins. > + > endif > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 35e3b6026665..00f7576ce23f 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -105,6 +105,7 @@ obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o > obj-$(CONFIG_GPIO_MSC313) += gpio-msc313.o > obj-$(CONFIG_GPIO_MSIC) += gpio-msic.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 This does not apply to mainline. I've added it manually to my drivers/gpio/Makefile but something to fix in v2. > diff --git a/drivers/gpio/gpio-mux-input.c b/drivers/gpio/gpio-mux-input.c > new file mode 100644 > index 000000000000..ec0c7acbab2f > --- /dev/null > +++ b/drivers/gpio/gpio-mux-input.c > @@ -0,0 +1,143 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * A generic GPIO input multiplexer driver > + * > + * Copyright (C) 2021 Mauri Sandberg <sandberg@mailfence.com> > + * > + */ > + > +#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_direction_input(struct gpio_chip *gc, > + unsigned int offset) > +{ > + return 0; > +} > + > +static int gpio_mux_input_direction_output(struct gpio_chip *gc, > + unsigned int offset, int val) > +{ > + return -EINVAL; > +} > + > +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); I'm not too familiar with how mux_control works but does there need to be locking here? Or is not possible for mux_pin to change to another offset before if gpiod_get_value() if gpio_mux_input_get_value() runs concurrently? > + mux_control_deselect(mux->mux_control); > + return ret; > +} > + > +static void gpio_mux_input_set_value(struct gpio_chip *gc, > + unsigned int offset, int val) > +{ > + /* not supported */ I'm not sure but maybe it is better not to define gc->set in the probe? > +} > + > +static int gpio_mux_input_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct gpio_mux_input *mux; > + struct gpio_chip *gc; > + struct mux_control *mc; > + struct gpio_desc *pin; > + int err; > + > + mux = kzalloc(sizeof(struct gpio_mux_input), GFP_KERNEL); > + if (mux == NULL) > + return -ENOMEM; > + > + mc = mux_control_get(&pdev->dev, NULL); > + if (IS_ERR(mc)) { > + err = (int) PTR_ERR(mc); > + if (err != -EPROBE_DEFER) > + dev_err(&pdev->dev, "unable to get mux-control: %d\n", > + err); > + goto err_free_mux; > + } > + > + mux->mux_control = mc; > + pin = gpiod_get(&pdev->dev, "pin", GPIOD_IN); > + if (IS_ERR(pin)) { > + err = (int) PTR_ERR(pin); > + dev_err(&pdev->dev, "unable to claim pin GPIOs: %d\n", err); > + goto err_free_mc; > + } > + > + mux->mux_pin = pin; > + mux->parent = &pdev->dev; > + > + gc = &mux->gpio_chip; > + gc->direction_input = gpio_mux_input_direction_input; > + gc->direction_output = gpio_mux_input_direction_output; > + gc->get = gpio_mux_input_get_value; > + gc->set = gpio_mux_input_set_value; > + gc->can_sleep = 1; > + > + 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(&pdev->dev, "unable to add gpio chip, err=%d\n", err); > + goto err_free_pin; > + } > + > + platform_set_drvdata(pdev, mux); > + return 0; > + > +err_free_pin: > + gpiod_put(pin); > +err_free_mc: > + mux_control_put(mc); > +err_free_mux: > + kfree(mux); > + return err; > +} > + > +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", > + .owner = THIS_MODULE, > + .of_match_table = gpio_mux_input_id, > + }, > + .probe = gpio_mux_input_probe, > +}; > +module_platform_driver(gpio_mux_input_driver); I believe you need to add: MODULE_AUTHOR("..."); MODULE_DESCRIPTION("..."); MODULE_LICENSE("GPL"); My build failed with: ERROR: modpost: missing MODULE_LICENSE() in drivers/gpio/gpio-mux-input.o LZMA arch/arm/boot/compressed/piggy_data make[1]: *** [scripts/Makefile.modpost:132: Module.symvers] Error 1 make[1]: *** Deleting file 'Module.symvers' make: *** [Makefile:1442: modules] Error 2 make: *** Waiting for unfinished jobs.... AS arch/arm/boot/compressed/piggy.o LD arch/arm/boot/compressed/vmlinux OBJCOPY arch/arm/boot/zImage Kernel: arch/arm/boot/zImage is ready I added those lines and it compiled successfully. -Drew
> ---------------------------------------- > From: Drew Fustini <drew@beagleboard.org> > Sent: Fri Mar 26 07:59:44 CET 2021 > To: Mauri Sandberg <sandberg@mailfence.com> > > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > > index 35e3b6026665..00f7576ce23f 100644 > > --- a/drivers/gpio/Makefile > > +++ b/drivers/gpio/Makefile > > @@ -105,6 +105,7 @@ obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o > > obj-$(CONFIG_GPIO_MSC313) += gpio-msc313.o > > obj-$(CONFIG_GPIO_MSIC) += gpio-msic.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 > > This does not apply to mainline. I've added it manually to my > drivers/gpio/Makefile but something to fix in v2. I was developing against gpio tree's [1] 'for-next' branch but should I go against mainline? > > diff --git a/drivers/gpio/gpio-mux-input.c b/drivers/gpio/gpio-mux-input.c > > +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); > > I'm not too familiar with how mux_control works but does there need to > be locking here? > > Or is not possible for mux_pin to change to another offset before if > gpiod_get_value() if gpio_mux_input_get_value() runs concurrently? According to mux documentation [2] successfully selecting a state locks the mux until it is deselected. So I reckon no extra locking is needed. > > + > > +static void gpio_mux_input_set_value(struct gpio_chip *gc, > > + unsigned int offset, int val) > > +{ > > + /* not supported */ > > I'm not sure but maybe it is better not to define gc->set in the probe? I will give it a go. > I believe you need to add: > > MODULE_AUTHOR("..."); > MODULE_DESCRIPTION("..."); > MODULE_LICENSE("GPL"); > > My build failed with: > > ERROR: modpost: missing MODULE_LICENSE() in drivers/gpio/gpio-mux-input.o I will add them, thanks. How do you do your build? Mine does not complain pretty much about anything. Also a bot gave me a warning and I would like to run those tests manually before submitting anything for review. Cheers, Mauri [1] https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mux/core.c#n322
On Fri, Mar 26, 2021 at 12:32 PM Mauri Sandberg <sandberg@mailfence.com> wrote: > > From: Drew Fustini <drew@beagleboard.org> > > Sent: Fri Mar 26 07:59:44 CET 2021 ... > > This does not apply to mainline. I've added it manually to my > > drivers/gpio/Makefile but something to fix in v2. > > I was developing against gpio tree's [1] 'for-next' branch but should I go against mainline? It's a Bart's tree nowadays [3], gpio/for-next branch. > > > + ret = gpiod_get_value(mux->mux_pin); > > > > I'm not too familiar with how mux_control works but does there need to > > be locking here? > > > > Or is not possible for mux_pin to change to another offset before if > > gpiod_get_value() if gpio_mux_input_get_value() runs concurrently? > > According to mux documentation [2] successfully selecting a state locks the mux > until it is deselected. So I reckon no extra locking is needed. ... > [1] https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mux/core.c#n322 [3]: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git/
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index c70f46e80a3b..41062d8f7d93 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1641,4 +1641,15 @@ config GPIO_MOCKUP endmenu +comment "Other GPIO expanders" + +config GPIO_MUX_INPUT + tristate "General GPIO input multiplexer" + select MULTIPLEXER + select MUX_GPIO + depends on OF_GPIO + help + Say yes here to enable support for generic GPIO input multiplexer. This + needs a multiplexer controller to drive the select pins. + endif diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 35e3b6026665..00f7576ce23f 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -105,6 +105,7 @@ obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o obj-$(CONFIG_GPIO_MSC313) += gpio-msc313.o obj-$(CONFIG_GPIO_MSIC) += gpio-msic.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..ec0c7acbab2f --- /dev/null +++ b/drivers/gpio/gpio-mux-input.c @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * A generic GPIO input multiplexer driver + * + * Copyright (C) 2021 Mauri Sandberg <sandberg@mailfence.com> + * + */ + +#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_direction_input(struct gpio_chip *gc, + unsigned int offset) +{ + return 0; +} + +static int gpio_mux_input_direction_output(struct gpio_chip *gc, + unsigned int offset, int val) +{ + return -EINVAL; +} + +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 void gpio_mux_input_set_value(struct gpio_chip *gc, + unsigned int offset, int val) +{ + /* not supported */ +} + +static int gpio_mux_input_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct gpio_mux_input *mux; + struct gpio_chip *gc; + struct mux_control *mc; + struct gpio_desc *pin; + int err; + + mux = kzalloc(sizeof(struct gpio_mux_input), GFP_KERNEL); + if (mux == NULL) + return -ENOMEM; + + mc = mux_control_get(&pdev->dev, NULL); + if (IS_ERR(mc)) { + err = (int) PTR_ERR(mc); + if (err != -EPROBE_DEFER) + dev_err(&pdev->dev, "unable to get mux-control: %d\n", + err); + goto err_free_mux; + } + + mux->mux_control = mc; + pin = gpiod_get(&pdev->dev, "pin", GPIOD_IN); + if (IS_ERR(pin)) { + err = (int) PTR_ERR(pin); + dev_err(&pdev->dev, "unable to claim pin GPIOs: %d\n", err); + goto err_free_mc; + } + + mux->mux_pin = pin; + mux->parent = &pdev->dev; + + gc = &mux->gpio_chip; + gc->direction_input = gpio_mux_input_direction_input; + gc->direction_output = gpio_mux_input_direction_output; + gc->get = gpio_mux_input_get_value; + gc->set = gpio_mux_input_set_value; + gc->can_sleep = 1; + + 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(&pdev->dev, "unable to add gpio chip, err=%d\n", err); + goto err_free_pin; + } + + platform_set_drvdata(pdev, mux); + return 0; + +err_free_pin: + gpiod_put(pin); +err_free_mc: + mux_control_put(mc); +err_free_mux: + kfree(mux); + return err; +} + +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", + .owner = THIS_MODULE, + .of_match_table = gpio_mux_input_id, + }, + .probe = gpio_mux_input_probe, +}; +module_platform_driver(gpio_mux_input_driver);
Suppport for a general GPIO multiplexer. To drive the multiplexer a mux-controller is needed. The output pin of the multiplexer is a GPIO pin. Signed-off-by: Mauri Sandberg <sandberg@mailfence.com> --- drivers/gpio/Kconfig | 11 +++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-mux-input.c | 143 ++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 drivers/gpio/gpio-mux-input.c