[lm-sensors] making a geode i2c slave driver

[lm-sensors] making a geode i2c slave driver

[Thomas Andrews]

Hi Jordan,

I hope you don't mind the imposition, Jean Delvare (khali on
#linux-sensors) suggested I contact you about a geode driver that I need
to write. (He said you wanted to do something similar a while back.)

I need to get the kernel to become an i2c slave from time to time. The
kernel is on a geode sc1100. Actually it is on one of Pascal's
(pc-engines) "WRAP" boards.

I have a little Atmel micro connected to the i2c bus on the geode, and
currently I have it working perfectly as a slave. My problem is that I
want the Atmel to be able to "push" info back to the geode without being
polled. You see I want fairly low latency - I don't want to have to poll
the Atmel every 10 milliseconds!

At the moment I am using an "adapter" driver for the SC1100 called
i2c-nscacb.c. It doesn't seem to have a maintainer, but it only had a
couple of little bug-fixes required, so once I'd done that it worked
perfectly.

The driver that I wrote for the Atmel is basically a vanilla sensor
driver. I need it to initially behave as a master to configure the
Atmel, and then sit back and wait for "events" to come in from the
Atmel. I am sure this means that I will need to modify the i2c-nscacb
module to become interrupt-driven, and that is apparently where my needs
overlap with yours?

I must warn you that I've only been looking at the lmsensors code for a
couple of days, so my knowledge is still full of pretty big holes, but I
have to get this thing working, so I'll get there :-)

How should I go about this problem? What would you suggest ?

Many thanks,
Thomas

[lm-sensors] making a geode i2c slave driver

[Thomas Andrews]

On Thu, Apr 27, 2006 at 11:21:59AM +0200, Thomas Andrews wrote:

> I need to get the kernel to become an i2c slave from time to time. The
> kernel is on a geode sc1100. Actually it is on one of Pascal's
> (pc-engines) "WRAP" boards.

Just a small additional note - I don't really need a "full" slave
implementation, because the Atmel is not likely to ever read from the
geode. It will basically write status info to the geode only. I think
that should make it much simpler.

Regards,
Thomas

[lm-sensors] making a geode i2c slave driver

[Mark M. Hoffman]

Hi Thomas:

* Thomas Andrews <tandrews at grok.co.za> [2006-04-27 11:21:59 +0200]:
> I hope you don't mind the imposition, Jean Delvare (khali on
> #linux-sensors) suggested I contact you about a geode driver that I need
> to write. (He said you wanted to do something similar a while back.)
> 
> I need to get the kernel to become an i2c slave from time to time. The
> kernel is on a geode sc1100. Actually it is on one of Pascal's
> (pc-engines) "WRAP" boards.
> 
> I have a little Atmel micro connected to the i2c bus on the geode, and
> currently I have it working perfectly as a slave. My problem is that I
> want the Atmel to be able to "push" info back to the geode without being
> polled. You see I want fairly low latency - I don't want to have to poll
> the Atmel every 10 milliseconds!
>
> At the moment I am using an "adapter" driver for the SC1100 called
> i2c-nscacb.c. It doesn't seem to have a maintainer, but it only had a
> couple of little bug-fixes required, so once I'd done that it worked
> perfectly.
> 
> The driver that I wrote for the Atmel is basically a vanilla sensor
> driver. I need it to initially behave as a master to configure the
> Atmel, and then sit back and wait for "events" to come in from the
> Atmel. I am sure this means that I will need to modify the i2c-nscacb
> module to become interrupt-driven, and that is apparently where my needs
> overlap with yours?
> 
> I must warn you that I've only been looking at the lmsensors code for a
> couple of days, so my knowledge is still full of pretty big holes, but I
> have to get this thing working, so I'll get there :-)
> 
> How should I go about this problem? What would you suggest ?

Another alternative would be to implement support for SMBALERT#.  This is
basically a third wire that acts as an interrupt signal from slave to
master.  At the moment, there is no support for it in the I2C core, but
it could be added... and it may be easier than adding host-as-slave
support (if your hardware can support SMBALERT#).

SMBALERT# is described here, in appendix A:

http://www.smbus.org/specs/smbus20.pdf

Regards,

-- 
Mark M. Hoffman
mhoffman at lightlink.com

[lm-sensors] making a geode i2c slave driver

[Jordan Crouse]

On 27/04/06 11:21 +0200, Thomas Andrews wrote:
> Hi Jordan,

Greetings Thomas!

> I hope you don't mind the imposition, Jean Delvare (khali on
> #linux-sensors) suggested I contact you about a geode driver that I need
> to write. (He said you wanted to do something similar a while back.)

Sort of.. :)  We've been asked to consider adding slave support to the
drivers before, and I've been trying to wrap my brain around how extensive
we need to go.  Is it just a simple little array in the driver, or is a 
full blown, all bells and whistles implementation complete with "slave"
meta drivers and userland interaction?  Unfortunately, part of the problem
is that we have been struggling to find usage models for slave mode.  I 
thank you for giving me my first taste of a real world model, beyond
just a simple EEPROM like device.

> I need to get the kernel to become an i2c slave from time to time. The
> kernel is on a geode sc1100. Actually it is on one of Pascal's
> (pc-engines) "WRAP" boards.
> 
> I have a little Atmel micro connected to the i2c bus on the geode, and
> currently I have it working perfectly as a slave. My problem is that I
> want the Atmel to be able to "push" info back to the geode without being
> polled. You see I want fairly low latency - I don't want to have to poll
> the Atmel every 10 milliseconds!
> 
> At the moment I am using an "adapter" driver for the SC1100 called
> i2c-nscacb.c. It doesn't seem to have a maintainer, but it only had a
> couple of little bug-fixes required, so once I'd done that it worked
> perfectly.
> 
> The driver that I wrote for the Atmel is basically a vanilla sensor
> driver. I need it to initially behave as a master to configure the
> Atmel, and then sit back and wait for "events" to come in from the
> Atmel. I am sure this means that I will need to modify the i2c-nscacb
> module to become interrupt-driven, and that is apparently where my needs
> overlap with yours?

Pretty much.   As you probably know, the SMBus/Access.BUS controller is the
same for the all of the Geode family, and that we can assign a slave 
address to the block, which will result in an interrupt back when a master
requests us. 

Our current thinking is that we will create a new class of I2C driver
called "slave" drivers - they will register themselves with the I2C
framework, and latch on to a particular bus driver with a specific address.

The bus driver will monitor that address, suck in the data when the interrupt
comes through, and push it back up through the framework to the slave
driver that will do with it as it will.   That will keep ugly stuff like
userland interaction outside of the driver, and it will let us plug in new
slave drivers easily without having to have knowledge of the underlying bus.

Or at least, thats the plan... :)

> I must warn you that I've only been looking at the lmsensors code for a
> couple of days, so my knowledge is still full of pretty big holes, but I
> have to get this thing working, so I'll get there :-)
> 
> How should I go about this problem? What would you suggest ?

Well, the first thing is to study the spec so you know what you're up
against, in terms of adding slave mode to the specific bus driver:

http://www.amd.com/us-en/ConnectivitySolutions/ProductInformation/0,,50_2330_9863_9883%5E9886,00.html

You might also want to consider moving up to the scx200_acb.c driver - that
way all Geodes can share in your efforts.  

Beyond that - you'll have to consider what works best for your situation.
Obviously, now that I have two possible models, I'm more convinced then ever
that a slave driver framework is the way to go; but on the other hand that
will result in much more work, and I know how deadlines work.. :)

So good luck.  Please keep me in the loop, and if you need any help,
my freenode nick is CosmicPenguin, and i try to hang out in #linux-sensors,
though I often forget to re-join after a disconnect.

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

[lm-sensors] making a geode i2c slave driver

[Jordan Crouse]

On 27/04/06 09:31 -0400, Mark M. Hoffman wrote:
> Another alternative would be to implement support for SMBALERT#.  This is
> basically a third wire that acts as an interrupt signal from slave to
> master.  At the moment, there is no support for it in the I2C core, but
> it could be added... and it may be easier than adding host-as-slave
> support (if your hardware can support SMBALERT#).

I don't think the 1100 has specific support for that, but you could
always plug it into an unused GPIO, I would think, and process the interrupt
from there.

Jordan
-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

[lm-sensors] making a geode i2c slave driver

[Thomas Andrews]

Hi Jordan,

On Thu, Apr 27, 2006 at 08:30:55AM -0600, Jordan Crouse wrote:

> [...] We've been asked to consider adding slave support to the drivers
> before, and I've been trying to wrap my brain around how extensive we
> need to go.  Is it just a simple little array in the driver, or is a
> full blown, all bells and whistles implementation complete with
> "slave" meta drivers and userland interaction?  [...]

I'm just going to start with the simplest approach. Once I understand
what I'm doing we can make it more 'generic'. At the moment I don't even
have a handle on the existing framework. :p

> Our current thinking is that we will create a new class of I2C driver
> called "slave" drivers - they will register themselves with the I2C
> framework, and latch on to a particular bus driver with a specific
> address.

This is much more "layered" than how I was thinking of doing it. My
initial thoughts were to take the existing adapter driver for the geode
and make it interrupt driven rather than polled. Once that is in place,
the interrupt handler should then be "aware" of the i2c state-machine's
current state. It can therefore make sensible decisions about what to do
when another master tries to talk. If enabled as a slave, the handler
can then use a call-back to the slave driver...
 
> The bus driver will monitor that address, suck in the data when the
> interrupt comes through, and push it back up through the framework to
> the slave driver that will do with it as it will.   That will keep
> ugly stuff like userland interaction outside of the driver, and it
> will let us plug in new slave drivers easily without having to have
> knowledge of the underlying bus.

As I understand it, this would put an additional layer between the
adapter driver and the hardware. This additional layer would do either
polling or handle interrupts. It effectively means splitting the adapter
driver into two parts, if I understand what you're saying. What would be
the advantage to splitting the driver though?

> > How should I go about this problem? What would you suggest ?
> 
> Well, the first thing is to study the spec so you know what you're up
> against, in terms of adding slave mode to the specific bus driver:
> 
> http://www.amd.com/us-en/ConnectivitySolutions/ProductInformation/0,,50_2330_9863_9883%5E9886,00.html

I'm currently using the sc1100 data book. I don't really want to be
reading two similar docs. Am I reading the wrong one for the more
generic solution?
 
> You might also want to consider moving up to the scx200_acb.c driver - that
> way all Geodes can share in your efforts.  

Thanks very much for that! I've moved over now :)
 
> So good luck.  Please keep me in the loop, and if you need any help,
> my freenode nick is CosmicPenguin, and i try to hang out in #linux-sensors,
> though I often forget to re-join after a disconnect.

Thanks very much, I *really* appreciate the help!

Regards,
Thomas

[lm-sensors] making a geode i2c slave driver

[Jean Delvare]

Hi Thomas,

> I hope you don't mind the imposition, Jean Delvare (khali on
> #linux-sensors) suggested I contact you about a geode driver that I need
> to write. (He said you wanted to do something similar a while back.)

That was Rudolf Marek who did, actually.

> At the moment I am using an "adapter" driver for the SC1100 called
> i2c-nscacb.c. It doesn't seem to have a maintainer, but it only had a
> couple of little bug-fixes required, so once I'd done that it worked
> perfectly.

We have two drivers for similar boards in the kernel already:
scx200_i2c and scx200_acb. I suspect that the latter is a replacement
for the older, never merged i2c-nscacb driver you're using. This guess
is backed up by this one-year old thread:
http://lists.lm-sensors.org/pipermail/lm-sensors/2005-April/011756.html
http://lists.lm-sensors.org/pipermail/lm-sensors/2005-May/012043.html

> The driver that I wrote for the Atmel is basically a vanilla sensor
> driver. (...)

How does a microcontroller qualify as a sensor? And what do you mean
with "vanilla"?

I will merge the following documentation update for the scx200_acb
driver, comments welcome.

* * * * *

Documentation update for the scx200_acb driver. Hopefully this should
help future users.

References:

[lm-sensors] Support of i2c-nscacb (April, May 2005)
  http://lists.lm-sensors.org/pipermail/lm-sensors/2005-April/011756.html
  http://lists.lm-sensors.org/pipermail/lm-sensors/2005-May/012043.html

[lm-sensors] making a geode i2c slave driver (April 2006)
  http://lists.lm-sensors.org/pipermail/lm-sensors/2006-April/015998.html

Kernel bug #6445
  http://bugzilla.kernel.org/show_bug.cgi?idd45

Signed-off-by: Jean Delvare <khali at linux-fr.org>
---
 Documentation/i2c/busses/scx200_acb |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

--- linux-2.6.17-rc3.orig/Documentation/i2c/busses/scx200_acb	2006-04-03 20:12:23.000000000 +0200
+++ linux-2.6.17-rc3/Documentation/i2c/busses/scx200_acb	2006-04-28 10:45:30.000000000 +0200
@@ -2,14 +2,31 @@
 
 Author: Christer Weinigel <wingel at nano-system.com>
 
+The driver supersedes the older, never merged driver named i2c-nscacb.
+
 Module Parameters
 -----------------
 
-* base: int
+* base: up to 4 ints
   Base addresses for the ACCESS.bus controllers on SCx200 and SC1100 devices
 
+  By default the driver uses two base addresses 0x820 and 0x840.
+  If you want only one base address, specify the second as 0 so as to
+  override this default.
+
 Description
 -----------
 
 Enable the use of the ACCESS.bus controller on the Geode SCx200 and
 SC1100 processors and the CS5535 and CS5536 Geode companion devices.
+
+Device-specific notes
+---------------------
+
+The SC1100 WRAP boards are known to use base addresses 0x810 and 0x820.
+If the scx200_acb driver is built into the kernel, add the following
+parameter to your boot command line:
+  scx200_acb.base=0x810,0x820
+If the scx200_acb driver is built as a module, add the following line to
+the file /etc/modprobe.conf instead:
+  options scx200_acb base=0x810,0x820


-- 
Jean Delvare

[lm-sensors] making a geode i2c slave driver

[Thomas Andrews]

Hi Jean,

On Fri, Apr 28, 2006 at 01:03:19PM +0200, Jean Delvare wrote:

> > I hope you don't mind the imposition, Jean Delvare (khali on
> > #linux-sensors) suggested I contact you about a geode driver that I need
> > to write. (He said you wanted to do something similar a while back.)
> 
> That was Rudolf Marek who did, actually.

My humble appologies! I got my wires crossed. :p
 
[...]

> > The driver that I wrote for the Atmel is basically a vanilla sensor
> > driver. (...)
> 
> How does a microcontroller qualify as a sensor? And what do you mean
> with "vanilla"?

It doesn't qualify, and it isn't intended to stay that way, but as a
beginner, I found it to be the simplest way to get my hardware to be
seen. I made my micro behave for the time being like a temperature
sensor would do. By vanilla, I really just meant simple.

When I've figured out the correct way to do it, I'll implement it. Any
hints or advice in this regard would be most welcome!
 
> I will merge the following documentation update for the scx200_acb
> driver, comments welcome.

Thanks. I figured the base address stuff out myself, so I think it would
be a good idea if the next person doesn't have to do so too. As an
aside, the old driver that I used had a method to find the base
addresses out by itself. It worked for me. Do you think it makes sense
to merge that functionality into the new driver? I posted an excerpt to
the list in another thread earlier today. I'm happy to make the patch,
but I'll need someone to test the 1200 family...

Thanks for the help!
Thomas

[lm-sensors] making a geode i2c slave driver

[Thomas Andrews]

Hi Jordan,

I'm having some difficulty getting the geode to generate an interrupt
for ACB events. I'm hoping that you can either spot some obvious mistake
that I'm making, or perhaps refer me to someone at AMD who can.

As you recall I'm using the SC1100 on a WRAP card. I'm using a version
of scx200_acb.c, modified as follows:

(a) I set ACB1/2 index 0x70 (Interrupt Number) to 5 (for IRQ 5):

        irq = 5;
        sio = 0x2e;
        val = 0x2f;

        // Set the IRQ on ACB0  (LDN 5)
        outb(0x07,sio); // LDN select register
        outb(0x05,val); // Choose LDN 5 ie ACCESS.bus 1
        outb(0x70,sio); // Select IRQ register
        outb(irq,val);  // Set IRQ

        // Set the IRQ on ACB1 (LDN 6)
        outb(0x07,sio); // LDN select register
        outb(0x06,val); // Choose LDN 6 ie ACCESS.bus 2
        outb(0x70,sio); // Select IRQ register
        outb(irq,val);  // Set IRQ

(b) I enable the device's slave address on each ACB:

        outb(0x80 | 1, ACBADDR);
 
(c) I enable interrupts on each ACB:

        outb(inb(ACBCTL1) | ACBCTL1_INTEN | ACBCTL1_NMINTE, ACBCTL1);

(d) I have an irq handler installed for IRQ 5, but that is not really
    important, because it isn't needed yet.

The problem is that I *never* see any interrupts generated by the ACB.
Here is what I see reported by the kernel:

8<----------------8<----------------8<----------------8<----------------
root at sprinkle:~/i2c# cat /proc/stat && cat /proc/interrupts 
cpu  3097 0 4466 16293239
cpu0 3097 0 4466 16293239
page 6618 18472
swap 0 0
intr 16372006 16300802 0 0 0 116 0 0 0 0 0 20905 0 0 0 50183 0
disk_io: (3,0):(5245,921,13236,4324,36944) 
ctxt 168525
btime 1146401337
processes 679
           CPU0       
  0:   16300803          XT-PIC  timer
  2:          0          XT-PIC  cascade
  4:        116          XT-PIC  serial
  5:          0          XT-PIC  scx200_acb
 10:      20907          XT-PIC  eth0
 14:      50183          XT-PIC  ide0
NMI:          0 
ERR:          0
8<----------------8<----------------8<----------------8<----------------

What am I missing here ?

I can get my Atmel chip to generate a packet as a master, and the geode
actually does ACK the address, so the geode is definitely behaving
correctly as a slave. Somehow the interrupt just never happens though.

Many thanks,
Thomas

[lm-sensors] making a geode i2c slave driver

[Jordan Crouse]

On 02/05/06 12:10 +0200, Thomas Andrews wrote:
> Hi Jordan,
> 
> What am I missing here ?
> 
> I can get my Atmel chip to generate a packet as a master, and the geode
> actually does ACK the address, so the geode is definitely behaving
> correctly as a slave. Somehow the interrupt just never happens though.

Try turning on the general match bit, and see if that causes the interrupt
to fire.  

Jordan

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

[lm-sensors] making a geode i2c slave driver

[Mark M. Hoffman]

Hi Thomas:

* Thomas Andrews <tandrews at grok.co.za> [2006-05-02 12:10:48 +0200]:
> I'm having some difficulty getting the geode to generate an interrupt
> for ACB events. I'm hoping that you can either spot some obvious mistake
> that I'm making, or perhaps refer me to someone at AMD who can.
> 
> As you recall I'm using the SC1100 on a WRAP card. I'm using a version
> of scx200_acb.c, modified as follows:

[...]

> The problem is that I *never* see any interrupts generated by the ACB.
> Here is what I see reported by the kernel:

[...]

> What am I missing here ?
> 
> I can get my Atmel chip to generate a packet as a master, and the geode
> actually does ACK the address, so the geode is definitely behaving
> correctly as a slave. Somehow the interrupt just never happens though.

Is this a PCI device?  If it is, you're probably just missing a call to
pci_enable_device().  I got hung up on that with similar symptoms while
doing a rewrite of the i2c-i801 driver (which is still unfinished).

Also btw: either a patch or a link to the whole source is appreciated
when asking for debug help.

Regards,

-- 
Mark M. Hoffman
mhoffman at lightlink.com