Re: [PATCH] XCEP PXA255 Processor Board

[Eric Miao]

On Mon, May 11, 2009 at 5:04 PM, Michael Abbott <michael@xxxxxxxxxxxxxxx> wrote:
> This is an update of a new architecture patch submitted last week.
> Although this is mailed to the linux-arm-kernel mailing list, my
> subscription seems to be stuck pending moderator approval, so this won't
> actually make it to the list for a bit. ÂIf anybody on that list fancys
> forwarding this mail, that might be helpful.
>
> The main change to this version of the patch is that I've now eliminated
> the obsolescent pxa gpio interface in favour of pxa2xx_mfp_config. ÂI've
> also added a suitable _defconfig file.
>
> I've also rebased the patch onto current git (at least, as of last
> Friday).
>
> Please cc any replies to me.
>
>
> Date: Wed, 18 Mar 2009 13:42:08 +0000
> Subject: [PATCH] XCEP PXA255 Processor Board
>
> Adds support for the XCEP processor from IskraTel. ÂThis is a small
> single board computer with an ARM XScale PXA255 processor, 32M flash,
> 64M RAM and an SMSC LAN91C111 network device.
>
> This board is used in a series of electron beam position monitors used
> in synchrotron light sources world wide.
>
> Signed-off-by: Michael Abbott <michael.abbott@xxxxxxxxxxxxx>
> ---
> Âarch/arm/configs/xcep_defconfig | 1089 +++++++++++++++++++++++++++++++++++++++

Please, separate the _defconfig into another patch. This makes
code review difficult. (And relevant content removed from this mail)

> +
> +/* SMC LAN91C111 network controller. */
> +
> +static struct resource smc91x_resources[] = {
> + Â Â Â [0] = {
> +        .name  = "smc91x-regs",
> + Â Â Â Â Â Â Â .start Â= XCEP_ETH_PHYS,
> +        .end  Â= XCEP_ETH_END,
> + Â Â Â Â Â Â Â .flags Â= IORESOURCE_MEM,
> + Â Â Â },
> + Â Â Â [1] = {
> + Â Â Â Â Â Â Â .start Â= IRQ_GPIO(XCEP_ETH_IRQ),
> +        .end  Â= IRQ_GPIO(XCEP_ETH_IRQ),
> + Â Â Â Â Â Â Â .flags Â= IORESOURCE_IRQ,

I'd suggest gpio_to_irq(XCEP_ETH_GPIO) - thus defining XCEP_ETH_GPIO
instead of XCEP_ETH_IRQ, which is confusing.

> + Â Â Â },
> + Â Â Â [2] = {
> +        .name  = "smc91x-attrib",
> + Â Â Â Â Â Â Â .start Â= 0x0e000000,
> +        .end  Â= 0x0e0fffff,
> + Â Â Â Â Â Â Â .flags Â= IORESOURCE_MEM,
> + Â Â Â },
> +};
> +
> +static struct platform_device smc91x_device = {
> +    .name      = "smc91x",
> +    .id       = -1,
> + Â Â Â .num_resources Â= ARRAY_SIZE(smc91x_resources),
> +    .resource    = smc91x_resources,
> +};
> +
> +
> +static struct map_desc xcep_io_desc[] __initdata = {
> + Â Â Â { Â Â Â /* CPLD */
> +        .virtual    Â= XCEP_CPLD_BASE,
> +        .pfn      Â= __phys_to_pfn(0x10000000),

You may want a PXA_CSx_... macro here for this magic number.

> +        .length     = 0x00000100,
> +        .type      = MT_DEVICE
> + Â Â Â }
> +};
> +
> +
> +static struct platform_device *devices[] __initdata = {
> + Â Â Â &flash_device,
> + Â Â Â &smc91x_device,
> +};
> +
> +
> +/* We have to state that there are HWMON devices on the I2C bus on XCEP.
> + * Drivers for HWMON verify capabilities of the adapter when loading and
> + * refuse to attach if the adapter doesn't support HWMON class of devices.
> + * See also Documentation/i2c/porting-clients. */
> +static struct i2c_pxa_platform_data xcep_i2c_platform_data Â= {
> + Â Â Â .class = I2C_CLASS_HWMON
> +};
> +
> +
> +static mfp_cfg_t xcep_pin_config[] __initdata = {
> + Â ÂGPIO79_nCS_3 Â Â Â /* SMC 91C111 chip select. */

I'm seeing the CPLD connected to one of the chip-select, which you
may also add it here?

> +};
> +
> +
> +static void __init xcep_init(void)
> +{
> + Â Â Â /* See Intel XScale Developer's Guide for details */
> + Â Â Â /* Set RDF and RDN to appropriate values (chip select 3 (smc91x)) */
> + Â Â Â MSC1 = (MSC1 & 0xffff) | 0xD5540000;
> + Â Â Â /* Set RDF and RDN to appropriate values (chip select 5 (fpga)) */
> + Â Â Â MSC2 = (MSC2 & 0xffff) | 0x72A00000;
> +
> + Â Â Â platform_add_devices(ARRAY_AND_SIZE(devices));
> + Â Â Â pxa_set_i2c_info(&xcep_i2c_platform_data);
> +}
> +
> +static void __init xcep_map_io(void)
> +{
> + Â Â Â pxa_map_io();
> + Â Â Â iotable_init(ARRAY_AND_SIZE(xcep_io_desc));
> + Â Â Â pxa2xx_mfp_config(ARRAY_AND_SIZE(xcep_pin_config));

I'm not sure if the MFP needs to be initialized this early, moving this
to xcep_init() would be better, at least to align with other platforms
from my POV.

> +}
> +
> +MACHINE_START(XCEP, "Iskratel XCEP")
> +    .phys_io    Â= 0x80000000,
> +    .io_pg_offst  Â= 0xf8000000,

...... you sure this is correct? Please have a reference to other platforms.

> +    .boot_params  Â= 0xa0000100,
> +    .init_machine  = xcep_init,
> +    .map_io     = xcep_map_io,
> +    .init_irq    = pxa25x_init_irq,
> +    .timer     Â= &pxa_timer,
> +MACHINE_END
> diff --git a/drivers/net/smc91x.h b/drivers/net/smc91x.h
> index 329f890..a9d20bc 100644
> --- a/drivers/net/smc91x.h
> +++ b/drivers/net/smc91x.h
> @@ -400,6 +400,11 @@ static inline void LPD7_SMC_outsw (unsigned char* a, int r,
>
> Â#endif
>
> +#if defined(CONFIG_MACH_XCEP)
> +#define SMC_USE_PXA_DMA Â Â Â Â Â Â Â Â1
> +#endif
> +

OK, this should be now encoded into the smc91x_platfdata, see
zylonite.c or other boards for example.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/