| Message ID | 20210621074152.306362-1-judyhsiao@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: snd-soc-dummy: add Device Tree support | expand |
On Mon, Jun 21, 2021 at 03:41:52PM +0800, Judy Hsiao wrote: > Support for loading the snd-soc-dummy via DeviceTree. > This is useful to create dummy codec devices where we need to have some > DAI links without a real Codec. Why would it be useful to create DAI links to a dummy device that has no properties? If you've got a device with no software control it's still going to have some limits on things like what formats and sample rates it can accept so you should describe that in DT. Please try to keep the CC lists for patches you are submitting relevant to the patch, people get a lot of mail and reviews for irrelevant patches add to the noise.
On Mon, Jun 21, 2021 at 7:46 PM Mark Brown <broonie@kernel.org> wrote: > > On Mon, Jun 21, 2021 at 03:41:52PM +0800, Judy Hsiao wrote: > > > Support for loading the snd-soc-dummy via DeviceTree. > > This is useful to create dummy codec devices where we need to have some > > DAI links without a real Codec. > > Why would it be useful to create DAI links to a dummy device that has > no properties? If you've got a device with no software control it's > still going to have some limits on things like what formats and sample > rates it can accept so you should describe that in DT. Thanks for your review comment. This patch is used to support multi-channel where we want one codec to control the only GPIO shared by 4 amps. (Please refer to :https://patchwork.kernel.org/project/alsa-devel/patch/20210526154704.114957-1-judyhsiao@chromium.org/) In snd_soc_runtime_calc_hw(), by creating dummy codecs that share a DAI link with a real codec: 1. The min/ max channel of CPU DAI will be directly adopted. 2. The formats and sample rates of the DAI link will be determined by the real codec unless the real codec supports the rate and format that do not intersect with the rate and format of snd-soc-dummy. That is the reason why we don’t specify the format and sample rates of the dummy codec with the real codec determining the properties . Does reposting a new patch with a more clear commit message to describe the use case sound good to you? > > Please try to keep the CC lists for patches you are submitting relevant > to the patch, people get a lot of mail and reviews for irrelevant > patches add to the noise. Sorry about that. I have adjusted the CC lists.
On Wed, Jun 23, 2021 at 12:10:53AM +0800, Judy Hsiao wrote: > Thanks for your review comment. > This patch is used to support multi-channel where we want one codec to > control the only GPIO shared by 4 amps. So you've got 4 instances of the same CODEC? Then I'd expect to see those all individually represented in DT. Or if there's a single physical CODEC then I'm not sure what the dummies are for? > In snd_soc_runtime_calc_hw(), by creating dummy codecs that share a > DAI link with a real codec: > 1. The min/ max channel of CPU DAI will be directly adopted. > 2. The formats and sample rates of the DAI link will be determined > by the real codec unless the real codec supports the rate > and format that do not intersect with the rate and format of > snd-soc-dummy. > That is the reason why we don’t specify the format and sample rates of > the dummy codec with the real codec determining the properties . It's not clear to me why you'd not just describe the actual CODECs here rather than using a dummy CODEC, the fact that the dummy CODEC is doing what you want is just an accident of the implementation rather than a description of the hardware.
On Wed, Jun 23, 2021 at 12:23 AM Mark Brown <broonie@kernel.org> wrote: > > On Wed, Jun 23, 2021 at 12:10:53AM +0800, Judy Hsiao wrote: > > > Thanks for your review comment. > > This patch is used to support multi-channel where we want one codec to > > control the only GPIO shared by 4 amps. > > So you've got 4 instances of the same CODEC? Then I'd expect to see > those all individually represented in DT. Or if there's a single > physical CODEC then I'm not sure what the dummies are for? > > > In snd_soc_runtime_calc_hw(), by creating dummy codecs that share a > > DAI link with a real codec: > > 1. The min/ max channel of CPU DAI will be directly adopted. > > 2. The formats and sample rates of the DAI link will be determined > > by the real codec unless the real codec supports the rate > > and format that do not intersect with the rate and format of > > snd-soc-dummy. > > That is the reason why we don’t specify the format and sample rates of > > the dummy codec with the real codec determining the properties . > > It's not clear to me why you'd not just describe the actual CODECs here > rather than using a dummy CODEC, the fact that the dummy CODEC is doing > what you want is just an accident of the implementation rather than a > description of the hardware. Thanks for your inputs. Specifying four codes for the multi-channel works fine. We have not thought of specifying four codes before as we want to avoid loading the codec driver multiple times, but actually loading the snd-soc-dummy just has the similar cost. By specifying four codes, the dtsi file describes the real hardware schematic. I will specify four codec in the dtsi file to support the four channel use case and this snd-soc-dummy patch is not needed. Thanks for the discussion!
diff --git a/Documentation/devicetree/bindings/sound/snd-soc-dummy.txt b/Documentation/devicetree/bindings/sound/snd-soc-dummy.txt new file mode 100644 index 000000000000..7fa8c5751e62 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/snd-soc-dummy.txt @@ -0,0 +1,16 @@ +* snd-soc-dummy + +This node models the snd-soc-dummy. +This is useful to create dummy codec devices where we need to have +some DAI links without a real Codec. + +Required properties: +- compatible : "asoc,snd-soc-dummy" + + +Example: + +dummy_codec { + compatible = "asoc,snd-soc-dummy"; + #sound-dai-cells = <0>; +}; diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 299b5d6ebfd1..def2cc687415 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -7,6 +7,8 @@ // Author: Mark Brown <broonie@opensource.wolfsonmicro.com> // Liam Girdwood <lrg@slimlogic.co.uk> +#include <linux/module.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/export.h> #include <sound/core.h> @@ -181,9 +183,18 @@ static int snd_soc_dummy_probe(struct platform_device *pdev) return ret; } +#ifdef CONFIG_OF +static const struct of_device_id soc_dummy_device_id[] = { + { .compatible = "asoc,snd-soc-dummy" }, + {} +}; +MODULE_DEVICE_TABLE(of, soc_dummy_device_id); +#endif + static struct platform_driver soc_dummy_driver = { .driver = { .name = "snd-soc-dummy", + .of_match_table = of_match_ptr(soc_dummy_device_id), }, .probe = snd_soc_dummy_probe, };
Support for loading the snd-soc-dummy via DeviceTree. This is useful to create dummy codec devices where we need to have some DAI links without a real Codec. Signed-off-by: Judy Hsiao <judyhsiao@chromium.org> --- .../devicetree/bindings/sound/snd-soc-dummy.txt | 16 ++++++++++++++++ sound/soc/soc-utils.c | 11 +++++++++++ 2 files changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/snd-soc-dummy.txt