Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-20 Thread Wolfram Sang
On Wed, Sep 19, 2012 at 11:08:13PM +0300, Aaro Koskinen wrote:
> On Fri, Sep 14, 2012 at 12:08:06PM +0200, Wolfram Sang wrote:
> > On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
> > > Add i2c driver to enable access to devices behind CBUS on Nokia Internet
> > > Tablets.
> > > 
> > > The patch also adds CBUS I2C configuration for N8x0 which is one of the
> > > users of this driver.
> > > 
> > > Cc: linux-...@vger.kernel.org
> > > Acked-by: Felipe Balbi 
> > > Acked-by: Tony Lindgren 
> > > Signed-off-by: Aaro Koskinen 
> > 
> > OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
> > be an appropriate place. Still, before deciding if it should rather be
> > in the core directory, I still have a few questions.
> 
> Thanks for your feedback. I will fix up the incorrect comments, and do
> other improvements (e.g. use devm_* stuff) in the next version.
> 
> The questions about delays and bit counts are very valid, but it's
> difficult to do anything with them due to lack of documentation and HW on
> which to test - I have Nokia tablets to test the driver, and the driver
> is very reliable, but I do not know of any other HW with CBUS. :-/

Fine enough, then please add comments saying that.

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-20 Thread Wolfram Sang
On Wed, Sep 19, 2012 at 11:08:13PM +0300, Aaro Koskinen wrote:
 On Fri, Sep 14, 2012 at 12:08:06PM +0200, Wolfram Sang wrote:
  On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
   Add i2c driver to enable access to devices behind CBUS on Nokia Internet
   Tablets.
   
   The patch also adds CBUS I2C configuration for N8x0 which is one of the
   users of this driver.
   
   Cc: linux-...@vger.kernel.org
   Acked-by: Felipe Balbi ba...@ti.com
   Acked-by: Tony Lindgren t...@atomide.com
   Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi
  
  OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
  be an appropriate place. Still, before deciding if it should rather be
  in the core directory, I still have a few questions.
 
 Thanks for your feedback. I will fix up the incorrect comments, and do
 other improvements (e.g. use devm_* stuff) in the next version.
 
 The questions about delays and bit counts are very valid, but it's
 difficult to do anything with them due to lack of documentation and HW on
 which to test - I have Nokia tablets to test the driver, and the driver
 is very reliable, but I do not know of any other HW with CBUS. :-/

Fine enough, then please add comments saying that.

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-19 Thread Aaro Koskinen
On Fri, Sep 14, 2012 at 12:08:06PM +0200, Wolfram Sang wrote:
> On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
> > Add i2c driver to enable access to devices behind CBUS on Nokia Internet
> > Tablets.
> > 
> > The patch also adds CBUS I2C configuration for N8x0 which is one of the
> > users of this driver.
> > 
> > Cc: linux-...@vger.kernel.org
> > Acked-by: Felipe Balbi 
> > Acked-by: Tony Lindgren 
> > Signed-off-by: Aaro Koskinen 
> 
> OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
> be an appropriate place. Still, before deciding if it should rather be
> in the core directory, I still have a few questions.

Thanks for your feedback. I will fix up the incorrect comments, and do
other improvements (e.g. use devm_* stuff) in the next version.

The questions about delays and bit counts are very valid, but it's
difficult to do anything with them due to lack of documentation and HW on
which to test - I have Nokia tablets to test the driver, and the driver
is very reliable, but I do not know of any other HW with CBUS. :-/

A.

> Also, does anybody know of a generic bit-banging implementation in the
> kernel which could be used here?
> 
> Jean: I'd appreciate your general opinion here, too.
> 
> > diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
> > new file mode 100644
> > index 000..bacf2a9
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-cbus.c
> > @@ -0,0 +1,369 @@
> > +/*
> > + * CBUS I2C driver for Nokia Internet Tablets.
> > + *
> > + * Copyright (C) 2004-2010 Nokia Corporation
> > + *
> > + * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
> > + * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
> > + *
> > + * This file is subject to the terms and conditions of the GNU General
> > + * Public License. See the file "COPYING" in the main directory of this
> > + * archive for more details.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +struct cbus_host {
> > +   /* host lock */
> > +   spinlock_t  lock;
> > +
> > +   struct device   *dev;
> > +
> > +   int clk_gpio;
> > +   int dat_gpio;
> > +   int sel_gpio;
> > +};
> > +
> > +/**
> > + * cbus_send_bit - sends one bit over the bus
> > + * @host: the host we're using
> > + * @bit: one bit of information to send
> > + * @input: whether to set data pin as input after sending
> > + */
> > +static int cbus_send_bit(struct cbus_host *host, unsigned bit,
> > +   unsigned input)
> > +{
> > +   int ret = 0;
> > +
> > +   gpio_set_value(host->dat_gpio, bit ? 1 : 0);
> > +   gpio_set_value(host->clk_gpio, 1);
> > +
> > +   /* The data bit is read on the rising edge of CLK */
> 
> This comment doesn't belong to the if-block, or?
> 
> > +   if (input)
> > +   ret = gpio_direction_input(host->dat_gpio);
> 
> No minimum delay for the signal to be on the bus?
> 
> > +
> > +   gpio_set_value(host->clk_gpio, 0);
> > +
> > +   return ret;
> > +}
> > +
> > +/**
> > + * cbus_send_data - sends @len amount of data over the bus
> > + * @host: the host we're using
> > + * @data: the data to send
> > + * @len: size of the transfer
> > + * @input: whether to set data pin as input after sending
> > + */
> > +static int cbus_send_data(struct cbus_host *host, unsigned data, unsigned 
> > len,
> > +   unsigned input)
> > +{
> > +   int ret = 0;
> > +   int i;
> > +
> > +   for (i = len; i > 0; i--) {
> > +   ret = cbus_send_bit(host, data & (1 << (i - 1)),
> > +   input && (i == 1));
> > +   if (ret < 0)
> > +   goto out;
> > +   }
> > +
> > +out:
> > +   return ret;
> > +}
> > +
> > +/**
> > + * cbus_receive_bit - receives one bit from the bus
> > + * @host: the host we're using
> > + */
> > +static int cbus_receive_bit(struct cbus_host *host)
> > +{
> > +   int ret;
> > +
> > +   gpio_set_value(host->clk_gpio, 1);
> 
> No delays to ensure the signal has stabilized?
> 
> > +   ret = gpio_get_value(host->dat_gpio);
> > +   if (ret < 0)
> > +   goto out;
> > +   gpio_set_value(host->clk_gpio, 0);
> > +
> > +out:
> > +   return ret;
> > +}
> > +
> > +/**
> > + * cbus_receive_word - receives 16-bit word from the bus
> > + * @host: the host we're using
> > + */
> > +static int cbus_receive_word(struct cbus_host *host)
> > +{
> > +   int ret = 0;
> > +   int i;
> > +
> > +   for (i = 16; i > 0; i--) {
> > +   int bit = cbus_receive_bit(host);
> > +
> > +   if (bit < 0)
> > +   goto out;
> > 

Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-19 Thread Aaro Koskinen
On Fri, Sep 14, 2012 at 12:08:06PM +0200, Wolfram Sang wrote:
 On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
  Add i2c driver to enable access to devices behind CBUS on Nokia Internet
  Tablets.
  
  The patch also adds CBUS I2C configuration for N8x0 which is one of the
  users of this driver.
  
  Cc: linux-...@vger.kernel.org
  Acked-by: Felipe Balbi ba...@ti.com
  Acked-by: Tony Lindgren t...@atomide.com
  Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi
 
 OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
 be an appropriate place. Still, before deciding if it should rather be
 in the core directory, I still have a few questions.

Thanks for your feedback. I will fix up the incorrect comments, and do
other improvements (e.g. use devm_* stuff) in the next version.

The questions about delays and bit counts are very valid, but it's
difficult to do anything with them due to lack of documentation and HW on
which to test - I have Nokia tablets to test the driver, and the driver
is very reliable, but I do not know of any other HW with CBUS. :-/

A.

 Also, does anybody know of a generic bit-banging implementation in the
 kernel which could be used here?
 
 Jean: I'd appreciate your general opinion here, too.
 
  diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
  new file mode 100644
  index 000..bacf2a9
  --- /dev/null
  +++ b/drivers/i2c/busses/i2c-cbus.c
  @@ -0,0 +1,369 @@
  +/*
  + * CBUS I2C driver for Nokia Internet Tablets.
  + *
  + * Copyright (C) 2004-2010 Nokia Corporation
  + *
  + * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
  + * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
  + *
  + * This file is subject to the terms and conditions of the GNU General
  + * Public License. See the file COPYING in the main directory of this
  + * archive for more details.
  + *
  + * This program is distributed in the hope that it will be useful,
  + * but WITHOUT ANY WARRANTY; without even the implied warranty of
  + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  + * GNU General Public License for more details.
  + */
  +
  +#include linux/io.h
  +#include linux/i2c.h
  +#include linux/gpio.h
  +#include linux/init.h
  +#include linux/slab.h
  +#include linux/delay.h
  +#include linux/errno.h
  +#include linux/kernel.h
  +#include linux/module.h
  +#include linux/of_gpio.h
  +#include linux/i2c-cbus.h
  +#include linux/interrupt.h
  +#include linux/platform_device.h
  +
  +struct cbus_host {
  +   /* host lock */
  +   spinlock_t  lock;
  +
  +   struct device   *dev;
  +
  +   int clk_gpio;
  +   int dat_gpio;
  +   int sel_gpio;
  +};
  +
  +/**
  + * cbus_send_bit - sends one bit over the bus
  + * @host: the host we're using
  + * @bit: one bit of information to send
  + * @input: whether to set data pin as input after sending
  + */
  +static int cbus_send_bit(struct cbus_host *host, unsigned bit,
  +   unsigned input)
  +{
  +   int ret = 0;
  +
  +   gpio_set_value(host-dat_gpio, bit ? 1 : 0);
  +   gpio_set_value(host-clk_gpio, 1);
  +
  +   /* The data bit is read on the rising edge of CLK */
 
 This comment doesn't belong to the if-block, or?
 
  +   if (input)
  +   ret = gpio_direction_input(host-dat_gpio);
 
 No minimum delay for the signal to be on the bus?
 
  +
  +   gpio_set_value(host-clk_gpio, 0);
  +
  +   return ret;
  +}
  +
  +/**
  + * cbus_send_data - sends @len amount of data over the bus
  + * @host: the host we're using
  + * @data: the data to send
  + * @len: size of the transfer
  + * @input: whether to set data pin as input after sending
  + */
  +static int cbus_send_data(struct cbus_host *host, unsigned data, unsigned 
  len,
  +   unsigned input)
  +{
  +   int ret = 0;
  +   int i;
  +
  +   for (i = len; i  0; i--) {
  +   ret = cbus_send_bit(host, data  (1  (i - 1)),
  +   input  (i == 1));
  +   if (ret  0)
  +   goto out;
  +   }
  +
  +out:
  +   return ret;
  +}
  +
  +/**
  + * cbus_receive_bit - receives one bit from the bus
  + * @host: the host we're using
  + */
  +static int cbus_receive_bit(struct cbus_host *host)
  +{
  +   int ret;
  +
  +   gpio_set_value(host-clk_gpio, 1);
 
 No delays to ensure the signal has stabilized?
 
  +   ret = gpio_get_value(host-dat_gpio);
  +   if (ret  0)
  +   goto out;
  +   gpio_set_value(host-clk_gpio, 0);
  +
  +out:
  +   return ret;
  +}
  +
  +/**
  + * cbus_receive_word - receives 16-bit word from the bus
  + * @host: the host we're using
  + */
  +static int cbus_receive_word(struct cbus_host *host)
  +{
  +   int ret = 0;
  +   int i;
  +
  +   for (i = 16; i  0; i--) {
  +   int bit = cbus_receive_bit(host);
  +
  +   if (bit  0)
  +   goto out;
  +
  +   if (bit)
  +   ret |= 1  (i - 1);
  +   }
  +
  +out:
  

Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-14 Thread Felipe Balbi
On Fri, Sep 14, 2012 at 12:08:06PM +0200, Wolfram Sang wrote:
> On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
> > Add i2c driver to enable access to devices behind CBUS on Nokia Internet
> > Tablets.
> > 
> > The patch also adds CBUS I2C configuration for N8x0 which is one of the
> > users of this driver.
> > 
> > Cc: linux-...@vger.kernel.org
> > Acked-by: Felipe Balbi 
> > Acked-by: Tony Lindgren 
> > Signed-off-by: Aaro Koskinen 
> 
> OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
> be an appropriate place. Still, before deciding if it should rather be
> in the core directory, I still have a few questions.
> 
> Also, does anybody know of a generic bit-banging implementation in the
> kernel which could be used here?

there is i2c-gpio, but it wouldn't help much for cbus, unless we change
it quite a lot. The generic i2c_algo_bit_data would have to be changed
to learn about a 3 wire bus. I guess the impact is much bigger than just
accepting this small, self-contained driver.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-14 Thread Jean Delvare
On Fri, 14 Sep 2012 12:08:06 +0200, Wolfram Sang wrote:
> OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
> be an appropriate place. Still, before deciding if it should rather be
> in the core directory, I still have a few questions.
> 
> Also, does anybody know of a generic bit-banging implementation in the
> kernel which could be used here?
> 
> Jean: I'd appreciate your general opinion here, too.

Out of time at the moment :(

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-14 Thread Wolfram Sang
On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
> Add i2c driver to enable access to devices behind CBUS on Nokia Internet
> Tablets.
> 
> The patch also adds CBUS I2C configuration for N8x0 which is one of the
> users of this driver.
> 
> Cc: linux-...@vger.kernel.org
> Acked-by: Felipe Balbi 
> Acked-by: Tony Lindgren 
> Signed-off-by: Aaro Koskinen 

OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
be an appropriate place. Still, before deciding if it should rather be
in the core directory, I still have a few questions.

Also, does anybody know of a generic bit-banging implementation in the
kernel which could be used here?

Jean: I'd appreciate your general opinion here, too.

> diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
> new file mode 100644
> index 000..bacf2a9
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-cbus.c
> @@ -0,0 +1,369 @@
> +/*
> + * CBUS I2C driver for Nokia Internet Tablets.
> + *
> + * Copyright (C) 2004-2010 Nokia Corporation
> + *
> + * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
> + * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
> + *
> + * This file is subject to the terms and conditions of the GNU General
> + * Public License. See the file "COPYING" in the main directory of this
> + * archive for more details.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct cbus_host {
> + /* host lock */
> + spinlock_t  lock;
> +
> + struct device   *dev;
> +
> + int clk_gpio;
> + int dat_gpio;
> + int sel_gpio;
> +};
> +
> +/**
> + * cbus_send_bit - sends one bit over the bus
> + * @host: the host we're using
> + * @bit: one bit of information to send
> + * @input: whether to set data pin as input after sending
> + */
> +static int cbus_send_bit(struct cbus_host *host, unsigned bit,
> + unsigned input)
> +{
> + int ret = 0;
> +
> + gpio_set_value(host->dat_gpio, bit ? 1 : 0);
> + gpio_set_value(host->clk_gpio, 1);
> +
> + /* The data bit is read on the rising edge of CLK */

This comment doesn't belong to the if-block, or?

> + if (input)
> + ret = gpio_direction_input(host->dat_gpio);

No minimum delay for the signal to be on the bus?

> +
> + gpio_set_value(host->clk_gpio, 0);
> +
> + return ret;
> +}
> +
> +/**
> + * cbus_send_data - sends @len amount of data over the bus
> + * @host: the host we're using
> + * @data: the data to send
> + * @len: size of the transfer
> + * @input: whether to set data pin as input after sending
> + */
> +static int cbus_send_data(struct cbus_host *host, unsigned data, unsigned 
> len,
> + unsigned input)
> +{
> + int ret = 0;
> + int i;
> +
> + for (i = len; i > 0; i--) {
> + ret = cbus_send_bit(host, data & (1 << (i - 1)),
> + input && (i == 1));
> + if (ret < 0)
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * cbus_receive_bit - receives one bit from the bus
> + * @host: the host we're using
> + */
> +static int cbus_receive_bit(struct cbus_host *host)
> +{
> + int ret;
> +
> + gpio_set_value(host->clk_gpio, 1);

No delays to ensure the signal has stabilized?

> + ret = gpio_get_value(host->dat_gpio);
> + if (ret < 0)
> + goto out;
> + gpio_set_value(host->clk_gpio, 0);
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * cbus_receive_word - receives 16-bit word from the bus
> + * @host: the host we're using
> + */
> +static int cbus_receive_word(struct cbus_host *host)
> +{
> + int ret = 0;
> + int i;
> +
> + for (i = 16; i > 0; i--) {
> + int bit = cbus_receive_bit(host);
> +
> + if (bit < 0)
> + goto out;
> +
> + if (bit)
> + ret |= 1 << (i - 1);
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * cbus_transfer - transfers data over the bus
> + * @host: the host we're using
> + * @rw: read/write flag
> + * @dev: device address
> + * @reg: register address
> + * @data: if @rw == I2C_SBUS_WRITE data to send otherwise 0
> + */
> +static int cbus_transfer(struct cbus_host *host, char rw, unsigned dev,
> +  unsigned reg, unsigned data)
> +{
> + unsigned long flags;
> + int ret;
> +
> + /* We don't want interrupts disturbing our transfer */
> + spin_lock_irqsave(>lock, flags);
> +
> + /* Reset state and start of transfer, SEL stays down 

Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-14 Thread Wolfram Sang
On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
 Add i2c driver to enable access to devices behind CBUS on Nokia Internet
 Tablets.
 
 The patch also adds CBUS I2C configuration for N8x0 which is one of the
 users of this driver.
 
 Cc: linux-...@vger.kernel.org
 Acked-by: Felipe Balbi ba...@ti.com
 Acked-by: Tony Lindgren t...@atomide.com
 Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi

OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
be an appropriate place. Still, before deciding if it should rather be
in the core directory, I still have a few questions.

Also, does anybody know of a generic bit-banging implementation in the
kernel which could be used here?

Jean: I'd appreciate your general opinion here, too.

 diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
 new file mode 100644
 index 000..bacf2a9
 --- /dev/null
 +++ b/drivers/i2c/busses/i2c-cbus.c
 @@ -0,0 +1,369 @@
 +/*
 + * CBUS I2C driver for Nokia Internet Tablets.
 + *
 + * Copyright (C) 2004-2010 Nokia Corporation
 + *
 + * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
 + * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
 + *
 + * This file is subject to the terms and conditions of the GNU General
 + * Public License. See the file COPYING in the main directory of this
 + * archive for more details.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/io.h
 +#include linux/i2c.h
 +#include linux/gpio.h
 +#include linux/init.h
 +#include linux/slab.h
 +#include linux/delay.h
 +#include linux/errno.h
 +#include linux/kernel.h
 +#include linux/module.h
 +#include linux/of_gpio.h
 +#include linux/i2c-cbus.h
 +#include linux/interrupt.h
 +#include linux/platform_device.h
 +
 +struct cbus_host {
 + /* host lock */
 + spinlock_t  lock;
 +
 + struct device   *dev;
 +
 + int clk_gpio;
 + int dat_gpio;
 + int sel_gpio;
 +};
 +
 +/**
 + * cbus_send_bit - sends one bit over the bus
 + * @host: the host we're using
 + * @bit: one bit of information to send
 + * @input: whether to set data pin as input after sending
 + */
 +static int cbus_send_bit(struct cbus_host *host, unsigned bit,
 + unsigned input)
 +{
 + int ret = 0;
 +
 + gpio_set_value(host-dat_gpio, bit ? 1 : 0);
 + gpio_set_value(host-clk_gpio, 1);
 +
 + /* The data bit is read on the rising edge of CLK */

This comment doesn't belong to the if-block, or?

 + if (input)
 + ret = gpio_direction_input(host-dat_gpio);

No minimum delay for the signal to be on the bus?

 +
 + gpio_set_value(host-clk_gpio, 0);
 +
 + return ret;
 +}
 +
 +/**
 + * cbus_send_data - sends @len amount of data over the bus
 + * @host: the host we're using
 + * @data: the data to send
 + * @len: size of the transfer
 + * @input: whether to set data pin as input after sending
 + */
 +static int cbus_send_data(struct cbus_host *host, unsigned data, unsigned 
 len,
 + unsigned input)
 +{
 + int ret = 0;
 + int i;
 +
 + for (i = len; i  0; i--) {
 + ret = cbus_send_bit(host, data  (1  (i - 1)),
 + input  (i == 1));
 + if (ret  0)
 + goto out;
 + }
 +
 +out:
 + return ret;
 +}
 +
 +/**
 + * cbus_receive_bit - receives one bit from the bus
 + * @host: the host we're using
 + */
 +static int cbus_receive_bit(struct cbus_host *host)
 +{
 + int ret;
 +
 + gpio_set_value(host-clk_gpio, 1);

No delays to ensure the signal has stabilized?

 + ret = gpio_get_value(host-dat_gpio);
 + if (ret  0)
 + goto out;
 + gpio_set_value(host-clk_gpio, 0);
 +
 +out:
 + return ret;
 +}
 +
 +/**
 + * cbus_receive_word - receives 16-bit word from the bus
 + * @host: the host we're using
 + */
 +static int cbus_receive_word(struct cbus_host *host)
 +{
 + int ret = 0;
 + int i;
 +
 + for (i = 16; i  0; i--) {
 + int bit = cbus_receive_bit(host);
 +
 + if (bit  0)
 + goto out;
 +
 + if (bit)
 + ret |= 1  (i - 1);
 + }
 +
 +out:
 + return ret;
 +}
 +
 +/**
 + * cbus_transfer - transfers data over the bus
 + * @host: the host we're using
 + * @rw: read/write flag
 + * @dev: device address
 + * @reg: register address
 + * @data: if @rw == I2C_SBUS_WRITE data to send otherwise 0
 + */
 +static int cbus_transfer(struct cbus_host *host, char rw, unsigned dev,
 +  unsigned reg, unsigned data)
 +{
 + unsigned long flags;
 + int ret;
 +
 + /* We don't want interrupts disturbing our transfer */
 + spin_lock_irqsave(host-lock, flags);
 +
 + /* Reset 

Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-14 Thread Jean Delvare
On Fri, 14 Sep 2012 12:08:06 +0200, Wolfram Sang wrote:
 OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
 be an appropriate place. Still, before deciding if it should rather be
 in the core directory, I still have a few questions.
 
 Also, does anybody know of a generic bit-banging implementation in the
 kernel which could be used here?
 
 Jean: I'd appreciate your general opinion here, too.

Out of time at the moment :(

-- 
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-14 Thread Felipe Balbi
On Fri, Sep 14, 2012 at 12:08:06PM +0200, Wolfram Sang wrote:
 On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
  Add i2c driver to enable access to devices behind CBUS on Nokia Internet
  Tablets.
  
  The patch also adds CBUS I2C configuration for N8x0 which is one of the
  users of this driver.
  
  Cc: linux-...@vger.kernel.org
  Acked-by: Felipe Balbi ba...@ti.com
  Acked-by: Tony Lindgren t...@atomide.com
  Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi
 
 OK, I found the short paragrahp about CBUS in the I2C spec, so I2C might
 be an appropriate place. Still, before deciding if it should rather be
 in the core directory, I still have a few questions.
 
 Also, does anybody know of a generic bit-banging implementation in the
 kernel which could be used here?

there is i2c-gpio, but it wouldn't help much for cbus, unless we change
it quite a lot. The generic i2c_algo_bit_data would have to be changed
to learn about a 3 wire bus. I guess the impact is much bigger than just
accepting this small, self-contained driver.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-13 Thread Aaro Koskinen
On Thu, Sep 13, 2012 at 12:53:09PM +0200, Wolfram Sang wrote:
> On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
> > Add i2c driver to enable access to devices behind CBUS on Nokia Internet
> > Tablets.
> > 
> > The patch also adds CBUS I2C configuration for N8x0 which is one of the
> > users of this driver.
> > 
> > Cc: linux-...@vger.kernel.org
> > Acked-by: Felipe Balbi 
> > Acked-by: Tony Lindgren 
> > Signed-off-by: Aaro Koskinen 
> 
> My main question is: what is CBUS? It doesn't look like an I2C/SMBUS
> host controller, but some bit-banging protocol? As such, it shouldn't go
> to i2c/busses/. And the protocol doesn't look much like I2C, neither.
> There is no ACK/NACK/START/STOP, so I wonder if it should be in i2c
> after all...

It's some legacy 3-wire bus protocol. Not much info is available,
there's some references to CBUS in i2c spec, so I thought it wouldn't
be a completly wrong place for it...

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-13 Thread Wolfram Sang
On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
> Add i2c driver to enable access to devices behind CBUS on Nokia Internet
> Tablets.
> 
> The patch also adds CBUS I2C configuration for N8x0 which is one of the
> users of this driver.
> 
> Cc: linux-...@vger.kernel.org
> Acked-by: Felipe Balbi 
> Acked-by: Tony Lindgren 
> Signed-off-by: Aaro Koskinen 

My main question is: what is CBUS? It doesn't look like an I2C/SMBUS
host controller, but some bit-banging protocol? As such, it shouldn't go
to i2c/busses/. And the protocol doesn't look much like I2C, neither.
There is no ACK/NACK/START/STOP, so I wonder if it should be in i2c
after all...

Jean, maybe you want to have a glimpse on this as well?

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-13 Thread Wolfram Sang
On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
 Add i2c driver to enable access to devices behind CBUS on Nokia Internet
 Tablets.
 
 The patch also adds CBUS I2C configuration for N8x0 which is one of the
 users of this driver.
 
 Cc: linux-...@vger.kernel.org
 Acked-by: Felipe Balbi ba...@ti.com
 Acked-by: Tony Lindgren t...@atomide.com
 Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi

My main question is: what is CBUS? It doesn't look like an I2C/SMBUS
host controller, but some bit-banging protocol? As such, it shouldn't go
to i2c/busses/. And the protocol doesn't look much like I2C, neither.
There is no ACK/NACK/START/STOP, so I wonder if it should be in i2c
after all...

Jean, maybe you want to have a glimpse on this as well?

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-13 Thread Aaro Koskinen
On Thu, Sep 13, 2012 at 12:53:09PM +0200, Wolfram Sang wrote:
 On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
  Add i2c driver to enable access to devices behind CBUS on Nokia Internet
  Tablets.
  
  The patch also adds CBUS I2C configuration for N8x0 which is one of the
  users of this driver.
  
  Cc: linux-...@vger.kernel.org
  Acked-by: Felipe Balbi ba...@ti.com
  Acked-by: Tony Lindgren t...@atomide.com
  Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi
 
 My main question is: what is CBUS? It doesn't look like an I2C/SMBUS
 host controller, but some bit-banging protocol? As such, it shouldn't go
 to i2c/busses/. And the protocol doesn't look much like I2C, neither.
 There is no ACK/NACK/START/STOP, so I wonder if it should be in i2c
 after all...

It's some legacy 3-wire bus protocol. Not much info is available,
there's some references to CBUS in i2c spec, so I thought it wouldn't
be a completly wrong place for it...

A.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-04 Thread Felipe Balbi
Hi,

On Tue, Sep 04, 2012 at 12:31:42PM +0300, Aaro Koskinen wrote:
> On Tue, Sep 04, 2012 at 12:05:07PM +0300, Felipe Balbi wrote:
> > > + * CBUS I2C driver for Nokia Internet Tablets.
> 
> [...]
> 
> > this version misses the entire IRQ handling we already had on linux-omap
> > tree, so it's quite a regression.
> 
> There's no interrupts used in plain CBUS protocol/communication I think?
> 
> But Retu MFD supports the GPIO IRQ. I added the code for this set. And
> the power button driver (patch 4/4) is using this functionality:
> 
> # cat /proc/interrupts
> [...]
> 204: 29  GPIO  retu-mfd
> 224: 29  RETU  retu-pwrbutton

oops, indeed. My bad. I looked at the wrong patch. Nevermind, this
series looks good to me.

FWIW:

Acked-by: Felipe Balbi 

(for entire series)

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-04 Thread Aaro Koskinen
On Tue, Sep 04, 2012 at 12:05:07PM +0300, Felipe Balbi wrote:
> > + * CBUS I2C driver for Nokia Internet Tablets.

[...]

> this version misses the entire IRQ handling we already had on linux-omap
> tree, so it's quite a regression.

There's no interrupts used in plain CBUS protocol/communication I think?

But Retu MFD supports the GPIO IRQ. I added the code for this set. And
the power button driver (patch 4/4) is using this functionality:

# cat /proc/interrupts
[...]
204: 29  GPIO  retu-mfd
224: 29  RETU  retu-pwrbutton

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-04 Thread Felipe Balbi
Hi,

On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
> diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
> new file mode 100644
> index 000..bacf2a9
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-cbus.c
> @@ -0,0 +1,369 @@
> +/*
> + * CBUS I2C driver for Nokia Internet Tablets.
> + *
> + * Copyright (C) 2004-2010 Nokia Corporation
> + *
> + * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
> + * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
> + *
> + * This file is subject to the terms and conditions of the GNU General
> + * Public License. See the file "COPYING" in the main directory of this
> + * archive for more details.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct cbus_host {
> + /* host lock */
> + spinlock_t  lock;
> +
> + struct device   *dev;
> +
> + int clk_gpio;
> + int dat_gpio;
> + int sel_gpio;
> +};
> +
> +/**
> + * cbus_send_bit - sends one bit over the bus
> + * @host: the host we're using
> + * @bit: one bit of information to send
> + * @input: whether to set data pin as input after sending
> + */
> +static int cbus_send_bit(struct cbus_host *host, unsigned bit,
> + unsigned input)
> +{
> + int ret = 0;
> +
> + gpio_set_value(host->dat_gpio, bit ? 1 : 0);
> + gpio_set_value(host->clk_gpio, 1);
> +
> + /* The data bit is read on the rising edge of CLK */
> + if (input)
> + ret = gpio_direction_input(host->dat_gpio);
> +
> + gpio_set_value(host->clk_gpio, 0);
> +
> + return ret;
> +}
> +
> +/**
> + * cbus_send_data - sends @len amount of data over the bus
> + * @host: the host we're using
> + * @data: the data to send
> + * @len: size of the transfer
> + * @input: whether to set data pin as input after sending
> + */
> +static int cbus_send_data(struct cbus_host *host, unsigned data, unsigned 
> len,
> + unsigned input)
> +{
> + int ret = 0;
> + int i;
> +
> + for (i = len; i > 0; i--) {
> + ret = cbus_send_bit(host, data & (1 << (i - 1)),
> + input && (i == 1));
> + if (ret < 0)
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * cbus_receive_bit - receives one bit from the bus
> + * @host: the host we're using
> + */
> +static int cbus_receive_bit(struct cbus_host *host)
> +{
> + int ret;
> +
> + gpio_set_value(host->clk_gpio, 1);
> + ret = gpio_get_value(host->dat_gpio);
> + if (ret < 0)
> + goto out;
> + gpio_set_value(host->clk_gpio, 0);
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * cbus_receive_word - receives 16-bit word from the bus
> + * @host: the host we're using
> + */
> +static int cbus_receive_word(struct cbus_host *host)
> +{
> + int ret = 0;
> + int i;
> +
> + for (i = 16; i > 0; i--) {
> + int bit = cbus_receive_bit(host);
> +
> + if (bit < 0)
> + goto out;
> +
> + if (bit)
> + ret |= 1 << (i - 1);
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * cbus_transfer - transfers data over the bus
> + * @host: the host we're using
> + * @rw: read/write flag
> + * @dev: device address
> + * @reg: register address
> + * @data: if @rw == I2C_SBUS_WRITE data to send otherwise 0
> + */
> +static int cbus_transfer(struct cbus_host *host, char rw, unsigned dev,
> +  unsigned reg, unsigned data)
> +{
> + unsigned long flags;
> + int ret;
> +
> + /* We don't want interrupts disturbing our transfer */
> + spin_lock_irqsave(>lock, flags);
> +
> + /* Reset state and start of transfer, SEL stays down during transfer */
> + gpio_set_value(host->sel_gpio, 0);
> +
> + /* Set the DAT pin to output */
> + gpio_direction_output(host->dat_gpio, 1);
> +
> + /* Send the device address */
> + ret = cbus_send_data(host, dev, 3, 0);
> + if (ret < 0) {
> + dev_dbg(host->dev, "failed sending device addr\n");
> + goto out;
> + }
> +
> + /* Send the rw flag */
> + ret = cbus_send_bit(host, rw == I2C_SMBUS_READ, 0);
> + if (ret < 0) {
> + dev_dbg(host->dev, "failed sending read/write flag\n");
> + goto out;
> + }
> +
> + /* Send the device address */
> + ret = cbus_send_data(host, reg, 5, rw == I2C_SMBUS_READ);
> + if (ret < 0) {
> + dev_dbg(host->dev, "failed sending register addr\n");
> + 

Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-04 Thread Felipe Balbi
Hi,

On Mon, Sep 03, 2012 at 11:23:22PM +0300, Aaro Koskinen wrote:
 diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
 new file mode 100644
 index 000..bacf2a9
 --- /dev/null
 +++ b/drivers/i2c/busses/i2c-cbus.c
 @@ -0,0 +1,369 @@
 +/*
 + * CBUS I2C driver for Nokia Internet Tablets.
 + *
 + * Copyright (C) 2004-2010 Nokia Corporation
 + *
 + * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
 + * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
 + *
 + * This file is subject to the terms and conditions of the GNU General
 + * Public License. See the file COPYING in the main directory of this
 + * archive for more details.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/io.h
 +#include linux/i2c.h
 +#include linux/gpio.h
 +#include linux/init.h
 +#include linux/slab.h
 +#include linux/delay.h
 +#include linux/errno.h
 +#include linux/kernel.h
 +#include linux/module.h
 +#include linux/of_gpio.h
 +#include linux/i2c-cbus.h
 +#include linux/interrupt.h
 +#include linux/platform_device.h
 +
 +struct cbus_host {
 + /* host lock */
 + spinlock_t  lock;
 +
 + struct device   *dev;
 +
 + int clk_gpio;
 + int dat_gpio;
 + int sel_gpio;
 +};
 +
 +/**
 + * cbus_send_bit - sends one bit over the bus
 + * @host: the host we're using
 + * @bit: one bit of information to send
 + * @input: whether to set data pin as input after sending
 + */
 +static int cbus_send_bit(struct cbus_host *host, unsigned bit,
 + unsigned input)
 +{
 + int ret = 0;
 +
 + gpio_set_value(host-dat_gpio, bit ? 1 : 0);
 + gpio_set_value(host-clk_gpio, 1);
 +
 + /* The data bit is read on the rising edge of CLK */
 + if (input)
 + ret = gpio_direction_input(host-dat_gpio);
 +
 + gpio_set_value(host-clk_gpio, 0);
 +
 + return ret;
 +}
 +
 +/**
 + * cbus_send_data - sends @len amount of data over the bus
 + * @host: the host we're using
 + * @data: the data to send
 + * @len: size of the transfer
 + * @input: whether to set data pin as input after sending
 + */
 +static int cbus_send_data(struct cbus_host *host, unsigned data, unsigned 
 len,
 + unsigned input)
 +{
 + int ret = 0;
 + int i;
 +
 + for (i = len; i  0; i--) {
 + ret = cbus_send_bit(host, data  (1  (i - 1)),
 + input  (i == 1));
 + if (ret  0)
 + goto out;
 + }
 +
 +out:
 + return ret;
 +}
 +
 +/**
 + * cbus_receive_bit - receives one bit from the bus
 + * @host: the host we're using
 + */
 +static int cbus_receive_bit(struct cbus_host *host)
 +{
 + int ret;
 +
 + gpio_set_value(host-clk_gpio, 1);
 + ret = gpio_get_value(host-dat_gpio);
 + if (ret  0)
 + goto out;
 + gpio_set_value(host-clk_gpio, 0);
 +
 +out:
 + return ret;
 +}
 +
 +/**
 + * cbus_receive_word - receives 16-bit word from the bus
 + * @host: the host we're using
 + */
 +static int cbus_receive_word(struct cbus_host *host)
 +{
 + int ret = 0;
 + int i;
 +
 + for (i = 16; i  0; i--) {
 + int bit = cbus_receive_bit(host);
 +
 + if (bit  0)
 + goto out;
 +
 + if (bit)
 + ret |= 1  (i - 1);
 + }
 +
 +out:
 + return ret;
 +}
 +
 +/**
 + * cbus_transfer - transfers data over the bus
 + * @host: the host we're using
 + * @rw: read/write flag
 + * @dev: device address
 + * @reg: register address
 + * @data: if @rw == I2C_SBUS_WRITE data to send otherwise 0
 + */
 +static int cbus_transfer(struct cbus_host *host, char rw, unsigned dev,
 +  unsigned reg, unsigned data)
 +{
 + unsigned long flags;
 + int ret;
 +
 + /* We don't want interrupts disturbing our transfer */
 + spin_lock_irqsave(host-lock, flags);
 +
 + /* Reset state and start of transfer, SEL stays down during transfer */
 + gpio_set_value(host-sel_gpio, 0);
 +
 + /* Set the DAT pin to output */
 + gpio_direction_output(host-dat_gpio, 1);
 +
 + /* Send the device address */
 + ret = cbus_send_data(host, dev, 3, 0);
 + if (ret  0) {
 + dev_dbg(host-dev, failed sending device addr\n);
 + goto out;
 + }
 +
 + /* Send the rw flag */
 + ret = cbus_send_bit(host, rw == I2C_SMBUS_READ, 0);
 + if (ret  0) {
 + dev_dbg(host-dev, failed sending read/write flag\n);
 + goto out;
 + }
 +
 + /* Send the device address */
 + ret = cbus_send_data(host, reg, 5, rw == I2C_SMBUS_READ);
 + if (ret  0) {
 + dev_dbg(host-dev, failed sending register addr\n);
 + goto out;
 + }
 +
 + 

Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-04 Thread Aaro Koskinen
On Tue, Sep 04, 2012 at 12:05:07PM +0300, Felipe Balbi wrote:
  + * CBUS I2C driver for Nokia Internet Tablets.

[...]

 this version misses the entire IRQ handling we already had on linux-omap
 tree, so it's quite a regression.

There's no interrupts used in plain CBUS protocol/communication I think?

But Retu MFD supports the GPIO IRQ. I added the code for this set. And
the power button driver (patch 4/4) is using this functionality:

# cat /proc/interrupts
[...]
204: 29  GPIO  retu-mfd
224: 29  RETU  retu-pwrbutton

A.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-04 Thread Felipe Balbi
Hi,

On Tue, Sep 04, 2012 at 12:31:42PM +0300, Aaro Koskinen wrote:
 On Tue, Sep 04, 2012 at 12:05:07PM +0300, Felipe Balbi wrote:
   + * CBUS I2C driver for Nokia Internet Tablets.
 
 [...]
 
  this version misses the entire IRQ handling we already had on linux-omap
  tree, so it's quite a regression.
 
 There's no interrupts used in plain CBUS protocol/communication I think?
 
 But Retu MFD supports the GPIO IRQ. I added the code for this set. And
 the power button driver (patch 4/4) is using this functionality:
 
 # cat /proc/interrupts
 [...]
 204: 29  GPIO  retu-mfd
 224: 29  RETU  retu-pwrbutton

oops, indeed. My bad. I looked at the wrong patch. Nevermind, this
series looks good to me.

FWIW:

Acked-by: Felipe Balbi ba...@ti.com

(for entire series)

-- 
balbi


signature.asc
Description: Digital signature


[PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-03 Thread Aaro Koskinen
Add i2c driver to enable access to devices behind CBUS on Nokia Internet
Tablets.

The patch also adds CBUS I2C configuration for N8x0 which is one of the
users of this driver.

Cc: linux-...@vger.kernel.org
Acked-by: Felipe Balbi 
Acked-by: Tony Lindgren 
Signed-off-by: Aaro Koskinen 
---
 arch/arm/mach-omap2/board-n8x0.c |   42 +
 drivers/i2c/busses/Kconfig   |   10 +
 drivers/i2c/busses/Makefile  |1 +
 drivers/i2c/busses/i2c-cbus.c|  369 ++
 include/linux/i2c-cbus.h |   27 +++
 5 files changed, 449 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-cbus.c
 create mode 100644 include/linux/i2c-cbus.h

diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 677357f..2495f2d 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -16,8 +16,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -42,6 +44,45 @@
 #define TUSB6010_GPIO_ENABLE   0
 #define TUSB6010_DMACHAN   0x3f
 
+#if defined(CONFIG_I2C_CBUS) || defined(CONFIG_I2C_CBUS_MODULE)
+static struct i2c_cbus_platform_data n8x0_cbus_data = {
+   .clk_gpio = 66,
+   .dat_gpio = 65,
+   .sel_gpio = 64,
+};
+
+static struct platform_device n8x0_cbus_device = {
+   .name   = "i2c-cbus",
+   .id = 3,
+   .dev= {
+   .platform_data = _cbus_data,
+   },
+};
+
+static struct i2c_board_info n8x0_i2c_board_info_3[] __initdata = {
+   {
+   I2C_BOARD_INFO("retu-mfd", 0x01),
+   },
+};
+
+static void __init n8x0_cbus_init(void)
+{
+   const int retu_irq_gpio = 108;
+
+   if (gpio_request_one(retu_irq_gpio, GPIOF_IN, "Retu IRQ"))
+   return;
+   irq_set_irq_type(gpio_to_irq(retu_irq_gpio), IRQ_TYPE_EDGE_RISING);
+   n8x0_i2c_board_info_3[0].irq = gpio_to_irq(retu_irq_gpio);
+   i2c_register_board_info(3, n8x0_i2c_board_info_3,
+   ARRAY_SIZE(n8x0_i2c_board_info_3));
+   platform_device_register(_cbus_device);
+}
+#else /* CONFIG_I2C_CBUS */
+static void __init n8x0_cbus_init(void)
+{
+}
+#endif /* CONFIG_I2C_CBUS */
+
 #if defined(CONFIG_USB_MUSB_TUSB6010) || 
defined(CONFIG_USB_MUSB_TUSB6010_MODULE)
 /*
  * Enable or disable power to TUSB6010. When enabling, turn on 3.3 V and
@@ -681,6 +722,7 @@ static void __init n8x0_init_machine(void)
gpmc_onenand_init(board_onenand_data);
n8x0_mmc_init();
n8x0_usb_init();
+   n8x0_cbus_init();
 }
 
 MACHINE_START(NOKIA_N800, "Nokia N800")
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index b4aaa1b..184ef43 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -331,6 +331,16 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
help
  The unit of the TWI clock is kHz.
 
+config I2C_CBUS
+   tristate "CBUS I2C driver"
+   depends on GENERIC_GPIO
+   help
+ Support for CBUS access using I2C API. Mostly relevant for Nokia
+ Internet Tablets (770, N800 and N810).
+
+ This driver can also be built as a module.  If so, the module
+ will be called i2c-cbus.
+
 config I2C_CPM
tristate "Freescale CPM1 or CPM2 (MPC8xx/826x)"
depends on (CPM1 || CPM2) && OF_I2C
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index ce3c2be..44dbfd1 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_I2C_POWERMAC)+= i2c-powermac.o
 obj-$(CONFIG_I2C_AT91) += i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)   += i2c-au1550.o
 obj-$(CONFIG_I2C_BLACKFIN_TWI) += i2c-bfin-twi.o
+obj-$(CONFIG_I2C_CBUS) += i2c-cbus.o
 obj-$(CONFIG_I2C_CPM)  += i2c-cpm.o
 obj-$(CONFIG_I2C_DAVINCI)  += i2c-davinci.o
 obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM)  += i2c-designware-platform.o
diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
new file mode 100644
index 000..bacf2a9
--- /dev/null
+++ b/drivers/i2c/busses/i2c-cbus.c
@@ -0,0 +1,369 @@
+/*
+ * CBUS I2C driver for Nokia Internet Tablets.
+ *
+ * Copyright (C) 2004-2010 Nokia Corporation
+ *
+ * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
+ * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct cbus_host {
+   /* host 

[PATCH 1/4] i2c: introduce i2c-cbus driver

2012-09-03 Thread Aaro Koskinen
Add i2c driver to enable access to devices behind CBUS on Nokia Internet
Tablets.

The patch also adds CBUS I2C configuration for N8x0 which is one of the
users of this driver.

Cc: linux-...@vger.kernel.org
Acked-by: Felipe Balbi ba...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi
---
 arch/arm/mach-omap2/board-n8x0.c |   42 +
 drivers/i2c/busses/Kconfig   |   10 +
 drivers/i2c/busses/Makefile  |1 +
 drivers/i2c/busses/i2c-cbus.c|  369 ++
 include/linux/i2c-cbus.h |   27 +++
 5 files changed, 449 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-cbus.c
 create mode 100644 include/linux/i2c-cbus.h

diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 677357f..2495f2d 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -16,8 +16,10 @@
 #include linux/gpio.h
 #include linux/init.h
 #include linux/io.h
+#include linux/irq.h
 #include linux/stddef.h
 #include linux/i2c.h
+#include linux/i2c-cbus.h
 #include linux/spi/spi.h
 #include linux/usb/musb.h
 #include sound/tlv320aic3x.h
@@ -42,6 +44,45 @@
 #define TUSB6010_GPIO_ENABLE   0
 #define TUSB6010_DMACHAN   0x3f
 
+#if defined(CONFIG_I2C_CBUS) || defined(CONFIG_I2C_CBUS_MODULE)
+static struct i2c_cbus_platform_data n8x0_cbus_data = {
+   .clk_gpio = 66,
+   .dat_gpio = 65,
+   .sel_gpio = 64,
+};
+
+static struct platform_device n8x0_cbus_device = {
+   .name   = i2c-cbus,
+   .id = 3,
+   .dev= {
+   .platform_data = n8x0_cbus_data,
+   },
+};
+
+static struct i2c_board_info n8x0_i2c_board_info_3[] __initdata = {
+   {
+   I2C_BOARD_INFO(retu-mfd, 0x01),
+   },
+};
+
+static void __init n8x0_cbus_init(void)
+{
+   const int retu_irq_gpio = 108;
+
+   if (gpio_request_one(retu_irq_gpio, GPIOF_IN, Retu IRQ))
+   return;
+   irq_set_irq_type(gpio_to_irq(retu_irq_gpio), IRQ_TYPE_EDGE_RISING);
+   n8x0_i2c_board_info_3[0].irq = gpio_to_irq(retu_irq_gpio);
+   i2c_register_board_info(3, n8x0_i2c_board_info_3,
+   ARRAY_SIZE(n8x0_i2c_board_info_3));
+   platform_device_register(n8x0_cbus_device);
+}
+#else /* CONFIG_I2C_CBUS */
+static void __init n8x0_cbus_init(void)
+{
+}
+#endif /* CONFIG_I2C_CBUS */
+
 #if defined(CONFIG_USB_MUSB_TUSB6010) || 
defined(CONFIG_USB_MUSB_TUSB6010_MODULE)
 /*
  * Enable or disable power to TUSB6010. When enabling, turn on 3.3 V and
@@ -681,6 +722,7 @@ static void __init n8x0_init_machine(void)
gpmc_onenand_init(board_onenand_data);
n8x0_mmc_init();
n8x0_usb_init();
+   n8x0_cbus_init();
 }
 
 MACHINE_START(NOKIA_N800, Nokia N800)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index b4aaa1b..184ef43 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -331,6 +331,16 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
help
  The unit of the TWI clock is kHz.
 
+config I2C_CBUS
+   tristate CBUS I2C driver
+   depends on GENERIC_GPIO
+   help
+ Support for CBUS access using I2C API. Mostly relevant for Nokia
+ Internet Tablets (770, N800 and N810).
+
+ This driver can also be built as a module.  If so, the module
+ will be called i2c-cbus.
+
 config I2C_CPM
tristate Freescale CPM1 or CPM2 (MPC8xx/826x)
depends on (CPM1 || CPM2)  OF_I2C
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index ce3c2be..44dbfd1 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_I2C_POWERMAC)+= i2c-powermac.o
 obj-$(CONFIG_I2C_AT91) += i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)   += i2c-au1550.o
 obj-$(CONFIG_I2C_BLACKFIN_TWI) += i2c-bfin-twi.o
+obj-$(CONFIG_I2C_CBUS) += i2c-cbus.o
 obj-$(CONFIG_I2C_CPM)  += i2c-cpm.o
 obj-$(CONFIG_I2C_DAVINCI)  += i2c-davinci.o
 obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM)  += i2c-designware-platform.o
diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
new file mode 100644
index 000..bacf2a9
--- /dev/null
+++ b/drivers/i2c/busses/i2c-cbus.c
@@ -0,0 +1,369 @@
+/*
+ * CBUS I2C driver for Nokia Internet Tablets.
+ *
+ * Copyright (C) 2004-2010 Nokia Corporation
+ *
+ * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
+ * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file COPYING in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+