Another dwiic(4) fix

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Another dwiic(4) fix

Mark Kettenis
The timeout when waiting for data to be received for polled mode is
too small for taling to the BMC on the Ampere/Lenovo arm64 server.
This bumps it to 50 ms, which is still lower than what it is for
non-polled mode.

I also found that things worked much better if we checked for the
actually STOP detection bit to come instead of looking at the activity
bit in the status register.

Also tested on my Asus x205ta, which has an i2c keyboard.

ok?


Index: dwiic.c
===================================================================
RCS file: /cvs/src/sys/dev/ic/dwiic.c,v
retrieving revision 1.6
diff -u -p -r1.6 dwiic.c
--- dwiic.c 6 Aug 2019 06:56:29 -0000 1.6
+++ dwiic.c 17 Aug 2019 16:12:39 -0000
@@ -350,7 +350,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
     sc->sc_dev.dv_xname, __func__, tx_limit, x));
 
  if (flags & I2C_F_POLL) {
- for (retries = 100; retries > 0; retries--) {
+ for (retries = 1000; retries > 0; retries--) {
  rx_avail = dwiic_read(sc, DW_IC_RXFLR);
  if (rx_avail > 0)
  break;
@@ -407,14 +407,13 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
 
  if (I2C_OP_STOP_P(op) && I2C_OP_WRITE_P(op)) {
  if (flags & I2C_F_POLL) {
- /* wait for bus to be idle */
  for (retries = 100; retries > 0; retries--) {
- st = dwiic_read(sc, DW_IC_STATUS);
- if (!(st & DW_IC_STATUS_ACTIVITY))
+ st = dwiic_read(sc, DW_IC_RAW_INTR_STAT);
+ if (st & DW_IC_INTR_STOP_DET)
  break;
  DELAY(1000);
  }
- if (st & DW_IC_STATUS_ACTIVITY)
+ if (!(st & DW_IC_INTR_STOP_DET))
  printf("%s: timed out waiting for bus idle\n",
     sc->sc_dev.dv_xname);
  } else {

Reply | Threaded
Open this post in threaded view
|

Re: Another dwiic(4) fix

Mike Larkin-2
On Sat, Aug 17, 2019 at 06:42:01PM +0200, Mark Kettenis wrote:

> The timeout when waiting for data to be received for polled mode is
> too small for taling to the BMC on the Ampere/Lenovo arm64 server.
> This bumps it to 50 ms, which is still lower than what it is for
> non-polled mode.
>
> I also found that things worked much better if we checked for the
> actually STOP detection bit to come instead of looking at the activity
> bit in the status register.
>
> Also tested on my Asus x205ta, which has an i2c keyboard.
>
> ok?
>
>

This causes no regressions on the 2 machines I tried.

Probably the right time to get it in, ok mlarkin

> Index: dwiic.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/dwiic.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 dwiic.c
> --- dwiic.c 6 Aug 2019 06:56:29 -0000 1.6
> +++ dwiic.c 17 Aug 2019 16:12:39 -0000
> @@ -350,7 +350,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>      sc->sc_dev.dv_xname, __func__, tx_limit, x));
>  
>   if (flags & I2C_F_POLL) {
> - for (retries = 100; retries > 0; retries--) {
> + for (retries = 1000; retries > 0; retries--) {
>   rx_avail = dwiic_read(sc, DW_IC_RXFLR);
>   if (rx_avail > 0)
>   break;
> @@ -407,14 +407,13 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>  
>   if (I2C_OP_STOP_P(op) && I2C_OP_WRITE_P(op)) {
>   if (flags & I2C_F_POLL) {
> - /* wait for bus to be idle */
>   for (retries = 100; retries > 0; retries--) {
> - st = dwiic_read(sc, DW_IC_STATUS);
> - if (!(st & DW_IC_STATUS_ACTIVITY))
> + st = dwiic_read(sc, DW_IC_RAW_INTR_STAT);
> + if (st & DW_IC_INTR_STOP_DET)
>   break;
>   DELAY(1000);
>   }
> - if (st & DW_IC_STATUS_ACTIVITY)
> + if (!(st & DW_IC_INTR_STOP_DET))
>   printf("%s: timed out waiting for bus idle\n",
>      sc->sc_dev.dv_xname);
>   } else {
>