| Message ID | 20240717073553.1821677-1-nichen@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dmaengine: qcom: bam_dma: Handle the return value of bam_dma_resume | expand |
On 17.07.2024 9:35 AM, Chen Ni wrote: > As pm_runtime_force_resume() can return error numbers, it should be > better to check the return value and deal with the exception. > > Fixes: 0ac9c3dd0d6f ("dmaengine: qcom: bam_dma: fix runtime PM underflow") > Signed-off-by: Chen Ni <nichen@iscas.ac.cn> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad
On Wed, Jul 17, 2024 at 03:35:53PM GMT, Chen Ni wrote: > As pm_runtime_force_resume() can return error numbers, it should be > better to check the return value and deal with the exception. > > Fixes: 0ac9c3dd0d6f ("dmaengine: qcom: bam_dma: fix runtime PM underflow") > Signed-off-by: Chen Ni <nichen@iscas.ac.cn> > --- > drivers/dma/qcom/bam_dma.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c > index 5e7d332731e0..d2f5a77dfade 100644 > --- a/drivers/dma/qcom/bam_dma.c > +++ b/drivers/dma/qcom/bam_dma.c > @@ -1460,9 +1460,7 @@ static int __maybe_unused bam_dma_resume(struct device *dev) > if (ret) > return ret; > > - pm_runtime_force_resume(dev); > - > - return 0; > + return pm_runtime_force_resume(dev); Which function will unroll the earlier clk_prepare() if we return an error here? > } > > static const struct dev_pm_ops bam_dma_pm_ops = { > -- > 2.25.1 >
> As pm_runtime_force_resume() can return error numbers, it should be > better to check the return value and deal with the exception. 1. Please improve such a change description with an imperative wording. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10#n94 2. Is there a need to complete the exception handling with a clk_unprepare() call? 3. How do you think about to use a summary phrase like “Return value from a pm_runtime_force_resume() call in bam_dma_resume()”? 4. Were any special source code analysis tools involved from your research organisation? Regards, Markus
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c index 5e7d332731e0..d2f5a77dfade 100644 --- a/drivers/dma/qcom/bam_dma.c +++ b/drivers/dma/qcom/bam_dma.c @@ -1460,9 +1460,7 @@ static int __maybe_unused bam_dma_resume(struct device *dev) if (ret) return ret; - pm_runtime_force_resume(dev); - - return 0; + return pm_runtime_force_resume(dev); } static const struct dev_pm_ops bam_dma_pm_ops = {
As pm_runtime_force_resume() can return error numbers, it should be better to check the return value and deal with the exception. Fixes: 0ac9c3dd0d6f ("dmaengine: qcom: bam_dma: fix runtime PM underflow") Signed-off-by: Chen Ni <nichen@iscas.ac.cn> --- drivers/dma/qcom/bam_dma.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)