Re: [RFC PATCH v2 1/4] media: i2c: adv748x: add adv748x driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sakari,

On 12/05/17 17:46, Sakari Ailus wrote:
> Hi Kieran,
> 
> Thanks for the patches!
> 
> Would you have a media-ctl -p && media-ctl --print-dot (or the PS file) to
> see how this looks like in practice?

Replying quickly to only this question for the moment :)

Please see:
  http://janet.linuxembedded.co.uk/board-salvator-mc.png

Thanks for the review - I'll go through on Monday. The weekend begins :)

--
Regards

Kieran


> 
> Please see my comments below.
> 
> On Thu, May 11, 2017 at 06:21:20PM +0100, Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>>
>> Provide basic support for the ADV7481 and ADV7482.
>>
>> The driver is modelled with 2 subdevices to allow simultaneous streaming
>> from the AFE (Analog front end) and HDMI inputs.
>>
>> Presently the HDMI is hardcoded to link to the TXA CSI bus, whilst the
>> AFE is linked to the TXB CSI bus.
>>
>> The driver is based on a prototype by Koji Matsuoka in the Renesas BSP,
>> and an earlier rework by Niklas Söderlund.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>>
>> ---
>>
>> v2:
>>  - Implement DT parsing
>>  - adv748x: Add CSI2 entity
>>  - adv748x: Rework pad allocations and fmts
>>  - Give AFE 8 input pads and move pad defines
>>  - Use the enums to ensure pads are referenced correctly.
>>  - adv748x: Rename AFE/HDMI entities
>>    Now they are 'just afe' and 'just hdmi'
>>  - Reorder the entity enum and structures
>>  - Added pad-format for the CSI2 entities
>>  - CSI2 s_stream pass through
>>  - CSI2 control pass through (with link following)
>> ---
>>  Documentation/devicetree/bindings/media/i2c/adv748x.txt |  63 +-
>>  MAINTAINERS                                             |   6 +-
>>  drivers/media/i2c/Kconfig                               |  10 +-
>>  drivers/media/i2c/Makefile                              |   1 +-
>>  drivers/media/i2c/adv748x/Makefile                      |   7 +-
>>  drivers/media/i2c/adv748x/adv748x-afe.c                 | 599 ++++++++-
>>  drivers/media/i2c/adv748x/adv748x-core.c                | 698 +++++++++-
>>  drivers/media/i2c/adv748x/adv748x-csi2.c                | 373 +++++-
>>  drivers/media/i2c/adv748x/adv748x-hdmi.c                | 671 +++++++++-
>>  drivers/media/i2c/adv748x/adv748x.h                     | 201 +++-
>>  10 files changed, 2629 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/adv748x.txt
>>  create mode 100644 drivers/media/i2c/adv748x/Makefile
>>  create mode 100644 drivers/media/i2c/adv748x/adv748x-afe.c
>>  create mode 100644 drivers/media/i2c/adv748x/adv748x-core.c
>>  create mode 100644 drivers/media/i2c/adv748x/adv748x-csi2.c
>>  create mode 100644 drivers/media/i2c/adv748x/adv748x-hdmi.c
>>  create mode 100644 drivers/media/i2c/adv748x/adv748x.h
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/adv748x.txt b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
>> new file mode 100644
>> index 000000000000..04d9ebc674a6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
>> @@ -0,0 +1,63 @@
>> +* Analog Devices ADV748X video decoder with HDMI receiver
>> +
>> +The ADV7481, and ADV7482 are multi format video decoders with an integrated
>> +HDMI receiver. It can output CSI-2 on two independent outputs TXA and TXB from
>> +three input sources HDMI, analog and TTL.
>> +
>> +Required Properties:
>> +
>> +  - compatible: Must contain one of the following
>> +    - "adi,adv7481" for the ADV7481
>> +    - "adi,adv7482" for the ADV7482
>> +
>> +  - reg: I2C slave address
>> +
>> +The device node must contain one 'port' child node per device input and output
>> +port, in accordance with the video interface bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
>> +are numbered as follows.
>> +
>> +  Name                  Type            Port
>> +------------------------------------------------------------
>> +  HDMI                  sink            0
>> +  AIN1                  sink            1
>> +  AIN2                  sink            2
>> +  AIN3                  sink            3
>> +  AIN4                  sink            4
>> +  AIN5                  sink            5
>> +  AIN6                  sink            6
>> +  AIN7                  sink            7
>> +  AIN8                  sink            8
>> +  TTL                   sink            9
>> +  TXA                   source          10
>> +  TXB                   source          11
>> +
>> +The digital output port node must contain at least one source endpoint.
>> +
>> +Example:
>> +
>> +    video_receiver@70 {
>> +            compatible = "adi,adv7482";
>> +            reg = <0x70>;
>> +
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            port@10 {
>> +                    reg = <10>;
>> +                    adv7482_txa: endpoint@1 {
>> +                            clock-lanes = <0>;
>> +                            data-lanes = <1 2 3 4>;
>> +                            remote-endpoint = <&csi40_in>;
>> +                    };
>> +            };
>> +
>> +            port@11 {
>> +                    reg = <11>;
>> +                    adv7482_txb: endpoint@1 {
>> +                            clock-lanes = <0>;
>> +                            data-lanes = <1>;
>> +                            remote-endpoint = <&csi20_in>;
>> +                    };
>> +            };
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 84f185731b3c..76356d2b8a69 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -774,6 +774,12 @@ L:	linux-media@xxxxxxxxxxxxxxx
>>  S:	Maintained
>>  F:	drivers/media/i2c/adv7511*
>>  
>> +ANALOG DEVICES INC ADV748X DRIVER
>> +M:	Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
>> +L:	linux-media@xxxxxxxxxxxxxxx
>> +S:	Maintained
>> +F:	drivers/media/i2c/adv748x/*
>> +
>>  ANALOG DEVICES INC ADV7604 DRIVER
>>  M:	Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>  L:	linux-media@xxxxxxxxxxxxxxx
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 7c23b7a1fd05..78f495c939a2 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -204,6 +204,16 @@ config VIDEO_ADV7183
>>  	  To compile this driver as a module, choose M here: the
>>  	  module will be called adv7183.
>>  
>> +config VIDEO_ADV748X
>> +	tristate "Analog Devices ADV748x decoder"
>> +	depends on VIDEO_V4L2 && I2C
> 
> VIDEO_V4L2_SUBDEV_API && 
> 
>> +	---help---
>> +	  V4l2 subdevice driver for the Analog Devices
>> +	  ADV7481 and ADV7482 HDMI/Analog video decoder.
>> +
>> +	  To compile this driver as a module, choose M here: the
>> +	  module will be called adv748x.
>> +
>>  config VIDEO_ADV7604
>>  	tristate "Analog Devices ADV7604 decoder"
>>  	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 62323ec66be8..e17faab108d6 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -1,6 +1,7 @@
>>  msp3400-objs	:=	msp3400-driver.o msp3400-kthreads.o
>>  obj-$(CONFIG_VIDEO_MSP3400) += msp3400.o
>>  
>> +obj-$(CONFIG_VIDEO_ADV748X) += adv748x/
>>  obj-$(CONFIG_VIDEO_SMIAPP)	+= smiapp/
>>  obj-$(CONFIG_VIDEO_ET8EK8)	+= et8ek8/
>>  obj-$(CONFIG_VIDEO_CX25840) += cx25840/
>> diff --git a/drivers/media/i2c/adv748x/Makefile b/drivers/media/i2c/adv748x/Makefile
>> new file mode 100644
>> index 000000000000..c0711e076f1d
>> --- /dev/null
>> +++ b/drivers/media/i2c/adv748x/Makefile
>> @@ -0,0 +1,7 @@
>> +adv748x-objs	:= \
>> +		adv748x-afe.o \
>> +		adv748x-core.o \
>> +		adv748x-csi2.o \
>> +		adv748x-hdmi.o
>> +
>> +obj-$(CONFIG_VIDEO_ADV748X)	+= adv748x.o
>> diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c b/drivers/media/i2c/adv748x/adv748x-afe.c
>> new file mode 100644
>> index 000000000000..ad2681c9d3b4
>> --- /dev/null
>> +++ b/drivers/media/i2c/adv748x/adv748x-afe.c
>> @@ -0,0 +1,599 @@
>> +/*
>> + * Driver for Analog Devices ADV748X 8 channel analog front end (AFE) receiver
>> + * with standard definition processor (SDP)
>> + *
>> + * Copyright (C) 2017 Renesas Electronics Corp.
>> + *
>> + * This program is free software; you can redistribute  it and/or modify it
>> + * under  the terms of  the GNU General  Public License as published by the
>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>> + * option) any later version.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/v4l2-dv-timings.h>
>> +
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-dv-timings.h>
>> +#include <media/v4l2-ioctl.h>
>> +
>> +#include "adv748x.h"
>> +
>> +/* -----------------------------------------------------------------------------
>> + * SDP
>> + */
>> +
>> +#define ADV748X_AFE_INPUT_CVBS_AIN1			0x00
>> +#define ADV748X_AFE_INPUT_CVBS_AIN2			0x01
>> +#define ADV748X_AFE_INPUT_CVBS_AIN3			0x02
>> +#define ADV748X_AFE_INPUT_CVBS_AIN4			0x03
>> +#define ADV748X_AFE_INPUT_CVBS_AIN5			0x04
>> +#define ADV748X_AFE_INPUT_CVBS_AIN6			0x05
>> +#define ADV748X_AFE_INPUT_CVBS_AIN7			0x06
>> +#define ADV748X_AFE_INPUT_CVBS_AIN8			0x07
>> +
>> +#define ADV748X_AFE_STD_AD_PAL_BG_NTSC_J_SECAM		0x0
>> +#define ADV748X_AFE_STD_AD_PAL_BG_NTSC_J_SECAM_PED	0x1
>> +#define ADV748X_AFE_STD_AD_PAL_N_NTSC_J_SECAM		0x2
>> +#define ADV748X_AFE_STD_AD_PAL_N_NTSC_M_SECAM		0x3
>> +#define ADV748X_AFE_STD_NTSC_J				0x4
>> +#define ADV748X_AFE_STD_NTSC_M				0x5
>> +#define ADV748X_AFE_STD_PAL60				0x6
>> +#define ADV748X_AFE_STD_NTSC_443			0x7
>> +#define ADV748X_AFE_STD_PAL_BG				0x8
>> +#define ADV748X_AFE_STD_PAL_N				0x9
>> +#define ADV748X_AFE_STD_PAL_M				0xa
>> +#define ADV748X_AFE_STD_PAL_M_PED			0xb
>> +#define ADV748X_AFE_STD_PAL_COMB_N			0xc
>> +#define ADV748X_AFE_STD_PAL_COMB_N_PED			0xd
>> +#define ADV748X_AFE_STD_PAL_SECAM			0xe
>> +#define ADV748X_AFE_STD_PAL_SECAM_PED			0xf
>> +
>> +static int adv748x_afe_read_ro_map(struct adv748x_state *state, u8 reg)
>> +{
>> +	int ret;
>> +
>> +	/* Select SDP Read-Only Main Map */
>> +	ret = sdp_write(state, 0x0e, 0x01);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return sdp_read(state, reg);
>> +}
>> +
>> +static int adv748x_afe_status(struct adv748x_state *state, u32 *signal,
>> +			      v4l2_std_id *std)
>> +{
>> +	int info;
>> +
>> +	/* Read status from reg 0x10 of SDP RO Map */
>> +	info = adv748x_afe_read_ro_map(state, 0x10);
>> +	if (info < 0)
>> +		return info;
>> +
>> +	if (signal)
>> +		*signal = info & BIT(0) ? 0 : V4L2_IN_ST_NO_SIGNAL;
>> +
>> +	if (std) {
> 
> if (!std)
> 	return 0;
> 
> And you can unindent the rest by one tab stop. I think it'd look nicer that
> way.
> 
>> +		*std = V4L2_STD_UNKNOWN;
>> +
>> +		/* Standard not valid if there is no signal */
>> +		if (info & BIT(0)) {
>> +			switch (info & 0x70) {
>> +			case 0x00:
>> +				*std = V4L2_STD_NTSC;
>> +				break;
>> +			case 0x10:
>> +				*std = V4L2_STD_NTSC_443;
>> +				break;
>> +			case 0x20:
>> +				*std = V4L2_STD_PAL_M;
>> +				break;
>> +			case 0x30:
>> +				*std = V4L2_STD_PAL_60;
>> +				break;
>> +			case 0x40:
>> +				*std = V4L2_STD_PAL;
>> +				break;
>> +			case 0x50:
>> +				*std = V4L2_STD_SECAM;
>> +				break;
>> +			case 0x60:
>> +				*std = V4L2_STD_PAL_Nc | V4L2_STD_PAL_N;
>> +				break;
>> +			case 0x70:
>> +				*std = V4L2_STD_SECAM;
>> +				break;
>> +			default:
>> +				*std = V4L2_STD_UNKNOWN;
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void adv748x_afe_fill_format(struct adv748x_state *state,
>> +				    struct v4l2_mbus_framefmt *fmt)
>> +{
>> +	v4l2_std_id std;
>> +
>> +	memset(fmt, 0, sizeof(*fmt));
> 
> I think it'd be good to leave this up to the V4L2 subdev framework to do:
> not every driver should be responsible for doing that. E.g. smiapp does not.
> I'm sure there are others. The V4L2 sub-device framework doesn't really help
> with that in any way and I'd say it's for historical reasons.
> 
> But please keep this for now...
> 
>> +
>> +	fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
>> +	fmt->colorspace = V4L2_COLORSPACE_SMPTE170M;
>> +	fmt->field = V4L2_FIELD_INTERLACED;
>> +
>> +	fmt->width = 720;
>> +
>> +	if (state->afe.curr_norm == V4L2_STD_ALL)
>> +		adv748x_afe_status(state, NULL,  &std);
>> +	else
>> +		std = state->afe.curr_norm;
>> +
>> +	fmt->height = std & V4L2_STD_525_60 ? 480 : 576;
>> +}
>> +
>> +static int adv748x_afe_std(v4l2_std_id std)
>> +{
>> +	if (std == V4L2_STD_ALL)
>> +		return ADV748X_AFE_STD_AD_PAL_BG_NTSC_J_SECAM;
>> +	if (std == V4L2_STD_PAL_60)
>> +		return ADV748X_AFE_STD_PAL60;
>> +	if (std == V4L2_STD_NTSC_443)
>> +		return ADV748X_AFE_STD_NTSC_443;
>> +	if (std == V4L2_STD_PAL_N)
>> +		return ADV748X_AFE_STD_PAL_N;
>> +	if (std == V4L2_STD_PAL_M)
>> +		return ADV748X_AFE_STD_PAL_M;
>> +	if (std == V4L2_STD_PAL_Nc)
>> +		return ADV748X_AFE_STD_PAL_COMB_N;
>> +	if (std & V4L2_STD_PAL)
>> +		return ADV748X_AFE_STD_PAL_BG;
>> +	if (std & V4L2_STD_NTSC)
>> +		return ADV748X_AFE_STD_NTSC_M;
>> +	if (std & V4L2_STD_SECAM)
>> +		return ADV748X_AFE_STD_PAL_SECAM;
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int adv748x_afe_set_video_standard(struct adv748x_state *state,
>> +					  v4l2_std_id std)
>> +{
>> +	int sdpstd;
>> +
>> +	sdpstd = adv748x_afe_std(std);
>> +	if (sdpstd < 0)
>> +		return sdpstd;
>> +
>> +	sdp_clrset(state, 0x02, 0xf0, (sdpstd & 0xf) << 4);
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_afe_g_pixelaspect(struct v4l2_subdev *sd,
>> +				     struct v4l2_fract *aspect)
>> +{
>> +	struct adv748x_state *state = adv748x_afe_to_state(sd);
>> +	v4l2_std_id std;
>> +
>> +	if (state->afe.curr_norm == V4L2_STD_ALL)
>> +		adv748x_afe_status(state, NULL,  &std);
>> +	else
>> +		std = state->afe.curr_norm;
>> +
>> +	if (std & V4L2_STD_525_60) {
>> +		aspect->numerator = 11;
>> +		aspect->denominator = 10;
>> +	} else {
>> +		aspect->numerator = 54;
>> +		aspect->denominator = 59;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_video_ops
>> + */
>> +
>> +static int adv748x_afe_g_std(struct v4l2_subdev *sd, v4l2_std_id *norm)
>> +{
>> +	struct adv748x_state *state = adv748x_afe_to_state(sd);
>> +
>> +	if (state->afe.curr_norm == V4L2_STD_ALL)
>> +		adv748x_afe_status(state, NULL,  norm);
>> +	else
>> +		*norm = state->afe.curr_norm;
>> +
>> +	return 0;
>> +}
>> +
>> +
>> +static int adv748x_afe_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
>> +{
>> +	struct adv748x_state *state = adv748x_afe_to_state(sd);
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&state->mutex);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = adv748x_afe_set_video_standard(state, std);
>> +	if (ret < 0)
>> +		goto out;
>> +
>> +	state->afe.curr_norm = std;
>> +
>> +out:
>> +	mutex_unlock(&state->mutex);
>> +	return ret;
>> +}
>> +
>> +static int adv748x_afe_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
>> +{
>> +	struct adv748x_state *state = adv748x_afe_to_state(sd);
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&state->mutex);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (state->afe.streaming) {
>> +		ret = -EBUSY;
>> +		goto unlock;
>> +	}
>> +
>> +	/* Set auto detect mode */
>> +	ret = adv748x_afe_set_video_standard(state, V4L2_STD_ALL);
>> +	if (ret)
>> +		goto unlock;
>> +
>> +	msleep(100);
>> +
>> +	/* Read detected standard */
>> +	ret = adv748x_afe_status(state, NULL, std);
>> +unlock:
>> +	mutex_unlock(&state->mutex);
>> +
>> +	return ret;
>> +}
>> +
>> +static int adv748x_afe_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
>> +{
>> +	*norm = V4L2_STD_ALL;
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_afe_g_input_status(struct v4l2_subdev *sd, u32 *status)
>> +{
>> +	struct adv748x_state *state = adv748x_afe_to_state(sd);
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&state->mutex);
> 
> I wonder if this is necessary. Do you expect the mutex to be taken for
> extended periods of time?
> 
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = adv748x_afe_status(state, status, NULL);
>> +
>> +	mutex_unlock(&state->mutex);
>> +	return ret;
>> +}
>> +
>> +static int adv748x_afe_s_stream(struct v4l2_subdev *sd, int enable)
>> +{
>> +	struct adv748x_state *state = adv748x_afe_to_state(sd);
>> +	int ret, signal = V4L2_IN_ST_NO_SIGNAL;
>> +
>> +	ret = mutex_lock_interruptible(&state->mutex);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = adv748x_txb_power(state, enable);
>> +	if (ret)
>> +		goto error;
>> +
>> +	state->afe.streaming = enable;
>> +
>> +	adv748x_afe_status(state, &signal, NULL);
>> +	if (signal != V4L2_IN_ST_NO_SIGNAL)
>> +		adv_dbg(state, "Detected SDP signal\n");
>> +	else
>> +		adv_info(state, "Couldn't detect SDP video signal\n");
>> +
>> +error:
>> +	mutex_unlock(&state->mutex);
>> +	return ret;
>> +}
>> +
>> +static const struct v4l2_subdev_video_ops adv748x_afe_video_ops = {
>> +	.g_std = adv748x_afe_g_std,
>> +	.s_std = adv748x_afe_s_std,
>> +	.querystd = adv748x_afe_querystd,
>> +	.g_tvnorms = adv748x_afe_g_tvnorms,
>> +	.g_input_status = adv748x_afe_g_input_status,
>> +	.s_stream = adv748x_afe_s_stream,
>> +	.g_pixelaspect = adv748x_afe_g_pixelaspect,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_pad_ops
>> + */
>> +
>> +static int adv748x_afe_enum_mbus_code(struct v4l2_subdev *sd,
>> +				      struct v4l2_subdev_pad_config *cfg,
>> +				      struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> +	if (code->index != 0)
>> +		return -EINVAL;
>> +
>> +	if (code->pad >= ADV748X_AFE_NR_PADS)
>> +		return -EINVAL;
> 
> That check exists in v4l2-subdev.c.
> 
>> +
>> +	code->code = MEDIA_BUS_FMT_UYVY8_2X8;
>> +
>> +	return 0;
>> +}
>> +
>> +
>> +static int adv748x_afe_get_pad_format(struct v4l2_subdev *sd,
>> +				      struct v4l2_subdev_pad_config *cfg,
>> +				      struct v4l2_subdev_format *format)
>> +{
>> +	struct adv748x_state *state = adv748x_afe_to_state(sd);
>> +
>> +	if (format->pad >= ADV748X_AFE_NR_PADS)
>> +		return -EINVAL;
> 
> Ditto.
> 
>> +
>> +	adv748x_afe_fill_format(state, &format->format);
>> +
>> +	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>> +		struct v4l2_mbus_framefmt *fmt;
>> +
>> +		fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
>> +		format->format.code = fmt->code;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_afe_set_pad_format(struct v4l2_subdev *sd,
>> +				      struct v4l2_subdev_pad_config *cfg,
>> +				      struct v4l2_subdev_format *format)
>> +{
>> +	struct adv748x_state *state = adv748x_afe_to_state(sd);
>> +
>> +	if (format->pad >= ADV748X_AFE_NR_PADS)
>> +		return -EINVAL;
> 
> Ditto.
> 
>> +
>> +	adv748x_afe_fill_format(state, &format->format);
>> +
>> +	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>> +		struct v4l2_mbus_framefmt *fmt;
>> +
>> +		fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
>> +		fmt->code = format->format.code;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_pad_ops adv748x_afe_pad_ops = {
>> +	.enum_mbus_code = adv748x_afe_enum_mbus_code,
>> +	.set_fmt = adv748x_afe_set_pad_format,
>> +	.get_fmt = adv748x_afe_get_pad_format,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_ops
>> + */
>> +
>> +static const struct v4l2_subdev_ops adv748x_afe_ops = {
>> +	.video = &adv748x_afe_video_ops,
>> +	.pad = &adv748x_afe_pad_ops,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * Controls
>> + */
>> +
>> +/* Contrast */
>> +#define ADV748X_AFE_REG_CON		0x08	/*Unsigned */
>> +#define ADV748X_AFE_CON_MIN		0
>> +#define ADV748X_AFE_CON_DEF		128
>> +#define ADV748X_AFE_CON_MAX		255
>> +/* Brightness*/
>> +#define ADV748X_AFE_REG_BRI		0x0a	/*Signed */
>> +#define ADV748X_AFE_BRI_MIN		-128
>> +#define ADV748X_AFE_BRI_DEF		0
>> +#define ADV748X_AFE_BRI_MAX		127
>> +/* Hue */
>> +#define ADV748X_AFE_REG_HUE		0x0b	/*Signed, inverted */
>> +#define ADV748X_AFE_HUE_MIN		-127
>> +#define ADV748X_AFE_HUE_DEF		0
>> +#define ADV748X_AFE_HUE_MAX		128
>> +
>> +/* Saturation */
>> +#define ADV748X_AFE_REG_SD_SAT_CB	0xe3
>> +#define ADV748X_AFE_REG_SD_SAT_CR	0xe4
>> +#define ADV748X_AFE_SAT_MIN		0
>> +#define ADV748X_AFE_SAT_DEF		128
>> +#define ADV748X_AFE_SAT_MAX		255
>> +
>> +static int __adv748x_afe_s_ctrl(struct v4l2_ctrl *ctrl,
>> +				struct adv748x_state *state)
>> +{
>> +	int ret;
>> +
>> +	ret = sdp_write(state, 0x0e, 0x00);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	switch (ctrl->id) {
>> +	case V4L2_CID_BRIGHTNESS:
>> +		if (ctrl->val < ADV748X_AFE_BRI_MIN ||
>> +		    ctrl->val > ADV748X_AFE_BRI_MAX)
>> +			return -ERANGE;
>> +
>> +		ret = sdp_write(state, ADV748X_AFE_REG_BRI, ctrl->val);
>> +		break;
>> +	case V4L2_CID_HUE:
>> +		if (ctrl->val < ADV748X_AFE_HUE_MIN ||
>> +		    ctrl->val > ADV748X_AFE_HUE_MAX)
>> +			return -ERANGE;
>> +
>> +		/* Hue is inverted according to HSL chart */
>> +		ret = sdp_write(state, ADV748X_AFE_REG_HUE, -ctrl->val);
>> +		break;
>> +	case V4L2_CID_CONTRAST:
>> +		if (ctrl->val < ADV748X_AFE_CON_MIN ||
>> +		    ctrl->val > ADV748X_AFE_CON_MAX)
>> +			return -ERANGE;
>> +
>> +		ret = sdp_write(state, ADV748X_AFE_REG_CON, ctrl->val);
>> +		break;
>> +	case V4L2_CID_SATURATION:
>> +		if (ctrl->val < ADV748X_AFE_SAT_MIN ||
>> +		    ctrl->val > ADV748X_AFE_SAT_MAX)
>> +			return -ERANGE;
>> +		/*
>> +		 * This could be V4L2_CID_BLUE_BALANCE/V4L2_CID_RED_BALANCE
>> +		 * Let's not confuse the user, everybody understands saturation
>> +		 */
>> +		ret = sdp_write(state, ADV748X_AFE_REG_SD_SAT_CB, ctrl->val);
>> +		if (ret)
>> +			break;
>> +		ret = sdp_write(state, ADV748X_AFE_REG_SD_SAT_CR, ctrl->val);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int adv748x_afe_s_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> +	struct adv748x_state *state =
>> +		container_of(ctrl->handler, struct adv748x_state, afe.ctrl_hdl);
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&state->mutex);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = __adv748x_afe_s_ctrl(ctrl, state);
>> +
>> +	mutex_unlock(&state->mutex);
>> +
>> +	return ret;
>> +}
>> +
>> +static int adv748x_afe_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> +	struct adv748x_state *state =
>> +		container_of(ctrl->handler, struct adv748x_state, afe.ctrl_hdl);
>> +	unsigned int width, height, fps;
>> +	v4l2_std_id std;
>> +
>> +	switch (ctrl->id) {
>> +	case V4L2_CID_PIXEL_RATE:
> 
> I presume you will know when PIXEL_RATE changes? You can use
> v4l2_ctrl_s_ctrl_int64() to change it. That way control events will be
> emitted correctly, too.
> 
> PIXEL_RATE is set READONLY (control flag) by the framework already.
> 
>> +		width = 720;
>> +		if (state->afe.curr_norm == V4L2_STD_ALL)
>> +			adv748x_afe_status(state, NULL,  &std);
>> +		else
>> +			std = state->afe.curr_norm;
>> +
>> +		height = std & V4L2_STD_525_60 ? 480 : 576;
>> +		fps = std & V4L2_STD_525_60 ? 30 : 25;
>> +
>> +		*ctrl->p_new.p_s64 = width * height * fps;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_ctrl_ops adv748x_afe_ctrl_ops = {
>> +	.s_ctrl = adv748x_afe_s_ctrl,
>> +	.g_volatile_ctrl = adv748x_afe_g_volatile_ctrl,
>> +};
>> +
>> +static int adv748x_afe_init_controls(struct adv748x_state *state)
>> +{
>> +	struct v4l2_ctrl *ctrl;
>> +
>> +	v4l2_ctrl_handler_init(&state->afe.ctrl_hdl, 5);
>> +
>> +	v4l2_ctrl_new_std(&state->afe.ctrl_hdl, &adv748x_afe_ctrl_ops,
>> +			  V4L2_CID_BRIGHTNESS, ADV748X_AFE_BRI_MIN,
>> +			  ADV748X_AFE_BRI_MAX, 1, ADV748X_AFE_BRI_DEF);
>> +	v4l2_ctrl_new_std(&state->afe.ctrl_hdl, &adv748x_afe_ctrl_ops,
>> +			  V4L2_CID_CONTRAST, ADV748X_AFE_CON_MIN,
>> +			  ADV748X_AFE_CON_MAX, 1, ADV748X_AFE_CON_DEF);
>> +	v4l2_ctrl_new_std(&state->afe.ctrl_hdl, &adv748x_afe_ctrl_ops,
>> +			  V4L2_CID_SATURATION, ADV748X_AFE_SAT_MIN,
>> +			  ADV748X_AFE_SAT_MAX, 1, ADV748X_AFE_SAT_DEF);
>> +	v4l2_ctrl_new_std(&state->afe.ctrl_hdl, &adv748x_afe_ctrl_ops,
>> +			  V4L2_CID_HUE, ADV748X_AFE_HUE_MIN,
>> +			  ADV748X_AFE_HUE_MAX, 1, ADV748X_AFE_HUE_DEF);
>> +	ctrl = v4l2_ctrl_new_std(&state->afe.ctrl_hdl, &adv748x_afe_ctrl_ops,
>> +				 V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, 1);
>> +	if (ctrl)
>> +		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> +
>> +	state->afe.sd.ctrl_handler = &state->afe.ctrl_hdl;
>> +	if (state->afe.ctrl_hdl.error) {
>> +		v4l2_ctrl_handler_free(&state->afe.ctrl_hdl);
>> +		return state->afe.ctrl_hdl.error;
>> +	}
>> +
>> +	return v4l2_ctrl_handler_setup(&state->afe.ctrl_hdl);
>> +}
>> +
>> +int adv748x_afe_probe(struct adv748x_state *state, struct device_node *ep)
>> +{
>> +	int ret;
>> +	unsigned int i;
>> +
>> +	state->afe.streaming = false;
>> +	state->afe.curr_norm = V4L2_STD_ALL;
>> +
>> +	adv748x_subdev_init(&state->afe.sd, state, &adv748x_afe_ops, "afe");
>> +
>> +	/* Ensure that matching is based upon the endpoint fwnodes */
>> +	state->afe.sd.fwnode = &ep->fwnode;
> 
> I wonder if you really need to convert all users of async matching to use
> endpoints --- could you opportunistically peek if the device node matches,
> just in case? You can't have an accidental positive match anyway.
> 
> So, the match is now for plain fwnode pointers, and it would be:
> 
> return async->fwnode == fwnode ||
> 	port_parent(parent(async->fwnode)) == fwnode ||
> 	async->fwnode == port_parent(parent(fwnode));
> 
>> +
>> +	for (i = ADV748X_AFE_SINK_AIN0; i <= ADV748X_AFE_SINK_AIN7; i++)
>> +		state->afe.pads[i].flags = MEDIA_PAD_FL_SINK;
>> +
>> +	state->afe.pads[ADV748X_AFE_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
>> +
>> +	ret = media_entity_pads_init(&state->afe.sd.entity, ADV748X_AFE_NR_PADS,
>> +			state->afe.pads);
>> +
>> +	ret = adv748x_afe_init_controls(state);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = v4l2_async_register_subdev(&state->afe.sd);
> 
> If v4l2_async_register_subdev() fails, you should release the control
> handler, shouldn't you?
> 
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +void adv748x_afe_remove(struct adv748x_state *state)
>> +{
>> +	v4l2_async_unregister_subdev(&state->afe.sd);
>> +	media_entity_cleanup(&state->afe.sd.entity);
>> +	v4l2_ctrl_handler_free(&state->afe.ctrl_hdl);
>> +}
>> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
>> new file mode 100644
>> index 000000000000..baa93c9f3fc6
>> --- /dev/null
>> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
>> @@ -0,0 +1,698 @@
>> +/*
>> + * Driver for Analog Devices ADV748X HDMI receiver with AFE
>> + *
>> + * Copyright (C) 2017 Renesas Electronics Corp.
>> + *
>> + * This program is free software; you can redistribute  it and/or modify it
>> + * under  the terms of  the GNU General  Public License as published by the
>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>> + * option) any later version.
>> + *
>> + * Authors:
>> + *	Koji Matsuoka <koji.matsuoka.xm@xxxxxxxxxxx>
>> + *	Niklas Söderlund <niklas.soderlund@xxxxxxxxxxxx>
>> + *	Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/errno.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/v4l2-dv-timings.h>
>> +
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-dv-timings.h>
>> +#include <media/v4l2-ioctl.h>
>> +
>> +#include "adv748x.h"
>> +
>> +/* -----------------------------------------------------------------------------
>> + * Register manipulation
>> + */
>> +
>> +/**
>> + * struct adv748x_reg_value - Register write instruction
>> + * @addr:		I2C slave address
>> + * @reg:		I2c register
>> + * @value:		value to write to @addr at @reg
>> + */
>> +struct adv748x_reg_value {
>> +	u8 addr;
>> +	u8 reg;
>> +	u8 value;
>> +};
>> +
>> +static int adv748x_write_regs(struct adv748x_state *state,
>> +			      const struct adv748x_reg_value *regs)
>> +{
>> +	struct i2c_msg msg;
>> +	u8 data_buf[2];
>> +	int ret = -EINVAL;
>> +
>> +	if (!state->client->adapter) {
>> +		adv_err(state, "No adapter for regs write\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	msg.flags = 0;
>> +	msg.len = 2;
>> +	msg.buf = &data_buf[0];
>> +
>> +	while (regs->addr != ADV748X_I2C_EOR) {
>> +
>> +		if (regs->addr == ADV748X_I2C_WAIT)
>> +			msleep(regs->value);
>> +		else {
>> +			msg.addr = regs->addr;
>> +			data_buf[0] = regs->reg;
>> +			data_buf[1] = regs->value;
>> +
>> +			ret = i2c_transfer(state->client->adapter, &msg, 1);
>> +			if (ret < 0) {
>> +				adv_err(state,
>> +					"Error regs addr: 0x%02x reg: 0x%02x\n",
>> +					regs->addr, regs->reg);
>> +				break;
>> +			}
>> +		}
>> +		regs++;
>> +	}
>> +
>> +	return (ret < 0) ? ret : 0;
>> +}
>> +
>> +int adv748x_write(struct adv748x_state *state, u8 addr, u8 reg, u8 value)
>> +{
>> +	struct adv748x_reg_value regs[2];
>> +	int ret;
>> +
>> +	regs[0].addr = addr;
>> +	regs[0].reg = reg;
>> +	regs[0].value = value;
>> +	regs[1].addr = ADV748X_I2C_EOR;
>> +	regs[1].reg = 0xFF;
>> +	regs[1].value = 0xFF;
>> +
>> +	ret = adv748x_write_regs(state, regs);
>> +
>> +	return ret;
>> +}
>> +
>> +int adv748x_read(struct adv748x_state *state, u8 addr, u8 reg)
>> +{
>> +	struct i2c_msg msg[2];
>> +	u8 reg_buf, data_buf;
>> +	int ret;
>> +
>> +	if (!state->client->adapter) {
>> +		adv_err(state, "No adapter reading addr: 0x%02x reg: 0x%02x\n",
>> +			addr, reg);
>> +		return -ENODEV;
>> +	}
>> +
>> +	msg[0].addr = addr;
>> +	msg[0].flags = 0;
>> +	msg[0].len = 1;
>> +	msg[0].buf = &reg_buf;
>> +	msg[1].addr = addr;
>> +	msg[1].flags = I2C_M_RD;
>> +	msg[1].len = 1;
>> +	msg[1].buf = &data_buf;
>> +
>> +	reg_buf = reg;
>> +
>> +	ret = i2c_transfer(state->client->adapter, msg, 2);
>> +	if (ret < 0) {
>> +		trace_printk("Error reading addr: 0x%02x reg: 0x%02x\n",
>> +			addr, reg);
>> +		adv_err(state, "Error reading addr: 0x%02x reg: 0x%02x\n",
>> +			addr, reg);
>> +		return ret;
>> +	}
>> +
>> +	return data_buf;
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * TXA and TXB
>> + */
>> +
>> +static const struct adv748x_reg_value adv748x_power_up_txa_4lane[] = {
>> +
>> +	{ADV748X_I2C_TXA, 0x00, 0x84},	/* Enable 4-lane MIPI */
>> +	{ADV748X_I2C_TXA, 0x00, 0xA4},	/* Set Auto DPHY Timing */
>> +
>> +	{ADV748X_I2C_TXA, 0x31, 0x82},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0x1E, 0x40},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0xDA, 0x01},	/* i2c_mipi_pll_en - 1'b1 */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x02},	/* delay 2 */
>> +	{ADV748X_I2C_TXA, 0x00, 0x24 },	/* Power-up CSI-TX */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x01},	/* delay 1 */
>> +	{ADV748X_I2C_TXA, 0xC1, 0x2B},	/* ADI Required Write */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x01},	/* delay 1 */
>> +	{ADV748X_I2C_TXA, 0x31, 0x80},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_EOR, 0xFF, 0xFF}	/* End of register table */
>> +};
>> +
>> +static const struct adv748x_reg_value adv748x_power_down_txa_4lane[] = {
>> +
>> +	{ADV748X_I2C_TXA, 0x31, 0x82},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0x1E, 0x00},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0x00, 0x84},	/* Enable 4-lane MIPI */
>> +	{ADV748X_I2C_TXA, 0xDA, 0x01},	/* i2c_mipi_pll_en - 1'b1 */
>> +	{ADV748X_I2C_TXA, 0xC1, 0x3B},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_EOR, 0xFF, 0xFF}	/* End of register table */
>> +};
>> +
>> +static const struct adv748x_reg_value adv748x_power_up_txb_1lane[] = {
>> +
>> +	{ADV748X_I2C_TXB, 0x00, 0x81},	/* Enable 1-lane MIPI */
>> +	{ADV748X_I2C_TXB, 0x00, 0xA1},	/* Set Auto DPHY Timing */
>> +
>> +	{ADV748X_I2C_TXB, 0x31, 0x82},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0x1E, 0x40},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0xDA, 0x01},	/* i2c_mipi_pll_en - 1'b1 */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x02},	/* delay 2 */
>> +	{ADV748X_I2C_TXB, 0x00, 0x21 },	/* Power-up CSI-TX */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x01},	/* delay 1 */
>> +	{ADV748X_I2C_TXB, 0xC1, 0x2B},	/* ADI Required Write */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x01},	/* delay 1 */
>> +	{ADV748X_I2C_TXB, 0x31, 0x80},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_EOR, 0xFF, 0xFF}	/* End of register table */
>> +};
>> +
>> +static const struct adv748x_reg_value adv748x_power_down_txb_1lane[] = {
>> +
>> +	{ADV748X_I2C_TXB, 0x31, 0x82},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0x1E, 0x00},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0x00, 0x81},	/* Enable 4-lane MIPI */
>> +	{ADV748X_I2C_TXB, 0xDA, 0x01},	/* i2c_mipi_pll_en - 1'b1 */
>> +	{ADV748X_I2C_TXB, 0xC1, 0x3B},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_EOR, 0xFF, 0xFF}	/* End of register table */
>> +};
>> +
>> +int adv748x_txa_power(struct adv748x_state *state, bool on)
>> +{
>> +	int val, ret;
>> +
>> +	val = txa_read(state, 0x1e);
>> +	if (val < 0)
>> +		return val;
>> +
>> +	if (on && ((val & 0x40) == 0))
>> +		ret = adv748x_write_regs(state, adv748x_power_up_txa_4lane);
>> +	else
>> +		ret = adv748x_write_regs(state, adv748x_power_down_txa_4lane);
>> +
>> +	return ret;
>> +}
>> +
>> +int adv748x_txb_power(struct adv748x_state *state, bool on)
>> +{
>> +	int val, ret;
>> +
>> +	val = txb_read(state, 0x1e);
>> +	if (val < 0)
>> +		return val;
>> +
>> +	if (on && ((val & 0x40) == 0))
>> +		ret = adv748x_write_regs(state, adv748x_power_up_txb_1lane);
>> +	else
>> +		ret = adv748x_write_regs(state, adv748x_power_down_txb_1lane);
>> +
>> +	return ret;
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * Media Operations
>> + */
>> +
>> +static const struct media_entity_operations adv748x_media_ops = {
>> +	.link_validate = v4l2_subdev_link_validate,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * HW setup
>> + */
>> +
>> +static const struct adv748x_reg_value adv748x_sw_reset[] = {
>> +
>> +	{ADV748X_I2C_IO, 0xFF, 0xFF},	/* SW reset */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x05},	/* delay 5 */
>> +	{ADV748X_I2C_IO, 0x01, 0x76},	/* ADI Required Write */
>> +	{ADV748X_I2C_IO, 0xF2, 0x01},	/* Enable I2C Read Auto-Increment */
>> +	{ADV748X_I2C_EOR, 0xFF, 0xFF}	/* End of register table */
>> +};
>> +
>> +static const struct adv748x_reg_value adv748x_set_slave_address[] = {
>> +	{ADV748X_I2C_IO, 0xF3, ADV748X_I2C_DPLL << 1},	/* DPLL */
>> +	{ADV748X_I2C_IO, 0xF4, ADV748X_I2C_CP << 1},	/* CP */
>> +	{ADV748X_I2C_IO, 0xF5, ADV748X_I2C_HDMI << 1},	/* HDMI */
>> +	{ADV748X_I2C_IO, 0xF6, ADV748X_I2C_EDID << 1},	/* EDID */
>> +	{ADV748X_I2C_IO, 0xF7, ADV748X_I2C_REPEATER << 1}, /* HDMI RX Repeater */
>> +	{ADV748X_I2C_IO, 0xF8, ADV748X_I2C_INFOFRAME << 1},/* HDMI RX InfoFrame*/
>> +	{ADV748X_I2C_IO, 0xFA, ADV748X_I2C_CEC << 1},	/* CEC */
>> +	{ADV748X_I2C_IO, 0xFB, ADV748X_I2C_SDP << 1},	/* SDP */
>> +	{ADV748X_I2C_IO, 0xFC, ADV748X_I2C_TXB << 1},	/* CSI-TXB */
>> +	{ADV748X_I2C_IO, 0xFD, ADV748X_I2C_TXA << 1},	/* CSI-TXA */
>> +	{ADV748X_I2C_EOR, 0xFF, 0xFF}	/* End of register table */
>> +};
>> +
>> +/* Supported Formats For Script Below */
>> +/* - 01-29 HDMI to MIPI TxA CSI 4-Lane - RGB888: */
>> +static const struct adv748x_reg_value adv748x_init_txa_4lane[] = {
>> +	/* Disable chip powerdown & Enable HDMI Rx block */
>> +	{ADV748X_I2C_IO, 0x00, 0x40},
>> +
>> +	{ADV748X_I2C_REPEATER, 0x40, 0x83}, /* Enable HDCP 1.1 */
>> +
>> +	{ADV748X_I2C_HDMI, 0x00, 0x08},	/* Foreground Channel = A */
>> +	{ADV748X_I2C_HDMI, 0x98, 0xFF},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x99, 0xA3},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x9A, 0x00},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x9B, 0x0A},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x9D, 0x40},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0xCB, 0x09},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x3D, 0x10},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x3E, 0x7B},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x3F, 0x5E},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x4E, 0xFE},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x4F, 0x18},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x57, 0xA3},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x58, 0x04},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0x85, 0x10},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_HDMI, 0x83, 0x00},	/* Enable All Terminations */
>> +	{ADV748X_I2C_HDMI, 0xA3, 0x01},	/* ADI Required Write */
>> +	{ADV748X_I2C_HDMI, 0xBE, 0x00},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_HDMI, 0x6C, 0x01},	/* HPA Manual Enable */
>> +	{ADV748X_I2C_HDMI, 0xF8, 0x01},	/* HPA Asserted */
>> +	{ADV748X_I2C_HDMI, 0x0F, 0x00},	/* Audio Mute Speed Set to Fastest */
>> +	/* (Smallest Step Size) */
>> +
>> +	{ADV748X_I2C_IO, 0x04, 0x02},	/* RGB Out of CP */
>> +	{ADV748X_I2C_IO, 0x12, 0xF0},	/* CSC Depends on ip Packets, SDR 444 */
>> +	{ADV748X_I2C_IO, 0x17, 0x80},	/* Luma & Chroma can reach 254d */
>> +	{ADV748X_I2C_IO, 0x03, 0x86},	/* CP-Insert_AV_Code */
>> +
>> +	{ADV748X_I2C_CP, 0x7C, 0x00},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_IO, 0x0C, 0xE0},	/* Enable LLC_DLL & Double LLC Timing */
>> +	{ADV748X_I2C_IO, 0x0E, 0xDD},	/* LLC/PIX/SPI PINS TRISTATED AUD */
>> +	/* Outputs Enabled */
>> +	{ADV748X_I2C_IO, 0x10, 0xA0},	/* Enable 4-lane CSI Tx & Pixel Port */
>> +
>> +	{ADV748X_I2C_TXA, 0x00, 0x84},	/* Enable 4-lane MIPI */
>> +	{ADV748X_I2C_TXA, 0x00, 0xA4},	/* Set Auto DPHY Timing */
>> +	{ADV748X_I2C_TXA, 0xDB, 0x10},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0xD6, 0x07},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0xC4, 0x0A},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0x71, 0x33},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0x72, 0x11},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0xF0, 0x00},	/* i2c_dphy_pwdn - 1'b0 */
>> +
>> +	{ADV748X_I2C_TXA, 0x31, 0x82},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0x1E, 0x40},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXA, 0xDA, 0x01},	/* i2c_mipi_pll_en - 1'b1 */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x02},	/* delay 2 */
>> +	{ADV748X_I2C_TXA, 0x00, 0x24 },	/* Power-up CSI-TX */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x01},	/* delay 1 */
>> +	{ADV748X_I2C_TXA, 0xC1, 0x2B},	/* ADI Required Write */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x01},	/* delay 1 */
>> +	{ADV748X_I2C_TXA, 0x31, 0x80},	/* ADI Required Write */
>> +
>> +#ifdef REL_DGB_FORCE_TO_SEND_COLORBAR
>> +	{ADV748X_I2C_CP, 0x37, 0x81},	/* Output Colorbars Pattern */
>> +#endif
>> +	{ADV748X_I2C_EOR, 0xFF, 0xFF}	/* End of register table */
>> +};
>> +
>> +/* TODO:KPB: Need to work out how to provide AFE port select! More entities? */
>> +#define ADV748X_SDP_INPUT_CVBS_AIN8 0x07
>> +
>> +/* 02-01 Analog CVBS to MIPI TX-B CSI 1-Lane - */
>> +/* Autodetect CVBS Single Ended In Ain 1 - MIPI Out */
>> +static const struct adv748x_reg_value adv748x_init_txb_1lane[] = {
>> +
>> +	{ADV748X_I2C_IO, 0x00, 0x30},  /* Disable chip powerdown powerdown Rx */
>> +	{ADV748X_I2C_IO, 0xF2, 0x01},  /* Enable I2C Read Auto-Increment */
>> +
>> +	{ADV748X_I2C_IO, 0x0E, 0xFF},  /* LLC/PIX/AUD/SPI PINS TRISTATED */
>> +
>> +	{ADV748X_I2C_SDP, 0x0f, 0x00}, /* Exit Power Down Mode */
>> +	{ADV748X_I2C_SDP, 0x52, 0xCD},/* ADI Required Write */
>> +	/* TODO: do not use hard codeded INSEL */
>> +	{ADV748X_I2C_SDP, 0x00, ADV748X_SDP_INPUT_CVBS_AIN8},
>> +	{ADV748X_I2C_SDP, 0x0E, 0x80},	/* ADI Required Write */
>> +	{ADV748X_I2C_SDP, 0x9C, 0x00},	/* ADI Required Write */
>> +	{ADV748X_I2C_SDP, 0x9C, 0xFF},	/* ADI Required Write */
>> +	{ADV748X_I2C_SDP, 0x0E, 0x00},	/* ADI Required Write */
>> +
>> +	/* ADI recommended writes for improved video quality */
>> +	{ADV748X_I2C_SDP, 0x80, 0x51},	/* ADI Required Write */
>> +	{ADV748X_I2C_SDP, 0x81, 0x51},	/* ADI Required Write */
>> +	{ADV748X_I2C_SDP, 0x82, 0x68},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_SDP, 0x03, 0x42},  /* Tri-S Output , PwrDwn 656 pads */
>> +	{ADV748X_I2C_SDP, 0x04, 0xB5},	/* ITU-R BT.656-4 compatible */
>> +	{ADV748X_I2C_SDP, 0x13, 0x00},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_SDP, 0x17, 0x41},	/* Select SH1 */
>> +	{ADV748X_I2C_SDP, 0x31, 0x12},	/* ADI Required Write */
>> +	{ADV748X_I2C_SDP, 0xE6, 0x4F},  /* V bit end pos manually in NTSC */
>> +
>> +	/* TODO: Convert this to a control option */
>> +#ifdef REL_DGB_FORCE_TO_SEND_COLORBAR
>> +	{ADV748X_I2C_SDP, 0x0C, 0x01},	/* ColorBar */
>> +	{ADV748X_I2C_SDP, 0x14, 0x01},	/* ColorBar */
>> +#endif
>> +	/* Enable 1-Lane MIPI Tx, */
>> +	/* enable pixel output and route SD through Pixel port */
>> +	{ADV748X_I2C_IO, 0x10, 0x70},
>> +
>> +	{ADV748X_I2C_TXB, 0x00, 0x81},	/* Enable 1-lane MIPI */
>> +	{ADV748X_I2C_TXB, 0x00, 0xA1},	/* Set Auto DPHY Timing */
>> +	{ADV748X_I2C_TXB, 0xD2, 0x40},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0xC4, 0x0A},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0x71, 0x33},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0x72, 0x11},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0xF0, 0x00},	/* i2c_dphy_pwdn - 1'b0 */
>> +	{ADV748X_I2C_TXB, 0x31, 0x82},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0x1E, 0x40},	/* ADI Required Write */
>> +	{ADV748X_I2C_TXB, 0xDA, 0x01},	/* i2c_mipi_pll_en - 1'b1 */
>> +
>> +	{ADV748X_I2C_WAIT, 0x00, 0x02},	/* delay 2 */
>> +	{ADV748X_I2C_TXB, 0x00, 0x21 },	/* Power-up CSI-TX */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x01},	/* delay 1 */
>> +	{ADV748X_I2C_TXB, 0xC1, 0x2B},	/* ADI Required Write */
>> +	{ADV748X_I2C_WAIT, 0x00, 0x01},	/* delay 1 */
>> +	{ADV748X_I2C_TXB, 0x31, 0x80},	/* ADI Required Write */
>> +
>> +	{ADV748X_I2C_EOR, 0xFF, 0xFF}	/* End of register table */
>> +};
>> +
>> +static int adv748x_reset(struct adv748x_state *state)
>> +{
>> +	int ret;
>> +
>> +	ret = adv748x_write_regs(state, adv748x_sw_reset);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = adv748x_write_regs(state, adv748x_set_slave_address);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* Init and power down TXA */
>> +	ret = adv748x_write_regs(state, adv748x_init_txa_4lane);
>> +	if (ret)
>> +		return ret;
>> +
>> +	adv748x_txa_power(state, 0);
>> +	/* Set VC 0 */
>> +	txa_clrset(state, 0x0d, 0xc0, 0x00);
>> +
>> +	/* Init and power down TXB */
>> +	ret = adv748x_write_regs(state, adv748x_init_txb_1lane);
>> +	if (ret)
>> +		return ret;
>> +
>> +	adv748x_txb_power(state, 0);
>> +
>> +	/* Set VC 0 */
>> +	txb_clrset(state, 0x0d, 0xc0, 0x00);
>> +
>> +	/* Disable chip powerdown & Enable HDMI Rx block */
>> +	io_write(state, 0x00, 0x40);
>> +
>> +	/* Enable 4-lane CSI Tx & Pixel Port */
>> +	io_write(state, 0x10, 0xe0);
>> +
>> +	/* Use vid_std and v_freq as freerun resolution for CP */
>> +	cp_clrset(state, 0xc9, 0x01, 0x01);
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_print_info(struct adv748x_state *state)
>> +{
>> +	int msb, lsb;
>> +
>> +	lsb = io_read(state, 0xdf);
>> +	msb = io_read(state, 0xe0);
>> +
>> +	if (lsb < 0 || msb < 0) {
>> +		adv_err(state, "Failed to read chip revision\n");
>> +		return -EIO;
>> +	}
>> +
>> +	adv_info(state, "chip found @ 0x%02x revision %02x%02x\n",
>> +		 state->client->addr << 1, lsb, msb);
>> +
>> +	return 0;
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * i2c driver
>> + */
>> +
>> +void adv748x_subdev_init(struct v4l2_subdev *sd, struct adv748x_state *state,
>> +		const struct v4l2_subdev_ops *ops, const char *ident)
>> +{
>> +	v4l2_subdev_init(sd, ops);
>> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +
>> +	/* the owner is the same as the i2c_client's driver owner */
>> +	sd->owner = state->dev->driver->owner;
>> +	sd->dev = state->dev;
>> +
>> +	v4l2_set_subdevdata(sd, state);
> 
> A lot of this would be done by v4l2_i2c_subdev_init(). You'd have to
> relinquish your use of v4l2_set_subdevdata() --- it always points to the I²C
> client in that case. You'll just need to dig your device state in a bit
> different manner in that case.
> 
>> +
>> +	/* initialize name */
>> +	snprintf(sd->name, sizeof(sd->name), "%s %d-%04x %s",
>> +		state->dev->driver->name,
>> +		i2c_adapter_id(state->client->adapter),
>> +		state->client->addr, ident);
>> +
>> +	sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
>> +	sd->entity.ops = &adv748x_media_ops;
>> +}
>> +
>> +static int adv748x_parse_dt(struct adv748x_state *state)
>> +{
>> +	struct device_node *ep_np = NULL;
>> +	struct of_endpoint ep;
>> +	unsigned int found = 0;
>> +	int ret;
>> +
> 
> Would of_graph_get_remote_node() do this more simple? Not necessarily, just
> wondering.
> 
>> +	for_each_endpoint_of_node(state->dev->of_node, ep_np) {
>> +		ret = of_graph_parse_endpoint(ep_np, &ep);
>> +		if (!ret) {
>> +			adv_info(state, "Endpoint %s on port %d",
>> +					of_node_full_name(ep.local_node),
>> +					ep.port);
>> +
>> +			if (ep.port > ADV748X_PORT_MAX) {
>> +				adv_err(state, "Invalid endpoint %s on port %d",
>> +					of_node_full_name(ep.local_node),
>> +					ep.port);
>> +
>> +				continue;
>> +			}
>> +
>> +			state->endpoints[ep.port] = ep_np;
>> +			found++;
>> +		}
>> +	}
>> +
>> +	return found ? 0 : -ENODEV;
>> +}
>> +
>> +int adv748x_setup_links(struct adv748x_state *state)
>> +{
>> +	int ret;
>> +	int enabled = MEDIA_LNK_FL_ENABLED;
>> +
>> +/*
>> + * HACK/Workaround:
>> + *
>> + * Currently non-immutable link resets go through the RVin
>> + * driver, and cause the links to fail, due to not being part of RVIN.
>> + * As a temporary workaround until the RVIN driver knows better than to parse
>> + * links that do not belong to it, use static immutable links for our internal
>> + * media paths.
>> + */
>> +#define ADV748x_DEV_STATIC_LINKS
>> +#ifdef ADV748x_DEV_STATIC_LINKS
>> +	enabled |= MEDIA_LNK_FL_IMMUTABLE;
>> +#endif
>> +
>> +	/* TXA - Default link is with HDMI */
>> +	ret = media_create_pad_link(&state->hdmi.sd.entity, 1,
>> +				    &state->txa.sd.entity, 0, enabled);
>> +	if (ret) {
>> +		adv_err(state, "Failed to create HDMI-TXA pad links");
>> +		return ret;
>> +	}
>> +
>> +#ifndef ADV748x_DEV_STATIC_LINKS
>> +	ret = media_create_pad_link(&state->afe.sd.entity, ADV748X_AFE_SOURCE,
>> +				    &state->txa.sd.entity, 0, 0);
>> +	if (ret) {
>> +		adv_err(state, "Failed to create AFE-TXA pad links");
>> +		return ret;
>> +	}
>> +#endif
>> +
>> +	/* TXB - Can only output from the AFE */
>> +	ret = media_create_pad_link(&state->afe.sd.entity, ADV748X_AFE_SOURCE,
>> +				    &state->txb.sd.entity, 0, enabled);
>> +	if (ret) {
>> +		adv_err(state, "Failed to create AFE-TXB pad links");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_probe(struct i2c_client *client,
>> +			 const struct i2c_device_id *id)
>> +{
>> +	struct adv748x_state *state;
>> +	struct device_node *ep;
>> +	int ret;
>> +
>> +	/* Check if the adapter supports the needed features */
>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>> +		return -EIO;
>> +
>> +	state = devm_kzalloc(&client->dev, sizeof(struct adv748x_state),
>> +			     GFP_KERNEL);
>> +	if (!state)
>> +		return -ENOMEM;
>> +
>> +	mutex_init(&state->mutex);
>> +
>> +	state->dev = &client->dev;
>> +	state->client = client;
>> +	i2c_set_clientdata(client, state);
>> +
>> +	/* SW reset ADV748X to its default values */
>> +	ret = adv748x_reset(state);
>> +	if (ret) {
>> +		adv_err(state, "Failed to reset hardware");
>> +		return ret;
> 
> Remember mutex_destroy(&state->mutex); . Perhaps worth using goto?
> 
>> +	}
>> +
>> +	ret = adv748x_print_info(state);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Discover and process ports declared by the Device tree endpoints */
>> +	ret = adv748x_parse_dt(state);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Process HDMI/TXA */
>> +	if (state->endpoints[ADV748X_PORT_HDMI]) {
>> +		ep = state->endpoints[ADV748X_PORT_HDMI];
>> +		adv_info(state, "Got TXA/HDMI EP: %s\n", of_node_full_name(ep));
>> +
>> +		/* Initialise HDMI */
>> +		ret = adv748x_hdmi_probe(state, ep);
>> +		if (ret) {
>> +			adv_err(state, "Failed to probe HDMI");
>> +			return ret;
>> +		}
>> +	} else {
>> +		adv_err(state, "No HDMI endpoint found at %s",
>> +				of_node_full_name(state->dev->of_node));
>> +	}
>> +
>> +	/* Process AFE/TXB */
>> +	/* TODO: EEP - HARDWARED AFE */
>> +	if (state->endpoints[ADV748X_PORT_AIN8]) {
>> +		ep = state->endpoints[ADV748X_PORT_AIN8];
>> +		adv_info(state, "Got TXB/AFE EP: %s\n", of_node_full_name(ep));
>> +
>> +		/* Initialise AFE */
>> +		ret = adv748x_afe_probe(state, ep);
>> +		if (ret) {
>> +			adv_err(state, "Failed to probe AFE");
>> +			return ret;
>> +		}
>> +	} else {
>> +		/* if has_afe(state) */
>> +		adv_err(state, "No AFE endpoint found at %s",
>> +				of_node_full_name(state->dev->of_node));
>> +	}
>> +
>> +
>> +	/* Initialise TXA */
>> +	ret = adv748x_csi2_probe(state, &state->txa);
>> +	if (ret) {
>> +		adv_err(state, "Failed to probe TXA");
>> +		return ret;
>> +	}
>> +
>> +	/* Initialise TXB  (Not 7480) */
>> +	ret = adv748x_csi2_probe(state, &state->txb);
> 
> Don't forget to clean up state->txa if this fails.
> 
>> +	if (ret) {
>> +		adv_err(state, "Failed to probe TXB");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_remove(struct i2c_client *client)
>> +{
>> +	struct adv748x_state *state = i2c_get_clientdata(client);
>> +
>> +	adv748x_afe_remove(state);
>> +	adv748x_hdmi_remove(state);
>> +
>> +	adv748x_csi2_remove(&state->txa);
>> +	adv748x_csi2_remove(&state->txb);
>> +
>> +	mutex_destroy(&state->mutex);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct i2c_device_id adv748x_id[] = {
>> +	{ "adv7481", 0 },
>> +	{ "adv7482", 0 },
>> +	{ },
>> +};
>> +
>> +static const struct of_device_id adv748x_of_table[] = {
>> +	{ .compatible = "adi,adv7481", },
>> +	{ .compatible = "adi,adv7482", },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, adv748x_of_table);
>> +
>> +static struct i2c_driver adv748x_driver = {
>> +	.driver = {
>> +		.name = "adv748x",
>> +		.of_match_table = of_match_ptr(adv748x_of_table),
>> +	},
>> +	.probe = adv748x_probe,
>> +	.remove = adv748x_remove,
>> +	.id_table = adv748x_id,
>> +};
>> +
>> +module_i2c_driver(adv748x_driver);
>> +
>> +MODULE_AUTHOR("Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>");
>> +MODULE_DESCRIPTION("ADV748X video decoder");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
>> new file mode 100644
>> index 000000000000..d0a0fcfeae2e
>> --- /dev/null
>> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
>> @@ -0,0 +1,373 @@
>> +/*
>> + * Driver for Analog Devices ADV748X CSI-2 Transmitter
>> + *
>> + * Copyright (C) 2017 Renesas Electronics Corp.
>> + *
>> + * This program is free software; you can redistribute  it and/or modify it
>> + * under  the terms of  the GNU General  Public License as published by the
>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>> + * option) any later version.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-ioctl.h>
>> +
>> +#include "adv748x.h"
>> +
>> +static bool is_txa(struct adv748x_csi2 *tx)
>> +{
>> +	return (tx == &tx->state->txa);
> 
> The parentheses are extra.
> 
>> +}
>> +
>> +static struct v4l2_subdev *adv748x_csi2_get_remote_sd(struct media_pad *local)
>> +{
>> +	struct media_pad *pad;
>> +
>> +	pad = media_entity_remote_pad(local);
>> +	if (!pad) {
>> +		return NULL;
> 
> if (...)
> 	return NULL;
> 
>> +	}
>> +
>> +	return media_entity_to_v4l2_subdev(pad->entity);
> 
> Or:
> 
> return pad ? media...(pad->entity) : NULL;
> 
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_internal_ops
>> + *
>> + * We use the internal registered operation to be able to ensure that our
>> + * incremental subdevices (not connected in the forward path) can be registered
>> + * against the resulting video path and media device.
>> + */
>> +
>> +static int adv748x_csi2_notify_complete(struct v4l2_async_notifier *notifier)
>> +{
>> +	struct adv748x_csi2 *tx = notifier_to_csi2(notifier);
>> +	struct adv748x_state *state = tx->state;
>> +	int ret;
>> +
>> +	ret = v4l2_device_register_subdev_nodes(tx->sd.v4l2_dev);
>> +	if (ret) {
>> +		adv_err(state, "Failed to register subdev nodes\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Return early until we register TXB */
>> +	if (is_txa(tx))
>> +		return ret;
>> +
>> +	ret = adv748x_setup_links(state);
> 
> Could you set up links first? It'd be really annoying to clean up the mess
> after the sub-device nodes have been registered. Actually --- it's not even
> doable at the moment.
> 
>> +	if (ret) {
>> +		adv_err(state, "Failed to setup entity links");
>> +		return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int adv748x_csi2_notify_bound(struct v4l2_async_notifier *notifier,
>> +				   struct v4l2_subdev *subdev,
>> +				   struct v4l2_async_subdev *asd)
>> +{
>> +	struct adv748x_csi2 *tx = notifier_to_csi2(notifier);
>> +	struct adv748x_state *state = tx->state;
>> +
>> +	v4l2_set_subdev_hostdata(subdev, tx);
>> +
>> +	adv_info(state, "Bind %s -> %s", is_txa(tx)? "TXA":"TXB", subdev->name);
>> +
>> +	return 0;
>> +}
>> +static void adv748x_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
>> +				     struct v4l2_subdev *subdev,
>> +				     struct v4l2_async_subdev *asd)
>> +{
>> +	struct adv748x_csi2 *tx = notifier_to_csi2(notifier);
>> +	struct adv748x_state *state = tx->state;
>> +
>> +	adv_info(state, "Unbind %s -> %s", is_txa(tx)? "TXA":"TXB",
>> +			subdev->name);
>> +}
>> +
>> +static int adv748x_csi2_registered(struct v4l2_subdev *sd)
>> +{
>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>> +	struct adv748x_state *state = tx->state;
>> +	int ret;
>> +
>> +	adv_info(state, "Registered %s (%s)", is_txa(tx)? "TXA":"TXB",
>> +			sd->name);
>> +
>> +	/*
>> +	 * Register HDMI on TXA, and AFE on TXB.
>> +	 */
>> +	if (is_txa(tx)) {
>> +		tx->subdevs[0].match_type = V4L2_ASYNC_MATCH_FWNODE;
>> +		tx->subdevs[0].match.fwnode.fwnode =
>> +			of_fwnode_handle(state->endpoints[ADV748X_PORT_HDMI]);
>> +	} else {
>> +		/* TODO: This isn't right - Hardwiring to AIN8 ... ???? */
>> +		tx->subdevs[0].match_type = V4L2_ASYNC_MATCH_FWNODE;
>> +		tx->subdevs[0].match.fwnode.fwnode =
>> +			of_fwnode_handle(state->endpoints[ADV748X_PORT_AIN8]);
>> +	}
>> +
>> +	tx->subdev_p[0] = &tx->subdevs[0];
>> +
>> +	tx->notifier.num_subdevs = 1;
>> +	tx->notifier.subdevs = tx->subdev_p;
>> +
>> +	tx->notifier.bound = adv748x_csi2_notify_bound;
>> +	tx->notifier.unbind = adv748x_csi2_notify_unbind;
>> +	tx->notifier.complete = adv748x_csi2_notify_complete;
>> +
>> +	ret = v4l2_async_subnotifier_register(&tx->sd, &tx->notifier);
> 
> How do things work out if there's a failure here? Any idea?
> 
>> +	if (ret < 0) {
>> +		adv_err(state, "Notifier registration failed\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void adv748x_csi2_unregistered(struct v4l2_subdev *sd)
>> +{
>> +	struct adv748x_csi2 *tx = container_of(sd, struct adv748x_csi2, sd);
>> +
>> +	v4l2_async_subnotifier_unregister(&tx->notifier);
>> +}
>> +
>> +static const struct v4l2_subdev_internal_ops adv748x_csi2_internal_ops = {
>> +	.registered = adv748x_csi2_registered,
>> +	.unregistered = adv748x_csi2_unregistered,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_pad_ops
>> + */
>> +
>> +static int adv748x_csi2_s_stream(struct v4l2_subdev *sd, int enable)
>> +{
>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>> +	struct v4l2_subdev *src;
>> +	int ret;
>> +
>> +	src = adv748x_csi2_get_remote_sd(&tx->pads[ADV748X_CSI2_SINK]);
>> +	if (!src)
>> +		return -ENODEV;
>> +
>> +	ret = v4l2_subdev_call(src, video, s_stream, enable);
> 
> return v4l2_subdev_call();
> 
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct v4l2_subdev_video_ops adv748x_csi2_video_ops = {
>> +	.s_stream = adv748x_csi2_s_stream,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_pad_ops
>> + *
>> + * The CSI2 bus pads, are ignorant to the data sizes or formats.
>> + * But we must support setting the pad formats for format propagation.
>> + */
>> +
>> +static struct v4l2_mbus_framefmt *
>> +adv748x_csi2_get_pad_format(struct v4l2_subdev *sd,
>> +			    struct v4l2_subdev_pad_config *cfg,
>> +			    unsigned int pad, u32 which)
>> +{
>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>> +
>> +	switch (which) {
>> +	case V4L2_SUBDEV_FORMAT_TRY:
>> +		return v4l2_subdev_get_try_format(sd, cfg, pad);
>> +	case V4L2_SUBDEV_FORMAT_ACTIVE:
>> +		return &tx->format;
>> +	default:
> 
> This won't happen. I think a lot of drivers assume that.
> 
>> +		return NULL;
>> +	}
>> +}
>> +
>> +static int adv748x_csi2_get_format(struct v4l2_subdev *sd,
>> +				   struct v4l2_subdev_pad_config *cfg,
>> +				   struct v4l2_subdev_format *sdformat)
>> +{
>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>> +	struct adv748x_state *state = tx->state;
>> +	struct v4l2_mbus_framefmt *mbusformat;
>> +
>> +	mbusformat = adv748x_csi2_get_pad_format(sd, cfg, sdformat->pad,
>> +						 sdformat->which);
>> +	if (!mbusformat)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&state->mutex);
>> +
>> +	sdformat->format = *mbusformat;
>> +
>> +	mutex_unlock(&state->mutex);
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_csi2_set_format(struct v4l2_subdev *sd,
>> +				   struct v4l2_subdev_pad_config *cfg,
>> +				   struct v4l2_subdev_format *sdformat)
>> +{
>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>> +	struct adv748x_state *state = tx->state;
>> +	struct media_pad *pad = &tx->pads[sdformat->pad];
>> +	struct v4l2_mbus_framefmt *mbusformat;
>> +
>> +	mbusformat = adv748x_csi2_get_pad_format(sd, cfg, sdformat->pad,
>> +						 sdformat->which);
>> +	if (!mbusformat)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&state->mutex);
>> +
>> +	if (pad->flags & MEDIA_PAD_FL_SOURCE)
>> +		sdformat->format = tx->format;
>> +
>> +	*mbusformat = sdformat->format;
>> +
>> +	mutex_unlock(&state->mutex);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_pad_ops adv748x_csi2_pad_ops = {
>> +	.get_fmt = adv748x_csi2_get_format,
>> +	.set_fmt = adv748x_csi2_set_format,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_ops
>> + */
>> +
>> +static const struct v4l2_subdev_ops adv748x_csi2_ops = {
>> +	.video = &adv748x_csi2_video_ops,
>> +	.pad = &adv748x_csi2_pad_ops,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * Subdev module and controls
>> + */
>> +
>> +static int adv748x_csi2_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> +	struct adv748x_csi2 *tx = container_of(ctrl->handler,
>> +					struct adv748x_csi2, ctrl_hdl);
>> +	struct adv748x_state *state = tx->state;
>> +
>> +	switch (ctrl->id) {
>> +	case V4L2_CID_PIXEL_RATE:
>> +	{
>> +		/* Direct control pass through */
>> +		struct v4l2_ext_controls ctrls;
>> +		struct v4l2_ext_control ectrl;
>> +		struct v4l2_subdev *src;
>> +		int ret;
>> +
>> +		src = adv748x_csi2_get_remote_sd(&tx->pads[ADV748X_CSI2_SINK]);
>> +		if (!src)
>> +			return -ENODEV;
>> +
>> +		memset(&ctrls, 0, sizeof(ctrls));
>> +		memset(&ectrl, 0, sizeof(ectrl));
>> +
>> +		ectrl.id = V4L2_CID_PIXEL_RATE;
>> +
>> +		ctrls.count = 1;
>> +		ctrls.controls = &ectrl;
> 
> Can you use v4l2_ctrl_g_ctrl_int64()?
> 
>> +
>> +		ret = v4l2_g_ext_ctrls(src->ctrl_handler, &ctrls);
>> +		if (ret < 0) {
>> +			adv_err(state, "%s: subdev link freq control failed\n",
>> +				tx->sd.name);
>> +			return ret;
>> +		}
>> +
>> +		*ctrl->p_new.p_s64 = ectrl.value64;
>> +
>> +		break;
>> +	}
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_ctrl_ops adv748x_csi2_ctrl_ops = {
>> +	.g_volatile_ctrl = adv748x_csi2_g_volatile_ctrl,
>> +};
>> +
>> +static int adv748x_csi2_init_controls(struct adv748x_csi2 *tx)
>> +{
>> +	struct v4l2_ctrl *ctrl;
>> +
>> +	v4l2_ctrl_handler_init(&tx->ctrl_hdl, 1);
>> +
>> +	ctrl = v4l2_ctrl_new_std(&tx->ctrl_hdl, &adv748x_csi2_ctrl_ops,
>> +				 V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, 1);
>> +	if (ctrl)
>> +		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> +
>> +	tx->sd.ctrl_handler = &tx->ctrl_hdl;
>> +	if (tx->ctrl_hdl.error) {
>> +		v4l2_ctrl_handler_free(&tx->ctrl_hdl);
>> +		return tx->ctrl_hdl.error;
>> +	}
>> +
>> +	return v4l2_ctrl_handler_setup(&tx->ctrl_hdl);
>> +}
>> +
>> +int adv748x_csi2_probe(struct adv748x_state *state, struct adv748x_csi2 *tx)
>> +{
>> +	struct device_node *ep;
>> +	int ret;
>> +
>> +	/* We can not use container_of to get back to the state with two TXs */
>> +	tx->state = state;
>> +
>> +	adv748x_subdev_init(&tx->sd, state, &adv748x_csi2_ops,
>> +			is_txa(tx) ? "txa" : "txb");
>> +
>> +	ep = state->endpoints[is_txa(tx) ? ADV748X_PORT_TXA : ADV748X_PORT_TXB];
>> +
>> +	/* Ensure that matching is based upon the endpoint fwnodes */
>> +	tx->sd.fwnode = of_fwnode_handle(ep);
>> +
>> +	/* Register internal ops for Incremental subdev discovery */
>> +	tx->sd.internal_ops = &adv748x_csi2_internal_ops;
>> +
>> +	tx->pads[ADV748X_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
>> +	tx->pads[ADV748X_CSI2_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
>> +
>> +	ret = media_entity_pads_init(&tx->sd.entity, ADV748X_CSI2_NR_PADS,
>> +				     tx->pads);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = adv748x_csi2_init_controls(tx);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = v4l2_async_register_subdev(&tx->sd);
> 
> Clean up the control handler on error?
> 
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +
>> +void adv748x_csi2_remove(struct adv748x_csi2 *tx)
>> +{
>> +	v4l2_async_unregister_subdev(&tx->sd);
>> +	media_entity_cleanup(&tx->sd.entity);
>> +}
>> diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c
>> new file mode 100644
>> index 000000000000..19c1bd41cc6c
>> --- /dev/null
>> +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
>> @@ -0,0 +1,671 @@
>> +/*
>> + * Driver for Analog Devices ADV748X HDMI receiver and Component Processor (CP)
>> + *
>> + * Copyright (C) 2017 Renesas Electronics Corp.
>> + *
>> + * This program is free software; you can redistribute  it and/or modify it
>> + * under  the terms of  the GNU General  Public License as published by the
>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>> + * option) any later version.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/v4l2-dv-timings.h>
> 
> Why is that header here...? Is it a uAPI header? Then add uapi/.
> 
>> +
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-dv-timings.h>
>> +#include <media/v4l2-ioctl.h>
>> +
>> +#include "adv748x.h"
>> +
>> +/* -----------------------------------------------------------------------------
>> + * HDMI and CP
>> + */
>> +
>> +#define ADV748X_HDMI_MIN_WIDTH		640
>> +#define ADV748X_HDMI_MAX_WIDTH		1920
>> +#define ADV748X_HDMI_MIN_HEIGHT		480
>> +#define ADV748X_HDMI_MAX_HEIGHT		1200
>> +#define ADV748X_HDMI_MIN_PIXELCLOCK	0		/* unknown */
>> +#define ADV748X_HDMI_MAX_PIXELCLOCK	162000000
>> +
>> +static const struct v4l2_dv_timings_cap adv748x_hdmi_timings_cap = {
>> +	.type = V4L2_DV_BT_656_1120,
>> +	/* keep this initialization for compatibility with GCC < 4.4.6 */
>> +	.reserved = { 0 },
> 
> Is this necessary? Aren't the uninitialised fields zeroed if any field in a
> struct is initialised, also on those versions of GCC? (Of course there have
> been bugs, but nothing rings a bell.)
> 
>> +	/* Min pixelclock value is unknown */
>> +	V4L2_INIT_BT_TIMINGS(ADV748X_HDMI_MIN_WIDTH, ADV748X_HDMI_MAX_WIDTH,
>> +			     ADV748X_HDMI_MIN_HEIGHT, ADV748X_HDMI_MAX_HEIGHT,
>> +			     ADV748X_HDMI_MIN_PIXELCLOCK,
>> +			     ADV748X_HDMI_MAX_PIXELCLOCK,
>> +			     V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT,
>> +			     V4L2_DV_BT_CAP_INTERLACED |
>> +			     V4L2_DV_BT_CAP_PROGRESSIVE)
>> +};
>> +
>> +struct adv748x_hdmi_video_standards {
>> +	struct v4l2_dv_timings timings;
>> +	u8 vid_std;
>> +	u8 v_freq;
>> +};
>> +
>> +static const struct adv748x_hdmi_video_standards
>> +adv748x_hdmi_video_standards[] = {
>> +	{ V4L2_DV_BT_CEA_720X480I59_94, 0x40, 0x00 },
>> +	{ V4L2_DV_BT_CEA_720X576I50, 0x41, 0x01 },
>> +	{ V4L2_DV_BT_CEA_720X480P59_94, 0x4a, 0x00 },
>> +	{ V4L2_DV_BT_CEA_720X576P50, 0x4b, 0x00 },
>> +	{ V4L2_DV_BT_CEA_1280X720P60, 0x53, 0x00 },
>> +	{ V4L2_DV_BT_CEA_1280X720P50, 0x53, 0x01 },
>> +	{ V4L2_DV_BT_CEA_1280X720P30, 0x53, 0x02 },
>> +	{ V4L2_DV_BT_CEA_1280X720P25, 0x53, 0x03 },
>> +	{ V4L2_DV_BT_CEA_1280X720P24, 0x53, 0x04 },
>> +	{ V4L2_DV_BT_CEA_1920X1080I60, 0x54, 0x00 },
>> +	{ V4L2_DV_BT_CEA_1920X1080I50, 0x54, 0x01 },
>> +	{ V4L2_DV_BT_CEA_1920X1080P60, 0x5e, 0x00 },
>> +	{ V4L2_DV_BT_CEA_1920X1080P50, 0x5e, 0x01 },
>> +	{ V4L2_DV_BT_CEA_1920X1080P30, 0x5e, 0x02 },
>> +	{ V4L2_DV_BT_CEA_1920X1080P25, 0x5e, 0x03 },
>> +	{ V4L2_DV_BT_CEA_1920X1080P24, 0x5e, 0x04 },
>> +	/* SVGA */
>> +	{ V4L2_DV_BT_DMT_800X600P56, 0x80, 0x00 },
>> +	{ V4L2_DV_BT_DMT_800X600P60, 0x81, 0x00 },
>> +	{ V4L2_DV_BT_DMT_800X600P72, 0x82, 0x00 },
>> +	{ V4L2_DV_BT_DMT_800X600P75, 0x83, 0x00 },
>> +	{ V4L2_DV_BT_DMT_800X600P85, 0x84, 0x00 },
>> +	/* SXGA */
>> +	{ V4L2_DV_BT_DMT_1280X1024P60, 0x85, 0x00 },
>> +	{ V4L2_DV_BT_DMT_1280X1024P75, 0x86, 0x00 },
>> +	/* VGA */
>> +	{ V4L2_DV_BT_DMT_640X480P60, 0x88, 0x00 },
>> +	{ V4L2_DV_BT_DMT_640X480P72, 0x89, 0x00 },
>> +	{ V4L2_DV_BT_DMT_640X480P75, 0x8a, 0x00 },
>> +	{ V4L2_DV_BT_DMT_640X480P85, 0x8b, 0x00 },
>> +	/* XGA */
>> +	{ V4L2_DV_BT_DMT_1024X768P60, 0x8c, 0x00 },
>> +	{ V4L2_DV_BT_DMT_1024X768P70, 0x8d, 0x00 },
>> +	{ V4L2_DV_BT_DMT_1024X768P75, 0x8e, 0x00 },
>> +	{ V4L2_DV_BT_DMT_1024X768P85, 0x8f, 0x00 },
>> +	/* UXGA */
>> +	{ V4L2_DV_BT_DMT_1600X1200P60, 0x96, 0x00 },
>> +	/* End of standards */
>> +	{ },
>> +};
>> +
>> +static void adv748x_hdmi_fill_format(struct adv748x_state *state,
>> +				     struct v4l2_mbus_framefmt *fmt)
>> +{
>> +	memset(fmt, 0, sizeof(*fmt));
>> +
>> +	fmt->code = MEDIA_BUS_FMT_RGB888_1X24;
>> +	fmt->colorspace = V4L2_COLORSPACE_SRGB;
>> +	fmt->field = state->hdmi.timings.bt.interlaced ?
>> +		V4L2_FIELD_INTERLACED : V4L2_FIELD_NONE;
>> +
>> +	fmt->width = state->hdmi.timings.bt.width;
>> +	fmt->height = state->hdmi.timings.bt.height;
>> +}
>> +
>> +static void adv748x_fill_optional_dv_timings(struct adv748x_state *state,
>> +					     struct v4l2_dv_timings *timings)
>> +{
>> +	v4l2_find_dv_timings_cap(timings, &adv748x_hdmi_timings_cap,
>> +				 250000, NULL, NULL);
>> +}
>> +
>> +static bool adv748x_hdmi_have_signal(struct adv748x_state *state)
>> +{
>> +	int val;
>> +
>> +	/* Check that VERT_FILTER and DG_REGEN is locked */
>> +	val = hdmi_read(state, 0x07);
>> +	return !!((val & BIT(7)) && (val & BIT(5)));
> 
> No need for !!; casting non-zero value to bool will be true.
> 
>> +}
>> +
>> +static unsigned int adv748x_hdmi_read_pixelclock(struct adv748x_state *state)
>> +{
>> +	int a, b;
>> +
>> +	a = hdmi_read(state, 0x51);
>> +	b = hdmi_read(state, 0x52);
>> +	if (a < 0 || b < 0)
>> +		return 0;
> 
> I think I'd have explicit error handling. A lot of users of the control
> value would be upset if they'd get 0.
> 
>> +	return ((a << 1) | (b >> 7)) * 1000000 + (b & 0x7f) * 1000000 / 128;
>> +}
>> +
>> +static int adv748x_hdmi_set_video_timings(struct adv748x_state *state,
>> +					  const struct v4l2_dv_timings *timings)
>> +{
>> +	const struct adv748x_hdmi_video_standards *stds =
>> +		adv748x_hdmi_video_standards;
>> +	int i;
>> +
>> +	for (i = 0; stds[i].timings.bt.width; i++) {
>> +		if (!v4l2_match_dv_timings(timings, &stds[i].timings, 250000,
>> +					   false))
>> +			continue;
>> +		/*
>> +		 * The resolution of 720p, 1080i and 1080p is Hsync width of
>> +		 * 40 pixelclock cycles. These resolutions must be shifted
>> +		 * horizontally to the left in active video mode.
>> +		 */
>> +		switch (stds[i].vid_std) {
>> +		case 0x53: /* 720p */
>> +			cp_write(state, 0x8B, 0x43);
>> +			cp_write(state, 0x8C, 0xD8);
>> +			cp_write(state, 0x8B, 0x4F);
>> +			cp_write(state, 0x8D, 0xD8);
>> +			break;
>> +		case 0x54: /* 1080i */
>> +		case 0x5e: /* 1080p */
>> +			cp_write(state, 0x8B, 0x43);
>> +			cp_write(state, 0x8C, 0xD4);
>> +			cp_write(state, 0x8B, 0x4F);
>> +			cp_write(state, 0x8D, 0xD4);
>> +			break;
>> +		default:
>> +			cp_write(state, 0x8B, 0x40);
>> +			cp_write(state, 0x8C, 0x00);
>> +			cp_write(state, 0x8B, 0x40);
>> +			cp_write(state, 0x8D, 0x00);
>> +			break;
>> +		}
>> +
>> +		io_write(state, 0x05, stds[i].vid_std);
>> +		io_clrset(state, 0x03, 0x70, stds[i].v_freq << 4);
>> +
>> +		return 0;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_video_ops
>> + */
>> +
>> +static int adv748x_hdmi_s_dv_timings(struct v4l2_subdev *sd,
>> +				     struct v4l2_dv_timings *timings)
>> +{
>> +	struct adv748x_state *state = adv748x_hdmi_to_state(sd);
>> +	struct v4l2_bt_timings *bt;
>> +	int ret;
>> +
>> +	if (!timings)
>> +		return -EINVAL;
>> +
>> +	if (v4l2_match_dv_timings(&state->hdmi.timings, timings, 0, false))
>> +		return 0;
>> +
>> +	bt = &timings->bt;
> 
> You're using this once...
> 
>> +
>> +	if (!v4l2_valid_dv_timings(timings, &adv748x_hdmi_timings_cap,
>> +				   NULL, NULL))
>> +		return -ERANGE;
>> +
>> +	adv748x_fill_optional_dv_timings(state, timings);
>> +
>> +	ret = adv748x_hdmi_set_video_timings(state, timings);
>> +	if (ret)
>> +		return ret;
>> +
>> +	state->hdmi.timings = *timings;
>> +
>> +	cp_clrset(state, 0x91, 0x40, bt->interlaced ? 0x40 : 0x00);
> 
> timings->bt.interlaced ?
> 
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_hdmi_g_dv_timings(struct v4l2_subdev *sd,
>> +				     struct v4l2_dv_timings *timings)
>> +{
>> +	struct adv748x_state *state = adv748x_hdmi_to_state(sd);
>> +
>> +	*timings = state->hdmi.timings;
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_hdmi_query_dv_timings(struct v4l2_subdev *sd,
>> +					 struct v4l2_dv_timings *timings)
>> +{
>> +	struct adv748x_state *state = adv748x_hdmi_to_state(sd);
>> +	struct v4l2_bt_timings *bt = &timings->bt;
>> +	int tmp;
>> +
>> +	if (!timings)
>> +		return -EINVAL;
>> +
>> +	memset(timings, 0, sizeof(struct v4l2_dv_timings));
>> +
>> +	if (!adv748x_hdmi_have_signal(state))
>> +		return -ENOLINK;
>> +
>> +	timings->type = V4L2_DV_BT_656_1120;
>> +
>> +	bt->interlaced = hdmi_read(state, 0x0b) & BIT(5) ?
>> +		V4L2_DV_INTERLACED : V4L2_DV_PROGRESSIVE;
>> +
>> +	bt->width = hdmi_read16(state, 0x07, 0x1fff);
>> +	bt->height = hdmi_read16(state, 0x09, 0x1fff);
>> +	bt->hfrontporch = hdmi_read16(state, 0x20, 0x1fff);
>> +	bt->hsync = hdmi_read16(state, 0x22, 0x1fff);
>> +	bt->hbackporch = hdmi_read16(state, 0x24, 0x1fff);
>> +	bt->vfrontporch = hdmi_read16(state, 0x2a, 0x3fff) / 2;
>> +	bt->vsync = hdmi_read16(state, 0x2e, 0x3fff) / 2;
>> +	bt->vbackporch = hdmi_read16(state, 0x32, 0x3fff) / 2;
>> +
>> +	bt->pixelclock = adv748x_hdmi_read_pixelclock(state);
>> +
>> +	tmp = hdmi_read(state, 0x05);
>> +	bt->polarities = (tmp & BIT(4) ? V4L2_DV_VSYNC_POS_POL : 0) |
>> +		(tmp & BIT(5) ? V4L2_DV_HSYNC_POS_POL : 0);
>> +
>> +	if (bt->interlaced == V4L2_DV_INTERLACED) {
>> +		bt->height += hdmi_read16(state, 0x0b, 0x1fff);
>> +		bt->il_vfrontporch = hdmi_read16(state, 0x2c, 0x3fff) / 2;
>> +		bt->il_vsync = hdmi_read16(state, 0x30, 0x3fff) / 2;
>> +		bt->il_vbackporch = hdmi_read16(state, 0x34, 0x3fff) / 2;
>> +	}
>> +
>> +	adv748x_fill_optional_dv_timings(state, timings);
>> +
>> +	if (!adv748x_hdmi_have_signal(state)) {
>> +		adv_info(state, "HDMI signal lost during readout\n");
>> +		return -ENOLINK;
>> +	}
>> +
>> +	/*
>> +	 * TODO: No interrupt handling is implemented yet.
>> +	 * There should be an IRQ when a cable is plugged and a the new
>> +	 * timings figured out and stored to state. This the next best thing
>> +	 */
>> +	state->hdmi.timings = *timings;
>> +
>> +	adv_dbg(state, "HDMI %dx%d%c clock: %llu Hz pol: %x "
>> +		"hfront: %d hsync: %d hback: %d "
>> +		"vfront: %d vsync: %d vback: %d "
>> +		"il_vfron: %d il_vsync: %d il_vback: %d\n",
>> +		bt->width, bt->height,
>> +		bt->interlaced == V4L2_DV_INTERLACED ? 'i' : 'p',
>> +		bt->pixelclock, bt->polarities,
>> +		bt->hfrontporch, bt->hsync, bt->hbackporch,
>> +		bt->vfrontporch, bt->vsync, bt->vbackporch,
>> +		bt->il_vfrontporch, bt->il_vsync, bt->il_vbackporch);
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_hdmi_g_input_status(struct v4l2_subdev *sd, u32 *status)
>> +{
>> +	struct adv748x_state *state = adv748x_hdmi_to_state(sd);
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&state->mutex);
>> +	if (ret)
>> +		return ret;
>> +
>> +	*status = adv748x_hdmi_have_signal(state) ? 0 : V4L2_IN_ST_NO_SIGNAL;
>> +
>> +	mutex_unlock(&state->mutex);
>> +
>> +	return ret;
>> +}
>> +
>> +static int adv748x_hdmi_s_stream(struct v4l2_subdev *sd, int enable)
>> +{
>> +	struct adv748x_state *state = adv748x_hdmi_to_state(sd);
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&state->mutex);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = adv748x_txa_power(state, enable);
>> +	if (ret)
>> +		goto error;
>> +
>> +	if (adv748x_hdmi_have_signal(state))
>> +		adv_dbg(state, "Detected HDMI signal\n");
>> +	else
>> +		adv_info(state, "Couldn't detect HDMI video signal\n");
>> +
>> +error:
>> +	mutex_unlock(&state->mutex);
>> +	return ret;
>> +}
>> +
>> +static int adv748x_hdmi_g_pixelaspect(struct v4l2_subdev *sd,
>> +				      struct v4l2_fract *aspect)
>> +{
>> +	aspect->numerator = 1;
>> +	aspect->denominator = 1;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_video_ops adv748x_video_ops_hdmi = {
>> +	.s_dv_timings = adv748x_hdmi_s_dv_timings,
>> +	.g_dv_timings = adv748x_hdmi_g_dv_timings,
>> +	.query_dv_timings = adv748x_hdmi_query_dv_timings,
>> +	.g_input_status = adv748x_hdmi_g_input_status,
>> +	.s_stream = adv748x_hdmi_s_stream,
>> +	.g_pixelaspect = adv748x_hdmi_g_pixelaspect,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_pad_ops
>> + */
>> +
>> +static int adv748x_hdmi_enum_mbus_code(struct v4l2_subdev *sd,
>> +				  struct v4l2_subdev_pad_config *cfg,
>> +				  struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> +	if (code->index != 0)
>> +		return -EINVAL;
>> +
>> +	if (code->pad >= ADV748X_HDMI_NR_PADS)
>> +		return -EINVAL;
> 
> Checked in v4l2-subdev.c.
> 
>> +
>> +	code->code = MEDIA_BUS_FMT_RGB888_1X24;
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_hdmi_get_pad_format(struct v4l2_subdev *sd,
>> +				  struct v4l2_subdev_pad_config *cfg,
>> +				  struct v4l2_subdev_format *format)
>> +{
>> +	struct adv748x_state *state = adv748x_hdmi_to_state(sd);
>> +
>> +	if (format->pad >= ADV748X_HDMI_NR_PADS)
>> +		return -EINVAL;
> 
> Ditto.
> 
>> +
>> +	adv748x_hdmi_fill_format(state, &format->format);
>> +
>> +	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>> +		struct v4l2_mbus_framefmt *fmt;
>> +
>> +		fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
>> +		format->format.code = fmt->code;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int adv748x_hdmi_set_pad_format(struct v4l2_subdev *sd,
>> +				       struct v4l2_subdev_pad_config *cfg,
>> +				       struct v4l2_subdev_format *format)
>> +{
>> +	struct adv748x_state *state = adv748x_hdmi_to_state(sd);
>> +
>> +	if (format->pad >= ADV748X_HDMI_NR_PADS)
>> +		return -EINVAL;
> 
> Ditto.
> 
>> +
>> +	adv748x_hdmi_fill_format(state, &format->format);
>> +
>> +	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>> +		struct v4l2_mbus_framefmt *fmt;
>> +
>> +		fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
>> +		fmt->code = format->format.code;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static bool adv748x_hdmi_check_dv_timings(const struct v4l2_dv_timings *timings,
>> +					  void *hdl)
>> +{
>> +	const struct adv748x_hdmi_video_standards *stds =
>> +		adv748x_hdmi_video_standards;
>> +	unsigned int i;
>> +
>> +	for (i = 0; stds[i].timings.bt.width; i++)
>> +		if (v4l2_match_dv_timings(timings, &stds[i].timings, 0, false))
>> +			return true;
>> +
>> +	return false;
>> +}
>> +
>> +static int adv748x_hdmi_enum_dv_timings(struct v4l2_subdev *sd,
>> +					struct v4l2_enum_dv_timings *timings)
>> +{
>> +	return v4l2_enum_dv_timings_cap(timings, &adv748x_hdmi_timings_cap,
>> +					adv748x_hdmi_check_dv_timings, NULL);
>> +}
>> +
>> +static int adv748x_hdmi_dv_timings_cap(struct v4l2_subdev *sd,
>> +				       struct v4l2_dv_timings_cap *cap)
>> +{
>> +	*cap = adv748x_hdmi_timings_cap;
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_pad_ops adv748x_pad_ops_hdmi = {
>> +	.enum_mbus_code = adv748x_hdmi_enum_mbus_code,
>> +	.set_fmt = adv748x_hdmi_set_pad_format,
>> +	.get_fmt = adv748x_hdmi_get_pad_format,
>> +	.dv_timings_cap = adv748x_hdmi_dv_timings_cap,
>> +	.enum_dv_timings = adv748x_hdmi_enum_dv_timings,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * v4l2_subdev_ops
>> + */
>> +
>> +static const struct v4l2_subdev_ops adv748x_ops_hdmi = {
>> +	.video = &adv748x_video_ops_hdmi,
>> +	.pad = &adv748x_pad_ops_hdmi,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * Controls
>> + */
>> +
>> +/* Contrast Control */
>> +#define ADV748X_HDMI_CON_REG	0x3a	/* Contrast (unsigned) */
>> +#define ADV748X_HDMI_CON_MIN	0	/* Minimum contrast */
>> +#define ADV748X_HDMI_CON_DEF	128	/* Default */
>> +#define ADV748X_HDMI_CON_MAX	255	/* Maximum contrast */
>> +
>> +/* Saturation Control */
>> +#define ADV748X_HDMI_SAT_REG	0x3b	/* Saturation (unsigned) */
>> +#define ADV748X_HDMI_SAT_MIN	0	/* Minimum saturation */
>> +#define ADV748X_HDMI_SAT_DEF	128	/* Default */
>> +#define ADV748X_HDMI_SAT_MAX	255	/* Maximum saturation */
>> +
>> +/* Brightness Control */
>> +#define ADV748X_HDMI_BRI_REG	0x3c	/* Brightness (signed) */
>> +#define ADV748X_HDMI_BRI_MIN	-128	/* Luma is -512d */
>> +#define ADV748X_HDMI_BRI_DEF	0	/* Luma is 0 */
>> +#define ADV748X_HDMI_BRI_MAX	127	/* Luma is 508d */
>> +
>> +/* Hue Control */
>> +#define ADV748X_HDMI_HUE_REG	0x3d	/* Hue (unsigned) */
>> +#define ADV748X_HDMI_HUE_MIN	0	/* -90 degree */
>> +#define ADV748X_HDMI_HUE_DEF	0	/* -90 degree */
>> +#define ADV748X_HDMI_HUE_MAX	255	/* +90 degree */
>> +
>> +/* Video adjustment register */
>> +#define ADV748X_HDMI_VID_ADJ_REG		0x3e
>> +/* Video adjustment mask */
>> +#define ADV748X_HDMI_VID_ADJ_MASK		0x7F
>> +/* Enable color controls */
>> +#define ADV748X_HDMI_VID_ADJ_ENABLE	0x80
>> +
>> +static int __adv748x_hdmi_s_ctrl(struct v4l2_ctrl *ctrl,
>> +				 struct adv748x_state *state)
>> +{
>> +	int ret;
>> +
>> +	/* Enable video adjustment first */
>> +	ret = cp_read(state, ADV748X_HDMI_VID_ADJ_REG);
>> +	if (ret < 0)
>> +		return ret;
>> +	ret |= ADV748X_HDMI_VID_ADJ_ENABLE;
>> +
>> +	ret = cp_write(state, ADV748X_HDMI_VID_ADJ_REG, ret);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	switch (ctrl->id) {
>> +	case V4L2_CID_BRIGHTNESS:
>> +		if (ctrl->val < ADV748X_HDMI_BRI_MIN ||
>> +		    ctrl->val > ADV748X_HDMI_BRI_MAX)
>> +			return -ERANGE;
>> +
>> +		ret = cp_write(state, ADV748X_HDMI_BRI_REG, ctrl->val);
>> +		break;
>> +	case V4L2_CID_HUE:
>> +		if (ctrl->val < ADV748X_HDMI_HUE_MIN ||
>> +		    ctrl->val > ADV748X_HDMI_HUE_MAX)
>> +			return -ERANGE;
>> +
>> +		ret = cp_write(state, ADV748X_HDMI_HUE_REG, ctrl->val);
>> +		break;
>> +	case V4L2_CID_CONTRAST:
>> +		if (ctrl->val < ADV748X_HDMI_CON_MIN ||
>> +		    ctrl->val > ADV748X_HDMI_CON_MAX)
>> +			return -ERANGE;
>> +
>> +		ret = cp_write(state, ADV748X_HDMI_CON_REG, ctrl->val);
>> +		break;
>> +	case V4L2_CID_SATURATION:
>> +		if (ctrl->val < ADV748X_HDMI_SAT_MIN ||
>> +		    ctrl->val > ADV748X_HDMI_SAT_MAX)
>> +			return -ERANGE;
>> +
>> +		ret = cp_write(state, ADV748X_HDMI_SAT_REG, ctrl->val);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int adv748x_hdmi_s_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> +	struct adv748x_state *state = container_of(ctrl->handler,
>> +					struct adv748x_state, hdmi.ctrl_hdl);
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&state->mutex);
> 
> Could you use the same mutex for the control handler? It'd make things
> simpler...
> 
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = __adv748x_hdmi_s_ctrl(ctrl, state);
>> +
>> +	mutex_unlock(&state->mutex);
>> +
>> +	return ret;
>> +}
>> +
>> +static int adv748x_hdmi_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> +	struct adv748x_state *state = container_of(ctrl->handler,
>> +					struct adv748x_state, hdmi.ctrl_hdl);
>> +	unsigned int width, height, fps;
>> +
>> +	switch (ctrl->id) {
>> +	case V4L2_CID_PIXEL_RATE:
>> +	{
>> +		struct v4l2_dv_timings timings;
>> +		struct v4l2_bt_timings *bt = &timings.bt;
>> +
>> +		adv748x_hdmi_query_dv_timings(&state->hdmi.sd, &timings);
>> +
>> +		width = bt->width;
>> +		height = bt->height;
>> +		fps = DIV_ROUND_CLOSEST(bt->pixelclock,
>> +					V4L2_DV_BT_FRAME_WIDTH(bt) *
>> +					V4L2_DV_BT_FRAME_HEIGHT(bt));
>> +
>> +		*ctrl->p_new.p_s64 = width * height * fps;
>> +		break;
>> +	}
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_ctrl_ops adv748x_hdmi_ctrl_ops = {
>> +	.s_ctrl = adv748x_hdmi_s_ctrl,
>> +	.g_volatile_ctrl = adv748x_hdmi_g_volatile_ctrl,
>> +};
>> +
>> +static int adv748x_hdmi_init_controls(struct adv748x_state *state)
>> +{
>> +	struct v4l2_ctrl *ctrl;
>> +
>> +	v4l2_ctrl_handler_init(&state->hdmi.ctrl_hdl, 5);
>> +
>> +	v4l2_ctrl_new_std(&state->hdmi.ctrl_hdl, &adv748x_hdmi_ctrl_ops,
>> +			  V4L2_CID_BRIGHTNESS, ADV748X_HDMI_BRI_MIN,
>> +			  ADV748X_HDMI_BRI_MAX, 1, ADV748X_HDMI_BRI_DEF);
>> +	v4l2_ctrl_new_std(&state->hdmi.ctrl_hdl, &adv748x_hdmi_ctrl_ops,
>> +			  V4L2_CID_CONTRAST, ADV748X_HDMI_CON_MIN,
>> +			  ADV748X_HDMI_CON_MAX, 1, ADV748X_HDMI_CON_DEF);
>> +	v4l2_ctrl_new_std(&state->hdmi.ctrl_hdl, &adv748x_hdmi_ctrl_ops,
>> +			  V4L2_CID_SATURATION, ADV748X_HDMI_SAT_MIN,
>> +			  ADV748X_HDMI_SAT_MAX, 1, ADV748X_HDMI_SAT_DEF);
>> +	v4l2_ctrl_new_std(&state->hdmi.ctrl_hdl, &adv748x_hdmi_ctrl_ops,
>> +			  V4L2_CID_HUE, ADV748X_HDMI_HUE_MIN,
>> +			  ADV748X_HDMI_HUE_MAX, 1, ADV748X_HDMI_HUE_DEF);
>> +	ctrl = v4l2_ctrl_new_std(&state->hdmi.ctrl_hdl, &adv748x_hdmi_ctrl_ops,
>> +				 V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, 1);
>> +	if (ctrl)
>> +		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> +
>> +	state->hdmi.sd.ctrl_handler = &state->hdmi.ctrl_hdl;
>> +	if (state->hdmi.ctrl_hdl.error) {
>> +		v4l2_ctrl_handler_free(&state->hdmi.ctrl_hdl);
>> +		return state->hdmi.ctrl_hdl.error;
>> +	}
>> +
>> +	return v4l2_ctrl_handler_setup(&state->hdmi.ctrl_hdl);
>> +}
>> +
>> +int adv748x_hdmi_probe(struct adv748x_state *state, struct device_node *ep)
>> +{
>> +	static const struct v4l2_dv_timings cea720x480 =
>> +		V4L2_DV_BT_CEA_720X480I59_94;
>> +	int ret;
>> +
>> +	state->hdmi.timings = cea720x480;
>> +
>> +	adv748x_subdev_init(&state->hdmi.sd, state, &adv748x_ops_hdmi, "hdmi");
>> +
>> +	/* HDMI is currently statically routed to TXA */
>> +	state->hdmi.sd.fwnode = &ep->fwnode;
>> +
>> +	state->hdmi.pads[ADV748X_HDMI_SINK].flags = MEDIA_PAD_FL_SINK;
>> +	state->hdmi.pads[ADV748X_HDMI_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
>> +
>> +	ret = media_entity_pads_init(&state->hdmi.sd.entity,
>> +				     ADV748X_HDMI_NR_PADS, state->hdmi.pads);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = adv748x_hdmi_init_controls(state);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = v4l2_async_register_subdev(&state->hdmi.sd);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +void adv748x_hdmi_remove(struct adv748x_state *state)
>> +{
>> +	v4l2_async_unregister_subdev(&state->hdmi.sd);
>> +	media_entity_cleanup(&state->hdmi.sd.entity);
>> +	v4l2_ctrl_handler_free(&state->hdmi.ctrl_hdl);
>> +}
>> diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
>> new file mode 100644
>> index 000000000000..1943995a2bad
>> --- /dev/null
>> +++ b/drivers/media/i2c/adv748x/adv748x.h
>> @@ -0,0 +1,201 @@
>> +/*
>> + * Driver for Analog Devices ADV748X video decoder and HDMI receiver
>> + *
>> + * Copyright (C) 2017 Renesas Electronics Corp.
>> + *
>> + * This program is free software; you can redistribute  it and/or modify it
>> + * under  the terms of  the GNU General  Public License as published by the
>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>> + * option) any later version.
>> + *
>> + * Authors:
>> + *	Koji Matsuoka <koji.matsuoka.xm@xxxxxxxxxxx>
>> + *	Niklas Söderlund <niklas.soderlund@xxxxxxxxxxxx>
>> + *	Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
>> + */
>> +
>> +#include <linux/i2c.h>
>> +
>> +#ifndef _ADV748X_H_
>> +#define _ADV748X_H_
>> +
>> +/* I2C slave addresses */
>> +#define ADV748X_I2C_IO			0x70	/* IO Map */
>> +#define ADV748X_I2C_DPLL		0x26	/* DPLL Map */
>> +#define ADV748X_I2C_CP			0x22	/* CP Map */
>> +#define ADV748X_I2C_HDMI		0x34	/* HDMI Map */
>> +#define ADV748X_I2C_EDID		0x36	/* EDID Map */
>> +#define ADV748X_I2C_REPEATER		0x32	/* HDMI RX Repeater Map */
>> +#define ADV748X_I2C_INFOFRAME		0x31	/* HDMI RX InfoFrame Map */
>> +#define ADV748X_I2C_CEC			0x41	/* CEC Map */
>> +#define ADV748X_I2C_SDP			0x79	/* SDP Map */
>> +#define ADV748X_I2C_TXB			0x48	/* CSI-TXB Map */
>> +#define ADV748X_I2C_TXA			0x4A	/* CSI-TXA Map */
>> +#define ADV748X_I2C_WAIT		0xFE	/* Wait x mesec */
>> +#define ADV748X_I2C_EOR			0xFF	/* End Mark */
>> +
>> +/**
>> + * enum adv748x_ports - Device tree port number definitions
>> + *
>> + * The ADV748X ports define the mapping between subdevices
>> + * and the device tree specification
>> + */
>> +enum adv748x_ports {
>> +	ADV748X_PORT_HDMI = 0,
>> +	ADV748X_PORT_AIN1 = 1,
>> +	ADV748X_PORT_AIN2 = 2,
>> +	ADV748X_PORT_AIN3 = 3,
>> +	ADV748X_PORT_AIN4 = 4,
>> +	ADV748X_PORT_AIN5 = 5,
>> +	ADV748X_PORT_AIN6 = 6,
>> +	ADV748X_PORT_AIN7 = 7,
>> +	ADV748X_PORT_AIN8 = 8,
>> +	ADV748X_PORT_TTL = 9,
>> +	ADV748X_PORT_TXA = 10,
>> +	ADV748X_PORT_TXB = 11,
>> +	ADV748X_PORT_MAX = 12,
>> +};
>> +
>> +enum adv748x_csi2_pads {
>> +	ADV748X_CSI2_SINK,
>> +	ADV748X_CSI2_SOURCE,
>> +	ADV748X_CSI2_NR_PADS,
>> +};
>> +
>> +/* CSI2 transmitters can have 3 internal connections, HDMI/AFE/TTL */
>> +#define ADV748X_CSI2_MAX_SUBDEVS 3
>> +
>> +struct adv748x_csi2 {
>> +	struct adv748x_state *state;
>> +	struct v4l2_mbus_framefmt format;
>> +
>> +	struct media_pad pads[ADV748X_CSI2_NR_PADS];
>> +	struct v4l2_ctrl_handler ctrl_hdl;
>> +	struct v4l2_subdev sd;
>> +
>> +	/* Incremental async - not used now */
>> +	struct v4l2_async_subdev subdevs[ADV748X_CSI2_MAX_SUBDEVS];
>> +	struct v4l2_async_subdev *subdev_p[ADV748X_CSI2_MAX_SUBDEVS];
>> +	struct v4l2_async_notifier notifier;
>> +};
>> +
>> +#define notifier_to_csi2(n) container_of(n, struct adv748x_csi2, notifier)
>> +#define adv748x_sd_to_csi2(a) container_of(a, struct adv748x_csi2, sd)
>> +
>> +enum adv748x_hdmi_pads {
>> +	ADV748X_HDMI_SINK,
>> +	ADV748X_HDMI_SOURCE,
>> +	ADV748X_HDMI_NR_PADS,
>> +};
>> +
>> +struct adv748x_hdmi {
>> +	struct media_pad pads[ADV748X_HDMI_NR_PADS];
>> +	struct v4l2_ctrl_handler ctrl_hdl;
>> +	struct v4l2_subdev sd;
>> +
>> +	struct v4l2_dv_timings timings;
>> +};
>> +
>> +enum adv748x_afe_pads {
>> +	ADV748X_AFE_SINK_AIN0,
>> +	ADV748X_AFE_SINK_AIN1,
>> +	ADV748X_AFE_SINK_AIN2,
>> +	ADV748X_AFE_SINK_AIN3,
>> +	ADV748X_AFE_SINK_AIN4,
>> +	ADV748X_AFE_SINK_AIN5,
>> +	ADV748X_AFE_SINK_AIN6,
>> +	ADV748X_AFE_SINK_AIN7,
>> +	ADV748X_AFE_SOURCE,
>> +	ADV748X_AFE_NR_PADS,
>> +};
>> +
>> +struct adv748x_afe {
>> +	struct media_pad pads[ADV748X_AFE_NR_PADS];
>> +	struct v4l2_ctrl_handler ctrl_hdl;
>> +	struct v4l2_subdev sd;
>> +
>> +	bool streaming;
>> +	v4l2_std_id curr_norm;
>> +};
>> +
>> +/**
>> + * struct adv748x_state - State of ADV748X
>> + * @dev:		(OF) device
>> + * @client:		I2C client
>> + * @mutex:		protect global state
>> + *
>> + * @endpoints:		parsed device node endpoints for each port
>> + *
>> + * @hdmi:		state of HDMI receiver context
>> + * @sdp:		state of AFE receiver context
>> + * @txa:		state of TXA transmitter context
>> + * @txb:		state of TXB transmitter context
>> + */
>> +struct adv748x_state {
>> +	struct device *dev;
>> +	struct i2c_client *client;
>> +	struct mutex mutex;
>> +
>> +	struct device_node *endpoints[ADV748X_PORT_MAX];
>> +
>> +	struct adv748x_hdmi hdmi;
>> +	struct adv748x_afe afe;
>> +
>> +	struct adv748x_csi2 txa;
>> +	struct adv748x_csi2 txb;
>> +};
>> +
>> +#define adv748x_hdmi_to_state(a) container_of(a, struct adv748x_state, hdmi.sd)
>> +#define adv748x_afe_to_state(a) container_of(a, struct adv748x_state, afe.sd)
>> +
>> +#define adv_err(a, fmt, arg...)	dev_err(a->dev, fmt, ##arg)
>> +#define adv_info(a, fmt, arg...) dev_info(a->dev, fmt, ##arg)
>> +#define adv_dbg(a, fmt, arg...)	dev_dbg(a->dev, fmt, ##arg)
>> +
>> +/* Register handling */
>> +int adv748x_read(struct adv748x_state *state, u8 addr, u8 reg);
>> +int adv748x_write(struct adv748x_state *state, u8 addr, u8 reg, u8 value);
>> +
>> +#define io_read(s, r) adv748x_read(s, ADV748X_I2C_IO, r)
>> +#define io_write(s, r, v) adv748x_write(s, ADV748X_I2C_IO, r, v)
>> +#define io_clrset(s, r, m, v) io_write(s, r, (io_read(s, r) & ~m) | v)
>> +
>> +#define hdmi_read(s, r) adv748x_read(s, ADV748X_I2C_HDMI, r)
>> +#define hdmi_read16(s, r, m) (((hdmi_read(s, r) << 8) | hdmi_read(s, r+1)) & m)
>> +#define hdmi_write(s, r, v) adv748x_write(s, ADV748X_I2C_HDMI, r, v)
>> +#define hdmi_clrset(s, r, m, v) hdmi_write(s, r, (hdmi_read(s, r) & ~m) | v)
>> +
>> +#define sdp_read(s, r) adv748x_read(s, ADV748X_I2C_SDP, r)
>> +#define sdp_write(s, r, v) adv748x_write(s, ADV748X_I2C_SDP, r, v)
>> +#define sdp_clrset(s, r, m, v) sdp_write(s, r, (sdp_read(s, r) & ~m) | v)
>> +
>> +#define cp_read(s, r) adv748x_read(s, ADV748X_I2C_CP, r)
>> +#define cp_write(s, r, v) adv748x_write(s, ADV748X_I2C_CP, r, v)
>> +#define cp_clrset(s, r, m, v) cp_write(s, r, (cp_read(s, r) & ~m) | v)
>> +
>> +#define txa_read(s, r) adv748x_read(s, ADV748X_I2C_TXA, r)
>> +#define txa_write(s, r, v) adv748x_write(s, ADV748X_I2C_TXA, r, v)
>> +#define txa_clrset(s, r, m, v) txa_write(s, r, (txa_read(s, r) & ~m) | v)
>> +
>> +#define txb_read(s, r) adv748x_read(s, ADV748X_I2C_TXB, r)
>> +#define txb_write(s, r, v) adv748x_write(s, ADV748X_I2C_TXB, r, v)
>> +#define txb_clrset(s, r, m, v) txb_write(s, r, (txb_read(s, r) & ~m) | v)
>> +
>> +void adv748x_subdev_init(struct v4l2_subdev *sd, struct adv748x_state *state,
>> +		const struct v4l2_subdev_ops *ops, const char *ident);
>> +
>> +int adv748x_setup_links(struct adv748x_state *state);
>> +
>> +int adv748x_txa_power(struct adv748x_state *state, bool on);
>> +int adv748x_txb_power(struct adv748x_state *state, bool on);
>> +
>> +int adv748x_afe_probe(struct adv748x_state *state, struct device_node *ep);
>> +void adv748x_afe_remove(struct adv748x_state *state);
>> +
>> +int adv748x_csi2_probe(struct adv748x_state *state, struct adv748x_csi2 *tx);
>> +void adv748x_csi2_remove(struct adv748x_csi2 *tx);
>> +
>> +int adv748x_hdmi_probe(struct adv748x_state *state, struct device_node *ep);
>> +void adv748x_hdmi_remove(struct adv748x_state *state);
>> +
>> +#endif /* _ADV748X_H_ */
> 




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux