| Message ID | 20230901131502.1549809-1-quic_omprsing@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | crypto: qcom-rng: Add hwrng support | expand |
On Fri, Sep 01, 2023 at 06:45:02PM +0530, Om Prakash Singh wrote: > This is follow patch on top of [1] This information does not add value to the git history, if you need to inform the maintainer that the patch should be applied after some in-flight dependency then state so after the "---" line below. But, this patch strictly conflicts with [1], so the statement won't make sense if this is merged. > to add hwrng support for newer platform with trng capability. Please rewrite this so that it's clear that the problem you're trying to solve with this patch (i.e. the problem description) is that newer platforms has trng. Describe how this relates to the existing driver (e.g. same/similar hardware interface). State that you purposefully kept the crypto interface in place for the new hardware as well (so that it's clear that this isn't an accident or oversight). > > [1] https://lore.kernel.org/lkml/20230824-topic-sm8550-rng-v2-4-dfcafbb16a3e@linaro.org/ > > Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com> > --- > drivers/crypto/qcom-rng.c | 72 ++++++++++++++++++++++++++++++++++----- > 1 file changed, 63 insertions(+), 9 deletions(-) > > diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c > index fb54b8cfc35f..f78ffdcc66ec 100644 > --- a/drivers/crypto/qcom-rng.c > +++ b/drivers/crypto/qcom-rng.c > @@ -13,6 +13,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/platform_device.h> > +#include <linux/hw_random.h> Please keep these sorted, alphabetically. > > /* Device specific register offsets */ > #define PRNG_DATA_OUT 0x0000 > @@ -32,13 +33,18 @@ struct qcom_rng { > struct mutex lock; > void __iomem *base; > struct clk *clk; > - unsigned int skip_init; > + struct qcom_rng_of_data *of_data; > }; > > struct qcom_rng_ctx { > struct qcom_rng *rng; > }; > > +struct qcom_rng_of_data { > + bool skip_init; > + bool hwrng_support; > +}; > + > static struct qcom_rng *qcom_rng_dev; > > static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max) > @@ -70,7 +76,7 @@ static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max) > } > } while (currsize < max); > > - return 0; > + return currsize; > } > > static int qcom_rng_generate(struct crypto_rng *tfm, > @@ -79,7 +85,8 @@ static int qcom_rng_generate(struct crypto_rng *tfm, > { > struct qcom_rng_ctx *ctx = crypto_rng_ctx(tfm); > struct qcom_rng *rng = ctx->rng; > - int ret; > + int ret = -EFAULT; This initialization is useless, the very first thing you do with ret is to overwrite it with the return value of clk_prepare_enable(). > + int read_size = 0; Similarly, the first use of this variable is an assignment. And "ret" was a good, short, variable name. > > ret = clk_prepare_enable(rng->clk); > if (ret) > @@ -87,11 +94,14 @@ static int qcom_rng_generate(struct crypto_rng *tfm, > > mutex_lock(&rng->lock); > > - ret = qcom_rng_read(rng, dstn, dlen); > + read_size = qcom_rng_read(rng, dstn, dlen); > > mutex_unlock(&rng->lock); > clk_disable_unprepare(rng->clk); > > + if (read_size == dlen) This function used to return < 0 if qcom_rng_read() returned an error, and 0 in all other cases. Now you will pass through negative values, you will return 0 if the loop in qcom_rng_read() reached "dlen" ("max)", and you will return some positive number if the break condition in the loop hit. In other words, this is wrong. > + ret = 0; > + > return ret; > } > > @@ -101,6 +111,16 @@ static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed, > return 0; > } > > +static int qcom_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) > +{ > + int ret; > + struct qcom_rng *qrng; > + > + qrng = (struct qcom_rng *)rng->priv; > + ret = qcom_rng_read(qrng, data, max); > + return ret; Initialize qrng directly when you define it, drop ret and just return qcom_rng_read() directly. > +} > + > static int qcom_rng_enable(struct qcom_rng *rng) > { > u32 val; > @@ -136,7 +156,7 @@ static int qcom_rng_init(struct crypto_tfm *tfm) > > ctx->rng = qcom_rng_dev; > > - if (!ctx->rng->skip_init) > + if (!ctx->rng->of_data->skip_init) > return qcom_rng_enable(ctx->rng); > > return 0; > @@ -157,9 +177,16 @@ static struct rng_alg qcom_rng_alg = { > } > }; > > +static struct hwrng qcom_hwrng = { > + .name = "qcom-hwrng", > + .read = qcom_hwrng_read, > + .quality = 1024, > +}; > + > static int qcom_rng_probe(struct platform_device *pdev) > { > struct qcom_rng *rng; > + const struct qcom_rng_of_data *data; Move this one line up, so we maintain the reverse xmas tree. > int ret; > > rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL); > @@ -177,7 +204,8 @@ static int qcom_rng_probe(struct platform_device *pdev) > if (IS_ERR(rng->clk)) > return PTR_ERR(rng->clk); > > - rng->skip_init = (unsigned long)device_get_match_data(&pdev->dev); > + data = of_device_get_match_data(&pdev->dev); > + rng->of_data = (struct qcom_rng_of_data *)data; Why didn't you assign rng->of_data directly? Also, of_device_get_match_data() returns a void *, so you should have to explicitly cast this. > > qcom_rng_dev = rng; > ret = crypto_register_rng(&qcom_rng_alg); > @@ -185,6 +213,14 @@ static int qcom_rng_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "Register crypto rng failed: %d\n", ret); > qcom_rng_dev = NULL; > } Would be nice with a newline here, to get some separation between the "paragraphs". > + if (rng->of_data->hwrng_support) { > + qcom_hwrng.priv = (unsigned long)qcom_rng_dev; > + ret = hwrng_register(&qcom_hwrng); > + if (ret) { > + dev_err(&pdev->dev, "Register hwrng failed: %d\n", ret); > + qcom_rng_dev = NULL; You're leaving the rng registered with crypto here. Not sure if that will result in a use after free, or just a NULL pointer dereference - but it's not good either way. > + } > + } > > return ret; > } > @@ -193,11 +229,29 @@ static int qcom_rng_remove(struct platform_device *pdev) > { > crypto_unregister_rng(&qcom_rng_alg); > > + if (qcom_rng_dev->of_data->hwrng_support) > + hwrng_unregister(&qcom_hwrng); Why not use devm_hwrng_register() above instead? Then you wouldn't have to unregister it here. > + > qcom_rng_dev = NULL; > > return 0; > } > > +struct qcom_rng_of_data qcom_prng_of_data = { > + .skip_init = false, > + .hwrng_support = false, > +}; > + > +struct qcom_rng_of_data qcom_prng_ee_of_data = { > + .skip_init = true, > + .hwrng_support = false, > +}; > + > +struct qcom_rng_of_data qcom_trng_of_data = { > + .skip_init = true, Can you please confirm that it's appropriate to name this "trng" without the "-ee" suffix. Should all trng instances (v2 and v3) skip initialization? > + .hwrng_support = true, > +}; > + > static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = { > { .id = "QCOM8160", .driver_data = 1 }, > {} > @@ -205,9 +259,9 @@ static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = { > MODULE_DEVICE_TABLE(acpi, qcom_rng_acpi_match); > > static const struct of_device_id __maybe_unused qcom_rng_of_match[] = { > - { .compatible = "qcom,prng", .data = (void *)0}, > - { .compatible = "qcom,prng-ee", .data = (void *)1}, > - { .compatible = "qcom,trng", .data = (void *)1}, > + { .compatible = "qcom,prng", .data = &qcom_prng_of_data }, > + { .compatible = "qcom,prng-ee", .data = &qcom_prng_ee_of_data }, > + { .compatible = "qcom,trng", .data = &qcom_trng_of_data }, So, should this be qcom,trng or qcom,trng-ee? Where is your devicetree binding patch? Regards, Bjorn
On 01/09/2023 15:15, Om Prakash Singh wrote: > This is follow patch on top of [1] to add hwrng support for newer > platform with trng capability. Please use subject prefixes matching the subsystem. Best regards, Krzysztof
On 9/1/2023 8:16 PM, Bjorn Andersson wrote: > On Fri, Sep 01, 2023 at 06:45:02PM +0530, Om Prakash Singh wrote: >> This is follow patch on top of [1] > > This information does not add value to the git history, if you need to > inform the maintainer that the patch should be applied after some > in-flight dependency then state so after the "---" line below. > > But, this patch strictly conflicts with [1], so the statement won't make > sense if this is merged. > >> to add hwrng support for newer platform with trng capability. > > Please rewrite this so that it's clear that the problem you're trying to > solve with this patch (i.e. the problem description) is that newer > platforms has trng. Describe how this relates to the existing driver > (e.g. same/similar hardware interface). State that you purposefully kept > the crypto interface in place for the new hardware as well (so that it's > clear that this isn't an accident or oversight). > >> >> [1] https://lore.kernel.org/lkml/20230824-topic-sm8550-rng-v2-4-dfcafbb16a3e@linaro.org/ >> >> Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com> >> --- >> drivers/crypto/qcom-rng.c | 72 ++++++++++++++++++++++++++++++++++----- >> 1 file changed, 63 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c >> index fb54b8cfc35f..f78ffdcc66ec 100644 >> --- a/drivers/crypto/qcom-rng.c >> +++ b/drivers/crypto/qcom-rng.c >> @@ -13,6 +13,7 @@ >> #include <linux/module.h> >> #include <linux/of.h> >> #include <linux/platform_device.h> >> +#include <linux/hw_random.h> > > Please keep these sorted, alphabetically. > >> >> /* Device specific register offsets */ >> #define PRNG_DATA_OUT 0x0000 >> @@ -32,13 +33,18 @@ struct qcom_rng { >> struct mutex lock; >> void __iomem *base; >> struct clk *clk; >> - unsigned int skip_init; >> + struct qcom_rng_of_data *of_data; >> }; >> >> struct qcom_rng_ctx { >> struct qcom_rng *rng; >> }; >> >> +struct qcom_rng_of_data { >> + bool skip_init; >> + bool hwrng_support; >> +}; >> + >> static struct qcom_rng *qcom_rng_dev; >> >> static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max) >> @@ -70,7 +76,7 @@ static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max) >> } >> } while (currsize < max); >> >> - return 0; >> + return currsize; >> } >> >> static int qcom_rng_generate(struct crypto_rng *tfm, >> @@ -79,7 +85,8 @@ static int qcom_rng_generate(struct crypto_rng *tfm, >> { >> struct qcom_rng_ctx *ctx = crypto_rng_ctx(tfm); >> struct qcom_rng *rng = ctx->rng; >> - int ret; >> + int ret = -EFAULT; > > This initialization is useless, the very first thing you do with ret is > to overwrite it with the return value of clk_prepare_enable(). > >> + int read_size = 0; > > Similarly, the first use of this variable is an assignment. And "ret" > was a good, short, variable name. > >> >> ret = clk_prepare_enable(rng->clk); >> if (ret) >> @@ -87,11 +94,14 @@ static int qcom_rng_generate(struct crypto_rng *tfm, >> >> mutex_lock(&rng->lock); >> >> - ret = qcom_rng_read(rng, dstn, dlen); >> + read_size = qcom_rng_read(rng, dstn, dlen); >> >> mutex_unlock(&rng->lock); >> clk_disable_unprepare(rng->clk); >> >> + if (read_size == dlen) > > This function used to return < 0 if qcom_rng_read() returned an error, > and 0 in all other cases. > > Now you will pass through negative values, you will return 0 if the loop > in qcom_rng_read() reached "dlen" ("max)", and you will return some > positive number if the break condition in the loop hit. > > In other words, this is wrong. > >> + ret = 0; >> + >> return ret; >> } >> >> @@ -101,6 +111,16 @@ static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed, >> return 0; >> } >> >> +static int qcom_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) >> +{ >> + int ret; >> + struct qcom_rng *qrng; >> + >> + qrng = (struct qcom_rng *)rng->priv; >> + ret = qcom_rng_read(qrng, data, max); >> + return ret; > > Initialize qrng directly when you define it, drop ret and just return > qcom_rng_read() directly. > >> +} >> + >> static int qcom_rng_enable(struct qcom_rng *rng) >> { >> u32 val; >> @@ -136,7 +156,7 @@ static int qcom_rng_init(struct crypto_tfm *tfm) >> >> ctx->rng = qcom_rng_dev; >> >> - if (!ctx->rng->skip_init) >> + if (!ctx->rng->of_data->skip_init) >> return qcom_rng_enable(ctx->rng); >> >> return 0; >> @@ -157,9 +177,16 @@ static struct rng_alg qcom_rng_alg = { >> } >> }; >> >> +static struct hwrng qcom_hwrng = { >> + .name = "qcom-hwrng", >> + .read = qcom_hwrng_read, >> + .quality = 1024, >> +}; >> + >> static int qcom_rng_probe(struct platform_device *pdev) >> { >> struct qcom_rng *rng; >> + const struct qcom_rng_of_data *data; > > Move this one line up, so we maintain the reverse xmas tree. > >> int ret; >> >> rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL); >> @@ -177,7 +204,8 @@ static int qcom_rng_probe(struct platform_device *pdev) >> if (IS_ERR(rng->clk)) >> return PTR_ERR(rng->clk); >> >> - rng->skip_init = (unsigned long)device_get_match_data(&pdev->dev); >> + data = of_device_get_match_data(&pdev->dev); >> + rng->of_data = (struct qcom_rng_of_data *)data; > > Why didn't you assign rng->of_data directly? > > Also, of_device_get_match_data() returns a void *, so you should have to > explicitly cast this. > >> >> qcom_rng_dev = rng; >> ret = crypto_register_rng(&qcom_rng_alg); >> @@ -185,6 +213,14 @@ static int qcom_rng_probe(struct platform_device *pdev) >> dev_err(&pdev->dev, "Register crypto rng failed: %d\n", ret); >> qcom_rng_dev = NULL; >> } > > Would be nice with a newline here, to get some separation between the > "paragraphs". > >> + if (rng->of_data->hwrng_support) { >> + qcom_hwrng.priv = (unsigned long)qcom_rng_dev; >> + ret = hwrng_register(&qcom_hwrng); >> + if (ret) { >> + dev_err(&pdev->dev, "Register hwrng failed: %d\n", ret); >> + qcom_rng_dev = NULL; > > You're leaving the rng registered with crypto here. Not sure if that > will result in a use after free, or just a NULL pointer dereference - > but it's not good either way. > >> + } >> + } >> >> return ret; >> } >> @@ -193,11 +229,29 @@ static int qcom_rng_remove(struct platform_device *pdev) >> { >> crypto_unregister_rng(&qcom_rng_alg); >> >> + if (qcom_rng_dev->of_data->hwrng_support) >> + hwrng_unregister(&qcom_hwrng); > > Why not use devm_hwrng_register() above instead? Then you wouldn't have > to unregister it here. > >> + >> qcom_rng_dev = NULL; >> >> return 0; >> } >> >> +struct qcom_rng_of_data qcom_prng_of_data = { >> + .skip_init = false, >> + .hwrng_support = false, >> +}; >> + >> +struct qcom_rng_of_data qcom_prng_ee_of_data = { >> + .skip_init = true, >> + .hwrng_support = false, >> +}; >> + >> +struct qcom_rng_of_data qcom_trng_of_data = { >> + .skip_init = true, > > Can you please confirm that it's appropriate to name this "trng" without > the "-ee" suffix. Should all trng instances (v2 and v3) skip > initialization? All trng supported platform needs to skip initialzation. we don't need to have both "trng-ee" and "trng". If "trng-ee" is prefer we shold update it in patch [1] it itself, > >> + .hwrng_support = true, >> +}; >> + >> static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = { >> { .id = "QCOM8160", .driver_data = 1 }, >> {} >> @@ -205,9 +259,9 @@ static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = { >> MODULE_DEVICE_TABLE(acpi, qcom_rng_acpi_match); >> >> static const struct of_device_id __maybe_unused qcom_rng_of_match[] = { >> - { .compatible = "qcom,prng", .data = (void *)0}, >> - { .compatible = "qcom,prng-ee", .data = (void *)1}, >> - { .compatible = "qcom,trng", .data = (void *)1}, >> + { .compatible = "qcom,prng", .data = &qcom_prng_of_data }, >> + { .compatible = "qcom,prng-ee", .data = &qcom_prng_ee_of_data }, >> + { .compatible = "qcom,trng", .data = &qcom_trng_of_data }, > > So, should this be qcom,trng or qcom,trng-ee? > > > Where is your devicetree binding patch? DT binding patch is submitted by Neil [1] I will address all other review comments in next patch version. [1] https://lore.kernel.org/lkml/20230824-topic-sm8550-rng-v2-4-dfcafbb16a3e@linaro.org/ > > Regards, > Bjorn
On 5.09.2023 04:50, Om Prakash Singh wrote: > > > On 9/1/2023 8:16 PM, Bjorn Andersson wrote: >> On Fri, Sep 01, 2023 at 06:45:02PM +0530, Om Prakash Singh wrote: >>> This is follow patch on top of [1] >> >> This information does not add value to the git history, if you need to >> inform the maintainer that the patch should be applied after some >> in-flight dependency then state so after the "---" line below. >> >> But, this patch strictly conflicts with [1], so the statement won't make >> sense if this is merged. >> >>> to add hwrng support for newer platform with trng capability. >> >> Please rewrite this so that it's clear that the problem you're trying to >> solve with this patch (i.e. the problem description) is that newer >> platforms has trng. Describe how this relates to the existing driver >> (e.g. same/similar hardware interface). State that you purposefully kept >> the crypto interface in place for the new hardware as well (so that it's >> clear that this isn't an accident or oversight). >> >>> >>> [1] https://lore.kernel.org/lkml/20230824-topic-sm8550-rng-v2-4-dfcafbb16a3e@linaro.org/ >>> >>> Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com> >>> --- [...] >> >> Can you please confirm that it's appropriate to name this "trng" without >> the "-ee" suffix. Should all trng instances (v2 and v3) skip >> initialization? > All trng supported platform needs to skip initialzation. > we don't need to have both "trng-ee" and "trng". > If "trng-ee" is prefer we shold update it in patch [1] it itself, Looking back at ba3ab6371cdd ("crypto: qcom-rng - Add support for prng-ee"), it was solved in a way that we would stray from today - nowadays we'd call it qcom,msm8996-prng or something. The -ee part was only there to discern parts that were initialized by other software. Since you said that all TRNGs need that, I'm also for dropping "-ee". Konrad
On Fri, Sep 01, 2023 at 06:45:02PM +0530, Om Prakash Singh wrote: > > + qrng = (struct qcom_rng *)rng->priv; Please stop using rng->priv, it is obsolete. Instead embed the rng object inside qcom_rng. Cheers,
diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c index fb54b8cfc35f..f78ffdcc66ec 100644 --- a/drivers/crypto/qcom-rng.c +++ b/drivers/crypto/qcom-rng.c @@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/platform_device.h> +#include <linux/hw_random.h> /* Device specific register offsets */ #define PRNG_DATA_OUT 0x0000 @@ -32,13 +33,18 @@ struct qcom_rng { struct mutex lock; void __iomem *base; struct clk *clk; - unsigned int skip_init; + struct qcom_rng_of_data *of_data; }; struct qcom_rng_ctx { struct qcom_rng *rng; }; +struct qcom_rng_of_data { + bool skip_init; + bool hwrng_support; +}; + static struct qcom_rng *qcom_rng_dev; static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max) @@ -70,7 +76,7 @@ static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max) } } while (currsize < max); - return 0; + return currsize; } static int qcom_rng_generate(struct crypto_rng *tfm, @@ -79,7 +85,8 @@ static int qcom_rng_generate(struct crypto_rng *tfm, { struct qcom_rng_ctx *ctx = crypto_rng_ctx(tfm); struct qcom_rng *rng = ctx->rng; - int ret; + int ret = -EFAULT; + int read_size = 0; ret = clk_prepare_enable(rng->clk); if (ret) @@ -87,11 +94,14 @@ static int qcom_rng_generate(struct crypto_rng *tfm, mutex_lock(&rng->lock); - ret = qcom_rng_read(rng, dstn, dlen); + read_size = qcom_rng_read(rng, dstn, dlen); mutex_unlock(&rng->lock); clk_disable_unprepare(rng->clk); + if (read_size == dlen) + ret = 0; + return ret; } @@ -101,6 +111,16 @@ static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed, return 0; } +static int qcom_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{ + int ret; + struct qcom_rng *qrng; + + qrng = (struct qcom_rng *)rng->priv; + ret = qcom_rng_read(qrng, data, max); + return ret; +} + static int qcom_rng_enable(struct qcom_rng *rng) { u32 val; @@ -136,7 +156,7 @@ static int qcom_rng_init(struct crypto_tfm *tfm) ctx->rng = qcom_rng_dev; - if (!ctx->rng->skip_init) + if (!ctx->rng->of_data->skip_init) return qcom_rng_enable(ctx->rng); return 0; @@ -157,9 +177,16 @@ static struct rng_alg qcom_rng_alg = { } }; +static struct hwrng qcom_hwrng = { + .name = "qcom-hwrng", + .read = qcom_hwrng_read, + .quality = 1024, +}; + static int qcom_rng_probe(struct platform_device *pdev) { struct qcom_rng *rng; + const struct qcom_rng_of_data *data; int ret; rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL); @@ -177,7 +204,8 @@ static int qcom_rng_probe(struct platform_device *pdev) if (IS_ERR(rng->clk)) return PTR_ERR(rng->clk); - rng->skip_init = (unsigned long)device_get_match_data(&pdev->dev); + data = of_device_get_match_data(&pdev->dev); + rng->of_data = (struct qcom_rng_of_data *)data; qcom_rng_dev = rng; ret = crypto_register_rng(&qcom_rng_alg); @@ -185,6 +213,14 @@ static int qcom_rng_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Register crypto rng failed: %d\n", ret); qcom_rng_dev = NULL; } + if (rng->of_data->hwrng_support) { + qcom_hwrng.priv = (unsigned long)qcom_rng_dev; + ret = hwrng_register(&qcom_hwrng); + if (ret) { + dev_err(&pdev->dev, "Register hwrng failed: %d\n", ret); + qcom_rng_dev = NULL; + } + } return ret; } @@ -193,11 +229,29 @@ static int qcom_rng_remove(struct platform_device *pdev) { crypto_unregister_rng(&qcom_rng_alg); + if (qcom_rng_dev->of_data->hwrng_support) + hwrng_unregister(&qcom_hwrng); + qcom_rng_dev = NULL; return 0; } +struct qcom_rng_of_data qcom_prng_of_data = { + .skip_init = false, + .hwrng_support = false, +}; + +struct qcom_rng_of_data qcom_prng_ee_of_data = { + .skip_init = true, + .hwrng_support = false, +}; + +struct qcom_rng_of_data qcom_trng_of_data = { + .skip_init = true, + .hwrng_support = true, +}; + static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = { { .id = "QCOM8160", .driver_data = 1 }, {} @@ -205,9 +259,9 @@ static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = { MODULE_DEVICE_TABLE(acpi, qcom_rng_acpi_match); static const struct of_device_id __maybe_unused qcom_rng_of_match[] = { - { .compatible = "qcom,prng", .data = (void *)0}, - { .compatible = "qcom,prng-ee", .data = (void *)1}, - { .compatible = "qcom,trng", .data = (void *)1}, + { .compatible = "qcom,prng", .data = &qcom_prng_of_data }, + { .compatible = "qcom,prng-ee", .data = &qcom_prng_ee_of_data }, + { .compatible = "qcom,trng", .data = &qcom_trng_of_data }, {} }; MODULE_DEVICE_TABLE(of, qcom_rng_of_match);
This is follow patch on top of [1] to add hwrng support for newer platform with trng capability. [1] https://lore.kernel.org/lkml/20230824-topic-sm8550-rng-v2-4-dfcafbb16a3e@linaro.org/ Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com> --- drivers/crypto/qcom-rng.c | 72 ++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 9 deletions(-)