[01/10] ASoC: core: Add snd_soc_dai_set_tdm_slot_xlate().
diff mbox

Message ID 1393387175-15539-2-git-send-email-Li.Xiubo@freescale.com
State New, archived
Headers show

Commit Message

Xiubo Li Feb. 26, 2014, 3:59 a.m. UTC
For most cases the rx_mask and tx_mask params have no use for
snd_soc_dai_set_tdm_slot(), because they could be generated by
{XXX_ .}of_xlate_tdm_slot_mask().

This patch add snd_soc_dai_set_tdm_slot_xlate() which will replace
the snd_soc_dai_set_tdm_slot() in some use cases to simplify the
code. And for some CODECs or CPU DAI devices there needed much more
work to support the .of_xlate_tdm_slot_mask feature.

This patch can be applied to most use case of the current DAI drivers.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 include/sound/soc-dai.h |  3 +++
 sound/soc/soc-core.c    | 33 ++++++++++++++++++++++++++++-----
 2 files changed, 31 insertions(+), 5 deletions(-)

Comments

Lars-Peter Clausen March 1, 2014, 1:19 p.m. UTC | #1
On 02/26/2014 04:59 AM, Xiubo Li wrote:
> For most cases the rx_mask and tx_mask params have no use for
> snd_soc_dai_set_tdm_slot(), because they could be generated by
> {XXX_ .}of_xlate_tdm_slot_mask().
>
> This patch add snd_soc_dai_set_tdm_slot_xlate() which will replace
> the snd_soc_dai_set_tdm_slot() in some use cases to simplify the
> code. And for some CODECs or CPU DAI devices there needed much more
> work to support the .of_xlate_tdm_slot_mask feature.
>
> This patch can be applied to most use case of the current DAI drivers.

Hi,

I'm not quite sure I fully understand what this patch is trying to solve. It 
adds a variant snd_soc_dai_set_tdm_slot() that instead of taking a rx and tx 
mask calculates the masks based on the number of slots? In that case I don't 
really see how the xlate in the name relates to that. xlate is something 
you'd typically expect in a devicetree context. Maybe one should be called 
snd_soc_dai_set_tdm_slot() and the other snd_soc_dai_set_tdm_slot_and_masks()?

But another question is do we really need this? I don't see the problem that 
is solved by this patchset.

- Lars
Mark Brown March 5, 2014, 3:39 a.m. UTC | #2
On Sat, Mar 01, 2014 at 02:19:44PM +0100, Lars-Peter Clausen wrote:

> I'm not quite sure I fully understand what this patch is trying to
> solve. It adds a variant snd_soc_dai_set_tdm_slot() that instead of
> taking a rx and tx mask calculates the masks based on the number of
> slots? In that case I don't really see how the xlate in the name
> relates to that. xlate is something you'd typically expect in a
> devicetree context. Maybe one should be called
> snd_soc_dai_set_tdm_slot() and the other
> snd_soc_dai_set_tdm_slot_and_masks()?

> But another question is do we really need this? I don't see the
> problem that is solved by this patchset.

My understanding is that the patch set aims to provide a way of using
the TDM features of drivers from DT, providing a standardised format for
expressing the TDM setup in the DT.  I've not looked at the actual code
yet though.
Xiubo Li March 5, 2014, 3:55 a.m. UTC | #3
> Subject: Re: [alsa-devel] [PATCH 01/10] ASoC: core: Add
> snd_soc_dai_set_tdm_slot_xlate().
> 
> On Sat, Mar 01, 2014 at 02:19:44PM +0100, Lars-Peter Clausen wrote:
> 
> > I'm not quite sure I fully understand what this patch is trying to
> > solve. It adds a variant snd_soc_dai_set_tdm_slot() that instead of
> > taking a rx and tx mask calculates the masks based on the number of
> > slots? In that case I don't really see how the xlate in the name
> > relates to that. xlate is something you'd typically expect in a
> > devicetree context. Maybe one should be called
> > snd_soc_dai_set_tdm_slot() and the other
> > snd_soc_dai_set_tdm_slot_and_masks()?
> 
> > But another question is do we really need this? I don't see the
> > problem that is solved by this patchset.
> 
> My understanding is that the patch set aims to provide a way of using
> the TDM features of drivers from DT, providing a standardised format for
> expressing the TDM setup in the DT.  I've not looked at the actual code
> yet though.

@Lars,

Sorry for late, many mails had been discard by my outlook, including the
Last one.

@Mark, @Lars,

This adds the function of snd_soc_dai_set_tdm_slot_xlate, which is almost
One new signature of snd_soc_dai_set_tdm_slot discarding the mask
Parameters, which could be generated by itself.

And I want to provide one standard method for the drivers who are parsing
The TDM information from the DT node. 


Thanks,

--
Best Regards,
Xiubo
Mark Brown March 5, 2014, 6:30 a.m. UTC | #4
On Wed, Mar 05, 2014 at 03:55:50AM +0000, Li.Xiubo@freescale.com wrote:

> This adds the function of snd_soc_dai_set_tdm_slot_xlate, which is almost
> One new signature of snd_soc_dai_set_tdm_slot discarding the mask
> Parameters, which could be generated by itself.

Right, so that's not a bad thing but the _xlate() naming is confusing -
Lars' point was that if a function is called _xlate() it would usually
be an operation used as part of DT parsing, not something called from
non-DT code.  The new interface is probably a good thing but needs a
different name (perhaps _simple or something?) or we need to rename the
old one out of the way (it's slightly less flexible so we probably don't
want to remove it totally).

> And I want to provide one standard method for the drivers who are parsing
> The TDM information from the DT node. 

This is a good goal.

Patch
diff mbox

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index d86e0fc..68569ee 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -110,6 +110,9 @@  int snd_soc_dai_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio);
 /* Digital Audio interface formatting */
 int snd_soc_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt);
 
+int snd_soc_dai_set_tdm_slot_xlate(struct snd_soc_dai *dai,
+				   unsigned int slots,
+				   unsigned int slot_width);
 int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
 	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width);
 
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 0911856..e5a535b 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3687,19 +3687,20 @@  static int snd_soc_of_xlate_tdm_slot_mask(unsigned int slots,
 }
 
 /**
- * snd_soc_dai_set_tdm_slot - configure DAI TDM.
+ * snd_soc_dai_set_tdm_slot_xlate - configure DAI TDM with of xlate.
  * @dai: DAI
- * @tx_mask: bitmask representing active TX slots.
- * @rx_mask: bitmask representing active RX slots.
  * @slots: Number of slots in use.
  * @slot_width: Width in bits for each slot.
  *
  * Configures a DAI for TDM operation. Both mask and slots are codec and DAI
  * specific.
  */
-int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
-	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
+int snd_soc_dai_set_tdm_slot_xlate(struct snd_soc_dai *dai,
+				   unsigned int slots,
+				   unsigned int slot_width)
 {
+	unsigned int tx_mask, rx_mask;
+
 	if (dai->driver && dai->driver->ops->of_xlate_tdm_slot_mask)
 		dai->driver->ops->of_xlate_tdm_slot_mask(slots,
 						&tx_mask, &rx_mask);
@@ -3712,6 +3713,28 @@  int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
 	else
 		return -ENOTSUPP;
 }
+EXPORT_SYMBOL_GPL(snd_soc_dai_set_tdm_slot_xlate);
+
+/**
+ * snd_soc_dai_set_tdm_slot - configure DAI TDM.
+ * @dai: DAI
+ * @tx_mask: bitmask representing active TX slots.
+ * @rx_mask: bitmask representing active RX slots.
+ * @slots: Number of slots in use.
+ * @slot_width: Width in bits for each slot.
+ *
+ * Configures a DAI for TDM operation. Both mask and slots are codec and DAI
+ * specific.
+ */
+int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
+{
+	if (dai->driver && dai->driver->ops->set_tdm_slot)
+		return dai->driver->ops->set_tdm_slot(dai, tx_mask, rx_mask,
+				slots, slot_width);
+	else
+		return -ENOTSUPP;
+}
 EXPORT_SYMBOL_GPL(snd_soc_dai_set_tdm_slot);
 
 /**