| Message ID | 1372780860-12972-1-git-send-email-fabio.estevam@freescale.com |
|---|---|
| State | Deferred |
| Headers | show |
On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote: > According to mx23 erratum 2727: > > "2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. > > Description: > > When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. > However, the SDA line is read at the proper timing interval. If > RETAIN_CLOCK is cleared, the ninth clock pulse is generated. > Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. > It should be 1.3. > > Workaround: > HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to > enable the fix for this issue." > > It has also been noticed that mx28 needs to implement this fix in order to have > SMBus to work properly. > > Reported-by: Christoph Baumann <cb@sgoc.de> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> Did you see this making the driver handle some situations that caused failure before? > --- > drivers/i2c/busses/i2c-mxs.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c > index 6d8094d..ce7ac86 100644 > --- a/drivers/i2c/busses/i2c-mxs.c > +++ b/drivers/i2c/busses/i2c-mxs.c > @@ -56,6 +56,7 @@ > #define MXS_I2C_CTRL1_CLR (0x48) > > #define MXS_I2C_CTRL1_CLR_GOT_A_NAK 0x10000000 > +#define MXS_I2C_CTRL1_ACK_MODE 0x08000000 > #define MXS_I2C_CTRL1_BUS_FREE_IRQ 0x80 > #define MXS_I2C_CTRL1_DATA_ENGINE_CMPLT_IRQ 0x40 > #define MXS_I2C_CTRL1_NO_SLAVE_ACK_IRQ 0x20 > @@ -140,6 +141,14 @@ static void mxs_i2c_reset(struct mxs_i2c_dev *i2c) > writel(0x00300030, i2c->regs + MXS_I2C_TIMING2); > > writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET); > + > + /* > + * According to mx23 erratum 2727: > + * "I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set" > + * > + * HW_I2C_CTRL1[ACK_MODE] needs to be set when RETAIN_CLOCK is set. > + */ > + writel(MXS_I2C_CTRL1_ACK_MODE, i2c->regs + MXS_I2C_CTRL1_SET); So you set this bis in the reset routine. The first thing in mxs_i2c_pio_setup_xfer is however: writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_CLR); which unsets the ACK_MODE bit. Also when reading the docs I couldn't find out the motivation to set RETAIN_CLOCK at all in the select command. Best regards Uwe
Hi Uwe, On 07/02/2013 03:11 PM, Uwe Kleine-König wrote: > On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote: >> According to mx23 erratum 2727: >> >> "2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. >> >> Description: >> >> When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. >> However, the SDA line is read at the proper timing interval. If >> RETAIN_CLOCK is cleared, the ninth clock pulse is generated. >> Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. >> It should be 1.3. >> >> Workaround: >> HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to >> enable the fix for this issue." >> >> It has also been noticed that mx28 needs to implement this fix in order to have >> SMBus to work properly. >> >> Reported-by: Christoph Baumann <cb@sgoc.de> >> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > Did you see this making the driver handle some situations that caused > failure before? No, I haven't. I saw the report from Christoph in the linux-arm-kernel mailing list: http://marc.info/?l=linux-arm-kernel&m=137277422127826&w=2 And thought it could be nice if we could get it fixed for mx23 and mx28. > >> --- >> drivers/i2c/busses/i2c-mxs.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c >> index 6d8094d..ce7ac86 100644 >> --- a/drivers/i2c/busses/i2c-mxs.c >> +++ b/drivers/i2c/busses/i2c-mxs.c >> @@ -56,6 +56,7 @@ >> #define MXS_I2C_CTRL1_CLR (0x48) >> >> #define MXS_I2C_CTRL1_CLR_GOT_A_NAK 0x10000000 >> +#define MXS_I2C_CTRL1_ACK_MODE 0x08000000 >> #define MXS_I2C_CTRL1_BUS_FREE_IRQ 0x80 >> #define MXS_I2C_CTRL1_DATA_ENGINE_CMPLT_IRQ 0x40 >> #define MXS_I2C_CTRL1_NO_SLAVE_ACK_IRQ 0x20 >> @@ -140,6 +141,14 @@ static void mxs_i2c_reset(struct mxs_i2c_dev *i2c) >> writel(0x00300030, i2c->regs + MXS_I2C_TIMING2); >> >> writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET); >> + >> + /* >> + * According to mx23 erratum 2727: >> + * "I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set" >> + * >> + * HW_I2C_CTRL1[ACK_MODE] needs to be set when RETAIN_CLOCK is set. >> + */ >> + writel(MXS_I2C_CTRL1_ACK_MODE, i2c->regs + MXS_I2C_CTRL1_SET); > So you set this bis in the reset routine. The first thing in > mxs_i2c_pio_setup_xfer is however: > > writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_CLR); > > which unsets the ACK_MODE bit. Also when reading the docs I couldn't Not really. This write is via MXS_I2C_CTRL1_CLR register and does not touch the ACK_MODE bit. Keep in mind that mxs has separate registers from write and read to same register. > find out the motivation to set RETAIN_CLOCK at all in the select > command. I tried to not set RETAIN_CLOCK bit and the sgtl5000 codec failed to probe. Regards, Fabio Estevam -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Fabio Estevam, > Hi Uwe, > > On 07/02/2013 03:11 PM, Uwe Kleine-König wrote: > > On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote: > >> According to mx23 erratum 2727: > >> > >> "2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. > >> > >> Description: > >> > >> When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. > >> However, the SDA line is read at the proper timing interval. If > >> RETAIN_CLOCK is cleared, the ninth clock pulse is generated. > >> Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. > >> It should be 1.3. > >> > >> Workaround: > >> HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to > >> enable the fix for this issue." > >> > >> It has also been noticed that mx28 needs to implement this fix in order > >> to have SMBus to work properly. > >> > >> Reported-by: Christoph Baumann <cb@sgoc.de> > >> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > > > > Did you see this making the driver handle some situations that caused > > failure before? > > No, I haven't. I saw the report from Christoph in the linux-arm-kernel > mailing list: > http://marc.info/?l=linux-arm-kernel&m=137277422127826&w=2 > > And thought it could be nice if we could get it fixed for mx23 and mx28. How/when does this error manifest on the scope/LA? Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Marek, > > No, I haven't. I saw the report from Christoph in the linux-arm-kernel > > mailing list: > > http://marc.info/?l=linux-arm-kernel&m=137277422127826&w=2 > > > > And thought it could be nice if we could get it fixed for mx23 and mx28. > > How/when does this error manifest on the scope/LA? the problem turned up when Uwe Kleine-König worked on implementing SMBus and I2C_M_RECV_LEN support for i.MX28 (my employer commissioned Pengutronix in that case). The receiving of such messages stops after the first (length information) byte. Maybe Uwe can comment on that. Regards Christoph -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Wed, Jul 03, 2013 at 03:34:12PM +0200, Christoph G. Baumann wrote: > > > No, I haven't. I saw the report from Christoph in the linux-arm-kernel > > > mailing list: > > > http://marc.info/?l=linux-arm-kernel&m=137277422127826&w=2 > > > > > > And thought it could be nice if we could get it fixed for mx23 and mx28. > > > > How/when does this error manifest on the scope/LA? > > the problem turned up when Uwe Kleine-König worked on implementing SMBus and > I2C_M_RECV_LEN support for i.MX28 (my employer commissioned Pengutronix in that > case). The receiving of such messages stops after the first (length information) > byte. > Maybe Uwe can comment on that. OK, I'm trying to sum up: To support I2C_M_RECV_LEN in the mxs driver I did: 1 // prepare sending SAD+R 2 CTRL0 = RUN | RETAIN_CLOCK | PRE_SEND_START | MASTER_MODE | DIRECTION | XFER_COUNT(1); 3 poll DEBUG0 until DMAREQ is set; 4 // send SAD+R 5 DATA = addr_data 6 // prepare reading length byte 7 CTRL0 = RUN | RETAIN_CLOCK | MASTER_MODE | XFER_COUNT(1); 8 poll DEBUG0 until DMAREQ is set; 9 // read first data indicating length 10 data = DATA & 0xff 11 // send an ack, i.e. clean CTRL0_CLOCK_HELD 12 CTRL0 = RUN | ACKNOWLEDGE | MASTER_MODE 13 sleep 1ms 14 // trigger reading rest of the message 15 CTRL0 = RUN | SEND_NAK_ON_LAST | MASTER_MODE | MXS_I2C_CTRL0_POST_SEND_STOP 16 for (i = 0; i < (data + 3) / 4; ++i) 17 read from DATA In line 10 the length bit is read out successfully, but sending the ACK in line 12 doesn't work, the i.MX28 pulls SDA low, but doesn't generate a clock pulse on SCL and instead releases SDA and starts reading the following byte. Dropping RETAIN_CLOCK in line 7 and/or dropping RUN from line 12 didn't help. Freescale's support suggested to set CTRL1.ACK_MODE and some other things that didn't help and pointed out the imx23 i2c errata. (I.e. "when RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. However, the SDA line is read at the proper timing interval. If RETAIN_CLOCK is cleared, the ninth clock pulse is generated.") The suggested workaround was to either use a Big buffer, read Many bytes until the slave sends a NACK and interpret the result as smaller read. Or use gpio bit banging. I didn't understand the suggested workaround in the errata document, and the support guy didn't succeed to explain it to me. "The suggested workaround in this errata is to set the ACK_MODE bit in HW_I2C_CTRL1 register. In i.MX233, this bit is default zero and requires software to explicitly set it to 1. In i.MX28, this bit is '1' by default already. Unfortunately, this does not solve the 9th clock pulse issue if RETAIN_CLOCK is set in the receiving data phase." Best regards Uwe
Dear Uwe Kleine-König, > Hello, > > On Wed, Jul 03, 2013 at 03:34:12PM +0200, Christoph G. Baumann wrote: > > > > No, I haven't. I saw the report from Christoph in the > > > > linux-arm-kernel mailing list: > > > > http://marc.info/?l=linux-arm-kernel&m=137277422127826&w=2 > > > > > > > > And thought it could be nice if we could get it fixed for mx23 and > > > > mx28. > > > > > > How/when does this error manifest on the scope/LA? > > > > the problem turned up when Uwe Kleine-König worked on implementing SMBus > > and I2C_M_RECV_LEN support for i.MX28 (my employer commissioned > > Pengutronix in that case). The receiving of such messages stops after > > the first (length information) byte. > > Maybe Uwe can comment on that. > > OK, I'm trying to sum up: To support I2C_M_RECV_LEN in the mxs driver I > did: See the other patch I sent , esp. the write PIO command [1], then the order would be: 1 // prepare sending SAD+R 2 CTRL0 = RETAIN_CLOCK | PRE_SEND_START | MASTER_MODE | DIRECTION | XFER_COUNT(1); 3 DATA = addr_data 4 CTRL0 |= RUN ^ this stuff is explained in MX23 RM, see the comment above mxs_i2c_pio_trigger_write_cmd() in the patch. > 6 // prepare reading length byte > 7 CTRL0 = RUN | RETAIN_CLOCK | MASTER_MODE | XFER_COUNT(1); I think you can force the controller to send ACK here automatically. > 8 poll DEBUG0 until DMAREQ is set; DMAREQ doesn't work in READ xfer :-( > 9 // read first data indicating length > 10 data = DATA & 0xff > 11 // send an ack, i.e. clean CTRL0_CLOCK_HELD > 12 CTRL0 = RUN | ACKNOWLEDGE | MASTER_MODE > 13 sleep 1ms See above, also don't use RETAIN_CLOCK above then. > 14 // trigger reading rest of the message > 15 CTRL0 = RUN | SEND_NAK_ON_LAST | MASTER_MODE | > MXS_I2C_CTRL0_POST_SEND_STOP 16 for (i = 0; i < (data + 3) / 4; > ++i) > 17 read from DATA Use DMA here, you can't PIO READ more than 4 bytes. > In line 10 the length bit is read out successfully, but sending the ACK > in line 12 doesn't work, the i.MX28 pulls SDA low, but doesn't generate > a clock pulse on SCL and instead releases SDA and starts reading the > following byte. > > Dropping RETAIN_CLOCK in line 7 and/or dropping RUN from line 12 didn't > help. > > Freescale's support suggested to set CTRL1.ACK_MODE and some other > things that didn't help and pointed out the imx23 i2c errata. (I.e. > "when RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. > However, the SDA line is read at the proper timing interval. If > RETAIN_CLOCK is cleared, the ninth clock pulse is generated.") > > The suggested workaround was to either use a Big buffer, read Many bytes > until the slave sends a NACK and interpret the result as smaller read. > Or use gpio bit banging. I suspect is likely doable, but it'd need non-trivial amount of fiddling. Unless I get motivated enough to implement this, I'm not plumbing in it. Rather than that, I'd like to find out what's wrong with the PIO on MX23. > I didn't understand the suggested workaround in the errata document, and > the support guy didn't succeed to explain it to me. "The suggested > workaround in this errata is to set the ACK_MODE bit in HW_I2C_CTRL1 > register. In i.MX233, this bit is default zero and requires software to > explicitly set it to 1. In i.MX28, this bit is '1' by default already. > Unfortunately, this does not solve the 9th clock pulse issue if > RETAIN_CLOCK is set in the receiving data phase." > > Best regards > Uwe [1] http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg12699.html Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote: > According to mx23 erratum 2727: > > "2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. > > Description: > > When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. > However, the SDA line is read at the proper timing interval. If > RETAIN_CLOCK is cleared, the ninth clock pulse is generated. > Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. > It should be 1.3. > > Workaround: > HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to > enable the fix for this issue." > > It has also been noticed that mx28 needs to implement this fix in order to have > SMBus to work properly. > > Reported-by: Christoph Baumann <cb@sgoc.de> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> What's with this one? Bogus? Needed? Still needed after PIO rework?
Dear Wolfram Sang, > On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote: > > According to mx23 erratum 2727: > > > > "2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. > > > > Description: > > > > When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. > > However, the SDA line is read at the proper timing interval. If > > RETAIN_CLOCK is cleared, the ninth clock pulse is generated. > > Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. > > It should be 1.3. > > > > Workaround: > > HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to > > enable the fix for this issue." > > > > It has also been noticed that mx28 needs to implement this fix in order > > to have SMBus to work properly. > > > > Reported-by: Christoph Baumann <cb@sgoc.de> > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > > What's with this one? Bogus? Needed? Still needed after PIO rework? I dont think this is needed on MX28 as this is set by default. On MX23, I dont see any difference with this enabled/disabled. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/15/2013 07:08 AM, Wolfram Sang wrote: > On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote: >> According to mx23 erratum 2727: >> >> "2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. >> >> Description: >> >> When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. >> However, the SDA line is read at the proper timing interval. If >> RETAIN_CLOCK is cleared, the ninth clock pulse is generated. >> Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. >> It should be 1.3. >> >> Workaround: >> HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to >> enable the fix for this issue." >> >> It has also been noticed that mx28 needs to implement this fix in order to have >> SMBus to work properly. >> >> Reported-by: Christoph Baumann <cb@sgoc.de> >> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > > What's with this one? Bogus? Needed? Still needed after PIO rework? According to the mx23 erratum 2727 this patch is needed. Regards, Fabio Estevam -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Mon, Aug 19, 2013 at 09:19:01AM -0300, Fabio Estevam wrote: > On 08/15/2013 07:08 AM, Wolfram Sang wrote: > >On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote: > >>According to mx23 erratum 2727: > >> > >>"2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. > >> > >>Description: > >> > >>When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. > >>However, the SDA line is read at the proper timing interval. If > >>RETAIN_CLOCK is cleared, the ninth clock pulse is generated. > >>Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. > >>It should be 1.3. > >> > >>Workaround: > >>HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to > >>enable the fix for this issue." > >> > >>It has also been noticed that mx28 needs to implement this fix in order to have > >>SMBus to work properly. > >> > >>Reported-by: Christoph Baumann <cb@sgoc.de> > >>Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > > > >What's with this one? Bogus? Needed? Still needed after PIO rework? > > According to the mx23 erratum 2727 this patch is needed. Last time I worked with i2c stuff on mxs my impression was the mx23 erratum 2727 is non-sense. Having HW_I2C_CTRL1[ACK_MODE] set or not didn't made any difference in my tests. I suspect that the description isn't complete. So the driver does already use RETAIN_CLOCK (for the select phase) which up to know didn't made any problems on mx23 even without having ACK_MODE=1. The statement about the HW_I2C_VERSION register appears just misplaced there. So I'd vote for not taking this patch until there is a better understanding of the eventual problem. Best regards Uwe
> According to the mx23 erratum 2727 this patch is needed.
Have you measured it yourself?
On Tue, Aug 20, 2013 at 4:04 PM, Wolfram Sang <wsa@the-dreams.de> wrote: > >> According to the mx23 erratum 2727 this patch is needed. > > Have you measured it yourself? No, only relying on the mx23 errata doc here. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Have you measured it yourself? > > No, only relying on the mx23 errata doc here. Only send patches when you KNOW what you are changing, i.e. you verified to the best of your knowledge. Really! Docs have so many bugs... Honestly, I hate dealing with patches which are more or less guesses. They offload the Q&A to me and I surely have enough own tasks to do...
On Tue, Aug 20, 2013 at 4:20 PM, Wolfram Sang <wsa@the-dreams.de> wrote: > >> > Have you measured it yourself? >> >> No, only relying on the mx23 errata doc here. > > Only send patches when you KNOW what you are changing, i.e. you verified > to the best of your knowledge. Really! Docs have so many bugs... > > Honestly, I hate dealing with patches which are more or less guesses. > They offload the Q&A to me and I surely have enough own tasks to do... Ok. sir. Will investigate more about this erratum entry. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Uwe Kleine-König, > Hello, > > On Mon, Aug 19, 2013 at 09:19:01AM -0300, Fabio Estevam wrote: > > On 08/15/2013 07:08 AM, Wolfram Sang wrote: > > >On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote: > > >>According to mx23 erratum 2727: > > >> > > >>"2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. > > >> > > >>Description: > > >> > > >>When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. > > >>However, the SDA line is read at the proper timing interval. If > > >>RETAIN_CLOCK is cleared, the ninth clock pulse is generated. > > >>Also, the HW_I2C_VERSION register incorrectly states the version is > > >>1.2. It should be 1.3. > > >> > > >>Workaround: > > >>HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to > > >>enable the fix for this issue." > > >> > > >>It has also been noticed that mx28 needs to implement this fix in order > > >>to have SMBus to work properly. > > >> > > >>Reported-by: Christoph Baumann <cb@sgoc.de> > > >>Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > > > > > >What's with this one? Bogus? Needed? Still needed after PIO rework? > > > > According to the mx23 erratum 2727 this patch is needed. > > Last time I worked with i2c stuff on mxs my impression was the mx23 > erratum 2727 is non-sense. Having HW_I2C_CTRL1[ACK_MODE] set or not > didn't made any difference in my tests. Yeah, I can confirm this one. > I suspect that the description isn't complete. So the driver does > already use RETAIN_CLOCK (for the select phase) which up to know didn't > made any problems on mx23 even without having ACK_MODE=1. > > The statement about the HW_I2C_VERSION register appears just misplaced > there. > > So I'd vote for not taking this patch until there is a better > understanding of the eventual problem. I was doing some pretty thorough test when doing the PIO rework and watched the bus with LA, but didn't notice any difference whether the bit was or wasnt set. So either way WFM. On MX28 though, the bit is set after the I2C block is reset, on MX23 it is unset, that's the only thing I observed. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c index 6d8094d..ce7ac86 100644 --- a/drivers/i2c/busses/i2c-mxs.c +++ b/drivers/i2c/busses/i2c-mxs.c @@ -56,6 +56,7 @@ #define MXS_I2C_CTRL1_CLR (0x48) #define MXS_I2C_CTRL1_CLR_GOT_A_NAK 0x10000000 +#define MXS_I2C_CTRL1_ACK_MODE 0x08000000 #define MXS_I2C_CTRL1_BUS_FREE_IRQ 0x80 #define MXS_I2C_CTRL1_DATA_ENGINE_CMPLT_IRQ 0x40 #define MXS_I2C_CTRL1_NO_SLAVE_ACK_IRQ 0x20 @@ -140,6 +141,14 @@ static void mxs_i2c_reset(struct mxs_i2c_dev *i2c) writel(0x00300030, i2c->regs + MXS_I2C_TIMING2); writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET); + + /* + * According to mx23 erratum 2727: + * "I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set" + * + * HW_I2C_CTRL1[ACK_MODE] needs to be set when RETAIN_CLOCK is set. + */ + writel(MXS_I2C_CTRL1_ACK_MODE, i2c->regs + MXS_I2C_CTRL1_SET); } static void mxs_i2c_dma_finish(struct mxs_i2c_dev *i2c)
According to mx23 erratum 2727: "2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set. Description: When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. However, the SDA line is read at the proper timing interval. If RETAIN_CLOCK is cleared, the ninth clock pulse is generated. Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. It should be 1.3. Workaround: HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to enable the fix for this issue." It has also been noticed that mx28 needs to implement this fix in order to have SMBus to work properly. Reported-by: Christoph Baumann <cb@sgoc.de> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> --- drivers/i2c/busses/i2c-mxs.c | 9 +++++++++ 1 file changed, 9 insertions(+)