Re: [PATCH 3/4 v2] omap: hwspinlock: Created driver for OMAP hardware spinlock.

[Tony Lindgren <tony@atomide.com>]

* Que, Simon <sque@ti.com> [100811 17:22]:
> Created driver for OMAP hardware spinlock.
> 
>  - Reserved spinlocks for internal use
>  - Dynamic allocation of unreserved locks
>  - Lock, unlock, and trylock functions, with or without disabling irqs/preempt
>  - Registered as a platform device driver
 
> --- /dev/null
> +++ b/arch/arm/mach-omap2/hwspinlocks.c
> @@ -0,0 +1,65 @@
> +int __init hwspinlocks_init(void)
> +{
> +       int retval = 0;
> +
> +       struct omap_hwmod *oh;
> +       const char *oh_name, *pdev_name;
> +
> +       oh_name = "spinlock";
> +       oh = omap_hwmod_lookup(oh_name);
> +       if (WARN_ON(oh == NULL))
> +               return -EINVAL;
> +
> +       pdev_name = "hwspinlock";
> +
> +       /* Pass data to device initialization */
> +       omap_device_build(pdev_name, 0, oh, NULL, 0, omap_spinlock_latency,
> +                               ARRAY_SIZE(omap_spinlock_latency), false);
> +
> +       return retval;
> +}
> +postcore_initcall(hwspinlocks_init);

Is this initcall safe to run on all omaps or do you need some
extra checks for it?

> --- /dev/null
> +++ b/arch/arm/plat-omap/hwspinlock.c
> @@ -0,0 +1,353 @@
> +EXPORT_SYMBOL(hwspinlock_lock);
> +EXPORT_SYMBOL(hwspinlock_trylock);
> +EXPORT_SYMBOL(hwspinlock_unlock);
> +EXPORT_SYMBOL(hwspinlock_request);
> +EXPORT_SYMBOL(hwspinlock_request_specific);
> +EXPORT_SYMBOL(hwspinlock_free);

Do we really want to export these functions? I think we're better
off implementing low-level functions in the platform init code that
are passed to the drivers in the platform_data.

This way the drivers stay generic, and we don't add yet another
non-standard layer that will get misused all over the drivers.

If you really want to export these functions to the drivers,
then all this code should live somewherew under drivers as well.

> +static int __init hwspinlock_init(void)
> +{
> +       int retval = 0;
> +
> +       /* Register spinlock driver */
> +       retval = platform_driver_register(&hwspinlock_driver);
> +
> +       return retval;
> +}
> +postcore_initcall(hwspinlock_init);

Is this initcall safe to run on all omaps?

Tony