| Message ID | 1529594276-12210-2-git-send-email-timur@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21-06-18, 10:17, Timur Tabi 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. so if you are using v2 hardware, are you pointing to High Level OS EE or some other..? > Signed-off-by: Timur Tabi <timur@codeaurora.org> > --- > drivers/char/hw_random/msm-rng.c | 40 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c > index 44580588b938..f34713d23d77 100644 > --- a/drivers/char/hw_random/msm-rng.c > +++ b/drivers/char/hw_random/msm-rng.c > @@ -20,6 +20,7 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/spinlock.h> > +#include <linux/acpi.h> > > /* Device specific register offsets */ > #define PRNG_DATA_OUT 0x0000 > @@ -186,16 +187,32 @@ static int msm_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->hwrng.name = KBUILD_MODNAME; > - rng->hwrng.init = msm_rng_init; > - rng->hwrng.cleanup = msm_rng_cleanup; > rng->hwrng.read = msm_rng_read; > spin_lock_init(&rng->lock); > > + /* > + * 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)) { > + u32 val; > + > + val = readl(rng->base + PRNG_CONFIG); v2 EEs dont seem to have CONFIG register, so not sure about this one > + if (!(val & PRNG_CONFIG_HW_ENABLE)) { > + dev_err(&pdev->dev, "device is not enabled\n"); > + return -ENODEV; > + } > + } else { > + rng->clk = devm_clk_get(&pdev->dev, "core"); > + if (IS_ERR(rng->clk)) > + return PTR_ERR(rng->clk); > + > + rng->hwrng.init = msm_rng_init; > + rng->hwrng.cleanup = msm_rng_cleanup; > + } > + > ret = devm_hwrng_register(&pdev->dev, &rng->hwrng); > if (ret) { > dev_err(&pdev->dev, "failed to register hwrng\n"); > @@ -211,11 +228,22 @@ static int msm_rng_probe(struct platform_device *pdev) > }; > MODULE_DEVICE_TABLE(of, msm_rng_of_match); > > +#if IS_ENABLED(CONFIG_ACPI) > +static const struct acpi_device_id msm_rng_acpi_match[] = { > + { > + .id = "QCOM8160", /* v2 PRNG */ > + }, > + {} > +}; > +MODULE_DEVICE_TABLE(acpi, msm_rng_acpi_match); > +#endif > + > static struct platform_driver msm_rng_driver = { > .probe = msm_rng_probe, > .driver = { > .name = KBUILD_MODNAME, > .of_match_table = of_match_ptr(msm_rng_of_match), > + .acpi_match_table = ACPI_PTR(msm_rng_acpi_match), > } > }; > module_platform_driver(msm_rng_driver); > -- > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm > Technologies, Inc. Qualcomm Technologies, Inc. is a member of the > Code Aurora Forum, a Linux Foundation Collaborative Project.
On 6/21/18 11:23 PM, Vinod wrote: > On 21-06-18, 10:17, Timur Tabi 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. > > so if you are using v2 hardware, are you pointing to High Level OS EE or > some other..? I'm not sure what you mean. >> + /* >> + * 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)) { >> + u32 val; >> + >> + val = readl(rng->base + PRNG_CONFIG); > > v2 EEs dont seem to have CONFIG register, so not sure about this one I can post the register set, but this works on my silicon. The first device has all the registers. Then there are about 12-13 other devices with their own 64K register regions, and those don't have a config register. I don't know why you would choose to support a one of the secondary register sets when you can use the primary.
On 21-06-18, 23:26, Timur Tabi wrote: > On 6/21/18 11:23 PM, Vinod wrote: > > On 21-06-18, 10:17, Timur Tabi 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. > > > > so if you are using v2 hardware, are you pointing to High Level OS EE or > > some other..? > > I'm not sure what you mean. > > > > + /* > > > + * 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)) { > > > + u32 val; > > > + > > > + val = readl(rng->base + PRNG_CONFIG); > > > > v2 EEs dont seem to have CONFIG register, so not sure about this one > > I can post the register set, but this works on my silicon. The first device > has all the registers. Then there are about 12-13 other devices with their > own 64K register regions, and those don't have a config register. The one on MSM8996 have 4K register space for each region and first two regions are not accessible to the SW. They are part of security ring and hence the CONFIG register is not accessible. If i try to access then it borks! Are you sure you are supposed to use the TZ there, I would presume the lower level firmware would use that? > I don't know why you would choose to support a one of the secondary register > sets when you can use the primary. Access denied is the reason :D So this make me think you should do 2 level support for ACPI, one ACPI support and one V2 support where we dont touch CONFIG register. That way both regions will work
On 6/21/18 11:44 PM, Vinod wrote: > So this make me think you should do 2 level support for ACPI, one ACPI > support and one V2 support where we dont touch CONFIG register. That way > both regions will work The ACPI system is a v2 system. If you want, I can just remove the read of the CONFIG register.
On 21-06-18, 23:46, Timur Tabi wrote: > On 6/21/18 11:44 PM, Vinod wrote: > > So this make me think you should do 2 level support for ACPI, one ACPI > > support and one V2 support where we dont touch CONFIG register. That way > > both regions will work > > The ACPI system is a v2 system. If you want, I can just remove the read of > the CONFIG register. Okay, so in this case who configures v2. My guess here is that firmware hasn't locked access, default everyone can access per spec
diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c index 44580588b938..f34713d23d77 100644 --- a/drivers/char/hw_random/msm-rng.c +++ b/drivers/char/hw_random/msm-rng.c @@ -20,6 +20,7 @@ #include <linux/of.h> #include <linux/platform_device.h> #include <linux/spinlock.h> +#include <linux/acpi.h> /* Device specific register offsets */ #define PRNG_DATA_OUT 0x0000 @@ -186,16 +187,32 @@ static int msm_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->hwrng.name = KBUILD_MODNAME; - rng->hwrng.init = msm_rng_init; - rng->hwrng.cleanup = msm_rng_cleanup; rng->hwrng.read = msm_rng_read; spin_lock_init(&rng->lock); + /* + * 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)) { + u32 val; + + val = readl(rng->base + PRNG_CONFIG); + if (!(val & PRNG_CONFIG_HW_ENABLE)) { + dev_err(&pdev->dev, "device is not enabled\n"); + return -ENODEV; + } + } else { + rng->clk = devm_clk_get(&pdev->dev, "core"); + if (IS_ERR(rng->clk)) + return PTR_ERR(rng->clk); + + rng->hwrng.init = msm_rng_init; + rng->hwrng.cleanup = msm_rng_cleanup; + } + ret = devm_hwrng_register(&pdev->dev, &rng->hwrng); if (ret) { dev_err(&pdev->dev, "failed to register hwrng\n"); @@ -211,11 +228,22 @@ static int msm_rng_probe(struct platform_device *pdev) }; MODULE_DEVICE_TABLE(of, msm_rng_of_match); +#if IS_ENABLED(CONFIG_ACPI) +static const struct acpi_device_id msm_rng_acpi_match[] = { + { + .id = "QCOM8160", /* v2 PRNG */ + }, + {} +}; +MODULE_DEVICE_TABLE(acpi, msm_rng_acpi_match); +#endif + static struct platform_driver msm_rng_driver = { .probe = msm_rng_probe, .driver = { .name = KBUILD_MODNAME, .of_match_table = of_match_ptr(msm_rng_of_match), + .acpi_match_table = ACPI_PTR(msm_rng_acpi_match), } }; module_platform_driver(msm_rng_driver);
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> --- drivers/char/hw_random/msm-rng.c | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-)