| Message ID | 20160919161314.25858-2-eric@anholt.net |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 19, 2016 at 6:13 PM, Eric Anholt <eric@anholt.net> wrote: > This driver will be used for accessing the FXL6408 GPIO exander on the > Pi3. We can't drive it directly from Linux because the firmware is > continuously polling one of the expander's lines to do its > undervoltage detection. > > Signed-off-by: Eric Anholt <eric@anholt.net> (...) > +config GPIO_RASPBERRYPI > + tristate "Raspberry Pi firmware-based GPIO access" > + depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST) > + help > + Turns on support for using the Raspberry Pi firmware to > + control GPIO pins. Used for access to the FXL6408 GPIO > + expander on the Pi 3. Maybe it should be named GPIO_RPI_FXL6408 ? (No strong opinion.) > +#include <linux/err.h> > +#include <linux/gpio.h> No, only #include <linux/gpio/driver.h> > +static int rpi_gpio_dir_in(struct gpio_chip *gc, unsigned off) > +{ > + /* We don't have direction control. */ > + return -EINVAL; > +} > + > +static int rpi_gpio_dir_out(struct gpio_chip *gc, unsigned off, int val) > +{ > + /* We don't have direction control. */ > + return -EINVAL; > +} IMO this should return OK if you try to set it to the direction that the line is hardwired for in that case, not just fail everything. If you return errors here, any generic driver that tries to set the line as input or output will fail and then require a second workaround in that driver if it is used on rpi etc etc. Try to return zero if the consumer asks for the direction that the line is set to. Also implement .get_direction(). Doing so will show how to do the above two calls in the right way. > +static void rpi_gpio_set(struct gpio_chip *gc, unsigned off, int val) > +{ > + struct rpi_gpio *rpi = container_of(gc, struct rpi_gpio, gc); > + u32 packet[2] = { rpi->offset + off, val }; > + int ret; > + > + ret = rpi_firmware_property(rpi->firmware, > + RPI_FIRMWARE_SET_GPIO_STATE, > + &packet, sizeof(packet)); > + if (ret) > + dev_err(rpi->dev, "Error setting GPIO %d state: %d\n", off, ret); > +} > + > +static int rpi_gpio_get(struct gpio_chip *gc, unsigned off) > +{ > + struct rpi_gpio *rpi = container_of(gc, struct rpi_gpio, gc); > + u32 packet[2] = { rpi->offset + off, 0 }; > + int ret; > + > + ret = rpi_firmware_property(rpi->firmware, > + RPI_FIRMWARE_GET_GPIO_STATE, > + &packet, sizeof(packet)); > + if (ret) { > + dev_err(rpi->dev, "Error getting GPIO state: %d\n", ret); > + return ret; > + } else if (packet[0]) { > + dev_err(rpi->dev, "Firmware error getting GPIO state: %d\n", > + packet[0]); > + return -EINVAL; > + } else { > + return packet[1]; > + } You can just remove the last } else { } clause and return on a single line. Please do it like this though: return !!packet[1]; So you clamp the returned value to a boolean. > +static int rpi_gpio_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct device_node *fw_node; > + struct rpi_gpio *rpi; > + u32 ngpio; > + int ret; > + > + rpi = devm_kzalloc(dev, sizeof *rpi, GFP_KERNEL); > + if (!rpi) > + return -ENOMEM; > + rpi->dev = dev; > + > + fw_node = of_parse_phandle(np, "firmware", 0); > + if (!fw_node) { > + dev_err(dev, "Missing firmware node\n"); > + return -ENOENT; > + } > + > + rpi->firmware = rpi_firmware_get(fw_node); > + if (!rpi->firmware) > + return -EPROBE_DEFER; > + > + if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio)) { > + dev_err(dev, "Missing ngpios"); > + return -ENOENT; > + } This is suspect you could skip and just hardcode to 8. > + if (of_property_read_u32(pdev->dev.of_node, > + "raspberrypi,firmware-gpio-offset", > + &rpi->offset)) { > + dev_err(dev, "Missing raspberrypi,firmware-gpio-offset"); > + return -ENOENT; > + } Can't you just hardcode this into the driver as well? > + rpi->gc.label = np->full_name; > + rpi->gc.owner = THIS_MODULE; > + rpi->gc.of_node = np; > + rpi->gc.ngpio = ngpio; > + rpi->gc.direction_input = rpi_gpio_dir_in; > + rpi->gc.direction_output = rpi_gpio_dir_out; Implement .get_direction() > + rpi->gc.get = rpi_gpio_get; > + rpi->gc.set = rpi_gpio_set; > + rpi->gc.can_sleep = true; > + > + ret = gpiochip_add(&rpi->gc); > + if (ret) > + return ret; Use devm_gpiochip_add_data() and pass NULL as data so you can still use the devm* function. > + platform_set_drvdata(pdev, rpi); Should not be needed after that. > + return 0; > +} > + > +static int rpi_gpio_remove(struct platform_device *pdev) > +{ > + struct rpi_gpio *rpi = platform_get_drvdata(pdev); > + > + gpiochip_remove(&rpi->gc); > + > + return 0; > +} Should be possible to drop after using devm_gpiochip_add_data() > diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h > index 3fb357193f09..671ccd00aea2 100644 > --- a/include/soc/bcm2835/raspberrypi-firmware.h > +++ b/include/soc/bcm2835/raspberrypi-firmware.h > @@ -73,11 +73,13 @@ enum rpi_firmware_property_tag { > RPI_FIRMWARE_GET_DISPMANX_RESOURCE_MEM_HANDLE = 0x00030014, > RPI_FIRMWARE_GET_EDID_BLOCK = 0x00030020, > RPI_FIRMWARE_GET_DOMAIN_STATE = 0x00030030, > + RPI_FIRMWARE_GET_GPIO_STATE = 0x00030041, > RPI_FIRMWARE_SET_CLOCK_STATE = 0x00038001, > RPI_FIRMWARE_SET_CLOCK_RATE = 0x00038002, > RPI_FIRMWARE_SET_VOLTAGE = 0x00038003, > RPI_FIRMWARE_SET_TURBO = 0x00038009, > RPI_FIRMWARE_SET_DOMAIN_STATE = 0x00038030, > + RPI_FIRMWARE_SET_GPIO_STATE = 0x00038041, Can you please merge this orthogonally into the rpi tree to ARM SoC? Yours, Linus Walleij
Linus Walleij <linus.walleij@linaro.org> writes: > On Mon, Sep 19, 2016 at 6:13 PM, Eric Anholt <eric@anholt.net> wrote: > >> This driver will be used for accessing the FXL6408 GPIO exander on the >> Pi3. We can't drive it directly from Linux because the firmware is >> continuously polling one of the expander's lines to do its >> undervoltage detection. >> >> Signed-off-by: Eric Anholt <eric@anholt.net> > (...) > >> +config GPIO_RASPBERRYPI >> + tristate "Raspberry Pi firmware-based GPIO access" >> + depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST) >> + help >> + Turns on support for using the Raspberry Pi firmware to >> + control GPIO pins. Used for access to the FXL6408 GPIO >> + expander on the Pi 3. > > Maybe it should be named GPIO_RPI_FXL6408 ? > > (No strong opinion.) See DT binding comment -- I think since this driver has no dependency on being to the 6408 on the pi3, we shouldn't needlessly bind it to the FXL6408. (the help comment was just context for why you would want the driver today). >> +static int rpi_gpio_dir_in(struct gpio_chip *gc, unsigned off) >> +{ >> + /* We don't have direction control. */ >> + return -EINVAL; >> +} >> + >> +static int rpi_gpio_dir_out(struct gpio_chip *gc, unsigned off, int val) >> +{ >> + /* We don't have direction control. */ >> + return -EINVAL; >> +} > > IMO this should return OK if you try to set it to the direction > that the line is hardwired for in that case, not just fail everything. > > If you return errors here, any generic driver that tries to > set the line as input or output will fail and then require a > second workaround in that driver if it is used on rpi etc etc. > > Try to return zero if the consumer asks for the direction that > the line is set to. > > Also implement .get_direction(). Doing so will show how to > do the above two calls in the right way. I was worried about the lack of direction support. The firmware interface doesn't give me anything for direction, and a get or set of the value doesn't try to set direction. I can try to bother them to give me support for that, but if they cooperate on that it means that users will only get HDMI detection once they update firmware. The alternative to new firmware interface would be to provide a bunch of DT saying which of these should be in/out at boot time and refuse to change them after that. That seems like a mess, though. >> +static void rpi_gpio_set(struct gpio_chip *gc, unsigned off, int val) >> +{ >> + struct rpi_gpio *rpi = container_of(gc, struct rpi_gpio, gc); >> + u32 packet[2] = { rpi->offset + off, val }; >> + int ret; >> + >> + ret = rpi_firmware_property(rpi->firmware, >> + RPI_FIRMWARE_SET_GPIO_STATE, >> + &packet, sizeof(packet)); >> + if (ret) >> + dev_err(rpi->dev, "Error setting GPIO %d state: %d\n", off, ret); >> +} >> + >> +static int rpi_gpio_get(struct gpio_chip *gc, unsigned off) >> +{ >> + struct rpi_gpio *rpi = container_of(gc, struct rpi_gpio, gc); >> + u32 packet[2] = { rpi->offset + off, 0 }; >> + int ret; >> + >> + ret = rpi_firmware_property(rpi->firmware, >> + RPI_FIRMWARE_GET_GPIO_STATE, >> + &packet, sizeof(packet)); >> + if (ret) { >> + dev_err(rpi->dev, "Error getting GPIO state: %d\n", ret); >> + return ret; >> + } else if (packet[0]) { >> + dev_err(rpi->dev, "Firmware error getting GPIO state: %d\n", >> + packet[0]); >> + return -EINVAL; >> + } else { >> + return packet[1]; >> + } > > You can just remove the last } else { } clause and return on a > single line. > > Please do it like this though: > > return !!packet[1]; > > So you clamp the returned value to a boolean. Will do. >> + rpi->gc.get = rpi_gpio_get; >> + rpi->gc.set = rpi_gpio_set; >> + rpi->gc.can_sleep = true; >> + >> + ret = gpiochip_add(&rpi->gc); >> + if (ret) >> + return ret; > > Use devm_gpiochip_add_data() and pass NULL as data > so you can still use the devm* function. Oh, nice. >> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h >> index 3fb357193f09..671ccd00aea2 100644 >> --- a/include/soc/bcm2835/raspberrypi-firmware.h >> +++ b/include/soc/bcm2835/raspberrypi-firmware.h >> @@ -73,11 +73,13 @@ enum rpi_firmware_property_tag { >> RPI_FIRMWARE_GET_DISPMANX_RESOURCE_MEM_HANDLE = 0x00030014, >> RPI_FIRMWARE_GET_EDID_BLOCK = 0x00030020, >> RPI_FIRMWARE_GET_DOMAIN_STATE = 0x00030030, >> + RPI_FIRMWARE_GET_GPIO_STATE = 0x00030041, >> RPI_FIRMWARE_SET_CLOCK_STATE = 0x00038001, >> RPI_FIRMWARE_SET_CLOCK_RATE = 0x00038002, >> RPI_FIRMWARE_SET_VOLTAGE = 0x00038003, >> RPI_FIRMWARE_SET_TURBO = 0x00038009, >> RPI_FIRMWARE_SET_DOMAIN_STATE = 0x00038030, >> + RPI_FIRMWARE_SET_GPIO_STATE = 0x00038041, > > Can you please merge this orthogonally into the rpi tree to ARM SoC? This driver would appear in the rpi downstream tree once we settle the driver here. Or are you asking me to delay this series until I can get them to pull just a patch extending the set of packets?
On Fri, Sep 23, 2016 at 3:15 PM, Eric Anholt <eric@anholt.net> wrote: > Linus Walleij <linus.walleij@linaro.org> writes: >> Maybe it should be named GPIO_RPI_FXL6408 ? >> >> (No strong opinion.) > > See DT binding comment -- I think since this driver has no dependency on > being to the 6408 on the pi3, we shouldn't needlessly bind it to the > FXL6408. (the help comment was just context for why you would want the > driver today). OK >>> +static int rpi_gpio_dir_in(struct gpio_chip *gc, unsigned off) >>> +{ >>> + /* We don't have direction control. */ >>> + return -EINVAL; >>> +} >>> + >>> +static int rpi_gpio_dir_out(struct gpio_chip *gc, unsigned off, int val) >>> +{ >>> + /* We don't have direction control. */ >>> + return -EINVAL; >>> +} >> >> IMO this should return OK if you try to set it to the direction >> that the line is hardwired for in that case, not just fail everything. >> >> If you return errors here, any generic driver that tries to >> set the line as input or output will fail and then require a >> second workaround in that driver if it is used on rpi etc etc. >> >> Try to return zero if the consumer asks for the direction that >> the line is set to. >> >> Also implement .get_direction(). Doing so will show how to >> do the above two calls in the right way. > > I was worried about the lack of direction support. The firmware > interface doesn't give me anything for direction, and a get or set > of the value doesn't try to set direction. > > I can try to bother them to give me support for that, but if they > cooperate on that it means that users will only get HDMI detection once > they update firmware. > > The alternative to new firmware interface would be to provide a bunch of > DT saying which of these should be in/out at boot time and refuse to > change them after that. That seems like a mess, though. It has to be resolved one way or another I'm afraid. Do you have an API in place to ask for the firmware version? RPI_FIRMWARE_GET_FIRMWARE_REVISION seems to exist at least? In that case try to make some compromise, e.g. if lines 0 and 1 are output and the rest input in a certain firmware version: struct rpi_gpio { (...) u8 dirs; }; if (fw_version <= a) rpi->dirs = 0x03; else if (fw_version > a && fw_version <= b) rpi->dirs = 0x07; else /* Ask firmware */ Then in e.g. static int rpi_gpio_dir_in(struct gpio_chip *gc, unsigned off) { struct rpi_gpio *rpi = gpiochip_get_data(gc); if (!(rpi->dirs & BIT(off))) return 0; return -EINVAL; } I think this should be managed by code in the driver like this and not by any DT properties. I suspect also the ngpio number is better to determine from looking at the fw version number. >> Use devm_gpiochip_add_data() and pass NULL as data >> so you can still use the devm* function. > > Oh, nice. I forgot this: with devm_gpiochip_add_data() pass struct rpi_gpio * as data then you can just: static void rpi_gpio_set(struct gpio_chip *gc, unsigned off, int val) { - struct rpi_gpio *rpi = container_of(gc, struct rpi_gpio, gc); + struct rpi_gpio *rpi = gpiochip_get_data(gc); Applies everywhere. >>> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h >>> index 3fb357193f09..671ccd00aea2 100644 >>> --- a/include/soc/bcm2835/raspberrypi-firmware.h >>> +++ b/include/soc/bcm2835/raspberrypi-firmware.h >>> @@ -73,11 +73,13 @@ enum rpi_firmware_property_tag { >>> RPI_FIRMWARE_GET_DISPMANX_RESOURCE_MEM_HANDLE = 0x00030014, >>> RPI_FIRMWARE_GET_EDID_BLOCK = 0x00030020, >>> RPI_FIRMWARE_GET_DOMAIN_STATE = 0x00030030, >>> + RPI_FIRMWARE_GET_GPIO_STATE = 0x00030041, >>> RPI_FIRMWARE_SET_CLOCK_STATE = 0x00038001, >>> RPI_FIRMWARE_SET_CLOCK_RATE = 0x00038002, >>> RPI_FIRMWARE_SET_VOLTAGE = 0x00038003, >>> RPI_FIRMWARE_SET_TURBO = 0x00038009, >>> RPI_FIRMWARE_SET_DOMAIN_STATE = 0x00038030, >>> + RPI_FIRMWARE_SET_GPIO_STATE = 0x00038041, >> >> Can you please merge this orthogonally into the rpi tree to ARM SoC? > > This driver would appear in the rpi downstream tree once we settle the > driver here. Or are you asking me to delay this series until I can get > them to pull just a patch extending the set of packets? Sorry I am not familiar with your development model. I don't know about any RPI downstream tree... What I mean is that the patch to include/soc/bcm2835/raspberrypi-firmware.h should be merged by whoever is maintaining that file, it is not a GPIO file. If I get an ACK from the maintainer I can take it into the GPIO tree. Yours, Linus Walleij
Hi Eric, > Eric Anholt <eric@anholt.net> hat am 19. September 2016 um 18:13 geschrieben: > > > This driver will be used for accessing the FXL6408 GPIO exander on the > Pi3. We can't drive it directly from Linux because the firmware is > continuously polling one of the expander's lines to do its > undervoltage detection. > > Signed-off-by: Eric Anholt <eric@anholt.net> > --- > ... > + > +static int rpi_gpio_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct device_node *fw_node; > + struct rpi_gpio *rpi; > + u32 ngpio; > + int ret; > + > + rpi = devm_kzalloc(dev, sizeof *rpi, GFP_KERNEL); > + if (!rpi) > + return -ENOMEM; > + rpi->dev = dev; > + > + fw_node = of_parse_phandle(np, "firmware", 0); AFAIK fw_node must be freed with of_node_put() after usage > + if (!fw_node) { > + dev_err(dev, "Missing firmware node\n"); > + return -ENOENT; > + } > + > + rpi->firmware = rpi_firmware_get(fw_node); > + if (!rpi->firmware) > + return -EPROBE_DEFER; > + > + if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio)) { > + dev_err(dev, "Missing ngpios"); > + return -ENOENT; > + } > + if (of_property_read_u32(pdev->dev.of_node, > + "raspberrypi,firmware-gpio-offset", > + &rpi->offset)) { > + dev_err(dev, "Missing raspberrypi,firmware-gpio-offset"); > + return -ENOENT; > + } > + > + rpi->gc.label = np->full_name; > + rpi->gc.owner = THIS_MODULE; > + rpi->gc.of_node = np; > + rpi->gc.ngpio = ngpio; > + rpi->gc.direction_input = rpi_gpio_dir_in; > + rpi->gc.direction_output = rpi_gpio_dir_out; > + rpi->gc.get = rpi_gpio_get; > + rpi->gc.set = rpi_gpio_set; > + rpi->gc.can_sleep = true; i think it's better to assign rpi->gc.base explicit. Stefan
Linus Walleij <linus.walleij@linaro.org> writes: > On Fri, Sep 23, 2016 at 3:15 PM, Eric Anholt <eric@anholt.net> wrote: >> Linus Walleij <linus.walleij@linaro.org> writes: >>>> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h >>>> index 3fb357193f09..671ccd00aea2 100644 >>>> --- a/include/soc/bcm2835/raspberrypi-firmware.h >>>> +++ b/include/soc/bcm2835/raspberrypi-firmware.h >>>> @@ -73,11 +73,13 @@ enum rpi_firmware_property_tag { >>>> RPI_FIRMWARE_GET_DISPMANX_RESOURCE_MEM_HANDLE = 0x00030014, >>>> RPI_FIRMWARE_GET_EDID_BLOCK = 0x00030020, >>>> RPI_FIRMWARE_GET_DOMAIN_STATE = 0x00030030, >>>> + RPI_FIRMWARE_GET_GPIO_STATE = 0x00030041, >>>> RPI_FIRMWARE_SET_CLOCK_STATE = 0x00038001, >>>> RPI_FIRMWARE_SET_CLOCK_RATE = 0x00038002, >>>> RPI_FIRMWARE_SET_VOLTAGE = 0x00038003, >>>> RPI_FIRMWARE_SET_TURBO = 0x00038009, >>>> RPI_FIRMWARE_SET_DOMAIN_STATE = 0x00038030, >>>> + RPI_FIRMWARE_SET_GPIO_STATE = 0x00038041, >>> >>> Can you please merge this orthogonally into the rpi tree to ARM SoC? >> >> This driver would appear in the rpi downstream tree once we settle the >> driver here. Or are you asking me to delay this series until I can get >> them to pull just a patch extending the set of packets? > > Sorry I am not familiar with your development model. I don't know > about any RPI downstream tree... What I mean is that the patch to > include/soc/bcm2835/raspberrypi-firmware.h should be merged by > whoever is maintaining that file, it is not a GPIO file. > > If I get an ACK from the maintainer I can take it into the GPIO > tree. Oh, people often say "the rpi tree" to mean downstream (currently 4.4). The maintainer of that file upstream is me, and I was hoping you could merge through your tree.
On 09/23/2016 07:15 AM, Eric Anholt wrote: > Linus Walleij <linus.walleij@linaro.org> writes: > >> On Mon, Sep 19, 2016 at 6:13 PM, Eric Anholt <eric@anholt.net> wrote: >> >>> This driver will be used for accessing the FXL6408 GPIO exander on the >>> Pi3. We can't drive it directly from Linux because the firmware is >>> continuously polling one of the expander's lines to do its >>> undervoltage detection. >>> >>> Signed-off-by: Eric Anholt <eric@anholt.net> >> (...) >> >>> +config GPIO_RASPBERRYPI >>> + tristate "Raspberry Pi firmware-based GPIO access" >>> + depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST) >>> + help >>> + Turns on support for using the Raspberry Pi firmware to >>> + control GPIO pins. Used for access to the FXL6408 GPIO >>> + expander on the Pi 3. >> >> Maybe it should be named GPIO_RPI_FXL6408 ? >> >> (No strong opinion.) > > See DT binding comment -- I think since this driver has no dependency on > being to the 6408 on the pi3, we shouldn't needlessly bind it to the > FXL6408. (the help comment was just context for why you would want the > driver today). I'd suggest including "FW" or "FIRMWARE" in the Kconfig option too; the Raspberry Pi has multiple GPIO drivers; one accessing the BCM283x HW directly and the other going through the FW. It'd be good if each Kconfig name was pretty explicit re: which one it represented.
On Sat, Sep 24, 2016 at 9:01 AM, Eric Anholt <eric@anholt.net> wrote: > Linus Walleij <linus.walleij@linaro.org> writes: >> Sorry I am not familiar with your development model. I don't know >> about any RPI downstream tree... What I mean is that the patch to >> include/soc/bcm2835/raspberrypi-firmware.h should be merged by >> whoever is maintaining that file, it is not a GPIO file. >> >> If I get an ACK from the maintainer I can take it into the GPIO >> tree. > > Oh, people often say "the rpi tree" to mean downstream (currently 4.4). > The maintainer of that file upstream is me, and I was hoping you could > merge through your tree. OK no problem, I can merge it once we agree on the mechanics :) Yours, Linus Walleij
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index fcbd8e318474..60cf38bb3a44 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -136,6 +136,14 @@ config GPIO_BCM_KONA help Turn on GPIO support for Broadcom "Kona" chips. +config GPIO_RASPBERRYPI + tristate "Raspberry Pi firmware-based GPIO access" + depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST) + help + Turns on support for using the Raspberry Pi firmware to + control GPIO pins. Used for access to the FXL6408 GPIO + expander on the Pi 3. + config GPIO_BRCMSTB tristate "BRCMSTB GPIO support" default y if (ARCH_BRCMSTB || BMIPS_GENERIC) diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index b3e039c3ae8d..fa10cf4030ad 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -31,6 +31,7 @@ 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_KONA) += gpio-bcm-kona.o +obj-$(CONFIG_GPIO_RASPBERRYPI) += gpio-raspberrypi.o obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o obj-$(CONFIG_GPIO_CLPS711X) += gpio-clps711x.o diff --git a/drivers/gpio/gpio-raspberrypi.c b/drivers/gpio/gpio-raspberrypi.c new file mode 100644 index 000000000000..233c211f45ba --- /dev/null +++ b/drivers/gpio/gpio-raspberrypi.c @@ -0,0 +1,159 @@ +/* + * Copyright © 2016 Broadcom + * + * 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. + */ + +/* This driver supports using the Raspberry Pi's firmware interface to + * access its GPIO lines. This lets us interact with the GPIO lines + * on the Raspberry Pi 3's FXL6408 expander, which we otherwise have + * no way to access (since the firmware is polling the chip + * continuously). + */ + +#include <linux/err.h> +#include <linux/gpio.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <soc/bcm2835/raspberrypi-firmware.h> + +struct rpi_gpio { + struct device *dev; + struct gpio_chip gc; + struct rpi_firmware *firmware; + + /* Offset of our pins in the GET_GPIO_STATE/SET_GPIO_STATE calls. */ + unsigned offset; +}; + +static int rpi_gpio_dir_in(struct gpio_chip *gc, unsigned off) +{ + /* We don't have direction control. */ + return -EINVAL; +} + +static int rpi_gpio_dir_out(struct gpio_chip *gc, unsigned off, int val) +{ + /* We don't have direction control. */ + return -EINVAL; +} + +static void rpi_gpio_set(struct gpio_chip *gc, unsigned off, int val) +{ + struct rpi_gpio *rpi = container_of(gc, struct rpi_gpio, gc); + u32 packet[2] = { rpi->offset + off, val }; + int ret; + + ret = rpi_firmware_property(rpi->firmware, + RPI_FIRMWARE_SET_GPIO_STATE, + &packet, sizeof(packet)); + if (ret) + dev_err(rpi->dev, "Error setting GPIO %d state: %d\n", off, ret); +} + +static int rpi_gpio_get(struct gpio_chip *gc, unsigned off) +{ + struct rpi_gpio *rpi = container_of(gc, struct rpi_gpio, gc); + u32 packet[2] = { rpi->offset + off, 0 }; + int ret; + + ret = rpi_firmware_property(rpi->firmware, + RPI_FIRMWARE_GET_GPIO_STATE, + &packet, sizeof(packet)); + if (ret) { + dev_err(rpi->dev, "Error getting GPIO state: %d\n", ret); + return ret; + } else if (packet[0]) { + dev_err(rpi->dev, "Firmware error getting GPIO state: %d\n", + packet[0]); + return -EINVAL; + } else { + return packet[1]; + } +} + +static int rpi_gpio_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + struct device_node *fw_node; + struct rpi_gpio *rpi; + u32 ngpio; + int ret; + + rpi = devm_kzalloc(dev, sizeof *rpi, GFP_KERNEL); + if (!rpi) + return -ENOMEM; + rpi->dev = dev; + + fw_node = of_parse_phandle(np, "firmware", 0); + if (!fw_node) { + dev_err(dev, "Missing firmware node\n"); + return -ENOENT; + } + + rpi->firmware = rpi_firmware_get(fw_node); + if (!rpi->firmware) + return -EPROBE_DEFER; + + if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio)) { + dev_err(dev, "Missing ngpios"); + return -ENOENT; + } + if (of_property_read_u32(pdev->dev.of_node, + "raspberrypi,firmware-gpio-offset", + &rpi->offset)) { + dev_err(dev, "Missing raspberrypi,firmware-gpio-offset"); + return -ENOENT; + } + + rpi->gc.label = np->full_name; + rpi->gc.owner = THIS_MODULE; + rpi->gc.of_node = np; + rpi->gc.ngpio = ngpio; + rpi->gc.direction_input = rpi_gpio_dir_in; + rpi->gc.direction_output = rpi_gpio_dir_out; + rpi->gc.get = rpi_gpio_get; + rpi->gc.set = rpi_gpio_set; + rpi->gc.can_sleep = true; + + ret = gpiochip_add(&rpi->gc); + if (ret) + return ret; + + platform_set_drvdata(pdev, rpi); + + return 0; +} + +static int rpi_gpio_remove(struct platform_device *pdev) +{ + struct rpi_gpio *rpi = platform_get_drvdata(pdev); + + gpiochip_remove(&rpi->gc); + + return 0; +} + +static const struct of_device_id __maybe_unused rpi_gpio_ids[] = { + { .compatible = "raspberrypi,firmware-gpio" }, + { } +}; +MODULE_DEVICE_TABLE(of, rpi_gpio_ids); + +static struct platform_driver rpi_gpio_driver = { + .driver = { + .name = "gpio-raspberrypi-firmware", + .of_match_table = of_match_ptr(rpi_gpio_ids), + }, + .probe = rpi_gpio_probe, + .remove = rpi_gpio_remove, +}; +module_platform_driver(rpi_gpio_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Eric Anholt <eric@anholt.net>"); +MODULE_DESCRIPTION("Raspberry Pi firmware GPIO driver"); diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h index 3fb357193f09..671ccd00aea2 100644 --- a/include/soc/bcm2835/raspberrypi-firmware.h +++ b/include/soc/bcm2835/raspberrypi-firmware.h @@ -73,11 +73,13 @@ enum rpi_firmware_property_tag { RPI_FIRMWARE_GET_DISPMANX_RESOURCE_MEM_HANDLE = 0x00030014, RPI_FIRMWARE_GET_EDID_BLOCK = 0x00030020, RPI_FIRMWARE_GET_DOMAIN_STATE = 0x00030030, + RPI_FIRMWARE_GET_GPIO_STATE = 0x00030041, RPI_FIRMWARE_SET_CLOCK_STATE = 0x00038001, RPI_FIRMWARE_SET_CLOCK_RATE = 0x00038002, RPI_FIRMWARE_SET_VOLTAGE = 0x00038003, RPI_FIRMWARE_SET_TURBO = 0x00038009, RPI_FIRMWARE_SET_DOMAIN_STATE = 0x00038030, + RPI_FIRMWARE_SET_GPIO_STATE = 0x00038041, /* Dispmanx TAGS */ RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE = 0x00040001,
This driver will be used for accessing the FXL6408 GPIO exander on the Pi3. We can't drive it directly from Linux because the firmware is continuously polling one of the expander's lines to do its undervoltage detection. Signed-off-by: Eric Anholt <eric@anholt.net> --- drivers/gpio/Kconfig | 8 ++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-raspberrypi.c | 159 +++++++++++++++++++++++++++++ include/soc/bcm2835/raspberrypi-firmware.h | 2 + 4 files changed, 170 insertions(+) create mode 100644 drivers/gpio/gpio-raspberrypi.c