| Message ID | e42576b69ef3d4e624fbfa2f32f6f79a931b55d6.1377202730.git.joshc@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/22/2013 01:59 PM, Josh Cartwright wrote: > Signed-off-by: Josh Cartwright <joshc@codeaurora.org> > --- > I'm introducing this as an RFC, because there are set of assumptions > made in this binding spec, that currently hold true for the supported > controller/addressing scheme for the Snapdragon 800 series, but don't > necessarily hold true for SPMI in general. > > 1. No use of Group Slave Identifiers (GSIDs) > (SPMI allows for a slave to belong to zero or more groups specified > by GSID, however this feature isn't currently implemented) > > 2. No specification of Master Identifier (MID) > (A "system integrator" allocates to each master a 2-bit MID, this > currently isn't being specified, as it isn't needed by software for > the PMIC Arb; not sure if this is of use to other SPMI controllers) > > 3. Single SPMI master per controller > > Effectively, only a subset of possible SPMI configurations are specified > in this document. > > So, if it's considered necessary to provide a generic SPMI binding > specification, is it acceptable to only define a subset at this time, > expanding only when necessary, or shall I expand the definition to at > least address 1 & 2, even though they are of no use in the current > implementation? It's best to define the whole thing from the start if possible. It's easier to ensure the whole binding is consistent, and nothing has been left out. However, it's probably OK to define a subset binding initially and then expand it later, as long as some thought it put into how it can be expanded in a way that is 100% compatible: old DTs will still operate with new kernels and perhaps even new DTs will still operate with old kernels. That said, if the thought is put in to ensure that's possible, it's probably just as easy to define the whole binding from the start.
Hey Stephen- Thanks for the comments. On Fri, Aug 23, 2013 at 03:58:36PM -0600, Stephen Warren wrote: > On 08/22/2013 01:59 PM, Josh Cartwright wrote: > > Signed-off-by: Josh Cartwright <joshc@codeaurora.org> > > --- > > I'm introducing this as an RFC, because there are set of assumptions > > made in this binding spec, that currently hold true for the supported > > controller/addressing scheme for the Snapdragon 800 series, but don't > > necessarily hold true for SPMI in general. > > > > 1. No use of Group Slave Identifiers (GSIDs) > > (SPMI allows for a slave to belong to zero or more groups specified > > by GSID, however this feature isn't currently implemented) > > > > 2. No specification of Master Identifier (MID) > > (A "system integrator" allocates to each master a 2-bit MID, this > > currently isn't being specified, as it isn't needed by software for > > the PMIC Arb; not sure if this is of use to other SPMI controllers) > > > > 3. Single SPMI master per controller > > > > Effectively, only a subset of possible SPMI configurations are specified > > in this document. > > > > So, if it's considered necessary to provide a generic SPMI binding > > specification, is it acceptable to only define a subset at this time, > > expanding only when necessary, or shall I expand the definition to at > > least address 1 & 2, even though they are of no use in the current > > implementation? > > It's best to define the whole thing from the start if possible. It's > easier to ensure the whole binding is consistent, and nothing has been > left out. That makes sense. I think I'll go down this route for v3 of this patchset: For #1 above, extend the 'reg' property of a slave node to include the group slave ID's in which the slave is a member. The first 'reg' entry will remain the slave's Unique Slave Identifier (USID) as before. For #2, add additional required 'spmi-mid' property in the controller/master node that describes the 2-bit Master Identifier (MID). For #3, rename the SPMI API's s/controller/master/. The current controller/master terminology difference is confusing and unnecessary. > However, it's probably OK to define a subset binding initially and then > expand it later, as long as some thought it put into how it can be > expanded in a way that is 100% compatible: old DTs will still operate > with new kernels and perhaps even new DTs will still operate with old > kernels. > > That said, if the thought is put in to ensure that's possible, it's > probably just as easy to define the whole binding from the start. That all makes sense. If we want to ensure for the generic bindings that we are fulling characterizing/describing the SPMI bus, then we'll additionally need to tackle an additional identified assumption: 4. One master per SPMI bus. (The SPMI spec allows for up to 4 masters) On the Snapdragon 800 series, there exists only one software-controlled master, but it is conceivably possible to have a setup with two software-controlled masters on the same SPMI bus. This necessarily means that the description of the slaves and the masters will need to be decoupled; I'm imagining a generic binding supporting multiple masters would look something like this: master0: master@0 { compatible = "..."; #spmi-master-cells = <0>; spmi-mid = <0>; ... }; master2: master@2 { compatible = "..."; #spmi-master-cells = <0>; spmi-mid = <2>; ... }; spmi_bus { compatible = "..."; spmi-masters = <&master0 &master2>; foo@0 { compatible = "..."; reg = <0 ...>; }; foo@8 { compatible = "..."; reg = <8 ...>; }; }; (This will also necessitate a change in the underlying SPMI driver model, in the current implementation, a SPMI master 'owns' a particular device. This is not a valid assumption to make.) Would this property-containing-phandle-vector be considered the canonical way of representing nodes with multiple parents in the device tree? Thanks, Josh
On 08/27/2013 11:01 AM, Josh Cartwright wrote: ... > If we want to ensure for the generic bindings that we are fulling > characterizing/describing the SPMI bus, then we'll additionally need to > tackle an additional identified assumption: > > 4. One master per SPMI bus. (The SPMI spec allows for up to 4 > masters) > > On the Snapdragon 800 series, there exists only one software-controlled > master, but it is conceivably possible to have a setup with two > software-controlled masters on the same SPMI bus. > > This necessarily means that the description of the slaves and the > masters will need to be decoupled; I'm imagining a generic binding > supporting multiple masters would look something like this: Is there a need to represent the other masters in the DT? Sure they're there in HW, but if there's no specific way for the CPU-to-which-the-DT-applies to actually interact with those other masters (except perhaps by experiencing some arbitration delays) then presumably there's no need to represent the other masters in DT? > master0: master@0 { > compatible = "..."; > #spmi-master-cells = <0>; > spmi-mid = <0>; > > ... > }; > > master2: master@2 { > compatible = "..."; > #spmi-master-cells = <0>; > spmi-mid = <2>; > > ... > }; > > spmi_bus { > compatible = "..."; > > spmi-masters = <&master0 &master2>; > > foo@0 { > compatible = "..."; > reg = <0 ...>; > }; > > foo@8 { > compatible = "..."; > reg = <8 ...>; > }; > }; > > (This will also necessitate a change in the underlying SPMI driver > model, in the current implementation, a SPMI master 'owns' a particular > device. This is not a valid assumption to make.) > > Would this property-containing-phandle-vector be considered the > canonical way of representing nodes with multiple parents in the device > tree? I don't think I've seen anything like this before, although that in-and-of-itself doesn't make it wrong. Another approach might be to encode master-vs-slave into a cell in the reg property? Something like: cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...) cell 1 - address value I haven't thought much about that; perhaps there are disadvantages doing that.
On Tue, Aug 27, 2013 at 03:55:19PM -0600, Stephen Warren wrote: > On 08/27/2013 11:01 AM, Josh Cartwright wrote: > ... > > If we want to ensure for the generic bindings that we are fulling > > characterizing/describing the SPMI bus, then we'll additionally need to > > tackle an additional identified assumption: > > > > 4. One master per SPMI bus. (The SPMI spec allows for up to 4 > > masters) > > > > On the Snapdragon 800 series, there exists only one software-controlled > > master, but it is conceivably possible to have a setup with two > > software-controlled masters on the same SPMI bus. > > > > This necessarily means that the description of the slaves and the > > masters will need to be decoupled; I'm imagining a generic binding > > supporting multiple masters would look something like this: > > Is there a need to represent the other masters in the DT? Sure they're > there in HW, but if there's no specific way for the > CPU-to-which-the-DT-applies to actually interact with those other > masters (except perhaps by experiencing some arbitration delays) then > presumably there's no need to represent the other masters in DT? My example is contrived, but there is nothing in the SPMI spec preventing two masters from being controllable by the same CPU-to-which-the-DT-applies, sharing the same underlying bus. I would also expect this configuration to be uncommon, I'm checking with some folks with more SPMI experience to make sure they agree. Interestingly, i2c as far as I can tell has also made the same assumption. There doesn't appear to be any way to express a multi-master i2c setup where both masters are controlled by the same OS. > > master0: master@0 { > > compatible = "..."; > > #spmi-master-cells = <0>; > > spmi-mid = <0>; > > > > ... > > }; > > > > master2: master@2 { > > compatible = "..."; > > #spmi-master-cells = <0>; > > spmi-mid = <2>; > > > > ... > > }; > > > > spmi_bus { > > compatible = "..."; > > > > spmi-masters = <&master0 &master2>; > > > > foo@0 { > > compatible = "..."; > > reg = <0 ...>; > > }; > > > > foo@8 { > > compatible = "..."; > > reg = <8 ...>; > > }; > > }; > > > > (This will also necessitate a change in the underlying SPMI driver > > model, in the current implementation, a SPMI master 'owns' a particular > > device. This is not a valid assumption to make.) > > > > Would this property-containing-phandle-vector be considered the > > canonical way of representing nodes with multiple parents in the device > > tree? > > I don't think I've seen anything like this before, although that > in-and-of-itself doesn't make it wrong. > > Another approach might be to encode master-vs-slave into a cell in the > reg property? Something like: > > cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...) > cell 1 - address value > > I haven't thought much about that; perhaps there are disadvantages doing > that.
On 08/28/2013 12:00 PM, Josh Cartwright wrote: > On Tue, Aug 27, 2013 at 03:55:19PM -0600, Stephen Warren wrote: >> On 08/27/2013 11:01 AM, Josh Cartwright wrote: >> ... >>> If we want to ensure for the generic bindings that we are fulling >>> characterizing/describing the SPMI bus, then we'll additionally need to >>> tackle an additional identified assumption: >>> >>> 4. One master per SPMI bus. (The SPMI spec allows for up to 4 >>> masters) >>> >>> On the Snapdragon 800 series, there exists only one software-controlled >>> master, but it is conceivably possible to have a setup with two >>> software-controlled masters on the same SPMI bus. >>> >>> This necessarily means that the description of the slaves and the >>> masters will need to be decoupled; I'm imagining a generic binding >>> supporting multiple masters would look something like this: >> >> Is there a need to represent the other masters in the DT? Sure they're >> there in HW, but if there's no specific way for the >> CPU-to-which-the-DT-applies to actually interact with those other >> masters (except perhaps by experiencing some arbitration delays) then >> presumably there's no need to represent the other masters in DT? > > My example is contrived, but there is nothing in the SPMI spec > preventing two masters from being controllable by the same > CPU-to-which-the-DT-applies, sharing the same underlying bus. That's true. > I would also expect this configuration to be uncommon, I'm checking with > some folks with more SPMI experience to make sure they agree. > > Interestingly, i2c as far as I can tell has also made the same > assumption. There doesn't appear to be any way to express a > multi-master i2c setup where both masters are controlled by the same OS. Yes, I think it's a fair assumption that we don't need to represent that; I immediately thought about the I2C counter-example after reading your first paragraph.
On Tue, Aug 27, 2013 at 2:55 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 08/27/2013 11:01 AM, Josh Cartwright wrote: > ... > cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...) I think it would make sense to have the master id as a property of the bus, as you could consider this to indicate different buses and then usid, gsid and base being part of the reg. > cell 1 - address value I did hack up Josh patchset to read a reg touple of <usid, base> instead of just usid. I stored the second value in the spmi_device struct for easy access, but maybe it should be done like on codeaurora; in a resource? I believe this looks nice, but as I haven't read the mipi spec I wonder, will there be a case where you don't have an offset/base? Should it just be made optional? Can we make the address <usid, [base]> and have the code populate a resource based on a reg-names property? That way it would be possible to extend it to support gsid in case we want to (would require reg-names though). With the hack to Josh's patchset I quickly ported qpnp-revision and qpnp-vibrator, and it seems to work quite nicely. Regards, Bjorn
Hey Bjorn- On Sat, Oct 05, 2013 at 11:11:36PM -0700, Bjorn Andersson wrote: > On Tue, Aug 27, 2013 at 2:55 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > > On 08/27/2013 11:01 AM, Josh Cartwright wrote: > > ... > > cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...) > > I think it would make sense to have the master id as a property of the > bus, Except that SPMI supports bus configurations with multiple masters. Unless by 'bus' you meant 'bus controller' here? > as you could consider this to indicate different buses and then > usid, gsid and base being part of the reg. > > > cell 1 - address value > > I did hack up Josh patchset to read a reg touple of <usid, base> > instead of just usid. I stored the second value in the spmi_device > struct for easy access, but maybe it should be done like on > codeaurora; in a resource? > I believe this looks nice, but as I haven't read the mipi spec I > wonder, will there be a case where you don't have an offset/base? > Should it just be made optional? The SPMI spec says nothing about partitioning up the slave address to support multiple functions. AFAICT, this is a Qualcomm-created construct (QPNP) for the 8x41 PMICs. It's difficult to tell at this point whether or not other vendors might implement a similarly partitioned scheme. I suspect the intent is that implementations make use of logical slave ID for each function in a multi-function device. > Can we make the address <usid, [base]> and have the code populate a > resource based on a reg-names property? That way it would be possible > to extend it to support gsid in case we want to (would require > reg-names though). It is certainly possible, and, as you've seen, is how the current codeaurora.org tree implements SPMI. But, I'm actually actively trying to avoid doing so, as it conflates Qualcomm-implementation details and what is actually in the spec (and not just the address space partitioning, but also the of_spmi.c[2] parsing must know about interrupts, which are _also_ completely outside the SPMI spec). Instead what I hope to do for v3 is either: A) Make QPNP its own bus type (for which I have a prototyped implementation). A PMIC driver sits on the SPMI bus and registers itself as a QPNP controller. QPNP controllers have very simple 8-bit register read/write operations used by QPNP devices. B) Effectively the same as A, but gets rid of a special QPNP bus type and uses mfd/platform devices, similar to other in-tree PMIC drivers (currently working on prototyping this approach) > With the hack to Josh's patchset I quickly ported qpnp-revision and > qpnp-vibrator, and it seems to work quite nicely. Great! Thanks for testing. Josh 1: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/spmi 2: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/of/of_spmi.c
diff --git a/Documentation/devicetree/bindings/spmi/spmi.txt b/Documentation/devicetree/bindings/spmi/spmi.txt new file mode 100644 index 0000000..a01b064 --- /dev/null +++ b/Documentation/devicetree/bindings/spmi/spmi.txt @@ -0,0 +1,36 @@ +System Power Management Interface (SPMI) Controller + +This document defines a generic set of bindings for use by SPMI controllers. A +controller is modelled in device tree as a node with zero or more child nodes, +each representing a unique slave on the bus. + +Required properties: +- #address-cells : must be set to 1 +- #size-cells : must be set to 0 + +Child nodes: + +An SPMI controller node can contain zero or more children. Each child must +have a reg property defining its 4-bit Unique Slave Identifier (USID) on the +SPMI bus. This is the ID that has been "statically assigned by the system +integrator", as per the SPMI spec. + +Each child node represents a slave device on the bus. + + controller@.. { + compatible = "..."; + reg = <...>; + + #address-cells = <1>; + #size-cells <0>; + + child@0 { + compatible = "..."; + reg = <0>; + }; + + child@7 { + compatible = "..."; + reg = <7>; + }; + };
Signed-off-by: Josh Cartwright <joshc@codeaurora.org> --- I'm introducing this as an RFC, because there are set of assumptions made in this binding spec, that currently hold true for the supported controller/addressing scheme for the Snapdragon 800 series, but don't necessarily hold true for SPMI in general. 1. No use of Group Slave Identifiers (GSIDs) (SPMI allows for a slave to belong to zero or more groups specified by GSID, however this feature isn't currently implemented) 2. No specification of Master Identifier (MID) (A "system integrator" allocates to each master a 2-bit MID, this currently isn't being specified, as it isn't needed by software for the PMIC Arb; not sure if this is of use to other SPMI controllers) 3. Single SPMI master per controller Effectively, only a subset of possible SPMI configurations are specified in this document. So, if it's considered necessary to provide a generic SPMI binding specification, is it acceptable to only define a subset at this time, expanding only when necessary, or shall I expand the definition to at least address 1 & 2, even though they are of no use in the current implementation? Documentation/devicetree/bindings/spmi/spmi.txt | 36 +++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/spmi/spmi.txt