Re: [PATCH 2/2] pinctrl: Introduce TI IOdelay configuration driver

Tony Lindgren
2015-03-04T23:04:26Z

* Paul Bolle <pebolle@tiscali.nl> [150304 14:58]:
> Nishanth Menon schreef op di 03-03-2015 om 18:00 [-0600]:
> > 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.
>
> > + 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.
>
> > 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.
>
> > +#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>
>
> [...]
>
> > +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.
>
> > + .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.

Regards,

Tony