linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint
@ 2018-09-17 11:30 Jacopo Mondi
  2018-09-17 11:30 ` [PATCH v3 1/4] media: i2c: adv748x: Support probing a single output Jacopo Mondi
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Jacopo Mondi @ 2018-09-17 11:30 UTC (permalink / raw)
  To: laurent.pinchart, kieran.bingham+renesas, niklas.soderlund+renesas
  Cc: Jacopo Mondi, linux-renesas-soc, linux-media

Hello Laurent, Kieran, Niklas,
   to address the Ebisu board use case, this series allows the adv748x driver
to probe with a single output connection defined.

Compared to v2, I have dropped the last patch, as without any dynamic routing
support it is not that helpful, and I've fixed most of commit messages as
suggested by Kieran.

I have tested in 3 conditions on Salvator-X M3-W:
- AFE input not registered
- TXB not registered (Ebisu use case)
- AFE and TXB not registered

Let me know if I can help testing this on Ebisu.

Thanks
   j

v2 -> v3:
- Drop v2 patch [5/5]
- Add Kieran's tags and modify commit messages as he suggested

Jacopo Mondi (4):
  media: i2c: adv748x: Support probing a single output
  media: i2c: adv748x: Handle TX[A|B] power management
  media: i2c: adv748x: Conditionally enable only CSI-2 outputs
  media: i2c: adv748x: Register only enabled inputs

 drivers/media/i2c/adv748x/adv748x-afe.c  |  2 +-
 drivers/media/i2c/adv748x/adv748x-core.c | 83 +++++++++++++++++---------------
 drivers/media/i2c/adv748x/adv748x-csi2.c | 29 ++++-------
 drivers/media/i2c/adv748x/adv748x-hdmi.c |  2 +-
 drivers/media/i2c/adv748x/adv748x.h      | 19 ++++++--
 5 files changed, 68 insertions(+), 67 deletions(-)

--
2.7.4

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

* [PATCH v3 1/4] media: i2c: adv748x: Support probing a single output
  2018-09-17 11:30 [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Jacopo Mondi
@ 2018-09-17 11:30 ` Jacopo Mondi
  2018-09-17 11:30 ` [PATCH v3 2/4] media: i2c: adv748x: Handle TX[A|B] power management Jacopo Mondi
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jacopo Mondi @ 2018-09-17 11:30 UTC (permalink / raw)
  To: laurent.pinchart, kieran.bingham+renesas, niklas.soderlund+renesas
  Cc: Jacopo Mondi, linux-renesas-soc, linux-media

Currently the adv748x driver will fail to probe unless both of it's
output endpoints (TXA and TXB) are connected.

Make the driver support probing provided that there is at least one
input, and one output connected and protect the clean-up function from
accessing un-initialized fields.

Following patches will fix other uses of un-initialized TXs in the driver,
such as power management functions.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/adv748x/adv748x-core.c | 25 ++++++++++++++++++++++---
 drivers/media/i2c/adv748x/adv748x-csi2.c | 18 ++++++------------
 drivers/media/i2c/adv748x/adv748x.h      |  2 ++
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 6ca88daa..65c3024 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -569,7 +569,8 @@ static int adv748x_parse_dt(struct adv748x_state *state)
 {
 	struct device_node *ep_np = NULL;
 	struct of_endpoint ep;
-	bool found = false;
+	bool out_found = false;
+	bool in_found = false;
 
 	for_each_endpoint_of_node(state->dev->of_node, ep_np) {
 		of_graph_parse_endpoint(ep_np, &ep);
@@ -592,10 +593,17 @@ static int adv748x_parse_dt(struct adv748x_state *state)
 		of_node_get(ep_np);
 		state->endpoints[ep.port] = ep_np;
 
-		found = true;
+		/*
+		 * At least one input endpoint and one output endpoint shall
+		 * be defined.
+		 */
+		if (ep.port < ADV748X_PORT_TXA)
+			in_found = true;
+		else
+			out_found = true;
 	}
 
-	return found ? 0 : -ENODEV;
+	return in_found && out_found ? 0 : -ENODEV;
 }
 
 static void adv748x_dt_cleanup(struct adv748x_state *state)
@@ -627,6 +635,17 @@ static int adv748x_probe(struct i2c_client *client,
 	state->i2c_clients[ADV748X_PAGE_IO] = client;
 	i2c_set_clientdata(client, state);
 
+	/*
+	 * We can not use container_of to get back to the state with two TXs;
+	 * Initialize the TXs's fields unconditionally on the endpoint
+	 * presence to access them later.
+	 */
+	state->txa.state = state->txb.state = state;
+	state->txa.page = ADV748X_PAGE_TXA;
+	state->txb.page = ADV748X_PAGE_TXB;
+	state->txa.port = ADV748X_PORT_TXA;
+	state->txb.port = ADV748X_PORT_TXB;
+
 	/* Discover and process ports declared by the Device tree endpoints */
 	ret = adv748x_parse_dt(state);
 	if (ret) {
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 469be87..556e13c 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -266,19 +266,10 @@ static int adv748x_csi2_init_controls(struct adv748x_csi2 *tx)
 
 int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx)
 {
-	struct device_node *ep;
 	int ret;
 
-	/* We can not use container_of to get back to the state with two TXs */
-	tx->state = state;
-	tx->page = is_txa(tx) ? ADV748X_PAGE_TXA : ADV748X_PAGE_TXB;
-
-	ep = state->endpoints[is_txa(tx) ? ADV748X_PORT_TXA : ADV748X_PORT_TXB];
-	if (!ep) {
-		adv_err(state, "No endpoint found for %s\n",
-				is_txa(tx) ? "txa" : "txb");
-		return -ENODEV;
-	}
+	if (!is_tx_enabled(tx))
+		return 0;
 
 	/* Initialise the virtual channel */
 	adv748x_csi2_set_virtual_channel(tx, 0);
@@ -288,7 +279,7 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx)
 			    is_txa(tx) ? "txa" : "txb");
 
 	/* Ensure that matching is based upon the endpoint fwnodes */
-	tx->sd.fwnode = of_fwnode_handle(ep);
+	tx->sd.fwnode = of_fwnode_handle(state->endpoints[tx->port]);
 
 	/* Register internal ops for incremental subdev registration */
 	tx->sd.internal_ops = &adv748x_csi2_internal_ops;
@@ -321,6 +312,9 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx)
 
 void adv748x_csi2_cleanup(struct adv748x_csi2 *tx)
 {
+	if (!is_tx_enabled(tx))
+		return;
+
 	v4l2_async_unregister_subdev(&tx->sd);
 	media_entity_cleanup(&tx->sd.entity);
 	v4l2_ctrl_handler_free(&tx->ctrl_hdl);
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index 65f8374..1cf46c40 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -82,6 +82,7 @@ struct adv748x_csi2 {
 	struct adv748x_state *state;
 	struct v4l2_mbus_framefmt format;
 	unsigned int page;
+	unsigned int port;
 
 	struct media_pad pads[ADV748X_CSI2_NR_PADS];
 	struct v4l2_ctrl_handler ctrl_hdl;
@@ -91,6 +92,7 @@ struct adv748x_csi2 {
 
 #define notifier_to_csi2(n) container_of(n, struct adv748x_csi2, notifier)
 #define adv748x_sd_to_csi2(sd) container_of(sd, struct adv748x_csi2, sd)
+#define is_tx_enabled(_tx) ((_tx)->state->endpoints[(_tx)->port] != NULL)
 
 enum adv748x_hdmi_pads {
 	ADV748X_HDMI_SINK,
-- 
2.7.4

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

* [PATCH v3 2/4] media: i2c: adv748x: Handle TX[A|B] power management
  2018-09-17 11:30 [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Jacopo Mondi
  2018-09-17 11:30 ` [PATCH v3 1/4] media: i2c: adv748x: Support probing a single output Jacopo Mondi
@ 2018-09-17 11:30 ` Jacopo Mondi
  2018-09-17 11:30 ` [PATCH v3 3/4] media: i2c: adv748x: Conditionally enable only CSI-2 outputs Jacopo Mondi
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jacopo Mondi @ 2018-09-17 11:30 UTC (permalink / raw)
  To: laurent.pinchart, kieran.bingham+renesas, niklas.soderlund+renesas
  Cc: Jacopo Mondi, linux-renesas-soc, linux-media

As the driver is now allowed to probe with a single output endpoint,
power management routines shall now take into account the case a CSI-2 TX
is not enabled.

Unify the adv748x_tx_power() routine to handle transparently TXA and TXB,
and enable the CSI-2 outputs conditionally.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/adv748x/adv748x-afe.c  |  2 +-
 drivers/media/i2c/adv748x/adv748x-core.c | 52 +++++++++++++-------------------
 drivers/media/i2c/adv748x/adv748x-csi2.c |  5 ---
 drivers/media/i2c/adv748x/adv748x-hdmi.c |  2 +-
 drivers/media/i2c/adv748x/adv748x.h      |  7 ++---
 5 files changed, 25 insertions(+), 43 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c b/drivers/media/i2c/adv748x/adv748x-afe.c
index edd25e8..6d78105 100644
--- a/drivers/media/i2c/adv748x/adv748x-afe.c
+++ b/drivers/media/i2c/adv748x/adv748x-afe.c
@@ -286,7 +286,7 @@ static int adv748x_afe_s_stream(struct v4l2_subdev *sd, int enable)
 			goto unlock;
 	}
 
-	ret = adv748x_txb_power(state, enable);
+	ret = adv748x_tx_power(&state->txb, enable);
 	if (ret)
 		goto unlock;
 
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 65c3024..72a6692 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -292,33 +292,16 @@ static const struct adv748x_reg_value adv748x_power_down_txb_1lane[] = {
 	{ADV748X_PAGE_EOR, 0xff, 0xff}	/* End of register table */
 };
 
-int adv748x_txa_power(struct adv748x_state *state, bool on)
+int adv748x_tx_power(struct adv748x_csi2 *tx, bool on)
 {
+	struct adv748x_state *state = tx->state;
+	const struct adv748x_reg_value *reglist;
 	int val;
 
-	val = txa_read(state, ADV748X_CSI_FS_AS_LS);
-	if (val < 0)
-		return val;
-
-	/*
-	 * This test against BIT(6) is not documented by the datasheet, but was
-	 * specified in the downstream driver.
-	 * Track with a WARN_ONCE to determine if it is ever set by HW.
-	 */
-	WARN_ONCE((on && val & ADV748X_CSI_FS_AS_LS_UNKNOWN),
-			"Enabling with unknown bit set");
-
-	if (on)
-		return adv748x_write_regs(state, adv748x_power_up_txa_4lane);
-
-	return adv748x_write_regs(state, adv748x_power_down_txa_4lane);
-}
-
-int adv748x_txb_power(struct adv748x_state *state, bool on)
-{
-	int val;
+	if (!is_tx_enabled(tx))
+		return 0;
 
-	val = txb_read(state, ADV748X_CSI_FS_AS_LS);
+	val = tx_read(tx, ADV748X_CSI_FS_AS_LS);
 	if (val < 0)
 		return val;
 
@@ -331,9 +314,13 @@ int adv748x_txb_power(struct adv748x_state *state, bool on)
 			"Enabling with unknown bit set");
 
 	if (on)
-		return adv748x_write_regs(state, adv748x_power_up_txb_1lane);
+		reglist = is_txa(tx) ? adv748x_power_up_txa_4lane :
+				       adv748x_power_up_txb_1lane;
+	else
+		reglist = is_txa(tx) ? adv748x_power_down_txa_4lane :
+				       adv748x_power_down_txb_1lane;
 
-	return adv748x_write_regs(state, adv748x_power_down_txb_1lane);
+	return adv748x_write_regs(state, reglist);
 }
 
 /* -----------------------------------------------------------------------------
@@ -482,6 +469,7 @@ static const struct adv748x_reg_value adv748x_init_txb_1lane[] = {
 static int adv748x_reset(struct adv748x_state *state)
 {
 	int ret;
+	u8 regval = ADV748X_IO_10_PIX_OUT_EN;
 
 	ret = adv748x_write_regs(state, adv748x_sw_reset);
 	if (ret < 0)
@@ -496,22 +484,24 @@ static int adv748x_reset(struct adv748x_state *state)
 	if (ret)
 		return ret;
 
-	adv748x_txa_power(state, 0);
+	adv748x_tx_power(&state->txa, 0);
 
 	/* Init and power down TXB */
 	ret = adv748x_write_regs(state, adv748x_init_txb_1lane);
 	if (ret)
 		return ret;
 
-	adv748x_txb_power(state, 0);
+	adv748x_tx_power(&state->txb, 0);
 
 	/* Disable chip powerdown & Enable HDMI Rx block */
 	io_write(state, ADV748X_IO_PD, ADV748X_IO_PD_RX_EN);
 
-	/* Enable 4-lane CSI Tx & Pixel Port */
-	io_write(state, ADV748X_IO_10, ADV748X_IO_10_CSI4_EN |
-				       ADV748X_IO_10_CSI1_EN |
-				       ADV748X_IO_10_PIX_OUT_EN);
+	/* Conditionally enable TXa and TXb. */
+	if (is_tx_enabled(&state->txa))
+		regval |= ADV748X_IO_10_CSI4_EN;
+	if (is_tx_enabled(&state->txb))
+		regval |= ADV748X_IO_10_CSI1_EN;
+	io_write(state, ADV748X_IO_10, regval);
 
 	/* Use vid_std and v_freq as freerun resolution for CP */
 	cp_clrset(state, ADV748X_CP_CLMP_POS, ADV748X_CP_CLMP_POS_DIS_AUTO,
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 556e13c..034fd93 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -18,11 +18,6 @@
 
 #include "adv748x.h"
 
-static bool is_txa(struct adv748x_csi2 *tx)
-{
-	return tx == &tx->state->txa;
-}
-
 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 aecc2a8..abb6568 100644
--- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
+++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
@@ -362,7 +362,7 @@ static int adv748x_hdmi_s_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&state->mutex);
 
-	ret = adv748x_txa_power(state, enable);
+	ret = adv748x_tx_power(&state->txa, enable);
 	if (ret)
 		goto done;
 
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index 1cf46c40..eeadf05 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -93,6 +93,7 @@ struct adv748x_csi2 {
 #define notifier_to_csi2(n) container_of(n, struct adv748x_csi2, notifier)
 #define adv748x_sd_to_csi2(sd) container_of(sd, struct adv748x_csi2, sd)
 #define is_tx_enabled(_tx) ((_tx)->state->endpoints[(_tx)->port] != NULL)
+#define is_txa(_tx) ((_tx) == &(_tx)->state->txa)
 
 enum adv748x_hdmi_pads {
 	ADV748X_HDMI_SINK,
@@ -378,9 +379,6 @@ int adv748x_write_block(struct adv748x_state *state, int client_page,
 #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 txa_read(s, r) adv748x_read(s, ADV748X_PAGE_TXA, r)
-#define txb_read(s, r) adv748x_read(s, ADV748X_PAGE_TXB, r)
-
 #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)
 
@@ -400,8 +398,7 @@ void adv748x_subdev_init(struct v4l2_subdev *sd, struct adv748x_state *state,
 int adv748x_register_subdevs(struct adv748x_state *state,
 			     struct v4l2_device *v4l2_dev);
 
-int adv748x_txa_power(struct adv748x_state *state, bool on);
-int adv748x_txb_power(struct adv748x_state *state, bool on);
+int adv748x_tx_power(struct adv748x_csi2 *tx, bool on);
 
 int adv748x_afe_init(struct adv748x_afe *afe);
 void adv748x_afe_cleanup(struct adv748x_afe *afe);
-- 
2.7.4

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

* [PATCH v3 3/4] media: i2c: adv748x: Conditionally enable only CSI-2 outputs
  2018-09-17 11:30 [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Jacopo Mondi
  2018-09-17 11:30 ` [PATCH v3 1/4] media: i2c: adv748x: Support probing a single output Jacopo Mondi
  2018-09-17 11:30 ` [PATCH v3 2/4] media: i2c: adv748x: Handle TX[A|B] power management Jacopo Mondi
@ 2018-09-17 11:30 ` Jacopo Mondi
  2018-09-17 11:30 ` [PATCH v3 4/4] media: i2c: adv748x: Register only enabled inputs Jacopo Mondi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jacopo Mondi @ 2018-09-17 11:30 UTC (permalink / raw)
  To: laurent.pinchart, kieran.bingham+renesas, niklas.soderlund+renesas
  Cc: Jacopo Mondi, linux-renesas-soc, linux-media

The ADV748x has two CSI-2 output port and one TTL input/output port for
digital video reception/transmission. The TTL digital pad is unconditionally
enabled during the device reset even if not used. Same goes for the TXA
and TXB CSI-2 outputs, which are enabled by the initial settings blob
programmed into the chip.

In order to improve power saving, do not enable unused output interfaces:
keep TTL output disabled, as it is not used, and drop CSI-2 output enabling
from the initial settings list, as they get conditionally enabled later.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/adv748x/adv748x-core.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 72a6692..7b79b0c 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -386,8 +386,6 @@ static const struct adv748x_reg_value adv748x_init_txa_4lane[] = {
 
 	{ADV748X_PAGE_IO, 0x0c, 0xe0},	/* Enable LLC_DLL & Double LLC Timing */
 	{ADV748X_PAGE_IO, 0x0e, 0xdd},	/* LLC/PIX/SPI PINS TRISTATED AUD */
-	/* Outputs Enabled */
-	{ADV748X_PAGE_IO, 0x10, 0xa0},	/* Enable 4-lane CSI Tx & Pixel Port */
 
 	{ADV748X_PAGE_TXA, 0x00, 0x84},	/* Enable 4-lane MIPI */
 	{ADV748X_PAGE_TXA, 0x00, 0xa4},	/* Set Auto DPHY Timing */
@@ -441,10 +439,6 @@ static const struct adv748x_reg_value adv748x_init_txb_1lane[] = {
 	{ADV748X_PAGE_SDP, 0x31, 0x12},	/* ADI Required Write */
 	{ADV748X_PAGE_SDP, 0xe6, 0x4f},  /* V bit end pos manually in NTSC */
 
-	/* Enable 1-Lane MIPI Tx, */
-	/* enable pixel output and route SD through Pixel port */
-	{ADV748X_PAGE_IO, 0x10, 0x70},
-
 	{ADV748X_PAGE_TXB, 0x00, 0x81},	/* Enable 1-lane MIPI */
 	{ADV748X_PAGE_TXB, 0x00, 0xa1},	/* Set Auto DPHY Timing */
 	{ADV748X_PAGE_TXB, 0xd2, 0x40},	/* ADI Required Write */
@@ -469,7 +463,7 @@ static const struct adv748x_reg_value adv748x_init_txb_1lane[] = {
 static int adv748x_reset(struct adv748x_state *state)
 {
 	int ret;
-	u8 regval = ADV748X_IO_10_PIX_OUT_EN;
+	u8 regval = 0;
 
 	ret = adv748x_write_regs(state, adv748x_sw_reset);
 	if (ret < 0)
-- 
2.7.4

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

* [PATCH v3 4/4] media: i2c: adv748x: Register only enabled inputs
  2018-09-17 11:30 [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Jacopo Mondi
                   ` (2 preceding siblings ...)
  2018-09-17 11:30 ` [PATCH v3 3/4] media: i2c: adv748x: Conditionally enable only CSI-2 outputs Jacopo Mondi
@ 2018-09-17 11:30 ` Jacopo Mondi
  2018-09-17 15:14 ` [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Laurent Pinchart
  2018-09-26 13:47 ` Kieran Bingham
  5 siblings, 0 replies; 7+ messages in thread
From: Jacopo Mondi @ 2018-09-17 11:30 UTC (permalink / raw)
  To: laurent.pinchart, kieran.bingham+renesas, niklas.soderlund+renesas
  Cc: Jacopo Mondi, linux-renesas-soc, linux-media

The adv748x assumes input endpoints are always enabled, and registers
a subdevice for each of them when the corresponding output subdevice
is registered.

Fix this by conditionally registering the input subdevice only if it is
actually described in device tree.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/adv748x/adv748x-csi2.c |  6 +++---
 drivers/media/i2c/adv748x/adv748x.h      | 10 ++++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 034fd93..9e9df51 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -82,15 +82,15 @@ static int adv748x_csi2_registered(struct v4l2_subdev *sd)
 	 *
 	 * Link HDMI->TXA, and AFE->TXB directly.
 	 */
-	if (is_txa(tx)) {
+	if (is_txa(tx) && is_hdmi_enabled(state))
 		return adv748x_csi2_register_link(tx, sd->v4l2_dev,
 						  &state->hdmi.sd,
 						  ADV748X_HDMI_SOURCE);
-	} else {
+	if (!is_txa(tx) && is_afe_enabled(state))
 		return adv748x_csi2_register_link(tx, sd->v4l2_dev,
 						  &state->afe.sd,
 						  ADV748X_AFE_SOURCE);
-	}
+	return 0;
 }
 
 static const struct v4l2_subdev_internal_ops adv748x_csi2_internal_ops = {
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index eeadf05..a34004e 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -94,6 +94,16 @@ struct adv748x_csi2 {
 #define adv748x_sd_to_csi2(sd) container_of(sd, struct adv748x_csi2, sd)
 #define is_tx_enabled(_tx) ((_tx)->state->endpoints[(_tx)->port] != NULL)
 #define is_txa(_tx) ((_tx) == &(_tx)->state->txa)
+#define is_afe_enabled(_state)					\
+	((_state)->endpoints[ADV748X_PORT_AIN0] != NULL ||	\
+	 (_state)->endpoints[ADV748X_PORT_AIN1] != NULL ||	\
+	 (_state)->endpoints[ADV748X_PORT_AIN2] != NULL ||	\
+	 (_state)->endpoints[ADV748X_PORT_AIN3] != NULL ||	\
+	 (_state)->endpoints[ADV748X_PORT_AIN4] != NULL ||	\
+	 (_state)->endpoints[ADV748X_PORT_AIN5] != NULL ||	\
+	 (_state)->endpoints[ADV748X_PORT_AIN6] != NULL ||	\
+	 (_state)->endpoints[ADV748X_PORT_AIN7] != NULL)
+#define is_hdmi_enabled(_state) ((_state)->endpoints[ADV748X_PORT_HDMI] != NULL)
 
 enum adv748x_hdmi_pads {
 	ADV748X_HDMI_SINK,
-- 
2.7.4

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

* Re: [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint
  2018-09-17 11:30 [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Jacopo Mondi
                   ` (3 preceding siblings ...)
  2018-09-17 11:30 ` [PATCH v3 4/4] media: i2c: adv748x: Register only enabled inputs Jacopo Mondi
@ 2018-09-17 15:14 ` Laurent Pinchart
  2018-09-26 13:47 ` Kieran Bingham
  5 siblings, 0 replies; 7+ messages in thread
From: Laurent Pinchart @ 2018-09-17 15:14 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: kieran.bingham+renesas, niklas.soderlund+renesas,
	linux-renesas-soc, linux-media

Hi Jacopo,

Thank you for the patches.

On Monday, 17 September 2018 14:30:53 EEST Jacopo Mondi wrote:
> Hello Laurent, Kieran, Niklas,
>    to address the Ebisu board use case, this series allows the adv748x
> driver to probe with a single output connection defined.
> 
> Compared to v2, I have dropped the last patch, as without any dynamic
> routing support it is not that helpful, and I've fixed most of commit
> messages as suggested by Kieran.
> 
> I have tested in 3 conditions on Salvator-X M3-W:
> - AFE input not registered
> - TXB not registered (Ebisu use case)
> - AFE and TXB not registered
> 
> Let me know if I can help testing this on Ebisu.

For the whole series,

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

on Ebisu with your "[PATCH/RFT v2 0/8] arm64: dts: renesas: Ebisu: Add HDMI 
and CVBS input" patches.

The driver now probes properly and I can capture frames, but they're all black 
:-S I don't think that's related to this series though.

> v2 -> v3:
> - Drop v2 patch [5/5]
> - Add Kieran's tags and modify commit messages as he suggested
> 
> Jacopo Mondi (4):
>   media: i2c: adv748x: Support probing a single output
>   media: i2c: adv748x: Handle TX[A|B] power management
>   media: i2c: adv748x: Conditionally enable only CSI-2 outputs
>   media: i2c: adv748x: Register only enabled inputs
> 
>  drivers/media/i2c/adv748x/adv748x-afe.c  |  2 +-
>  drivers/media/i2c/adv748x/adv748x-core.c | 83  +++++++++++++++-------------
>  drivers/media/i2c/adv748x/adv748x-csi2.c | 29 ++++-------
>  drivers/media/i2c/adv748x/adv748x-hdmi.c |  2 +-
>  drivers/media/i2c/adv748x/adv748x.h      | 19 ++++++--
>  5 files changed, 68 insertions(+), 67 deletions(-)

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint
  2018-09-17 11:30 [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Jacopo Mondi
                   ` (4 preceding siblings ...)
  2018-09-17 15:14 ` [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Laurent Pinchart
@ 2018-09-26 13:47 ` Kieran Bingham
  5 siblings, 0 replies; 7+ messages in thread
From: Kieran Bingham @ 2018-09-26 13:47 UTC (permalink / raw)
  To: Jacopo Mondi, laurent.pinchart, niklas.soderlund+renesas
  Cc: linux-renesas-soc, linux-media

Hi Jacopo,

On 17/09/18 12:30, Jacopo Mondi wrote:
> Hello Laurent, Kieran, Niklas,
>    to address the Ebisu board use case, this series allows the adv748x driver
> to probe with a single output connection defined.
> 
> Compared to v2, I have dropped the last patch, as without any dynamic routing
> support it is not that helpful, and I've fixed most of commit messages as
> suggested by Kieran.

Thank you,

I've applied all of these patches to my tree and submitted as a pull
request to Hans.

--
Regards

Kieran


> 
> I have tested in 3 conditions on Salvator-X M3-W:
> - AFE input not registered
> - TXB not registered (Ebisu use case)
> - AFE and TXB not registered
> 
> Let me know if I can help testing this on Ebisu.
> 
> Thanks
>    j
> 
> v2 -> v3:
> - Drop v2 patch [5/5]
> - Add Kieran's tags and modify commit messages as he suggested
> 
> Jacopo Mondi (4):
>   media: i2c: adv748x: Support probing a single output
>   media: i2c: adv748x: Handle TX[A|B] power management
>   media: i2c: adv748x: Conditionally enable only CSI-2 outputs
>   media: i2c: adv748x: Register only enabled inputs
> 
>  drivers/media/i2c/adv748x/adv748x-afe.c  |  2 +-
>  drivers/media/i2c/adv748x/adv748x-core.c | 83 +++++++++++++++++---------------
>  drivers/media/i2c/adv748x/adv748x-csi2.c | 29 ++++-------
>  drivers/media/i2c/adv748x/adv748x-hdmi.c |  2 +-
>  drivers/media/i2c/adv748x/adv748x.h      | 19 ++++++--
>  5 files changed, 68 insertions(+), 67 deletions(-)
> 
> --
> 2.7.4
> 

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

end of thread, other threads:[~2018-09-26 20:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 11:30 [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Jacopo Mondi
2018-09-17 11:30 ` [PATCH v3 1/4] media: i2c: adv748x: Support probing a single output Jacopo Mondi
2018-09-17 11:30 ` [PATCH v3 2/4] media: i2c: adv748x: Handle TX[A|B] power management Jacopo Mondi
2018-09-17 11:30 ` [PATCH v3 3/4] media: i2c: adv748x: Conditionally enable only CSI-2 outputs Jacopo Mondi
2018-09-17 11:30 ` [PATCH v3 4/4] media: i2c: adv748x: Register only enabled inputs Jacopo Mondi
2018-09-17 15:14 ` [PATCH v3 0/4] media: adv748x: Allow probe with a single output endpoint Laurent Pinchart
2018-09-26 13:47 ` Kieran Bingham

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).