[PATCH 1/5] [media] tc358743: register v4l2 asynchronous subdevice

[PATCH 1/5] [media] tc358743: register v4l2 asynchronous subdevice

[Philipp Zabel]

Add support for registering the sensor subdevice using the v4l2-async API.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/i2c/tc358743.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 34d4f32..48d1575 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1710,6 +1710,16 @@ static int tc358743_probe(struct i2c_client *client,
 		goto err_hdl;
 	}
 
+	state->pad.flags = MEDIA_PAD_FL_SOURCE;
+	err = media_entity_init(&sd->entity, 1, &state->pad, 0);
+	if (err < 0)
+		goto err_hdl;
+
+	sd->dev = &client->dev;
+	err = v4l2_async_register_subdev(sd);
+	if (err < 0)
+		goto err_hdl;
+
 	mutex_init(&state->confctl_mutex);
 
 	INIT_DELAYED_WORK(&state->delayed_work_enable_hotplug,
@@ -1740,6 +1750,7 @@ err_work_queues:
 	destroy_workqueue(state->work_queues);
 	mutex_destroy(&state->confctl_mutex);
 err_hdl:
+	media_entity_cleanup(&sd->entity);
 	v4l2_ctrl_handler_free(&state->hdl);
 	return err;
 }
@@ -1751,6 +1762,7 @@ static int tc358743_remove(struct i2c_client *client)
 
 	cancel_delayed_work(&state->delayed_work_enable_hotplug);
 	destroy_workqueue(state->work_queues);
+	v4l2_async_unregister_subdev(sd);
 	v4l2_device_unregister_subdev(sd);
 	mutex_destroy(&state->confctl_mutex);
 	v4l2_ctrl_handler_free(&state->hdl);
-- 
2.1.4

[PATCH 2/5] [media] tc358743: enable v4l2 subdevice devnode

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/i2c/tc358743.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 48d1575..0be6d9f 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1668,7 +1668,7 @@ static int tc358743_probe(struct i2c_client *client,
 	state->i2c_client = client;
 	sd = &state->sd;
 	v4l2_i2c_subdev_init(sd, client, &tc358743_ops);
-	sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
 
 	/* i2c access */
 	if ((i2c_rd16(sd, CHIPID) & MASK_CHIPID) != 0) {
-- 
2.1.4

Re: [PATCH 2/5] [media] tc358743: enable v4l2 subdevice devnode

[Hans Verkuil]

On 07/10/2015 03:11 PM, Philipp Zabel wrote:
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/i2c/tc358743.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 48d1575..0be6d9f 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1668,7 +1668,7 @@ static int tc358743_probe(struct i2c_client *client,
>  	state->i2c_client = client;
>  	sd = &state->sd;
>  	v4l2_i2c_subdev_init(sd, client, &tc358743_ops);
> -	sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS;
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;

Note: HAS_EVENTS won't do much since there are no .subscribe_event or
.unsubscribe_event ops. You should add those.

Regards,

	Hans

>  
>  	/* i2c access */
>  	if ((i2c_rd16(sd, CHIPID) & MASK_CHIPID) != 0) {
> 

[PATCH 3/5] [media] tc358743: support probe from device tree

[Philipp Zabel]

Add support for probing the TC358743 subdevice from device tree.
The reference clock must be supplied using the common clock bindings.
MIPI CSI-2 specific properties are parsed from the OF graph endpoint
node and support for a non-continuous MIPI CSI-2 clock is added.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 .../devicetree/bindings/media/i2c/tc358743.txt     |  48 +++++++
 drivers/media/i2c/tc358743.c                       | 153 ++++++++++++++++++++-
 2 files changed, 195 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/tc358743.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/tc358743.txt b/Documentation/devicetree/bindings/media/i2c/tc358743.txt
new file mode 100644
index 0000000..5218921
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/tc358743.txt
@@ -0,0 +1,48 @@
+* Toshiba TC358743 HDMI-RX to MIPI CSI2-TX Bridge
+
+The Toshiba TC358743 HDMI-RX to MIPI CSI2-TX (H2C) is a bridge that converts
+a HDMI stream to MIPI CSI-2 TX. It is programmable through I2C.
+
+Required Properties:
+
+- compatible: value should be "toshiba,tc358743"
+- clocks, clock-names: should contain a phandle link to the reference clock
+		       source, the clock input is named "refclk".
+
+Optional Properties:
+
+- reset-gpios: gpio phandle GPIO connected to the reset pin
+- interrupts, interrupt-parent: GPIO connected to the interrupt pin
+- data-lanes: should be <1 2 3 4> for four-lane operation,
+	      or <1 2> for two-lane operation
+- clock-lanes: should be <0>
+- clock-noncontinuous: Presence of this boolean property decides whether the
+		       MIPI CSI-2 clock is continuous or non-continuous.
+- link-frequencies: List of allowed link frequencies in Hz. Each frequency is
+		    expressed as a 64-bit big-endian integer. The frequency
+		    is half of the bps per lane due to DDR transmission.
+
+For further information on the MIPI CSI-2 endpoint node properties, see
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+	tc358743@0f {
+		compatible = "toshiba,tc358743";
+		reg = <0x0f>;
+		clocks = <&hdmi_osc>;
+		clock-names = "refclk";
+		reset-gpios = <&gpio6 9 GPIO_ACTIVE_LOW>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+		port {
+			tc358743_out: endpoint {
+				remote-endpoint = <&mipi_csi2_in>;
+				data-lanes = <1 2 3 4>;
+				clock-lanes = <0>;
+				clock-noncontinuous;
+				link-frequencies = /bits/ 64 <297000000>;
+			};
+		};
+	};
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 0be6d9f..02c60b3 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -29,7 +29,9 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/videodev2.h>
 #include <linux/workqueue.h>
 #include <linux/v4l2-dv-timings.h>
@@ -37,6 +39,7 @@
 #include <media/v4l2-dv-timings.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-of.h>
 #include <media/tc358743.h>
 
 #include "tc358743_regs.h"
@@ -69,6 +72,7 @@ static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
 
 struct tc358743_state {
 	struct tc358743_platform_data pdata;
+	struct v4l2_of_bus_mipi_csi2 bus;
 	struct v4l2_subdev sd;
 	struct media_pad pad;
 	struct v4l2_ctrl_handler hdl;
@@ -90,6 +94,8 @@ struct tc358743_state {
 
 	struct v4l2_dv_timings timings;
 	u32 mbus_fmt_code;
+
+	struct gpio_desc *reset_gpio;
 };
 
 static void tc358743_enable_interrupts(struct v4l2_subdev *sd,
@@ -700,7 +706,8 @@ static void tc358743_set_csi(struct v4l2_subdev *sd)
 			((lanes > 2) ? MASK_D2M_HSTXVREGEN : 0x0) |
 			((lanes > 3) ? MASK_D3M_HSTXVREGEN : 0x0));
 
-	i2c_wr32(sd, TXOPTIONCNTRL, MASK_CONTCLKMODE);
+	i2c_wr32(sd, TXOPTIONCNTRL, (state->bus.flags &
+		 V4L2_MBUS_CSI2_CONTINUOUS_CLOCK) ? MASK_CONTCLKMODE : 0);
 	i2c_wr32(sd, STARTCNTRL, MASK_START);
 	i2c_wr32(sd, CSI_START, MASK_STRT);
 
@@ -1638,6 +1645,135 @@ static const struct v4l2_ctrl_config tc358743_ctrl_audio_present = {
 
 /* --------------- PROBE / REMOVE --------------- */
 
+#if CONFIG_OF
+static void tc358743_gpio_reset(struct tc358743_state *state)
+{
+	gpiod_set_value(state->reset_gpio, 0);
+	usleep_range(5000, 10000);
+	gpiod_set_value(state->reset_gpio, 1);
+	usleep_range(1000, 2000);
+	gpiod_set_value(state->reset_gpio, 0);
+	msleep(20);
+}
+
+static int tc358743_probe_of(struct tc358743_state *state)
+{
+	struct device *dev = &state->i2c_client->dev;
+	struct device_node *np = dev->of_node;
+	struct v4l2_of_endpoint *endpoint;
+	struct device_node *ep;
+	struct clk *refclk;
+	u32 bps_pr_lane;
+	int ret = -EINVAL;
+
+	refclk = devm_clk_get(dev, "refclk");
+	if (IS_ERR(refclk)) {
+		if (PTR_ERR(refclk) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get refclk: %ld\n",
+				PTR_ERR(refclk));
+		return PTR_ERR(refclk);
+	}
+
+	ep = of_graph_get_next_endpoint(dev->of_node, NULL);
+	if (!ep) {
+		dev_err(dev, "missing endpoint node\n");
+		return -EINVAL;
+	}
+
+	endpoint = v4l2_of_alloc_parse_endpoint(ep);
+	if (IS_ERR(endpoint)) {
+		dev_err(dev, "failed to parse endpoint\n");
+		return PTR_ERR(endpoint);
+	}
+
+	if (endpoint->bus_type != V4L2_MBUS_CSI2 ||
+	    endpoint->bus.mipi_csi2.num_data_lanes == 0 ||
+	    endpoint->nr_of_link_frequencies == 0) {
+		dev_err(dev, "missing CSI-2 properties in endpoint\n");
+		goto free_endpoint;
+	}
+
+	clk_prepare_enable(refclk);
+
+	state->pdata.refclk_hz = clk_get_rate(refclk);
+	state->pdata.ddc5v_delay = DDC5V_DELAY_100_MS;
+	state->pdata.enable_hdcp = false;
+	/* A FIFO level of 16 should be enough for 2-lane 720p60 at 594 MHz. */
+	state->pdata.fifo_level = 16;
+	/*
+	 * The PLL input clock is obtained by dividing refclk by pll_prd.
+	 * It must be between 6 MHz and 40 MHz, lower frequency is better.
+	 */
+	switch (state->pdata.refclk_hz) {
+	case 26000000:
+	case 27000000:
+	case 42000000:
+		state->pdata.pll_prd = state->pdata.refclk_hz / 6000000;
+		break;
+	default:
+		dev_err(dev, "unsupported refclk rate: %u Hz\n",
+			state->pdata.refclk_hz);
+		goto disable_clk;
+	}
+
+	/*
+	 * The CSI bps per lane must be between 62.5 Mbps and 1 Gbps.
+	 * The default is 594 Mbps for 4-lane 1080p60 or 2-lane 720p60.
+	 */
+	bps_pr_lane = 2 * endpoint->link_frequencies[0];
+	if (bps_pr_lane < 62500000U || bps_pr_lane > 1000000000U) {
+		dev_err(dev, "unsupported bps per lane: %u bps\n", bps_pr_lane);
+		goto disable_clk;
+	}
+
+	/* The CSI speed per lane is refclk / pll_prd * pll_fbd */
+	state->pdata.pll_fbd = bps_pr_lane /
+			       state->pdata.refclk_hz * state->pdata.pll_prd;
+
+	/*
+	 * FIXME: These timings are from REF_02 for 594 Mbps per lane (297 MHz
+	 * link frequency). In principle it should be possible to calculate
+	 * them based on link frequency and resolution.
+	 */
+	if (bps_pr_lane != 594000000U)
+		dev_warn(dev, "untested bps per lane: %u bps\n", bps_pr_lane);
+	state->pdata.lineinitcnt = 0xe80;
+	state->pdata.lptxtimecnt = 0x003;
+	/* tclk-preparecnt: 3, tclk-zerocnt: 20 */
+	state->pdata.tclk_headercnt = 0x1403;
+	state->pdata.tclk_trailcnt = 0x00;
+	/* ths-preparecnt: 3, ths-zerocnt: 1 */
+	state->pdata.ths_headercnt = 0x0103;
+	state->pdata.twakeup = 0x4882;
+	state->pdata.tclk_postcnt = 0x008;
+	state->pdata.ths_trailcnt = 0x2;
+	state->pdata.hstxvregcnt = 0;
+
+	state->reset_gpio = devm_gpiod_get(dev, "reset");
+	if (IS_ERR(state->reset_gpio)) {
+		dev_err(dev, "failed to get reset gpio\n");
+		ret = PTR_ERR(state->reset_gpio);
+		goto disable_clk;
+	}
+
+	tc358743_gpio_reset(state);
+
+	ret = 0;
+	goto free_endpoint;
+
+disable_clk:
+	clk_disable_unprepare(refclk);
+free_endpoint:
+	v4l2_of_free_endpoint(endpoint);
+	return ret;
+}
+#else
+static inline int tc358743_probe_of(struct tc358743_state *state)
+{
+	return -ENODEV;
+}
+#endif
+
 static int tc358743_probe(struct i2c_client *client,
 			  const struct i2c_device_id *id)
 {
@@ -1658,14 +1794,19 @@ static int tc358743_probe(struct i2c_client *client,
 	if (!state)
 		return -ENOMEM;
 
+	state->i2c_client = client;
+
 	/* platform data */
-	if (!pdata) {
-		v4l_err(client, "No platform data!\n");
-		return -ENODEV;
+	if (pdata) {
+		state->pdata = *pdata;
+	} else {
+		err = tc358743_probe_of(state);
+		if (err == -ENODEV)
+			v4l_err(client, "No platform data!\n");
+		if (err)
+			return err;
 	}
-	state->pdata = *pdata;
 
-	state->i2c_client = client;
 	sd = &state->sd;
 	v4l2_i2c_subdev_init(sd, client, &tc358743_ops);
 	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
-- 
2.1.4

Re: [PATCH 3/5] [media] tc358743: support probe from device tree

[Hans Verkuil]

On 07/10/2015 03:11 PM, Philipp Zabel wrote:
> Add support for probing the TC358743 subdevice from device tree.
> The reference clock must be supplied using the common clock bindings.
> MIPI CSI-2 specific properties are parsed from the OF graph endpoint
> node and support for a non-continuous MIPI CSI-2 clock is added.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  .../devicetree/bindings/media/i2c/tc358743.txt     |  48 +++++++
>  drivers/media/i2c/tc358743.c                       | 153 ++++++++++++++++++++-
>  2 files changed, 195 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/tc358743.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/tc358743.txt b/Documentation/devicetree/bindings/media/i2c/tc358743.txt
> new file mode 100644
> index 0000000..5218921
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/tc358743.txt
> @@ -0,0 +1,48 @@
> +* Toshiba TC358743 HDMI-RX to MIPI CSI2-TX Bridge
> +
> +The Toshiba TC358743 HDMI-RX to MIPI CSI2-TX (H2C) is a bridge that converts
> +a HDMI stream to MIPI CSI-2 TX. It is programmable through I2C.
> +
> +Required Properties:
> +
> +- compatible: value should be "toshiba,tc358743"
> +- clocks, clock-names: should contain a phandle link to the reference clock
> +		       source, the clock input is named "refclk".
> +
> +Optional Properties:
> +
> +- reset-gpios: gpio phandle GPIO connected to the reset pin
> +- interrupts, interrupt-parent: GPIO connected to the interrupt pin
> +- data-lanes: should be <1 2 3 4> for four-lane operation,
> +	      or <1 2> for two-lane operation
> +- clock-lanes: should be <0>
> +- clock-noncontinuous: Presence of this boolean property decides whether the
> +		       MIPI CSI-2 clock is continuous or non-continuous.
> +- link-frequencies: List of allowed link frequencies in Hz. Each frequency is
> +		    expressed as a 64-bit big-endian integer. The frequency
> +		    is half of the bps per lane due to DDR transmission.
> +
> +For further information on the MIPI CSI-2 endpoint node properties, see
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Example:
> +
> +	tc358743@0f {
> +		compatible = "toshiba,tc358743";
> +		reg = <0x0f>;
> +		clocks = <&hdmi_osc>;
> +		clock-names = "refclk";
> +		reset-gpios = <&gpio6 9 GPIO_ACTIVE_LOW>;
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		port {
> +			tc358743_out: endpoint {
> +				remote-endpoint = <&mipi_csi2_in>;
> +				data-lanes = <1 2 3 4>;
> +				clock-lanes = <0>;
> +				clock-noncontinuous;
> +				link-frequencies = /bits/ 64 <297000000>;
> +			};
> +		};
> +	};
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 0be6d9f..02c60b3 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -29,7 +29,9 @@
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
> +#include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/videodev2.h>
>  #include <linux/workqueue.h>
>  #include <linux/v4l2-dv-timings.h>
> @@ -37,6 +39,7 @@
>  #include <media/v4l2-dv-timings.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-of.h>
>  #include <media/tc358743.h>
>  
>  #include "tc358743_regs.h"
> @@ -69,6 +72,7 @@ static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
>  
>  struct tc358743_state {
>  	struct tc358743_platform_data pdata;
> +	struct v4l2_of_bus_mipi_csi2 bus;

Where is this bus struct set?

>  	struct v4l2_subdev sd;
>  	struct media_pad pad;
>  	struct v4l2_ctrl_handler hdl;
> @@ -90,6 +94,8 @@ struct tc358743_state {
>  
>  	struct v4l2_dv_timings timings;
>  	u32 mbus_fmt_code;
> +
> +	struct gpio_desc *reset_gpio;
>  };
>  
>  static void tc358743_enable_interrupts(struct v4l2_subdev *sd,
> @@ -700,7 +706,8 @@ static void tc358743_set_csi(struct v4l2_subdev *sd)
>  			((lanes > 2) ? MASK_D2M_HSTXVREGEN : 0x0) |
>  			((lanes > 3) ? MASK_D3M_HSTXVREGEN : 0x0));
>  
> -	i2c_wr32(sd, TXOPTIONCNTRL, MASK_CONTCLKMODE);
> +	i2c_wr32(sd, TXOPTIONCNTRL, (state->bus.flags &
> +		 V4L2_MBUS_CSI2_CONTINUOUS_CLOCK) ? MASK_CONTCLKMODE : 0);

It's used here.

BTW, since I don't see state->bus being set, that means bus.flags == 0 and
so this register is now set to 0 instead of MASK_CONTCLKMODE.

When using platform data I guess bus.flags should be set to
V4L2_MBUS_CSI2_CONTINUOUS_CLOCK to prevent breakage.

>  	i2c_wr32(sd, STARTCNTRL, MASK_START);
>  	i2c_wr32(sd, CSI_START, MASK_STRT);
>  
> @@ -1638,6 +1645,135 @@ static const struct v4l2_ctrl_config tc358743_ctrl_audio_present = {
>  
>  /* --------------- PROBE / REMOVE --------------- */
>  
> +#if CONFIG_OF
> +static void tc358743_gpio_reset(struct tc358743_state *state)
> +{
> +	gpiod_set_value(state->reset_gpio, 0);
> +	usleep_range(5000, 10000);
> +	gpiod_set_value(state->reset_gpio, 1);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value(state->reset_gpio, 0);
> +	msleep(20);
> +}
> +
> +static int tc358743_probe_of(struct tc358743_state *state)
> +{
> +	struct device *dev = &state->i2c_client->dev;
> +	struct device_node *np = dev->of_node;
> +	struct v4l2_of_endpoint *endpoint;
> +	struct device_node *ep;
> +	struct clk *refclk;
> +	u32 bps_pr_lane;
> +	int ret = -EINVAL;
> +
> +	refclk = devm_clk_get(dev, "refclk");
> +	if (IS_ERR(refclk)) {
> +		if (PTR_ERR(refclk) != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get refclk: %ld\n",
> +				PTR_ERR(refclk));
> +		return PTR_ERR(refclk);
> +	}
> +
> +	ep = of_graph_get_next_endpoint(dev->of_node, NULL);
> +	if (!ep) {
> +		dev_err(dev, "missing endpoint node\n");
> +		return -EINVAL;
> +	}
> +
> +	endpoint = v4l2_of_alloc_parse_endpoint(ep);
> +	if (IS_ERR(endpoint)) {
> +		dev_err(dev, "failed to parse endpoint\n");
> +		return PTR_ERR(endpoint);
> +	}
> +
> +	if (endpoint->bus_type != V4L2_MBUS_CSI2 ||
> +	    endpoint->bus.mipi_csi2.num_data_lanes == 0 ||
> +	    endpoint->nr_of_link_frequencies == 0) {
> +		dev_err(dev, "missing CSI-2 properties in endpoint\n");
> +		goto free_endpoint;
> +	}
> +
> +	clk_prepare_enable(refclk);
> +
> +	state->pdata.refclk_hz = clk_get_rate(refclk);
> +	state->pdata.ddc5v_delay = DDC5V_DELAY_100_MS;
> +	state->pdata.enable_hdcp = false;
> +	/* A FIFO level of 16 should be enough for 2-lane 720p60 at 594 MHz. */
> +	state->pdata.fifo_level = 16;
> +	/*
> +	 * The PLL input clock is obtained by dividing refclk by pll_prd.
> +	 * It must be between 6 MHz and 40 MHz, lower frequency is better.
> +	 */
> +	switch (state->pdata.refclk_hz) {
> +	case 26000000:
> +	case 27000000:
> +	case 42000000:
> +		state->pdata.pll_prd = state->pdata.refclk_hz / 6000000;
> +		break;
> +	default:
> +		dev_err(dev, "unsupported refclk rate: %u Hz\n",
> +			state->pdata.refclk_hz);
> +		goto disable_clk;
> +	}
> +
> +	/*
> +	 * The CSI bps per lane must be between 62.5 Mbps and 1 Gbps.
> +	 * The default is 594 Mbps for 4-lane 1080p60 or 2-lane 720p60.
> +	 */
> +	bps_pr_lane = 2 * endpoint->link_frequencies[0];
> +	if (bps_pr_lane < 62500000U || bps_pr_lane > 1000000000U) {
> +		dev_err(dev, "unsupported bps per lane: %u bps\n", bps_pr_lane);
> +		goto disable_clk;
> +	}
> +
> +	/* The CSI speed per lane is refclk / pll_prd * pll_fbd */
> +	state->pdata.pll_fbd = bps_pr_lane /
> +			       state->pdata.refclk_hz * state->pdata.pll_prd;
> +
> +	/*
> +	 * FIXME: These timings are from REF_02 for 594 Mbps per lane (297 MHz
> +	 * link frequency). In principle it should be possible to calculate
> +	 * them based on link frequency and resolution.
> +	 */
> +	if (bps_pr_lane != 594000000U)
> +		dev_warn(dev, "untested bps per lane: %u bps\n", bps_pr_lane);
> +	state->pdata.lineinitcnt = 0xe80;
> +	state->pdata.lptxtimecnt = 0x003;
> +	/* tclk-preparecnt: 3, tclk-zerocnt: 20 */
> +	state->pdata.tclk_headercnt = 0x1403;
> +	state->pdata.tclk_trailcnt = 0x00;
> +	/* ths-preparecnt: 3, ths-zerocnt: 1 */
> +	state->pdata.ths_headercnt = 0x0103;
> +	state->pdata.twakeup = 0x4882;
> +	state->pdata.tclk_postcnt = 0x008;
> +	state->pdata.ths_trailcnt = 0x2;
> +	state->pdata.hstxvregcnt = 0;
> +
> +	state->reset_gpio = devm_gpiod_get(dev, "reset");
> +	if (IS_ERR(state->reset_gpio)) {
> +		dev_err(dev, "failed to get reset gpio\n");
> +		ret = PTR_ERR(state->reset_gpio);
> +		goto disable_clk;
> +	}
> +
> +	tc358743_gpio_reset(state);
> +
> +	ret = 0;
> +	goto free_endpoint;
> +
> +disable_clk:
> +	clk_disable_unprepare(refclk);
> +free_endpoint:
> +	v4l2_of_free_endpoint(endpoint);
> +	return ret;
> +}
> +#else
> +static inline int tc358743_probe_of(struct tc358743_state *state)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
>  static int tc358743_probe(struct i2c_client *client,
>  			  const struct i2c_device_id *id)
>  {
> @@ -1658,14 +1794,19 @@ static int tc358743_probe(struct i2c_client *client,
>  	if (!state)
>  		return -ENOMEM;
>  
> +	state->i2c_client = client;
> +
>  	/* platform data */
> -	if (!pdata) {
> -		v4l_err(client, "No platform data!\n");
> -		return -ENODEV;
> +	if (pdata) {
> +		state->pdata = *pdata;
> +	} else {
> +		err = tc358743_probe_of(state);
> +		if (err == -ENODEV)
> +			v4l_err(client, "No platform data!\n");

I'd replace this with "No device tree data!" or something like that.

Regards,

	Hans

> +		if (err)
> +			return err;
>  	}
> -	state->pdata = *pdata;
>  
> -	state->i2c_client = client;
>  	sd = &state->sd;
>  	v4l2_i2c_subdev_init(sd, client, &tc358743_ops);
>  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> 

Re: [PATCH 3/5] [media] tc358743: support probe from device tree

[Philipp Zabel]

Hi Hans,

Am Montag, den 13.07.2015, 12:57 +0200 schrieb Hans Verkuil:
[...]
> > @@ -69,6 +72,7 @@ static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
> >  
> >  struct tc358743_state {
> >  	struct tc358743_platform_data pdata;
> > +	struct v4l2_of_bus_mipi_csi2 bus;
> 
> Where is this bus struct set?

Uh-oh, I have accidentally dropped setting the bus struct when switching
to v4l2_of_alloc_parse_endpoint.

[...]
> > @@ -700,7 +706,8 @@ static void tc358743_set_csi(struct v4l2_subdev *sd)
> >  			((lanes > 2) ? MASK_D2M_HSTXVREGEN : 0x0) |
> >  			((lanes > 3) ? MASK_D3M_HSTXVREGEN : 0x0));
> >  
> > -	i2c_wr32(sd, TXOPTIONCNTRL, MASK_CONTCLKMODE);
> > +	i2c_wr32(sd, TXOPTIONCNTRL, (state->bus.flags &
> > +		 V4L2_MBUS_CSI2_CONTINUOUS_CLOCK) ? MASK_CONTCLKMODE : 0);
> 
> It's used here.
> 
> BTW, since I don't see state->bus being set, that means bus.flags == 0 and
> so this register is now set to 0 instead of MASK_CONTCLKMODE.
> 
> When using platform data I guess bus.flags should be set to
> V4L2_MBUS_CSI2_CONTINUOUS_CLOCK to prevent breakage.

Ok.

[..]
> > +	/*
> > +	 * The CSI bps per lane must be between 62.5 Mbps and 1 Gbps.
> > +	 * The default is 594 Mbps for 4-lane 1080p60 or 2-lane 720p60.
> > +	 */
> > +	bps_pr_lane = 2 * endpoint->link_frequencies[0];
> > +	if (bps_pr_lane < 62500000U || bps_pr_lane > 1000000000U) {
> > +		dev_err(dev, "unsupported bps per lane: %u bps\n", bps_pr_lane);
> > +		goto disable_clk;
> > +	}
> > +
> > +	/* The CSI speed per lane is refclk / pll_prd * pll_fbd */
> > +	state->pdata.pll_fbd = bps_pr_lane /
> > +			       state->pdata.refclk_hz * state->pdata.pll_prd;
> > +
> > +	/*
> > +	 * FIXME: These timings are from REF_02 for 594 Mbps per lane (297 MHz
> > +	 * link frequency). In principle it should be possible to calculate
> > +	 * them based on link frequency and resolution.
> > +	 */
> > +	if (bps_pr_lane != 594000000U)
> > +		dev_warn(dev, "untested bps per lane: %u bps\n", bps_pr_lane);
> > +	state->pdata.lineinitcnt = 0xe80;
> > +	state->pdata.lptxtimecnt = 0x003;
> > +	/* tclk-preparecnt: 3, tclk-zerocnt: 20 */
> > +	state->pdata.tclk_headercnt = 0x1403;
> > +	state->pdata.tclk_trailcnt = 0x00;
> > +	/* ths-preparecnt: 3, ths-zerocnt: 1 */
> > +	state->pdata.ths_headercnt = 0x0103;
> > +	state->pdata.twakeup = 0x4882;
> > +	state->pdata.tclk_postcnt = 0x008;
> > +	state->pdata.ths_trailcnt = 0x2;
> > +	state->pdata.hstxvregcnt = 0;

Do you have any suggestion how to handle this? AFAIK REF_02 is not
public, and I do not know the formulas it uses internally to calculate
these timings. I wouldn't want to add all the timing parameters to the
device tree just because of that.

[...]
> > @@ -1658,14 +1794,19 @@ static int tc358743_probe(struct i2c_client *client,
> >  	if (!state)
> >  		return -ENOMEM;
> >  
> > +	state->i2c_client = client;
> > +
> >  	/* platform data */
> > -	if (!pdata) {
> > -		v4l_err(client, "No platform data!\n");
> > -		return -ENODEV;
> > +	if (pdata) {
> > +		state->pdata = *pdata;
> > +	} else {
> > +		err = tc358743_probe_of(state);
> > +		if (err == -ENODEV)
> > +			v4l_err(client, "No platform data!\n");
> 
> I'd replace this with "No device tree data!" or something like that.

I'll do that, thank you.

regards
Philipp

Re: [PATCH 3/5] [media] tc358743: support probe from device tree

[Hans Verkuil]

On 07/14/15 12:10, Philipp Zabel wrote:
> Hi Hans,
> 
> Am Montag, den 13.07.2015, 12:57 +0200 schrieb Hans Verkuil:
> [...]
>>> @@ -69,6 +72,7 @@ static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
>>>  
>>>  struct tc358743_state {
>>>  	struct tc358743_platform_data pdata;
>>> +	struct v4l2_of_bus_mipi_csi2 bus;
>>
>> Where is this bus struct set?
> 
> Uh-oh, I have accidentally dropped setting the bus struct when switching
> to v4l2_of_alloc_parse_endpoint.
> 
> [...]
>>> @@ -700,7 +706,8 @@ static void tc358743_set_csi(struct v4l2_subdev *sd)
>>>  			((lanes > 2) ? MASK_D2M_HSTXVREGEN : 0x0) |
>>>  			((lanes > 3) ? MASK_D3M_HSTXVREGEN : 0x0));
>>>  
>>> -	i2c_wr32(sd, TXOPTIONCNTRL, MASK_CONTCLKMODE);
>>> +	i2c_wr32(sd, TXOPTIONCNTRL, (state->bus.flags &
>>> +		 V4L2_MBUS_CSI2_CONTINUOUS_CLOCK) ? MASK_CONTCLKMODE : 0);
>>
>> It's used here.
>>
>> BTW, since I don't see state->bus being set, that means bus.flags == 0 and
>> so this register is now set to 0 instead of MASK_CONTCLKMODE.
>>
>> When using platform data I guess bus.flags should be set to
>> V4L2_MBUS_CSI2_CONTINUOUS_CLOCK to prevent breakage.
> 
> Ok.
> 
> [..]
>>> +	/*
>>> +	 * The CSI bps per lane must be between 62.5 Mbps and 1 Gbps.
>>> +	 * The default is 594 Mbps for 4-lane 1080p60 or 2-lane 720p60.
>>> +	 */
>>> +	bps_pr_lane = 2 * endpoint->link_frequencies[0];
>>> +	if (bps_pr_lane < 62500000U || bps_pr_lane > 1000000000U) {
>>> +		dev_err(dev, "unsupported bps per lane: %u bps\n", bps_pr_lane);
>>> +		goto disable_clk;
>>> +	}
>>> +
>>> +	/* The CSI speed per lane is refclk / pll_prd * pll_fbd */
>>> +	state->pdata.pll_fbd = bps_pr_lane /
>>> +			       state->pdata.refclk_hz * state->pdata.pll_prd;
>>> +
>>> +	/*
>>> +	 * FIXME: These timings are from REF_02 for 594 Mbps per lane (297 MHz
>>> +	 * link frequency). In principle it should be possible to calculate
>>> +	 * them based on link frequency and resolution.
>>> +	 */
>>> +	if (bps_pr_lane != 594000000U)
>>> +		dev_warn(dev, "untested bps per lane: %u bps\n", bps_pr_lane);
>>> +	state->pdata.lineinitcnt = 0xe80;
>>> +	state->pdata.lptxtimecnt = 0x003;
>>> +	/* tclk-preparecnt: 3, tclk-zerocnt: 20 */
>>> +	state->pdata.tclk_headercnt = 0x1403;
>>> +	state->pdata.tclk_trailcnt = 0x00;
>>> +	/* ths-preparecnt: 3, ths-zerocnt: 1 */
>>> +	state->pdata.ths_headercnt = 0x0103;
>>> +	state->pdata.twakeup = 0x4882;
>>> +	state->pdata.tclk_postcnt = 0x008;
>>> +	state->pdata.ths_trailcnt = 0x2;
>>> +	state->pdata.hstxvregcnt = 0;
> 
> Do you have any suggestion how to handle this? AFAIK REF_02 is not
> public, and I do not know the formulas it uses internally to calculate
> these timings. I wouldn't want to add all the timing parameters to the
> device tree just because of that.

As you said, it's not public and without the formulas there is nothing you
can do but hardcode it.

If I understand this correctly these values depend on the link frequency,
so the DT should contain the link frequency and the driver can hardcode the
values based on that. Which means that if someone needs to support a new
link frequency the driver needs to be extended for that frequency.

As long as Toshiba keeps the formulas under NDA there isn't much else you can
do.

Regards,

	Hans

> 
> [...]
>>> @@ -1658,14 +1794,19 @@ static int tc358743_probe(struct i2c_client *client,
>>>  	if (!state)
>>>  		return -ENOMEM;
>>>  
>>> +	state->i2c_client = client;
>>> +
>>>  	/* platform data */
>>> -	if (!pdata) {
>>> -		v4l_err(client, "No platform data!\n");
>>> -		return -ENODEV;
>>> +	if (pdata) {
>>> +		state->pdata = *pdata;
>>> +	} else {
>>> +		err = tc358743_probe_of(state);
>>> +		if (err == -ENODEV)
>>> +			v4l_err(client, "No platform data!\n");
>>
>> I'd replace this with "No device tree data!" or something like that.
> 
> I'll do that, thank you.
> 
> regards
> Philipp
> 

Re: [PATCH 3/5] [media] tc358743: support probe from device tree

[Philipp Zabel]

Am Dienstag, den 14.07.2015, 12:15 +0200 schrieb Hans Verkuil:
[...]
> As you said, it's not public and without the formulas there is nothing you
> can do but hardcode it.
> 
> If I understand this correctly these values depend on the link frequency,
> so the DT should contain the link frequency and the driver can hardcode the
> values based on that. Which means that if someone needs to support a new
> link frequency the driver needs to be extended for that frequency.
> 
> As long as Toshiba keeps the formulas under NDA there isn't much else you can
> do.

Ok.

[...]
> >>>  	/* platform data */
> >>> -	if (!pdata) {
> >>> -		v4l_err(client, "No platform data!\n");
> >>> -		return -ENODEV;
> >>> +	if (pdata) {
> >>> +		state->pdata = *pdata;
> >>> +	} else {
> >>> +		err = tc358743_probe_of(state);
> >>> +		if (err == -ENODEV)
> >>> +			v4l_err(client, "No platform data!\n");
> >>
> >> I'd replace this with "No device tree data!" or something like that.
> > 
> > I'll do that, thank you.

On second thought, I'll keep it as is. The tc358743_probe_of function
prints its own error messages. In the platform data case it returns
-ENODEV, so that'd still be the correct message, then.

regards
Philipp

[PATCH 4/5] [media] tc358743: add direct interrupt handling

[Philipp Zabel]

When probed from device tree, the i2c client driver can handle the interrupt
on its own.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/i2c/tc358743.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 02c60b3..4a889d4 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -32,6 +32,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
 #include <linux/videodev2.h>
 #include <linux/workqueue.h>
 #include <linux/v4l2-dv-timings.h>
@@ -1306,6 +1307,16 @@ static int tc358743_isr(struct v4l2_subdev *sd, u32 status, bool *handled)
 	return 0;
 }
 
+static irqreturn_t tc358743_irq_handler(int irq, void *dev_id)
+{
+	struct tc358743_state *state = dev_id;
+	bool handled;
+
+	tc358743_isr(&state->sd, 0, &handled);
+
+	return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
 /* --------------- VIDEO OPS --------------- */
 
 static int tc358743_g_input_status(struct v4l2_subdev *sd, u32 *status)
@@ -1874,6 +1885,17 @@ static int tc358743_probe(struct i2c_client *client,
 	tc358743_set_csi_color_space(sd);
 
 	tc358743_init_interrupts(sd);
+
+	if (state->i2c_client->irq) {
+		err = devm_request_threaded_irq(&client->dev,
+						state->i2c_client->irq,
+						NULL, tc358743_irq_handler,
+						IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+						"tc358743", state);
+		if (err)
+			goto err_work_queues;
+	}
+
 	tc358743_enable_interrupts(sd, tx_5v_power_present(sd));
 	i2c_wr16(sd, INTMASK, ~(MASK_HDMI_MSK | MASK_CSI_MSK) & 0xffff);
 
-- 
2.1.4

[PATCH 5/5] [media] tc358743: allow event subscription

[Philipp Zabel]

This is useful to subscribe to HDMI hotplug events via the
V4L2_CID_DV_RX_POWER_PRESENT control.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/i2c/tc358743.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 4a889d4..91fffa8 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -40,6 +40,7 @@
 #include <media/v4l2-dv-timings.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-of.h>
 #include <media/tc358743.h>
 
@@ -1604,6 +1605,8 @@ static const struct v4l2_subdev_core_ops tc358743_core_ops = {
 	.s_register = tc358743_s_register,
 #endif
 	.interrupt_service_routine = tc358743_isr,
+	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
 };
 
 static const struct v4l2_subdev_video_ops tc358743_video_ops = {
-- 
2.1.4

Re: [PATCH 5/5] [media] tc358743: allow event subscription

[Hans Verkuil]

On 07/10/2015 03:11 PM, Philipp Zabel wrote:
> This is useful to subscribe to HDMI hotplug events via the
> V4L2_CID_DV_RX_POWER_PRESENT control.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/i2c/tc358743.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 4a889d4..91fffa8 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -40,6 +40,7 @@
>  #include <media/v4l2-dv-timings.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
>  #include <media/v4l2-of.h>
>  #include <media/tc358743.h>
>  
> @@ -1604,6 +1605,8 @@ static const struct v4l2_subdev_core_ops tc358743_core_ops = {
>  	.s_register = tc358743_s_register,
>  #endif
>  	.interrupt_service_routine = tc358743_isr,
> +	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,

Ah, they are set here.

But note that v4l2_ctrl_subdev_subscribe_event is not enough, since this driver
also issues the V4L2_EVENT_SOURCE_CHANGE event.

See this patch on how to do that:

http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/commit/?h=for-v4.3a&id=85c9b0b83795dac3d27043619a727af5c7313fe7

Note: requires the new v4l2_subdev_notify_event function that's not yet
merged (just posted the pull request for that).

Regards,

	Hans

> +	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
>  };
>  
>  static const struct v4l2_subdev_video_ops tc358743_video_ops = {
> 

Re: [PATCH 5/5] [media] tc358743: allow event subscription

[Philipp Zabel]

Am Montag, den 13.07.2015, 13:07 +0200 schrieb Hans Verkuil:
> On 07/10/2015 03:11 PM, Philipp Zabel wrote:
> > This is useful to subscribe to HDMI hotplug events via the
> > V4L2_CID_DV_RX_POWER_PRESENT control.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/media/i2c/tc358743.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> > index 4a889d4..91fffa8 100644
> > --- a/drivers/media/i2c/tc358743.c
> > +++ b/drivers/media/i2c/tc358743.c
> > @@ -40,6 +40,7 @@
> >  #include <media/v4l2-dv-timings.h>
> >  #include <media/v4l2-device.h>
> >  #include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-event.h>
> >  #include <media/v4l2-of.h>
> >  #include <media/tc358743.h>
> >  
> > @@ -1604,6 +1605,8 @@ static const struct v4l2_subdev_core_ops tc358743_core_ops = {
> >  	.s_register = tc358743_s_register,
> >  #endif
> >  	.interrupt_service_routine = tc358743_isr,
> > +	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> 
> Ah, they are set here.
> 
> But note that v4l2_ctrl_subdev_subscribe_event is not enough, since this driver
> also issues the V4L2_EVENT_SOURCE_CHANGE event.
> 
> See this patch on how to do that:
> 
> http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/commit/?h=for-v4.3a&id=85c9b0b83795dac3d27043619a727af5c7313fe7
> 
> Note: requires the new v4l2_subdev_notify_event function that's not yet
> merged (just posted the pull request for that).

Ok, I think I'll split this up and send patch 5 separately, then.

regards
Philipp

Re: [PATCH 5/5] [media] tc358743: allow event subscription

[Hans Verkuil]

On 07/14/2015 12:10 PM, Philipp Zabel wrote:
> Am Montag, den 13.07.2015, 13:07 +0200 schrieb Hans Verkuil:
>> On 07/10/2015 03:11 PM, Philipp Zabel wrote:
>>> This is useful to subscribe to HDMI hotplug events via the
>>> V4L2_CID_DV_RX_POWER_PRESENT control.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> ---
>>>  drivers/media/i2c/tc358743.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
>>> index 4a889d4..91fffa8 100644
>>> --- a/drivers/media/i2c/tc358743.c
>>> +++ b/drivers/media/i2c/tc358743.c
>>> @@ -40,6 +40,7 @@
>>>  #include <media/v4l2-dv-timings.h>
>>>  #include <media/v4l2-device.h>
>>>  #include <media/v4l2-ctrls.h>
>>> +#include <media/v4l2-event.h>
>>>  #include <media/v4l2-of.h>
>>>  #include <media/tc358743.h>
>>>  
>>> @@ -1604,6 +1605,8 @@ static const struct v4l2_subdev_core_ops tc358743_core_ops = {
>>>  	.s_register = tc358743_s_register,
>>>  #endif
>>>  	.interrupt_service_routine = tc358743_isr,
>>> +	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
>>
>> Ah, they are set here.
>>
>> But note that v4l2_ctrl_subdev_subscribe_event is not enough, since this driver
>> also issues the V4L2_EVENT_SOURCE_CHANGE event.
>>
>> See this patch on how to do that:
>>
>> http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/commit/?h=for-v4.3a&id=85c9b0b83795dac3d27043619a727af5c7313fe7
>>
>> Note: requires the new v4l2_subdev_notify_event function that's not yet
>> merged (just posted the pull request for that).
> 
> Ok, I think I'll split this up and send patch 5 separately, then.

FYI: the v4l2_subdev_notify_event was just merged today.

Regards,

	Hans

Re: [PATCH 1/5] [media] tc358743: register v4l2 asynchronous subdevice

[Hans Verkuil]

On 07/10/2015 03:11 PM, Philipp Zabel wrote:
> Add support for registering the sensor subdevice using the v4l2-async API.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/i2c/tc358743.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 34d4f32..48d1575 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1710,6 +1710,16 @@ static int tc358743_probe(struct i2c_client *client,
>  		goto err_hdl;
>  	}
>  
> +	state->pad.flags = MEDIA_PAD_FL_SOURCE;
> +	err = media_entity_init(&sd->entity, 1, &state->pad, 0);
> +	if (err < 0)
> +		goto err_hdl;
> +
> +	sd->dev = &client->dev;
> +	err = v4l2_async_register_subdev(sd);
> +	if (err < 0)
> +		goto err_hdl;
> +
>  	mutex_init(&state->confctl_mutex);
>  
>  	INIT_DELAYED_WORK(&state->delayed_work_enable_hotplug,
> @@ -1740,6 +1750,7 @@ err_work_queues:
>  	destroy_workqueue(state->work_queues);
>  	mutex_destroy(&state->confctl_mutex);
>  err_hdl:
> +	media_entity_cleanup(&sd->entity);
>  	v4l2_ctrl_handler_free(&state->hdl);
>  	return err;
>  }
> @@ -1751,6 +1762,7 @@ static int tc358743_remove(struct i2c_client *client)
>  
>  	cancel_delayed_work(&state->delayed_work_enable_hotplug);
>  	destroy_workqueue(state->work_queues);
> +	v4l2_async_unregister_subdev(sd);

Shouldn't there be a media_entity_cleanup() call in tc358743_remove() as well?

Regards,

	Hans

>  	v4l2_device_unregister_subdev(sd);
>  	mutex_destroy(&state->confctl_mutex);
>  	v4l2_ctrl_handler_free(&state->hdl);
>