| Message ID | 20190808144504.24823-2-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: codecs: Add WSA881x Smart Speaker amplifier support | expand |
> +++ b/Documentation/devicetree/bindings/soundwire/slave.txt > @@ -0,0 +1,46 @@ > +SoundWire slave device bindings. > + > +SoundWire is a 2-pin multi-drop interface with data and clock line. > +It facilitates development of low cost, efficient, high performance systems. > + > +SoundWire slave devices: > +Every SoundWire controller node can contain zero or more child nodes > +representing slave devices on the bus. Every SoundWire slave device is > +uniquely determined by the enumeration address containing 5 fields: > +SoundWire Version, Instance ID, Manufacturer ID, Part ID and Class ID > +for a device. Addition to below required properties, child nodes can > +have device specific bindings. In case the controller supports multiple links, what's the encoding then? in the MIPI DisCo spec there is a linkId field in the _ADR encoding that helps identify which link the Slave device is connected to > + > +Required property for SoundWire child node if it is present: > +- compatible: "sdwVER,MFD,PID,CID". The textual representation of > + SoundWire Enumeration address comprising SoundWire > + Version, Manufacturer ID, Part ID and Class ID, > + shall be in lower-case hexadecimal with leading > + zeroes suppressed. > + Version number '0x10' represents SoundWire 1.0 > + Version number '0x11' represents SoundWire 1.1 > + ex: "sdw10,0217,2010,0" > + > +- sdw-instance-id: Should be ('Instance ID') from SoundWire > + Enumeration Address. Instance ID is for the cases > + where multiple Devices of the same type or Class > + are attached to the bus. so it is actually required if you have a single Slave device? Or is it only required when you have more than 1 device of the same type? FWIW in the MIPI DisCo spec we kept the instanceID as part of the _ADR, so it's implicitly mandatory (and ignored by the bus if there is only one device of the same time) > + > +SoundWire example for Qualcomm's SoundWire controller: > + > +soundwire@c2d0000 { > + compatible = "qcom,soundwire-v1.5.0" > + reg = <0x0c2d0000 0x2000>; > + > + spkr_left:wsa8810-left{ > + compatible = "sdw10,0217,2010,0"; > + sdw-instance-id = <1>; > + ... > + }; > + > + spkr_right:wsa8810-right{ > + compatible = "sdw10,0217,2010,0"; > + sdw-instance-id = <2>; Isn't the MIPI encoding reported in the Dev_ID0..5 registers 0-based? > + ... > + }; > +}; > And now that I think of it, wouldn't it be simpler for everyone if we aligned on that MIPI DisCo public spec? e.g. you'd have one property with a 64-bit number that follows the MIPI spec. No special encoding necessary for device tree cases, your DT blob would use this: soundwire@c2d0000 { compatible = "qcom,soundwire-v1.5.0" reg = <0x0c2d0000 0x2000>; spkr_left:wsa8810-left{ compatible = "sdw0000100217201000" } spkr_right:wsa8810-right{ compatible = "sdw0000100217201100" } } We could use parentheses if it makes people happier, but the information from the MIPI DisCo spec can be used as is, and provide a means for spec changes via reserved bits.
On 08/08/2019 16:58, Pierre-Louis Bossart wrote: > >> +++ b/Documentation/devicetree/bindings/soundwire/slave.txt >> @@ -0,0 +1,46 @@ >> +SoundWire slave device bindings. >> + >> +SoundWire is a 2-pin multi-drop interface with data and clock line. >> +It facilitates development of low cost, efficient, high performance >> systems. >> + >> +SoundWire slave devices: >> +Every SoundWire controller node can contain zero or more child nodes >> +representing slave devices on the bus. Every SoundWire slave device is >> +uniquely determined by the enumeration address containing 5 fields: >> +SoundWire Version, Instance ID, Manufacturer ID, Part ID and Class ID >> +for a device. Addition to below required properties, child nodes can >> +have device specific bindings. > > In case the controller supports multiple links, what's the encoding then? > in the MIPI DisCo spec there is a linkId field in the _ADR encoding that > helps identify which link the Slave device is connected to > >> + >> +Required property for SoundWire child node if it is present: >> +- compatible: "sdwVER,MFD,PID,CID". The textual representation of >> + SoundWire Enumeration address comprising SoundWire >> + Version, Manufacturer ID, Part ID and Class ID, >> + shall be in lower-case hexadecimal with leading >> + zeroes suppressed. >> + Version number '0x10' represents SoundWire 1.0 >> + Version number '0x11' represents SoundWire 1.1 >> + ex: "sdw10,0217,2010,0" >> + >> +- sdw-instance-id: Should be ('Instance ID') from SoundWire >> + Enumeration Address. Instance ID is for the cases >> + where multiple Devices of the same type or Class >> + are attached to the bus. > > so it is actually required if you have a single Slave device? Or is it > only required when you have more than 1 device of the same type? > This is mandatory for any slave device! > FWIW in the MIPI DisCo spec we kept the instanceID as part of the _ADR, > so it's implicitly mandatory (and ignored by the bus if there is only > one device of the same time) > >> + >> +SoundWire example for Qualcomm's SoundWire controller: >> + >> +soundwire@c2d0000 { >> + compatible = "qcom,soundwire-v1.5.0" >> + reg = <0x0c2d0000 0x2000>; >> + >> + spkr_left:wsa8810-left{ >> + compatible = "sdw10,0217,2010,0"; >> + sdw-instance-id = <1>; >> + ... >> + }; >> + >> + spkr_right:wsa8810-right{ >> + compatible = "sdw10,0217,2010,0"; >> + sdw-instance-id = <2>; > > Isn't the MIPI encoding reported in the Dev_ID0..5 registers 0-based? > >> + ... >> + }; >> +}; >> > > And now that I think of it, wouldn't it be simpler for everyone if we > aligned on that MIPI DisCo public spec? e.g. you'd have one property > with a 64-bit number that follows the MIPI spec. No special encoding > necessary for device tree cases, your DT blob would use this: Thanks for the suggestion, adding 64 device bits as compatible string should take care of linkID too. I will give that a go! > > soundwire@c2d0000 { > compatible = "qcom,soundwire-v1.5.0" > reg = <0x0c2d0000 0x2000>; > > spkr_left:wsa8810-left{ > compatible = "sdw00 00 10 02 17 20 10 00" > } > > spkr_right:wsa8810-right{ > compatible = "sdw0000100217201100" > } > } > > We could use parentheses if it makes people happier, but the information > from the MIPI DisCo spec can be used as is, and provide a means for spec > changes via reserved bits.
On Thu, Aug 08, 2019 at 05:48:56PM +0100, Srinivas Kandagatla wrote: > On 08/08/2019 16:58, Pierre-Louis Bossart wrote: > > > +- sdw-instance-id: Should be ('Instance ID') from SoundWire > > > + Enumeration Address. Instance ID is for the cases > > > + where multiple Devices of the same type or Class > > > + are attached to the bus. > > so it is actually required if you have a single Slave device? Or is it > > only required when you have more than 1 device of the same type? > This is mandatory for any slave device! If it's mandatory the wording is a bit unclear. How about something like: Should be ('Instance ID') from the SoundWire Enumeration Address. This must always be provided, if multiple devices with the same type or class or attached to the bus each instance must have a distinct value.
On 08-08-19, 20:52, Mark Brown wrote: > On Thu, Aug 08, 2019 at 05:48:56PM +0100, Srinivas Kandagatla wrote: > > On 08/08/2019 16:58, Pierre-Louis Bossart wrote: > > > > > +- sdw-instance-id: Should be ('Instance ID') from SoundWire > > > > + Enumeration Address. Instance ID is for the cases > > > > + where multiple Devices of the same type or Class > > > > + are attached to the bus. > > > > so it is actually required if you have a single Slave device? Or is it > > > only required when you have more than 1 device of the same type? > > > This is mandatory for any slave device! > > If it's mandatory the wording is a bit unclear. How about something > like: > > Should be ('Instance ID') from the SoundWire Enumeration > Address. This must always be provided, if multiple devices > with the same type or class or attached to the bus each > instance must have a distinct value. That helps to make it clear. Also the section of properties starts with Mandatory property, it should be made Mandatory Properties instead, like in other binding docs to make it clear that properties mentioned in the section are mandatory
On 08-08-19, 15:45, Srinivas Kandagatla wrote: > This patch adds bindings for Soundwire Slave devices which includes how > SoundWire enumeration address is represented in SoundWire slave device > tree nodes. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > .../devicetree/bindings/soundwire/slave.txt | 46 +++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soundwire/slave.txt > > diff --git a/Documentation/devicetree/bindings/soundwire/slave.txt b/Documentation/devicetree/bindings/soundwire/slave.txt > new file mode 100644 > index 000000000000..b8e8d34bbc92 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soundwire/slave.txt > @@ -0,0 +1,46 @@ > +SoundWire slave device bindings. > + > +SoundWire is a 2-pin multi-drop interface with data and clock line. > +It facilitates development of low cost, efficient, high performance systems. > + > +SoundWire slave devices: > +Every SoundWire controller node can contain zero or more child nodes > +representing slave devices on the bus. Every SoundWire slave device is > +uniquely determined by the enumeration address containing 5 fields: > +SoundWire Version, Instance ID, Manufacturer ID, Part ID and Class ID > +for a device. Addition to below required properties, child nodes can It would help to list them rather than free flowing text > +have device specific bindings. > + > +Required property for SoundWire child node if it is present: As said earlier, lets make it "Required properties:" > +- compatible: "sdwVER,MFD,PID,CID". The textual representation of > + SoundWire Enumeration address comprising SoundWire > + Version, Manufacturer ID, Part ID and Class ID, > + shall be in lower-case hexadecimal with leading > + zeroes suppressed. > + Version number '0x10' represents SoundWire 1.0 > + Version number '0x11' represents SoundWire 1.1 > + ex: "sdw10,0217,2010,0" > + > +- sdw-instance-id: Should be ('Instance ID') from SoundWire > + Enumeration Address. Instance ID is for the cases > + where multiple Devices of the same type or Class > + are attached to the bus. > + > +SoundWire example for Qualcomm's SoundWire controller: > + > +soundwire@c2d0000 { > + compatible = "qcom,soundwire-v1.5.0" > + reg = <0x0c2d0000 0x2000>; > + > + spkr_left:wsa8810-left{ > + compatible = "sdw10,0217,2010,0"; > + sdw-instance-id = <1>; > + ... > + }; > + > + spkr_right:wsa8810-right{ > + compatible = "sdw10,0217,2010,0"; > + sdw-instance-id = <2>; > + ... > + }; > +}; > -- > 2.21.0
On 09/08/2019 05:54, Vinod Koul wrote: > On 08-08-19, 20:52, Mark Brown wrote: >> On Thu, Aug 08, 2019 at 05:48:56PM +0100, Srinivas Kandagatla wrote: >>> On 08/08/2019 16:58, Pierre-Louis Bossart wrote: >> >>>>> +- sdw-instance-id: Should be ('Instance ID') from SoundWire >>>>> + Enumeration Address. Instance ID is for the cases >>>>> + where multiple Devices of the same type or Class >>>>> + are attached to the bus. >> >>>> so it is actually required if you have a single Slave device? Or is it >>>> only required when you have more than 1 device of the same type? >> >>> This is mandatory for any slave device! >> >> If it's mandatory the wording is a bit unclear. How about something >> like: >> >> Should be ('Instance ID') from the SoundWire Enumeration >> Address. This must always be provided, if multiple devices >> with the same type or class or attached to the bus each >> instance must have a distinct value. > > That helps to make it clear. > > Also the section of properties starts with Mandatory property, it should > be made Mandatory Properties instead, like in other binding docs to make > it clear that properties mentioned in the section are mandatory Will update as suggested! thanks, srini >
diff --git a/Documentation/devicetree/bindings/soundwire/slave.txt b/Documentation/devicetree/bindings/soundwire/slave.txt new file mode 100644 index 000000000000..b8e8d34bbc92 --- /dev/null +++ b/Documentation/devicetree/bindings/soundwire/slave.txt @@ -0,0 +1,46 @@ +SoundWire slave device bindings. + +SoundWire is a 2-pin multi-drop interface with data and clock line. +It facilitates development of low cost, efficient, high performance systems. + +SoundWire slave devices: +Every SoundWire controller node can contain zero or more child nodes +representing slave devices on the bus. Every SoundWire slave device is +uniquely determined by the enumeration address containing 5 fields: +SoundWire Version, Instance ID, Manufacturer ID, Part ID and Class ID +for a device. Addition to below required properties, child nodes can +have device specific bindings. + +Required property for SoundWire child node if it is present: +- compatible: "sdwVER,MFD,PID,CID". The textual representation of + SoundWire Enumeration address comprising SoundWire + Version, Manufacturer ID, Part ID and Class ID, + shall be in lower-case hexadecimal with leading + zeroes suppressed. + Version number '0x10' represents SoundWire 1.0 + Version number '0x11' represents SoundWire 1.1 + ex: "sdw10,0217,2010,0" + +- sdw-instance-id: Should be ('Instance ID') from SoundWire + Enumeration Address. Instance ID is for the cases + where multiple Devices of the same type or Class + are attached to the bus. + +SoundWire example for Qualcomm's SoundWire controller: + +soundwire@c2d0000 { + compatible = "qcom,soundwire-v1.5.0" + reg = <0x0c2d0000 0x2000>; + + spkr_left:wsa8810-left{ + compatible = "sdw10,0217,2010,0"; + sdw-instance-id = <1>; + ... + }; + + spkr_right:wsa8810-right{ + compatible = "sdw10,0217,2010,0"; + sdw-instance-id = <2>; + ... + }; +};
This patch adds bindings for Soundwire Slave devices which includes how SoundWire enumeration address is represented in SoundWire slave device tree nodes. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- .../devicetree/bindings/soundwire/slave.txt | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 Documentation/devicetree/bindings/soundwire/slave.txt