Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Message-ID: <854762fb-1767-4208-a7fc-10580730c1f3@os.amperecomputing.com>
Date:   Mon, 27 Nov 2023 15:08:02 +0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] i2c: aspeed: Acknowledge Tx ack late when in
 SLAVE_READ_PROCESSED
Content-Language: en-CA
To:     Cosmo Chou <chou.cosmo@gmail.com>,
        Andrew Jeffery <andrew@codeconstruct.com.au>
Cc:     linux-arm-kernel@lists.infradead.org, jae.hyun.yoo@linux.intel.com,
        andi.shyti@kernel.org, linux-aspeed@lists.ozlabs.org,
        openbmc@lists.ozlabs.org, linux-kernel@vger.kernel.org,
        wsa@kernel.org, brendan.higgins@linux.dev, cosmo.chou@quantatw.com,
        joel@jms.id.au, linux@roeck-us.net, linux-i2c@vger.kernel.org
References: <20231120091746.2866232-1-chou.cosmo@gmail.com>
 <fdd884426497486c6b17795b4edc66243bdc7350.camel@codeconstruct.com.au>
 <CAOeEDyumVdi-3O3apMUFJ695V3YcZqZQ7wvzYL2YfU88XJ3Dxw@mail.gmail.com>
From:   Quan Nguyen <quan@os.amperecomputing.com>
In-Reply-To: <CAOeEDyumVdi-3O3apMUFJ695V3YcZqZQ7wvzYL2YfU88XJ3Dxw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bG5XU2F4THhJd2FpSkhqOHRDNDhTRWVSSkxFRXhVU0dvcm5OOENpaUtUN0xN?=
 =?utf-8?B?dmJwRjRKdGJYWWt6Yi8vcWlzY2VCSURiRE81Mk44ejFDT1NkcDZPcGFTQmg0?=
 =?utf-8?B?SDdXUVdwZDMvbktpZmU2RjBFV3QrMmhYSFYxQnFGeENvSDBycFd5Y2MvaDFJ?=
 =?utf-8?B?VWt6UWVCZ0xVWXRTMzZKODhRYkZ2dDVER2plRGNVdVZBZzdXS2l1N1hKZkRq?=
 =?utf-8?B?ZzhFQkZjSDJZM2FvZlVNWEFKSm9GZFdPRU83d21XYjh3VDkzQVZpbFhQdDlt?=
 =?utf-8?B?WGd3c1dGU3M2ZWJzM2FkWmVvemFqYmNNMDd3OEVDSk1EbVkxMzFKbkNycjQy?=
 =?utf-8?B?WWF0WEJvNEI4dG9GbGx6eVhlZGdRQ2MySGVrNGszU2xXRTFBOHRuV29DYTdW?=
 =?utf-8?B?UkZuZ0hRVVJBVmlna1N5U2s1V0FWanN0UVhVencxcUduVmVwU2V2OUFWeit2?=
 =?utf-8?B?clI5cjhWckhhN1dpNXJ1K3FJdWdqeHo5a3hTenNnQk1USXNRTlMwS21LQWkx?=
 =?utf-8?B?dnRQSlJtdEtscVhqZ01LWHBBL1VxcmZ4d1ZSSFlSeC9kQysvekVpMXZObk1O?=
 =?utf-8?B?UXJWek03NXhYNlI3MnhIbDV5RWhyVkxWcHdrYkY0dkpuNkZJcTlGNktRa2VU?=
 =?utf-8?B?Wjdmd0tSRHVPSVZaQ2ZOeTZFUHdzUFYxWWlNMkRJUkhUaEdDeXgvaktCWWxx?=
 =?utf-8?B?eVUxUURsUUZxenlpeFB4eGFiZDBDMmtYbVJBcC9MQ0s3dmNFcEFEc0hqUWtU?=
 =?utf-8?B?cHFVSU5YN1cyUStpMVdBa3BGRFVQYWRGQ2h5ajdOS2d4alBSM3VUVTB5WDNZ?=
 =?utf-8?B?bGlGcDZCZnBzYVl2TGRQTm9qMDl4dFlPWjQ4MmNLT3lVTzhhYzRtbGt2Yk4r?=
 =?utf-8?B?NnRKUFEwY0d6UldtQ1orUHdxL1VCN1czcjZxMnRYanJxeUtLRnhGc0tOR2xj?=
 =?utf-8?B?N2F5dUwwUk05UWxGeVE0TURzL1RYaDMralhFR21UV2FxVElGcURvWmU3Y1J0?=
 =?utf-8?B?QXdpVmJwUXhzeVBIQ01tdDNCU1Z4YU1TbDlhRUVMOHYydjhoVUFKcmt1TlY4?=
 =?utf-8?B?c2xnZTZtblY3bFZPNlVkMlVCQVFUZ0grcDk5OXpzUTVNU0o4aWg5cGJla3Mv?=
 =?utf-8?B?MlJQUG0yWDd4cXhSeTZQSFhVRDViaGdjbEI5THQvWVlnemMwdEw2d3o0Ylpm?=
 =?utf-8?B?QkFMcGJXVk9SMG9WbFU4MzhoZ2JaN1ozNXRqSU04Nnk4VVBzRThvRDZZNERT?=
 =?utf-8?B?a1M4dGdRZlNySVU3ME1rWUVQZ2hCK3Fyd0pvRjBoZW5BellBVnQ0WFd6MFls?=
 =?utf-8?B?OUVrUkEzZVVqcmRPa2FyeWhJSDJEMU1pRXQxbzV6TzdYN2RkdG5YcHByL3lq?=
 =?utf-8?B?NHBjNys4SEVOTExSRnFBWXNOMXljV25lSFdpQjJNeStlaWUzRGdQN0paZ2ta?=
 =?utf-8?B?eHlwQmhPRExyVFlveWxKdStQZ0hieVdDeUFwRzV2Vlg4bFFaVlRRN1ZvU0NL?=
 =?utf-8?B?VkxseVVyeEZPblZYVTZpZ0l5ZDZza0ozSnROeHQycnNqejd2K0prbXExd25M?=
 =?utf-8?B?UHpqM3JpNFIyRDhBS0hXMVlRU2ZaZ0FQL00rNndZRGw4S1BBTDVPbEJ3dUdF?=
 =?utf-8?B?UzlkT0VSYjY2ZTNRR1pJU3ZFKzhtVWhpVktBVFN0UkNCSFdGOUgyV0FBMWZm?=
 =?utf-8?B?Z2pIN0lnYVBuQUxuNnN1eVVXaWpqbVNWWTBOaW1jMzh5bjdnRkdGVlpxeVlF?=
 =?utf-8?B?dGFLNzR6Z1E3S29URzNNZ0E5SkRsUU1lTk1lQWNOSWMrbEtFSmc3WTJHWWto?=
 =?utf-8?B?YXZpb0VvWFpVYlQ1bWNmeUNuc214aWV5SGpqM2tNb2ErSERkYm1xTVFwRUo4?=
 =?utf-8?B?akJCYTNZRE1hV1JicjNnS2puYjVSY29UNmx6U3RJZk1OdU0zWjhPZkdmZnJj?=
 =?utf-8?B?c3Y2MTdIeDlSUkNvdHZVZW80NkplUXNkNFAwTmJhSXhaM1hKYUhSZEN1cDVh?=
 =?utf-8?B?NGk4b2VTdVhaUU9qMWtpL3cxVUQwOW5WVmhVRlFJeCtFREdOSFZyRTAwZWZh?=
 =?utf-8?B?WVZ3ODN2dFd1bnJzWEpvaENDc0t0dDNYMFMyOVBReUc2SlQrcXF0b1dBZ1cz?=
 =?utf-8?B?NFJNZEc1QmZuMHBwTU95OERKMEs2dkFEYVk1Zk5uekUwWjRpOG9pRDRaSnNL?=
 =?utf-8?Q?uhYi3dTxt99f+bT8oB0sk1o=3D?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 720d0133-50cc-4631-47ed-08dbef2001bb
X-MS-Exchange-CrossTenant-AuthSource: SN4PR01MB7455.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Nov 2023 08:08:15.3089
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: HjlDUBqxWg+lidbXLN+cxeTyI+MMnSc6OSIL9ppJWYazX2L1sPhZdFAv73VlLHQ3fZkmPN9Dj9lYC8vxq/67BwZokK53mva/IKRlJU+4oEI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR01MB8077
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Mon, 27 Nov 2023 00:08:38 -0800 (PST)


On 27/11/2023 14:04, Cosmo Chou wrote:
> Andrew Jeffery <andrew@codeconstruct.com.au> wrote on Mon, 2023-11-27
> at 11:23 AM:
>>
>> On Mon, 2023-11-20 at 17:17 +0800, Cosmo Chou wrote:
>>> commit 2be6b47211e1 ("i2c: aspeed: Acknowledge most interrupts early
>>> in interrupt handler") moved most interrupt acknowledgments to the
>>> start of the interrupt handler to avoid race conditions. However,
>>> slave Tx ack status shouldn't be cleared before SLAVE_READ_PROCESSED
>>> is handled.
>>>
>>> Acknowledge Tx ack status after handling SLAVE_READ_PROCESSED to fix
>>> the problem that the next byte is not sent correctly.
>>
>> What does this mean in practice? Can you provide more details? It
>> sounds like you've seen concrete problems and it would be nice to
>> capture what it was that occurred.
>>
>> Andrew
> 
> For a normal slave transaction, a master attempts to read out N bytes
> from BMC: (BMC addr: 0x20)
> [S] [21] [A] [1st_B] [1_ack] [2nd_B] [2_ack] ... [Nth_B] [N] [P]
> 
> T1: when [21] [A]: Both INTR_SLAVE_MATCH and INTR_RX_DONE rise,
> INTR_RX_DONE is not cleared until BMC is ready to send the 1st_B:
> https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-aspeed.c#L294
> That is, BMC stretches the SCL until ready to send the 1st_B.
> 
> T2: when [1_ack]: INTR_TX_ACK rises, but it's cleared at the start of
> the ISR, so that BMC does not stretch the SCL, the master continues
> to read 2nd_B before BMC is ready to send the 2nd_B.
> 
> To fix this, do not clear INTR_TX_ACK until BMC is ready to send data:
> https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-aspeed.c#L302
> 

This looks like the same issue, but we chose to ack them late. Same with 
INTR_RX_DONE.

https://lore.kernel.org/all/20210616031046.2317-3-quan@os.amperecomputing.com/


- Quan