2015-02-11 11:29:41

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock

On Thu, Jan 29, 2015 at 03:58:42AM +0000, Suman Anna wrote:
> On 01/22/2015 12:56 PM, Mark Rutland wrote:
> > On Wed, Jan 21, 2015 at 05:56:37PM +0000, Suman Anna wrote:
> >> On 01/21/2015 06:41 AM, Ohad Ben-Cohen wrote:
> >>> On Tue, Jan 20, 2015 at 8:05 PM, Tony Lindgren <[email protected]> wrote:
> >>>> How about default to Linux id space and allow overriding that with
> >>>> a module param option if needed?
> >>>
> >>> I'm not sure I'm following.
> >>>
> >>> If the main point of contention is the base_id field, I'm also fine
> >>> with removing it entirely, as I'm not aware of any actual user for it
> >>> (Suman please confirm?).
> >>
> >> Yeah, well the current implementations that I am aware of only have a
> >> single bank, so all of them would be using a value of 0. I am yet to see
> >> a platform with multiple instances where the property really makes a
> >> difference. v7 has the property mandatory, so all the implementations
> >> would need to define this value even if it is 0.
> >>
> >> regards
> >> Suman
> >>
> >>>
> >>> Mark? Rob? Will you accept Suman's patches if the base_id field is removed?
> >
> > My concern is that the mapping of hwspinlock IDs doesn't seem to be
> > explicit in the DT on a per-context basis, which is what I'd expect.
> >
> > e.g.
> >
> > lck: hwspinlock-device@f00 {
> > ...
> > #hwlock-cells = <1>;
> > };
> >
> > some-other-os-interface {
> > ...
> > hwlocks = <&lck 0>, <&lck 1>, <&lck 2>, <&lck 3>;
> > hwlock-names = "glbl", "pool0", "pool1", "pool2";
> > };
> >
> > a-different-os-interface {
> > ...
> > hwlocks = <&lck 18>, <&lck 21>, <&lck 4>, <&lck 5>;
> > hwlock-names = "init", "teardown", "pool0", "pool1";
> > };
> >
> > That's the only way I would expect this to possibly remain a stable
> > over time, and it's the entire reason for #hwlock-cells, no?
> >
> > How do you expect the other components sharing the hwspinlocks to be
> > described?
>
> Yes indeed, this is what any of the clients will use on Linux. But
> this is not necessarily the semantics for exchanging hwlocks with the
> other processor(s) which is where the global id space comes into
> picture.

I did try to consider that above. Rather than thinking about the
numbering as "global", think of it as unique within the a given pool
shared between processors. That's what the "poolN" names are about
above.

That way you can dynamically allocate within the pool and know that
Linux and the SW on the other processors will use the same ID. You can
have pools that span multiple hwlock hardware blocks, and you can have
multiple separate pools in operation at once.

Surely that covers the cases you care about?

If using names is clunky, we could instead have a pool-hwlocks property
for that purpose.

Thanks,
Mark.


2015-02-16 18:06:30

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock

On Wed, Feb 11, 2015 at 3:29 AM, Mark Rutland <[email protected]> wrote:
> On Thu, Jan 29, 2015 at 03:58:42AM +0000, Suman Anna wrote:
>> On 01/22/2015 12:56 PM, Mark Rutland wrote:
[..]
>> > That's the only way I would expect this to possibly remain a stable
>> > over time, and it's the entire reason for #hwlock-cells, no?
>> >
>> > How do you expect the other components sharing the hwspinlocks to be
>> > described?
>>
>> Yes indeed, this is what any of the clients will use on Linux. But
>> this is not necessarily the semantics for exchanging hwlocks with the
>> other processor(s) which is where the global id space comes into
>> picture.
>
> I did try to consider that above. Rather than thinking about the
> numbering as "global", think of it as unique within the a given pool
> shared between processors. That's what the "poolN" names are about
> above.
>
> That way you can dynamically allocate within the pool and know that
> Linux and the SW on the other processors will use the same ID. You can
> have pools that span multiple hwlock hardware blocks, and you can have
> multiple separate pools in operation at once.
>
> Surely that covers the cases you care about?
>
> If using names is clunky, we could instead have a pool-hwlocks property
> for that purpose.
>

Just to make I understand your suggestion.

We would have the communication entity list all the potential hwlocks
(and gpios etc) that it can share and the key to be communicated would
then basically be the index in that list?

Like:
awesome-hub {
pool-hwlocks = <&a 1>, <&a 3>, <&b 5>;
};

And a communicated "lock 2" would mean lock 3 from block a?


This would make it possible to describe what locks are available in
this "allocation pool" and would keep such allocation logic out from
the hwlock core - as the awesome-hub driver could simply trial and
error (with some logic) through the list.

Is this understanding correct?

Regards,
Bjorn