RE: [RFC] omap: hwspinlock: Added hwspinlock driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Benoit,

Thanks for the comments.  My responses are below.

> Why using a Kconfig option in that case? You can reserve the locks at
> run time based on other driver request. The goal of the hwspinlock is to
> protect data shared between various processors inside OMAP4 like ipu,
> iva, dsp and mpu. So in anycase the syslink driver can request the
> needed locks at init time on behalf of the dsp or the ipu.
> 
> Since you don't even know the number of locks because it is determined
> at init time, you cannot know at build time that information.
> 
> You should add an API to reserve some locks at run time instead of doing
> that.

The reserved locks are not meant for syslink use only.  It will also be used by i2c, which does not go through syslink.  If we were to follow your suggestion, who should call the function to establish number of reserved locks?  We can instead think of the config option as requesting a particular number of reserved locks for system use, independent of how many locks actually exist in the system.  How many reserved locks there are, and which driver is using which reserved locks -- that is a system integration issue that should be sorted out at a higher level.

> It is a details, but why are you renaming the registers with the HW
> prefix? The module name is spinlock not hwspinlock.
> 
> The names should be that:
> #define SPINLOCK_REVISION                    0x0000
> #define SPINLOCK_SYSCONFIG                   0x0010
> #define SPINLOCK_SYSSTATUS                   0x0014
> #define SPINLOCK_LOCK_BASE                   0x0800
> 
> or even that since you are not sharing these defines:
> #define REVISION                    0x0000
> #define SYSCONFIG                   0x0010
> #define SYSSTATUS                   0x0014
> #define LOCK_BASE                   0x0800
> 
> Every IPs are by definition an HW module, so that information is useless.

You have a good point, I will make this change.

> 
> Since it is a new driver for a new IP, why don't you use directly the
> omap_device / omap_hwmod abstraction?

hwmod for spinlock is not currently ready.  We want to integrate with the platform/driver/device system for now, as a first step.  hwmod can be implemented later.


Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux