2024-03-27 18:10:38

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v3 05/19] media: venus: pm_helpers: Kill dead code

A situation like:

if (!foo)
goto bar;

for (i = 0; i < foo; i++)
...1...

bar:
...2...

is totally identical to:

for (i = 0; i < 0; i++) // === if (0)
...1...

..2...

Get rid of such boilerplate.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/pm_helpers.c | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index ef4b0f0da36f..730c4b593cec 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -878,14 +878,10 @@ static int vcodec_domains_get(struct venus_core *core)
.pd_flags = PD_FLAG_NO_DEV_LINK,
};

- if (!res->vcodec_pmdomains_num)
- goto skip_pmdomains;
-
ret = dev_pm_domain_attach_list(dev, &vcodec_data, &core->pmdomains);
if (ret < 0)
return ret;

-skip_pmdomains:
if (!core->res->opp_pmdomain)
return 0;

@@ -928,9 +924,6 @@ static int core_resets_reset(struct venus_core *core)
unsigned int i;
int ret;

- if (!res->resets_num)
- return 0;
-
for (i = 0; i < res->resets_num; i++) {
ret = reset_control_assert(core->resets[i]);
if (ret)
@@ -953,9 +946,6 @@ static int core_resets_get(struct venus_core *core)
unsigned int i;
int ret;

- if (!res->resets_num)
- return 0;
-
for (i = 0; i < res->resets_num; i++) {
core->resets[i] =
devm_reset_control_get_exclusive(dev, res->resets[i]);

--
2.44.0



2024-04-05 07:50:05

by Dikshita Agarwal

[permalink] [raw]
Subject: Re: [PATCH v3 05/19] media: venus: pm_helpers: Kill dead code



On 3/27/2024 11:38 PM, Konrad Dybcio wrote:
> A situation like:
>
> if (!foo)
> goto bar;
>
> for (i = 0; i < foo; i++)
> ...1...
>
> bar:
> ...2...
>
> is totally identical to:
>
> for (i = 0; i < 0; i++) // === if (0)
> ...1...
>
> ...2...
>
> Get rid of such boilerplate.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/media/platform/qcom/venus/pm_helpers.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index ef4b0f0da36f..730c4b593cec 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -878,14 +878,10 @@ static int vcodec_domains_get(struct venus_core *core)
> .pd_flags = PD_FLAG_NO_DEV_LINK,
> };
>
> - if (!res->vcodec_pmdomains_num)
> - goto skip_pmdomains;
> -
this condition should still be there to skip calling
dev_pm_domain_attach_list if vcodec_pmdomains_num is 0.
> ret = dev_pm_domain_attach_list(dev, &vcodec_data, &core->pmdomains);
> if (ret < 0)
> return ret;
>
> -skip_pmdomains:
> if (!core->res->opp_pmdomain)
> return 0;
>
> @@ -928,9 +924,6 @@ static int core_resets_reset(struct venus_core *core)
> unsigned int i;
> int ret;
>
> - if (!res->resets_num)
> - return 0;
> -
ACK
> for (i = 0; i < res->resets_num; i++) {
> ret = reset_control_assert(core->resets[i]);
> if (ret)
> @@ -953,9 +946,6 @@ static int core_resets_get(struct venus_core *core)
> unsigned int i;
> int ret;
>
> - if (!res->resets_num)
> - return 0;
> -
ACK
> for (i = 0; i < res->resets_num; i++) {
> core->resets[i] =
> devm_reset_control_get_exclusive(dev, res->resets[i]);
>

Thanks,
Dikshita

2024-04-09 18:37:15

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v3 05/19] media: venus: pm_helpers: Kill dead code



On 4/5/24 09:49, Dikshita Agarwal wrote:
>
>
> On 3/27/2024 11:38 PM, Konrad Dybcio wrote:
>> A situation like:
>>
>> if (!foo)
>> goto bar;
>>
>> for (i = 0; i < foo; i++)
>> ...1...
>>
>> bar:
>> ...2...
>>
>> is totally identical to:
>>
>> for (i = 0; i < 0; i++) // === if (0)
>> ...1...
>>
>> ...2...
>>
>> Get rid of such boilerplate.
>>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
>> drivers/media/platform/qcom/venus/pm_helpers.c | 10 ----------
>> 1 file changed, 10 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
>> index ef4b0f0da36f..730c4b593cec 100644
>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>> @@ -878,14 +878,10 @@ static int vcodec_domains_get(struct venus_core *core)
>> .pd_flags = PD_FLAG_NO_DEV_LINK,
>> };
>>
>> - if (!res->vcodec_pmdomains_num)
>> - goto skip_pmdomains;
>> -
> this condition should still be there to skip calling
> dev_pm_domain_attach_list if vcodec_pmdomains_num is 0.

That should be totally fine, within that function there's this bit

if (num_pds <= 0)
return 0;

which bails out gracefully

Konrad

2024-04-23 08:03:03

by Dikshita Agarwal

[permalink] [raw]
Subject: Re: [PATCH v3 05/19] media: venus: pm_helpers: Kill dead code



On 4/9/2024 11:54 PM, Konrad Dybcio wrote:
>
>
> On 4/5/24 09:49, Dikshita Agarwal wrote:
>>
>>
>> On 3/27/2024 11:38 PM, Konrad Dybcio wrote:
>>> A situation like:
>>>
>>> if (!foo)
>>>     goto bar;
>>>
>>> for (i = 0; i < foo; i++)
>>>     ...1...
>>>
>>> bar:
>>>     ...2...
>>>
>>> is totally identical to:
>>>
>>> for (i = 0; i < 0; i++) // === if (0)
>>>     ...1...
>>>
>>> ...2...
>>>
>>> Get rid of such boilerplate.
>>>
>>> Signed-off-by: Konrad Dybcio <[email protected]>
>>> ---
>>>   drivers/media/platform/qcom/venus/pm_helpers.c | 10 ----------
>>>   1 file changed, 10 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c
>>> b/drivers/media/platform/qcom/venus/pm_helpers.c
>>> index ef4b0f0da36f..730c4b593cec 100644
>>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>>> @@ -878,14 +878,10 @@ static int vcodec_domains_get(struct venus_core
>>> *core)
>>>           .pd_flags = PD_FLAG_NO_DEV_LINK,
>>>       };
>>>   -    if (!res->vcodec_pmdomains_num)
>>> -        goto skip_pmdomains;
>>> -
>> this condition should still be there to skip calling
>> dev_pm_domain_attach_list if vcodec_pmdomains_num is 0.
>
> That should be totally fine, within that function there's this bit
>
> if (num_pds <= 0)
>     return 0;
>
> which bails out gracefully
>
AFAIU, this condition won't be true eg: for 8916 and 8996.
because in the else condition[1], num_pds will be non-zero, as there is one
entry for power-domains in dtsi file for 8916, 8996 as well.

[1]https://elixir.bootlin.com/linux/v6.9-rc5/source/drivers/base/power/common.c#L213
Am I missing something here?

> Konrad