| Message ID | 20190822233759.12663-2-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: codecs: Add WSA881x Smart Speaker amplifier support | expand |
On 23-08-19, 00:37, Srinivas Kandagatla wrote: > This patch adds bindings for Soundwire Slave devices that includes how > SoundWire enumeration address and Link ID are used to represented in > SoundWire slave device tree nodes. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > .../soundwire/soundwire-controller.yaml | 75 +++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > > diff --git a/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > new file mode 100644 > index 000000000000..91aa6c6d6266 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > @@ -0,0 +1,75 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soundwire/soundwire-controller.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: SoundWire Controller Generic Binding Controller does not make sense here, why not use spec terminology and say "SoundWire Slave Generic Binding" > + > +maintainers: > + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > + > +description: | > + SoundWire busses can be described with a node for the SoundWire controller > + device and a set of child nodes for each SoundWire slave on the bus. > + > +properties: > + $nodename: > + pattern: "^soundwire(@.*|-[0-9a-f])*$" > + > + "#address-cells": > + const: 2 > + > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^.*@[0-9a-f]+$": > + type: object > + > + properties: > + compatible: > + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" > + description: > + Is the textual representation of SoundWire Enumeration > + address. compatible string should contain SoundWire Version ID, > + Manufacturer ID, Part ID and Class ID in order and shall be in > + lower-case hexadecimal with leading zeroes. > + Valid sizes of these fields are > + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 > + and '0x2' represents SoundWire 1.1 and so on. > + MFD is 4 nibbles > + PID is 4 nibbles > + CID is 2 nibbles > + More Information on detail of encoding of these fields can be > + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. > + > + reg: > + maxItems: 1 > + description: > + Instance ID and Link ID of SoundWire Device Address. This looks better :) Thanks. Apart from the minor nit above this looks good to me, I can merge the sdw parts if Rob is fine with them. Thanks > + > + required: > + - compatible > + - reg > + > +examples: > + - | > + soundwire@c2d0000 { > + #address-cells = <2>; > + #size-cells = <0>; > + compatible = "qcom,soundwire-v1.5.0"; > + reg = <0x0c2d0000 0x2000>; > + > + speaker@1 { > + compatible = "sdw10217201000"; > + reg = <1 0>; > + }; > + > + speaker@2 { > + compatible = "sdw10217201000"; > + reg = <2 0>; > + }; > + }; > + > +... > -- > 2.21.0
On 8/22/19 6:37 PM, Srinivas Kandagatla wrote: > This patch adds bindings for Soundwire Slave devices that includes how > SoundWire enumeration address and Link ID are used to represented in > SoundWire slave device tree nodes. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > .../soundwire/soundwire-controller.yaml | 75 +++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > > diff --git a/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > new file mode 100644 > index 000000000000..91aa6c6d6266 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > @@ -0,0 +1,75 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soundwire/soundwire-controller.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: SoundWire Controller Generic Binding > + > +maintainers: > + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > + > +description: | > + SoundWire busses can be described with a node for the SoundWire controller > + device and a set of child nodes for each SoundWire slave on the bus. > + > +properties: > + $nodename: > + pattern: "^soundwire(@.*|-[0-9a-f])*$" > + > + "#address-cells": > + const: 2 > + > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^.*@[0-9a-f]+$": > + type: object > + > + properties: > + compatible: > + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" So is this a 64-bit value, as in the MIPI spec, or is this part of the _ADR description? I also don't get why the first item in in base10? > + description: > + Is the textual representation of SoundWire Enumeration > + address. compatible string should contain SoundWire Version ID, > + Manufacturer ID, Part ID and Class ID in order and shall be in > + lower-case hexadecimal with leading zeroes. > + Valid sizes of these fields are > + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 > + and '0x2' represents SoundWire 1.1 and so on. > + MFD is 4 nibbles > + PID is 4 nibbles > + CID is 2 nibbles > + More Information on detail of encoding of these fields can be > + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. > + > + reg: > + maxItems: 1 > + description: > + Instance ID and Link ID of SoundWire Device Address. > + > + required: > + - compatible > + - reg > + > +examples: > + - | > + soundwire@c2d0000 { > + #address-cells = <2>; > + #size-cells = <0>; > + compatible = "qcom,soundwire-v1.5.0"; > + reg = <0x0c2d0000 0x2000>; > + > + speaker@1 { > + compatible = "sdw10217201000"; > + reg = <1 0>; > + }; > + > + speaker@2 { > + compatible = "sdw10217201000"; > + reg = <2 0>; > + }; > + }; > + > +... >
On 23/08/2019 16:41, Pierre-Louis Bossart wrote: > > > On 8/22/19 6:37 PM, Srinivas Kandagatla wrote: >> This patch adds bindings for Soundwire Slave devices that includes how >> SoundWire enumeration address and Link ID are used to represented in >> SoundWire slave device tree nodes. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> .../soundwire/soundwire-controller.yaml | 75 +++++++++++++++++++ >> 1 file changed, 75 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >> >> new file mode 100644 >> index 000000000000..91aa6c6d6266 >> --- /dev/null >> +++ >> b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >> @@ -0,0 +1,75 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/soundwire/soundwire-controller.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: SoundWire Controller Generic Binding >> + >> +maintainers: >> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> + >> +description: | >> + SoundWire busses can be described with a node for the SoundWire >> controller >> + device and a set of child nodes for each SoundWire slave on the bus. >> + >> +properties: >> + $nodename: >> + pattern: "^soundwire(@.*|-[0-9a-f])*$" >> + >> + "#address-cells": >> + const: 2 >> + >> + "#size-cells": >> + const: 0 >> + >> +patternProperties: >> + "^.*@[0-9a-f]+$": >> + type: object >> + >> + properties: >> + compatible: >> + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" > > So is this a 64-bit value, as in the MIPI spec, or is this part of the > _ADR description? Rob did not like encoding compatible string exactly like _ADR encoding. https://lkml.org/lkml/2019/8/22/490 > I also don't get why the first item in in base10? > As this corresponds to Soundwire Version, and I have no visibility of version number encoding after reaching number 9 in this field. This can be updated once we have more info on how the Version encoding will look like in future. Idea of limiting regex to [0-9] for version is to enforce some checking! --srini > >> + description: >> + Is the textual representation of SoundWire Enumeration >> + address. compatible string should contain SoundWire Version ID, >> + Manufacturer ID, Part ID and Class ID in order and shall be in >> + lower-case hexadecimal with leading zeroes. >> + Valid sizes of these fields are >> + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 >> + and '0x2' represents SoundWire 1.1 and so on. >> + MFD is 4 nibbles >> + PID is 4 nibbles >> + CID is 2 nibbles >> + More Information on detail of encoding of these fields can be >> + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. >> + >> + reg: >> + maxItems: 1 >> + description: >> + Instance ID and Link ID of SoundWire Device Address. >> + >> + required: >> + - compatible >> + - reg >> + >> +examples: >> + - | >> + soundwire@c2d0000 { >> + #address-cells = <2>; >> + #size-cells = <0>; >> + compatible = "qcom,soundwire-v1.5.0"; >> + reg = <0x0c2d0000 0x2000>; >> + >> + speaker@1 { >> + compatible = "sdw10217201000"; >> + reg = <1 0>; >> + }; >> + >> + speaker@2 { >> + compatible = "sdw10217201000"; >> + reg = <2 0>; >> + }; >> + }; >> + >> +... >>
On 8/23/19 10:57 AM, Srinivas Kandagatla wrote: > > > On 23/08/2019 16:41, Pierre-Louis Bossart wrote: >> >> >> On 8/22/19 6:37 PM, Srinivas Kandagatla wrote: >>> This patch adds bindings for Soundwire Slave devices that includes how >>> SoundWire enumeration address and Link ID are used to represented in >>> SoundWire slave device tree nodes. >>> >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> --- >>> .../soundwire/soundwire-controller.yaml | 75 +++++++++++++++++++ >>> 1 file changed, 75 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>> b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>> new file mode 100644 >>> index 000000000000..91aa6c6d6266 >>> --- /dev/null >>> +++ >>> b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>> @@ -0,0 +1,75 @@ >>> +# SPDX-License-Identifier: GPL-2.0 >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/soundwire/soundwire-controller.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: SoundWire Controller Generic Binding >>> + >>> +maintainers: >>> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> + >>> +description: | >>> + SoundWire busses can be described with a node for the SoundWire >>> controller >>> + device and a set of child nodes for each SoundWire slave on the bus. >>> + >>> +properties: >>> + $nodename: >>> + pattern: "^soundwire(@.*|-[0-9a-f])*$" re-reading this, it looks like you are defining the controller bindings, but there are no real controller-level properties except for the fact that they include slave bindings? In MIPI the notion of controller is that it can deal with multiple links, each of which having specific properties (clock speed, clock stop properties, etc). >>> + >>> + "#address-cells": >>> + const: 2 >>> + >>> + "#size-cells": >>> + const: 0 >>> + >>> +patternProperties: >>> + "^.*@[0-9a-f]+$": >>> + type: object >>> + >>> + properties: >>> + compatible: >>> + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" >> >> So is this a 64-bit value, as in the MIPI spec, or is this part of the >> _ADR description? > > Rob did not like encoding compatible string exactly like _ADR encoding. > > https://lkml.org/lkml/2019/8/22/490 Wondering if we are talking about different concepts? Rob's point was about the InstanceID "Assuming you could have more than 1 of the same device on the bus, then you need some way to distinguish them and the way that's done for DT is unit-address/reg. And compatible strings should be constant for each instance." You can use the MIPI encoding *except* for the InstanceID, that'd be fine. It'll just be a bit weird since the Slave device will report the 48 bits that include the Instance ID, so you'll have to special case this field, but if this is a DT requirement then fine. Rob's point does not apply to the link ID - which is used when you have multiple masters in your controller. The Slave device is attached in one location and will never move, so that is a constant value. > >> I also don't get why the first item in in base10? >> > > As this corresponds to Soundwire Version, and I have no visibility of > version number encoding after reaching number 9 in this field. > > This can be updated once we have more info on how the Version encoding > will look like in future. > > Idea of limiting regex to [0-9] for version is to enforce some checking! the version is a 4 bit value starting at 1 for SoundWire 1.0. There is nothing in the spec that talks about a limit to 9. It's unlikely we'll ever reach that but you are interpreting a spec here. plus just below you mention all fields as being hexadecimal. > > --srini > >> >>> + description: >>> + Is the textual representation of SoundWire Enumeration >>> + address. compatible string should contain SoundWire Version ID, >>> + Manufacturer ID, Part ID and Class ID in order and shall be in >>> + lower-case hexadecimal with leading zeroes. >>> + Valid sizes of these fields are >>> + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 >>> + and '0x2' represents SoundWire 1.1 and so on. >>> + MFD is 4 nibbles >>> + PID is 4 nibbles >>> + CID is 2 nibbles >>> + More Information on detail of encoding of these fields can be >>> + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. >>> + >>> + reg: >>> + maxItems: 1 >>> + description: >>> + Instance ID and Link ID of SoundWire Device Address. >>> + >>> + required: >>> + - compatible >>> + - reg >>> + >>> +examples: >>> + - | >>> + soundwire@c2d0000 { >>> + #address-cells = <2>; >>> + #size-cells = <0>; >>> + compatible = "qcom,soundwire-v1.5.0"; >>> + reg = <0x0c2d0000 0x2000>; >>> + >>> + speaker@1 { >>> + compatible = "sdw10217201000"; >>> + reg = <1 0>; >>> + }; >>> + >>> + speaker@2 { >>> + compatible = "sdw10217201000"; >>> + reg = <2 0>; >>> + }; >>> + }; >>> + >>> +... >>>
On 23/08/2019 17:44, Pierre-Louis Bossart wrote: > > > On 8/23/19 10:57 AM, Srinivas Kandagatla wrote: >> >> >> On 23/08/2019 16:41, Pierre-Louis Bossart wrote: >>> >>> >>> On 8/22/19 6:37 PM, Srinivas Kandagatla wrote: >>>> This patch adds bindings for Soundwire Slave devices that includes how >>>> SoundWire enumeration address and Link ID are used to represented in >>>> SoundWire slave device tree nodes. >>>> >>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>>> --- >>>> .../soundwire/soundwire-controller.yaml | 75 >>>> +++++++++++++++++++ >>>> 1 file changed, 75 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>>> b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>>> new file mode 100644 >>>> index 000000000000..91aa6c6d6266 >>>> --- /dev/null >>>> +++ >>>> b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>>> @@ -0,0 +1,75 @@ >>>> +# SPDX-License-Identifier: GPL-2.0 >>>> +%YAML 1.2 >>>> +--- >>>> +$id: >>>> http://devicetree.org/schemas/soundwire/soundwire-controller.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: SoundWire Controller Generic Binding >>>> + >>>> +maintainers: >>>> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>>> + >>>> +description: | >>>> + SoundWire busses can be described with a node for the SoundWire >>>> controller >>>> + device and a set of child nodes for each SoundWire slave on the bus. >>>> + >>>> +properties: >>>> + $nodename: >>>> + pattern: "^soundwire(@.*|-[0-9a-f])*$" > > re-reading this, it looks like you are defining the controller bindings, > but there are no real controller-level properties except for the fact > that they include slave bindings? > Yes, Each vendor specific master can have there specific properties here, this is just to represent how slave nodes represented w.r.t to master nodes. > In MIPI the notion of controller is that it can deal with multiple > links, each of which having specific properties (clock speed, clock stop > properties, etc). > >>>> + >>>> + "#address-cells": >>>> + const: 2 >>>> + >>>> + "#size-cells": >>>> + const: 0 >>>> + >>>> +patternProperties: >>>> + "^.*@[0-9a-f]+$": >>>> + type: object >>>> + >>>> + properties: >>>> + compatible: >>>> + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" >>> >>> So is this a 64-bit value, as in the MIPI spec, or is this part of >>> the _ADR description? >> >> Rob did not like encoding compatible string exactly like _ADR encoding. >> >> https://lkml.org/lkml/2019/8/22/490 > > Wondering if we are talking about different concepts? > > Rob's point was about the InstanceID > > "Assuming you could have more than 1 of the same device on the bus, > then you need some way to distinguish them and the way that's done for > DT is unit-address/reg. And compatible strings should be constant for > each instance." > > You can use the MIPI encoding *except* for the InstanceID, that'd be > fine. It'll just be a bit weird since the Slave device will report the > 48 bits that include the Instance ID, so you'll have to special case > this field, but if this is a DT requirement then fine. > > Rob's point does not apply to the link ID - which is used when you have > multiple masters in your controller. The Slave device is attached in one > location and will never move, so that is a constant value. Point is that this compatible is for slave device, it should not matter where and how the slave is connected, compatible should be constant irrespective of Link ID. Lets say for example if WSA881x slave device is connected to a single-Link Controller and the same device is connected to a MultiLink-controller then we would endup in more than one compatible string for WSA881x driver. From Disco Specic it clearly says that LinkID values are relative to the immediate parent Device. So having LinkID in compatible string is very fragile. > >> >>> I also don't get why the first item in in base10? >>> >> >> As this corresponds to Soundwire Version, and I have no visibility of >> version number encoding after reaching number 9 in this field. >> >> This can be updated once we have more info on how the Version encoding >> will look like in future. >> >> Idea of limiting regex to [0-9] for version is to enforce some checking! > > the version is a 4 bit value starting at 1 for SoundWire 1.0. There is > nothing in the spec that talks about a limit to 9. > > It's unlikely we'll ever reach that but you are interpreting a spec > here. plus just below you mention all fields as being hexadecimal. > Am happy to change this to pattern: "^sdw[0-9a-f][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" if you are okay with rest of the stuff. thanks, srini >> >> --srini >> >>> >>>> + description: >>>> + Is the textual representation of SoundWire Enumeration >>>> + address. compatible string should contain SoundWire Version ID, >>>> + Manufacturer ID, Part ID and Class ID in order and shall be in >>>> + lower-case hexadecimal with leading zeroes. >>>> + Valid sizes of these fields are >>>> + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 >>>> + and '0x2' represents SoundWire 1.1 and so on. >>>> + MFD is 4 nibbles >>>> + PID is 4 nibbles >>>> + CID is 2 nibbles >>>> + More Information on detail of encoding of these fields can be >>>> + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. >>>> + >>>> + reg: >>>> + maxItems: 1 >>>> + description: >>>> + Instance ID and Link ID of SoundWire Device Address. >>>> + >>>> + required: >>>> + - compatible >>>> + - reg >>>> + >>>> +examples: >>>> + - | >>>> + soundwire@c2d0000 { >>>> + #address-cells = <2>; >>>> + #size-cells = <0>; >>>> + compatible = "qcom,soundwire-v1.5.0"; >>>> + reg = <0x0c2d0000 0x2000>; >>>> + >>>> + speaker@1 { >>>> + compatible = "sdw10217201000"; >>>> + reg = <1 0>; >>>> + }; >>>> + >>>> + speaker@2 { >>>> + compatible = "sdw10217201000"; >>>> + reg = <2 0>; >>>> + }; >>>> + }; >>>> + >>>> +... >>>>
>>>>> +title: SoundWire Controller Generic Binding >>>>> + >>>>> +maintainers: >>>>> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>>>> + >>>>> +description: | >>>>> + SoundWire busses can be described with a node for the SoundWire >>>>> controller >>>>> + device and a set of child nodes for each SoundWire slave on the >>>>> bus. >>>>> + >>>>> +properties: >>>>> + $nodename: >>>>> + pattern: "^soundwire(@.*|-[0-9a-f])*$" >> >> re-reading this, it looks like you are defining the controller >> bindings, but there are no real controller-level properties except for >> the fact that they include slave bindings? >> > Yes, Each vendor specific master can have there specific properties > here, this is just to represent how slave nodes represented w.r.t to > master nodes. I am not clear on how a vendor would document those controller properties then? And the number of links doesn't seem like a vendor-specific definition, if you include the linkID in the register information below it'd help to known how many links can be enabled, which ones are used (if there is any pin-mux) and check if the configurations are correct. > >> In MIPI the notion of controller is that it can deal with multiple >> links, each of which having specific properties (clock speed, clock >> stop properties, etc). >> >>>>> + >>>>> + "#address-cells": >>>>> + const: 2 >>>>> + >>>>> + "#size-cells": >>>>> + const: 0 >>>>> + >>>>> +patternProperties: >>>>> + "^.*@[0-9a-f]+$": >>>>> + type: object >>>>> + >>>>> + properties: >>>>> + compatible: >>>>> + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" >>>> >>>> So is this a 64-bit value, as in the MIPI spec, or is this part of >>>> the _ADR description? >>> >>> Rob did not like encoding compatible string exactly like _ADR encoding. >>> >>> https://lkml.org/lkml/2019/8/22/490 >> >> Wondering if we are talking about different concepts? >> >> Rob's point was about the InstanceID >> >> "Assuming you could have more than 1 of the same device on the bus, >> then you need some way to distinguish them and the way that's done for >> DT is unit-address/reg. And compatible strings should be constant for >> each instance." >> >> You can use the MIPI encoding *except* for the InstanceID, that'd be >> fine. It'll just be a bit weird since the Slave device will report the >> 48 bits that include the Instance ID, so you'll have to special case >> this field, but if this is a DT requirement then fine. >> >> Rob's point does not apply to the link ID - which is used when you >> have multiple masters in your controller. The Slave device is attached >> in one location and will never move, so that is a constant value. > > Point is that this compatible is for slave device, it should not matter > where and how the slave is connected, compatible should be constant > irrespective of Link ID. > Lets say for example if WSA881x slave device is connected to a > single-Link Controller and the same device is connected to a > MultiLink-controller then we would endup in more than one compatible > string for WSA881x driver. > > > From Disco Specic it clearly says that LinkID values are relative > to the immediate parent Device. So having LinkID in compatible string is > very fragile. ok, fine then. > >> >>> >>>> I also don't get why the first item in in base10? >>>> >>> >>> As this corresponds to Soundwire Version, and I have no visibility of >>> version number encoding after reaching number 9 in this field. >>> >>> This can be updated once we have more info on how the Version >>> encoding will look like in future. >>> >>> Idea of limiting regex to [0-9] for version is to enforce some checking! >> >> the version is a 4 bit value starting at 1 for SoundWire 1.0. There is >> nothing in the spec that talks about a limit to 9. >> >> It's unlikely we'll ever reach that but you are interpreting a spec >> here. plus just below you mention all fields as being hexadecimal. >> > Am happy to change this to > > pattern: "^sdw[0-9a-f][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" > > if you are okay with rest of the stuff. yes, ok. > > thanks, > srini >>> >>> --srini >>> >>>> >>>>> + description: >>>>> + Is the textual representation of SoundWire Enumeration >>>>> + address. compatible string should contain SoundWire Version ID, >>>>> + Manufacturer ID, Part ID and Class ID in order and shall be in >>>>> + lower-case hexadecimal with leading zeroes. >>>>> + Valid sizes of these fields are >>>>> + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 >>>>> + and '0x2' represents SoundWire 1.1 and so on. >>>>> + MFD is 4 nibbles >>>>> + PID is 4 nibbles >>>>> + CID is 2 nibbles >>>>> + More Information on detail of encoding of these fields can be >>>>> + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. >>>>> + >>>>> + reg: >>>>> + maxItems: 1 >>>>> + description: >>>>> + Instance ID and Link ID of SoundWire Device Address. maybe put link first and make it clear that both are required. >>>>> + >>>>> + required: >>>>> + - compatible >>>>> + - reg >>>>> + >>>>> +examples: >>>>> + - | >>>>> + soundwire@c2d0000 { >>>>> + #address-cells = <2>; >>>>> + #size-cells = <0>; >>>>> + compatible = "qcom,soundwire-v1.5.0"; >>>>> + reg = <0x0c2d0000 0x2000>; >>>>> + >>>>> + speaker@1 { >>>>> + compatible = "sdw10217201000"; >>>>> + reg = <1 0>; >>>>> + }; >>>>> + >>>>> + speaker@2 { >>>>> + compatible = "sdw10217201000"; >>>>> + reg = <2 0>; >>>>> + }; >>>>> + }; >>>>> + >>>>> +... >>>>>
On Fri, Aug 23, 2019 at 12:23:40PM +0530, Vinod Koul wrote: > On 23-08-19, 00:37, Srinivas Kandagatla wrote: > > This patch adds bindings for Soundwire Slave devices that includes how > > SoundWire enumeration address and Link ID are used to represented in > > SoundWire slave device tree nodes. > > > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > --- > > .../soundwire/soundwire-controller.yaml | 75 +++++++++++++++++++ > > 1 file changed, 75 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > > > > diff --git a/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > > new file mode 100644 > > index 000000000000..91aa6c6d6266 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml > > @@ -0,0 +1,75 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/soundwire/soundwire-controller.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: SoundWire Controller Generic Binding > > Controller does not make sense here, why not use spec terminology and > say "SoundWire Slave Generic Binding" It's both IMO. It's describing the structure of child devices of a controller (aka a bus). > > > + > > +maintainers: > > + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > + > > +description: | > > + SoundWire busses can be described with a node for the SoundWire controller > > + device and a set of child nodes for each SoundWire slave on the bus. > > + > > +properties: > > + $nodename: > > + pattern: "^soundwire(@.*|-[0-9a-f])*$" '-[0-9a-f]' was to handle cases like spi-gpio or i2c-gpio. Would a bit banged interface be possible here? > > + > > + "#address-cells": > > + const: 2 > > + > > + "#size-cells": > > + const: 0 > > + > > +patternProperties: > > + "^.*@[0-9a-f]+$": If there are distinct fields in the address, they are typically comma separated in the unit-address. > > + type: object > > + > > + properties: > > + compatible: > > + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" > > + description: > > + Is the textual representation of SoundWire Enumeration > > + address. compatible string should contain SoundWire Version ID, > > + Manufacturer ID, Part ID and Class ID in order and shall be in > > + lower-case hexadecimal with leading zeroes. > > + Valid sizes of these fields are > > + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 > > + and '0x2' represents SoundWire 1.1 and so on. > > + MFD is 4 nibbles > > + PID is 4 nibbles > > + CID is 2 nibbles > > + More Information on detail of encoding of these fields can be > > + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. > > + > > + reg: > > + maxItems: 1 > > + description: > > + Instance ID and Link ID of SoundWire Device Address. > > This looks better :) Thanks. > > Apart from the minor nit above this looks good to me, I can merge the > sdw parts if Rob is fine with them. > > Thanks > > > + > > + required: > > + - compatible > > + - reg > > + > > +examples: > > + - | > > + soundwire@c2d0000 { > > + #address-cells = <2>; > > + #size-cells = <0>; > > + compatible = "qcom,soundwire-v1.5.0"; This will probably change once I review it. :) > > + reg = <0x0c2d0000 0x2000>; > > + > > + speaker@1 { > > + compatible = "sdw10217201000"; > > + reg = <1 0>; > > + }; > > + > > + speaker@2 { > > + compatible = "sdw10217201000"; > > + reg = <2 0>; > > + }; > > + }; > > + > > +... > > -- > > 2.21.0 > > -- > ~Vinod
thanks for reviewing the patch! On 27/08/2019 21:20, Rob Herring wrote: > On Fri, Aug 23, 2019 at 12:23:40PM +0530, Vinod Koul wrote: >> On 23-08-19, 00:37, Srinivas Kandagatla wrote: >>> This patch adds bindings for Soundwire Slave devices that includes how >>> SoundWire enumeration address and Link ID are used to represented in >>> SoundWire slave device tree nodes. >>> >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> --- >>> .../soundwire/soundwire-controller.yaml | 75 +++++++++++++++++++ >>> 1 file changed, 75 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>> new file mode 100644 >>> index 000000000000..91aa6c6d6266 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml >>> @@ -0,0 +1,75 @@ >>> +# SPDX-License-Identifier: GPL-2.0 >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/soundwire/soundwire-controller.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: SoundWire Controller Generic Binding >> >> Controller does not make sense here, why not use spec terminology and >> say "SoundWire Slave Generic Binding" > > It's both IMO. It's describing the structure of child devices of a > controller (aka a bus). > >> >>> + >>> +maintainers: >>> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> + >>> +description: | >>> + SoundWire busses can be described with a node for the SoundWire controller >>> + device and a set of child nodes for each SoundWire slave on the bus. >>> + >>> +properties: >>> + $nodename: >>> + pattern: "^soundwire(@.*|-[0-9a-f])*$" > > '-[0-9a-f]' was to handle cases like spi-gpio or i2c-gpio. Would a > bit banged interface be possible here? Highly unlikely! > >>> + >>> + "#address-cells": >>> + const: 2 >>> + >>> + "#size-cells": >>> + const: 0 >>> + >>> +patternProperties: >>> + "^.*@[0-9a-f]+$": > > If there are distinct fields in the address, they are typically comma > separated in the unit-address. okay, will fix that in next version! > >>> + type: object >>> + >>> + properties: >>> + compatible: >>> + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" >>> + description: >>> + Is the textual representation of SoundWire Enumeration >>> + address. compatible string should contain SoundWire Version ID, >>> + Manufacturer ID, Part ID and Class ID in order and shall be in >>> + lower-case hexadecimal with leading zeroes. >>> + Valid sizes of these fields are >>> + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 >>> + and '0x2' represents SoundWire 1.1 and so on. >>> + MFD is 4 nibbles >>> + PID is 4 nibbles >>> + CID is 2 nibbles >>> + More Information on detail of encoding of these fields can be >>> + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. >>> + >>> + reg: >>> + maxItems: 1 >>> + description: >>> + Instance ID and Link ID of SoundWire Device Address. >> >> This looks better :) Thanks. >> >> Apart from the minor nit above this looks good to me, I can merge the >> sdw parts if Rob is fine with them. >> >> Thanks >> >>> + >>> + required: >>> + - compatible >>> + - reg >>> + >>> +examples: >>> + - | >>> + soundwire@c2d0000 { >>> + #address-cells = <2>; >>> + #size-cells = <0>; >>> + compatible = "qcom,soundwire-v1.5.0"; > > This will probably change once I review it. :) :-) thanks, srini > >>> + reg = <0x0c2d0000 0x2000>; >>> + >>> + speaker@1 { >>> + compatible = "sdw10217201000"; >>> + reg = <1 0>; >>> + }; >>> + >>> + speaker@2 { >>> + compatible = "sdw10217201000"; >>> + reg = <2 0>; >>> + }; >>> + }; >>> + >>> +... >>> -- >>> 2.21.0 >> >> -- >> ~Vinod
On 26/08/2019 17:22, Pierre-Louis Bossart wrote: >>>>> >>>>>> + description: >>>>>> + Is the textual representation of SoundWire Enumeration >>>>>> + address. compatible string should contain SoundWire Version >>>>>> ID, >>>>>> + Manufacturer ID, Part ID and Class ID in order and shall be in >>>>>> + lower-case hexadecimal with leading zeroes. >>>>>> + Valid sizes of these fields are >>>>>> + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 >>>>>> + and '0x2' represents SoundWire 1.1 and so on. >>>>>> + MFD is 4 nibbles >>>>>> + PID is 4 nibbles >>>>>> + CID is 2 nibbles >>>>>> + More Information on detail of encoding of these fields can be >>>>>> + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. >>>>>> + >>>>>> + reg: >>>>>> + maxItems: 1 >>>>>> + description: >>>>>> + Instance ID and Link ID of SoundWire Device Address. > > maybe put link first and make it clear that both are required. Okay, I will give that a go in next version! thanks, srini
diff --git a/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml new file mode 100644 index 000000000000..91aa6c6d6266 --- /dev/null +++ b/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soundwire/soundwire-controller.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SoundWire Controller Generic Binding + +maintainers: + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> + +description: | + SoundWire busses can be described with a node for the SoundWire controller + device and a set of child nodes for each SoundWire slave on the bus. + +properties: + $nodename: + pattern: "^soundwire(@.*|-[0-9a-f])*$" + + "#address-cells": + const: 2 + + "#size-cells": + const: 0 + +patternProperties: + "^.*@[0-9a-f]+$": + type: object + + properties: + compatible: + pattern: "^sdw[0-9][0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" + description: + Is the textual representation of SoundWire Enumeration + address. compatible string should contain SoundWire Version ID, + Manufacturer ID, Part ID and Class ID in order and shall be in + lower-case hexadecimal with leading zeroes. + Valid sizes of these fields are + Version ID is 1 nibble, number '0x1' represents SoundWire 1.0 + and '0x2' represents SoundWire 1.1 and so on. + MFD is 4 nibbles + PID is 4 nibbles + CID is 2 nibbles + More Information on detail of encoding of these fields can be + found in MIPI Alliance DisCo & SoundWire 1.0 Specifications. + + reg: + maxItems: 1 + description: + Instance ID and Link ID of SoundWire Device Address. + + required: + - compatible + - reg + +examples: + - | + soundwire@c2d0000 { + #address-cells = <2>; + #size-cells = <0>; + compatible = "qcom,soundwire-v1.5.0"; + reg = <0x0c2d0000 0x2000>; + + speaker@1 { + compatible = "sdw10217201000"; + reg = <1 0>; + }; + + speaker@2 { + compatible = "sdw10217201000"; + reg = <2 0>; + }; + }; + +...
This patch adds bindings for Soundwire Slave devices that includes how SoundWire enumeration address and Link ID are used to represented in SoundWire slave device tree nodes. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- .../soundwire/soundwire-controller.yaml | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml