| Message ID | bcac00c71b29c359b59053a30d817a8f17d6731d.1514898134.git.baruch@tkos.co.il (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Baruch, additionally the my comments on Michael's patches in March 2017, some new below. > Baruch Siach <baruch@tkos.co.il> hat am 2. Januar 2018 um 14:19 geschrieben: > > > From: Dave Stevenson <dave.stevenson@raspberrypi.org> > > Pi3 and Compute Module 3 have a GPIO expander that the > VPU communicates with. > There is a mailbox service that now allows control of this > expander, so add a kernel driver that can make use of it. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > drivers/gpio/Kconfig | 7 ++ > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-bcm-exp.c | 254 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 262 insertions(+) > create mode 100644 drivers/gpio/gpio-bcm-exp.c > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index d6a8e851ad13..e2aab64ea772 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -128,6 +128,13 @@ config GPIO_AXP209 > help > Say yes to enable GPIO support for the AXP209 PMIC > > +config GPIO_BCM_EXP > + bool "Broadcom Exp GPIO" same as in the binding, i don't think this is specific to Broadcom. > + depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST) This is too long. Please split up. > + help > + Turn on GPIO support for Broadcom chips using the firmware mailbox > + to communicate with VideoCore on BCM283x chips. > + > config GPIO_BCM_KONA > bool "Broadcom Kona GPIO" > depends on OF_GPIO && (ARCH_BCM_MOBILE || COMPILE_TEST) > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 4bc24febb889..c5f481b1d53c 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -33,6 +33,7 @@ obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o > obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o > obj-$(CONFIG_GPIO_ASPEED) += gpio-aspeed.o > obj-$(CONFIG_GPIO_AXP209) += gpio-axp209.o > +obj-$(CONFIG_GPIO_BCM_EXP) += gpio-bcm-exp.o > obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o > obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o > obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o > diff --git a/drivers/gpio/gpio-bcm-exp.c b/drivers/gpio/gpio-bcm-exp.c > new file mode 100644 > index 000000000000..d68adafaee4a > --- /dev/null > +++ b/drivers/gpio/gpio-bcm-exp.c > @@ -0,0 +1,254 @@ > +/* > + * Broadcom expander GPIO driver > + * > + * Uses the firmware mailbox service to communicate with the > + * GPIO expander on the VPU. > + * > + * Copyright (C) 2017 Raspberry Pi Trading Ltd. > + * > + * Author: Dave Stevenson <dave.stevenson@raspberrypi.org> > + * Based on gpio-bcm-virt.c by Dom Cobley <popcornmix@gmail.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ SPDX identifier? > + > +#include <linux/err.h> > +#include <linux/gpio.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/dma-mapping.h> > +#include <soc/bcm2835/raspberrypi-firmware.h> > + > +#define MODULE_NAME "brcmexp-gpio" > +#define NUM_GPIO 8 > + > +struct brcmexp_gpio { > + struct gpio_chip gc; > + struct device *dev; > + struct rpi_firmware *fw; > +}; > + > +struct gpio_set_config { > + u32 gpio, direction, polarity, term_en, term_pull_up, state; > +}; > + > +struct gpio_get_config { > + u32 gpio, direction, polarity, term_en, term_pull_up; > +}; > + > +struct gpio_get_set_state { > + u32 gpio, state; > +}; > + > +static int brcmexp_gpio_get_polarity(struct gpio_chip *gc, unsigned int off) > +{ > + struct brcmexp_gpio *gpio; > + struct gpio_get_config get; > + int ret; > + > + gpio = container_of(gc, struct brcmexp_gpio, gc); > + > + get.gpio = off + gpio->gc.base; /* GPIO to update */ Please don't misuse the gpiochip base to communicate with the firmware. AFAIK the gc.base should be -1 (dynamic), so better use a define for the base. > + > + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG, > + &get, sizeof(get)); > + if (ret) { > + dev_err(gpio->dev, > + "Failed to get GPIO %u config (%d)\n", off, ret); > + return ret; > + } Shouldn't we also check the in-bound status at get.gpio? And in all the other gpio ops? > ... > + > +static int brcmexp_gpio_probe(struct platform_device *pdev) > +{ > + int err = 0; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct device_node *fw_node; > + struct rpi_firmware *fw; > + struct brcmexp_gpio *ucb; > + > + fw_node = of_parse_phandle(np, "firmware", 0); > + if (!fw_node) { > + dev_err(dev, "Missing firmware node\n"); > + return -ENOENT; > + } > + > + fw = rpi_firmware_get(fw_node); > + if (!fw) > + return -EPROBE_DEFER; > + > + ucb = devm_kzalloc(dev, sizeof(*ucb), GFP_KERNEL); > + if (!ucb) > + return -EINVAL; > + > + ucb->fw = fw; > + ucb->dev = dev; > + ucb->gc.label = MODULE_NAME; > + ucb->gc.owner = THIS_MODULE; > + ucb->gc.of_node = np; > + ucb->gc.base = 128; As said above this should be -1 Stefan
On Tue, Jan 2, 2018 at 2:19 PM, Baruch Siach <baruch@tkos.co.il> wrote: > +config GPIO_BCM_EXP > + bool "Broadcom Exp GPIO" > + depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST) > + help > + Turn on GPIO support for Broadcom chips using the firmware mailbox > + to communicate with VideoCore on BCM283x chips. Should this be default RASPBERRYPI_FIRMWARE So it is always available if the firmware interface is there? > +#include <linux/err.h> > +#include <linux/gpio.h> Just use #include <linux/driver.h> > +#define MODULE_NAME "brcmexp-gpio" > +#define NUM_GPIO 8 > + > +struct brcmexp_gpio { > + struct gpio_chip gc; > + struct device *dev; > + struct rpi_firmware *fw; > +}; > + > +struct gpio_set_config { > + u32 gpio, direction, polarity, term_en, term_pull_up, state; > +}; > + > +struct gpio_get_config { > + u32 gpio, direction, polarity, term_en, term_pull_up; > +}; Seems to support some pin control stuff, hm? The pull ups seems unused though so OK. Yours, Linus Walleij
Hi Stefan, On Tue, Jan 02, 2018 at 07:49:44PM +0100, Stefan Wahren wrote: [...] > > + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG, > > + &get, sizeof(get)); > > + if (ret) { > > + dev_err(gpio->dev, > > + "Failed to get GPIO %u config (%d)\n", off, ret); > > + return ret; > > + } > > Shouldn't we also check the in-bound status at get.gpio? What is the in-bound status value? May you refer me to the documentation? > And in all the other gpio ops? Thanks, baruch
Hi Baruch, > Baruch Siach <baruch@tkos.co.il> hat am 9. Januar 2018 um 14:41 geschrieben: > > > Hi Stefan, > > On Tue, Jan 02, 2018 at 07:49:44PM +0100, Stefan Wahren wrote: > > [...] > > > > + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG, > > > + &get, sizeof(get)); > > > + if (ret) { > > > + dev_err(gpio->dev, > > > + "Failed to get GPIO %u config (%d)\n", off, ret); > > > + return ret; > > > + } > > > > Shouldn't we also check the in-bound status at get.gpio? > > What is the in-bound status value? May you refer me to the documentation? unfortunately this mailbox property isn't documented here [1]. So please refer to Phil's explanation [2]. I assume gpio_get_config.gpio of the response would be 0x80000000 in case of an invalid request. But i didn't test it yet. This was my solution [3] for the RPI_FIRMWARE_GET_CUSTOMER_OTP. But it hadn't been reviewed yet. Stefan [1] - https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface [2] - http://lists.infradead.org/pipermail/linux-rpi-kernel/2018-January/007237.html [3] - https://github.com/lategoodbye/rpi-zero/blob/bcm2835-otp/drivers/nvmem/raspberrypi-otp.c#L61 > > > And in all the other gpio ops? > > Thanks, > baruch > > -- > http://baruch.siach.name/blog/ ~. .~ Tk Open Systems > =}------------------------------------------------ooO--U--Ooo------------{= > - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il - > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Linus, On Wed, Jan 03, 2018 at 11:08:15AM +0100, Linus Walleij wrote: > On Tue, Jan 2, 2018 at 2:19 PM, Baruch Siach <baruch@tkos.co.il> wrote: > > +#include <linux/err.h> > > +#include <linux/gpio.h> > > Just use > > #include <linux/driver.h> You mean linux/gpio/driver.h, right? I still need linux/gpio.h for GPIOF_DIR_*. baruch
On Wed, Jan 10, 2018 at 4:45 AM, Baruch Siach <baruch@tkos.co.il> wrote: > Hi Linus, > > On Wed, Jan 03, 2018 at 11:08:15AM +0100, Linus Walleij wrote: >> On Tue, Jan 2, 2018 at 2:19 PM, Baruch Siach <baruch@tkos.co.il> wrote: >> > +#include <linux/err.h> >> > +#include <linux/gpio.h> >> >> Just use >> >> #include <linux/driver.h> > > You mean linux/gpio/driver.h, right? Yes. > I still need linux/gpio.h for GPIOF_DIR_*. Don't use these, just use 0 and 1 open coded for the moment (if you check my devel branch, the references to these flags are gone). We need to make new defines for the new API. Yours, Linus Walleij
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index d6a8e851ad13..e2aab64ea772 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -128,6 +128,13 @@ config GPIO_AXP209 help Say yes to enable GPIO support for the AXP209 PMIC +config GPIO_BCM_EXP + bool "Broadcom Exp GPIO" + depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST) + help + Turn on GPIO support for Broadcom chips using the firmware mailbox + to communicate with VideoCore on BCM283x chips. + config GPIO_BCM_KONA bool "Broadcom Kona GPIO" depends on OF_GPIO && (ARCH_BCM_MOBILE || COMPILE_TEST) diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 4bc24febb889..c5f481b1d53c 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -33,6 +33,7 @@ obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o obj-$(CONFIG_GPIO_ASPEED) += gpio-aspeed.o obj-$(CONFIG_GPIO_AXP209) += gpio-axp209.o +obj-$(CONFIG_GPIO_BCM_EXP) += gpio-bcm-exp.o obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o diff --git a/drivers/gpio/gpio-bcm-exp.c b/drivers/gpio/gpio-bcm-exp.c new file mode 100644 index 000000000000..d68adafaee4a --- /dev/null +++ b/drivers/gpio/gpio-bcm-exp.c @@ -0,0 +1,254 @@ +/* + * Broadcom expander GPIO driver + * + * Uses the firmware mailbox service to communicate with the + * GPIO expander on the VPU. + * + * Copyright (C) 2017 Raspberry Pi Trading Ltd. + * + * Author: Dave Stevenson <dave.stevenson@raspberrypi.org> + * Based on gpio-bcm-virt.c by Dom Cobley <popcornmix@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/err.h> +#include <linux/gpio.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/dma-mapping.h> +#include <soc/bcm2835/raspberrypi-firmware.h> + +#define MODULE_NAME "brcmexp-gpio" +#define NUM_GPIO 8 + +struct brcmexp_gpio { + struct gpio_chip gc; + struct device *dev; + struct rpi_firmware *fw; +}; + +struct gpio_set_config { + u32 gpio, direction, polarity, term_en, term_pull_up, state; +}; + +struct gpio_get_config { + u32 gpio, direction, polarity, term_en, term_pull_up; +}; + +struct gpio_get_set_state { + u32 gpio, state; +}; + +static int brcmexp_gpio_get_polarity(struct gpio_chip *gc, unsigned int off) +{ + struct brcmexp_gpio *gpio; + struct gpio_get_config get; + int ret; + + gpio = container_of(gc, struct brcmexp_gpio, gc); + + get.gpio = off + gpio->gc.base; /* GPIO to update */ + + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG, + &get, sizeof(get)); + if (ret) { + dev_err(gpio->dev, + "Failed to get GPIO %u config (%d)\n", off, ret); + return ret; + } + return get.polarity; +} + +static int brcmexp_gpio_dir_in(struct gpio_chip *gc, unsigned int off) +{ + struct brcmexp_gpio *gpio; + struct gpio_set_config set_in; + int ret; + + gpio = container_of(gc, struct brcmexp_gpio, gc); + + set_in.gpio = off + gpio->gc.base; /* GPIO to update */ + set_in.direction = 0; /* Input */ + set_in.polarity = brcmexp_gpio_get_polarity(gc, off); + /* Retain existing setting */ + set_in.term_en = 0; /* termination disabled */ + set_in.term_pull_up = 0; /* n/a as termination disabled */ + set_in.state = 0; /* n/a as configured as an input */ + + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_CONFIG, + &set_in, sizeof(set_in)); + if (ret) { + dev_err(gpio->dev, + "Failed to set GPIO %u to input (%d)\n", + off, ret); + return ret; + } + return 0; +} + +static int brcmexp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val) +{ + struct brcmexp_gpio *gpio; + struct gpio_set_config set_out; + int ret; + + gpio = container_of(gc, struct brcmexp_gpio, gc); + + set_out.gpio = off + gpio->gc.base; /* GPIO to update */ + set_out.direction = 1; /* Output */ + set_out.polarity = brcmexp_gpio_get_polarity(gc, off); + /* Retain existing setting */ + set_out.term_en = 0; /* n/a as an output */ + set_out.term_pull_up = 0; /* n/a as termination disabled */ + set_out.state = val; /* Output state */ + + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_CONFIG, + &set_out, sizeof(set_out)); + if (ret) { + dev_err(gpio->dev, + "Failed to set GPIO %u to output (%d)\n", off, ret); + return ret; + } + return 0; +} + +static int brcmexp_gpio_get_direction(struct gpio_chip *gc, unsigned int off) +{ + struct brcmexp_gpio *gpio; + struct gpio_get_config get; + int ret; + + gpio = container_of(gc, struct brcmexp_gpio, gc); + + get.gpio = off + gpio->gc.base; /* GPIO to update */ + + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG, + &get, sizeof(get)); + if (ret) { + dev_err(gpio->dev, + "Failed to get GPIO %u config (%d)\n", off, ret); + return ret; + } + return get.direction ? GPIOF_DIR_OUT : GPIOF_DIR_IN; +} + +static int brcmexp_gpio_get(struct gpio_chip *gc, unsigned int off) +{ + struct brcmexp_gpio *gpio; + struct gpio_get_set_state get; + int ret; + + gpio = container_of(gc, struct brcmexp_gpio, gc); + + get.gpio = off + gpio->gc.base; /* GPIO to update */ + get.state = 0; /* storage for returned value */ + + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_STATE, + &get, sizeof(get)); + if (ret) { + dev_err(gpio->dev, + "Failed to get GPIO %u state (%d)\n", off, ret); + return ret; + } + return !!get.state; +} + +static void brcmexp_gpio_set(struct gpio_chip *gc, unsigned int off, int val) +{ + struct brcmexp_gpio *gpio; + struct gpio_get_set_state set; + int ret; + + gpio = container_of(gc, struct brcmexp_gpio, gc); + + set.gpio = off + gpio->gc.base; /* GPIO to update */ + set.state = val; /* Output state */ + + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_STATE, + &set, sizeof(set)); + if (ret) + dev_err(gpio->dev, + "Failed to set GPIO %u state (%d)\n", off, ret); +} + +static int brcmexp_gpio_probe(struct platform_device *pdev) +{ + int err = 0; + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + struct device_node *fw_node; + struct rpi_firmware *fw; + struct brcmexp_gpio *ucb; + + fw_node = of_parse_phandle(np, "firmware", 0); + if (!fw_node) { + dev_err(dev, "Missing firmware node\n"); + return -ENOENT; + } + + fw = rpi_firmware_get(fw_node); + if (!fw) + return -EPROBE_DEFER; + + ucb = devm_kzalloc(dev, sizeof(*ucb), GFP_KERNEL); + if (!ucb) + return -EINVAL; + + ucb->fw = fw; + ucb->dev = dev; + ucb->gc.label = MODULE_NAME; + ucb->gc.owner = THIS_MODULE; + ucb->gc.of_node = np; + ucb->gc.base = 128; + ucb->gc.ngpio = NUM_GPIO; + + ucb->gc.direction_input = brcmexp_gpio_dir_in; + ucb->gc.direction_output = brcmexp_gpio_dir_out; + ucb->gc.get_direction = brcmexp_gpio_get_direction; + ucb->gc.get = brcmexp_gpio_get; + ucb->gc.set = brcmexp_gpio_set; + ucb->gc.can_sleep = true; + + err = gpiochip_add(&ucb->gc); + if (err) + return err; + + platform_set_drvdata(pdev, ucb); + + return 0; +} + +static int brcmexp_gpio_remove(struct platform_device *pdev) +{ + struct brcmexp_gpio *ucb = platform_get_drvdata(pdev); + + gpiochip_remove(&ucb->gc); + + return 0; +} + +static const struct of_device_id __maybe_unused brcmexp_gpio_ids[] = { + { .compatible = "brcm,bcm2835-expgpio" }, + { } +}; +MODULE_DEVICE_TABLE(of, brcmexp_gpio_ids); + +static struct platform_driver brcmexp_gpio_driver = { + .driver = { + .name = MODULE_NAME, + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(brcmexp_gpio_ids), + }, + .probe = brcmexp_gpio_probe, + .remove = brcmexp_gpio_remove, +}; +module_platform_driver(brcmexp_gpio_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Dave Stevenson <dave.stevenson@raspberrypi.org>"); +MODULE_DESCRIPTION("brcm-exp GPIO driver"); +MODULE_ALIAS("platform:brcmexp-gpio");