Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 23 May 2023 09:46:21 -0700
From:   David Zheng <david.zheng@intel.com>
To:     <linux-i2c@vger.kernel.org>, <linux-kernel@vger.kernel.org>
CC:     <jarkko.nikula@linux.intel.com>,
        <andriy.shevchenko@linux.intel.com>,
        <mika.westerberg@linux.intel.com>, <jsd@semihalf.com>
Subject: [PATCH v1] i2c: designware: fix idx_write_cnt in read loop
Message-ID: <ZGzt3dGW0dxa5fqc@davidzhe-DESK>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?qBrZ/yCKt5len7hEjsjA16JgWUCukyqunhiEHFApRJt1Ueie97HFACICZ4td?=
 =?us-ascii?Q?CXgovCzG675WY1dCXbkdFAvQnbSmSFjQnkb8HERbnAYJCOsBopaB3MJgafuI?=
 =?us-ascii?Q?LzObFpixRpdkTtSg4Wi5znhfg37h6Aj+VGFlz2FcM5MHMGHXKY3mrvMScQuU?=
 =?us-ascii?Q?Vp/RMMefQk7OyefjIROIm2BvNfgqDtxuKucMWT1GThRn0q6pK9kP8fDv6cV1?=
 =?us-ascii?Q?Rdrf8P64heAg3f5JdI75WwnRWNmdwcEkn/VAy0z4+BCT7tu/3UmS33RIq9bh?=
 =?us-ascii?Q?XNUK74+AdGiwnzj4bIcfbm5nW0keLcUlhov03LYF+oCsg9Z31SFLJoEvY1fE?=
 =?us-ascii?Q?sRBK9m+SOFivIu2/BHocKgcVA3uSfWpFRNhDUEe9Jx+XBXy5YcTOYs4uzYGS?=
 =?us-ascii?Q?zskiylQiNo8vEuoerK3NJCRFGvgkL5khOCmByyHybyjaEqO9ZyycgMCFK8o3?=
 =?us-ascii?Q?L2eor46aunDx3rhYX1dV9vWchWYyYX1DvUrTdh4WKaoBCqmvg9sQml2Onaiz?=
 =?us-ascii?Q?N9ERlZA6cFU5/8SfUqhV/UAqbt6DGv95VNANZR/5iEN2tZTx0NRICEqBO0X8?=
 =?us-ascii?Q?SNRlePZh99VUHiTwE78JI6/jT4blKG7+S3PiixwaVAiTCjdcKWDpb3lCN88G?=
 =?us-ascii?Q?UEQaqs00yo6pip32x5FxT2rU/Y4i5efilF9QoJECZ2Jd6C61FIqisSlRuKTq?=
 =?us-ascii?Q?5V4aBjKfT9hFYgswqJez+2yAGCep+zuRkc1jDBO/6GAzEmZabhqv9FPnuInS?=
 =?us-ascii?Q?PUgbD7zsicQs5CAXM75ykLTs0eS9CgoEnB3hQtclut0HZ2OxxEMrV3w1gXA7?=
 =?us-ascii?Q?ao5OZhGXaJve00enlx7lld0ZoIfDCsgRghWRKQQQAwOHNegg/U1PiZbbKMvm?=
 =?us-ascii?Q?1bA87RnTv+kNOVjHNZRuiF4KdlsDrppATdBAAbJ544va9AyEQdgMOTTi2eLN?=
 =?us-ascii?Q?PLplNK5s2eIBYTlwanT2t3BHNqybOQ8rHZANrgyI1PPNUvBHrgeyGLzu7c8f?=
 =?us-ascii?Q?45R5ZKqK1b+493zGhezCEq/M+rokc9jC412vojXYfUkSCZI/n/wxyMjJjJUB?=
 =?us-ascii?Q?F1419wwybWtIsGkBYZBJvELDntSUcKjUZsv/+I4nNlpTGxqJXv1Kz/81KJY9?=
 =?us-ascii?Q?5AZAe/0KaxuztFtj+Ko4QYF6qDs+zzQ80ndOfVwXLYr5KcOqMrAVdqGZo9fe?=
 =?us-ascii?Q?urWoHJRLGpTjIbgm+hG06/UkXX/e36IWRlUbae5967Zna8W4pHV/RhntcEyr?=
 =?us-ascii?Q?n7fdO1vwExIA0OiK0igEx8fi9Z6vgCISVIaQZ2Am9+fzEuqxmW3CCP0SsEYP?=
 =?us-ascii?Q?rOeD/gMtMYEZI+zilWtsxDxDONDIW7zbFtGqwebf46zE7PylYf9HZwqaG4Zj?=
 =?us-ascii?Q?LzjlzVFq3qzCPN5I9Z3SNk1LYdyy0MNJYy2JMU9iNjfYm6zvCm7bRjB80JMn?=
 =?us-ascii?Q?xGRdRl07y9OE7cnmaCcSOR7I16Xabsk17pA30DBjdhptUZlEG/sTHXOMBU4l?=
 =?us-ascii?Q?R7VKyw1AbPQST93cHk6jEXgB5deMgMNZaBvYPH4l87O9yqGrVoeS6t/1BdM4?=
 =?us-ascii?Q?VWabLv9pv05DIubaCZBbeasrgB/hF84a7xCNEXpfmRx7H79pGkJixT5YbicS?=
 =?us-ascii?Q?Nw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e48f8c31-757e-42ce-f0f0-08db5bad3f4b
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3004.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2023 16:46:25.4260
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ASlgDaQJWk4pCWp3uBqFIotzUq9TncneawKB3l3djm8qA2azgLx7UhYhtcewuxywVQQ2bSjXt9QAYxXVTL9EVw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR11MB5761
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_PASS,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

With IC_INTR_RX_FULL slave interrupt handler reads data in a loop until
RX FIFO is empty. When testing with the slave-eeprom, each transaction
has 2 bytes for address/index and 1 byte for value, the address byte
can be written as data byte due to dropping STOP condition.

In the test below, the master continuously writes to the slave, first 2
bytes are index, 3rd byte is value and follow by a STOP condition.

 i2c_write: i2c-3 #0 a=04b f=0000 l=3 [00-D1-D1]
 i2c_write: i2c-3 #0 a=04b f=0000 l=3 [00-D2-D2]
 i2c_write: i2c-3 #0 a=04b f=0000 l=3 [00-D3-D3]

Upon receiving STOP condition slave eeprom would reset `idx_write_cnt` so
next 2 bytes can be treated as buffer index for upcoming transaction.
Supposedly the slave eeprom buffer would be written as

 EEPROM[0x00D1] = 0xD1
 EEPROM[0x00D2] = 0xD2
 EEPROM[0x00D3] = 0xD3

When CPU load is high the slave irq handler may not read fast enough,
the interrupt status can be seen as 0x204 with both DW_IC_INTR_STOP_DET
(0x200) and DW_IC_INTR_RX_FULL (0x4) bits. The slave device may see
the transactions below.

 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x1594 : INTR_STAT=0x4
 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x1594 : INTR_STAT=0x4
 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x1594 : INTR_STAT=0x4
 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x1794 : INTR_STAT=0x204
 0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x1790 : INTR_STAT=0x200
 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x1594 : INTR_STAT=0x4
 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x1594 : INTR_STAT=0x4
 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x1594 : INTR_STAT=0x4

After `D1` is received, read loop continues to read `00` which is the
first bype of next index. Since STOP condition is ignored by the loop,
eeprom buffer index increased to `D2` and `00` is written as value.

So the slave eeprom buffer becomes

 EEPROM[0x00D1] = 0xD1
 EEPROM[0x00D2] = 0x00
 EEPROM[0x00D3] = 0xD3

The fix is to use `FIRST_DATA_BYTE` (bit 11) in `IC_DATA_CMD` to split
the transactions. The first index byte in this case would have bit 11
set. Check this indication to inject I2C_SLAVE_WRITE_REQUESTED event
which will reset `idx_write_cnt` in slave eeprom.

Signed-off-by: David Zheng <david.zheng@intel.com>
---
 drivers/i2c/busses/i2c-designware-core.h  | 2 ++
 drivers/i2c/busses/i2c-designware-slave.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index c5d87aae39c6..8b85147bd518 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -123,6 +123,8 @@
 #define DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH	(BIT(2) | BIT(3))
 #define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK	GENMASK(3, 2)
 
+#define DW_IC_DATA_CMD_FIRST_DATA_BYTE		BIT(11)
+
 /*
  * Sofware status flags
  */
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index cec25054bb24..2e079cf20bb5 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -176,6 +176,10 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
 
 		do {
 			regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+			if (tmp & DW_IC_DATA_CMD_FIRST_DATA_BYTE)
+				i2c_slave_event(dev->slave,
+						I2C_SLAVE_WRITE_REQUESTED,
+						&val);
 			val = tmp;
 			i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
 					&val);
-- 
2.40.1