linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/4] Add HDMI jack support on RK3288
@ 2019-09-19 13:54 Cheng-Yi Chiang
  2019-09-19 13:54 ` [PATCH v7 1/4] drm: bridge: dw-hdmi: Report connector status using callback Cheng-Yi Chiang
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Cheng-Yi Chiang @ 2019-09-19 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Jerome Brunet, Jernej Skrabec, Jonas Karlman,
	Hans Verkuil, Mark Brown, Liam Girdwood, Takashi Iwai,
	Jaroslav Kysela, Russell King, Andrzej Hajda, Laurent Pinchart,
	David Airlie, Daniel Vetter, Heiko Stuebner, dianders, dgreid,
	tzungbi, alsa-devel, dri-devel, linux-arm-kernel, linux-rockchip,
	Cheng-Yi Chiang

This patch series supports HDMI jack reporting on RK3288, which uses
DRM dw-hdmi driver and hdmi-codec codec driver.

The previous discussion about reporting jack status using hdmi-notifier
and drm_audio_component is at

https://lore.kernel.org/patchwork/patch/1083027/

The new approach is to use a callback mechanism that is
specific to hdmi-codec.

The dependent change on hdmi-codec.c

https://patchwork.kernel.org/patch/11047447

has been picked up by Mark Brown in ASoC tree for-5.4 branch.

Changes from v6 to v7:

1. rockchip_max98090: Use phandle of HDMI from DTS to find
   codec_dai. With this we don't need to set fixed id for the
   created hdmi-audio-codec device.

Cheng-Yi Chiang (4):
  drm: bridge: dw-hdmi: Report connector status using callback
  ASoC: rockchip-max98090: Add description for rockchip,hdmi-codec
  ASoC: rockchip_max98090: Add dai_link for HDMI
  ASoC: rockchip_max98090: Add HDMI jack support

 .../bindings/sound/rockchip-max98090.txt      |   2 +
 .../boot/dts/rk3288-veyron-analog-audio.dtsi  |   1 +
 .../drm/bridge/synopsys/dw-hdmi-i2s-audio.c   |  11 ++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     |  41 ++++-
 include/drm/bridge/dw_hdmi.h                  |   4 +
 sound/soc/rockchip/Kconfig                    |   3 +-
 sound/soc/rockchip/rockchip_max98090.c        | 149 ++++++++++++++----
 7 files changed, 182 insertions(+), 29 deletions(-)

-- 
2.23.0.237.gc6a4ce50a0-goog


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

* [PATCH v7 1/4] drm: bridge: dw-hdmi: Report connector status using callback
  2019-09-19 13:54 [PATCH v7 0/4] Add HDMI jack support on RK3288 Cheng-Yi Chiang
@ 2019-09-19 13:54 ` Cheng-Yi Chiang
  2019-09-19 13:54 ` [PATCH v7 2/4] ASoC: rockchip-max98090: Add description for rockchip,hdmi-codec Cheng-Yi Chiang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Cheng-Yi Chiang @ 2019-09-19 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Jerome Brunet, Jernej Skrabec, Jonas Karlman,
	Hans Verkuil, Mark Brown, Liam Girdwood, Takashi Iwai,
	Jaroslav Kysela, Russell King, Andrzej Hajda, Laurent Pinchart,
	David Airlie, Daniel Vetter, Heiko Stuebner, dianders, dgreid,
	tzungbi, alsa-devel, dri-devel, linux-arm-kernel, linux-rockchip,
	Cheng-Yi Chiang

Allow codec driver register callback function for plug event.

The callback registration flow:
dw-hdmi <--- hw-hdmi-i2s-audio <--- hdmi-codec

dw-hdmi-i2s-audio implements hook_plugged_cb op
so codec driver can register the callback.

dw-hdmi exports a function dw_hdmi_set_plugged_cb so platform device
can register the callback.

When connector plug/unplug event happens, report this event using the
callback.

Make sure that audio and drm are using the single source of truth for
connector status.

Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
---
 .../drm/bridge/synopsys/dw-hdmi-i2s-audio.c   | 11 +++++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     | 41 ++++++++++++++++++-
 include/drm/bridge/dw_hdmi.h                  |  4 ++
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
index 20f4f92dd866..d7e65c869415 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
@@ -160,12 +160,23 @@ static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
 	return -EINVAL;
 }
 
+static int dw_hdmi_i2s_hook_plugged_cb(struct device *dev, void *data,
+				       hdmi_codec_plugged_cb fn,
+				       struct device *codec_dev)
+{
+	struct dw_hdmi_i2s_audio_data *audio = data;
+	struct dw_hdmi *hdmi = audio->hdmi;
+
+	return dw_hdmi_set_plugged_cb(hdmi, fn, codec_dev);
+}
+
 static struct hdmi_codec_ops dw_hdmi_i2s_ops = {
 	.hw_params	= dw_hdmi_i2s_hw_params,
 	.audio_startup  = dw_hdmi_i2s_audio_startup,
 	.audio_shutdown	= dw_hdmi_i2s_audio_shutdown,
 	.get_eld	= dw_hdmi_i2s_get_eld,
 	.get_dai_id	= dw_hdmi_i2s_get_dai_id,
+	.hook_plugged_cb = dw_hdmi_i2s_hook_plugged_cb,
 };
 
 static int snd_dw_hdmi_probe(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index aa7efd4da1c8..7ffe8ed675ff 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -193,6 +193,10 @@ struct dw_hdmi {
 
 	struct mutex cec_notifier_mutex;
 	struct cec_notifier *cec_notifier;
+
+	hdmi_codec_plugged_cb plugged_cb;
+	struct device *codec_dev;
+	enum drm_connector_status last_connector_result;
 };
 
 #define HDMI_IH_PHY_STAT0_RX_SENSE \
@@ -217,6 +221,28 @@ static inline u8 hdmi_readb(struct dw_hdmi *hdmi, int offset)
 	return val;
 }
 
+static void handle_plugged_change(struct dw_hdmi *hdmi, bool plugged)
+{
+	if (hdmi->plugged_cb && hdmi->codec_dev)
+		hdmi->plugged_cb(hdmi->codec_dev, plugged);
+}
+
+int dw_hdmi_set_plugged_cb(struct dw_hdmi *hdmi, hdmi_codec_plugged_cb fn,
+			   struct device *codec_dev)
+{
+	bool plugged;
+
+	mutex_lock(&hdmi->mutex);
+	hdmi->plugged_cb = fn;
+	hdmi->codec_dev = codec_dev;
+	plugged = hdmi->last_connector_result == connector_status_connected;
+	handle_plugged_change(hdmi, plugged);
+	mutex_unlock(&hdmi->mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_set_plugged_cb);
+
 static void hdmi_modb(struct dw_hdmi *hdmi, u8 data, u8 mask, unsigned reg)
 {
 	regmap_update_bits(hdmi->regm, reg << hdmi->reg_shift, mask, data);
@@ -2183,6 +2209,7 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 {
 	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
 					     connector);
+	enum drm_connector_status result;
 
 	mutex_lock(&hdmi->mutex);
 	hdmi->force = DRM_FORCE_UNSPECIFIED;
@@ -2190,7 +2217,18 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 	dw_hdmi_update_phy_mask(hdmi);
 	mutex_unlock(&hdmi->mutex);
 
-	return hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+	result = hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+
+	mutex_lock(&hdmi->mutex);
+	if (result != hdmi->last_connector_result) {
+		dev_dbg(hdmi->dev, "read_hpd result: %d", result);
+		handle_plugged_change(hdmi,
+				      result == connector_status_connected);
+		hdmi->last_connector_result = result;
+	}
+	mutex_unlock(&hdmi->mutex);
+
+	return result;
 }
 
 static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
@@ -2641,6 +2679,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	hdmi->rxsense = true;
 	hdmi->phy_mask = (u8)~(HDMI_PHY_HPD | HDMI_PHY_RX_SENSE);
 	hdmi->mc_clkdis = 0x7f;
+	hdmi->last_connector_result = connector_status_disconnected;
 
 	mutex_init(&hdmi->mutex);
 	mutex_init(&hdmi->audio_mutex);
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 4b3e863c4f8a..45a05e97e78a 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -6,6 +6,8 @@
 #ifndef __DW_HDMI__
 #define __DW_HDMI__
 
+#include <sound/hdmi-codec.h>
+
 struct drm_connector;
 struct drm_display_mode;
 struct drm_encoder;
@@ -154,6 +156,8 @@ void dw_hdmi_resume(struct dw_hdmi *hdmi);
 
 void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
 
+int dw_hdmi_set_plugged_cb(struct dw_hdmi *hdmi, hdmi_codec_plugged_cb fn,
+			   struct device *codec_dev);
 void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
 void dw_hdmi_set_channel_count(struct dw_hdmi *hdmi, unsigned int cnt);
 void dw_hdmi_set_channel_status(struct dw_hdmi *hdmi, u8 *channel_status);
-- 
2.23.0.237.gc6a4ce50a0-goog


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

* [PATCH v7 2/4] ASoC: rockchip-max98090: Add description for rockchip,hdmi-codec
  2019-09-19 13:54 [PATCH v7 0/4] Add HDMI jack support on RK3288 Cheng-Yi Chiang
  2019-09-19 13:54 ` [PATCH v7 1/4] drm: bridge: dw-hdmi: Report connector status using callback Cheng-Yi Chiang
@ 2019-09-19 13:54 ` Cheng-Yi Chiang
  2019-09-19 13:54 ` [PATCH v7 3/4] ASoC: rockchip_max98090: Add dai_link for HDMI Cheng-Yi Chiang
  2019-09-19 13:54 ` [PATCH v7 4/4] ASoC: rockchip_max98090: Add HDMI jack support Cheng-Yi Chiang
  3 siblings, 0 replies; 7+ messages in thread
From: Cheng-Yi Chiang @ 2019-09-19 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Jerome Brunet, Jernej Skrabec, Jonas Karlman,
	Hans Verkuil, Mark Brown, Liam Girdwood, Takashi Iwai,
	Jaroslav Kysela, Russell King, Andrzej Hajda, Laurent Pinchart,
	David Airlie, Daniel Vetter, Heiko Stuebner, dianders, dgreid,
	tzungbi, alsa-devel, dri-devel, linux-arm-kernel, linux-rockchip,
	Cheng-Yi Chiang

Add support for HDMI codec in rockchip-max98090.
Let user specify HDMI device node in DTS so machine driver can find
hdmi-codec device node for codec DAI.

Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
---
 Documentation/devicetree/bindings/sound/rockchip-max98090.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rockchip-max98090.txt b/Documentation/devicetree/bindings/sound/rockchip-max98090.txt
index a805aa99ad75..97fc838458f7 100644
--- a/Documentation/devicetree/bindings/sound/rockchip-max98090.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip-max98090.txt
@@ -7,6 +7,7 @@ Required properties:
   connected to the CODEC
 - rockchip,audio-codec: The phandle of the MAX98090 audio codec
 - rockchip,headset-codec: The phandle of Ext chip for jack detection
+- rockchip,hdmi-codec: The phandle of HDMI device for HDMI codec.
 
 Example:
 
@@ -16,4 +17,5 @@ sound {
 	rockchip,i2s-controller = <&i2s>;
 	rockchip,audio-codec = <&max98090>;
 	rockchip,headset-codec = <&headsetcodec>;
+	rockchip,hdmi-codec = <&hdmi>;
 };
-- 
2.23.0.237.gc6a4ce50a0-goog


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

* [PATCH v7 3/4] ASoC: rockchip_max98090: Add dai_link for HDMI
  2019-09-19 13:54 [PATCH v7 0/4] Add HDMI jack support on RK3288 Cheng-Yi Chiang
  2019-09-19 13:54 ` [PATCH v7 1/4] drm: bridge: dw-hdmi: Report connector status using callback Cheng-Yi Chiang
  2019-09-19 13:54 ` [PATCH v7 2/4] ASoC: rockchip-max98090: Add description for rockchip,hdmi-codec Cheng-Yi Chiang
@ 2019-09-19 13:54 ` Cheng-Yi Chiang
  2019-09-19 15:08   ` Jernej Škrabec
  2019-09-19 13:54 ` [PATCH v7 4/4] ASoC: rockchip_max98090: Add HDMI jack support Cheng-Yi Chiang
  3 siblings, 1 reply; 7+ messages in thread
From: Cheng-Yi Chiang @ 2019-09-19 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Jerome Brunet, Jernej Skrabec, Jonas Karlman,
	Hans Verkuil, Mark Brown, Liam Girdwood, Takashi Iwai,
	Jaroslav Kysela, Russell King, Andrzej Hajda, Laurent Pinchart,
	David Airlie, Daniel Vetter, Heiko Stuebner, dianders, dgreid,
	tzungbi, alsa-devel, dri-devel, linux-arm-kernel, linux-rockchip,
	Cheng-Yi Chiang

Use two dai_links. One for HDMI and one for max98090.
With this setup, audio can play to speaker and HDMI selectively.

Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
---
 .../boot/dts/rk3288-veyron-analog-audio.dtsi  |   1 +
 sound/soc/rockchip/rockchip_max98090.c        | 129 ++++++++++++++----
 2 files changed, 103 insertions(+), 27 deletions(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
index 445270aa136e..51208d161d65 100644
--- a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
@@ -17,6 +17,7 @@
 		rockchip,hp-det-gpios = <&gpio6 RK_PA5 GPIO_ACTIVE_HIGH>;
 		rockchip,mic-det-gpios = <&gpio6 RK_PB3 GPIO_ACTIVE_LOW>;
 		rockchip,headset-codec = <&headsetcodec>;
+		rockchip,hdmi-codec = <&hdmi>;
 	};
 };
 
diff --git a/sound/soc/rockchip/rockchip_max98090.c b/sound/soc/rockchip/rockchip_max98090.c
index c5fc24675a33..6c217492bb30 100644
--- a/sound/soc/rockchip/rockchip_max98090.c
+++ b/sound/soc/rockchip/rockchip_max98090.c
@@ -11,6 +11,7 @@
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <sound/core.h>
+#include <sound/hdmi-codec.h>
 #include <sound/jack.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -41,6 +42,7 @@ static const struct snd_soc_dapm_widget rk_dapm_widgets[] = {
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
 	SND_SOC_DAPM_MIC("Int Mic", NULL),
 	SND_SOC_DAPM_SPK("Speaker", NULL),
+	SND_SOC_DAPM_LINE("HDMI", NULL),
 };
 
 static const struct snd_soc_dapm_route rk_audio_map[] = {
@@ -52,6 +54,7 @@ static const struct snd_soc_dapm_route rk_audio_map[] = {
 	{"Headphone", NULL, "HPR"},
 	{"Speaker", NULL, "SPKL"},
 	{"Speaker", NULL, "SPKR"},
+	{"HDMI", NULL, "TX"},
 };
 
 static const struct snd_kcontrol_new rk_mc_controls[] = {
@@ -59,6 +62,7 @@ static const struct snd_kcontrol_new rk_mc_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
 	SOC_DAPM_PIN_SWITCH("Int Mic"),
 	SOC_DAPM_PIN_SWITCH("Speaker"),
+	SOC_DAPM_PIN_SWITCH("HDMI"),
 };
 
 static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
@@ -92,38 +96,63 @@ static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
 
 	ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
 				     SND_SOC_CLOCK_OUT);
-	if (ret < 0) {
-		dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+	if (ret) {
+		dev_err(cpu_dai->dev, "Can't set cpu dai clock %d\n", ret);
 		return ret;
 	}
 
+	/* HDMI codec dai does not need to set sysclk. */
+	if (!strcmp(rtd->dai_link->name, "HDMI"))
+		return 0;
+
 	ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
 				     SND_SOC_CLOCK_IN);
-	if (ret < 0) {
-		dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+	if (ret) {
+		dev_err(codec_dai->dev, "Can't set codec dai clock %d\n", ret);
 		return ret;
 	}
 
-	return ret;
+	return 0;
 }
 
 static const struct snd_soc_ops rk_aif1_ops = {
 	.hw_params = rk_aif1_hw_params,
 };
 
-SND_SOC_DAILINK_DEFS(hifi,
-	DAILINK_COMP_ARRAY(COMP_EMPTY()),
-	DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
-	DAILINK_COMP_ARRAY(COMP_EMPTY()));
-
-static struct snd_soc_dai_link rk_dailink = {
-	.name = "max98090",
-	.stream_name = "Audio",
-	.ops = &rk_aif1_ops,
-	/* set max98090 as slave */
-	.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
-		SND_SOC_DAIFMT_CBS_CFS,
-	SND_SOC_DAILINK_REG(hifi),
+SND_SOC_DAILINK_DEFS(analog,
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()),
+		     DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(hdmi,
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()),
+		     DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "i2s-hifi")),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+enum {
+	DAILINK_MAX98090,
+	DAILINK_HDMI,
+};
+
+/* max98090 and HDMI codec dai_link */
+static struct snd_soc_dai_link rk_dailinks[] = {
+	[DAILINK_MAX98090] = {
+		.name = "max98090",
+		.stream_name = "Analog",
+		.ops = &rk_aif1_ops,
+		/* set max98090 as slave */
+		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_CBS_CFS,
+		SND_SOC_DAILINK_REG(analog),
+	},
+	[DAILINK_HDMI] = {
+		.name = "HDMI",
+		.stream_name = "HDMI",
+		.ops = &rk_aif1_ops,
+		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_CBS_CFS,
+		SND_SOC_DAILINK_REG(hdmi),
+	}
 };
 
 static int rk_98090_headset_init(struct snd_soc_component *component);
@@ -136,8 +165,8 @@ static struct snd_soc_aux_dev rk_98090_headset_dev = {
 static struct snd_soc_card snd_soc_card_rk = {
 	.name = "ROCKCHIP-I2S",
 	.owner = THIS_MODULE,
-	.dai_link = &rk_dailink,
-	.num_links = 1,
+	.dai_link = rk_dailinks,
+	.num_links = ARRAY_SIZE(rk_dailinks),
 	.aux_dev = &rk_98090_headset_dev,
 	.num_aux_devs = 1,
 	.dapm_widgets = rk_dapm_widgets,
@@ -173,27 +202,73 @@ static int snd_rk_mc_probe(struct platform_device *pdev)
 	int ret = 0;
 	struct snd_soc_card *card = &snd_soc_card_rk;
 	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np_analog;
+	struct device_node *np_cpu;
+	struct device_node *np_hdmi_codec;
+	struct of_phandle_args args;
 
 	/* register the soc card */
 	card->dev = &pdev->dev;
 
-	rk_dailink.codecs->of_node = of_parse_phandle(np,
-			"rockchip,audio-codec", 0);
-	if (!rk_dailink.codecs->of_node) {
+	np_analog = of_parse_phandle(np, "rockchip,audio-codec", 0);
+	if (!np_analog) {
 		dev_err(&pdev->dev,
 			"Property 'rockchip,audio-codec' missing or invalid\n");
 		return -EINVAL;
 	}
+	rk_dailinks[DAILINK_MAX98090].codecs->of_node = np_analog;
+
+	ret = of_parse_phandle_with_fixed_args(np, "rockchip,audio-codec",
+					       0, 0, &args);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Unable to parse property 'rockchip,audio-codec'\n");
+		return ret;
+	}
+
+	ret = snd_soc_get_dai_name(
+			&args, &rk_dailinks[DAILINK_MAX98090].codecs->dai_name);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to get codec dai_name\n");
+		return ret;
+	}
+
+	np_cpu = of_parse_phandle(np, "rockchip,i2s-controller", 0);
 
-	rk_dailink.cpus->of_node = of_parse_phandle(np,
-			"rockchip,i2s-controller", 0);
-	if (!rk_dailink.cpus->of_node) {
+	if (!np_cpu) {
 		dev_err(&pdev->dev,
 			"Property 'rockchip,i2s-controller' missing or invalid\n");
 		return -EINVAL;
 	}
 
-	rk_dailink.platforms->of_node = rk_dailink.cpus->of_node;
+	np_hdmi_codec = of_parse_phandle(np, "rockchip,hdmi-codec", 0);
+	if (!np_hdmi_codec) {
+		dev_err(&pdev->dev,
+			"Property 'rockchip,hdmi-codec' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	rk_dailinks[DAILINK_HDMI].codecs->of_node = np_hdmi_codec;
+
+	ret = of_parse_phandle_with_fixed_args(np, "rockchip,hdmi-codec",
+					       0, 0, &args);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Unable to parse property 'rockchip,hdmi-codec'\n");
+		return ret;
+	}
+
+	ret = snd_soc_get_dai_name(
+			&args, &rk_dailinks[DAILINK_HDMI].codecs->dai_name);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to get hdmi codec dai_name\n");
+		return ret;
+	}
+
+	rk_dailinks[DAILINK_MAX98090].cpus->of_node = np_cpu;
+	rk_dailinks[DAILINK_MAX98090].platforms->of_node = np_cpu;
+	rk_dailinks[DAILINK_HDMI].cpus->of_node = np_cpu;
+	rk_dailinks[DAILINK_HDMI].platforms->of_node = np_cpu;
 
 	rk_98090_headset_dev.codec_of_node = of_parse_phandle(np,
 			"rockchip,headset-codec", 0);
-- 
2.23.0.237.gc6a4ce50a0-goog


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

* [PATCH v7 4/4] ASoC: rockchip_max98090: Add HDMI jack support
  2019-09-19 13:54 [PATCH v7 0/4] Add HDMI jack support on RK3288 Cheng-Yi Chiang
                   ` (2 preceding siblings ...)
  2019-09-19 13:54 ` [PATCH v7 3/4] ASoC: rockchip_max98090: Add dai_link for HDMI Cheng-Yi Chiang
@ 2019-09-19 13:54 ` Cheng-Yi Chiang
  3 siblings, 0 replies; 7+ messages in thread
From: Cheng-Yi Chiang @ 2019-09-19 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Jerome Brunet, Jernej Skrabec, Jonas Karlman,
	Hans Verkuil, Mark Brown, Liam Girdwood, Takashi Iwai,
	Jaroslav Kysela, Russell King, Andrzej Hajda, Laurent Pinchart,
	David Airlie, Daniel Vetter, Heiko Stuebner, dianders, dgreid,
	tzungbi, alsa-devel, dri-devel, linux-arm-kernel, linux-rockchip,
	Cheng-Yi Chiang

In machine driver, create a jack and let hdmi-codec report jack status.

Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
---
 sound/soc/rockchip/Kconfig             |  3 ++-
 sound/soc/rockchip/rockchip_max98090.c | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index b43657e6e655..d610b553ea3b 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -40,9 +40,10 @@ config SND_SOC_ROCKCHIP_MAX98090
 	select SND_SOC_ROCKCHIP_I2S
 	select SND_SOC_MAX98090
 	select SND_SOC_TS3A227E
+	select SND_SOC_HDMI_CODEC
 	help
 	  Say Y or M here if you want to add support for SoC audio on Rockchip
-	  boards using the MAX98090 codec, such as Veyron.
+	  boards using the MAX98090 codec and HDMI codec, such as Veyron.
 
 config SND_SOC_ROCKCHIP_RT5645
 	tristate "ASoC support for Rockchip boards using a RT5645/RT5650 codec"
diff --git a/sound/soc/rockchip/rockchip_max98090.c b/sound/soc/rockchip/rockchip_max98090.c
index 6c217492bb30..11cf252e8391 100644
--- a/sound/soc/rockchip/rockchip_max98090.c
+++ b/sound/soc/rockchip/rockchip_max98090.c
@@ -134,6 +134,25 @@ enum {
 	DAILINK_HDMI,
 };
 
+static struct snd_soc_jack rk_hdmi_jack;
+
+static int rk_hdmi_init(struct snd_soc_pcm_runtime *runtime)
+{
+	struct snd_soc_card *card = runtime->card;
+	struct snd_soc_component *component = runtime->codec_dai->component;
+	int ret;
+
+	/* enable jack detection */
+	ret = snd_soc_card_jack_new(card, "HDMI Jack", SND_JACK_LINEOUT,
+				    &rk_hdmi_jack, NULL, 0);
+	if (ret) {
+		dev_err(card->dev, "Can't new HDMI Jack %d\n", ret);
+		return ret;
+	}
+
+	return hdmi_codec_set_jack_detect(component, &rk_hdmi_jack);
+}
+
 /* max98090 and HDMI codec dai_link */
 static struct snd_soc_dai_link rk_dailinks[] = {
 	[DAILINK_MAX98090] = {
@@ -151,6 +170,7 @@ static struct snd_soc_dai_link rk_dailinks[] = {
 		.ops = &rk_aif1_ops,
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			SND_SOC_DAIFMT_CBS_CFS,
+		.init = rk_hdmi_init,
 		SND_SOC_DAILINK_REG(hdmi),
 	}
 };
-- 
2.23.0.237.gc6a4ce50a0-goog


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

* Re: [PATCH v7 3/4] ASoC: rockchip_max98090: Add dai_link for HDMI
  2019-09-19 13:54 ` [PATCH v7 3/4] ASoC: rockchip_max98090: Add dai_link for HDMI Cheng-Yi Chiang
@ 2019-09-19 15:08   ` Jernej Škrabec
  2019-09-20 13:16     ` Cheng-yi Chiang
  0 siblings, 1 reply; 7+ messages in thread
From: Jernej Škrabec @ 2019-09-19 15:08 UTC (permalink / raw)
  To: Cheng-Yi Chiang
  Cc: linux-kernel, Neil Armstrong, Jerome Brunet, Jonas Karlman,
	Hans Verkuil, Mark Brown, Liam Girdwood, Takashi Iwai,
	Jaroslav Kysela, Russell King, Andrzej Hajda, Laurent Pinchart,
	David Airlie, Daniel Vetter, Heiko Stuebner, dianders, dgreid,
	tzungbi, alsa-devel, dri-devel, linux-arm-kernel, linux-rockchip

Hi!

Dne četrtek, 19. september 2019 ob 15:54:49 CEST je Cheng-Yi Chiang 
napisal(a):
> Use two dai_links. One for HDMI and one for max98090.
> With this setup, audio can play to speaker and HDMI selectively.
> 
> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> ---
>  .../boot/dts/rk3288-veyron-analog-audio.dtsi  |   1 +
>  sound/soc/rockchip/rockchip_max98090.c        | 129 ++++++++++++++----
>  2 files changed, 103 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi index
> 445270aa136e..51208d161d65 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> +++ b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> @@ -17,6 +17,7 @@
>  		rockchip,hp-det-gpios = <&gpio6 RK_PA5 
GPIO_ACTIVE_HIGH>;
>  		rockchip,mic-det-gpios = <&gpio6 RK_PB3 
GPIO_ACTIVE_LOW>;
>  		rockchip,headset-codec = <&headsetcodec>;
> +		rockchip,hdmi-codec = <&hdmi>;
>  	};
>  };
> 
> diff --git a/sound/soc/rockchip/rockchip_max98090.c
> b/sound/soc/rockchip/rockchip_max98090.c index c5fc24675a33..6c217492bb30
> 100644
> --- a/sound/soc/rockchip/rockchip_max98090.c
> +++ b/sound/soc/rockchip/rockchip_max98090.c
> @@ -11,6 +11,7 @@
>  #include <linux/gpio.h>
>  #include <linux/of_gpio.h>
>  #include <sound/core.h>
> +#include <sound/hdmi-codec.h>
>  #include <sound/jack.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm_params.h>
> @@ -41,6 +42,7 @@ static const struct snd_soc_dapm_widget rk_dapm_widgets[]
> = { SND_SOC_DAPM_MIC("Headset Mic", NULL),
>  	SND_SOC_DAPM_MIC("Int Mic", NULL),
>  	SND_SOC_DAPM_SPK("Speaker", NULL),
> +	SND_SOC_DAPM_LINE("HDMI", NULL),
>  };
> 
>  static const struct snd_soc_dapm_route rk_audio_map[] = {
> @@ -52,6 +54,7 @@ static const struct snd_soc_dapm_route rk_audio_map[] = {
>  	{"Headphone", NULL, "HPR"},
>  	{"Speaker", NULL, "SPKL"},
>  	{"Speaker", NULL, "SPKR"},
> +	{"HDMI", NULL, "TX"},
>  };
> 
>  static const struct snd_kcontrol_new rk_mc_controls[] = {
> @@ -59,6 +62,7 @@ static const struct snd_kcontrol_new rk_mc_controls[] = {
>  	SOC_DAPM_PIN_SWITCH("Headset Mic"),
>  	SOC_DAPM_PIN_SWITCH("Int Mic"),
>  	SOC_DAPM_PIN_SWITCH("Speaker"),
> +	SOC_DAPM_PIN_SWITCH("HDMI"),
>  };
> 
>  static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
> @@ -92,38 +96,63 @@ static int rk_aif1_hw_params(struct snd_pcm_substream
> *substream,
> 
>  	ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
>  				     SND_SOC_CLOCK_OUT);
> -	if (ret < 0) {
> -		dev_err(codec_dai->dev, "Can't set codec clock %d\n", 
ret);
> +	if (ret) {
> +		dev_err(cpu_dai->dev, "Can't set cpu dai clock %d\n", 
ret);
>  		return ret;
>  	}
> 
> +	/* HDMI codec dai does not need to set sysclk. */
> +	if (!strcmp(rtd->dai_link->name, "HDMI"))
> +		return 0;
> +
>  	ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
>  				     SND_SOC_CLOCK_IN);
> -	if (ret < 0) {
> -		dev_err(codec_dai->dev, "Can't set codec clock %d\n", 
ret);
> +	if (ret) {
> +		dev_err(codec_dai->dev, "Can't set codec dai clock 
%d\n", ret);
>  		return ret;
>  	}
> 
> -	return ret;
> +	return 0;
>  }
> 
>  static const struct snd_soc_ops rk_aif1_ops = {
>  	.hw_params = rk_aif1_hw_params,
>  };
> 
> -SND_SOC_DAILINK_DEFS(hifi,
> -	DAILINK_COMP_ARRAY(COMP_EMPTY()),
> -	DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
> -	DAILINK_COMP_ARRAY(COMP_EMPTY()));
> -
> -static struct snd_soc_dai_link rk_dailink = {
> -	.name = "max98090",
> -	.stream_name = "Audio",
> -	.ops = &rk_aif1_ops,
> -	/* set max98090 as slave */
> -	.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> -		SND_SOC_DAIFMT_CBS_CFS,
> -	SND_SOC_DAILINK_REG(hifi),
> +SND_SOC_DAILINK_DEFS(analog,
> +		     DAILINK_COMP_ARRAY(COMP_EMPTY()),
> +		     DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
> +		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
> +
> +SND_SOC_DAILINK_DEFS(hdmi,
> +		     DAILINK_COMP_ARRAY(COMP_EMPTY()),
> +		     DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "i2s-hifi")),
> +		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
> +
> +enum {
> +	DAILINK_MAX98090,
> +	DAILINK_HDMI,
> +};
> +
> +/* max98090 and HDMI codec dai_link */
> +static struct snd_soc_dai_link rk_dailinks[] = {
> +	[DAILINK_MAX98090] = {
> +		.name = "max98090",
> +		.stream_name = "Analog",
> +		.ops = &rk_aif1_ops,
> +		/* set max98090 as slave */
> +		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> +			SND_SOC_DAIFMT_CBS_CFS,
> +		SND_SOC_DAILINK_REG(analog),
> +	},
> +	[DAILINK_HDMI] = {
> +		.name = "HDMI",
> +		.stream_name = "HDMI",
> +		.ops = &rk_aif1_ops,
> +		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> +			SND_SOC_DAIFMT_CBS_CFS,
> +		SND_SOC_DAILINK_REG(hdmi),
> +	}
>  };
> 
>  static int rk_98090_headset_init(struct snd_soc_component *component);
> @@ -136,8 +165,8 @@ static struct snd_soc_aux_dev rk_98090_headset_dev = {
>  static struct snd_soc_card snd_soc_card_rk = {
>  	.name = "ROCKCHIP-I2S",
>  	.owner = THIS_MODULE,
> -	.dai_link = &rk_dailink,
> -	.num_links = 1,
> +	.dai_link = rk_dailinks,
> +	.num_links = ARRAY_SIZE(rk_dailinks),
>  	.aux_dev = &rk_98090_headset_dev,
>  	.num_aux_devs = 1,
>  	.dapm_widgets = rk_dapm_widgets,
> @@ -173,27 +202,73 @@ static int snd_rk_mc_probe(struct platform_device
> *pdev) int ret = 0;
>  	struct snd_soc_card *card = &snd_soc_card_rk;
>  	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np_analog;
> +	struct device_node *np_cpu;
> +	struct device_node *np_hdmi_codec;
> +	struct of_phandle_args args;
> 
>  	/* register the soc card */
>  	card->dev = &pdev->dev;
> 
> -	rk_dailink.codecs->of_node = of_parse_phandle(np,
> -			"rockchip,audio-codec", 0);
> -	if (!rk_dailink.codecs->of_node) {
> +	np_analog = of_parse_phandle(np, "rockchip,audio-codec", 0);
> +	if (!np_analog) {
>  		dev_err(&pdev->dev,
>  			"Property 'rockchip,audio-codec' missing or 
invalid\n");
>  		return -EINVAL;
>  	}
> +	rk_dailinks[DAILINK_MAX98090].codecs->of_node = np_analog;
> +
> +	ret = of_parse_phandle_with_fixed_args(np, "rockchip,audio-codec",
> +					       0, 0, &args);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Unable to parse property 'rockchip,audio-
codec'\n");
> +		return ret;
> +	}
> +
> +	ret = snd_soc_get_dai_name(
> +			&args, &rk_dailinks[DAILINK_MAX98090].codecs-
>dai_name);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Unable to get codec dai_name\n");
> +		return ret;
> +	}
> +
> +	np_cpu = of_parse_phandle(np, "rockchip,i2s-controller", 0);
> 
> -	rk_dailink.cpus->of_node = of_parse_phandle(np,
> -			"rockchip,i2s-controller", 0);
> -	if (!rk_dailink.cpus->of_node) {
> +	if (!np_cpu) {
>  		dev_err(&pdev->dev,
>  			"Property 'rockchip,i2s-controller' missing 
or invalid\n");
>  		return -EINVAL;
>  	}
> 
> -	rk_dailink.platforms->of_node = rk_dailink.cpus->of_node;
> +	np_hdmi_codec = of_parse_phandle(np, "rockchip,hdmi-codec", 0);
> +	if (!np_hdmi_codec) {
> +		dev_err(&pdev->dev,
> +			"Property 'rockchip,hdmi-codec' missing or 
invalid\n");
> +		return -EINVAL;
> +	}

Property "rockchip,hdmi-codec" is added in this series, right? You can't make 
it mandatory, because kernel must be backward compatible with old device tree 
files and they don't have this property.

Think about use case when user happily used this driver and after kernel 
update, it suddenly stops working. You can't assume that board DTB file will be 
updated along with kernel update.

Just make it optional and don't expose jack functionality if it's not present.

Best regards,
Jernej

> +
> +	rk_dailinks[DAILINK_HDMI].codecs->of_node = np_hdmi_codec;
> +
> +	ret = of_parse_phandle_with_fixed_args(np, "rockchip,hdmi-codec",
> +					       0, 0, &args);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Unable to parse property 'rockchip,hdmi-
codec'\n");
> +		return ret;
> +	}
> +
> +	ret = snd_soc_get_dai_name(
> +			&args, &rk_dailinks[DAILINK_HDMI].codecs-
>dai_name);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Unable to get hdmi codec 
dai_name\n");
> +		return ret;
> +	}
> +
> +	rk_dailinks[DAILINK_MAX98090].cpus->of_node = np_cpu;
> +	rk_dailinks[DAILINK_MAX98090].platforms->of_node = np_cpu;
> +	rk_dailinks[DAILINK_HDMI].cpus->of_node = np_cpu;
> +	rk_dailinks[DAILINK_HDMI].platforms->of_node = np_cpu;
> 
>  	rk_98090_headset_dev.codec_of_node = of_parse_phandle(np,
>  			"rockchip,headset-codec", 0);





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

* Re: [PATCH v7 3/4] ASoC: rockchip_max98090: Add dai_link for HDMI
  2019-09-19 15:08   ` Jernej Škrabec
@ 2019-09-20 13:16     ` Cheng-yi Chiang
  0 siblings, 0 replies; 7+ messages in thread
From: Cheng-yi Chiang @ 2019-09-20 13:16 UTC (permalink / raw)
  To: Jernej Škrabec
  Cc: linux-kernel, Neil Armstrong, Jerome Brunet, Jonas Karlman,
	Hans Verkuil, Mark Brown, Liam Girdwood, Takashi Iwai,
	Jaroslav Kysela, Russell King, Andrzej Hajda, Laurent Pinchart,
	David Airlie, Daniel Vetter, Heiko Stuebner, Doug Anderson,
	Dylan Reid, tzungbi,
	moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	dri-devel, linux-arm-kernel, linux-rockchip

On Thu, Sep 19, 2019 at 11:08 PM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Hi!
>
> Dne četrtek, 19. september 2019 ob 15:54:49 CEST je Cheng-Yi Chiang
> napisal(a):
> > Use two dai_links. One for HDMI and one for max98090.
> > With this setup, audio can play to speaker and HDMI selectively.
> >
> > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > ---
> >  .../boot/dts/rk3288-veyron-analog-audio.dtsi  |   1 +
> >  sound/soc/rockchip/rockchip_max98090.c        | 129 ++++++++++++++----
> >  2 files changed, 103 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> > b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi index
> > 445270aa136e..51208d161d65 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> > +++ b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> > @@ -17,6 +17,7 @@
> >               rockchip,hp-det-gpios = <&gpio6 RK_PA5
> GPIO_ACTIVE_HIGH>;
> >               rockchip,mic-det-gpios = <&gpio6 RK_PB3
> GPIO_ACTIVE_LOW>;
> >               rockchip,headset-codec = <&headsetcodec>;
> > +             rockchip,hdmi-codec = <&hdmi>;
> >       };
> >  };
> >
> > diff --git a/sound/soc/rockchip/rockchip_max98090.c
> > b/sound/soc/rockchip/rockchip_max98090.c index c5fc24675a33..6c217492bb30
> > 100644
> > --- a/sound/soc/rockchip/rockchip_max98090.c
> > +++ b/sound/soc/rockchip/rockchip_max98090.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/gpio.h>
> >  #include <linux/of_gpio.h>
> >  #include <sound/core.h>
> > +#include <sound/hdmi-codec.h>
> >  #include <sound/jack.h>
> >  #include <sound/pcm.h>
> >  #include <sound/pcm_params.h>
> > @@ -41,6 +42,7 @@ static const struct snd_soc_dapm_widget rk_dapm_widgets[]
> > = { SND_SOC_DAPM_MIC("Headset Mic", NULL),
> >       SND_SOC_DAPM_MIC("Int Mic", NULL),
> >       SND_SOC_DAPM_SPK("Speaker", NULL),
> > +     SND_SOC_DAPM_LINE("HDMI", NULL),
> >  };
> >
> >  static const struct snd_soc_dapm_route rk_audio_map[] = {
> > @@ -52,6 +54,7 @@ static const struct snd_soc_dapm_route rk_audio_map[] = {
> >       {"Headphone", NULL, "HPR"},
> >       {"Speaker", NULL, "SPKL"},
> >       {"Speaker", NULL, "SPKR"},
> > +     {"HDMI", NULL, "TX"},
> >  };
> >
> >  static const struct snd_kcontrol_new rk_mc_controls[] = {
> > @@ -59,6 +62,7 @@ static const struct snd_kcontrol_new rk_mc_controls[] = {
> >       SOC_DAPM_PIN_SWITCH("Headset Mic"),
> >       SOC_DAPM_PIN_SWITCH("Int Mic"),
> >       SOC_DAPM_PIN_SWITCH("Speaker"),
> > +     SOC_DAPM_PIN_SWITCH("HDMI"),
> >  };
> >
> >  static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
> > @@ -92,38 +96,63 @@ static int rk_aif1_hw_params(struct snd_pcm_substream
> > *substream,
> >
> >       ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
> >                                    SND_SOC_CLOCK_OUT);
> > -     if (ret < 0) {
> > -             dev_err(codec_dai->dev, "Can't set codec clock %d\n",
> ret);
> > +     if (ret) {
> > +             dev_err(cpu_dai->dev, "Can't set cpu dai clock %d\n",
> ret);
> >               return ret;
> >       }
> >
> > +     /* HDMI codec dai does not need to set sysclk. */
> > +     if (!strcmp(rtd->dai_link->name, "HDMI"))
> > +             return 0;
> > +
> >       ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
> >                                    SND_SOC_CLOCK_IN);
> > -     if (ret < 0) {
> > -             dev_err(codec_dai->dev, "Can't set codec clock %d\n",
> ret);
> > +     if (ret) {
> > +             dev_err(codec_dai->dev, "Can't set codec dai clock
> %d\n", ret);
> >               return ret;
> >       }
> >
> > -     return ret;
> > +     return 0;
> >  }
> >
> >  static const struct snd_soc_ops rk_aif1_ops = {
> >       .hw_params = rk_aif1_hw_params,
> >  };
> >
> > -SND_SOC_DAILINK_DEFS(hifi,
> > -     DAILINK_COMP_ARRAY(COMP_EMPTY()),
> > -     DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
> > -     DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > -
> > -static struct snd_soc_dai_link rk_dailink = {
> > -     .name = "max98090",
> > -     .stream_name = "Audio",
> > -     .ops = &rk_aif1_ops,
> > -     /* set max98090 as slave */
> > -     .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> > -             SND_SOC_DAIFMT_CBS_CFS,
> > -     SND_SOC_DAILINK_REG(hifi),
> > +SND_SOC_DAILINK_DEFS(analog,
> > +                  DAILINK_COMP_ARRAY(COMP_EMPTY()),
> > +                  DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
> > +                  DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > +
> > +SND_SOC_DAILINK_DEFS(hdmi,
> > +                  DAILINK_COMP_ARRAY(COMP_EMPTY()),
> > +                  DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "i2s-hifi")),
> > +                  DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > +
> > +enum {
> > +     DAILINK_MAX98090,
> > +     DAILINK_HDMI,
> > +};
> > +
> > +/* max98090 and HDMI codec dai_link */
> > +static struct snd_soc_dai_link rk_dailinks[] = {
> > +     [DAILINK_MAX98090] = {
> > +             .name = "max98090",
> > +             .stream_name = "Analog",
> > +             .ops = &rk_aif1_ops,
> > +             /* set max98090 as slave */
> > +             .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> > +                     SND_SOC_DAIFMT_CBS_CFS,
> > +             SND_SOC_DAILINK_REG(analog),
> > +     },
> > +     [DAILINK_HDMI] = {
> > +             .name = "HDMI",
> > +             .stream_name = "HDMI",
> > +             .ops = &rk_aif1_ops,
> > +             .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> > +                     SND_SOC_DAIFMT_CBS_CFS,
> > +             SND_SOC_DAILINK_REG(hdmi),
> > +     }
> >  };
> >
> >  static int rk_98090_headset_init(struct snd_soc_component *component);
> > @@ -136,8 +165,8 @@ static struct snd_soc_aux_dev rk_98090_headset_dev = {
> >  static struct snd_soc_card snd_soc_card_rk = {
> >       .name = "ROCKCHIP-I2S",
> >       .owner = THIS_MODULE,
> > -     .dai_link = &rk_dailink,
> > -     .num_links = 1,
> > +     .dai_link = rk_dailinks,
> > +     .num_links = ARRAY_SIZE(rk_dailinks),
> >       .aux_dev = &rk_98090_headset_dev,
> >       .num_aux_devs = 1,
> >       .dapm_widgets = rk_dapm_widgets,
> > @@ -173,27 +202,73 @@ static int snd_rk_mc_probe(struct platform_device
> > *pdev) int ret = 0;
> >       struct snd_soc_card *card = &snd_soc_card_rk;
> >       struct device_node *np = pdev->dev.of_node;
> > +     struct device_node *np_analog;
> > +     struct device_node *np_cpu;
> > +     struct device_node *np_hdmi_codec;
> > +     struct of_phandle_args args;
> >
> >       /* register the soc card */
> >       card->dev = &pdev->dev;
> >
> > -     rk_dailink.codecs->of_node = of_parse_phandle(np,
> > -                     "rockchip,audio-codec", 0);
> > -     if (!rk_dailink.codecs->of_node) {
> > +     np_analog = of_parse_phandle(np, "rockchip,audio-codec", 0);
> > +     if (!np_analog) {
> >               dev_err(&pdev->dev,
> >                       "Property 'rockchip,audio-codec' missing or
> invalid\n");
> >               return -EINVAL;
> >       }
> > +     rk_dailinks[DAILINK_MAX98090].codecs->of_node = np_analog;
> > +
> > +     ret = of_parse_phandle_with_fixed_args(np, "rockchip,audio-codec",
> > +                                            0, 0, &args);
> > +     if (ret) {
> > +             dev_err(&pdev->dev,
> > +                     "Unable to parse property 'rockchip,audio-
> codec'\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = snd_soc_get_dai_name(
> > +                     &args, &rk_dailinks[DAILINK_MAX98090].codecs-
> >dai_name);
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "Unable to get codec dai_name\n");
> > +             return ret;
> > +     }
> > +
> > +     np_cpu = of_parse_phandle(np, "rockchip,i2s-controller", 0);
> >
> > -     rk_dailink.cpus->of_node = of_parse_phandle(np,
> > -                     "rockchip,i2s-controller", 0);
> > -     if (!rk_dailink.cpus->of_node) {
> > +     if (!np_cpu) {
> >               dev_err(&pdev->dev,
> >                       "Property 'rockchip,i2s-controller' missing
> or invalid\n");
> >               return -EINVAL;
> >       }
> >
> > -     rk_dailink.platforms->of_node = rk_dailink.cpus->of_node;
> > +     np_hdmi_codec = of_parse_phandle(np, "rockchip,hdmi-codec", 0);
> > +     if (!np_hdmi_codec) {
> > +             dev_err(&pdev->dev,
> > +                     "Property 'rockchip,hdmi-codec' missing or
> invalid\n");
> > +             return -EINVAL;
> > +     }
>
> Property "rockchip,hdmi-codec" is added in this series, right? You can't make
> it mandatory, because kernel must be backward compatible with old device tree
> files and they don't have this property.
>
> Think about use case when user happily used this driver and after kernel
> update, it suddenly stops working. You can't assume that board DTB file will be
> updated along with kernel update.
>
> Just make it optional and don't expose jack functionality if it's not present.
Hi Jernej,
Thanks for the reply.
I see. Yes I can make it optional.
But it will become a little bit messy for two types of usage to share
one machine driver.

I think I will create two instances of structs for

dapm widgets,
dapm routes,
kcontrols,
dai_links

for "max98090 only" and "max98090+hdmi"

and set those fields in snd_soc_card depending on depending on the property.
These two usages can still share most of the function calls.
Hope this looks clean.

If you have a cleaner way of sharing machine driver please let me know.
I'll post an update probably next week.
Thanks a lot!


>
> Best regards,
> Jernej
>
> > +
> > +     rk_dailinks[DAILINK_HDMI].codecs->of_node = np_hdmi_codec;
> > +
> > +     ret = of_parse_phandle_with_fixed_args(np, "rockchip,hdmi-codec",
> > +                                            0, 0, &args);
> > +     if (ret) {
> > +             dev_err(&pdev->dev,
> > +                     "Unable to parse property 'rockchip,hdmi-
> codec'\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = snd_soc_get_dai_name(
> > +                     &args, &rk_dailinks[DAILINK_HDMI].codecs-
> >dai_name);
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "Unable to get hdmi codec
> dai_name\n");
> > +             return ret;
> > +     }
> > +
> > +     rk_dailinks[DAILINK_MAX98090].cpus->of_node = np_cpu;
> > +     rk_dailinks[DAILINK_MAX98090].platforms->of_node = np_cpu;
> > +     rk_dailinks[DAILINK_HDMI].cpus->of_node = np_cpu;
> > +     rk_dailinks[DAILINK_HDMI].platforms->of_node = np_cpu;
> >
> >       rk_98090_headset_dev.codec_of_node = of_parse_phandle(np,
> >                       "rockchip,headset-codec", 0);
>
>
>
>

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

end of thread, other threads:[~2019-09-20 13:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19 13:54 [PATCH v7 0/4] Add HDMI jack support on RK3288 Cheng-Yi Chiang
2019-09-19 13:54 ` [PATCH v7 1/4] drm: bridge: dw-hdmi: Report connector status using callback Cheng-Yi Chiang
2019-09-19 13:54 ` [PATCH v7 2/4] ASoC: rockchip-max98090: Add description for rockchip,hdmi-codec Cheng-Yi Chiang
2019-09-19 13:54 ` [PATCH v7 3/4] ASoC: rockchip_max98090: Add dai_link for HDMI Cheng-Yi Chiang
2019-09-19 15:08   ` Jernej Škrabec
2019-09-20 13:16     ` Cheng-yi Chiang
2019-09-19 13:54 ` [PATCH v7 4/4] ASoC: rockchip_max98090: Add HDMI jack support Cheng-Yi Chiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).