Re: [PATCH v4 2/3] hwrng: add Rockchip SoC hwrng driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 23/06/2024 05:33, Daniel Golle wrote:
> +
> +	rk_rng->rng.name = dev_driver_string(dev);
> +#ifndef CONFIG_PM
> +	rk_rng->rng.init = rk_rng_init;
> +	rk_rng->rng.cleanup = rk_rng_cleanup;
> +#endif
> +	rk_rng->rng.read = rk_rng_read;
> +	rk_rng->rng.priv = (unsigned long) dev;
> +	rk_rng->rng.quality = 900;

I doubt in this value. Usually SoC vendors do not provide datasheet with
any reliable and verifiable (so one which could be proven by 3rd party)
information. Can you provide a source? (and vendor downstream tree does
not really count)

> +
> +	pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
> +	pm_runtime_use_autosuspend(dev);
> +	devm_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_dbg(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, it is not useful at all. Srsly, we had already long enough talk,
which wasted time of three people. Why do you insist on wasting more?

There is no single benefit of such debug statement. sysfs already
provides you this information. Simple entry/exit  is provided by
tracing. You duplicate existing interfaces without any benefit, because
this prints nothing more.

Best regards,
Krzysztof