Hi All,
This patchset is very first version of Qualcomm SoundWire Master Controller
found in most of Qualcomm SoCs and WCD audio codecs.
This driver along with WCD934x codec and WSA881x Class-D Smart Speaker Amplifier
drivers is on DragonBoard DB845c based of SDM845 SoC.
WCD934x and WSA881x patches will be posted soon.
SoundWire controller on SDM845 is integrated in WCD934x audio codec via
SlimBus interface.
Currently this driver is very minimal and only supports PDM.
Most of the code in this driver is rework of Qualcomm downstream drivers
used in Andriod. Credits to Banajit Goswami and Patrick Lai's Team.
TODO:
Test and add PCM support.
Thanks,
srini
Srinivas Kandagatla (5):
ASoC: core: add support to snd_soc_dai_get_sdw_stream()
soundwire: core: define SDW_MAX_PORT
soundwire: stream: make stream name a const pointer
dt-bindings: soundwire: add bindings for Qcom controller
soundwire: qcom: add support for SoundWire controller
Vinod Koul (1):
soundwire: Add compute_params callback
.../bindings/soundwire/qcom,swr.txt | 62 ++
drivers/soundwire/Kconfig | 9 +
drivers/soundwire/Makefile | 4 +
drivers/soundwire/qcom.c | 983 ++++++++++++++++++
drivers/soundwire/stream.c | 11 +-
include/linux/soundwire/sdw.h | 7 +-
include/sound/soc-dai.h | 10 +
7 files changed, 1083 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,swr.txt
create mode 100644 drivers/soundwire/qcom.c
--
2.21.0
On platforms which have smart speaker amplifiers connected via
soundwire and modeled as aux devices in ASoC, in such usecases machine
driver should be able to get sdw master stream from dai so that it can
use the runtime stream to setup slave streams.
soundwire already as a set function, get function would provide more
flexibility to above configurations.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
include/sound/soc-dai.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index f5d70041108f..9f90b936fd9a 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -177,6 +177,7 @@ struct snd_soc_dai_ops {
int (*set_sdw_stream)(struct snd_soc_dai *dai,
void *stream, int direction);
+ void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction);
/*
* DAI digital mute - optional.
* Called by soc-core to minimise any pops.
@@ -385,4 +386,13 @@ static inline int snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai,
return -ENOTSUPP;
}
+static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, int direction)
+{
+ if (dai->driver->ops->get_sdw_stream)
+ return dai->driver->ops->get_sdw_stream(dai, direction);
+ else
+ return NULL;
+
+}
+
#endif
--
2.21.0
This patch adds bindings for Qualcomm soundwire controller.
Qualcomm SoundWire Master controller is present in most Qualcomm SoCs
either integrated as part of WCD audio codecs via slimbus or
as part of SOC I/O.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
.../bindings/soundwire/qcom,swr.txt | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,swr.txt
diff --git a/Documentation/devicetree/bindings/soundwire/qcom,swr.txt b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
new file mode 100644
index 000000000000..eb84d0f4f36f
--- /dev/null
+++ b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
@@ -0,0 +1,62 @@
+Qualcomm SoundWire Controller
+
+This binding describes the Qualcomm SoundWire Controller Bindings.
+
+Required properties:
+
+- compatible: Must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>",
+ example:
+ "qcom,soundwire-v1.3.0"
+ "qcom,soundwire-v1.5.0"
+ "qcom,soundwire-v1.6.0"
+- reg: SoundWire controller address space.
+- interrupts: SoundWire controller interrupt.
+- clock-names: Must contain "iface".
+- clocks: Interface clocks needed for controller.
+- #sound-dai-cells: Must be 1 for digital audio interfaces on the controllers.
+- #address-cells: Must be 1 for SoundWire devices;
+- #size-cells: Must be <0> as SoundWire addresses have no size component.
+- qcom,dout-ports: Must be count of data out ports
+- qcom,din-ports: Must be count of data in ports
+- qcom,ports-offset1: Must be frame offset1 of each data port.
+ Out followed by In. Used for Block size calculation.
+- qcom,ports-offset2: Must be frame offset2 of each data port.
+ Out followed by In. Used for Block size calculation.
+- qcom,ports-sinterval-low: Must be sample interval low of each data port.
+ Out followed by In. Used for Sample Interval calculation.
+
+= SoundWire devices
+Each subnode of the bus represents SoundWire device attached to it.
+The properties of these nodes are defined by the individual bindings.
+
+= EXAMPLE
+The following example represents a SoundWire controller on DB845c board
+which has controller integrated inside WCD934x codec on SDM845 SoC.
+
+soundwire: soundwire@c85 {
+ compatible = "qcom,soundwire-v1.3.0";
+ reg = <0xc85 0x20>;
+ interrupts = <20 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&wcc>;
+ clock-names = "iface";
+ #sound-dai-cells = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ qcom,dout-ports = <6>;
+ qcom,din-ports = <2>;
+ qcom,ports-sinterval-low =/bits/ 8 <0x07 0x1F 0x3F 0x7 0x1F 0x3F 0x0F 0x0F>;
+ qcom,ports-offset1 = /bits/ 8 <0x01 0x02 0x0C 0x6 0x12 0x0D 0x07 0x0A >;
+ qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x1F 0x00 0x00 0x1F 0x00 0x00>;
+
+ /* Left Speaker */
+ wsa8810@1{
+ ....
+ reg = <1>;
+ };
+
+ /* Right Speaker */
+ wsa8810@2{
+ ....
+ reg = <2>;
+ };
+};
--
2.21.0
On 6/7/19 3:56 AM, Srinivas Kandagatla wrote:
> This patch adds bindings for Qualcomm soundwire controller.
>
> Qualcomm SoundWire Master controller is present in most Qualcomm SoCs
> either integrated as part of WCD audio codecs via slimbus or
> as part of SOC I/O.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> .../bindings/soundwire/qcom,swr.txt | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,swr.txt
you seem to use the 'swr' prefix in this patch. Most implementers use
'sdw', and that's the default also used in the MIPI DisCo spec for
properties. Can we align on the same naming conventions?
>
> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,swr.txt b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
> new file mode 100644
> index 000000000000..eb84d0f4f36f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
> @@ -0,0 +1,62 @@
> +Qualcomm SoundWire Controller
> +
> +This binding describes the Qualcomm SoundWire Controller Bindings.
> +
> +Required properties:
> +
> +- compatible: Must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>",
> + example:
> + "qcom,soundwire-v1.3.0"
> + "qcom,soundwire-v1.5.0"
> + "qcom,soundwire-v1.6.0"
> +- reg: SoundWire controller address space.
> +- interrupts: SoundWire controller interrupt.
> +- clock-names: Must contain "iface".
> +- clocks: Interface clocks needed for controller.
> +- #sound-dai-cells: Must be 1 for digital audio interfaces on the controllers.
> +- #address-cells: Must be 1 for SoundWire devices;
> +- #size-cells: Must be <0> as SoundWire addresses have no size component.
> +- qcom,dout-ports: Must be count of data out ports
> +- qcom,din-ports: Must be count of data in ports
> +- qcom,ports-offset1: Must be frame offset1 of each data port.
> + Out followed by In. Used for Block size calculation.
> +- qcom,ports-offset2: Must be frame offset2 of each data port.
> + Out followed by In. Used for Block size calculation.
> +- qcom,ports-sinterval-low: Must be sample interval low of each data port.
> + Out followed by In. Used for Sample Interval calculation.
These definitions are valid only for specific types of ports, I believe
here it's a 'reduced' port since offset2 is not required for simpler
ports and you don't have Hstart/Hstop.
so if you state that all of these properties are required, you are
explicitly ruling out future implementations of simple ports or will
have to redefine them later.
Also the definition 'frame offset1/2' is incorrect. the offset is
defined within each Payload Transport Window - not each frame - and its
definition depends on the packing mode used, which isn't defined or
stated here.
And last it looks like you assume a fixed frame shape - likely 50 rows
by 8 columns, it might be worth adding a note on the max values for
offset1/2 implied by this frame shape.
> +
> += SoundWire devices
> +Each subnode of the bus represents SoundWire device attached to it.
> +The properties of these nodes are defined by the individual bindings.
> +
> += EXAMPLE
> +The following example represents a SoundWire controller on DB845c board
> +which has controller integrated inside WCD934x codec on SDM845 SoC.
> +
> +soundwire: soundwire@c85 {
> + compatible = "qcom,soundwire-v1.3.0";
> + reg = <0xc85 0x20>;
> + interrupts = <20 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&wcc>;
> + clock-names = "iface";
> + #sound-dai-cells = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + qcom,dout-ports = <6>;
> + qcom,din-ports = <2>;
> + qcom,ports-sinterval-low =/bits/ 8 <0x07 0x1F 0x3F 0x7 0x1F 0x3F 0x0F 0x0F>;
> + qcom,ports-offset1 = /bits/ 8 <0x01 0x02 0x0C 0x6 0x12 0x0D 0x07 0x0A >;
> + qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x1F 0x00 0x00 0x1F 0x00 0x00>;
> +
> + /* Left Speaker */
> + wsa8810@1{
> + ....
> + reg = <1>;
> + };
> +
> + /* Right Speaker */
> + wsa8810@2{
> + ....
> + reg = <2>;
> + };
> +};
>
On 2019-06-07 10:56, Srinivas Kandagatla wrote:
> On platforms which have smart speaker amplifiers connected via
> soundwire and modeled as aux devices in ASoC, in such usecases machine
> driver should be able to get sdw master stream from dai so that it can
> use the runtime stream to setup slave streams.
>
> soundwire already as a set function, get function would provide more
> flexibility to above configurations.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> include/sound/soc-dai.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index f5d70041108f..9f90b936fd9a 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -177,6 +177,7 @@ struct snd_soc_dai_ops {
>
> int (*set_sdw_stream)(struct snd_soc_dai *dai,
> void *stream, int direction);
> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction);
> /*
> * DAI digital mute - optional.
> * Called by soc-core to minimise any pops.
> @@ -385,4 +386,13 @@ static inline int snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai,
> return -ENOTSUPP;
> }
>
> +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, int direction)
Exceeds character limit?
> +{
> + if (dai->driver->ops->get_sdw_stream)
> + return dai->driver->ops->get_sdw_stream(dai, direction);
> + else
> + return NULL;
set_ equivalent returns -ENOTSUPP instead.
ERR_PTR seems to make more sense here.
> +
Unnecessary newline.
> +}
> +
> #endif
>
Thanks for taking time to review,
On 07/06/2019 13:50, Pierre-Louis Bossart wrote:
> On 6/7/19 3:56 AM, Srinivas Kandagatla wrote:
>> This patch adds bindings for Qualcomm soundwire controller.
>>
>> Qualcomm SoundWire Master controller is present in most Qualcomm SoCs
>> either integrated as part of WCD audio codecs via slimbus or
>> as part of SOC I/O.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> .../bindings/soundwire/qcom,swr.txt | 62 +++++++++++++++++++
>> 1 file changed, 62 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>
> you seem to use the 'swr' prefix in this patch. Most implementers use
> 'sdw', and that's the default also used in the MIPI DisCo spec for
> properties. Can we align on the same naming conventions?
>
>>
>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>> b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>> new file mode 100644
>> index 000000000000..eb84d0f4f36f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>> @@ -0,0 +1,62 @@
>> +Qualcomm SoundWire Controller
>> +
>> +This binding describes the Qualcomm SoundWire Controller Bindings.
>> +
>> +Required properties:
>> +
>> +- compatible: Must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>",
>> + example:
>> + "qcom,soundwire-v1.3.0"
>> + "qcom,soundwire-v1.5.0"
>> + "qcom,soundwire-v1.6.0"
>> +- reg: SoundWire controller address space.
>> +- interrupts: SoundWire controller interrupt.
>> +- clock-names: Must contain "iface".
>> +- clocks: Interface clocks needed for controller.
>> +- #sound-dai-cells: Must be 1 for digital audio interfaces on the
>> controllers.
>> +- #address-cells: Must be 1 for SoundWire devices;
>> +- #size-cells: Must be <0> as SoundWire addresses have no size
>> component.
>> +- qcom,dout-ports: Must be count of data out ports
>> +- qcom,din-ports: Must be count of data in ports
>> +- qcom,ports-offset1: Must be frame offset1 of each data port.
>> + Out followed by In. Used for Block size calculation.
>> +- qcom,ports-offset2: Must be frame offset2 of each data port.
>> + Out followed by In. Used for Block size calculation.
>> +- qcom,ports-sinterval-low: Must be sample interval low of each data
>> port.
>> + Out followed by In. Used for Sample Interval calculation.
>
> These definitions are valid only for specific types of ports, I believe
> here it's a 'reduced' port since offset2 is not required for simpler
> ports and you don't have Hstart/Hstop.
>
Yes, this version of the controller which am working on does not have
DPN_SampleCtrl2 register for SampleIntervalHigh Field and has registers
for programming offset2 does indeed indicate that these ports are
reduced ports.
However looks like new versions of the this controller does support full
data ports.
I can add more flexibility in bindings by adding qcom,dport-type field
indicating this in next version.
> so if you state that all of these properties are required, you are
> explicitly ruling out future implementations of simple ports or will
> have to redefine them later.
>
> Also the definition 'frame offset1/2' is incorrect. the offset is
> defined within each Payload Transport Window - not each frame - and its
> definition depends on the packing mode used, which isn't defined or
> stated here.
Yep, BlockPackingMode is missing. 1.3 version of this controller only
supports Block Per Port Block mode.
I guess this can be derived with in the driver by using compatible
string, I can add some notes in the binding to make this more explicit.
I will also reword offset1/2 description to include transport window
within frame
>
> And last it looks like you assume a fixed frame shape - likely 50 rows
> by 8 columns, it might be worth adding a note on the max values for
> offset1/2 implied by this frame shape.
Its 48x16 in this case.
Thanks,
srini
Thanks for taking time to review,
On 08/06/2019 20:22, Cezary Rojewski wrote:
> On 2019-06-07 10:56, Srinivas Kandagatla wrote:
>> On platforms which have smart speaker amplifiers connected via
>> soundwire and modeled as aux devices in ASoC, in such usecases machine
>> driver should be able to get sdw master stream from dai so that it can
>> use the runtime stream to setup slave streams.
>>
>> soundwire already as a set function, get function would provide more
>> flexibility to above configurations.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> include/sound/soc-dai.h | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
>> index f5d70041108f..9f90b936fd9a 100644
>> --- a/include/sound/soc-dai.h
>> +++ b/include/sound/soc-dai.h
>> @@ -177,6 +177,7 @@ struct snd_soc_dai_ops {
>> int (*set_sdw_stream)(struct snd_soc_dai *dai,
>> void *stream, int direction);
>> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction);
>> /*
>> * DAI digital mute - optional.
>> * Called by soc-core to minimise any pops.
>> @@ -385,4 +386,13 @@ static inline int
>> snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai,
>> return -ENOTSUPP;
>> }
>> +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai
>> *dai, int direction)
>
> Exceeds character limit?
>
>> +{
>> + if (dai->driver->ops->get_sdw_stream)
>> + return dai->driver->ops->get_sdw_stream(dai, direction);
>> + else
>> + return NULL;
>
> set_ equivalent returns -ENOTSUPP instead.
> ERR_PTR seems to make more sense here.
>
>> +
>
> Unnecessary newline.
I agree with all the comment, will fix this in next version.
thanks,
srini
>
>> +}
>> +
>> #endif
>>
On 07-06-19, 09:56, Srinivas Kandagatla wrote:
> On platforms which have smart speaker amplifiers connected via
> soundwire and modeled as aux devices in ASoC, in such usecases machine
> driver should be able to get sdw master stream from dai so that it can
> use the runtime stream to setup slave streams.
>
> soundwire already as a set function, get function would provide more
> flexibility to above configurations.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> include/sound/soc-dai.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index f5d70041108f..9f90b936fd9a 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -177,6 +177,7 @@ struct snd_soc_dai_ops {
>
> int (*set_sdw_stream)(struct snd_soc_dai *dai,
> void *stream, int direction);
> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction);
So who would be calling this API? Machine or someone else?
--
~Vinod
On 07-06-19, 09:56, Srinivas Kandagatla wrote:
> This patch adds bindings for Qualcomm soundwire controller.
>
> Qualcomm SoundWire Master controller is present in most Qualcomm SoCs
> either integrated as part of WCD audio codecs via slimbus or
> as part of SOC I/O.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> .../bindings/soundwire/qcom,swr.txt | 62 +++++++++++++++++++
So I was expecting to see bus support patches for OF first. I think you
have missed those changes. Please do include those in v2 with bindings
and OF support for bus.
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>
> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,swr.txt b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
> new file mode 100644
> index 000000000000..eb84d0f4f36f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
> @@ -0,0 +1,62 @@
> +Qualcomm SoundWire Controller
> +
> +This binding describes the Qualcomm SoundWire Controller Bindings.
> +
> +Required properties:
> +
> +- compatible: Must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>",
> + example:
> + "qcom,soundwire-v1.3.0"
> + "qcom,soundwire-v1.5.0"
> + "qcom,soundwire-v1.6.0"
> +- reg: SoundWire controller address space.
> +- interrupts: SoundWire controller interrupt.
> +- clock-names: Must contain "iface".
> +- clocks: Interface clocks needed for controller.
> +- #sound-dai-cells: Must be 1 for digital audio interfaces on the controllers.
> +- #address-cells: Must be 1 for SoundWire devices;
> +- #size-cells: Must be <0> as SoundWire addresses have no size component.
> +- qcom,dout-ports: Must be count of data out ports
> +- qcom,din-ports: Must be count of data in ports
On these I think we should have specified dpn properties as specified in
DisCo, but then looking at spec we do not define that for master, but
bus seems to have it.
Pierre do you why master does not have dpn properties in DisCo?
> +- qcom,ports-offset1: Must be frame offset1 of each data port.
> + Out followed by In. Used for Block size calculation.
> +- qcom,ports-offset2: Must be frame offset2 of each data port.
> + Out followed by In. Used for Block size calculation.
> +- qcom,ports-sinterval-low: Must be sample interval low of each data port.
> + Out followed by In. Used for Sample Interval calculation.
These are software so do not belong here
--
~Vinod
On 10/06/2019 05:34, Vinod Koul wrote:
> On 07-06-19, 09:56, Srinivas Kandagatla wrote:
>> On platforms which have smart speaker amplifiers connected via
>> soundwire and modeled as aux devices in ASoC, in such usecases machine
>> driver should be able to get sdw master stream from dai so that it can
>> use the runtime stream to setup slave streams.
>>
>> soundwire already as a set function, get function would provide more
>> flexibility to above configurations.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> include/sound/soc-dai.h | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
>> index f5d70041108f..9f90b936fd9a 100644
>> --- a/include/sound/soc-dai.h
>> +++ b/include/sound/soc-dai.h
>> @@ -177,6 +177,7 @@ struct snd_soc_dai_ops {
>>
>> int (*set_sdw_stream)(struct snd_soc_dai *dai,
>> void *stream, int direction);
>> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction);
>
> So who would be calling this API? Machine or someone else?
From Machine driver in my case where smart speaker amplifiers are
modeled as aux device which is also dai less.
--srini
>
On 10/06/2019 05:51, Vinod Koul wrote:
> On 07-06-19, 09:56, Srinivas Kandagatla wrote:
>> This patch adds bindings for Qualcomm soundwire controller.
>>
>> Qualcomm SoundWire Master controller is present in most Qualcomm SoCs
>> either integrated as part of WCD audio codecs via slimbus or
>> as part of SOC I/O.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> .../bindings/soundwire/qcom,swr.txt | 62 +++++++++++++++++++
>
> So I was expecting to see bus support patches for OF first. I think you
> have missed those changes. Please do include those in v2 with bindings
> and OF support for bus.
I was planning to post bus bindings along with soundwire slave bindings
of WSA881x driver!
I will be sending this soon!
>
>> 1 file changed, 62 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>>
>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,swr.txt b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>> new file mode 100644
>> index 000000000000..eb84d0f4f36f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>> @@ -0,0 +1,62 @@
>> +Qualcomm SoundWire Controller
>> +
>> +This binding describes the Qualcomm SoundWire Controller Bindings.
>> +
>> +Required properties:
>> +
>> +- compatible: Must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>",
>> + example:
>> + "qcom,soundwire-v1.3.0"
>> + "qcom,soundwire-v1.5.0"
>> + "qcom,soundwire-v1.6.0"
>> +- reg: SoundWire controller address space.
>> +- interrupts: SoundWire controller interrupt.
>> +- clock-names: Must contain "iface".
>> +- clocks: Interface clocks needed for controller.
>> +- #sound-dai-cells: Must be 1 for digital audio interfaces on the controllers.
>> +- #address-cells: Must be 1 for SoundWire devices;
>> +- #size-cells: Must be <0> as SoundWire addresses have no size component.
>> +- qcom,dout-ports: Must be count of data out ports
>> +- qcom,din-ports: Must be count of data in ports
>
> On these I think we should have specified dpn properties as specified in
> DisCo, but then looking at spec we do not define that for master, but
> bus seems to have it.
>
> Pierre do you why master does not have dpn properties in DisCo?
>
>> +- qcom,ports-offset1: Must be frame offset1 of each data port.
>> + Out followed by In. Used for Block size calculation.
>> +- qcom,ports-offset2: Must be frame offset2 of each data port.
>> + Out followed by In. Used for Block size calculation.
>> +- qcom,ports-sinterval-low: Must be sample interval low of each data port.
>> + Out followed by In. Used for Sample Interval calculation.
>
> These are software so do not belong here
These are board-specfic properties w.r.t this controller ports
configuration.
I don't see any other option to specify this?
Do you have any alternative suggestions?
--srini
>
>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,swr.txt b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>> new file mode 100644
>> index 000000000000..eb84d0f4f36f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
>> @@ -0,0 +1,62 @@
>> +Qualcomm SoundWire Controller
>> +
>> +This binding describes the Qualcomm SoundWire Controller Bindings.
>> +
>> +Required properties:
>> +
>> +- compatible: Must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>",
>> + example:
>> + "qcom,soundwire-v1.3.0"
>> + "qcom,soundwire-v1.5.0"
>> + "qcom,soundwire-v1.6.0"
>> +- reg: SoundWire controller address space.
>> +- interrupts: SoundWire controller interrupt.
>> +- clock-names: Must contain "iface".
>> +- clocks: Interface clocks needed for controller.
>> +- #sound-dai-cells: Must be 1 for digital audio interfaces on the controllers.
>> +- #address-cells: Must be 1 for SoundWire devices;
>> +- #size-cells: Must be <0> as SoundWire addresses have no size component.
>> +- qcom,dout-ports: Must be count of data out ports
>> +- qcom,din-ports: Must be count of data in ports
>
> On these I think we should have specified dpn properties as specified in
> DisCo, but then looking at spec we do not define that for master, but
> bus seems to have it.
>
> Pierre do you why master does not have dpn properties in DisCo?
Because there are no DP0 or DPn registers defined for Masters in the
SoundWire 1.x spec. DisCo is about specifying properties for standard
registers, when they are not standard vendor extensions need to come
into play.
>
>> +- qcom,ports-offset1: Must be frame offset1 of each data port.
>> + Out followed by In. Used for Block size calculation.
>> +- qcom,ports-offset2: Must be frame offset2 of each data port.
>> + Out followed by In. Used for Block size calculation.
>> +- qcom,ports-sinterval-low: Must be sample interval low of each data port.
>> + Out followed by In. Used for Sample Interval calculation.
>
> These are software so do not belong here
Not necessarily. They define the allocation expected on that link and I
see no problem specifying those values here. It's the moral equivalent
of specifying which TDM slots and the bit depth of one slot you'd use
for DSP_A/B.
And if you push back, then what would be your alternate proposal on
where those values might be stored? This is a very specific usage of the
link and it makes sense to me to have the information in firmware -
exposed with properties - rather than hard-coded in a pretend bus
allocation routine in the kernel. Either the allocation is fully dynamic
and it's handled by the kernel or it's static and it's better to store
it in firmware to deal with platform-to-platform variations.