* [U-Boot] [PATCH 0/2] Fix the I2C on SAMA5D3 @ 2017-11-29 3:25 Alan Ott 2017-11-29 3:25 ` [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte Alan Ott 2017-11-29 3:25 ` [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function Alan Ott 0 siblings, 2 replies; 9+ messages in thread From: Alan Ott @ 2017-11-29 3:25 UTC (permalink / raw) To: u-boot Two patches to fix the I2C on SAMA5D3. The first fixes a race condition in pushing TX data to the hardware, and the second removes an errant probe_chip function which prevents devices from being probed properly These have been tested on master and on 2017.07 Alan Ott (2): i2c: at91_i2c: Wait for TXRDY after sending the first byte i2c: at91_i2c: remove the .probe_chip function drivers/i2c/at91_i2c.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte 2017-11-29 3:25 [U-Boot] [PATCH 0/2] Fix the I2C on SAMA5D3 Alan Ott @ 2017-11-29 3:25 ` Alan Ott 2017-11-30 1:14 ` Yang, Wenyou ` (2 more replies) 2017-11-29 3:25 ` [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function Alan Ott 1 sibling, 3 replies; 9+ messages in thread From: Alan Ott @ 2017-11-29 3:25 UTC (permalink / raw) To: u-boot The driver must wait for TXRDY after each byte is pushed into the i2c FIFO before pushing the next byte. Previously this was not done for the first byte, causing a race condition with zeros sometimes being sent for the next byte (which is typically the first actual data byte). Signed-off-by: Alan Ott <alan@softiron.com> --- drivers/i2c/at91_i2c.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/i2c/at91_i2c.c b/drivers/i2c/at91_i2c.c index d394044..20d0929 100644 --- a/drivers/i2c/at91_i2c.c +++ b/drivers/i2c/at91_i2c.c @@ -72,6 +72,8 @@ static int at91_i2c_xfer_msg(struct at91_i2c_bus *bus, struct i2c_msg *msg) } else { writel(msg->buf[0], ®->thr); + ret = at91_wait_for_xfer(bus, TWI_SR_TXRDY); + for (i = 1; !ret && (i < msg->len); i++) { writel(msg->buf[i], ®->thr); ret = at91_wait_for_xfer(bus, TWI_SR_TXRDY); -- 2.9.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte 2017-11-29 3:25 ` [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte Alan Ott @ 2017-11-30 1:14 ` Yang, Wenyou 2017-12-07 8:26 ` Heiko Schocher 2017-12-07 12:21 ` Heiko Schocher 2 siblings, 0 replies; 9+ messages in thread From: Yang, Wenyou @ 2017-11-30 1:14 UTC (permalink / raw) To: u-boot On 2017/11/29 11:25, Alan Ott wrote: > The driver must wait for TXRDY after each byte is pushed into > the i2c FIFO before pushing the next byte. Previously this was > not done for the first byte, causing a race condition with zeros > sometimes being sent for the next byte (which is typically the > first actual data byte). > > Signed-off-by: Alan Ott <alan@softiron.com> > --- Acked-by:Â Wenyou Yang <wenyou.yang@microchip.com> Thank you for your patch. > drivers/i2c/at91_i2c.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/i2c/at91_i2c.c b/drivers/i2c/at91_i2c.c > index d394044..20d0929 100644 > --- a/drivers/i2c/at91_i2c.c > +++ b/drivers/i2c/at91_i2c.c > @@ -72,6 +72,8 @@ static int at91_i2c_xfer_msg(struct at91_i2c_bus *bus, struct i2c_msg *msg) > > } else { > writel(msg->buf[0], ®->thr); > + ret = at91_wait_for_xfer(bus, TWI_SR_TXRDY); > + > for (i = 1; !ret && (i < msg->len); i++) { > writel(msg->buf[i], ®->thr); > ret = at91_wait_for_xfer(bus, TWI_SR_TXRDY); Best Regards, Wenyou Yang ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte 2017-11-29 3:25 ` [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte Alan Ott 2017-11-30 1:14 ` Yang, Wenyou @ 2017-12-07 8:26 ` Heiko Schocher 2017-12-07 12:21 ` Heiko Schocher 2 siblings, 0 replies; 9+ messages in thread From: Heiko Schocher @ 2017-12-07 8:26 UTC (permalink / raw) To: u-boot Hello Alan, Am 29.11.2017 um 04:25 schrieb Alan Ott: > The driver must wait for TXRDY after each byte is pushed into > the i2c FIFO before pushing the next byte. Previously this was > not done for the first byte, causing a race condition with zeros > sometimes being sent for the next byte (which is typically the > first actual data byte). > > Signed-off-by: Alan Ott <alan@softiron.com> > --- > drivers/i2c/at91_i2c.c | 2 ++ > 1 file changed, 2 insertions(+) Thanks! Reviewed-by: Heiko Schocher <hs@denx.de> bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte 2017-11-29 3:25 ` [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte Alan Ott 2017-11-30 1:14 ` Yang, Wenyou 2017-12-07 8:26 ` Heiko Schocher @ 2017-12-07 12:21 ` Heiko Schocher 2 siblings, 0 replies; 9+ messages in thread From: Heiko Schocher @ 2017-12-07 12:21 UTC (permalink / raw) To: u-boot Hello Alan, Am 29.11.2017 um 04:25 schrieb Alan Ott: > The driver must wait for TXRDY after each byte is pushed into > the i2c FIFO before pushing the next byte. Previously this was > not done for the first byte, causing a race condition with zeros > sometimes being sent for the next byte (which is typically the > first actual data byte). > > Signed-off-by: Alan Ott <alan@softiron.com> > --- > drivers/i2c/at91_i2c.c | 2 ++ > 1 file changed, 2 insertions(+) Applied to u-boot-i2c master Thanks! bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function 2017-11-29 3:25 [U-Boot] [PATCH 0/2] Fix the I2C on SAMA5D3 Alan Ott 2017-11-29 3:25 ` [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte Alan Ott @ 2017-11-29 3:25 ` Alan Ott 2017-11-30 1:20 ` Yang, Wenyou ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Alan Ott @ 2017-11-29 3:25 UTC (permalink / raw) To: u-boot The .probe_chip function is supposed to probe an i2c device on the bus to determine whether a device is answering to a particular address. at91_i2c_probe_chip() did not do anything resembling this and always returned 0. It looks as though at91_i2c_probe_chip() was intended to be a .probe function for the controller, as it was copied-and-pasted to become at91_i2c_probe() in 0bc8f640a4d7ed. Removing the at91_i2c_probe_chip() function makes the higher layer (i2c_probe_chip()) try a zero-length read transfer to test for the presence of a device instead, which does work. Signed-off-by: Alan Ott <alan@softiron.com> --- drivers/i2c/at91_i2c.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/drivers/i2c/at91_i2c.c b/drivers/i2c/at91_i2c.c index 20d0929..7917ca1 100644 --- a/drivers/i2c/at91_i2c.c +++ b/drivers/i2c/at91_i2c.c @@ -201,27 +201,6 @@ static int at91_i2c_enable_clk(struct udevice *dev) return 0; } -static int at91_i2c_probe_chip(struct udevice *dev, uint chip, uint chip_flags) -{ - struct at91_i2c_bus *bus = dev_get_priv(dev); - struct at91_i2c_regs *reg = bus->regs; - int ret; - - ret = at91_i2c_enable_clk(dev); - if (ret) - return ret; - - writel(TWI_CR_SWRST, ®->cr); - - at91_calc_i2c_clock(dev, bus->clock_frequency); - - writel(bus->cwgr_val, ®->cwgr); - writel(TWI_CR_MSEN, ®->cr); - writel(TWI_CR_SVDIS, ®->cr); - - return 0; -} - static int at91_i2c_set_bus_speed(struct udevice *dev, unsigned int speed) { struct at91_i2c_bus *bus = dev_get_priv(dev); @@ -256,7 +235,6 @@ static int at91_i2c_ofdata_to_platdata(struct udevice *dev) static const struct dm_i2c_ops at91_i2c_ops = { .xfer = at91_i2c_xfer, - .probe_chip = at91_i2c_probe_chip, .set_bus_speed = at91_i2c_set_bus_speed, .get_bus_speed = at91_i2c_get_bus_speed, }; -- 2.9.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function 2017-11-29 3:25 ` [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function Alan Ott @ 2017-11-30 1:20 ` Yang, Wenyou 2017-12-07 8:26 ` Heiko Schocher 2017-12-07 12:22 ` Heiko Schocher 2 siblings, 0 replies; 9+ messages in thread From: Yang, Wenyou @ 2017-11-30 1:20 UTC (permalink / raw) To: u-boot On 2017/11/29 11:25, Alan Ott wrote: > The .probe_chip function is supposed to probe an i2c device on the bus to > determine whether a device is answering to a particular address. > at91_i2c_probe_chip() did not do anything resembling this and always > returned 0. > > It looks as though at91_i2c_probe_chip() was intended to be a .probe > function for the controller, as it was copied-and-pasted to become > at91_i2c_probe() in 0bc8f640a4d7ed. > > Removing the at91_i2c_probe_chip() function makes the higher layer > (i2c_probe_chip()) try a zero-length read transfer to test for the > presence of a device instead, which does work. > > Signed-off-by: Alan Ott <alan@softiron.com> > --- Acked-by: Wenyou Yang <wenyou.yang@microchip.com> Thank you for your patch. > drivers/i2c/at91_i2c.c | 22 ---------------------- > 1 file changed, 22 deletions(-) > > diff --git a/drivers/i2c/at91_i2c.c b/drivers/i2c/at91_i2c.c > index 20d0929..7917ca1 100644 > --- a/drivers/i2c/at91_i2c.c > +++ b/drivers/i2c/at91_i2c.c > @@ -201,27 +201,6 @@ static int at91_i2c_enable_clk(struct udevice *dev) > return 0; > } > > -static int at91_i2c_probe_chip(struct udevice *dev, uint chip, uint chip_flags) > -{ > - struct at91_i2c_bus *bus = dev_get_priv(dev); > - struct at91_i2c_regs *reg = bus->regs; > - int ret; > - > - ret = at91_i2c_enable_clk(dev); > - if (ret) > - return ret; > - > - writel(TWI_CR_SWRST, ®->cr); > - > - at91_calc_i2c_clock(dev, bus->clock_frequency); > - > - writel(bus->cwgr_val, ®->cwgr); > - writel(TWI_CR_MSEN, ®->cr); > - writel(TWI_CR_SVDIS, ®->cr); > - > - return 0; > -} > - > static int at91_i2c_set_bus_speed(struct udevice *dev, unsigned int speed) > { > struct at91_i2c_bus *bus = dev_get_priv(dev); > @@ -256,7 +235,6 @@ static int at91_i2c_ofdata_to_platdata(struct udevice *dev) > > static const struct dm_i2c_ops at91_i2c_ops = { > .xfer = at91_i2c_xfer, > - .probe_chip = at91_i2c_probe_chip, > .set_bus_speed = at91_i2c_set_bus_speed, > .get_bus_speed = at91_i2c_get_bus_speed, > }; Best Regards, Wenyou Yang ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function 2017-11-29 3:25 ` [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function Alan Ott 2017-11-30 1:20 ` Yang, Wenyou @ 2017-12-07 8:26 ` Heiko Schocher 2017-12-07 12:22 ` Heiko Schocher 2 siblings, 0 replies; 9+ messages in thread From: Heiko Schocher @ 2017-12-07 8:26 UTC (permalink / raw) To: u-boot Hello Alan, Am 29.11.2017 um 04:25 schrieb Alan Ott: > The .probe_chip function is supposed to probe an i2c device on the bus to > determine whether a device is answering to a particular address. > at91_i2c_probe_chip() did not do anything resembling this and always > returned 0. > > It looks as though at91_i2c_probe_chip() was intended to be a .probe > function for the controller, as it was copied-and-pasted to become > at91_i2c_probe() in 0bc8f640a4d7ed. > > Removing the at91_i2c_probe_chip() function makes the higher layer > (i2c_probe_chip()) try a zero-length read transfer to test for the > presence of a device instead, which does work. > > Signed-off-by: Alan Ott <alan@softiron.com> > --- > drivers/i2c/at91_i2c.c | 22 ---------------------- > 1 file changed, 22 deletions(-) Thanks! Reviewed-by: Heiko Schocher <hs@denx.de> bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function 2017-11-29 3:25 ` [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function Alan Ott 2017-11-30 1:20 ` Yang, Wenyou 2017-12-07 8:26 ` Heiko Schocher @ 2017-12-07 12:22 ` Heiko Schocher 2 siblings, 0 replies; 9+ messages in thread From: Heiko Schocher @ 2017-12-07 12:22 UTC (permalink / raw) To: u-boot Hello Alan, Am 29.11.2017 um 04:25 schrieb Alan Ott: > The .probe_chip function is supposed to probe an i2c device on the bus to > determine whether a device is answering to a particular address. > at91_i2c_probe_chip() did not do anything resembling this and always > returned 0. > > It looks as though at91_i2c_probe_chip() was intended to be a .probe > function for the controller, as it was copied-and-pasted to become > at91_i2c_probe() in 0bc8f640a4d7ed. > > Removing the at91_i2c_probe_chip() function makes the higher layer > (i2c_probe_chip()) try a zero-length read transfer to test for the > presence of a device instead, which does work. > > Signed-off-by: Alan Ott <alan@softiron.com> > --- > drivers/i2c/at91_i2c.c | 22 ---------------------- > 1 file changed, 22 deletions(-) Applied to u-boot-i2c master Thanks! bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-12-07 12:22 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-29 3:25 [U-Boot] [PATCH 0/2] Fix the I2C on SAMA5D3 Alan Ott 2017-11-29 3:25 ` [U-Boot] [PATCH 1/2] i2c: at91_i2c: Wait for TXRDY after sending the first byte Alan Ott 2017-11-30 1:14 ` Yang, Wenyou 2017-12-07 8:26 ` Heiko Schocher 2017-12-07 12:21 ` Heiko Schocher 2017-11-29 3:25 ` [U-Boot] [PATCH 2/2] i2c: at91_i2c: remove the .probe_chip function Alan Ott 2017-11-30 1:20 ` Yang, Wenyou 2017-12-07 8:26 ` Heiko Schocher 2017-12-07 12:22 ` Heiko Schocher
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.