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

[David Miller]

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 
--
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 1/2] of_i2c: sparc: Allow OF_I2C for sparc

[David Miller]

From: Andreas Larsson 
Date: Tue,  4 Dec 2012 15:09:37 +0100

> Signed-off-by: Andreas Larsson 

Applied.
--
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 v2 4/4] HID: i2c-hid: fix i2c_hid_get_raw_report count mismatches

[Benjamin Tissoires]

On Thu, Dec 6, 2012 at 11:01 AM, Jiri Kosina  wrote:
>> > -   count = ihid->inbuf[0] | (ihid->inbuf[1] << 8);
>> > +   ret_count = ihid->inbuf[0] | (ihid->inbuf[1] << 8);
>> >
>> > +   if (!ret_count)
>>
>> I'd make this (ret_count <= 2), as this would let you call memcpy with a
>> null or even negative length.
>
> Good catch, it doesn't account for the 2 bytes needed for storing the
> reply size.
>
> I have fixed that and applied the patch, thanks everybody!
>

Hi Jiri, Jean,

thank you very much for the work done. I was in a meeting past two
days, so I was not able to answer sooner.

Cheers,
Benjamin
--
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

[Russell King - ARM Linux]

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?

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, and how are you returning a
response to the master in reply to a read request?

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.
--
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

[Tushar Behera]

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.

> +   /* 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
>  */
> 
> -   if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) {
> +   if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) &&
> is_master(i2c)) {
> if (iicstat & S3C2410_IICSTAT_LAS

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

[Giridhar Maruthy]

Thanks for the review Subash. Please find my reply below.

On 6 December 2012 23:05, Subash Patel  wrote:
> Hi Giridhar,
>
>
> 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)) {
>> +   /* Start/Stop required only for master */
>> +   iicstat &= ~S3C2410_IICSTAT_START;
>> +   }
>> +   }
>> +   writel(iicstat, i2c->regs + S3C2410_IICSTAT);
>
>
> I don't see i2c controller for HDMIPHY working in slave mode. So do we need
> to check if its master and proceed? Cant the quirks check enough for it?
> Even if it is configured as slave, there is no error indication for this
> here.

My understanding is that the HDMIPHY always commands and hence,
HDMIPHY does not need to be slave at all.
But if you think it is a policy decision which need not be implemented
in driver, I agree with you.
>
>>  }
>> -   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

Re: [PATCH RFC] misc/at24: distinguish between eeprom and fram chips

[Lars Poeschel]

> > > > I wanted to use a fm24c04 i2c fram chip with linux. I grepped the
> > > > source and found nothing. I later found that my chip can be handled
> > > > by at24 eeprom driver. It creates a sysfs file called eeprom to
> > > > read from and write to the chip. Userspace has no chance to
> > > > distinguish if it is writing an eeprom or a fram chip.
> > > 
> > > Why should it?
> > 
> > Because writes are much faster and it doesn't have to take care on erase
> > cycles. It could use other write strategies on such devices and update
> > informations that have to survive power downs more often.
> 
> I agree. I think that a seperate attribute named e.g. 'page_size' would
> be more helpful than renaming the binary file to fram?

Yes, this is a much better solution! Adding a seperate sysfs file page_size 
and a file for the type of device which would read eeprom, fram, etc then.
If you also think this is the way to go, I would spent one of my next free 
timeslots to this.

> > > The method of accessing EEPROMs is used by way more chips than FRAMs.
> > > So, I'd prefer to have the text updated more generic like "EEPROMs and
> > > similar devices like RAMs, ROMs, etc...". Describing setting .flags in
> > > Kconfig is overkill.
> > 
> > A patch updating Kconfig is below.
> 
> Looks good from a glimpse, will apply it later.

Thank you!

Lars
--
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