| Message ID | 20220719101903.v2.1.I22460c4f4a9ccf2c96c3f9bb392b409926d80b2f@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] scsi: ufs: ufs-pci: Correct check for RESET DSM | expand |
On 7/18/22 17:19, Daniil Lunev wrote: > dsm_fns is a bitmap, and it is 0-indexed according to the check in > __intel_dsm funciton. But common initialization was checking it as if it > was 1-indexed. The CL corrects the discrepancy. This change won't break > any existing calls to the function, since before the change both bits 0 > and 1 were checked and needed to be set. Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On 19/07/22 03:19, Daniil Lunev wrote: > dsm_fns is a bitmap, and it is 0-indexed according to the check in > __intel_dsm funciton. But common initialization was checking it as if it funciton -> function > was 1-indexed. The CL corrects the discrepancy. This change won't break "The CL" -> "This patch" > any existing calls to the function, since before the change both bits 0 > and 1 were checked and needed to be set. > > Signed-off-by: Daniil Lunev <dlunev@chromium.org> Some minor cosmetic issues, but with those amended: Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> > > --- > > Changes in v2: > * Make __INTEL_DSM_SUPPORTED a function > * use `1u` instead of `1` in shift operator > > drivers/ufs/host/ufshcd-pci.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c > index 04166bda41daa..c13c31c2ae793 100644 > --- a/drivers/ufs/host/ufshcd-pci.c > +++ b/drivers/ufs/host/ufshcd-pci.c > @@ -24,7 +24,7 @@ struct ufs_host { > void (*late_init)(struct ufs_hba *hba); > }; > > -enum { > +enum intel_ufs_dsm_func_id { > INTEL_DSM_FNS = 0, > INTEL_DSM_RESET = 1, > }; > @@ -42,6 +42,15 @@ static const guid_t intel_dsm_guid = > GUID_INIT(0x1A4832A0, 0x7D03, 0x43CA, > 0xB0, 0x20, 0xF6, 0xDC, 0xD1, 0x2A, 0x19, 0x50); > > +static bool __intel_dsm_supported(struct intel_host *host, > + enum intel_ufs_dsm_func_id fn) CHECK: Alignment should match open parenthesis #35: FILE: drivers/ufs/host/ufshcd-pci.c:46: +static bool __intel_dsm_supported(struct intel_host *host, + enum intel_ufs_dsm_func_id fn) > +{ > + return fn < 32 && fn >= 0 && (host->dsm_fns & (1u << fn)); > +} > + > +#define INTEL_DSM_SUPPORTED(host, name) \ > + __intel_dsm_supported(host, INTEL_DSM_##name) > + > static int __intel_dsm(struct intel_host *intel_host, struct device *dev, > unsigned int fn, u32 *result) > { > @@ -71,7 +80,7 @@ static int __intel_dsm(struct intel_host *intel_host, struct device *dev, > static int intel_dsm(struct intel_host *intel_host, struct device *dev, > unsigned int fn, u32 *result) > { > - if (fn > 31 || !(intel_host->dsm_fns & (1 << fn))) > + if (!__intel_dsm_supported(intel_host, fn)) > return -EOPNOTSUPP; > > return __intel_dsm(intel_host, dev, fn, result); > @@ -300,7 +309,7 @@ static int ufs_intel_device_reset(struct ufs_hba *hba) > { > struct intel_host *host = ufshcd_get_variant(hba); > > - if (host->dsm_fns & INTEL_DSM_RESET) { > + if (INTEL_DSM_SUPPORTED(host, RESET)) { > u32 result = 0; > int err; > > @@ -342,7 +351,7 @@ static int ufs_intel_common_init(struct ufs_hba *hba) > return -ENOMEM; > ufshcd_set_variant(hba, host); > intel_dsm_init(host, hba->dev); > - if (host->dsm_fns & INTEL_DSM_RESET) { > + if (INTEL_DSM_SUPPORTED(host, RESET)) { > if (hba->vops->device_reset) > hba->caps |= UFSHCD_CAP_DEEPSLEEP; > } else {
diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c index 04166bda41daa..c13c31c2ae793 100644 --- a/drivers/ufs/host/ufshcd-pci.c +++ b/drivers/ufs/host/ufshcd-pci.c @@ -24,7 +24,7 @@ struct ufs_host { void (*late_init)(struct ufs_hba *hba); }; -enum { +enum intel_ufs_dsm_func_id { INTEL_DSM_FNS = 0, INTEL_DSM_RESET = 1, }; @@ -42,6 +42,15 @@ static const guid_t intel_dsm_guid = GUID_INIT(0x1A4832A0, 0x7D03, 0x43CA, 0xB0, 0x20, 0xF6, 0xDC, 0xD1, 0x2A, 0x19, 0x50); +static bool __intel_dsm_supported(struct intel_host *host, + enum intel_ufs_dsm_func_id fn) +{ + return fn < 32 && fn >= 0 && (host->dsm_fns & (1u << fn)); +} + +#define INTEL_DSM_SUPPORTED(host, name) \ + __intel_dsm_supported(host, INTEL_DSM_##name) + static int __intel_dsm(struct intel_host *intel_host, struct device *dev, unsigned int fn, u32 *result) { @@ -71,7 +80,7 @@ static int __intel_dsm(struct intel_host *intel_host, struct device *dev, static int intel_dsm(struct intel_host *intel_host, struct device *dev, unsigned int fn, u32 *result) { - if (fn > 31 || !(intel_host->dsm_fns & (1 << fn))) + if (!__intel_dsm_supported(intel_host, fn)) return -EOPNOTSUPP; return __intel_dsm(intel_host, dev, fn, result); @@ -300,7 +309,7 @@ static int ufs_intel_device_reset(struct ufs_hba *hba) { struct intel_host *host = ufshcd_get_variant(hba); - if (host->dsm_fns & INTEL_DSM_RESET) { + if (INTEL_DSM_SUPPORTED(host, RESET)) { u32 result = 0; int err; @@ -342,7 +351,7 @@ static int ufs_intel_common_init(struct ufs_hba *hba) return -ENOMEM; ufshcd_set_variant(hba, host); intel_dsm_init(host, hba->dev); - if (host->dsm_fns & INTEL_DSM_RESET) { + if (INTEL_DSM_SUPPORTED(host, RESET)) { if (hba->vops->device_reset) hba->caps |= UFSHCD_CAP_DEEPSLEEP; } else {
dsm_fns is a bitmap, and it is 0-indexed according to the check in __intel_dsm funciton. But common initialization was checking it as if it was 1-indexed. The CL corrects the discrepancy. This change won't break any existing calls to the function, since before the change both bits 0 and 1 were checked and needed to be set. Signed-off-by: Daniil Lunev <dlunev@chromium.org> --- Changes in v2: * Make __INTEL_DSM_SUPPORTED a function * use `1u` instead of `1` in shift operator drivers/ufs/host/ufshcd-pci.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)