Re: [PATCH v8 2/4] pinctrl: cygnus: add gpio/pinconf driver
On 2/9/2015 11:20 AM, Dmitry Torokhov wrote:
Hi Ray,
On Wed, Feb 04, 2015 at 09:21:01AM -0800, Ray Jui wrote:
+static int cygnus_gpio_pinmux_add_range(struct cygnus_gpio *chip)
+{
+struct device_node *node = chip-dev-of_node;
+struct device_node *pinmux_node;
+struct platform_device *pinmux_pdev;
+struct gpio_chip *gc = chip-gc;
+int i, ret;
+
+/* parse DT to find the phandle to the pinmux controller */
+pinmux_node = of_parse_phandle(node, pinmux, 0);
+if (!pinmux_node)
+return -ENODEV;
+
+pinmux_pdev = of_find_device_by_node(pinmux_node);
+if (!pinmux_pdev) {
+dev_err(chip-dev, failed to get pinmux device\n);
+return -EINVAL;
+}
+
+/* now need to create the mapping between local GPIO and PINMUX pins */
+for (i = 0; i ARRAY_SIZE(cygnus_gpio_pintable); i++) {
+ret = gpiochip_add_pin_range(gc, dev_name(pinmux_pdev-dev),
+ cygnus_gpio_pintable[i].offset,
+ cygnus_gpio_pintable[i].pin_base,
+ cygnus_gpio_pintable[i].num_pins);
+if (ret) {
+dev_err(chip-dev, unable to add GPIO pin range\n);
+goto err_put_device;
+}
+}
+
+chip-pinmux_is_supported = true;
+
+/* no need for pinmux_pdev device reference anymore */
+put_device(pinmux_pdev-dev);
Sorry I did not notice it before, but of_parse_phandle() takes reference
to the returned device node, so you need to put it here and in error
path as well. Actually you can do:
int ret = 0;
pinmux_node = of_parse_phandle(node, pinmux, 0);
if (!pinmux_node)
return -ENODEV;
pinmux_pdev = of_find_device_by_node(pinmux_node);
/* We do not longer need pinmux node */
of_node_put(pinmux_node);
if (!pinmux_dev)
for (..) {
...
if (ret) {
dev_err(...);
break;
}
}
chip-pinmux_is_supported = (ret == 0);
put_device(..);
return ret;
}
This way you free resources in the same path.
Thanks. I'll make the change.
...
+
+static struct platform_driver cygnus_gpio_driver = {
+.driver = {
+.name = cygnus-gpio,
+.of_match_table = cygnus_gpio_of_match,
+.suppress_bind_attrs = true,
+},
+.probe = cygnus_gpio_probe,
+};
+
+static int __init cygnus_gpio_init(void)
+{
+return platform_driver_probe(cygnus_gpio_driver, cygnus_gpio_probe);
When I asked you to add .suppress_bind_attrs = true I missed the fact
that you were using platform_driver_probe() which already does this
internally. However platform_driver_probe() can't handle deferred
probing, which may or may not be OK. Is there a chance that any of the
resources needed by the driver return -EPROBE_DEFER? If not then it is
safe to continue using platform_driver_probe() and you can drop
suppress_bind_attrs assignment, otherwise it may be better to switch to
platform_driver_register().
Thanks.
No I do not expect any resource that this driver depends on to return
-EPROBE_DEFER. The IOMUX driver that this driver depends on should be
initialized before this driver.
I'll drop .suppress_bind_attrs then. Thanks.
Ray
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/4] pinctrl: cygnus: add gpio/pinconf driver
Hi Ray,
On Wed, Feb 04, 2015 at 09:21:01AM -0800, Ray Jui wrote:
+static int cygnus_gpio_pinmux_add_range(struct cygnus_gpio *chip)
+{
+ struct device_node *node = chip-dev-of_node;
+ struct device_node *pinmux_node;
+ struct platform_device *pinmux_pdev;
+ struct gpio_chip *gc = chip-gc;
+ int i, ret;
+
+ /* parse DT to find the phandle to the pinmux controller */
+ pinmux_node = of_parse_phandle(node, pinmux, 0);
+ if (!pinmux_node)
+ return -ENODEV;
+
+ pinmux_pdev = of_find_device_by_node(pinmux_node);
+ if (!pinmux_pdev) {
+ dev_err(chip-dev, failed to get pinmux device\n);
+ return -EINVAL;
+ }
+
+ /* now need to create the mapping between local GPIO and PINMUX pins */
+ for (i = 0; i ARRAY_SIZE(cygnus_gpio_pintable); i++) {
+ ret = gpiochip_add_pin_range(gc, dev_name(pinmux_pdev-dev),
+ cygnus_gpio_pintable[i].offset,
+ cygnus_gpio_pintable[i].pin_base,
+ cygnus_gpio_pintable[i].num_pins);
+ if (ret) {
+ dev_err(chip-dev, unable to add GPIO pin range\n);
+ goto err_put_device;
+ }
+ }
+
+ chip-pinmux_is_supported = true;
+
+ /* no need for pinmux_pdev device reference anymore */
+ put_device(pinmux_pdev-dev);
Sorry I did not notice it before, but of_parse_phandle() takes reference
to the returned device node, so you need to put it here and in error
path as well. Actually you can do:
int ret = 0;
pinmux_node = of_parse_phandle(node, pinmux, 0);
if (!pinmux_node)
return -ENODEV;
pinmux_pdev = of_find_device_by_node(pinmux_node);
/* We do not longer need pinmux node */
of_node_put(pinmux_node);
if (!pinmux_dev)
for (..) {
...
if (ret) {
dev_err(...);
break;
}
}
chip-pinmux_is_supported = (ret == 0);
put_device(..);
return ret;
}
This way you free resources in the same path.
...
+
+static struct platform_driver cygnus_gpio_driver = {
+ .driver = {
+ .name = cygnus-gpio,
+ .of_match_table = cygnus_gpio_of_match,
+ .suppress_bind_attrs = true,
+ },
+ .probe = cygnus_gpio_probe,
+};
+
+static int __init cygnus_gpio_init(void)
+{
+ return platform_driver_probe(cygnus_gpio_driver, cygnus_gpio_probe);
When I asked you to add .suppress_bind_attrs = true I missed the fact
that you were using platform_driver_probe() which already does this
internally. However platform_driver_probe() can't handle deferred
probing, which may or may not be OK. Is there a chance that any of the
resources needed by the driver return -EPROBE_DEFER? If not then it is
safe to continue using platform_driver_probe() and you can drop
suppress_bind_attrs assignment, otherwise it may be better to switch to
platform_driver_register().
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 2/4] pinctrl: cygnus: add gpio/pinconf driver
This adds the initial support of the Broadcom Cygnus GPIO/PINCONF driver that supports all 3 GPIO controllers on Cygnus including the ASIU GPIO controller, the chipCommonG GPIO controller, and the always-on GPIO controller. Basic PINCONF configurations such as bias pull up/down, and drive strength are also supported in this driver. Pins from the ASIU GPIO controller can be individually muxed to GPIO function, through interaction with the Cygnus IOMUX controller Signed-off-by: Ray Jui r...@broadcom.com Reviewed-by: Scott Branden sbran...@broadcom.com --- drivers/pinctrl/bcm/Kconfig | 22 + drivers/pinctrl/bcm/Makefile |1 + drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c | 907 + 3 files changed, 930 insertions(+) create mode 100644 drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig index eb13201..cd11d4d 100644 --- a/drivers/pinctrl/bcm/Kconfig +++ b/drivers/pinctrl/bcm/Kconfig @@ -20,6 +20,28 @@ config PINCTRL_BCM2835 select PINMUX select PINCONF +config PINCTRL_CYGNUS_GPIO + bool Broadcom Cygnus GPIO (with PINCONF) driver + depends on OF_GPIO ARCH_BCM_CYGNUS + select GPIOLIB_IRQCHIP + select PINCONF + select GENERIC_PINCONF + default ARCH_BCM_CYGNUS + help + Say yes here to enable the Broadcom Cygnus GPIO driver. + + The Broadcom Cygnus SoC has 3 GPIO controllers including the ASIU + GPIO controller (ASIU), the chipCommonG GPIO controller (CCM), and + the always-ON GPIO controller (CRMU/AON). All 3 GPIO controllers are + supported by this driver. + + All 3 Cygnus GPIO controllers support basic PINCONF functions such + as bias pull up, pull down, and drive strength configurations, when + these pins are muxed to GPIO. + + Pins from the ASIU GPIO can be individually muxed to GPIO function, + through interaction with the Cygnus IOMUX controller. + config PINCTRL_CYGNUS_MUX bool Broadcom Cygnus IOMUX driver depends on (ARCH_BCM_CYGNUS || COMPILE_TEST) diff --git a/drivers/pinctrl/bcm/Makefile b/drivers/pinctrl/bcm/Makefile index bb6beb6..2b2f70e 100644 --- a/drivers/pinctrl/bcm/Makefile +++ b/drivers/pinctrl/bcm/Makefile @@ -2,4 +2,5 @@ obj-$(CONFIG_PINCTRL_BCM281XX) += pinctrl-bcm281xx.o obj-$(CONFIG_PINCTRL_BCM2835) += pinctrl-bcm2835.o +obj-$(CONFIG_PINCTRL_CYGNUS_GPIO) += pinctrl-cygnus-gpio.o obj-$(CONFIG_PINCTRL_CYGNUS_MUX) += pinctrl-cygnus-mux.o diff --git a/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c b/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c new file mode 100644 index 000..1feab0c --- /dev/null +++ b/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c @@ -0,0 +1,907 @@ +/* + * Copyright (C) 2014-2015 Broadcom Corporation + * + * 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 version 2. + * + * This program is distributed as is WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * This file contains the Broadcom Cygnus GPIO driver that supports 3 + * GPIO controllers on Cygnus including the ASIU GPIO controller, the + * chipCommonG GPIO controller, and the always-on GPIO controller. Basic + * PINCONF such as bias pull up/down, and drive strength are also supported + * in this driver. + * + * Pins from the ASIU GPIO can be individually muxed to GPIO function, + * through the interaction with the Cygnus IOMUX controller + */ + +#include linux/kernel.h +#include linux/slab.h +#include linux/module.h +#include linux/interrupt.h +#include linux/io.h +#include linux/gpio.h +#include linux/ioport.h +#include linux/of_device.h +#include linux/of_irq.h +#include linux/pinctrl/pinctrl.h +#include linux/pinctrl/pinmux.h +#include linux/pinctrl/pinconf.h +#include linux/pinctrl/pinconf-generic.h + +#include ../pinctrl-utils.h + +#define CYGNUS_GPIO_DATA_IN_OFFSET 0x00 +#define CYGNUS_GPIO_DATA_OUT_OFFSET 0x04 +#define CYGNUS_GPIO_OUT_EN_OFFSET0x08 +#define CYGNUS_GPIO_IN_TYPE_OFFSET 0x0c +#define CYGNUS_GPIO_INT_DE_OFFSET0x10 +#define CYGNUS_GPIO_INT_EDGE_OFFSET 0x14 +#define CYGNUS_GPIO_INT_MSK_OFFSET 0x18 +#define CYGNUS_GPIO_INT_STAT_OFFSET 0x1c +#define CYGNUS_GPIO_INT_MSTAT_OFFSET 0x20 +#define CYGNUS_GPIO_INT_CLR_OFFSET 0x24 +#define CYGNUS_GPIO_PAD_RES_OFFSET 0x34 +#define CYGNUS_GPIO_RES_EN_OFFSET0x38 + +/* drive strength control for ASIU GPIO */ +#define CYGNUS_GPIO_ASIU_DRV0_CTRL_OFFSET 0x58 + +/* drive strength control for CCM/CRMU (AON) GPIO */ +#define CYGNUS_GPIO_DRV0_CTRL_OFFSET 0x00 + +#define GPIO_BANK_SIZE 0x200 +#define NGPIOS_PER_BANK 32 +#define GPIO_BANK(pin) ((pin) /
