lkml.org 
[lkml]   [2013]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5] gpio: Add MOXA ART GPIO driver
From
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


\
 
 \ /
  Last update: 2013-10-11 18:01    [W:0.154 / U:1.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site