linux-kernel - Re: [PATCH 5/7] hwrng: rockchip: add support for rk3588's standalone TRNG

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: <c32d8455-1bf6-44de-acf8-34fe00ae2868@kernel.org>
Date: Fri, 31 Jan 2025 09:34:02 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>,
 Philipp Zabel <p.zabel@...gutronix.de>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 Olivia Mackall <olivia@...enic.com>, Herbert Xu
 <herbert@...dor.apana.org.au>, Daniel Golle <daniel@...rotopia.org>,
 Aurelien Jarno <aurelien@...el32.net>
Cc: Sebastian Reichel <sebastian.reichel@...labora.com>,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-crypto@...r.kernel.org, Lin Jinhan <troy.lin@...k-chips.com>
Subject: Re: [PATCH 5/7] hwrng: rockchip: add support for rk3588's standalone
 TRNG

On 30/01/2025 17:31, Nicolas Frattaroli wrote:
> +MODULE_DEVICE_TABLE(of, rk_rng_dt_match);
> +
>  static int rk_rng_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	const struct of_device_id *match;
>  	struct reset_control *rst;
>  	struct rk_rng *rk_rng;
>  	int ret;
> @@ -139,6 +333,8 @@ static int rk_rng_probe(struct platform_device *pdev)
>  	if (!rk_rng)
>  		return -ENOMEM;
>  
> +	match = of_match_node(rk_rng_dt_match, dev->of_node);
> +	rk_rng->soc_data = (struct rk_rng_soc_data *)match->data;

Don't open code getting match data.

>  	rk_rng->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(rk_rng->base))
>  		return PTR_ERR(rk_rng->base);
> @@ -148,24 +344,30 @@ static int rk_rng_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, rk_rng->clk_num,
>  				     "Failed to get clks property\n");
>  
> -	rst = devm_reset_control_array_get_exclusive(dev);
> -	if (IS_ERR(rst))
> -		return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset property\n");
> +	if (rk_rng->soc_data->reset_optional)
> +		rst = devm_reset_control_array_get_optional_exclusive(dev);
> +	else
> +		rst = devm_reset_control_array_get_exclusive(dev);
> +
> +	if (rst) {
> +		if (IS_ERR(rst))
> +			return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset property\n");
>  
> -	reset_control_assert(rst);
> -	udelay(2);
> -	reset_control_deassert(rst);
> +		reset_control_assert(rst);
> +		udelay(2);
> +		reset_control_deassert(rst);
> +	}
>  
>  	platform_set_drvdata(pdev, rk_rng);
>  
>  	rk_rng->rng.name = dev_driver_string(dev);
>  	if (!IS_ENABLED(CONFIG_PM)) {
> -		rk_rng->rng.init = rk_rng_init;
> -		rk_rng->rng.cleanup = rk_rng_cleanup;
> +		rk_rng->rng.init = rk_rng->soc_data->rk_rng_init;
> +		rk_rng->rng.cleanup = rk_rng->soc_data->rk_rng_cleanup;
>  	}
> -	rk_rng->rng.read = rk_rng_read;
> +	rk_rng->rng.read = rk_rng->soc_data->rk_rng_read;
>  	rk_rng->dev = dev;
> -	rk_rng->rng.quality = 900;
> +	rk_rng->rng.quality = rk_rng->soc_data->quality;
>  
>  	pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
>  	pm_runtime_use_autosuspend(dev);
> @@ -184,7 +386,7 @@ static int __maybe_unused rk_rng_runtime_suspend(struct device *dev)
>  {
>  	struct rk_rng *rk_rng = dev_get_drvdata(dev);
>  
> -	rk_rng_cleanup(&rk_rng->rng);
> +	rk_rng->soc_data->rk_rng_cleanup(&rk_rng->rng);
>  
>  	return 0;
>  }
> @@ -193,7 +395,7 @@ static int __maybe_unused rk_rng_runtime_resume(struct device *dev)
>  {
>  	struct rk_rng *rk_rng = dev_get_drvdata(dev);
>  
> -	return rk_rng_init(&rk_rng->rng);
> +	return rk_rng->soc_data->rk_rng_init(&rk_rng->rng);
>  }
>  
>  static const struct dev_pm_ops rk_rng_pm_ops = {
> @@ -203,13 +405,6 @@ static const struct dev_pm_ops rk_rng_pm_ops = {
>  				pm_runtime_force_resume)
>  };
>  
> -static const struct of_device_id rk_rng_dt_match[] = {
> -	{ .compatible = "rockchip,rk3568-rng", },
> -	{ /* sentinel */ },
> -};


No, don't move it. Not necessary and not expected to be in other place.

Best regards,
Krzysztof