[PATCH 0/5] hwrng: meson: simplify driver

[PATCH 0/5] hwrng: meson: simplify driver

[Heiner Kallweit]

This series simplifies the driver. No functional change intended.

Heiner Kallweit (5):
  hwrng: meson: remove unused member of struct meson_rng_data
  hwrng: meson: use devm_clk_get_optional_enabled
  hwrng: meson: remove not needed call to platform_set_drvdata
  hwrng: meson: use struct hw_random priv data
  hwrng: meson: remove struct meson_rng_data

 drivers/char/hw_random/meson-rng.c | 59 +++++++++---------------------
 1 file changed, 17 insertions(+), 42 deletions(-)

-- 
2.39.2

[PATCH 1/5] hwrng: meson: remove unused member of struct meson_rng_data

[Heiner Kallweit]

Member pdev isn't used, remove it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/char/hw_random/meson-rng.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/char/hw_random/meson-rng.c b/drivers/char/hw_random/meson-rng.c
index 8bb30282c..e79069b6d 100644
--- a/drivers/char/hw_random/meson-rng.c
+++ b/drivers/char/hw_random/meson-rng.c
@@ -18,7 +18,6 @@
 
 struct meson_rng_data {
 	void __iomem *base;
-	struct platform_device *pdev;
 	struct hwrng rng;
 	struct clk *core_clk;
 };
@@ -48,8 +47,6 @@ static int meson_rng_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
-	data->pdev = pdev;
-
 	data->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(data->base))
 		return PTR_ERR(data->base);
-- 
2.39.2

Re: [PATCH 1/5] hwrng: meson: remove unused member of struct meson_rng_data

[Martin Blumenstingl]

On Sat, Feb 18, 2023 at 9:59 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> Member pdev isn't used, remove it.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

[PATCH 2/5] hwrng: meson: use devm_clk_get_optional_enabled

[Heiner Kallweit]

Use devm_clk_get_optional_enabled() to simplify the code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/char/hw_random/meson-rng.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/char/hw_random/meson-rng.c b/drivers/char/hw_random/meson-rng.c
index e79069b6d..22e3dcc6f 100644
--- a/drivers/char/hw_random/meson-rng.c
+++ b/drivers/char/hw_random/meson-rng.c
@@ -19,7 +19,6 @@
 struct meson_rng_data {
 	void __iomem *base;
 	struct hwrng rng;
-	struct clk *core_clk;
 };
 
 static int meson_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
@@ -32,16 +31,11 @@ static int meson_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
 	return sizeof(u32);
 }
 
-static void meson_rng_clk_disable(void *data)
-{
-	clk_disable_unprepare(data);
-}
-
 static int meson_rng_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct meson_rng_data *data;
-	int ret;
+	struct clk *core_clk;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -51,21 +45,11 @@ static int meson_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(data->base))
 		return PTR_ERR(data->base);
 
-	data->core_clk = devm_clk_get_optional(dev, "core");
-	if (IS_ERR(data->core_clk))
-		return dev_err_probe(dev, PTR_ERR(data->core_clk),
+	core_clk = devm_clk_get_optional_enabled(dev, "core");
+	if (IS_ERR(core_clk))
+		return dev_err_probe(dev, PTR_ERR(core_clk),
 				     "Failed to get core clock\n");
 
-	if (data->core_clk) {
-		ret = clk_prepare_enable(data->core_clk);
-		if (ret)
-			return ret;
-		ret = devm_add_action_or_reset(dev, meson_rng_clk_disable,
-					       data->core_clk);
-		if (ret)
-			return ret;
-	}
-
 	data->rng.name = pdev->name;
 	data->rng.read = meson_rng_read;
 
-- 
2.39.2

Re: [PATCH 2/5] hwrng: meson: use devm_clk_get_optional_enabled

[Martin Blumenstingl]

On Sat, Feb 18, 2023 at 9:59 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> Use devm_clk_get_optional_enabled() to simplify the code.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

[PATCH 3/5] hwrng: meson: remove not needed call to platform_set_drvdata

[Heiner Kallweit]

drvdata isn't used, therefore remove this call.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/char/hw_random/meson-rng.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/char/hw_random/meson-rng.c b/drivers/char/hw_random/meson-rng.c
index 22e3dcc6f..a4eb8e35f 100644
--- a/drivers/char/hw_random/meson-rng.c
+++ b/drivers/char/hw_random/meson-rng.c
@@ -53,8 +53,6 @@ static int meson_rng_probe(struct platform_device *pdev)
 	data->rng.name = pdev->name;
 	data->rng.read = meson_rng_read;
 
-	platform_set_drvdata(pdev, data);
-
 	return devm_hwrng_register(dev, &data->rng);
 }
 
-- 
2.39.2

Re: [PATCH 3/5] hwrng: meson: remove not needed call to platform_set_drvdata

[Martin Blumenstingl]

On Sat, Feb 18, 2023 at 9:59 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> drvdata isn't used, therefore remove this call.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

[PATCH 4/5] hwrng: meson: use struct hw_random priv data

[Heiner Kallweit]

Use the priv data member of struct hwrng to make the iomem base address
available in meson_rng_read(). This allows for removing struct
meson_rng_data completely in the next step.
__force is used to silence sparse warnings.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/char/hw_random/meson-rng.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/char/hw_random/meson-rng.c b/drivers/char/hw_random/meson-rng.c
index a4eb8e35f..bf7a6e594 100644
--- a/drivers/char/hw_random/meson-rng.c
+++ b/drivers/char/hw_random/meson-rng.c
@@ -17,16 +17,14 @@
 #define RNG_DATA 0x00
 
 struct meson_rng_data {
-	void __iomem *base;
 	struct hwrng rng;
 };
 
 static int meson_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
 {
-	struct meson_rng_data *data =
-			container_of(rng, struct meson_rng_data, rng);
+	void __iomem *base = (__force void __iomem *)rng->priv;
 
-	*(u32 *)buf = readl_relaxed(data->base + RNG_DATA);
+	*(u32 *)buf = readl_relaxed(base + RNG_DATA);
 
 	return sizeof(u32);
 }
@@ -36,14 +34,15 @@ static int meson_rng_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct meson_rng_data *data;
 	struct clk *core_clk;
+	void __iomem *base;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
-	data->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(data->base))
-		return PTR_ERR(data->base);
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
 
 	core_clk = devm_clk_get_optional_enabled(dev, "core");
 	if (IS_ERR(core_clk))
@@ -52,6 +51,7 @@ static int meson_rng_probe(struct platform_device *pdev)
 
 	data->rng.name = pdev->name;
 	data->rng.read = meson_rng_read;
+	data->rng.priv = (__force unsigned long)base;
 
 	return devm_hwrng_register(dev, &data->rng);
 }
-- 
2.39.2

Re: [PATCH 4/5] hwrng: meson: use struct hw_random priv data

[Martin Blumenstingl]

Hi Heiner,

On Sat, Feb 18, 2023 at 9:59 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> +       void __iomem *base = (__force void __iomem *)rng->priv;
I find that "(void __force __iomem *)" is the more common order in
other drivers. Which of these is correct is something I cannot tell
though.

Also I would like to hear some other opinions because to me the code
was easier to read before.
Your way is shorter and may save a few bytes (including alignment etc.) though.


Best regards,
Martin

Re: [PATCH 4/5] hwrng: meson: use struct hw_random priv data

[Heiner Kallweit]

On 19.02.2023 19:26, Martin Blumenstingl wrote:
> Hi Heiner,
> 
> On Sat, Feb 18, 2023 at 9:59 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> +       void __iomem *base = (__force void __iomem *)rng->priv;
> I find that "(void __force __iomem *)" is the more common order in
> other drivers. Which of these is correct is something I cannot tell
> though.
> 
I just looked at this one:
#define IOMEM_ERR_PTR(err) (__force void __iomem *)ERR_PTR(err)
But right, both orders exist.

> Also I would like to hear some other opinions because to me the code
> was easier to read before.
Right, if possible I'd like to avoid it too.
However it's the prerequisite for getting rid of struct meson_rng_data.
And this simplification outweighs this small drawback IMO.

> Your way is shorter and may save a few bytes (including alignment etc.) though.
> 
> 
> Best regards,
> Martin

Heiner

Re: [PATCH 4/5] hwrng: meson: use struct hw_random priv data

[Herbert Xu]

On Sat, Feb 18, 2023 at 09:58:07PM +0100, Heiner Kallweit wrote:
> Use the priv data member of struct hwrng to make the iomem base address
> available in meson_rng_read(). This allows for removing struct
> meson_rng_data completely in the next step.
> __force is used to silence sparse warnings.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/char/hw_random/meson-rng.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/hw_random/meson-rng.c b/drivers/char/hw_random/meson-rng.c
> index a4eb8e35f..bf7a6e594 100644
> --- a/drivers/char/hw_random/meson-rng.c
> +++ b/drivers/char/hw_random/meson-rng.c
> @@ -17,16 +17,14 @@
>  #define RNG_DATA 0x00
>  
>  struct meson_rng_data {
> -	void __iomem *base;
>  	struct hwrng rng;
>  };

This is too ugly for words.  If you're trying to save memory we
should instead get rid of rng.priv and convert the drivers that
user it over to this model of embedding struct hwrng inside the
driver struct.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 4/5] hwrng: meson: use struct hw_random priv data

[Heiner Kallweit]

On 20.02.2023 05:41, Herbert Xu wrote:
> On Sat, Feb 18, 2023 at 09:58:07PM +0100, Heiner Kallweit wrote:
>> Use the priv data member of struct hwrng to make the iomem base address
>> available in meson_rng_read(). This allows for removing struct
>> meson_rng_data completely in the next step.
>> __force is used to silence sparse warnings.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/char/hw_random/meson-rng.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/char/hw_random/meson-rng.c b/drivers/char/hw_random/meson-rng.c
>> index a4eb8e35f..bf7a6e594 100644
>> --- a/drivers/char/hw_random/meson-rng.c
>> +++ b/drivers/char/hw_random/meson-rng.c
>> @@ -17,16 +17,14 @@
>>  #define RNG_DATA 0x00
>>  
>>  struct meson_rng_data {
>> -	void __iomem *base;
>>  	struct hwrng rng;
>>  };
> 
> This is too ugly for words.  If you're trying to save memory we
> should instead get rid of rng.priv and convert the drivers that
> user it over to this model of embedding struct hwrng inside the
> driver struct.
> 
OK, then let's omit patches 4 and 5.
Patches 1-3 have a Reviewed-by, can you apply them as-is or do
you need a resubmit of the series with patch 1-3 only?

> Thanks,

Re: [PATCH 4/5] hwrng: meson: use struct hw_random priv data

[Herbert Xu]

On Mon, Feb 20, 2023 at 08:18:18AM +0100, Heiner Kallweit wrote:
>
> OK, then let's omit patches 4 and 5.
> Patches 1-3 have a Reviewed-by, can you apply them as-is or do
> you need a resubmit of the series with patch 1-3 only?

Sure, no problems.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 5/5] hwrng: meson: remove struct meson_rng_data

[Heiner Kallweit]

Because no other members of struct meson_rng_data are left,
we can remove it completely.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/char/hw_random/meson-rng.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/char/hw_random/meson-rng.c b/drivers/char/hw_random/meson-rng.c
index bf7a6e594..633d98b48 100644
--- a/drivers/char/hw_random/meson-rng.c
+++ b/drivers/char/hw_random/meson-rng.c
@@ -16,10 +16,6 @@
 
 #define RNG_DATA 0x00
 
-struct meson_rng_data {
-	struct hwrng rng;
-};
-
 static int meson_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
 {
 	void __iomem *base = (__force void __iomem *)rng->priv;
@@ -32,12 +28,12 @@ static int meson_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
 static int meson_rng_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct meson_rng_data *data;
 	struct clk *core_clk;
 	void __iomem *base;
+	struct hwrng *rng;
 
-	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
+	rng = devm_kzalloc(dev, sizeof(*rng), GFP_KERNEL);
+	if (!rng)
 		return -ENOMEM;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
@@ -49,11 +45,11 @@ static int meson_rng_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(core_clk),
 				     "Failed to get core clock\n");
 
-	data->rng.name = pdev->name;
-	data->rng.read = meson_rng_read;
-	data->rng.priv = (__force unsigned long)base;
+	rng->name = pdev->name;
+	rng->read = meson_rng_read;
+	rng->priv = (__force unsigned long)base;
 
-	return devm_hwrng_register(dev, &data->rng);
+	return devm_hwrng_register(dev, rng);
 }
 
 static const struct of_device_id meson_rng_of_match[] = {
-- 
2.39.2

Re: [PATCH 0/5] hwrng: meson: simplify driver

[Herbert Xu]

On Sat, Feb 18, 2023 at 09:54:25PM +0100, Heiner Kallweit wrote:
> This series simplifies the driver. No functional change intended.
> 
> Heiner Kallweit (5):
>   hwrng: meson: remove unused member of struct meson_rng_data
>   hwrng: meson: use devm_clk_get_optional_enabled
>   hwrng: meson: remove not needed call to platform_set_drvdata
>   hwrng: meson: use struct hw_random priv data
>   hwrng: meson: remove struct meson_rng_data
> 
>  drivers/char/hw_random/meson-rng.c | 59 +++++++++---------------------
>  1 file changed, 17 insertions(+), 42 deletions(-)
> 
> -- 
> 2.39.2

Patches 1-3 applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt