linux-kernel - Re: [PATCH v2] Bitbanging i2c bus driver using the GPIO API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1173694079.6638.13.camel@roc-desktop>
Date:	Mon, 12 Mar 2007 18:07:59 +0800
From:	"Wu, Bryan" <bryan.wu@...log.com>
To:	Haavard Skinnemoen <hskinnemoen@...el.com>
Cc:	David Brownell <david-b@...bell.net>,
	Jean Delvare <khali@...ux-fr.org>,
	Bryan Wu <bryan.wu@...log.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Deepak Saxena <dsaxena@...xity.net>,
	linux-kernel@...r.kernel.org
Subject: 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 <hskinnemoen@...el.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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/