| Message ID | 1393387175-15539-2-git-send-email-Li.Xiubo@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
> 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
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.
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); /**
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(-)