Re: [PATCH 4/9] Migrate ML300 reference design to the platform bus

[Andrei Konovalov <akonovalov@ru.mvista.com>]

Hi Grant,

Grant C. Likely wrote:
> Signed-off-by: Grant C. Likely <grant.likely@secretlab.ca>
> 
> ---
> 
>  arch/ppc/Kconfig.debug                |    2 -
>  arch/ppc/platforms/4xx/xilinx_ml300.c |   74 +++++++++++++++++++++++----------
>  arch/ppc/platforms/4xx/xilinx_ml300.h |    2 -
>  arch/ppc/syslib/Makefile              |    2 -
>  4 files changed, 55 insertions(+), 25 deletions(-)

<snip>

> +/* Board specifications structures */
> +struct ppc_sys_spec *cur_ppc_sys_spec;
> +struct ppc_sys_spec ppc_sys_specs[] = {
> +	{
> +		/* Only one entry, always assume the same design */
> +		.ppc_sys_name	= "Xilinx ML300 Reference Design",
> +		.mask 		= 0x00000000,
> +		.value 		= 0x00000000,

"Always assume the same design" could be a considerable limitation
for the Virtex FPGAs.

<snip>

> @@ -131,6 +159,8 @@ platform_init(unsigned long r3, unsigned
>  {
>  	ppc4xx_init(r3, r4, r5, r6, r7);
>  
> +	identify_ppc_sys_by_id(mfspr(SPRN_PVR));

This is OK for the single ppc_sys_specs[] case, but in general
I am not sure using PVR to identify the system makes much sense
in case of Virtex FPGAs. IIRC, for the mpc8xxx Freescale SOCs PVR
gives enough information to determine what on-chip peripherals are
present (but not how multi-function peripherals like SCC are configured).
In case of Xilinx the situation is worse: depending on the bitstream
loaded into the FPGA, the same chip (the same PVR) and the board
can have several sets of on-chip peripherals which could be completely
different from each other. And knowing the PVR value alone puts no limitation
on what peripherals could get into the FPGA (unless e.g. a Virtex-4 specific
hardware block is used by an IP - like in case of TEMAC).


What do you think?


So far I've used a fairly dumb code like:

   static int __init xilinx_platform_init(void)
   {
   #ifdef XPAR_EMAC_0_BASEADDR
         memcpy(xemac_0_pdata.mac_addr, __res.bi_enetaddr, 6);
         platform_device_register(&xilinx_emac_0_device);
   #endif /* XPAR_EMAC_0_BASEADDR */

   #ifdef XPAR_TEMAC_0_BASEADDR
         memcpy(xtemac_0_pdata.mac_addr, __res.bi_enetaddr, 6);
         platform_device_register(&xilinx_temac_0_device);
   #endif /* XPAR_TEMAC_0_BASEADDR */

   #ifdef XPAR_TFT_0_BASEADDR
         platform_device_register(&xilinx_lcd_0_device);
   #endif /* XPAR_TFT_0_BASEADDR */

   #ifdef XPAR_GPIO_0_BASEADDR
         platform_device_register(&xilinx_gpio_0_device);
   #endif /* XPAR_GPIO_0_BASEADDR */
   #ifdef XPAR_GPIO_1_BASEADDR
         platform_device_register(&xilinx_gpio_1_device);
   #endif /* XPAR_GPIO_1_BASEADDR */

   #ifdef XPAR_PS2_0_BASEADDR
         platform_device_register(&xilinx_ps2_0_device);
   #endif /* XPAR_PS2_0_BASEADDR */
   #ifdef XPAR_PS2_1_BASEADDR
         platform_device_register(&xilinx_ps2_1_device);
   #endif /* XPAR_PS2_1_BASEADDR */

	return 0;
   }

- to associate the devices to the drivers (the drivers
call driver_register() from their module_init function).
This helps me holding all that ugly stuff in one file
(this single file can be used by multiple boards),
and should make it easier to switch to something using
the flattened device tree parsing when the OF DT comes
into play.

Probably using ppc_sys infrastructure now has some advantages,
but they are not evident to me.


Thanks,
Andrei