| Message ID | 20240828132526.25719-1-quic_mapa@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2] scsi: ufs: qcom: update MODE_MAX cfg_bw value | expand |
On Wed, Aug 28, 2024 at 06:55:26PM +0530, Manish Pandey wrote: > The cfg_bw value for max mode was incorrect for the Qualcomm SoC. What do you mean by 'incorrect'? I extracted the value from downstream DTs. So it cannot be incorrect. If you want to update it, please clearly provide the reason. And if this patch is addressing an issue, then a Fixes tag should be present. If you want to get it backported (if it is a critical fix), then stable list should be CCed. - Mani > Update it to the correct value for cfg_bw max mode. > > Signed-off-by: Manish Pandey <quic_mapa@quicinc.com> > --- > drivers/ufs/host/ufs-qcom.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index c87fdc849c62..ecdfff2456e3 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -93,7 +93,7 @@ static const struct __ufs_qcom_bw_table { > [MODE_HS_RB][UFS_HS_G3][UFS_LANE_2] = { 1492582, 204800 }, > [MODE_HS_RB][UFS_HS_G4][UFS_LANE_2] = { 2915200, 409600 }, > [MODE_HS_RB][UFS_HS_G5][UFS_LANE_2] = { 5836800, 819200 }, > - [MODE_MAX][0][0] = { 7643136, 307200 }, > + [MODE_MAX][0][0] = { 7643136, 819200 }, > }; > > static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host); > -- > 2.17.1 >
On 8/28/2024 7:01 PM, Manivannan Sadhasivam wrote: > On Wed, Aug 28, 2024 at 06:55:26PM +0530, Manish Pandey wrote: >> The cfg_bw value for max mode was incorrect for the Qualcomm SoC. > > What do you mean by 'incorrect'? I extracted the value from downstream DTs. So > it cannot be incorrect. > > If you want to update it, please clearly provide the reason. Hi Mani, From the snip from commit message "The bandwidth values defined in ufs_qcom_bw_table struct are taken from Qcom downstream vendor devicetree source and are calculated as per the UFS3.1 Spec." we have UFS 4.x devices, and ufs_qcom_bw_table is already updated with Gear 5 support (8db8f6ce556a - "scsi: ufs: qcom: Add missing interconnect bandwidth values for Gear 5"). So the max cfg_bw is not updated. Also for UFS 3.x devices, [MODE_HS_RB][UFS_HS_G3][UFS_LANE_2] = { 1492582, 204800 }, [MODE_HS_RB][UFS_HS_G4][UFS_LANE_2] = { 2915200, 409600 }, [MODE_MAX][0][0] = { 7643136, 307200 }, Please have a look for current max mode value(307200), it is even less than UFS_HS_G4 (409600). So it should be updated. > And if this patch is addressing an issue, then a Fixes tag should be present. If > you want to get it backported (if it is a critical fix), then stable list should > be CCed. > > - Mani > My bad.. will update the patch. also yes, we need it to be backported, i should cc add stable list. Thanks for guidance. >> Update it to the correct value for cfg_bw max mode. >> >> Signed-off-by: Manish Pandey <quic_mapa@quicinc.com> >> --- >> drivers/ufs/host/ufs-qcom.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c >> index c87fdc849c62..ecdfff2456e3 100644 >> --- a/drivers/ufs/host/ufs-qcom.c >> +++ b/drivers/ufs/host/ufs-qcom.c >> @@ -93,7 +93,7 @@ static const struct __ufs_qcom_bw_table { >> [MODE_HS_RB][UFS_HS_G3][UFS_LANE_2] = { 1492582, 204800 }, >> [MODE_HS_RB][UFS_HS_G4][UFS_LANE_2] = { 2915200, 409600 }, >> [MODE_HS_RB][UFS_HS_G5][UFS_LANE_2] = { 5836800, 819200 }, >> - [MODE_MAX][0][0] = { 7643136, 307200 }, >> + [MODE_MAX][0][0] = { 7643136, 819200 }, >> }; >> >> static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host); >> -- >> 2.17.1 >> >
On Thu, Aug 29, 2024 at 05:44:21PM +0530, MANISH PANDEY wrote: > > On 8/28/2024 7:01 PM, Manivannan Sadhasivam wrote: > > On Wed, Aug 28, 2024 at 06:55:26PM +0530, Manish Pandey wrote: > > > The cfg_bw value for max mode was incorrect for the Qualcomm SoC. > > > > What do you mean by 'incorrect'? I extracted the value from downstream DTs. So > > it cannot be incorrect. > > > > If you want to update it, please clearly provide the reason. > > Hi Mani, > > From the snip from commit message > "The bandwidth values defined in ufs_qcom_bw_table struct are taken from > Qcom downstream vendor devicetree source and are calculated as per the > UFS3.1 Spec." > > we have UFS 4.x devices, and ufs_qcom_bw_table is already updated with Gear > 5 support (8db8f6ce556a - "scsi: ufs: qcom: Add missing interconnect > bandwidth values for Gear 5"). So the max cfg_bw is not updated. > > Also for UFS 3.x devices, > [MODE_HS_RB][UFS_HS_G3][UFS_LANE_2] = { 1492582, 204800 }, > [MODE_HS_RB][UFS_HS_G4][UFS_LANE_2] = { 2915200, 409600 }, > [MODE_MAX][0][0] = { 7643136, 307200 }, > > Please have a look for current max mode value(307200), it is even less than > UFS_HS_G4 (409600). So it should be updated. > Okay, then you should mention that the max value is updated for UFS 4.x devices and mention that commit 8db8f6ce556a missed adding them. Also add the fixes tag for 8db8f6ce556a. - Mani
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index c87fdc849c62..ecdfff2456e3 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -93,7 +93,7 @@ static const struct __ufs_qcom_bw_table { [MODE_HS_RB][UFS_HS_G3][UFS_LANE_2] = { 1492582, 204800 }, [MODE_HS_RB][UFS_HS_G4][UFS_LANE_2] = { 2915200, 409600 }, [MODE_HS_RB][UFS_HS_G5][UFS_LANE_2] = { 5836800, 819200 }, - [MODE_MAX][0][0] = { 7643136, 307200 }, + [MODE_MAX][0][0] = { 7643136, 819200 }, }; static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host);
The cfg_bw value for max mode was incorrect for the Qualcomm SoC. Update it to the correct value for cfg_bw max mode. Signed-off-by: Manish Pandey <quic_mapa@quicinc.com> --- drivers/ufs/host/ufs-qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)