Message ID | 20240112135332.24957-1-quic_vdadhani@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-24713-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:693c:2614:b0:101:6a76:bbe3 with SMTP id mm20csp184982dyc; Fri, 12 Jan 2024 05:54:24 -0800 (PST) X-Google-Smtp-Source: AGHT+IHsSMyqhsaKcCBJQtkp6AYzgFH/xdBVnRdNAGOrouD+HOTxClDVtZxlKVhVTKUYMPeLlBqr X-Received: by 2002:a05:620a:2414:b0:783:3004:831 with SMTP id d20-20020a05620a241400b0078330040831mr1866372qkn.72.1705067664452; Fri, 12 Jan 2024 05:54:24 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1705067664; cv=none; d=google.com; s=arc-20160816; b=U3/5eMBke4Oj1YtviEcFbYsZwCGoRev7h+k/R2mh+PAcj9/9APhzTShn/RqBevNcmV di9XC0BrTy2aE0rNQgfZojGN3c6buSfA3so5UsHLIZy2p5PLdOdi4cAhTuVkmWQxeeTU ncpGTFCRKFQAqfILkEdT/GqXeMMgYHETJr8hFtGNr2S3KThZOiCVkC41sFLzNUpyOyH4 ylQnB5bSJwO6mPN1wahvwU49pYEkQK2aHhaJYdTZ+7+0ed06tkXLG+dj1yfeU6IUuwyj coSylcxnD5VcrpIMwfZUhun7q5XEe+1aZbbT4p+2J7S+Phdy0un+hoyYOFeM4P8c3h8R T4tQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-unsubscribe:list-subscribe:list-id:precedence:message-id:date :subject:cc:to:from:dkim-signature; bh=DhVXr6gyIbMgzR2b0surHFdqtEQz2xq2bAyxCY1BWNg=; fh=h7r/JXHXZXoLdkNXavM1/yylu1iYTEH44xKrqJ38ukQ=; b=uot7U/TUrcDcZET5VKCfd0f8a2SI6lOjr/0fZeOB8UyXQZNDm5kNVQ0jR3Y3p0JclR xxSDyiXmivrF6yYYgGryJG+KmrYR+4u7pjsGsR7U/zHBLye/EBuA0hDirhIzwoi7rLqj GpvSBYJCEXZbZxqSn69XOgfI9j13FtZ8XpJV3y0zLZaPakM8z4oAm/d7uPO7Jh6GX6I8 DWNZ34i7WcqzPKWKXyRydeHxywEAjmM22rBER0qTuJdB4Ox1pbaQL8LMXQPy/vKt+tvF /qcgdx48SB/6CrVv8paNOXj/P1Yr+SbaAFNEUPKg0345TwHNro3MjGttWEj2MdaxfAPD EvtQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=AievCUHb; spf=pass (google.com: domain of linux-kernel+bounces-24713-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-24713-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id br34-20020a05620a462200b0077d685d25b6si3045195qkb.252.2024.01.12.05.54.24 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Jan 2024 05:54:24 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-24713-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=AievCUHb; spf=pass (google.com: domain of linux-kernel+bounces-24713-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-24713-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 3DA7E1C23A07 for <ouuuleilei@gmail.com>; Fri, 12 Jan 2024 13:54:24 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id CCA956D1DD; Fri, 12 Jan 2024 13:54:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=quicinc.com header.i=@quicinc.com header.b="AievCUHb" Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C72B86D1B4; Fri, 12 Jan 2024 13:54:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=quicinc.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=qualcomm.com Received: from pps.filterd (m0279867.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.24/8.17.1.24) with ESMTP id 40CDIW99009300; Fri, 12 Jan 2024 13:53:46 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h= from:to:cc:subject:date:message-id; s=qcppdkim1; bh=DhVXr6gyIbMg zR2b0surHFdqtEQz2xq2bAyxCY1BWNg=; b=AievCUHbdq/ugHWm/OsV+lLygUYw K4NGsL8+BJKPUcnaL5SLCddxxnLGtTrMlu/YrkUMYAEX3mKJ6rMsXWz8ZVlcvz1+ RBDQwQjJHe8is7YTZnhNKGI+S7wOmHMyknU/EpYpZl2mcKI10rrNyU4Hf7WBbhch 1wmMuY95zzYuRv9R9lzZzgcpCo3LlP9Cj/oIwKLQQr780eBCGW5MvAsCX4JRWrEA J7LObRPepcjDFQVOLNC57IDNKcy+qdEboAgBom+AEedwPK8shDK9XUCrT+FAobpM obsUYXxRqVwwkVmwKo53WKfamBm698dM9jDzQXwJXgJM4wUIjpdJfpA1MA== Received: from apblrppmta02.qualcomm.com (blr-bdr-fw-01_GlobalNAT_AllZones-Outside.qualcomm.com [103.229.18.19]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3vjrqj1ysn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 12 Jan 2024 13:53:46 +0000 (GMT) Received: from pps.filterd (APBLRPPMTA02.qualcomm.com [127.0.0.1]) by APBLRPPMTA02.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTP id 40CDrhs3005326; Fri, 12 Jan 2024 13:53:43 GMT Received: from pps.reinject (localhost [127.0.0.1]) by APBLRPPMTA02.qualcomm.com (PPS) with ESMTPS id 3veyxm4mxw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 12 Jan 2024 13:53:43 +0000 Received: from APBLRPPMTA02.qualcomm.com (APBLRPPMTA02.qualcomm.com [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 40CDrgeK005319; Fri, 12 Jan 2024 13:53:42 GMT Received: from hu-maiyas-hyd.qualcomm.com (hu-vdadhani-hyd.qualcomm.com [10.213.106.28]) by APBLRPPMTA02.qualcomm.com (PPS) with ESMTP id 40CDrg91005316; Fri, 12 Jan 2024 13:53:42 +0000 Received: by hu-maiyas-hyd.qualcomm.com (Postfix, from userid 4047106) id 29C915013A9; Fri, 12 Jan 2024 19:23:41 +0530 (+0530) From: Viken Dadhaniya <quic_vdadhani@quicinc.com> To: andersson@kernel.org, konrad.dybcio@linaro.org, andi.shyti@kernel.org, linux-arm-msm@vger.kernel.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, vkoul@kernel.org Cc: quic_msavaliy@quicinc.com, quic_vtanuku@quicinc.com, Viken Dadhaniya <quic_vdadhani@quicinc.com> Subject: [PATCH 1/1] i2c: i2c-qcom-geni: Correct I2C TRE sequence Date: Fri, 12 Jan 2024 19:23:32 +0530 Message-Id: <20240112135332.24957-1-quic_vdadhani@quicinc.com> X-Mailer: git-send-email 2.17.1 X-QCInternal: smtphost X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: OoHBo6GdfJgBuPdX_brWoMYa6rkH5ixG X-Proofpoint-GUID: OoHBo6GdfJgBuPdX_brWoMYa6rkH5ixG X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-09_02,2023-12-07_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 impostorscore=0 malwarescore=0 mlxlogscore=720 mlxscore=0 clxscore=1011 adultscore=0 phishscore=0 priorityscore=1501 lowpriorityscore=0 bulkscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2311290000 definitions=main-2401120109 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> X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1787893031538131773 X-GMAIL-MSGID: 1787893031538131773 |
Series |
[1/1] i2c: i2c-qcom-geni: Correct I2C TRE sequence
|
|
Commit Message
Viken Dadhaniya
Jan. 12, 2024, 1:53 p.m. UTC
For i2c read operation, we are getting gsi mode timeout due
to malformed TRE(Transfer Ring Element). currently for read opreration,
we are configuring incorrect TRE sequence(config->dma->go).
So correct TRE sequence(config->go->dma) to resolve timeout
issue for read operation.
Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com>
---
drivers/i2c/busses/i2c-qcom-geni.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
Comments
On 12/01/2024 13:53, Viken Dadhaniya wrote: > For i2c read operation, we are getting gsi mode timeout due > to malformed TRE(Transfer Ring Element). currently for read opreration, > we are configuring incorrect TRE sequence(config->dma->go). > > So correct TRE sequence(config->go->dma) to resolve timeout > issue for read operation. I don't think this commit log really captures what the code does. - Sets up optional RX DMA - Sets up TX DMA - Issues optional RX dma_async_issue_pending - Issues TX dma_async_issue_pending What your change does is sets up the TX DMA first - Sets up TX DMA - Sets up optional RX DMA - Issues optional RX dma_async_issue_pending - Issues TX dma_async_issue_pending but you've not really root-caused by re-ordering the calls fixes anything for you. This may be the right fix but I don't really think you've captured here in the commit log _why_ its the right fix if indeed it is correct. > Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com> You should have a Fixes: tag > --- > drivers/i2c/busses/i2c-qcom-geni.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c > index 0d2e7171e3a6..5904fc8bba71 100644 > --- a/drivers/i2c/busses/i2c-qcom-geni.c > +++ b/drivers/i2c/busses/i2c-qcom-geni.c > @@ -613,6 +613,11 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i > > peripheral.addr = msgs[i].addr; > > + ret = geni_i2c_gpi(gi2c, &msgs[i], &config, > + &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c); > + if (ret) > + goto err; > + > if (msgs[i].flags & I2C_M_RD) { > ret = geni_i2c_gpi(gi2c, &msgs[i], &config, > &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c); > @@ -620,11 +625,6 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i > goto err; > } > > - ret = geni_i2c_gpi(gi2c, &msgs[i], &config, > - &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c); > - if (ret) > - goto err; > - > if (msgs[i].flags & I2C_M_RD) > dma_async_issue_pending(gi2c->rx_c); If TX gets moved up top then the second check for if (msgs[i].flags & I2C_M_RD) is redundant. You could just have if (msgs[i].flags & I2C_M_RD) { ret = geni_i2c_gpi(gi2c, &msgs[i], &config, &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c); if (ret) goto err; dma_async_issue_pending(gi2c->rx_c); } - Please investigate further. Why/how does the new sequence TX DMA setup RX DMA setup RX DMA sync TX DMA sync Improve the situation over the existing and more logical RX DMA setup TX DMA setup RX DMA sync TX DMA sync - Add a Fixes tag if you work that out so we know which kernel version to back port to - Include the SoC version(s) you have tested on in the commit or cover letter - And drop the redundant check --- bod
> -----Original Message----- > From: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Sent: Friday, January 12, 2024 8:25 PM > To: Viken Dadhaniya (QUIC) <quic_vdadhani@quicinc.com>; > andersson@kernel.org; konrad.dybcio@linaro.org; andi.shyti@kernel.org; linux- > arm-msm@vger.kernel.org; linux-i2c@vger.kernel.org; linux- > kernel@vger.kernel.org; vkoul@kernel.org > Cc: Mukesh Savaliya (QUIC) <quic_msavaliy@quicinc.com>; Visweswara Tanuku > (QUIC) <quic_vtanuku@quicinc.com> > Subject: Re: [PATCH 1/1] i2c: i2c-qcom-geni: Correct I2C TRE sequence > > WARNING: This email originated from outside of Qualcomm. Please be wary of > any links or attachments, and do not enable macros. > > On 12/01/2024 13:53, Viken Dadhaniya wrote: > > For i2c read operation, we are getting gsi mode timeout due to > > malformed TRE(Transfer Ring Element). currently for read opreration, > > we are configuring incorrect TRE sequence(config->dma->go). > > > > So correct TRE sequence(config->go->dma) to resolve timeout issue for > > read operation. > > I don't think this commit log really captures what the code does. > > - Sets up optional RX DMA > - Sets up TX DMA > - Issues optional RX dma_async_issue_pending > - Issues TX dma_async_issue_pending > > What your change does is sets up the TX DMA first > > - Sets up TX DMA > - Sets up optional RX DMA > - Issues optional RX dma_async_issue_pending > - Issues TX dma_async_issue_pending > > but you've not really root-caused by re-ordering the calls fixes anything for you. > > This may be the right fix but I don't really think you've captured here in the > commit log _why_ its the right fix if indeed it is correct. Updated commit massage with proper information. > > > Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com> > > You should have a Fixes: tag Added fixes tag. > > > --- > > drivers/i2c/busses/i2c-qcom-geni.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c > > b/drivers/i2c/busses/i2c-qcom-geni.c > > index 0d2e7171e3a6..5904fc8bba71 100644 > > --- a/drivers/i2c/busses/i2c-qcom-geni.c > > +++ b/drivers/i2c/busses/i2c-qcom-geni.c > > @@ -613,6 +613,11 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev > > *gi2c, struct i2c_msg msgs[], i > > > > peripheral.addr = msgs[i].addr; > > > > + ret = geni_i2c_gpi(gi2c, &msgs[i], &config, > > + &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c); > > + if (ret) > > + goto err; > > + > > if (msgs[i].flags & I2C_M_RD) { > > ret = geni_i2c_gpi(gi2c, &msgs[i], &config, > > &rx_addr, &rx_buf, I2C_READ, > > gi2c->rx_c); @@ -620,11 +625,6 @@ static int geni_i2c_gpi_xfer(struct > geni_i2c_dev *gi2c, struct i2c_msg msgs[], i > > goto err; > > } > > > > - ret = geni_i2c_gpi(gi2c, &msgs[i], &config, > > - &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c); > > - if (ret) > > - goto err; > > - > > if (msgs[i].flags & I2C_M_RD) > > dma_async_issue_pending(gi2c->rx_c); > > If TX gets moved up top then the second check for if (msgs[i].flags & > I2C_M_RD) is redundant. > > You could just have > > if (msgs[i].flags & I2C_M_RD) { > ret = geni_i2c_gpi(gi2c, &msgs[i], &config, > &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c); > if (ret) > goto err; > > dma_async_issue_pending(gi2c->rx_c); > } > > - Please investigate further. > Why/how does the new sequence > > TX DMA setup > RX DMA setup > RX DMA sync > TX DMA sync > > Improve the situation over the existing and more logical > > RX DMA setup > TX DMA setup > RX DMA sync > TX DMA sync > > - Add a Fixes tag if you work that out so we know > which kernel version to back port to > > - Include the SoC version(s) you have tested on in the commit > or cover letter > > - And drop the redundant check Removed redundant check. Added SoC information. > > --- > bod
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index 0d2e7171e3a6..5904fc8bba71 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -613,6 +613,11 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i peripheral.addr = msgs[i].addr; + ret = geni_i2c_gpi(gi2c, &msgs[i], &config, + &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c); + if (ret) + goto err; + if (msgs[i].flags & I2C_M_RD) { ret = geni_i2c_gpi(gi2c, &msgs[i], &config, &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c); @@ -620,11 +625,6 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i goto err; } - ret = geni_i2c_gpi(gi2c, &msgs[i], &config, - &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c); - if (ret) - goto err; - if (msgs[i].flags & I2C_M_RD) dma_async_issue_pending(gi2c->rx_c); dma_async_issue_pending(gi2c->tx_c);