Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Fri, 20 Oct 2023 10:18:55 -0400
From:   Frank Li <Frank.li@nxp.com>
To:     Miquel Raynal <miquel.raynal@bootlin.com>
Cc:     alexandre.belloni@bootlin.com, conor.culhane@silvaco.com,
        imx@lists.linux.dev, joe@perches.com,
        linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 Resent 6/6] i3c: master: svc: fix random hot join
 failure since timeout error
Message-ID: <ZTKMTwU6cVAfGCKG@lizhi-Precision-Tower-5810>
References: <20231018155926.3305476-1-Frank.Li@nxp.com>
 <20231018155926.3305476-7-Frank.Li@nxp.com>
 <20231019084452.11fd0645@xps-13>
 <ZTFNvrsSoHOS3P2g@lizhi-Precision-Tower-5810>
 <20231020160645.67e678ee@xps-13>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231020160645.67e678ee@xps-13>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RmR6VlRxL0VYVW45aVFmVmM4UTIxb1pHN1A5OW5xN2VWWFMzTHJsWGgzTGp2?=
 =?utf-8?B?K3lZdGVyZGE1NTlTRllmK2FYWXhoYTU4bllkeVJMNDduT3l5ek0wV1ZmeFhm?=
 =?utf-8?B?MEt6RW5jOTBuOUk5OEhvRjFoWjNNa3A4OGJ5amtmRmhqWlFCNHo3OWRQelFO?=
 =?utf-8?B?dW5Hd0tXOVIwV0JjWnBLR3M1OEY2ZFFxdHduc0IxVjVqTnNSdGtGNFFMYXQ4?=
 =?utf-8?B?dWdQRlRFWHk3dm1GS290RGY0RVlvSXNpcWRIemkwclRHVFViSHQrV0ZXL3J4?=
 =?utf-8?B?d0QvMjdvblRndjNpVGw5ekowQnNWZzg2QUdZRFdHZWsrdWIvbVBxY2U4SUlz?=
 =?utf-8?B?M0pGenFiYXhaRHQ4d2pzSk9HTFJWeXFnMFFsaUVQMEVkNHlBWU91clF5cUY0?=
 =?utf-8?B?TXZ3NDFwWHhnL0o0ZjdlbWt0dFVER3RBNlk0eC9HYjlKUkwxWlZsWUpSeU5t?=
 =?utf-8?B?Zy9MWVEwbmZrNGY3bUpzYjBJdjYwMlB0UEdyTExaUlVremtRVzhIWmNPSzQ1?=
 =?utf-8?B?eFlJc2krNFdVeG9lUDBsbW92RzFqc1R4ZWxnRDNWWHFLUUJkcDJGbGZlNTVp?=
 =?utf-8?B?VlA1dnAxaU9FMEVVdVYreGVZbEdlL0lXTTNybGN4K2grVE4zQk1FV3NPQVFz?=
 =?utf-8?B?OGZFTEtMK2R3bjZuZHVVdTdaUGJZS2s3RjdObFhOVnFyanJoWXNMMlc3cGNY?=
 =?utf-8?B?WmpjazQ0OXNQSldjTk5ZNjNkcUtueXMreHBWN084aFJGK2txMHNnVGg5ZzN6?=
 =?utf-8?B?akF4R0dXUnpxNE41UU45R3dGRjRIS3lqVDdJNGxrdVE2MHJmZ05KUllTVmNW?=
 =?utf-8?B?K0NwS29nWEZ5bkxnaGxDMDVXRW5XMjJzQWZHWlNXWGI4R0hGSXBXOUNmWDlC?=
 =?utf-8?B?Sllsb0RhdWxYQlVxblcrSmw5UmozR09EZm5ZdHljZ1VnMzl4OVpsMk1LN3Zl?=
 =?utf-8?B?QzBWdDZPVEl4Wmp6dE1LM3FxN2srSm1pZHhvdm5VWTFBQWpSWWNvZThYRVlN?=
 =?utf-8?B?Y1JOclFMQUdiQnBZOThVcTc0RVdybjltVlUrRjNvNXJ4OFhwVFUrZ0pUV0lB?=
 =?utf-8?B?cVdMZUc3VlptZkk5UkRGT29NL3duRmtxbnQxRXlUQjArZE9lUXlyVzhCc1BJ?=
 =?utf-8?B?Q2RHbDFycGtrR1RHMFY3S1JlbmpsSWtsZllzVm9zV3Jpb0I0c1BkMlljV21W?=
 =?utf-8?B?Z1pnUmZ2cXBRWno2NHhiUW9hRGJrSDh6SURxTitFc1VZTSt0VTJteVpXVU45?=
 =?utf-8?B?YnFTSTl6Ynowak95aFBLNENFNmptWUlrNDJmekx1Y202Vlp0MFMrVWtkZEZK?=
 =?utf-8?B?dU9EbFFsYktNMjk1cS9iQURkbTdTTzFLSVAxazU1NURWVGtYK1ltUHBqTmdq?=
 =?utf-8?B?TVBUNzJUcVJ2dXhJT3d1NVp6N2NDQkhZYXhHZXNoc0V1YzVlMkw2UkNnOGZQ?=
 =?utf-8?B?bUNZSmpPVnF1ZWxST2hHN2RZY21aUFV5cHpsTUwxZ3dkSHl4OVpWYVRaZEJV?=
 =?utf-8?B?UG44KzdQaVU3WTBOSlMxK3d3Y1I4OUlLN2N6Mk5HS1puS0l1Q2hPZzd3bnJJ?=
 =?utf-8?B?QUhnWFF5UENwNjZOL2ZUcVBUWG1KcktJT2cwUVhkTi8zTFM3a1Vydm9WZnRt?=
 =?utf-8?B?RTF0QnUwNUtGVHpBby9DVVgzV2Z5c0htTUZ5TjZKNnZJZmJoaUhzVm4vRmxZ?=
 =?utf-8?B?VlNqV092L242QWxyTzlzV0VVZzNJMGNCRnBTdEs5cGNCK0hxL3FsY2ZpZGp3?=
 =?utf-8?B?NkxKMEV5R2h4SXE3WE0waXhJcUdNVVlZNDBNZXI2QUZTbkx3UFd6THoycDR1?=
 =?utf-8?B?eXRIMTFwSHdjSXphalo5SU5RcUVLK0VGQ01GZHdaZWdBTnBJQmJBTUVOb1Iz?=
 =?utf-8?B?VzF2OC8waFM5cjhWa1hGRGZSaVFjVmdFa0hucTBTdE1ocmE0ZVJUczRkc2cv?=
 =?utf-8?B?eUptZ0djczRzaUhEYmxSOFJYcGVMekZHRFBad1F4TU1KWUwwZnhGNS90M3By?=
 =?utf-8?B?S1NQeTlVWWVXdHIxZTVDb0lBMnRWUExxY2hLS25INlc4QytXQUhBMTBtY0lO?=
 =?utf-8?B?SmJjc3JORDR1SnFqdFh1U2lheDl2K2lPVUJwcmJKcGoxT0RIRlFJdWRVTWFV?=
 =?utf-8?Q?0ViM=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c68eab64-2003-43a9-7f06-08dbd1778208
X-MS-Exchange-CrossTenant-AuthSource: AM6PR04MB4838.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Oct 2023 14:19:01.7527
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Wp/fO9057pPOvppddgSbBl+KysH6Qi0mW8KfHOG59+WMFQGjidMuWddNOh1HYXC/HsRNVLOhUkLCtrKXH0XAqQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB7619
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.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 (lipwig.vger.email [0.0.0.0]); Fri, 20 Oct 2023 07:19:22 -0700 (PDT)

On Fri, Oct 20, 2023 at 04:06:45PM +0200, Miquel Raynal wrote:
> Hi Frank,
> 
> Frank.li@nxp.com wrote on Thu, 19 Oct 2023 11:39:42 -0400:
> 
> > On Thu, Oct 19, 2023 at 08:44:52AM +0200, Miquel Raynal wrote:
> > > Hi Frank,
> > > 
> > > Frank.Li@nxp.com wrote on Wed, 18 Oct 2023 11:59:26 -0400:
> > >   
> > > > master side report:
> > > >   silvaco-i3c-master 44330000.i3c-master: Error condition: MSTATUS 0x020090c7, MERRWARN 0x00100000
> > > > 
> > > > BIT 20: TIMEOUT error
> > > >   The module has stalled too long in a frame. This happens when:
> > > >   - The TX FIFO or RX FIFO is not handled and the bus is stuck in the
> > > > middle of a message,
> > > >   - No STOP was issued and between messages,
> > > >   - IBI manual is used and no decision was made.  
> > > 
> > > I am still not convinced this should be ignored in all cases.
> > > 
> > > Case 1 is a problem because the hardware failed somehow.  
> > 
> > But so far, no action to handle this case in current code.
> 
> Yes, but if you detect an issue and ignore it, it's not better than
> reporting it without handling it. Instead of totally ignoring this I
> would at least write a debug message (identical to what's below) before
> returning false, even though I am not convinced unconditionally
> returning false here is wise. If you fail a hardware sequence because
> you added a printk, it's a problem. Maybe you consider this line as
> noise, but I believe it's still an error condition. Maybe, however,
> this bit gets set after the whole sequence, and this is just a "bus
> is idle" condition. If that's the case, then you need some
> additional heuristics to properly ignore the bit?
> 

                dev_err(master->dev,                                       
                        "Error condition: MSTATUS 0x%08x, MERRWARN 0x%08x\n",
                        mstatus, merrwarn);
+
+		/* ignore timeout error */
+		if (merrwarn & SVC_I3C_MERRWARN_TIMEOUT)
+			return false;
+

Is it okay move SVC_I3C_MERRWARN_TIMEOUT after dev_err?

Frank


> > In svc_i3c_master_xfer() have not check this flags. also have not enable
> > ERRWARN irq.
> > 
> > If we met this case, we can add new functions/argument to handle this.
> > Then we can real debug the code and recover bus.
> > 
> > Without this patch, simplest add some debug message before issue
> > SVC_I3C_MCTRL_REQUEST_AUTO_IBI, TIMEOUT will be set.
> 
> Yes, and sometimes it won't be an issue, but sometimes it may. Maybe we
> can find more advanced heuristics there.
> 
> > And svc_i3c_master_error() was only called by svc_i3c_master_ibi_work().
> >
> > So I can think only case 3 happen in svc_i3c_master_ibi_work().
> 
> Case 3 cannot be handled by Linux (because of the natural latency of
> the OS).
> 
> > 
> > Frank
> > 
> > > Case 2 is fine I guess.
> > > Case 3 is not possible in Linux, this will not be supported.
> > >   
> > > >   The maximum stall period is 10 KHz or 100 μs.  
> > > 
> > > s/10 KHz//
> > >   
> > > > 
> > > > This is a just warning. System irq thread schedule latency is possible
> > > > bigger than 100us. Just omit this waring.  
> > > 
> > > This can be considered as being just a warning as the system IRQ
> > > latency can easily be greater than 100us.
> 
> This was skipped in your v3.
> 
> > > > Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> 
> Thanks,
> Miquèl