[v4,6/6] crypto: qcom: Add ACPI support
diff mbox

Message ID 20180704114427.29953-7-vkoul@kernel.org
State Superseded
Delegated to: Herbert Xu
Headers show

Commit Message

Vinod Koul July 4, 2018, 11:44 a.m. UTC
From: Timur Tabi <timur@codeaurora.org>

Add support for probing on ACPI systems, with ACPI HID QCOM8160.

On ACPI systems, clocks are always enabled, the PRNG should
already be enabled, and the register region is read-only.
The driver only verifies that the hardware is already
enabled never tries to disable or configure it.

Signed-off-by: Timur Tabi <timur@codeaurora.org>
[port to crypto API]
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/crypto/qcom-rng.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

Comments

Timur Tabi July 4, 2018, 2:46 p.m. UTC | #1
On 7/4/18 6:44 AM, Vinod Koul wrote:
> +	if (has_acpi_companion(&pdev->dev)) {
> +		rng->skip_init = 1;
> +	} else {
> +		rng->clk = devm_clk_get(&pdev->dev, "core");
> +		if (IS_ERR(rng->clk))
> +			return PTR_ERR(rng->clk);
> +
> +		rng->skip_init =
> +			(unsigned long)of_device_get_match_data(&pdev->dev);
> +	}

There is a device_get_match_data() function, if you want to be 
consistent between ACPI and DT.
Vinod Koul July 5, 2018, 6:07 a.m. UTC | #2
On 04-07-18, 09:46, Timur Tabi wrote:
> On 7/4/18 6:44 AM, Vinod Koul wrote:
> > +	if (has_acpi_companion(&pdev->dev)) {
> > +		rng->skip_init = 1;
> > +	} else {
> > +		rng->clk = devm_clk_get(&pdev->dev, "core");
> > +		if (IS_ERR(rng->clk))
> > +			return PTR_ERR(rng->clk);
> > +
> > +		rng->skip_init =
> > +			(unsigned long)of_device_get_match_data(&pdev->dev);
> > +	}
> 
> There is a device_get_match_data() function, if you want to be consistent
> between ACPI and DT.

Yes we can add driver data in ACPI ID as well so code can be:

        if (!has_acpi_companion(&pdev->dev)) {
                rng->clk = devm_clk_get(&pdev->dev, "core");
                if (IS_ERR(rng->clk))
                        return PTR_ERR(rng->clk);
        }
        rng->skip_init = device_get_match_data(&pdev->dev);

Looks much neater.

I will wait for feedback on other patches before updating this.

Meanwhile any word from testing on ACPI systems?

Thanks
Timur Tabi July 5, 2018, 12:23 p.m. UTC | #3
On 7/5/18 1:07 AM, Vinod wrote:
> Meanwhile any word from testing on ACPI systems?

Jeff Hugo is handling that for me.  Since it's a whole new API, he's had 
to start over.
Vinod Koul July 5, 2018, 1:24 p.m. UTC | #4
On 05-07-18, 07:23, Timur Tabi wrote:
> On 7/5/18 1:07 AM, Vinod wrote:
> > Meanwhile any word from testing on ACPI systems?
> 
> Jeff Hugo is handling that for me.  Since it's a whole new API, he's had to
> start over.

Okay great, I have already been thru the hoops, so please do ping me if
you folks have questions.

Thanks
Jeffrey Hugo July 5, 2018, 7:51 p.m. UTC | #5
On 7/4/2018 5:44 AM, Vinod Koul wrote:
> From: Timur Tabi <timur@codeaurora.org>
> 
> Add support for probing on ACPI systems, with ACPI HID QCOM8160.
> 
> On ACPI systems, clocks are always enabled, the PRNG should
> already be enabled, and the register region is read-only.
> The driver only verifies that the hardware is already
> enabled never tries to disable or configure it.
> 
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
> [port to crypto API]
> Signed-off-by: Vinod Koul <vkoul@kernel.org>

Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>

> ---
>   drivers/crypto/qcom-rng.c | 30 ++++++++++++++++++++++++++----
>   1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
> index f1bd86acaf9d..385352e200db 100644
> --- a/drivers/crypto/qcom-rng.c
> +++ b/drivers/crypto/qcom-rng.c
> @@ -4,6 +4,7 @@
>   // Based on msm-rng.c and downstream driver
>   
>   #include <crypto/internal/rng.h>
> +#include <linux/acpi.h>
>   #include <linux/clk.h>
>   #include <linux/crypto.h>
>   #include <linux/module.h>
> @@ -168,11 +169,21 @@ static int qcom_rng_probe(struct platform_device *pdev)
>   	if (IS_ERR(rng->base))
>   		return PTR_ERR(rng->base);
>   
> -	rng->clk = devm_clk_get(&pdev->dev, "core");
> -	if (IS_ERR(rng->clk))
> -		return PTR_ERR(rng->clk);
>   
> -	rng->skip_init = (unsigned long)of_device_get_match_data(&pdev->dev);
> +	/*
> +	 * ACPI systems have v2 hardware. The clocks are always enabled,
> +	 * and we should skip init
> +	 */
> +	if (has_acpi_companion(&pdev->dev)) {
> +		rng->skip_init = 1;
> +	} else {
> +		rng->clk = devm_clk_get(&pdev->dev, "core");
> +		if (IS_ERR(rng->clk))
> +			return PTR_ERR(rng->clk);
> +
> +		rng->skip_init =
> +			(unsigned long)of_device_get_match_data(&pdev->dev);
> +	}
>   
>   	qcom_rng_dev = rng;
>   	ret = crypto_register_rng(&qcom_rng_alg);
> @@ -193,6 +204,16 @@ static int qcom_rng_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +#if IS_ENABLED(CONFIG_ACPI)
> +static const struct acpi_device_id qcom_rng_acpi_match[] = {
> +	{
> +		.id = "QCOM8160",
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, qcom_rng_acpi_match);
> +#endif
> +
>   static const struct of_device_id qcom_rng_of_match[] = {
>   	{ .compatible = "qcom,prng", .data = (void *)0},
>   	{ .compatible = "qcom,prng-ee", .data = (void *)1},
> @@ -206,6 +227,7 @@ static struct platform_driver qcom_rng_driver = {
>   	.driver = {
>   		.name = KBUILD_MODNAME,
>   		.of_match_table = of_match_ptr(qcom_rng_of_match),
> +		.acpi_match_table = ACPI_PTR(qcom_rng_acpi_match),
>   	}
>   };
>   module_platform_driver(qcom_rng_driver);
>
Timur Tabi July 7, 2018, 6:05 p.m. UTC | #6
On 7/5/18 1:07 AM, Vinod wrote:
> Yes we can add driver data in ACPI ID as well so code can be:
> 
>          if (!has_acpi_companion(&pdev->dev)) {
>                  rng->clk = devm_clk_get(&pdev->dev, "core");
>                  if (IS_ERR(rng->clk))
>                          return PTR_ERR(rng->clk);
>          }
>          rng->skip_init = device_get_match_data(&pdev->dev);
> 
> Looks much neater.

Yeah, I like it.

> I will wait for feedback on other patches before updating this.

Jeff said it works, so I guess v5 will be good to go.
Vinod Koul July 9, 2018, 6:01 a.m. UTC | #7
On 07-07-18, 13:05, Timur Tabi wrote:
> On 7/5/18 1:07 AM, Vinod wrote:
> > Yes we can add driver data in ACPI ID as well so code can be:
> > 
> >          if (!has_acpi_companion(&pdev->dev)) {
> >                  rng->clk = devm_clk_get(&pdev->dev, "core");
> >                  if (IS_ERR(rng->clk))
> >                          return PTR_ERR(rng->clk);
> >          }
> >          rng->skip_init = device_get_match_data(&pdev->dev);
> > 
> > Looks much neater.
> 
> Yeah, I like it.
> 
> > I will wait for feedback on other patches before updating this.
> 
> Jeff said it works, so I guess v5 will be good to go.

Yes posting that right now. It would be great if you folks can test
again on ACPI systems

Thanks
Jeffrey Hugo July 9, 2018, 2:52 p.m. UTC | #8
On 7/9/2018 12:01 AM, Vinod wrote:
> On 07-07-18, 13:05, Timur Tabi wrote:
>> On 7/5/18 1:07 AM, Vinod wrote:
>>> Yes we can add driver data in ACPI ID as well so code can be:
>>>
>>>           if (!has_acpi_companion(&pdev->dev)) {
>>>                   rng->clk = devm_clk_get(&pdev->dev, "core");
>>>                   if (IS_ERR(rng->clk))
>>>                           return PTR_ERR(rng->clk);
>>>           }
>>>           rng->skip_init = device_get_match_data(&pdev->dev);
>>>
>>> Looks much neater.
>>
>> Yeah, I like it.
>>
>>> I will wait for feedback on other patches before updating this.
>>
>> Jeff said it works, so I guess v5 will be good to go.
> 
> Yes posting that right now. It would be great if you folks can test
> again on ACPI systems
> 
> Thanks
> 
I might need a day, but I'll get to it.

Patch
diff mbox

diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index f1bd86acaf9d..385352e200db 100644
--- a/drivers/crypto/qcom-rng.c
+++ b/drivers/crypto/qcom-rng.c
@@ -4,6 +4,7 @@ 
 // Based on msm-rng.c and downstream driver
 
 #include <crypto/internal/rng.h>
+#include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/crypto.h>
 #include <linux/module.h>
@@ -168,11 +169,21 @@  static int qcom_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(rng->base))
 		return PTR_ERR(rng->base);
 
-	rng->clk = devm_clk_get(&pdev->dev, "core");
-	if (IS_ERR(rng->clk))
-		return PTR_ERR(rng->clk);
 
-	rng->skip_init = (unsigned long)of_device_get_match_data(&pdev->dev);
+	/*
+	 * ACPI systems have v2 hardware. The clocks are always enabled,
+	 * and we should skip init
+	 */
+	if (has_acpi_companion(&pdev->dev)) {
+		rng->skip_init = 1;
+	} else {
+		rng->clk = devm_clk_get(&pdev->dev, "core");
+		if (IS_ERR(rng->clk))
+			return PTR_ERR(rng->clk);
+
+		rng->skip_init =
+			(unsigned long)of_device_get_match_data(&pdev->dev);
+	}
 
 	qcom_rng_dev = rng;
 	ret = crypto_register_rng(&qcom_rng_alg);
@@ -193,6 +204,16 @@  static int qcom_rng_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_ACPI)
+static const struct acpi_device_id qcom_rng_acpi_match[] = {
+	{
+		.id = "QCOM8160",
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, qcom_rng_acpi_match);
+#endif
+
 static const struct of_device_id qcom_rng_of_match[] = {
 	{ .compatible = "qcom,prng", .data = (void *)0},
 	{ .compatible = "qcom,prng-ee", .data = (void *)1},
@@ -206,6 +227,7 @@  static struct platform_driver qcom_rng_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.of_match_table = of_match_ptr(qcom_rng_of_match),
+		.acpi_match_table = ACPI_PTR(qcom_rng_acpi_match),
 	}
 };
 module_platform_driver(qcom_rng_driver);