| Message ID | 1421269101-51105-2-git-send-email-s-anna@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 14, 2015 at 08:58:18PM +0000, Suman Anna wrote: > This patch adds the generic common bindings used to represent > a hwlock device and use/request locks in a device-tree build. > > All the platform-specific hwlock driver implementations need the > number of locks and associated base id for registering the locks > present within the device with the driver core. This base id > needs to be unique across multiple IP instances of a hwspinlock > device, so that each hwlock can be represented uniquely in a > system. > > The number of locks is represented by 'hwlock-num-locks' property, > and the base id is represented by the 'hwlock-base-id' property. > The args specifier length is dependent on each vendor-specific > implementation and is represented through the '#hwlock-cells' > property. Client users need to use the property 'hwlocks' for > requesting specific lock(s). > > Note that the document is named hwlock.txt deliberately to keep > it a bit more generic. > > Cc: Rob Herring <robh+dt@kernel.org> > Signed-off-by: Suman Anna <s-anna@ti.com> > --- > v7: Revised binding info for hwlock-base-id, it is mandatory now > > .../devicetree/bindings/hwlock/hwlock.txt | 55 ++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt > > diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt > new file mode 100644 > index 000000000000..8de7aaf878f9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt > @@ -0,0 +1,55 @@ > +Generic hwlock bindings > +======================= > + > +Generic bindings that are common to all the hwlock platform specific driver > +implementations. > + > +The validity and need of these common properties may vary from one platform > +implementation to another. The platform specific bindings should explicitly > +state if an optional property is used. Please also look through the individual > +platform specific hwlock binding documentations for identifying the applicable > +properties. > + > +Common properties: > +- #hwlock-cells: Specifies the number of cells needed to represent a > + specific lock. This property is mandatory for all > + platform implementations. > +- hwlock-num-locks: Number of locks present in a hwlock device. This > + property is needed on hwlock devices, where the number > + of supported locks within a hwlock device cannot be > + read from a register. > +- hwlock-base-id: An unique base Id for the locks for a particular hwlock > + device. This property is mandatory for all platform > + implementations. This property makes no sense. The ID encoded in the hwlock cells is relative to the instance (identified by phandle), not global. So the DT has no global ID space. Why do you think you need this? The definition here isn't sufficient, and the example is incomplete (lacking provider nodes and hence this property), which is unhelpful. Thanks, Mark. > + > +Hwlock Users: > +============= > + > +Nodes that require specific hwlock(s) should specify them using the property > +"hwlocks", each containing a phandle to the hwlock node and an args specifier > +value as indicated by #hwlock-cells. Multiple hwlocks can be requested using > +an array of the phandle and hwlock number specifier tuple. > + > +1. Example of a node using a single specific hwlock: > + > +The following example has a node requesting a hwlock in the bank defined by > +the node hwlock1. hwlock1 is a hwlock provider with an argument specifier > +of length 1. > + > + node { > + ... > + hwlocks = <&hwlock1 2>; > + ... > + }; > + > +2. Example of a node using multiple specific hwlocks: > + > +The following example has a node requesting two hwlocks, a hwlock within > +the hwlock device node 'hwlock1' with #hwlock-cells value of 1, and another > +hwlock within the hwlock device node 'hwlock2' with #hwlock-cells value of 2. > + > + node { > + ... > + hwlocks = <&hwlock1 2>, <&hwlock2 0 3>; > + ... > + }; > -- > 2.2.1 > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 15, 2015 at 01:52:01PM +0000, Mark Rutland wrote: > On Wed, Jan 14, 2015 at 08:58:18PM +0000, Suman Anna wrote: > > This patch adds the generic common bindings used to represent > > a hwlock device and use/request locks in a device-tree build. > > > > All the platform-specific hwlock driver implementations need the > > number of locks and associated base id for registering the locks > > present within the device with the driver core. This base id > > needs to be unique across multiple IP instances of a hwspinlock > > device, so that each hwlock can be represented uniquely in a > > system. > > > > The number of locks is represented by 'hwlock-num-locks' property, > > and the base id is represented by the 'hwlock-base-id' property. > > The args specifier length is dependent on each vendor-specific > > implementation and is represented through the '#hwlock-cells' > > property. Client users need to use the property 'hwlocks' for > > requesting specific lock(s). > > > > Note that the document is named hwlock.txt deliberately to keep > > it a bit more generic. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Signed-off-by: Suman Anna <s-anna@ti.com> > > --- > > v7: Revised binding info for hwlock-base-id, it is mandatory now > > > > .../devicetree/bindings/hwlock/hwlock.txt | 55 ++++++++++++++++++++++ > > 1 file changed, 55 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt > > > > diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt > > new file mode 100644 > > index 000000000000..8de7aaf878f9 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt > > @@ -0,0 +1,55 @@ > > +Generic hwlock bindings > > +======================= > > + > > +Generic bindings that are common to all the hwlock platform specific driver > > +implementations. > > + > > +The validity and need of these common properties may vary from one platform > > +implementation to another. The platform specific bindings should explicitly > > +state if an optional property is used. Please also look through the individual > > +platform specific hwlock binding documentations for identifying the applicable > > +properties. > > + > > +Common properties: > > +- #hwlock-cells: Specifies the number of cells needed to represent a > > + specific lock. This property is mandatory for all > > + platform implementations. > > +- hwlock-num-locks: Number of locks present in a hwlock device. This > > + property is needed on hwlock devices, where the number > > + of supported locks within a hwlock device cannot be > > + read from a register. > > +- hwlock-base-id: An unique base Id for the locks for a particular hwlock > > + device. This property is mandatory for all platform > > + implementations. > > This property makes no sense. The ID encoded in the hwlock cells is > relative to the instance (identified by phandle), not global. So the DT > has no global ID space. > > Why do you think you need this? Having looked at the way this proeprty is used, NAK. If you need to carve up a Linux-internal ID space, do that dynamically. There is no need for this property. Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Thu, Jan 15, 2015 at 01:52:01PM +0000, Mark Rutland wrote: >> On Wed, Jan 14, 2015 at 08:58:18PM +0000, Suman Anna wrote: >> > This patch adds the generic common bindings used to represent >> > a hwlock device and use/request locks in a device-tree build. >> > >> > All the platform-specific hwlock driver implementations need the >> > number of locks and associated base id for registering the locks >> > present within the device with the driver core. This base id >> > needs to be unique across multiple IP instances of a hwspinlock >> > device, so that each hwlock can be represented uniquely in a >> > system. >> > >> > The number of locks is represented by 'hwlock-num-locks' property, >> > and the base id is represented by the 'hwlock-base-id' property. >> > The args specifier length is dependent on each vendor-specific >> > implementation and is represented through the '#hwlock-cells' >> > property. Client users need to use the property 'hwlocks' for >> > requesting specific lock(s). >> > >> > Note that the document is named hwlock.txt deliberately to keep >> > it a bit more generic. >> > >> > Cc: Rob Herring <robh+dt@kernel.org> >> > Signed-off-by: Suman Anna <s-anna@ti.com> >> > --- >> > v7: Revised binding info for hwlock-base-id, it is mandatory now >> > >> > .../devicetree/bindings/hwlock/hwlock.txt | 55 ++++++++++++++++++++++ >> > 1 file changed, 55 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt >> > >> > diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt >> > new file mode 100644 >> > index 000000000000..8de7aaf878f9 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt >> > @@ -0,0 +1,55 @@ >> > +Generic hwlock bindings >> > +======================= >> > + >> > +Generic bindings that are common to all the hwlock platform specific driver >> > +implementations. >> > + >> > +The validity and need of these common properties may vary from one platform >> > +implementation to another. The platform specific bindings should explicitly >> > +state if an optional property is used. Please also look through the individual >> > +platform specific hwlock binding documentations for identifying the applicable >> > +properties. >> > + >> > +Common properties: >> > +- #hwlock-cells: Specifies the number of cells needed to represent a >> > + specific lock. This property is mandatory for all >> > + platform implementations. >> > +- hwlock-num-locks: Number of locks present in a hwlock device. This >> > + property is needed on hwlock devices, where the number >> > + of supported locks within a hwlock device cannot be >> > + read from a register. >> > +- hwlock-base-id: An unique base Id for the locks for a particular hwlock >> > + device. This property is mandatory for all platform >> > + implementations. >> >> This property makes no sense. The ID encoded in the hwlock cells is >> relative to the instance (identified by phandle), not global. So the DT >> has no global ID space. >> >> Why do you think you need this? > > Having looked at the way this proeprty is used, NAK. > > If you need to carve up a Linux-internal ID space, do that dynamically. > There is no need for this property. Better yet, don't create a Linux ID space for this. Everywhere we have one, we want to get rid of it. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/15/2015 08:42 AM, Rob Herring wrote: > On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland <mark.rutland@arm.com> wrote: >> On Thu, Jan 15, 2015 at 01:52:01PM +0000, Mark Rutland wrote: >>> On Wed, Jan 14, 2015 at 08:58:18PM +0000, Suman Anna wrote: >>>> This patch adds the generic common bindings used to represent >>>> a hwlock device and use/request locks in a device-tree build. >>>> >>>> All the platform-specific hwlock driver implementations need the >>>> number of locks and associated base id for registering the locks >>>> present within the device with the driver core. This base id >>>> needs to be unique across multiple IP instances of a hwspinlock >>>> device, so that each hwlock can be represented uniquely in a >>>> system. >>>> >>>> The number of locks is represented by 'hwlock-num-locks' property, >>>> and the base id is represented by the 'hwlock-base-id' property. >>>> The args specifier length is dependent on each vendor-specific >>>> implementation and is represented through the '#hwlock-cells' >>>> property. Client users need to use the property 'hwlocks' for >>>> requesting specific lock(s). >>>> >>>> Note that the document is named hwlock.txt deliberately to keep >>>> it a bit more generic. >>>> >>>> Cc: Rob Herring <robh+dt@kernel.org> >>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>> --- >>>> v7: Revised binding info for hwlock-base-id, it is mandatory now >>>> >>>> .../devicetree/bindings/hwlock/hwlock.txt | 55 ++++++++++++++++++++++ >>>> 1 file changed, 55 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>> new file mode 100644 >>>> index 000000000000..8de7aaf878f9 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>> @@ -0,0 +1,55 @@ >>>> +Generic hwlock bindings >>>> +======================= >>>> + >>>> +Generic bindings that are common to all the hwlock platform specific driver >>>> +implementations. >>>> + >>>> +The validity and need of these common properties may vary from one platform >>>> +implementation to another. The platform specific bindings should explicitly >>>> +state if an optional property is used. Please also look through the individual >>>> +platform specific hwlock binding documentations for identifying the applicable >>>> +properties. >>>> + >>>> +Common properties: >>>> +- #hwlock-cells: Specifies the number of cells needed to represent a >>>> + specific lock. This property is mandatory for all >>>> + platform implementations. >>>> +- hwlock-num-locks: Number of locks present in a hwlock device. This >>>> + property is needed on hwlock devices, where the number >>>> + of supported locks within a hwlock device cannot be >>>> + read from a register. >>>> +- hwlock-base-id: An unique base Id for the locks for a particular hwlock >>>> + device. This property is mandatory for all platform >>>> + implementations. >>> >>> This property makes no sense. The ID encoded in the hwlock cells is >>> relative to the instance (identified by phandle), not global. So the DT >>> has no global ID space. >>> >>> Why do you think you need this? >> >> Having looked at the way this proeprty is used, NAK. >> >> If you need to carve up a Linux-internal ID space, do that dynamically. >> There is no need for this property. > > Better yet, don't create a Linux ID space for this. Everywhere we have > one, we want to get rid of it. Hi Mark, Rob, Thank you for your comments. The id space is not limited to Linux, as this is indeed a global id space for all the processors in an SoC, as that is the primary usage of the individual locks in these IP blocks. It needs to be static across the SoC irrespective of whether a node was enabled or not. Now, it is debatable whether we do this in DT or do this in each individual implementation. This is something that the hwspinlock core cannot do, and probably would have to be done as static driver match data. I have gone from having this as an optional property, to not having it, and now back to having it as mandatory - because of the different perspectives from bindings vs driver subsystem maintainers. I brought this back mainly based on the reasonings in [1] Ohad, Do you have any comments or suggestions here? It looks like I have no choice but to bring back "hwspinlock/core: maintain a list of registered hwspinlock banks", if we were to have the bindings without a hwlock-base-id property. regards Suman [1] http://marc.info/?l=linux-omap&m=139510004009415&w=2 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 15, 2015 at 4:42 PM, Rob Herring <robherring2@gmail.com> wrote: > On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland <mark.rutland@arm.com> wrote: >> On Thu, Jan 15, 2015 at 01:52:01PM +0000, Mark Rutland wrote: >>> On Wed, Jan 14, 2015 at 08:58:18PM +0000, Suman Anna wrote: >>> > +- hwlock-base-id: An unique base Id for the locks for a particular hwlock >>> > + device. This property is mandatory for all platform >>> > + implementations. >>> >>> This property makes no sense. The ID encoded in the hwlock cells is >>> relative to the instance (identified by phandle), not global. So the DT >>> has no global ID space. >>> >>> Why do you think you need this? >> >> Having looked at the way this proeprty is used, NAK. >> >> If you need to carve up a Linux-internal ID space, do that dynamically. >> There is no need for this property. > > Better yet, don't create a Linux ID space for this. Everywhere we have > one, we want to get rid of it. Rob, Mark, The hwlock is a basic hardware primitive that allow synchronization between different processors in the system, which may be running Linux as well as other operating systems, and may have no other means of communication. The hwlock id numbers are predefined, global and static across the entire system: Linux may boot well after other operating systems are already running and using these hwlocks to communicate, and therefore, in order to use these hardware devices, it must not enumerate them differently than the rest of the system. Given that these id numbers are global, system-wide, static and predefined, where Linux may just be one user of them, please reconsider the approach as implemented by Suman, or suggest an alternative one you may prefer. Thanks, Ohad. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 16, 2015 at 06:09:00AM +0000, Ohad Ben-Cohen wrote: > On Thu, Jan 15, 2015 at 4:42 PM, Rob Herring <robherring2@gmail.com> wrote: > > On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland <mark.rutland@arm.com> wrote: > >> On Thu, Jan 15, 2015 at 01:52:01PM +0000, Mark Rutland wrote: > >>> On Wed, Jan 14, 2015 at 08:58:18PM +0000, Suman Anna wrote: > >>> > +- hwlock-base-id: An unique base Id for the locks for a particular hwlock > >>> > + device. This property is mandatory for all platform > >>> > + implementations. > >>> > >>> This property makes no sense. The ID encoded in the hwlock cells is > >>> relative to the instance (identified by phandle), not global. So the DT > >>> has no global ID space. > >>> > >>> Why do you think you need this? > >> > >> Having looked at the way this proeprty is used, NAK. > >> > >> If you need to carve up a Linux-internal ID space, do that dynamically. > >> There is no need for this property. > > > > Better yet, don't create a Linux ID space for this. Everywhere we have > > one, we want to get rid of it. > > Rob, Mark, > > The hwlock is a basic hardware primitive that allow synchronization > between different processors in the system, which may be running Linux > as well as other operating systems, and may have no other means of > communication. > > The hwlock id numbers are predefined, global and static across the > entire system: Linux may boot well after other operating systems are > already running and using these hwlocks to communicate, and therefore, > in order to use these hardware devices, it must not enumerate them > differently than the rest of the system. That's not true. In order to communicate it must agree with the other users as to the meaning of each instance, and the protocol for use. That doesn't necessarily mean that Linux needs to know the numerical ID from a datasheet, and regardless that ID is separate from the logical ID Linux uses internally. > Given that these id numbers are global, system-wide, static and > predefined, where Linux may just be one user of them, please > reconsider the approach as implemented by Suman, or suggest an > alternative one you may prefer. To communicate with the other processors, Linux will need to understand the protocol. So there will need to be nodes in the DT describing the protocol. Those nodes can refer to the relevant locks using phandle + args (with a hwlock-names list if indexing is not appropriate). The entire point of the hwlock-cells is to allow individual locks to be referred to in this way. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 15, 2015 at 02:42:23PM +0000, Rob Herring wrote: > On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > On Thu, Jan 15, 2015 at 01:52:01PM +0000, Mark Rutland wrote: > >> On Wed, Jan 14, 2015 at 08:58:18PM +0000, Suman Anna wrote: > >> > This patch adds the generic common bindings used to represent > >> > a hwlock device and use/request locks in a device-tree build. > >> > > >> > All the platform-specific hwlock driver implementations need the > >> > number of locks and associated base id for registering the locks > >> > present within the device with the driver core. This base id > >> > needs to be unique across multiple IP instances of a hwspinlock > >> > device, so that each hwlock can be represented uniquely in a > >> > system. > >> > > >> > The number of locks is represented by 'hwlock-num-locks' property, > >> > and the base id is represented by the 'hwlock-base-id' property. > >> > The args specifier length is dependent on each vendor-specific > >> > implementation and is represented through the '#hwlock-cells' > >> > property. Client users need to use the property 'hwlocks' for > >> > requesting specific lock(s). > >> > > >> > Note that the document is named hwlock.txt deliberately to keep > >> > it a bit more generic. > >> > > >> > Cc: Rob Herring <robh+dt@kernel.org> > >> > Signed-off-by: Suman Anna <s-anna@ti.com> > >> > --- > >> > v7: Revised binding info for hwlock-base-id, it is mandatory now > >> > > >> > .../devicetree/bindings/hwlock/hwlock.txt | 55 ++++++++++++++++++++++ > >> > 1 file changed, 55 insertions(+) > >> > create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt > >> > > >> > diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt > >> > new file mode 100644 > >> > index 000000000000..8de7aaf878f9 > >> > --- /dev/null > >> > +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt > >> > @@ -0,0 +1,55 @@ > >> > +Generic hwlock bindings > >> > +======================= > >> > + > >> > +Generic bindings that are common to all the hwlock platform specific driver > >> > +implementations. > >> > + > >> > +The validity and need of these common properties may vary from one platform > >> > +implementation to another. The platform specific bindings should explicitly > >> > +state if an optional property is used. Please also look through the individual > >> > +platform specific hwlock binding documentations for identifying the applicable > >> > +properties. > >> > + > >> > +Common properties: > >> > +- #hwlock-cells: Specifies the number of cells needed to represent a > >> > + specific lock. This property is mandatory for all > >> > + platform implementations. > >> > +- hwlock-num-locks: Number of locks present in a hwlock device. This > >> > + property is needed on hwlock devices, where the number > >> > + of supported locks within a hwlock device cannot be > >> > + read from a register. > >> > +- hwlock-base-id: An unique base Id for the locks for a particular hwlock > >> > + device. This property is mandatory for all platform > >> > + implementations. > >> > >> This property makes no sense. The ID encoded in the hwlock cells is > >> relative to the instance (identified by phandle), not global. So the DT > >> has no global ID space. > >> > >> Why do you think you need this? > > > > Having looked at the way this proeprty is used, NAK. > > > > If you need to carve up a Linux-internal ID space, do that dynamically. > > There is no need for this property. > > Better yet, don't create a Linux ID space for this. Everywhere we have > one, we want to get rid of it. Agreed. A completely opaque token / desc structure would prevent a lot of potential abuse and save us from painful breakage. Mark -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/16/2015 04:19 AM, Mark Rutland wrote: > On Thu, Jan 15, 2015 at 02:42:23PM +0000, Rob Herring wrote: >> On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland <mark.rutland@arm.com> wrote: >>> On Thu, Jan 15, 2015 at 01:52:01PM +0000, Mark Rutland wrote: >>>> On Wed, Jan 14, 2015 at 08:58:18PM +0000, Suman Anna wrote: >>>>> This patch adds the generic common bindings used to represent >>>>> a hwlock device and use/request locks in a device-tree build. >>>>> >>>>> All the platform-specific hwlock driver implementations need the >>>>> number of locks and associated base id for registering the locks >>>>> present within the device with the driver core. This base id >>>>> needs to be unique across multiple IP instances of a hwspinlock >>>>> device, so that each hwlock can be represented uniquely in a >>>>> system. >>>>> >>>>> The number of locks is represented by 'hwlock-num-locks' property, >>>>> and the base id is represented by the 'hwlock-base-id' property. >>>>> The args specifier length is dependent on each vendor-specific >>>>> implementation and is represented through the '#hwlock-cells' >>>>> property. Client users need to use the property 'hwlocks' for >>>>> requesting specific lock(s). >>>>> >>>>> Note that the document is named hwlock.txt deliberately to keep >>>>> it a bit more generic. >>>>> >>>>> Cc: Rob Herring <robh+dt@kernel.org> >>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>>> --- >>>>> v7: Revised binding info for hwlock-base-id, it is mandatory now >>>>> >>>>> .../devicetree/bindings/hwlock/hwlock.txt | 55 ++++++++++++++++++++++ >>>>> 1 file changed, 55 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>>> new file mode 100644 >>>>> index 000000000000..8de7aaf878f9 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>>> @@ -0,0 +1,55 @@ >>>>> +Generic hwlock bindings >>>>> +======================= >>>>> + >>>>> +Generic bindings that are common to all the hwlock platform specific driver >>>>> +implementations. >>>>> + >>>>> +The validity and need of these common properties may vary from one platform >>>>> +implementation to another. The platform specific bindings should explicitly >>>>> +state if an optional property is used. Please also look through the individual >>>>> +platform specific hwlock binding documentations for identifying the applicable >>>>> +properties. >>>>> + >>>>> +Common properties: >>>>> +- #hwlock-cells: Specifies the number of cells needed to represent a >>>>> + specific lock. This property is mandatory for all >>>>> + platform implementations. >>>>> +- hwlock-num-locks: Number of locks present in a hwlock device. This >>>>> + property is needed on hwlock devices, where the number >>>>> + of supported locks within a hwlock device cannot be >>>>> + read from a register. >>>>> +- hwlock-base-id: An unique base Id for the locks for a particular hwlock >>>>> + device. This property is mandatory for all platform >>>>> + implementations. >>>> >>>> This property makes no sense. The ID encoded in the hwlock cells is >>>> relative to the instance (identified by phandle), not global. So the DT >>>> has no global ID space. >>>> >>>> Why do you think you need this? >>> >>> Having looked at the way this proeprty is used, NAK. >>> >>> If you need to carve up a Linux-internal ID space, do that dynamically. >>> There is no need for this property. >> >> Better yet, don't create a Linux ID space for this. Everywhere we have >> one, we want to get rid of it. > > Agreed. A completely opaque token / desc structure would prevent a lot > of potential abuse and save us from painful breakage. The regular API to acquire or release a lock on Linux does indeed use opaque handles, but the id space is needed for communication with other processors as the handles have no meaning on non-Linux processors. The id space is the simplest to exchange which lock to use with other processors compared to the device instance plus the lock number within that device. regards Suman -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark, On Fri, Jan 16, 2015 at 12:17 PM, Mark Rutland <mark.rutland@arm.com> wrote: >> The hwlock is a basic hardware primitive that allow synchronization >> between different processors in the system, which may be running Linux >> as well as other operating systems, and may have no other means of >> communication. >> >> The hwlock id numbers are predefined, global and static across the >> entire system: Linux may boot well after other operating systems are >> already running and using these hwlocks to communicate, and therefore, >> in order to use these hardware devices, it must not enumerate them >> differently than the rest of the system. > > That's not true. > > In order to communicate it must agree with the other users as to the > meaning of each instance, and the protocol for use. That doesn't > necessarily mean that Linux needs to know the numerical ID from a > datasheet, and regardless that ID is separate from the logical ID Linux > uses internally. Let me describe hwspinlocks a bit more so we all get to know it better and can then agree on a proper solution. - What makes handling of hwspinlock ID numbers convenient is the fact that it's not based on random datasheet numbers. In fact, hwspinlocks is just special memory: usually datasheets just define the base address and the size of the hwspinlock area. So any numerical ID we use to call the locks themselves are already logical and sane, similar to the way we handle memory (i.e. if we have 32 locks we'll always use 0..31). So hwlocks ids are very much like memory addressing, and not irq numbers. - Sometimes Linux will have to dynamically allocate a hwlock, and send the ID of the allocated lock to a remote processor (which may not be running Linux). - Sometimes a remote processor, which may not be running Linux, will have to dynamically allocate a hwlock, and send the ID of the allocated lock to us (another processor running Linux) We cannot tell in advance what kind of IPC is going to be used for sending and receiving this hwlock ID. Some are handled by Linux (kernel) and some by the user space. So we must be able to expose an ID the system will understand as well as receive one. Thanks, Ohad. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Ohad Ben-Cohen <ohad@wizery.com> [150116 16:50]: > Mark, > > On Fri, Jan 16, 2015 at 12:17 PM, Mark Rutland <mark.rutland@arm.com> wrote: > >> The hwlock is a basic hardware primitive that allow synchronization > >> between different processors in the system, which may be running Linux > >> as well as other operating systems, and may have no other means of > >> communication. > >> > >> The hwlock id numbers are predefined, global and static across the > >> entire system: Linux may boot well after other operating systems are > >> already running and using these hwlocks to communicate, and therefore, > >> in order to use these hardware devices, it must not enumerate them > >> differently than the rest of the system. > > > > That's not true. > > > > In order to communicate it must agree with the other users as to the > > meaning of each instance, and the protocol for use. That doesn't > > necessarily mean that Linux needs to know the numerical ID from a > > datasheet, and regardless that ID is separate from the logical ID Linux > > uses internally. > > Let me describe hwspinlocks a bit more so we all get to know it better > and can then agree on a proper solution. > > - What makes handling of hwspinlock ID numbers convenient is the fact > that it's not based on random datasheet numbers. In fact, hwspinlocks > is just special memory: usually datasheets just define the base > address and the size of the hwspinlock area. So any numerical ID we > use to call the locks themselves are already logical and sane, similar > to the way we handle memory (i.e. if we have 32 locks we'll always use > 0..31). So hwlocks ids are very much like memory addressing, and not > irq numbers. > > - Sometimes Linux will have to dynamically allocate a hwlock, and send > the ID of the allocated lock to a remote processor (which may not be > running Linux). > - Sometimes a remote processor, which may not be running Linux, will > have to dynamically allocate a hwlock, and send the ID of the > allocated lock to us (another processor running Linux) > > We cannot tell in advance what kind of IPC is going to be used for > sending and receiving this hwlock ID. Some are handled by Linux > (kernel) and some by the user space. So we must be able to expose an > ID the system will understand as well as receive one. How about default to Linux id space and allow overriding that with a module param option if needed? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 20, 2015 at 8:05 PM, Tony Lindgren <tony@atomide.com> 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?). Mark? Rob? Will you accept Suman's patches if the base_id field is removed? Thanks, Ohad. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/21/2015 06:41 AM, Ohad Ben-Cohen wrote: > On Tue, Jan 20, 2015 at 8:05 PM, Tony Lindgren <tony@atomide.com> 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? > > Thanks, > Ohad. > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <tony@atomide.com> 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? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <tony@atomide.com> 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. regards Suman -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 16, 2015 at 4:46 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Mark, > > On Fri, Jan 16, 2015 at 12:17 PM, Mark Rutland <mark.rutland@arm.com> wrote: >>> The hwlock is a basic hardware primitive that allow synchronization >>> between different processors in the system, which may be running Linux >>> as well as other operating systems, and may have no other means of >>> communication. >>> >>> The hwlock id numbers are predefined, global and static across the >>> entire system: Linux may boot well after other operating systems are >>> already running and using these hwlocks to communicate, and therefore, >>> in order to use these hardware devices, it must not enumerate them >>> differently than the rest of the system. >> >> That's not true. >> >> In order to communicate it must agree with the other users as to the >> meaning of each instance, and the protocol for use. That doesn't >> necessarily mean that Linux needs to know the numerical ID from a >> datasheet, and regardless that ID is separate from the logical ID Linux >> uses internally. > > Let me describe hwspinlocks a bit more so we all get to know it better > and can then agree on a proper solution. > > - What makes handling of hwspinlock ID numbers convenient is the fact > that it's not based on random datasheet numbers. In fact, hwspinlocks > is just special memory: usually datasheets just define the base > address and the size of the hwspinlock area. So any numerical ID we > use to call the locks themselves are already logical and sane, similar > to the way we handle memory (i.e. if we have 32 locks we'll always use > 0..31). So hwlocks ids are very much like memory addressing, and not > irq numbers. > But that's exactly how irqs or gpios work as well. If you have 32 gpios in a system they used to be numbered 0-31 and people would reference them directly by that number. Every one of the systems that was designed in this way is moving away from it. > - Sometimes Linux will have to dynamically allocate a hwlock, and send > the ID of the allocated lock to a remote processor (which may not be > running Linux). In a system where you have two hwlock blocks lckA and lckB, each consisting of 8 locks and you have dspB that can only access lckB; will you tell the firmware engineers to always subtract 8 from the numbers you pass them? Wouldn't it make much more sense to have local indexes here and pass them e.g lckB:2? > - Sometimes a remote processor, which may not be running Linux, will > have to dynamically allocate a hwlock, and send the ID of the > allocated lock to us (another processor running Linux) > I'm sorry but you cannot have a system on both sides that is allowed to do dynamic allocation from a limited set of resources. Further more this dynamic allocation leads to interesting race conditions as what happens if you dynamically allocate a hwlock that is statically allocated by another part of the system? The only solution I can think of is to have a static allocation of ids that the dynamic allocator might use, and then we're just carrying extra code when the system is already statically configured... > We cannot tell in advance what kind of IPC is going to be used for > sending and receiving this hwlock ID. Some are handled by Linux > (kernel) and some by the user space. So we must be able to expose an > ID the system will understand as well as receive one. > Designing this interface to take into consideration that someone might send us something completely crazy isn't productive. The only reason for having num-locks and base-id in device tree is because of the current Linux implementation. base-id is not a property of the hardware and num-locks is not needed for anything but book keeping of base-id's in the hwlock framework. This is why I preferred Sumans earlier suggestion of having the binding consist of #hwlock-cells = <X> and the necessary accessor functions for resolving a hwlock based on a dt reference. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson <bjorn@kryo.se> wrote: > In a system where you have two hwlock blocks lckA and lckB, each > consisting of 8 locks and you have dspB that can only access lckB This is a good example - thanks. To be able to cope with such cases we will have to pass a hwlock block reference and its relative lock id. The DT binding should definitely be prepared for such cases (just kill the base-id field?), but let's see what it means about the Linux implementation. Since the existence of several hwblocks is still fictional (Bjorn, please confirm too?), we may prefer to introduce changes to support it only when it shows up; it all depends on the amount of changes needed. Suman, care to take a look please? >> - Sometimes a remote processor, which may not be running Linux, will >> have to dynamically allocate a hwlock, and send the ID of the >> allocated lock to us (another processor running Linux) >> > I'm sorry but you cannot have a system on both sides that is allowed > to do dynamic allocation from a limited set of resources. Of course not. On such systems, Linux is not the one responsible for allocating the hwlocks, at least not during part of the time or from part of the hwlocks. There were a few different use cases, with different semantics, that required communicating to Linux an hwlock id, but since none of them have reached mainline, we should only remember they may show up one day, but not put too much effort to support them right now. Thanks, Ohad. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jan 31, 2015 at 7:41 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson <bjorn@kryo.se> wrote: > > In a system where you have two hwlock blocks lckA and lckB, each > > consisting of 8 locks and you have dspB that can only access lckB > > This is a good example - thanks. To be able to cope with such cases we > will have to pass a hwlock block reference and its relative lock id. Additionally, to support such a scenario, we can no longer retain the simple dynamic allocation API we have today, because it might end up allocating dspB an hwlock from IckA. We will have to make sure hwlocks are allocated only from pools visible to the user, something that will change not only the hwspinlock API but also the way it maintains the hwlocks. I suspect we want to wait for such hardware to show up first, and only then add framework support for it. Regardless, we obviously do want to make sure our DT binding is prepared for the worse, so we'll drop the "base-id" field. Thanks, Ohad. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson <bjorn@kryo.se> wrote: >> In a system where you have two hwlock blocks lckA and lckB, each >> consisting of 8 locks and you have dspB that can only access lckB > > This is a good example - thanks. To be able to cope with such cases we > will have to pass a hwlock block reference and its relative lock id. > Correct, so the #hwlock-cells and hwlock part from the proposal are the important one. Having an optional hwlock-names will make things easier to read as well, but is not necessary. > The DT binding should definitely be prepared for such cases (just kill > the base-id field?), but let's see what it means about the Linux > implementation. > From the dt binding PoV, we should be able to skip num-locks as well. It seems most hwlock blocks have a fixed amount of locks provided and the drivers are reporting this to the core when registering. So I think we can reduce the binding to: Providers: #hwlock-cells Consumers: hwlocks hwlock-names For the hardware where number of locks is actually variable (e.g. different variants of same block) there can be driver specific entries for this. > Since the existence of several hwblocks is still fictional (Bjorn, > please confirm too?), we may prefer to introduce changes to support it > only when it shows up; it all depends on the amount of changes needed. > Suman, care to take a look please? > I haven't seen any such systems yet. We could introduce the logic found in other subsystems of allowing -1 as base_id and having the core find a available range. This will work for all cases where the global ids doesn't have to be static; either due to the fact that we use block:local-id or that the indices are hard coded. (Referencing locks via dt is equivalent of the block:local-id case) >>> - Sometimes a remote processor, which may not be running Linux, will >>> have to dynamically allocate a hwlock, and send the ID of the >>> allocated lock to us (another processor running Linux) >>> >> I'm sorry but you cannot have a system on both sides that is allowed >> to do dynamic allocation from a limited set of resources. > > Of course not. On such systems, Linux is not the one responsible for > allocating the hwlocks, at least not during part of the time or from > part of the hwlocks. There were a few different use cases, with > different semantics, that required communicating to Linux an hwlock > id, but since none of them have reached mainline, we should only > remember they may show up one day, but not put too much effort to > support them right now. > That makes more sense, although I still believe that you end up with a system wide setup where locks are statically allocated in some document beforehand. Either that or you will have to feed that other system with the list of constraints. Non the less, that's "unrelated". If we get an incoming message saying lckX:Y (or the global lckZ), we probably wouldn't define that in DT. We would need a way to resolve the hwlock-block "lckX" so we can request lock Y from that block. We would basically build the DT reference on the fly. I think this is a future problem to be solved and more importantly I don't think it's limited to hwlocks. If a system architect designs a system where a remote entity will do allocation of resources for us, they will most likely do so not only for hwlocks but also for gpios, irqs and other resources that we today reference with arguments in DT. Hopefully that will not happen anytime soon, so let's just ignore that problem for now and go for a simple binding that will cover todays use cases (with some thought into multi-block support). Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/01/2015 11:55 AM, Bjorn Andersson wrote: > On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson <bjorn@kryo.se> wrote: >>> In a system where you have two hwlock blocks lckA and lckB, each >>> consisting of 8 locks and you have dspB that can only access lckB >> >> This is a good example - thanks. To be able to cope with such cases we >> will have to pass a hwlock block reference and its relative lock id. >> > > Correct, so the #hwlock-cells and hwlock part from the proposal are > the important one. Having an optional hwlock-names will make things > easier to read as well, but is not necessary. Right, if anything, it would be useful only for the clients, but the hwspinlock core itself would not need it. So, I would forgo adding the hwlock-names for now. > >> The DT binding should definitely be prepared for such cases (just kill >> the base-id field?), but let's see what it means about the Linux >> implementation. >> > > From the dt binding PoV, we should be able to skip num-locks as well. > It seems most hwlock blocks have a fixed amount of locks provided and > the drivers are reporting this to the core when registering. I added this originally based on the initial MSM HW Mutex block bindings. > > So I think we can reduce the binding to: > > Providers: > #hwlock-cells > > Consumers: > hwlocks > hwlock-names > > For the hardware where number of locks is actually variable (e.g. > different variants of same block) there can be driver specific entries > for this. Right, we should be able to drop this and use the driver match data. As it is, the field is used during registration of the block with the hwspinlock core. regards Suman -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/01/2015 05:00 AM, Ohad Ben-Cohen wrote: > On Sat, Jan 31, 2015 at 7:41 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson <bjorn@kryo.se> wrote: >>> In a system where you have two hwlock blocks lckA and lckB, each >>> consisting of 8 locks and you have dspB that can only access lckB >> >> This is a good example - thanks. To be able to cope with such cases we >> will have to pass a hwlock block reference and its relative lock id. So, you mean lckB is only between the host and dspB. Obviously, if it were shared between dspA and dspB only, then the allocation and management would be completely outside the host Linux driver's scope. > > Additionally, to support such a scenario, we can no longer retain the > simple dynamic allocation API we have today, because it might end up > allocating dspB an hwlock from IckA. > > We will have to make sure hwlocks are allocated only from pools > visible to the user, something that will change not only the > hwspinlock API but also the way it maintains the hwlocks. Right, the current API definitely will not scale for that. It was designed around the concept that it's easier to exchange a single global id, rather than a lcbB:id or some other similar semantics that needs to be interpreted. > > I suspect we want to wait for such hardware to show up first, and only > then add framework support for it. Agreed. regards Suman > Regardless, we obviously do want to > make sure our DT binding is prepared for the worse, so we'll drop the > "base-id" field. > > Thanks, > Ohad. > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 2, 2015 at 1:07 PM, Suman Anna <s-anna@ti.com> wrote: > On 02/01/2015 11:55 AM, Bjorn Andersson wrote: >> On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: >>> On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson <bjorn@kryo.se> wrote: >>>> In a system where you have two hwlock blocks lckA and lckB, each >>>> consisting of 8 locks and you have dspB that can only access lckB >>> >>> This is a good example - thanks. To be able to cope with such cases we >>> will have to pass a hwlock block reference and its relative lock id. >>> >> >> Correct, so the #hwlock-cells and hwlock part from the proposal are >> the important one. Having an optional hwlock-names will make things >> easier to read as well, but is not necessary. > > Right, if anything, it would be useful only for the clients, but the > hwspinlock core itself would not need it. So, I would forgo adding the > hwlock-names for now. > >> >>> The DT binding should definitely be prepared for such cases (just kill >>> the base-id field?), but let's see what it means about the Linux >>> implementation. >>> >> >> From the dt binding PoV, we should be able to skip num-locks as well. >> It seems most hwlock blocks have a fixed amount of locks provided and >> the drivers are reporting this to the core when registering. > > I added this originally based on the initial MSM HW Mutex block bindings. > It's not entirely correct to have this in DT for the MSM HW, as the hardware has a fixed number of mutexes. As soon as we have the binding sorted out I will follow up with a new revision of the tcsr/sfpb-mutex driver. >> >> So I think we can reduce the binding to: >> >> Providers: >> #hwlock-cells >> >> Consumers: >> hwlocks >> hwlock-names >> >> For the hardware where number of locks is actually variable (e.g. >> different variants of same block) there can be driver specific entries >> for this. > > Right, we should be able to drop this and use the driver match data. As > it is, the field is used during registration of the block with the > hwspinlock core. > If we have certain systems where it actually is a property to be configured then they can have individual properties, extending the standard set. Either way, it's not a dynamic property shared by all hwlock drivers, so it should not be in the common binding. Will you send out a new revision of the binding? I would love to get this integrated so I can move on with the dependents. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, On 02/05/2015 05:01 PM, Bjorn Andersson wrote: > On Mon, Feb 2, 2015 at 1:07 PM, Suman Anna <s-anna@ti.com> wrote: >> On 02/01/2015 11:55 AM, Bjorn Andersson wrote: >>> On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: >>>> On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson <bjorn@kryo.se> wrote: >>>>> In a system where you have two hwlock blocks lckA and lckB, each >>>>> consisting of 8 locks and you have dspB that can only access lckB >>>> >>>> This is a good example - thanks. To be able to cope with such cases we >>>> will have to pass a hwlock block reference and its relative lock id. >>>> >>> >>> Correct, so the #hwlock-cells and hwlock part from the proposal are >>> the important one. Having an optional hwlock-names will make things >>> easier to read as well, but is not necessary. >> >> Right, if anything, it would be useful only for the clients, but the >> hwspinlock core itself would not need it. So, I would forgo adding the >> hwlock-names for now. >> >>> >>>> The DT binding should definitely be prepared for such cases (just kill >>>> the base-id field?), but let's see what it means about the Linux >>>> implementation. >>>> >>> >>> From the dt binding PoV, we should be able to skip num-locks as well. >>> It seems most hwlock blocks have a fixed amount of locks provided and >>> the drivers are reporting this to the core when registering. >> >> I added this originally based on the initial MSM HW Mutex block bindings. >> > > It's not entirely correct to have this in DT for the MSM HW, as the > hardware has a fixed number of mutexes. As soon as we have the binding > sorted out I will follow up with a new revision of the tcsr/sfpb-mutex > driver. > >>> >>> So I think we can reduce the binding to: >>> >>> Providers: >>> #hwlock-cells >>> >>> Consumers: >>> hwlocks >>> hwlock-names >>> >>> For the hardware where number of locks is actually variable (e.g. >>> different variants of same block) there can be driver specific entries >>> for this. >> >> Right, we should be able to drop this and use the driver match data. As >> it is, the field is used during registration of the block with the >> hwspinlock core. >> > > If we have certain systems where it actually is a property to be > configured then they can have individual properties, extending the > standard set. Either way, it's not a dynamic property shared by all > hwlock drivers, so it should not be in the common binding. > > Will you send out a new revision of the binding? I would love to get > this integrated so I can move on with the dependents. Yep, will do as soon as I hear from Ohad on what to do with the patch "hwspinlock/core: maintain a list of registered hwspinlock banks" that I dropped from v7. Without that and dropping hwlock-base-id, I can't get the translations done. regards Suman -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 5, 2015 at 4:11 PM, Suman Anna <s-anna@ti.com> wrote: > Hi Bjorn, > > On 02/05/2015 05:01 PM, Bjorn Andersson wrote: [..] >> Will you send out a new revision of the binding? I would love to get >> this integrated so I can move on with the dependents. > > Yep, will do as soon as I hear from Ohad on what to do with the patch > "hwspinlock/core: maintain a list of registered hwspinlock banks" that I > dropped from v7. Without that and dropping hwlock-base-id, I can't get > the translations done. > My suggestion is to replace the global id-tree with a list of hwlocks and iterate over these if we ever get more than one driver registered. As long as #hwlock-drivers < log(#total-hwlocks) this should be faster. I would however argue that clients that would notice any kind of difference are using the API incorrectly. Unfortunately this is a somewhat larger change than just slapping DT support on the framework :/ Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 6, 2015 at 2:34 AM, Bjorn Andersson <bjorn@kryo.se> wrote: >> Yep, will do as soon as I hear from Ohad on what to do with the patch >> "hwspinlock/core: maintain a list of registered hwspinlock banks" that I >> dropped from v7. Without that and dropping hwlock-base-id, I can't get >> the translations done. >> > > My suggestion is to replace the global id-tree with a list of hwlocks > and iterate over these if we ever get more than one driver registered. > As long as #hwlock-drivers < log(#total-hwlocks) this should be > faster. > > I would however argue that clients that would notice any kind of > difference are using the API incorrectly. > > Unfortunately this is a somewhat larger change than just slapping DT > support on the framework :/ I suspect we want to wait with framework changes until there's a real hardware use case justifying them. With regard to DT, however, we obviously do want to be prepared for multiple hwlock blocks even if they do not exist today. So how about adopting an approach where: - DT fully supports multi hwlock blocks (i.e. no global id field) - Framework left mostly unchanged at the moment. This means issuing an explicit error in case a secondary hwlock block shows up, and then hwlock id could trivially be the lock index. If multi hwlock hardware use case, or some new hwlock id translation requirements, do show up in the future, it's always easy to add framework support for it. At that point we'll know better what the requirements are, and framework changes would be justifiable. Thanks, Ohad. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <tony@atomide.com> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Sorry I've been away from this thread for a while. On Wed, Feb 11, 2015 at 10:29:37AM +0000, Ohad Ben-Cohen wrote: > On Fri, Feb 6, 2015 at 2:34 AM, Bjorn Andersson <bjorn@kryo.se> wrote: > >> Yep, will do as soon as I hear from Ohad on what to do with the patch > >> "hwspinlock/core: maintain a list of registered hwspinlock banks" that I > >> dropped from v7. Without that and dropping hwlock-base-id, I can't get > >> the translations done. > >> > > > > My suggestion is to replace the global id-tree with a list of hwlocks > > and iterate over these if we ever get more than one driver registered. > > As long as #hwlock-drivers < log(#total-hwlocks) this should be > > faster. > > > > I would however argue that clients that would notice any kind of > > difference are using the API incorrectly. > > > > Unfortunately this is a somewhat larger change than just slapping DT > > support on the framework :/ > > I suspect we want to wait with framework changes until there's a real > hardware use case justifying them. > > With regard to DT, however, we obviously do want to be prepared for > multiple hwlock blocks even if they do not exist today. > > So how about adopting an approach where: > - DT fully supports multi hwlock blocks (i.e. no global id field) > - Framework left mostly unchanged at the moment. This means issuing an > explicit error in case a secondary hwlock block shows up, and then > hwlock id could trivially be the lock index. > > If multi hwlock hardware use case, or some new hwlock id translation > requirements, do show up in the future, it's always easy to add > framework support for it. At that point we'll know better what the > requirements are, and framework changes would be justifiable. As mentioned in my other reply I think we need to be explicit now when defining the set of hwlocks (and their namming/numbering) shared between a given set of processors, as we do with other resources (GPIOs/regulators/whatever). We also need to be explicit in describing the set of actors which use those locks. I think my previous proposal covered both of those. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 11, 2015 at 3:29 AM, Mark Rutland <mark.rutland@arm.com> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: [..] > Since the existence of several hwblocks is still fictional (Bjorn, > please confirm too?), we may prefer to introduce changes to support it > only when it shows up; it all depends on the amount of changes needed. > Suman, care to take a look please? > It turns out that the Qualcomm platforms have two additional "remote spinlock" mechanisms - both using shared memory. On most platforms only 1 (out of these 3) is actually consumed, but e.g. the older msm7x30 uses 2 of them (SMEM and DAL). Due to its age I don't think it's on anyones todo list to actually support this platform as of today; but it's out there. None of these hwlocks are allocated dynamically, so if needed a dynamic base_id (-1 like other frameworks) would be a acceptable solution - and can easily be implemented when we need it. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index 000000000000..8de7aaf878f9 --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt @@ -0,0 +1,55 @@ +Generic hwlock bindings +======================= + +Generic bindings that are common to all the hwlock platform specific driver +implementations. + +The validity and need of these common properties may vary from one platform +implementation to another. The platform specific bindings should explicitly +state if an optional property is used. Please also look through the individual +platform specific hwlock binding documentations for identifying the applicable +properties. + +Common properties: +- #hwlock-cells: Specifies the number of cells needed to represent a + specific lock. This property is mandatory for all + platform implementations. +- hwlock-num-locks: Number of locks present in a hwlock device. This + property is needed on hwlock devices, where the number + of supported locks within a hwlock device cannot be + read from a register. +- hwlock-base-id: An unique base Id for the locks for a particular hwlock + device. This property is mandatory for all platform + implementations. + +Hwlock Users: +============= + +Nodes that require specific hwlock(s) should specify them using the property +"hwlocks", each containing a phandle to the hwlock node and an args specifier +value as indicated by #hwlock-cells. Multiple hwlocks can be requested using +an array of the phandle and hwlock number specifier tuple. + +1. Example of a node using a single specific hwlock: + +The following example has a node requesting a hwlock in the bank defined by +the node hwlock1. hwlock1 is a hwlock provider with an argument specifier +of length 1. + + node { + ... + hwlocks = <&hwlock1 2>; + ... + }; + +2. Example of a node using multiple specific hwlocks: + +The following example has a node requesting two hwlocks, a hwlock within +the hwlock device node 'hwlock1' with #hwlock-cells value of 1, and another +hwlock within the hwlock device node 'hwlock2' with #hwlock-cells value of 2. + + node { + ... + hwlocks = <&hwlock1 2>, <&hwlock2 0 3>; + ... + };
This patch adds the generic common bindings used to represent a hwlock device and use/request locks in a device-tree build. All the platform-specific hwlock driver implementations need the number of locks and associated base id for registering the locks present within the device with the driver core. This base id needs to be unique across multiple IP instances of a hwspinlock device, so that each hwlock can be represented uniquely in a system. The number of locks is represented by 'hwlock-num-locks' property, and the base id is represented by the 'hwlock-base-id' property. The args specifier length is dependent on each vendor-specific implementation and is represented through the '#hwlock-cells' property. Client users need to use the property 'hwlocks' for requesting specific lock(s). Note that the document is named hwlock.txt deliberately to keep it a bit more generic. Cc: Rob Herring <robh+dt@kernel.org> Signed-off-by: Suman Anna <s-anna@ti.com> --- v7: Revised binding info for hwlock-base-id, it is mandatory now .../devicetree/bindings/hwlock/hwlock.txt | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt