diff mbox series

[v2,2/2] hwrng: msm - Add support for prng v2

Message ID 20180619095430.26358-3-vkoul@kernel.org
State New
Headers show
Series None | expand

Commit Message

Vinod Koul June 19, 2018, 9:54 a.m. UTC
Qcom 8996 and later chips support prng v2 which requires to
implement only .read callback for hwrng.

This version of chip has multiple Execution Environments (EE) and
secure world is typically responsible for configuring the prng.

Add driver data for qcom,prng as 0 and qcom,prng-v2 as 1 and use
that to skip initialization and cleanup routines.

Signed-off-by: Vinod Koul <vkoul@kernel.org>

---
 drivers/char/hw_random/msm-rng.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

-- 
2.14.4

Comments

Stanimir Varbanov June 19, 2018, 12:11 p.m. UTC | #1
Hi Vinod,

On 06/19/2018 12:54 PM, Vinod Koul wrote:
> Qcom 8996 and later chips support prng v2 which requires to

> implement only .read callback for hwrng.

> 

> This version of chip has multiple Execution Environments (EE) and

> secure world is typically responsible for configuring the prng.

> 

> Add driver data for qcom,prng as 0 and qcom,prng-v2 as 1 and use

> that to skip initialization and cleanup routines.

> 

> Signed-off-by: Vinod Koul <vkoul@kernel.org>

> ---

>  drivers/char/hw_random/msm-rng.c | 14 ++++++++++----

>  1 file changed, 10 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c

> index 841fee845ec9..4676520e1f16 100644

> --- a/drivers/char/hw_random/msm-rng.c

> +++ b/drivers/char/hw_random/msm-rng.c

> @@ -17,6 +17,7 @@

>  #include <linux/io.h>

>  #include <linux/module.h>

>  #include <linux/of.h>

> +#include <linux/of_device.h>

>  #include <linux/platform_device.h>

>  

>  /* Device specific register offsets */

> @@ -131,6 +132,7 @@ static int msm_rng_probe(struct platform_device *pdev)

>  {

>  	struct resource *res;

>  	struct msm_rng *rng;

> +	unsigned int skip_init;

>  	int ret;

>  

>  	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);

> @@ -149,9 +151,12 @@ static int msm_rng_probe(struct platform_device *pdev)

>  		return PTR_ERR(rng->clk);

>  

>  	rng->hwrng.name = KBUILD_MODNAME,

> -	rng->hwrng.init = msm_rng_init,

> -	rng->hwrng.cleanup = msm_rng_cleanup,

> -	rng->hwrng.read = msm_rng_read,

> +	rng->hwrng.read = msm_rng_read;

> +	skip_init = (unsigned long)of_device_get_match_data(&pdev->dev);


skip_init is unsigned int, despite I think you don't need to cast it.

> +	if (!skip_init) {

> +		rng->hwrng.init = msm_rng_init;

> +		rng->hwrng.cleanup = msm_rng_cleanup;

> +	}

>  

>  	ret = devm_hwrng_register(&pdev->dev, &rng->hwrng);

>  	if (ret) {

> @@ -163,7 +168,8 @@ static int msm_rng_probe(struct platform_device *pdev)

>  }

>  

>  static const struct of_device_id msm_rng_of_match[] = {

> -	{ .compatible = "qcom,prng", },

> +	{ .compatible = "qcom,prng", .data = (void *)0},

> +	{ .compatible = "qcom,prng-v2", .data = (void *)1},


No need to cast. IMO it'd be better to create defines for these numbers:

#define PRNG_V1	0
#define PRNG_V2	1


-- 
regards,
Stan
Vinod Koul June 20, 2018, 1:37 p.m. UTC | #2
On 19-06-18, 22:58, Stephen Boyd wrote:
> Quoting Vinod Koul (2018-06-19 02:54:30)

> > Qcom 8996 and later chips support prng v2 which requires to

> > implement only .read callback for hwrng.

> > 

> > This version of chip has multiple Execution Environments (EE) and

> > secure world is typically responsible for configuring the prng.

> 

> Sometimes secure world is not configuring the rng though. I prefer we

> have a DT flag for this to indicate if secure world has configured it or

> not and then skip the init logic when the bool property is present in

> DT. Then the DT property can be set on firmwares that are making things

> blow up when we try to read the 'configured' register. I'd also file a

> bug to qcom to tell them to unlock that config register for reads so

> that things can work simpler, but who knows how that will work out.


I dont feel that would be required. See below..

> It really sounds like the hardware isn't actually different, just the

> firmware has decided to be more strict about making reads fail now.


So in this case base hw block seems similar but consists of multiple
Execution Environment (EEs) and all of these contain only data
registers. Only secure environment has configuration. Each one has its
own register space.

In a case where we dont have secure world, we can point to secure
environment with v1 ops, so driver shall configure and run.

Thanks
-- 
~Vinod
diff mbox series

Patch

diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c
index 841fee845ec9..4676520e1f16 100644
--- a/drivers/char/hw_random/msm-rng.c
+++ b/drivers/char/hw_random/msm-rng.c
@@ -17,6 +17,7 @@ 
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 
 /* Device specific register offsets */
@@ -131,6 +132,7 @@  static int msm_rng_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct msm_rng *rng;
+	unsigned int skip_init;
 	int ret;
 
 	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
@@ -149,9 +151,12 @@  static int msm_rng_probe(struct platform_device *pdev)
 		return PTR_ERR(rng->clk);
 
 	rng->hwrng.name = KBUILD_MODNAME,
-	rng->hwrng.init = msm_rng_init,
-	rng->hwrng.cleanup = msm_rng_cleanup,
-	rng->hwrng.read = msm_rng_read,
+	rng->hwrng.read = msm_rng_read;
+	skip_init = (unsigned long)of_device_get_match_data(&pdev->dev);
+	if (!skip_init) {
+		rng->hwrng.init = msm_rng_init;
+		rng->hwrng.cleanup = msm_rng_cleanup;
+	}
 
 	ret = devm_hwrng_register(&pdev->dev, &rng->hwrng);
 	if (ret) {
@@ -163,7 +168,8 @@  static int msm_rng_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id msm_rng_of_match[] = {
-	{ .compatible = "qcom,prng", },
+	{ .compatible = "qcom,prng", .data = (void *)0},
+	{ .compatible = "qcom,prng-v2", .data = (void *)1},
 	{}
 };
 MODULE_DEVICE_TABLE(of, msm_rng_of_match);