* [PATCH v4] gpio: Add MOXA ART GPIO driver
@ 2013-07-29 13:06 ` Jonas Jensen
0 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-07-29 13:06 UTC (permalink / raw)
To: linux-arm-kernel
Add GPIO driver for MOXA ART SoCs.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Changes since v3:
1. use local struct device *dev pointer, replace "&pdev->dev" with "dev"
device tree bindings document:
2. describe compatible variable "Must be" instead of "Should be".
Applies to next-20130729
.../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 23 +++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-moxart.c | 189 +++++++++++++++++++++
4 files changed, 220 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
create mode 100644 drivers/gpio/gpio-moxart.c
diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
new file mode 100644
index 0000000..795afab
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
@@ -0,0 +1,23 @@
+MOXA ART GPIO Controller
+
+Required properties:
+
+- #gpio-cells : Should be 2
+- compatible : Must be "moxa,moxart-gpio"
+- reg : Should contain registers location and length
+ index 0 : input, output, and direction control
+ index 1 : enable/disable individual pins, pin 0-31
+- gpio-ready-led : ready LED gpio, with zero flags
+- gpio-reset-switch : reset switch gpio, with zero flags
+
+Example:
+
+ gpio: gpio at 98700000 {
+ gpio-controller;
+ #gpio-cells = <2>;
+ compatible = "moxa,moxart-gpio";
+ reg = <0x98700000 0xC>,
+ <0x98100100 0x4>;
+ gpio-ready-led = <&gpio 27 0>;
+ gpio-reset-switch = <&gpio 25 0>;
+ };
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 4b7ba53..5419413 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -741,6 +741,13 @@ config GPIO_MSIC
Enable support for GPIO on intel MSIC controllers found in
intel MID devices
+config GPIO_MOXART
+ bool "MOXART GPIO support"
+ depends on ARCH_MOXART
+ help
+ Select this option to enable GPIO driver for
+ MOXA ART SoC devices.
+
comment "USB GPIO expanders:"
config GPIO_VIPERBOARD
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 156fd28..c7a7a2b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08) += gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
new file mode 100644
index 0000000..a5bdbca
--- /dev/null
+++ b/drivers/gpio/gpio-moxart.c
@@ -0,0 +1,189 @@
+/*
+ * MOXA ART SoCs GPIO driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+
+#define GPIO_DATA_OUT 0x00
+#define GPIO_DATA_IN 0x04
+#define GPIO_PIN_DIRECTION 0x08
+
+static void __iomem *moxart_pincontrol_base;
+static void __iomem *moxart_gpio_base;
+
+void moxart_gpio_enable(u32 gpio)
+{
+ writel(readl(moxart_pincontrol_base) | gpio, moxart_pincontrol_base);
+}
+
+void moxart_gpio_disable(u32 gpio)
+{
+ writel(readl(moxart_pincontrol_base) & ~gpio, moxart_pincontrol_base);
+}
+
+static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ moxart_gpio_enable(1 << offset);
+ return pinctrl_request_gpio(offset);
+}
+
+static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(offset);
+ moxart_gpio_disable(1 << offset);
+}
+
+static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) & ~(1 << offset), ioaddr);
+ return 0;
+}
+
+static int moxart_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) | (1 << offset), ioaddr);
+ return 0;
+}
+
+static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_DATA_OUT;
+ u32 reg = readl(ioaddr);
+
+ (value) ? writel(reg | (1 << offset), ioaddr) :
+ writel(reg & ~(1 << offset), ioaddr);
+}
+
+static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ u32 ret = readl(moxart_gpio_base + GPIO_PIN_DIRECTION);
+
+ if (ret & (1 << offset))
+ ret = readl(moxart_gpio_base + GPIO_DATA_OUT) & (1 << offset);
+ else
+ ret = readl(moxart_gpio_base + GPIO_DATA_IN) & (1 << offset);
+
+ return ret;
+}
+
+static struct gpio_chip moxart_gpio_chip = {
+ .label = "moxart-gpio",
+ .request = moxart_gpio_request,
+ .free = moxart_gpio_free,
+ .direction_input = moxart_gpio_direction_input,
+ .direction_output = moxart_gpio_direction_output,
+ .set = moxart_gpio_set,
+ .get = moxart_gpio_get,
+ .base = 0,
+ .ngpio = 32,
+ .can_sleep = 0,
+};
+
+static int moxart_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int ret, gpio_ready_led, gpio_reset_switch;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ moxart_gpio_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(moxart_gpio_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(moxart_gpio_base);
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ moxart_pincontrol_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(moxart_pincontrol_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_pmu failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(moxart_pincontrol_base);
+ }
+
+ moxart_gpio_chip.dev = dev;
+
+ ret = gpiochip_add(&moxart_gpio_chip);
+ if (ret)
+ dev_err(dev, "%s: gpiochip_add failed\n",
+ dev->of_node->full_name);
+
+
+ gpio_ready_led = of_get_named_gpio(dev->of_node,
+ "gpio-ready-led", 0);
+ if (!gpio_is_valid(gpio_ready_led)) {
+ dev_err(dev, "invalid gpio (gpio-ready-led): %d\n",
+ gpio_ready_led);
+ return gpio_ready_led;
+ }
+
+ gpio_reset_switch = of_get_named_gpio(dev->of_node,
+ "gpio-reset-switch", 0);
+ if (!gpio_is_valid(gpio_reset_switch)) {
+ dev_err(dev, "invalid gpio (gpio-reset-switch): %d\n",
+ gpio_reset_switch);
+ return gpio_reset_switch;
+ }
+
+ moxart_gpio_enable(gpio_ready_led | gpio_reset_switch);
+
+ moxart_gpio_direction_input(&moxart_gpio_chip, gpio_reset_switch);
+
+ /*
+ * gpio_ready_led=0 ready LED on
+ * gpio_ready_led=1 ready LED off
+ */
+ moxart_gpio_direction_output(&moxart_gpio_chip, gpio_ready_led, 0);
+ moxart_gpio_set(&moxart_gpio_chip, gpio_ready_led, 0);
+
+ return 0;
+}
+
+static const struct of_device_id moxart_gpio_match[] = {
+ { .compatible = "moxa,moxart-gpio" },
+ { }
+};
+
+static struct platform_driver moxart_gpio_driver = {
+ .driver = {
+ .name = "moxart-gpio",
+ .owner = THIS_MODULE,
+ .of_match_table = moxart_gpio_match,
+ },
+ .probe = moxart_gpio_probe,
+};
+
+static int __init moxart_gpio_init(void)
+{
+ return platform_driver_register(&moxart_gpio_driver);
+}
+
+postcore_initcall(moxart_gpio_init);
+
+MODULE_DESCRIPTION("MOXART GPIO chip driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4] gpio: Add MOXA ART GPIO driver
2013-07-29 13:06 ` Jonas Jensen
@ 2013-08-02 11:34 ` Mark Rutland
-1 siblings, 0 replies; 47+ messages in thread
From: Mark Rutland @ 2013-08-02 11:34 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, arnd, linus.walleij, linux-kernel, arm, grant.likely,
linux-arm-kernel
On Mon, Jul 29, 2013 at 02:06:01PM +0100, Jonas Jensen wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
>
> Notes:
> Changes since v3:
>
> 1. use local struct device *dev pointer, replace "&pdev->dev" with "dev"
>
> device tree bindings document:
> 2. describe compatible variable "Must be" instead of "Should be".
>
> Applies to next-20130729
>
> .../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 23 +++
> drivers/gpio/Kconfig | 7 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-moxart.c | 189 +++++++++++++++++++++
> 4 files changed, 220 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
> create mode 100644 drivers/gpio/gpio-moxart.c
>
> diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
> new file mode 100644
> index 0000000..795afab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
> @@ -0,0 +1,23 @@
> +MOXA ART GPIO Controller
> +
> +Required properties:
> +
> +- #gpio-cells : Should be 2
Could you elaborate on what those cells represent?
> +- compatible : Must be "moxa,moxart-gpio"
> +- reg : Should contain registers location and length
> + index 0 : input, output, and direction control
> + index 1 : enable/disable individual pins, pin 0-31
These seem rather fine-grained. Are they not part of a larger bank of
registers? The example seems to indicate otherwise, but I don't trust
examples :)
> +- gpio-ready-led : ready LED gpio, with zero flags
> +- gpio-reset-switch : reset switch gpio, with zero flags
I'm not sure about these. It feels odd for the gpio node to refer to
itself in this way. Why is the use of these gpios a concern of the gpio
controller. Surely an external user described elsewhere in dt will be
assigned these (even if it's general platform code rather than a
specific hardware driver)?
I thought there were some conventions for gpio-driven LEDs...
Also, I believe the convention is to have ${NAME}-gpios, or just gpios.
[...]
> +static int moxart_gpio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + int ret, gpio_ready_led, gpio_reset_switch;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + moxart_gpio_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(moxart_gpio_base)) {
> + dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
> + dev->of_node->full_name);
> + return PTR_ERR(moxart_gpio_base);
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + moxart_pincontrol_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(moxart_pincontrol_base)) {
> + dev_err(dev, "%s: devm_ioremap_resource res_pmu failed\n",
> + dev->of_node->full_name);
> + return PTR_ERR(moxart_pincontrol_base);
> + }
> +
> + moxart_gpio_chip.dev = dev;
> +
> + ret = gpiochip_add(&moxart_gpio_chip);
> + if (ret)
> + dev_err(dev, "%s: gpiochip_add failed\n",
> + dev->of_node->full_name);
> +
> +
> + gpio_ready_led = of_get_named_gpio(dev->of_node,
> + "gpio-ready-led", 0);
> + if (!gpio_is_valid(gpio_ready_led)) {
> + dev_err(dev, "invalid gpio (gpio-ready-led): %d\n",
> + gpio_ready_led);
> + return gpio_ready_led;
> + }
> +
> + gpio_reset_switch = of_get_named_gpio(dev->of_node,
> + "gpio-reset-switch", 0);
> + if (!gpio_is_valid(gpio_reset_switch)) {
> + dev_err(dev, "invalid gpio (gpio-reset-switch): %d\n",
> + gpio_reset_switch);
> + return gpio_reset_switch;
> + }
> +
> + moxart_gpio_enable(gpio_ready_led | gpio_reset_switch);
> +
> + moxart_gpio_direction_input(&moxart_gpio_chip, gpio_reset_switch);
We never seem to do anything else with the reset switch. Is it used
elsewhere? Surely the "real" user should call in to initialise this.
> +
> + /*
> + * gpio_ready_led=0 ready LED on
> + * gpio_ready_led=1 ready LED off
> + */
> + moxart_gpio_direction_output(&moxart_gpio_chip, gpio_ready_led, 0);
> + moxart_gpio_set(&moxart_gpio_chip, gpio_ready_led, 0);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id moxart_gpio_match[] = {
> + { .compatible = "moxa,moxart-gpio" },
> + { }
> +};
> +
> +static struct platform_driver moxart_gpio_driver = {
> + .driver = {
> + .name = "moxart-gpio",
> + .owner = THIS_MODULE,
> + .of_match_table = moxart_gpio_match,
> + },
> + .probe = moxart_gpio_probe,
> +};
> +
> +static int __init moxart_gpio_init(void)
> +{
> + return platform_driver_register(&moxart_gpio_driver);
> +}
> +
> +postcore_initcall(moxart_gpio_init);
> +
> +MODULE_DESCRIPTION("MOXART GPIO chip driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
> --
> 1.8.2.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v4] gpio: Add MOXA ART GPIO driver
@ 2013-08-02 11:34 ` Mark Rutland
0 siblings, 0 replies; 47+ messages in thread
From: Mark Rutland @ 2013-08-02 11:34 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 29, 2013 at 02:06:01PM +0100, Jonas Jensen wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
>
> Notes:
> Changes since v3:
>
> 1. use local struct device *dev pointer, replace "&pdev->dev" with "dev"
>
> device tree bindings document:
> 2. describe compatible variable "Must be" instead of "Should be".
>
> Applies to next-20130729
>
> .../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 23 +++
> drivers/gpio/Kconfig | 7 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-moxart.c | 189 +++++++++++++++++++++
> 4 files changed, 220 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
> create mode 100644 drivers/gpio/gpio-moxart.c
>
> diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
> new file mode 100644
> index 0000000..795afab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
> @@ -0,0 +1,23 @@
> +MOXA ART GPIO Controller
> +
> +Required properties:
> +
> +- #gpio-cells : Should be 2
Could you elaborate on what those cells represent?
> +- compatible : Must be "moxa,moxart-gpio"
> +- reg : Should contain registers location and length
> + index 0 : input, output, and direction control
> + index 1 : enable/disable individual pins, pin 0-31
These seem rather fine-grained. Are they not part of a larger bank of
registers? The example seems to indicate otherwise, but I don't trust
examples :)
> +- gpio-ready-led : ready LED gpio, with zero flags
> +- gpio-reset-switch : reset switch gpio, with zero flags
I'm not sure about these. It feels odd for the gpio node to refer to
itself in this way. Why is the use of these gpios a concern of the gpio
controller. Surely an external user described elsewhere in dt will be
assigned these (even if it's general platform code rather than a
specific hardware driver)?
I thought there were some conventions for gpio-driven LEDs...
Also, I believe the convention is to have ${NAME}-gpios, or just gpios.
[...]
> +static int moxart_gpio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + int ret, gpio_ready_led, gpio_reset_switch;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + moxart_gpio_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(moxart_gpio_base)) {
> + dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
> + dev->of_node->full_name);
> + return PTR_ERR(moxart_gpio_base);
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + moxart_pincontrol_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(moxart_pincontrol_base)) {
> + dev_err(dev, "%s: devm_ioremap_resource res_pmu failed\n",
> + dev->of_node->full_name);
> + return PTR_ERR(moxart_pincontrol_base);
> + }
> +
> + moxart_gpio_chip.dev = dev;
> +
> + ret = gpiochip_add(&moxart_gpio_chip);
> + if (ret)
> + dev_err(dev, "%s: gpiochip_add failed\n",
> + dev->of_node->full_name);
> +
> +
> + gpio_ready_led = of_get_named_gpio(dev->of_node,
> + "gpio-ready-led", 0);
> + if (!gpio_is_valid(gpio_ready_led)) {
> + dev_err(dev, "invalid gpio (gpio-ready-led): %d\n",
> + gpio_ready_led);
> + return gpio_ready_led;
> + }
> +
> + gpio_reset_switch = of_get_named_gpio(dev->of_node,
> + "gpio-reset-switch", 0);
> + if (!gpio_is_valid(gpio_reset_switch)) {
> + dev_err(dev, "invalid gpio (gpio-reset-switch): %d\n",
> + gpio_reset_switch);
> + return gpio_reset_switch;
> + }
> +
> + moxart_gpio_enable(gpio_ready_led | gpio_reset_switch);
> +
> + moxart_gpio_direction_input(&moxart_gpio_chip, gpio_reset_switch);
We never seem to do anything else with the reset switch. Is it used
elsewhere? Surely the "real" user should call in to initialise this.
> +
> + /*
> + * gpio_ready_led=0 ready LED on
> + * gpio_ready_led=1 ready LED off
> + */
> + moxart_gpio_direction_output(&moxart_gpio_chip, gpio_ready_led, 0);
> + moxart_gpio_set(&moxart_gpio_chip, gpio_ready_led, 0);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id moxart_gpio_match[] = {
> + { .compatible = "moxa,moxart-gpio" },
> + { }
> +};
> +
> +static struct platform_driver moxart_gpio_driver = {
> + .driver = {
> + .name = "moxart-gpio",
> + .owner = THIS_MODULE,
> + .of_match_table = moxart_gpio_match,
> + },
> + .probe = moxart_gpio_probe,
> +};
> +
> +static int __init moxart_gpio_init(void)
> +{
> + return platform_driver_register(&moxart_gpio_driver);
> +}
> +
> +postcore_initcall(moxart_gpio_init);
> +
> +MODULE_DESCRIPTION("MOXART GPIO chip driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
> --
> 1.8.2.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v4] gpio: Add MOXA ART GPIO driver
2013-07-29 13:06 ` Jonas Jensen
(?)
@ 2013-08-16 14:05 ` Linus Walleij
-1 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-08-16 14:05 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, devicetree
On Mon, Jul 29, 2013 at 3:06 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
(...)
> +++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
This needs to be copied to devicetree@vger.kernel.org and I want
an ACK from some DT maintainer as well.
> @@ -0,0 +1,23 @@
> +MOXA ART GPIO Controller
> +
> +Required properties:
> +
> +- #gpio-cells : Should be 2
> +- compatible : Must be "moxa,moxart-gpio"
> +- reg : Should contain registers location and length
> + index 0 : input, output, and direction control
> + index 1 : enable/disable individual pins, pin 0-31
What? Why is the same block deploying these things in two
totally different places in memory?? Random HW engineer
having fun at work or...?
> +- gpio-ready-led : ready LED gpio, with zero flags
> +- gpio-reset-switch : reset switch gpio, with zero flags
This does not belong in the GPIO node, or in the GPIO
driver for that matter. This shall be handled by e.g. connecting
drivers/leds/leds-gpio.c to a GPIO using device tree by
selecting that driver in your config and do like this in your
device tree:
leds {
compatible = "gpio-leds";
user-led {
label = "user_led";
gpios = <&gpio0 2 0x1>;
default-state = "off";
linux,default-trigger = "heartbeat";
};
};
(Gives a headtbeat trigger as well, skip that if you don't
want it.)
We have a generic reset-over-gpio driver as well, see
drivers/power/reset/gpio-poweroff.c
> +++ b/drivers/gpio/gpio-moxart.c
> @@ -0,0 +1,189 @@
> +/*
> + * MOXA ART SoCs GPIO driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/io.h>
> +#include <linux/gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/delay.h>
> +#include <linux/timer.h>
> +
> +#define GPIO_DATA_OUT 0x00
> +#define GPIO_DATA_IN 0x04
> +#define GPIO_PIN_DIRECTION 0x08
> +
> +static void __iomem *moxart_pincontrol_base;
> +static void __iomem *moxart_gpio_base;
> +
> +void moxart_gpio_enable(u32 gpio)
> +{
> + writel(readl(moxart_pincontrol_base) | gpio, moxart_pincontrol_base);
> +}
> +
> +void moxart_gpio_disable(u32 gpio)
> +{
> + writel(readl(moxart_pincontrol_base) & ~gpio, moxart_pincontrol_base);
> +}
> +
> +static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> + moxart_gpio_enable(1 << offset);
Do this:
#include <linux/bitops.h>
moxart_gpio_enable(BIT(offset));
Repeat the comment for every similar instance below.
> +static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> + void __iomem *ioaddr = moxart_gpio_base + GPIO_DATA_OUT;
> + u32 reg = readl(ioaddr);
> +
> + (value) ? writel(reg | (1 << offset), ioaddr) :
> + writel(reg & ~(1 << offset), ioaddr);
Isn't that a bit hard to read?
if (value)
reg |= BIT(offset);
else
reg &= ~BIT(offset);
writel(reg, ioaddr);
> +static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + u32 ret = readl(moxart_gpio_base + GPIO_PIN_DIRECTION);
> +
> + if (ret & (1 << offset))
> + ret = readl(moxart_gpio_base + GPIO_DATA_OUT) & (1 << offset);
> + else
> + ret = readl(moxart_gpio_base + GPIO_DATA_IN) & (1 << offset);
Use this construct:
if (ret & BIT(offset)
return !!(readl(moxart_gpio_base + GPIO_DATA_OUT) & BIT(offset));
return !!(readl(moxart_gpio_base + GPIO_DATA_IN) & BIT(offset));
> + ret = gpiochip_add(&moxart_gpio_chip);
> + if (ret)
> + dev_err(dev, "%s: gpiochip_add failed\n",
> + dev->of_node->full_name);
You need to bail out here, right? Not continue to do
dangerous stuff.
> + gpio_ready_led = of_get_named_gpio(dev->of_node,
> + "gpio-ready-led", 0);
> + if (!gpio_is_valid(gpio_ready_led)) {
> + dev_err(dev, "invalid gpio (gpio-ready-led): %d\n",
> + gpio_ready_led);
> + return gpio_ready_led;
> + }
> +
> + gpio_reset_switch = of_get_named_gpio(dev->of_node,
> + "gpio-reset-switch", 0);
> + if (!gpio_is_valid(gpio_reset_switch)) {
> + dev_err(dev, "invalid gpio (gpio-reset-switch): %d\n",
> + gpio_reset_switch);
> + return gpio_reset_switch;
> + }
Please get rid of this. Use standard drivers as described above.
> +static int __init moxart_gpio_init(void)
> +{
> + return platform_driver_register(&moxart_gpio_driver);
> +}
> +
> +postcore_initcall(moxart_gpio_init);
Do you really need to have them this early?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v4] gpio: Add MOXA ART GPIO driver
@ 2013-08-16 14:05 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-08-16 14:05 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 29, 2013 at 3:06 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
(...)
> +++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
This needs to be copied to devicetree at vger.kernel.org and I want
an ACK from some DT maintainer as well.
> @@ -0,0 +1,23 @@
> +MOXA ART GPIO Controller
> +
> +Required properties:
> +
> +- #gpio-cells : Should be 2
> +- compatible : Must be "moxa,moxart-gpio"
> +- reg : Should contain registers location and length
> + index 0 : input, output, and direction control
> + index 1 : enable/disable individual pins, pin 0-31
What? Why is the same block deploying these things in two
totally different places in memory?? Random HW engineer
having fun at work or...?
> +- gpio-ready-led : ready LED gpio, with zero flags
> +- gpio-reset-switch : reset switch gpio, with zero flags
This does not belong in the GPIO node, or in the GPIO
driver for that matter. This shall be handled by e.g. connecting
drivers/leds/leds-gpio.c to a GPIO using device tree by
selecting that driver in your config and do like this in your
device tree:
leds {
compatible = "gpio-leds";
user-led {
label = "user_led";
gpios = <&gpio0 2 0x1>;
default-state = "off";
linux,default-trigger = "heartbeat";
};
};
(Gives a headtbeat trigger as well, skip that if you don't
want it.)
We have a generic reset-over-gpio driver as well, see
drivers/power/reset/gpio-poweroff.c
> +++ b/drivers/gpio/gpio-moxart.c
> @@ -0,0 +1,189 @@
> +/*
> + * MOXA ART SoCs GPIO driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/io.h>
> +#include <linux/gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/delay.h>
> +#include <linux/timer.h>
> +
> +#define GPIO_DATA_OUT 0x00
> +#define GPIO_DATA_IN 0x04
> +#define GPIO_PIN_DIRECTION 0x08
> +
> +static void __iomem *moxart_pincontrol_base;
> +static void __iomem *moxart_gpio_base;
> +
> +void moxart_gpio_enable(u32 gpio)
> +{
> + writel(readl(moxart_pincontrol_base) | gpio, moxart_pincontrol_base);
> +}
> +
> +void moxart_gpio_disable(u32 gpio)
> +{
> + writel(readl(moxart_pincontrol_base) & ~gpio, moxart_pincontrol_base);
> +}
> +
> +static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> + moxart_gpio_enable(1 << offset);
Do this:
#include <linux/bitops.h>
moxart_gpio_enable(BIT(offset));
Repeat the comment for every similar instance below.
> +static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> + void __iomem *ioaddr = moxart_gpio_base + GPIO_DATA_OUT;
> + u32 reg = readl(ioaddr);
> +
> + (value) ? writel(reg | (1 << offset), ioaddr) :
> + writel(reg & ~(1 << offset), ioaddr);
Isn't that a bit hard to read?
if (value)
reg |= BIT(offset);
else
reg &= ~BIT(offset);
writel(reg, ioaddr);
> +static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + u32 ret = readl(moxart_gpio_base + GPIO_PIN_DIRECTION);
> +
> + if (ret & (1 << offset))
> + ret = readl(moxart_gpio_base + GPIO_DATA_OUT) & (1 << offset);
> + else
> + ret = readl(moxart_gpio_base + GPIO_DATA_IN) & (1 << offset);
Use this construct:
if (ret & BIT(offset)
return !!(readl(moxart_gpio_base + GPIO_DATA_OUT) & BIT(offset));
return !!(readl(moxart_gpio_base + GPIO_DATA_IN) & BIT(offset));
> + ret = gpiochip_add(&moxart_gpio_chip);
> + if (ret)
> + dev_err(dev, "%s: gpiochip_add failed\n",
> + dev->of_node->full_name);
You need to bail out here, right? Not continue to do
dangerous stuff.
> + gpio_ready_led = of_get_named_gpio(dev->of_node,
> + "gpio-ready-led", 0);
> + if (!gpio_is_valid(gpio_ready_led)) {
> + dev_err(dev, "invalid gpio (gpio-ready-led): %d\n",
> + gpio_ready_led);
> + return gpio_ready_led;
> + }
> +
> + gpio_reset_switch = of_get_named_gpio(dev->of_node,
> + "gpio-reset-switch", 0);
> + if (!gpio_is_valid(gpio_reset_switch)) {
> + dev_err(dev, "invalid gpio (gpio-reset-switch): %d\n",
> + gpio_reset_switch);
> + return gpio_reset_switch;
> + }
Please get rid of this. Use standard drivers as described above.
> +static int __init moxart_gpio_init(void)
> +{
> + return platform_driver_register(&moxart_gpio_driver);
> +}
> +
> +postcore_initcall(moxart_gpio_init);
Do you really need to have them this early?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v4] gpio: Add MOXA ART GPIO driver
@ 2013-08-16 14:05 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-08-16 14:05 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, devicetree
On Mon, Jul 29, 2013 at 3:06 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
(...)
> +++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
This needs to be copied to devicetree@vger.kernel.org and I want
an ACK from some DT maintainer as well.
> @@ -0,0 +1,23 @@
> +MOXA ART GPIO Controller
> +
> +Required properties:
> +
> +- #gpio-cells : Should be 2
> +- compatible : Must be "moxa,moxart-gpio"
> +- reg : Should contain registers location and length
> + index 0 : input, output, and direction control
> + index 1 : enable/disable individual pins, pin 0-31
What? Why is the same block deploying these things in two
totally different places in memory?? Random HW engineer
having fun at work or...?
> +- gpio-ready-led : ready LED gpio, with zero flags
> +- gpio-reset-switch : reset switch gpio, with zero flags
This does not belong in the GPIO node, or in the GPIO
driver for that matter. This shall be handled by e.g. connecting
drivers/leds/leds-gpio.c to a GPIO using device tree by
selecting that driver in your config and do like this in your
device tree:
leds {
compatible = "gpio-leds";
user-led {
label = "user_led";
gpios = <&gpio0 2 0x1>;
default-state = "off";
linux,default-trigger = "heartbeat";
};
};
(Gives a headtbeat trigger as well, skip that if you don't
want it.)
We have a generic reset-over-gpio driver as well, see
drivers/power/reset/gpio-poweroff.c
> +++ b/drivers/gpio/gpio-moxart.c
> @@ -0,0 +1,189 @@
> +/*
> + * MOXA ART SoCs GPIO driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/io.h>
> +#include <linux/gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/delay.h>
> +#include <linux/timer.h>
> +
> +#define GPIO_DATA_OUT 0x00
> +#define GPIO_DATA_IN 0x04
> +#define GPIO_PIN_DIRECTION 0x08
> +
> +static void __iomem *moxart_pincontrol_base;
> +static void __iomem *moxart_gpio_base;
> +
> +void moxart_gpio_enable(u32 gpio)
> +{
> + writel(readl(moxart_pincontrol_base) | gpio, moxart_pincontrol_base);
> +}
> +
> +void moxart_gpio_disable(u32 gpio)
> +{
> + writel(readl(moxart_pincontrol_base) & ~gpio, moxart_pincontrol_base);
> +}
> +
> +static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> + moxart_gpio_enable(1 << offset);
Do this:
#include <linux/bitops.h>
moxart_gpio_enable(BIT(offset));
Repeat the comment for every similar instance below.
> +static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> + void __iomem *ioaddr = moxart_gpio_base + GPIO_DATA_OUT;
> + u32 reg = readl(ioaddr);
> +
> + (value) ? writel(reg | (1 << offset), ioaddr) :
> + writel(reg & ~(1 << offset), ioaddr);
Isn't that a bit hard to read?
if (value)
reg |= BIT(offset);
else
reg &= ~BIT(offset);
writel(reg, ioaddr);
> +static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + u32 ret = readl(moxart_gpio_base + GPIO_PIN_DIRECTION);
> +
> + if (ret & (1 << offset))
> + ret = readl(moxart_gpio_base + GPIO_DATA_OUT) & (1 << offset);
> + else
> + ret = readl(moxart_gpio_base + GPIO_DATA_IN) & (1 << offset);
Use this construct:
if (ret & BIT(offset)
return !!(readl(moxart_gpio_base + GPIO_DATA_OUT) & BIT(offset));
return !!(readl(moxart_gpio_base + GPIO_DATA_IN) & BIT(offset));
> + ret = gpiochip_add(&moxart_gpio_chip);
> + if (ret)
> + dev_err(dev, "%s: gpiochip_add failed\n",
> + dev->of_node->full_name);
You need to bail out here, right? Not continue to do
dangerous stuff.
> + gpio_ready_led = of_get_named_gpio(dev->of_node,
> + "gpio-ready-led", 0);
> + if (!gpio_is_valid(gpio_ready_led)) {
> + dev_err(dev, "invalid gpio (gpio-ready-led): %d\n",
> + gpio_ready_led);
> + return gpio_ready_led;
> + }
> +
> + gpio_reset_switch = of_get_named_gpio(dev->of_node,
> + "gpio-reset-switch", 0);
> + if (!gpio_is_valid(gpio_reset_switch)) {
> + dev_err(dev, "invalid gpio (gpio-reset-switch): %d\n",
> + gpio_reset_switch);
> + return gpio_reset_switch;
> + }
Please get rid of this. Use standard drivers as described above.
> +static int __init moxart_gpio_init(void)
> +{
> + return platform_driver_register(&moxart_gpio_driver);
> +}
> +
> +postcore_initcall(moxart_gpio_init);
Do you really need to have them this early?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v5] gpio: Add MOXA ART GPIO driver
2013-07-29 13:06 ` Jonas Jensen
@ 2013-10-11 14:53 ` Jonas Jensen
-1 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-10-11 14:53 UTC (permalink / raw)
To: linux-gpio
Cc: grant.likely, linus.walleij, linux-arm-kernel, linux-kernel, arm,
arnd, mark.rutland, devicetree, Jonas Jensen
Add GPIO driver for MOXA ART SoCs.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks for the replies,
I agree it is a bit strange GPIO control is divided in two
separate registers. Unfortunately I can't offer an explanation
because the documentation is not publicly available.
The register responsible for doing enable/disable is located
at <0x98100100 0x4>, the clock register is very close at
<0x98100000 0x34>.
I don't think gpio_poweroff driver is right for this hardware
because the pin is not connected to anything that can do reset.
The old 2.6.9 kernel driver uses timer poll with eventual call
to userspace.
To test that it works, I added gpio_poweroff anyway, modified
with gpio_export() the pin can then be seen switching between
0 and 1 (on "cat /sys/class/gpio/gpio25/value").
The DT file I use on UC-7112-LX:
clk_pll: pll@98100000 {
compatible = "moxa,moxart-pll-clock";
#clock-cells = <0>;
reg = <0x98100000 0x34>;
clocks = <&ref12>;
};
clk_apb: clk_apb@98100000 {
compatible = "moxa,moxart-apb-clock";
#clock-cells = <0>;
reg = <0x98100000 0x34>;
clocks = <&clk_pll>;
};
gpio: gpio@98700000 {
gpio-controller;
#gpio-cells = <2>;
compatible = "moxa,moxart-gpio";
reg = <0x98700000 0xC>,
<0x98100100 0x4>;
};
leds {
compatible = "gpio-leds";
user-led {
label = "ready-led";
gpios = <&gpio 27 0x1>;
default-state = "on";
linux,default-trigger = "default-on";
};
};
gpio_poweroff {
compatible = "gpio-poweroff";
pinctrl-names = "default";
input = <1>;
gpios = <&gpio 25 0x0>;
};
Changes since v4:
1. elaborate DT binding #gpio-cells description
2. remove ready-led / reset-switch from driver and binding
3. use BIT() macro
4. remove ternary operator in moxart_gpio_set()
5. use !!(condition) construct in moxart_gpio_get()
6. replace postcore_initcall() with module_platform_driver()
7. return gpiochip_add() return value on failure
Applies to next-20130927
.../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 22 +++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-moxart.c | 163 +++++++++++++++++++++
4 files changed, 193 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
create mode 100644 drivers/gpio/gpio-moxart.c
diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
new file mode 100644
index 0000000..5039e17
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
@@ -0,0 +1,22 @@
+MOXA ART GPIO Controller
+
+Required properties:
+
+- #gpio-cells : Should be 2, The first cell is the pin number and
+ the second cell is used to specify polarity:
+ 0 = active high
+ 1 = active low
+- compatible : Must be "moxa,moxart-gpio"
+- reg : Should contain registers location and length
+ index 0 : input, output, and direction control
+ index 1 : enable/disable individual pins, pin 0-31
+
+Example:
+
+ gpio: gpio@98700000 {
+ gpio-controller;
+ #gpio-cells = <2>;
+ compatible = "moxa,moxart-gpio";
+ reg = <0x98700000 0xC>,
+ <0x98100100 0x4>;
+ };
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c8b02a5..c5a2767 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -156,6 +156,13 @@ config GPIO_F7188X
To compile this driver as a module, choose M here: the module will
be called f7188x-gpio.
+config GPIO_MOXART
+ bool "MOXART GPIO support"
+ depends on ARCH_MOXART
+ help
+ Select this option to enable GPIO driver for
+ MOXA ART SoC devices.
+
config GPIO_MPC5200
def_bool y
depends on PPC_MPC52xx
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5c353df..a26029d 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08) += gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
new file mode 100644
index 0000000..5796846
--- /dev/null
+++ b/drivers/gpio/gpio-moxart.c
@@ -0,0 +1,163 @@
+/*
+ * MOXA ART SoCs GPIO driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/bitops.h>
+
+#define GPIO_DATA_OUT 0x00
+#define GPIO_DATA_IN 0x04
+#define GPIO_PIN_DIRECTION 0x08
+
+static void __iomem *moxart_pincontrol_base;
+static void __iomem *moxart_gpio_base;
+
+void moxart_gpio_enable(u32 gpio)
+{
+ writel(readl(moxart_pincontrol_base) | gpio, moxart_pincontrol_base);
+}
+
+void moxart_gpio_disable(u32 gpio)
+{
+ writel(readl(moxart_pincontrol_base) & ~gpio, moxart_pincontrol_base);
+}
+
+static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ moxart_gpio_enable(BIT(offset));
+ return pinctrl_request_gpio(offset);
+}
+
+static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(offset);
+ moxart_gpio_disable(BIT(offset));
+}
+
+static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) & ~BIT(offset), ioaddr);
+ return 0;
+}
+
+static int moxart_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) | BIT(offset), ioaddr);
+ return 0;
+}
+
+static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_DATA_OUT;
+ u32 reg = readl(ioaddr);
+
+ if (value)
+ reg = reg | BIT(offset);
+ else
+ reg = reg & ~BIT(offset);
+
+
+ writel(reg, ioaddr);
+}
+
+static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ u32 ret = readl(moxart_gpio_base + GPIO_PIN_DIRECTION);
+
+ if (ret & BIT(offset))
+ return !!(readl(moxart_gpio_base + GPIO_DATA_OUT) &
+ BIT(offset));
+ else
+ return !!(readl(moxart_gpio_base + GPIO_DATA_IN) &
+ BIT(offset));
+}
+
+static struct gpio_chip moxart_gpio_chip = {
+ .label = "moxart-gpio",
+ .request = moxart_gpio_request,
+ .free = moxart_gpio_free,
+ .direction_input = moxart_gpio_direction_input,
+ .direction_output = moxart_gpio_direction_output,
+ .set = moxart_gpio_set,
+ .get = moxart_gpio_get,
+ .base = 0,
+ .ngpio = 32,
+ .can_sleep = 0,
+};
+
+static int moxart_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ moxart_gpio_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(moxart_gpio_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(moxart_gpio_base);
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ moxart_pincontrol_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(moxart_pincontrol_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_pmu failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(moxart_pincontrol_base);
+ }
+
+ moxart_gpio_chip.dev = dev;
+
+ ret = gpiochip_add(&moxart_gpio_chip);
+ if (ret) {
+ dev_err(dev, "%s: gpiochip_add failed\n",
+ dev->of_node->full_name);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id moxart_gpio_match[] = {
+ { .compatible = "moxa,moxart-gpio" },
+ { }
+};
+
+static struct platform_driver moxart_gpio_driver = {
+ .driver = {
+ .name = "moxart-gpio",
+ .owner = THIS_MODULE,
+ .of_match_table = moxart_gpio_match,
+ },
+ .probe = moxart_gpio_probe,
+};
+module_platform_driver(moxart_gpio_driver);
+
+MODULE_DESCRIPTION("MOXART GPIO chip driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v5] gpio: Add MOXA ART GPIO driver
@ 2013-10-11 14:53 ` Jonas Jensen
0 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-10-11 14:53 UTC (permalink / raw)
To: linux-arm-kernel
Add GPIO driver for MOXA ART SoCs.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks for the replies,
I agree it is a bit strange GPIO control is divided in two
separate registers. Unfortunately I can't offer an explanation
because the documentation is not publicly available.
The register responsible for doing enable/disable is located
at <0x98100100 0x4>, the clock register is very close at
<0x98100000 0x34>.
I don't think gpio_poweroff driver is right for this hardware
because the pin is not connected to anything that can do reset.
The old 2.6.9 kernel driver uses timer poll with eventual call
to userspace.
To test that it works, I added gpio_poweroff anyway, modified
with gpio_export() the pin can then be seen switching between
0 and 1 (on "cat /sys/class/gpio/gpio25/value").
The DT file I use on UC-7112-LX:
clk_pll: pll at 98100000 {
compatible = "moxa,moxart-pll-clock";
#clock-cells = <0>;
reg = <0x98100000 0x34>;
clocks = <&ref12>;
};
clk_apb: clk_apb at 98100000 {
compatible = "moxa,moxart-apb-clock";
#clock-cells = <0>;
reg = <0x98100000 0x34>;
clocks = <&clk_pll>;
};
gpio: gpio at 98700000 {
gpio-controller;
#gpio-cells = <2>;
compatible = "moxa,moxart-gpio";
reg = <0x98700000 0xC>,
<0x98100100 0x4>;
};
leds {
compatible = "gpio-leds";
user-led {
label = "ready-led";
gpios = <&gpio 27 0x1>;
default-state = "on";
linux,default-trigger = "default-on";
};
};
gpio_poweroff {
compatible = "gpio-poweroff";
pinctrl-names = "default";
input = <1>;
gpios = <&gpio 25 0x0>;
};
Changes since v4:
1. elaborate DT binding #gpio-cells description
2. remove ready-led / reset-switch from driver and binding
3. use BIT() macro
4. remove ternary operator in moxart_gpio_set()
5. use !!(condition) construct in moxart_gpio_get()
6. replace postcore_initcall() with module_platform_driver()
7. return gpiochip_add() return value on failure
Applies to next-20130927
.../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 22 +++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-moxart.c | 163 +++++++++++++++++++++
4 files changed, 193 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
create mode 100644 drivers/gpio/gpio-moxart.c
diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
new file mode 100644
index 0000000..5039e17
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
@@ -0,0 +1,22 @@
+MOXA ART GPIO Controller
+
+Required properties:
+
+- #gpio-cells : Should be 2, The first cell is the pin number and
+ the second cell is used to specify polarity:
+ 0 = active high
+ 1 = active low
+- compatible : Must be "moxa,moxart-gpio"
+- reg : Should contain registers location and length
+ index 0 : input, output, and direction control
+ index 1 : enable/disable individual pins, pin 0-31
+
+Example:
+
+ gpio: gpio at 98700000 {
+ gpio-controller;
+ #gpio-cells = <2>;
+ compatible = "moxa,moxart-gpio";
+ reg = <0x98700000 0xC>,
+ <0x98100100 0x4>;
+ };
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c8b02a5..c5a2767 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -156,6 +156,13 @@ config GPIO_F7188X
To compile this driver as a module, choose M here: the module will
be called f7188x-gpio.
+config GPIO_MOXART
+ bool "MOXART GPIO support"
+ depends on ARCH_MOXART
+ help
+ Select this option to enable GPIO driver for
+ MOXA ART SoC devices.
+
config GPIO_MPC5200
def_bool y
depends on PPC_MPC52xx
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5c353df..a26029d 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08) += gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
new file mode 100644
index 0000000..5796846
--- /dev/null
+++ b/drivers/gpio/gpio-moxart.c
@@ -0,0 +1,163 @@
+/*
+ * MOXA ART SoCs GPIO driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/bitops.h>
+
+#define GPIO_DATA_OUT 0x00
+#define GPIO_DATA_IN 0x04
+#define GPIO_PIN_DIRECTION 0x08
+
+static void __iomem *moxart_pincontrol_base;
+static void __iomem *moxart_gpio_base;
+
+void moxart_gpio_enable(u32 gpio)
+{
+ writel(readl(moxart_pincontrol_base) | gpio, moxart_pincontrol_base);
+}
+
+void moxart_gpio_disable(u32 gpio)
+{
+ writel(readl(moxart_pincontrol_base) & ~gpio, moxart_pincontrol_base);
+}
+
+static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ moxart_gpio_enable(BIT(offset));
+ return pinctrl_request_gpio(offset);
+}
+
+static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(offset);
+ moxart_gpio_disable(BIT(offset));
+}
+
+static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) & ~BIT(offset), ioaddr);
+ return 0;
+}
+
+static int moxart_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) | BIT(offset), ioaddr);
+ return 0;
+}
+
+static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_DATA_OUT;
+ u32 reg = readl(ioaddr);
+
+ if (value)
+ reg = reg | BIT(offset);
+ else
+ reg = reg & ~BIT(offset);
+
+
+ writel(reg, ioaddr);
+}
+
+static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ u32 ret = readl(moxart_gpio_base + GPIO_PIN_DIRECTION);
+
+ if (ret & BIT(offset))
+ return !!(readl(moxart_gpio_base + GPIO_DATA_OUT) &
+ BIT(offset));
+ else
+ return !!(readl(moxart_gpio_base + GPIO_DATA_IN) &
+ BIT(offset));
+}
+
+static struct gpio_chip moxart_gpio_chip = {
+ .label = "moxart-gpio",
+ .request = moxart_gpio_request,
+ .free = moxart_gpio_free,
+ .direction_input = moxart_gpio_direction_input,
+ .direction_output = moxart_gpio_direction_output,
+ .set = moxart_gpio_set,
+ .get = moxart_gpio_get,
+ .base = 0,
+ .ngpio = 32,
+ .can_sleep = 0,
+};
+
+static int moxart_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ moxart_gpio_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(moxart_gpio_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(moxart_gpio_base);
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ moxart_pincontrol_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(moxart_pincontrol_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_pmu failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(moxart_pincontrol_base);
+ }
+
+ moxart_gpio_chip.dev = dev;
+
+ ret = gpiochip_add(&moxart_gpio_chip);
+ if (ret) {
+ dev_err(dev, "%s: gpiochip_add failed\n",
+ dev->of_node->full_name);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id moxart_gpio_match[] = {
+ { .compatible = "moxa,moxart-gpio" },
+ { }
+};
+
+static struct platform_driver moxart_gpio_driver = {
+ .driver = {
+ .name = "moxart-gpio",
+ .owner = THIS_MODULE,
+ .of_match_table = moxart_gpio_match,
+ },
+ .probe = moxart_gpio_probe,
+};
+module_platform_driver(moxart_gpio_driver);
+
+MODULE_DESCRIPTION("MOXART GPIO chip driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v5] gpio: Add MOXA ART GPIO driver
2013-10-11 14:53 ` Jonas Jensen
(?)
@ 2013-10-11 15:44 ` Linus Walleij
-1 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-10-11 15:44 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, Mark Rutland, devicetree
On Fri, Oct 11, 2013 at 4:53 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> I agree it is a bit strange GPIO control is divided in two
> separate registers. Unfortunately I can't offer an explanation
> because the documentation is not publicly available.
>
> The register responsible for doing enable/disable is located
> at <0x98100100 0x4>, the clock register is very close at
> <0x98100000 0x34>.
If we don't know we have to guess.
This layout makes me think that the IO-window at 0x98100000 is
a power-clock-and-reset controller. It contains some register
to latch the pins enable/disable them, or if this is even a clock
gate? Are you sure about this? Is it now a gated clock, simply,
so that this bit should be handled in the clock driver, i.e.
this bit gets set by clk_enable() from the GPIO driver?
I am very suspicious about this especially since the GPIO
driver is lacking clk_get() and friends.
If it's not a clock gate, and you are convinced that you must still
reach out into this range I think you want something like this:
syscon: syscon@98100000 {
compatible = "syscon";
reg = <0x98100000 0x1000>;
};
gpio: gpio@98700000 {
gpio-controller;
#gpio-cells = <2>;
syscon = <&syscon>;
compatible = "moxa,moxart-gpio";
reg = <0x98700000 0xC>,
<0x98100100 0x4>;
};
Then the driver can use something like:
struct device_node *np = pdev->dev.of_node;
struct device_node *syscon_np;
struct regmap *regmap;
int err;
syscon_np = of_parse_phandle(np, "syscon", 0);
if (!syscon_np) {
pr_crit("no syscon node\n");
return -ENODEV;
}
regmap = syscon_node_to_regmap(syscon_np);
if (!regmap) {
pr_crit("could not locate syscon regmap\n");
return -ENODEV;
}
Then update the registers using regmap_update_bits() and
friends.
> I don't think gpio_poweroff driver is right for this hardware
> because the pin is not connected to anything that can do reset.
> The old 2.6.9 kernel driver uses timer poll with eventual call
> to userspace.
>
> To test that it works, I added gpio_poweroff anyway, modified
> with gpio_export() the pin can then be seen switching between
> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
Hmmmm not quite following this...
> +Required properties:
> +
> +- #gpio-cells : Should be 2, The first cell is the pin number and
> + the second cell is used to specify polarity:
> + 0 = active high
> + 1 = active low
Could reference <dt-bindings/gpio/gpio.h> I guess?
Oh well, no big deal.
The driver as such is looking nice but now I strongly suspect
it should clk_get/clk_prepare/clk_enable ... etc.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v5] gpio: Add MOXA ART GPIO driver
@ 2013-10-11 15:44 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-10-11 15:44 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 11, 2013 at 4:53 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> I agree it is a bit strange GPIO control is divided in two
> separate registers. Unfortunately I can't offer an explanation
> because the documentation is not publicly available.
>
> The register responsible for doing enable/disable is located
> at <0x98100100 0x4>, the clock register is very close at
> <0x98100000 0x34>.
If we don't know we have to guess.
This layout makes me think that the IO-window at 0x98100000 is
a power-clock-and-reset controller. It contains some register
to latch the pins enable/disable them, or if this is even a clock
gate? Are you sure about this? Is it now a gated clock, simply,
so that this bit should be handled in the clock driver, i.e.
this bit gets set by clk_enable() from the GPIO driver?
I am very suspicious about this especially since the GPIO
driver is lacking clk_get() and friends.
If it's not a clock gate, and you are convinced that you must still
reach out into this range I think you want something like this:
syscon: syscon at 98100000 {
compatible = "syscon";
reg = <0x98100000 0x1000>;
};
gpio: gpio at 98700000 {
gpio-controller;
#gpio-cells = <2>;
syscon = <&syscon>;
compatible = "moxa,moxart-gpio";
reg = <0x98700000 0xC>,
<0x98100100 0x4>;
};
Then the driver can use something like:
struct device_node *np = pdev->dev.of_node;
struct device_node *syscon_np;
struct regmap *regmap;
int err;
syscon_np = of_parse_phandle(np, "syscon", 0);
if (!syscon_np) {
pr_crit("no syscon node\n");
return -ENODEV;
}
regmap = syscon_node_to_regmap(syscon_np);
if (!regmap) {
pr_crit("could not locate syscon regmap\n");
return -ENODEV;
}
Then update the registers using regmap_update_bits() and
friends.
> I don't think gpio_poweroff driver is right for this hardware
> because the pin is not connected to anything that can do reset.
> The old 2.6.9 kernel driver uses timer poll with eventual call
> to userspace.
>
> To test that it works, I added gpio_poweroff anyway, modified
> with gpio_export() the pin can then be seen switching between
> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
Hmmmm not quite following this...
> +Required properties:
> +
> +- #gpio-cells : Should be 2, The first cell is the pin number and
> + the second cell is used to specify polarity:
> + 0 = active high
> + 1 = active low
Could reference <dt-bindings/gpio/gpio.h> I guess?
Oh well, no big deal.
The driver as such is looking nice but now I strongly suspect
it should clk_get/clk_prepare/clk_enable ... etc.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v5] gpio: Add MOXA ART GPIO driver
@ 2013-10-11 15:44 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-10-11 15:44 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, Mark Rutland, devicetree
On Fri, Oct 11, 2013 at 4:53 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> I agree it is a bit strange GPIO control is divided in two
> separate registers. Unfortunately I can't offer an explanation
> because the documentation is not publicly available.
>
> The register responsible for doing enable/disable is located
> at <0x98100100 0x4>, the clock register is very close at
> <0x98100000 0x34>.
If we don't know we have to guess.
This layout makes me think that the IO-window at 0x98100000 is
a power-clock-and-reset controller. It contains some register
to latch the pins enable/disable them, or if this is even a clock
gate? Are you sure about this? Is it now a gated clock, simply,
so that this bit should be handled in the clock driver, i.e.
this bit gets set by clk_enable() from the GPIO driver?
I am very suspicious about this especially since the GPIO
driver is lacking clk_get() and friends.
If it's not a clock gate, and you are convinced that you must still
reach out into this range I think you want something like this:
syscon: syscon@98100000 {
compatible = "syscon";
reg = <0x98100000 0x1000>;
};
gpio: gpio@98700000 {
gpio-controller;
#gpio-cells = <2>;
syscon = <&syscon>;
compatible = "moxa,moxart-gpio";
reg = <0x98700000 0xC>,
<0x98100100 0x4>;
};
Then the driver can use something like:
struct device_node *np = pdev->dev.of_node;
struct device_node *syscon_np;
struct regmap *regmap;
int err;
syscon_np = of_parse_phandle(np, "syscon", 0);
if (!syscon_np) {
pr_crit("no syscon node\n");
return -ENODEV;
}
regmap = syscon_node_to_regmap(syscon_np);
if (!regmap) {
pr_crit("could not locate syscon regmap\n");
return -ENODEV;
}
Then update the registers using regmap_update_bits() and
friends.
> I don't think gpio_poweroff driver is right for this hardware
> because the pin is not connected to anything that can do reset.
> The old 2.6.9 kernel driver uses timer poll with eventual call
> to userspace.
>
> To test that it works, I added gpio_poweroff anyway, modified
> with gpio_export() the pin can then be seen switching between
> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
Hmmmm not quite following this...
> +Required properties:
> +
> +- #gpio-cells : Should be 2, The first cell is the pin number and
> + the second cell is used to specify polarity:
> + 0 = active high
> + 1 = active low
Could reference <dt-bindings/gpio/gpio.h> I guess?
Oh well, no big deal.
The driver as such is looking nice but now I strongly suspect
it should clk_get/clk_prepare/clk_enable ... etc.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v5] gpio: Add MOXA ART GPIO driver
2013-10-11 15:44 ` Linus Walleij
(?)
@ 2013-10-14 11:15 ` Jonas Jensen
-1 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-10-14 11:15 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, Mark Rutland, devicetree
Thank you for the replies.
On 11 October 2013 17:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>> The register responsible for doing enable/disable is located
>> at <0x98100100 0x4>, the clock register is very close at
>> <0x98100000 0x34>.
>
> If we don't know we have to guess.
>
> This layout makes me think that the IO-window at 0x98100000 is
> a power-clock-and-reset controller. It contains some register
> to latch the pins enable/disable them, or if this is even a clock
> gate? Are you sure about this? Is it now a gated clock, simply,
> so that this bit should be handled in the clock driver, i.e.
> this bit gets set by clk_enable() from the GPIO driver?
The IO-window at 0x98100000 contains registers that are read to
determine PLL and APB clock frequencies. Sorry I don't know more than
that.
This is part of a pending patch adding the clock driver:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/203494.html
Arnd made a similar comment suggesting syscon back when MMC mapped the
same register:
https://groups.google.com/d/msg/linux.kernel/eeS7vhMWMAc/zNYhzyKilh8J
I think I prefer to have this in the clock driver opposed to using syscon.
The one downside I can see, individual control of the pins would be
lost? Does it make sense to enable all pins once? this is acceptable
for at least UC-7112-LX, it doesn't need to disable/enable pins beyond
the initial clk_enable().
I say the above because I currently have nothing that requires
individual pin control. However, I removed one line from MMC that
turned out to be unnecessary. That line directly access the "PMU"
register disabling pins 10-17 with the following comment:
"
/* change I/O multiplexing to SD, so the GPIO 17-10 will be fail */
moxart_gpio_mp_clear(0xff << 10);
"
http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/mmc/host/moxasd.c#619
>> I don't think gpio_poweroff driver is right for this hardware
>> because the pin is not connected to anything that can do reset.
>> The old 2.6.9 kernel driver uses timer poll with eventual call
>> to userspace.
>>
>> To test that it works, I added gpio_poweroff anyway, modified
>> with gpio_export() the pin can then be seen switching between
>> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
>
> Hmmmm not quite following this...
I'll try to elaborate. What happens in gpio_poweroff driver does not
look like something that can reset the hardware. Reset on UC-7112-LX
is implemented using the same register as the watchdog, in platform
code hooked up to arm_pm_restart.
The old sources "solved" this by polling the reset pin with eventual
call_usermodehelper (/sbin/reboot):
http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/char/moxa_watchdog.c#174
What was previously in a kernel driver, would now be solved in userspace?
gpio_export() allowed me to verify the pin number, pressing reset
toggles the value.
Adding the gpio-leds driver, that pin was automatically exported to
sysfs, that got me thinking:
How do I export the reset button to sysfs? Should gpio_export() be
added to platform code?
from drivers/power/reset/gpio-poweroff.c:
static void gpio_poweroff_do_poweroff(void)
{
BUG_ON(!gpio_is_valid(gpio_num));
/* drive it active, also inactive->active edge */
gpio_direction_output(gpio_num, !gpio_active_low);
mdelay(100);
/* drive inactive, also active->inactive edge */
gpio_set_value(gpio_num, gpio_active_low);
mdelay(100);
/* drive it active, also inactive->active edge */
gpio_set_value(gpio_num, !gpio_active_low);
/* give it some time */
mdelay(3000);
WARN_ON(1);
}
Regards,
Jonas
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v5] gpio: Add MOXA ART GPIO driver
@ 2013-10-14 11:15 ` Jonas Jensen
0 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-10-14 11:15 UTC (permalink / raw)
To: linux-arm-kernel
Thank you for the replies.
On 11 October 2013 17:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>> The register responsible for doing enable/disable is located
>> at <0x98100100 0x4>, the clock register is very close at
>> <0x98100000 0x34>.
>
> If we don't know we have to guess.
>
> This layout makes me think that the IO-window at 0x98100000 is
> a power-clock-and-reset controller. It contains some register
> to latch the pins enable/disable them, or if this is even a clock
> gate? Are you sure about this? Is it now a gated clock, simply,
> so that this bit should be handled in the clock driver, i.e.
> this bit gets set by clk_enable() from the GPIO driver?
The IO-window at 0x98100000 contains registers that are read to
determine PLL and APB clock frequencies. Sorry I don't know more than
that.
This is part of a pending patch adding the clock driver:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/203494.html
Arnd made a similar comment suggesting syscon back when MMC mapped the
same register:
https://groups.google.com/d/msg/linux.kernel/eeS7vhMWMAc/zNYhzyKilh8J
I think I prefer to have this in the clock driver opposed to using syscon.
The one downside I can see, individual control of the pins would be
lost? Does it make sense to enable all pins once? this is acceptable
for at least UC-7112-LX, it doesn't need to disable/enable pins beyond
the initial clk_enable().
I say the above because I currently have nothing that requires
individual pin control. However, I removed one line from MMC that
turned out to be unnecessary. That line directly access the "PMU"
register disabling pins 10-17 with the following comment:
"
/* change I/O multiplexing to SD, so the GPIO 17-10 will be fail */
moxart_gpio_mp_clear(0xff << 10);
"
http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/mmc/host/moxasd.c#619
>> I don't think gpio_poweroff driver is right for this hardware
>> because the pin is not connected to anything that can do reset.
>> The old 2.6.9 kernel driver uses timer poll with eventual call
>> to userspace.
>>
>> To test that it works, I added gpio_poweroff anyway, modified
>> with gpio_export() the pin can then be seen switching between
>> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
>
> Hmmmm not quite following this...
I'll try to elaborate. What happens in gpio_poweroff driver does not
look like something that can reset the hardware. Reset on UC-7112-LX
is implemented using the same register as the watchdog, in platform
code hooked up to arm_pm_restart.
The old sources "solved" this by polling the reset pin with eventual
call_usermodehelper (/sbin/reboot):
http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/char/moxa_watchdog.c#174
What was previously in a kernel driver, would now be solved in userspace?
gpio_export() allowed me to verify the pin number, pressing reset
toggles the value.
Adding the gpio-leds driver, that pin was automatically exported to
sysfs, that got me thinking:
How do I export the reset button to sysfs? Should gpio_export() be
added to platform code?
from drivers/power/reset/gpio-poweroff.c:
static void gpio_poweroff_do_poweroff(void)
{
BUG_ON(!gpio_is_valid(gpio_num));
/* drive it active, also inactive->active edge */
gpio_direction_output(gpio_num, !gpio_active_low);
mdelay(100);
/* drive inactive, also active->inactive edge */
gpio_set_value(gpio_num, gpio_active_low);
mdelay(100);
/* drive it active, also inactive->active edge */
gpio_set_value(gpio_num, !gpio_active_low);
/* give it some time */
mdelay(3000);
WARN_ON(1);
}
Regards,
Jonas
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v5] gpio: Add MOXA ART GPIO driver
@ 2013-10-14 11:15 ` Jonas Jensen
0 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-10-14 11:15 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, Mark Rutland, devicetree
Thank you for the replies.
On 11 October 2013 17:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>> The register responsible for doing enable/disable is located
>> at <0x98100100 0x4>, the clock register is very close at
>> <0x98100000 0x34>.
>
> If we don't know we have to guess.
>
> This layout makes me think that the IO-window at 0x98100000 is
> a power-clock-and-reset controller. It contains some register
> to latch the pins enable/disable them, or if this is even a clock
> gate? Are you sure about this? Is it now a gated clock, simply,
> so that this bit should be handled in the clock driver, i.e.
> this bit gets set by clk_enable() from the GPIO driver?
The IO-window at 0x98100000 contains registers that are read to
determine PLL and APB clock frequencies. Sorry I don't know more than
that.
This is part of a pending patch adding the clock driver:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/203494.html
Arnd made a similar comment suggesting syscon back when MMC mapped the
same register:
https://groups.google.com/d/msg/linux.kernel/eeS7vhMWMAc/zNYhzyKilh8J
I think I prefer to have this in the clock driver opposed to using syscon.
The one downside I can see, individual control of the pins would be
lost? Does it make sense to enable all pins once? this is acceptable
for at least UC-7112-LX, it doesn't need to disable/enable pins beyond
the initial clk_enable().
I say the above because I currently have nothing that requires
individual pin control. However, I removed one line from MMC that
turned out to be unnecessary. That line directly access the "PMU"
register disabling pins 10-17 with the following comment:
"
/* change I/O multiplexing to SD, so the GPIO 17-10 will be fail */
moxart_gpio_mp_clear(0xff << 10);
"
http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/mmc/host/moxasd.c#619
>> I don't think gpio_poweroff driver is right for this hardware
>> because the pin is not connected to anything that can do reset.
>> The old 2.6.9 kernel driver uses timer poll with eventual call
>> to userspace.
>>
>> To test that it works, I added gpio_poweroff anyway, modified
>> with gpio_export() the pin can then be seen switching between
>> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
>
> Hmmmm not quite following this...
I'll try to elaborate. What happens in gpio_poweroff driver does not
look like something that can reset the hardware. Reset on UC-7112-LX
is implemented using the same register as the watchdog, in platform
code hooked up to arm_pm_restart.
The old sources "solved" this by polling the reset pin with eventual
call_usermodehelper (/sbin/reboot):
http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/char/moxa_watchdog.c#174
What was previously in a kernel driver, would now be solved in userspace?
gpio_export() allowed me to verify the pin number, pressing reset
toggles the value.
Adding the gpio-leds driver, that pin was automatically exported to
sysfs, that got me thinking:
How do I export the reset button to sysfs? Should gpio_export() be
added to platform code?
from drivers/power/reset/gpio-poweroff.c:
static void gpio_poweroff_do_poweroff(void)
{
BUG_ON(!gpio_is_valid(gpio_num));
/* drive it active, also inactive->active edge */
gpio_direction_output(gpio_num, !gpio_active_low);
mdelay(100);
/* drive inactive, also active->inactive edge */
gpio_set_value(gpio_num, gpio_active_low);
mdelay(100);
/* drive it active, also inactive->active edge */
gpio_set_value(gpio_num, !gpio_active_low);
/* give it some time */
mdelay(3000);
WARN_ON(1);
}
Regards,
Jonas
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v5] gpio: Add MOXA ART GPIO driver
2013-10-14 11:15 ` Jonas Jensen
(?)
@ 2013-10-17 9:24 ` Linus Walleij
-1 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-10-17 9:24 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, Mark Rutland, devicetree
On Mon, Oct 14, 2013 at 1:15 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> On 11 October 2013 17:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> The register responsible for doing enable/disable is located
>>> at <0x98100100 0x4>, the clock register is very close at
>>> <0x98100000 0x34>.
(...)
> Arnd made a similar comment suggesting syscon back when MMC mapped the
> same register:
> https://groups.google.com/d/msg/linux.kernel/eeS7vhMWMAc/zNYhzyKilh8J
>
> I think I prefer to have this in the clock driver opposed to using syscon.
OK I can live with this. I've seen both approaches in practice.
> The one downside I can see, individual control of the pins would be
> lost? Does it make sense to enable all pins once? this is acceptable
> for at least UC-7112-LX, it doesn't need to disable/enable pins beyond
> the initial clk_enable().
>
> I say the above because I currently have nothing that requires
> individual pin control. However, I removed one line from MMC that
> turned out to be unnecessary. That line directly access the "PMU"
> register disabling pins 10-17 with the following comment:
>
> "
> /* change I/O multiplexing to SD, so the GPIO 17-10 will be fail */
> moxart_gpio_mp_clear(0xff << 10);
> "
> http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/mmc/host/moxasd.c#619
Sorry I don't fully follow this. But don't poke into pin control registers
from other drivers, for separation of concerns. Request and handle
pins through the pin control API.
What you mean is likely that power-on states and boot loaders
have set up the pins for your immediate use cases and everything
seems to work, that is of course nice.
However as development and requirements progress people start
to come in with complex usecases where devices need pins set
to states that differ from the power-on defaults, for example for
power management. And then you run into this.
When/if you need pin control you can wither implement that as
a separate driver which is used by this GPIO driver as a "back-end"
or you can move this driver over to drivers/pinctrl and extend it
with a pin control API there, making it a combined pin control
and GPIO driver.
>>> I don't think gpio_poweroff driver is right for this hardware
>>> because the pin is not connected to anything that can do reset.
>>> The old 2.6.9 kernel driver uses timer poll with eventual call
>>> to userspace.
>>>
>>> To test that it works, I added gpio_poweroff anyway, modified
>>> with gpio_export() the pin can then be seen switching between
>>> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
>>
>> Hmmmm not quite following this...
>
> I'll try to elaborate. What happens in gpio_poweroff driver does not
> look like something that can reset the hardware. Reset on UC-7112-LX
> is implemented using the same register as the watchdog, in platform
> code hooked up to arm_pm_restart.
>
> The old sources "solved" this by polling the reset pin with eventual
> call_usermodehelper (/sbin/reboot):
> http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/char/moxa_watchdog.c#174
>
> What was previously in a kernel driver, would now be solved in userspace?
>
> gpio_export() allowed me to verify the pin number, pressing reset
> toggles the value.
>
> Adding the gpio-leds driver, that pin was automatically exported to
> sysfs, that got me thinking:
>
> How do I export the reset button to sysfs? Should gpio_export() be
> added to platform code?
Aha argh what a mess. No the GPIO poweroff driver is not
intended to fix this. I *think* you should do this:
- Register the gpio pin as a polled input device using
drivers/input/keyboard/gpio_keys_polled.c
- Have it emit KEY_POWER (from include/uapi/linux/input.h)
- Have your userspace input event (whatever issues select()
on the /dev/input/* stuff) react to this input by calling
/sbin/reboot or issueing the same thing that does, i.e.
call reboot() with magic 0x01234567 I think.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v5] gpio: Add MOXA ART GPIO driver
@ 2013-10-17 9:24 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-10-17 9:24 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 14, 2013 at 1:15 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> On 11 October 2013 17:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> The register responsible for doing enable/disable is located
>>> at <0x98100100 0x4>, the clock register is very close at
>>> <0x98100000 0x34>.
(...)
> Arnd made a similar comment suggesting syscon back when MMC mapped the
> same register:
> https://groups.google.com/d/msg/linux.kernel/eeS7vhMWMAc/zNYhzyKilh8J
>
> I think I prefer to have this in the clock driver opposed to using syscon.
OK I can live with this. I've seen both approaches in practice.
> The one downside I can see, individual control of the pins would be
> lost? Does it make sense to enable all pins once? this is acceptable
> for at least UC-7112-LX, it doesn't need to disable/enable pins beyond
> the initial clk_enable().
>
> I say the above because I currently have nothing that requires
> individual pin control. However, I removed one line from MMC that
> turned out to be unnecessary. That line directly access the "PMU"
> register disabling pins 10-17 with the following comment:
>
> "
> /* change I/O multiplexing to SD, so the GPIO 17-10 will be fail */
> moxart_gpio_mp_clear(0xff << 10);
> "
> http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/mmc/host/moxasd.c#619
Sorry I don't fully follow this. But don't poke into pin control registers
from other drivers, for separation of concerns. Request and handle
pins through the pin control API.
What you mean is likely that power-on states and boot loaders
have set up the pins for your immediate use cases and everything
seems to work, that is of course nice.
However as development and requirements progress people start
to come in with complex usecases where devices need pins set
to states that differ from the power-on defaults, for example for
power management. And then you run into this.
When/if you need pin control you can wither implement that as
a separate driver which is used by this GPIO driver as a "back-end"
or you can move this driver over to drivers/pinctrl and extend it
with a pin control API there, making it a combined pin control
and GPIO driver.
>>> I don't think gpio_poweroff driver is right for this hardware
>>> because the pin is not connected to anything that can do reset.
>>> The old 2.6.9 kernel driver uses timer poll with eventual call
>>> to userspace.
>>>
>>> To test that it works, I added gpio_poweroff anyway, modified
>>> with gpio_export() the pin can then be seen switching between
>>> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
>>
>> Hmmmm not quite following this...
>
> I'll try to elaborate. What happens in gpio_poweroff driver does not
> look like something that can reset the hardware. Reset on UC-7112-LX
> is implemented using the same register as the watchdog, in platform
> code hooked up to arm_pm_restart.
>
> The old sources "solved" this by polling the reset pin with eventual
> call_usermodehelper (/sbin/reboot):
> http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/char/moxa_watchdog.c#174
>
> What was previously in a kernel driver, would now be solved in userspace?
>
> gpio_export() allowed me to verify the pin number, pressing reset
> toggles the value.
>
> Adding the gpio-leds driver, that pin was automatically exported to
> sysfs, that got me thinking:
>
> How do I export the reset button to sysfs? Should gpio_export() be
> added to platform code?
Aha argh what a mess. No the GPIO poweroff driver is not
intended to fix this. I *think* you should do this:
- Register the gpio pin as a polled input device using
drivers/input/keyboard/gpio_keys_polled.c
- Have it emit KEY_POWER (from include/uapi/linux/input.h)
- Have your userspace input event (whatever issues select()
on the /dev/input/* stuff) react to this input by calling
/sbin/reboot or issueing the same thing that does, i.e.
call reboot() with magic 0x01234567 I think.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v5] gpio: Add MOXA ART GPIO driver
@ 2013-10-17 9:24 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-10-17 9:24 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, Mark Rutland, devicetree
On Mon, Oct 14, 2013 at 1:15 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> On 11 October 2013 17:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> The register responsible for doing enable/disable is located
>>> at <0x98100100 0x4>, the clock register is very close at
>>> <0x98100000 0x34>.
(...)
> Arnd made a similar comment suggesting syscon back when MMC mapped the
> same register:
> https://groups.google.com/d/msg/linux.kernel/eeS7vhMWMAc/zNYhzyKilh8J
>
> I think I prefer to have this in the clock driver opposed to using syscon.
OK I can live with this. I've seen both approaches in practice.
> The one downside I can see, individual control of the pins would be
> lost? Does it make sense to enable all pins once? this is acceptable
> for at least UC-7112-LX, it doesn't need to disable/enable pins beyond
> the initial clk_enable().
>
> I say the above because I currently have nothing that requires
> individual pin control. However, I removed one line from MMC that
> turned out to be unnecessary. That line directly access the "PMU"
> register disabling pins 10-17 with the following comment:
>
> "
> /* change I/O multiplexing to SD, so the GPIO 17-10 will be fail */
> moxart_gpio_mp_clear(0xff << 10);
> "
> http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/mmc/host/moxasd.c#619
Sorry I don't fully follow this. But don't poke into pin control registers
from other drivers, for separation of concerns. Request and handle
pins through the pin control API.
What you mean is likely that power-on states and boot loaders
have set up the pins for your immediate use cases and everything
seems to work, that is of course nice.
However as development and requirements progress people start
to come in with complex usecases where devices need pins set
to states that differ from the power-on defaults, for example for
power management. And then you run into this.
When/if you need pin control you can wither implement that as
a separate driver which is used by this GPIO driver as a "back-end"
or you can move this driver over to drivers/pinctrl and extend it
with a pin control API there, making it a combined pin control
and GPIO driver.
>>> I don't think gpio_poweroff driver is right for this hardware
>>> because the pin is not connected to anything that can do reset.
>>> The old 2.6.9 kernel driver uses timer poll with eventual call
>>> to userspace.
>>>
>>> To test that it works, I added gpio_poweroff anyway, modified
>>> with gpio_export() the pin can then be seen switching between
>>> 0 and 1 (on "cat /sys/class/gpio/gpio25/value").
>>
>> Hmmmm not quite following this...
>
> I'll try to elaborate. What happens in gpio_poweroff driver does not
> look like something that can reset the hardware. Reset on UC-7112-LX
> is implemented using the same register as the watchdog, in platform
> code hooked up to arm_pm_restart.
>
> The old sources "solved" this by polling the reset pin with eventual
> call_usermodehelper (/sbin/reboot):
> http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/char/moxa_watchdog.c#174
>
> What was previously in a kernel driver, would now be solved in userspace?
>
> gpio_export() allowed me to verify the pin number, pressing reset
> toggles the value.
>
> Adding the gpio-leds driver, that pin was automatically exported to
> sysfs, that got me thinking:
>
> How do I export the reset button to sysfs? Should gpio_export() be
> added to platform code?
Aha argh what a mess. No the GPIO poweroff driver is not
intended to fix this. I *think* you should do this:
- Register the gpio pin as a polled input device using
drivers/input/keyboard/gpio_keys_polled.c
- Have it emit KEY_POWER (from include/uapi/linux/input.h)
- Have your userspace input event (whatever issues select()
on the /dev/input/* stuff) react to this input by calling
/sbin/reboot or issueing the same thing that does, i.e.
call reboot() with magic 0x01234567 I think.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v6] gpio: Add MOXA ART GPIO driver
2013-10-11 14:53 ` Jonas Jensen
@ 2013-11-28 15:19 ` Jonas Jensen
-1 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-11-28 15:19 UTC (permalink / raw)
To: linux-gpio
Cc: grant.likely, linus.walleij, linux-arm-kernel, linux-kernel, arm,
arnd, mark.rutland, devicetree, Jonas Jensen
Add GPIO driver for MOXA ART SoCs.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks for reviewing!
v5 writes to "pincontrol" are not needed, the pins work regardless.
I took notes probing the pins with a multimeter, and I don't know the proper
place for it, so I'm attaching the text here:
GPIOs on the UC-7112-LX base board can be found at "JP2", this is a 2x10 hole through
area that comes without header pins. The holes can be used to measure and test GPIOs.
A header could theoretically be soldered in.
"JP2" is described with the following PCB print:
|PIO1|PIO3|PIO5|PIO7|PIO9|NC|NC|NC|GND|+5V|
|PIO0| |PIO4|PIO6|PIO8|NC|NC|NC|GND|+5V|
PIO2 (no text / PIO2) to PIO9 were verified working.
PIO0, PIO1 could not be verified.
The driver adds support for 32 pins via the register at 0x98700000, the following
GPIO numbers (corresponding pins on the right) were tested and verified:
05: RTC SCLK
06: RTC DATA
07: RTC RESET
12: (no text / PIO2)
13: PIO3
14: PIO4
15: PIO5
16: PIO6
17: PIO7
18: PIO8
19: PIO9
24: HCM1206EN buzzer
25: reset button
27: ready LED
GPIOs that can be set from sysfs are:
1. GPIO 12-19 (measures 0V / 3.3V respectively)
2. GPIO 24, 27
GPIOs that can be "seen":
1. GPIO 25
Thanks for telling me about drivers/input/keyboard/gpio_keys_polled.c,
it works, events are triggered on reset "key" and print to console,
the relevant DT parts are:
leds {
compatible = "gpio-leds";
user-led {
label = "ready-led";
gpios = <&gpio 27 0x1>;
default-state = "on";
linux,default-trigger = "default-on";
};
};
gpio_keys_polled {
compatible = "gpio-keys-polled";
#address-cells = <1>;
#size-cells = <0>;
poll-interval = <500>;
button@25 {
label = "GPIO Reset";
linux,code = <116>;
gpios = <&gpio 25 1>;
};
};
Changes since v5:
1. remove pincontrol register
2. update DT binding and example
Applies to next-20131128
.../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 19 +++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-moxart.c | 142 +++++++++++++++++++++
4 files changed, 169 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
create mode 100644 drivers/gpio/gpio-moxart.c
diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
new file mode 100644
index 0000000..f8e8f18
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
@@ -0,0 +1,19 @@
+MOXA ART GPIO Controller
+
+Required properties:
+
+- #gpio-cells : Should be 2, The first cell is the pin number,
+ the second cell is used to specify polarity:
+ 0 = active high
+ 1 = active low
+- compatible : Must be "moxa,moxart-gpio"
+- reg : Should contain registers location and length
+
+Example:
+
+ gpio: gpio@98700000 {
+ gpio-controller;
+ #gpio-cells = <2>;
+ compatible = "moxa,moxart-gpio";
+ reg = <0x98700000 0xC>;
+ };
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0f04444..e48c523 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -156,6 +156,13 @@ config GPIO_F7188X
To compile this driver as a module, choose M here: the module will
be called f7188x-gpio.
+config GPIO_MOXART
+ bool "MOXART GPIO support"
+ depends on ARCH_MOXART
+ help
+ Select this option to enable GPIO driver for
+ MOXA ART SoC devices.
+
config GPIO_MPC5200
def_bool y
depends on PPC_MPC52xx
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 7971e36..ee95154 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08) += gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
new file mode 100644
index 0000000..96ec03c
--- /dev/null
+++ b/drivers/gpio/gpio-moxart.c
@@ -0,0 +1,142 @@
+/*
+ * MOXA ART SoCs GPIO driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/bitops.h>
+
+#define GPIO_DATA_OUT 0x00
+#define GPIO_DATA_IN 0x04
+#define GPIO_PIN_DIRECTION 0x08
+
+static void __iomem *moxart_gpio_base;
+
+static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ return pinctrl_request_gpio(offset);
+}
+
+static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(offset);
+}
+
+static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) & ~BIT(offset), ioaddr);
+ return 0;
+}
+
+static int moxart_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) | BIT(offset), ioaddr);
+ return 0;
+}
+
+static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_DATA_OUT;
+ u32 reg = readl(ioaddr);
+
+ if (value)
+ reg = reg | BIT(offset);
+ else
+ reg = reg & ~BIT(offset);
+
+
+ writel(reg, ioaddr);
+}
+
+static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ u32 ret = readl(moxart_gpio_base + GPIO_PIN_DIRECTION);
+
+ if (ret & BIT(offset))
+ return !!(readl(moxart_gpio_base + GPIO_DATA_OUT) &
+ BIT(offset));
+ else
+ return !!(readl(moxart_gpio_base + GPIO_DATA_IN) &
+ BIT(offset));
+}
+
+static struct gpio_chip moxart_gpio_chip = {
+ .label = "moxart-gpio",
+ .request = moxart_gpio_request,
+ .free = moxart_gpio_free,
+ .direction_input = moxart_gpio_direction_input,
+ .direction_output = moxart_gpio_direction_output,
+ .set = moxart_gpio_set,
+ .get = moxart_gpio_get,
+ .base = 0,
+ .ngpio = 32,
+ .can_sleep = 0,
+};
+
+static int moxart_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ moxart_gpio_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(moxart_gpio_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(moxart_gpio_base);
+ }
+
+ moxart_gpio_chip.dev = dev;
+
+ ret = gpiochip_add(&moxart_gpio_chip);
+ if (ret) {
+ dev_err(dev, "%s: gpiochip_add failed\n",
+ dev->of_node->full_name);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id moxart_gpio_match[] = {
+ { .compatible = "moxa,moxart-gpio" },
+ { }
+};
+
+static struct platform_driver moxart_gpio_driver = {
+ .driver = {
+ .name = "moxart-gpio",
+ .owner = THIS_MODULE,
+ .of_match_table = moxart_gpio_match,
+ },
+ .probe = moxart_gpio_probe,
+};
+module_platform_driver(moxart_gpio_driver);
+
+MODULE_DESCRIPTION("MOXART GPIO chip driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v6] gpio: Add MOXA ART GPIO driver
@ 2013-11-28 15:19 ` Jonas Jensen
0 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-11-28 15:19 UTC (permalink / raw)
To: linux-arm-kernel
Add GPIO driver for MOXA ART SoCs.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks for reviewing!
v5 writes to "pincontrol" are not needed, the pins work regardless.
I took notes probing the pins with a multimeter, and I don't know the proper
place for it, so I'm attaching the text here:
GPIOs on the UC-7112-LX base board can be found at "JP2", this is a 2x10 hole through
area that comes without header pins. The holes can be used to measure and test GPIOs.
A header could theoretically be soldered in.
"JP2" is described with the following PCB print:
|PIO1|PIO3|PIO5|PIO7|PIO9|NC|NC|NC|GND|+5V|
|PIO0| |PIO4|PIO6|PIO8|NC|NC|NC|GND|+5V|
PIO2 (no text / PIO2) to PIO9 were verified working.
PIO0, PIO1 could not be verified.
The driver adds support for 32 pins via the register at 0x98700000, the following
GPIO numbers (corresponding pins on the right) were tested and verified:
05: RTC SCLK
06: RTC DATA
07: RTC RESET
12: (no text / PIO2)
13: PIO3
14: PIO4
15: PIO5
16: PIO6
17: PIO7
18: PIO8
19: PIO9
24: HCM1206EN buzzer
25: reset button
27: ready LED
GPIOs that can be set from sysfs are:
1. GPIO 12-19 (measures 0V / 3.3V respectively)
2. GPIO 24, 27
GPIOs that can be "seen":
1. GPIO 25
Thanks for telling me about drivers/input/keyboard/gpio_keys_polled.c,
it works, events are triggered on reset "key" and print to console,
the relevant DT parts are:
leds {
compatible = "gpio-leds";
user-led {
label = "ready-led";
gpios = <&gpio 27 0x1>;
default-state = "on";
linux,default-trigger = "default-on";
};
};
gpio_keys_polled {
compatible = "gpio-keys-polled";
#address-cells = <1>;
#size-cells = <0>;
poll-interval = <500>;
button at 25 {
label = "GPIO Reset";
linux,code = <116>;
gpios = <&gpio 25 1>;
};
};
Changes since v5:
1. remove pincontrol register
2. update DT binding and example
Applies to next-20131128
.../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 19 +++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-moxart.c | 142 +++++++++++++++++++++
4 files changed, 169 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
create mode 100644 drivers/gpio/gpio-moxart.c
diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
new file mode 100644
index 0000000..f8e8f18
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
@@ -0,0 +1,19 @@
+MOXA ART GPIO Controller
+
+Required properties:
+
+- #gpio-cells : Should be 2, The first cell is the pin number,
+ the second cell is used to specify polarity:
+ 0 = active high
+ 1 = active low
+- compatible : Must be "moxa,moxart-gpio"
+- reg : Should contain registers location and length
+
+Example:
+
+ gpio: gpio at 98700000 {
+ gpio-controller;
+ #gpio-cells = <2>;
+ compatible = "moxa,moxart-gpio";
+ reg = <0x98700000 0xC>;
+ };
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0f04444..e48c523 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -156,6 +156,13 @@ config GPIO_F7188X
To compile this driver as a module, choose M here: the module will
be called f7188x-gpio.
+config GPIO_MOXART
+ bool "MOXART GPIO support"
+ depends on ARCH_MOXART
+ help
+ Select this option to enable GPIO driver for
+ MOXA ART SoC devices.
+
config GPIO_MPC5200
def_bool y
depends on PPC_MPC52xx
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 7971e36..ee95154 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08) += gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
new file mode 100644
index 0000000..96ec03c
--- /dev/null
+++ b/drivers/gpio/gpio-moxart.c
@@ -0,0 +1,142 @@
+/*
+ * MOXA ART SoCs GPIO driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/bitops.h>
+
+#define GPIO_DATA_OUT 0x00
+#define GPIO_DATA_IN 0x04
+#define GPIO_PIN_DIRECTION 0x08
+
+static void __iomem *moxart_gpio_base;
+
+static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ return pinctrl_request_gpio(offset);
+}
+
+static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(offset);
+}
+
+static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) & ~BIT(offset), ioaddr);
+ return 0;
+}
+
+static int moxart_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) | BIT(offset), ioaddr);
+ return 0;
+}
+
+static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ void __iomem *ioaddr = moxart_gpio_base + GPIO_DATA_OUT;
+ u32 reg = readl(ioaddr);
+
+ if (value)
+ reg = reg | BIT(offset);
+ else
+ reg = reg & ~BIT(offset);
+
+
+ writel(reg, ioaddr);
+}
+
+static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ u32 ret = readl(moxart_gpio_base + GPIO_PIN_DIRECTION);
+
+ if (ret & BIT(offset))
+ return !!(readl(moxart_gpio_base + GPIO_DATA_OUT) &
+ BIT(offset));
+ else
+ return !!(readl(moxart_gpio_base + GPIO_DATA_IN) &
+ BIT(offset));
+}
+
+static struct gpio_chip moxart_gpio_chip = {
+ .label = "moxart-gpio",
+ .request = moxart_gpio_request,
+ .free = moxart_gpio_free,
+ .direction_input = moxart_gpio_direction_input,
+ .direction_output = moxart_gpio_direction_output,
+ .set = moxart_gpio_set,
+ .get = moxart_gpio_get,
+ .base = 0,
+ .ngpio = 32,
+ .can_sleep = 0,
+};
+
+static int moxart_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ moxart_gpio_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(moxart_gpio_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(moxart_gpio_base);
+ }
+
+ moxart_gpio_chip.dev = dev;
+
+ ret = gpiochip_add(&moxart_gpio_chip);
+ if (ret) {
+ dev_err(dev, "%s: gpiochip_add failed\n",
+ dev->of_node->full_name);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id moxart_gpio_match[] = {
+ { .compatible = "moxa,moxart-gpio" },
+ { }
+};
+
+static struct platform_driver moxart_gpio_driver = {
+ .driver = {
+ .name = "moxart-gpio",
+ .owner = THIS_MODULE,
+ .of_match_table = moxart_gpio_match,
+ },
+ .probe = moxart_gpio_probe,
+};
+module_platform_driver(moxart_gpio_driver);
+
+MODULE_DESCRIPTION("MOXART GPIO chip driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v6] gpio: Add MOXA ART GPIO driver
2013-11-28 15:19 ` Jonas Jensen
@ 2013-11-28 16:37 ` Arnd Bergmann
-1 siblings, 0 replies; 47+ messages in thread
From: Arnd Bergmann @ 2013-11-28 16:37 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, grant.likely, linus.walleij, linux-arm-kernel,
linux-kernel, arm, mark.rutland, devicetree
On Thursday 28 November 2013, Jonas Jensen wrote:
> +static void __iomem *moxart_gpio_base;
Just one comment: the usual way to do such a driver is to have
a derived data structure like
struct moxart_gpio_chip {
struct gpio_chip chip;
void __iomem *moxart_gpio_base;
};
and dynamically allocate that from probe(), using container_of() to
get from the gpio_chip pointer to your own structure.
You obviously rely on the fact that there is only one gpio_chip
in a moxart soc, which is a safe assumption, the only real disadvantage
of your approach is that it makes your driver less suitable as an
example for others to look at when they are not dealing with
just a single instance, so decide for yourself whether you want
to change it or not.
Arnd
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v6] gpio: Add MOXA ART GPIO driver
@ 2013-11-28 16:37 ` Arnd Bergmann
0 siblings, 0 replies; 47+ messages in thread
From: Arnd Bergmann @ 2013-11-28 16:37 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 28 November 2013, Jonas Jensen wrote:
> +static void __iomem *moxart_gpio_base;
Just one comment: the usual way to do such a driver is to have
a derived data structure like
struct moxart_gpio_chip {
struct gpio_chip chip;
void __iomem *moxart_gpio_base;
};
and dynamically allocate that from probe(), using container_of() to
get from the gpio_chip pointer to your own structure.
You obviously rely on the fact that there is only one gpio_chip
in a moxart soc, which is a safe assumption, the only real disadvantage
of your approach is that it makes your driver less suitable as an
example for others to look at when they are not dealing with
just a single instance, so decide for yourself whether you want
to change it or not.
Arnd
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v6] gpio: Add MOXA ART GPIO driver
2013-11-28 16:37 ` Arnd Bergmann
(?)
@ 2013-11-29 20:21 ` Linus Walleij
-1 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-11-29 20:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jonas Jensen, linux-gpio, Grant Likely, linux-arm-kernel,
linux-kernel, arm, Mark Rutland, devicetree
On Thu, Nov 28, 2013 at 5:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 28 November 2013, Jonas Jensen wrote:
>> +static void __iomem *moxart_gpio_base;
>
> Just one comment: the usual way to do such a driver is to have
> a derived data structure like
>
> struct moxart_gpio_chip {
> struct gpio_chip chip;
> void __iomem *moxart_gpio_base;
> };
>
> and dynamically allocate that from probe(), using container_of() to
> get from the gpio_chip pointer to your own structure.
I see we make this comment a lot.
On my TODO there is an item to create
Documentation/driver-model/design-patterns.txt
And document things like this. And other fun stuff like
container_of().
What do you think about this idea?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v6] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 20:21 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-11-29 20:21 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 28, 2013 at 5:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 28 November 2013, Jonas Jensen wrote:
>> +static void __iomem *moxart_gpio_base;
>
> Just one comment: the usual way to do such a driver is to have
> a derived data structure like
>
> struct moxart_gpio_chip {
> struct gpio_chip chip;
> void __iomem *moxart_gpio_base;
> };
>
> and dynamically allocate that from probe(), using container_of() to
> get from the gpio_chip pointer to your own structure.
I see we make this comment a lot.
On my TODO there is an item to create
Documentation/driver-model/design-patterns.txt
And document things like this. And other fun stuff like
container_of().
What do you think about this idea?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v6] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 20:21 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-11-29 20:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jonas Jensen, linux-gpio, Grant Likely, linux-arm-kernel,
linux-kernel, arm, Mark Rutland, devicetree
On Thu, Nov 28, 2013 at 5:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 28 November 2013, Jonas Jensen wrote:
>> +static void __iomem *moxart_gpio_base;
>
> Just one comment: the usual way to do such a driver is to have
> a derived data structure like
>
> struct moxart_gpio_chip {
> struct gpio_chip chip;
> void __iomem *moxart_gpio_base;
> };
>
> and dynamically allocate that from probe(), using container_of() to
> get from the gpio_chip pointer to your own structure.
I see we make this comment a lot.
On my TODO there is an item to create
Documentation/driver-model/design-patterns.txt
And document things like this. And other fun stuff like
container_of().
What do you think about this idea?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v6] gpio: Add MOXA ART GPIO driver
2013-11-29 20:21 ` Linus Walleij
(?)
@ 2013-11-29 21:45 ` Arnd Bergmann
-1 siblings, 0 replies; 47+ messages in thread
From: Arnd Bergmann @ 2013-11-29 21:45 UTC (permalink / raw)
To: Linus Walleij
Cc: Jonas Jensen, linux-gpio, Grant Likely, linux-arm-kernel,
linux-kernel, arm, Mark Rutland, devicetree
On Friday 29 November 2013, Linus Walleij wrote:
> On Thu, Nov 28, 2013 at 5:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 28 November 2013, Jonas Jensen wrote:
> >> +static void __iomem *moxart_gpio_base;
> >
> > Just one comment: the usual way to do such a driver is to have
> > a derived data structure like
> >
> > struct moxart_gpio_chip {
> > struct gpio_chip chip;
> > void __iomem *moxart_gpio_base;
> > };
> >
> > and dynamically allocate that from probe(), using container_of() to
> > get from the gpio_chip pointer to your own structure.
>
> I see we make this comment a lot.
>
> On my TODO there is an item to create
> Documentation/driver-model/design-patterns.txt
>
> And document things like this. And other fun stuff like
> container_of().
>
> What do you think about this idea?
Great idea!
Arnd
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v6] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 21:45 ` Arnd Bergmann
0 siblings, 0 replies; 47+ messages in thread
From: Arnd Bergmann @ 2013-11-29 21:45 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 29 November 2013, Linus Walleij wrote:
> On Thu, Nov 28, 2013 at 5:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 28 November 2013, Jonas Jensen wrote:
> >> +static void __iomem *moxart_gpio_base;
> >
> > Just one comment: the usual way to do such a driver is to have
> > a derived data structure like
> >
> > struct moxart_gpio_chip {
> > struct gpio_chip chip;
> > void __iomem *moxart_gpio_base;
> > };
> >
> > and dynamically allocate that from probe(), using container_of() to
> > get from the gpio_chip pointer to your own structure.
>
> I see we make this comment a lot.
>
> On my TODO there is an item to create
> Documentation/driver-model/design-patterns.txt
>
> And document things like this. And other fun stuff like
> container_of().
>
> What do you think about this idea?
Great idea!
Arnd
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v6] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 21:45 ` Arnd Bergmann
0 siblings, 0 replies; 47+ messages in thread
From: Arnd Bergmann @ 2013-11-29 21:45 UTC (permalink / raw)
To: Linus Walleij
Cc: Jonas Jensen, linux-gpio, Grant Likely, linux-arm-kernel,
linux-kernel, arm, Mark Rutland, devicetree
On Friday 29 November 2013, Linus Walleij wrote:
> On Thu, Nov 28, 2013 at 5:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 28 November 2013, Jonas Jensen wrote:
> >> +static void __iomem *moxart_gpio_base;
> >
> > Just one comment: the usual way to do such a driver is to have
> > a derived data structure like
> >
> > struct moxart_gpio_chip {
> > struct gpio_chip chip;
> > void __iomem *moxart_gpio_base;
> > };
> >
> > and dynamically allocate that from probe(), using container_of() to
> > get from the gpio_chip pointer to your own structure.
>
> I see we make this comment a lot.
>
> On my TODO there is an item to create
> Documentation/driver-model/design-patterns.txt
>
> And document things like this. And other fun stuff like
> container_of().
>
> What do you think about this idea?
Great idea!
Arnd
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v7] gpio: Add MOXA ART GPIO driver
2013-11-28 15:19 ` Jonas Jensen
@ 2013-11-29 11:11 ` Jonas Jensen
-1 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-11-29 11:11 UTC (permalink / raw)
To: linux-gpio
Cc: grant.likely, linus.walleij, linux-arm-kernel, linux-kernel, arm,
arnd, mark.rutland, devicetree, Jonas Jensen
Add GPIO driver for MOXA ART SoCs.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks Arnd, done in other drivers no reason to not do it here.
Changes since v6:
1. remove global __iomem pointer
2. add derived data structure "struct moxart_gpio_chip"
3. use container_of()
4. add container_of() helper function
Applies to next-20131129
.../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 19 +++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-moxart.c | 162 +++++++++++++++++++++
4 files changed, 189 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
create mode 100644 drivers/gpio/gpio-moxart.c
diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
new file mode 100644
index 0000000..f8e8f18
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
@@ -0,0 +1,19 @@
+MOXA ART GPIO Controller
+
+Required properties:
+
+- #gpio-cells : Should be 2, The first cell is the pin number,
+ the second cell is used to specify polarity:
+ 0 = active high
+ 1 = active low
+- compatible : Must be "moxa,moxart-gpio"
+- reg : Should contain registers location and length
+
+Example:
+
+ gpio: gpio@98700000 {
+ gpio-controller;
+ #gpio-cells = <2>;
+ compatible = "moxa,moxart-gpio";
+ reg = <0x98700000 0xC>;
+ };
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0f04444..e48c523 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -156,6 +156,13 @@ config GPIO_F7188X
To compile this driver as a module, choose M here: the module will
be called f7188x-gpio.
+config GPIO_MOXART
+ bool "MOXART GPIO support"
+ depends on ARCH_MOXART
+ help
+ Select this option to enable GPIO driver for
+ MOXA ART SoC devices.
+
config GPIO_MPC5200
def_bool y
depends on PPC_MPC52xx
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 7971e36..ee95154 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08) += gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
new file mode 100644
index 0000000..d662cde
--- /dev/null
+++ b/drivers/gpio/gpio-moxart.c
@@ -0,0 +1,162 @@
+/*
+ * MOXA ART SoCs GPIO driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/bitops.h>
+
+#define GPIO_DATA_OUT 0x00
+#define GPIO_DATA_IN 0x04
+#define GPIO_PIN_DIRECTION 0x08
+
+struct moxart_gpio_chip {
+ struct gpio_chip gpio;
+ void __iomem *moxart_gpio_base;
+};
+
+static inline struct moxart_gpio_chip *to_moxart_gpio(struct gpio_chip *chip)
+{
+ return container_of(chip, struct moxart_gpio_chip, gpio);
+}
+
+static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ return pinctrl_request_gpio(offset);
+}
+
+static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(offset);
+}
+
+static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
+ void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) & ~BIT(offset), ioaddr);
+ return 0;
+}
+
+static int moxart_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
+ void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) | BIT(offset), ioaddr);
+ return 0;
+}
+
+static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
+ void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_DATA_OUT;
+ u32 reg = readl(ioaddr);
+
+ if (value)
+ reg = reg | BIT(offset);
+ else
+ reg = reg & ~BIT(offset);
+
+
+ writel(reg, ioaddr);
+}
+
+static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
+ u32 ret = readl(gc->moxart_gpio_base + GPIO_PIN_DIRECTION);
+
+ if (ret & BIT(offset))
+ return !!(readl(gc->moxart_gpio_base + GPIO_DATA_OUT) &
+ BIT(offset));
+ else
+ return !!(readl(gc->moxart_gpio_base + GPIO_DATA_IN) &
+ BIT(offset));
+}
+
+static struct gpio_chip moxart_template_chip = {
+ .label = "moxart-gpio",
+ .request = moxart_gpio_request,
+ .free = moxart_gpio_free,
+ .direction_input = moxart_gpio_direction_input,
+ .direction_output = moxart_gpio_direction_output,
+ .set = moxart_gpio_set,
+ .get = moxart_gpio_get,
+ .base = 0,
+ .ngpio = 32,
+ .can_sleep = 0,
+};
+
+static int moxart_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct moxart_gpio_chip *mgc;
+ int ret;
+
+ mgc = devm_kzalloc(dev, sizeof(*mgc), GFP_KERNEL);
+ if (!mgc) {
+ dev_err(dev, "can't allocate GPIO chip container\n");
+ return -ENOMEM;
+ }
+ mgc->gpio = moxart_template_chip;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mgc->moxart_gpio_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(mgc->moxart_gpio_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(mgc->moxart_gpio_base);
+ }
+
+ mgc->gpio.dev = dev;
+
+ ret = gpiochip_add(&mgc->gpio);
+ if (ret) {
+ dev_err(dev, "%s: gpiochip_add failed\n",
+ dev->of_node->full_name);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id moxart_gpio_match[] = {
+ { .compatible = "moxa,moxart-gpio" },
+ { }
+};
+
+static struct platform_driver moxart_gpio_driver = {
+ .driver = {
+ .name = "moxart-gpio",
+ .owner = THIS_MODULE,
+ .of_match_table = moxart_gpio_match,
+ },
+ .probe = moxart_gpio_probe,
+};
+module_platform_driver(moxart_gpio_driver);
+
+MODULE_DESCRIPTION("MOXART GPIO chip driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 11:11 ` Jonas Jensen
0 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-11-29 11:11 UTC (permalink / raw)
To: linux-arm-kernel
Add GPIO driver for MOXA ART SoCs.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks Arnd, done in other drivers no reason to not do it here.
Changes since v6:
1. remove global __iomem pointer
2. add derived data structure "struct moxart_gpio_chip"
3. use container_of()
4. add container_of() helper function
Applies to next-20131129
.../devicetree/bindings/gpio/moxa,moxart-gpio.txt | 19 +++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-moxart.c | 162 +++++++++++++++++++++
4 files changed, 189 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
create mode 100644 drivers/gpio/gpio-moxart.c
diff --git a/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
new file mode 100644
index 0000000..f8e8f18
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/moxa,moxart-gpio.txt
@@ -0,0 +1,19 @@
+MOXA ART GPIO Controller
+
+Required properties:
+
+- #gpio-cells : Should be 2, The first cell is the pin number,
+ the second cell is used to specify polarity:
+ 0 = active high
+ 1 = active low
+- compatible : Must be "moxa,moxart-gpio"
+- reg : Should contain registers location and length
+
+Example:
+
+ gpio: gpio at 98700000 {
+ gpio-controller;
+ #gpio-cells = <2>;
+ compatible = "moxa,moxart-gpio";
+ reg = <0x98700000 0xC>;
+ };
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0f04444..e48c523 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -156,6 +156,13 @@ config GPIO_F7188X
To compile this driver as a module, choose M here: the module will
be called f7188x-gpio.
+config GPIO_MOXART
+ bool "MOXART GPIO support"
+ depends on ARCH_MOXART
+ help
+ Select this option to enable GPIO driver for
+ MOXA ART SoC devices.
+
config GPIO_MPC5200
def_bool y
depends on PPC_MPC52xx
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 7971e36..ee95154 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08) += gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
new file mode 100644
index 0000000..d662cde
--- /dev/null
+++ b/drivers/gpio/gpio-moxart.c
@@ -0,0 +1,162 @@
+/*
+ * MOXA ART SoCs GPIO driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/bitops.h>
+
+#define GPIO_DATA_OUT 0x00
+#define GPIO_DATA_IN 0x04
+#define GPIO_PIN_DIRECTION 0x08
+
+struct moxart_gpio_chip {
+ struct gpio_chip gpio;
+ void __iomem *moxart_gpio_base;
+};
+
+static inline struct moxart_gpio_chip *to_moxart_gpio(struct gpio_chip *chip)
+{
+ return container_of(chip, struct moxart_gpio_chip, gpio);
+}
+
+static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ return pinctrl_request_gpio(offset);
+}
+
+static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(offset);
+}
+
+static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
+ void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) & ~BIT(offset), ioaddr);
+ return 0;
+}
+
+static int moxart_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
+ void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_PIN_DIRECTION;
+
+ writel(readl(ioaddr) | BIT(offset), ioaddr);
+ return 0;
+}
+
+static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
+ void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_DATA_OUT;
+ u32 reg = readl(ioaddr);
+
+ if (value)
+ reg = reg | BIT(offset);
+ else
+ reg = reg & ~BIT(offset);
+
+
+ writel(reg, ioaddr);
+}
+
+static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
+ u32 ret = readl(gc->moxart_gpio_base + GPIO_PIN_DIRECTION);
+
+ if (ret & BIT(offset))
+ return !!(readl(gc->moxart_gpio_base + GPIO_DATA_OUT) &
+ BIT(offset));
+ else
+ return !!(readl(gc->moxart_gpio_base + GPIO_DATA_IN) &
+ BIT(offset));
+}
+
+static struct gpio_chip moxart_template_chip = {
+ .label = "moxart-gpio",
+ .request = moxart_gpio_request,
+ .free = moxart_gpio_free,
+ .direction_input = moxart_gpio_direction_input,
+ .direction_output = moxart_gpio_direction_output,
+ .set = moxart_gpio_set,
+ .get = moxart_gpio_get,
+ .base = 0,
+ .ngpio = 32,
+ .can_sleep = 0,
+};
+
+static int moxart_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct moxart_gpio_chip *mgc;
+ int ret;
+
+ mgc = devm_kzalloc(dev, sizeof(*mgc), GFP_KERNEL);
+ if (!mgc) {
+ dev_err(dev, "can't allocate GPIO chip container\n");
+ return -ENOMEM;
+ }
+ mgc->gpio = moxart_template_chip;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mgc->moxart_gpio_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(mgc->moxart_gpio_base)) {
+ dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
+ dev->of_node->full_name);
+ return PTR_ERR(mgc->moxart_gpio_base);
+ }
+
+ mgc->gpio.dev = dev;
+
+ ret = gpiochip_add(&mgc->gpio);
+ if (ret) {
+ dev_err(dev, "%s: gpiochip_add failed\n",
+ dev->of_node->full_name);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id moxart_gpio_match[] = {
+ { .compatible = "moxa,moxart-gpio" },
+ { }
+};
+
+static struct platform_driver moxart_gpio_driver = {
+ .driver = {
+ .name = "moxart-gpio",
+ .owner = THIS_MODULE,
+ .of_match_table = moxart_gpio_match,
+ },
+ .probe = moxart_gpio_probe,
+};
+module_platform_driver(moxart_gpio_driver);
+
+MODULE_DESCRIPTION("MOXART GPIO chip driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread[parent not found: <1385723494-8033-1-git-send-email-jonas.jensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v7] gpio: Add MOXA ART GPIO driver
2013-11-29 11:11 ` Jonas Jensen
(?)
@ 2013-11-29 19:06 ` Arnd Bergmann
-1 siblings, 0 replies; 47+ messages in thread
From: Arnd Bergmann @ 2013-11-29 19:06 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, arm-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA
On Friday 29 November 2013, Jonas Jensen wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
One more comment, no need to resend for another review if this is the only
thing you want to change:
> +struct moxart_gpio_chip {
> + struct gpio_chip gpio;
> + void __iomem *moxart_gpio_base;
> +};
If you rename 'moxart_gpio_base' to 'base'
> +static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
> + u32 ret = readl(gc->moxart_gpio_base + GPIO_PIN_DIRECTION);
> +
> + if (ret & BIT(offset))
> + return !!(readl(gc->moxart_gpio_base + GPIO_DATA_OUT) &
> + BIT(offset));
> + else
> + return !!(readl(gc->moxart_gpio_base + GPIO_DATA_IN) &
> + BIT(offset));
> +}
These will fit in one line with no loss of readability.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v7] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 19:06 ` Arnd Bergmann
0 siblings, 0 replies; 47+ messages in thread
From: Arnd Bergmann @ 2013-11-29 19:06 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 29 November 2013, Jonas Jensen wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
One more comment, no need to resend for another review if this is the only
thing you want to change:
> +struct moxart_gpio_chip {
> + struct gpio_chip gpio;
> + void __iomem *moxart_gpio_base;
> +};
If you rename 'moxart_gpio_base' to 'base'
> +static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
> + u32 ret = readl(gc->moxart_gpio_base + GPIO_PIN_DIRECTION);
> +
> + if (ret & BIT(offset))
> + return !!(readl(gc->moxart_gpio_base + GPIO_DATA_OUT) &
> + BIT(offset));
> + else
> + return !!(readl(gc->moxart_gpio_base + GPIO_DATA_IN) &
> + BIT(offset));
> +}
These will fit in one line with no loss of readability.
Arnd
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v7] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 19:06 ` Arnd Bergmann
0 siblings, 0 replies; 47+ messages in thread
From: Arnd Bergmann @ 2013-11-29 19:06 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, grant.likely, linus.walleij, linux-arm-kernel,
linux-kernel, arm, mark.rutland, devicetree
On Friday 29 November 2013, Jonas Jensen wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
One more comment, no need to resend for another review if this is the only
thing you want to change:
> +struct moxart_gpio_chip {
> + struct gpio_chip gpio;
> + void __iomem *moxart_gpio_base;
> +};
If you rename 'moxart_gpio_base' to 'base'
> +static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
> + u32 ret = readl(gc->moxart_gpio_base + GPIO_PIN_DIRECTION);
> +
> + if (ret & BIT(offset))
> + return !!(readl(gc->moxart_gpio_base + GPIO_DATA_OUT) &
> + BIT(offset));
> + else
> + return !!(readl(gc->moxart_gpio_base + GPIO_DATA_IN) &
> + BIT(offset));
> +}
These will fit in one line with no loss of readability.
Arnd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7] gpio: Add MOXA ART GPIO driver
2013-11-29 11:11 ` Jonas Jensen
(?)
@ 2013-11-29 20:29 ` Linus Walleij
-1 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-11-29 20:29 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, Mark Rutland, devicetree
On Fri, Nov 29, 2013 at 12:11 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
OK this v6 version is looking *really good* so I have
applied it, couldn't hesitate.
> +static struct gpio_chip moxart_template_chip = {
> + .label = "moxart-gpio",
> + .request = moxart_gpio_request,
> + .free = moxart_gpio_free,
> + .direction_input = moxart_gpio_direction_input,
> + .direction_output = moxart_gpio_direction_output,
> + .set = moxart_gpio_set,
> + .get = moxart_gpio_get,
> + .base = 0,
> + .ngpio = 32,
> + .can_sleep = 0,
> +};
No need to initialized unused fields to zero, that is by
default. But no big deal.
> +static const struct of_device_id moxart_gpio_match[] = {
> + { .compatible = "moxa,moxart-gpio" },
> + { }
> +};
This vendor prefix does not exist in
Documentation/devicetree/bindings/vendor-prefixes.txt
Please send a follow-up patch adding it and I can merge
that through the GPIO tree as well.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v7] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 20:29 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-11-29 20:29 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 29, 2013 at 12:11 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
OK this v6 version is looking *really good* so I have
applied it, couldn't hesitate.
> +static struct gpio_chip moxart_template_chip = {
> + .label = "moxart-gpio",
> + .request = moxart_gpio_request,
> + .free = moxart_gpio_free,
> + .direction_input = moxart_gpio_direction_input,
> + .direction_output = moxart_gpio_direction_output,
> + .set = moxart_gpio_set,
> + .get = moxart_gpio_get,
> + .base = 0,
> + .ngpio = 32,
> + .can_sleep = 0,
> +};
No need to initialized unused fields to zero, that is by
default. But no big deal.
> +static const struct of_device_id moxart_gpio_match[] = {
> + { .compatible = "moxa,moxart-gpio" },
> + { }
> +};
This vendor prefix does not exist in
Documentation/devicetree/bindings/vendor-prefixes.txt
Please send a follow-up patch adding it and I can merge
that through the GPIO tree as well.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v7] gpio: Add MOXA ART GPIO driver
@ 2013-11-29 20:29 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-11-29 20:29 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, Grant Likely, linux-arm-kernel, linux-kernel, arm,
Arnd Bergmann, Mark Rutland, devicetree
On Fri, Nov 29, 2013 at 12:11 PM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Add GPIO driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
OK this v6 version is looking *really good* so I have
applied it, couldn't hesitate.
> +static struct gpio_chip moxart_template_chip = {
> + .label = "moxart-gpio",
> + .request = moxart_gpio_request,
> + .free = moxart_gpio_free,
> + .direction_input = moxart_gpio_direction_input,
> + .direction_output = moxart_gpio_direction_output,
> + .set = moxart_gpio_set,
> + .get = moxart_gpio_get,
> + .base = 0,
> + .ngpio = 32,
> + .can_sleep = 0,
> +};
No need to initialized unused fields to zero, that is by
default. But no big deal.
> +static const struct of_device_id moxart_gpio_match[] = {
> + { .compatible = "moxa,moxart-gpio" },
> + { }
> +};
This vendor prefix does not exist in
Documentation/devicetree/bindings/vendor-prefixes.txt
Please send a follow-up patch adding it and I can merge
that through the GPIO tree as well.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH] gpio: MOXA ART: rename moxart_gpio_base to base
2013-11-29 11:11 ` Jonas Jensen
@ 2013-12-02 10:27 ` Jonas Jensen
-1 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-12-02 10:27 UTC (permalink / raw)
To: linux-gpio
Cc: linus.walleij, linux-arm-kernel, linux-kernel, arnd,
mark.rutland, devicetree, Jonas Jensen
Renaming "moxart_gpio_base" to "base" allows better fit,
remove line breaks in moxart_gpio_get().
While doing trivial cleanup, also remove fields initialized
with zero in moxart_template_chip.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks for applying!
This is a follow-up with trivial changes per your comments.
Regarding Documentation/devicetree/bindings/vendor-prefixes.txt,
there's already a patch submitted (Mark Rutland requested this):
https://lkml.org/lkml/2013/10/8/451
Applies to next-20131202
drivers/gpio/gpio-moxart.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
index d662cde..4ecd195 100644
--- a/drivers/gpio/gpio-moxart.c
+++ b/drivers/gpio/gpio-moxart.c
@@ -30,7 +30,7 @@
struct moxart_gpio_chip {
struct gpio_chip gpio;
- void __iomem *moxart_gpio_base;
+ void __iomem *base;
};
static inline struct moxart_gpio_chip *to_moxart_gpio(struct gpio_chip *chip)
@@ -51,7 +51,7 @@ static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
{
struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
- void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_PIN_DIRECTION;
+ void __iomem *ioaddr = gc->base + GPIO_PIN_DIRECTION;
writel(readl(ioaddr) & ~BIT(offset), ioaddr);
return 0;
@@ -61,7 +61,7 @@ static int moxart_gpio_direction_output(struct gpio_chip *chip,
unsigned offset, int value)
{
struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
- void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_PIN_DIRECTION;
+ void __iomem *ioaddr = gc->base + GPIO_PIN_DIRECTION;
writel(readl(ioaddr) | BIT(offset), ioaddr);
return 0;
@@ -70,7 +70,7 @@ static int moxart_gpio_direction_output(struct gpio_chip *chip,
static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
{
struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
- void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_DATA_OUT;
+ void __iomem *ioaddr = gc->base + GPIO_DATA_OUT;
u32 reg = readl(ioaddr);
if (value)
@@ -85,14 +85,12 @@ static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
{
struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
- u32 ret = readl(gc->moxart_gpio_base + GPIO_PIN_DIRECTION);
+ u32 ret = readl(gc->base + GPIO_PIN_DIRECTION);
if (ret & BIT(offset))
- return !!(readl(gc->moxart_gpio_base + GPIO_DATA_OUT) &
- BIT(offset));
+ return !!(readl(gc->base + GPIO_DATA_OUT) & BIT(offset));
else
- return !!(readl(gc->moxart_gpio_base + GPIO_DATA_IN) &
- BIT(offset));
+ return !!(readl(gc->base + GPIO_DATA_IN) & BIT(offset));
}
static struct gpio_chip moxart_template_chip = {
@@ -103,9 +101,7 @@ static struct gpio_chip moxart_template_chip = {
.direction_output = moxart_gpio_direction_output,
.set = moxart_gpio_set,
.get = moxart_gpio_get,
- .base = 0,
.ngpio = 32,
- .can_sleep = 0,
};
static int moxart_gpio_probe(struct platform_device *pdev)
@@ -123,11 +119,11 @@ static int moxart_gpio_probe(struct platform_device *pdev)
mgc->gpio = moxart_template_chip;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- mgc->moxart_gpio_base = devm_ioremap_resource(dev, res);
- if (IS_ERR(mgc->moxart_gpio_base)) {
+ mgc->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(mgc->base)) {
dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
dev->of_node->full_name);
- return PTR_ERR(mgc->moxart_gpio_base);
+ return PTR_ERR(mgc->base);
}
mgc->gpio.dev = dev;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH] gpio: MOXA ART: rename moxart_gpio_base to base
@ 2013-12-02 10:27 ` Jonas Jensen
0 siblings, 0 replies; 47+ messages in thread
From: Jonas Jensen @ 2013-12-02 10:27 UTC (permalink / raw)
To: linux-arm-kernel
Renaming "moxart_gpio_base" to "base" allows better fit,
remove line breaks in moxart_gpio_get().
While doing trivial cleanup, also remove fields initialized
with zero in moxart_template_chip.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks for applying!
This is a follow-up with trivial changes per your comments.
Regarding Documentation/devicetree/bindings/vendor-prefixes.txt,
there's already a patch submitted (Mark Rutland requested this):
https://lkml.org/lkml/2013/10/8/451
Applies to next-20131202
drivers/gpio/gpio-moxart.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
index d662cde..4ecd195 100644
--- a/drivers/gpio/gpio-moxart.c
+++ b/drivers/gpio/gpio-moxart.c
@@ -30,7 +30,7 @@
struct moxart_gpio_chip {
struct gpio_chip gpio;
- void __iomem *moxart_gpio_base;
+ void __iomem *base;
};
static inline struct moxart_gpio_chip *to_moxart_gpio(struct gpio_chip *chip)
@@ -51,7 +51,7 @@ static void moxart_gpio_free(struct gpio_chip *chip, unsigned offset)
static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
{
struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
- void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_PIN_DIRECTION;
+ void __iomem *ioaddr = gc->base + GPIO_PIN_DIRECTION;
writel(readl(ioaddr) & ~BIT(offset), ioaddr);
return 0;
@@ -61,7 +61,7 @@ static int moxart_gpio_direction_output(struct gpio_chip *chip,
unsigned offset, int value)
{
struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
- void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_PIN_DIRECTION;
+ void __iomem *ioaddr = gc->base + GPIO_PIN_DIRECTION;
writel(readl(ioaddr) | BIT(offset), ioaddr);
return 0;
@@ -70,7 +70,7 @@ static int moxart_gpio_direction_output(struct gpio_chip *chip,
static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
{
struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
- void __iomem *ioaddr = gc->moxart_gpio_base + GPIO_DATA_OUT;
+ void __iomem *ioaddr = gc->base + GPIO_DATA_OUT;
u32 reg = readl(ioaddr);
if (value)
@@ -85,14 +85,12 @@ static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
{
struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
- u32 ret = readl(gc->moxart_gpio_base + GPIO_PIN_DIRECTION);
+ u32 ret = readl(gc->base + GPIO_PIN_DIRECTION);
if (ret & BIT(offset))
- return !!(readl(gc->moxart_gpio_base + GPIO_DATA_OUT) &
- BIT(offset));
+ return !!(readl(gc->base + GPIO_DATA_OUT) & BIT(offset));
else
- return !!(readl(gc->moxart_gpio_base + GPIO_DATA_IN) &
- BIT(offset));
+ return !!(readl(gc->base + GPIO_DATA_IN) & BIT(offset));
}
static struct gpio_chip moxart_template_chip = {
@@ -103,9 +101,7 @@ static struct gpio_chip moxart_template_chip = {
.direction_output = moxart_gpio_direction_output,
.set = moxart_gpio_set,
.get = moxart_gpio_get,
- .base = 0,
.ngpio = 32,
- .can_sleep = 0,
};
static int moxart_gpio_probe(struct platform_device *pdev)
@@ -123,11 +119,11 @@ static int moxart_gpio_probe(struct platform_device *pdev)
mgc->gpio = moxart_template_chip;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- mgc->moxart_gpio_base = devm_ioremap_resource(dev, res);
- if (IS_ERR(mgc->moxart_gpio_base)) {
+ mgc->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(mgc->base)) {
dev_err(dev, "%s: devm_ioremap_resource res_gpio failed\n",
dev->of_node->full_name);
- return PTR_ERR(mgc->moxart_gpio_base);
+ return PTR_ERR(mgc->base);
}
mgc->gpio.dev = dev;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 47+ messages in thread[parent not found: <1385980079-20175-1-git-send-email-jonas.jensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] gpio: MOXA ART: rename moxart_gpio_base to base
2013-12-02 10:27 ` Jonas Jensen
(?)
@ 2013-12-04 12:28 ` Linus Walleij
-1 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-12-04 12:28 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Mon, Dec 2, 2013 at 11:27 AM, Jonas Jensen <jonas.jensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Renaming "moxart_gpio_base" to "base" allows better fit,
> remove line breaks in moxart_gpio_get().
>
> While doing trivial cleanup, also remove fields initialized
> with zero in moxart_template_chip.
>
> Signed-off-by: Jonas Jensen <jonas.jensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Patch applied!
Thanks!
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH] gpio: MOXA ART: rename moxart_gpio_base to base
@ 2013-12-04 12:28 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-12-04 12:28 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 2, 2013 at 11:27 AM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Renaming "moxart_gpio_base" to "base" allows better fit,
> remove line breaks in moxart_gpio_get().
>
> While doing trivial cleanup, also remove fields initialized
> with zero in moxart_template_chip.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
Patch applied!
Thanks!
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] gpio: MOXA ART: rename moxart_gpio_base to base
@ 2013-12-04 12:28 ` Linus Walleij
0 siblings, 0 replies; 47+ messages in thread
From: Linus Walleij @ 2013-12-04 12:28 UTC (permalink / raw)
To: Jonas Jensen
Cc: linux-gpio, linux-arm-kernel, linux-kernel, Arnd Bergmann,
Mark Rutland, devicetree
On Mon, Dec 2, 2013 at 11:27 AM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Renaming "moxart_gpio_base" to "base" allows better fit,
> remove line breaks in moxart_gpio_get().
>
> While doing trivial cleanup, also remove fields initialized
> with zero in moxart_template_chip.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
Patch applied!
Thanks!
Linus Walleij
^ permalink raw reply [flat|nested] 47+ messages in thread