Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Mon, 12 Mar 2007 16:11:09 +0100 Jean Delvare <[EMAIL PROTECTED]> wrote: > > By the way, timeout seems to be hardcoded to 100 jiffies in the > > i2c-algo-bit driver, so there's probably not much point passing it from > > the board code when it's going to be overridden anyway. I'll add just a > > udelay parameter to the platform struct for now. > > No, it's not hardcoded. I know it looks confusing. struct i2c_adapter > has a timeout field, that's the one being set to 100 in i2c-algo-bit, > but i2c-algo-bit uses the i2c_algo_bit_data timeout field. The > i2c_adapter timeout field is unused. Ah, I see. Now that you mention it, I seem to recall I came to that conclusion last time I looked at the code, but I've apparently forgotten it since then ;) I'll add a timeout field to the platform struct as well, then. Haavard - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
Hi Haavard, On Mon, 12 Mar 2007 15:53:59 +0100, Haavard Skinnemoen wrote: > On Mon, 12 Mar 2007 15:34:57 +0100 > Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > > > > > + bit_data->udelay= 5,/* 100 kHz */ > > > > + bit_data->timeout = HZ / 10, /* 100 ms */ > > > > > > Can we add these udelay/timeout to struct i2c_gpio_platform_data? And > > > let customer to choose these according their specific requirement. We > > > use Kconfig to do this, but Jean and David don't like the idea, -:( > > > > Yeah, they need to be a bit more configurable than they currently are. > > And I think it makes sense to pass them from the board setup code, since > > this is where things depending on board-specific details (signal quality > > issues, pullup resistor values, etc.) are supposed to go. > > By the way, timeout seems to be hardcoded to 100 jiffies in the > i2c-algo-bit driver, so there's probably not much point passing it from > the board code when it's going to be overridden anyway. I'll add just a > udelay parameter to the platform struct for now. No, it's not hardcoded. I know it looks confusing. struct i2c_adapter has a timeout field, that's the one being set to 100 in i2c-algo-bit, but i2c-algo-bit uses the i2c_algo_bit_data timeout field. The i2c_adapter timeout field is unused. This is clearly calling for a cleanup but I don't have time for this right now. -- Jean Delvare - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Mon, 12 Mar 2007 15:34:57 +0100 Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > > > + bit_data->udelay= 5,/* 100 kHz */ > > > + bit_data->timeout = HZ / 10, /* 100 ms */ > > > > Can we add these udelay/timeout to struct i2c_gpio_platform_data? And > > let customer to choose these according their specific requirement. We > > use Kconfig to do this, but Jean and David don't like the idea, -:( > > Yeah, they need to be a bit more configurable than they currently are. > And I think it makes sense to pass them from the board setup code, since > this is where things depending on board-specific details (signal quality > issues, pullup resistor values, etc.) are supposed to go. By the way, timeout seems to be hardcoded to 100 jiffies in the i2c-algo-bit driver, so there's probably not much point passing it from the board code when it's going to be overridden anyway. I'll add just a udelay parameter to the platform struct for now. Haavard - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Mon, 12 Mar 2007 18:07:59 +0800
"Wu, Bryan" <[EMAIL PROTECTED]> wrote:
> > static struct i2c_gpio_platform_data i2c_gpio_data = {
> > .sda_pin= GPIO_PIN_FOO,
> > .scl_pin= GPIO_PIN_BAR,
> > };
>
> Is this usage right, because 3 flags are added to this structure as
> below:
>
> struct i2c_gpio_platform_data {
> unsigned int sda_pin;
> unsigned int scl_pin;
> unsigned int sda_is_open_drain:1;
> unsigned int scl_is_open_drain:1;
> unsigned int scl_is_output_only:1;
> };
Well, it is the simplest possible example. The last 3 fields will be 0,
which is a valid configuration.
> Thanks a lot, I will drop our GPIO based I2C driver and try this one on
> our platform.
I hope it works for you.
> > + if (!pdata->scl_is_output_only)
> > + bit_data->getscl = i2c_gpio_getscl,
> > +
> > + bit_data->getsda= i2c_gpio_getsda,
> > + bit_data->udelay= 5,/* 100 kHz */
> > + bit_data->timeout = HZ / 10, /* 100 ms */
>
> Can we add these udelay/timeout to struct i2c_gpio_platform_data? And
> let customer to choose these according their specific requirement. We
> use Kconfig to do this, but Jean and David don't like the idea, -:(
Yeah, they need to be a bit more configurable than they currently are.
And I think it makes sense to pass them from the board setup code, since
this is where things depending on board-specific details (signal quality
issues, pullup resistor values, etc.) are supposed to go.
Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Sat, 10 Mar 2007 21:15:50 +0100
Jean Delvare <[EMAIL PROTECTED]> wrote:
> I like the idea very much. Would this let us get rid of i2c-ixp2000?
> i2c-ixp4xx? scx200_i2c? Other drivers?
Any platform that implements the generic gpio api should be able to use
this driver. So yes, I hope we might be able to get rid of a few
existing bitbanging drivers.
> > +/*
> > + * Bitbanging i2c bus driver using the GPIO API
> > + *
> > + * Copyright (C) 2006 Atmel Corporation
>
> I'm told we're in year 2007 ;)
I'm also told that copyright protection lasts infinitely long in
practice ;)
I'll update it. I probably just copied it blindly from a different
driver.
> > +int i2c_gpio_getsda(void *data)
> > +{
> > + struct i2c_gpio_platform_data *pdata = data;
> > +
> > + return gpio_get_value(pdata->sda_pin);
> > +}
>
>
> What value will you get if the SDA pin is open-drain and currently in
> output mode? Are such GPIO pins actually able to detect that the pin is
> low while they are not themselves driving it low?
I guess that depends on the GPIO controller. But being able to read the
pin state even when the pin is configured as an output is a
prerequisite for using this driver with "open drain" pins, so if the
hardware doesn't support this, the board code should just set
{sda,scl}_is_opendrain to zero.
> > + bit_data->udelay= 5,/* 100 kHz */
>
> Actually, no, i2c-algo-bit has a 1/3-2/3 duty cycle, so a complete
> cycle is 3 times the udelay value. So udelay=5 gives you 66 kHz. If
> someone wants to fix that...
Ok. I guess we should move this parameter into the platform data struct
anyway.
> Also, I wouldn't recommend such a low value when SCL cannot be sensed,
> if a slave stretches the line even very briefly, you won't notice and
> havoc will ensue. udelay=50 sounds more reasonable for such half-baked
> busses.
Makes sense.
> > + ret = platform_driver_probe(_gpio_driver, i2c_gpio_probe);
> > + if (ret)
> > + printk("i2c-gpio: probe failed: %d\n", ret);
>
> Add KERN_ERR or similar.
Will do.
> Would you mind also adding yourself to MAINTAINERS for this driver? I
> would appreciate it.
Sure. I'm hoping this driver won't cause that much maintenance overhead
anyway since all the complicated stuff is in i2c-algo-bit. But I agree
it needs a maintainer.
Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Sat, 10 Mar 2007 21:15:50 +0100
Jean Delvare [EMAIL PROTECTED] wrote:
I like the idea very much. Would this let us get rid of i2c-ixp2000?
i2c-ixp4xx? scx200_i2c? Other drivers?
Any platform that implements the generic gpio api should be able to use
this driver. So yes, I hope we might be able to get rid of a few
existing bitbanging drivers.
+/*
+ * Bitbanging i2c bus driver using the GPIO API
+ *
+ * Copyright (C) 2006 Atmel Corporation
I'm told we're in year 2007 ;)
I'm also told that copyright protection lasts infinitely long in
practice ;)
I'll update it. I probably just copied it blindly from a different
driver.
+int i2c_gpio_getsda(void *data)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ return gpio_get_value(pdata-sda_pin);
+}
What value will you get if the SDA pin is open-drain and currently in
output mode? Are such GPIO pins actually able to detect that the pin is
low while they are not themselves driving it low?
I guess that depends on the GPIO controller. But being able to read the
pin state even when the pin is configured as an output is a
prerequisite for using this driver with open drain pins, so if the
hardware doesn't support this, the board code should just set
{sda,scl}_is_opendrain to zero.
+ bit_data-udelay= 5,/* 100 kHz */
Actually, no, i2c-algo-bit has a 1/3-2/3 duty cycle, so a complete
cycle is 3 times the udelay value. So udelay=5 gives you 66 kHz. If
someone wants to fix that...
Ok. I guess we should move this parameter into the platform data struct
anyway.
Also, I wouldn't recommend such a low value when SCL cannot be sensed,
if a slave stretches the line even very briefly, you won't notice and
havoc will ensue. udelay=50 sounds more reasonable for such half-baked
busses.
Makes sense.
+ ret = platform_driver_probe(i2c_gpio_driver, i2c_gpio_probe);
+ if (ret)
+ printk(i2c-gpio: probe failed: %d\n, ret);
Add KERN_ERR or similar.
Will do.
Would you mind also adding yourself to MAINTAINERS for this driver? I
would appreciate it.
Sure. I'm hoping this driver won't cause that much maintenance overhead
anyway since all the complicated stuff is in i2c-algo-bit. But I agree
it needs a maintainer.
Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Mon, 12 Mar 2007 18:07:59 +0800
Wu, Bryan [EMAIL PROTECTED] wrote:
static struct i2c_gpio_platform_data i2c_gpio_data = {
.sda_pin= GPIO_PIN_FOO,
.scl_pin= GPIO_PIN_BAR,
};
Is this usage right, because 3 flags are added to this structure as
below:
struct i2c_gpio_platform_data {
unsigned int sda_pin;
unsigned int scl_pin;
unsigned int sda_is_open_drain:1;
unsigned int scl_is_open_drain:1;
unsigned int scl_is_output_only:1;
};
Well, it is the simplest possible example. The last 3 fields will be 0,
which is a valid configuration.
Thanks a lot, I will drop our GPIO based I2C driver and try this one on
our platform.
I hope it works for you.
+ if (!pdata-scl_is_output_only)
+ bit_data-getscl = i2c_gpio_getscl,
+
+ bit_data-getsda= i2c_gpio_getsda,
+ bit_data-udelay= 5,/* 100 kHz */
+ bit_data-timeout = HZ / 10, /* 100 ms */
Can we add these udelay/timeout to struct i2c_gpio_platform_data? And
let customer to choose these according their specific requirement. We
use Kconfig to do this, but Jean and David don't like the idea, -:(
Yeah, they need to be a bit more configurable than they currently are.
And I think it makes sense to pass them from the board setup code, since
this is where things depending on board-specific details (signal quality
issues, pullup resistor values, etc.) are supposed to go.
Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Mon, 12 Mar 2007 15:34:57 +0100 Haavard Skinnemoen [EMAIL PROTECTED] wrote: + bit_data-udelay= 5,/* 100 kHz */ + bit_data-timeout = HZ / 10, /* 100 ms */ Can we add these udelay/timeout to struct i2c_gpio_platform_data? And let customer to choose these according their specific requirement. We use Kconfig to do this, but Jean and David don't like the idea, -:( Yeah, they need to be a bit more configurable than they currently are. And I think it makes sense to pass them from the board setup code, since this is where things depending on board-specific details (signal quality issues, pullup resistor values, etc.) are supposed to go. By the way, timeout seems to be hardcoded to 100 jiffies in the i2c-algo-bit driver, so there's probably not much point passing it from the board code when it's going to be overridden anyway. I'll add just a udelay parameter to the platform struct for now. Haavard - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
Hi Haavard, On Mon, 12 Mar 2007 15:53:59 +0100, Haavard Skinnemoen wrote: On Mon, 12 Mar 2007 15:34:57 +0100 Haavard Skinnemoen [EMAIL PROTECTED] wrote: + bit_data-udelay= 5,/* 100 kHz */ + bit_data-timeout = HZ / 10, /* 100 ms */ Can we add these udelay/timeout to struct i2c_gpio_platform_data? And let customer to choose these according their specific requirement. We use Kconfig to do this, but Jean and David don't like the idea, -:( Yeah, they need to be a bit more configurable than they currently are. And I think it makes sense to pass them from the board setup code, since this is where things depending on board-specific details (signal quality issues, pullup resistor values, etc.) are supposed to go. By the way, timeout seems to be hardcoded to 100 jiffies in the i2c-algo-bit driver, so there's probably not much point passing it from the board code when it's going to be overridden anyway. I'll add just a udelay parameter to the platform struct for now. No, it's not hardcoded. I know it looks confusing. struct i2c_adapter has a timeout field, that's the one being set to 100 in i2c-algo-bit, but i2c-algo-bit uses the i2c_algo_bit_data timeout field. The i2c_adapter timeout field is unused. This is clearly calling for a cleanup but I don't have time for this right now. -- Jean Delvare - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Mon, 12 Mar 2007 16:11:09 +0100 Jean Delvare [EMAIL PROTECTED] wrote: By the way, timeout seems to be hardcoded to 100 jiffies in the i2c-algo-bit driver, so there's probably not much point passing it from the board code when it's going to be overridden anyway. I'll add just a udelay parameter to the platform struct for now. No, it's not hardcoded. I know it looks confusing. struct i2c_adapter has a timeout field, that's the one being set to 100 in i2c-algo-bit, but i2c-algo-bit uses the i2c_algo_bit_data timeout field. The i2c_adapter timeout field is unused. Ah, I see. Now that you mention it, I seem to recall I came to that conclusion last time I looked at the code, but I've apparently forgotten it since then ;) I'll add a timeout field to the platform struct as well, then. Haavard - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Sat, 2007-03-10 at 14:13 +0100, 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.
>
Sorry for missing this hot discussion. Your idea is exactly what I want.
So many arch specific GPIO based I2C adapter implementation will benefit
from this.
> To use, include something similar to the following in the
> board-specific setup code:
>
> #include
>
> static struct i2c_gpio_platform_data i2c_gpio_data = {
> .sda_pin= GPIO_PIN_FOO,
> .scl_pin= GPIO_PIN_BAR,
> };
Is this usage right, because 3 flags are added to this structure as
below:
struct i2c_gpio_platform_data {
unsigned int sda_pin;
unsigned int scl_pin;
unsigned int sda_is_open_drain:1;
unsigned int scl_is_open_drain:1;
unsigned int scl_is_output_only:1;
};
> static struct platform_device i2c_gpio_device = {
> .name = "i2c-gpio",
> .id = 0,
> .dev= {
> .platform_data = _gpio_data,
> },
> };
>
> Register this platform_device, set up the i2c pins as GPIO if
> required and you're ready to go.
>
> Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]>
> ---
> This patch is different from the first patch in the following ways:
> * Handles pins set up as open drain (aka multidrive) by toggling
> the output value instead of the direction
> * Handles output-only SCL pins the same way, and also does not
> install a getscl() callback for such pins
> * Does not add anything to include/linux/i2c-ids.h
> * Sets the output value explicitly after changing the direction to
> output.
> * Plugs a memory leak in remove() -- algo_data wasn't freed.
> * Prints out the pin IDs in decimal, with an extra note when clock
> stretching isn't supported
>
> This version has been compile-tested only. I'll give it a spin when I
> get back to work on monday.
>
> Dave, does this address your concerns?
>
> Haavard
Thanks a lot, I will drop our GPIO based I2C driver and try this one on
our platform.
> + if (!pdata->scl_is_output_only)
> + bit_data->getscl = i2c_gpio_getscl,
> +
> + bit_data->getsda= i2c_gpio_getsda,
> + bit_data->udelay= 5,/* 100 kHz */
> + bit_data->timeout = HZ / 10, /* 100 ms */
Can we add these udelay/timeout to struct i2c_gpio_platform_data? And
let customer to choose these according their specific requirement. We
use Kconfig to do this, but Jean and David don't like the idea, -:(
Regards,
-Bryan Wu
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Sat, 2007-03-10 at 14:13 +0100, 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.
Sorry for missing this hot discussion. Your idea is exactly what I want.
So many arch specific GPIO based I2C adapter implementation will benefit
from this.
To use, include something similar to the following in the
board-specific setup code:
#include linux/i2c-gpio.h
static struct i2c_gpio_platform_data i2c_gpio_data = {
.sda_pin= GPIO_PIN_FOO,
.scl_pin= GPIO_PIN_BAR,
};
Is this usage right, because 3 flags are added to this structure as
below:
struct i2c_gpio_platform_data {
unsigned int sda_pin;
unsigned int scl_pin;
unsigned int sda_is_open_drain:1;
unsigned int scl_is_open_drain:1;
unsigned int scl_is_output_only:1;
};
static struct platform_device i2c_gpio_device = {
.name = i2c-gpio,
.id = 0,
.dev= {
.platform_data = i2c_gpio_data,
},
};
Register this platform_device, set up the i2c pins as GPIO if
required and you're ready to go.
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED]
---
This patch is different from the first patch in the following ways:
* Handles pins set up as open drain (aka multidrive) by toggling
the output value instead of the direction
* Handles output-only SCL pins the same way, and also does not
install a getscl() callback for such pins
* Does not add anything to include/linux/i2c-ids.h
* Sets the output value explicitly after changing the direction to
output.
* Plugs a memory leak in remove() -- algo_data wasn't freed.
* Prints out the pin IDs in decimal, with an extra note when clock
stretching isn't supported
This version has been compile-tested only. I'll give it a spin when I
get back to work on monday.
Dave, does this address your concerns?
Haavard
Thanks a lot, I will drop our GPIO based I2C driver and try this one on
our platform.
+ if (!pdata-scl_is_output_only)
+ bit_data-getscl = i2c_gpio_getscl,
+
+ bit_data-getsda= i2c_gpio_getsda,
+ bit_data-udelay= 5,/* 100 kHz */
+ bit_data-timeout = HZ / 10, /* 100 ms */
Can we add these udelay/timeout to struct i2c_gpio_platform_data? And
let customer to choose these according their specific requirement. We
use Kconfig to do this, but Jean and David don't like the idea, -:(
Regards,
-Bryan Wu
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Saturday 10 March 2007 5:13 am, Haavard Skinnemoen wrote: > This is a very simple bitbanging i2c bus driver utilizing the new > arch-neutral GPIO API. ... > --- > This patch is different from the first patch in the following ways: > * Handles pins set up as open drain (aka multidrive) by toggling > the output value instead of the direction > * Handles output-only SCL pins the same way, and also does not > install a getscl() callback for such pins > * Does not add anything to include/linux/i2c-ids.h > * Sets the output value explicitly after changing the direction to > output. > * Plugs a memory leak in remove() -- algo_data wasn't freed. > * Prints out the pin IDs in decimal, with an extra note when clock > stretching isn't supported > > This version has been compile-tested only. I'll give it a spin when I > get back to work on monday. > > Dave, does this address your concerns? Yes, though see my followup to Jean's note. Unless I make time to test this out on some system, the issues seem to be: (a) will need to change once gpio_direction_output() gains that second argument; (b) i2c-gpio.h could stand one minor comment addition to highlight an assumption. Looking good! - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Saturday 10 March 2007 12:15 pm, Jean Delvare wrote: > Hi Haavard, > > On Sat, 10 Mar 2007 14:13:28 +0100, 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. This updated version looks a lot better. However it doesn't address the API change -- gpio_direction_output(gpio, initial_value) -- which is understandable since that patch hasn't yet merged. > I like the idea very much. Would this let us get rid of i2c-ixp2000? > i2c-ixp4xx? scx200_i2c? Other drivers? There's CONFIG_GENERIC_GPIO support for ixp4xx (nyet upstream, ISTR it's waiting on the gpio_direction_output update), so that one should be particularly easy to replace. Presumably some other bitbang drivers could vanish before long too. > What value will you get if the SDA pin is open-drain and currently in > output mode? For output GPIOs, gpio_get_value() is specified to either return the actual value at the pin ... or zero, if the hardware can't do that. Most GPIO pins *can* do that. (Specifically, that's how AT91 GPIOs work, open drain or otherwise.) (However, there can be various latencies involved. On one chip when I wrote the output value, then immediately read it back, I got the old value. Reason: the GPIO controller clock needed to tick first in order to latch the new input value! It was only about 30 MHz, so the back-to-back instructions were too fast. You can also sometimes notice capacitance causing similar delays. Of course those latencies apply regardless of pin direction.) I think Haavard is assuming the GPIO actually returns that value, since otherwise there'd be no point in trying to use the open drain mode. It'd be worth capturing that in the i2c-gpio.h definition for that struct. > Are such GPIO pins actually able to detect that the pin is > low while they are not themselves driving it low? Given a "yes" to the above, then clearly "yes" here too. As I noted, if it can't actually sense the value at the pin, that function should always return zero. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
Hi Haavard,
On Sat, 10 Mar 2007 14:13:28 +0100, 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.
>
> To use, include something similar to the following in the
> board-specific setup code:
>
> #include
>
> static struct i2c_gpio_platform_data i2c_gpio_data = {
> .sda_pin= GPIO_PIN_FOO,
> .scl_pin= GPIO_PIN_BAR,
> };
> static struct platform_device i2c_gpio_device = {
> .name = "i2c-gpio",
> .id = 0,
> .dev= {
> .platform_data = _gpio_data,
> },
> };
>
> Register this platform_device, set up the i2c pins as GPIO if
> required and you're ready to go.
I like the idea very much. Would this let us get rid of i2c-ixp2000?
i2c-ixp4xx? scx200_i2c? Other drivers?
> drivers/i2c/busses/Kconfig|8 ++
> drivers/i2c/busses/Makefile |1 +
> drivers/i2c/busses/i2c-gpio.c | 211
> +
> include/linux/i2c-gpio.h | 30 ++
> 4 files changed, 250 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index fb19dbb..52f79d1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -102,6 +102,14 @@ config I2C_ELEKTOR
> This support is also available as a module. If so, the module
> will be called i2c-elektor.
>
> +config I2C_GPIO
> + tristate "GPIO-based bitbanging i2c driver"
> + depends on I2C && GENERIC_GPIO
> + select I2C_ALGOBIT
> + help
> + This is a very simple bitbanging i2c driver utilizing the
> + arch-neutral GPIO API to control the SCL and SDA lines.
> +
> config I2C_HYDRA
> tristate "CHRP Apple Hydra Mac I/O I2C interface"
> depends on I2C && PCI && PPC_CHRP && EXPERIMENTAL
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 290b540..68f2b05 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_AMD8111) += i2c-amd8111.o
> obj-$(CONFIG_I2C_AT91) += i2c-at91.o
> obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
> obj-$(CONFIG_I2C_ELEKTOR)+= i2c-elektor.o
> +obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o
> obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
> obj-$(CONFIG_I2C_I801) += i2c-i801.o
> obj-$(CONFIG_I2C_I810) += i2c-i810.o
> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> new file mode 100644
> index 000..423db0a
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-gpio.c
> @@ -0,0 +1,211 @@
> +/*
> + * Bitbanging i2c bus driver using the GPIO API
> + *
> + * Copyright (C) 2006 Atmel Corporation
I'm told we're in year 2007 ;)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +/* Toggle SDA by changing the direction of the pin */
> +static void i2c_gpio_setsda_dir(void *data, int state)
> +{
> + struct i2c_gpio_platform_data *pdata = data;
> +
> + if (state)
> + gpio_direction_input(pdata->sda_pin);
> + else {
> + gpio_direction_output(pdata->sda_pin);
> + gpio_set_value(pdata->sda_pin, 0);
> + }
> +}
> +
> +/*
> + * Toggle SDA by changing the output value of the pin. This is only
> + * valid for pins configured as open drain (i.e. setting the value
> + * high effectively turns off the output driver.)
> + */
> +static void i2c_gpio_setsda_val(void *data, int state)
> +{
> + struct i2c_gpio_platform_data *pdata = data;
> +
> + gpio_set_value(pdata->sda_pin, state);
> +}
> +
> +/* Toggle SCL by changing the direction of the pin. */
> +static void i2c_gpio_setscl_dir(void *data, int state)
> +{
> + struct i2c_gpio_platform_data *pdata = data;
> +
> + if (state)
> + gpio_direction_input(pdata->scl_pin);
> + else {
> + gpio_direction_output(pdata->scl_pin);
> + gpio_set_value(pdata->scl_pin, 0);
> + }
> +}
> +
> +/*
> + * Toggle SCL by changing the output value of the pin. This is used
> + * for pins that are configured as open drain and for output-only
> + * pins. The latter case will break the i2c protocol, but it will
> + * often work in practice.
> + */
> +static void i2c_gpio_setscl_val(void *data, int state)
> +{
> + struct i2c_gpio_platform_data *pdata = data;
> +
> + gpio_set_value(pdata->scl_pin, state);
> +}
> +
> +int i2c_gpio_getsda(void *data)
> +{
> + struct i2c_gpio_platform_data *pdata = data;
> +
> + return gpio_get_value(pdata->sda_pin);
> +}
What value
[PATCH v2] Bitbanging i2c bus driver using the GPIO API
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.
To use, include something similar to the following in the
board-specific setup code:
#include
static struct i2c_gpio_platform_data i2c_gpio_data = {
.sda_pin= GPIO_PIN_FOO,
.scl_pin= GPIO_PIN_BAR,
};
static struct platform_device i2c_gpio_device = {
.name = "i2c-gpio",
.id = 0,
.dev= {
.platform_data = _gpio_data,
},
};
Register this platform_device, set up the i2c pins as GPIO if
required and you're ready to go.
Signed-off-by: Haavard Skinnemoen <[EMAIL PROTECTED]>
---
This patch is different from the first patch in the following ways:
* Handles pins set up as open drain (aka multidrive) by toggling
the output value instead of the direction
* Handles output-only SCL pins the same way, and also does not
install a getscl() callback for such pins
* Does not add anything to include/linux/i2c-ids.h
* Sets the output value explicitly after changing the direction to
output.
* Plugs a memory leak in remove() -- algo_data wasn't freed.
* Prints out the pin IDs in decimal, with an extra note when clock
stretching isn't supported
This version has been compile-tested only. I'll give it a spin when I
get back to work on monday.
Dave, does this address your concerns?
Haavard
drivers/i2c/busses/Kconfig|8 ++
drivers/i2c/busses/Makefile |1 +
drivers/i2c/busses/i2c-gpio.c | 211 +
include/linux/i2c-gpio.h | 30 ++
4 files changed, 250 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index fb19dbb..52f79d1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -102,6 +102,14 @@ config I2C_ELEKTOR
This support is also available as a module. If so, the module
will be called i2c-elektor.
+config I2C_GPIO
+ tristate "GPIO-based bitbanging i2c driver"
+ depends on I2C && GENERIC_GPIO
+ select I2C_ALGOBIT
+ help
+ This is a very simple bitbanging i2c driver utilizing the
+ arch-neutral GPIO API to control the SCL and SDA lines.
+
config I2C_HYDRA
tristate "CHRP Apple Hydra Mac I/O I2C interface"
depends on I2C && PCI && PPC_CHRP && EXPERIMENTAL
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 290b540..68f2b05 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_AMD8111) += i2c-amd8111.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
+obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o
obj-$(CONFIG_I2C_HYDRA)+= i2c-hydra.o
obj-$(CONFIG_I2C_I801) += i2c-i801.o
obj-$(CONFIG_I2C_I810) += i2c-i810.o
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
new file mode 100644
index 000..423db0a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -0,0 +1,211 @@
+/*
+ * Bitbanging i2c bus driver using the GPIO API
+ *
+ * Copyright (C) 2006 Atmel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+/* Toggle SDA by changing the direction of the pin */
+static void i2c_gpio_setsda_dir(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ if (state)
+ gpio_direction_input(pdata->sda_pin);
+ else {
+ gpio_direction_output(pdata->sda_pin);
+ gpio_set_value(pdata->sda_pin, 0);
+ }
+}
+
+/*
+ * Toggle SDA by changing the output value of the pin. This is only
+ * valid for pins configured as open drain (i.e. setting the value
+ * high effectively turns off the output driver.)
+ */
+static void i2c_gpio_setsda_val(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ gpio_set_value(pdata->sda_pin, state);
+}
+
+/* Toggle SCL by changing the direction of the pin. */
+static void i2c_gpio_setscl_dir(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ if (state)
+ gpio_direction_input(pdata->scl_pin);
+ else {
+ gpio_direction_output(pdata->scl_pin);
+ gpio_set_value(pdata->scl_pin, 0);
+ }
+}
+
+/*
+ * Toggle SCL by changing the output value of the pin. This is used
+ * for pins that are configured as open drain and for output-only
+ * pins. The latter case will break the i2c protocol, but it will
[PATCH v2] Bitbanging i2c bus driver using the GPIO API
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.
To use, include something similar to the following in the
board-specific setup code:
#include linux/i2c-gpio.h
static struct i2c_gpio_platform_data i2c_gpio_data = {
.sda_pin= GPIO_PIN_FOO,
.scl_pin= GPIO_PIN_BAR,
};
static struct platform_device i2c_gpio_device = {
.name = i2c-gpio,
.id = 0,
.dev= {
.platform_data = i2c_gpio_data,
},
};
Register this platform_device, set up the i2c pins as GPIO if
required and you're ready to go.
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED]
---
This patch is different from the first patch in the following ways:
* Handles pins set up as open drain (aka multidrive) by toggling
the output value instead of the direction
* Handles output-only SCL pins the same way, and also does not
install a getscl() callback for such pins
* Does not add anything to include/linux/i2c-ids.h
* Sets the output value explicitly after changing the direction to
output.
* Plugs a memory leak in remove() -- algo_data wasn't freed.
* Prints out the pin IDs in decimal, with an extra note when clock
stretching isn't supported
This version has been compile-tested only. I'll give it a spin when I
get back to work on monday.
Dave, does this address your concerns?
Haavard
drivers/i2c/busses/Kconfig|8 ++
drivers/i2c/busses/Makefile |1 +
drivers/i2c/busses/i2c-gpio.c | 211 +
include/linux/i2c-gpio.h | 30 ++
4 files changed, 250 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index fb19dbb..52f79d1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -102,6 +102,14 @@ config I2C_ELEKTOR
This support is also available as a module. If so, the module
will be called i2c-elektor.
+config I2C_GPIO
+ tristate GPIO-based bitbanging i2c driver
+ depends on I2C GENERIC_GPIO
+ select I2C_ALGOBIT
+ help
+ This is a very simple bitbanging i2c driver utilizing the
+ arch-neutral GPIO API to control the SCL and SDA lines.
+
config I2C_HYDRA
tristate CHRP Apple Hydra Mac I/O I2C interface
depends on I2C PCI PPC_CHRP EXPERIMENTAL
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 290b540..68f2b05 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_AMD8111) += i2c-amd8111.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
+obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o
obj-$(CONFIG_I2C_HYDRA)+= i2c-hydra.o
obj-$(CONFIG_I2C_I801) += i2c-i801.o
obj-$(CONFIG_I2C_I810) += i2c-i810.o
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
new file mode 100644
index 000..423db0a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -0,0 +1,211 @@
+/*
+ * Bitbanging i2c bus driver using the GPIO API
+ *
+ * Copyright (C) 2006 Atmel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include linux/i2c.h
+#include linux/i2c-algo-bit.h
+#include linux/i2c-gpio.h
+#include linux/init.h
+#include linux/module.h
+#include linux/platform_device.h
+
+#include asm/gpio.h
+
+/* Toggle SDA by changing the direction of the pin */
+static void i2c_gpio_setsda_dir(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ if (state)
+ gpio_direction_input(pdata-sda_pin);
+ else {
+ gpio_direction_output(pdata-sda_pin);
+ gpio_set_value(pdata-sda_pin, 0);
+ }
+}
+
+/*
+ * Toggle SDA by changing the output value of the pin. This is only
+ * valid for pins configured as open drain (i.e. setting the value
+ * high effectively turns off the output driver.)
+ */
+static void i2c_gpio_setsda_val(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ gpio_set_value(pdata-sda_pin, state);
+}
+
+/* Toggle SCL by changing the direction of the pin. */
+static void i2c_gpio_setscl_dir(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ if (state)
+ gpio_direction_input(pdata-scl_pin);
+ else {
+ gpio_direction_output(pdata-scl_pin);
+ gpio_set_value(pdata-scl_pin, 0);
+ }
+}
+
+/*
+ * Toggle SCL by changing the output value of the pin. This is used
+ * for pins that are configured
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
Hi Haavard,
On Sat, 10 Mar 2007 14:13:28 +0100, 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.
To use, include something similar to the following in the
board-specific setup code:
#include linux/i2c-gpio.h
static struct i2c_gpio_platform_data i2c_gpio_data = {
.sda_pin= GPIO_PIN_FOO,
.scl_pin= GPIO_PIN_BAR,
};
static struct platform_device i2c_gpio_device = {
.name = i2c-gpio,
.id = 0,
.dev= {
.platform_data = i2c_gpio_data,
},
};
Register this platform_device, set up the i2c pins as GPIO if
required and you're ready to go.
I like the idea very much. Would this let us get rid of i2c-ixp2000?
i2c-ixp4xx? scx200_i2c? Other drivers?
drivers/i2c/busses/Kconfig|8 ++
drivers/i2c/busses/Makefile |1 +
drivers/i2c/busses/i2c-gpio.c | 211
+
include/linux/i2c-gpio.h | 30 ++
4 files changed, 250 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index fb19dbb..52f79d1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -102,6 +102,14 @@ config I2C_ELEKTOR
This support is also available as a module. If so, the module
will be called i2c-elektor.
+config I2C_GPIO
+ tristate GPIO-based bitbanging i2c driver
+ depends on I2C GENERIC_GPIO
+ select I2C_ALGOBIT
+ help
+ This is a very simple bitbanging i2c driver utilizing the
+ arch-neutral GPIO API to control the SCL and SDA lines.
+
config I2C_HYDRA
tristate CHRP Apple Hydra Mac I/O I2C interface
depends on I2C PCI PPC_CHRP EXPERIMENTAL
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 290b540..68f2b05 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_AMD8111) += i2c-amd8111.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_ELEKTOR)+= i2c-elektor.o
+obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o
obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
obj-$(CONFIG_I2C_I801) += i2c-i801.o
obj-$(CONFIG_I2C_I810) += i2c-i810.o
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
new file mode 100644
index 000..423db0a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -0,0 +1,211 @@
+/*
+ * Bitbanging i2c bus driver using the GPIO API
+ *
+ * Copyright (C) 2006 Atmel Corporation
I'm told we're in year 2007 ;)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include linux/i2c.h
+#include linux/i2c-algo-bit.h
+#include linux/i2c-gpio.h
+#include linux/init.h
+#include linux/module.h
+#include linux/platform_device.h
+
+#include asm/gpio.h
+
+/* Toggle SDA by changing the direction of the pin */
+static void i2c_gpio_setsda_dir(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ if (state)
+ gpio_direction_input(pdata-sda_pin);
+ else {
+ gpio_direction_output(pdata-sda_pin);
+ gpio_set_value(pdata-sda_pin, 0);
+ }
+}
+
+/*
+ * Toggle SDA by changing the output value of the pin. This is only
+ * valid for pins configured as open drain (i.e. setting the value
+ * high effectively turns off the output driver.)
+ */
+static void i2c_gpio_setsda_val(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ gpio_set_value(pdata-sda_pin, state);
+}
+
+/* Toggle SCL by changing the direction of the pin. */
+static void i2c_gpio_setscl_dir(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ if (state)
+ gpio_direction_input(pdata-scl_pin);
+ else {
+ gpio_direction_output(pdata-scl_pin);
+ gpio_set_value(pdata-scl_pin, 0);
+ }
+}
+
+/*
+ * Toggle SCL by changing the output value of the pin. This is used
+ * for pins that are configured as open drain and for output-only
+ * pins. The latter case will break the i2c protocol, but it will
+ * often work in practice.
+ */
+static void i2c_gpio_setscl_val(void *data, int state)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ gpio_set_value(pdata-scl_pin, state);
+}
+
+int i2c_gpio_getsda(void *data)
+{
+ struct i2c_gpio_platform_data *pdata = data;
+
+ return gpio_get_value(pdata-sda_pin);
+}
What value will you get if the SDA pin is open-drain
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Saturday 10 March 2007 5:13 am, Haavard Skinnemoen wrote: This is a very simple bitbanging i2c bus driver utilizing the new arch-neutral GPIO API. ... --- This patch is different from the first patch in the following ways: * Handles pins set up as open drain (aka multidrive) by toggling the output value instead of the direction * Handles output-only SCL pins the same way, and also does not install a getscl() callback for such pins * Does not add anything to include/linux/i2c-ids.h * Sets the output value explicitly after changing the direction to output. * Plugs a memory leak in remove() -- algo_data wasn't freed. * Prints out the pin IDs in decimal, with an extra note when clock stretching isn't supported This version has been compile-tested only. I'll give it a spin when I get back to work on monday. Dave, does this address your concerns? Yes, though see my followup to Jean's note. Unless I make time to test this out on some system, the issues seem to be: (a) will need to change once gpio_direction_output() gains that second argument; (b) i2c-gpio.h could stand one minor comment addition to highlight an assumption. Looking good! - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
On Saturday 10 March 2007 12:15 pm, Jean Delvare wrote: Hi Haavard, On Sat, 10 Mar 2007 14:13:28 +0100, 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. This updated version looks a lot better. However it doesn't address the API change -- gpio_direction_output(gpio, initial_value) -- which is understandable since that patch hasn't yet merged. I like the idea very much. Would this let us get rid of i2c-ixp2000? i2c-ixp4xx? scx200_i2c? Other drivers? There's CONFIG_GENERIC_GPIO support for ixp4xx (nyet upstream, ISTR it's waiting on the gpio_direction_output update), so that one should be particularly easy to replace. Presumably some other bitbang drivers could vanish before long too. What value will you get if the SDA pin is open-drain and currently in output mode? For output GPIOs, gpio_get_value() is specified to either return the actual value at the pin ... or zero, if the hardware can't do that. Most GPIO pins *can* do that. (Specifically, that's how AT91 GPIOs work, open drain or otherwise.) (However, there can be various latencies involved. On one chip when I wrote the output value, then immediately read it back, I got the old value. Reason: the GPIO controller clock needed to tick first in order to latch the new input value! It was only about 30 MHz, so the back-to-back instructions were too fast. You can also sometimes notice capacitance causing similar delays. Of course those latencies apply regardless of pin direction.) I think Haavard is assuming the GPIO actually returns that value, since otherwise there'd be no point in trying to use the open drain mode. It'd be worth capturing that in the i2c-gpio.h definition for that struct. Are such GPIO pins actually able to detect that the pin is low while they are not themselves driving it low? Given a yes to the above, then clearly yes here too. As I noted, if it can't actually sense the value at the pin, that function should always return zero. - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
