Hi Ohad,
On 03/17/2014 09:23 AM, Ohad Ben-Cohen wrote:
Hi Suman,
On Sat, Mar 15, 2014 at 1:58 AM, Suman Anna <s-anna@xxxxxx> wrote:
The series doesn't change the semantics of hwspinlock registration or adds a
new OF controller registration function. Implementations would still need to
register a controller using a base_id and number of locks. The series rather
adds a DT-friendly function _ONLY_ for requesting a specific hwlock, and
there are no restrictions on the args specifier being relative id numbers.
Though this is what the simple default xlate helper does (most common
usage), the added xlate ops and #hwlock-cells should allow individual
implementation drivers to adjust any variations, and return a relative lock
w.r.t its registered base_id, as this is how a lock gets registered in the
first place.
I might be missing something, but why can't we have the
specifier+base_id be the hwlock id and then we can entirely drop most
of the core changes in this patch-set?
base_id would be a property (if added) of the hwspinlock controller
node, and from DT perspective, we will be using the phandle for the
controller anyway. So, using a base_id+specifier seems redundant, as the
specifier is already w.r.t a hwspinlock controller node. It is best to
leave the base_id out, just use the specifier. This is pretty much the
standard practice (GPIOs, DMAs, etc all follow this). Please see the
comments from Mark regarding the same on an earlier version.
http://marc.info/?l=linux-omap&m=138135487703774&w=2
I realize we couldn't easily
support sparse id numbers, but not sure this is relevant to
hwspinlocks? do we have a use case that couldn't be supported in this
case?
I agree on the sparse id numbers on hwspinlock, I don't see a need for it.
I actually started out this series with the base_id property, and dropped it
in v3 based on comments looking at it from the request-specific-lock
semantics with DT. That said, the drivers still need to manage a 'base_id'
needed for registration when they get probed for multiple controllers.
Getting the base_id from DT _may_ be useful just for registration purposes,
but for requesting a hwlock, a controller phandle and an implementation
defined args-specifier should suffice IMHO.
How could drivers know what the base_id is if DT doesn't provide it?
please note that we can't depend on order of controller probing; the
hwlock id numbers cannot depend on implementation details.
Yes, I agree this is an issue if we have to have the base_ids fixed per
controller. But I don't think it makes any difference from requesting a
lock from a client DTS node. I can bring it back if Mark agrees.
The exact notion of informing the hwspinlock core about a list of reserved
locks is missing at the moment (even in the non-DT case). I am not sure if
this got lost during the conversion of the registration from per lock to
registering a bank of locks together, or if it is implied by the base_id +
num_locks combination. The core today supports requesting only those locks
that were actually registered, whether allocating a free one dynamically or
giving a specific one.
Before DT came along, early board code could have reserved specific
hwspinlocks if needed. Now with DT, we should add the list of reserved
locks to the controller node, in order to prevent them from being
dynamically allocated by others.
But that strictly relied on the order of requests without any core
changes in the hwspinlock core, right. Also mandates that unique locks
were requested for different clients (left to board integration). The
early board code also has to pass on the reserved hwspinlock information
to the actual client driver somehow (platform data).
With DT, the early board code is much simplified. Looking at the same
scenario from DT case, it seems kinda redundant to specify a set of
reserved locks both in the controller node, as well as the respective
client drivers, as there is almost no platform data with DT. The only
use case for DT client nodes would be for requesting specific locks. I
agree with the problem you described, and I think it will require a
different set of changes to the core.
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
