Re: [PATCH] Bitbanging i2c bus driver using the GPIO API

From: David Brownell
Date: Fri Mar 09 2007 - 16:46:05 EST


On Friday 09 March 2007 12:43 pm, Håvard Skinnemoen wrote:
> On Fri, March 9, 2007 20:30, David Brownell wrote:
> > On Friday 09 March 2007 10:48 am, Haavard Skinnemoen wrote:
> >> This is a very simple bitbanging i2c bus driver utilizing the new
> >> arch-neutral GPIO API. Useful for chips that don't have a built-in
> >> i2c controller, additional i2c busses, or testing purposes.
> >
> > That's the right idea! But remember that not all GPIOs support
> > reading back the actual value on SCL ...
>
> The idea is to keep the output value at 0 and switch the output driver on
> and off. I assumed that changing the direction was the easiest way to
> achieve this.
>
> I never really thought about output-only pins. Is it actually possible to
> implement i2c properly on such hardware?

Not strictly; but folk do so, and the results are compatible-enough
for many purposes. Certainly a quick glance at i2c-algo-bit tells
me that it knows how to cope with output-only SCL. A generic GPIO
driver "should" be able to at least act sanely given such a pin.


> > I2C has another interesting special case. at91_set_multi_drive()
> > would be appropriate (yes?) for ARCH_AT91 to use on SCL, to best
> > support both clock stretching and multi-master configurations.
>
> Isn't it better to switch the direction to input since the driver needs to
> watch the pin state in order to support clock stretching and multi-master?

Not necessarily ... reading the GPIO pad state works regardless of what
direction was configured on most chips (including AT91 and AVR32), but
not all of them.

Certainly multi-drive/open-drain outputs get the electical stuff right
without that. Russell's explanation says the reason to switch direction
is to disable the non-open-drain output drivers.

There are several options lurking, that a generic I2C bitbanger "ought"
to handle. Existence of open-drain outputs is one issue. Ability to
change direction is another; as is ability to read the value on output
pads. Your code assumed one set of answers; others are possible.


> The gpio_set_value() calls should go away and the output value should be
> explicitly set to 0 when turning on the output drivers. In its present
> form, the driver happens to work on my hardware, which is all I really
> cared about when writing it ;-)

As I said in a different way above! If open drain outputs are available,
and the actual values can be read on output pins, then I think both pins
can be configured as outputs and left that way.


> >> + printk(KERN_INFO "i2c-gpio: using pins 0x%x (sda) 0x%x (scl)\n",
> >> + pdata->sda_pin, pdata->scl_pin);
> >
> > Please, no hex there. I think dev_info() would be better; and it
> > might be nice to report whether clock stretching is supported.
>
> Ok, but how do I inform i2c-algo-bit about whether or not clock stretching
> is supported?

Reading the code, I see it's automatic if you don't provide getscl()...

- Dave

-
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/