[PATCH 2/2] i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED

2020-10-30 Thread Michael Wu 
Sometimes we would get the following flow when doing an i2cset:

0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
I2C_SLAVE_WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
I2C_SLAVE_WRITE_REQUESTED
I2C_SLAVE_WRITE_RECEIVED

Documentation/i2c/slave-interface.rst says that I2C_SLAVE_WRITE_REQUESTED,
which is mandatory, should be sent while the data did not arrive yet. It
means in a write-request I2C_SLAVE_WRITE_REQUESTED should be reported
before any I2C_SLAVE_WRITE_RECEIVED.

By the way, I2C_SLAVE_STOP didn't be reported in the above case because
DW_IC_INTR_STAT was not 0x200.

dev->status can be used to record the current state, especially Designware
I2C controller has no interrupts to identify a write-request. This patch
makes not only I2C_SLAVE_WRITE_REQUESTED been reported first when
IC_INTR_RX_FULL is rising and dev->status isn't STATUS_WRITE_IN_PROGRESS
but also I2C_SLAVE_STOP been reported when a STOP condition is received.

Signed-off-by: Michael Wu 
---
 drivers/i2c/busses/i2c-designware-slave.c | 45 +--
 1 file changed, 18 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 13de01a0f75f..0d15f4c1e9f7 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -172,26 +172,25 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
 
-   if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
-   i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
+   if (stat & DW_IC_INTR_RX_FULL) {
+   if (dev->status != STATUS_WRITE_IN_PROGRESS) {
+   dev->status = STATUS_WRITE_IN_PROGRESS;
+   i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED,
+   &val);
+   }
+
+   regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+   val = tmp;
+   if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
+&val))
+   dev_vdbg(dev->dev, "Byte %X acked!", val);
+   }
 
if (stat & DW_IC_INTR_RD_REQ) {
if (slave_activity) {
-   if (stat & DW_IC_INTR_RX_FULL) {
-   regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
-   val = tmp;
-
-   if (!i2c_slave_event(dev->slave,
-I2C_SLAVE_WRITE_RECEIVED,
-&val)) {
-   dev_vdbg(dev->dev, "Byte %X acked!",
-val);
-   }
-   regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   } else {
-   regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
-   }
+   regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
+
+   dev->status = STATUS_READ_IN_PROGRESS;
if (!i2c_slave_event(dev->slave,
 I2C_SLAVE_READ_REQUESTED,
 &val))
@@ -203,18 +202,10 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
 &val))
regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
-
-   i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   return 1;
}
 
-   if (stat & DW_IC_INTR_RX_FULL) {
-   regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
-   val = tmp;
-   if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
-&val))
-   dev_vdbg(dev->dev, "Byte %X acked!", val);
-   } else {
+   if (stat & DW_IC_INTR_STOP_DET) {
+   dev->status = STATUS_IDLE;
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
}
 
-- 
2.17.1

[PATCH 1/2] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once

2020-10-30 Thread Michael Wu 
If some bits were cleared by i2c_dw_read_clear_intrbits_slave() in
i2c_dw_isr_slave() and not handled immediately, those cleared bits would
not be shown again by later i2c_dw_read_clear_intrbits_slave(). They
therefore were forgotten to be handled.

i2c_dw_read_clear_intrbits_slave() should be called once in an ISR and take
its returned state for all later handlings.

Signed-off-by: Michael Wu 
Acked-by: Jarkko Nikula 
---
 drivers/i2c/busses/i2c-designware-slave.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 44974b53a626..13de01a0f75f 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -159,7 +159,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
 
-   regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
regmap_read(dev->map, DW_IC_STATUS, &tmp);
@@ -168,6 +167,7 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
 
+   stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
@@ -188,11 +188,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 val);
}
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
} else {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
if (!i2c_slave_event(dev->slave,
 I2C_SLAVE_READ_REQUESTED,
@@ -207,7 +205,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
 
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
return 1;
}
 
@@ -219,7 +216,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
dev_vdbg(dev->dev, "Byte %X acked!", val);
} else {
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
 
return 1;
@@ -230,7 +226,6 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void 
*dev_id)
struct dw_i2c_dev *dev = dev_id;
int ret;
 
-   i2c_dw_read_clear_intrbits_slave(dev);
ret = i2c_dw_irq_handler_slave(dev);
if (ret > 0)
complete(&dev->cmd_complete);
-- 
2.17.1

[PATCH 0/2] Designware I2C slave confusing IC_INTR_STOP_DET handle

2020-10-30 Thread Michael Wu 
When an I2C slave works, sometimes both IC_INTR_RX_FULL and
IC_INTR_STOP_DET may be rising during an IRQ handle, especially when
system is busy or too late to handle interrupts.

If IC_INTR_RX_FULL is rising and the system doesn't handle immediately,
IC_INTR_STOP_DET may be rising and the system has to handle these two
events. For this there may be two problems:

1. IC_INTR_STOP_DET is rising after i2c_dw_read_clear_intrbits_slave()
   done: It seems invalidated because I2C_SLAVE_WRITE_REQUESTED is
   reported after the 1st I2C_SLAVE_WRITE_RECEIVED.

$ i2cset -f -y 2 0x42 0x00 0x41; dmesg -c
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
I2C_SLAVE_WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : 
INTR_STAT=0x204
I2C_SLAVE_WRITE_REQUESTED
I2C_SLAVE_WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x710 : 
INTR_STAT=0x200
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x510 : 
INTR_STAT=0x0
I2C_SLAVE_STOP
[2][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x510 : 
INTR_STAT=0x0

  t1: ISR with the 1st IC_INTR_RX_FULL.
  t2: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave().
  t3: Enter i2c_dw_irq_handler_slave() and then report
  I2C_SLAVE_WRITE_RECEIVED due to 'if (stat & DW_IC_INTR_RX_FULL)'
  matched.
  t4: ISR with the 2nd IC_INTR_RX_FULL.
  t5: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave() while
  IC_INTR_STOP_DET has not risen yet.
  t6: IC_INTR_STOP_DET is rising after entering i2c_dw_irq_handler_slave().
  The driver reports I2C_SLAVE_WRITE_REQUESTED first due to
  'if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))'
  matched and then reports I2C_SLAVE_WRITE_RECEIVED.
  t7: Reports I2C_SLAVE_STOP due to IC_INTR_STOP_DET not be cleared yet.

2. Both IC_INTR_STOP_DET and IC_INTR_RX_FULL are rising before
   i2c_dw_read_clear_intrbits_slave(): I2C_SLAVE_STOP never be reported
   because IC_INTR_STOP_DET was cleared by
   i2c_dw_read_clear_intrbits_slave().

$ i2cset -f -y 2 0x42 0x00 0x41; dmesg -c
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
I2C_SLAVE_WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : 
INTR_STAT=0x204
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
I2C_SLAVE_WRITE_RECEIVED

  t1: ISR with the 1st IC_INTR_RX_FULL.
  t2: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave().
  t3: Enter i2c_dw_irq_handler_slave() and then report
  I2C_SLAVE_WRITE_RECEIVED due to 'if (stat & DW_IC_INTR_RX_FULL)'
  matched.
  t4: ISR with both IC_INTR_STOP_DET and the 2nd IC_INTR_RX_FULL.
  t5: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave().
  The current IC_INTR_STOP_DET was cleared by this
  i2c_dw_read_clear_intrbits_slave().
  t6: Enter i2c_dw_irq_handler_slave() and then report
  i2c_slave_event(WRITE_RECEIVED) due to
  'if (stat & DW_IC_INTR_RX_FULL)' matched.
  t7: I2C_SLAVE_STOP never be reported because IC_INTR_STOP_DET was
  cleared in t5.

In order to resolve these problems, i2c_dw_read_clear_intrbits_slave()
should be called only once in an ISR and take the returned stat to handle
those occurred events. The ISR handling has to be adjusted to conform event
reporting described in Documentation/i2c/slave-interface.rst.

Michael Wu (2):
  i2c: designware: call i2c_dw_read_clear_intrbits_slave() once
  i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED

 drivers/i2c/busses/i2c-designware-slave.c | 52 +--
 1 file changed, 19 insertions(+), 33 deletions(-)

-- 
2.17.1

[PATCH v3] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once

2020-10-22 Thread Michael Wu 
If some bits were cleared by i2c_dw_read_clear_intrbits_slave() in
i2c_dw_isr_slave() and not handled immediately, those cleared bits would
not be shown again by later i2c_dw_read_clear_intrbits_slave(). They
therefore were forgotten to be handled.

i2c_dw_read_clear_intrbits_slave() should be called once in an ISR and take
its returned state for all later handlings.

Signed-off-by: Michael Wu 
---

Change in v3:
 - revert deleted braces of 'else' branch in v2

Change in v2:
 - revert moving I2C_SLAVE_WRITE_REQUESTED reporting in v1

 drivers/i2c/busses/i2c-designware-slave.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 44974b53a626..13de01a0f75f 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -159,7 +159,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
 
-   regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
regmap_read(dev->map, DW_IC_STATUS, &tmp);
@@ -168,6 +167,7 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
 
+   stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
@@ -188,11 +188,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 val);
}
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
} else {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
if (!i2c_slave_event(dev->slave,
 I2C_SLAVE_READ_REQUESTED,
@@ -207,7 +205,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
 
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
return 1;
}
 
@@ -219,7 +216,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
dev_vdbg(dev->dev, "Byte %X acked!", val);
} else {
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
 
return 1;
@@ -230,7 +226,6 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void 
*dev_id)
struct dw_i2c_dev *dev = dev_id;
int ret;
 
-   i2c_dw_read_clear_intrbits_slave(dev);
ret = i2c_dw_irq_handler_slave(dev);
if (ret > 0)
complete(&dev->cmd_complete);
-- 
2.17.1

[PATCH v2] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once

2020-10-21 Thread Michael Wu 
If some bits were cleared by i2c_dw_read_clear_intrbits_slave() in
i2c_dw_isr_slave() and not handled immediately, those cleared bits would
not be shown again by later i2c_dw_read_clear_intrbits_slave(). They
therefore were forgotten to be handled.

i2c_dw_read_clear_intrbits_slave() should be called once in an ISR and take
its returned state for all later handlings.

Signed-off-by: Michael Wu 
---

Changes in v2:
 - revert moving I2C_SLAVE_WRITE_REQUESTED reporting

 drivers/i2c/busses/i2c-designware-slave.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 44974b53a626..8eced99b7aeb 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -159,7 +159,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
 
-   regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
regmap_read(dev->map, DW_IC_STATUS, &tmp);
@@ -168,6 +167,7 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
 
+   stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
@@ -188,11 +188,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 val);
}
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
} else {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
if (!i2c_slave_event(dev->slave,
 I2C_SLAVE_READ_REQUESTED,
@@ -207,7 +205,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
 
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
return 1;
}
 
@@ -217,10 +214,8 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
 &val))
dev_vdbg(dev->dev, "Byte %X acked!", val);
-   } else {
+   } else
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
-   }
 
return 1;
 }
@@ -230,7 +225,6 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void 
*dev_id)
struct dw_i2c_dev *dev = dev_id;
int ret;
 
-   i2c_dw_read_clear_intrbits_slave(dev);
ret = i2c_dw_irq_handler_slave(dev);
if (ret > 0)
complete(&dev->cmd_complete);
-- 
2.17.1

[PATCH] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once

2020-10-20 Thread Michael Wu 
i2c_dw_read_clear_intrbits_slave() was called per each interrupt handle.
It caused some interrupt bits which haven't been handled yet were cleared,
the corresponding handlers would do nothing due to interrupt bits been
discarded. For example,

$ i2cset -f -y 2 0x42 0x00 0x41; dmesg -c
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : 
INTR_STAT=0x204
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
WRITE_RECEIVED

  t1: ISR with the 1st IC_INTR_RX_FULL.
  t2: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave().
  t3: Enter i2c_dw_irq_handler_slave() and then do
  i2c_slave_event(WRITE_RECEIVED) because
  if (stat & DW_IC_INTR_RX_FULL).
  t4: ISR with both IC_INTR_STOP_DET and the 2nd IC_INTR_RX_FULL.
  t5: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave(). The
  current IC_INTR_STOP_DET is cleared by this
  i2c_dw_read_clear_intrbits_slave().
  t6: Enter i2c_dw_irq_handler_slave() and then do
  i2c_slave_event(WRITE_RECEIVED) because
  if (stat & DW_IC_INTR_RX_FULL).
  t7: i2c_slave_event(STOP) never be done because IC_INTR_STOP_DET was
  cleared in t5.

The root cause is that i2c_dw_read_clear_intrbits_slave() was called many
times. Calling i2c_dw_read_clear_intrbits_slave() once in one ISR and take
the returned stat for later handling is the solution.

Signed-off-by: Michael Wu 
---
 drivers/i2c/busses/i2c-designware-slave.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 44974b53a626..02e7c5171827 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -159,7 +159,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
 
-   regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
regmap_read(dev->map, DW_IC_STATUS, &tmp);
@@ -168,13 +167,11 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
 
+   stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
 
-   if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
-   i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
-
if (stat & DW_IC_INTR_RD_REQ) {
if (slave_activity) {
if (stat & DW_IC_INTR_RX_FULL) {
@@ -188,11 +185,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 val);
}
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
} else {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
if (!i2c_slave_event(dev->slave,
 I2C_SLAVE_READ_REQUESTED,
@@ -207,7 +202,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
 
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
return 1;
}
 
@@ -217,10 +211,11 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
 &val))
dev_vdbg(dev->dev, "Byte %X acked!", val);
-   } else {
+   } else
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
-   }
+
+   if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
+   i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
 
return 1;
 }
@@ -230,7 +225,6 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void

[PATCH 1/2] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once

2020-10-15 Thread Michael Wu 
i2c_dw_read_clear_intrbits_slave() was called per each interrupt handle.
It caused some interrupt bits which haven't been handled yet were cleared,
the corresponding handlers would do nothing due to interrupt bits been
discarded. For example,

$ i2cset -f -y 2 0x42 0x00 0x41; dmesg -c
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : 
INTR_STAT=0x204
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
WRITE_RECEIVED

  t1: ISR with the 1st IC_INTR_RX_FULL.
  t2: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave().
  t3: Enter i2c_dw_irq_handler_slave() and then do
  i2c_slave_event(WRITE_RECEIVED) because
  if (stat & DW_IC_INTR_RX_FULL).
  t4: ISR with both IC_INTR_STOP_DET and the 2nd IC_INTR_RX_FULL.
  t5: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave(). The
  current IC_INTR_STOP_DET is cleared by this
  i2c_dw_read_clear_intrbits_slave().
  t6: Enter i2c_dw_irq_handler_slave() and then do
  i2c_slave_event(WRITE_RECEIVED) because
  if (stat & DW_IC_INTR_RX_FULL).
  t7: i2c_slave_event(STOP) never be done because IC_INTR_STOP_DET was
  cleared in t5.

The root cause is that i2c_dw_read_clear_intrbits_slave() was called many
times. Calling i2c_dw_read_clear_intrbits_slave() once in one ISR and take
the returned stat for later handling is the solution.

Signed-off-by: Michael Wu 
---
 drivers/i2c/busses/i2c-designware-slave.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 44974b53a626..02e7c5171827 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -159,7 +159,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
 
-   regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
regmap_read(dev->map, DW_IC_STATUS, &tmp);
@@ -168,13 +167,11 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
 
+   stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
 
-   if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
-   i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
-
if (stat & DW_IC_INTR_RD_REQ) {
if (slave_activity) {
if (stat & DW_IC_INTR_RX_FULL) {
@@ -188,11 +185,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 val);
}
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
} else {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
if (!i2c_slave_event(dev->slave,
 I2C_SLAVE_READ_REQUESTED,
@@ -207,7 +202,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
 
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
return 1;
}
 
@@ -217,10 +211,11 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
 &val))
dev_vdbg(dev->dev, "Byte %X acked!", val);
-   } else {
+   } else
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
-   }
+
+   if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
+   i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
 
return 1;
 }
@@ -230,7 +225,6 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void

[PATCH 2/2] i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED

2020-10-15 Thread Michael Wu 
Sometime we would get the following flows when doing an i2cset:

1. No any WRITE_REQUESTED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x710 : INTR_STAT=0x200
STOP

2. WRITE_REQUESTED after WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
WRITE_RECEIVED
WRITE_REQUESTED

Documentation/i2c/slave-interface.rst said that I2C_SLAVE_WRITE_REQUESTED,
which is mandatory, comes without any data arrived. It means
I2C_SLAVE_WRITE_REQUESTED should be in front of any
I2C_SLAVE_WRITE_RECEIVED in a write-request. Obviously the 1st log shows
no I2C_SLAVE_WRITE_REQUESTED, and 2nd log shows I2C_SLAVE_WRITE_REQUESTED
occurred after I2C_SLAVE_WRITE_RECEIVED.

I2C_SLAVE_STOP is mandatory to notify a request finish to reset the state
machine of the backend. In case 2 it never do I2C_SLAVE_STOP when STOP
condition is received. This is one of illegal issues to be fixed.

dev->status can be used to record the current machine state, especially
DesignWare I2C controller has no interrupts to notify a new write-request
coming, an IC_INTR_RX_FULL rising while dev->status isn't
STATUS_WRITE_IN_PROGRESS is notified to do an I2C_SLAVE_WRITE_REQUESTED
by checking dev->status. dev->status also helps to resolve the omitted
I2C_SLAVE_STOP in case 2.

Signed-off-by: Michael Wu 
---
 drivers/i2c/busses/i2c-designware-slave.c | 73 ---
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 02e7c5171827..57ce1f0b403c 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -172,50 +172,55 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
 
+   if (stat & DW_IC_INTR_RX_FULL) {
+   if (dev->status != STATUS_WRITE_IN_PROGRESS) {
+   if (dev->status != STATUS_IDLE)
+   i2c_slave_event(dev->slave, I2C_SLAVE_STOP,
+   &val);
+
+   dev->status = STATUS_WRITE_IN_PROGRESS;
+   i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED,
+   &val);
+   }
+
+   regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+   val = tmp;
+   if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
+&val))
+   dev_vdbg(dev->dev, "Byte %X acked!", val);
+   }
+
if (stat & DW_IC_INTR_RD_REQ) {
+   if (dev->status != STATUS_IDLE) {
+   dev->status = STATUS_IDLE;
+   i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
+   }
+
if (slave_activity) {
-   if (stat & DW_IC_INTR_RX_FULL) {
-   regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
-   val = tmp;
-
-   if (!i2c_slave_event(dev->slave,
-I2C_SLAVE_WRITE_RECEIVED,
-&val)) {
-   dev_vdbg(dev->dev, "Byte %X acked!",
-val);
-   }
-   regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   } else {
-   regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-   regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
-   }
-   if (!i2c_slave_event(dev->slave,
-I2C_SLAVE_READ_REQUESTED,
-&val))
-   regmap_write(dev->map, DW_IC_DATA_CMD, val);
+   regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
+   regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
+
+   dev->status = STATUS_READ_IN_PROGRESS;
+   i2c_slave_event(dev->slave, I2C_SLAVE_READ_REQUESTED,
+   &val);
+   regmap_write(dev->map, DW_IC_DATA_CMD, val);
}
}
 
if (stat & DW_IC_INTR_RX_DONE)

[PATCH] i2c: designware: fix slave omitted IC_INTR_STOP_DET

2020-10-13 Thread Michael Wu 
When an I2C slave works, sometimes both IC_INTR_RX_FULL and
IC_INTR_STOP_DET are rising during an IRQ handle, especially when system
is busy or too late to handle interrupts.

If IC_INTR_RX_FULL is rising and the system doesn't handle immediately,
IC_INTR_STOP_DET may be rising and the system has to handle these two
events. For this there may be two problems:

1. IC_INTR_STOP_DET is rising after i2c_dw_read_clear_intrbits_slave()
   done: It seems invalidated because WRITE_REQUESTED is done after the
   1st WRITE_RECEIVED.

$ i2cset -f -y 2 0x42 0x00 0x41; dmesg -c
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : 
INTR_STAT=0x204
WRITE_REQUESTED
WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x710 : 
INTR_STAT=0x200
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x510 : 
INTR_STAT=0x0
STOP
[2][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x510 : 
INTR_STAT=0x0

  t1: ISR with the 1st IC_INTR_RX_FULL.
  t2: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave().
  t3: Enter i2c_dw_irq_handler_slave() and then do
  i2c_slave_event(WRITE_RECEIVED) because
  if (stat & DW_IC_INTR_RX_FULL).
  t4: ISR with the 2nd IC_INTR_RX_FULL.
  t5: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave(),
  while IC_INTR_STOP_DET has not risen yet.
  t6: Enter i2c_dw_irq_handler_slave() and then IC_INTR_STOP_DET is
  rising. i2c_slave_event(WRITE_REQUESTED) will be done first because
  if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET)) and
  then doing i2c_slave_event(WRITE_RECEIVED).
  t7: do i2c_slave_event(STOP) due to IC_INTR_STOP_DET not be cleared yet.

2. Both IC_INTR_STOP_DET and IC_INTR_RX_FULL are rising before
   i2c_dw_read_clear_intrbits_slave(): STOP cannot wait because
   IC_INTR_STOP_DET is cleared by i2c_dw_read_clear_intrbits_slave().

$ i2cset -f -y 2 0x42 0x00 0x41; dmesg -c
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : 
INTR_STAT=0x204
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : 
INTR_STAT=0x4
WRITE_RECEIVED

  t1: ISR with the 1st IC_INTR_RX_FULL.
  t2: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave().
  t3: Enter i2c_dw_irq_handler_slave() and then do
  i2c_slave_event(WRITE_RECEIVED) because
  if (stat & DW_IC_INTR_RX_FULL).
  t4: ISR with both IC_INTR_STOP_DET and the 2nd IC_INTR_RX_FULL.
  t5: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave(). The
  current IC_INTR_STOP_DET is cleared by this
  i2c_dw_read_clear_intrbits_slave().
  t6: Enter i2c_dw_irq_handler_slave() and then do
  i2c_slave_event(WRITE_RECEIVED) because
  if (stat & DW_IC_INTR_RX_FULL).
  t7: i2c_slave_event(STOP) never be done because IC_INTR_STOP_DET was
  cleared in t5.

In order to resolve these problems, i2c_dw_read_clear_intrbits_slave()
should be called only one time in ISR and take the returned stat to handle
those occurred events.

Signed-off-by: Michael Wu 
---
 drivers/i2c/busses/i2c-designware-slave.c | 79 ---
 1 file changed, 40 insertions(+), 39 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 44974b53a626..8b3047fb2eae 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -159,7 +159,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
 
-   regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
regmap_read(dev->map, DW_IC_STATUS, &tmp);
@@ -168,58 +167,61 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
 
+   stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
 
-   if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
-   i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
+   if (stat & DW_IC_INTR_RX_FULL) {
+

[PATCH v2] gpiolib: fix incorrect IRQ requesting of an active-low lineevent

2019-07-07 Thread Michael Wu 
When a pin is active-low, logical trigger edge should be inverted to match
the same interrupt opportunity.

For example, a button pushed triggers falling edge in ACTIVE_HIGH case; in
ACTIVE_LOW case, the button pushed triggers rising edge. For user space the
IRQ requesting doesn't need to do any modification except to configuring
GPIOHANDLE_REQUEST_ACTIVE_LOW.

For example, we want to catch the event when the button is pushed. The
button on the original board drives level to be low when it is pushed, and
drives level to be high when it is released.

In user space we can do:

req.handleflags = GPIOHANDLE_REQUEST_INPUT;
req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;

while (1) {
read(fd, &dat, sizeof(dat));
if (dat.id == GPIOEVENT_EVENT_FALLING_EDGE)
printf("button pushed\n");
}

Run the same logic on another board which the polarity of the button is
inverted; it drives level to be high when pushed, and level to be low when
released. For this inversion we add flag GPIOHANDLE_REQUEST_ACTIVE_LOW:

req.handleflags = GPIOHANDLE_REQUEST_INPUT |
GPIOHANDLE_REQUEST_ACTIVE_LOW;
req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;

At the result, there are no any events caught when the button is pushed.
By the way, button releasing will emit a "falling" event. The timing of
"falling" catching is not expected.

Cc: sta...@vger.kernel.org
Signed-off-by: Michael Wu 
---
Changes from v1:
- Correct undeclared 'IRQ_TRIGGER_RISING'
- Add an example to descibe the issue
---
 drivers/gpio/gpiolib.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d417a936..9c9597f929d7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -956,9 +956,11 @@ static int lineevent_create(struct gpio_device *gdev, void 
__user *ip)
}
 
if (eflags & GPIOEVENT_REQUEST_RISING_EDGE)
-   irqflags |= IRQF_TRIGGER_RISING;
+   irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
+   IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE)
-   irqflags |= IRQF_TRIGGER_FALLING;
+   irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
+   IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
irqflags |= IRQF_ONESHOT;
 
INIT_KFIFO(le->events);
-- 
2.17.1

[PATCH] gpiolib: fix incorrect IRQ requesting of an active-low lineevent

2019-07-05 Thread Michael Wu 
When a pin is active-low, logical trigger edge should be inverted
to match the same interrupt opportunity.

For example, a button pushed trigger falling edge in ACTIVE_HIGH
case; in ACTIVE_LOW case, the button pushed trigger rising edge.
For user space the IRQ requesting doesn't need to do any
modification except to configuring GPIOHANDLE_REQUEST_ACTIVE_LOW.

Signed-off-by: Michael Wu 
---
 drivers/gpio/gpiolib.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d417a936..b98466a05091 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -956,9 +956,11 @@ static int lineevent_create(struct gpio_device *gdev, void 
__user *ip)
}
 
if (eflags & GPIOEVENT_REQUEST_RISING_EDGE)
-   irqflags |= IRQF_TRIGGER_RISING;
+   irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
+   IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE)
-   irqflags |= IRQF_TRIGGER_FALLING;
+   irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
+   IRQ_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
irqflags |= IRQF_ONESHOT;
 
INIT_KFIFO(le->events);
-- 
2.17.1

Re: [PATCH] NET: mac80211 - fix missed mutex unlock

2007-12-03 Thread Michael Wu 
On Monday 03 December 2007 05:50:38 Cyrill Gorcunov wrote:
> This patch does fix missed mutex unlock. Please see it attached.
> (Can't send it as a plain text patch - have only IE based mail access)
>
> Cyrill
Acked-by: Michael Wu <[EMAIL PROTECTED]>

Thanks,
-Michael Wu


signature.asc
Description: This is a digitally signed message part.

Re: RFC: Reproducible oops with lockdep on count_matching_names()

2007-11-01 Thread Michael Wu 
On Thursday 01 November 2007 15:17:16 Luis R. Rodriguez wrote:
> [EMAIL PROTECTED]:~/devel/wireless-2.6$ git-describe
> v2.6.24-rc1-146-g2280253
>
> So I hit segfault with lockdep on count_matching_names() on the
> strcmp() multiple times now. This is reproducible and with different
> wireless drivers.
>
I've found the problem. It appears to be in lockdep. struct lock_class has a 
const char *name field which points to a statically allocated string that 
comes from the code which uses the lock. If that code/string is in a module 
and gets unloaded, the pointer in |name| is no longer valid. Next time this 
field is dereferenced (count_matching_names, in this case), we crash.

The following patch fixes the issue but there's probably a better way.

-Michael Wu

---

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 4c4d236..2aa0d35 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -114,7 +114,7 @@ struct lock_class {
 */
unsigned long   ops;
 
-   const char  *name;
+   charname[128];
int name_version;
 
 #ifdef CONFIG_LOCK_STAT
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 55fe0c7..63c4d8f 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -768,7 +768,7 @@ register_lock_class(struct lockdep_map *lock, unsigned int 
subclass, int force)
class = lock_classes + nr_lock_classes++;
debug_atomic_inc(&nr_unused_locks);
class->key = key;
-   class->name = lock->name;
+   strcpy(class->name, lock->name);
class->subclass = subclass;
INIT_LIST_HEAD(&class->lock_entry);
INIT_LIST_HEAD(&class->locks_before);


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] prism54usb: Fix hail of warnings on 64bit platforms

2007-03-07 Thread Michael Wu 
On Wednesday 07 March 2007 11:40, Alan Cox wrote:
> If you want to cast a pointer to a small value then start by turning it
> into an unsigned long so the compiler knows what is going on.
>
I already have a fix for that - just haven't pushed it up to wireless-dev yet.

> Personally I find the whole approach used by this driver for types of
> registers (which are really USB register numbers) utterly perverse...
>
The same register offsets are used in the PCI driver since version 1 usb 
devices are really just the PCI device with a usb->pci bridge chip attached.

-Michael Wu


pgpD3lVdUiHz9.pgp
Description: PGP signature

Re: Network drivers that don't suspend on interface down

2006-12-20 Thread Michael Wu 
On Wednesday 20 December 2006 20:15, Matthew Garrett wrote:
> Because it works on the common hardware? If there's documentation about
> what userspace can legitimately expect, then I'm happy to defer to that.
> But in the absence of any indication as to what functionality users can
> depend on, deciding that existing functionality is a bug is, well,
> impolite.
>
No, it's absolutely a bug. It just so happens that some drivers incorrectly 
allowed it.

-Michael Wu


pgp88tOEHFma3.pgp
Description: PGP signature

Re: Network drivers that don't suspend on interface down

2006-12-20 Thread Michael Wu 
On Wednesday 20 December 2006 20:12, Matthew Garrett wrote:
> Veering off at something of a tangent - how much of this should be true
> for wireless devices? Softmac seems to be unhappy about setting the
> essid unless the card is up, which breaks various assumptions...
>
Softmac isn't the only wireless code that likes to be configured after going 
up first. Configuring after the card goes up has generally been more 
reliable, though that should not be necessary and is a bug IMHO. 

> Beyond that, I think your descriptions of up and down states make sense
> for userspace. As Arjan suggests, there's then the intermediate state of
> "disable as much as possible while still providing scanning and link
> detection".
>
In order to scan, we need to have the radio on and we need to be able to send 
and receive. What are you gonna turn off?

-Michael Wu


pgpGWfZrTsIlb.pgp
Description: PGP signature

Re: 2.6.x wireless update and status

2005-03-23 Thread Michael Wu 
On Wednesday 23 March 2005 12:52 am, Jouni Malinen wrote:
> > Moving forward, the next "todo" for kernel wireless hackers is to get
> > ieee80211 common code lib into shape, namely:
> > * Merge Intel ipw drivers, which use ieee80211
> > * Update HostAP to use ieee80211
> > * Merge/convert other drivers to use ieee80211?
>
> I'll be working on HostAP driver next; and ieee80211 code of course at
> the same time, since it is likely to need some changes for this. As far
> as other drivers are concerned, I'd like to see Atheros cards working
> with the generic ieee80211 code. They would be a good test target since
> they are an example of design where very large part of functionality is
> in the driver/network stack (no firmware used). This would be a good
> test to verify that the 802.11 code is generic enough for such a design.
>
The ADM8211 would be a good test too. So far, the main thing I see missing 
from the 802.11 code is software scanning/authentication/association. Is all 
of that being moved to userspace? I prefer having a little bit of that in the 
kernel so minimal managed, adhoc, and WEP features can be used without a 
daemon.

-Michael Wu


pgpTVdA6cwhvZ.pgp
Description: PGP signature