Re: [PATCH v3 5/7] venus: Add debugfs interface to set firmware log level

Stanimir Varbanov
2020-06-11T11:51:06Z

Hi Greg,

Thanks for the comments!

On 6/9/20 2:12 PM, Greg Kroah-Hartman wrote:
> On Tue, Jun 09, 2020 at 01:46:02PM +0300, Stanimir Varbanov wrote:
>> +int venus_dbgfs_init(struct venus_core *core)
>> +{
>> + core->root = debugfs_create_dir("venus", NULL);
>> + if (IS_ERR(core->root))
>> + return IS_ERR(core->root);
>
> You really do not care, and obviously did not test this on a system with
> CONFIG_DEBUG_FS disabled :)

Probably not :(

>
> Just make the call to debugfs, and move on, feed it into other debugfs
> calls, all is good.
>
> This function should just return 'void', no need to care about this at
> all.
>
>> + ret = venus_sys_set_debug(hdev, venus_fw_debug);
>> + if (ret)
>> + dev_warn(dev, "setting fw debug msg ON failed (%d)\n", ret);
>
> Why do you care about this "error"?

I don't care so much, that's why it is dev_warn. But if I enable debug
messages from the firmware and I don't see them this warn will give me
an idea why.

--
regards,
Stan