[PATCH v3 0/3] media: i2c: Introduce driver for the TW9900 decoder

[PATCH v3 0/3] media: i2c: Introduce driver for the TW9900 decoder

[Maxime Chevallier]

Hello everyone,

This is the third version of the series adding support for the Techwell
TW9900 multi standard decoder. It's a pretty simple decoder compared to
the TW9910, since it doesn't have a built-in scaler/crop engine.

This V3 addresses coding-style issues reported by Paul, and fixes the
example binding.

Any feedback is appreciated,

Thanks,

Maxime

Maxime Chevallier (3):
  dt-bindings: vendor-prefixes: Add techwell vendor prefix
  media: dt-bindings: media: i2c: Add bindings for TW9900
  media: i2c: Introduce a driver for the Techwell TW9900 decoder

 .../devicetree/bindings/media/i2c/tw9900.yaml |  60 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   6 +
 drivers/media/i2c/Kconfig                     |  11 +
 drivers/media/i2c/Makefile                    |   1 +
 drivers/media/i2c/tw9900.c                    | 617 ++++++++++++++++++
 6 files changed, 697 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/tw9900.yaml
 create mode 100644 drivers/media/i2c/tw9900.c

-- 
2.25.4

[PATCH v3 1/3] dt-bindings: vendor-prefixes: Add techwell vendor prefix

[Maxime Chevallier]

Add prefix for Techwell, Inc.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2735be1a8470..ff65b8bff3c5 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1057,6 +1057,8 @@ patternProperties:
     description: TechNexion
   "^technologic,.*":
     description: Technologic Systems
+  "^techwell,.*":
+    description: Techwell, Inc.
   "^tempo,.*":
     description: Tempo Semiconductor
   "^techstar,.*":
-- 
2.25.4

[PATCH v3 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900

[Maxime Chevallier]

The Techwell TW9900 is a video decoder supporting multiple input
standards, such as PAL, NTSC and SECAM, and outputs a BT.656 video
signal.

It's designed to be low-power, posesses some features such as a
programmable comb-filter, and automatic input standard detection.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V2->V3 : Fix the example not compiling due to a typo in the reset-gpios
node.

 .../devicetree/bindings/media/i2c/tw9900.yaml | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/tw9900.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/tw9900.yaml b/Documentation/devicetree/bindings/media/i2c/tw9900.yaml
new file mode 100644
index 000000000000..91eff26668fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/tw9900.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/tw9900.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Techwell TW9900 NTSC/PAL/SECAM video decoder
+
+maintainers:
+  - Maxime Chevallier <maxime.chevallier@bootlin.com>
+
+description:
+  The tw9900 is a multi-standard video decoder, supporting NTSC, PAL and SECAM
+  standards with auto-detection features.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - techwell,tw9900
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: VDD power supply
+
+  port:
+    type: object
+    description:
+      A node containing a single endpoint as doucmented in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            tw9900: tw9900@44 {
+                    compatible = "techwell,tw9900";
+                    reg = <0x44>;
+
+                    vdd-supply = <&tw9900_supply>;
+                    reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
+
+                    port {
+                            tw9900_out: endpoint {
+                                    remote-endpoint = <&vip_in>;
+                            };
+                    };
+            };
+    };
-- 
2.25.4

Re: [PATCH v3 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900

[Rob Herring]

On Tue, 22 Dec 2020 17:04:06 +0100, Maxime Chevallier wrote:
> The Techwell TW9900 is a video decoder supporting multiple input
> standards, such as PAL, NTSC and SECAM, and outputs a BT.656 video
> signal.
> 
> It's designed to be low-power, posesses some features such as a
> programmable comb-filter, and automatic input standard detection.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> V2->V3 : Fix the example not compiling due to a typo in the reset-gpios
> node.
> 
>  .../devicetree/bindings/media/i2c/tw9900.yaml | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/tw9900.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/tw9900.example.dt.yaml: tw9900@44: 'reset-gpios' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/tw9900.yaml

See https://patchwork.ozlabs.org/patch/1419441

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

[PATCH v3 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder

[Maxime Chevallier]

The Techwell video decoder supports PAL, NTSC and SECAM input formats,
and outputs a BT.656 signal.

This commit adds support for this device, with basic support for NTSC
and PAL, along with brightness and contrast controls.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
v1 -> v2: Set the media entity type to decoder, and implement the
s_std/g_std ops

V2 ->V3 : Fix coding-style issues, and remove the use of the bulk API
for regulators. Make the driver select the media-controller and
V4L2-subdev APIs.

 MAINTAINERS                |   6 +
 drivers/media/i2c/Kconfig  |  11 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/tw9900.c | 617 +++++++++++++++++++++++++++++++++++++
 4 files changed, 635 insertions(+)
 create mode 100644 drivers/media/i2c/tw9900.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 281de213ef47..1031cbb5dc65 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17163,6 +17163,12 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	drivers/media/rc/ttusbir.c
 
+TECHWELL TW9900 VIDEO DECODER
+M:	Maxime Chevallier <maxime.chevallier@bootlin.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	drivers/media/i2c/tw9900.c
+
 TECHWELL TW9910 VIDEO DECODER
 L:	linux-media@vger.kernel.org
 S:	Orphan
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 878f66ef2719..53a6f1ba082e 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -426,6 +426,17 @@ config VIDEO_TW2804
 	  To compile this driver as a module, choose M here: the
 	  module will be called tw2804.
 
+config VIDEO_TW9900
+	tristate "Techwell TW9900 video decoder"
+	depends on VIDEO_V4L2 && I2C
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	help
+	  Support for the Techwell tw9900 multi-standard video decoder.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called tw9900.
+
 config VIDEO_TW9903
 	tristate "Techwell TW9903 video decoder"
 	depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index f0a77473979d..cbc1d9aedd38 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_VIDEO_TVP5150) += tvp5150.o
 obj-$(CONFIG_VIDEO_TVP514X) += tvp514x.o
 obj-$(CONFIG_VIDEO_TVP7002) += tvp7002.o
 obj-$(CONFIG_VIDEO_TW2804) += tw2804.o
+obj-$(CONFIG_VIDEO_TW9900) += tw9900.o
 obj-$(CONFIG_VIDEO_TW9903) += tw9903.o
 obj-$(CONFIG_VIDEO_TW9906) += tw9906.o
 obj-$(CONFIG_VIDEO_TW9910) += tw9910.o
diff --git a/drivers/media/i2c/tw9900.c b/drivers/media/i2c/tw9900.c
new file mode 100644
index 000000000000..7bb105707060
--- /dev/null
+++ b/drivers/media/i2c/tw9900.c
@@ -0,0 +1,617 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Techwell TW9900 multi-standard video decoder.
+ *
+ * Copyright (C) 2018 Fuzhou Rockchip Electronics Co., Ltd.
+ * Copyright (C) 2020 Maxime Chevallier <maxime.chevallier@bootlin.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sysfs.h>
+#include <linux/timer.h>
+#include <linux/delay.h>
+#include <media/media-entity.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-subdev.h>
+
+#define TW9900_REG_CHIP_ID	0x00
+#define TW9900_REG_CHIP_STATUS	0x01
+#define TW9900_REG_CHIP_STATUS_VLOCK	0x08
+#define TW9900_REG_CHIP_STATUS_VDLOSS	0x80
+#define TW9900_REG_OUT_FMT_CTL	0x03
+#define TW9900_REG_OUT_FMT_CTL_STANDBY	0xA7
+#define TW9900_REG_OUT_FMT_CTL_STREAMING	0xA0
+#define TW9900_REG_CKHY_HSDLY	0x04
+#define TW9900_REG_OUT_CTRL_I	0x05
+#define TW9900_REG_ANALOG_CTL	0x06
+#define TW9900_REG_CROP_HI	0x07
+#define TW9900_REG_VDELAY_LO	0x08
+#define TW9900_REG_VACTIVE_LO	0x09
+#define TW9900_REG_HACTIVE_LO	0x0B
+#define TW9900_REG_CNTRL1	0x0C
+#define TW9900_REG_BRIGHT_CTL	0x10
+#define TW9900_REG_CONTRAST_CTL	0x11
+#define TW9900_REG_VBI_CNTL	0x19
+#define TW9900_REG_ANAL_CTL_II	0x1A
+#define TW9900_REG_OUT_CTRL_II	0x1B
+#define TW9900_REG_STD_SEL	0x1C
+#define TW9900_REG_MISSCNT	0x26
+#define TW9900_REG_MISC_CTL_II	0x2F
+#define TW9900_REG_VVBI		0x55
+
+#define TW9900_CHIP_ID		0x00
+
+#define VSYNC_POLL_INTERVAL_MS	20
+#define VSYNC_WAIT_MAX_POLLS	50
+
+struct regval {
+	u8 addr;
+	u8 val;
+};
+
+struct tw9900_mode {
+	u32 width;
+	u32 height;
+	u32 skip_top;
+	u32 std;
+	u32 field;
+	const struct regval *reg_list;
+	int n_regs;
+};
+
+struct tw9900 {
+	struct i2c_client *client;
+	struct gpio_desc *reset_gpio;
+	struct regulator *regulator;
+
+	bool streaming;
+
+	struct v4l2_subdev subdev;
+	struct v4l2_ctrl_handler hdl;
+	struct media_pad pad;
+
+	struct timer_list timer;
+	struct work_struct work_i2c_poll;
+
+	const struct tw9900_mode *cur_mode;
+};
+
+#define to_tw9900(sd) container_of(sd, struct tw9900, subdev)
+
+static const struct regval tw9900_init_regs[] = {
+	{ TW9900_REG_MISC_CTL_II,	0xE6 },
+	{ TW9900_REG_MISSCNT,		0x24 },
+	{ TW9900_REG_OUT_FMT_CTL,	0xA7 },
+	{ TW9900_REG_ANAL_CTL_II,	0x0A },
+	{ TW9900_REG_VDELAY_LO,		0x19 },
+	{ TW9900_REG_STD_SEL,		0x00 },
+	{ TW9900_REG_VACTIVE_LO,	0xF0 },
+	{ TW9900_REG_STD_SEL,		0x07 },
+	{ TW9900_REG_CKHY_HSDLY,	0x40 },
+	{ TW9900_REG_ANALOG_CTL,	0x80 },
+	{ TW9900_REG_CNTRL1,		0xDC },
+	{ TW9900_REG_OUT_CTRL_I,	0x98 },
+};
+
+static const struct regval tw9900_pal_regs[] = {
+	{ TW9900_REG_STD_SEL,		0x01 },
+};
+
+static const struct regval tw9900_ntsc_regs[] = {
+	{ TW9900_REG_OUT_FMT_CTL,	0xA4 },
+	{ TW9900_REG_VDELAY_LO,		0x12 },
+	{ TW9900_REG_VACTIVE_LO,	0xF0 },
+	{ TW9900_REG_CROP_HI,		0x02 },
+	{ TW9900_REG_HACTIVE_LO,	0xD0 },
+	{ TW9900_REG_VBI_CNTL,		0x01 },
+	{ TW9900_REG_STD_SEL,		0x00 },
+};
+
+static const struct tw9900_mode supported_modes[] = {
+	{
+		.width = 720,
+		.height = 576,
+		.skip_top = 0,
+		.std = V4L2_STD_PAL,
+		.field = V4L2_FIELD_NONE,
+		.reg_list = tw9900_pal_regs,
+		.n_regs = ARRAY_SIZE(tw9900_pal_regs),
+	},
+	{
+		.width = 720,
+		.height = 480,
+		.skip_top = 0,
+		.std = V4L2_STD_NTSC,
+		.field = V4L2_FIELD_NONE,
+		.reg_list = tw9900_ntsc_regs,
+		.n_regs = ARRAY_SIZE(tw9900_ntsc_regs),
+	},
+};
+
+static int tw9900_write_reg(struct i2c_client *client, u8 reg, u8 val)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, reg, val);
+
+	if (ret < 0)
+		dev_err(&client->dev, "write reg error: %d\n", ret);
+
+	return ret;
+}
+
+static int tw9900_write_array(struct i2c_client *client,
+			      const struct regval *regs, int n_regs)
+{
+	int i, ret = 0;
+
+	for (i = 0; ret == 0 && i <= n_regs; i++)
+		ret = tw9900_write_reg(client, regs[i].addr, regs[i].val);
+
+	return ret;
+}
+
+static inline u8 tw9900_read_reg(struct i2c_client *client, u8 reg)
+{
+	return i2c_smbus_read_byte_data(client, reg);
+}
+
+static void tw9900_fill_fmt(const struct tw9900_mode *mode,
+			    struct v4l2_mbus_framefmt *fmt)
+{
+	fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
+	fmt->width = mode->width;
+	fmt->height = mode->height;
+	fmt->field = mode->field;
+	fmt->colorspace = V4L2_COLORSPACE_SMPTE170M;
+}
+
+static int tw9900_set_fmt(struct v4l2_subdev *sd,
+			  struct v4l2_subdev_pad_config *cfg,
+			  struct v4l2_subdev_format *fmt)
+{
+	struct tw9900 *tw9900 = to_tw9900(sd);
+	struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
+
+	tw9900_fill_fmt(tw9900->cur_mode, mbus_fmt);
+
+	mbus_fmt->width = tw9900->cur_mode->width;
+	mbus_fmt->height = tw9900->cur_mode->height;
+
+	return 0;
+}
+
+static int tw9900_get_fmt(struct v4l2_subdev *sd,
+			  struct v4l2_subdev_pad_config *cfg,
+			  struct v4l2_subdev_format *fmt)
+{
+	struct tw9900 *tw9900 = to_tw9900(sd);
+	struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
+
+	tw9900_fill_fmt(tw9900->cur_mode, mbus_fmt);
+
+	return 0;
+}
+
+static int tw9900_enum_mbus_code(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_pad_config *cfg,
+				 struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->index >= 1)
+		return -EINVAL;
+
+	code->code = MEDIA_BUS_FMT_UYVY8_2X8;
+
+	return 0;
+}
+
+static int tw9900_enum_frame_sizes(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_pad_config *cfg,
+				   struct v4l2_subdev_frame_size_enum *fse)
+{
+	u32 index = fse->index;
+
+	if (index >= 1)
+		return -EINVAL;
+
+	fse->code = MEDIA_BUS_FMT_UYVY8_2X8;
+
+	fse->min_width  = supported_modes[index].width;
+	fse->max_width  = supported_modes[index].width;
+	fse->max_height = supported_modes[index].height;
+	fse->min_height = supported_modes[index].height;
+
+	return 0;
+}
+
+static int tw9900_power_on(struct tw9900 *tw9900)
+{
+	int ret;
+	struct device *dev = &tw9900->client->dev;
+
+	if (tw9900->reset_gpio)
+		gpiod_set_value_cansleep(tw9900->reset_gpio, 1);
+
+	ret = regulator_enable(tw9900->regulator);
+	if (ret < 0)
+		goto error;
+
+	usleep_range(50000, 52000);
+
+	if (tw9900->reset_gpio)
+		gpiod_set_value_cansleep(tw9900->reset_gpio, 0);
+
+	usleep_range(1000, 2000);
+
+	ret = tw9900_write_array(tw9900->client, tw9900_init_regs,
+				 ARRAY_SIZE(tw9900_init_regs));
+	if (ret) {
+		dev_err(dev, "Failed to init tw9900\n");
+		goto error;
+	}
+
+	return 0;
+
+error:
+
+	return ret;
+}
+
+static void tw9900_power_off(struct tw9900 *tw9900)
+{
+	if (tw9900->reset_gpio)
+		gpiod_set_value_cansleep(tw9900->reset_gpio, 1);
+
+	regulator_disable(tw9900->regulator);
+}
+
+static int tw9900_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct tw9900 *tw9900 = container_of(ctrl->handler, struct tw9900, hdl);
+
+	if (pm_runtime_suspended(&tw9900->client->dev))
+		return 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_BRIGHTNESS:
+		tw9900_write_reg(tw9900->client, 0x10, (u8)ctrl->val);
+		break;
+	case V4L2_CID_CONTRAST:
+		tw9900_write_reg(tw9900->client, 0x11, (u8)ctrl->val);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int tw9900_s_stream(struct v4l2_subdev *sd, int on)
+{
+	struct tw9900 *tw9900 = to_tw9900(sd);
+	struct i2c_client *client = tw9900->client;
+	int i, ret = 0;
+
+	on = !!on;
+	if (on == tw9900->streaming)
+		return 0;
+
+	if (on) {
+		ret = pm_runtime_get_sync(&tw9900->client->dev);
+		if (ret < 0) {
+			pm_runtime_put_noidle(&client->dev);
+			return ret;
+		}
+
+		ret = v4l2_ctrl_handler_setup(sd->ctrl_handler);
+		if (ret)
+			goto put_and_return;
+
+		ret = tw9900_write_array(tw9900->client,
+					 tw9900->cur_mode->reg_list,
+					 tw9900->cur_mode->n_regs);
+		if (ret)
+			goto put_and_return;
+
+		/* Wait for VSync lock */
+		for (i = 0; i < VSYNC_WAIT_MAX_POLLS; i++) {
+			u8 status = tw9900_read_reg(tw9900->client,
+						    TW9900_REG_CHIP_STATUS);
+			if (!(status & TW9900_REG_CHIP_STATUS_VDLOSS) &&
+			    (status & TW9900_REG_CHIP_STATUS_VLOCK))
+				break;
+
+			msleep(VSYNC_POLL_INTERVAL_MS);
+		}
+
+		ret = tw9900_write_reg(client, TW9900_REG_OUT_FMT_CTL,
+				       TW9900_REG_OUT_FMT_CTL_STREAMING);
+		if (ret)
+			goto put_and_return;
+
+	} else {
+		tw9900_write_reg(client, TW9900_REG_OUT_FMT_CTL,
+				 TW9900_REG_OUT_FMT_CTL_STANDBY);
+		pm_runtime_put(&client->dev);
+	}
+
+	tw9900->streaming = on;
+
+	return ret;
+
+put_and_return:
+	pm_runtime_put(&client->dev);
+
+	return ret;
+}
+
+static int tw9900_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	struct tw9900 *tw9900 = to_tw9900(sd);
+	struct v4l2_mbus_framefmt *try_fmt;
+
+	try_fmt = v4l2_subdev_get_try_format(sd, fh->pad, 0);
+
+	/* Initialize try_fmt */
+	tw9900_fill_fmt(tw9900->cur_mode, try_fmt);
+
+	return 0;
+}
+
+static int tw9900_runtime_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct tw9900 *tw9900 = to_tw9900(sd);
+
+	return tw9900_power_on(tw9900);
+}
+
+static int tw9900_runtime_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct tw9900 *tw9900 = to_tw9900(sd);
+
+	tw9900_power_off(tw9900);
+
+	return 0;
+}
+
+static int tw9900_subscribe_event(struct v4l2_subdev *sd,
+				  struct v4l2_fh *fh,
+				  struct v4l2_event_subscription *sub)
+{
+	switch (sub->type) {
+	case V4L2_EVENT_SOURCE_CHANGE:
+		return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
+	case V4L2_EVENT_CTRL:
+		return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct tw9900_mode *tw9900_get_mode_from_std(v4l2_std_id std)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(supported_modes); i++)
+		if (supported_modes[i].std == std)
+			return &supported_modes[i];
+
+	return NULL;
+}
+
+static int tw9900_s_std(struct v4l2_subdev *sd, v4l2_std_id norm)
+{
+	struct tw9900 *tw9900 = to_tw9900(sd);
+	const struct tw9900_mode *mode;
+	int ret;
+
+	if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
+		return -EINVAL;
+
+	mode = tw9900_get_mode_from_std(norm);
+	if (!mode)
+		return -EINVAL;
+
+	ret = tw9900_write_array(tw9900->client, mode->reg_list, mode->n_regs);
+	if (ret)
+		return ret;
+
+	tw9900->cur_mode = mode;
+
+	return 0;
+}
+
+static int tw9900_g_std(struct v4l2_subdev *sd, v4l2_std_id *norm)
+{
+	struct tw9900 *tw9900 = to_tw9900(sd);
+
+	*norm = tw9900->cur_mode->std;
+
+	return 0;
+}
+
+static const struct dev_pm_ops tw9900_pm_ops = {
+	SET_RUNTIME_PM_OPS(tw9900_runtime_suspend,
+			   tw9900_runtime_resume, NULL)
+};
+
+static const struct v4l2_subdev_core_ops tw9900_core_ops = {
+	.subscribe_event	= tw9900_subscribe_event,
+	.unsubscribe_event	= v4l2_event_subdev_unsubscribe,
+};
+
+static const struct v4l2_subdev_video_ops tw9900_video_ops = {
+	.s_std		= tw9900_s_std,
+	.g_std		= tw9900_g_std,
+	.s_stream	= tw9900_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops tw9900_pad_ops = {
+	.enum_mbus_code		= tw9900_enum_mbus_code,
+	.enum_frame_size	= tw9900_enum_frame_sizes,
+	.get_fmt		= tw9900_get_fmt,
+	.set_fmt		= tw9900_set_fmt,
+};
+
+static const struct v4l2_subdev_ops tw9900_subdev_ops = {
+	.core	= &tw9900_core_ops,
+	.video	= &tw9900_video_ops,
+	.pad	= &tw9900_pad_ops,
+};
+
+static const struct v4l2_ctrl_ops tw9900_ctrl_ops = {
+	.s_ctrl	= tw9900_s_ctrl,
+};
+
+static const struct v4l2_subdev_internal_ops tw9900_internal_ops = {
+	.open	= tw9900_open,
+};
+
+static int tw9900_check_id(struct tw9900 *tw9900,
+			   struct i2c_client *client)
+{
+	struct device *dev = &tw9900->client->dev;
+	u8 id;
+
+	id = tw9900_read_reg(client, TW9900_CHIP_ID);
+
+	if (id != TW9900_CHIP_ID) {
+		dev_err(dev, "Unexpected decoder id(%04x)\n", id);
+		return -EINVAL;
+	}
+
+	dev_info(dev, "Detected TW9900 (%04x) decoder\n", TW9900_CHIP_ID);
+
+	return 0;
+}
+
+static int tw9900_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct v4l2_ctrl_handler *hdl;
+	struct tw9900 *tw9900;
+	int ret;
+
+	tw9900 = devm_kzalloc(dev, sizeof(*tw9900), GFP_KERNEL);
+	if (!tw9900)
+		return -ENOMEM;
+
+	tw9900->client = client;
+	tw9900->cur_mode = &supported_modes[0];
+
+	tw9900->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(tw9900->reset_gpio))
+		tw9900->reset_gpio = NULL;
+
+	tw9900->regulator = devm_regulator_get(&tw9900->client->dev, "vdd");
+	if (IS_ERR(tw9900->regulator)) {
+		dev_err(dev, "Failed to get power regulator\n");
+		return ret;
+	}
+
+	v4l2_i2c_subdev_init(&tw9900->subdev, client, &tw9900_subdev_ops);
+	tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
+				V4L2_SUBDEV_FL_HAS_EVENTS;
+
+	hdl = &tw9900->hdl;
+
+	v4l2_ctrl_handler_init(hdl, 2);
+
+	v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_BRIGHTNESS,
+			  -128, 127, 1, 0);
+	v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_CONTRAST,
+			  0, 255, 1, 0x60);
+
+	tw9900->subdev.ctrl_handler = hdl;
+	if (hdl->error) {
+		int err = hdl->error;
+
+		v4l2_ctrl_handler_free(hdl);
+		return err;
+	}
+
+	ret = tw9900_power_on(tw9900);
+	if (ret)
+		return ret;
+
+	ret = tw9900_check_id(tw9900, client);
+	if (ret)
+		goto err_power_off;
+
+	tw9900->subdev.internal_ops = &tw9900_internal_ops;
+	tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	tw9900->pad.flags = MEDIA_PAD_FL_SOURCE;
+	tw9900->subdev.entity.function = MEDIA_ENT_F_DV_DECODER;
+
+	ret = media_entity_pads_init(&tw9900->subdev.entity, 1, &tw9900->pad);
+	if (ret < 0)
+		goto err_power_off;
+
+	ret = v4l2_async_register_subdev(&tw9900->subdev);
+	if (ret) {
+		dev_err(dev, "v4l2 async register subdev failed\n");
+		goto err_clean_entity;
+	}
+
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	pm_runtime_idle(dev);
+
+	return 0;
+
+err_clean_entity:
+	media_entity_cleanup(&tw9900->subdev.entity);
+err_power_off:
+	tw9900_power_off(tw9900);
+
+	return ret;
+}
+
+static int tw9900_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct tw9900 *tw9900 = to_tw9900(sd);
+
+	v4l2_async_unregister_subdev(sd);
+	media_entity_cleanup(&sd->entity);
+
+	pm_runtime_disable(&client->dev);
+	if (!pm_runtime_status_suspended(&client->dev))
+		tw9900_power_off(tw9900);
+	pm_runtime_set_suspended(&client->dev);
+
+	return 0;
+}
+
+static const struct of_device_id tw9900_of_match[] = {
+	{ .compatible = "techwell,tw9900" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, tw9900_of_match);
+
+static struct i2c_driver tw9900_i2c_driver = {
+	.driver = {
+		.name		= "tw9900",
+		.pm		= &tw9900_pm_ops,
+		.of_match_table	= tw9900_of_match
+	},
+	.probe	= tw9900_probe,
+	.remove	= tw9900_remove,
+};
+
+module_i2c_driver(tw9900_i2c_driver);
+
+MODULE_DESCRIPTION("tw9900 decoder driver");
+MODULE_LICENSE("GPL v2");
-- 
2.25.4

Re: [PATCH v3 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4862 bytes --]

Hi Maxime,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on robh/for-next linus/master v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Maxime-Chevallier/media-i2c-Introduce-driver-for-the-TW9900-decoder/20201223-000948
base:   git://linuxtv.org/media_tree.git master
config: arm64-randconfig-r001-20210105 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/cdf8ecd519454783c60d4bca02b9279f8133ef77
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Maxime-Chevallier/media-i2c-Introduce-driver-for-the-TW9900-decoder/20201223-000948
        git checkout cdf8ecd519454783c60d4bca02b9279f8133ef77
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/i2c/tw9900.c:521:10: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
                   return ret;
                          ^~~
   drivers/media/i2c/tw9900.c:505:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +/ret +521 drivers/media/i2c/tw9900.c

   498	
   499	static int tw9900_probe(struct i2c_client *client,
   500				const struct i2c_device_id *id)
   501	{
   502		struct device *dev = &client->dev;
   503		struct v4l2_ctrl_handler *hdl;
   504		struct tw9900 *tw9900;
   505		int ret;
   506	
   507		tw9900 = devm_kzalloc(dev, sizeof(*tw9900), GFP_KERNEL);
   508		if (!tw9900)
   509			return -ENOMEM;
   510	
   511		tw9900->client = client;
   512		tw9900->cur_mode = &supported_modes[0];
   513	
   514		tw9900->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
   515		if (IS_ERR(tw9900->reset_gpio))
   516			tw9900->reset_gpio = NULL;
   517	
   518		tw9900->regulator = devm_regulator_get(&tw9900->client->dev, "vdd");
   519		if (IS_ERR(tw9900->regulator)) {
   520			dev_err(dev, "Failed to get power regulator\n");
 > 521			return ret;
   522		}
   523	
   524		v4l2_i2c_subdev_init(&tw9900->subdev, client, &tw9900_subdev_ops);
   525		tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
   526					V4L2_SUBDEV_FL_HAS_EVENTS;
   527	
   528		hdl = &tw9900->hdl;
   529	
   530		v4l2_ctrl_handler_init(hdl, 2);
   531	
   532		v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_BRIGHTNESS,
   533				  -128, 127, 1, 0);
   534		v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_CONTRAST,
   535				  0, 255, 1, 0x60);
   536	
   537		tw9900->subdev.ctrl_handler = hdl;
   538		if (hdl->error) {
   539			int err = hdl->error;
   540	
   541			v4l2_ctrl_handler_free(hdl);
   542			return err;
   543		}
   544	
   545		ret = tw9900_power_on(tw9900);
   546		if (ret)
   547			return ret;
   548	
   549		ret = tw9900_check_id(tw9900, client);
   550		if (ret)
   551			goto err_power_off;
   552	
   553		tw9900->subdev.internal_ops = &tw9900_internal_ops;
   554		tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
   555		tw9900->pad.flags = MEDIA_PAD_FL_SOURCE;
   556		tw9900->subdev.entity.function = MEDIA_ENT_F_DV_DECODER;
   557	
   558		ret = media_entity_pads_init(&tw9900->subdev.entity, 1, &tw9900->pad);
   559		if (ret < 0)
   560			goto err_power_off;
   561	
   562		ret = v4l2_async_register_subdev(&tw9900->subdev);
   563		if (ret) {
   564			dev_err(dev, "v4l2 async register subdev failed\n");
   565			goto err_clean_entity;
   566		}
   567	
   568		pm_runtime_set_active(dev);
   569		pm_runtime_enable(dev);
   570		pm_runtime_idle(dev);
   571	
   572		return 0;
   573	
   574	err_clean_entity:
   575		media_entity_cleanup(&tw9900->subdev.entity);
   576	err_power_off:
   577		tw9900_power_off(tw9900);
   578	
   579		return ret;
   580	}
   581	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 46997 bytes --]

Re: [PATCH v3 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4729 bytes --]

Hi Maxime,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on robh/for-next linus/master v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Maxime-Chevallier/media-i2c-Introduce-driver-for-the-TW9900-decoder/20201223-000948
base:   git://linuxtv.org/media_tree.git master
config: arm64-randconfig-r001-20210105 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/cdf8ecd519454783c60d4bca02b9279f8133ef77
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Maxime-Chevallier/media-i2c-Introduce-driver-for-the-TW9900-decoder/20201223-000948
        git checkout cdf8ecd519454783c60d4bca02b9279f8133ef77
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/i2c/tw9900.c:521:10: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
                   return ret;
                          ^~~
   drivers/media/i2c/tw9900.c:505:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +/ret +521 drivers/media/i2c/tw9900.c

   498	
   499	static int tw9900_probe(struct i2c_client *client,
   500				const struct i2c_device_id *id)
   501	{
   502		struct device *dev = &client->dev;
   503		struct v4l2_ctrl_handler *hdl;
   504		struct tw9900 *tw9900;
   505		int ret;
   506	
   507		tw9900 = devm_kzalloc(dev, sizeof(*tw9900), GFP_KERNEL);
   508		if (!tw9900)
   509			return -ENOMEM;
   510	
   511		tw9900->client = client;
   512		tw9900->cur_mode = &supported_modes[0];
   513	
   514		tw9900->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
   515		if (IS_ERR(tw9900->reset_gpio))
   516			tw9900->reset_gpio = NULL;
   517	
   518		tw9900->regulator = devm_regulator_get(&tw9900->client->dev, "vdd");
   519		if (IS_ERR(tw9900->regulator)) {
   520			dev_err(dev, "Failed to get power regulator\n");
 > 521			return ret;
   522		}
   523	
   524		v4l2_i2c_subdev_init(&tw9900->subdev, client, &tw9900_subdev_ops);
   525		tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
   526					V4L2_SUBDEV_FL_HAS_EVENTS;
   527	
   528		hdl = &tw9900->hdl;
   529	
   530		v4l2_ctrl_handler_init(hdl, 2);
   531	
   532		v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_BRIGHTNESS,
   533				  -128, 127, 1, 0);
   534		v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_CONTRAST,
   535				  0, 255, 1, 0x60);
   536	
   537		tw9900->subdev.ctrl_handler = hdl;
   538		if (hdl->error) {
   539			int err = hdl->error;
   540	
   541			v4l2_ctrl_handler_free(hdl);
   542			return err;
   543		}
   544	
   545		ret = tw9900_power_on(tw9900);
   546		if (ret)
   547			return ret;
   548	
   549		ret = tw9900_check_id(tw9900, client);
   550		if (ret)
   551			goto err_power_off;
   552	
   553		tw9900->subdev.internal_ops = &tw9900_internal_ops;
   554		tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
   555		tw9900->pad.flags = MEDIA_PAD_FL_SOURCE;
   556		tw9900->subdev.entity.function = MEDIA_ENT_F_DV_DECODER;
   557	
   558		ret = media_entity_pads_init(&tw9900->subdev.entity, 1, &tw9900->pad);
   559		if (ret < 0)
   560			goto err_power_off;
   561	
   562		ret = v4l2_async_register_subdev(&tw9900->subdev);
   563		if (ret) {
   564			dev_err(dev, "v4l2 async register subdev failed\n");
   565			goto err_clean_entity;
   566		}
   567	
   568		pm_runtime_set_active(dev);
   569		pm_runtime_enable(dev);
   570		pm_runtime_idle(dev);
   571	
   572		return 0;
   573	
   574	err_clean_entity:
   575		media_entity_cleanup(&tw9900->subdev.entity);
   576	err_power_off:
   577		tw9900_power_off(tw9900);
   578	
   579		return ret;
   580	}
   581	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46997 bytes --]