From: arnd@arndb•de (Arnd Bergmann)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] pinctrl: Add SPEAr pinctrl drivers
Date: Tue, 3 Apr 2012 13:47:34 +0000 [thread overview]
Message-ID: <201204031347.35256.arnd@arndb.de> (raw)
In-Reply-To: <a1b7f612544a09989cec80a0318f35b6e48eb1d3.1333453148.git.viresh.kumar@st.com>
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@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
next parent reply other threads:[~2012-04-03 13:47 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <a1b7f612544a09989cec80a0318f35b6e48eb1d3.1333453148.git.viresh.kumar@st.com>
2012-04-03 13:47 ` Arnd Bergmann [this message]
2012-04-03 14:10 ` [PATCH] pinctrl: Add SPEAr pinctrl drivers viresh kumar
2012-04-03 17:09 ` viresh kumar
2012-04-03 19:33 ` Arnd Bergmann
2012-04-04 4:14 ` Viresh Kumar
2012-04-03 16:19 ` Stephen Warren
2012-04-03 19:37 ` Arnd Bergmann
2012-04-03 21:24 ` Linus Walleij
2012-04-03 22:43 ` Stephen Warren
2012-04-10 7:54 ` Linus Walleij
2012-04-04 7:45 ` Arnd Bergmann
2012-04-03 21:18 ` Linus Walleij
2012-04-04 4:17 ` Viresh Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201204031347.35256.arnd@arndb.de \
--to=arnd@arndb$(echo .)de \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox