[PATCH 1/7] pinctrl: enable pxa3xx pinmux

[Haojian Zhuang]

On Mon, Nov 28, 2011 at 9:17 PM, Linus Walleij <linus.walleij at linaro.org> wrote:
> On Mon, Nov 28, 2011 at 10:50 AM, Haojian Zhuang <hzhuang1 at marvell.com> wrote:
>
>> The only block issue at here is that cpu_is_pxa3xx() is defined in arch-pxa and cpu_is_pxa910()
>> is defined in arch-mmp. If I only enable arch-pxa, I would meet build error for undefined
>> cpu_is_pxa910().
>
> Please #ifdef and cpu_is_* in arch/arm/mach-* then pass a flag or enumerator
> with the platform data to the driver. This way the platform data uniformly
> tells the driver what machine/cpu it is, and the driver need not hold any
> knowledge of the system or use machine-specific functions.
>
> I don't understand where your cpu_is* functions come from anyway,
> which of these includes:
>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/gpio.h>
> +#include <linux/io.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/platform_device.h>
>
> Actually provides it?
>
> Please include that file explicitly and I'll tell you to
> not include it :-D
>
> Since this driver will be used by multiple platforms, why not
> create
> include/linux/pinctrl/pxa3xx.h and define your platform data
> struct there like:
>
> #include <linux/pinctrl/machine.h>
>
> /* Unless an enumerator or defines like this exist already */
> enum pxa_cpu_type {
>   PXA_FOO,
>   PXA_BAR,
>   PXA_FNORD,
> };
>
> struct pxa3xx_pinmux_plat {
>    enum pxa_cpu_type cputype;
>    ...other interesting stuff ...
> };
>
> Fill it in in your machine and pick it up in your
> driver.
>
> Yours,
> Linus Walleij
>

How about define cpu_is_pxa3xx() in both arch-pxa and arch-mmp? Maybe
it could be simpler. Since we have similar issue in gpio driver too.

Thanks
Haojian