diff mbox

[v4,29/32] cxlflash: Fix to double the delay each time

Message ID 1443223157-10039-1-git-send-email-mrochs@linux.vnet.ibm.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Matthew R. Ochs Sept. 25, 2015, 11:19 p.m. UTC
From: Manoj Kumar <manoj@linux.vnet.ibm.com>

The operator used to double the delay is incorrect and
does not result in delay doubling.

To fix, use a left shift instead of the XOR operator.

Reported-by: Tomas Henzl <thenzl@redhat.com>
Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>
Reviewed-by: Brian King <brking@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andrew Donnellan Sept. 29, 2015, 1:19 a.m. UTC | #1
On 26/09/15 09:19, Matthew R. Ochs wrote:
> From: Manoj Kumar <manoj@linux.vnet.ibm.com>
>
> The operator used to double the delay is incorrect and
> does not result in delay doubling.
>
> To fix, use a left shift instead of the XOR operator.
>
> Reported-by: Tomas Henzl <thenzl@redhat.com>
> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>
> Reviewed-by: Brian King <brking@linux.vnet.ibm.com>

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Daniel Axtens Sept. 29, 2015, 1:40 a.m. UTC | #2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

"Matthew R. Ochs" <mrochs@linux.vnet.ibm.com> writes:

> From: Manoj Kumar <manoj@linux.vnet.ibm.com>
>
> The operator used to double the delay is incorrect and
> does not result in delay doubling.
>
> To fix, use a left shift instead of the XOR operator.
>
I can see that the patch is correct, but this commit message is a bit
confusing. What delay? In what circumstances are you doubling it? Why?

Regards,
Daniel

> Reported-by: Tomas Henzl <thenzl@redhat.com>
> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>
> Reviewed-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>  drivers/scsi/cxlflash/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index ab11ee6..be78906 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -303,7 +303,7 @@ write_rrin:
>  		if (rrin != 0x1)
>  			break;
>  		/* Double delay each time */
> -		udelay(2 ^ nretry);
> +		udelay(2 << nretry);
>  	} while (nretry++ < MC_ROOM_RETRY_CNT);
>  }
>  
> -- 
> 2.1.0
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: GPGTools - https://gpgtools.org

iQIcBAEBCgAGBQJWCewRAAoJEPC3R3P2I92F7xQQAKxvcnUpB/xLeq6CyB3LwsB/
cqFMP7OwDRfmQZG5ld5uqfXsjCCW4c0BaVknMgpD5gAdHbrwbc5gFgYuC0kIDUrH
aEwTyXwaH7+o82agjxf7TRrsh2NtLzdSB6NN1UNL73aPVXJFKWITajIKUAnYF5d1
AvB3yb4KtX0qklo205igVceuLbBBaXY6Lz3V58XDSG2JAtVlVkamJK7M8zySrNTY
Y627zTVkbopOGaBMLmu0q92dWDk2saU2wqKolo+pRy1NpOmY2XOU9hSRqCqAhnS4
xOWj7Bdrh1HdnQX26EC+f485nOijf0zddpJ6LsgPWMElJ04N4siQwmrwG55FbOs4
QDFKPdsVdZ8qmVueGCLRdTsMNAxcobi44P1/QXxGTmsn9xqyQBgZUzbd5oRUVswI
md21qir3xjRN7g4R78084I88dM1yrHpJ6NuHDxFeXbtrw8087lKgU05Ys6m+ebuC
7nPnYRJ54frAi6a7mmBq+Xc4IbVnaGl9IZi1gujM2mkTaAk7/wl7oRMYyUTKlblp
BeoQN0cdG1fChP1yLHzZV6G1JXwSw7nP6otf7YfUIrmjbzeZ5zWovSqJjnvNYBCT
zgW681gIF3GwaN9z9I+XY9BO1P6EkYp+dgAQM7PqdR00GjFjTJO0yn8WHh113lmE
L2sC0Nezuq8V4/qtspmh
=Km/z
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew R. Ochs Sept. 29, 2015, 8:28 p.m. UTC | #3
> On Sep 28, 2015, at 8:40 PM, Daniel Axtens <dja@axtens.net> wrote:
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com> writes:
> 
>> From: Manoj Kumar <manoj@linux.vnet.ibm.com>
>> 
>> The operator used to double the delay is incorrect and
>> does not result in delay doubling.
>> 
>> To fix, use a left shift instead of the XOR operator.
>> 
> I can see that the patch is correct, but this commit message is a bit
> confusing. What delay? In what circumstances are you doubling it? Why?

This is the response delay while resetting the master context. The reset
is performed by writing a bit and then waiting for it to clear. While waiting
for it to clear, the code relaxes the delta between MMIO reads.



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Axtens Sept. 30, 2015, 12:08 a.m. UTC | #4
"Matthew R. Ochs" <mrochs@linux.vnet.ibm.com> writes:

>> On Sep 28, 2015, at 8:40 PM, Daniel Axtens <dja@axtens.net> wrote:
>> 
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA512
>> 
>> "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com> writes:
>> 
>>> From: Manoj Kumar <manoj@linux.vnet.ibm.com>
>>> 
>>> The operator used to double the delay is incorrect and
>>> does not result in delay doubling.
>>> 
>>> To fix, use a left shift instead of the XOR operator.
>>> 
>> I can see that the patch is correct, but this commit message is a bit
>> confusing. What delay? In what circumstances are you doubling it? Why?
>
> This is the response delay while resetting the master context. The reset
> is performed by writing a bit and then waiting for it to clear. While waiting
> for it to clear, the code relaxes the delta between MMIO reads.

OK. If you do a v5, please include this in the commit message.

Regards,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index ab11ee6..be78906 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -303,7 +303,7 @@  write_rrin:
 		if (rrin != 0x1)
 			break;
 		/* Double delay each time */
-		udelay(2 ^ nretry);
+		udelay(2 << nretry);
 	} while (nretry++ < MC_ROOM_RETRY_CNT);
 }