All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/15] [media] adv7180: Add support for more chip variants
@ 2015-01-23 15:52 Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 01/15] [media] adv7180: Do not request the IRQ again during resume Lars-Peter Clausen
                   ` (15 more replies)
  0 siblings, 16 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Changes from v1:
	* Reserved custom user control range for the fast switch control
	* Dropped the free-run mode control patch for now. The controls should
	  probably be standardized first, but that is going to be a different
	  patch series.

Original cover letter below:

The adv7180 is part of a larger family of chips which all implement
different features from a feature superset. This patch series step by step
extends the current adv7180 with features from the superset that are
currently not supported and gradually adding support for more variations of
the chip.

The first half of this series contains fixes and cleanups while the second
half adds new features and support for new chips


Lars-Peter Clausen (15):
  [media] adv7180: Do not request the IRQ again during resume
  [media] adv7180: Pass correct flags to request_threaded_irq()
  [media] adv7180: Use inline function instead of macro
  [media] adv7180: Cleanup register define naming
  [media] adv7180: Do implicit register paging
  [media] adv7180: Reset the device before initialization
  [media] adv7180: Add media controller support
  [media] adv7180: Consolidate video mode setting
  [media] adv7180: Prepare for multi-chip support
  [media] adv7180: Add support for the adv7182
  [media] adv7180: Add support for the adv7280/adv7281/adv7282
  [media] adv7180: Add support for the
    adv7280-m/adv7281-m/adv7281-ma/adv7282-m
  [media] adv7180: Add I2P support
  [media] adv7180: Add fast switch support
  [media] Add MAINTAINERS entry for the adv7180

 MAINTAINERS                        |    7 +
 drivers/media/i2c/Kconfig          |    2 +-
 drivers/media/i2c/adv7180.c        | 1016 +++++++++++++++++++++++++++++-------
 drivers/media/pci/sta2x11/Kconfig  |    1 +
 drivers/media/platform/Kconfig     |    2 +-
 include/uapi/linux/v4l2-controls.h |    4 +
 6 files changed, 831 insertions(+), 201 deletions(-)

-- 
1.8.0


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

* [PATCH v2 01/15] [media] adv7180: Do not request the IRQ again during resume
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 02/15] [media] adv7180: Pass correct flags to request_threaded_irq() Lars-Peter Clausen
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Currently the IRQ is requested from within the init_device() function. This
function is not only called during device probe, but also during resume
causing the driver to try to request the IRQ again. Move requesting the IRQ
from init_device() to the probe function to make sure that it is only
requested once.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index bffe6eb..172e4a2 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -553,11 +553,6 @@ static int init_device(struct i2c_client *client, struct adv7180_state *state)
 
 	/* register for interrupts */
 	if (state->irq > 0) {
-		ret = request_threaded_irq(state->irq, NULL, adv7180_irq,
-					   IRQF_ONESHOT, KBUILD_MODNAME, state);
-		if (ret)
-			return ret;
-
 		ret = i2c_smbus_write_byte_data(client, ADV7180_ADI_CTRL_REG,
 						ADV7180_ADI_CTRL_IRQ_SPACE);
 		if (ret < 0)
@@ -597,7 +592,6 @@ static int init_device(struct i2c_client *client, struct adv7180_state *state)
 	return 0;
 
 err:
-	free_irq(state->irq, state);
 	return ret;
 }
 
@@ -636,6 +630,13 @@ static int adv7180_probe(struct i2c_client *client,
 	if (ret)
 		goto err_free_ctrl;
 
+	if (state->irq) {
+		ret = request_threaded_irq(client->irq, NULL, adv7180_irq,
+					   IRQF_ONESHOT, KBUILD_MODNAME, state);
+		if (ret)
+			goto err_free_ctrl;
+	}
+
 	ret = v4l2_async_register_subdev(sd);
 	if (ret)
 		goto err_free_irq;
-- 
1.8.0


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

* [PATCH v2 02/15] [media] adv7180: Pass correct flags to request_threaded_irq()
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 01/15] [media] adv7180: Do not request the IRQ again during resume Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro Lars-Peter Clausen
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Most IRQ controllers support different types of interrupts. The adv7180
generates falling edge interrupts, so make sure to pass IRQF_TRIGGER_FALLING
to request_threaded_irq() so the IRQ controller is configured for the
correct mode.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 172e4a2..f424a4d 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -632,7 +632,8 @@ static int adv7180_probe(struct i2c_client *client,
 
 	if (state->irq) {
 		ret = request_threaded_irq(client->irq, NULL, adv7180_irq,
-					   IRQF_ONESHOT, KBUILD_MODNAME, state);
+					   IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+					   KBUILD_MODNAME, state);
 		if (ret)
 			goto err_free_ctrl;
 	}
-- 
1.8.0


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

* [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 01/15] [media] adv7180: Do not request the IRQ again during resume Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 02/15] [media] adv7180: Pass correct flags to request_threaded_irq() Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-02-02 13:36   ` Mauro Carvalho Chehab
  2015-01-23 15:52 ` [PATCH v2 04/15] [media] adv7180: Cleanup register define naming Lars-Peter Clausen
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Use a inline function instead of a macro for the container_of helper for
getting the driver's state struct from a control. A inline function has the
advantage that it is more typesafe and nicer in general.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index f424a4d..f2508abe 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -130,9 +130,11 @@ struct adv7180_state {
 	bool			powered;
 	u8			input;
 };
-#define to_adv7180_sd(_ctrl) (&container_of(_ctrl->handler,		\
-					    struct adv7180_state,	\
-					    ctrl_hdl)->sd)
+
+static struct adv7180_state *ctrl_to_adv7180(struct v4l2_ctrl *ctrl)
+{
+	return container_of(ctrl->handler, struct adv7180_state, ctrl_hdl);
+}
 
 static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
 {
@@ -345,9 +347,8 @@ static int adv7180_s_power(struct v4l2_subdev *sd, int on)
 
 static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct v4l2_subdev *sd = to_adv7180_sd(ctrl);
-	struct adv7180_state *state = to_state(sd);
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct adv7180_state *state = ctrl_to_adv7180(ctrl);
+	struct i2c_client *client = v4l2_get_subdevdata(&state->sd);
 	int ret = mutex_lock_interruptible(&state->mutex);
 	int val;
 
-- 
1.8.0


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

* [PATCH v2 04/15] [media] adv7180: Cleanup register define naming
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (2 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 05/15] [media] adv7180: Do implicit register paging Lars-Peter Clausen
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Consistently prefix register defines with ADV7180_REG. Also remove the "ADI"
from register names, the ADV7180 prefix should provide enough of a namespace
separation.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 105 ++++++++++++++++++++++----------------------
 1 file changed, 52 insertions(+), 53 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index f2508abe..00ba845 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -31,7 +31,7 @@
 #include <media/v4l2-ctrls.h>
 #include <linux/mutex.h>
 
-#define ADV7180_INPUT_CONTROL_REG			0x00
+#define ADV7180_REG_INPUT_CONTROL			0x00
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM	0x00
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM_PED 0x10
 #define ADV7180_INPUT_CONTROL_AD_PAL_N_NTSC_J_SECAM	0x20
@@ -50,36 +50,36 @@
 #define ADV7180_INPUT_CONTROL_PAL_SECAM_PED		0xf0
 #define ADV7180_INPUT_CONTROL_INSEL_MASK		0x0f
 
-#define ADV7180_EXTENDED_OUTPUT_CONTROL_REG		0x04
+#define ADV7180_REG_EXTENDED_OUTPUT_CONTROL		0x04
 #define ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS		0xC5
 
-#define ADV7180_AUTODETECT_ENABLE_REG			0x07
+#define ADV7180_REG_AUTODETECT_ENABLE			0x07
 #define ADV7180_AUTODETECT_DEFAULT			0x7f
 /* Contrast */
-#define ADV7180_CON_REG		0x08	/*Unsigned */
+#define ADV7180_REG_CON		0x08	/*Unsigned */
 #define ADV7180_CON_MIN		0
 #define ADV7180_CON_DEF		128
 #define ADV7180_CON_MAX		255
 /* Brightness*/
-#define ADV7180_BRI_REG		0x0a	/*Signed */
+#define ADV7180_REG_BRI		0x0a	/*Signed */
 #define ADV7180_BRI_MIN		-128
 #define ADV7180_BRI_DEF		0
 #define ADV7180_BRI_MAX		127
 /* Hue */
-#define ADV7180_HUE_REG		0x0b	/*Signed, inverted */
+#define ADV7180_REG_HUE		0x0b	/*Signed, inverted */
 #define ADV7180_HUE_MIN		-127
 #define ADV7180_HUE_DEF		0
 #define ADV7180_HUE_MAX		128
 
-#define ADV7180_ADI_CTRL_REG				0x0e
-#define ADV7180_ADI_CTRL_IRQ_SPACE			0x20
+#define ADV7180_REG_CTRL		0x0e
+#define ADV7180_CTRL_IRQ_SPACE		0x20
 
-#define ADV7180_PWR_MAN_REG		0x0f
+#define ADV7180_REG_PWR_MAN		0x0f
 #define ADV7180_PWR_MAN_ON		0x04
 #define ADV7180_PWR_MAN_OFF		0x24
 #define ADV7180_PWR_MAN_RES		0x80
 
-#define ADV7180_STATUS1_REG				0x10
+#define ADV7180_REG_STATUS1		0x10
 #define ADV7180_STATUS1_IN_LOCK		0x01
 #define ADV7180_STATUS1_AUTOD_MASK	0x70
 #define ADV7180_STATUS1_AUTOD_NTSM_M_J	0x00
@@ -91,33 +91,33 @@
 #define ADV7180_STATUS1_AUTOD_PAL_COMB	0x60
 #define ADV7180_STATUS1_AUTOD_SECAM_525	0x70
 
-#define ADV7180_IDENT_REG 0x11
+#define ADV7180_REG_IDENT 0x11
 #define ADV7180_ID_7180 0x18
 
-#define ADV7180_ICONF1_ADI		0x40
+#define ADV7180_REG_ICONF1		0x40
 #define ADV7180_ICONF1_ACTIVE_LOW	0x01
 #define ADV7180_ICONF1_PSYNC_ONLY	0x10
 #define ADV7180_ICONF1_ACTIVE_TO_CLR	0xC0
 /* Saturation */
-#define ADV7180_SD_SAT_CB_REG	0xe3	/*Unsigned */
-#define ADV7180_SD_SAT_CR_REG	0xe4	/*Unsigned */
+#define ADV7180_REG_SD_SAT_CB	0xe3	/*Unsigned */
+#define ADV7180_REG_SD_SAT_CR	0xe4	/*Unsigned */
 #define ADV7180_SAT_MIN		0
 #define ADV7180_SAT_DEF		128
 #define ADV7180_SAT_MAX		255
 
 #define ADV7180_IRQ1_LOCK	0x01
 #define ADV7180_IRQ1_UNLOCK	0x02
-#define ADV7180_ISR1_ADI	0x42
-#define ADV7180_ICR1_ADI	0x43
-#define ADV7180_IMR1_ADI	0x44
-#define ADV7180_IMR2_ADI	0x48
+#define ADV7180_REG_ISR1	0x42
+#define ADV7180_REG_ICR1	0x43
+#define ADV7180_REG_IMR1	0x44
+#define ADV7180_REG_IMR2	0x48
 #define ADV7180_IRQ3_AD_CHANGE	0x08
-#define ADV7180_ISR3_ADI	0x4A
-#define ADV7180_ICR3_ADI	0x4B
-#define ADV7180_IMR3_ADI	0x4C
-#define ADV7180_IMR4_ADI	0x50
+#define ADV7180_REG_ISR3	0x4A
+#define ADV7180_REG_ICR3	0x4B
+#define ADV7180_REG_IMR3	0x4C
+#define ADV7180_REG_IMR4	0x50
 
-#define ADV7180_NTSC_V_BIT_END_REG	0xE6
+#define ADV7180_REG_NTSC_V_BIT_END	0xE6
 #define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND	0x4F
 
 struct adv7180_state {
@@ -198,7 +198,7 @@ static u32 adv7180_status_to_v4l2(u8 status1)
 static int __adv7180_status(struct i2c_client *client, u32 *status,
 			    v4l2_std_id *std)
 {
-	int status1 = i2c_smbus_read_byte_data(client, ADV7180_STATUS1_REG);
+	int status1 = i2c_smbus_read_byte_data(client, ADV7180_REG_STATUS1);
 
 	if (status1 < 0)
 		return status1;
@@ -249,14 +249,13 @@ static int adv7180_s_routing(struct v4l2_subdev *sd, u32 input,
 	if ((input & ADV7180_INPUT_CONTROL_INSEL_MASK) != input)
 		goto out;
 
-	ret = i2c_smbus_read_byte_data(client, ADV7180_INPUT_CONTROL_REG);
-
+	ret = i2c_smbus_read_byte_data(client, ADV7180_REG_INPUT_CONTROL);
 	if (ret < 0)
 		goto out;
 
 	ret &= ~ADV7180_INPUT_CONTROL_INSEL_MASK;
 	ret = i2c_smbus_write_byte_data(client,
-					ADV7180_INPUT_CONTROL_REG, ret | input);
+					ADV7180_REG_INPUT_CONTROL, ret | input);
 	state->input = input;
 out:
 	mutex_unlock(&state->mutex);
@@ -286,7 +285,7 @@ static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 	/* all standards -> autodetect */
 	if (std == V4L2_STD_ALL) {
 		ret =
-		    i2c_smbus_write_byte_data(client, ADV7180_INPUT_CONTROL_REG,
+		    i2c_smbus_write_byte_data(client, ADV7180_REG_INPUT_CONTROL,
 				ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
 					      | state->input);
 		if (ret < 0)
@@ -300,7 +299,7 @@ static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 			goto out;
 
 		ret = i2c_smbus_write_byte_data(client,
-						ADV7180_INPUT_CONTROL_REG,
+						ADV7180_REG_INPUT_CONTROL,
 						ret | state->input);
 		if (ret < 0)
 			goto out;
@@ -324,7 +323,7 @@ static int adv7180_set_power(struct adv7180_state *state,
 	else
 		val = ADV7180_PWR_MAN_OFF;
 
-	return i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG, val);
+	return i2c_smbus_write_byte_data(client, ADV7180_REG_PWR_MAN, val);
 }
 
 static int adv7180_s_power(struct v4l2_subdev *sd, int on)
@@ -357,25 +356,25 @@ static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl)
 	val = ctrl->val;
 	switch (ctrl->id) {
 	case V4L2_CID_BRIGHTNESS:
-		ret = i2c_smbus_write_byte_data(client, ADV7180_BRI_REG, val);
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_BRI, val);
 		break;
 	case V4L2_CID_HUE:
 		/*Hue is inverted according to HSL chart */
-		ret = i2c_smbus_write_byte_data(client, ADV7180_HUE_REG, -val);
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_HUE, -val);
 		break;
 	case V4L2_CID_CONTRAST:
-		ret = i2c_smbus_write_byte_data(client, ADV7180_CON_REG, val);
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_CON, val);
 		break;
 	case V4L2_CID_SATURATION:
 		/*
 		 *This could be V4L2_CID_BLUE_BALANCE/V4L2_CID_RED_BALANCE
 		 *Let's not confuse the user, everybody understands saturation
 		 */
-		ret = i2c_smbus_write_byte_data(client, ADV7180_SD_SAT_CB_REG,
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_SD_SAT_CB,
 						val);
 		if (ret < 0)
 			break;
-		ret = i2c_smbus_write_byte_data(client, ADV7180_SD_SAT_CR_REG,
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_SD_SAT_CR,
 						val);
 		break;
 	default:
@@ -489,12 +488,12 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
 	u8 isr3;
 
 	mutex_lock(&state->mutex);
-	i2c_smbus_write_byte_data(client, ADV7180_ADI_CTRL_REG,
-				  ADV7180_ADI_CTRL_IRQ_SPACE);
-	isr3 = i2c_smbus_read_byte_data(client, ADV7180_ISR3_ADI);
+	i2c_smbus_write_byte_data(client, ADV7180_REG_CTRL,
+				  ADV7180_CTRL_IRQ_SPACE);
+	isr3 = i2c_smbus_read_byte_data(client, ADV7180_REG_ISR3);
 	/* clear */
-	i2c_smbus_write_byte_data(client, ADV7180_ICR3_ADI, isr3);
-	i2c_smbus_write_byte_data(client, ADV7180_ADI_CTRL_REG, 0);
+	i2c_smbus_write_byte_data(client, ADV7180_REG_ICR3, isr3);
+	i2c_smbus_write_byte_data(client, ADV7180_REG_CTRL, 0);
 
 	if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
 		__adv7180_status(client, NULL, &state->curr_norm);
@@ -511,7 +510,7 @@ static int init_device(struct i2c_client *client, struct adv7180_state *state)
 	/* Enable autodetection */
 	if (state->autodetect) {
 		ret =
-		    i2c_smbus_write_byte_data(client, ADV7180_INPUT_CONTROL_REG,
+		    i2c_smbus_write_byte_data(client, ADV7180_REG_INPUT_CONTROL,
 				ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
 					      | state->input);
 		if (ret < 0)
@@ -519,7 +518,7 @@ static int init_device(struct i2c_client *client, struct adv7180_state *state)
 
 		ret =
 		    i2c_smbus_write_byte_data(client,
-					      ADV7180_AUTODETECT_ENABLE_REG,
+					      ADV7180_REG_AUTODETECT_ENABLE,
 					      ADV7180_AUTODETECT_DEFAULT);
 		if (ret < 0)
 			return ret;
@@ -529,7 +528,7 @@ static int init_device(struct i2c_client *client, struct adv7180_state *state)
 			return ret;
 
 		ret =
-		    i2c_smbus_write_byte_data(client, ADV7180_INPUT_CONTROL_REG,
+		    i2c_smbus_write_byte_data(client, ADV7180_REG_INPUT_CONTROL,
 					      ret | state->input);
 		if (ret < 0)
 			return ret;
@@ -537,14 +536,14 @@ static int init_device(struct i2c_client *client, struct adv7180_state *state)
 	}
 	/* ITU-R BT.656-4 compatible */
 	ret = i2c_smbus_write_byte_data(client,
-			ADV7180_EXTENDED_OUTPUT_CONTROL_REG,
+			ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
 			ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS);
 	if (ret < 0)
 		return ret;
 
 	/* Manually set V bit end position in NTSC mode */
 	ret = i2c_smbus_write_byte_data(client,
-					ADV7180_NTSC_V_BIT_END_REG,
+					ADV7180_REG_NTSC_V_BIT_END,
 					ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
 	if (ret < 0)
 		return ret;
@@ -554,37 +553,37 @@ static int init_device(struct i2c_client *client, struct adv7180_state *state)
 
 	/* register for interrupts */
 	if (state->irq > 0) {
-		ret = i2c_smbus_write_byte_data(client, ADV7180_ADI_CTRL_REG,
-						ADV7180_ADI_CTRL_IRQ_SPACE);
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_CTRL,
+						ADV7180_CTRL_IRQ_SPACE);
 		if (ret < 0)
 			goto err;
 
 		/* config the Interrupt pin to be active low */
-		ret = i2c_smbus_write_byte_data(client, ADV7180_ICONF1_ADI,
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_ICONF1,
 						ADV7180_ICONF1_ACTIVE_LOW |
 						ADV7180_ICONF1_PSYNC_ONLY);
 		if (ret < 0)
 			goto err;
 
-		ret = i2c_smbus_write_byte_data(client, ADV7180_IMR1_ADI, 0);
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_IMR1, 0);
 		if (ret < 0)
 			goto err;
 
-		ret = i2c_smbus_write_byte_data(client, ADV7180_IMR2_ADI, 0);
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_IMR2, 0);
 		if (ret < 0)
 			goto err;
 
 		/* enable AD change interrupts interrupts */
-		ret = i2c_smbus_write_byte_data(client, ADV7180_IMR3_ADI,
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_IMR3,
 						ADV7180_IRQ3_AD_CHANGE);
 		if (ret < 0)
 			goto err;
 
-		ret = i2c_smbus_write_byte_data(client, ADV7180_IMR4_ADI, 0);
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_IMR4, 0);
 		if (ret < 0)
 			goto err;
 
-		ret = i2c_smbus_write_byte_data(client, ADV7180_ADI_CTRL_REG,
+		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_CTRL,
 						0);
 		if (ret < 0)
 			goto err;
-- 
1.8.0


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

* [PATCH v2 05/15] [media] adv7180: Do implicit register paging
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (3 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 04/15] [media] adv7180: Cleanup register define naming Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 06/15] [media] adv7180: Reset the device before initialization Lars-Peter Clausen
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

The ad7180 has multiple register pages which can be switched between by
writing to a register. Currently the driver manually switches between pages
whenever a register outside of the default register map is accessed and
switches back after it has been accessed. This is a bit tedious and also
potential source for bugs.

This patch adds two helper functions that take care of switching between
pages and reading/writing the register. The register numbers for registers
are updated to encode both the page (in the upper 8-bits) and the register
(in the lower 8-bits) numbers.

Having multiple pages means that a register access is not a single atomic
i2c_smbus_write_byte_data() or i2c_smbus_read_byte_data() call and we need
to make sure that concurrent register access does not race against each
other.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 206 ++++++++++++++++++++++----------------------
 1 file changed, 105 insertions(+), 101 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 00ba845..cc05db9 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -31,7 +31,7 @@
 #include <media/v4l2-ctrls.h>
 #include <linux/mutex.h>
 
-#define ADV7180_REG_INPUT_CONTROL			0x00
+#define ADV7180_REG_INPUT_CONTROL			0x0000
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM	0x00
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM_PED 0x10
 #define ADV7180_INPUT_CONTROL_AD_PAL_N_NTSC_J_SECAM	0x20
@@ -50,28 +50,28 @@
 #define ADV7180_INPUT_CONTROL_PAL_SECAM_PED		0xf0
 #define ADV7180_INPUT_CONTROL_INSEL_MASK		0x0f
 
-#define ADV7180_REG_EXTENDED_OUTPUT_CONTROL		0x04
+#define ADV7180_REG_EXTENDED_OUTPUT_CONTROL		0x0004
 #define ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS		0xC5
 
 #define ADV7180_REG_AUTODETECT_ENABLE			0x07
 #define ADV7180_AUTODETECT_DEFAULT			0x7f
 /* Contrast */
-#define ADV7180_REG_CON		0x08	/*Unsigned */
+#define ADV7180_REG_CON		0x0008	/*Unsigned */
 #define ADV7180_CON_MIN		0
 #define ADV7180_CON_DEF		128
 #define ADV7180_CON_MAX		255
 /* Brightness*/
-#define ADV7180_REG_BRI		0x0a	/*Signed */
+#define ADV7180_REG_BRI		0x000a	/*Signed */
 #define ADV7180_BRI_MIN		-128
 #define ADV7180_BRI_DEF		0
 #define ADV7180_BRI_MAX		127
 /* Hue */
-#define ADV7180_REG_HUE		0x0b	/*Signed, inverted */
+#define ADV7180_REG_HUE		0x000b	/*Signed, inverted */
 #define ADV7180_HUE_MIN		-127
 #define ADV7180_HUE_DEF		0
 #define ADV7180_HUE_MAX		128
 
-#define ADV7180_REG_CTRL		0x0e
+#define ADV7180_REG_CTRL		0x000e
 #define ADV7180_CTRL_IRQ_SPACE		0x20
 
 #define ADV7180_REG_PWR_MAN		0x0f
@@ -79,7 +79,7 @@
 #define ADV7180_PWR_MAN_OFF		0x24
 #define ADV7180_PWR_MAN_RES		0x80
 
-#define ADV7180_REG_STATUS1		0x10
+#define ADV7180_REG_STATUS1		0x0010
 #define ADV7180_STATUS1_IN_LOCK		0x01
 #define ADV7180_STATUS1_AUTOD_MASK	0x70
 #define ADV7180_STATUS1_AUTOD_NTSM_M_J	0x00
@@ -91,33 +91,33 @@
 #define ADV7180_STATUS1_AUTOD_PAL_COMB	0x60
 #define ADV7180_STATUS1_AUTOD_SECAM_525	0x70
 
-#define ADV7180_REG_IDENT 0x11
+#define ADV7180_REG_IDENT 0x0011
 #define ADV7180_ID_7180 0x18
 
-#define ADV7180_REG_ICONF1		0x40
+#define ADV7180_REG_ICONF1		0x0040
 #define ADV7180_ICONF1_ACTIVE_LOW	0x01
 #define ADV7180_ICONF1_PSYNC_ONLY	0x10
 #define ADV7180_ICONF1_ACTIVE_TO_CLR	0xC0
 /* Saturation */
-#define ADV7180_REG_SD_SAT_CB	0xe3	/*Unsigned */
-#define ADV7180_REG_SD_SAT_CR	0xe4	/*Unsigned */
+#define ADV7180_REG_SD_SAT_CB	0x00e3	/*Unsigned */
+#define ADV7180_REG_SD_SAT_CR	0x00e4	/*Unsigned */
 #define ADV7180_SAT_MIN		0
 #define ADV7180_SAT_DEF		128
 #define ADV7180_SAT_MAX		255
 
 #define ADV7180_IRQ1_LOCK	0x01
 #define ADV7180_IRQ1_UNLOCK	0x02
-#define ADV7180_REG_ISR1	0x42
-#define ADV7180_REG_ICR1	0x43
-#define ADV7180_REG_IMR1	0x44
-#define ADV7180_REG_IMR2	0x48
+#define ADV7180_REG_ISR1	0x0042
+#define ADV7180_REG_ICR1	0x0043
+#define ADV7180_REG_IMR1	0x0044
+#define ADV7180_REG_IMR2	0x0048
 #define ADV7180_IRQ3_AD_CHANGE	0x08
-#define ADV7180_REG_ISR3	0x4A
-#define ADV7180_REG_ICR3	0x4B
-#define ADV7180_REG_IMR3	0x4C
+#define ADV7180_REG_ISR3	0x004A
+#define ADV7180_REG_ICR3	0x004B
+#define ADV7180_REG_IMR3	0x004C
 #define ADV7180_REG_IMR4	0x50
 
-#define ADV7180_REG_NTSC_V_BIT_END	0xE6
+#define ADV7180_REG_NTSC_V_BIT_END	0x00E6
 #define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND	0x4F
 
 struct adv7180_state {
@@ -129,6 +129,9 @@ struct adv7180_state {
 	bool			autodetect;
 	bool			powered;
 	u8			input;
+
+	struct i2c_client	*client;
+	unsigned int		register_page;
 };
 
 static struct adv7180_state *ctrl_to_adv7180(struct v4l2_ctrl *ctrl)
@@ -136,6 +139,33 @@ static struct adv7180_state *ctrl_to_adv7180(struct v4l2_ctrl *ctrl)
 	return container_of(ctrl->handler, struct adv7180_state, ctrl_hdl);
 }
 
+static int adv7180_select_page(struct adv7180_state *state, unsigned int page)
+{
+	if (state->register_page != page) {
+		i2c_smbus_write_byte_data(state->client, ADV7180_REG_CTRL,
+			page);
+		state->register_page = page;
+	}
+
+	return 0;
+}
+
+static int adv7180_write(struct adv7180_state *state, unsigned int reg,
+	unsigned int value)
+{
+	lockdep_assert_held(&state->mutex);
+	adv7180_select_page(state, reg >> 8);
+	return i2c_smbus_write_byte_data(state->client, reg & 0xff, value);
+}
+
+static int adv7180_read(struct adv7180_state *state, unsigned int reg)
+{
+	lockdep_assert_held(&state->mutex);
+	adv7180_select_page(state, reg >> 8);
+	return i2c_smbus_read_byte_data(state->client, reg & 0xff);
+}
+
+
 static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
 {
 	/* in case V4L2_IN_ST_NO_SIGNAL */
@@ -195,10 +225,10 @@ static u32 adv7180_status_to_v4l2(u8 status1)
 	return 0;
 }
 
-static int __adv7180_status(struct i2c_client *client, u32 *status,
+static int __adv7180_status(struct adv7180_state *state, u32 *status,
 			    v4l2_std_id *std)
 {
-	int status1 = i2c_smbus_read_byte_data(client, ADV7180_REG_STATUS1);
+	int status1 = adv7180_read(state, ADV7180_REG_STATUS1);
 
 	if (status1 < 0)
 		return status1;
@@ -227,7 +257,7 @@ static int adv7180_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
 	if (!state->autodetect || state->irq > 0)
 		*std = state->curr_norm;
 	else
-		err = __adv7180_status(v4l2_get_subdevdata(sd), NULL, std);
+		err = __adv7180_status(state, NULL, std);
 
 	mutex_unlock(&state->mutex);
 	return err;
@@ -238,7 +268,6 @@ static int adv7180_s_routing(struct v4l2_subdev *sd, u32 input,
 {
 	struct adv7180_state *state = to_state(sd);
 	int ret = mutex_lock_interruptible(&state->mutex);
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
 
 	if (ret)
 		return ret;
@@ -249,13 +278,12 @@ static int adv7180_s_routing(struct v4l2_subdev *sd, u32 input,
 	if ((input & ADV7180_INPUT_CONTROL_INSEL_MASK) != input)
 		goto out;
 
-	ret = i2c_smbus_read_byte_data(client, ADV7180_REG_INPUT_CONTROL);
+	ret = adv7180_read(state, ADV7180_REG_INPUT_CONTROL);
 	if (ret < 0)
 		goto out;
 
 	ret &= ~ADV7180_INPUT_CONTROL_INSEL_MASK;
-	ret = i2c_smbus_write_byte_data(client,
-					ADV7180_REG_INPUT_CONTROL, ret | input);
+	ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL, ret | input);
 	state->input = input;
 out:
 	mutex_unlock(&state->mutex);
@@ -269,7 +297,7 @@ static int adv7180_g_input_status(struct v4l2_subdev *sd, u32 *status)
 	if (ret)
 		return ret;
 
-	ret = __adv7180_status(v4l2_get_subdevdata(sd), status, NULL);
+	ret = __adv7180_status(state, status, NULL);
 	mutex_unlock(&state->mutex);
 	return ret;
 }
@@ -277,30 +305,27 @@ static int adv7180_g_input_status(struct v4l2_subdev *sd, u32 *status)
 static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 {
 	struct adv7180_state *state = to_state(sd);
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = mutex_lock_interruptible(&state->mutex);
 	if (ret)
 		return ret;
 
 	/* all standards -> autodetect */
 	if (std == V4L2_STD_ALL) {
-		ret =
-		    i2c_smbus_write_byte_data(client, ADV7180_REG_INPUT_CONTROL,
-				ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
-					      | state->input);
+		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
+				    ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
+				    | state->input);
 		if (ret < 0)
 			goto out;
 
-		__adv7180_status(client, NULL, &state->curr_norm);
+		__adv7180_status(state, NULL, &state->curr_norm);
 		state->autodetect = true;
 	} else {
 		ret = v4l2_std_to_adv7180(std);
 		if (ret < 0)
 			goto out;
 
-		ret = i2c_smbus_write_byte_data(client,
-						ADV7180_REG_INPUT_CONTROL,
-						ret | state->input);
+		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
+				    ret | state->input);
 		if (ret < 0)
 			goto out;
 
@@ -313,8 +338,7 @@ out:
 	return ret;
 }
 
-static int adv7180_set_power(struct adv7180_state *state,
-	struct i2c_client *client, bool on)
+static int adv7180_set_power(struct adv7180_state *state, bool on)
 {
 	u8 val;
 
@@ -323,20 +347,19 @@ static int adv7180_set_power(struct adv7180_state *state,
 	else
 		val = ADV7180_PWR_MAN_OFF;
 
-	return i2c_smbus_write_byte_data(client, ADV7180_REG_PWR_MAN, val);
+	return adv7180_write(state, ADV7180_REG_PWR_MAN, val);
 }
 
 static int adv7180_s_power(struct v4l2_subdev *sd, int on)
 {
 	struct adv7180_state *state = to_state(sd);
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret;
 
 	ret = mutex_lock_interruptible(&state->mutex);
 	if (ret)
 		return ret;
 
-	ret = adv7180_set_power(state, client, on);
+	ret = adv7180_set_power(state, on);
 	if (ret == 0)
 		state->powered = on;
 
@@ -347,7 +370,6 @@ static int adv7180_s_power(struct v4l2_subdev *sd, int on)
 static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct adv7180_state *state = ctrl_to_adv7180(ctrl);
-	struct i2c_client *client = v4l2_get_subdevdata(&state->sd);
 	int ret = mutex_lock_interruptible(&state->mutex);
 	int val;
 
@@ -356,26 +378,24 @@ static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl)
 	val = ctrl->val;
 	switch (ctrl->id) {
 	case V4L2_CID_BRIGHTNESS:
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_BRI, val);
+		ret = adv7180_write(state, ADV7180_REG_BRI, val);
 		break;
 	case V4L2_CID_HUE:
 		/*Hue is inverted according to HSL chart */
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_HUE, -val);
+		ret = adv7180_write(state, ADV7180_REG_HUE, -val);
 		break;
 	case V4L2_CID_CONTRAST:
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_CON, val);
+		ret = adv7180_write(state, ADV7180_REG_CON, val);
 		break;
 	case V4L2_CID_SATURATION:
 		/*
 		 *This could be V4L2_CID_BLUE_BALANCE/V4L2_CID_RED_BALANCE
 		 *Let's not confuse the user, everybody understands saturation
 		 */
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_SD_SAT_CB,
-						val);
+		ret = adv7180_write(state, ADV7180_REG_SD_SAT_CB, val);
 		if (ret < 0)
 			break;
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_SD_SAT_CR,
-						val);
+		ret = adv7180_write(state, ADV7180_REG_SD_SAT_CR, val);
 		break;
 	default:
 		ret = -EINVAL;
@@ -484,114 +504,96 @@ static const struct v4l2_subdev_ops adv7180_ops = {
 static irqreturn_t adv7180_irq(int irq, void *devid)
 {
 	struct adv7180_state *state = devid;
-	struct i2c_client *client = v4l2_get_subdevdata(&state->sd);
 	u8 isr3;
 
 	mutex_lock(&state->mutex);
-	i2c_smbus_write_byte_data(client, ADV7180_REG_CTRL,
-				  ADV7180_CTRL_IRQ_SPACE);
-	isr3 = i2c_smbus_read_byte_data(client, ADV7180_REG_ISR3);
+	isr3 = adv7180_read(state, ADV7180_REG_ISR3);
 	/* clear */
-	i2c_smbus_write_byte_data(client, ADV7180_REG_ICR3, isr3);
-	i2c_smbus_write_byte_data(client, ADV7180_REG_CTRL, 0);
+	adv7180_write(state, ADV7180_REG_ICR3, isr3);
 
 	if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
-		__adv7180_status(client, NULL, &state->curr_norm);
+		__adv7180_status(state, NULL, &state->curr_norm);
 	mutex_unlock(&state->mutex);
 
 	return IRQ_HANDLED;
 }
 
-static int init_device(struct i2c_client *client, struct adv7180_state *state)
+static int init_device(struct adv7180_state *state)
 {
 	int ret;
 
+	mutex_lock(&state->mutex);
+
 	/* Initialize adv7180 */
 	/* Enable autodetection */
 	if (state->autodetect) {
-		ret =
-		    i2c_smbus_write_byte_data(client, ADV7180_REG_INPUT_CONTROL,
+		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
 				ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
 					      | state->input);
 		if (ret < 0)
-			return ret;
+			goto out_unlock;
 
-		ret =
-		    i2c_smbus_write_byte_data(client,
-					      ADV7180_REG_AUTODETECT_ENABLE,
+		ret = adv7180_write(state, ADV7180_REG_AUTODETECT_ENABLE,
 					      ADV7180_AUTODETECT_DEFAULT);
 		if (ret < 0)
-			return ret;
+			goto out_unlock;
 	} else {
 		ret = v4l2_std_to_adv7180(state->curr_norm);
 		if (ret < 0)
-			return ret;
+			goto out_unlock;
 
-		ret =
-		    i2c_smbus_write_byte_data(client, ADV7180_REG_INPUT_CONTROL,
+		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
 					      ret | state->input);
 		if (ret < 0)
-			return ret;
+			goto out_unlock;
 
 	}
 	/* ITU-R BT.656-4 compatible */
-	ret = i2c_smbus_write_byte_data(client,
-			ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
+	ret = adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
 			ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS);
 	if (ret < 0)
-		return ret;
+		goto out_unlock;
 
 	/* Manually set V bit end position in NTSC mode */
-	ret = i2c_smbus_write_byte_data(client,
-					ADV7180_REG_NTSC_V_BIT_END,
+	ret = adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
 					ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
 	if (ret < 0)
-		return ret;
+		goto out_unlock;
 
 	/* read current norm */
-	__adv7180_status(client, NULL, &state->curr_norm);
+	__adv7180_status(state, NULL, &state->curr_norm);
 
 	/* register for interrupts */
 	if (state->irq > 0) {
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_CTRL,
-						ADV7180_CTRL_IRQ_SPACE);
-		if (ret < 0)
-			goto err;
-
 		/* config the Interrupt pin to be active low */
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_ICONF1,
+		ret = adv7180_write(state, ADV7180_REG_ICONF1,
 						ADV7180_ICONF1_ACTIVE_LOW |
 						ADV7180_ICONF1_PSYNC_ONLY);
 		if (ret < 0)
-			goto err;
+			goto out_unlock;
 
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_IMR1, 0);
+		ret = adv7180_write(state, ADV7180_REG_IMR1, 0);
 		if (ret < 0)
-			goto err;
+			goto out_unlock;
 
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_IMR2, 0);
+		ret = adv7180_write(state, ADV7180_REG_IMR2, 0);
 		if (ret < 0)
-			goto err;
+			goto out_unlock;
 
 		/* enable AD change interrupts interrupts */
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_IMR3,
+		ret = adv7180_write(state, ADV7180_REG_IMR3,
 						ADV7180_IRQ3_AD_CHANGE);
 		if (ret < 0)
-			goto err;
-
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_IMR4, 0);
-		if (ret < 0)
-			goto err;
+			goto out_unlock;
 
-		ret = i2c_smbus_write_byte_data(client, ADV7180_REG_CTRL,
-						0);
+		ret = adv7180_write(state, ADV7180_REG_IMR4, 0);
 		if (ret < 0)
-			goto err;
+			goto out_unlock;
 	}
 
-	return 0;
+out_unlock:
+	mutex_unlock(&state->mutex);
 
-err:
 	return ret;
 }
 
@@ -615,6 +617,8 @@ static int adv7180_probe(struct i2c_client *client,
 		goto err;
 	}
 
+	state->client = client;
+
 	state->irq = client->irq;
 	mutex_init(&state->mutex);
 	state->autodetect = true;
@@ -626,7 +630,7 @@ static int adv7180_probe(struct i2c_client *client,
 	ret = adv7180_init_controls(state);
 	if (ret)
 		goto err_unreg_subdev;
-	ret = init_device(client, state);
+	ret = init_device(state);
 	if (ret)
 		goto err_free_ctrl;
 
@@ -682,7 +686,7 @@ static int adv7180_suspend(struct device *dev)
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	struct adv7180_state *state = to_state(sd);
 
-	return adv7180_set_power(state, client, false);
+	return adv7180_set_power(state, false);
 }
 
 static int adv7180_resume(struct device *dev)
@@ -693,11 +697,11 @@ static int adv7180_resume(struct device *dev)
 	int ret;
 
 	if (state->powered) {
-		ret = adv7180_set_power(state, client, true);
+		ret = adv7180_set_power(state, true);
 		if (ret)
 			return ret;
 	}
-	ret = init_device(client, state);
+	ret = init_device(state);
 	if (ret < 0)
 		return ret;
 	return 0;
-- 
1.8.0


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

* [PATCH v2 06/15] [media] adv7180: Reset the device before initialization
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (4 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 05/15] [media] adv7180: Do implicit register paging Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 07/15] [media] adv7180: Add media controller support Lars-Peter Clausen
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Reset the device when initializing it so it is in a good known state and the
assumed register settings matche the actual register settings.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index cc05db9..eeb5a4a 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -30,6 +30,7 @@
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
 #include <linux/mutex.h>
+#include <linux/delay.h>
 
 #define ADV7180_REG_INPUT_CONTROL			0x0000
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM	0x00
@@ -524,6 +525,9 @@ static int init_device(struct adv7180_state *state)
 
 	mutex_lock(&state->mutex);
 
+	adv7180_write(state, ADV7180_REG_PWR_MAN, ADV7180_PWR_MAN_RES);
+	usleep_range(2000, 10000);
+
 	/* Initialize adv7180 */
 	/* Enable autodetection */
 	if (state->autodetect) {
@@ -696,14 +700,14 @@ static int adv7180_resume(struct device *dev)
 	struct adv7180_state *state = to_state(sd);
 	int ret;
 
-	if (state->powered) {
-		ret = adv7180_set_power(state, true);
-		if (ret)
-			return ret;
-	}
 	ret = init_device(state);
 	if (ret < 0)
 		return ret;
+
+	ret = adv7180_set_power(state, state->powered);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
-- 
1.8.0


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

* [PATCH v2 07/15] [media] adv7180: Add media controller support
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (5 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 06/15] [media] adv7180: Reset the device before initialization Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 08/15] [media] adv7180: Consolidate video mode setting Lars-Peter Clausen
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Add media controller support to the adv7180 driver by registering a media
entity instance for it as well as implementing pad ops for configuring the
format.

As there currently don't seem to be any users of the video ops format
operations those are removed as well in this patch.

Also set the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the subdevice so it is
possible to create a subdevice device node.

Since the driver now depends on VIDEO_V4L2_SUBDEV_API all drivers which
select the driver need to depend on that symbol as well.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/Kconfig         |  2 +-
 drivers/media/i2c/adv7180.c       | 50 +++++++++++++++++++++++++++++++--------
 drivers/media/pci/sta2x11/Kconfig |  1 +
 drivers/media/platform/Kconfig    |  2 +-
 4 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index ca84543..f37890a 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -177,7 +177,7 @@ comment "Video decoders"
 
 config VIDEO_ADV7180
 	tristate "Analog Devices ADV7180 decoder"
-	depends on VIDEO_V4L2 && I2C
+	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
 	---help---
 	  Support for the Analog Devices ADV7180 video decoder.
 
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index eeb5a4a..349cae3 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -124,6 +124,7 @@
 struct adv7180_state {
 	struct v4l2_ctrl_handler ctrl_hdl;
 	struct v4l2_subdev	sd;
+	struct media_pad	pad;
 	struct mutex		mutex; /* mutual excl. when accessing chip */
 	int			irq;
 	v4l2_std_id		curr_norm;
@@ -442,13 +443,14 @@ static void adv7180_exit_controls(struct adv7180_state *state)
 	v4l2_ctrl_handler_free(&state->ctrl_hdl);
 }
 
-static int adv7180_enum_mbus_fmt(struct v4l2_subdev *sd, unsigned int index,
-				 u32 *code)
+static int adv7180_enum_mbus_code(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_fh *fh,
+				  struct v4l2_subdev_mbus_code_enum *code)
 {
-	if (index > 0)
+	if (code->index != 0)
 		return -EINVAL;
 
-	*code = MEDIA_BUS_FMT_YUYV8_2X8;
+	code->code = MEDIA_BUS_FMT_YUYV8_2X8;
 
 	return 0;
 }
@@ -467,6 +469,20 @@ static int adv7180_mbus_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int adv7180_get_pad_format(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_fh *fh,
+				  struct v4l2_subdev_format *format)
+{
+	return adv7180_mbus_fmt(sd, &format->format);
+}
+
+static int adv7180_set_pad_format(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_fh *fh,
+				  struct v4l2_subdev_format *format)
+{
+	return adv7180_mbus_fmt(sd, &format->format);
+}
+
 static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
 				 struct v4l2_mbus_config *cfg)
 {
@@ -486,10 +502,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
 	.querystd = adv7180_querystd,
 	.g_input_status = adv7180_g_input_status,
 	.s_routing = adv7180_s_routing,
-	.enum_mbus_fmt = adv7180_enum_mbus_fmt,
-	.try_mbus_fmt = adv7180_mbus_fmt,
-	.g_mbus_fmt = adv7180_mbus_fmt,
-	.s_mbus_fmt = adv7180_mbus_fmt,
 	.g_mbus_config = adv7180_g_mbus_config,
 };
 
@@ -497,9 +509,16 @@ static const struct v4l2_subdev_core_ops adv7180_core_ops = {
 	.s_power = adv7180_s_power,
 };
 
+static const struct v4l2_subdev_pad_ops adv7180_pad_ops = {
+	.enum_mbus_code = adv7180_enum_mbus_code,
+	.set_fmt = adv7180_set_pad_format,
+	.get_fmt = adv7180_get_pad_format,
+};
+
 static const struct v4l2_subdev_ops adv7180_ops = {
 	.core = &adv7180_core_ops,
 	.video = &adv7180_video_ops,
+	.pad = &adv7180_pad_ops,
 };
 
 static irqreturn_t adv7180_irq(int irq, void *devid)
@@ -630,20 +649,28 @@ static int adv7180_probe(struct i2c_client *client,
 	state->input = 0;
 	sd = &state->sd;
 	v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
+	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
 
 	ret = adv7180_init_controls(state);
 	if (ret)
 		goto err_unreg_subdev;
-	ret = init_device(state);
+
+	state->pad.flags = MEDIA_PAD_FL_SOURCE;
+	sd->entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
+	ret = media_entity_init(&sd->entity, 1, &state->pad, 0);
 	if (ret)
 		goto err_free_ctrl;
 
+	ret = init_device(state);
+	if (ret)
+		goto err_media_entity_cleanup;
+
 	if (state->irq) {
 		ret = request_threaded_irq(client->irq, NULL, adv7180_irq,
 					   IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
 					   KBUILD_MODNAME, state);
 		if (ret)
-			goto err_free_ctrl;
+			goto err_media_entity_cleanup;
 	}
 
 	ret = v4l2_async_register_subdev(sd);
@@ -655,6 +682,8 @@ static int adv7180_probe(struct i2c_client *client,
 err_free_irq:
 	if (state->irq > 0)
 		free_irq(client->irq, state);
+err_media_entity_cleanup:
+	media_entity_cleanup(&sd->entity);
 err_free_ctrl:
 	adv7180_exit_controls(state);
 err_unreg_subdev:
@@ -673,6 +702,7 @@ static int adv7180_remove(struct i2c_client *client)
 	if (state->irq > 0)
 		free_irq(client->irq, state);
 
+	media_entity_cleanup(&sd->entity);
 	adv7180_exit_controls(state);
 	mutex_destroy(&state->mutex);
 	return 0;
diff --git a/drivers/media/pci/sta2x11/Kconfig b/drivers/media/pci/sta2x11/Kconfig
index f6f30ab..e03587b 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -5,6 +5,7 @@ config STA2X11_VIP
 	select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEOBUF2_DMA_CONTIG
 	depends on PCI && VIDEO_V4L2 && VIRT_TO_BUS
+	depends on VIDEO_V4L2_SUBDEV_API
 	depends on I2C
 	help
 	  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 71e8873..d446b66 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -56,7 +56,7 @@ config VIDEO_VIU
 
 config VIDEO_TIMBERDALE
 	tristate "Support for timberdale Video In/LogiWIN"
-	depends on VIDEO_V4L2 && I2C && DMADEVICES
+	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && DMADEVICES
 	depends on MFD_TIMBERDALE || COMPILE_TEST
 	select DMA_ENGINE
 	select TIMB_DMA
-- 
1.8.0


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

* [PATCH v2 08/15] [media] adv7180: Consolidate video mode setting
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (6 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 07/15] [media] adv7180: Add media controller support Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 09/15] [media] adv7180: Prepare for multi-chip support Lars-Peter Clausen
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

We have basically the same code to set the video standard in init_device()
and adv7180_s_std(). Factor this out into a common helper function.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 67 ++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 349cae3..4d9bcc8 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -304,37 +304,54 @@ static int adv7180_g_input_status(struct v4l2_subdev *sd, u32 *status)
 	return ret;
 }
 
-static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
+static int adv7180_program_std(struct adv7180_state *state)
 {
-	struct adv7180_state *state = to_state(sd);
-	int ret = mutex_lock_interruptible(&state->mutex);
-	if (ret)
-		return ret;
+	int ret;
 
-	/* all standards -> autodetect */
-	if (std == V4L2_STD_ALL) {
+	if (state->autodetect) {
 		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
 				    ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
 				    | state->input);
 		if (ret < 0)
-			goto out;
+			return ret;
 
 		__adv7180_status(state, NULL, &state->curr_norm);
-		state->autodetect = true;
 	} else {
-		ret = v4l2_std_to_adv7180(std);
+		ret = v4l2_std_to_adv7180(state->curr_norm);
 		if (ret < 0)
-			goto out;
+			return ret;
 
 		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
 				    ret | state->input);
 		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
+{
+	struct adv7180_state *state = to_state(sd);
+	int ret = mutex_lock_interruptible(&state->mutex);
+
+	if (ret)
+		return ret;
+
+	/* all standards -> autodetect */
+	if (std == V4L2_STD_ALL) {
+		state->autodetect = true;
+	} else {
+		/* Make sure we can support this std */
+		ret = v4l2_std_to_adv7180(std);
+		if (ret < 0)
 			goto out;
 
 		state->curr_norm = std;
 		state->autodetect = false;
 	}
-	ret = 0;
+
+	ret = adv7180_program_std(state);
 out:
 	mutex_unlock(&state->mutex);
 	return ret;
@@ -547,30 +564,10 @@ static int init_device(struct adv7180_state *state)
 	adv7180_write(state, ADV7180_REG_PWR_MAN, ADV7180_PWR_MAN_RES);
 	usleep_range(2000, 10000);
 
-	/* Initialize adv7180 */
-	/* Enable autodetection */
-	if (state->autodetect) {
-		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
-				ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
-					      | state->input);
-		if (ret < 0)
-			goto out_unlock;
-
-		ret = adv7180_write(state, ADV7180_REG_AUTODETECT_ENABLE,
-					      ADV7180_AUTODETECT_DEFAULT);
-		if (ret < 0)
-			goto out_unlock;
-	} else {
-		ret = v4l2_std_to_adv7180(state->curr_norm);
-		if (ret < 0)
-			goto out_unlock;
-
-		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
-					      ret | state->input);
-		if (ret < 0)
-			goto out_unlock;
+	ret = adv7180_program_std(state);
+	if (ret)
+		goto out_unlock;
 
-	}
 	/* ITU-R BT.656-4 compatible */
 	ret = adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
 			ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS);
-- 
1.8.0


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

* [PATCH v2 09/15] [media] adv7180: Prepare for multi-chip support
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (7 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 08/15] [media] adv7180: Consolidate video mode setting Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 10/15] [media] adv7180: Add support for the adv7182 Lars-Peter Clausen
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

The adv7180 is part of a larger family of device, which have all a very
similar register map layout. This patch prepares the adv7180 driver for
support for multiple different devices. For now the only difference we care
about is the number of input channel configurations. Also the way the input
format is configured slightly differs between some devices.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 187 ++++++++++++++++++++++++++++++--------------
 1 file changed, 130 insertions(+), 57 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 4d9bcc8..e3f91d5 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -32,23 +32,24 @@
 #include <linux/mutex.h>
 #include <linux/delay.h>
 
+#define ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM		0x0
+#define ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM_PED		0x1
+#define ADV7180_STD_AD_PAL_N_NTSC_J_SECAM		0x2
+#define ADV7180_STD_AD_PAL_N_NTSC_M_SECAM		0x3
+#define ADV7180_STD_NTSC_J				0x4
+#define ADV7180_STD_NTSC_M				0x5
+#define ADV7180_STD_PAL60				0x6
+#define ADV7180_STD_NTSC_443				0x7
+#define ADV7180_STD_PAL_BG				0x8
+#define ADV7180_STD_PAL_N				0x9
+#define ADV7180_STD_PAL_M				0xa
+#define ADV7180_STD_PAL_M_PED				0xb
+#define ADV7180_STD_PAL_COMB_N				0xc
+#define ADV7180_STD_PAL_COMB_N_PED			0xd
+#define ADV7180_STD_PAL_SECAM				0xe
+#define ADV7180_STD_PAL_SECAM_PED			0xf
+
 #define ADV7180_REG_INPUT_CONTROL			0x0000
-#define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM	0x00
-#define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM_PED 0x10
-#define ADV7180_INPUT_CONTROL_AD_PAL_N_NTSC_J_SECAM	0x20
-#define ADV7180_INPUT_CONTROL_AD_PAL_N_NTSC_M_SECAM	0x30
-#define ADV7180_INPUT_CONTROL_NTSC_J			0x40
-#define ADV7180_INPUT_CONTROL_NTSC_M			0x50
-#define ADV7180_INPUT_CONTROL_PAL60			0x60
-#define ADV7180_INPUT_CONTROL_NTSC_443			0x70
-#define ADV7180_INPUT_CONTROL_PAL_BG			0x80
-#define ADV7180_INPUT_CONTROL_PAL_N			0x90
-#define ADV7180_INPUT_CONTROL_PAL_M			0xa0
-#define ADV7180_INPUT_CONTROL_PAL_M_PED			0xb0
-#define ADV7180_INPUT_CONTROL_PAL_COMB_N		0xc0
-#define ADV7180_INPUT_CONTROL_PAL_COMB_N_PED		0xd0
-#define ADV7180_INPUT_CONTROL_PAL_SECAM			0xe0
-#define ADV7180_INPUT_CONTROL_PAL_SECAM_PED		0xf0
 #define ADV7180_INPUT_CONTROL_INSEL_MASK		0x0f
 
 #define ADV7180_REG_EXTENDED_OUTPUT_CONTROL		0x0004
@@ -121,6 +122,30 @@
 #define ADV7180_REG_NTSC_V_BIT_END	0x00E6
 #define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND	0x4F
 
+#define ADV7180_INPUT_CVBS_AIN1 0x00
+#define ADV7180_INPUT_CVBS_AIN2 0x01
+#define ADV7180_INPUT_CVBS_AIN3 0x02
+#define ADV7180_INPUT_CVBS_AIN4 0x03
+#define ADV7180_INPUT_CVBS_AIN5 0x04
+#define ADV7180_INPUT_CVBS_AIN6 0x05
+#define ADV7180_INPUT_SVIDEO_AIN1_AIN2 0x06
+#define ADV7180_INPUT_SVIDEO_AIN3_AIN4 0x07
+#define ADV7180_INPUT_SVIDEO_AIN5_AIN6 0x08
+#define ADV7180_INPUT_YPRPB_AIN1_AIN2_AIN3 0x09
+#define ADV7180_INPUT_YPRPB_AIN4_AIN5_AIN6 0x0a
+
+struct adv7180_state;
+
+#define ADV7180_FLAG_RESET_POWERED	BIT(0)
+
+struct adv7180_chip_info {
+	unsigned int flags;
+	unsigned int valid_input_mask;
+	int (*set_std)(struct adv7180_state *st, unsigned int std);
+	int (*select_input)(struct adv7180_state *st, unsigned int input);
+	int (*init)(struct adv7180_state *state);
+};
+
 struct adv7180_state {
 	struct v4l2_ctrl_handler ctrl_hdl;
 	struct v4l2_subdev	sd;
@@ -134,6 +159,7 @@ struct adv7180_state {
 
 	struct i2c_client	*client;
 	unsigned int		register_page;
+	const struct adv7180_chip_info *chip_info;
 };
 
 static struct adv7180_state *ctrl_to_adv7180(struct v4l2_ctrl *ctrl)
@@ -167,6 +193,11 @@ static int adv7180_read(struct adv7180_state *state, unsigned int reg)
 	return i2c_smbus_read_byte_data(state->client, reg & 0xff);
 }
 
+static int adv7180_set_video_standard(struct adv7180_state *state,
+	unsigned int std)
+{
+	return state->chip_info->set_std(state, std);
+}
 
 static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
 {
@@ -199,22 +230,22 @@ static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
 static int v4l2_std_to_adv7180(v4l2_std_id std)
 {
 	if (std == V4L2_STD_PAL_60)
-		return ADV7180_INPUT_CONTROL_PAL60;
+		return ADV7180_STD_PAL60;
 	if (std == V4L2_STD_NTSC_443)
-		return ADV7180_INPUT_CONTROL_NTSC_443;
+		return ADV7180_STD_NTSC_443;
 	if (std == V4L2_STD_PAL_N)
-		return ADV7180_INPUT_CONTROL_PAL_N;
+		return ADV7180_STD_PAL_N;
 	if (std == V4L2_STD_PAL_M)
-		return ADV7180_INPUT_CONTROL_PAL_M;
+		return ADV7180_STD_PAL_M;
 	if (std == V4L2_STD_PAL_Nc)
-		return ADV7180_INPUT_CONTROL_PAL_COMB_N;
+		return ADV7180_STD_PAL_COMB_N;
 
 	if (std & V4L2_STD_PAL)
-		return ADV7180_INPUT_CONTROL_PAL_BG;
+		return ADV7180_STD_PAL_BG;
 	if (std & V4L2_STD_NTSC)
-		return ADV7180_INPUT_CONTROL_NTSC_M;
+		return ADV7180_STD_NTSC_M;
 	if (std & V4L2_STD_SECAM)
-		return ADV7180_INPUT_CONTROL_PAL_SECAM;
+		return ADV7180_STD_PAL_SECAM;
 
 	return -EINVAL;
 }
@@ -274,19 +305,15 @@ static int adv7180_s_routing(struct v4l2_subdev *sd, u32 input,
 	if (ret)
 		return ret;
 
-	/* We cannot discriminate between LQFP and 40-pin LFCSP, so accept
-	 * all inputs and let the card driver take care of validation
-	 */
-	if ((input & ADV7180_INPUT_CONTROL_INSEL_MASK) != input)
+	if (input > 31 || !(BIT(input) & state->chip_info->valid_input_mask)) {
+		ret = -EINVAL;
 		goto out;
+	}
 
-	ret = adv7180_read(state, ADV7180_REG_INPUT_CONTROL);
-	if (ret < 0)
-		goto out;
+	ret = state->chip_info->select_input(state, input);
 
-	ret &= ~ADV7180_INPUT_CONTROL_INSEL_MASK;
-	ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL, ret | input);
-	state->input = input;
+	if (ret == 0)
+		state->input = input;
 out:
 	mutex_unlock(&state->mutex);
 	return ret;
@@ -309,9 +336,8 @@ static int adv7180_program_std(struct adv7180_state *state)
 	int ret;
 
 	if (state->autodetect) {
-		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
-				    ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
-				    | state->input);
+		ret = adv7180_set_video_standard(state,
+			ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
 		if (ret < 0)
 			return ret;
 
@@ -321,8 +347,7 @@ static int adv7180_program_std(struct adv7180_state *state)
 		if (ret < 0)
 			return ret;
 
-		ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
-				    ret | state->input);
+		ret = adv7180_set_video_standard(state, ret);
 		if (ret < 0)
 			return ret;
 	}
@@ -522,6 +547,7 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
 	.g_mbus_config = adv7180_g_mbus_config,
 };
 
+
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
 	.s_power = adv7180_s_power,
 };
@@ -555,33 +581,77 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
 	return IRQ_HANDLED;
 }
 
-static int init_device(struct adv7180_state *state)
+static int adv7180_init(struct adv7180_state *state)
 {
 	int ret;
 
-	mutex_lock(&state->mutex);
-
-	adv7180_write(state, ADV7180_REG_PWR_MAN, ADV7180_PWR_MAN_RES);
-	usleep_range(2000, 10000);
-
-	ret = adv7180_program_std(state);
-	if (ret)
-		goto out_unlock;
-
 	/* ITU-R BT.656-4 compatible */
 	ret = adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
 			ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS);
 	if (ret < 0)
-		goto out_unlock;
+		return ret;
 
 	/* Manually set V bit end position in NTSC mode */
-	ret = adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
+	return adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
 					ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
+}
+
+static int adv7180_set_std(struct adv7180_state *state, unsigned int std)
+{
+	return adv7180_write(state, ADV7180_REG_INPUT_CONTROL,
+		(std << 4) | state->input);
+}
+
+static int adv7180_select_input(struct adv7180_state *state, unsigned int input)
+{
+	int ret;
+
+	ret = adv7180_read(state, ADV7180_REG_INPUT_CONTROL);
 	if (ret < 0)
+		return ret;
+
+	ret &= ~ADV7180_INPUT_CONTROL_INSEL_MASK;
+	ret |= input;
+	return adv7180_write(state, ADV7180_REG_INPUT_CONTROL, ret);
+}
+
+static const struct adv7180_chip_info adv7180_info = {
+	.flags = ADV7180_FLAG_RESET_POWERED,
+	/* We cannot discriminate between LQFP and 40-pin LFCSP, so accept
+	 * all inputs and let the card driver take care of validation
+	 */
+	.valid_input_mask = BIT(ADV7180_INPUT_CVBS_AIN1) |
+		BIT(ADV7180_INPUT_CVBS_AIN2) |
+		BIT(ADV7180_INPUT_CVBS_AIN3) |
+		BIT(ADV7180_INPUT_CVBS_AIN4) |
+		BIT(ADV7180_INPUT_CVBS_AIN5) |
+		BIT(ADV7180_INPUT_CVBS_AIN6) |
+		BIT(ADV7180_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7180_INPUT_SVIDEO_AIN3_AIN4) |
+		BIT(ADV7180_INPUT_SVIDEO_AIN5_AIN6) |
+		BIT(ADV7180_INPUT_YPRPB_AIN1_AIN2_AIN3) |
+		BIT(ADV7180_INPUT_YPRPB_AIN4_AIN5_AIN6),
+	.init = adv7180_init,
+	.set_std = adv7180_set_std,
+	.select_input = adv7180_select_input,
+};
+
+static int init_device(struct adv7180_state *state)
+{
+	int ret;
+
+	mutex_lock(&state->mutex);
+
+	adv7180_write(state, ADV7180_REG_PWR_MAN, ADV7180_PWR_MAN_RES);
+	usleep_range(2000, 10000);
+
+	ret = state->chip_info->init(state);
+	if (ret)
 		goto out_unlock;
 
-	/* read current norm */
-	__adv7180_status(state, NULL, &state->curr_norm);
+	ret = adv7180_program_std(state);
+	if (ret)
+		goto out_unlock;
 
 	/* register for interrupts */
 	if (state->irq > 0) {
@@ -638,11 +708,15 @@ static int adv7180_probe(struct i2c_client *client,
 	}
 
 	state->client = client;
+	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
 
 	state->irq = client->irq;
 	mutex_init(&state->mutex);
 	state->autodetect = true;
-	state->powered = true;
+	if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
+		state->powered = true;
+	else
+		state->powered = false;
 	state->input = 0;
 	sd = &state->sd;
 	v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
@@ -706,9 +780,10 @@ static int adv7180_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id adv7180_id[] = {
-	{KBUILD_MODNAME, 0},
+	{ "adv7180", (kernel_ulong_t)&adv7180_info },
 	{},
 };
+MODULE_DEVICE_TABLE(i2c, adv7180_id);
 
 #ifdef CONFIG_PM_SLEEP
 static int adv7180_suspend(struct device *dev)
@@ -745,8 +820,6 @@ static SIMPLE_DEV_PM_OPS(adv7180_pm_ops, adv7180_suspend, adv7180_resume);
 #define ADV7180_PM_OPS NULL
 #endif
 
-MODULE_DEVICE_TABLE(i2c, adv7180_id);
-
 static struct i2c_driver adv7180_driver = {
 	.driver = {
 		   .owner = THIS_MODULE,
-- 
1.8.0


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

* [PATCH v2 10/15] [media] adv7180: Add support for the adv7182
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (8 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 09/15] [media] adv7180: Prepare for multi-chip support Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 11/15] [media] adv7180: Add support for the adv7280/adv7281/adv7282 Lars-Peter Clausen
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

This patch adds support for the adv7182 to the adv7180 driver. The adv7182
is similar to the adv7180, the main difference from the driver's point of
view is how the video input and how the input format are selected.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 149 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 149 insertions(+)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index e3f91d5..4e518d5 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -52,6 +52,8 @@
 #define ADV7180_REG_INPUT_CONTROL			0x0000
 #define ADV7180_INPUT_CONTROL_INSEL_MASK		0x0f
 
+#define ADV7182_REG_INPUT_VIDSEL			0x0002
+
 #define ADV7180_REG_EXTENDED_OUTPUT_CONTROL		0x0004
 #define ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS		0xC5
 
@@ -134,6 +136,25 @@
 #define ADV7180_INPUT_YPRPB_AIN1_AIN2_AIN3 0x09
 #define ADV7180_INPUT_YPRPB_AIN4_AIN5_AIN6 0x0a
 
+#define ADV7182_INPUT_CVBS_AIN1 0x00
+#define ADV7182_INPUT_CVBS_AIN2 0x01
+#define ADV7182_INPUT_CVBS_AIN3 0x02
+#define ADV7182_INPUT_CVBS_AIN4 0x03
+#define ADV7182_INPUT_CVBS_AIN5 0x04
+#define ADV7182_INPUT_CVBS_AIN6 0x05
+#define ADV7182_INPUT_CVBS_AIN7 0x06
+#define ADV7182_INPUT_CVBS_AIN8 0x07
+#define ADV7182_INPUT_SVIDEO_AIN1_AIN2 0x08
+#define ADV7182_INPUT_SVIDEO_AIN3_AIN4 0x09
+#define ADV7182_INPUT_SVIDEO_AIN5_AIN6 0x0a
+#define ADV7182_INPUT_SVIDEO_AIN7_AIN8 0x0b
+#define ADV7182_INPUT_YPRPB_AIN1_AIN2_AIN3 0x0c
+#define ADV7182_INPUT_YPRPB_AIN4_AIN5_AIN6 0x0d
+#define ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2 0x0e
+#define ADV7182_INPUT_DIFF_CVBS_AIN3_AIN4 0x0f
+#define ADV7182_INPUT_DIFF_CVBS_AIN5_AIN6 0x10
+#define ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8 0x11
+
 struct adv7180_state;
 
 #define ADV7180_FLAG_RESET_POWERED	BIT(0)
@@ -615,6 +636,118 @@ static int adv7180_select_input(struct adv7180_state *state, unsigned int input)
 	return adv7180_write(state, ADV7180_REG_INPUT_CONTROL, ret);
 }
 
+static int adv7182_init(struct adv7180_state *state)
+{
+	/* ADI required writes */
+	adv7180_write(state, 0x0003, 0x0c);
+	adv7180_write(state, 0x0004, 0x07);
+	adv7180_write(state, 0x0013, 0x00);
+	adv7180_write(state, 0x001d, 0x40);
+
+	return 0;
+}
+
+static int adv7182_set_std(struct adv7180_state *state, unsigned int std)
+{
+	return adv7180_write(state, ADV7182_REG_INPUT_VIDSEL, std << 4);
+}
+
+enum adv7182_input_type {
+	ADV7182_INPUT_TYPE_CVBS,
+	ADV7182_INPUT_TYPE_DIFF_CVBS,
+	ADV7182_INPUT_TYPE_SVIDEO,
+	ADV7182_INPUT_TYPE_YPBPR,
+};
+
+static enum adv7182_input_type adv7182_get_input_type(unsigned int input)
+{
+	switch (input) {
+	case ADV7182_INPUT_CVBS_AIN1:
+	case ADV7182_INPUT_CVBS_AIN2:
+	case ADV7182_INPUT_CVBS_AIN3:
+	case ADV7182_INPUT_CVBS_AIN4:
+	case ADV7182_INPUT_CVBS_AIN5:
+	case ADV7182_INPUT_CVBS_AIN6:
+	case ADV7182_INPUT_CVBS_AIN7:
+	case ADV7182_INPUT_CVBS_AIN8:
+		return ADV7182_INPUT_TYPE_CVBS;
+	case ADV7182_INPUT_SVIDEO_AIN1_AIN2:
+	case ADV7182_INPUT_SVIDEO_AIN3_AIN4:
+	case ADV7182_INPUT_SVIDEO_AIN5_AIN6:
+	case ADV7182_INPUT_SVIDEO_AIN7_AIN8:
+		return ADV7182_INPUT_TYPE_SVIDEO;
+	case ADV7182_INPUT_YPRPB_AIN1_AIN2_AIN3:
+	case ADV7182_INPUT_YPRPB_AIN4_AIN5_AIN6:
+		return ADV7182_INPUT_TYPE_YPBPR;
+	case ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2:
+	case ADV7182_INPUT_DIFF_CVBS_AIN3_AIN4:
+	case ADV7182_INPUT_DIFF_CVBS_AIN5_AIN6:
+	case ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8:
+		return ADV7182_INPUT_TYPE_DIFF_CVBS;
+	default: /* Will never happen */
+		return 0;
+	}
+}
+
+/* ADI recommended writes to registers 0x52, 0x53, 0x54 */
+static unsigned int adv7182_lbias_settings[][3] = {
+	[ADV7182_INPUT_TYPE_CVBS] = { 0xCB, 0x4E, 0x80 },
+	[ADV7182_INPUT_TYPE_DIFF_CVBS] = { 0xC0, 0x4E, 0x80 },
+	[ADV7182_INPUT_TYPE_SVIDEO] = { 0x0B, 0xCE, 0x80 },
+	[ADV7182_INPUT_TYPE_YPBPR] = { 0x0B, 0x4E, 0xC0 },
+};
+
+static int adv7182_select_input(struct adv7180_state *state, unsigned int input)
+{
+	enum adv7182_input_type input_type;
+	unsigned int *lbias;
+	unsigned int i;
+	int ret;
+
+	ret = adv7180_write(state, ADV7180_REG_INPUT_CONTROL, input);
+	if (ret)
+		return ret;
+
+	/* Reset clamp circuitry - ADI recommended writes */
+	adv7180_write(state, 0x809c, 0x00);
+	adv7180_write(state, 0x809c, 0xff);
+
+	input_type = adv7182_get_input_type(input);
+
+	switch (input_type) {
+	case ADV7182_INPUT_TYPE_CVBS:
+	case ADV7182_INPUT_TYPE_DIFF_CVBS:
+		/* ADI recommends to use the SH1 filter */
+		adv7180_write(state, 0x0017, 0x41);
+		break;
+	default:
+		adv7180_write(state, 0x0017, 0x01);
+		break;
+	}
+
+	lbias = adv7182_lbias_settings[input_type];
+
+	for (i = 0; i < ARRAY_SIZE(adv7182_lbias_settings[0]); i++)
+		adv7180_write(state, 0x0052 + i, lbias[i]);
+
+	if (input_type == ADV7182_INPUT_TYPE_DIFF_CVBS) {
+		/* ADI required writes to make differential CVBS work */
+		adv7180_write(state, 0x005f, 0xa8);
+		adv7180_write(state, 0x005a, 0x90);
+		adv7180_write(state, 0x0060, 0xb0);
+		adv7180_write(state, 0x80b6, 0x08);
+		adv7180_write(state, 0x80c0, 0xa0);
+	} else {
+		adv7180_write(state, 0x005f, 0xf0);
+		adv7180_write(state, 0x005a, 0xd0);
+		adv7180_write(state, 0x0060, 0x10);
+		adv7180_write(state, 0x80b6, 0x9c);
+		adv7180_write(state, 0x80c0, 0x00);
+	}
+
+	return 0;
+}
+
 static const struct adv7180_chip_info adv7180_info = {
 	.flags = ADV7180_FLAG_RESET_POWERED,
 	/* We cannot discriminate between LQFP and 40-pin LFCSP, so accept
@@ -636,6 +769,21 @@ static const struct adv7180_chip_info adv7180_info = {
 	.select_input = adv7180_select_input,
 };
 
+static const struct adv7180_chip_info adv7182_info = {
+	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
+		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN3) |
+		BIT(ADV7182_INPUT_CVBS_AIN4) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_YPRPB_AIN1_AIN2_AIN3) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN3_AIN4),
+	.init = adv7182_init,
+	.set_std = adv7182_set_std,
+	.select_input = adv7182_select_input,
+};
+
 static int init_device(struct adv7180_state *state)
 {
 	int ret;
@@ -781,6 +929,7 @@ static int adv7180_remove(struct i2c_client *client)
 
 static const struct i2c_device_id adv7180_id[] = {
 	{ "adv7180", (kernel_ulong_t)&adv7180_info },
+	{ "adv7182", (kernel_ulong_t)&adv7182_info },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, adv7180_id);
-- 
1.8.0


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

* [PATCH v2 11/15] [media] adv7180: Add support for the adv7280/adv7281/adv7282
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (9 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 10/15] [media] adv7180: Add support for the adv7182 Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 12/15] [media] adv7180: Add support for the adv7280-m/adv7281-m/adv7281-ma/adv7282-m Lars-Peter Clausen
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

This patch adds support for the adv7280/adv7281/adv7282 devices to the
adv7180 driver. They are very similar to the adv7182, the main difference
from the drivers point of view are some different tuning constants for
improved video performance.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 56 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 4e518d5..ea6695c 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -158,6 +158,7 @@
 struct adv7180_state;
 
 #define ADV7180_FLAG_RESET_POWERED	BIT(0)
+#define ADV7180_FLAG_V2			BIT(1)
 
 struct adv7180_chip_info {
 	unsigned int flags;
@@ -638,9 +639,18 @@ static int adv7180_select_input(struct adv7180_state *state, unsigned int input)
 
 static int adv7182_init(struct adv7180_state *state)
 {
+	if (state->chip_info->flags & ADV7180_FLAG_V2) {
+		/* ADI recommended writes for improved video quality */
+		adv7180_write(state, 0x0080, 0x51);
+		adv7180_write(state, 0x0081, 0x51);
+		adv7180_write(state, 0x0082, 0x68);
+		adv7180_write(state, 0x0004, 0x17);
+	} else {
+		adv7180_write(state, 0x0004, 0x07);
+	}
+
 	/* ADI required writes */
 	adv7180_write(state, 0x0003, 0x0c);
-	adv7180_write(state, 0x0004, 0x07);
 	adv7180_write(state, 0x0013, 0x00);
 	adv7180_write(state, 0x001d, 0x40);
 
@@ -697,6 +707,13 @@ static unsigned int adv7182_lbias_settings[][3] = {
 	[ADV7182_INPUT_TYPE_YPBPR] = { 0x0B, 0x4E, 0xC0 },
 };
 
+static unsigned int adv7280_lbias_settings[][3] = {
+	[ADV7182_INPUT_TYPE_CVBS] = { 0xCD, 0x4E, 0x80 },
+	[ADV7182_INPUT_TYPE_DIFF_CVBS] = { 0xC0, 0x4E, 0x80 },
+	[ADV7182_INPUT_TYPE_SVIDEO] = { 0x0B, 0xCE, 0x80 },
+	[ADV7182_INPUT_TYPE_YPBPR] = { 0x0B, 0x4E, 0xC0 },
+};
+
 static int adv7182_select_input(struct adv7180_state *state, unsigned int input)
 {
 	enum adv7182_input_type input_type;
@@ -725,7 +742,10 @@ static int adv7182_select_input(struct adv7180_state *state, unsigned int input)
 		break;
 	}
 
-	lbias = adv7182_lbias_settings[input_type];
+	if (state->chip_info->flags & ADV7180_FLAG_V2)
+		lbias = adv7280_lbias_settings[input_type];
+	else
+		lbias = adv7182_lbias_settings[input_type];
 
 	for (i = 0; i < ARRAY_SIZE(adv7182_lbias_settings[0]); i++)
 		adv7180_write(state, 0x0052 + i, lbias[i]);
@@ -784,6 +804,35 @@ static const struct adv7180_chip_info adv7182_info = {
 	.select_input = adv7182_select_input,
 };
 
+static const struct adv7180_chip_info adv7280_info = {
+	.flags = ADV7180_FLAG_V2,
+	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
+		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN3) |
+		BIT(ADV7182_INPUT_CVBS_AIN4) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_YPRPB_AIN1_AIN2_AIN3),
+	.init = adv7182_init,
+	.set_std = adv7182_set_std,
+	.select_input = adv7182_select_input,
+};
+
+static const struct adv7180_chip_info adv7281_info = {
+	.flags = ADV7180_FLAG_V2,
+	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
+		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN7) |
+		BIT(ADV7182_INPUT_CVBS_AIN8) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN7_AIN8) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8),
+	.init = adv7182_init,
+	.set_std = adv7182_set_std,
+	.select_input = adv7182_select_input,
+};
+
 static int init_device(struct adv7180_state *state)
 {
 	int ret;
@@ -930,6 +979,9 @@ static int adv7180_remove(struct i2c_client *client)
 static const struct i2c_device_id adv7180_id[] = {
 	{ "adv7180", (kernel_ulong_t)&adv7180_info },
 	{ "adv7182", (kernel_ulong_t)&adv7182_info },
+	{ "adv7280", (kernel_ulong_t)&adv7280_info },
+	{ "adv7281", (kernel_ulong_t)&adv7281_info },
+	{ "adv7282", (kernel_ulong_t)&adv7281_info },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, adv7180_id);
-- 
1.8.0


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

* [PATCH v2 12/15] [media] adv7180: Add support for the adv7280-m/adv7281-m/adv7281-ma/adv7282-m
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (10 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 11/15] [media] adv7180: Add support for the adv7280/adv7281/adv7282 Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 13/15] [media] adv7180: Add I2P support Lars-Peter Clausen
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

This patch adds support for the adv7280-m/adv2781-m/adv7281-ma/adv7282-m
devices to the adv7180 driver. They are very similar to the
adv7280/adv7281/adv7282 but instead of parallel video out they feature a
MIPI CSI2 transmitter.

The CSI2 transmitter is configured via a separate I2C address, so we need to
register a dummy device for it.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 170 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 154 insertions(+), 16 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index ea6695c..868a677 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -124,6 +124,11 @@
 #define ADV7180_REG_NTSC_V_BIT_END	0x00E6
 #define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND	0x4F
 
+#define ADV7180_REG_CSI_SLAVE_ADDR	0xFE
+
+#define ADV7180_CSI_REG_PWRDN	0x00
+#define ADV7180_CSI_PWRDN	0x80
+
 #define ADV7180_INPUT_CVBS_AIN1 0x00
 #define ADV7180_INPUT_CVBS_AIN2 0x01
 #define ADV7180_INPUT_CVBS_AIN3 0x02
@@ -155,10 +160,13 @@
 #define ADV7182_INPUT_DIFF_CVBS_AIN5_AIN6 0x10
 #define ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8 0x11
 
+#define ADV7180_DEFAULT_CSI_I2C_ADDR 0x44
+
 struct adv7180_state;
 
 #define ADV7180_FLAG_RESET_POWERED	BIT(0)
 #define ADV7180_FLAG_V2			BIT(1)
+#define ADV7180_FLAG_MIPI_CSI2		BIT(2)
 
 struct adv7180_chip_info {
 	unsigned int flags;
@@ -181,6 +189,7 @@ struct adv7180_state {
 
 	struct i2c_client	*client;
 	unsigned int		register_page;
+	struct i2c_client	*csi_client;
 	const struct adv7180_chip_info *chip_info;
 };
 
@@ -215,6 +224,12 @@ static int adv7180_read(struct adv7180_state *state, unsigned int reg)
 	return i2c_smbus_read_byte_data(state->client, reg & 0xff);
 }
 
+static int adv7180_csi_write(struct adv7180_state *state, unsigned int reg,
+	unsigned int value)
+{
+	return i2c_smbus_write_byte_data(state->csi_client, reg, value);
+}
+
 static int adv7180_set_video_standard(struct adv7180_state *state,
 	unsigned int std)
 {
@@ -407,13 +422,31 @@ out:
 static int adv7180_set_power(struct adv7180_state *state, bool on)
 {
 	u8 val;
+	int ret;
 
 	if (on)
 		val = ADV7180_PWR_MAN_ON;
 	else
 		val = ADV7180_PWR_MAN_OFF;
 
-	return adv7180_write(state, ADV7180_REG_PWR_MAN, val);
+	ret = adv7180_write(state, ADV7180_REG_PWR_MAN, val);
+	if (ret)
+		return ret;
+
+	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
+		if (on) {
+			adv7180_csi_write(state, 0xDE, 0x02);
+			adv7180_csi_write(state, 0xD2, 0xF7);
+			adv7180_csi_write(state, 0xD8, 0x65);
+			adv7180_csi_write(state, 0xE0, 0x09);
+			adv7180_csi_write(state, 0x2C, 0x00);
+			adv7180_csi_write(state, 0x00, 0x00);
+		} else {
+			adv7180_csi_write(state, 0x00, 0x80);
+		}
+	}
+
+	return 0;
 }
 
 static int adv7180_s_power(struct v4l2_subdev *sd, int on)
@@ -550,13 +583,22 @@ static int adv7180_set_pad_format(struct v4l2_subdev *sd,
 static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
 				 struct v4l2_mbus_config *cfg)
 {
-	/*
-	 * The ADV7180 sensor supports BT.601/656 output modes.
-	 * The BT.656 is default and not yet configurable by s/w.
-	 */
-	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-		     V4L2_MBUS_DATA_ACTIVE_HIGH;
-	cfg->type = V4L2_MBUS_BT656;
+	struct adv7180_state *state = to_state(sd);
+
+	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
+		cfg->type = V4L2_MBUS_CSI2;
+		cfg->flags = V4L2_MBUS_CSI2_1_LANE |
+				V4L2_MBUS_CSI2_CHANNEL_0 |
+				V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+	} else {
+		/*
+		 * The ADV7180 sensor supports BT.601/656 output modes.
+		 * The BT.656 is default and not yet configurable by s/w.
+		 */
+		cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
+				 V4L2_MBUS_DATA_ACTIVE_HIGH;
+		cfg->type = V4L2_MBUS_BT656;
+	}
 
 	return 0;
 }
@@ -639,20 +681,32 @@ static int adv7180_select_input(struct adv7180_state *state, unsigned int input)
 
 static int adv7182_init(struct adv7180_state *state)
 {
+	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2)
+		adv7180_write(state, ADV7180_REG_CSI_SLAVE_ADDR,
+			ADV7180_DEFAULT_CSI_I2C_ADDR << 1);
+
 	if (state->chip_info->flags & ADV7180_FLAG_V2) {
 		/* ADI recommended writes for improved video quality */
 		adv7180_write(state, 0x0080, 0x51);
 		adv7180_write(state, 0x0081, 0x51);
 		adv7180_write(state, 0x0082, 0x68);
-		adv7180_write(state, 0x0004, 0x17);
-	} else {
-		adv7180_write(state, 0x0004, 0x07);
 	}
 
 	/* ADI required writes */
-	adv7180_write(state, 0x0003, 0x0c);
+	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
+		adv7180_write(state, 0x0003, 0x4e);
+		adv7180_write(state, 0x0004, 0x57);
+		adv7180_write(state, 0x001d, 0xc0);
+	} else {
+		if (state->chip_info->flags & ADV7180_FLAG_V2)
+			adv7180_write(state, 0x0004, 0x17);
+		else
+			adv7180_write(state, 0x0004, 0x07);
+		adv7180_write(state, 0x0003, 0x0c);
+		adv7180_write(state, 0x001d, 0x40);
+	}
+
 	adv7180_write(state, 0x0013, 0x00);
-	adv7180_write(state, 0x001d, 0x40);
 
 	return 0;
 }
@@ -818,15 +872,81 @@ static const struct adv7180_chip_info adv7280_info = {
 	.select_input = adv7182_select_input,
 };
 
+static const struct adv7180_chip_info adv7280_m_info = {
+	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_MIPI_CSI2,
+	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
+		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN3) |
+		BIT(ADV7182_INPUT_CVBS_AIN4) |
+		BIT(ADV7182_INPUT_CVBS_AIN5) |
+		BIT(ADV7182_INPUT_CVBS_AIN6) |
+		BIT(ADV7182_INPUT_CVBS_AIN7) |
+		BIT(ADV7182_INPUT_CVBS_AIN8) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN5_AIN6) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN7_AIN8) |
+		BIT(ADV7182_INPUT_YPRPB_AIN1_AIN2_AIN3) |
+		BIT(ADV7182_INPUT_YPRPB_AIN4_AIN5_AIN6),
+	.init = adv7182_init,
+	.set_std = adv7182_set_std,
+	.select_input = adv7182_select_input,
+};
+
 static const struct adv7180_chip_info adv7281_info = {
-	.flags = ADV7180_FLAG_V2,
+	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_MIPI_CSI2,
+	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
+		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN7) |
+		BIT(ADV7182_INPUT_CVBS_AIN8) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN7_AIN8) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8),
+	.init = adv7182_init,
+	.set_std = adv7182_set_std,
+	.select_input = adv7182_select_input,
+};
+
+static const struct adv7180_chip_info adv7281_m_info = {
+	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_MIPI_CSI2,
+	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
+		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN3) |
+		BIT(ADV7182_INPUT_CVBS_AIN4) |
+		BIT(ADV7182_INPUT_CVBS_AIN7) |
+		BIT(ADV7182_INPUT_CVBS_AIN8) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN7_AIN8) |
+		BIT(ADV7182_INPUT_YPRPB_AIN1_AIN2_AIN3) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8),
+	.init = adv7182_init,
+	.set_std = adv7182_set_std,
+	.select_input = adv7182_select_input,
+};
+
+static const struct adv7180_chip_info adv7281_ma_info = {
+	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_MIPI_CSI2,
 	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
 		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN3) |
+		BIT(ADV7182_INPUT_CVBS_AIN4) |
+		BIT(ADV7182_INPUT_CVBS_AIN5) |
+		BIT(ADV7182_INPUT_CVBS_AIN6) |
 		BIT(ADV7182_INPUT_CVBS_AIN7) |
 		BIT(ADV7182_INPUT_CVBS_AIN8) |
 		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN5_AIN6) |
 		BIT(ADV7182_INPUT_SVIDEO_AIN7_AIN8) |
+		BIT(ADV7182_INPUT_YPRPB_AIN1_AIN2_AIN3) |
+		BIT(ADV7182_INPUT_YPRPB_AIN4_AIN5_AIN6) |
 		BIT(ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN5_AIN6) |
 		BIT(ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8),
 	.init = adv7182_init,
 	.set_std = adv7182_set_std,
@@ -907,6 +1027,13 @@ static int adv7180_probe(struct i2c_client *client,
 	state->client = client;
 	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
 
+	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
+		state->csi_client = i2c_new_dummy(client->adapter,
+				ADV7180_DEFAULT_CSI_I2C_ADDR);
+		if (!state->csi_client)
+			return -ENOMEM;
+	}
+
 	state->irq = client->irq;
 	mutex_init(&state->mutex);
 	state->autodetect = true;
@@ -921,7 +1048,7 @@ static int adv7180_probe(struct i2c_client *client,
 
 	ret = adv7180_init_controls(state);
 	if (ret)
-		goto err_unreg_subdev;
+		goto err_unregister_csi_client;
 
 	state->pad.flags = MEDIA_PAD_FL_SOURCE;
 	sd->entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
@@ -954,7 +1081,9 @@ err_media_entity_cleanup:
 	media_entity_cleanup(&sd->entity);
 err_free_ctrl:
 	adv7180_exit_controls(state);
-err_unreg_subdev:
+err_unregister_csi_client:
+	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2)
+		i2c_unregister_device(state->csi_client);
 	mutex_destroy(&state->mutex);
 err:
 	return ret;
@@ -972,7 +1101,12 @@ static int adv7180_remove(struct i2c_client *client)
 
 	media_entity_cleanup(&sd->entity);
 	adv7180_exit_controls(state);
+
+	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2)
+		i2c_unregister_device(state->csi_client);
+
 	mutex_destroy(&state->mutex);
+
 	return 0;
 }
 
@@ -980,8 +1114,12 @@ static const struct i2c_device_id adv7180_id[] = {
 	{ "adv7180", (kernel_ulong_t)&adv7180_info },
 	{ "adv7182", (kernel_ulong_t)&adv7182_info },
 	{ "adv7280", (kernel_ulong_t)&adv7280_info },
+	{ "adv7280-m", (kernel_ulong_t)&adv7280_m_info },
 	{ "adv7281", (kernel_ulong_t)&adv7281_info },
+	{ "adv7281-m", (kernel_ulong_t)&adv7281_m_info },
+	{ "adv7281-ma", (kernel_ulong_t)&adv7281_ma_info },
 	{ "adv7282", (kernel_ulong_t)&adv7281_info },
+	{ "adv7282-m", (kernel_ulong_t)&adv7281_m_info },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, adv7180_id);
-- 
1.8.0


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

* [PATCH v2 13/15] [media] adv7180: Add I2P support
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (11 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 12/15] [media] adv7180: Add support for the adv7280-m/adv7281-m/adv7281-ma/adv7282-m Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 14/15] [media] adv7180: Add fast switch support Lars-Peter Clausen
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Some of the devices supported by this driver have a interlaced-to-
progressive converter which can optionally be enabled. This patch adds
support for enabling and disabling the I2P converter on such devices.

I2P mode can be enabled by selecting V4L2_FIELD_NONE instead of
V4L2_FIELD_INTERLACED for the format.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7180.c | 156 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 148 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 868a677..4d789c7 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -124,6 +124,7 @@
 #define ADV7180_REG_NTSC_V_BIT_END	0x00E6
 #define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND	0x4F
 
+#define ADV7180_REG_VPP_SLAVE_ADDR	0xFD
 #define ADV7180_REG_CSI_SLAVE_ADDR	0xFE
 
 #define ADV7180_CSI_REG_PWRDN	0x00
@@ -161,12 +162,14 @@
 #define ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8 0x11
 
 #define ADV7180_DEFAULT_CSI_I2C_ADDR 0x44
+#define ADV7180_DEFAULT_VPP_I2C_ADDR 0x42
 
 struct adv7180_state;
 
 #define ADV7180_FLAG_RESET_POWERED	BIT(0)
 #define ADV7180_FLAG_V2			BIT(1)
 #define ADV7180_FLAG_MIPI_CSI2		BIT(2)
+#define ADV7180_FLAG_I2P		BIT(3)
 
 struct adv7180_chip_info {
 	unsigned int flags;
@@ -190,7 +193,9 @@ struct adv7180_state {
 	struct i2c_client	*client;
 	unsigned int		register_page;
 	struct i2c_client	*csi_client;
+	struct i2c_client	*vpp_client;
 	const struct adv7180_chip_info *chip_info;
+	enum v4l2_field		field;
 };
 
 static struct adv7180_state *ctrl_to_adv7180(struct v4l2_ctrl *ctrl)
@@ -236,6 +241,12 @@ static int adv7180_set_video_standard(struct adv7180_state *state,
 	return state->chip_info->set_std(state, std);
 }
 
+static int adv7180_vpp_write(struct adv7180_state *state, unsigned int reg,
+	unsigned int value)
+{
+	return i2c_smbus_write_byte_data(state->vpp_client, reg, value);
+}
+
 static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
 {
 	/* in case V4L2_IN_ST_NO_SIGNAL */
@@ -440,6 +451,8 @@ static int adv7180_set_power(struct adv7180_state *state, bool on)
 			adv7180_csi_write(state, 0xD8, 0x65);
 			adv7180_csi_write(state, 0xE0, 0x09);
 			adv7180_csi_write(state, 0x2C, 0x00);
+			if (state->field == V4L2_FIELD_NONE)
+				adv7180_csi_write(state, 0x1D, 0x80);
 			adv7180_csi_write(state, 0x00, 0x00);
 		} else {
 			adv7180_csi_write(state, 0x00, 0x80);
@@ -559,25 +572,97 @@ static int adv7180_mbus_fmt(struct v4l2_subdev *sd,
 
 	fmt->code = MEDIA_BUS_FMT_YUYV8_2X8;
 	fmt->colorspace = V4L2_COLORSPACE_SMPTE170M;
-	fmt->field = V4L2_FIELD_INTERLACED;
 	fmt->width = 720;
 	fmt->height = state->curr_norm & V4L2_STD_525_60 ? 480 : 576;
 
 	return 0;
 }
 
+static int adv7180_set_field_mode(struct adv7180_state *state)
+{
+	if (!(state->chip_info->flags & ADV7180_FLAG_I2P))
+		return 0;
+
+	if (state->field == V4L2_FIELD_NONE) {
+		if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
+			adv7180_csi_write(state, 0x01, 0x20);
+			adv7180_csi_write(state, 0x02, 0x28);
+			adv7180_csi_write(state, 0x03, 0x38);
+			adv7180_csi_write(state, 0x04, 0x30);
+			adv7180_csi_write(state, 0x05, 0x30);
+			adv7180_csi_write(state, 0x06, 0x80);
+			adv7180_csi_write(state, 0x07, 0x70);
+			adv7180_csi_write(state, 0x08, 0x50);
+		}
+		adv7180_vpp_write(state, 0xa3, 0x00);
+		adv7180_vpp_write(state, 0x5b, 0x00);
+		adv7180_vpp_write(state, 0x55, 0x80);
+	} else {
+		if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
+			adv7180_csi_write(state, 0x01, 0x18);
+			adv7180_csi_write(state, 0x02, 0x18);
+			adv7180_csi_write(state, 0x03, 0x30);
+			adv7180_csi_write(state, 0x04, 0x20);
+			adv7180_csi_write(state, 0x05, 0x28);
+			adv7180_csi_write(state, 0x06, 0x40);
+			adv7180_csi_write(state, 0x07, 0x58);
+			adv7180_csi_write(state, 0x08, 0x30);
+		}
+		adv7180_vpp_write(state, 0xa3, 0x70);
+		adv7180_vpp_write(state, 0x5b, 0x80);
+		adv7180_vpp_write(state, 0x55, 0x00);
+	}
+
+	return 0;
+}
+
 static int adv7180_get_pad_format(struct v4l2_subdev *sd,
 				  struct v4l2_subdev_fh *fh,
 				  struct v4l2_subdev_format *format)
 {
-	return adv7180_mbus_fmt(sd, &format->format);
+	struct adv7180_state *state = to_state(sd);
+
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+		format->format = *v4l2_subdev_get_try_format(fh, 0);
+	} else {
+		adv7180_mbus_fmt(sd, &format->format);
+		format->format.field = state->field;
+	}
+
+	return 0;
 }
 
 static int adv7180_set_pad_format(struct v4l2_subdev *sd,
 				  struct v4l2_subdev_fh *fh,
 				  struct v4l2_subdev_format *format)
 {
-	return adv7180_mbus_fmt(sd, &format->format);
+	struct adv7180_state *state = to_state(sd);
+	struct v4l2_mbus_framefmt *framefmt;
+
+	switch (format->format.field) {
+	case V4L2_FIELD_NONE:
+		if (!(state->chip_info->flags & ADV7180_FLAG_I2P))
+			format->format.field = V4L2_FIELD_INTERLACED;
+		break;
+	default:
+		format->format.field = V4L2_FIELD_INTERLACED;
+		break;
+	}
+
+	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+		framefmt = &format->format;
+		if (state->field != format->format.field) {
+			state->field = format->format.field;
+			adv7180_set_power(state, false);
+			adv7180_set_field_mode(state);
+			adv7180_set_power(state, true);
+		}
+	} else {
+		framefmt = v4l2_subdev_get_try_format(fh, 0);
+		*framefmt = format->format;
+	}
+
+	return adv7180_mbus_fmt(sd, framefmt);
 }
 
 static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
@@ -685,6 +770,10 @@ static int adv7182_init(struct adv7180_state *state)
 		adv7180_write(state, ADV7180_REG_CSI_SLAVE_ADDR,
 			ADV7180_DEFAULT_CSI_I2C_ADDR << 1);
 
+	if (state->chip_info->flags & ADV7180_FLAG_I2P)
+		adv7180_write(state, ADV7180_REG_VPP_SLAVE_ADDR,
+			ADV7180_DEFAULT_VPP_I2C_ADDR << 1);
+
 	if (state->chip_info->flags & ADV7180_FLAG_V2) {
 		/* ADI recommended writes for improved video quality */
 		adv7180_write(state, 0x0080, 0x51);
@@ -859,7 +948,7 @@ static const struct adv7180_chip_info adv7182_info = {
 };
 
 static const struct adv7180_chip_info adv7280_info = {
-	.flags = ADV7180_FLAG_V2,
+	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_I2P,
 	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
 		BIT(ADV7182_INPUT_CVBS_AIN2) |
 		BIT(ADV7182_INPUT_CVBS_AIN3) |
@@ -873,7 +962,7 @@ static const struct adv7180_chip_info adv7280_info = {
 };
 
 static const struct adv7180_chip_info adv7280_m_info = {
-	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_MIPI_CSI2,
+	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_MIPI_CSI2 | ADV7180_FLAG_I2P,
 	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
 		BIT(ADV7182_INPUT_CVBS_AIN2) |
 		BIT(ADV7182_INPUT_CVBS_AIN3) |
@@ -953,6 +1042,40 @@ static const struct adv7180_chip_info adv7281_ma_info = {
 	.select_input = adv7182_select_input,
 };
 
+static const struct adv7180_chip_info adv7282_info = {
+	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_I2P,
+	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
+		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN7) |
+		BIT(ADV7182_INPUT_CVBS_AIN8) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN7_AIN8) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8),
+	.init = adv7182_init,
+	.set_std = adv7182_set_std,
+	.select_input = adv7182_select_input,
+};
+
+static const struct adv7180_chip_info adv7282_m_info = {
+	.flags = ADV7180_FLAG_V2 | ADV7180_FLAG_MIPI_CSI2 | ADV7180_FLAG_I2P,
+	.valid_input_mask = BIT(ADV7182_INPUT_CVBS_AIN1) |
+		BIT(ADV7182_INPUT_CVBS_AIN2) |
+		BIT(ADV7182_INPUT_CVBS_AIN3) |
+		BIT(ADV7182_INPUT_CVBS_AIN4) |
+		BIT(ADV7182_INPUT_CVBS_AIN7) |
+		BIT(ADV7182_INPUT_CVBS_AIN8) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_SVIDEO_AIN7_AIN8) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN1_AIN2) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN3_AIN4) |
+		BIT(ADV7182_INPUT_DIFF_CVBS_AIN7_AIN8),
+	.init = adv7182_init,
+	.set_std = adv7182_set_std,
+	.select_input = adv7182_select_input,
+};
+
 static int init_device(struct adv7180_state *state)
 {
 	int ret;
@@ -970,6 +1093,8 @@ static int init_device(struct adv7180_state *state)
 	if (ret)
 		goto out_unlock;
 
+	adv7180_set_field_mode(state);
+
 	/* register for interrupts */
 	if (state->irq > 0) {
 		/* config the Interrupt pin to be active low */
@@ -1025,6 +1150,7 @@ static int adv7180_probe(struct i2c_client *client,
 	}
 
 	state->client = client;
+	state->field = V4L2_FIELD_INTERLACED;
 	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
 
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
@@ -1034,6 +1160,15 @@ static int adv7180_probe(struct i2c_client *client,
 			return -ENOMEM;
 	}
 
+	if (state->chip_info->flags & ADV7180_FLAG_I2P) {
+		state->vpp_client = i2c_new_dummy(client->adapter,
+				ADV7180_DEFAULT_VPP_I2C_ADDR);
+		if (!state->vpp_client) {
+			ret = -ENOMEM;
+			goto err_unregister_csi_client;
+		}
+	}
+
 	state->irq = client->irq;
 	mutex_init(&state->mutex);
 	state->autodetect = true;
@@ -1048,7 +1183,7 @@ static int adv7180_probe(struct i2c_client *client,
 
 	ret = adv7180_init_controls(state);
 	if (ret)
-		goto err_unregister_csi_client;
+		goto err_unregister_vpp_client;
 
 	state->pad.flags = MEDIA_PAD_FL_SOURCE;
 	sd->entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
@@ -1081,6 +1216,9 @@ err_media_entity_cleanup:
 	media_entity_cleanup(&sd->entity);
 err_free_ctrl:
 	adv7180_exit_controls(state);
+err_unregister_vpp_client:
+	if (state->chip_info->flags & ADV7180_FLAG_I2P)
+		i2c_unregister_device(state->vpp_client);
 err_unregister_csi_client:
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2)
 		i2c_unregister_device(state->csi_client);
@@ -1102,6 +1240,8 @@ static int adv7180_remove(struct i2c_client *client)
 	media_entity_cleanup(&sd->entity);
 	adv7180_exit_controls(state);
 
+	if (state->chip_info->flags & ADV7180_FLAG_I2P)
+		i2c_unregister_device(state->vpp_client);
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2)
 		i2c_unregister_device(state->csi_client);
 
@@ -1118,8 +1258,8 @@ static const struct i2c_device_id adv7180_id[] = {
 	{ "adv7281", (kernel_ulong_t)&adv7281_info },
 	{ "adv7281-m", (kernel_ulong_t)&adv7281_m_info },
 	{ "adv7281-ma", (kernel_ulong_t)&adv7281_ma_info },
-	{ "adv7282", (kernel_ulong_t)&adv7281_info },
-	{ "adv7282-m", (kernel_ulong_t)&adv7281_m_info },
+	{ "adv7282", (kernel_ulong_t)&adv7282_info },
+	{ "adv7282-m", (kernel_ulong_t)&adv7282_m_info },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, adv7180_id);
-- 
1.8.0


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

* [PATCH v2 14/15] [media] adv7180: Add fast switch support
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (12 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 13/15] [media] adv7180: Add I2P support Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-23 15:52 ` [PATCH v2 15/15] [media] Add MAINTAINERS entry for the adv7180 Lars-Peter Clausen
  2015-01-25 17:34 ` [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Federico Vaga
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

In fast switch mode the adv7180 (and similar) can lock onto a new signal
faster when switching between different inputs. As a downside though it is
no longer able to auto-detect the incoming format.

The fast switch mode is exposed as a boolean v4l control that allows
userspace applications to either enable or disable fast switch mode.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
--
Changes since v1:
	* Reserve private control range and use it for the fast switch control
	* Changed control name from "Fast switch" to "Fast Switch"
---
 drivers/media/i2c/adv7180.c        | 29 +++++++++++++++++++++++++++++
 include/uapi/linux/v4l2-controls.h |  4 ++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 4d789c7..ee73b29 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -127,6 +127,9 @@
 #define ADV7180_REG_VPP_SLAVE_ADDR	0xFD
 #define ADV7180_REG_CSI_SLAVE_ADDR	0xFE
 
+#define ADV7180_REG_FLCONTROL 0x40e0
+#define ADV7180_FLCONTROL_FL_ENABLE 0x1
+
 #define ADV7180_CSI_REG_PWRDN	0x00
 #define ADV7180_CSI_PWRDN	0x80
 
@@ -164,6 +167,8 @@
 #define ADV7180_DEFAULT_CSI_I2C_ADDR 0x44
 #define ADV7180_DEFAULT_VPP_I2C_ADDR 0x42
 
+#define V4L2_CID_ADV_FAST_SWITCH	(V4L2_CID_USER_ADV7180_BASE + 0x00)
+
 struct adv7180_state;
 
 #define ADV7180_FLAG_RESET_POWERED	BIT(0)
@@ -509,6 +514,18 @@ static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl)
 			break;
 		ret = adv7180_write(state, ADV7180_REG_SD_SAT_CR, val);
 		break;
+	case V4L2_CID_ADV_FAST_SWITCH:
+		if (ctrl->val) {
+			/* ADI required write */
+			adv7180_write(state, 0x80d9, 0x44);
+			adv7180_write(state, ADV7180_REG_FLCONTROL,
+				ADV7180_FLCONTROL_FL_ENABLE);
+		} else {
+			/* ADI required write */
+			adv7180_write(state, 0x80d9, 0xc4);
+			adv7180_write(state, ADV7180_REG_FLCONTROL, 0x00);
+		}
+		break;
 	default:
 		ret = -EINVAL;
 	}
@@ -521,6 +538,16 @@ static const struct v4l2_ctrl_ops adv7180_ctrl_ops = {
 	.s_ctrl = adv7180_s_ctrl,
 };
 
+static const struct v4l2_ctrl_config adv7180_ctrl_fast_switch = {
+	.ops = &adv7180_ctrl_ops,
+	.id = V4L2_CID_ADV_FAST_SWITCH,
+	.name = "Fast Switching",
+	.type = V4L2_CTRL_TYPE_BOOLEAN,
+	.min = 0,
+	.max = 1,
+	.step = 1,
+};
+
 static int adv7180_init_controls(struct adv7180_state *state)
 {
 	v4l2_ctrl_handler_init(&state->ctrl_hdl, 4);
@@ -537,6 +564,8 @@ static int adv7180_init_controls(struct adv7180_state *state)
 	v4l2_ctrl_new_std(&state->ctrl_hdl, &adv7180_ctrl_ops,
 			  V4L2_CID_HUE, ADV7180_HUE_MIN,
 			  ADV7180_HUE_MAX, 1, ADV7180_HUE_DEF);
+	v4l2_ctrl_new_custom(&state->ctrl_hdl, &adv7180_ctrl_fast_switch, NULL);
+
 	state->sd.ctrl_handler = &state->ctrl_hdl;
 	if (state->ctrl_hdl.error) {
 		int err = state->ctrl_hdl.error;
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 661f119..9f6e108 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -170,6 +170,10 @@ enum v4l2_colorfx {
  * We reserve 16 controls for this driver. */
 #define V4L2_CID_USER_SAA7134_BASE		(V4L2_CID_USER_BASE + 0x1060)
 
+/* The base for the adv7180 driver controls.
+ * We reserve 16 controls for this driver. */
+#define V4L2_CID_USER_ADV7180_BASE		(V4L2_CID_USER_BASE + 0x1070)
+
 /* MPEG-class control IDs */
 /* The MPEG controls are applicable to all codec controls
  * and the 'MPEG' part of the define is historical */
-- 
1.8.0


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

* [PATCH v2 15/15] [media] Add MAINTAINERS entry for the adv7180
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (13 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 14/15] [media] adv7180: Add fast switch support Lars-Peter Clausen
@ 2015-01-23 15:52 ` Lars-Peter Clausen
  2015-01-25 17:34 ` [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Federico Vaga
  15 siblings, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-01-23 15:52 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media, Lars-Peter Clausen

Add myself as the maintainer for the adv7180 video subdev driver.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4318f34..22bb77e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -659,6 +659,13 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	drivers/media/i2c/ad9389b*
 
+ANALOG DEVICES INC ADV7180 DRIVER
+M:	Lars-Peter Clausen <lars@metafoo.de>
+L:	linux-media@vger.kernel.org
+W:	http://ez.analog.com/community/linux-device-drivers
+S:	Supported
+F:	drivers/media/i2c/adv7180.c
+
 ANALOG DEVICES INC ADV7511 DRIVER
 M:	Hans Verkuil <hans.verkuil@cisco.com>
 L:	linux-media@vger.kernel.org
-- 
1.8.0


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

* Re: [PATCH v2 00/15] [media] adv7180: Add support for more chip variants
  2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
                   ` (14 preceding siblings ...)
  2015-01-23 15:52 ` [PATCH v2 15/15] [media] Add MAINTAINERS entry for the adv7180 Lars-Peter Clausen
@ 2015-01-25 17:34 ` Federico Vaga
  15 siblings, 0 replies; 20+ messages in thread
From: Federico Vaga @ 2015-01-25 17:34 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Hans Verkuil, Sergei Shtylyov, Vladimir Barinov,
	Richard Röjfors, linux-media

On Friday 23 January 2015 16:52:19 Lars-Peter Clausen wrote:
> Changes from v1:
> 	* Reserved custom user control range for the fast switch control
> 	* Dropped the free-run mode control patch for now. The controls
> should probably be standardized first, but that is going to be a
> different patch series.
> 
> Original cover letter below:
> 
> The adv7180 is part of a larger family of chips which all implement
> different features from a feature superset. This patch series step
> by step extends the current adv7180 with features from the superset
> that are currently not supported and gradually adding support for
> more variations of the chip.
> 
> The first half of this series contains fixes and cleanups while the
> second half adds new features and support for new chips

I don't have any more the hardware to test the patches but everything 
seems fine to me. My 2 cents acked-by to the whole set of patches:

Acked-by: Federico Vaga <federico.vaga@gmail.com>

-- 
Federico Vaga

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

* Re: [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro
  2015-01-23 15:52 ` [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro Lars-Peter Clausen
@ 2015-02-02 13:36   ` Mauro Carvalho Chehab
  2015-02-02 13:42     ` Hans Verkuil
  2015-02-02 13:49     ` Lars-Peter Clausen
  0 siblings, 2 replies; 20+ messages in thread
From: Mauro Carvalho Chehab @ 2015-02-02 13:36 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Hans Verkuil, Sergei Shtylyov, Vladimir Barinov,
	Richard Röjfors, Federico Vaga, linux-media

Em Fri, 23 Jan 2015 16:52:22 +0100
Lars-Peter Clausen <lars@metafoo.de> escreveu:

> Use a inline function instead of a macro for the container_of helper for
> getting the driver's state struct from a control. A inline function has the
> advantage that it is more typesafe and nicer in general.

I don't see any advantage on this.

See: container_of is already a macro, and it is written in a way that, if
you use it with inconsistent values, the compilation will break.

Also, there's the risk that, for whatever reason, gcc to decide to not
inline this.

So, this doesn't sound a good idea.

Regards,
Mauro

> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/i2c/adv7180.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index f424a4d..f2508abe 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -130,9 +130,11 @@ struct adv7180_state {
>  	bool			powered;
>  	u8			input;
>  };
> -#define to_adv7180_sd(_ctrl) (&container_of(_ctrl->handler,		\
> -					    struct adv7180_state,	\
> -					    ctrl_hdl)->sd)
> +
> +static struct adv7180_state *ctrl_to_adv7180(struct v4l2_ctrl *ctrl)
> +{
> +	return container_of(ctrl->handler, struct adv7180_state, ctrl_hdl);
> +}
>  
>  static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
>  {
> @@ -345,9 +347,8 @@ static int adv7180_s_power(struct v4l2_subdev *sd, int on)
>  
>  static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
> -	struct v4l2_subdev *sd = to_adv7180_sd(ctrl);
> -	struct adv7180_state *state = to_state(sd);
> -	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +	struct adv7180_state *state = ctrl_to_adv7180(ctrl);
> +	struct i2c_client *client = v4l2_get_subdevdata(&state->sd);
>  	int ret = mutex_lock_interruptible(&state->mutex);
>  	int val;
>  

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

* Re: [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro
  2015-02-02 13:36   ` Mauro Carvalho Chehab
@ 2015-02-02 13:42     ` Hans Verkuil
  2015-02-02 13:49     ` Lars-Peter Clausen
  1 sibling, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2015-02-02 13:42 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Lars-Peter Clausen
  Cc: Sergei Shtylyov, Vladimir Barinov, Richard Röjfors,
	Federico Vaga, linux-media

On 02/02/2015 02:36 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 23 Jan 2015 16:52:22 +0100
> Lars-Peter Clausen <lars@metafoo.de> escreveu:
> 
>> Use a inline function instead of a macro for the container_of helper for
>> getting the driver's state struct from a control. A inline function has the
>> advantage that it is more typesafe and nicer in general.
> 
> I don't see any advantage on this.
> 
> See: container_of is already a macro, and it is written in a way that, if
> you use it with inconsistent values, the compilation will break.
> 
> Also, there's the risk that, for whatever reason, gcc to decide to not
> inline this.

For the record: I disagree with this. I think a static inline is more readable
than a macro. It is also consistent with other existing i2c subdev drivers since
they all use a static inline as well. And if in rare cases gcc might decide not
to inline this, then so what? You really won't notice any difference. It's an
i2c device, so it's slow as molasses anyway. Readability is much more important
IMHO.

On the other hand, it's not critical enough for me to make a big deal out of
it if this patch is dropped.

Regards,

	Hans

> 
> So, this doesn't sound a good idea.
> 
> Regards,
> Mauro
> 
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/i2c/adv7180.c | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
>> index f424a4d..f2508abe 100644
>> --- a/drivers/media/i2c/adv7180.c
>> +++ b/drivers/media/i2c/adv7180.c
>> @@ -130,9 +130,11 @@ struct adv7180_state {
>>  	bool			powered;
>>  	u8			input;
>>  };
>> -#define to_adv7180_sd(_ctrl) (&container_of(_ctrl->handler,		\
>> -					    struct adv7180_state,	\
>> -					    ctrl_hdl)->sd)
>> +
>> +static struct adv7180_state *ctrl_to_adv7180(struct v4l2_ctrl *ctrl)
>> +{
>> +	return container_of(ctrl->handler, struct adv7180_state, ctrl_hdl);
>> +}
>>  
>>  static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
>>  {
>> @@ -345,9 +347,8 @@ static int adv7180_s_power(struct v4l2_subdev *sd, int on)
>>  
>>  static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl)
>>  {
>> -	struct v4l2_subdev *sd = to_adv7180_sd(ctrl);
>> -	struct adv7180_state *state = to_state(sd);
>> -	struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +	struct adv7180_state *state = ctrl_to_adv7180(ctrl);
>> +	struct i2c_client *client = v4l2_get_subdevdata(&state->sd);
>>  	int ret = mutex_lock_interruptible(&state->mutex);
>>  	int val;
>>  


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

* Re: [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro
  2015-02-02 13:36   ` Mauro Carvalho Chehab
  2015-02-02 13:42     ` Hans Verkuil
@ 2015-02-02 13:49     ` Lars-Peter Clausen
  1 sibling, 0 replies; 20+ messages in thread
From: Lars-Peter Clausen @ 2015-02-02 13:49 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hans Verkuil, Sergei Shtylyov, Vladimir Barinov,
	Richard Röjfors, Federico Vaga, linux-media

On 02/02/2015 02:36 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 23 Jan 2015 16:52:22 +0100
> Lars-Peter Clausen <lars@metafoo.de> escreveu:
>
>> Use a inline function instead of a macro for the container_of helper for
>> getting the driver's state struct from a control. A inline function has the
>> advantage that it is more typesafe and nicer in general.
>
> I don't see any advantage on this.
>
> See: container_of is already a macro, and it is written in a way that, if
> you use it with inconsistent values, the compilation will break.

Yes, container_of is a macro, because it needs to be a macro. Compilation 
will also not always break if you pass in a incorrect type, it might succeed 
with even generating a warning. Furthermore if compilation breaks the error 
message is completely incomprehensible. Using a function instead makes sure 
that the error message you get is in the style of "passing argument of wrong 
type to function, expected typeX, got typeY".

>
> Also, there's the risk that, for whatever reason, gcc to decide to not
> inline this.

If the compiler does not inline this it probably has a good reason to do so. 
Not inlining this will not break the functionality, so it is not a problem.

- Lars

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

end of thread, other threads:[~2015-02-02 13:49 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-23 15:52 [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 01/15] [media] adv7180: Do not request the IRQ again during resume Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 02/15] [media] adv7180: Pass correct flags to request_threaded_irq() Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro Lars-Peter Clausen
2015-02-02 13:36   ` Mauro Carvalho Chehab
2015-02-02 13:42     ` Hans Verkuil
2015-02-02 13:49     ` Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 04/15] [media] adv7180: Cleanup register define naming Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 05/15] [media] adv7180: Do implicit register paging Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 06/15] [media] adv7180: Reset the device before initialization Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 07/15] [media] adv7180: Add media controller support Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 08/15] [media] adv7180: Consolidate video mode setting Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 09/15] [media] adv7180: Prepare for multi-chip support Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 10/15] [media] adv7180: Add support for the adv7182 Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 11/15] [media] adv7180: Add support for the adv7280/adv7281/adv7282 Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 12/15] [media] adv7180: Add support for the adv7280-m/adv7281-m/adv7281-ma/adv7282-m Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 13/15] [media] adv7180: Add I2P support Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 14/15] [media] adv7180: Add fast switch support Lars-Peter Clausen
2015-01-23 15:52 ` [PATCH v2 15/15] [media] Add MAINTAINERS entry for the adv7180 Lars-Peter Clausen
2015-01-25 17:34 ` [PATCH v2 00/15] [media] adv7180: Add support for more chip variants Federico Vaga

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.