[PATCH] pinctrl: Add SPEAr pinctrl drivers
Arnd Bergmann
arnd at arndb.de
Tue Apr 3 09:47:34 EDT 2012
On Tuesday 03 April 2012, Viresh Kumar wrote:
> This adds pinctrl driver for SPEAr family. Currently it contains machine/SoC
> drivers for SPEAr3xx family only. SPEAr13xx family drivers will follow.
>
> Signed-off-by: Viresh Kumar <viresh.kumar at st.com>
Hi Viresh,
This is quite a lot of data, especially for the spear320. Have you
tried moving some or all of the data into device tree properties?
I can only guess that the spear13xx file would be even larger than
these.
> +
> +static struct of_device_id spear_pinctrl_of_match[] __devinitdata = {
> +#ifdef CONFIG_PINCTRL_SPEAR300
> + {
> + .compatible = "st,spear300-pinmux",
> + .data = spear300_mach_init,
> + },
> +#endif
> +#ifdef CONFIG_PINCTRL_SPEAR310
> + {
> + .compatible = "st,spear310-pinmux",
> + .data = spear310_mach_init,
> + },
> +#endif
> +#ifdef CONFIG_PINCTRL_SPEAR320
> + {
> + .compatible = "st,spear320-pinmux",
> + .data = spear320_mach_init,
> + },
> +#endif
> + {},
> +};
I would recommend turning the logic around here: instead of having
a common driver that calls into the soc specific drivers, make the
common code a library that just exports a few symbols, and move
each of the socs into its own module with one init function
that calls the generic spear_pinctrl_probe, passing the appropriate
arguments on the code that that differs.
> +static struct platform_driver spear_pinctrl_driver = {
> + .driver = {
> + .name = DRIVER_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = spear_pinctrl_of_match,
> + },
> + .probe = spear_pinctrl_probe,
> + .remove = __devexit_p(spear_pinctrl_remove),
> +};
> +
> +static int __init spear_pinctrl_init(void)
> +{
> + return platform_driver_register(&spear_pinctrl_driver);
> +}
> +arch_initcall(spear_pinctrl_init);
> +
> +static void __exit spear_pinctrl_exit(void)
> +{
> + platform_driver_unregister(&spear_pinctrl_driver);
> +}
> +module_exit(spear_pinctrl_exit);
Does this need to be an arch_initcall? If it becomes a regular
module_init(), you can condense all of this into a single
line using module_platform_driver(), one per soc if you do as
I suggest above.
The initialization order dependencies can now be handled using
the deferred probe mechanism, by returning -EPROBE_DEFER from
the probe() function of any driver that is loaded before its
pins are available.
> +static struct spear_pmx_mode pmx_mode_nand = {
> + .name = "nand",
> + .mode = NAND_MODE,
> + .reg = MODE_CONFIG_REG,
> + .mask = 0x0000000F,
> + .val = 0x00,
> +};
> +
> +static struct spear_pmx_mode pmx_mode_nor = {
> + .name = "nor",
> + .mode = NOR_MODE,
> + .reg = MODE_CONFIG_REG,
> + .mask = 0x0000000F,
> + .val = 0x01,
> +};
> +
> +static struct spear_pmx_mode pmx_mode_photo_frame = {
> + .name = "photo frame mode",
> + .mode = PHOTO_FRAME_MODE,
> + .reg = MODE_CONFIG_REG,
> + .mask = 0x0000000F,
> + .val = 0x02,
> +};
These all look like they can easily get transformed into
device nodes in the device tree.
> +/* pingroups */
> +static struct spear_pingroup *spear310_pingroups[] = {
> + SPEAR3XX_COMMON_PINGROUPS,
> + &emi_cs_0_to_5_pingroup,
> + &uart1_pingroup,
> + &uart2_pingroup,
> + &uart3_pingroup,
> + &uart4_pingroup,
> + &uart5_pingroup,
> + &fsmc_pingroup,
> + &rs485_0_pingroup,
> + &rs485_1_pingroup,
> + &tdm_pingroup,
> +};
> +
> +/* functions */
> +static struct spear_function *spear310_functions[] = {
> + SPEAR3XX_COMMON_FUNCTIONS,
> + &emi_cs_0_to_5_function,
> + &uart1_function,
> + &uart2_function,
> + &uart3_function,
> + &uart4_function,
> + &uart5_function,
> + &fsmc_function,
> + &rs485_0_function,
> + &rs485_1_function,
> + &tdm_function,
> +};
I believe the macros like SPEAR3XX_COMMON_FUNCTIONS are not
needed any more, you can simply register multiple sets of pingroups
and functions.
Arnd
More information about the linux-arm-kernel
mailing list