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

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Olivia Mackall <olivia@selenic.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Daniel Golle <daniel@makrotopia.org>,
	Aurelien Jarno <aurelien@aurel32.net>
Cc: Sebastian Reichel <sebastian.reichel@collabora.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-crypto@vger.kernel.org,
	Lin Jinhan <troy.lin@rock-chips.com>
Subject: Re: [PATCH 5/7] hwrng: rockchip: add support for rk3588's standalone TRNG
Date: Fri, 31 Jan 2025 09:34:02 +0100	[thread overview]
Message-ID: <c32d8455-1bf6-44de-acf8-34fe00ae2868@kernel.org> (raw)
In-Reply-To: <20250130-rk3588-trng-submission-v1-5-97ff76568e49@collabora.com>

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip