Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure

2012-12-10 Thread Viresh Kumar
On 7 December 2012 02:00, Paul Carpenter
 wrote:
> OK by me
>
> Reviewed-by: Paul Carpenter 

Are you taking it for 3.8 now?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] sparc: Enable OF functionality for sparc for i2c and spi

2012-12-10 Thread Grant Likely
On Fri, 07 Dec 2012 12:30:57 -0500 (EST), David Miller  
wrote:
> From: Grant Likely 
> Date: Tue, 4 Dec 2012 21:49:26 +
> 
> > On Tue, Dec 4, 2012 at 9:10 PM, David Miller  wrote:
> >> From: Grant Likely 
> >> Date: Tue, 4 Dec 2012 14:44:57 +
> >>
> >>> On Tue, Dec 4, 2012 at 2:09 PM, Andreas Larsson  
> >>> wrote:
>  This series removes the dependency on !SPARC for OF_I2C and removes the
>  depencency of !defined(CONFIG_SPARC) for the function
>  of_register_spi_devices. I find no reason for these to be unavailable
>  for sparc.
> 
>  I am not sure if these should go through the sparc tree or the
>  corresponding subsystem trees.
> >>>
> >>> They should go through the subsystem trees.
> >>
> >> I'll take this into the sparc tree after some build testing, thanks.
> > 
> > I'd prefer to take the SPI one through my tree, but I won't get my
> > knickers in a knot if you insist.
> 
> That's fine:
> 
> Acked-by: David S. Miller 

Applied, thanks.

g.


-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [tpmdd-devel] [PATCH] char/tpm: Convert struct i2c_msg initialization to C99 format

2012-12-10 Thread Jean Delvare
On Mon, 10 Dec 2012 16:54:08 +, peter.hu...@infineon.com wrote:
> -Original Message-
> From: Huewe Peter (IFAG CCS TI SWT SW ESW) 
> Sent: Wednesday, October 10, 2012 4:15 PM
> To: 'Shubhrajyoti Datta'; Jean Delvare
> Cc: tp...@sirrix.com; m...@srajiv.net; tpmdd-de...@lists.sourceforge.net; 
> linux-i2c@vger.kernel.org; k...@linux.vnet.ibm.com; Shubhrajyoti D
> Subject: RE: [tpmdd-devel] [PATCH] char/tpm: Convert struct i2c_msg 
> initialization to C99 format
> 
> > -Original Message-
> > From: Shubhrajyoti Datta [mailto:omaplinuxker...@gmail.com]
> > Subject: [PATCHv2] char/tpm: Convert struct i2c_msg initialization to 
> > C99 format
> 
> > Convert the struct i2c_msg initialization to C99 format. This makes 
> > maintaining and editing the code simpler. Also helps once other fields 
> > like transferred are added in future.
> 
> > Thanks to Julia Lawall   for automating the 
> > conversion
> 
> > Acked-by: Jean Delvare 
> > Signed-off-by: Shubhrajyoti D 
> 
> > Acked-by: Peter Huewe 
> 
> 
> Hi,
> 
> was this applied somewhere? 

Not that I am aware of. Would be a good idea to resend it with Cc to
Andrew so that it doesn't get lost.

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


RE: [tpmdd-devel] [PATCH] char/tpm: Convert struct i2c_msg initialization to C99 format

2012-12-10 Thread Peter.Huewe
-Original Message-
From: Huewe Peter (IFAG CCS TI SWT SW ESW) 
Sent: Wednesday, October 10, 2012 4:15 PM
To: 'Shubhrajyoti Datta'; Jean Delvare
Cc: tp...@sirrix.com; m...@srajiv.net; tpmdd-de...@lists.sourceforge.net; 
linux-i2c@vger.kernel.org; k...@linux.vnet.ibm.com; Shubhrajyoti D
Subject: RE: [tpmdd-devel] [PATCH] char/tpm: Convert struct i2c_msg 
initialization to C99 format

> -Original Message-
> From: Shubhrajyoti Datta [mailto:omaplinuxker...@gmail.com]
> Subject: [PATCHv2] char/tpm: Convert struct i2c_msg initialization to 
> C99 format

> Convert the struct i2c_msg initialization to C99 format. This makes 
> maintaining and editing the code simpler. Also helps once other fields 
> like transferred are added in future.

> Thanks to Julia Lawall   for automating the 
> conversion

> Acked-by: Jean Delvare 
> Signed-off-by: Shubhrajyoti D 

> Acked-by: Peter Huewe 


Hi,

was this applied somewhere? 


Thanks,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] I2C: EXYNOS: Add slave support to i2c

2012-12-10 Thread Russell King - ARM Linux
On Mon, Dec 10, 2012 at 03:02:28PM +0530, Giridhar Maruthy wrote:
> Hi Russel,
> 
> Thanks for review and please find my replies below.
> 
> On 7 December 2012 18:03, Russell King - ARM Linux
>  wrote:
> > On Fri, Dec 07, 2012 at 05:33:17PM +0530, Tushar Behera wrote:
> >> On 12/03/2012 05:46 PM, Giridhar Maruthy wrote:
> >> > This patch adds slave support to i2c. The dt entry i2c-mode
> >> > decides at probe time if the controller needs to work in
> >> > slave mode and the controller is accordingly programmed.
> >
> > (I don't have the original patches.)
> >
> > Hmm.  How has slave-mode support been tested?
> 
> I have taken two I2C controllers in exynos5250.
> I configured one as slave and the other as master port.
> Then physically connected the pins of the two ports.
> >
> > Remembering that I2C slave devices do not initiate bus accesses, all
> > accesses will be started by some other master.  How are you dealing
> > with the bytes received from the master,
> 
> I run the slave read application in background.
> The resulting slave read will sleep till all bytes are received
> (wait_event_interruptible)

Oh god no, not more broken interruptible waits in I2C code.  I've seen
already the results of crap like this with a kernel I2C peripheral driver
falling over because the I2C layer returns -ERESTARTcrap with the Marvell
I2C bus adapter.

> once the master sends the data, an ack is given out by the slave controller
> in the ISR and the data is cached in the buffer(
> buffer sent by slave receive application).
> After all data is received, the read wakes up and the
> slave receive program gets the data.
> 
> > and how are you returning a response to the master in reply to a read 
> > request?
> 
> Similarl logic works in slave transmit mode (read request). Slave
> sleeps till the master initiates the transfer.
> >
> > We had support for this on PXA I2C through a callback from the driver
> > into PXA code (used for the Psion Teklogix Netbook device) and it worked
> > really well, but what you can't do is use the standard I2C interfaces
> > for slave mode.
> 
> I have a question here. Since the same framework can work for both
> master and slave, is there any technical limitations I have overseen
> which prevents the slave mode to work?

I don't really call your description above as "working" - it sounds like
a bodge - it sounds like trying to bend a layer designed for master mode
to sort-of-maybe-if-the-wind-is-in-the-right-direction-and-the-cows-are-
all-standing-up-work.  What if the application is trying to read in slave
mode while the master is also trying to read?  How do you deal with that?
What about the converse?  What if the application is trying to write data
and the master also issues a write request?

Finally, what about a multi-master situation?  I can see no way for your
implementation to have any hope of working there because there is no
distinction between operating the interface in master mode and slave mode.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] I2C: EXYNOS: Add slave support to i2c

2012-12-10 Thread Giridhar Maruthy
Hi Russel,

Thanks for review and please find my replies below.

On 7 December 2012 18:03, Russell King - ARM Linux
 wrote:
> On Fri, Dec 07, 2012 at 05:33:17PM +0530, Tushar Behera wrote:
>> On 12/03/2012 05:46 PM, Giridhar Maruthy wrote:
>> > This patch adds slave support to i2c. The dt entry i2c-mode
>> > decides at probe time if the controller needs to work in
>> > slave mode and the controller is accordingly programmed.
>
> (I don't have the original patches.)
>
> Hmm.  How has slave-mode support been tested?

I have taken two I2C controllers in exynos5250.
I configured one as slave and the other as master port.
Then physically connected the pins of the two ports.
>
> Remembering that I2C slave devices do not initiate bus accesses, all
> accesses will be started by some other master.  How are you dealing
> with the bytes received from the master,

I run the slave read application in background.
The resulting slave read will sleep till all bytes are received
(wait_event_interruptible)

once the master sends the data, an ack is given out by the slave controller
in the ISR and the data is cached in the buffer(
buffer sent by slave receive application).
After all data is received, the read wakes up and the
slave receive program gets the data.

> and how are you returning a response to the master in reply to a read request?

Similarl logic works in slave transmit mode (read request). Slave
sleeps till the master initiates the transfer.
>
> We had support for this on PXA I2C through a callback from the driver
> into PXA code (used for the Psion Teklogix Netbook device) and it worked
> really well, but what you can't do is use the standard I2C interfaces
> for slave mode.

I have a question here. Since the same framework can work for both
master and slave, is there any technical limitations I have overseen
which prevents the slave mode to work?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] I2C: EXYNOS: Add slave support to i2c

2012-12-10 Thread Giridhar Maruthy
Thanks Tushar,

On 7 December 2012 17:33, Tushar Behera  wrote:
> On 12/03/2012 05:46 PM, Giridhar Maruthy wrote:
>> This patch adds slave support to i2c. The dt entry i2c-mode
>> decides at probe time if the controller needs to work in
>> slave mode and the controller is accordingly programmed.
>>
>> Signed-off-by: Giridhar Maruthy 
>> ---
>>  drivers/i2c/busses/i2c-s3c2410.c |  100
>> ++
>>  1 file changed, 68 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-s3c2410.c
>> b/drivers/i2c/busses/i2c-s3c2410.c
>> index e93e7d6..d83a6d7 100644
>> --- a/drivers/i2c/busses/i2c-s3c2410.c
>> +++ b/drivers/i2c/busses/i2c-s3c2410.c
>> @@ -53,6 +53,9 @@
>>  /* Max time to wait for bus to become idle after a xfer (in us) */
>>  #define S3C2410_IDLE_TIMEOUT   5000
>>
>> +/* To find the master/slave mode of current controller */
>> +#define is_master(i2c) (!i2c->i2c_mode)
>> +
>>  /* i2c controller state */
>>  enum s3c24xx_i2c_state {
>> STATE_IDLE,
>> @@ -89,6 +92,8 @@ struct s3c24xx_i2c {
>>  #ifdef CONFIG_CPU_FREQ
>> struct notifier_block   freq_transition;
>>  #endif
>> +   /* i2c_mode: 0 is for master; and 1 is for slave */
>> +   unsigned inti2c_mode;
>>  };
>>
>>  static struct platform_device_id s3c24xx_driver_ids[] = {
>> @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct
>> s3c24xx_i2c *i2c,
>> stat = 0;
>> stat |=  S3C2410_IICSTAT_TXRXEN;
>>
>> -   if (msg->flags & I2C_M_RD) {
>> -   stat |= S3C2410_IICSTAT_MASTER_RX;
>> -   addr |= 1;
>> -   } else
>> -   stat |= S3C2410_IICSTAT_MASTER_TX;
>> +   if (is_master(i2c)) {
>> +   /* Master mode */
>> +   if (msg->flags & I2C_M_RD) {
>> +   stat |= S3C2410_IICSTAT_MASTER_RX;
>> +   addr |= 1;
>> +   } else
>> +   stat |= S3C2410_IICSTAT_MASTER_TX;
>> +   } else {
>> +   /* Slave mode */
>> +   if (msg->flags & I2C_M_RD) {
>> +   stat |= S3C2410_IICSTAT_SLAVE_RX;
>> +   addr |= 1;
>> +   } else
>> +   stat |= S3C2410_IICSTAT_SLAVE_TX;
>> +   }
>>
>> if (msg->flags & I2C_M_REV_DIR_ADDR)
>> addr ^= 1;
>> @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct
>> s3c24xx_i2c *i2c,
>> dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon);
>> writel(iiccon, i2c->regs + S3C2410_IICCON);
>>
>> -   stat |= S3C2410_IICSTAT_START;
>> -   writel(stat, i2c->regs + S3C2410_IICSTAT);
>> +   if (is_master(i2c)) {
>> +   stat |= S3C2410_IICSTAT_START;
>> +   writel(stat, i2c->regs + S3C2410_IICSTAT);
>> +   }
>>  }
>>
>>  static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret)
>> @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct
>> s3c24xx_i2c *i2c, int ret)
>>  * devices, the host as Master and the HDMIPHY device as the slave.
>>  * Skipping the STOP condition has been tested on this bus and
>> works.
>>  */
>> -   if (i2c->quirks & QUIRK_HDMIPHY) {
>> -   /* Stop driving the I2C pins */
>> -   iicstat &= ~S3C2410_IICSTAT_TXRXEN;
>> -   } else {
>> -   /* stop the transfer */
>> -   iicstat &= ~S3C2410_IICSTAT_START;
>> +   if (is_master(i2c)) {
>> +   if (i2c->quirks & QUIRK_HDMIPHY) {
>> +   /* Stop driving the I2C pins */
>> +   iicstat &= ~S3C2410_IICSTAT_TXRXEN;
>> +   } else {
>> +   /* stop the transfer */
>> +   if (is_master(i2c)) {
>
> This is executed only if is_master(i2c) is true, so there seems no need
> to checking it again.
>

Agreed, I will correct this.
>> +   /* Start/Stop required only for master */
>> +   iicstat &= ~S3C2410_IICSTAT_START;
>> +   }
>> +   }
>> +   writel(iicstat, i2c->regs + S3C2410_IICSTAT);
>> }
>> -   writel(iicstat, i2c->regs + S3C2410_IICSTAT);
>>
>> i2c->state = STATE_STOP;
>>
>> @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c
>> *i2c, unsigned long iicstat)
>>  */
>>
>> if (iicstat & S3C2410_IICSTAT_LASTBIT &&
>> -   !(i2c->msg->flags & I2C_M_IGNORE_NAK)) {
>> +   !(i2c->msg->flags & I2C_M_IGNORE_NAK) &&
>> +   is_master(i2c)) {
>> /* ack was not received... */
>>
>> dev_dbg(i2c->dev, "ack was not received\n");
>> @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c
>> *i2c, unsigned long iicstat)
>>  * end of the message, and if so, work out what to do
>>  *