Add static channel mapping between master and slave rx/tx ports for
Qualcomm wcd937x soundwire codec.
Currently, the channel mask for each soundwire port is hardcoded in the
wcd937x-sdw driver, and the same channel mask value is configured in the
soundwire master.
The Qualcomm boards like the QCM6490-IDP require different channel mask settings
for the soundwire master and slave ports.
With the introduction of the following channel mapping properties, it is now possible
to configure the master channel mask directly from the device tree.
The qcom,tx-channel-mapping property specifies the static channel mapping between the slave
and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4,
dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
The qcom,rx-channel-mapping property specifies static channel mapping between the slave
and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh,
comp_l, comp_r, lo, dsd_r, dsd_l.
Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
.../bindings/sound/qcom,wcd937x-sdw.yaml | 36 +++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
index d3cf8f59cb23..a6bc9b391db0 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
@@ -58,6 +58,38 @@ properties:
items:
enum: [1, 2, 3, 4, 5]
+ qcom,tx-channel-mapping:
+ description: |
+ Specifies static channel mapping between slave and master tx port
+ channels.
+ In the order of slave port channels which is adc1, adc2, adc3, adc4,
+ dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
+ ch_mask1 ==> bit mask value 1
+ ch_mask2 ==> bit mask value 2
+ ch_mask3 ==> bit mask value 4
+ ch_mask4 ==> bit mask value 8
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ minItems: 8
+ maxItems: 13
+ items:
+ enum: [1, 2, 4, 8]
+
+ qcom,rx-channel-mapping:
+ description: |
+ Specifies static channels mapping between slave and master rx port
+ channels.
+ In the order of slave port channels, which is
+ hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l.
+ ch_mask1 ==> bit mask value 1
+ ch_mask2 ==> bit mask value 2
+ ch_mask3 ==> bit mask value 4
+ ch_mask4 ==> bit mask value 8
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ minItems: 8
+ maxItems: 8
+ items:
+ enum: [1, 2, 4, 8]
+
required:
- compatible
- reg
@@ -74,6 +106,8 @@ examples:
compatible = "sdw20217010a00";
reg = <0 4>;
qcom,rx-port-mapping = <1 2 3 4 5>;
+ qcom,rx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02
+ 0x01 0x01 0x02>;
};
};
@@ -85,6 +119,8 @@ examples:
compatible = "sdw20217010a00";
reg = <0 3>;
qcom,tx-port-mapping = <2 2 3 4>;
+ qcom,tx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02 0x04
+ 0x04 0x08 0x01 0x02 0x04 0x8>;
};
};
--
2.25.1On Wed, Oct 23, 2024 at 11:43:23AM +0530, Mohammad Rafi Shaik wrote: > Add static channel mapping between master and slave rx/tx ports for > Qualcomm wcd937x soundwire codec. > > Currently, the channel mask for each soundwire port is hardcoded in the > wcd937x-sdw driver, and the same channel mask value is configured in the > soundwire master. > > The Qualcomm boards like the QCM6490-IDP require different channel mask settings > for the soundwire master and slave ports. Different than what? Other wcd937x? Which are these? > > With the introduction of the following channel mapping properties, it is now possible > to configure the master channel mask directly from the device tree. > > The qcom,tx-channel-mapping property specifies the static channel mapping between the slave > and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4, > dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7. I still don't get what is the channel here. > > The qcom,rx-channel-mapping property specifies static channel mapping between the slave > and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh, > comp_l, comp_r, lo, dsd_r, dsd_l. And this description copies binding :/. Please wrap commit message according to Linux coding style / submission process (neither too early nor over the limit): https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597 > > Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> > --- > .../bindings/sound/qcom,wcd937x-sdw.yaml | 36 +++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > index d3cf8f59cb23..a6bc9b391db0 100644 > --- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > @@ -58,6 +58,38 @@ properties: > items: > enum: [1, 2, 3, 4, 5] > > + qcom,tx-channel-mapping: > + description: | > + Specifies static channel mapping between slave and master tx port > + channels. > + In the order of slave port channels which is adc1, adc2, adc3, adc4, > + dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7. > + ch_mask1 ==> bit mask value 1 > + ch_mask2 ==> bit mask value 2 > + ch_mask3 ==> bit mask value 4 > + ch_mask4 ==> bit mask value 8 > + $ref: /schemas/types.yaml#/definitions/uint8-array > + minItems: 8 > + maxItems: 13 Why size is variable? This device has fixed amount of slave ports, I think. > + items: > + enum: [1, 2, 4, 8] What is the point of using bits if you cannot actually create a bit mask out of it? Why this cannot be 7? > + > + qcom,rx-channel-mapping: > + description: | > + Specifies static channels mapping between slave and master rx port > + channels. > + In the order of slave port channels, which is > + hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l. > + ch_mask1 ==> bit mask value 1 > + ch_mask2 ==> bit mask value 2 > + ch_mask3 ==> bit mask value 4 > + ch_mask4 ==> bit mask value 8 and the value is what exactly? Index is channel, but what does "ch_mask4 ==> bit mask value 8" mean? I don't understand this at all. > + $ref: /schemas/types.yaml#/definitions/uint8-array > + minItems: 8 > + maxItems: 8 > + items: > + enum: [1, 2, 4, 8] > + > required: > - compatible > - reg > @@ -74,6 +106,8 @@ examples: > compatible = "sdw20217010a00"; > reg = <0 4>; > qcom,rx-port-mapping = <1 2 3 4 5>; > + qcom,rx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02 > + 0x01 0x01 0x02>; > }; > }; > > @@ -85,6 +119,8 @@ examples: > compatible = "sdw20217010a00"; > reg = <0 3>; > qcom,tx-port-mapping = <2 2 3 4>; > + qcom,tx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02 0x04 > + 0x04 0x08 0x01 0x02 0x04 0x8>; Keep it consistent, e.g. everywhere without leading 0... actually not sure why this is hex, either. > }; > }; > > -- > 2.25.1 >
On 10/23/2024 1:22 PM, Krzysztof Kozlowski wrote:
> On Wed, Oct 23, 2024 at 11:43:23AM +0530, Mohammad Rafi Shaik wrote:
>> Add static channel mapping between master and slave rx/tx ports for
>> Qualcomm wcd937x soundwire codec.
>>
>> Currently, the channel mask for each soundwire port is hardcoded in the
>> wcd937x-sdw driver, and the same channel mask value is configured in the
>> soundwire master.
>>
>> The Qualcomm boards like the QCM6490-IDP require different channel mask settings
>> for the soundwire master and slave ports.
>
> Different than what? Other wcd937x? Which are these?
>
For Qualcomm QCM6490-IDP board soundwire master needs a different
channel mask setting.
The wcd937x channel mask values are hardcoded in wcd driver.
https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x-sdw.c#L35
https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd938x-sdw.c#L37
In case of QCM6490-IDP the soundwire master and wcd937x require
different channel mask settings, not the same.
For Example, wcd937x ADC2 connection
Master Slave (wcd937x)
+--------------+ +--------------+
| +--------+ | | +--------+ |
ADC1 ----->| | PORT1 | | | | TX1 |
|<-----------ADC1
ADC2 ----->| | | | | | | |
| +--------+ | | +--------+ |
| | | |
ADC3 ----->| +--------+ | | +--------+ |
| | PORT2 | | | | TX2 |
|<-----------ADC2
| | | | | | |
|<-----------ADC3
| +--------+ | | +--------+ |
| | | |
| +--------+ | | +--------+ |
DMIC0...DMIC3------>| | PORT3 | | | | TX3 |
|<-----------DMIC0...DMIC3
| | | | | | |
|<-----------MBHC
| +--------+ | | +--------+ |
| | | |
| +--------+ | | +--------+ |
DMIC4...DMIC3 ----->| | PORT4 | | | | TX4 |
|<-----------DMIC4...DMIC7
| | | | | | | |
| +--------+ | | +--------+ |
| | | |
+------------- + +--------------+
For ADC2, The Slave needs to configure TX2 Port with channel mask value
1 and
For Master, it required PORT1 with channel mask value 2.
In existing design master and slave configured with same channel mask,
it will fail ADC2.
The new design will help to configure channel mapping between master and
slave from DT.
>>
>> With the introduction of the following channel mapping properties, it is now possible
>> to configure the master channel mask directly from the device tree.
>>
>> The qcom,tx-channel-mapping property specifies the static channel mapping between the slave
>> and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4,
>> dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
>
> I still don't get what is the channel here.
>
Typo error,
The qcom,tx-channel-mapping property specifies the static channel
mapping between the slave
and master tx ports in the order of slave port channel index which are
adc1, adc2, adc3, adc4,
dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x.h#L599
Will be fixed in the next version
>>
>> The qcom,rx-channel-mapping property specifies static channel mapping between the slave
>> and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh,
>> comp_l, comp_r, lo, dsd_r, dsd_l.
>
> And this description copies binding :/.
>
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
Ack
>>
>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> ---
>> .../bindings/sound/qcom,wcd937x-sdw.yaml | 36 +++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> index d3cf8f59cb23..a6bc9b391db0 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> @@ -58,6 +58,38 @@ properties:
>> items:
>> enum: [1, 2, 3, 4, 5]
>>
>> + qcom,tx-channel-mapping:
>> + description: |
>> + Specifies static channel mapping between slave and master tx port
>> + channels.
>> + In the order of slave port channels which is adc1, adc2, adc3, adc4,
>> + dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
>> + ch_mask1 ==> bit mask value 1
>> + ch_mask2 ==> bit mask value 2
>> + ch_mask3 ==> bit mask value 4
>> + ch_mask4 ==> bit mask value 8
>> + $ref: /schemas/types.yaml#/definitions/uint8-array
>> + minItems: 8
>> + maxItems: 13
>
> Why size is variable? This device has fixed amount of slave ports, I
> think.
>
yes will check modify
>> + items:
>> + enum: [1, 2, 4, 8]
>
> What is the point of using bits if you cannot actually create a bit mask
> out of it? Why this cannot be 7?
>
Actually, these values should be fixed: 1 (0001), 2 (0010), 4(0100),
8(1000).
If required to set 7, it is handled in wcd driver based on mixer commands.
https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x.c#L1199
Example:
WCD937X_HPH_L -> channel mask value is 1
WCD937X_HPH_R -> channel mask value is 2
The final channel mask for that specific port is 3
>> +
>> + qcom,rx-channel-mapping:
>> + description: |
>> + Specifies static channels mapping between slave and master rx port
>> + channels.
>> + In the order of slave port channels, which is
>> + hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l.
>> + ch_mask1 ==> bit mask value 1
>> + ch_mask2 ==> bit mask value 2
>> + ch_mask3 ==> bit mask value 4
>> + ch_mask4 ==> bit mask value 8
>
> and the value is what exactly? Index is channel, but what does "ch_mask4 ==> bit
> mask value 8" mean? I don't understand this at all.
>
Master
+--------------+
| +--------+ |
ADC1 ----->| | PORT1 | |
ADC2 ----->| | | |
| +--------+ |
| |
ADC3 ----->| +--------+ |
| | PORT2 | |
| | | |
| +--------+ |
| |
| +--------+ |
DMIC0...DMIC3 ---->| | PORT3 | |
| | | |
| +--------+ |
| |
| +--------+ |
DMIC4...DMIC7----->| | PORT4 | |
| | | |
| +--------+ |
| |
+------------- +
The PORT1 has 2 ADC connections,
ADC1 -> PORT1 ch_mask index 1 -> channel mask value 1 (0001)
ADC2 -> PORT1 ch_mask index 2 -> channel mask value 2 (0010)
DMIC0 -> PORT3 ch_mask index 1 -> channel mask value 1 (0001)
DMIC1 -> PORT3 ch_mask index 2 -> channel mask value 2 (0010)
DMIC2 -> PORT3 ch_mask index 3 -> channel mask value 4 (0100)
DMIC3 -> PORT3 ch_mask index 4 -> channel mask value 8 (1000)
Will check and add a proper description.
>> + $ref: /schemas/types.yaml#/definitions/uint8-array
>> + minItems: 8
>> + maxItems: 8
>> + items:
>> + enum: [1, 2, 4, 8]
>> +
>> required:
>> - compatible
>> - reg
>> @@ -74,6 +106,8 @@ examples:
>> compatible = "sdw20217010a00";
>> reg = <0 4>;
>> qcom,rx-port-mapping = <1 2 3 4 5>;
>> + qcom,rx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02
>> + 0x01 0x01 0x02>;
>> };
>> };
>>
>> @@ -85,6 +119,8 @@ examples:
>> compatible = "sdw20217010a00";
>> reg = <0 3>;
>> qcom,tx-port-mapping = <2 2 3 4>;
>> + qcom,tx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02 0x04
>> + 0x04 0x08 0x01 0x02 0x04 0x8>;
>
> Keep it consistent, e.g. everywhere without leading 0... actually not
> sure why this is hex, either.
>
Ack, okay will check and modify.
For hex value, Actually took the reference from port mask values in swr
DT entry.
https://elixir.bootlin.com/linux/v6.12-rc5/source/arch/arm64/boot/dts/qcom/sc7280.dtsi#L2528
>> };
>> };
>>
>> --
>> 2.25.1
Thanks & Regards,
Rafi.
>>
On 30/10/2024 06:07, Mohammad Rafi Shaik wrote: > On 10/23/2024 1:22 PM, Krzysztof Kozlowski wrote: >> On Wed, Oct 23, 2024 at 11:43:23AM +0530, Mohammad Rafi Shaik wrote: >>> Add static channel mapping between master and slave rx/tx ports for >>> Qualcomm wcd937x soundwire codec. >>> >>> Currently, the channel mask for each soundwire port is hardcoded in the >>> wcd937x-sdw driver, and the same channel mask value is configured in the >>> soundwire master. >>> >>> The Qualcomm boards like the QCM6490-IDP require different channel mask settings >>> for the soundwire master and slave ports. >> >> Different than what? Other wcd937x? Which are these? >> > For Qualcomm QCM6490-IDP board soundwire master needs a different > channel mask setting. I understand, but I asked different than which board? Maybe all boards needs this different channel setting, so basically it is "not different". > > The wcd937x channel mask values are hardcoded in wcd driver. > https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x-sdw.c#L35 > https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd938x-sdw.c#L37 > > In case of QCM6490-IDP the soundwire master and wcd937x require > different channel mask settings, not the same. > For Example, wcd937x ADC2 connection > > Master Slave (wcd937x) > +--------------+ +--------------+ > | +--------+ | | +--------+ | > ADC1 ----->| | PORT1 | | | | TX1 | > |<-----------ADC1 > ADC2 ----->| | | | | | | | > | +--------+ | | +--------+ | > | | | | > ADC3 ----->| +--------+ | | +--------+ | > | | PORT2 | | | | TX2 | > |<-----------ADC2 > | | | | | | | > |<-----------ADC3 > | +--------+ | | +--------+ | > | | | | > | +--------+ | | +--------+ | > DMIC0...DMIC3------>| | PORT3 | | | | TX3 | > |<-----------DMIC0...DMIC3 > | | | | | | | > |<-----------MBHC > | +--------+ | | +--------+ | > | | | | > | +--------+ | | +--------+ | > DMIC4...DMIC3 ----->| | PORT4 | | | | TX4 | > |<-----------DMIC4...DMIC7 > | | | | | | | | > | +--------+ | | +--------+ | > | | | | > +------------- + +--------------+ > > > For ADC2, The Slave needs to configure TX2 Port with channel mask value > 1 and > For Master, it required PORT1 with channel mask value 2. > > > In existing design master and slave configured with same channel mask, > it will fail ADC2. > The new design will help to configure channel mapping between master and > slave from DT. > >>> >>> With the introduction of the following channel mapping properties, it is now possible >>> to configure the master channel mask directly from the device tree. >>> >>> The qcom,tx-channel-mapping property specifies the static channel mapping between the slave >>> and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4, >>> dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7. >> >> I still don't get what is the channel here. >> > Typo error, > > The qcom,tx-channel-mapping property specifies the static channel > mapping between the slave > > and master tx ports in the order of slave port channel index which are > adc1, adc2, adc3, adc4, > > dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7. > > > > https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x.h#L599 > > > > Will be fixed in the next version > >>> >>> The qcom,rx-channel-mapping property specifies static channel mapping between the slave >>> and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh, >>> comp_l, comp_r, lo, dsd_r, dsd_l. >> >> And this description copies binding :/. >> >> Please wrap commit message according to Linux coding style / submission >> process (neither too early nor over the limit): >> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597 >> > > Ack > >>> >>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> >>> --- >>> .../bindings/sound/qcom,wcd937x-sdw.yaml | 36 +++++++++++++++++++ >>> 1 file changed, 36 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >>> index d3cf8f59cb23..a6bc9b391db0 100644 >>> --- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >>> @@ -58,6 +58,38 @@ properties: >>> items: >>> enum: [1, 2, 3, 4, 5] >>> >>> + qcom,tx-channel-mapping: >>> + description: | >>> + Specifies static channel mapping between slave and master tx port >>> + channels. >>> + In the order of slave port channels which is adc1, adc2, adc3, adc4, >>> + dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7. >>> + ch_mask1 ==> bit mask value 1 >>> + ch_mask2 ==> bit mask value 2 >>> + ch_mask3 ==> bit mask value 4 >>> + ch_mask4 ==> bit mask value 8 >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + minItems: 8 >>> + maxItems: 13 >> >> Why size is variable? This device has fixed amount of slave ports, I >> think. >> > > yes will check modify > >>> + items: >>> + enum: [1, 2, 4, 8] >> >> What is the point of using bits if you cannot actually create a bit mask >> out of it? Why this cannot be 7? >> > Actually, these values should be fixed: 1 (0001), 2 (0010), 4(0100), > 8(1000). What is fixed here? I asked why these look like bitmasks but they cannot be used as bitmask. This is a mapping, so index is channel slave port channel number and the value is master port channel number, no? > > > If required to set 7, it is handled in wcd driver based on mixer commands. > https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x.c#L1199 I talk about binding. Why you are not allowing value of 7 if this is a mask? If this is not a mask - property says it is channel mapping - then these should be [1-4]. > > > Example: > WCD937X_HPH_L -> channel mask value is 1 > WCD937X_HPH_R -> channel mask value is 2 > > > The final channel mask for that specific port is 3 >>> + >>> + qcom,rx-channel-mapping: >>> + description: | >>> + Specifies static channels mapping between slave and master rx port >>> + channels. >>> + In the order of slave port channels, which is >>> + hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l. >>> + ch_mask1 ==> bit mask value 1 >>> + ch_mask2 ==> bit mask value 2 >>> + ch_mask3 ==> bit mask value 4 >>> + ch_mask4 ==> bit mask value 8 >> >> and the value is what exactly? Index is channel, but what does "ch_mask4 ==> bit >> mask value 8" mean? I don't understand this at all. >> > > Master > +--------------+ > | +--------+ | > ADC1 ----->| | PORT1 | | > ADC2 ----->| | | | > | +--------+ | > | | > ADC3 ----->| +--------+ | > | | PORT2 | | > | | | | > | +--------+ | > | | > | +--------+ | > DMIC0...DMIC3 ---->| | PORT3 | | > | | | | > | +--------+ | > | | > | +--------+ | > DMIC4...DMIC7----->| | PORT4 | | > | | | | > | +--------+ | > | | > +------------- + > > > The PORT1 has 2 ADC connections, > > ADC1 -> PORT1 ch_mask index 1 -> channel mask value 1 (0001) > ADC2 -> PORT1 ch_mask index 2 -> channel mask value 2 (0010) > > > DMIC0 -> PORT3 ch_mask index 1 -> channel mask value 1 (0001) > DMIC1 -> PORT3 ch_mask index 2 -> channel mask value 2 (0010) > DMIC2 -> PORT3 ch_mask index 3 -> channel mask value 4 (0100) > DMIC3 -> PORT3 ch_mask index 4 -> channel mask value 8 (1000) > > > Will check and add a proper description. Best regards, Krzysztof
On 23/10/2024 09:52, Krzysztof Kozlowski wrote: > On Wed, Oct 23, 2024 at 11:43:23AM +0530, Mohammad Rafi Shaik wrote: >> Add static channel mapping between master and slave rx/tx ports for >> Qualcomm wcd937x soundwire codec. >> >> Currently, the channel mask for each soundwire port is hardcoded in the >> wcd937x-sdw driver, and the same channel mask value is configured in the >> soundwire master. >> >> The Qualcomm boards like the QCM6490-IDP require different channel mask settings >> for the soundwire master and slave ports. > > Different than what? Other wcd937x? Which are these? > >> >> With the introduction of the following channel mapping properties, it is now possible >> to configure the master channel mask directly from the device tree. >> >> The qcom,tx-channel-mapping property specifies the static channel mapping between the slave >> and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4, >> dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7. > > I still don't get what is the channel here. > >> >> The qcom,rx-channel-mapping property specifies static channel mapping between the slave >> and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh, >> comp_l, comp_r, lo, dsd_r, dsd_l. > > And this description copies binding :/. > > Please wrap commit message according to Linux coding style / submission > process (neither too early nor over the limit): > https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597 > >> >> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> >> --- >> .../bindings/sound/qcom,wcd937x-sdw.yaml | 36 +++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> index d3cf8f59cb23..a6bc9b391db0 100644 >> --- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> @@ -58,6 +58,38 @@ properties: >> items: >> enum: [1, 2, 3, 4, 5] >> >> + qcom,tx-channel-mapping: >> + description: | >> + Specifies static channel mapping between slave and master tx port >> + channels. >> + In the order of slave port channels which is adc1, adc2, adc3, adc4, >> + dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7. >> + ch_mask1 ==> bit mask value 1 >> + ch_mask2 ==> bit mask value 2 >> + ch_mask3 ==> bit mask value 4 >> + ch_mask4 ==> bit mask value 8 >> + $ref: /schemas/types.yaml#/definitions/uint8-array >> + minItems: 8 >> + maxItems: 13 > > Why size is variable? This device has fixed amount of slave ports, I > think. > >> + items: >> + enum: [1, 2, 4, 8] > > What is the point of using bits if you cannot actually create a bit mask > out of it? Why this cannot be 7? > >> + >> + qcom,rx-channel-mapping: >> + description: | >> + Specifies static channels mapping between slave and master rx port >> + channels. >> + In the order of slave port channels, which is >> + hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l. >> + ch_mask1 ==> bit mask value 1 >> + ch_mask2 ==> bit mask value 2 >> + ch_mask3 ==> bit mask value 4 >> + ch_mask4 ==> bit mask value 8 > > and the value is what exactly? Index is channel, but what does "ch_mask4 ==> bit > mask value 8" mean? I don't understand this at all. Ah, and previous feedback was to use strings, no? Best regards, Krzysztof
© 2016 - 2024 Red Hat, Inc.