All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Wu, Bryan" <bryan.wu@analog.com>
To: Haavard Skinnemoen <hskinnemoen@atmel.com>
Cc: David Brownell <david-b@pacbell.net>,
	Jean Delvare <khali@linux-fr.org>, Bryan Wu <bryan.wu@analog.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Deepak Saxena <dsaxena@plexity.net>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API
Date: Mon, 12 Mar 2007 18:07:59 +0800	[thread overview]
Message-ID: <1173694079.6638.13.camel@roc-desktop> (raw)
In-Reply-To: <11735324081261-git-send-email-hskinnemoen@atmel.com>

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 <hskinnemoen@atmel.com>
> ---
> 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

  parent reply	other threads:[~2007-03-12  2:10 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-09 10:13 [PATCH] i2c-core: i2c bitbang gpio structure Wu, Bryan
2007-03-09 16:55 ` Jean Delvare
2007-03-09 17:45   ` David Brownell
2007-03-09 18:48     ` [PATCH] Bitbanging i2c bus driver using the GPIO API Haavard Skinnemoen
2007-03-09 19:30       ` David Brownell
2007-03-09 20:08         ` Russell King
2007-03-09 21:17           ` David Brownell
2007-03-09 20:43         ` Håvard Skinnemoen
2007-03-09 21:45           ` David Brownell
2007-03-10 13:13             ` [PATCH v2] " Haavard Skinnemoen
2007-03-10 20:15               ` Jean Delvare
2007-03-11  4:31                 ` David Brownell
2007-03-12 14:11                 ` Haavard Skinnemoen
2007-03-11  4:02               ` David Brownell
2007-03-12 10:07               ` Wu, Bryan [this message]
2007-03-12 14:34                 ` Haavard Skinnemoen
2007-03-12 14:53                   ` Haavard Skinnemoen
2007-03-12 15:11                     ` Jean Delvare
2007-03-12 15:30                       ` Haavard Skinnemoen
2007-03-10 19:15         ` [PATCH] " Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1173694079.6638.13.camel@roc-desktop \
    --to=bryan.wu@analog.com \
    --cc=akpm@linux-foundation.org \
    --cc=david-b@pacbell.net \
    --cc=dsaxena@plexity.net \
    --cc=hskinnemoen@atmel.com \
    --cc=khali@linux-fr.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.