Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Date:   Tue, 17 Oct 2023 16:55:41 -0400
From:   Frank Li <Frank.li@nxp.com>
To:     Miquel Raynal <miquel.raynal@bootlin.com>
Cc:     conor.culhane@silvaco.com, alexandre.belloni@bootlin.com,
        joe@perches.com, linux-i3c@lists.infradead.org,
        linux-kernel@vger.kernel.org, imx@lists.linux.dev
Subject: Re: [PATCH 3/6] i3c: master: svc: fix ibi may not return mandatory
 data byte
Message-ID: <ZS70zTwcGOjxB6MS@lizhi-Precision-Tower-5810>
References: <20231016153232.2851095-1-Frank.Li@nxp.com>
 <20231016153232.2851095-4-Frank.Li@nxp.com>
 <20231017162707.187bebcf@xps-13>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231017162707.187bebcf@xps-13>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?EfkhRvv/kz20xjJxegxX1sS1r80WmlqFQ23NcAipckv867zkEqXy8Riydl?=
 =?iso-8859-1?Q?Doi6XWL2MvxHa3Q6osFQYBuYwGnKr5dktaF9vu/mldbIo8n6NkaehIp/V9?=
 =?iso-8859-1?Q?mKcNWx0l4imAT8MzlwuEWKmoRV905LXe4rUFewZh0xQC/VDtm1gfmj+bZw?=
 =?iso-8859-1?Q?ekPuFmM3PsXehNv6g8ylJcXHeBlDXOv3TXvA3dVzUcB0tvzN92+klqIJ8a?=
 =?iso-8859-1?Q?kpBztTTnRqWrNq9MWXjkR4TEDiGIbWo9oKTXvqDqvSUAdeR/agQADX1RCC?=
 =?iso-8859-1?Q?Czdubp644eS4rF/yA5MmexQStV7X1DdTqlTX+FJi6s5TPjHq4j+chiYXdJ?=
 =?iso-8859-1?Q?grlRErJwW5sSdD9F1dkePRSWwrlJ8gulBrLDAi2d8fXcGtMLad9HOtKF4f?=
 =?iso-8859-1?Q?U/Nn3aJR4R+FAjBn4mGL+wuwNf/eNfx54w9zHAQ4MDeyAqa6+eZXQImI1h?=
 =?iso-8859-1?Q?Y8WoA1LF/HfeKKykAUAy7kCpTQRT4+zD7XS9KR3jLtV94bPp+QFZ+CxHQ0?=
 =?iso-8859-1?Q?JzY3mX8JYdDTt1Xw9fAAMiPZ4rrCvRj92DVS44Sc1cOADM35Vzw9quHcgj?=
 =?iso-8859-1?Q?6J1cOGw2k4JWjoZE/8iUvWawqK4PjNHxp3SEzdks1yDv4mcj1BUsde8TVw?=
 =?iso-8859-1?Q?7CCM4fcyYsFeEN5hMjxi9FTT2WxT8h8UvbKpmiwItUaTUrcTK6eS5Y+qUl?=
 =?iso-8859-1?Q?UoyaJNPF4voI1ClbwyuUZRLaFMKWlSchLgpngel7NKybdCWqaKapsmFLky?=
 =?iso-8859-1?Q?xYxPej4nzGEqxPcMg/+weU4kgyPhBd00sPaSN8nUnsZqgxmw9HiNupQhWG?=
 =?iso-8859-1?Q?jr6vlWpfEq3U8lGsmZb4Af7PGyHxYOHw6EV1Ecbf+p1hXUvd7szIdeMfx6?=
 =?iso-8859-1?Q?YsgzfWEBBKR3fkKkgBytR3WIXVTER7TqXRpp2KporaDzE9y+1BliSivTAK?=
 =?iso-8859-1?Q?QgbtfXQAF4HVYZaRIr+qzLrg3LXnhDvsFE0+poT7xIoWan06l5w6qGgwpf?=
 =?iso-8859-1?Q?SoDI+OYXrRbkXmwiYZahGX6/u4Q3b5A0vzoPO5eWGQH5EOOFzJ9GhY+k/D?=
 =?iso-8859-1?Q?p6AckbyUcll0pUr0Q+thAtMOHIgjNIZXoQ+3TZlAIDb1dEm3xecVPAbSat?=
 =?iso-8859-1?Q?V2RLPiINdYrjC28FIIeEdwypnj/A071r2prR0TiqLnWJjhidPYzvye4BrO?=
 =?iso-8859-1?Q?liQnKy3wtmJpE/yLB/IR0/omzimLXC7n9lSIuvTox6XyBE0Cde8WGGZ9sN?=
 =?iso-8859-1?Q?6NycQoNy93mvKdFffoje1O4RKset2l6s/KzRP/IPtUpiMKJfOgCqNqXymV?=
 =?iso-8859-1?Q?eVBPmJANcmL6HFr6Who1kswiTVTnZ5vnBQopaUrSGK4IVAulcsEC/cj43z?=
 =?iso-8859-1?Q?U5ONQ1tPWSeynB4xOWXIiF4zK3IvBDTzK+BEB1jzp86t0mFWRLXv1dul02?=
 =?iso-8859-1?Q?5YDoeP0pdgAsElA4GyWqHEFxS2okudeGLy4tnCQ8iECHxWoi+mwKaTp8J+?=
 =?iso-8859-1?Q?Ysy26lUHoK9Yf06F2WHWhCwETceR3KQpRLEsl3/DilDBeqV4qM5groskq+?=
 =?iso-8859-1?Q?/TOfR9fABKKZubN0QEt9r7efDa02MnRdoZRewC6mgg1b58cZWY9MITyeBN?=
 =?iso-8859-1?Q?bewAJqb9ga/0tQC/N8CYB/vEO50bE3ipbf?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1523c1cb-54dc-4c06-e0d0-08dbcf53707f
X-MS-Exchange-CrossTenant-AuthSource: AM6PR04MB4838.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Oct 2023 20:55:48.0622
 (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: QS2pILYuPmOKgjoIH/+TDyITNhW5EWw2tjU5b/zFwFG2GwJCeyGwo9T01ze++kIYXY3Vy2ZDclpSZTZee1EbWA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8488
X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED,
	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 agentk.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 (agentk.vger.email [0.0.0.0]); Tue, 17 Oct 2023 13:56:03 -0700 (PDT)

On Tue, Oct 17, 2023 at 04:27:07PM +0200, Miquel Raynal wrote:
> Hi Frank,
> 
> Frank.Li@nxp.com wrote on Mon, 16 Oct 2023 11:32:29 -0400:
> 
> > MSTATUS[RXPEND] is only updated after the data transfer cycle started. This
> > creates an issue when the I3C clock is slow, and the CPU is running fast
> > enough that MSTATUS[RXPEND] may not be updated when the code reach checking
> > point. As a result, mandatory data are being missed.
> 
> 					can be missed.
> 
> > Add a wait for MSTATUS[COMPLETE] to ensure that all mandatory data already
> > in FIFO.
> 
> is already in the FIFO
> 
> > 
> > Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/i3c/master/svc-i3c-master.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> > index 0f57a5f75e39..c252446b2bc5 100644
> > --- a/drivers/i3c/master/svc-i3c-master.c
> > +++ b/drivers/i3c/master/svc-i3c-master.c
> > @@ -331,6 +331,7 @@ static int svc_i3c_master_handle_ibi(struct svc_i3c_master *master,
> >  	struct i3c_ibi_slot *slot;
> >  	unsigned int count;
> >  	u32 mdatactrl;
> > +	int ret, val;
> >  	u8 *buf;
> >  
> >  	slot = i3c_generic_ibi_get_free_slot(data->ibi_pool);
> > @@ -340,6 +341,13 @@ static int svc_i3c_master_handle_ibi(struct svc_i3c_master *master,
> >  	slot->len = 0;
> >  	buf = slot->data;
> >  
> > +	ret = readl_relaxed_poll_timeout(master->regs + SVC_I3C_MSTATUS, val,
> > +						SVC_I3C_MSTATUS_COMPLETE(val), 0, 1000);
> 
> Are you sure !MSTATUS_COMPLETE(val) is never a valid condition?
> Especially with non-mandatory bytes?
> 
> Also, are you sure of the indentation here?

If no-mandatory data, it is equal RDTERAM is 0. It takes some times to
change my slave code to create such test case.

Frank Li

> 
> > +	if (ret) {
> > +		dev_err(master->dev, "Timeout when polling for COMPLETE\n");
> > +		return ret;
> > +	}
> > +
> >  	while (SVC_I3C_MSTATUS_RXPEND(readl(master->regs + SVC_I3C_MSTATUS))  &&
> >  	       slot->len < SVC_I3C_FIFO_SIZE) {
> >  		mdatactrl = readl(master->regs + SVC_I3C_MDATACTRL);
> 
> 
> Thanks,
> Miqu?l