Re: [PATCH] Revert "venus: pm_helpers: Fix error check in vcodec_domains_get()"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

On 7/3/2023 9:00 PM, Dan Carpenter wrote:
> This reverts commit 0f6e8d8c94a82e85e1b9b62a7671990740dc6f70.
> 
> The reverted commit was based on static analysis and a misunderstanding
> of how PTR_ERR() and NULLs are supposed to work.  When a function
> returns both pointer errors and NULL then normally the NULL means
> "continue operating without a feature because it was deliberately
> turned off".  The NULL should not be treated as a failure.  If a driver
> cannot work when that feature is disabled then the KConfig should
> enforce that the function cannot return NULL.  We should not need to
> test for it.
> 
> In this code, the patch breaks the venus driver if CONFIG_PM is
> disabled.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> This patch is also based on static analysis and review so probably best
> to be cautious.  My guess is that very few people disable CONFIG_PM
> these days so that's why the bug wasn't caught.
> 
>  drivers/media/platform/qcom/venus/pm_helpers.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index 48c9084bb4db..c93d2906e4c7 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -869,8 +869,8 @@ static int vcodec_domains_get(struct venus_core *core)
>  	for (i = 0; i < res->vcodec_pmdomains_num; i++) {
>  		pd = dev_pm_domain_attach_by_name(dev,
>  						  res->vcodec_pmdomains[i]);
> -		if (IS_ERR_OR_NULL(pd))
> -			return PTR_ERR(pd) ? : -ENODATA;
> +		if (IS_ERR(pd))
> +			return PTR_ERR(pd);
Trying to understand if pm domains for Venus (pd) is returned NULL here, how
would the driver be able to enable and disable those power domains at [1]. And
without that, video usecase would be functional ?

[1]
https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/qcom/venus/pm_helpers.c#L1043

>  		core->pmdomains[i] = pd;
>  	}
> 



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux