This adds minimal support for accessing the HDMI audio provided through the
I2S port available on ADV7481 and ADV7482 decoder devices by ADI.
The port carries audio signal from the decoded HDMI stream.
Currently, the driver only supports I2S in TDM, 8 channels a 24bit at 48kHz.
Furthermore, only left-justified, 8 slots, 32bit/slot TDM, at 256fs has been
ever tried.
An ADV7482 on the Renesas Salvator-X ES1.1 (R8A77950 SoC) was used during
development of this code.
Changes since v2:
- prepare/enable the clock when it is used, as it seems nothing else does
this otherwise
- give the clock a unique name to ensure it can be registered if there are
multiple adv748x devices in the system
- remove optionality note from clock cell description to ensure the device
description matches the real device (the line is always present, even
if not used)
Changes since v1:
- Add ssi4_ctrl pin group to the sound pins. The pins are responsible for
SCK4 (sample clock) WS4 and (word boundary input), and are required for
SSI audio input over I2S.
Reported-by: Geert Uytterhoeven <[email protected]>
- Removed the audio clock C from the list of clocks of adv748x,
it is exactly the other way around.
Reported-by: Geert Uytterhoeven <[email protected]>
- Add an instance of (currently) fixed rate I2S master clock (MCLK),
connected to the audio_clk_c line of the R-Car SoC.
Explicitly declare the device a clock producer and add it to the
list of clocks used by the audio system of the Salvator-X board.
Suggested-by: Geert Uytterhoeven <[email protected]>
- The implementation of DAI driver has been moved in a separate file
and modified to activate audio decoding and I2S streaming using
snd_soc_dai_... interfaces. This allows the driver to be used with
just ALSA interfaces.
- The ioctls for selecting audio output and muting have been removed,
as not applicable.
Suggested-by: Hans Verkuil <[email protected]>
I have left implementation of the QUERYCAP in, as it seems to be required
by v4l-ctl to support loading of EDID for this node. And setting the EDID
is one feature I desperately need: there are devices which plainly refuse
to talk to the sink if it does not provide EDID they like.
- A device tree configuration without audio port will disable the audio code
altogether, supporting integrations where the port is not connected.
- The patches have been re-arranged, starting with the generic changes and
changes not related to audio directly. Those will be probably sent as a
separate series later.
- The whole series has been rebased on top of v5.6-rc6
Alex Riesen (11):
media: adv748x: fix end-of-line terminators in diagnostic statements
media: adv748x: include everything adv748x.h needs into the file
media: adv748x: reduce amount of code for bitwise modifications of
device registers
media: adv748x: add definitions for audio output related registers
media: adv748x: add support for HDMI audio
media: adv748x: prepare/enable mclk when the audio is used
media: adv748x: only activate DAI if it is described in device tree
dt-bindings: adv748x: add information about serial audio interface
(I2S/TDM)
arm64: dts: renesas: salvator: add a connection from adv748x codec
(HDMI input) to the R-Car SoC
media: adv748x: add support for log_status ioctl
media: adv748x: allow the HDMI sub-device to accept EDID
.../devicetree/bindings/media/i2c/adv748x.txt | 16 +-
.../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +-
.../boot/dts/renesas/salvator-common.dtsi | 47 ++-
drivers/media/i2c/adv748x/Makefile | 3 +-
drivers/media/i2c/adv748x/adv748x-afe.c | 6 +-
drivers/media/i2c/adv748x/adv748x-core.c | 60 ++--
drivers/media/i2c/adv748x/adv748x-csi2.c | 8 +-
drivers/media/i2c/adv748x/adv748x-dai.c | 282 ++++++++++++++++++
drivers/media/i2c/adv748x/adv748x-hdmi.c | 212 ++++++++++++-
drivers/media/i2c/adv748x/adv748x.h | 67 ++++-
10 files changed, 662 insertions(+), 42 deletions(-)
create mode 100644 drivers/media/i2c/adv748x/adv748x-dai.c
--
2.25.1.25.g9ecbe7eb18
Signed-off-by: Alexander Riesen <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/adv748x/adv748x-core.c | 24 ++++++++++++------------
drivers/media/i2c/adv748x/adv748x-csi2.c | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 23e02ff27b17..c3fb113cef62 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -623,11 +623,11 @@ static int adv748x_parse_dt(struct adv748x_state *state)
for_each_endpoint_of_node(state->dev->of_node, ep_np) {
of_graph_parse_endpoint(ep_np, &ep);
- adv_info(state, "Endpoint %pOF on port %d", ep.local_node,
+ adv_info(state, "Endpoint %pOF on port %d\n", ep.local_node,
ep.port);
if (ep.port >= ADV748X_PORT_MAX) {
- adv_err(state, "Invalid endpoint %pOF on port %d",
+ adv_err(state, "Invalid endpoint %pOF on port %d\n",
ep.local_node, ep.port);
continue;
@@ -635,7 +635,7 @@ static int adv748x_parse_dt(struct adv748x_state *state)
if (state->endpoints[ep.port]) {
adv_err(state,
- "Multiple port endpoints are not supported");
+ "Multiple port endpoints are not supported\n");
continue;
}
@@ -702,62 +702,62 @@ static int adv748x_probe(struct i2c_client *client)
/* Discover and process ports declared by the Device tree endpoints */
ret = adv748x_parse_dt(state);
if (ret) {
- adv_err(state, "Failed to parse device tree");
+ adv_err(state, "Failed to parse device tree\n");
goto err_free_mutex;
}
/* Configure IO Regmap region */
ret = adv748x_configure_regmap(state, ADV748X_PAGE_IO);
if (ret) {
- adv_err(state, "Error configuring IO regmap region");
+ adv_err(state, "Error configuring IO regmap region\n");
goto err_cleanup_dt;
}
ret = adv748x_identify_chip(state);
if (ret) {
- adv_err(state, "Failed to identify chip");
+ adv_err(state, "Failed to identify chip\n");
goto err_cleanup_dt;
}
/* Configure remaining pages as I2C clients with regmap access */
ret = adv748x_initialise_clients(state);
if (ret) {
- adv_err(state, "Failed to setup client regmap pages");
+ adv_err(state, "Failed to setup client regmap pages\n");
goto err_cleanup_clients;
}
/* SW reset ADV748X to its default values */
ret = adv748x_reset(state);
if (ret) {
- adv_err(state, "Failed to reset hardware");
+ adv_err(state, "Failed to reset hardware\n");
goto err_cleanup_clients;
}
/* Initialise HDMI */
ret = adv748x_hdmi_init(&state->hdmi);
if (ret) {
- adv_err(state, "Failed to probe HDMI");
+ adv_err(state, "Failed to probe HDMI\n");
goto err_cleanup_clients;
}
/* Initialise AFE */
ret = adv748x_afe_init(&state->afe);
if (ret) {
- adv_err(state, "Failed to probe AFE");
+ adv_err(state, "Failed to probe AFE\n");
goto err_cleanup_hdmi;
}
/* Initialise TXA */
ret = adv748x_csi2_init(state, &state->txa);
if (ret) {
- adv_err(state, "Failed to probe TXA");
+ adv_err(state, "Failed to probe TXA\n");
goto err_cleanup_afe;
}
/* Initialise TXB */
ret = adv748x_csi2_init(state, &state->txb);
if (ret) {
- adv_err(state, "Failed to probe TXB");
+ adv_err(state, "Failed to probe TXB\n");
goto err_cleanup_txa;
}
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 2091cda50935..c43ce5d78723 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -72,7 +72,7 @@ static int adv748x_csi2_registered(struct v4l2_subdev *sd)
struct adv748x_state *state = tx->state;
int ret;
- adv_dbg(state, "Registered %s (%s)", is_txa(tx) ? "TXA":"TXB",
+ adv_dbg(state, "Registered %s (%s)\n", is_txa(tx) ? "TXA":"TXB",
sd->name);
/*
--
2.25.1.25.g9ecbe7eb18
To follow the established practice of not depending on others to
pull everything in. While at it, make sure it stays like this.
Signed-off-by: Alexander Riesen <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/adv748x/adv748x-afe.c | 6 ++----
drivers/media/i2c/adv748x/adv748x-core.c | 6 ++----
drivers/media/i2c/adv748x/adv748x-csi2.c | 6 ++----
drivers/media/i2c/adv748x/adv748x-hdmi.c | 6 ++----
drivers/media/i2c/adv748x/adv748x.h | 2 ++
5 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c b/drivers/media/i2c/adv748x/adv748x-afe.c
index dbbb1e4d6363..5a25d1fbe25f 100644
--- a/drivers/media/i2c/adv748x/adv748x-afe.c
+++ b/drivers/media/i2c/adv748x/adv748x-afe.c
@@ -6,18 +6,16 @@
* Copyright (C) 2017 Renesas Electronics Corp.
*/
+#include "adv748x.h"
+
#include <linux/delay.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/v4l2-dv-timings.h>
-#include <media/v4l2-ctrls.h>
-#include <media/v4l2-device.h>
#include <media/v4l2-dv-timings.h>
#include <media/v4l2-ioctl.h>
-#include "adv748x.h"
-
/* -----------------------------------------------------------------------------
* SDP
*/
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index c3fb113cef62..5c59aad319d1 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -10,6 +10,8 @@
* Kieran Bingham <[email protected]>
*/
+#include "adv748x.h"
+
#include <linux/delay.h>
#include <linux/errno.h>
#include <linux/i2c.h>
@@ -20,14 +22,10 @@
#include <linux/slab.h>
#include <linux/v4l2-dv-timings.h>
-#include <media/v4l2-ctrls.h>
-#include <media/v4l2-device.h>
#include <media/v4l2-dv-timings.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-ioctl.h>
-#include "adv748x.h"
-
/* -----------------------------------------------------------------------------
* Register manipulation
*/
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index c43ce5d78723..c00d4f347d95 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -5,15 +5,13 @@
* Copyright (C) 2017 Renesas Electronics Corp.
*/
+#include "adv748x.h"
+
#include <linux/module.h>
#include <linux/mutex.h>
-#include <media/v4l2-ctrls.h>
-#include <media/v4l2-device.h>
#include <media/v4l2-ioctl.h>
-#include "adv748x.h"
-
static int adv748x_csi2_set_virtual_channel(struct adv748x_csi2 *tx,
unsigned int vc)
{
diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c
index c557f8fdf11a..f598acec3b5c 100644
--- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
+++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
@@ -5,18 +5,16 @@
* Copyright (C) 2017 Renesas Electronics Corp.
*/
+#include "adv748x.h"
+
#include <linux/module.h>
#include <linux/mutex.h>
-#include <media/v4l2-ctrls.h>
-#include <media/v4l2-device.h>
#include <media/v4l2-dv-timings.h>
#include <media/v4l2-ioctl.h>
#include <uapi/linux/v4l2-dv-timings.h>
-#include "adv748x.h"
-
/* -----------------------------------------------------------------------------
* HDMI and CP
*/
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index fccb388ce179..09aab4138c3f 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -19,6 +19,8 @@
*/
#include <linux/i2c.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
#ifndef _ADV748X_H_
#define _ADV748X_H_
--
2.25.1.25.g9ecbe7eb18
The regmap provides a convenient utility for this.
The hdmi_* and dpll_* register modification macros added for symmetry
with the existing operations (io_*, sdp_*).
Signed-off-by: Alexander Riesen <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
--
v3: remove _update name in favor of existing _clrset
---
drivers/media/i2c/adv748x/adv748x-core.c | 6 ++++++
drivers/media/i2c/adv748x/adv748x.h | 14 +++++++++++---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 5c59aad319d1..8580e6624276 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -133,6 +133,12 @@ static int adv748x_write_check(struct adv748x_state *state, u8 page, u8 reg,
return *error;
}
+int adv748x_update_bits(struct adv748x_state *state, u8 page, u8 reg, u8 mask,
+ u8 value)
+{
+ return regmap_update_bits(state->regmap[page], reg, mask, value);
+}
+
/* adv748x_write_block(): Write raw data with a maximum of I2C_SMBUS_BLOCK_MAX
* size to one or more registers.
*
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index 09aab4138c3f..0a9d78c2870b 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -393,25 +393,33 @@ int adv748x_write(struct adv748x_state *state, u8 page, u8 reg, u8 value);
int adv748x_write_block(struct adv748x_state *state, int client_page,
unsigned int init_reg, const void *val,
size_t val_len);
+int adv748x_update_bits(struct adv748x_state *state, u8 page, u8 reg,
+ u8 mask, u8 value);
#define io_read(s, r) adv748x_read(s, ADV748X_PAGE_IO, r)
#define io_write(s, r, v) adv748x_write(s, ADV748X_PAGE_IO, r, v)
-#define io_clrset(s, r, m, v) io_write(s, r, (io_read(s, r) & ~(m)) | (v))
+#define io_clrset(s, r, m, v) adv748x_update_bits(s, ADV748X_PAGE_IO, r, m, v)
#define hdmi_read(s, r) adv748x_read(s, ADV748X_PAGE_HDMI, r)
#define hdmi_read16(s, r, m) (((hdmi_read(s, r) << 8) | hdmi_read(s, (r)+1)) & (m))
#define hdmi_write(s, r, v) adv748x_write(s, ADV748X_PAGE_HDMI, r, v)
+#define hdmi_clrset(s, r, m, v) \
+ adv748x_update_bits(s, ADV748X_PAGE_HDMI, r, m, v)
+
+#define dpll_read(s, r) adv748x_read(s, ADV748X_PAGE_DPLL, r)
+#define dpll_clrset(s, r, m, v) \
+ adv748x_update_bits(s, ADV748X_PAGE_DPLL, r, m, v)
#define repeater_read(s, r) adv748x_read(s, ADV748X_PAGE_REPEATER, r)
#define repeater_write(s, r, v) adv748x_write(s, ADV748X_PAGE_REPEATER, r, v)
#define sdp_read(s, r) adv748x_read(s, ADV748X_PAGE_SDP, r)
#define sdp_write(s, r, v) adv748x_write(s, ADV748X_PAGE_SDP, r, v)
-#define sdp_clrset(s, r, m, v) sdp_write(s, r, (sdp_read(s, r) & ~(m)) | (v))
+#define sdp_clrset(s, r, m, v) adv748x_update_bits(s, ADV748X_PAGE_SDP, r, m, v)
#define cp_read(s, r) adv748x_read(s, ADV748X_PAGE_CP, r)
#define cp_write(s, r, v) adv748x_write(s, ADV748X_PAGE_CP, r, v)
-#define cp_clrset(s, r, m, v) cp_write(s, r, (cp_read(s, r) & ~(m)) | (v))
+#define cp_clrset(s, r, m, v) adv748x_update_bits(s, ADV748X_PAGE_CP, r, m, v)
#define tx_read(t, r) adv748x_read(t->state, t->page, r)
#define tx_write(t, r, v) adv748x_write(t->state, t->page, r, v)
--
2.25.1.25.g9ecbe7eb18
Signed-off-by: Alexander Riesen <[email protected]>
---
drivers/media/i2c/adv748x/adv748x.h | 32 +++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index 0a9d78c2870b..1a1ea70086c6 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -226,6 +226,11 @@ struct adv748x_state {
#define ADV748X_IO_VID_STD 0x05
+#define ADV748X_IO_PAD_CONTROLS 0x0e
+#define ADV748X_IO_PAD_CONTROLS_TRI_AUD BIT(5)
+#define ADV748X_IO_PAD_CONTROLS_PDN_AUD BIT(1)
+#define ADV748X_IO_PAD_CONTROLS1 0x1d
+
#define ADV748X_IO_10 0x10 /* io_reg_10 */
#define ADV748X_IO_10_CSI4_EN BIT(7)
#define ADV748X_IO_10_CSI1_EN BIT(6)
@@ -248,7 +253,21 @@ struct adv748x_state {
#define ADV748X_IO_REG_FF 0xff
#define ADV748X_IO_REG_FF_MAIN_RESET 0xff
+/* DPLL Map */
+#define ADV748X_DPLL_MCLK_FS 0xb5
+#define ADV748X_DPLL_MCLK_FS_N_MASK GENMASK(2, 0)
+
/* HDMI RX Map */
+#define ADV748X_HDMI_I2S 0x03 /* I2S mode and width */
+#define ADV748X_HDMI_I2SBITWIDTH_MASK GENMASK(4, 0)
+#define ADV748X_HDMI_I2SOUTMODE_SHIFT 5
+#define ADV748X_HDMI_I2SOUTMODE_MASK \
+ GENMASK(6, ADV748X_HDMI_I2SOUTMODE_SHIFT)
+#define ADV748X_I2SOUTMODE_I2S 0
+#define ADV748X_I2SOUTMODE_RIGHT_J 1
+#define ADV748X_I2SOUTMODE_LEFT_J 2
+#define ADV748X_I2SOUTMODE_SPDIF 3
+
#define ADV748X_HDMI_LW1 0x07 /* line width_1 */
#define ADV748X_HDMI_LW1_VERT_FILTER BIT(7)
#define ADV748X_HDMI_LW1_DE_REGEN BIT(5)
@@ -260,6 +279,16 @@ struct adv748x_state {
#define ADV748X_HDMI_F1H1 0x0b /* field1 height_1 */
#define ADV748X_HDMI_F1H1_INTERLACED BIT(5)
+#define ADV748X_HDMI_MUTE_CTRL 0x1a
+#define ADV748X_HDMI_MUTE_CTRL_MUTE_AUDIO BIT(4)
+#define ADV748X_HDMI_MUTE_CTRL_WAIT_UNMUTE_MASK GENMASK(3, 1)
+#define ADV748X_HDMI_MUTE_CTRL_NOT_AUTO_UNMUTE BIT(0)
+
+#define ADV748X_HDMI_AUDIO_MUTE_SPEED 0x0f
+#define ADV748X_HDMI_AUDIO_MUTE_SPEED_MASK GENMASK(4, 0)
+#define ADV748X_MAN_AUDIO_DL_BYPASS BIT(7)
+#define ADV748X_AUDIO_DELAY_LINE_BYPASS BIT(6)
+
#define ADV748X_HDMI_HFRONT_PORCH 0x20 /* hsync_front_porch_1 */
#define ADV748X_HDMI_HFRONT_PORCH_MASK 0x1fff
@@ -281,6 +310,9 @@ struct adv748x_state {
#define ADV748X_HDMI_TMDS_1 0x51 /* hdmi_reg_51 */
#define ADV748X_HDMI_TMDS_2 0x52 /* hdmi_reg_52 */
+#define ADV748X_HDMI_REG_6D 0x6d /* hdmi_reg_6d */
+#define ADV748X_I2S_TDM_MODE_ENABLE BIT(7)
+
/* HDMI RX Repeater Map */
#define ADV748X_REPEATER_EDID_SZ 0x70 /* primary_edid_size */
#define ADV748X_REPEATER_EDID_SZ_SHIFT 4
--
2.25.1.25.g9ecbe7eb18
This adds an implemention of SoC DAI driver which provides access to the
I2S port of the device.
Signed-off-by: Alexander Riesen <[email protected]>
--
v3: fix clock registration in case of multiple adv748x devices
Suggested-by: Geert Uytterhoeven <[email protected]>
---
drivers/media/i2c/adv748x/Makefile | 3 +-
drivers/media/i2c/adv748x/adv748x-core.c | 9 +-
drivers/media/i2c/adv748x/adv748x-dai.c | 265 +++++++++++++++++++++++
drivers/media/i2c/adv748x/adv748x.h | 17 +-
4 files changed, 291 insertions(+), 3 deletions(-)
create mode 100644 drivers/media/i2c/adv748x/adv748x-dai.c
diff --git a/drivers/media/i2c/adv748x/Makefile b/drivers/media/i2c/adv748x/Makefile
index 93844f14cb10..6e7a302ef199 100644
--- a/drivers/media/i2c/adv748x/Makefile
+++ b/drivers/media/i2c/adv748x/Makefile
@@ -3,6 +3,7 @@ adv748x-objs := \
adv748x-afe.o \
adv748x-core.o \
adv748x-csi2.o \
- adv748x-hdmi.o
+ adv748x-hdmi.o \
+ adv748x-dai.o
obj-$(CONFIG_VIDEO_ADV748X) += adv748x.o
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 8580e6624276..3513ca138e53 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -765,8 +765,14 @@ static int adv748x_probe(struct i2c_client *client)
goto err_cleanup_txa;
}
+ ret = adv748x_dai_init(&state->dai);
+ if (ret) {
+ adv_err(state, "Failed to probe DAI\n");
+ goto err_cleanup_txb;
+ }
return 0;
-
+err_cleanup_txb:
+ adv748x_csi2_cleanup(&state->txb);
err_cleanup_txa:
adv748x_csi2_cleanup(&state->txa);
err_cleanup_afe:
@@ -787,6 +793,7 @@ static int adv748x_remove(struct i2c_client *client)
{
struct adv748x_state *state = i2c_get_clientdata(client);
+ adv748x_dai_cleanup(&state->dai);
adv748x_afe_cleanup(&state->afe);
adv748x_hdmi_cleanup(&state->hdmi);
diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c b/drivers/media/i2c/adv748x/adv748x-dai.c
new file mode 100644
index 000000000000..6fce7d000423
--- /dev/null
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Analog Devices ADV748X HDMI receiver with AFE
+ * The implementation of DAI.
+ */
+
+#include "adv748x.h"
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <sound/pcm_params.h>
+
+#define state_of(soc_dai) \
+ adv748x_dai_to_state(container_of((soc_dai)->driver, \
+ struct adv748x_dai, \
+ drv))
+
+static const char ADV748X_DAI_NAME[] = "adv748x-i2s";
+
+static int set_audio_pads_state(struct adv748x_state *state, int on)
+{
+ return io_clrset(state, ADV748X_IO_PAD_CONTROLS,
+ ADV748X_IO_PAD_CONTROLS_TRI_AUD |
+ ADV748X_IO_PAD_CONTROLS_PDN_AUD,
+ on ? 0 : 0xff);
+}
+
+static int set_dpll_mclk_fs(struct adv748x_state *state, int fs)
+{
+ return dpll_clrset(state, ADV748X_DPLL_MCLK_FS,
+ ADV748X_DPLL_MCLK_FS_N_MASK, (fs / 128) - 1);
+}
+
+static int set_i2s_format(struct adv748x_state *state, uint outmode,
+ uint bitwidth)
+{
+ return hdmi_clrset(state, ADV748X_HDMI_I2S,
+ ADV748X_HDMI_I2SBITWIDTH_MASK |
+ ADV748X_HDMI_I2SOUTMODE_MASK,
+ (outmode << ADV748X_HDMI_I2SOUTMODE_SHIFT) |
+ bitwidth);
+}
+
+static int set_i2s_tdm_mode(struct adv748x_state *state, int is_tdm)
+{
+ int ret;
+
+ ret = hdmi_clrset(state, ADV748X_HDMI_AUDIO_MUTE_SPEED,
+ ADV748X_MAN_AUDIO_DL_BYPASS |
+ ADV748X_AUDIO_DELAY_LINE_BYPASS,
+ is_tdm ? 0xff : 0);
+ if (ret < 0)
+ return ret;
+ ret = hdmi_clrset(state, ADV748X_HDMI_REG_6D,
+ ADV748X_I2S_TDM_MODE_ENABLE,
+ is_tdm ? 0xff : 0);
+ return ret;
+}
+
+static int set_audio_mute(struct adv748x_state *state, int enable)
+{
+ return hdmi_clrset(state, ADV748X_HDMI_MUTE_CTRL,
+ ADV748X_HDMI_MUTE_CTRL_MUTE_AUDIO,
+ enable ? 0xff : 0);
+}
+
+static int adv748x_dai_set_sysclk(struct snd_soc_dai *dai,
+ int clk_id, unsigned int freq, int dir)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ /* currently supporting only one fixed rate clock */
+ if (clk_id != 0 || freq != clk_get_rate(state->dai.mclk)) {
+ dev_err(dai->dev, "invalid clock (%d) or frequency (%u, dir %d)\n",
+ clk_id, freq, dir);
+ return -EINVAL;
+ }
+ state->dai.freq = freq;
+ return 0;
+}
+
+static int adv748x_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+ struct adv748x_state *state = state_of(dai);
+ int ret = 0;
+
+ if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM) {
+ dev_err(dai->dev, "only I2S master clock mode supported\n");
+ ret = -EINVAL;
+ goto done;
+ }
+ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAI_FORMAT_I2S:
+ state->dai.tdm = 0;
+ state->dai.fmt = ADV748X_I2SOUTMODE_I2S;
+ break;
+ case SND_SOC_DAI_FORMAT_RIGHT_J:
+ state->dai.tdm = 1;
+ state->dai.fmt = ADV748X_I2SOUTMODE_RIGHT_J;
+ break;
+ case SND_SOC_DAI_FORMAT_LEFT_J:
+ state->dai.tdm = 1;
+ state->dai.fmt = ADV748X_I2SOUTMODE_LEFT_J;
+ break;
+ default:
+ dev_err(dai->dev, "only i2s, left_j and right_j supported\n");
+ ret = -EINVAL;
+ goto done;
+ }
+ if ((fmt & SND_SOC_DAIFMT_INV_MASK) != SND_SOC_DAIFMT_NB_NF) {
+ dev_err(dai->dev, "only normal bit clock + frame supported\n");
+ ret = -EINVAL;
+ }
+done:
+ return ret;
+}
+
+static int adv748x_dai_startup(struct snd_pcm_substream *sub, struct snd_soc_dai *dai)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ if (sub->stream != SNDRV_PCM_STREAM_CAPTURE)
+ return -EINVAL;
+ return set_audio_pads_state(state, 1);
+}
+
+static int adv748x_dai_hw_params(struct snd_pcm_substream *sub,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ int ret;
+ struct adv748x_state *state = state_of(dai);
+ uint fs = state->dai.freq / params_rate(params);
+
+ dev_dbg(dai->dev, "dai %s substream %s rate=%u (fs=%u), channels=%u sample width=%u(%u)\n",
+ dai->name, sub->name,
+ params_rate(params), fs,
+ params_channels(params),
+ params_width(params),
+ params_physical_width(params));
+ switch (fs) {
+ case 128:
+ case 256:
+ case 384:
+ case 512:
+ case 640:
+ case 768:
+ break;
+ default:
+ ret = -EINVAL;
+ dev_err(dai->dev, "invalid clock frequency (%u) or rate (%u)\n",
+ state->dai.freq, params_rate(params));
+ goto done;
+ }
+ ret = set_dpll_mclk_fs(state, fs);
+ if (ret)
+ goto done;
+ ret = set_i2s_tdm_mode(state, state->dai.tdm);
+ if (ret)
+ goto done;
+ ret = set_i2s_format(state, state->dai.fmt, params_width(params));
+done:
+ return ret;
+}
+
+static int adv748x_dai_mute_stream(struct snd_soc_dai *dai, int mute, int dir)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ return set_audio_mute(state, mute);
+}
+
+static void adv748x_dai_shutdown(struct snd_pcm_substream *sub, struct snd_soc_dai *dai)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ set_audio_pads_state(state, 0);
+}
+
+static const struct snd_soc_dai_ops adv748x_dai_ops = {
+ .set_sysclk = adv748x_dai_set_sysclk,
+ .set_fmt = adv748x_dai_set_fmt,
+ .startup = adv748x_dai_startup,
+ .hw_params = adv748x_dai_hw_params,
+ .mute_stream = adv748x_dai_mute_stream,
+ .shutdown = adv748x_dai_shutdown,
+};
+
+static int adv748x_of_xlate_dai_name(struct snd_soc_component *component,
+ struct of_phandle_args *args,
+ const char **dai_name)
+{
+ if (dai_name)
+ *dai_name = ADV748X_DAI_NAME;
+ return 0;
+}
+
+static const struct snd_soc_component_driver adv748x_codec = {
+ .of_xlate_dai_name = adv748x_of_xlate_dai_name,
+};
+
+int adv748x_dai_init(struct adv748x_dai *dai)
+{
+ int ret;
+ struct adv748x_state *state = adv748x_dai_to_state(dai);
+
+ dai->mclk_name = kasprintf(GFP_KERNEL, "%s.%s-i2s-mclk",
+ state->dev->driver->name,
+ dev_name(state->dev));
+ if (!dai->mclk_name) {
+ ret = -ENOMEM;
+ adv_err(state, "No memory for MCLK\n");
+ goto fail;
+ }
+ dai->mclk = clk_register_fixed_rate(state->dev,
+ dai->mclk_name,
+ NULL /* parent_name */,
+ 0 /* flags */,
+ 12288000 /* rate */);
+ if (IS_ERR_OR_NULL(dai->mclk)) {
+ ret = PTR_ERR(dai->mclk);
+ adv_err(state, "Failed to register MCLK (%d)\n", ret);
+ goto fail;
+ }
+ ret = of_clk_add_provider(state->dev->of_node, of_clk_src_simple_get,
+ dai->mclk);
+ if (ret < 0) {
+ adv_err(state, "Failed to add MCLK provider (%d)\n", ret);
+ goto unreg_mclk;
+ }
+ dai->drv.name = ADV748X_DAI_NAME;
+ dai->drv.ops = &adv748x_dai_ops;
+ dai->drv.capture = (struct snd_soc_pcm_stream){
+ .stream_name = "Capture",
+ .channels_min = 8,
+ .channels_max = 8,
+ .rates = SNDRV_PCM_RATE_48000,
+ .formats = SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE,
+ };
+
+ ret = devm_snd_soc_register_component(state->dev, &adv748x_codec,
+ &dai->drv, 1);
+ if (ret < 0) {
+ adv_err(state, "Failed to register the codec (%d)\n", ret);
+ goto cleanup_mclk;
+ }
+ return 0;
+
+cleanup_mclk:
+ of_clk_del_provider(state->dev->of_node);
+unreg_mclk:
+ clk_unregister_fixed_rate(dai->mclk);
+fail:
+ return ret;
+}
+
+void adv748x_dai_cleanup(struct adv748x_dai *dai)
+{
+ struct adv748x_state *state = adv748x_dai_to_state(dai);
+
+ of_clk_del_provider(state->dev->of_node);
+ clk_unregister_fixed_rate(dai->mclk);
+ kfree(dai->mclk_name);
+}
+
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index 1a1ea70086c6..b0735de0ced5 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -19,6 +19,7 @@
*/
#include <linux/i2c.h>
+#include <sound/soc.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
@@ -63,7 +64,8 @@ enum adv748x_ports {
ADV748X_PORT_TTL = 9,
ADV748X_PORT_TXA = 10,
ADV748X_PORT_TXB = 11,
- ADV748X_PORT_MAX = 12,
+ ADV748X_PORT_I2S = 12,
+ ADV748X_PORT_MAX = 13,
};
enum adv748x_csi2_pads {
@@ -166,6 +168,13 @@ struct adv748x_afe {
container_of(ctrl->handler, struct adv748x_afe, ctrl_hdl)
#define adv748x_sd_to_afe(sd) container_of(sd, struct adv748x_afe, sd)
+struct adv748x_dai {
+ struct snd_soc_dai_driver drv;
+ struct clk *mclk;
+ char *mclk_name;
+ unsigned int freq, fmt, tdm;
+};
+
/**
* struct adv748x_state - State of ADV748X
* @dev: (OF) device
@@ -182,6 +191,7 @@ struct adv748x_afe {
* @afe: state of AFE receiver context
* @txa: state of TXA transmitter context
* @txb: state of TXB transmitter context
+ * @mclk: MCLK clock of the I2S port
*/
struct adv748x_state {
struct device *dev;
@@ -197,10 +207,12 @@ struct adv748x_state {
struct adv748x_afe afe;
struct adv748x_csi2 txa;
struct adv748x_csi2 txb;
+ struct adv748x_dai dai;
};
#define adv748x_hdmi_to_state(h) container_of(h, struct adv748x_state, hdmi)
#define adv748x_afe_to_state(a) container_of(a, struct adv748x_state, afe)
+#define adv748x_dai_to_state(p) container_of(p, struct adv748x_state, dai)
#define adv_err(a, fmt, arg...) dev_err(a->dev, fmt, ##arg)
#define adv_info(a, fmt, arg...) dev_info(a->dev, fmt, ##arg)
@@ -484,4 +496,7 @@ int adv748x_csi2_set_pixelrate(struct v4l2_subdev *sd, s64 rate);
int adv748x_hdmi_init(struct adv748x_hdmi *hdmi);
void adv748x_hdmi_cleanup(struct adv748x_hdmi *hdmi);
+int adv748x_dai_init(struct adv748x_dai *);
+void adv748x_dai_cleanup(struct adv748x_dai *);
+
#endif /* _ADV748X_H_ */
--
2.25.1.25.g9ecbe7eb18
As there is nothing else (the consumers are supposed to do that) which
enables the clock, do it in the driver.
Signed-off-by: Alexander Riesen <[email protected]>
--
v3: added
---
drivers/media/i2c/adv748x/adv748x-dai.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c b/drivers/media/i2c/adv748x/adv748x-dai.c
index 6fce7d000423..1c673efd4745 100644
--- a/drivers/media/i2c/adv748x/adv748x-dai.c
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -117,11 +117,22 @@ static int adv748x_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
static int adv748x_dai_startup(struct snd_pcm_substream *sub, struct snd_soc_dai *dai)
{
+ int ret;
struct adv748x_state *state = state_of(dai);
if (sub->stream != SNDRV_PCM_STREAM_CAPTURE)
return -EINVAL;
- return set_audio_pads_state(state, 1);
+ ret = set_audio_pads_state(state, 1);
+ if (ret)
+ goto fail;
+ ret = clk_prepare_enable(state->dai.mclk);
+ if (ret)
+ goto fail_pwdn;
+ return 0;
+fail_pwdn:
+ set_audio_pads_state(state, 0);
+fail:
+ return ret;
}
static int adv748x_dai_hw_params(struct snd_pcm_substream *sub,
@@ -174,6 +185,7 @@ static void adv748x_dai_shutdown(struct snd_pcm_substream *sub, struct snd_soc_d
{
struct adv748x_state *state = state_of(dai);
+ clk_disable_unprepare(state->dai.mclk);
set_audio_pads_state(state, 0);
}
--
2.25.1.25.g9ecbe7eb18
To avoid setting it up even if the hardware is not actually connected
to anything physically.
Besides, the bindings explicitly notes that port definitions are
"optional if they are not connected to anything at the hardware level".
Signed-off-by: Alexander Riesen <[email protected]>
---
drivers/media/i2c/adv748x/adv748x-dai.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c b/drivers/media/i2c/adv748x/adv748x-dai.c
index 1c673efd4745..f22566094568 100644
--- a/drivers/media/i2c/adv748x/adv748x-dai.c
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -216,6 +216,11 @@ int adv748x_dai_init(struct adv748x_dai *dai)
int ret;
struct adv748x_state *state = adv748x_dai_to_state(dai);
+ if (!state->endpoints[ADV748X_PORT_I2S]) {
+ adv_info(state, "no I2S port, DAI disabled\n");
+ ret = 0;
+ goto fail;
+ }
dai->mclk_name = kasprintf(GFP_KERNEL, "%s.%s-i2s-mclk",
state->dev->driver->name,
dev_name(state->dev));
--
2.25.1.25.g9ecbe7eb18
As the driver has some support for the audio interface of the device,
the bindings file should mention it.
Signed-off-by: Alexander Riesen <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
--
v3: remove optionality off MCLK clock cell to ensure the description
matches the hardware no matter if the line is connected.
Suggested-by: Geert Uytterhoeven <[email protected]>
---
.../devicetree/bindings/media/i2c/adv748x.txt | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/adv748x.txt b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
index 4f91686e54a6..50a753189b81 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv748x.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
@@ -2,7 +2,9 @@
The ADV7481 and ADV7482 are multi format video decoders with an integrated
HDMI receiver. They can output CSI-2 on two independent outputs TXA and TXB
-from three input sources HDMI, analog and TTL.
+from three input sources HDMI, analog and TTL. There is also support for an
+I2S-compatible interface connected to the audio processor of the HDMI decoder.
+The interface has TDM capability (8 slots, 32 bits, left or right justified).
Required Properties:
@@ -16,6 +18,8 @@ Required Properties:
slave device on the I2C bus. The main address is mandatory, others are
optional and remain at default values if not specified.
+ - #clock-cells: must be <0>
+
Optional Properties:
- interrupt-names: Should specify the interrupts as "intrq1", "intrq2" and/or
@@ -47,6 +51,7 @@ are numbered as follows.
TTL sink 9
TXA source 10
TXB source 11
+ I2S source 12
The digital output port nodes, when present, shall contain at least one
endpoint. Each of those endpoints shall contain the data-lanes property as
@@ -72,6 +77,7 @@ Example:
#address-cells = <1>;
#size-cells = <0>;
+ #clock-cells = <0>;
interrupt-parent = <&gpio6>;
interrupt-names = "intrq1", "intrq2";
@@ -113,4 +119,12 @@ Example:
remote-endpoint = <&csi20_in>;
};
};
+
+ port@c {
+ reg = <12>;
+
+ adv7482_i2s: endpoint {
+ remote-endpoint = <&i2s_in>;
+ };
+ };
};
--
2.25.1.25.g9ecbe7eb18
As all known variants of the Salvator board have the HDMI decoder
chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
endpoint and the connection definitions are placed in the common board
file.
For the same reason, the CLK_C clock line and I2C configuration (similar
to the ak4613, on the same interface) are added into the common file.
Signed-off-by: Alexander Riesen <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
--
v2:
Also add ssi4_ctrl pin group in the sound pins. The pins are
responsible for SCK4 (sample clock) WS4 and (word boundary input),
and are required for SSI audio input over I2S.
Suggested-by: Geert Uytterhoeven <[email protected]>
The adv748x shall provide its own implementation of the output clock
(MCLK), connected to the audio_clk_c line of the R-Car SoC.
If the frequency of the ADV748x MCLK were fixed, the clock
implementation were not necessary, but it does not seem so: the MCLK
depends on the value in a speed multiplier register and the input sample
rate (48kHz).
Suggested-by: Geert Uytterhoeven <[email protected]>
Remove audio clock C from the clocks of adv7482.
The clocks property of the video-receiver node lists the input
clocks of the device, which is quite the opposite from the
original intention: the adv7482 on Salvator X boards is a
provide of the MCLK clock for I2S audio output.
Suggested-by: Geert Uytterhoeven <[email protected]>
Remove old definition of &sound_card.dais and reduce size of changes
in the Salvator-X specific device tree source.
Declare video-receiver a clock producer, as the adv748x driver
implements the master clock used I2S audio output.
Suggested-by: Geert Uytterhoeven <[email protected]>
The driver provides only MCLK clock, not the SCLK and LRCLK,
which are part of the I2S protocol.
Reviewed-by: Laurent Pinchart <[email protected]>
---
.../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +-
.../boot/dts/renesas/salvator-common.dtsi | 47 +++++++++++++++++--
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
index 2438825c9b22..e16c146808b6 100644
--- a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
@@ -146,7 +146,8 @@ &sata {
&sound_card {
dais = <&rsnd_port0 /* ak4613 */
&rsnd_port1 /* HDMI0 */
- &rsnd_port2>; /* HDMI1 */
+ &rsnd_port2 /* HDMI1 */
+ &rsnd_port3>; /* adv7482 hdmi-in */
};
&usb2_phy2 {
diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 98bbcafc8c0d..ead7f8d7a929 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -460,7 +460,7 @@ pca9654: gpio@20 {
#gpio-cells = <2>;
};
- video-receiver@70 {
+ adv7482_hdmi_in: video-receiver@70 {
compatible = "adi,adv7482";
reg = <0x70 0x71 0x72 0x73 0x74 0x75
0x60 0x61 0x62 0x63 0x64 0x65>;
@@ -469,6 +469,7 @@ video-receiver@70 {
#address-cells = <1>;
#size-cells = <0>;
+ #clock-cells = <0>; /* the MCLK for I2S output */
interrupt-parent = <&gpio6>;
interrupt-names = "intrq1", "intrq2";
@@ -510,6 +511,15 @@ adv7482_txb: endpoint {
remote-endpoint = <&csi20_in>;
};
};
+
+ port@c {
+ reg = <12>;
+
+ adv7482_i2s: endpoint {
+ remote-endpoint = <&rsnd_endpoint3>;
+ system-clock-direction-out;
+ };
+ };
};
csa_vdd: adc@7c {
@@ -684,7 +694,8 @@ sdhi3_pins_uhs: sd3_uhs {
};
sound_pins: sound {
- groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a";
+ groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a",
+ "ssi4_data", "ssi4_ctrl";
function = "ssi";
};
@@ -733,8 +744,8 @@ &rcar_sound {
pinctrl-0 = <&sound_pins &sound_clk_pins>;
pinctrl-names = "default";
- /* Single DAI */
- #sound-dai-cells = <0>;
+ /* multi DAI */
+ #sound-dai-cells = <1>;
/* audio_clkout0/1/2/3 */
#clock-cells = <1>;
@@ -758,8 +769,19 @@ &rcar_sound {
<&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
<&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
<&audio_clk_a>, <&cs2000>,
- <&audio_clk_c>,
+ <&adv7482_hdmi_in>,
<&cpg CPG_CORE CPG_AUDIO_CLK_I>;
+ clock-names = "ssi-all",
+ "ssi.9", "ssi.8", "ssi.7", "ssi.6",
+ "ssi.5", "ssi.4", "ssi.3", "ssi.2",
+ "ssi.1", "ssi.0",
+ "src.9", "src.8", "src.7", "src.6",
+ "src.5", "src.4", "src.3", "src.2",
+ "src.1", "src.0",
+ "mix.1", "mix.0",
+ "ctu.1", "ctu.0",
+ "dvc.0", "dvc.1",
+ "clk_a", "clk_b", "clk_c", "clk_i";
ports {
#address-cells = <1>;
@@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint {
capture = <&ssi1 &src1 &dvc1>;
};
};
+ rsnd_port3: port@3 {
+ reg = <3>;
+ rsnd_endpoint3: endpoint {
+ remote-endpoint = <&adv7482_i2s>;
+
+ dai-tdm-slot-num = <8>;
+ dai-tdm-slot-width = <32>;
+ dai-format = "left_j";
+ mclk-fs = <256>;
+ bitclock-master = <&adv7482_i2s>;
+ frame-master = <&adv7482_i2s>;
+
+ capture = <&ssi4>;
+ };
+ };
};
};
--
2.25.1.25.g9ecbe7eb18
The logged information provides insights about cable connection and the
state of the HDMI decoder. It is very useful when debugging hardware
problems in environments without easy access to the connectors.
This change adds a device-specific wrapper for register block read,
because some of the devices I2C-accessible registers (for instance,
cs_data for stereo channel information or tmds_params for TMDS channel
information) located in adjacent cells. According to manufacturers
information, these registers can be read using block transactions.
Signed-off-by: Alexander Riesen <[email protected]>
---
drivers/media/i2c/adv748x/adv748x-core.c | 15 ++
drivers/media/i2c/adv748x/adv748x-hdmi.c | 179 +++++++++++++++++++++++
drivers/media/i2c/adv748x/adv748x.h | 2 +
3 files changed, 196 insertions(+)
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 3513ca138e53..775450ab2bad 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -95,6 +95,21 @@ static const struct adv748x_register_map adv748x_default_addresses[] = {
[ADV748X_PAGE_TXA] = { "txa", 0x4a },
};
+int adv748x_read_block(struct adv748x_state *state, u8 client_page, u8 reg,
+ void *val, size_t reg_count)
+{
+ struct i2c_client *client = state->i2c_clients[client_page];
+ int err;
+
+ err = regmap_bulk_read(state->regmap[client_page], reg, val, reg_count);
+ if (err) {
+ adv_err(state, "error reading %02x, %02x-%02lx: %d\n",
+ client->addr, reg, reg + reg_count - 1, err);
+ return err;
+ }
+ return 0;
+}
+
static int adv748x_read_check(struct adv748x_state *state,
int client_page, u8 reg)
{
diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c
index f598acec3b5c..664930e55baf 100644
--- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
+++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
@@ -7,6 +7,7 @@
#include "adv748x.h"
+#include <linux/version.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -601,11 +602,189 @@ static const struct v4l2_subdev_pad_ops adv748x_pad_ops_hdmi = {
.enum_dv_timings = adv748x_hdmi_enum_dv_timings,
};
+struct tmds_params {
+ u32 cts, n;
+ u16 tmdsfreq, tmdsfreq_frac;
+};
+
+static inline const char *cs_data_smpl_freq_str(u8 cs_data_3)
+{
+ switch (cs_data_3 & 0xf) {
+ case 0:
+ return "44.1";
+ case 2:
+ return "48";
+ case 3:
+ return "32";
+ case 8:
+ return "88.2";
+ case 10:
+ return "96";
+ case 12:
+ return "176";
+ case 14:
+ return "192";
+ }
+ return "reserved";
+}
+
+static inline const char *cs_data_clk_lvl_str(u8 cs_data_3)
+{
+ switch (cs_data_3 & 0x30) {
+ case 0:
+ return "Level II";
+ case 1:
+ return "Level I";
+ case 2:
+ return "Level III, variable pitch shifted";
+ }
+ return "reserved";
+}
+
+static inline const char *i2s_out_mode_str(u8 i2s_mode)
+{
+ switch (i2s_mode & ADV748X_HDMI_I2SOUTMODE_MASK) {
+ case 0 << ADV748X_HDMI_I2SOUTMODE_SHIFT:
+ return "I2S";
+ case 1 << ADV748X_HDMI_I2SOUTMODE_SHIFT:
+ return "right";
+ case 2 << ADV748X_HDMI_I2SOUTMODE_SHIFT:
+ return "left";
+ case 3 << ADV748X_HDMI_I2SOUTMODE_SHIFT:
+ return "spdif";
+ }
+ return "";
+}
+
+static int adv748x_hdmi_log_status(struct v4l2_subdev *sd)
+{
+ struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
+ struct adv748x_state *state = adv748x_hdmi_to_state(hdmi);
+ u8 rv, i2s_tdm_mode_enable;
+ u8 cts_n[5];
+ u8 cs_data[0x3a - 0x36 + 1];
+ u8 tmdsfreq[2]; /* both tmdsfreq and tmdsfreq_frac */
+ struct tmds_params tmds_params;
+
+ /* Audio control and configuration */
+ rv = io_read(state, 0x71);
+ pr_info("cable_det_a_raw %s\n",
+ rv & BIT(6) ? "detected" : "no cable");
+ pr_info("tmds_clk_a_raw %s\n",
+ rv & BIT(3) ? "detected" : "no TMDS clock");
+ pr_info("tmdspll_lck_a_raw %s\n",
+ rv & BIT(7) ? "locked to incoming clock" : "not locked");
+ pr_info("hdmi_encrpt_a_raw %s\n",
+ rv & BIT(5) ? "current frame encrypted" : "not encrypted");
+ rv = hdmi_read(state, 0x04);
+ pr_info("audio_pll_locked 0x%02lx\n", rv & BIT(0));
+ pr_info("tmds_pll_locked 0x%02lx\n", rv & BIT(1));
+ rv = io_read(state, 0x6c);
+ pr_info("gamut_mdata_raw %s\n",
+ rv & BIT(0) ? "received" : "-");
+ pr_info("audio_c_pckt_raw %s\n",
+ rv & BIT(1) ? "ACR received" : "-");
+ pr_info("gen_ctl_pckt_raw %s\n",
+ rv & BIT(2) ? "received" : "-");
+ pr_info("hdmi_mode_raw %s\n",
+ rv & BIT(3) ? "HDMI/MHL" : "-");
+ pr_info("audio_ch_md_raw %s\n",
+ rv & BIT(4) ? "multichannel" : "-");
+ pr_info("av_mute_raw %s\n",
+ rv & BIT(5) ? "received" : "-");
+ pr_info("internal_mute_raw %s\n",
+ rv & BIT(6) ? "asserted" : "-");
+ pr_info("cs_data_valid_raw %s\n",
+ rv & BIT(7) ? "valid" : "-");
+ rv = hdmi_read(state, 0x6d);
+ pr_info("i2s_tdm_mode_enable %s\n",
+ rv & BIT(7) ? "TDM (multichannel)" : "I2S (stereo)");
+ i2s_tdm_mode_enable = rv & BIT(7);
+
+ /* i2s_tdm_mode_enable must be unset */
+ if (adv748x_read_block(state, ADV748X_PAGE_HDMI, 0x36,
+ cs_data, ARRAY_SIZE(cs_data)) == 0) {
+ pr_info("... cs_data %s\n",
+ cs_data[0] & BIT(0) ? "pro" : "consumer");
+ pr_info("... cs_data %s\n",
+ cs_data[0] & BIT(1) ? "other" : "L-PCM");
+ pr_info("... cs_data %s copyright\n",
+ cs_data[0] & BIT(2) ? "no" : "asserted");
+ pr_info("... cs_data %s (%lu)\n",
+ cs_data[0] & GENMASK(5, 3) ?
+ "50/15" : "no pre-emphasis",
+ (cs_data[0] & GENMASK(5, 3)) >> 4);
+ pr_info("... cs_data channels status mode %lu\n",
+ (cs_data[0] & GENMASK(7, 6)) >> 7);
+ pr_info("... cs_data category code 0x%02x\n", cs_data[1]);
+ pr_info("... cs_data source number %u\n", cs_data[2] & 0xf);
+ pr_info("... cs_data channel number %u\n",
+ (cs_data[2] & 0xf0) >> 4);
+ pr_info("... cs_data sampling frequency %s (%u)\n",
+ cs_data_smpl_freq_str(cs_data[3]), cs_data[3] & 0xf);
+ pr_info("... cs_data clock accuracy %s\n",
+ cs_data_clk_lvl_str(cs_data[3]));
+ }
+ rv = hdmi_read(state, ADV748X_HDMI_I2S);
+ pr_info("i2soutmode %s\n", i2s_out_mode_str(rv));
+ pr_info("i2sbitwidth %u\n", rv & 0x1fu);
+ rv = hdmi_read(state, 0x05);
+ pr_info("hdmi_mode %s\n", rv & BIT(7) ? "HDMI" : "DVI");
+ rv = hdmi_read(state, 0x07);
+ pr_info("audio_channel_mode %s\n",
+ rv & BIT(6) ? "multichannel" : "stereo or compressed");
+ rv = hdmi_read(state, 0x0f);
+ /* The bits 6 and 7 must be 1 if TDM mode */
+ pr_info("man_audio_dl_bypass 0x%02lx\n", rv & BIT(7));
+ pr_info("audio_delay_line_bypass 0x%02lx\n", rv & BIT(6));
+ rv = hdmi_read(state, 0x6e);
+ pr_info("mux_spdif_to_i2s_enable %s\n", rv & BIT(3) ? "SPDIF" : "I2S");
+ rv = dpll_read(state, ADV748X_DPLL_MCLK_FS);
+ pr_info("mclk_fs_n %lu\n",
+ ((rv & ADV748X_DPLL_MCLK_FS_N_MASK) + 1) * 128);
+
+ /* i2s_tdm_mode_enable must be set */
+ memset(&tmds_params, 0, sizeof(tmds_params));
+ if (adv748x_read_block(state, ADV748X_PAGE_HDMI, 0x5b, cts_n, 5) == 0) {
+ tmds_params.cts = cts_n[0] << 12;
+ tmds_params.cts |= cts_n[1] << 4;
+ tmds_params.cts |= cts_n[2] >> 4;
+ tmds_params.n = (cts_n[2] & 0xf) << 16;
+ tmds_params.n |= cts_n[3] << 8;
+ tmds_params.n |= cts_n[4];
+ pr_info("... TDM: ACR cts %u\n", tmds_params.cts);
+ pr_info("... TDM: ACR n %u\n", tmds_params.n);
+ }
+ if (adv748x_read_block(state, ADV748X_PAGE_HDMI, 0x51,
+ tmdsfreq, 2) == 0) {
+ tmds_params.tmdsfreq = tmdsfreq[0] << 1;
+ tmds_params.tmdsfreq |= tmdsfreq[1] >> 7;
+ tmds_params.tmdsfreq_frac = tmdsfreq[1] & 0x7f;
+ pr_info("... TDM: tmdsfreq %d MHz\n",
+ tmds_params.tmdsfreq);
+ pr_info("... TDM: tmdsfreq_frac %d 1/128\n",
+ tmds_params.tmdsfreq_frac);
+ }
+ if (i2s_tdm_mode_enable)
+ pr_info("... TDM: sampling frequency %u Hz\n",
+ tmds_params.cts ?
+ (tmds_params.tmdsfreq * tmds_params.n +
+ tmds_params.tmdsfreq_frac * tmds_params.n / 128) *
+ 1000 / (128 * tmds_params.cts / 1000) :
+ UINT_MAX);
+ return 0;
+}
+
+static const struct v4l2_subdev_core_ops adv748x_core_ops_hdmi = {
+ .log_status = adv748x_hdmi_log_status,
+};
+
/* -----------------------------------------------------------------------------
* v4l2_subdev_ops
*/
static const struct v4l2_subdev_ops adv748x_ops_hdmi = {
+ .core = &adv748x_core_ops_hdmi,
.video = &adv748x_video_ops_hdmi,
.pad = &adv748x_pad_ops_hdmi,
};
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index b0735de0ced5..6421a4c02c60 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -433,6 +433,8 @@ struct adv748x_state {
/* Register handling */
int adv748x_read(struct adv748x_state *state, u8 addr, u8 reg);
+int adv748x_read_block(struct adv748x_state *state, u8 page, u8 reg,
+ void *val, size_t reg_count);
int adv748x_write(struct adv748x_state *state, u8 page, u8 reg, u8 value);
int adv748x_write_block(struct adv748x_state *state, int client_page,
unsigned int init_reg, const void *val,
--
2.25.1.25.g9ecbe7eb18
This makes it possible to load a EDID reported by the device
with v4l2-ctl utility:
vdev=/dev/$(grep -l '^adv748x.*hdmi$' /sys/class/video4linux/v4l-subdev*/name |cut -d/ -f5-5)
v4l2-ctl -d $vdev --set-edid=pad=0,file=/etc/adv7482.edid
Signed-off-by: Alexander Riesen <[email protected]>
--
I would like to avoid doing that, but found no other way yet.
Suggestions very welcome.
---
drivers/media/i2c/adv748x/adv748x-hdmi.c | 27 ++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c
index 664930e55baf..fea133472911 100644
--- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
+++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
@@ -775,7 +775,34 @@ static int adv748x_hdmi_log_status(struct v4l2_subdev *sd)
return 0;
}
+static long adv748x_hdmi_querycap(struct adv748x_hdmi *hdmi,
+ struct v4l2_capability *cap)
+{
+ struct adv748x_state *state = adv748x_hdmi_to_state(hdmi);
+
+ cap->version = LINUX_VERSION_CODE;
+ strlcpy(cap->driver, state->dev->driver->name, sizeof(cap->driver));
+ strlcpy(cap->card, "hdmi", sizeof(cap->card));
+ snprintf(cap->bus_info, sizeof(cap->bus_info), "i2c:%d-%04x",
+ i2c_adapter_id(state->client->adapter),
+ state->client->addr);
+ cap->device_caps = V4L2_CAP_AUDIO | V4L2_CAP_VIDEO_CAPTURE;
+ cap->capabilities = V4L2_CAP_DEVICE_CAPS;
+ return 0;
+}
+
+static long adv748x_hdmi_ioctl(struct v4l2_subdev *sd,
+ unsigned int cmd, void *arg)
+{
+ struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
+
+ if (cmd == VIDIOC_QUERYCAP)
+ return adv748x_hdmi_querycap(hdmi, arg);
+ return -ENOTTY;
+}
+
static const struct v4l2_subdev_core_ops adv748x_core_ops_hdmi = {
+ .ioctl = adv748x_hdmi_ioctl,
.log_status = adv748x_hdmi_log_status,
};
--
2.25.1.25.g9ecbe7eb18
Quoting Alex Riesen (2020-03-20 09:12:00)
> diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c b/drivers/media/i2c/adv748x/adv748x-dai.c
> new file mode 100644
> index 000000000000..6fce7d000423
> --- /dev/null
> +++ b/drivers/media/i2c/adv748x/adv748x-dai.c
> @@ -0,0 +1,265 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for Analog Devices ADV748X HDMI receiver with AFE
> + * The implementation of DAI.
> + */
> +
> +#include "adv748x.h"
> +
> +#include <linux/clk.h>
Is this include used? Please try to make a clk provider or a clk
consumer and not both unless necessary.
> +#include <linux/clk-provider.h>
> +#include <sound/pcm_params.h>
> +
> +#define state_of(soc_dai) \
> + adv748x_dai_to_state(container_of((soc_dai)->driver, \
> + struct adv748x_dai, \
> + drv))
> +
> +static const char ADV748X_DAI_NAME[] = "adv748x-i2s";
> +
[...]
> + .set_sysclk = adv748x_dai_set_sysclk,
> + .set_fmt = adv748x_dai_set_fmt,
> + .startup = adv748x_dai_startup,
> + .hw_params = adv748x_dai_hw_params,
> + .mute_stream = adv748x_dai_mute_stream,
> + .shutdown = adv748x_dai_shutdown,
> +};
> +
> +static int adv748x_of_xlate_dai_name(struct snd_soc_component *component,
> + struct of_phandle_args *args,
> + const char **dai_name)
> +{
> + if (dai_name)
> + *dai_name = ADV748X_DAI_NAME;
> + return 0;
> +}
> +
> +static const struct snd_soc_component_driver adv748x_codec = {
> + .of_xlate_dai_name = adv748x_of_xlate_dai_name,
> +};
> +
> +int adv748x_dai_init(struct adv748x_dai *dai)
> +{
> + int ret;
> + struct adv748x_state *state = adv748x_dai_to_state(dai);
> +
> + dai->mclk_name = kasprintf(GFP_KERNEL, "%s.%s-i2s-mclk",
> + state->dev->driver->name,
> + dev_name(state->dev));
> + if (!dai->mclk_name) {
> + ret = -ENOMEM;
> + adv_err(state, "No memory for MCLK\n");
> + goto fail;
> + }
> + dai->mclk = clk_register_fixed_rate(state->dev,
Please register with clk_hw_register_fixed_rate() instead.
> + dai->mclk_name,
> + NULL /* parent_name */,
> + 0 /* flags */,
> + 12288000 /* rate */);
Not sure these comments are useful.
> + if (IS_ERR_OR_NULL(dai->mclk)) {
> + ret = PTR_ERR(dai->mclk);
> + adv_err(state, "Failed to register MCLK (%d)\n", ret);
> + goto fail;
> + }
> + ret = of_clk_add_provider(state->dev->of_node, of_clk_src_simple_get,
> + dai->mclk);
> + if (ret < 0) {
> + adv_err(state, "Failed to add MCLK provider (%d)\n", ret);
> + goto unreg_mclk;
> + }
> + dai->drv.name = ADV748X_DAI_NAME;
> + dai->drv.ops = &adv748x_dai_ops;
> + dai->drv.capture = (struct snd_soc_pcm_stream){
Can this be const?
> + .stream_name = "Capture",
> + .channels_min = 8,
> + .channels_max = 8,
> + .rates = SNDRV_PCM_RATE_48000,
> + .formats = SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE,
> + };
> +
> + ret = devm_snd_soc_register_component(state->dev, &adv748x_codec,
> + &dai->drv, 1);
> + if (ret < 0) {
> + adv_err(state, "Failed to register the codec (%d)\n", ret);
> + goto cleanup_mclk;
> + }
> + return 0;
> +
> +cleanup_mclk:
> + of_clk_del_provider(state->dev->of_node);
> +unreg_mclk:
> + clk_unregister_fixed_rate(dai->mclk);
> +fail:
> + return ret;
> +}
> +
> +void adv748x_dai_cleanup(struct adv748x_dai *dai)
> +{
> + struct adv748x_state *state = adv748x_dai_to_state(dai);
> +
> + of_clk_del_provider(state->dev->of_node);
> + clk_unregister_fixed_rate(dai->mclk);
> + kfree(dai->mclk_name);
> +}
> +
Please drop extra newline at end of file.
Hi Alex,
On Fri, Mar 20, 2020 at 5:43 PM Alex Riesen
<[email protected]> wrote:
> As all known variants of the Salvator board have the HDMI decoder
> chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
> endpoint and the connection definitions are placed in the common board
> file.
> For the same reason, the CLK_C clock line and I2C configuration (similar
> to the ak4613, on the same interface) are added into the common file.
>
> Signed-off-by: Alexander Riesen <[email protected]>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
Did I provide a Reviewed-by?
> The driver provides only MCLK clock, not the SCLK and LRCLK,
> which are part of the I2S protocol.
>
> Reviewed-by: Laurent Pinchart <[email protected]>
Perhaps you mixed it up with Laurent's?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Stephen Boyd, Sat, Mar 21, 2020 02:09:31 +0100:
> Quoting Alex Riesen (2020-03-20 09:12:00)
> > diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c b/drivers/media/i2c/adv748x/adv748x-dai.c
> > new file mode 100644
> > index 000000000000..6fce7d000423
> > --- /dev/null
> > +++ b/drivers/media/i2c/adv748x/adv748x-dai.c
> > @@ -0,0 +1,265 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Driver for Analog Devices ADV748X HDMI receiver with AFE
> > + * The implementation of DAI.
> > + */
> > +
> > +#include "adv748x.h"
> > +
> > +#include <linux/clk.h>
>
> Is this include used? Please try to make a clk provider or a clk
> consumer and not both unless necessary.
Yes, they're both used: I use clk_get_rate to get the frequency of the clock to validate
the master clock frequency configuration.
> > +int adv748x_dai_init(struct adv748x_dai *dai)
> > +{
> > + int ret;
> > + struct adv748x_state *state = adv748x_dai_to_state(dai);
> > +
> > + dai->mclk_name = kasprintf(GFP_KERNEL, "%s.%s-i2s-mclk",
> > + state->dev->driver->name,
> > + dev_name(state->dev));
> > + if (!dai->mclk_name) {
> > + ret = -ENOMEM;
> > + adv_err(state, "No memory for MCLK\n");
> > + goto fail;
> > + }
> > + dai->mclk = clk_register_fixed_rate(state->dev,
>
> Please register with clk_hw_register_fixed_rate() instead.
Ok, I see the clk_register_fixed_rate isn't even commented...
I shall change it to used clk_hw_register_fixed_rate.
But could you please point me at some documentation, example, or just add a
few words to explain the preference?
BTW, the clock which the driver attempts to provide can actually be switched
on and off (it output pad can be even tristated). As the device is connected
by I2C to the how, I seems to be unable to use the existing gated clock
implementation because of the mmio register it requires. Or am I wrong?
> > + dai->mclk_name,
> > + NULL /* parent_name */,
> > + 0 /* flags */,
> > + 12288000 /* rate */);
>
> Not sure these comments are useful.
You're right. Removed.
> > + dai->drv.name = ADV748X_DAI_NAME;
> > + dai->drv.ops = &adv748x_dai_ops;
> > + dai->drv.capture = (struct snd_soc_pcm_stream){
>
> Can this be const?
It can. Fixed.
> Please drop extra newline at end of file.
Done.
Thanks a lot for the review and commentary!
Regards,
Alex
Geert Uytterhoeven, Mon, Mar 23, 2020 09:34:45 +0100:
> On Fri, Mar 20, 2020 at 5:43 PM Alex Riesen <[email protected]> wrote:
> > As all known variants of the Salvator board have the HDMI decoder
> > chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
> > endpoint and the connection definitions are placed in the common board
> > file.
> > For the same reason, the CLK_C clock line and I2C configuration (similar
> > to the ak4613, on the same interface) are added into the common file.
> >
> > Signed-off-by: Alexander Riesen <[email protected]>
> > Reviewed-by: Geert Uytterhoeven <[email protected]>
>
> Did I provide a Reviewed-by?
>
> > The driver provides only MCLK clock, not the SCLK and LRCLK,
> > which are part of the I2S protocol.
> >
> > Reviewed-by: Laurent Pinchart <[email protected]>
>
> Perhaps you mixed it up with Laurent's?
Sorry. I actually did: he did provded Reviewed-by in his email, and you
did not. I was ... a little overwhelmed.
But you really did provide a lot of very useful information and it did help
to improve the code. Shall I remove the tag still?
Regards,
Alex
Alex Riesen, Mon, Mar 23, 2020 09:40:11 +0100:
> Geert Uytterhoeven, Mon, Mar 23, 2020 09:34:45 +0100:
> > On Fri, Mar 20, 2020 at 5:43 PM Alex Riesen <[email protected]> wrote:
> > > As all known variants of the Salvator board have the HDMI decoder
> > > chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
> > > endpoint and the connection definitions are placed in the common board
> > > file.
> > > For the same reason, the CLK_C clock line and I2C configuration (similar
> > > to the ak4613, on the same interface) are added into the common file.
> > >
> > > Signed-off-by: Alexander Riesen <[email protected]>
> > > Reviewed-by: Geert Uytterhoeven <[email protected]>
> >
> > Did I provide a Reviewed-by?
> >
> > > The driver provides only MCLK clock, not the SCLK and LRCLK,
> > > which are part of the I2S protocol.
> > >
> > > Reviewed-by: Laurent Pinchart <[email protected]>
> >
> > Perhaps you mixed it up with Laurent's?
>
> Sorry. I actually did: he did provded Reviewed-by in his email, and you
> did not. I was ... a little overwhelmed.
It's even worse: even Laurent didn't provide his Reviewed-by for this
particular patch (it was bindings). Corrected.
Regards,
Alex
Hi Alex,
On Mon, Mar 23, 2020 at 9:41 AM Alex Riesen
<[email protected]> wrote:
> Geert Uytterhoeven, Mon, Mar 23, 2020 09:34:45 +0100:
> > On Fri, Mar 20, 2020 at 5:43 PM Alex Riesen <[email protected]> wrote:
> > > As all known variants of the Salvator board have the HDMI decoder
> > > chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
> > > endpoint and the connection definitions are placed in the common board
> > > file.
> > > For the same reason, the CLK_C clock line and I2C configuration (similar
> > > to the ak4613, on the same interface) are added into the common file.
> > >
> > > Signed-off-by: Alexander Riesen <[email protected]>
> > > Reviewed-by: Geert Uytterhoeven <[email protected]>
> >
> > Did I provide a Reviewed-by?
> >
> > > The driver provides only MCLK clock, not the SCLK and LRCLK,
> > > which are part of the I2S protocol.
> > >
> > > Reviewed-by: Laurent Pinchart <[email protected]>
> >
> > Perhaps you mixed it up with Laurent's?
>
> Sorry. I actually did: he did provded Reviewed-by in his email, and you
> did not. I was ... a little overwhelmed.
>
> But you really did provide a lot of very useful information and it did help
> to improve the code. Shall I remove the tag still?
Please do so.
While I can point out issues in audio patches, my audio-foo is not strong
enough to provide an R-B, and I'll rely on the R-bs provided by others.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
Geert Uytterhoeven, Mon, Mar 23, 2020 09:48:00 +0100:
> On Mon, Mar 23, 2020 at 9:41 AM Alex Riesen <[email protected]> wrote:
> > Geert Uytterhoeven, Mon, Mar 23, 2020 09:34:45 +0100:
> > > On Fri, Mar 20, 2020 at 5:43 PM Alex Riesen <[email protected]> wrote:
> > > > Reviewed-by: Geert Uytterhoeven <[email protected]>
> > >
> > > Did I provide a Reviewed-by?
> > >
> > But you really did provide a lot of very useful information and it did help
> > to improve the code. Shall I remove the tag still?
>
> Please do so.
>
> While I can point out issues in audio patches, my audio-foo is not strong
> enough to provide an R-B, and I'll rely on the R-bs provided by others.
Done. I left the suggested-bys in the trailer. Hope those are alright.
Regards,
Alex