Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1683309 > unrolled thread

[PATCH 0/3] Motorola Droid 4 Audio Support

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] Motorola Droid 4 Audio Support Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-07-07 18:50 +0200
    [PATCH 3/3] ARM: dts: omap4-droid4: add soundcard Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-07-07 18:50 +0200
      Re: [PATCH 3/3] ARM: dts: omap4-droid4: add soundcard Mark Brown <broonie@kernel.org> - 2017-07-10 14:30 +0200
    Re: [PATCH 0/3] Motorola Droid 4 Audio Support Tony Lindgren <tony@atomide.com> - 2017-07-08 07:30 +0200
      Re: [PATCH 0/3] Motorola Droid 4 Audio Support Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-07-08 13:40 +0200
        Re: [PATCH 0/3] Motorola Droid 4 Audio Support Tony Lindgren <tony@atomide.com> - 2017-07-09 07:40 +0200
    Re: [PATCH 1/3] ASoC: codec: cpcap: new codec Tony Lindgren <tony@atomide.com> - 2017-07-09 07:40 +0200
    Re: [PATCH 1/3] ASoC: codec: cpcap: new codec Rob Herring <robh@kernel.org> - 2017-07-10 17:30 +0200
    Re: [PATCH 1/3] ASoC: codec: cpcap: new codec Mark Brown <broonie@kernel.org> - 2017-07-10 20:00 +0200
      Re: [PATCH 1/3] ASoC: codec: cpcap: new codec Tony Lindgren <tony@atomide.com> - 2017-07-11 07:20 +0200
        Re: [PATCH 1/3] ASoC: codec: cpcap: new codec Mark Brown <broonie@kernel.org> - 2017-07-11 12:50 +0200
          Re: [PATCH 1/3] ASoC: codec: cpcap: new codec Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-07-11 14:20 +0200
            Re: [PATCH 1/3] ASoC: codec: cpcap: new codec Mark Brown <broonie@kernel.org> - 2017-07-11 15:40 +0200

#1683309 — [PATCH 0/3] Motorola Droid 4 Audio Support

Hi,

I got working sound on Droid 4 with mainline \o/. The codec is
currently missing support for detecting if something has been
plugged into the 3.5mm connector, since that seems to require
some closed source firmware and needs further investigation. I
think this can be added later.

Tested:
 - Playing via [HiFi] using Speaker, Earpiece, Headphone
 - Playing via [Voice] using Speaker, Earpiece, Headphone
 - Recording via [Voice] using Mic
 - Volume Controls
 - Checking that DAPM graph look correct

-- Sebastian

Sebastian Reichel (3):
  ASoC: codec: cpcap: new codec
  ARM: dts: motorola-cpcap-mapphone: add audio-codec
  ARM: dts: omap4-droid4: add soundcard

 .../bindings/sound/motorola,cpcap-audio-codec.txt  |   19 +
 arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi     |    6 +
 arch/arm/boot/dts/omap4-droid4-xt894.dts           |   76 ++
 sound/soc/codecs/Kconfig                           |    3 +
 sound/soc/codecs/Makefile                          |    2 +
 sound/soc/codecs/cpcap.c                           | 1422 ++++++++++++++++++++
 6 files changed, 1528 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/motorola,cpcap-audio-codec.txt
 create mode 100644 sound/soc/codecs/cpcap.c

-- 
2.13.2

[toc] | [next] | [standalone]

#1683311 — [PATCH 3/3] ARM: dts: omap4-droid4: add soundcard

Add sound support to Motorola Droid 4 using simple-soundcard
and CPCAP's audio codec. This does not yet correctly represent
the whole audio routing, since McBSP3 is also connected to
Bluetooth and MDM6600 modem (and probably also 4G modem).
These extra DAI links are not yet supported and have not been
tested.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 arch/arm/boot/dts/omap4-droid4-xt894.dts | 76 ++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
index 10ca1c174995..642b7ed1713b 100644
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -116,6 +116,50 @@
 
 		};
 	};
+
+	soundcard {
+		compatible = "simple-audio-card";
+		simple-audio-card,name = "Droid 4 Audio";
+
+		simple-audio-card,widgets =
+			"Speaker", "Earpiece",
+			"Speaker", "Loudspeaker",
+			"Headphone", "Headphone Jack",
+			"Microphone", "Internal Mic";
+
+		simple-audio-card,routing =
+			"Earpiece", "EP",
+			"Loudspeaker", "SPKR",
+			"Headphone Jack", "HSL",
+			"Headphone Jack", "HSR",
+			"MICR", "Internal Mic";
+
+		simple-audio-card,dai-link@0 {
+			format = "i2s";
+			frame-master = <&codec0>;
+			bitclock-master = <&codec0>;
+			cpu {
+				sound-dai = <&mcbsp2>;
+			};
+			codec0: codec {
+				sound-dai = <&cpcap_audio 0>;
+			};
+		};
+
+		simple-audio-card,dai-link@1 {
+			format = "dsp_a";
+			frame-master = <&codec1>;
+			bitclock-master = <&codec1>;
+			bitclock-inversion;
+
+			cpu {
+				sound-dai = <&mcbsp3>;
+			};
+			codec1: codec {
+				sound-dai = <&cpcap_audio 1>;
+			};
+		};
+	};
 };
 
 &dss {
@@ -479,6 +523,24 @@
 		OMAP4_IOPAD(0x112, PIN_OUTPUT_PULLUP | MUX_MODE5)	/* uart4_rts */
 		>;
 	};
+
+	mcbsp2_pins: pinmux_mcbsp2_pins {
+		pinctrl-single,pins = <
+		OMAP4_IOPAD(0x0f6, PIN_INPUT | MUX_MODE0)	/* abe_mcbsp2_clkx */
+		OMAP4_IOPAD(0x0f8, PIN_INPUT | MUX_MODE0)	/* abe_mcbsp2_dr */
+		OMAP4_IOPAD(0x0fa, PIN_OUTPUT | MUX_MODE0)	/* abe_mcbsp2_dx */
+		OMAP4_IOPAD(0x0fc, PIN_INPUT | MUX_MODE0)	/* abe_mcbsp2_fsx */
+		>;
+	};
+
+	mcbsp3_pins: pinmux_mcbsp3_pins {
+		pinctrl-single,pins = <
+		OMAP4_IOPAD(0x106, PIN_INPUT | MUX_MODE1)	/* abe_mcbsp3_dr */
+		OMAP4_IOPAD(0x108, PIN_OUTPUT | MUX_MODE1)	/* abe_mcbsp3_dx */
+		OMAP4_IOPAD(0x10a, PIN_INPUT | MUX_MODE1)	/* abe_mcbsp3_clkx */
+		OMAP4_IOPAD(0x10c, PIN_INPUT | MUX_MODE1)	/* abe_mcbsp3_fsx */
+		>;
+	};
 };
 
 &omap4_pmx_wkup {
@@ -552,3 +614,17 @@
 				  "0", "0", "1";
 	};
 };
+
+&mcbsp2 {
+	#sound-dai-cells = <0>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&mcbsp2_pins>;
+	status = "okay";
+};
+
+&mcbsp3 {
+	#sound-dai-cells = <0>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&mcbsp3_pins>;
+	status = "okay";
+};
-- 
2.13.2

[toc] | [prev] | [next] | [standalone]

#1684184 — Re: [PATCH 3/3] ARM: dts: omap4-droid4: add soundcard

[Multipart message — attachments visible in raw view] — view raw

On Fri, Jul 07, 2017 at 06:42:29PM +0200, Sebastian Reichel wrote:

> +	soundcard {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "Droid 4 Audio";

New systems should be using the of-graph card which is more flexible and
extensible than simple-card.

[toc] | [prev] | [next] | [standalone]

#1683536

* Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170707 09:43]:
> Hi,
> 
> I got working sound on Droid 4 with mainline \o/. The codec is
> currently missing support for detecting if something has been
> plugged into the 3.5mm connector, since that seems to require
> some closed source firmware and needs further investigation. I
> think this can be added later.

Hey that's great! I'll give it a try this weekend.. Does that mean
that 3G voice calls work too now or is something more needed there?

For the CPCAP PMIC macro interrupts I think it's best to set up
a separate driver as it seems separate from the core CPCAP
functionality. So I think we can just move the unused "cpcap-m2"
IRQ banks out of motorola-cpcap.c and put them into a separate child
driver that loads it's firmware on init and provides interrupts for
the 3.5mm connector.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1683588

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Fri, Jul 07, 2017 at 10:27:53PM -0700, Tony Lindgren wrote:
> * Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170707 09:43]:
> > I got working sound on Droid 4 with mainline \o/. The codec is
> > currently missing support for detecting if something has been
> > plugged into the 3.5mm connector, since that seems to require
> > some closed source firmware and needs further investigation. I
> > think this can be added later.
> 
> Hey that's great! I'll give it a try this weekend.. Does that mean
> that 3G voice calls work too now or is something more needed there?

I added some text about that in the 3rd patch. Basically the codec driver
is ready (maybe a few quirks will be needed, though), but the soundcard
driver is not. It currently assumes CPU <-> Voice Codec, but
actually its a network of CPU, Voice Codec, Modem(s?) and Bluetooth.

> For the CPCAP PMIC macro interrupts I think it's best to set up
> a separate driver as it seems separate from the core CPCAP
> functionality. So I think we can just move the unused "cpcap-m2"
> IRQ banks out of motorola-cpcap.c and put them into a separate child
> driver that loads it's firmware on init and provides interrupts for
> the 3.5mm connector.

I also think the macro block fw handling should not be handled in
the driver for the audio codec. IIRC it is also used to provide a
blink feature for the status LEDs. OTOH some of the registers of the
audio-codec are also used for 3.5mm detection. I also did not yet look
into the details of this one.

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1683689

* Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170708 04:34]:
> Hi,
> 
> On Fri, Jul 07, 2017 at 10:27:53PM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170707 09:43]:
> > > I got working sound on Droid 4 with mainline \o/. The codec is
> > > currently missing support for detecting if something has been
> > > plugged into the 3.5mm connector, since that seems to require
> > > some closed source firmware and needs further investigation. I
> > > think this can be added later.
> > 
> > Hey that's great! I'll give it a try this weekend.. Does that mean
> > that 3G voice calls work too now or is something more needed there?
> 
> I added some text about that in the 3rd patch. Basically the codec driver
> is ready (maybe a few quirks will be needed, though), but the soundcard
> driver is not. It currently assumes CPU <-> Voice Codec, but
> actually its a network of CPU, Voice Codec, Modem(s?) and Bluetooth.

OK

> > For the CPCAP PMIC macro interrupts I think it's best to set up
> > a separate driver as it seems separate from the core CPCAP
> > functionality. So I think we can just move the unused "cpcap-m2"
> > IRQ banks out of motorola-cpcap.c and put them into a separate child
> > driver that loads it's firmware on init and provides interrupts for
> > the 3.5mm connector.
> 
> I also think the macro block fw handling should not be handled in
> the driver for the audio codec. IIRC it is also used to provide a
> blink feature for the status LEDs. OTOH some of the registers of the
> audio-codec are also used for 3.5mm detection. I also did not yet look
> into the details of this one.

Also the battery driver has three mystery interrupts coming from the
macro. At least the cycle count is missing so maybe that too comes
from the macro.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1683688 — Re: [PATCH 1/3] ASoC: codec: cpcap: new codec

* Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170707 09:43]:
> Motorola CPCAP is a PMIC with audio functionality, that can be
> found on Motorola Droid 4 and probably a few other phones from
> Motorola's Droid series.
> 
> The driver has been written from scratch using Motorola's Android
> driver, register dumps from running Android and datasheet for NXP
> MC13783UG (which is similar to Motorola CPCAP, but not the same).
> 
> The chip provides two audio interfaces, that can be muxed to two
> different audio codecs. One provides support for stereo output
> (named StDAC or HiFi), while the other only provides mono output
> (named Voice). Only the Voice codec provides a Capture interface.

I needed the patch below for modular .config. Other than that,
plays music fine for me, so feel free to add:

Acked-by: Tony Lindgren <tony@atomide.com>

8< -----------------------
From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Sat, 8 Jul 2017 21:43:46 -0700
Subject: [PATCH] ALSA: pcm: Export soc_dpcm_runtime_update

Some drivers may need to use this from loadable modules. Otherwise we
will get:

ERROR: "soc_dpcm_runtime_update" [sound/soc/codecs/snd-soc-cpcap.ko]
undefined!

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 sound/soc/soc-pcm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2539,6 +2539,8 @@ int soc_dpcm_runtime_update(struct snd_soc_card *card)
 	mutex_unlock(&card->mutex);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(soc_dpcm_runtime_update);
+
 int soc_dpcm_be_digital_mute(struct snd_soc_pcm_runtime *fe, int mute)
 {
 	struct snd_soc_dpcm *dpcm;
-- 
2.13.2

[toc] | [prev] | [next] | [standalone]

#1684355 — Re: [PATCH 1/3] ASoC: codec: cpcap: new codec

On Fri, Jul 07, 2017 at 06:42:27PM +0200, Sebastian Reichel wrote:
> Motorola CPCAP is a PMIC with audio functionality, that can be
> found on Motorola Droid 4 and probably a few other phones from
> Motorola's Droid series.
> 
> The driver has been written from scratch using Motorola's Android
> driver, register dumps from running Android and datasheet for NXP
> MC13783UG (which is similar to Motorola CPCAP, but not the same).
> 
> The chip provides two audio interfaces, that can be muxed to two
> different audio codecs. One provides support for stereo output
> (named StDAC or HiFi), while the other only provides mono output
> (named Voice). Only the Voice codec provides a Capture interface.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  .../bindings/sound/motorola,cpcap-audio-codec.txt  |   19 +

Acked-by: Rob Herring <robh@kernel.org>

>  sound/soc/codecs/Kconfig                           |    3 +
>  sound/soc/codecs/Makefile                          |    2 +
>  sound/soc/codecs/cpcap.c                           | 1422 ++++++++++++++++++++
>  4 files changed, 1446 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/motorola,cpcap-audio-codec.txt
>  create mode 100644 sound/soc/codecs/cpcap.c

[toc] | [prev] | [next] | [standalone]

#1684552 — Re: [PATCH 1/3] ASoC: codec: cpcap: new codec

[Multipart message — attachments visible in raw view] — view raw

On Fri, Jul 07, 2017 at 06:42:27PM +0200, Sebastian Reichel wrote:

>  snd-soc-cq93vc-objs := cq93vc.o
> +snd-soc-cpcap-objs := cpcap.o

Please keep Kconfig and Makefile lexically sorted.

> +static int cpcap_audio_write(struct cpcap_audio *cpcap,
> +			     u16 reg, u16 mask, u16 val)
> +{
> +	struct snd_soc_codec *codec = cpcap->codec;
> +	struct device *dev = codec->dev;
> +	int err;
> +
> +	err = regmap_update_bits(cpcap->regmap, reg, mask, val);
> +	if (err)
> +		dev_err(dev, "write failed: reg=%04x mask=%04x val=%04x err=%d",
> +			reg, mask, val, err);
> +
> +	return err;
> +}

If we're going to have wrappers for the regmap functions it'd be good if
they were named in a similar way to the functions that they wrap, just
for clarity.  They're also not used consistently (and TBH I'm not sure
what they buy but if you want them that's fine).

> +	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		err += regmap_write(cpcap->regmap, CPCAP_REG_TEST,
> +				    STM_STDAC_EN_TEST_PRE);
> +		err += regmap_write(cpcap->regmap, CPCAP_REG_ST_TEST1,
> +				    STM_STDAC_EN_ST_TEST1_PRE);
> +		return err;

This'll return a nonsense error code if both error out, better to do
something like

	if (!err)
		err = regmap_write(...

> +static const char * const cpcap_onoff_texts[] = {
> +	"Off", "On"
> +};
> +static const SOC_ENUM_SINGLE_DECL(cpcap_ext_cap_l_enum,
> +	CPCAP_REG_TXI, CPCAP_BIT_RX_L_ENCODE, cpcap_onoff_texts);
> +static const SOC_ENUM_SINGLE_DECL(cpcap_ext_cap_r_enum,
> +	CPCAP_REG_TXI, CPCAP_BIT_RX_R_ENCODE, cpcap_onoff_texts);
> +static const struct snd_kcontrol_new cpcap_extr_cap_control =
> +	SOC_DAPM_ENUM("Ext Right Capture", cpcap_ext_cap_r_enum);
> +static const struct snd_kcontrol_new cpcap_extl_cap_control =
> +	SOC_DAPM_ENUM("Ext Left Capture", cpcap_ext_cap_l_enum);

Why are these enums and not simple Switch controls?  They appear to be
muxes with only one arm connected.

> +static int cpcap_hifi_set_mute(struct snd_soc_dai *dai, int mute)
> +{
> +	struct snd_soc_codec *codec = dai->codec;
> +	struct cpcap_audio *cpcap = snd_soc_codec_get_drvdata(codec);
> +	static const u16 reg = CPCAP_REG_RXSDOA;
> +	static const u16 mask = BIT(CPCAP_BIT_ST_DAC_SW);
> +	u16 val = mute ? 0 : BIT(CPCAP_BIT_ST_DAC_SW);

Please write a normal if statement, this was a bit confusing at first
glance.

> +#ifdef CONFIG_OF
> +static const struct of_device_id cpcap_audio_of_match[] = {
> +	{ .compatible = "motorola,cpcap-audio-codec", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, cpcap_audio_of_match);
> +#endif

If this is part of a MFD shouldn't the parent device register it without
it needing to be in the DT?

[toc] | [prev] | [next] | [standalone]

#1684780 — Re: [PATCH 1/3] ASoC: codec: cpcap: new codec

* Mark Brown <broonie@kernel.org> [170710 10:52]:
> On Fri, Jul 07, 2017 at 06:42:27PM +0200, Sebastian Reichel wrote:
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id cpcap_audio_of_match[] = {
> > +	{ .compatible = "motorola,cpcap-audio-codec", },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, cpcap_audio_of_match);
> > +#endif
> 
> If this is part of a MFD shouldn't the parent device register it without
> it needing to be in the DT?

Having the MFD core part just do devm_of_platform_populate() leaves out
dependencies between the MFD core and it's child devices.

There are also a lot of board specific configuration to be done for many
child devices such as ADC wiring, GPIO pins used, and macro firmware
usage.

Not sure what all board specific stuff needs to be configured for this
driver yet, but it seems at least the macro interrupt wiring needs to
be configured. I would not be surprised to find GPIOs or ADC being used
for some connector detection too.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1684958 — Re: [PATCH 1/3] ASoC: codec: cpcap: new codec

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jul 10, 2017 at 10:15:41PM -0700, Tony Lindgren wrote:
> * Mark Brown <broonie@kernel.org> [170710 10:52]:

> > If this is part of a MFD shouldn't the parent device register it without
> > it needing to be in the DT?

> Having the MFD core part just do devm_of_platform_populate() leaves out
> dependencies between the MFD core and it's child devices.

> There are also a lot of board specific configuration to be done for many
> child devices such as ADC wiring, GPIO pins used, and macro firmware
> usage.

> Not sure what all board specific stuff needs to be configured for this
> driver yet, but it seems at least the macro interrupt wiring needs to
> be configured. I would not be surprised to find GPIOs or ADC being used
> for some connector detection too.

We can use subnodes without compatible strings, that's not a problem,
but having to have compatible strings means we're encoding the way Linux
splits device drivers up into the DT which might not work for other OSs
or even future versions of Linux.  For example with CODEC drivers it's
common to have a good chunk of clock control in there which we might at
some point want to move over to the clock subsystem.

[toc] | [prev] | [next] | [standalone]

#1685002 — Re: [PATCH 1/3] ASoC: codec: cpcap: new codec

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Tue, Jul 11, 2017 at 11:49:11AM +0100, Mark Brown wrote:
> On Mon, Jul 10, 2017 at 10:15:41PM -0700, Tony Lindgren wrote:
> > * Mark Brown <broonie@kernel.org> [170710 10:52]:
> 
> > > If this is part of a MFD shouldn't the parent device register it without
> > > it needing to be in the DT?
> 
> > Having the MFD core part just do devm_of_platform_populate() leaves out
> > dependencies between the MFD core and it's child devices.
> 
> > There are also a lot of board specific configuration to be done for many
> > child devices such as ADC wiring, GPIO pins used, and macro firmware
> > usage.
> 
> > Not sure what all board specific stuff needs to be configured for this
> > driver yet, but it seems at least the macro interrupt wiring needs to
> > be configured. I would not be surprised to find GPIOs or ADC being used
> > for some connector detection too.
> 
> We can use subnodes without compatible strings, that's not a problem,
> but having to have compatible strings means we're encoding the way Linux
> splits device drivers up into the DT which might not work for other OSs
> or even future versions of Linux.  For example with CODEC drivers it's
> common to have a good chunk of clock control in there which we might at
> some point want to move over to the clock subsystem.

How is having a subnode without a compatible property different?
Sure we will bind the driver manually and make our code (a little
bit) more complex, but we still encoded the way Linux splits the
device into DT. My understanding is, that adding a node without a
compatible value is almost always not ok.

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1685034 — Re: [PATCH 1/3] ASoC: codec: cpcap: new codec

[Multipart message — attachments visible in raw view] — view raw

On Tue, Jul 11, 2017 at 02:13:31PM +0200, Sebastian Reichel wrote:

> How is having a subnode without a compatible property different?

You don't *need* to have the subnode, I was only mentioning that if for
some reason it was super useful for organizing the properties.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web