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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/