| Message ID | dc4e5a52a90982cea95445e27a30f2ad6cb73513.1439349623.git.zidan.wang@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 12, 2015 at 11:22:30AM +0800, Zidan Wang wrote: > add wm8960 support for fsl-asoc-card > > Signed-off-by: Zidan Wang <zidan.wang@freescale.com> > + gpr_np = of_parse_phandle(np, "gpr", 0); > + if (gpr_np) { > + priv->gpr = syscon_node_to_regmap(gpr_np); > + if (IS_ERR(priv->gpr)) { > + ret = PTR_ERR(priv->gpr); > + dev_err(&pdev->dev, "failed to get gpr regmap\n"); > + goto fail; > + } > + /* > + * for imx6ul we should set SAI2_MCLK_DIR to enable > + * codec MCLK > + */ > + if (of_device_is_compatible(np, "fsl,imx6ul-evk-wm8960")) > + regmap_update_bits(priv->gpr, 4, 1 << 20, 1 << 20); What's this SAI2_MCLK_DIR? Can you provide the description of it? And why not put it inside: + } else if (of_device_is_compatible(np, "fsl,imx-audio-wm8960")) { + priv->card.set_bias_level = fsl_asoc_card_set_bias_level; + priv->codec_priv.fll_id = WM8960_SYSCLK_AUTO; + priv->codec_priv.pll_id = WM8960_SYSCLK_AUTO; + priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM; > @@ -521,7 +545,10 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) > /* Normal DAI Link */ > priv->dai_link[0].cpu_of_node = cpu_np; > priv->dai_link[0].codec_of_node = codec_np; > - priv->dai_link[0].codec_dai_name = codec_dev->name; > + if (strstr(codec_dev->name, "wm8960")) > + priv->dai_link[0].codec_dai_name = "wm8960-hifi"; > + else > + priv->dai_link[0].codec_dai_name = codec_dev->name; What happens to the codec_dev->name over here so that you need to specify the name? Nicolin
On Tue, Aug 11, 2015 at 08:42:59PM -0700, Nicolin Chen wrote: > On Wed, Aug 12, 2015 at 11:22:30AM +0800, Zidan Wang wrote: > > add wm8960 support for fsl-asoc-card > > > > Signed-off-by: Zidan Wang <zidan.wang@freescale.com> > > > + gpr_np = of_parse_phandle(np, "gpr", 0); > > + if (gpr_np) { > > + priv->gpr = syscon_node_to_regmap(gpr_np); > > + if (IS_ERR(priv->gpr)) { > > + ret = PTR_ERR(priv->gpr); > > + dev_err(&pdev->dev, "failed to get gpr regmap\n"); > > + goto fail; > > + } > > + /* > > + * for imx6ul we should set SAI2_MCLK_DIR to enable > > + * codec MCLK > > + */ > > + if (of_device_is_compatible(np, "fsl,imx6ul-evk-wm8960")) > > + regmap_update_bits(priv->gpr, 4, 1 << 20, 1 << 20); > > What's this SAI2_MCLK_DIR? Can you provide the description of it? SAI2_MCLK_DIR: SD1_CLK data direction control when sai2.MCLK is selected (ALT2) 0 SD1_CLK output driver is disabled when configured for ALT2 1 SD1_CLK output driver is enabled when configured for ALT2 We are using SAI2 now, so we should enable SAI2_MCLK_DIR. If we want to use SAI1 and SAI3, we should also enable SAI1_MCLK_DIR and SAI3_MCLK_DIR. I don't consider SAI1 and SAI3 case, should i add it like below: gpr = <&gpr reg shift val> > > And why not put it inside: > + } else if (of_device_is_compatible(np, "fsl,imx-audio-wm8960")) { > + priv->card.set_bias_level = fsl_asoc_card_set_bias_level; > + priv->codec_priv.fll_id = WM8960_SYSCLK_AUTO; > + priv->codec_priv.pll_id = WM8960_SYSCLK_AUTO; > + priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM; > Just imx6ul should enable such bit. Put inside is also ok. > > @@ -521,7 +545,10 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) > > /* Normal DAI Link */ > > priv->dai_link[0].cpu_of_node = cpu_np; > > priv->dai_link[0].codec_of_node = codec_np; > > - priv->dai_link[0].codec_dai_name = codec_dev->name; > > + if (strstr(codec_dev->name, "wm8960")) > > + priv->dai_link[0].codec_dai_name = "wm8960-hifi"; > > + else > > + priv->dai_link[0].codec_dai_name = codec_dev->name; > > What happens to the codec_dev->name over here so that you need to > specify the name? In wm8960 codec driver, codec dai name is "wm8960-hifi". Using codec_dev->name will cauce register card failed. > > Nicolin
On Wed, Aug 12, 2015 at 12:45:10PM +0800, Zidan Wang wrote: > > > + /* > > > + * for imx6ul we should set SAI2_MCLK_DIR to enable > > > + * codec MCLK > > > + */ > > > + if (of_device_is_compatible(np, "fsl,imx6ul-evk-wm8960")) > > > + regmap_update_bits(priv->gpr, 4, 1 << 20, 1 << 20); > > What's this SAI2_MCLK_DIR? Can you provide the description of it? > SAI2_MCLK_DIR: SD1_CLK data direction control when sai2.MCLK is selected (ALT2) > 0 SD1_CLK output driver is disabled when configured for ALT2 > 1 SD1_CLK output driver is enabled when configured for ALT2 > We are using SAI2 now, so we should enable SAI2_MCLK_DIR. If we want to > use SAI1 and SAI3, we should also enable SAI1_MCLK_DIR and SAI3_MCLK_DIR. > I don't consider SAI1 and SAI3 case, should i add it like below: Yes. Just think about a general interface/configuration that can satisfy all the MCLK cases you can imagine. Since it's a clock dealing with SAI, I think it would be better to put the configuration inside: 494 } else if (strstr(cpu_np->name, "sai")) { 495 priv->cpu_priv.sysclk_id[1] = FSL_SAI_CLK_MAST1; 496 priv->cpu_priv.sysclk_id[0] = FSL_SAI_CLK_MAST1; 497 } And I still don't understand the meaning of this MCLK at all. Does it indicate SAI's MCLK or an MCLK for external CODEC chips? Will it be possible that a user needs this MCLK bit to be unset? Can you share the document to me privately so I can give you a suggestion in detail? > > > - priv->dai_link[0].codec_dai_name = codec_dev->name; > > > + if (strstr(codec_dev->name, "wm8960")) > > > + priv->dai_link[0].codec_dai_name = "wm8960-hifi"; > > > + else > > > + priv->dai_link[0].codec_dai_name = codec_dev->name; > > What happens to the codec_dev->name over here so that you need to > > specify the name? > In wm8960 codec driver, codec dai name is "wm8960-hifi". > Using codec_dev->name will cauce register card failed. When we meet a problem, the first response should be to find out the route cause instead of fixing it with a work around directly. If the codec_dev->name caused a card registration failure, you need to find out why. There must be something wrong with its codec driver or this sound card driver. Nicolin
On Tue, Aug 11, 2015 at 11:40:54PM -0700, Nicolin Chen wrote: > On Wed, Aug 12, 2015 at 12:45:10PM +0800, Zidan Wang wrote: > > > > > + /* > > > > + * for imx6ul we should set SAI2_MCLK_DIR to enable > > > > + * codec MCLK > > > > + */ > > > > + if (of_device_is_compatible(np, "fsl,imx6ul-evk-wm8960")) > > > > + regmap_update_bits(priv->gpr, 4, 1 << 20, 1 << 20); > > > > What's this SAI2_MCLK_DIR? Can you provide the description of it? > > SAI2_MCLK_DIR: SD1_CLK data direction control when sai2.MCLK is selected (ALT2) > > 0 SD1_CLK output driver is disabled when configured for ALT2 > > 1 SD1_CLK output driver is enabled when configured for ALT2 > > > We are using SAI2 now, so we should enable SAI2_MCLK_DIR. If we want to > > use SAI1 and SAI3, we should also enable SAI1_MCLK_DIR and SAI3_MCLK_DIR. > > I don't consider SAI1 and SAI3 case, should i add it like below: > > Yes. Just think about a general interface/configuration that can > satisfy all the MCLK cases you can imagine. > > Since it's a clock dealing with SAI, I think it would be better to > put the configuration inside: > > 494 } else if (strstr(cpu_np->name, "sai")) { > 495 priv->cpu_priv.sysclk_id[1] = FSL_SAI_CLK_MAST1; > 496 priv->cpu_priv.sysclk_id[0] = FSL_SAI_CLK_MAST1; > 497 } > > And I still don't understand the meaning of this MCLK at all. > Does it indicate SAI's MCLK or an MCLK for external CODEC chips? > Will it be possible that a user needs this MCLK bit to be unset? For imx6ul-evk, MCLK for wm8960 CODEC chips and SAI's MCLK is the same pin. So it should always be set for SAI. > > Can you share the document to me privately so I can give you a > suggestion in detail? > > > > > - priv->dai_link[0].codec_dai_name = codec_dev->name; > > > > + if (strstr(codec_dev->name, "wm8960")) > > > > + priv->dai_link[0].codec_dai_name = "wm8960-hifi"; > > > > + else > > > > + priv->dai_link[0].codec_dai_name = codec_dev->name; > > > > What happens to the codec_dev->name over here so that you need to > > > specify the name? > > In wm8960 codec driver, codec dai name is "wm8960-hifi". > > Using codec_dev->name will cauce register card failed. > > When we meet a problem, the first response should be to find out > the route cause instead of fixing it with a work around directly. > If the codec_dev->name caused a card registration failure, you > need to find out why. There must be something wrong with its codec > driver or this sound card driver. > The root cause is that the codec dai name is not always be codec_dev->name. It will cause the sound card can't find such codec dai. You can refer to function snd_soc_find_dai(). For wm8962 the codec dai name and codec_dev->name are "wm8962". But some other codec like wm8960, it may have different codec dai name. > Nicolin
On Wed, Aug 12, 2015 at 03:39:00PM +0800, Zidan Wang wrote: > > > > > - priv->dai_link[0].codec_dai_name = codec_dev->name; > > > > > + if (strstr(codec_dev->name, "wm8960")) > > > > > + priv->dai_link[0].codec_dai_name = "wm8960-hifi"; > > > > > + else > > > > > + priv->dai_link[0].codec_dai_name = codec_dev->name; > > > > > > What happens to the codec_dev->name over here so that you need to > > > > specify the name? > > > In wm8960 codec driver, codec dai name is "wm8960-hifi". > > > Using codec_dev->name will cauce register card failed. > > > > When we meet a problem, the first response should be to find out > > the route cause instead of fixing it with a work around directly. > > If the codec_dev->name caused a card registration failure, you > > need to find out why. There must be something wrong with its codec > > driver or this sound card driver. > > > The root cause is that the codec dai name is not always be codec_dev->name. > It will cause the sound card can't find such codec dai. You can refer to > function snd_soc_find_dai(). > For wm8962 the codec dai name and codec_dev->name are "wm8962". But some > other codec like wm8960, it may have different codec dai name. Then we should fix this by finding out a better way to get the dai name. I will look into it tomorrow.
On Wed, Aug 12, 2015 at 12:22 AM, Zidan Wang <zidan.wang@freescale.com> wrote: > --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt > +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt > @@ -59,6 +59,8 @@ Optional properties: > - audio-asrc : The phandle of ASRC. It can be absent if there's no > need to add ASRC support via DPCM. > > + - gpr : The gpr handle This deserves a better explanation.
On Wed, Aug 12, 2015 at 03:39:00PM +0800, Zidan Wang wrote: > On Tue, Aug 11, 2015 at 11:40:54PM -0700, Nicolin Chen wrote: > > On Wed, Aug 12, 2015 at 12:45:10PM +0800, Zidan Wang wrote: > > > > > > > + /* > > > > > + * for imx6ul we should set SAI2_MCLK_DIR to enable > > > > > + * codec MCLK > > > > > + */ > > > > > + if (of_device_is_compatible(np, "fsl,imx6ul-evk-wm8960")) > > > > > + regmap_update_bits(priv->gpr, 4, 1 << 20, 1 << 20); > > > > > > What's this SAI2_MCLK_DIR? Can you provide the description of it? > > > SAI2_MCLK_DIR: SD1_CLK data direction control when sai2.MCLK is selected (ALT2) > > > 0 SD1_CLK output driver is disabled when configured for ALT2 > > > 1 SD1_CLK output driver is enabled when configured for ALT2 > > > > > We are using SAI2 now, so we should enable SAI2_MCLK_DIR. If we want to > > > use SAI1 and SAI3, we should also enable SAI1_MCLK_DIR and SAI3_MCLK_DIR. > > > I don't consider SAI1 and SAI3 case, should i add it like below: > > > > Yes. Just think about a general interface/configuration that can > > satisfy all the MCLK cases you can imagine. > > And I still don't understand the meaning of this MCLK at all. > > Does it indicate SAI's MCLK or an MCLK for external CODEC chips? > > Will it be possible that a user needs this MCLK bit to be unset? > For imx6ul-evk, MCLK for wm8960 CODEC chips and SAI's MCLK is the same pin. > So it should always be set for SAI. Since the MCLK is driven to the external CODEC chip, it should be similar to the CLKO for wm8962 on the imx6qdl-sabresd board. So this configuration might be included in the clock tree in my opinion. And I Just checked the Reference Manual, it seems the direction could be totally set to input by leaving this bit unset: MCLK outputs via the SD1_CLK pad of IOMUX when setting this bit in the GPR; and MCLK inputs from an external clock source on the board level when clearing this bit. The SAI chapter mentions something similar in its MCLK section as well. But I can't find any clock MUX for the MCLK of SAI by selecting between the CCM and the external PAD: there is only a MUX inside the CCM. So I think you need to confirm with the IC team about this bit and the clock source of SAI MCLK. And there is a crucial problem I can imagine: Is it okay to set this bit while setting SD1_CLK PAD as one of other functions rather than SAI MCLK? -- Customers might keep your code as they also use SAI2 while using SD1_CLK PAD as a GPIO input. Nicolin
On Wed, Aug 12, 2015 at 03:27:36PM -0700, Nicolin Chen wrote: > On Wed, Aug 12, 2015 at 03:39:00PM +0800, Zidan Wang wrote: > > On Tue, Aug 11, 2015 at 11:40:54PM -0700, Nicolin Chen wrote: > > > On Wed, Aug 12, 2015 at 12:45:10PM +0800, Zidan Wang wrote: > > > > > > > > > + /* > > > > > > + * for imx6ul we should set SAI2_MCLK_DIR to enable > > > > > > + * codec MCLK > > > > > > + */ > > > > > > + if (of_device_is_compatible(np, "fsl,imx6ul-evk-wm8960")) > > > > > > + regmap_update_bits(priv->gpr, 4, 1 << 20, 1 << 20); > > > > > > > > What's this SAI2_MCLK_DIR? Can you provide the description of it? > > > > SAI2_MCLK_DIR: SD1_CLK data direction control when sai2.MCLK is selected (ALT2) > > > > 0 SD1_CLK output driver is disabled when configured for ALT2 > > > > 1 SD1_CLK output driver is enabled when configured for ALT2 > > > > > > > We are using SAI2 now, so we should enable SAI2_MCLK_DIR. If we want to > > > > use SAI1 and SAI3, we should also enable SAI1_MCLK_DIR and SAI3_MCLK_DIR. > > > > I don't consider SAI1 and SAI3 case, should i add it like below: > > > > > > Yes. Just think about a general interface/configuration that can > > > satisfy all the MCLK cases you can imagine. > > > > And I still don't understand the meaning of this MCLK at all. > > > Does it indicate SAI's MCLK or an MCLK for external CODEC chips? > > > Will it be possible that a user needs this MCLK bit to be unset? > > > For imx6ul-evk, MCLK for wm8960 CODEC chips and SAI's MCLK is the same pin. > > So it should always be set for SAI. > > Since the MCLK is driven to the external CODEC chip, it should > be similar to the CLKO for wm8962 on the imx6qdl-sabresd board. > So this configuration might be included in the clock tree in my > opinion. > > And I Just checked the Reference Manual, it seems the direction > could be totally set to input by leaving this bit unset: MCLK > outputs via the SD1_CLK pad of IOMUX when setting this bit in > the GPR; and MCLK inputs from an external clock source on the > board level when clearing this bit. The SAI chapter mentions > something similar in its MCLK section as well. > > But I can't find any clock MUX for the MCLK of SAI by selecting > between the CCM and the external PAD: there is only a MUX inside > the CCM. So I think you need to confirm with the IC team about > this bit and the clock source of SAI MCLK. Don't have such clock MUX. So i think it can't be add to clock tree. > > And there is a crucial problem I can imagine: > Is it okay to set this bit while setting SD1_CLK PAD as one > of other functions rather than SAI MCLK? -- Customers might > keep your code as they also use SAI2 while using SD1_CLK PAD > as a GPIO input. > I just add a gpr node in device tree, if customer want to use SD1_CLK PAD as a GPIO input, remove the gpr node. Best Regards, Zidan Wang > Nicolin
On Fri, Aug 14, 2015 at 10:21:18AM +0800, Zidan Wang wrote: > > And there is a crucial problem I can imagine: > > Is it okay to set this bit while setting SD1_CLK PAD as one > > of other functions rather than SAI MCLK? -- Customers might > > keep your code as they also use SAI2 while using SD1_CLK PAD > > as a GPIO input. > I just add a gpr node in device tree, if customer want to use SD1_CLK > PAD as a GPIO input, remove the gpr node. My question was whether setting this bit would cause a hardware damage -- Customers might not notice your gpr node in the Device Tree at all, not to mention the possibility of an accidental wrong configurations. And another reason I asked that is to find out if we can set these bits anyway in the SoC level driver of imx6ul when this bit would not cause anything dangerous and when there's no extra clock MUX for the SAI MCLK between internal CCM and external clock source. Nicolin
On Thu, Aug 13, 2015 at 08:58:06PM -0700, Nicolin Chen wrote: > On Fri, Aug 14, 2015 at 10:21:18AM +0800, Zidan Wang wrote: > > > > And there is a crucial problem I can imagine: > > > Is it okay to set this bit while setting SD1_CLK PAD as one > > > of other functions rather than SAI MCLK? -- Customers might > > > keep your code as they also use SAI2 while using SD1_CLK PAD > > > as a GPIO input. > > > I just add a gpr node in device tree, if customer want to use SD1_CLK > > PAD as a GPIO input, remove the gpr node. > > My question was whether setting this bit would cause a hardware > damage -- Customers might not notice your gpr node in the Device > Tree at all, not to mention the possibility of an accidental wrong > configurations. > > And another reason I asked that is to find out if we can set these > bits anyway in the SoC level driver of imx6ul when this bit would > not cause anything dangerous and when there's no extra clock MUX > for the SAI MCLK between internal CCM and external clock source. > This bit will not cause any hardware damage. ccm ----->1------------------>2<------------>3 | | | | 4 SAIx MCLK1 5 SAIx MCLK2 If this bit is set, the clock route is ccm->1->2->3, 2 to 5 is disconnect. If this bit is clear, the clock route is 3->2->5, 1 to 2 is disconnect. Best Regards, Zidan Wang > Nicolin
I have to add Shawn for help :) On Fri, Aug 14, 2015 at 03:38:16PM +0800, Zidan Wang wrote: > ccm ----->1------------------>2<------------>3 > | | > | | > 4 SAIx MCLK1 5 SAIx MCLK2 > > If this bit is set, the clock route is ccm->1->2->3, 2 to 5 is disconnect. > If this bit is clear, the clock route is 3->2->5, 1 to 2 is disconnect. @Shawn Suppose 1 and 2 are clock MUXs that we can't specifically touch via registers. And 3 is an IO PAD that can either output a clock from CCM or input a clock from an external source outside SoC. Both 4 and 5 are two clock sources for one single SAI, SAI2 for example. There is a bit in the GPR register to control above routes. And my question is where should be the best place to put this bit. Apparently it's outside the CCM and SAI, not to mention it is hard to relate this bit to this ASoC dai-link driver which's supposed to solve problems between the SAI and CODECs. Thanks Nicolin
On Fri, Aug 14, 2015 at 02:22:53AM -0700, Nicolin Chen wrote: > I have to add Shawn for help :) > > On Fri, Aug 14, 2015 at 03:38:16PM +0800, Zidan Wang wrote: > > > ccm ----->1------------------>2<------------>3 > > | | > > | | > > 4 SAIx MCLK1 5 SAIx MCLK2 > > > > If this bit is set, the clock route is ccm->1->2->3, 2 to 5 is disconnect. > > If this bit is clear, the clock route is 3->2->5, 1 to 2 is disconnect. > > @Shawn > > Suppose 1 and 2 are clock MUXs that we can't specifically touch > via registers. And 3 is an IO PAD that can either output a clock > from CCM or input a clock from an external source outside SoC. > Both 4 and 5 are two clock sources for one single SAI, SAI2 for > example. > > There is a bit in the GPR register to control above routes. And > my question is where should be the best place to put this bit. If this route selection is a decision of board design, it should fit device tree quite well. As the platform maintainer, I surely hope we can handle this in audio driver. But if there is really no place better than platform code to handle it, I can accept it. Shawn > > Apparently it's outside the CCM and SAI, not to mention it is > hard to relate this bit to this ASoC dai-link driver which's > supposed to solve problems between the SAI and CODECs. > > Thanks > Nicolin
Hi Shawn, Sorry for my late reply. On Sun, Sep 06, 2015 at 07:33:44PM +0800, Shawn Guo wrote: > On Fri, Aug 14, 2015 at 02:22:53AM -0700, Nicolin Chen wrote: > > I have to add Shawn for help :) > > > > On Fri, Aug 14, 2015 at 03:38:16PM +0800, Zidan Wang wrote: > > > > > ccm ----->1------------------>2<------------>3 > > > | | > > > | | > > > 4 SAIx MCLK1 5 SAIx MCLK2 > > > > > > If this bit is set, the clock route is ccm->1->2->3, 2 to 5 is disconnect. > > > If this bit is clear, the clock route is 3->2->5, 1 to 2 is disconnect. > > > > @Shawn > > > > Suppose 1 and 2 are clock MUXs that we can't specifically touch > > via registers. And 3 is an IO PAD that can either output a clock > > from CCM or input a clock from an external source outside SoC. > > Both 4 and 5 are two clock sources for one single SAI, SAI2 for > > example. > > > > There is a bit in the GPR register to control above routes. And > > my question is where should be the best place to put this bit. > > If this route selection is a decision of board design, it should fit > device tree quite well. As the platform maintainer, I surely hope we > can handle this in audio driver. But if there is really no place better > than platform code to handle it, I can accept it. I understand your concern. This route decides the source of mclk2, 955 sai1: sai@021d4000 { 956 compatible = "fsl,imx6sx-sai"; 957 reg = <0x021d4000 0x4000>; 958 interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>; 959 clocks = <&clks IMX6SX_CLK_SAI1_IPG>, 960 <&clks IMX6SX_CLK_SAI1>, 961 <&clks 0>, <&clks 0>; 962 clock-names = "bus", "mclk1", "mclk2", "mclk3"; So it doesn't make sense to me if putting it in the audio driver as it then would be out of the clock tree. I'm not familiar with the current platform code. Is that possible we put it in the clock tree? Would you please give me and Zidan another hint for the implementation? Thanks a lot Nicolin
On Thu, Sep 17, 2015 at 01:03:40AM -0700, Nicolin Chen wrote: > Hi Shawn, > > Sorry for my late reply. > > On Sun, Sep 06, 2015 at 07:33:44PM +0800, Shawn Guo wrote: > > On Fri, Aug 14, 2015 at 02:22:53AM -0700, Nicolin Chen wrote: > > > I have to add Shawn for help :) > > > > > > On Fri, Aug 14, 2015 at 03:38:16PM +0800, Zidan Wang wrote: > > > > > > > ccm ----->1------------------>2<------------>3 > > > > | | > > > > | | > > > > 4 SAIx MCLK1 5 SAIx MCLK2 > > > > > > > > If this bit is set, the clock route is ccm->1->2->3, 2 to 5 is disconnect. > > > > If this bit is clear, the clock route is 3->2->5, 1 to 2 is disconnect. > > > > > > @Shawn > > > > > > Suppose 1 and 2 are clock MUXs that we can't specifically touch > > > via registers. And 3 is an IO PAD that can either output a clock > > > from CCM or input a clock from an external source outside SoC. > > > Both 4 and 5 are two clock sources for one single SAI, SAI2 for > > > example. > > > > > > There is a bit in the GPR register to control above routes. And > > > my question is where should be the best place to put this bit. > > > > If this route selection is a decision of board design, it should fit > > device tree quite well. As the platform maintainer, I surely hope we > > can handle this in audio driver. But if there is really no place better > > than platform code to handle it, I can accept it. > > I understand your concern. This route decides the source of mclk2, > > 955 sai1: sai@021d4000 { > 956 compatible = "fsl,imx6sx-sai"; > 957 reg = <0x021d4000 0x4000>; > 958 interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>; > 959 clocks = <&clks IMX6SX_CLK_SAI1_IPG>, > 960 <&clks IMX6SX_CLK_SAI1>, > 961 <&clks 0>, <&clks 0>; > 962 clock-names = "bus", "mclk1", "mclk2", "mclk3"; > > So it doesn't make sense to me if putting it in the audio driver as > it then would be out of the clock tree. > > I'm not familiar with the current platform code. Is that possible we > put it in the clock tree? Would you please give me and Zidan another > hint for the implementation? The platform code we generally mean is the code in arch/arm/mach-imx, mostly these mach-imx*.c files. I guess you would need to define a device tree property to reflect the board design on this mclk2 route, and parse the property in platform code and set up the GPR register accordingly. Shawn
On Fri, Sep 18, 2015 at 09:19:18PM +0800, Shawn Guo wrote: > > > > On Fri, Aug 14, 2015 at 03:38:16PM +0800, Zidan Wang wrote: > > > > > > > > > ccm ----->1------------------>2<------------>3 > > > > > | | > > > > > | | > > > > > 4 SAIx MCLK1 5 SAIx MCLK2 > > > > > > > > > > If this bit is set, the clock route is ccm->1->2->3, 2 to 5 is disconnect. > > > > > If this bit is clear, the clock route is 3->2->5, 1 to 2 is disconnect. > > > > Suppose 1 and 2 are clock MUXs that we can't specifically touch > > > > via registers. And 3 is an IO PAD that can either output a clock > > > > from CCM or input a clock from an external source outside SoC. > > > > Both 4 and 5 are two clock sources for one single SAI, SAI2 for > > > > example. > > > > > > > > There is a bit in the GPR register to control above routes. And > > > > my question is where should be the best place to put this bit. > > > > > > If this route selection is a decision of board design, it should fit > > > device tree quite well. As the platform maintainer, I surely hope we > > > can handle this in audio driver. But if there is really no place better > > > than platform code to handle it, I can accept it. > > > > I understand your concern. This route decides the source of mclk2, > > > > 955 sai1: sai@021d4000 { > > 956 compatible = "fsl,imx6sx-sai"; > > 957 reg = <0x021d4000 0x4000>; > > 958 interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>; > > 959 clocks = <&clks IMX6SX_CLK_SAI1_IPG>, > > 960 <&clks IMX6SX_CLK_SAI1>, > > 961 <&clks 0>, <&clks 0>; > > 962 clock-names = "bus", "mclk1", "mclk2", "mclk3"; > > > > So it doesn't make sense to me if putting it in the audio driver as > > it then would be out of the clock tree. > > > > I'm not familiar with the current platform code. Is that possible we > > put it in the clock tree? Would you please give me and Zidan another > > hint for the implementation? > > The platform code we generally mean is the code in arch/arm/mach-imx, > mostly these mach-imx*.c files. I guess you would need to define a > device tree property to reflect the board design on this mclk2 route, > and parse the property in platform code and set up the GPR register > accordingly. I see. I noticed that the clock related files have been moved out of that directory. But I think we may still be able to register clocks in the clock files and call a set of clk_set_parent() in the platform code depending on the DT property right? -- As I'd love to see this clock route being included in the clock tree so that the mclk2 would have a corresponding rate in order to normally take part in the bclk and fsclk calculations.
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt index a96774c..fa0a10d 100644 --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt @@ -59,6 +59,8 @@ Optional properties: - audio-asrc : The phandle of ASRC. It can be absent if there's no need to add ASRC support via DPCM. + - gpr : The gpr handle + Example: sound-cs42888 { compatible = "fsl,imx-audio-cs42888"; diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c index de43887..c058e68 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -16,6 +16,7 @@ #include <linux/of_platform.h> #include <sound/pcm_params.h> #include <sound/soc.h> +#include <linux/mfd/syscon.h> #include "fsl_esai.h" #include "fsl_sai.h" @@ -23,6 +24,7 @@ #include "../codecs/sgtl5000.h" #include "../codecs/wm8962.h" +#include "../codecs/wm8960.h" #define RX 0 #define TX 1 @@ -90,6 +92,7 @@ struct fsl_asoc_card_priv { u32 asrc_format; u32 dai_fmt; char name[32]; + struct regmap *gpr; }; /** @@ -400,7 +403,7 @@ static int fsl_asoc_card_late_probe(struct snd_soc_card *card) static int fsl_asoc_card_probe(struct platform_device *pdev) { - struct device_node *cpu_np, *codec_np, *asrc_np; + struct device_node *cpu_np, *codec_np, *asrc_np, *gpr_np; struct device_node *np = pdev->dev.of_node; struct platform_device *asrc_pdev = NULL; struct platform_device *cpu_pdev; @@ -443,6 +446,22 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) if (asrc_np) asrc_pdev = of_find_device_by_node(asrc_np); + gpr_np = of_parse_phandle(np, "gpr", 0); + if (gpr_np) { + priv->gpr = syscon_node_to_regmap(gpr_np); + if (IS_ERR(priv->gpr)) { + ret = PTR_ERR(priv->gpr); + dev_err(&pdev->dev, "failed to get gpr regmap\n"); + goto fail; + } + /* + * for imx6ul we should set SAI2_MCLK_DIR to enable + * codec MCLK + */ + if (of_device_is_compatible(np, "fsl,imx6ul-evk-wm8960")) + regmap_update_bits(priv->gpr, 4, 1 << 20, 1 << 20); + } + /* Get the MCLK rate only, and leave it controlled by CODEC drivers */ codec_clk = clk_get(&codec_dev->dev, NULL); if (!IS_ERR(codec_clk)) { @@ -475,6 +494,11 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) priv->codec_priv.fll_id = WM8962_SYSCLK_FLL; priv->codec_priv.pll_id = WM8962_FLL; priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM; + } else if (of_device_is_compatible(np, "fsl,imx-audio-wm8960")) { + priv->card.set_bias_level = fsl_asoc_card_set_bias_level; + priv->codec_priv.fll_id = WM8960_SYSCLK_AUTO; + priv->codec_priv.pll_id = WM8960_SYSCLK_AUTO; + priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM; } else { dev_err(&pdev->dev, "unknown Device Tree compatible\n"); return -EINVAL; @@ -521,7 +545,10 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) /* Normal DAI Link */ priv->dai_link[0].cpu_of_node = cpu_np; priv->dai_link[0].codec_of_node = codec_np; - priv->dai_link[0].codec_dai_name = codec_dev->name; + if (strstr(codec_dev->name, "wm8960")) + priv->dai_link[0].codec_dai_name = "wm8960-hifi"; + else + priv->dai_link[0].codec_dai_name = codec_dev->name; priv->dai_link[0].platform_of_node = cpu_np; priv->dai_link[0].dai_fmt = priv->dai_fmt; priv->card.num_links = 1; @@ -530,7 +557,10 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) /* DPCM DAI Links only if ASRC exsits */ priv->dai_link[1].cpu_of_node = asrc_np; priv->dai_link[1].platform_of_node = asrc_np; - priv->dai_link[2].codec_dai_name = codec_dev->name; + if (strstr(codec_dev->name, "wm8960")) + priv->dai_link[2].codec_dai_name = "wm8960-hifi"; + else + priv->dai_link[2].codec_dai_name = codec_dev->name; priv->dai_link[2].codec_of_node = codec_np; priv->dai_link[2].cpu_of_node = cpu_np; priv->dai_link[2].dai_fmt = priv->dai_fmt; @@ -578,6 +608,7 @@ static const struct of_device_id fsl_asoc_card_dt_ids[] = { { .compatible = "fsl,imx-audio-cs42888", }, { .compatible = "fsl,imx-audio-sgtl5000", }, { .compatible = "fsl,imx-audio-wm8962", }, + { .compatible = "fsl,imx-audio-wm8960", }, {} };
add wm8960 support for fsl-asoc-card Signed-off-by: Zidan Wang <zidan.wang@freescale.com> --- .../devicetree/bindings/sound/fsl-asoc-card.txt | 2 ++ sound/soc/fsl/fsl-asoc-card.c | 37 ++++++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-)