diff mbox series

[v3,6/6] crypto: qcom: Add ACPI support

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

Commit Message

Vinod Koul July 3, 2018, 6:04 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 | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

-- 
2.14.4

Comments

Vinod Koul July 4, 2018, 4:11 a.m. UTC | #1
On 03-07-18, 09:10, Timur Tabi wrote:
> On 7/3/18 1:04 AM, Vinod Koul wrote:

> > 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>

> 

> I've asked a colleague who still works at Qualcomm to test this code on

> silicon.  It looks okay, but I just want to be sure.

> 

> > +	/*

> > +	 * ACPI systems have v2 hardware. The clocks are always enabled,

> > +	 * the PRNG register space is read-only and the PRNG should

> > +	 * already be enabled.

> > +	 */

> > +	if (has_acpi_companion(&pdev->dev)) {

> > +		val = readl(rng->base + PRNG_CONFIG);

> > +		if (!(val & PRNG_CONFIG_HW_ENABLE)) {

> > +			dev_err(&pdev->dev, "device is not enabled\n");

> > +			return -ENODEV;

> > +		}

> 

> I'm having second thoughts about this PRNG_CONFIG_HW_ENABLE check.  The PRNG

> on the QDF2400 is the same as the one on the 8996, so it should have the

> same register interface.  Currently, the ACPI table points to a full PRNG

> register block, but I'm beginning to believe that it should instead point to

> a "reduced" block that doesn't have a PRNG_CONFIG register.


That was my doubt too. I will go ahead and make it skip this then...

-- 
~Vinod
Jeffrey Hugo July 5, 2018, 2:26 p.m. UTC | #2
On 7/3/2018 10:13 PM, Vinod wrote:
> On 03-07-18, 11:08, Jeffrey Hugo wrote:

>> On 7/3/2018 12:04 AM, Vinod Koul wrote:

> 

>>> +#if IS_ENABLED(CONFIG_ACPI)

>>> +static const struct acpi_device_id qcom_rng_acpi_match[] = {

>>> +	{

>>> +		.id = "QCOM8160",

>>> +	},

>>> +	{}

>>> +};

>>> +MODULE_DEVICE_TABLE(acpi, msm_rng_acpi_match);

>>

>> qcom_rng_acpi_match?

>> Looks like a copy/paste issue.  This causes a build failure for me.

>> I'm trying to see if it works otherwise...

> 

> Ah sorry about that, I though I had fixed it, looks like I missed to

> fold the fix.

> 

> Did it work for you?

> 

It seems to work.  Driver compiles, and loads.  Still working on the 
verification.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
diff mbox series

Patch

diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index fdbbcac7bcb8..bc0131d130d6 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>
@@ -154,6 +155,7 @@  static int qcom_rng_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct qcom_rng *rng;
+	u32 val;
 	int ret;
 
 	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
@@ -168,11 +170,27 @@  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,
+	 * the PRNG register space is read-only and the PRNG should
+	 * already be enabled.
+	 */
+	if (has_acpi_companion(&pdev->dev)) {
+		val = readl(rng->base + PRNG_CONFIG);
+		if (!(val & PRNG_CONFIG_HW_ENABLE)) {
+			dev_err(&pdev->dev, "device is not enabled\n");
+			return -ENODEV;
+		}
+		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 +211,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, msm_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 +234,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);