[maverick][PATCH] UBUNTU: SAUCE: Load SysLink modules on start-up on OMAP4 devices

[Dechesne, Nicolas]

Lee,

Thanks for your patch. I have some comments below.

>
Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920

-----Original Message-----

> From: kernel-team-bounces at lists.ubuntu.com [mailto:kernel-team-
> bounces at lists.ubuntu.com] On Behalf Of Lee Jones
> Sent: Wednesday, July 07, 2010 11:08 AM
> To: kernel-team
> Subject: [maverick][PATCH] UBUNTU: SAUCE: Load SysLink modules on start-up
> on OMAP4 devices

snip

>
>
> diff --git a/arch/arm/mach-omap2/omap4-common.c b/arch/arm/mach-
> omap2/omap4-common.c
> index 91b5d36..193d233 100644
> --- a/arch/arm/mach-omap2/omap4-common.c
> +++ b/arch/arm/mach-omap2/omap4-common.c
> @@ -71,4 +71,27 @@ static int __init omap_l2_cache_init(void)
>  early_initcall(omap_l2_cache_init);
>  #endif
>
> +static struct platform_device omap4_syslink_device = {
> +     .name          = "syslink_ipc",
> +     .id            = -1,
> +     .num_resources = 0,
> +};
>
> +static struct platform_device *omap4_syslink_devices[] = {
> +        &omap4_syslink_device,
> +};
> +

is it worth using an array of platform_device for just 1 device? At least the array should be marked as __initdata so that it does not take memory after init. However I would prefer not to use an array.


> +static int __init omap4_syslink_init(void)
> +{
> +     int retval;
> +
> +     retval = platform_add_devices(omap4_syslink_devices,
> ARRAY_SIZE(omap4_syslink_devices));
> +
> +     if (retval != 0)
> +             pr_err("%s: Failed to register devices: %d\n", __func__,
> retval);
> +     else
> +             pr_info("%s: Successfully registered devices\n", __func__);
> +
> +     return retval;
> +}
> +module_init(omap4_syslink_init);

you aren't in a module here, this code is built-in in the kernel (and will always be). This is supposed to be called during platform init, not during do_initcall. Can we call this function from the machine init function instead of using this trick? (e.g. omap_4430sdp_init() and omap_panda_init())


> diff --git a/drivers/dsp/syslink/multicore_ipc/ipc_drv.c
> b/drivers/dsp/syslink/multicore_ipc/ipc_drv.c
> index e95eab2..e11998d 100644
> --- a/drivers/dsp/syslink/multicore_ipc/ipc_drv.c
> +++ b/drivers/dsp/syslink/multicore_ipc/ipc_drv.c
> @@ -240,3 +240,5 @@ static void __exit ipc_exit(void)
>   */
>  module_init(ipc_init);
>  module_exit(ipc_exit);
> +
> +MODULE_ALIAS("platform:" IPC_NAME);
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team