[v2,1/3] ASoC: binding: add tdm-slot.txt

Commit Message

Xiubo Li Feb. 12, 2014, 7:45 a.m. UTC

TDM slot:

This specifies audio DAI's TDM slot.

Each entry is has four non-negative integer values in DT:
       <tx_mask, rx_mask, slots, slot_width>

For instance:
       simple-slot-info = <0xffffffc 0xffffffc 2 0>;

And this will be parsed into the following format:
       struct soc_slot_info {
              unsigned int tx_mask;
              unsigned int rx_mask;
              int slots;
              int slot_width;
       };

Signed-off-by: Xiubo Li <[e-mail]>
---
 Documentation/devicetree/bindings/sound/tdm-slot.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/tdm-slot.txt

Comments

Lars-Peter Clausen Feb. 12, 2014, 9:26 a.m. UTC | #1

On 02/12/2014 08:45 AM, Xiubo Li wrote:
> TDM slot:
>
> This specifies audio DAI's TDM slot.
>
> Each entry is has four non-negative integer values in DT:
>         <tx_mask, rx_mask, slots, slot_width>
>
> For instance:
>         simple-slot-info = <0xffffffc 0xffffffc 2 0>;


The current internal API for TDM is very poor, I don't think we want to 
expose that 1 to 1 to the devicetree. Since this means we'd have to support 
that forever. The first thing is that the semantics of 
snd_soc_dai_set_tdm_slot() are very unclear. E.g. some drivers use a zero 
bit for a active slot, some drivers use a 1 bit for a active slot. The 
second thing is that we are not able to specify which channel should be 
mapped to which slot. You can merely specify from/to which slots the CODEC 
should read/write and then it is up to the driver to guess which channel 
should go to which slot. In my opinion a binding that allows to specify a 
explicit mapping of which channel goes to which slot would be much better.

Also those are four different settings. In my opinion they should not be 
expressed in one property, but rather in four. E.g. specifying a tx_mask for 
a rx only device does not make much sense.

- Lars

Mark Brown Feb. 12, 2014, 11:10 a.m. UTC | #2

On Wed, Feb 12, 2014 at 10:26:52AM +0100, Lars-Peter Clausen wrote:

> The current internal API for TDM is very poor, I don't think we want
> to expose that 1 to 1 to the devicetree. Since this means we'd have
> to support that forever. The first thing is that the semantics of
> snd_soc_dai_set_tdm_slot() are very unclear. E.g. some drivers use a
> zero bit for a active slot, some drivers use a 1 bit for a active

Yes, and if we do end up using masks we need to nail down what's going
on in the DT.

> slot. The second thing is that we are not able to specify which
> channel should be mapped to which slot. You can merely specify
> from/to which slots the CODEC should read/write and then it is up to
> the driver to guess which channel should go to which slot. In my
> opinion a binding that allows to specify a explicit mapping of which
> channel goes to which slot would be much better.

It'd certainly be good to be able to do that, though having a default
would make life easier.

> Also those are four different settings. In my opinion they should
> not be expressed in one property, but rather in four. E.g.
> specifying a tx_mask for a rx only device does not make much sense.

That makes sense.

Xiubo Li Feb. 13, 2014, 7:32 a.m. UTC | #3

> > The current internal API for TDM is very poor, I don't think we want
> > to expose that 1 to 1 to the devicetree. Since this means we'd have
> > to support that forever. The first thing is that the semantics of
> > snd_soc_dai_set_tdm_slot() are very unclear. E.g. some drivers use a
> > zero bit for a active slot, some drivers use a 1 bit for a active
> 
> Yes, and if we do end up using masks we need to nail down what's going
> on in the DT.
>

Yes, certainly.

> > slot. The second thing is that we are not able to specify which
> > channel should be mapped to which slot. You can merely specify
> > from/to which slots the CODEC should read/write and then it is up to
> > the driver to guess which channel should go to which slot. In my
> > opinion a binding that allows to specify a explicit mapping of which
> > channel goes to which slot would be much better.
> 
> It'd certainly be good to be able to do that, though having a default
> would make life easier.
> 

@Lars, @Mark,

Yes, then will that means that we can just end up parsing masks from DT
and the masks will be generated by the driver specified
dai->driver->ops->of_xlate_tdm_slot_mask(slots, &tx_mask, &rx_mask)
callback or one default
snd_soc_of_xlate_tdm_slot_mask(slots, &tx_mask, &rx_mask), and the
'slots' is the number of the slot parsed from the DT node ?

Or other better ways ?


Thanks,

--
Best Regards,
Xiubo





> > Also those are four different settings. In my opinion they should
> > not be expressed in one property, but rather in four. E.g.
> > specifying a tx_mask for a rx only device does not make much sense.
> 
> That makes sense.

Patch

diff --git a/Documentation/devicetree/bindings/sound/tdm-slot.txt b/Documentation/devicetree/bindings/sound/tdm-slot.txt
new file mode 100644
index 0000000..33dfb15
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tdm-slot.txt
@@ -0,0 +1,17 @@ 
+TDM slot:
+
+This specifies audio DAI's TDM slot.
+
+Each entry is has four non-negative integer values in DT:
+	<tx_mask, rx_mask, slots, slot_width>
+
+For instance:
+	simple-slot-info = <0xffffffc 0xffffffc 2 0>;
+
+And this will be parsed into the following format:
+	struct soc_slot_info {
+	       unsigned int tx_mask;
+	       unsigned int rx_mask;
+	       int slots;
+	       int slot_width;
+	};