Message ID | 20240618003743.2975-1-semen.protsenko@linaro.org |
---|---|
Headers | show |
Series | hwrng: exynos: Add support for Exynos850 | expand |
Hi Sam, On Tue, 18 Jun 2024 at 06:08, Sam Protsenko <semen.protsenko@linaro.org> wrote: > > Some SoCs like Exynos850 might require the SSS bus clock (PCLK) to be > enabled in order to access TRNG registers. Add and handle optional PCLK > clock accordingly to make it possible. > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > --- > drivers/char/hw_random/exynos-trng.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c > index 88a5088ed34d..4520a280134c 100644 > --- a/drivers/char/hw_random/exynos-trng.c > +++ b/drivers/char/hw_random/exynos-trng.c > @@ -47,7 +47,8 @@ > struct exynos_trng_dev { > struct device *dev; > void __iomem *mem; > - struct clk *clk; > + struct clk *clk; /* operating clock */ > + struct clk *pclk; /* bus clock */ > struct hwrng rng; > }; > > @@ -141,10 +142,23 @@ static int exynos_trng_probe(struct platform_device *pdev) > goto err_clock; > } > > + trng->pclk = devm_clk_get_optional(&pdev->dev, "pclk"); Use devm_clk_get_optional_enabled to avoid clk_prepare_enable > + if (IS_ERR(trng->pclk)) { > + ret = dev_err_probe(&pdev->dev, PTR_ERR(trng->pclk), > + "cannot get pclk"); > + goto err_clock; > + } > + > + ret = clk_prepare_enable(trng->pclk); > + if (ret) { > + dev_err(&pdev->dev, "Could not enable the pclk.\n"); > + goto err_clock; > + } > + > ret = clk_prepare_enable(trng->clk); Use devm_clk_get_enabled for this clock > if (ret) { > dev_err(&pdev->dev, "Could not enable the clk.\n"); > - goto err_clock; > + goto err_clock_enable; > } > > ret = devm_hwrng_register(&pdev->dev, &trng->rng); > @@ -160,6 +174,9 @@ static int exynos_trng_probe(struct platform_device *pdev) > err_register: > clk_disable_unprepare(trng->clk); > > +err_clock_enable: > + clk_disable_unprepare(trng->pclk); > + > err_clock: > pm_runtime_put_noidle(&pdev->dev); > > @@ -174,6 +191,7 @@ static void exynos_trng_remove(struct platform_device *pdev) > struct exynos_trng_dev *trng = platform_get_drvdata(pdev); > > clk_disable_unprepare(trng->clk); > + clk_disable_unprepare(trng->pclk); > > pm_runtime_put_sync(&pdev->dev); > pm_runtime_disable(&pdev->dev); > -- > 2.39.2 > > Thanks -Anand
On 18/06/2024 02:37, Sam Protsenko wrote: > Exynos TRNG (True Random Number Generator) hardware block is found in > various Exynos chips, not only in Exynos5250. Rename the binding doc to > reflect that fact and make the naming more precise. > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > --- > .../{samsung,exynos5250-trng.yaml => samsung,exynos-trng.yaml} | 2 +- > MAINTAINERS | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) No, no need. Best regards, Krzysztof
On 18/06/2024 02:37, Sam Protsenko wrote: > Add Exynos850 compatible and its driver data. It's only possible to > access TRNG block via SMC calls in Exynos850, so specify that fact using > QUIRK_SMC in the driver data. > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > --- > drivers/char/hw_random/exynos-trng.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c > index 98b7a8ebb909..3368a08df9ce 100644 > --- a/drivers/char/hw_random/exynos-trng.c > +++ b/drivers/char/hw_random/exynos-trng.c > @@ -333,6 +333,9 @@ static DEFINE_SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend, > static const struct of_device_id exynos_trng_dt_match[] = { > { > .compatible = "samsung,exynos5250-trng", > + }, { > + .compatible = "samsung,exynos850-trng", > + .data = (void *)QUIRK_SMC, Probably this (and in previous patch) should be called flags, not quirks. Quirks are for work-arounds. Best regards, Krzysztof
On Mon, Jun 17, 2024 at 11:26 PM Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Sam, > > On Tue, 18 Jun 2024 at 06:08, Sam Protsenko <semen.protsenko@linaro.org> wrote: > > > > Some SoCs like Exynos850 might require the SSS bus clock (PCLK) to be > > enabled in order to access TRNG registers. Add and handle optional PCLK > > clock accordingly to make it possible. > > > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > > --- > > drivers/char/hw_random/exynos-trng.c | 22 ++++++++++++++++++++-- > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c > > index 88a5088ed34d..4520a280134c 100644 > > --- a/drivers/char/hw_random/exynos-trng.c > > +++ b/drivers/char/hw_random/exynos-trng.c > > @@ -47,7 +47,8 @@ > > struct exynos_trng_dev { > > struct device *dev; > > void __iomem *mem; > > - struct clk *clk; > > + struct clk *clk; /* operating clock */ > > + struct clk *pclk; /* bus clock */ > > struct hwrng rng; > > }; > > > > @@ -141,10 +142,23 @@ static int exynos_trng_probe(struct platform_device *pdev) > > goto err_clock; > > } > > > > + trng->pclk = devm_clk_get_optional(&pdev->dev, "pclk"); > > Use devm_clk_get_optional_enabled to avoid clk_prepare_enable > Thanks for pointing that out! Will fix in v2. > > + if (IS_ERR(trng->pclk)) { > > + ret = dev_err_probe(&pdev->dev, PTR_ERR(trng->pclk), > > + "cannot get pclk"); > > + goto err_clock; > > + } > > + > > + ret = clk_prepare_enable(trng->pclk); > > + if (ret) { > > + dev_err(&pdev->dev, "Could not enable the pclk.\n"); > > + goto err_clock; > > + } > > + > > ret = clk_prepare_enable(trng->clk); > > Use devm_clk_get_enabled for this clock > > > if (ret) { > > dev_err(&pdev->dev, "Could not enable the clk.\n"); > > - goto err_clock; > > + goto err_clock_enable; > > } > > > > ret = devm_hwrng_register(&pdev->dev, &trng->rng); > > @@ -160,6 +174,9 @@ static int exynos_trng_probe(struct platform_device *pdev) > > err_register: > > clk_disable_unprepare(trng->clk); > > > > +err_clock_enable: > > + clk_disable_unprepare(trng->pclk); > > + > > err_clock: > > pm_runtime_put_noidle(&pdev->dev); > > > > @@ -174,6 +191,7 @@ static void exynos_trng_remove(struct platform_device *pdev) > > struct exynos_trng_dev *trng = platform_get_drvdata(pdev); > > > > clk_disable_unprepare(trng->clk); > > + clk_disable_unprepare(trng->pclk); > > > > pm_runtime_put_sync(&pdev->dev); > > pm_runtime_disable(&pdev->dev); > > -- > > 2.39.2 > > > > > > Thanks > -Anand
On Tue, Jun 18, 2024 at 1:39 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 18/06/2024 02:37, Sam Protsenko wrote: > > Add Exynos850 compatible and its driver data. It's only possible to > > access TRNG block via SMC calls in Exynos850, so specify that fact using > > QUIRK_SMC in the driver data. > > > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > > --- > > drivers/char/hw_random/exynos-trng.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c > > index 98b7a8ebb909..3368a08df9ce 100644 > > --- a/drivers/char/hw_random/exynos-trng.c > > +++ b/drivers/char/hw_random/exynos-trng.c > > @@ -333,6 +333,9 @@ static DEFINE_SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend, > > static const struct of_device_id exynos_trng_dt_match[] = { > > { > > .compatible = "samsung,exynos5250-trng", > > + }, { > > + .compatible = "samsung,exynos850-trng", > > + .data = (void *)QUIRK_SMC, > > Probably this (and in previous patch) should be called flags, not > quirks. Quirks are for work-arounds. > Thanks for the quick review! Will submit v2 soon with all the comments addressed. > Best regards, > Krzysztof >