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

login
register
mail settings
Submitter Vladimir Zapolskiy
Date March 5, 2018, 10:21 p.m.
Message ID <20180305222100.29351-3-vz@mleia.com>
Download mbox | patch
Permalink /patch/10260027/
State Accepted
Delegated to: Herbert Xu
Headers show

Comments

Vladimir Zapolskiy - March 5, 2018, 10:21 p.m.
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
indentation in platform driver struct are beautified a little.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
* a kernel for iMX boards is always built with multiplatform support,
  thus CONFIG_OF guards were removed, thanks to Kim Phillips for review,
* added "fsl,imx21-rnga" compatible to the list.

 drivers/char/hw_random/mxc-rnga.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
Fabio Estevam - March 5, 2018, 10:24 p.m.
Hi Vladimir,

On Mon, Mar 5, 2018 at 7:21 PM, Vladimir Zapolskiy <vz@mleia.com> wrote:
> 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
> indentation in platform driver struct are beautified a little.

Patch looks good.

I have an off-topic question though :-)

Do you have mx31 device tree related patches so that you can test this?

Do we have mx31 pinctrl support?

Please advise.
Vladimir Zapolskiy - March 5, 2018, 11:44 p.m.
Hi Fabio,

On 03/06/2018 12:24 AM, Fabio Estevam wrote:
> Hi Vladimir,
> 
> On Mon, Mar 5, 2018 at 7:21 PM, Vladimir Zapolskiy <vz@mleia.com> wrote:
>> 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
>> indentation in platform driver struct are beautified a little.
> 
> Patch looks good.
> 

you are welcome to ack :)

> I have an off-topic question though :-)
> 
> Do you have mx31 device tree related patches so that you can test this?

Yes, I have a pretty good i.MX31 dtsi change (tested everything but USB
and multimedia, and that notorious watchdog problem still has to be
agreed with Uwe and solved), but I'm trying to save my time a little, and
my plan is to send everything altogether as a single imx31.dtsi update
(without instant pinctrl though). And basically RNGA support is the only
missing part at the moment.

> Do we have mx31 pinctrl support?

Yes, I have a complete and well tested i.MX31 pinctrl driver, it is done
similarly to all known Freescale pin controller drivers.

I sort of dislike it, and my preference is to "upgrade" it to utilize
generic pin multiplexing with groups, functions and pin configuration
as it is described in pinctrl-bindings.txt, then hopefully some ideas
or code can be migrated to support modern Freescale/NXP SoCs. Legacy
i.MX31 could serve as a distant and convenient test site in my opinion.

> Please advise.
> 

--
With best wishes,
Vladimir
Fabio Estevam - March 5, 2018, 11:54 p.m.
On Mon, Mar 5, 2018 at 7:21 PM, Vladimir Zapolskiy <vz@mleia.com> wrote:
> 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
> indentation in platform driver struct are beautified a little.
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Fabio Estevam - March 5, 2018, 11:58 p.m.
Hi Vladimir,

On Mon, Mar 5, 2018 at 8:44 PM, Vladimir Zapolskiy <vz@mleia.com> wrote:

> Yes, I have a pretty good i.MX31 dtsi change (tested everything but USB
> and multimedia, and that notorious watchdog problem still has to be
> agreed with Uwe and solved), but I'm trying to save my time a little, and
> my plan is to send everything altogether as a single imx31.dtsi update
> (without instant pinctrl though). And basically RNGA support is the only
> missing part at the moment.

Ok, that's good news.

> Yes, I have a complete and well tested i.MX31 pinctrl driver, it is done
> similarly to all known Freescale pin controller drivers.

It would be nice if you could post it even in the current form.

> I sort of dislike it, and my preference is to "upgrade" it to utilize
> generic pin multiplexing with groups, functions and pin configuration
> as it is described in pinctrl-bindings.txt, then hopefully some ideas

On i.MX8M there was a recent discussion about using the new generic
pinctl versus the old method where we encode the pinmux value in dts.

i.MX maintainers are in favor of the old method, so that we can have
this same scheme for all i.MX variants.

Well, that is a topic for another mailing list :-)

Thanks
Kim Phillips - March 7, 2018, 4:13 p.m.
On Tue, 6 Mar 2018 00:21:00 +0200
Vladimir Zapolskiy <vz@mleia.com> wrote:

> 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
> indentation in platform driver struct are beautified a little.
> 
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
> Changes from v1 to v2:
> * a kernel for iMX boards is always built with multiplatform support,
>   thus CONFIG_OF guards were removed, thanks to Kim Phillips for review,

That's not necessarily the reason, e.g., of_match_table is available to
be assigned even if CONFIG_OF is not set.  Recall, I tested building
without CONFIG_OF by removing the SOC_IMX31 dependency in Kconfig, and
building with netwinder_defconfig as a base.

Nevertheless, this v2 is much easier to review without the ifdef
CONFIG_OF, so:

Reviewed-by: Kim Phillips <kim.phillips@arm.com>

Thanks,

Kim

p.s. my responses typically have lower latencies when I'm added to cc.

Patch

diff --git a/drivers/char/hw_random/mxc-rnga.c b/drivers/char/hw_random/mxc-rnga.c
index 4673622..f83bee5 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,18 @@  static int __exit mxc_rnga_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id mxc_rnga_of_match[] = {
+	{ .compatible = "fsl,imx21-rnga", },
+	{ .compatible = "fsl,imx31-rnga", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
+
 static struct platform_driver mxc_rnga_driver = {
 	.driver = {
-		   .name = "mxc_rnga",
-		   },
+		.name = "mxc_rnga",
+		.of_match_table = mxc_rnga_of_match,
+	},
 	.remove = __exit_p(mxc_rnga_remove),
 };