From mboxrd@z Thu Jan  1 00:00:00 1970
From: joshc@linux.com (Josh Cartwright)
Date: Thu, 29 Dec 2011 12:53:39 -0500
Subject: GPIO driver module for Jetway NF98 board
In-Reply-To: <4EFB4BE4.2090409@uib.cat>
References: <20111222012154.GB14353@joshcartwright.net>
	<4EFB4BE4.2090409@uib.cat>
Message-ID: <20111229175339.GE14353@joshcartwright.net>
To: kernelnewbies@lists.kernelnewbies.org
List-Id: kernelnewbies.lists.kernelnewbies.org

On Wed, Dec 28, 2011 at 06:03:32PM +0100, Joan Pau Beltran wrote:
> Thank you very much for your response.
> It brings up a lot of useful information.

Great, glad it helped!

*snip*

> Al 22/12/11 01:15, En/na Josh Cartwright ha escrit:
> > On Wed, Dec 21, 2011 at 06:34:58PM +0100, Joan Pau Beltran wrote:
> > Searching for the symbolic names brings up this document, which looks
> > like it may describe in more detail how to use this GPIO interface:
> >
> > http://download.intel.com/embedded/chipsets/appnote/322174.pdf
> >
>
> After looking at both documents, can you please confirm these ideas:
> 1.- The GPIO functionality of the jwnf98 comes from a hardware
> device in the chipset called LPC (Low Pin Cout) controller managed
> through the LPC Interface Bridge registers (section 9.1 page 366).
> This LPC controller provides other functionalities, too.
> 2.- The LPC resides in a PCI bus.
> 3.- GPIO is managed accessing the registers of the LPC. We should
> read from and write to these registers according to the notes in
> 13.10 (GPIO_USE_SEL, GPIO_IO_SEL and GPIO_LVL). Not all the GPIOs
> are available, only the ones given in the Jetway code.
> 4.- The GPIO registers of the LPC are mapped to some I/O port,
> starting at the address specified in the GPIOBASE register. From the
> first paragraph in section 13.10, page 546:
> > The control for the general purpose I/O signals is handled through
> > a 128-byte I/O space. The base offset for this space is selected by the GPIOBASE
> > register.

Yes, that is how I understand it, too.

> 5.- From Jetway code, it seems that the base address in the GPIOBASE
> register described in 13.1.14 is set by the manufacturer to 0x500.
> Conversely, the Intel code gets that base address reading the
> GPIOBASE register. Note that specific pci functions are used for
> that. If Tech Support info is ok, I do not need to do that, and can
> simply request the needed i/o ports taking as base offset 0x500.

I'd recommend getting the base address of the GPIO region as documented
in the Intel manuals, instead of what looks like throw-away testing code
from your vendor.

Take a peak at drivers/watchdog/iTCO_wdt.c, as this watchdog timer is
also accessed through the LPC device.  It might give you a few ideas.

> If the above points are ok, I have some doubts related with my
> previous questions:
> > > 1. How should I request/release the ports mentioned in the attached
> > > file? Should I use the request_region/release_region functions from
> > > linux/ioport.h? In such case, what should I do with the returned
> > > struct resource?
>
> I think I should userequest_region/release_region from
> linux/ioport.h, and simply ignore the returned resource, is it ok?

I think that would be fine (there are other drivers that currently do
this).

> > > 3. Should/could I use the test_bit, set_bit, clear_bit functions to
> > > get, set the bit in the needed read/write functions I am writing? Or
> > > should I use the sequence 'inb - mask the value properly - outb' ?
> > Nope, as far as I know these bitops only work with memory operands.
> Is this because we use port-based i/o instead of memory-mapped i/o?

Yes, precisely.  I'd recommend just using standard shift/masking (which
looks like what you are already doing).  Keep in mind, however,
you'll need some locking strategy to ensure that a read-modify-write
cycle happens atomically.

> Here it goes the code again.

While I appreciate it being inlined this time, your mailer seemed to
have munged whitespace, such that the code is very difficult to read :(.
You may want to see Documentation/email-clients.txt.

> The main question is, should the gpio_chip be statically allocated,
> or it should be created in the init function and destroyed in the
> exit function? If the latter, how to do that?

Having one statically allocated gpio_chip object assumes that there will
be only one of these chips installed in a system.  I think that is a
safe assumption to make, so you should be okay.

-- 
                                 joshc