| Message ID | 1618535202-11397-1-git-send-email-tiantao6@hisilicon.com |
|---|---|
| State | Accepted |
| Commit | 5d17c414e1029d245d535d10c6ead04dc2f65e15 |
| Headers | show |
| Series | [v2] crypto: ccp - Make ccp_dev_suspend and ccp_dev_resume void functions | expand |
On 4/15/21 8:06 PM, Tian Tao wrote: > Since ccp_dev_suspend() and ccp_dev_resume() only return 0 which causes > ret to equal 0 in sp_suspend and sp_resume, making the if condition > impossible to use. it might be a more appropriate fix to have these be > void functions and eliminate the if condition in sp_suspend() and > sp_resume(). > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Tom Lendacky <thomas.lendacky@amd.com> Acked-by: Tom Lendacky <thomas.lendacky@amd.com> And please copy the maintainers associated with the files being patched should you submit more patches in the future. Thanks, Tom > --- > v2: handle the case that didn't define CONFIG_CRYPTO_DEV_SP_CCP. > --- > drivers/crypto/ccp/ccp-dev.c | 12 ++++-------- > drivers/crypto/ccp/sp-dev.c | 12 ++---------- > drivers/crypto/ccp/sp-dev.h | 15 ++++----------- > 3 files changed, 10 insertions(+), 29 deletions(-) > > diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c > index 0971ee6..6777582 100644 > --- a/drivers/crypto/ccp/ccp-dev.c > +++ b/drivers/crypto/ccp/ccp-dev.c > @@ -548,7 +548,7 @@ bool ccp_queues_suspended(struct ccp_device *ccp) > return ccp->cmd_q_count == suspended; > } > > -int ccp_dev_suspend(struct sp_device *sp) > +void ccp_dev_suspend(struct sp_device *sp) > { > struct ccp_device *ccp = sp->ccp_data; > unsigned long flags; > @@ -556,7 +556,7 @@ int ccp_dev_suspend(struct sp_device *sp) > > /* If there's no device there's nothing to do */ > if (!ccp) > - return 0; > + return; > > spin_lock_irqsave(&ccp->cmd_lock, flags); > > @@ -572,11 +572,9 @@ int ccp_dev_suspend(struct sp_device *sp) > while (!ccp_queues_suspended(ccp)) > wait_event_interruptible(ccp->suspend_queue, > ccp_queues_suspended(ccp)); > - > - return 0; > } > > -int ccp_dev_resume(struct sp_device *sp) > +void ccp_dev_resume(struct sp_device *sp) > { > struct ccp_device *ccp = sp->ccp_data; > unsigned long flags; > @@ -584,7 +582,7 @@ int ccp_dev_resume(struct sp_device *sp) > > /* If there's no device there's nothing to do */ > if (!ccp) > - return 0; > + return; > > spin_lock_irqsave(&ccp->cmd_lock, flags); > > @@ -597,8 +595,6 @@ int ccp_dev_resume(struct sp_device *sp) > } > > spin_unlock_irqrestore(&ccp->cmd_lock, flags); > - > - return 0; > } > > int ccp_dev_init(struct sp_device *sp) > diff --git a/drivers/crypto/ccp/sp-dev.c b/drivers/crypto/ccp/sp-dev.c > index 6284a15..7eb3e46 100644 > --- a/drivers/crypto/ccp/sp-dev.c > +++ b/drivers/crypto/ccp/sp-dev.c > @@ -213,12 +213,8 @@ void sp_destroy(struct sp_device *sp) > > int sp_suspend(struct sp_device *sp) > { > - int ret; > - > if (sp->dev_vdata->ccp_vdata) { > - ret = ccp_dev_suspend(sp); > - if (ret) > - return ret; > + ccp_dev_suspend(sp); > } > > return 0; > @@ -226,12 +222,8 @@ int sp_suspend(struct sp_device *sp) > > int sp_resume(struct sp_device *sp) > { > - int ret; > - > if (sp->dev_vdata->ccp_vdata) { > - ret = ccp_dev_resume(sp); > - if (ret) > - return ret; > + ccp_dev_resume(sp); > } > > return 0; > diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h > index 0218d06..20377e6 100644 > --- a/drivers/crypto/ccp/sp-dev.h > +++ b/drivers/crypto/ccp/sp-dev.h > @@ -134,8 +134,8 @@ struct sp_device *sp_get_psp_master_device(void); > int ccp_dev_init(struct sp_device *sp); > void ccp_dev_destroy(struct sp_device *sp); > > -int ccp_dev_suspend(struct sp_device *sp); > -int ccp_dev_resume(struct sp_device *sp); > +void ccp_dev_suspend(struct sp_device *sp); > +void ccp_dev_resume(struct sp_device *sp); > > #else /* !CONFIG_CRYPTO_DEV_SP_CCP */ > > @@ -144,15 +144,8 @@ static inline int ccp_dev_init(struct sp_device *sp) > return 0; > } > static inline void ccp_dev_destroy(struct sp_device *sp) { } > - > -static inline int ccp_dev_suspend(struct sp_device *sp) > -{ > - return 0; > -} > -static inline int ccp_dev_resume(struct sp_device *sp) > -{ > - return 0; > -} > +static inline void ccp_dev_suspend(struct sp_device *sp) { } > +static inline void ccp_dev_resume(struct sp_device *sp) { } > #endif /* CONFIG_CRYPTO_DEV_SP_CCP */ > > #ifdef CONFIG_CRYPTO_DEV_SP_PSP >
On Fri, Apr 16, 2021 at 09:06:42AM +0800, Tian Tao wrote: > Since ccp_dev_suspend() and ccp_dev_resume() only return 0 which causes > ret to equal 0 in sp_suspend and sp_resume, making the if condition > impossible to use. it might be a more appropriate fix to have these be > void functions and eliminate the if condition in sp_suspend() and > sp_resume(). > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Tom Lendacky <thomas.lendacky@amd.com> Acked-by: John Allen <john.allen@amd.com>
On Fri, Apr 16, 2021 at 09:06:42AM +0800, Tian Tao wrote: > Since ccp_dev_suspend() and ccp_dev_resume() only return 0 which causes > ret to equal 0 in sp_suspend and sp_resume, making the if condition > impossible to use. it might be a more appropriate fix to have these be > void functions and eliminate the if condition in sp_suspend() and > sp_resume(). > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > --- > v2: handle the case that didn't define CONFIG_CRYPTO_DEV_SP_CCP. > --- > drivers/crypto/ccp/ccp-dev.c | 12 ++++-------- > drivers/crypto/ccp/sp-dev.c | 12 ++---------- > drivers/crypto/ccp/sp-dev.h | 15 ++++----------- > 3 files changed, 10 insertions(+), 29 deletions(-) Patch applied. Thanks. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c index 0971ee6..6777582 100644 --- a/drivers/crypto/ccp/ccp-dev.c +++ b/drivers/crypto/ccp/ccp-dev.c @@ -548,7 +548,7 @@ bool ccp_queues_suspended(struct ccp_device *ccp) return ccp->cmd_q_count == suspended; } -int ccp_dev_suspend(struct sp_device *sp) +void ccp_dev_suspend(struct sp_device *sp) { struct ccp_device *ccp = sp->ccp_data; unsigned long flags; @@ -556,7 +556,7 @@ int ccp_dev_suspend(struct sp_device *sp) /* If there's no device there's nothing to do */ if (!ccp) - return 0; + return; spin_lock_irqsave(&ccp->cmd_lock, flags); @@ -572,11 +572,9 @@ int ccp_dev_suspend(struct sp_device *sp) while (!ccp_queues_suspended(ccp)) wait_event_interruptible(ccp->suspend_queue, ccp_queues_suspended(ccp)); - - return 0; } -int ccp_dev_resume(struct sp_device *sp) +void ccp_dev_resume(struct sp_device *sp) { struct ccp_device *ccp = sp->ccp_data; unsigned long flags; @@ -584,7 +582,7 @@ int ccp_dev_resume(struct sp_device *sp) /* If there's no device there's nothing to do */ if (!ccp) - return 0; + return; spin_lock_irqsave(&ccp->cmd_lock, flags); @@ -597,8 +595,6 @@ int ccp_dev_resume(struct sp_device *sp) } spin_unlock_irqrestore(&ccp->cmd_lock, flags); - - return 0; } int ccp_dev_init(struct sp_device *sp) diff --git a/drivers/crypto/ccp/sp-dev.c b/drivers/crypto/ccp/sp-dev.c index 6284a15..7eb3e46 100644 --- a/drivers/crypto/ccp/sp-dev.c +++ b/drivers/crypto/ccp/sp-dev.c @@ -213,12 +213,8 @@ void sp_destroy(struct sp_device *sp) int sp_suspend(struct sp_device *sp) { - int ret; - if (sp->dev_vdata->ccp_vdata) { - ret = ccp_dev_suspend(sp); - if (ret) - return ret; + ccp_dev_suspend(sp); } return 0; @@ -226,12 +222,8 @@ int sp_suspend(struct sp_device *sp) int sp_resume(struct sp_device *sp) { - int ret; - if (sp->dev_vdata->ccp_vdata) { - ret = ccp_dev_resume(sp); - if (ret) - return ret; + ccp_dev_resume(sp); } return 0; diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h index 0218d06..20377e6 100644 --- a/drivers/crypto/ccp/sp-dev.h +++ b/drivers/crypto/ccp/sp-dev.h @@ -134,8 +134,8 @@ struct sp_device *sp_get_psp_master_device(void); int ccp_dev_init(struct sp_device *sp); void ccp_dev_destroy(struct sp_device *sp); -int ccp_dev_suspend(struct sp_device *sp); -int ccp_dev_resume(struct sp_device *sp); +void ccp_dev_suspend(struct sp_device *sp); +void ccp_dev_resume(struct sp_device *sp); #else /* !CONFIG_CRYPTO_DEV_SP_CCP */ @@ -144,15 +144,8 @@ static inline int ccp_dev_init(struct sp_device *sp) return 0; } static inline void ccp_dev_destroy(struct sp_device *sp) { } - -static inline int ccp_dev_suspend(struct sp_device *sp) -{ - return 0; -} -static inline int ccp_dev_resume(struct sp_device *sp) -{ - return 0; -} +static inline void ccp_dev_suspend(struct sp_device *sp) { } +static inline void ccp_dev_resume(struct sp_device *sp) { } #endif /* CONFIG_CRYPTO_DEV_SP_CCP */ #ifdef CONFIG_CRYPTO_DEV_SP_PSP
Since ccp_dev_suspend() and ccp_dev_resume() only return 0 which causes ret to equal 0 in sp_suspend and sp_resume, making the if condition impossible to use. it might be a more appropriate fix to have these be void functions and eliminate the if condition in sp_suspend() and sp_resume(). Signed-off-by: Tian Tao <tiantao6@hisilicon.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: Tom Lendacky <thomas.lendacky@amd.com> --- v2: handle the case that didn't define CONFIG_CRYPTO_DEV_SP_CCP. --- drivers/crypto/ccp/ccp-dev.c | 12 ++++-------- drivers/crypto/ccp/sp-dev.c | 12 ++---------- drivers/crypto/ccp/sp-dev.h | 15 ++++----------- 3 files changed, 10 insertions(+), 29 deletions(-)