All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades
@ 2018-10-16 15:02 Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 1/9] staging: bcm2835-audio: unify FOURCC command definitions Nicolas Saenz Julienne
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/9] staging: bcm2835-audio: unify FOURCC command definitions
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 2/9] staging: bcm2835-audio: don't initialize memory twice Nicolas Saenz Julienne
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/9] staging: bcm2835-audio: don't initialize memory twice
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 1/9] staging: bcm2835-audio: unify FOURCC command definitions Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 3/9] staging: bcm2835-audio: reorder variable declarations & remove trivial comments Nicolas Saenz Julienne
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/9] staging: bcm2835-audio: reorder variable declarations & remove trivial comments
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 1/9] staging: bcm2835-audio: unify FOURCC command definitions Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 2/9] staging: bcm2835-audio: don't initialize memory twice Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 4/9] staging: bcm2835-audio: use anonymous union in struct vc_audio_msg Nicolas Saenz Julienne
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/9] staging: bcm2835-audio: use anonymous union in struct vc_audio_msg
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
                   ` (2 preceding siblings ...)
  2018-10-16 15:02 ` [PATCH 3/9] staging: bcm2835-audio: reorder variable declarations & remove trivial comments Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 5/9] staging: bcm2835-audio: more generic probe function name Nicolas Saenz Julienne
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/9] staging: bcm2835-audio: more generic probe function name
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
                   ` (3 preceding siblings ...)
  2018-10-16 15:02 ` [PATCH 4/9] staging: bcm2835-audio: use anonymous union in struct vc_audio_msg Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 15:47   ` Takashi Iwai
  2018-10-16 15:02 ` [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings Nicolas Saenz Julienne
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
                   ` (4 preceding siblings ...)
  2018-10-16 15:02 ` [PATCH 5/9] staging: bcm2835-audio: more generic probe function name Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 15:56     ` Stefan Wahren
  2018-10-16 15:02 ` [PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device Nicolas Saenz Julienne
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
                   ` (5 preceding siblings ...)
  2018-10-16 15:02 ` [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 16:18     ` Eric Anholt
  2018-10-16 15:02 ` [PATCH 8/9] staging: vchiq_arm: add function to check if probe was successful Nicolas Saenz Julienne
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 8/9] staging: vchiq_arm: add function to check if probe was successful
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
                   ` (6 preceding siblings ...)
  2018-10-16 15:02 ` [PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 15:02 ` [PATCH 9/9] staging: bcm2835-audio: check for VCHIQ during probe Nicolas Saenz Julienne
  2018-10-16 15:47 ` [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Takashi Iwai
  9 siblings, 0 replies; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 9/9] staging: bcm2835-audio: check for VCHIQ during probe
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
                   ` (7 preceding siblings ...)
  2018-10-16 15:02 ` [PATCH 8/9] staging: vchiq_arm: add function to check if probe was successful Nicolas Saenz Julienne
@ 2018-10-16 15:02 ` Nicolas Saenz Julienne
  2018-10-16 15:47 ` [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Takashi Iwai
  9 siblings, 0 replies; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 15:02 UTC (permalink / raw)
  To: gregkh
  Cc: eric, stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades
  2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
                   ` (8 preceding siblings ...)
  2018-10-16 15:02 ` [PATCH 9/9] staging: bcm2835-audio: check for VCHIQ during probe Nicolas Saenz Julienne
@ 2018-10-16 15:47 ` Takashi Iwai
  9 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2018-10-16 15:47 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: gregkh, eric, stefan.wahren, linux-rpi-kernel, linux-kernel,
	devicetree, robh+dt, tiwai

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/9] staging: bcm2835-audio: more generic probe function name
  2018-10-16 15:02 ` [PATCH 5/9] staging: bcm2835-audio: more generic probe function name Nicolas Saenz Julienne
@ 2018-10-16 15:47   ` Takashi Iwai
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2018-10-16 15:47 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: gregkh, eric, stefan.wahren, linux-rpi-kernel, linux-kernel,
	devicetree, robh+dt, tiwai

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings
@ 2018-10-16 15:56     ` Stefan Wahren
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Wahren @ 2018-10-16 15:56 UTC (permalink / raw)
  To: Nicolas Saenz Julienne, gregkh
  Cc: eric, linux-rpi-kernel, linux-kernel, devicetree, robh+dt, tiwai

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings
@ 2018-10-16 15:56     ` Stefan Wahren
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Wahren @ 2018-10-16 15:56 UTC (permalink / raw)
  To: Nicolas Saenz Julienne, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, tiwai-l3A5Bk7waGM,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

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-l3A5Bk7waGM@public.gmane.org>
> ---
>  .../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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device
  2018-10-16 15:02 ` [PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device Nicolas Saenz Julienne
@ 2018-10-16 16:18     ` Eric Anholt
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Anholt @ 2018-10-16 16:18 UTC (permalink / raw)
  To: Nicolas Saenz Julienne, gregkh
  Cc: stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

[-- 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 --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device
@ 2018-10-16 16:18     ` Eric Anholt
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Anholt @ 2018-10-16 16:18 UTC (permalink / raw)
  To: gregkh
  Cc: stefan.wahren, linux-rpi-kernel, linux-kernel, devicetree,
	robh+dt, tiwai, nsaenzjulienne

[-- 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 --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings
  2018-10-16 15:56     ` Stefan Wahren
  (?)
@ 2018-10-16 16:48     ` Nicolas Saenz Julienne
  2018-10-16 19:26       ` Stefan Wahren
  -1 siblings, 1 reply; 18+ messages in thread
From: Nicolas Saenz Julienne @ 2018-10-16 16:48 UTC (permalink / raw)
  To: Stefan Wahren, gregkh
  Cc: eric, linux-rpi-kernel, linux-kernel, devicetree, robh+dt, tiwai

[-- 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 --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings
  2018-10-16 16:48     ` Nicolas Saenz Julienne
@ 2018-10-16 19:26       ` Stefan Wahren
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Wahren @ 2018-10-16 19:26 UTC (permalink / raw)
  To: Nicolas Saenz Julienne, gregkh
  Cc: eric, linux-rpi-kernel, linux-kernel, devicetree, robh+dt, tiwai

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-10-16 19:27 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-16 15:02 [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Nicolas Saenz Julienne
2018-10-16 15:02 ` [PATCH 1/9] staging: bcm2835-audio: unify FOURCC command definitions Nicolas Saenz Julienne
2018-10-16 15:02 ` [PATCH 2/9] staging: bcm2835-audio: don't initialize memory twice Nicolas Saenz Julienne
2018-10-16 15:02 ` [PATCH 3/9] staging: bcm2835-audio: reorder variable declarations & remove trivial comments Nicolas Saenz Julienne
2018-10-16 15:02 ` [PATCH 4/9] staging: bcm2835-audio: use anonymous union in struct vc_audio_msg Nicolas Saenz Julienne
2018-10-16 15:02 ` [PATCH 5/9] staging: bcm2835-audio: more generic probe function name Nicolas Saenz Julienne
2018-10-16 15:47   ` Takashi Iwai
2018-10-16 15:02 ` [PATCH 6/9] ASoC: dt-bindings: bcm2835-rpi: add onboard audio bindings Nicolas Saenz Julienne
2018-10-16 15:56   ` Stefan Wahren
2018-10-16 15:56     ` Stefan Wahren
2018-10-16 16:48     ` Nicolas Saenz Julienne
2018-10-16 19:26       ` Stefan Wahren
2018-10-16 15:02 ` [PATCH 7/9] ARM: dts: bcm2835-rpi: add onboard audio device Nicolas Saenz Julienne
2018-10-16 16:18   ` Eric Anholt
2018-10-16 16:18     ` Eric Anholt
2018-10-16 15:02 ` [PATCH 8/9] staging: vchiq_arm: add function to check if probe was successful Nicolas Saenz Julienne
2018-10-16 15:02 ` [PATCH 9/9] staging: bcm2835-audio: check for VCHIQ during probe Nicolas Saenz Julienne
2018-10-16 15:47 ` [PATCH 0/9] staging: bcm2835-audio: Cleanups and upgrades Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.