Re: [PATCH] gpio: support for Synopsys DesignWare APB GPIO

[Anton Vorontsov]

On Sun, Apr 03, 2011 at 11:30:41AM +0200, Thomas Gleixner wrote:
[...]
> > Actually, there is a starting point you can work with.  Look at
> > drivers/gpio/basic_mmio_gpio.c, and see if it can be molded to suit
> > your purposes.

It's surely doable for most memory-mapped GPIO controllers, the
logic for toggling the GPIO values/direction and reading input
state is pretty much the same across all the controllers.

> > There don't appear to be any actual users of that driver in mainline
> > at the moment, so feel free to propose changes if need be.

Yeah, I haven't had time to post the users (there are two: a custom
FPGA GPIO controller, and a GPIO controller in the Cavium ECONA
ARM machine).

I'll happily review/test any changes to the driver though.

> > I'm not
> > hugely thrilled with the current method that the driver uses to define
> > the register locations (using named resources).  My instinct would be
> > to use a single register resource region with offsets for each
> > register type defined from the base of it, but Anton can probably fill
> > us in on the reason that approach was used.

Well, I did it that way because you don't have to pass the offsets via
platform data (you don't need platform data most of the time, i.e. if
you use dynamic bases).

And with device tree, you wouldn't need any constructors:

gpio-controller@f00 {
	reg = <0xf00 0x4      // would map to "dat" resource
	       0xf04 0x4      // would maps to "set" resource
	       0xf08 0x4>;    // would maps to "clr" resource
};

That is, I dreamed about something like this, the way to (re)map
DT resources to Linux resources:

static const struct of_resource_map resmap[] = {
	/* type   DTnum  Lnum  Name  */
	{  MEM,   0,     1,    "mmio space1" },
	{  MEM,   1,     0,    "mmio space2" },
	{  IRQ,   0,     1,    "tx irq" },
	{  IRQ,   1,     0,    "rx irq" },
	{},
};

static struct platform_driver drv = {
	.driver.of_match_table = ...;
	.driver.of_resource_map_table = &resmap;
};

But still, I have no problem with either approaches, be it mutiple
resources or a resource + offsets.

> Right, that avoids multiple ioremaps for the same physical address
> space.

I guess this could be solved by a tiny logic inside the driver (i.e.
if the registers lie within a single page, map them with a single
ioremap call, otherwise fallback to multiple ioremaps). Not sure if
that worth a trouble though.

> There are more odds in that driver. It should setup proper
> function pointers for accessing the registers instead of runtime
> evaluation of everything.

Good idea, thanks.

-- 
Anton Vorontsov
Email: cbouatmailru@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/