diff mbox

No sound captured with SGTL5000 on i.MX6 in I²S master mode

Message ID CAOMZO5AqtNm3kwndUYszAuMfJ4CMOz4pwZRmE+04cEDtz98agg@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fabio Estevam Sept. 18, 2014, 5:28 p.m. UTC
Hi Nicolin,

On Thu, Sep 18, 2014 at 2:05 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:

> The problem here should be the AUDMUX configuration issue. The imx-
> sgtl5000.c driver only supports CODEC in master mode. So if you try
> to switch the CODEC slave mode, you shall also change not only the
> CBM_CFM to CBS_CFS but also swap the ext_port and int_port of AUDMUX
> (a little confusing approach here as the configuration of AUDMUX is
> routing the data and clocks from a source port to a destination port
> while each of side, external or internal, might be a source port --
> When using CBM_CFM, the source port should be external port; while
> using CBS_CFS, the source port should be the internal port.)

Thanks for the hint on audmux swap! I can get ssi in master mode to
work correctly with the change below:

Comments

Nicolin Chen Sept. 18, 2014, 5:49 p.m. UTC | #1
Hi Fabio,

On Thu, Sep 18, 2014 at 02:28:29PM -0300, Fabio Estevam wrote:
> Thanks for the hint on audmux swap! I can get ssi in master mode to
> work correctly with the change below:

> -               mux-int-port = <2>;
> -               mux-ext-port = <3>;
> +               mux-int-port = <3>;
> +               mux-ext-port = <2>;

Actually this looks a bit tricky as the internal and external ports
are not physically swapped :)

So that's why I put the swapping code in the fsl-asoc-card based on
CBx_CFx configurations.

Thanks
Nicolin
Fabio Estevam Sept. 18, 2014, 5:58 p.m. UTC | #2
Hi Nicolin,

On Thu, Sep 18, 2014 at 2:49 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> Hi Fabio,
>
> On Thu, Sep 18, 2014 at 02:28:29PM -0300, Fabio Estevam wrote:
>> Thanks for the hint on audmux swap! I can get ssi in master mode to
>> work correctly with the change below:
>
>> -               mux-int-port = <2>;
>> -               mux-ext-port = <3>;
>> +               mux-int-port = <3>;
>> +               mux-ext-port = <2>;
>
> Actually this looks a bit tricky as the internal and external ports
> are not physically swapped :)

Yes, this change is just for my own testing/debugging.

> So that's why I put the swapping code in the fsl-asoc-card based on
> CBx_CFx configurations.

Correct, fsl-asoc-card is a nice approach. Good job!
Nicolin Chen Sept. 18, 2014, 6:07 p.m. UTC | #3
On Thu, Sep 18, 2014 at 02:58:06PM -0300, Fabio Estevam wrote:
> > So that's why I put the swapping code in the fsl-asoc-card based on
> > CBx_CFx configurations.
> 
> Correct, fsl-asoc-card is a nice approach. Good job!

I plan to replace imx-sgtl5000 and imx-wm8962 with it as long as
people are happy about the new driver. Would you like to help me
testing it on sgtl5000 side when you have time? Because I only
have a board with WM8962.

Thank you
Nicolin
Fabio Estevam Sept. 18, 2014, 6:14 p.m. UTC | #4
On Thu, Sep 18, 2014 at 3:07 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> On Thu, Sep 18, 2014 at 02:58:06PM -0300, Fabio Estevam wrote:
>> > So that's why I put the swapping code in the fsl-asoc-card based on
>> > CBx_CFx configurations.
>>
>> Correct, fsl-asoc-card is a nice approach. Good job!
>
> I plan to replace imx-sgtl5000 and imx-wm8962 with it as long as
> people are happy about the new driver. Would you like to help me
> testing it on sgtl5000 side when you have time? Because I only
> have a board with WM8962.

Yes, I can test it on a imx53-qsb, which has sgtl5000. Just Cc me when
you have the patch available.

Regards,

Fabio Estevam
Nicolin Chen Sept. 18, 2014, 6:39 p.m. UTC | #5
On Thu, Sep 18, 2014 at 03:14:08PM -0300, Fabio Estevam wrote:
> On Thu, Sep 18, 2014 at 3:07 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > On Thu, Sep 18, 2014 at 02:58:06PM -0300, Fabio Estevam wrote:
> >> > So that's why I put the swapping code in the fsl-asoc-card based on
> >> > CBx_CFx configurations.
> >>
> >> Correct, fsl-asoc-card is a nice approach. Good job!
> >
> > I plan to replace imx-sgtl5000 and imx-wm8962 with it as long as
> > people are happy about the new driver. Would you like to help me
> > testing it on sgtl5000 side when you have time? Because I only
> > have a board with WM8962.
> 
> Yes, I can test it on a imx53-qsb, which has sgtl5000. Just Cc me when
> you have the patch available.

It actually is merged into broonie/for-next and even linux-next I think.
I'll later send a patch to Shawn's tree for imx_v6_v7_defconfig changes
after next merge window's done.

Thank you
Nicolin
Fabio Estevam Sept. 18, 2014, 11:35 p.m. UTC | #6
On Thu, Sep 18, 2014 at 3:39 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:

> It actually is merged into broonie/for-next and even linux-next I think.
> I'll later send a patch to Shawn's tree for imx_v6_v7_defconfig changes
> after next merge window's done.

Ok, I see it now, but master/slave configuration is still hardcoded there:

    } else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000")) {
        priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
        priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
Nicolin Chen Sept. 18, 2014, 11:50 p.m. UTC | #7
On Thu, Sep 18, 2014 at 08:35:31PM -0300, Fabio Estevam wrote:
> On Thu, Sep 18, 2014 at 3:39 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> 
> > It actually is merged into broonie/for-next and even linux-next I think.
> > I'll later send a patch to Shawn's tree for imx_v6_v7_defconfig changes
> > after next merge window's done.
> 
> Ok, I see it now, but master/slave configuration is still hardcoded there:
> 
>     } else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000")) {
>         priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
>         priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;

It's just a combinational driver and also made to be compatible with
old DTs. Unless we refine the DT binding like simple-card, this hard-
code is inevitable. As long as this dai_fmt has been defined, at least
AUDMUX part will be automatically configured.

I made this driver was initially to support ASRC for imx-cs42888. But
merging the code from imx-sgtl5000 and imx-wm8962 allows us to omit
a branch of duplicated code. And that's it :)

Nicolin
Jean-Michel Hautbois Sept. 19, 2014, 7:42 a.m. UTC | #8
Hi,

2014-09-19 1:50 GMT+02:00 Nicolin Chen <nicoleotsuka@gmail.com>:
> On Thu, Sep 18, 2014 at 08:35:31PM -0300, Fabio Estevam wrote:
>> On Thu, Sep 18, 2014 at 3:39 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>>
>> > It actually is merged into broonie/for-next and even linux-next I think.
>> > I'll later send a patch to Shawn's tree for imx_v6_v7_defconfig changes
>> > after next merge window's done.
>>
>> Ok, I see it now, but master/slave configuration is still hardcoded there:
>>
>>     } else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000")) {
>>         priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
>>         priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
>
> It's just a combinational driver and also made to be compatible with
> old DTs. Unless we refine the DT binding like simple-card, this hard-
> code is inevitable. As long as this dai_fmt has been defined, at least
> AUDMUX part will be automatically configured.
>
> I made this driver was initially to support ASRC for imx-cs42888. But
> merging the code from imx-sgtl5000 and imx-wm8962 allows us to omit
> a branch of duplicated code. And that's it :)
>

Well, I may have missed something, I don't have fsl-asoc-card in my
linux tree (vanilla). Where can I find it ? I can test a SGTL5000
master mode on i.MX6Q.

Thanks,
JM
Nicolin Chen Sept. 19, 2014, 7:52 a.m. UTC | #9
On Fri, Sep 19, 2014 at 09:42:22AM +0200, Jean-Michel Hautbois wrote:
> Hi,
> 
> 2014-09-19 1:50 GMT+02:00 Nicolin Chen <nicoleotsuka@gmail.com>:
> > On Thu, Sep 18, 2014 at 08:35:31PM -0300, Fabio Estevam wrote:
> >> On Thu, Sep 18, 2014 at 3:39 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> >>
> >> > It actually is merged into broonie/for-next and even linux-next I think.
> >> > I'll later send a patch to Shawn's tree for imx_v6_v7_defconfig changes
> >> > after next merge window's done.
> >>
> >> Ok, I see it now, but master/slave configuration is still hardcoded there:
> >>
> >>     } else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000")) {
> >>         priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
> >>         priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
> >
> > It's just a combinational driver and also made to be compatible with
> > old DTs. Unless we refine the DT binding like simple-card, this hard-
> > code is inevitable. As long as this dai_fmt has been defined, at least
> > AUDMUX part will be automatically configured.
> >
> > I made this driver was initially to support ASRC for imx-cs42888. But
> > merging the code from imx-sgtl5000 and imx-wm8962 allows us to omit
> > a branch of duplicated code. And that's it :)
> >
> 
> Well, I may have missed something, I don't have fsl-asoc-card in my
> linux tree (vanilla). Where can I find it ? I can test a SGTL5000
> master mode on i.MX6Q.

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

for-next branch.

Nicolin
Jean-Michel Hautbois Sept. 19, 2014, 9:18 a.m. UTC | #10
2014-09-19 9:52 GMT+02:00 Nicolin Chen <nicoleotsuka@gmail.com>:
> On Fri, Sep 19, 2014 at 09:42:22AM +0200, Jean-Michel Hautbois wrote:
>> Hi,
>>
>> 2014-09-19 1:50 GMT+02:00 Nicolin Chen <nicoleotsuka@gmail.com>:
>> > On Thu, Sep 18, 2014 at 08:35:31PM -0300, Fabio Estevam wrote:
>> >> On Thu, Sep 18, 2014 at 3:39 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>> >>
>> >> > It actually is merged into broonie/for-next and even linux-next I think.
>> >> > I'll later send a patch to Shawn's tree for imx_v6_v7_defconfig changes
>> >> > after next merge window's done.
>> >>
>> >> Ok, I see it now, but master/slave configuration is still hardcoded there:
>> >>
>> >>     } else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000")) {
>> >>         priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
>> >>         priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
>> >
>> > It's just a combinational driver and also made to be compatible with
>> > old DTs. Unless we refine the DT binding like simple-card, this hard-
>> > code is inevitable. As long as this dai_fmt has been defined, at least
>> > AUDMUX part will be automatically configured.
>> >
>> > I made this driver was initially to support ASRC for imx-cs42888. But
>> > merging the code from imx-sgtl5000 and imx-wm8962 allows us to omit
>> > a branch of duplicated code. And that's it :)
>> >
>>
>> Well, I may have missed something, I don't have fsl-asoc-card in my
>> linux tree (vanilla). Where can I find it ? I can test a SGTL5000
>> master mode on i.MX6Q.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
>
> for-next branch.
>
> Nicolin

Thanks, now I need to modify my DT in order to have fsl-asoc-card loaded :).
JM
Jean-Michel Hautbois Sept. 19, 2014, 12:53 p.m. UTC | #11
Hi,

2014-09-19 11:18 GMT+02:00 Jean-Michel Hautbois
<jean-michel.hautbois@vodalys.com>:
> 2014-09-19 9:52 GMT+02:00 Nicolin Chen <nicoleotsuka@gmail.com>:
>> On Fri, Sep 19, 2014 at 09:42:22AM +0200, Jean-Michel Hautbois wrote:
>>> Hi,
>>>
>>> 2014-09-19 1:50 GMT+02:00 Nicolin Chen <nicoleotsuka@gmail.com>:
>>> > On Thu, Sep 18, 2014 at 08:35:31PM -0300, Fabio Estevam wrote:
>>> >> On Thu, Sep 18, 2014 at 3:39 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>>> >>
>>> >> > It actually is merged into broonie/for-next and even linux-next I think.
>>> >> > I'll later send a patch to Shawn's tree for imx_v6_v7_defconfig changes
>>> >> > after next merge window's done.
>>> >>
>>> >> Ok, I see it now, but master/slave configuration is still hardcoded there:
>>> >>
>>> >>     } else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000")) {
>>> >>         priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
>>> >>         priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
>>> >
>>> > It's just a combinational driver and also made to be compatible with
>>> > old DTs. Unless we refine the DT binding like simple-card, this hard-
>>> > code is inevitable. As long as this dai_fmt has been defined, at least
>>> > AUDMUX part will be automatically configured.
>>> >
>>> > I made this driver was initially to support ASRC for imx-cs42888. But
>>> > merging the code from imx-sgtl5000 and imx-wm8962 allows us to omit
>>> > a branch of duplicated code. And that's it :)
>>> >
>>>
>>> Well, I may have missed something, I don't have fsl-asoc-card in my
>>> linux tree (vanilla). Where can I find it ? I can test a SGTL5000
>>> master mode on i.MX6Q.
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
>>
>> for-next branch.
>>
>> Nicolin
>
> Thanks, now I need to modify my DT in order to have fsl-asoc-card loaded :).
> JM

FYI, This is working !
I can capture my sound from line in with this :
    sound {
        compatible = "fsl,imx6q-vbx3-sgtl5000",
                 "fsl,imx-audio-sgtl5000";
        model = "imx6q-vbx3-sgtl5000";
        ssi-controller = <&ssi1>;
        audio-codec = <&codec>;
        audio-routing =
            "A1N1L", "Line In Jack",
            "A1N1R", "Line In Jack";
        mux-int-port = <1>;
        mux-ext-port = <5>;
    };

Now, I will add mic support, and I still need to modprobe manually, I
need to know why...

Thanks,
JM
Fabio Estevam Sept. 22, 2014, 3:26 a.m. UTC | #12
On Thu, Sep 18, 2014 at 3:07 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:

> I plan to replace imx-sgtl5000 and imx-wm8962 with it as long as
> people are happy about the new driver. Would you like to help me
> testing it on sgtl5000 side when you have time? Because I only
> have a board with WM8962.

Just tested fsl-asoc-card with sgtl5000 and it worked fine on my imx53-qsb.

These messages are a bit annoying though:

[    1.786201] fsl-asoc-card sound: ASoC: no source widget found for
ASRC-Playback
[    1.796065] fsl-asoc-card sound: ASoC: Failed to add route
ASRC-Playback -> direct -> CPU-Playback
[    1.807571] fsl-asoc-card sound: ASoC: no sink widget found for ASRC-Capture
[    1.817112] fsl-asoc-card sound: ASoC: Failed to add route
CPU-Capture -> direct -> ASRC-Capture
Nicolin Chen Sept. 22, 2014, 3:41 a.m. UTC | #13
On Mon, Sep 22, 2014 at 12:26:21AM -0300, Fabio Estevam wrote:
> On Thu, Sep 18, 2014 at 3:07 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> 
> > I plan to replace imx-sgtl5000 and imx-wm8962 with it as long as
> > people are happy about the new driver. Would you like to help me
> > testing it on sgtl5000 side when you have time? Because I only
> > have a board with WM8962.
> 
> Just tested fsl-asoc-card with sgtl5000 and it worked fine on my imx53-qsb.

Thank you! Then I think we can enable it from the next release version.
 
> These messages are a bit annoying though:
> 
> [    1.786201] fsl-asoc-card sound: ASoC: no source widget found for
> ASRC-Playback
> [    1.796065] fsl-asoc-card sound: ASoC: Failed to add route
> ASRC-Playback -> direct -> CPU-Playback
> [    1.807571] fsl-asoc-card sound: ASoC: no sink widget found for ASRC-Capture
> [    1.817112] fsl-asoc-card sound: ASoC: Failed to add route
> CPU-Capture -> direct -> ASRC-Capture


I'll try to find a way to circumvent the ASRC route for non-ASRC links.

Thanks,
Nicolin
Jean-Michel Hautbois Oct. 9, 2014, 2:02 p.m. UTC | #14
Hi

I use fsl-asoc-card now.

This is working when compiling as a module, and when doing a modprobe
manually, but when I compile it as built-in, with these :
CONFIG_SND_SOC_IMX_SGTL5000 is not set
CONFIG_SND_SOC_FSL_ASOC_CARD=y
CONFIG_SND_SOC_SGTL5000=y

I get the following error :
[   13.561618] fsl-asoc-card sound: ASoC: CODEC DAI sgtl5000 not registered
[   13.568503] fsl-asoc-card sound: snd_soc_register_card failed (-517)
[   13.574992] platform sound: Driver fsl-asoc-card requests probe deferral

It loops on it during boot, and never registers...

If I have :
CONFIG_SND_SOC_IMX_SGTL5000 is not set
CONFIG_SND_SOC_FSL_ASOC_CARD=m
CONFIG_SND_SOC_SGTL5000=m

And after login :
$> lsmod
Module                  Size  Used by
snd_soc_sgtl5000       14074  0
$> modprobe snd-soc-fsl-asoc-card
[   65.300995] fsl-asoc-card sound: ASoC: CODEC DAI sgtl5000 not registered
[   65.307963] fsl-asoc-card sound: snd_soc_register_card failed (-517)
[   65.314423] platform sound: Driver fsl-asoc-card requests probe deferral
$> rmmod snd-soc-sgtl5000
$> modprobe snd-soc-sgtl5000
[  138.936753] sgtl5000 1-000a: sgtl5000 revision 0x11
[  138.987974] 1-000a: 1200 mV normal
[  139.000923] sgtl5000 1-000a: Using internal LDO instead of VDDD
[  139.032169] fsl-asoc-card sound: sgtl5000 <-> 2028000.ssi mapping ok
[  139.038624] fsl-asoc-card sound: ASoC: no source widget found for
ASRC-Playback
[  139.045984] fsl-asoc-card sound: ASoC: Failed to add route
ASRC-Playback -> direct -> CPU-Playback
[  139.055187] fsl-asoc-card sound: ASoC: no sink widget found for ASRC-Capture
[  139.062253] fsl-asoc-card sound: ASoC: Failed to add route
CPU-Capture -> direct -> ASRC-Capture

The routes are not ok, but I think I read something about it another
thread, but it works.
It seems that if snd-soc-sgtl5000 is already registered then it does
not work. It is probably due to my DT ?

Here is an extract of the DT :
/ {
    sound {
        compatible = "fsl,imx6q-vbx3-sgtl5000",
                 "fsl,imx-audio-sgtl5000";
        model = "imx6q-vbx3-sgtl5000";
        ssi-controller = <&ssi1>;
        audio-codec = <&codec>;
        audio-routing =
            "MIC_IN", "Mic Jack",
            "Mic Jack", "Mic Bias",
            "A1N1L", "Line In Jack",
            "A1N1R", "Line In Jack";
        mux-int-port = <1>;
        mux-ext-port = <5>;
    };
};

&i2c2 {
    clock-frequency = <100000>;
    pinctrl-names = "default";
    pinctrl-0 = <&pinctrl_i2c2>;
    status = "okay";

    codec: sgtl5000@0a {
        compatible = "fsl,sgtl5000";
        reg = <0x0a>;
        clocks = <&clks 201>;
        VDDA-supply = <&vgen6_reg>;
        VDDIO-supply = <&vgen6_reg>;
        sysclk = <24000000>;
    };
};

Thanks,
JM
Fabio Estevam Oct. 9, 2014, 2:18 p.m. UTC | #15
On Thu, Oct 9, 2014 at 11:02 AM, Jean-Michel Hautbois
<jean-michel.hautbois@vodalys.com> wrote:

> Here is an extract of the DT :
> / {
>     sound {
>         compatible = "fsl,imx6q-vbx3-sgtl5000",
>                  "fsl,imx-audio-sgtl5000";

I am confused here: you say you use the simple-card, but your DTS
still shows imx-audio-sgtl5000.

arch/arm/boot/dts/vf610-twr.dts shows an example of the usage of
simple card with sgtl5000.

> &i2c2 {
>     clock-frequency = <100000>;
>     pinctrl-names = "default";
>     pinctrl-0 = <&pinctrl_i2c2>;
>     status = "okay";
>
>     codec: sgtl5000@0a {
>         compatible = "fsl,sgtl5000";
>         reg = <0x0a>;
>         clocks = <&clks 201>;
>         VDDA-supply = <&vgen6_reg>;
>         VDDIO-supply = <&vgen6_reg>;
>         sysclk = <24000000>;

This 'sysclk' property is unneeded.
Jean-Michel Hautbois Oct. 9, 2014, 2:26 p.m. UTC | #16
2014-10-09 16:18 GMT+02:00 Fabio Estevam <festevam@gmail.com>:
> On Thu, Oct 9, 2014 at 11:02 AM, Jean-Michel Hautbois
> <jean-michel.hautbois@vodalys.com> wrote:
>
>> Here is an extract of the DT :
>> / {
>>     sound {
>>         compatible = "fsl,imx6q-vbx3-sgtl5000",
>>                  "fsl,imx-audio-sgtl5000";
>
> I am confused here: you say you use the simple-card, but your DTS
> still shows imx-audio-sgtl5000.

I mentioned fsl-asoc-card, not simple card ?

> arch/arm/boot/dts/vf610-twr.dts shows an example of the usage of
> simple card with sgtl5000.
>
>> &i2c2 {
>>     clock-frequency = <100000>;
>>     pinctrl-names = "default";
>>     pinctrl-0 = <&pinctrl_i2c2>;
>>     status = "okay";
>>
>>     codec: sgtl5000@0a {
>>         compatible = "fsl,sgtl5000";
>>         reg = <0x0a>;
>>         clocks = <&clks 201>;
>>         VDDA-supply = <&vgen6_reg>;
>>         VDDIO-supply = <&vgen6_reg>;
>>         sysclk = <24000000>;
>
> This 'sysclk' property is unneeded.

Right, I removed it thanks.
JM
Fabio Estevam Oct. 9, 2014, 3:06 p.m. UTC | #17
On Thu, Oct 9, 2014 at 11:26 AM, Jean-Michel Hautbois
<jean-michel.hautbois@vodalys.com> wrote:
> 2014-10-09 16:18 GMT+02:00 Fabio Estevam <festevam@gmail.com>:
>> On Thu, Oct 9, 2014 at 11:02 AM, Jean-Michel Hautbois
>> <jean-michel.hautbois@vodalys.com> wrote:
>>
>>> Here is an extract of the DT :
>>> / {
>>>     sound {
>>>         compatible = "fsl,imx6q-vbx3-sgtl5000",
>>>                  "fsl,imx-audio-sgtl5000";
>>
>> I am confused here: you say you use the simple-card, but your DTS
>> still shows imx-audio-sgtl5000.
>
> I mentioned fsl-asoc-card, not simple card ?

Yes, right, but for using fsl-asoc-card you should reference
'audio-cpu' instead of 'ssi-controller'.

That's what Documentation/devicetree/bindings/sound/fsl-asoc-card.txt mentions.
Jean-Michel Hautbois Oct. 9, 2014, 3:14 p.m. UTC | #18
2014-10-09 17:06 GMT+02:00 Fabio Estevam <festevam@gmail.com>:
> On Thu, Oct 9, 2014 at 11:26 AM, Jean-Michel Hautbois
> <jean-michel.hautbois@vodalys.com> wrote:
>> 2014-10-09 16:18 GMT+02:00 Fabio Estevam <festevam@gmail.com>:
>>> On Thu, Oct 9, 2014 at 11:02 AM, Jean-Michel Hautbois
>>> <jean-michel.hautbois@vodalys.com> wrote:
>>>
>>>> Here is an extract of the DT :
>>>> / {
>>>>     sound {
>>>>         compatible = "fsl,imx6q-vbx3-sgtl5000",
>>>>                  "fsl,imx-audio-sgtl5000";
>>>
>>> I am confused here: you say you use the simple-card, but your DTS
>>> still shows imx-audio-sgtl5000.
>>
>> I mentioned fsl-asoc-card, not simple card ?
>
> Yes, right, but for using fsl-asoc-card you should reference
> 'audio-cpu' instead of 'ssi-controller'.
>
> That's what Documentation/devicetree/bindings/sound/fsl-asoc-card.txt mentions.

Ouch, nice catch :).

Here is what I understand, in my cas, sgtl5000 has its clock routed
through a FPGA.
If the FPGA has not yet be probed, then, the module fails in loading :
[    4.985762] sgtl5000: probe of 1-000a failed with error -5

This is why after removing it and probing it again manually, it works.

This module should be able to defer probing too, is it going to be
done by someone or do I try to send some patch for this ?

BTW, I don't have sound on the microphone input, and I think there is
no way to select micbias voltage level neither from the source code
nor the DT ?
It could be interetsing to have something like sgtl5000-micbias-vg
which would give control on the voltage bias parameter ?
And maybe also something for bias resistor too ?

I can try to patch something.
Thanks,
JM
Fabio Estevam Oct. 9, 2014, 3:19 p.m. UTC | #19
On Thu, Oct 9, 2014 at 12:14 PM, Jean-Michel Hautbois
<jean-michel.hautbois@vodalys.com> wrote:

> Ouch, nice catch :).
>
> Here is what I understand, in my cas, sgtl5000 has its clock routed
> through a FPGA.
> If the FPGA has not yet be probed, then, the module fails in loading :
> [    4.985762] sgtl5000: probe of 1-000a failed with error -5
>
> This is why after removing it and probing it again manually, it works.
>
> This module should be able to defer probing too, is it going to be
> done by someone or do I try to send some patch for this ?

We do defer probing already:

    sgtl5000->mclk = devm_clk_get(&client->dev, NULL);
    if (IS_ERR(sgtl5000->mclk)) {
        ret = PTR_ERR(sgtl5000->mclk);
        dev_err(&client->dev, "Failed to get mclock: %d\n", ret);
        /* Defer the probe to see if the clk will be provided later */
        if (ret == -ENOENT)
            return -EPROBE_DEFER;
        return ret;
    }

>
> BTW, I don't have sound on the microphone input, and I think there is
> no way to select micbias voltage level neither from the source code
> nor the DT ?
> It could be interetsing to have something like sgtl5000-micbias-vg
> which would give control on the voltage bias parameter ?
> And maybe also something for bias resistor too ?
>
> I can try to patch something.

That would be great, thanks.
Jean-Michel Hautbois Oct. 10, 2014, 3:49 p.m. UTC | #20
2014-10-09 17:19 GMT+02:00 Fabio Estevam <festevam@gmail.com>:
> On Thu, Oct 9, 2014 at 12:14 PM, Jean-Michel Hautbois
> <jean-michel.hautbois@vodalys.com> wrote:
>
>> Ouch, nice catch :).
>>
>> Here is what I understand, in my cas, sgtl5000 has its clock routed
>> through a FPGA.
>> If the FPGA has not yet be probed, then, the module fails in loading :
>> [    4.985762] sgtl5000: probe of 1-000a failed with error -5
>>
>> This is why after removing it and probing it again manually, it works.
>>
>> This module should be able to defer probing too, is it going to be
>> done by someone or do I try to send some patch for this ?
>
> We do defer probing already:
>
>     sgtl5000->mclk = devm_clk_get(&client->dev, NULL);
>     if (IS_ERR(sgtl5000->mclk)) {
>         ret = PTR_ERR(sgtl5000->mclk);
>         dev_err(&client->dev, "Failed to get mclock: %d\n", ret);
>         /* Defer the probe to see if the clk will be provided later */
>         if (ret == -ENOENT)
>             return -EPROBE_DEFER;
>         return ret;
>     }

Yes, sorry about that, missed it.

>>
>> BTW, I don't have sound on the microphone input, and I think there is
>> no way to select micbias voltage level neither from the source code
>> nor the DT ?
>> It could be interetsing to have something like sgtl5000-micbias-vg
>> which would give control on the voltage bias parameter ?
>> And maybe also something for bias resistor too ?
>>
>> I can try to patch something.
>
> That would be great, thanks.

Do I need to keep actual defaults if the resistor or voltage are not
specified in DT ?
Or should I set it to off (which is probably more logical) ?
And one or two patches (one resistor and one voltage patch, or both in
the same) ?

JM
Fabio Estevam Oct. 10, 2014, 3:54 p.m. UTC | #21
On Fri, Oct 10, 2014 at 12:49 PM, Jean-Michel Hautbois
<jean-michel.hautbois@vodalys.com> wrote:

> Do I need to keep actual defaults if the resistor or voltage are not
> specified in DT ?

Better to keep the behaviour unchanged, so that old dtb's can work
exactly in the same way.

> Or should I set it to off (which is probably more logical) ?
> And one or two patches (one resistor and one voltage patch, or both in
> the same) ?

Splitting in 2 patches is easier for reviewing.
diff mbox

Patch

--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -111,8 +111,8 @@ 
                        "IN3R", "MICBIAS",
                        "DMIC", "MICBIAS",
                        "DMICDAT", "DMIC";
-               mux-int-port = <2>;
-               mux-ext-port = <3>;
+               mux-int-port = <3>;
+               mux-ext-port = <2>;
        };

        backlight {
diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c
index 3a3d17c..09ea6a5 100644
--- a/sound/soc/fsl/imx-wm8962.c
+++ b/sound/soc/fsl/imx-wm8962.c
@@ -247,7 +247,7 @@  static int imx_wm8962_probe(struct platform_device *pdev)
        data->dai.platform_of_node = ssi_np;
        data->dai.ops = &imx_hifi_ops;
        data->dai.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
-                           SND_SOC_DAIFMT_CBM_CFM;
+                           SND_SOC_DAIFMT_CBS_CFS;

        data->card.dev = &pdev->dev;
        ret = snd_soc_of_parse_card_name(&data->card, "model");