Hi Linus, Looks pretty good. just some minors.
2011/8/19 Linus Walleij linus.walleij@stericsson.com:
From: Linus Walleij linus.walleij@linaro.org
This adds a driver for the U300 pinmux portions of the system controller "SYSCON". It also serves as an example of how to use the pinmux subsystem. This driver also houses the platform data for the only supported platform.
yes. it is a good example for us to follow.
Signed-off-by: Linus Walleij linus.walleij@linaro.org
+static int __exit u300_pmx_remove(struct platform_device *pdev) +{
- struct u300_pmx *upmx = platform_get_drvdata(pdev);
- if (upmx) {
why do you need to check whether upmx is not NULL. if it is NULL, we have failed in probe(). and it is also impossible to call this remove() twice.
- pinmux_unregister(upmx->pmx);
- iounmap(upmx->virtbase);
- release_mem_region(upmx->phybase, upmx->physize);
- platform_set_drvdata(pdev, NULL);
- kfree(upmx);
- }
- return 0;
+}
+static struct platform_driver u300_pmx_driver = {
- .driver = {
- .name = DRIVER_NAME,
- .owner = THIS_MODULE,
- },
- .remove = __exit_p(u300_pmx_remove),
if you want to add deepsleep support(i mean pinmux crl will lose power while suspending), what do you plan to add? save U300_SYSCON_PMC1LR, U300_SYSCON_PMC1HR, U300_SYSCON_PMC2R, U300_SYSCON_PMC3R, U300_SYSCON_PMC4R in suspend and restore them in resume?
it is also a generic issue to pinmux.
+/*
- Register definitions for the U300 Padmux control registers in the
- system controller
- */
+/* PAD MUX Control register 1 (LOW) 16bit (R/W) */ +#define U300_SYSCON_PMC1LR (0x007C)
...
+#define U300_SYSCON_PMC4R_APP_MISC_16_EMIF_1_STATIC_CS5_N (0x0200)
do you want these "(" ")" for macros?
Thanks barry