On 14/08/2023 20:54, Dikshita Agarwal wrote:
>>> +
>>> + core = devm_kzalloc(&pdev->dev, sizeof(struct msm_vidc_core), GFP_KERNEL);
>>> + if (!core) {
>>> + d_vpr_e("%s: failed to alloc memory for core\n", __func__);
>>
>> Ooops, this for sure did not pass any checks by tools. Sorry, please run
>> basic checks like coccinelle, smatch, sparse, W=1 builds.
>>
> we ran check patch and smatch on this code but no errors were reported.
coccinelle is missing
> please elaborate what issue you see with above code?
>>> + return -ENOMEM;
We did quite a lot of cleanups long, long time ago removing all unneeded
error messages from memory allocations failures.
Maybe Your use of custom printks() confuses coccinelle, which is a proof
that your code is here an anti-pattern.
...
>>
> Sure, will remove these custom wrapper for locks and use standard
> mutex_lock/unlock APIs directly.
>>> + allow = msm_vidc_allow_pm_suspend(core);
>>> +
>>> + if (allow == MSM_VIDC_IGNORE) {
>>> + d_vpr_h("%s: pm already suspended\n", __func__);
>>
>> So you have bug in PM runtime code? Runtime PM does not suspend devices
>> twice.
>> core is power collapsed in case of idle state when there is no processing
> happening by hardware, this can also change the core state to power_disable.
> this check is ensuring if the core is already in power disable state due to
> that, then no need to suspend it again.
No, you just re-implemented runtime PM.
Best regards,
Krzysztof
