| Message ID | 20250115022344.3967-1-dh0421.hwang@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: ufs: core: increase the NOP_OUT command timeout | expand |
On 1/14/25 6:23 PM, DooHyun Hwang wrote: > It is found that is UFS device may take longer than 500ms(50ms * 10times) > to respond to NOP_OUT command. > > The NOP_OUT command timeout was total 500ms that is from > a timeout value of 50ms(defined by NOP_OUT_TIMEOUT) > with 10 retries(defined by NOP_OUT_RETRIES) > > This change increase the NOP_OUT command timeout to total 1000ms > by changing timeout value to 100ms(NOP_OUT_TIMEOUT) > > Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com> > --- > drivers/ufs/core/ufshcd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index cd404ade48dc..bf5c4620ef6b 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -57,8 +57,8 @@ enum { > }; > /* NOP OUT retries waiting for NOP IN response */ > #define NOP_OUT_RETRIES 10 > -/* Timeout after 50 msecs if NOP OUT hangs without response */ > -#define NOP_OUT_TIMEOUT 50 /* msecs */ > +/* Timeout after 100 msecs if NOP OUT hangs without response */ > +#define NOP_OUT_TIMEOUT 100 /* msecs */ > > /* Query request retries */ > #define QUERY_REQ_RETRIES 3 The above change relies on all device management commands being issued with the same tag. If a single NOP OUT command may take longer than 500 ms, shouldn't NOP_OUT_TIMEOUT be increased to 1000 ms instead of 100 ms? The number of NOP OUT retries seems high to me and probably can be reduced? Thanks, Bart.
> On 1/14/25 6:23 PM, DooHyun Hwang wrote: > > It is found that is UFS device may take longer than 500ms(50ms * > > 10times) to respond to NOP_OUT command. > > > > The NOP_OUT command timeout was total 500ms that is from a timeout > > value of 50ms(defined by NOP_OUT_TIMEOUT) with 10 retries(defined by > > NOP_OUT_RETRIES) > > > > This change increase the NOP_OUT command timeout to total 1000ms by > > changing timeout value to 100ms(NOP_OUT_TIMEOUT) > > > > Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com> > > --- > > drivers/ufs/core/ufshcd.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > > index cd404ade48dc..bf5c4620ef6b 100644 > > --- a/drivers/ufs/core/ufshcd.c > > +++ b/drivers/ufs/core/ufshcd.c > > @@ -57,8 +57,8 @@ enum { > > }; > > /* NOP OUT retries waiting for NOP IN response */ > > #define NOP_OUT_RETRIES 10 > > -/* Timeout after 50 msecs if NOP OUT hangs without response */ > > -#define NOP_OUT_TIMEOUT 50 /* msecs */ > > +/* Timeout after 100 msecs if NOP OUT hangs without response */ > > +#define NOP_OUT_TIMEOUT 100 /* msecs */ > > > > /* Query request retries */ > > #define QUERY_REQ_RETRIES 3 > > The above change relies on all device management commands being issued > with the same tag. If a single NOP OUT command may take longer than > 500 ms, shouldn't NOP_OUT_TIMEOUT be increased to 1000 ms instead of > 100 ms? The number of NOP OUT retries seems high to me and probably can be > reduced? > > Thanks, > > Bart. I want to keep sending NOP_OUT commands repeatedly to get a response from the UFS device, as per the existing method. To accommodate this method, I propose increasing the total timeout duration by increasing the single timeout value(defined by NOP_OUT_TIMEOUT) from 50ms to 100ms rather than increasing the timeout value(defined by NOP_OUT_TIMEOUT) from 50ms to 1000ms or increasing the retry count value(defined by NOP_OUT_RETRIES). This is time measurement log confirmed on a real device with NOP_OUT_TIMEOUT is 100ms 1. normal operation [ 2.010156] [6: kworker/u18:0: 76] ufshcd-qcom 1d84000.ufshc: [TEST] ufshcd_verify_dev_init: takes 1271 us, retries = 1 * 100ms. 2. issued log : exceeds 500ms [ 2.524525] [6: kworker/u17:2: 141] ufshcd-qcom 1d84000.ufshc: [TEST] ufshcd_verify_dev_init: takes 533000 us, retries = 6 * 100ms. And a certain UFS vendor has confirmed that the response to NOP_OUT command can be delayed by up to 540ms in certain circumstances on a specific model. BR, Thank you. DooHyun Hwang.
> It is found that is UFS device may take longer than 500ms(50ms * 10times) to > respond to NOP_OUT command. > > The NOP_OUT command timeout was total 500ms that is from a timeout > value of 50ms(defined by NOP_OUT_TIMEOUT) with 10 retries(defined by > NOP_OUT_RETRIES) > > This change increase the NOP_OUT command timeout to total 1000ms by > changing timeout value to 100ms(NOP_OUT_TIMEOUT) > > Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com> Why not edit hba->nop_out_timeout in the .init vop? Like some vendors already do. Thanks, Avri > --- > drivers/ufs/core/ufshcd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index > cd404ade48dc..bf5c4620ef6b 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -57,8 +57,8 @@ enum { > }; > /* NOP OUT retries waiting for NOP IN response */ > #define NOP_OUT_RETRIES 10 > -/* Timeout after 50 msecs if NOP OUT hangs without response */ > -#define NOP_OUT_TIMEOUT 50 /* msecs */ > +/* Timeout after 100 msecs if NOP OUT hangs without response */ > +#define NOP_OUT_TIMEOUT 100 /* msecs */ > > /* Query request retries */ > #define QUERY_REQ_RETRIES 3 > -- > 2.48.0 >
> > > It is found that is UFS device may take longer than 500ms(50ms * > > 10times) to respond to NOP_OUT command. > > > > The NOP_OUT command timeout was total 500ms that is from a timeout > > value of 50ms(defined by NOP_OUT_TIMEOUT) with 10 retries(defined by > > NOP_OUT_RETRIES) > > > > This change increase the NOP_OUT command timeout to total 1000ms by > > changing timeout value to 100ms(NOP_OUT_TIMEOUT) > > > > Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com> > Why not edit hba->nop_out_timeout in the .init vop? > Like some vendors already do. > > Thanks, > Avri > Thank you for your suggestion. I'll fix that in .init vop as you said. And I'll reject this patch. BR, Thank you. DooHyun Hwang. > > --- > > drivers/ufs/core/ufshcd.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > > index cd404ade48dc..bf5c4620ef6b 100644 > > --- a/drivers/ufs/core/ufshcd.c > > +++ b/drivers/ufs/core/ufshcd.c > > @@ -57,8 +57,8 @@ enum { > > }; > > /* NOP OUT retries waiting for NOP IN response */ > > #define NOP_OUT_RETRIES 10 > > -/* Timeout after 50 msecs if NOP OUT hangs without response */ > > -#define NOP_OUT_TIMEOUT 50 /* msecs */ > > +/* Timeout after 100 msecs if NOP OUT hangs without response */ > > +#define NOP_OUT_TIMEOUT 100 /* msecs */ > > > > /* Query request retries */ > > #define QUERY_REQ_RETRIES 3 > > -- > > 2.48.0 > >
On 1/15/25 6:02 PM, DooHyun Hwang(황두현/Device S/W Solution Lab.(MX)/삼성전자) wrote: > I want to keep sending NOP_OUT commands repeatedly to get a response > from the UFS device, as per the existing method. To accommodate this method, > I propose increasing the total timeout duration by increasing the single timeout > value(defined by NOP_OUT_TIMEOUT) from 50ms to 100ms rather than > increasing the timeout value(defined by NOP_OUT_TIMEOUT) from 50ms to 1000ms > or increasing the retry count value(defined by NOP_OUT_RETRIES). > > This is time measurement log confirmed on a real device with NOP_OUT_TIMEOUT is 100ms > > 1. normal operation > [ 2.010156] [6: kworker/u18:0: 76] ufshcd-qcom 1d84000.ufshc: [TEST] ufshcd_verify_dev_init: takes 1271 us, retries = 1 * 100ms. > > 2. issued log : exceeds 500ms > [ 2.524525] [6: kworker/u17:2: 141] ufshcd-qcom 1d84000.ufshc: [TEST] ufshcd_verify_dev_init: takes 533000 us, retries = 6 * 100ms. > > And a certain UFS vendor has confirmed that the response to NOP_OUT command > can be delayed by up to 540ms in certain circumstances on a specific model. Thank you for having provided all this additional information. Because of the above clarification, feel free to add: Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On 1/16/2025 1:59 AM, DooHyun Hwang wrote: >> >>> It is found that is UFS device may take longer than 500ms(50ms * >>> 10times) to respond to NOP_OUT command. >>> >>> The NOP_OUT command timeout was total 500ms that is from a timeout >>> value of 50ms(defined by NOP_OUT_TIMEOUT) with 10 retries(defined by >>> NOP_OUT_RETRIES) >>> >>> This change increase the NOP_OUT command timeout to total 1000ms by >>> changing timeout value to 100ms(NOP_OUT_TIMEOUT) >>> >>> Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com> >> Why not edit hba->nop_out_timeout in the .init vop? >> Like some vendors already do. >> >> Thanks, >> Avri >> > Thank you for your suggestion. > I'll fix that in .init vop as you said. > > And I'll reject this patch. Hi DooHyun Hwang, Since this is a common issue that multiple platform vendors have to fix in their vops, should we fix it in the common code instead? Reviewed-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index cd404ade48dc..bf5c4620ef6b 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -57,8 +57,8 @@ enum { }; /* NOP OUT retries waiting for NOP IN response */ #define NOP_OUT_RETRIES 10 -/* Timeout after 50 msecs if NOP OUT hangs without response */ -#define NOP_OUT_TIMEOUT 50 /* msecs */ +/* Timeout after 100 msecs if NOP OUT hangs without response */ +#define NOP_OUT_TIMEOUT 100 /* msecs */ /* Query request retries */ #define QUERY_REQ_RETRIES 3
It is found that is UFS device may take longer than 500ms(50ms * 10times) to respond to NOP_OUT command. The NOP_OUT command timeout was total 500ms that is from a timeout value of 50ms(defined by NOP_OUT_TIMEOUT) with 10 retries(defined by NOP_OUT_RETRIES) This change increase the NOP_OUT command timeout to total 1000ms by changing timeout value to 100ms(NOP_OUT_TIMEOUT) Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com> --- drivers/ufs/core/ufshcd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)