[PATCH 0/2] hwrng: mxc-rnga - add driver support on boards with device tree

[PATCH 0/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Vladimir Zapolskiy]

The series is a trivial change to extend Freescale i.MX31 RNGA
driver to work on boards with device description taken from device
tree blob. The change was tested on a legacy LogicPD MX31Lite board.

Vladimir Zapolskiy (2):
  dt-bindings: rng: Document Freescale i.MX31 RNGA compatible
  hwrng: mxc-rnga - add driver support on boards with device tree

 .../bindings/rng/{imx-rngc.txt => imx-rng.txt}     | 10 ++++-----
 drivers/char/hw_random/mxc-rnga.c                  | 24 ++++++++++++++--------
 2 files changed, 19 insertions(+), 15 deletions(-)
 rename Documentation/devicetree/bindings/rng/{imx-rngc.txt => imx-rng.txt} (59%)

-- 
2.10.2

[PATCH 1/2] dt-bindings: rng: Document Freescale i.MX31 RNGA compatible

[Vladimir Zapolskiy]

Freescale i.MX31 SoC contains a Security Random Number Generator
Accelerator module (RNGA), which is replaced by RNGB and RNGC modules
on later i.MX SoC series, the change adds a new compatible property
to describe the controller.

Since all versions of Freescale RNG modules are legacy, apparently
the documentation file has no more potential for further extensions,
nevertheless generalize it by removing explicit RNGC specifics.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 .../devicetree/bindings/rng/{imx-rngc.txt => imx-rng.txt}      | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
 rename Documentation/devicetree/bindings/rng/{imx-rngc.txt => imx-rng.txt} (59%)

diff --git a/Documentation/devicetree/bindings/rng/imx-rngc.txt b/Documentation/devicetree/bindings/rng/imx-rng.txt
similarity index 59%
rename from Documentation/devicetree/bindings/rng/imx-rngc.txt
rename to Documentation/devicetree/bindings/rng/imx-rng.txt
index 93c7174..fc10933 100644
--- a/Documentation/devicetree/bindings/rng/imx-rngc.txt
+++ b/Documentation/devicetree/bindings/rng/imx-rng.txt
@@ -1,15 +1,13 @@
-Freescale RNGC (Random Number Generator Version C)
-
-The driver also supports version B, which is mostly compatible
-to version C.
+Freescale RNGA/RNGB/RNGC (Random Number Generator Versions A, B and C)
 
 Required properties:
 - compatible : should be one of
+               "fsl,imx31-rnga"
                "fsl,imx25-rngb"
                "fsl,imx35-rngc"
 - reg : offset and length of the register set of this block
-- interrupts : the interrupt number for the RNGC block
-- clocks : the RNGC clk source
+- interrupts : the interrupt number for the RNG block
+- clocks : the RNG clk source
 
 Example:
 
-- 
2.10.2

Re: [PATCH 1/2] dt-bindings: rng: Document Freescale i.MX31 RNGA compatible

[Rob Herring]

On Mon, Feb 26, 2018 at 08:38:48PM +0200, Vladimir Zapolskiy wrote:
> Freescale i.MX31 SoC contains a Security Random Number Generator
> Accelerator module (RNGA), which is replaced by RNGB and RNGC modules
> on later i.MX SoC series, the change adds a new compatible property
> to describe the controller.
> 
> Since all versions of Freescale RNG modules are legacy, apparently
> the documentation file has no more potential for further extensions,
> nevertheless generalize it by removing explicit RNGC specifics.
> 
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
>  .../devicetree/bindings/rng/{imx-rngc.txt => imx-rng.txt}      | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>  rename Documentation/devicetree/bindings/rng/{imx-rngc.txt => imx-rng.txt} (59%)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Vladimir Zapolskiy]

The driver works well on i.MX31 powered boards with device description
taken from board device tree, the only change to add to the driver is
the missing OF device id, the affected list of included headers and
platform driver struct are beautified a little.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/char/hw_random/mxc-rnga.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/char/hw_random/mxc-rnga.c b/drivers/char/hw_random/mxc-rnga.c
index 4673622..b749e00 100644
--- a/drivers/char/hw_random/mxc-rnga.c
+++ b/drivers/char/hw_random/mxc-rnga.c
@@ -16,16 +16,13 @@
  * This driver is based on other RNG drivers.
  */
 
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/kernel.h>
 #include <linux/clk.h>
-#include <linux/err.h>
-#include <linux/ioport.h>
-#include <linux/platform_device.h>
-#include <linux/hw_random.h>
 #include <linux/delay.h>
+#include <linux/hw_random.h>
 #include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
 
 /* RNGA Registers */
 #define RNGA_CONTROL			0x00
@@ -197,10 +194,19 @@ static int __exit mxc_rnga_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id mxc_rnga_of_match[] = {
+	{ .compatible = "fsl,imx31-rnga", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
+#endif
+
 static struct platform_driver mxc_rnga_driver = {
 	.driver = {
-		   .name = "mxc_rnga",
-		   },
+		.name = "mxc_rnga",
+		.of_match_table = of_match_ptr(mxc_rnga_of_match),
+	},
 	.remove = __exit_p(mxc_rnga_remove),
 };
 
-- 
2.10.2

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Kim Phillips]

On Mon, 26 Feb 2018 20:38:49 +0200
Vladimir Zapolskiy <vz@mleia.com> wrote:

> +#ifdef CONFIG_OF
> +static const struct of_device_id mxc_rnga_of_match[] = {
> +	{ .compatible = "fsl,imx31-rnga", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
> +#endif
> +
>  static struct platform_driver mxc_rnga_driver = {
>  	.driver = {
> -		   .name = "mxc_rnga",
> -		   },
> +		.name = "mxc_rnga",
> +		.of_match_table = of_match_ptr(mxc_rnga_of_match),

Does this build if CONFIG_OF is not set?

Thanks,

Kim

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Vladimir Zapolskiy]

On 02/27/2018 05:49 PM, Kim Phillips wrote:
> On Mon, 26 Feb 2018 20:38:49 +0200
> Vladimir Zapolskiy <vz@mleia.com> wrote:
> 
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id mxc_rnga_of_match[] = {
>> +	{ .compatible = "fsl,imx31-rnga", },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
>> +#endif
>> +
>>  static struct platform_driver mxc_rnga_driver = {
>>  	.driver = {
>> -		   .name = "mxc_rnga",
>> -		   },
>> +		.name = "mxc_rnga",
>> +		.of_match_table = of_match_ptr(mxc_rnga_of_match),
> 
> Does this build if CONFIG_OF is not set?
> 

Definitely it is expected to be built, you can verify it directly or
check of_match_ptr() macro definition from include/linux/of.h

--
With best wishes,
Vladimir

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Kim Phillips]

On Tue, 27 Feb 2018 18:53:08 +0200
Vladimir Zapolskiy <vz@mleia.com> wrote:

> On 02/27/2018 05:49 PM, Kim Phillips wrote:
> > On Mon, 26 Feb 2018 20:38:49 +0200
> > Vladimir Zapolskiy <vz@mleia.com> wrote:
> > 
> >> +#ifdef CONFIG_OF
> >> +static const struct of_device_id mxc_rnga_of_match[] = {
> >> +	{ .compatible = "fsl,imx31-rnga", },
> >> +	{ /* sentinel */ },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
> >> +#endif
> >> +
> >>  static struct platform_driver mxc_rnga_driver = {
> >>  	.driver = {
> >> -		   .name = "mxc_rnga",
> >> -		   },
> >> +		.name = "mxc_rnga",
> >> +		.of_match_table = of_match_ptr(mxc_rnga_of_match),
> > 
> > Does this build if CONFIG_OF is not set?
> 
> Definitely it is expected to be built, you can verify it directly or
> check of_match_ptr() macro definition from include/linux/of.h

Thanks, I verified it by removing the SOC_IMX31 dependency, and with
netwinder_defconfig as a base.  I also verified that the #ifdef
CONFIG_OF protecting the mxc_rnga_of_match definition is also not
needed.

Kim

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Vladimir Zapolskiy]

On 02/27/2018 09:39 PM, Kim Phillips wrote:
> On Tue, 27 Feb 2018 18:53:08 +0200
> Vladimir Zapolskiy <vz@mleia.com> wrote:
> 
>> On 02/27/2018 05:49 PM, Kim Phillips wrote:
>>> On Mon, 26 Feb 2018 20:38:49 +0200
>>> Vladimir Zapolskiy <vz@mleia.com> wrote:
>>>
>>>> +#ifdef CONFIG_OF
>>>> +static const struct of_device_id mxc_rnga_of_match[] = {
>>>> +	{ .compatible = "fsl,imx31-rnga", },
>>>> +	{ /* sentinel */ },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
>>>> +#endif
>>>> +
>>>>  static struct platform_driver mxc_rnga_driver = {
>>>>  	.driver = {
>>>> -		   .name = "mxc_rnga",
>>>> -		   },
>>>> +		.name = "mxc_rnga",
>>>> +		.of_match_table = of_match_ptr(mxc_rnga_of_match),
>>>
>>> Does this build if CONFIG_OF is not set?
>>
>> Definitely it is expected to be built, you can verify it directly or
>> check of_match_ptr() macro definition from include/linux/of.h
> 
> Thanks, I verified it by removing the SOC_IMX31 dependency, and with
> netwinder_defconfig as a base.  I also verified that the #ifdef
> CONFIG_OF protecting the mxc_rnga_of_match definition is also not
> needed.

That's a commonplace observation, but I have serious doubts, if it
has become a common practice to remove CONFIG_OF and CONFIG_ACPI
macro guards around device id lists. Still I would prefer to save
compiled code size.

Arnd or Greg, your valued opinion is much appreciated.

--
With best wishes,
Vladimir

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Vladimir Zapolskiy]

On 02/27/2018 10:07 PM, Vladimir Zapolskiy wrote:
> On 02/27/2018 09:39 PM, Kim Phillips wrote:
>> On Tue, 27 Feb 2018 18:53:08 +0200
>> Vladimir Zapolskiy <vz@mleia.com> wrote:
>>
>>> On 02/27/2018 05:49 PM, Kim Phillips wrote:
>>>> On Mon, 26 Feb 2018 20:38:49 +0200
>>>> Vladimir Zapolskiy <vz@mleia.com> wrote:
>>>>
>>>>> +#ifdef CONFIG_OF
>>>>> +static const struct of_device_id mxc_rnga_of_match[] = {
>>>>> +	{ .compatible = "fsl,imx31-rnga", },
>>>>> +	{ /* sentinel */ },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
>>>>> +#endif
>>>>> +
>>>>>  static struct platform_driver mxc_rnga_driver = {
>>>>>  	.driver = {
>>>>> -		   .name = "mxc_rnga",
>>>>> -		   },
>>>>> +		.name = "mxc_rnga",
>>>>> +		.of_match_table = of_match_ptr(mxc_rnga_of_match),
>>>>
>>>> Does this build if CONFIG_OF is not set?
>>>
>>> Definitely it is expected to be built, you can verify it directly or
>>> check of_match_ptr() macro definition from include/linux/of.h
>>
>> Thanks, I verified it by removing the SOC_IMX31 dependency, and with
>> netwinder_defconfig as a base.  I also verified that the #ifdef
>> CONFIG_OF protecting the mxc_rnga_of_match definition is also not
>> needed.
> 
> That's a commonplace observation, but I have serious doubts, if it
> has become a common practice to remove CONFIG_OF and CONFIG_ACPI
> macro guards around device id lists. Still I would prefer to save
> compiled code size.
> 

I checked that all flavours of i.MX SoCs are under multiplatform build.
FWIW only 1 iMX31 board has DT support and 10 of them use platform
data, and I'd like to change the ratio. So it would be proper to
remove the set CONFIG_OF guard.

But what is significantly more important is that i.MX31 RNGA should
be defined as compatible with i.MX21 RNGA, and it obligates me to send
v2 with the corrected compatible name, I'll make both changes.

Thanks Kim for attracting my attention to possible improvements.

--
With best wishes,
Vladimir