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

Groups > linux.kernel > #1261020 > unrolled thread

[PATCH v2 0/2] Add audio support for Broadcom Cygnus SoC

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/2] Add audio support for Broadcom Cygnus SoC Simran Rai <ssimran@broadcom.com> - 2015-11-02 23:20 +0100
    [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom Cygnus audio Simran Rai <ssimran@broadcom.com> - 2015-11-02 23:20 +0100
      Re: [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom  Cygnus audio Mark Brown <broonie@kernel.org> - 2015-11-04 15:10 +0100
        Re: [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom  Cygnus audio Simran Rai <ssimran@broadcom.com> - 2015-11-05 21:10 +0100
          Re: [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom  Cygnus audio Mark Brown <broonie@kernel.org> - 2015-11-06 11:40 +0100
            Re: [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom  Cygnus audio Simran Rai <ssimran@broadcom.com> - 2015-11-06 23:20 +0100
    Re: [PATCH v2 0/2] Add audio support for Broadcom Cygnus SoC Mark Brown <broonie@kernel.org> - 2015-11-04 15:10 +0100
    Re: [PATCH v2 2/2] sound: soc: Add Cygnus audio driver Mark Brown <broonie@kernel.org> - 2015-11-04 15:30 +0100

#1261020 — [PATCH v2 0/2] Add audio support for Broadcom Cygnus SoC

Hi,

This patchset contains audio support for Broadcom's Cygnus SoC. It
contains DT bindings and core audio driver. The audio driver
supports both capture and playback of Audio PCM samples over I2S/TDM
interface and provides playback support over SPDIF interface.

This patchset is derived from a previously submitted patchset:
http://lkml.iu.edu/hypermail/linux/kernel/1503.3/05434.html

This patchset has been tested on Cygnus wireless audio bcm958305K board.
It is based on v4.3-rc5  and is available from github:

repo: https://github.com/Broadcom/cygnus-linux/tree/cygnus-sound-v2

Changes from v1:
  - Address code review comments. Fixed print format of type size_t and
    pointer.

Simran Rai (2):
  dt-bindings: sound: Add DT bindings for Broadcom Cygnus audio
  VOIP-5227 Add Broadcom Cygnus audio driver.

 .../bindings/sound/brcm,cygnus-audio.txt           |   63 +
 sound/soc/bcm/Kconfig                              |   18 +
 sound/soc/bcm/Makefile                             |    5 +
 sound/soc/bcm/cygnus-pcm.c                         |  903 ++++++++++++
 sound/soc/bcm/cygnus-ssp.c                         | 1532 ++++++++++++++++++++
 sound/soc/bcm/cygnus-ssp.h                         |  129 ++
 6 files changed, 2650 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt
 create mode 100644 sound/soc/bcm/cygnus-pcm.c
 create mode 100644 sound/soc/bcm/cygnus-ssp.c
 create mode 100644 sound/soc/bcm/cygnus-ssp.h

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1261023 — [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom Cygnus audio

Add bindings for audio driver in Broadcom Cygnus.

Signed-off-by: Lori Hikichi <lhikichi@broadcom.com>
Signed-off-by: Simran Rai <ssimran@broadcom.com>
Reviewed-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 .../bindings/sound/brcm,cygnus-audio.txt           |   63 ++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt

diff --git a/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt b/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt
new file mode 100644
index 0000000..73cddb3
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt
@@ -0,0 +1,63 @@
+BROADCOM Cygnus Audio I2S/TDM/SPDIF controller
+
+Required properties:
+	- compatible : "brcm,cygnus-audio"
+	- #address-cells: 32bit valued, 1 cell.
+	- #size-cells:  32bit valued, 0 cell.
+	- reg : Should contain audio registers location and length
+	- reg-names: names of the registers listed in "reg" property
+		Valid names are "aud" and "i2s_in". "aud" contains a
+		set of DMA, I2S_OUT and SPDIF registers. "i2s_in" contains
+		a set of I2S_IN registers.
+	- clocks: PLL and leaf clocks used by audio ports
+	- clock-names: names of 3 leaf clocks used by audio ports
+		Valid names are "ch0_audio", "ch1_audio", "ch2_audio"
+	- interrupts: audio DMA interrupt number
+
+SSP Subnode properties:
+- reg: The index of ssp port interface to use
+	Valid value are 0, 1, 2, or 3 (for spdif)
+- channel-group: Surround sound grouping that controls which channel
+  outputs belong to a group, specifically useful in Multi-channel
+  Interfaces grouping of serial port. In multi-channel stereo, use
+  "2_0", in 3.1 multi-channel grouping, use "3_1" and in 5.1
+  multi-channel grouping, use "5_1".
+
+
+Example:
+	cygnus_audio: audio@180ae000 {
+		compatible = "brcm,cygnus-audio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x180ae000 0xafd>, <0x180aec00 0x1f8>;
+		reg-names = "aud", "i2s_in";
+		clocks = <&audiopll BCM_CYGNUS_AUDIOPLL_CH0>,
+				<&audiopll BCM_CYGNUS_AUDIOPLL_CH1>,
+				<&audiopll BCM_CYGNUS_AUDIOPLL_CH2>;
+		clock-names = "ch0_audio", "ch1_audio", "ch2_audio";
+		interrupts = <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
+
+		ssp0: ssp_port@0 {
+			reg = <0>;
+			channel-group = "2_0"; /* Use 2_0, 3_1, 5_1 */
+			status = "okay";
+		};
+
+		ssp1: ssp_port@1 {
+			reg = <1>;
+			channel-group = "2_0"; /* Use 2_0, 3_1, 5_1 */
+			status = "disabled";
+		};
+
+		ssp2: ssp_port@2 {
+			reg = <2>;
+			channel-group = "2_0"; /* Use 2_0, 3_1, 5_1 */
+			status = "disabled";
+		};
+
+		spdif: spdif_port@3 {
+			reg = <3>;
+			channel-group = "2_0"; /* Use 2_0, 3_1, 5_1 */
+			status = "disabled";
+		};
+	};
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1262297 — Re: [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom Cygnus audio

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

On Mon, Nov 02, 2015 at 02:11:23PM -0800, Simran Rai wrote:

> +- channel-group: Surround sound grouping that controls which channel
> +  outputs belong to a group, specifically useful in Multi-channel
> +  Interfaces grouping of serial port. In multi-channel stereo, use
> +  "2_0", in 3.1 multi-channel grouping, use "3_1" and in 5.1
> +  multi-channel grouping, use "5_1".

I don't really understand what this means or why this is not runtime
configured.

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

#1263526 — Re: [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom Cygnus audio

Hi Mark,

On 11/4/2015 6:07 AM, Mark Brown wrote:
> On Mon, Nov 02, 2015 at 02:11:23PM -0800, Simran Rai wrote:
>
>> +- channel-group: Surround sound grouping that controls which channel
>> +  outputs belong to a group, specifically useful in Multi-channel
>> +  Interfaces grouping of serial port. In multi-channel stereo, use
>> +  "2_0", in 3.1 multi-channel grouping, use "3_1" and in 5.1
>> +  multi-channel grouping, use "5_1".
> I don't really understand what this means or why this is not runtime
> configured.
By runtime configured, do you mean passing the channel grouping through 
machine file? Is there any specific callback that I can use?

I will implement your suggestions from other patches too and send out 
next revision by early next week.

Thanks,
Simran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1263906 — Re: [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom Cygnus audio

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

On Thu, Nov 05, 2015 at 12:01:52PM -0800, Simran Rai wrote:
> On 11/4/2015 6:07 AM, Mark Brown wrote:
> >On Mon, Nov 02, 2015 at 02:11:23PM -0800, Simran Rai wrote:

> >>+  Interfaces grouping of serial port. In multi-channel stereo, use
> >>+  "2_0", in 3.1 multi-channel grouping, use "3_1" and in 5.1
> >>+  multi-channel grouping, use "5_1".

> >I don't really understand what this means or why this is not runtime
> >configured.

> By runtime configured, do you mean passing the channel grouping through
> machine file? Is there any specific callback that I can use?

No, this appears to just be setting the number of channels which is
normally configured depending on how many channels userspace is playing.

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

#1264609 — Re: [PATCH v2 1/2] dt-bindings: sound: Add DT bindings for Broadcom Cygnus audio

Hi Mark,

On 11/6/2015 2:31 AM, Mark Brown wrote:
> On Thu, Nov 05, 2015 at 12:01:52PM -0800, Simran Rai wrote:
>> On 11/4/2015 6:07 AM, Mark Brown wrote:
>>> On Mon, Nov 02, 2015 at 02:11:23PM -0800, Simran Rai wrote:
>>>> +  Interfaces grouping of serial port. In multi-channel stereo, use
>>>> +  "2_0", in 3.1 multi-channel grouping, use "3_1" and in 5.1
>>>> +  multi-channel grouping, use "5_1".
>>> I don't really understand what this means or why this is not runtime
>>> configured.
>> By runtime configured, do you mean passing the channel grouping through
>> machine file? Is there any specific callback that I can use?
> No, this appears to just be setting the number of channels which is
> normally configured depending on how many channels userspace is playing.
This audio driver uses 'params_channels()' to determine the number of 
channels requested by userspace in TDM mode (multiple
channels on one audio port). Channel grouping is another mode where more 
than one audio ports are grouped together to form
3.1 or 5.1 surround sound and this feature has been implemented through 
device tree settings.
At this point, I will take out channel grouping functionality because it 
does not work as intended. In near future, once this feature
is fixed, how do you suggest implementing it in the driver?

Thanks,
Simran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1262301

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

On Mon, Nov 02, 2015 at 02:11:22PM -0800, Simran Rai wrote:

> Simran Rai (2):
>   dt-bindings: sound: Add DT bindings for Broadcom Cygnus audio
>   VOIP-5227 Add Broadcom Cygnus audio driver.

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

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

#1262307 — Re: [PATCH v2 2/2] sound: soc: Add Cygnus audio driver

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

On Mon, Nov 02, 2015 at 02:11:24PM -0800, Simran Rai wrote:

>  sound/soc/bcm/Kconfig      |   18 +
>  sound/soc/bcm/Makefile     |    5 +
>  sound/soc/bcm/cygnus-pcm.c |  903 ++++++++++++++++++++++++++
>  sound/soc/bcm/cygnus-ssp.c | 1532 ++++++++++++++++++++++++++++++++++++++++++++
>  sound/soc/bcm/cygnus-ssp.h |  129 ++++

This is a very big patch which contains at least two drivers (a DMA
driver and a DAI driver).  Please split it into at least per-driver
patches for ease of review.

> +config SND_SOC_CYGNUS_DIAG
> +	bool "SoC platform audio for Broadcom Cygnus chips diagnostics"
> +	depends on SND_SOC_CYGNUS
> +	help
> +	  Say Y if you want to add diagnostics support in ASoC audio
> +	  on Broadcom Cygnus chips (bcm958300, bcm958305, bcm911360)
> +
> +	  If you don't know what to do here, say N.

These look like extremely specific diagnostics that I'd have expected to
be mostly doable using the standard kernel trace infrastructure which is
very low overhead and can just be left in the kernel all the time.  Why
is this a configurable option?

> +/*
> + * Enable diagnostics through menuconfig to debug the time intervals
> + * when each playback interrupt happens.
> + */

This should've been in the Kconfig help text.

> +	is_play = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);

Why is_play?  It's only looked at once and makes things a bit more
confusing.

> +	/* If playback interrupt happened */
> +	if (ANY_PLAYBACK_IRQ & r5_status)
> +		handle_playback_irq(cygaud);
> +
> +	/* If  capture interrupt happened */
> +	if (ANY_CAPTURE_IRQ & r5_status)
> +		handle_capture_irq(cygaud);
> +
> +	/*
> +	 * clear r5 interrupts after servicing them
> +	 */
> +	writel(r5_status, cygaud->audio + INTH_R5F_CLEAR_OFFSET);

This will ack interrupts we didn't handle, it'd be better to mask out
unhandled interrupts.

> +	if (aio->port_type == PORT_TDM) {

> +	} else if (aio->port_type == PORT_SPDIF) {

> +	} else {
> +		dev_err(aio->cygaud->dev, "Port not supported\n");
> +		return -EINVAL;
> +	}

This looks like it should be a switch statement, you've got some other
similar constructs in the code.

> +	error = configure_vco(cygaud, p_entry);
> +	if (error)
> +		return error;

We appear to have multiple things calling configure_vco() but I can't
see what's ensuring that they all agree with each other about the
settings.

> +	/* Slot Width is either 16 or 32 */
> +	if (slot_width <= 16)
> +		bits_per_slot = 1;

The check doesnn't match the comment here.

> +}
> +static int cygnus_ssp_resume(struct snd_soc_dai *cpu_dai)

Blank line between functions and remove empty functions.  Though I'm not
clear why the result doesn't undo what the suspend did...

> +		ssp_regs[0] = (struct cygnus_ssp_regs) INIT_SSP_REGS(0);

Why the casts?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web