linux-kernel - Re: [PATCH v3 2/3] hwrng: add Rockchip SoC hwrng driver

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]

Date: Sat, 22 Jun 2024 00:16:56 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Dragan Simic <dsimic@...jaro.org>, Krzysztof Kozlowski <krzk@...nel.org>
Cc: Daniel Golle <daniel@...rotopia.org>,
 Aurelien Jarno <aurelien@...el32.net>, Olivia Mackall <olivia@...enic.com>,
 Herbert Xu <herbert@...dor.apana.org.au>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 Anand Moon <linux.amoon@...il.com>, Sascha Hauer <s.hauer@...gutronix.de>,
 Martin Kaiser <martin@...ser.cx>, Ard Biesheuvel <ardb@...nel.org>,
 linux-crypto@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] hwrng: add Rockchip SoC hwrng driver

Hello Dragan,

On 6/21/24 20:13, Dragan Simic wrote:
> On 2024-06-21 11:57, Krzysztof Kozlowski wrote:
>> On 21/06/2024 03:25, Daniel Golle wrote:
>>> From: Aurelien Jarno <aurelien@...el32.net>
> 
> [snip]
> 
>>> +    pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
>>> +    pm_runtime_use_autosuspend(dev);
>>> +    pm_runtime_enable(dev);
>>> +
>>> +    ret = devm_hwrng_register(dev, &rk_rng->rng);
>>> +    if (ret)
>>> +        return dev_err_probe(&pdev->dev, ret, "Failed to register 
>>> Rockchip hwrng\n");
>>> +
>>> +    dev_info(&pdev->dev, "Registered Rockchip hwrng\n");
>>
>> Drop, driver should be silent on success.
> 
> I respectfully disagree.  Many drivers print a single line upon
> successful probing, which I find very useful.  In this particular
> case, it's even more useful, because some people may be concerned
> about the use of hardware TRNGs, so we should actually make sure
> to announce it.

I agree to Krzysztof here. From the POV of a driver author, your own
driver is very important and while you write it, it really interests
*you* if the driver is successfully probed. However from a system
perspective these are annoying: There are easily >50 devices[1] on a
system, if all of these print a message in probe, you have little chance
to see the relevant messages. Even if every driver author thinks their
work is a special snow flake that is worth announcing, in practice users
only care about your driver if there is a problem. Additionally each
message takes time and so delays the boot process. Additionally each 
message takes place in the printk ring buffer and so edges out earlier 
messages that might be more important.

So +1 for dropping the dev_info() or at least using dev_debug() for it.

Best regards
Uwe

[1] On my laptop if have:

	$ find /sys/devices -name driver | wc -l
	87

     On a Raspberrypi it yields 66.