[PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades

[PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades

[Nicolas Saenz Julienne]

Hi,

I just received a RPi3B+ and decided to have a go at fixing stuff in
the audio driver:

The first half of the patchset are just random fixes found while reading
the code.

The second half provides devicetree bindings for bcm2835-audio and fixes
the probe sequence so it fails gracefully if VCHIQ isn't there (as per
TODO).

The series was developed on top of linux-next and tested on a RPi2B and
RPi3B+. Sadly I don't have an HDMI monitor with sound so I only tested
the mini jack output.

Thanks,
Nicolas

===

Nicolas Saenz Julienne (9):
  staging: bcm2835-audio: unify FOURCC command definitions
  staging: bcm2835-audio: don't initialize memory twice
  staging: bcm2835-audio: reorder variable declarations & remove trivial
    comments
  staging: bcm2835-audio: use anonymous union in struct vc_audio_msg
  staging: bcm2835-audio: more generic probe function name
  ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings
  ARM: dts: bcm2835-rpi: add onboard audio device
  staging: vchiq_arm: add function to check if probe was successful
  staging: bcm2835-audio: check for VCHIQ during probe

 .../bindings/sound/brcm,bcm2835-audio.txt     | 15 +++++++
 arch/arm/boot/dts/bcm2835-rpi.dtsi            |  5 +++
 .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 10 +----
 .../bcm2835-audio/bcm2835-vchiq.c             | 39 ++++++++-----------
 .../vc04_services/bcm2835-audio/bcm2835.c     | 30 ++++++++------
 .../bcm2835-audio/vc_vchi_audioserv_defs.h    |  6 ++-
 .../vc04_services/interface/vchi/vchi.h       |  3 ++
 .../interface/vchiq_arm/vchiq_2835_arm.c      |  2 +
 .../interface/vchiq_arm/vchiq_arm.c           | 16 ++++++++
 .../interface/vchiq_arm/vchiq_arm.h           |  1 +
 .../interface/vchiq_arm/vchiq_if.h            |  4 ++
 .../interface/vchiq_arm/vchiq_shim.c          |  7 ++++
 12 files changed, 95 insertions(+), 43 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt

-- 
2.19.1

[PATCH 1/9] staging: bcm2835-audio: unify FOURCC command definitions

[Nicolas Saenz Julienne]

The device communicates with the audio core using FOURCC codes. The
driver was generating them using different macros/expressions. We now
use the same macro to create them and centralize all the definitions.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 .../vc04_services/bcm2835-audio/bcm2835-vchiq.c     | 13 ++++---------
 .../bcm2835-audio/vc_vchi_audioserv_defs.h          |  4 +++-
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index 781754f36da7..aca7008e1921 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -89,11 +89,6 @@ static int bcm2835_audio_send_simple(struct bcm2835_audio_instance *instance,
 	return bcm2835_audio_send_msg(instance, &m, wait);
 }
 
-static const u32 BCM2835_AUDIO_WRITE_COOKIE1 = ('B' << 24 | 'C' << 16 |
-						'M' << 8  | 'A');
-static const u32 BCM2835_AUDIO_WRITE_COOKIE2 = ('D' << 24 | 'A' << 16 |
-						'T' << 8  | 'A');
-
 static void audio_vchi_callback(void *param,
 				const VCHI_CALLBACK_REASON_T reason,
 				void *msg_handle)
@@ -112,8 +107,8 @@ static void audio_vchi_callback(void *param,
 		instance->result = m.u.result.success;
 		complete(&instance->msg_avail_comp);
 	} else if (m.type == VC_AUDIO_MSG_TYPE_COMPLETE) {
-		if (m.u.complete.cookie1 != BCM2835_AUDIO_WRITE_COOKIE1 ||
-		    m.u.complete.cookie2 != BCM2835_AUDIO_WRITE_COOKIE2)
+		if (m.u.complete.cookie1 != VC_AUDIO_WRITE_COOKIE1 ||
+		    m.u.complete.cookie2 != VC_AUDIO_WRITE_COOKIE2)
 			dev_err(instance->dev, "invalid cookie\n");
 		else
 			bcm2835_playback_fifo(instance->alsa_stream,
@@ -329,8 +324,8 @@ int bcm2835_audio_write(struct bcm2835_alsa_stream *alsa_stream,
 		.type = VC_AUDIO_MSG_TYPE_WRITE,
 		.u.write.count = size,
 		.u.write.max_packet = instance->max_packet,
-		.u.write.cookie1 = BCM2835_AUDIO_WRITE_COOKIE1,
-		.u.write.cookie2 = BCM2835_AUDIO_WRITE_COOKIE2,
+		.u.write.cookie1 = VC_AUDIO_WRITE_COOKIE1,
+		.u.write.cookie2 = VC_AUDIO_WRITE_COOKIE2,
 	};
 	unsigned int count;
 	int err, status;
diff --git a/drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h b/drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h
index 1a7f0884ac9c..dc62875cfdca 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h
+++ b/drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h
@@ -7,8 +7,10 @@
 #define VC_AUDIOSERV_MIN_VER 1
 #define VC_AUDIOSERV_VER 2
 
-/* FourCC code used for VCHI connection */
+/* FourCC codes used for VCHI communication */
 #define VC_AUDIO_SERVER_NAME  MAKE_FOURCC("AUDS")
+#define VC_AUDIO_WRITE_COOKIE1 MAKE_FOURCC("BCMA")
+#define VC_AUDIO_WRITE_COOKIE2 MAKE_FOURCC("DATA")
 
 /*
  *  List of screens that are currently supported
-- 
2.19.1

[PATCH 2/9] staging: bcm2835-audio: don't initialize memory twice

[Nicolas Saenz Julienne]

The memory is being allocated with devres_alloc(), wich ultimately uses
__GFP_ZERO to call kmalloc. We don't need to zero the memory area again
in bcm2835-audio.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
index 87d56ab1ffa0..0efae7068fef 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
@@ -39,8 +39,6 @@ static int bcm2835_devm_add_vchi_ctx(struct device *dev)
 	if (!vchi_ctx)
 		return -ENOMEM;
 
-	memset(vchi_ctx, 0, sizeof(*vchi_ctx));
-
 	ret = bcm2835_new_vchi_ctx(dev, vchi_ctx);
 	if (ret) {
 		devres_free(vchi_ctx);
-- 
2.19.1

[PATCH 3/9] staging: bcm2835-audio: reorder variable declarations & remove trivial comments

[Nicolas Saenz Julienne]

When it comes to declaring variables it's preferred, when possible, to
use an inverted tree organization scheme.

Also, removes some comments that were useless.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 .../vc04_services/bcm2835-audio/bcm2835-pcm.c      | 10 ++--------
 .../vc04_services/bcm2835-audio/bcm2835-vchiq.c    |  4 ++--
 .../staging/vc04_services/bcm2835-audio/bcm2835.c  | 14 +++++++-------
 3 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
index e66da11af5cf..98b6977bdce7 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
@@ -164,14 +164,11 @@ static int snd_bcm2835_playback_spdif_open(struct snd_pcm_substream *substream)
 	return snd_bcm2835_playback_open_generic(substream, 1);
 }
 
-/* close callback */
 static int snd_bcm2835_playback_close(struct snd_pcm_substream *substream)
 {
-	/* the hardware-specific codes will be here */
-
-	struct bcm2835_chip *chip;
-	struct snd_pcm_runtime *runtime;
 	struct bcm2835_alsa_stream *alsa_stream;
+	struct snd_pcm_runtime *runtime;
+	struct bcm2835_chip *chip;
 
 	chip = snd_pcm_substream_chip(substream);
 	mutex_lock(&chip->audio_mutex);
@@ -195,20 +192,17 @@ static int snd_bcm2835_playback_close(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-/* hw_params callback */
 static int snd_bcm2835_pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_params *params)
 {
 	return snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params));
 }
 
-/* hw_free callback */
 static int snd_bcm2835_pcm_hw_free(struct snd_pcm_substream *substream)
 {
 	return snd_pcm_lib_free_pages(substream);
 }
 
-/* prepare callback */
 static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream)
 {
 	struct bcm2835_chip *chip = snd_pcm_substream_chip(substream);
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index aca7008e1921..932ef12ac5d2 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -94,9 +94,9 @@ static void audio_vchi_callback(void *param,
 				void *msg_handle)
 {
 	struct bcm2835_audio_instance *instance = param;
-	int status;
-	int msg_len;
 	struct vc_audio_msg m;
+	int msg_len;
+	int status;
 
 	if (reason != VCHI_CALLBACK_MSG_AVAILABLE)
 		return;
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
index 0efae7068fef..6ee8334dfc81 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
@@ -161,8 +161,8 @@ static int snd_add_child_device(struct device *dev,
 				struct bcm2835_audio_driver *audio_driver,
 				u32 numchans)
 {
-	struct snd_card *card;
 	struct bcm2835_chip *chip;
+	struct snd_card *card;
 	int err;
 
 	err = snd_card_new(dev, -1, NULL, THIS_MODULE, sizeof(*chip), &card);
@@ -225,12 +225,12 @@ static int snd_add_child_device(struct device *dev,
 
 static int snd_add_child_devices(struct device *device, u32 numchans)
 {
-	int i;
-	int count_devices = 0;
-	int minchannels = 0;
-	int extrachannels = 0;
 	int extrachannels_per_driver = 0;
 	int extrachannels_remainder = 0;
+	int count_devices = 0;
+	int extrachannels = 0;
+	int minchannels = 0;
+	int i;
 
 	for (i = 0; i < ARRAY_SIZE(children_devices); i++)
 		if (*children_devices[i].is_enabled)
@@ -258,9 +258,9 @@ static int snd_add_child_devices(struct device *device, u32 numchans)
 		extrachannels_remainder);
 
 	for (i = 0; i < ARRAY_SIZE(children_devices); i++) {
-		int err;
-		int numchannels_this_device;
 		struct bcm2835_audio_driver *audio_driver;
+		int numchannels_this_device;
+		int err;
 
 		if (!*children_devices[i].is_enabled)
 			continue;
-- 
2.19.1

[PATCH 4/9] staging: bcm2835-audio: use anonymous union in struct vc_audio_msg

[Nicolas Saenz Julienne]

In this case explicitly naming the union doesn't help overall code
comprehension and clutters it.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 .../bcm2835-audio/bcm2835-vchiq.c             | 30 +++++++++----------
 .../bcm2835-audio/vc_vchi_audioserv_defs.h    |  2 +-
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index 932ef12ac5d2..0db412fd7c55 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -104,15 +104,15 @@ static void audio_vchi_callback(void *param,
 	status = vchi_msg_dequeue(instance->vchi_handle,
 				  &m, sizeof(m), &msg_len, VCHI_FLAGS_NONE);
 	if (m.type == VC_AUDIO_MSG_TYPE_RESULT) {
-		instance->result = m.u.result.success;
+		instance->result = m.result.success;
 		complete(&instance->msg_avail_comp);
 	} else if (m.type == VC_AUDIO_MSG_TYPE_COMPLETE) {
-		if (m.u.complete.cookie1 != VC_AUDIO_WRITE_COOKIE1 ||
-		    m.u.complete.cookie2 != VC_AUDIO_WRITE_COOKIE2)
+		if (m.complete.cookie1 != VC_AUDIO_WRITE_COOKIE1 ||
+		    m.complete.cookie2 != VC_AUDIO_WRITE_COOKIE2)
 			dev_err(instance->dev, "invalid cookie\n");
 		else
 			bcm2835_playback_fifo(instance->alsa_stream,
-					      m.u.complete.count);
+					      m.complete.count);
 	} else {
 		dev_err(instance->dev, "unexpected callback type=%d\n", m.type);
 	}
@@ -249,11 +249,11 @@ int bcm2835_audio_set_ctls(struct bcm2835_alsa_stream *alsa_stream)
 	struct vc_audio_msg m = {};
 
 	m.type = VC_AUDIO_MSG_TYPE_CONTROL;
-	m.u.control.dest = chip->dest;
+	m.control.dest = chip->dest;
 	if (!chip->mute)
-		m.u.control.volume = CHIP_MIN_VOLUME;
+		m.control.volume = CHIP_MIN_VOLUME;
 	else
-		m.u.control.volume = alsa2chip(chip->volume);
+		m.control.volume = alsa2chip(chip->volume);
 
 	return bcm2835_audio_send_msg(alsa_stream->instance, &m, true);
 }
@@ -264,9 +264,9 @@ int bcm2835_audio_set_params(struct bcm2835_alsa_stream *alsa_stream,
 {
 	struct vc_audio_msg m = {
 		 .type = VC_AUDIO_MSG_TYPE_CONFIG,
-		 .u.config.channels = channels,
-		 .u.config.samplerate = samplerate,
-		 .u.config.bps = bps,
+		 .config.channels = channels,
+		 .config.samplerate = samplerate,
+		 .config.bps = bps,
 	};
 	int err;
 
@@ -294,7 +294,7 @@ int bcm2835_audio_drain(struct bcm2835_alsa_stream *alsa_stream)
 {
 	struct vc_audio_msg m = {
 		.type = VC_AUDIO_MSG_TYPE_STOP,
-		.u.stop.draining = 1,
+		.stop.draining = 1,
 	};
 
 	return bcm2835_audio_send_msg(alsa_stream->instance, &m, false);
@@ -322,10 +322,10 @@ int bcm2835_audio_write(struct bcm2835_alsa_stream *alsa_stream,
 	struct bcm2835_audio_instance *instance = alsa_stream->instance;
 	struct vc_audio_msg m = {
 		.type = VC_AUDIO_MSG_TYPE_WRITE,
-		.u.write.count = size,
-		.u.write.max_packet = instance->max_packet,
-		.u.write.cookie1 = VC_AUDIO_WRITE_COOKIE1,
-		.u.write.cookie2 = VC_AUDIO_WRITE_COOKIE2,
+		.write.count = size,
+		.write.max_packet = instance->max_packet,
+		.write.cookie1 = VC_AUDIO_WRITE_COOKIE1,
+		.write.cookie2 = VC_AUDIO_WRITE_COOKIE2,
 	};
 	unsigned int count;
 	int err, status;
diff --git a/drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h b/drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h
index dc62875cfdca..d6401e914ac9 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h
+++ b/drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h
@@ -93,7 +93,7 @@ struct vc_audio_msg {
 		struct vc_audio_write write;
 		struct vc_audio_result result;
 		struct vc_audio_complete complete;
-	} u;
+	};
 };
 
 #endif /* _VC_AUDIO_DEFS_H_ */
-- 
2.19.1

[PATCH 5/9] staging: bcm2835-audio: more generic probe function name

[Nicolas Saenz Julienne]

There will only be one probe function, there is no use for appendig
"_dt" the end of the name.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
index 6ee8334dfc81..039565311d10 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
@@ -291,7 +291,7 @@ static int snd_add_child_devices(struct device *device, u32 numchans)
 	return 0;
 }
 
-static int snd_bcm2835_alsa_probe_dt(struct platform_device *pdev)
+static int snd_bcm2835_alsa_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	u32 numchans;
@@ -344,7 +344,7 @@ static const struct of_device_id snd_bcm2835_of_match_table[] = {
 MODULE_DEVICE_TABLE(of, snd_bcm2835_of_match_table);
 
 static struct platform_driver bcm2835_alsa0_driver = {
-	.probe = snd_bcm2835_alsa_probe_dt,
+	.probe = snd_bcm2835_alsa_probe,
 #ifdef CONFIG_PM
 	.suspend = snd_bcm2835_alsa_suspend,
 	.resume = snd_bcm2835_alsa_resume,
-- 
2.19.1

[PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings

[Nicolas Saenz Julienne]

Adds a device tree binding file for Raspberry pi's Headphones and HDMI
audio output devices.

Based on raspberry's downstream kernel implementation:
https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2708-rpi.dtsi

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 .../bindings/sound/brcm,bcm2835-audio.txt         | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt

diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt
new file mode 100644
index 000000000000..ee6fa085aaa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt
@@ -0,0 +1,15 @@
+Broadcom BCM283x audio device
+
+Required properties:
+
+- compatible: Should be "brcm,bcm2835-audio"
+- brcm,pwm-channels: number of PWM channels, they are behind RPi's Video Core
+		     IV, not actual Linux PWM devices.
+
+Example:
+
+audio: audio {
+	compatible = "brcm,bcm2835-audio";
+	brcm,pwm-channels = <8>;
+};
+
-- 
2.19.1

[PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device

[Nicolas Saenz Julienne]

The audio device interfaces with VCHIQ to send audio through the rpi's
Headphones & HDMI.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/arm/boot/dts/bcm2835-rpi.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index cb2d6d78a7fb..6cc580d17862 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -35,6 +35,11 @@
 			reg = <0x7e00b840 0xf>;
 			interrupts = <0 2>;
 		};
+
+		audio: audio {
+			compatible = "brcm,bcm2835-audio";
+			brcm,pwm-channels = <8>;
+		};
 	};
 };
 
-- 
2.19.1

[PATCH 8/9] staging: vchiq_arm: add function to check if probe was successful

[Nicolas Saenz Julienne]

Devices depending on VCHIQ need to double check it's initialization
process was successful. This patch adds a helper function to do so.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 .../staging/vc04_services/interface/vchi/vchi.h  |  3 +++
 .../interface/vchiq_arm/vchiq_2835_arm.c         |  2 ++
 .../interface/vchiq_arm/vchiq_arm.c              | 16 ++++++++++++++++
 .../interface/vchiq_arm/vchiq_arm.h              |  1 +
 .../vc04_services/interface/vchiq_arm/vchiq_if.h |  4 ++++
 .../interface/vchiq_arm/vchiq_shim.c             |  7 +++++++
 6 files changed, 33 insertions(+)

diff --git a/drivers/staging/vc04_services/interface/vchi/vchi.h b/drivers/staging/vc04_services/interface/vchi/vchi.h
index 01381904775d..acf01352135f 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi.h
@@ -113,6 +113,9 @@ extern uint32_t vchi_current_time(VCHI_INSTANCE_T instance_handle);
 /******************************************************************************
  Global service API
  *****************************************************************************/
+// Routine to check if vchi is ready
+extern bool vchi_ready(struct device_node *firmware_node);
+
 // Routine to create a named service
 extern int32_t vchi_service_create(VCHI_INSTANCE_T instance_handle,
 				   SERVICE_CREATION_T *setup,
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 83d740feab96..31dd8a303a20 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -194,6 +194,8 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
 	}
 
 	g_dev = dev;
+	drvdata->ready = 1;
+
 	vchiq_log_info(vchiq_arm_log_level,
 		"vchiq_init - done (slots %pK, phys %pad)",
 		vchiq_slot_zero, &slot_phys);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index ea789376de0f..2690e751d1a5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -3682,6 +3682,22 @@ static int vchiq_remove(struct platform_device *pdev)
 	return 0;
 }
 
+bool vchiq_ready(struct device_node *firmware_node)
+{
+	struct platform_device *pdev = of_find_device_by_node(firmware_node);
+	struct vchiq_drvdata *drvdata;
+
+	if (!pdev)
+		return false;
+
+	drvdata = platform_get_drvdata(pdev);
+	if (!drvdata)
+		return false;
+
+	return drvdata->ready;
+}
+EXPORT_SYMBOL_GPL(vchiq_ready);
+
 static struct platform_driver vchiq_driver = {
 	.driver = {
 		.name = "bcm2835_vchiq",
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
index 2f3ebc99cbcf..8215904d219b 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -126,6 +126,7 @@ typedef struct vchiq_arm_state_struct {
 struct vchiq_drvdata {
 	const unsigned int cache_line_size;
 	struct rpi_firmware *fw;
+	unsigned int ready:1;
 };
 
 extern int vchiq_arm_log_level;
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
index e4109a83e628..54ba822f38ff 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
@@ -34,6 +34,8 @@
 #ifndef VCHIQ_IF_H
 #define VCHIQ_IF_H
 
+#include <linux/of.h>
+
 #include "interface/vchi/vchi_mh.h"
 
 #define VCHIQ_SERVICE_HANDLE_INVALID 0
@@ -179,4 +181,6 @@ extern VCHIQ_STATUS_T vchiq_dump_phys_mem(VCHIQ_SERVICE_HANDLE_T service,
 extern VCHIQ_STATUS_T vchiq_get_peer_version(VCHIQ_SERVICE_HANDLE_T handle,
       short *peer_version);
 
+extern bool vchiq_ready(struct device_node *firmware_node);
+
 #endif /* VCHIQ_IF_H */
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
index c3223fcdaf87..69fdface29fd 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
@@ -32,6 +32,7 @@
  */
 #include <linux/module.h>
 #include <linux/types.h>
+#include <linux/of.h>
 
 #include "interface/vchi/vchi.h"
 #include "vchiq.h"
@@ -50,6 +51,12 @@ struct shim_service {
 	void *callback_param;
 };
 
+bool vchi_ready(struct device_node *firmware_node)
+{
+	return vchiq_ready(firmware_node);
+}
+EXPORT_SYMBOL(vchi_ready);
+
 /***********************************************************
  * Name: vchi_msg_peek
  *
-- 
2.19.1

[PATCH 9/9] staging: bcm2835-audio: check for VCHIQ during probe

[Nicolas Saenz Julienne]

The audio data is sent to rpi's VideoCore IV trough VCHIQ. We make sure
it's available and that we fail gracefully in case it isn't there.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
index 039565311d10..f4b19a4a974b 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
@@ -294,9 +294,19 @@ static int snd_add_child_devices(struct device *device, u32 numchans)
 static int snd_bcm2835_alsa_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *fw_node;
 	u32 numchans;
 	int err;
 
+	fw_node = of_find_compatible_node(NULL, NULL, "brcm,bcm2835-vchiq");
+	if (!fw_node) {
+		dev_err(&pdev->dev, "no vchiq firmware node\n");
+		return -ENODEV;
+	}
+
+	if (!vchi_ready(fw_node))
+		return -EPROBE_DEFER;
+
 	err = of_property_read_u32(dev->of_node, "brcm,pwm-channels",
 				   &numchans);
 	if (err) {
-- 
2.19.1

Re: [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades

[Takashi Iwai]

On Tue, 16 Oct 2018 17:02:19 +0200,
Nicolas Saenz Julienne wrote:
> 
> Hi,
> 
> I just received a RPi3B+ and decided to have a go at fixing stuff in
> the audio driver:
> 
> The first half of the patchset are just random fixes found while reading
> the code.
> 
> The second half provides devicetree bindings for bcm2835-audio and fixes
> the probe sequence so it fails gracefully if VCHIQ isn't there (as per
> TODO).
> 
> The series was developed on top of linux-next and tested on a RPi2B and
> RPi3B+. Sadly I don't have an HDMI monitor with sound so I only tested
> the mini jack output.

All patches look good to me:
  Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

> 
> Thanks,
> Nicolas
> 
> ===
> 
> Nicolas Saenz Julienne (9):
>   staging: bcm2835-audio: unify FOURCC command definitions
>   staging: bcm2835-audio: don't initialize memory twice
>   staging: bcm2835-audio: reorder variable declarations & remove trivial
>     comments
>   staging: bcm2835-audio: use anonymous union in struct vc_audio_msg
>   staging: bcm2835-audio: more generic probe function name
>   ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings
>   ARM: dts: bcm2835-rpi: add onboard audio device
>   staging: vchiq_arm: add function to check if probe was successful
>   staging: bcm2835-audio: check for VCHIQ during probe
> 
>  .../bindings/sound/brcm,bcm2835-audio.txt     | 15 +++++++
>  arch/arm/boot/dts/bcm2835-rpi.dtsi            |  5 +++
>  .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 10 +----
>  .../bcm2835-audio/bcm2835-vchiq.c             | 39 ++++++++-----------
>  .../vc04_services/bcm2835-audio/bcm2835.c     | 30 ++++++++------
>  .../bcm2835-audio/vc_vchi_audioserv_defs.h    |  6 ++-
>  .../vc04_services/interface/vchi/vchi.h       |  3 ++
>  .../interface/vchiq_arm/vchiq_2835_arm.c      |  2 +
>  .../interface/vchiq_arm/vchiq_arm.c           | 16 ++++++++
>  .../interface/vchiq_arm/vchiq_arm.h           |  1 +
>  .../interface/vchiq_arm/vchiq_if.h            |  4 ++
>  .../interface/vchiq_arm/vchiq_shim.c          |  7 ++++
>  12 files changed, 95 insertions(+), 43 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt
> 
> -- 
> 2.19.1
> 

Re: [PATCH 5/9] staging: bcm2835-audio: more generic probe function name

[Takashi Iwai]

On Tue, 16 Oct 2018 17:02:24 +0200,
Nicolas Saenz Julienne wrote:
> 
> There will only be one probe function, there is no use for appendig
> "_dt" the end of the name.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/staging/vc04_services/bcm2835-audio/bcm2835.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
> index 6ee8334dfc81..039565311d10 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
> @@ -291,7 +291,7 @@ static int snd_add_child_devices(struct device *device, u32 numchans)
>  	return 0;
>  }
>  
> -static int snd_bcm2835_alsa_probe_dt(struct platform_device *pdev)
> +static int snd_bcm2835_alsa_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	u32 numchans;
> @@ -344,7 +344,7 @@ static const struct of_device_id snd_bcm2835_of_match_table[] = {
>  MODULE_DEVICE_TABLE(of, snd_bcm2835_of_match_table);
>  
>  static struct platform_driver bcm2835_alsa0_driver = {

Actually now I wonder why this "0" here...

It has nothing to do with the patch, but it's a good opportunity to
clean it up, too.


thanks,

Takashi

Re: [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings

[Stefan Wahren]

Hi Nicolas,

Am 16.10.2018 um 17:02 schrieb Nicolas Saenz Julienne:
> Adds a device tree binding file for Raspberry pi's Headphones and HDMI
> audio output devices.
>
> Based on raspberry's downstream kernel implementation:
> https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2708-rpi.dtsi
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  .../bindings/sound/brcm,bcm2835-audio.txt         | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt
>
> diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt
> new file mode 100644
> index 000000000000..ee6fa085aaa9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt
> @@ -0,0 +1,15 @@
> +Broadcom BCM283x audio device
> +
> +Required properties:
> +
> +- compatible: Should be "brcm,bcm2835-audio"
> +- brcm,pwm-channels: number of PWM channels, they are behind RPi's Video Core
> +		     IV, not actual Linux PWM devices.
> +
> +Example:
> +
> +audio: audio {
> +	compatible = "brcm,bcm2835-audio";
> +	brcm,pwm-channels = <8>;
> +};
> +

i apologize but it seems to me that the TODO mentioned in the cover
letter isn't update to date anymore.

Phil Elwell posted an important bugfix for vchiq before [1], but only
the driver part has been applied yet. After applying the DT changes i'm
not sure if it still works.

AFAIK the audio driver uses VCHIQ as a software interface and the
binding doesn't describe the real hardware.

Since the camera driver will be registered as a platform device [2], i
prefer this way for the audio driver, too.

I'm actually working on this here [3] (currently only compile tested).

Stefan

[1] - https://patchwork.ozlabs.org/cover/970434/
[2] - https://lore.kernel.org/patchwork/patch/904411/
[3] - https://github.com/anholt/linux/commits/bcm2835-audio

Re: [PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

Nicolas Saenz Julienne <nsaenzjulienne@suse.de> writes:

> The audio device interfaces with VCHIQ to send audio through the rpi's
> Headphones & HDMI.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  arch/arm/boot/dts/bcm2835-rpi.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> index cb2d6d78a7fb..6cc580d17862 100644
> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> @@ -35,6 +35,11 @@
>  			reg = <0x7e00b840 0xf>;
>  			interrupts = <0 2>;
>  		};
> +
> +		audio: audio {
> +			compatible = "brcm,bcm2835-audio";
> +			brcm,pwm-channels = <8>;
> +		};
>  	};
>  };

Assuming the DT binding gets acked,

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 2570 bytes --]

Hi Stefan, thanks for the review,

On Tue, 2018-10-16 at 17:56 +0200, Stefan Wahren wrote:
> Hi Nicolas,
> 
> Am 16.10.2018 um 17:02 schrieb Nicolas Saenz Julienne:
> > Adds a device tree binding file for Raspberry pi's Headphones and
> > HDMI
> > audio output devices.
> > 
> > Based on raspberry's downstream kernel implementation:
> > 

https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2708-rpi.dtsi
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> >  .../bindings/sound/brcm,bcm2835-audio.txt         | 15
> > +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835-
> > audio.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835-
> > audio.txt
> > new file mode 100644
> > index 000000000000..ee6fa085aaa9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835-
> > audio.txt
> > @@ -0,0 +1,15 @@
> > +Broadcom BCM283x audio device
> > +
> > +Required properties:
> > +
> > +- compatible: Should be "brcm,bcm2835-audio"
> > +- brcm,pwm-channels: number of PWM channels, they are behind RPi's
> > Video Core
> > +		     IV, not actual Linux PWM devices.
> > +
> > +Example:
> > +
> > +audio: audio {
> > +	compatible = "brcm,bcm2835-audio";
> > +	brcm,pwm-channels = <8>;
> > +};
> > +
> 
> i apologize but it seems to me that the TODO mentioned in the cover
> letter isn't update to date anymore.
> 
> Phil Elwell posted an important bugfix for vchiq before [1], but only
> the driver part has been applied yet. After applying the DT changes
> i'm
> not sure if it still works.
> 
> AFAIK the audio driver uses VCHIQ as a software interface and the
> binding doesn't describe the real hardware.
> 
> Since the camera driver will be registered as a platform device [2],
> i
> prefer this way for the audio driver, too.
> 
> I'm actually working on this here [3] (currently only compile
> tested).
> 

Fair enough. I wasn't feeling too good about the bindings myself. I
only went with them because I saw something similar was happening
between "raspberrypi,bcm2835-power" and "raspberrypi,bcm2835-firmware". 
I'll be glad to review & test your patches whenever they are ready.

This makes commits 6 to 9 useless. Do you want me to send a v2? I can
throw in an extra change Takashi suggested and update the TODO file.

Regards,
Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings

[Stefan Wahren]

Hi Nicolas,

> Nicolas Saenz Julienne <nsaenzjulienne@suse.de> hat am 16. Oktober 2018 um 18:48 geschrieben:
> 
> 
> Hi Stefan, thanks for the review,
> 
> On Tue, 2018-10-16 at 17:56 +0200, Stefan Wahren wrote:
> > Hi Nicolas,
> > 
> > Am 16.10.2018 um 17:02 schrieb Nicolas Saenz Julienne:
> > > Adds a device tree binding file for Raspberry pi's Headphones and
> > > HDMI
> > > audio output devices.
> > > 
> > > Based on raspberry's downstream kernel implementation:
> > > 
> 
> https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2708-rpi.dtsi
> > > 
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > ---
> > >  .../bindings/sound/brcm,bcm2835-audio.txt         | 15
> > > +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835-
> > > audio.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835-
> > > audio.txt
> > > new file mode 100644
> > > index 000000000000..ee6fa085aaa9
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835-
> > > audio.txt
> > > @@ -0,0 +1,15 @@
> > > +Broadcom BCM283x audio device
> > > +
> > > +Required properties:
> > > +
> > > +- compatible: Should be "brcm,bcm2835-audio"
> > > +- brcm,pwm-channels: number of PWM channels, they are behind RPi's
> > > Video Core
> > > +		     IV, not actual Linux PWM devices.
> > > +
> > > +Example:
> > > +
> > > +audio: audio {
> > > +	compatible = "brcm,bcm2835-audio";
> > > +	brcm,pwm-channels = <8>;
> > > +};
> > > +
> > 
> > i apologize but it seems to me that the TODO mentioned in the cover
> > letter isn't update to date anymore.
> > 
> > Phil Elwell posted an important bugfix for vchiq before [1], but only
> > the driver part has been applied yet. After applying the DT changes
> > i'm
> > not sure if it still works.
> > 
> > AFAIK the audio driver uses VCHIQ as a software interface and the
> > binding doesn't describe the real hardware.
> > 
> > Since the camera driver will be registered as a platform device [2],
> > i
> > prefer this way for the audio driver, too.
> > 
> > I'm actually working on this here [3] (currently only compile
> > tested).
> > 
> 
> Fair enough. I wasn't feeling too good about the bindings myself. I
> only went with them because I saw something similar was happening
> between "raspberrypi,bcm2835-power" and "raspberrypi,bcm2835-firmware". 
> I'll be glad to review & test your patches whenever they are ready.

i will inform you.

> 
> This makes commits 6 to 9 useless. Do you want me to send a v2? I can
> throw in an extra change Takashi suggested and update the TODO file.

You can add my

Acked-by: Stefan Wahren <stefan.wahren@i2se.com>

to patches 1 to 5. Please drop 6 to 9 from the series and add the other suggestions.

Btw please add devel@driverdev.osuosl.org to CC for V2

Thanks
Stefan

> 
> Regards,
> Nicolas