[alsa-devel] [PATCH 0/3] ASoC: atmel_ssc_dai: add option to choose clock
When SSC work in slave mode, the clock can come from TK pin and also can come from RK pin, this is hardware design decided. So, make it available to choose where the clock from.
Bo Shen (3): ASoC: atmel_ssc_dai: make option to choose clock ASoC: atmel_wm8904: make it available to choose clock Binding: atmel-wm8904: add option to choose clock
Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ sound/soc/atmel/atmel_ssc_dai.c | 16 ++++++++++++---- sound/soc/atmel/atmel_ssc_dai.h | 1 + sound/soc/atmel/atmel_wm8904.c | 10 ++++++++++ 4 files changed, 25 insertions(+), 4 deletions(-)
When SSC works in slave mode, according to the hardware design, the clock can get from TK pin, also can get from RK pin. So, add one parameter to choose where the clock from.
Signed-off-by: Bo Shen voice.shen@atmel.com ---
sound/soc/atmel/atmel_ssc_dai.c | 16 ++++++++++++---- sound/soc/atmel/atmel_ssc_dai.h | 1 + 2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c index 8697ced..03eb0be 100644 --- a/sound/soc/atmel/atmel_ssc_dai.c +++ b/sound/soc/atmel/atmel_ssc_dai.c @@ -340,6 +340,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { int id = dai->id; + struct snd_soc_card *card = dai->card; struct atmel_ssc_info *ssc_p = &ssc_info[id]; struct atmel_pcm_dma_params *dma_params; int dir, channels, bits; @@ -347,6 +348,9 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, int start_event; int ret;
+ ssc_p->clk_from_rk_pin = + ((struct atmel_ssc_info *)(card->drvdata))->clk_from_rk_pin; + /* * Currently, there is only one set of dma params for * each direction. If more are added, this code will @@ -466,7 +470,8 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, | SSC_BF(RCMR_START, start_event) | SSC_BF(RCMR_CKI, SSC_CKI_RISING) | SSC_BF(RCMR_CKO, SSC_CKO_NONE) - | SSC_BF(RCMR_CKS, SSC_CKS_CLOCK); + | SSC_BF(RCMR_CKS, ssc_p->clk_from_rk_pin ? + SSC_CKS_PIN : SSC_CKS_CLOCK);
rfmr = SSC_BF(RFMR_FSEDGE, SSC_FSEDGE_POSITIVE) | SSC_BF(RFMR_FSOS, SSC_FSOS_NONE) @@ -481,7 +486,8 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, | SSC_BF(TCMR_START, start_event) | SSC_BF(TCMR_CKI, SSC_CKI_FALLING) | SSC_BF(TCMR_CKO, SSC_CKO_NONE) - | SSC_BF(TCMR_CKS, SSC_CKS_PIN); + | SSC_BF(TCMR_CKS, ssc_p->clk_from_rk_pin ? + SSC_CKS_CLOCK : SSC_CKS_PIN);
tfmr = SSC_BF(TFMR_FSEDGE, SSC_FSEDGE_POSITIVE) | SSC_BF(TFMR_FSDEN, 0) @@ -550,7 +556,8 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, | SSC_BF(RCMR_START, SSC_START_RISING_RF) | SSC_BF(RCMR_CKI, SSC_CKI_RISING) | SSC_BF(RCMR_CKO, SSC_CKO_NONE) - | SSC_BF(RCMR_CKS, SSC_CKS_PIN); + | SSC_BF(RCMR_CKS, ssc_p->clk_from_rk_pin ? + SSC_CKS_PIN : SSC_CKS_CLOCK);
rfmr = SSC_BF(RFMR_FSEDGE, SSC_FSEDGE_POSITIVE) | SSC_BF(RFMR_FSOS, SSC_FSOS_NONE) @@ -565,7 +572,8 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, | SSC_BF(TCMR_START, SSC_START_RISING_RF) | SSC_BF(TCMR_CKI, SSC_CKI_FALLING) | SSC_BF(TCMR_CKO, SSC_CKO_NONE) - | SSC_BF(TCMR_CKS, SSC_CKS_PIN); + | SSC_BF(RCMR_CKS, ssc_p->clk_from_rk_pin ? + SSC_CKS_CLOCK : SSC_CKS_PIN);
tfmr = SSC_BF(TFMR_FSEDGE, SSC_FSEDGE_POSITIVE) | SSC_BF(TFMR_FSDEN, 0) diff --git a/sound/soc/atmel/atmel_ssc_dai.h b/sound/soc/atmel/atmel_ssc_dai.h index b1f08d5..f147895 100644 --- a/sound/soc/atmel/atmel_ssc_dai.h +++ b/sound/soc/atmel/atmel_ssc_dai.h @@ -113,6 +113,7 @@ struct atmel_ssc_info { unsigned short cmr_div; unsigned short tcmr_period; unsigned short rcmr_period; + bool clk_from_rk_pin; struct atmel_pcm_dma_params *dma_params[2]; struct atmel_ssc_state ssc_state; };
Make it available to choose the clock from TK pin or RK pin. This is hardware design decided.
Signed-off-by: Bo Shen voice.shen@atmel.com ---
sound/soc/atmel/atmel_wm8904.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/sound/soc/atmel/atmel_wm8904.c b/sound/soc/atmel/atmel_wm8904.c index b4e3690..b85088d 100644 --- a/sound/soc/atmel/atmel_wm8904.c +++ b/sound/soc/atmel/atmel_wm8904.c @@ -108,6 +108,7 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev) struct device_node *codec_np, *cpu_np; struct snd_soc_card *card = &atmel_asoc_wm8904_card; struct snd_soc_dai_link *dailink = &atmel_asoc_wm8904_dailink; + struct atmel_ssc_info *ssc_info; int ret;
if (!np) { @@ -115,6 +116,15 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev) return -EINVAL; }
+ ssc_info = devm_kzalloc(&pdev->dev, sizeof(*ssc_info), GFP_KERNEL); + if (!ssc_info) + return -ENOMEM; + + ssc_info->clk_from_rk_pin = + of_property_read_bool(np, "clk_from_rk_pin"); + + card->drvdata = (void *)ssc_info; + ret = snd_soc_of_parse_card_name(card, "atmel,model"); if (ret) { dev_err(&pdev->dev, "failed to parse card name\n");
On Tue, Jan 14, 2014 at 11:25:55AM +0800, Bo Shen wrote:
Make it available to choose the clock from TK pin or RK pin. This is hardware design decided.
--- a/sound/soc/atmel/atmel_wm8904.c +++ b/sound/soc/atmel/atmel_wm8904.c @@ -108,6 +108,7 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev) struct device_node *codec_np, *cpu_np; struct snd_soc_card *card = &atmel_asoc_wm8904_card; struct snd_soc_dai_link *dailink = &atmel_asoc_wm8904_dailink;
struct atmel_ssc_info *ssc_info; int ret;
if (!np) {
@@ -115,6 +116,15 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev) return -EINVAL; }
- ssc_info = devm_kzalloc(&pdev->dev, sizeof(*ssc_info), GFP_KERNEL);
- if (!ssc_info)
return -ENOMEM;- ssc_info->clk_from_rk_pin =
of_property_read_bool(np, "clk_from_rk_pin");- card->drvdata = (void *)ssc_info;
Shouldn't this code be in the DAI driver? Otherwise this series looks fine to me, though the DT folks might have something to say I guess.
Hi Mark,
On 01/15/2014 04:36 AM, Mark Brown wrote:
On Tue, Jan 14, 2014 at 11:25:55AM +0800, Bo Shen wrote:
Make it available to choose the clock from TK pin or RK pin. This is hardware design decided.
--- a/sound/soc/atmel/atmel_wm8904.c +++ b/sound/soc/atmel/atmel_wm8904.c @@ -108,6 +108,7 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev) struct device_node *codec_np, *cpu_np; struct snd_soc_card *card = &atmel_asoc_wm8904_card; struct snd_soc_dai_link *dailink = &atmel_asoc_wm8904_dailink;
struct atmel_ssc_info *ssc_info; int ret;
if (!np) {
@@ -115,6 +116,15 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev) return -EINVAL; }
- ssc_info = devm_kzalloc(&pdev->dev, sizeof(*ssc_info), GFP_KERNEL);
- if (!ssc_info)
return -ENOMEM;- ssc_info->clk_from_rk_pin =
of_property_read_bool(np, "clk_from_rk_pin");- card->drvdata = (void *)ssc_info;
Shouldn't this code be in the DAI driver? Otherwise this series looks fine to me, though the DT folks might have something to say I guess.
For audio on Atmel SoC, it depends on three device nodes, one is SSC node, one is the codec node and the sound node. The sound node will parse by machine driver, and machine driver is mainly for hardware connection. As the "clk_from_rk_pin" is decided by hardware, so, I put it here. If I move the code to dai driver, it will parse the sound node in dai driver, I think it will make the code a little bit not explicit. What do you think?
Best Regards, Bo Shen
On Thu, Jan 16, 2014 at 09:31:23AM +0800, Bo Shen wrote:
Shouldn't this code be in the DAI driver? Otherwise this series looks fine to me, though the DT folks might have something to say I guess.
For audio on Atmel SoC, it depends on three device nodes, one is SSC node, one is the codec node and the sound node. The sound node will parse by machine driver, and machine driver is mainly for hardware connection. As the "clk_from_rk_pin" is decided by hardware, so, I put it here. If I move the code to dai driver, it will parse the sound node in dai driver, I think it will make the code a little bit not explicit. What do you think?
I think it should just be a property of the DAI device. It's true that the card defines the connections but if something is going to be an option that's there for most if not all systems then putting it in the device driver means less effort on each integration.
On 14/01/2014 04:25, Bo Shen :
Make it available to choose the clock from TK pin or RK pin. This is hardware design decided.
Signed-off-by: Bo Shen voice.shen@atmel.com
sound/soc/atmel/atmel_wm8904.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/sound/soc/atmel/atmel_wm8904.c b/sound/soc/atmel/atmel_wm8904.c index b4e3690..b85088d 100644 --- a/sound/soc/atmel/atmel_wm8904.c +++ b/sound/soc/atmel/atmel_wm8904.c @@ -108,6 +108,7 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev) struct device_node *codec_np, *cpu_np; struct snd_soc_card *card = &atmel_asoc_wm8904_card; struct snd_soc_dai_link *dailink = &atmel_asoc_wm8904_dailink;
struct atmel_ssc_info *ssc_info; int ret;
if (!np) {
@@ -115,6 +116,15 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev) return -EINVAL; }
- ssc_info = devm_kzalloc(&pdev->dev, sizeof(*ssc_info), GFP_KERNEL);
Isn't an atmel_ssc_info structure table already instantiated in sound/soc/atmel/atmel_ssc_dai.c ... I see, you copy the information contained in this field in the proper ssc_info of the DAI in the previous patch... Well, isn't it a better way to pass parameters to the DAI than this one?
- if (!ssc_info)
return -ENOMEM;- ssc_info->clk_from_rk_pin =
of_property_read_bool(np, "clk_from_rk_pin");- card->drvdata = (void *)ssc_info;
- ret = snd_soc_of_parse_card_name(card, "atmel,model"); if (ret) { dev_err(&pdev->dev, "failed to parse card name\n");
Add the option to choose clock output on which pin connect to SSC. Default is on TK pin to SSC, add clk_from_rk_pin option, the clock is on RK pin to SSC.
Signed-off-by: Bo Shen voice.shen@atmel.com ---
Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..68a5c1a 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -33,6 +33,8 @@ Required properties:
Optional properties: - pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt + - clk_from_rk_pin: according to hardware design, clk privide on rk pin + to ssc device
Example: sound {
On 11:25 Tue 14 Jan , Bo Shen wrote:
Add the option to choose clock output on which pin connect to SSC. Default is on TK pin to SSC, add clk_from_rk_pin option, the clock is on RK pin to SSC.
Signed-off-by: Bo Shen voice.shen@atmel.com
Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..68a5c1a 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -33,6 +33,8 @@ Required properties:
Optional properties:
- pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt
- clk_from_rk_pin: according to hardware design, clk privide on rk pin
provide
?? no example?
- to ssc device
Example: sound { -- 1.8.5.2
Hi J,
On 01/15/2014 07:54 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 11:25 Tue 14 Jan , Bo Shen wrote:
Add the option to choose clock output on which pin connect to SSC. Default is on TK pin to SSC, add clk_from_rk_pin option, the clock is on RK pin to SSC.
Signed-off-by: Bo Shen voice.shen@atmel.com
Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..68a5c1a 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -33,6 +33,8 @@ Required properties:
Optional properties: - pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt
- clk_from_rk_pin: according to hardware design, clk privide on rk pin
provide?? no example?
If this patch set is acceptable. The sama5 audio will use it (I will send patch after this patch set). Do I still need to put the example in the binding document?
to ssc device
Example: sound {
-- 1.8.5.2
Best Regards, Bo Shen
On Thu, Jan 16, 2014 at 09:33:20AM +0800, Bo Shen wrote:
On 01/15/2014 07:54 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
Optional properties:
- pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt
- clk_from_rk_pin: according to hardware design, clk privide on rk pin
provide
?? no example?
If this patch set is acceptable. The sama5 audio will use it (I will send patch after this patch set). Do I still need to put the example in the binding document?
It doesn't seem terribly worthwhile to have the example include every boolean property, it's not like people should find it hard to work out how to use them.
On 14/01/2014 04:25, Bo Shen :
Add the option to choose clock output on which pin connect to SSC. Default is on TK pin to SSC, add clk_from_rk_pin option, the clock
Please do not use "_" in DT properties. It is a common pattern to use "-" instead.
is on RK pin to SSC.
Signed-off-by: Bo Shen voice.shen@atmel.com
Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..68a5c1a 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -33,6 +33,8 @@ Required properties:
Optional properties:
- pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt
- clk_from_rk_pin: according to hardware design, clk privide on rk pin
typo + more description is needed: Please tell that it is a boolean property, that by default it is using the TK pin (like in the commit message in fact).
Bye,
- to ssc device
Example: sound {
Hi Nicolas,
On 01/16/2014 01:17 AM, Nicolas Ferre wrote:
On 14/01/2014 04:25, Bo Shen :
Add the option to choose clock output on which pin connect to SSC. Default is on TK pin to SSC, add clk_from_rk_pin option, the clock
Please do not use "_" in DT properties. It is a common pattern to use "-" instead.
OK, I will use "-" instead in next version.
is on RK pin to SSC.
Signed-off-by: Bo Shen voice.shen@atmel.com
Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..68a5c1a 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -33,6 +33,8 @@ Required properties:
Optional properties: - pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt
- clk_from_rk_pin: according to hardware design, clk privide on rk pin
typo + more description is needed: Please tell that it is a boolean property, that by default it is using the TK pin (like in the commit message in fact).
OK, I will add more description for it.
Thanks
Bye,
to ssc device
Example: sound {
Best Regards, Bo Shen
participants (4)
-
Bo Shen -
Jean-Christophe PLAGNIOL-VILLARD -
Mark Brown -
Nicolas Ferre