Received-SPF: pass (google.com: domain of linux-kernel+bounces-29303-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Boddu, Sai Pavan" <sai.pavan.boddu@amd.com>
To: Andi Shyti <andi.shyti@kernel.org>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, "Simek, Michal"
	<michal.simek@amd.com>, Lars-Peter Clausen <lars@metafoo.de>, Wolfram Sang
	<wsa@kernel.org>
Subject: RE: [PATCH] i2c: cadence: Avoid fifo clear after start
Thread-Topic: [PATCH] i2c: cadence: Avoid fifo clear after start
Thread-Index: AQHaSUfcsQJjTnMwH0iIfDUOatck5LDeSs0A
Date: Wed, 17 Jan 2024 18:06:55 +0000
Message-ID:
 <DS7PR12MB5741FC0E92875C8DB7BFEEDFB6722@DS7PR12MB5741.namprd12.prod.outlook.com>
References: <20240105125258.2470397-1-sai.pavan.boddu@amd.com>
 <lzvb6oyinlmdrbaat6ayxioca5r2qf7wi3kt63lzorbjytmmbn@wfuz63znrzbr>
In-Reply-To: <lzvb6oyinlmdrbaat6ayxioca5r2qf7wi3kt63lzorbjytmmbn@wfuz63znrzbr>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?MxfkmN2Oyr7oXhUTOCj1D1CSniKtuXYFh1FpFB95Ciwbg+x3Do2AgENQmLxd?=
 =?us-ascii?Q?GM+HXCl0k1B1t/UBnR4PD8gyGLKoG98t+u7XjFMHhuOfymTPfgXreH75o1J/?=
 =?us-ascii?Q?EeKTdNvT/kk47xHClO1MUeNAOUWZuJNBtTPZnFF5+CKK9QX7DhGR/4oLiy+R?=
 =?us-ascii?Q?TugIX8U6umTpJ1bwy/xOYg8QwSKDxLSCsKOWhWPoHJ7Kk1exbzXIUtMQo4ZA?=
 =?us-ascii?Q?h+DtQGdceVV+BEBXrz6EumVn++FuCRAUVcZQf6XjpP1AE2qRHUQFhbmDeh4p?=
 =?us-ascii?Q?HV1+T5Cz9zPceHTdd2gkPs2Z1IJXngukgaX0XYxtk6WP2VBrBEmOseokoqHw?=
 =?us-ascii?Q?iGmb9X62PCIvcov5fJIBY+MTDZ67UcquWOlJmb47WF7mrwtCQJE5hHmRDWoI?=
 =?us-ascii?Q?JSZaEyTMFKik0UImd8Fly70YapEr5n3CMDfKRJbSKTQrbmwt3jhNMD7LaEa/?=
 =?us-ascii?Q?lLvC66bLJzW6bsfEgDI64q5mVH5hFhJrSRnzDli/Vh2j3QlHtsNHmed3KPQw?=
 =?us-ascii?Q?Gf1mnxZQUjDWy1SmbZpcS/h2ACM9gQrm6u7+w1D6Jx/0IT+cSDNXO0YAWD2L?=
 =?us-ascii?Q?82z3wZJXtZsBRJdbS0uRpXJ9r+VIM0L+UoQjcvcAFl6JS3BjnHb10hK8cVf5?=
 =?us-ascii?Q?SBQroMn6PrN3enYYOW3iGZ8B8wHC8CkzPfSu02wUmo/tHnRbZsXAX+FNGNXf?=
 =?us-ascii?Q?1UcEcyOd3vvDfqnk7kHXI+DU/Lb/obePQv/h3oWkkSdDbakOFDVC4xezNumJ?=
 =?us-ascii?Q?kaW+gAiyUFQcI50L96q4fjr5hY0HGNwJHrrCqlpK7Vx13d+F7PGjk/PlJAoX?=
 =?us-ascii?Q?AOhtf8U48SIdBkSbUWUwyl2d8X8ZY5Zrg3i4G33yrTBWY77qkI7bEX4Usa3x?=
 =?us-ascii?Q?E5848J5JPwnY9+GaxOAR09nJzkftNQtw/tUF7EBKqHqYDPgPHGOXXjtX1zoo?=
 =?us-ascii?Q?Edi0fpatmfIdVTIEUH4Nct5ri3ebLKoX23cd1V/Usux3u2lb5aRPJBzZQgqE?=
 =?us-ascii?Q?k+zbhzpRV1xA7dQZQsdTYvZJwKXD90KF8iOsnVcG4f4LJeyk2hlKLAG5Jhyo?=
 =?us-ascii?Q?9ohJy+A+APCI1Vbm9hyi48HxSDHqJdKGtr0tYoMrd0zN7xPflE8c7bdVNzs0?=
 =?us-ascii?Q?gO4qRiMeYbt3mf1E9LTF28iukrngtQpUD2z7Q3LnUfBpczl2k3Da+BfEwBDL?=
 =?us-ascii?Q?t9567FzZlpV/z9X0LhHev6eYnhVtOx8eM1rgp6bCrPzxpzSnYbZMyLBFWjJ0?=
 =?us-ascii?Q?zRHTrbWeYqjqo4JhMksZ02+cGvzw/r8zNDHBNNRFV9lPcqF6uT8rQJ9azAxw?=
 =?us-ascii?Q?Ej1sxpFAKz179fUxmV9Ud4uxkMM2QkyYBguIsqW3D6NodkjSjNxrVozulSUl?=
 =?us-ascii?Q?JcqkJ96Ikl9WGj7sDA+8JibI2hZrFKbFtwuvU+j+2WK/+n0oBaA/isDlMEBc?=
 =?us-ascii?Q?gpRjaPPsMgytHtt5vK8bNRPDetwLnko668DbEdxEP5dtrM3qSNvehPFYl5gB?=
 =?us-ascii?Q?wsb7nJ4cNUMsUqu8ZRvk1uKeuSUscTvUD7vT4m2oCKDtef0QxQ4tiqndN476?=
 =?us-ascii?Q?CS+VrNKqIBE7V5I7znU=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5741.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e952f3c3-668d-463d-4a9e-08dc1787175d
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Jan 2024 18:06:55.6912
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: h+oTposjRdgH6JyUVTv49DoA3rmbMfp210knoXciK8GT3yrDRR7CaKMqZFe2I66l
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB6932

Hi Andi,

>-----Original Message-----
>From: Andi Shyti <andi.shyti@kernel.org>
>Sent: Wednesday, January 17, 2024 6:50 PM
>To: Boddu, Sai Pavan <sai.pavan.boddu@amd.com>
>Cc: linux-kernel@vger.kernel.org; linux-i2c@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org; Simek, Michal <michal.simek@amd.com>; Lars-
>Peter Clausen <lars@metafoo.de>; Wolfram Sang <wsa@kernel.org>
>Subject: Re: [PATCH] i2c: cadence: Avoid fifo clear after start
>
>Hi Sai,
>
>sorry, but I'm not really understanding the issue here.
>On Fri, Jan 05, 2024 at 06:22:58PM +0530, Sai Pavan Boddu wrote:
>> Driver unintentionally programs ctrl reg to clear fifo which is
>> happening after start of transaction
>
>what does it mean "unintentionally"?
[Boddu, Sai Pavan] I mean, the previous patch which introduced the issue, w=
as unintentional.
>
>> this was not the case previously
>> as it was read-modified-write. This issue breaks i2c reads on QEMU as
>> i2c-read is done before guest starts programming ctrl register.
>
>this log can be improved. How about something like
>
>The driver unintentionally programs the control register to clear the FIFO=
,
>which occurs after the start of the transaction.
>Previously, this was not an issue as it involved read-modify-write operati=
ons.
>However, this current issue disrupts I2C reads on QEMU, as the I2C read is
>executed before the guest starts programming the control register.
[Boddu, Sai Pavan] Looks good. I will mention as above.

>> Fixes: ff0cf7bca6309 ("i2c: cadence: Remove unnecessary register
>> reads")
>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@amd.com>
>> ---
>>  drivers/i2c/busses/i2c-cadence.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-cadence.c
>> b/drivers/i2c/busses/i2c-cadence.c
>> index de3f58b60dce..6f7d753a8197 100644
>> --- a/drivers/i2c/busses/i2c-cadence.c
>> +++ b/drivers/i2c/busses/i2c-cadence.c
>> @@ -633,6 +633,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
>>
>>  	if (hold_clear) {
>>  		ctrl_reg &=3D ~CDNS_I2C_CR_HOLD;
>> +		ctrl_reg &=3D ~CDNS_I2C_CR_CLR_FIFO;
>
>I'm wondering whether the whole ctrl_reg should be reset after the first w=
rite.
[Boddu, Sai Pavan] previous implementation of read-modify-write was good th=
en ?

Regards,
Sai Pavan
>
>Andi
>
>>  		/*
>>  		 * In case of Xilinx Zynq SOC, clear the HOLD bit before transfer
>size
>>  		 * register reaches '0'. This is an IP bug which causes transfer
>> size
>> --
>> 2.25.1
>>