Thread (19 messages) 19 messages, 4 authors, 2013-12-04

[PATCH v5] gpio: Add MOXA ART GPIO driver

From: Linus Walleij <hidden>
Date: 2013-10-11 15:44:18
Also in: linux-devicetree, linux-gpio, lkml

On Fri, Oct 11, 2013 at 4:53 PM, Jonas Jensen [off-list ref] 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help