On 03/04/2015 04:58 PM, Tony Lindgren wrote:

* Paul Bolle [off-list ref] [150304 14:58]:

quoted

Nishanth Menon schreef op di 03-03-2015 om 18:00 [-0600]:

quoted

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index ee9f44ad7f02..8e463d75fbb2 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig

@@ -160,6 +160,17 @@ config PINCTRL_TEGRA_XUSB
 	select PINCONF
 	select PINMUX
 
+config PINCTRL_TI_IODELAY
+	bool "TI IODelay Module pinconf driver"

This adds a boolean Kconfig symbol.

Uggh.. thanks for the review and catching this..

quoted

quoted

+	depends on OF
+	select PINCONF
+	select GENERIC_PINCONF
+	select REGMAP_MMIO
+	help
+	  Say Y here to support Texas Instruments' IODelay pinconf driver.
+	  IODelay module is used for the DRA7 SoC family. This driver is in
+	  addition to PINCTRL_SINGLE which controls the mux.
+
 config PINCTRL_TZ1090
 	bool "Toumaz Xenif TZ1090 pin control driver"
 	depends on SOC_TZ1090

diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 0475206dd600..e441cd71aedf 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile

@@ -28,6 +28,7 @@ obj-$(CONFIG_PINCTRL_TEGRA30)	+= pinctrl-tegra30.o
 obj-$(CONFIG_PINCTRL_TEGRA114)	+= pinctrl-tegra114.o
 obj-$(CONFIG_PINCTRL_TEGRA124)	+= pinctrl-tegra124.o
 obj-$(CONFIG_PINCTRL_TEGRA_XUSB)	+= pinctrl-tegra-xusb.o
+obj-$(CONFIG_PINCTRL_TI_IODELAY)	+= pinctrl-ti-iodelay.o

So this objectfile will either be built-in or not be built at all.

quoted

 obj-$(CONFIG_PINCTRL_TZ1090)	+= pinctrl-tz1090.o
 obj-$(CONFIG_PINCTRL_TZ1090_PDC)	+= pinctrl-tz1090-pdc.o
 obj-$(CONFIG_PINCTRL_U300)	+= pinctrl-u300.o

diff --git a/drivers/pinctrl/pinctrl-ti-iodelay.c b/drivers/pinctrl/pinctrl-ti-iodelay.c
new file mode 100644
index 000000000000..e4c6e25a781c
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-ti-iodelay.c

@@ -0,0 +1,963 @@

[...]
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/list.h>
+#include <linux/module.h>

This is, I think, a bit of a red flag.

quoted

+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>

[...]

quoted

+static const struct of_device_id ti_iodelay_of_match[] = {
+	{.compatible = "ti,dra7-iodelay", .data = &dra7_iodelay_data},
+	{ /* Hopefully no more.. */ },
+};
+MODULE_DEVICE_TABLE(of, ti_iodelay_of_match);
+
+static struct platform_driver ti_iodelay_driver = {
+	.probe = ti_iodelay_probe,
+	.remove = ti_iodelay_remove,
+	.driver = {
+		   .owner = THIS_MODULE,

So THIS_MODULE will, basically, be NULL.

quoted

+		   .name = DRIVER_NAME,
+		   .of_match_table = ti_iodelay_of_match,
+		   },
+};
+module_platform_driver(ti_iodelay_driver);
+
+MODULE_AUTHOR("Texas Instruments, Inc.");
+MODULE_DESCRIPTION("Pinconf driver for TI's IO Delay module");
+MODULE_LICENSE("GPL v2");

And these three macros will be preprocessed away.

(Perhaps there are non-modular alternatives to MODULE_DEVICE_TABLE and
module_platform_driver, but I'm not sure how those two macros actually
work.)

This should be just made to work as a regular loadable module.
We already have pinctrl-single for the same platforms working
as a loadable module.

Yes, indeed. this can easily be fixed. my bad. will convert this to a
module in the next rev.

I will wait a week before reposting next rev to see if there are
improvements necessary in other areas - specifically in the way
pinconf is being used and if there are any binding related comments.

Will be great to hear any feedback on the same as well.


-- 
Regards,
Nishanth Menon