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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <32eeb0ce-49fd-34ed-d5c8-0406ea174525@linaro.org>
Date:   Thu, 11 Jun 2020 14:51:00 +0300
From:   Stanimir Varbanov <stanimir.varbanov@...aro.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Stanimir Varbanov <stanimir.varbanov@...aro.org>
Cc:     linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-btrfs@...r.kernel.org, linux-acpi@...r.kernel.org,
        netdev@...r.kernel.org, Joe Perches <joe@...ches.com>,
        Jason Baron <jbaron@...mai.com>
Subject: Re: [PATCH v3 5/7] venus: Add debugfs interface to set firmware log
 level

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