diff mbox

[RFC] s5k5baf: add camera sensor driver

Message ID 1366119522-29291-1-git-send-email-a.hajda@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrzej Hajda April 16, 2013, 1:38 p.m. UTC
Driver for Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor
with embedded SoC ISP.
The driver exposes the sensor as two V4L2 subdevices:
- S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format,
  no controls.
- S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200,
  pre/post ISP cropping, downscaling via selection API, controls.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../devicetree/bindings/media/samsung-s5k5baf.txt  |   50 +
 MAINTAINERS                                        |    8 +
 drivers/media/i2c/Kconfig                          |    7 +
 drivers/media/i2c/Makefile                         |    1 +
 drivers/media/i2c/s5k5baf.c                        | 1962 ++++++++++++++++++++
 include/media/s5k5baf.h                            |   48 +
 6 files changed, 2076 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
 create mode 100644 drivers/media/i2c/s5k5baf.c
 create mode 100644 include/media/s5k5baf.h

Comments

Hi Andrzej,

On 04/16/2013 03:38 PM, Andrzej Hajda wrote:
> Driver for Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor
> with embedded SoC ISP.
> The driver exposes the sensor as two V4L2 subdevices:
> - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format,
>   no controls.
> - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200,
>   pre/post ISP cropping, downscaling via selection API, controls.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---

It's worth to note that this driver currently doesn't use the asynchronous 
sub-device probing API [1] but the intention is to switch to it once it's
settled.

[1] http://www.spinics.net/lists/linux-sh/msg18565.html

A few more comments below...

>  .../devicetree/bindings/media/samsung-s5k5baf.txt  |   50 +
>  MAINTAINERS                                        |    8 +
>  drivers/media/i2c/Kconfig                          |    7 +
>  drivers/media/i2c/Makefile                         |    1 +
>  drivers/media/i2c/s5k5baf.c                        | 1962 ++++++++++++++++++++
>  include/media/s5k5baf.h                            |   48 +
>  6 files changed, 2076 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
>  create mode 100644 drivers/media/i2c/s5k5baf.c
>  create mode 100644 include/media/s5k5baf.h
> 
> diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
> new file mode 100644
> index 0000000..1099c1d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
> @@ -0,0 +1,50 @@
> +Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor with embedded SoC ISP
> +-------------------------------------------------------------
> +
> +Required properties:
> +
> +- compatible	  : "samsung,s5k5baf";
> +- reg		  : i2c slave address of the sensor;
> +- vdda-supply	  : analog power supply 2.8V (2.6V to 3.0V);
> +- vdd_reg-supply  : regulator input power 1.8V (1.7V to 1.9V)

s/power/power supply ?

Underscores should be avoided in the regulator supply names AFAIK, so

s/vdd_reg/vddreg

> +                    or 2.8V (2.6V to 3.0);
> +- vddio-supply	  : I/O supply 1.8V (1.65V to 1.95V)

s/supply/power supply ?

> +                    or 2.8V (2.5V to 3.1V);
> +- gpios		  : standby and reset gpios;

You are not clear about the order, how about

- gpios	: GPIOs connected to STDBYN and RSTN pins, in order: STBYN, RSTN;

?
> +- clock-frequency : master clock frequency in Hz;
> +
> +Optional properties:
> +
> +- samsung,hflip	  : horizontal image flip
> +- samsung,vflip	  : vertical image flip

Optional clock properties are missing:

 clocks : contains the sensor's master clock specifier;
 clock-names : contains "mclk" entry;

MCLK happens to be the clock input pin name in the datasheet.

> +The device node should contain one 'port' child node with one child 'endpoint'
> +node, according to the bindings defined in Documentation/devicetree/bindings/
> +media/video-interfaces.txt. The following are properties specific to those nodes.
> +
> +endpoint node
> +-------------
> +
> +- data-lanes	  : (optional) an array specifying active physical MIPI-CSI2
> +		    data output lanes and their mapping to logical lanes; the
> +		    array's content is unused, only its length is meaningful;
> +
> +Example:
> +
> +s5k5bafx@2d {
> +	compatible = "samsung,s5k5baf";
> +	reg = <0x2d>;
> +	vdda-supply = <&cam_io_en_reg>;
> +	vdd_reg-supply = <&vt_core_15v_reg>;
> +	vddio-supply = <&vtcam_reg>;
> +	gpios = <&gpl2 0 1>,
> +		<&gpl2 1 1>;
> +	clock-frequency = <24000000>;
> +
> +	port {
> +		s5k5bafx_ep: endpoint {
> +			remote-endpoint = <&csis1_ep>;
> +			data-lanes = <1>;
> +		};
> +	};
> +};

> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 9e7ce8b..e487f7d 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -553,6 +553,13 @@ config VIDEO_S5K4ECGX
>            This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M
>            camera sensor with an embedded SoC image signal processor.
>  
> +config VIDEO_S5K5BAF
> +	tristate "Samsung S5K5BAF sensor support"
> +	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> +	---help---
> +	  This is a V4L2 sensor-level driver for Samsung S5K5BAF 2M
> +	  camera sensor with an embedded SoC image signal processor.
> +
>  source "drivers/media/i2c/smiapp/Kconfig"
>  
>  config VIDEO_S5C73M3

> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> new file mode 100644
> index 0000000..0bae2d3
> --- /dev/null
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -0,0 +1,1962 @@
> +/*
> + * Driver for Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor
> + * with embedded SoC ISP.
> + *
> + * Copyright (C) 2013, Samsung Electronics Co., Ltd.
> + * Andrzej Hajda <a.hajda@samsung.com>
> + *
> + * Based on S5K6AA driver authored by Sylwester Nawrocki
> + * Sylwester Nawrocki <s.nawrocki@samsung.com>

I would rephrase it to:

  + * Based on S5K6AA driver authored by Sylwester Nawrocki
  + * Copyright (C) 2013, Samsung Electronics Co., Ltd.

> + * 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.
> + */

> +
> +static const char * const s5k5baf_supply_names[] = {
> +	"vdda",		/* Analog power supply 2.8V (2.6V to 3.0V) */
> +	"vdd_reg",	/* Regulator input power 1.8V (1.7V to 1.9V)

"vddreg"

> +			   or 2.8V (2.6V to 3.0) */
> +	"vddio",	/* I/O supply 1.8V (1.65V to 1.95V)
> +			   or 2.8V (2.5V to 3.1V) */
> +};
> +#define S5K5BAF_NUM_SUPPLIES ARRAY_SIZE(s5k5baf_supply_names)

> +struct s5k5baf_ctrls {
> +	struct v4l2_ctrl_handler handler;
> +	/* Auto / manual white balance cluster */
> +	struct v4l2_ctrl *awb;
> +	struct v4l2_ctrl *gain_red;
> +	struct v4l2_ctrl *gain_blue;
> +	struct v4l2_ctrl *gain_green;
> +	/* Mirror cluster */
> +	struct v4l2_ctrl *hflip;
> +	struct v4l2_ctrl *vflip;
> +	/* Auto exposure / manual exposure and gain cluster */
> +	struct v4l2_ctrl *auto_exp;
> +	struct v4l2_ctrl *exposure;
> +	struct v4l2_ctrl *gain;

Let's put each cluster in separate anonymous struct, so e.g.

+	struct { /* Auto exposure / manual exposure and gain cluster */
+		struct v4l2_ctrl *auto_exp;
+		struct v4l2_ctrl *exposure;
+		struct v4l2_ctrl *gain;
+	};

> +};
> +

> +/*
> + * V4L2 subdev controls
> + */

> +static int s5k5baf_log_status(struct v4l2_subdev *sd)
> +{
> +	v4l2_ctrl_handler_log_status(sd->ctrl_handler, sd->name);
> +	return 0;
> +}

This function is not needed, you could use now v4l2_ctrl_subdev_log_status()
directly.

> +#define V4L2_CID_RED_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1001)
> +#define V4L2_CID_GREEN_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1002)
> +#define V4L2_CID_BLUE_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1003)

A range should be reserved for the private controls in 
include/uapi/linux/v4l2-controls.h and used as a base here, so control
IDs of various drivers are not overlapping.

> +/*
> + * V4L2 subdev internal operations
> + */

> +static void s5k5baf_check_fw_revision(struct s5k5baf *state)
> +{
> +	u16 api_ver = 0, fw_rev = 0, s_id = 0;
> +
> +	api_ver = s5k5baf_read(state, REG_FW_APIVER);
> +	fw_rev = s5k5baf_read(state, REG_FW_REVISION) & 0xff;
> +	s_id = s5k5baf_read(state, REG_FW_SENSOR_ID);
> +	if (state->error)
> +		return;
> +
> +	v4l2_info(&state->sd, "FW API=%#x, revision=%#x sensor_id=%#x\n",
> +		  api_ver, fw_rev, s_id);
> +
> +	if (api_ver == S5K5BAF_FW_APIVER)
> +		return;
> +
> +	v4l2_err(&state->sd, "FW API version not supported\n");
> +	state->error = -ENODEV;

When we initially discussed it in private my intention was to use 
'state->error' mainly to ease handling of multiple I2C write calls. 
I have a feeling that the code would be easier to follow if it would
be returning here the error value explicitly.

> +}
> +
> +static int s5k5baf_registered(struct v4l2_subdev *sd)
> +{
> +	struct s5k5baf *state = to_s5k5baf(sd);
> +	int ret;
> +
> +	ret = v4l2_device_register_subdev(sd->v4l2_dev, &state->cis_sd);
> +	if (ret) {
> +		v4l2_err(sd, "failed to register subdev %s\n",
> +			 state->cis_sd.name);
> +		return ret;
> +	}
> +
> +	ret = media_entity_create_link(&state->cis_sd.entity,
> +			0, &state->sd.entity, 0,
> +			MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
> +
> +	mutex_lock(&state->lock);
> +
> +	s5k5baf_power_on(state);
> +	s5k5baf_hw_init(state);
> +	s5k5baf_check_fw_revision(state);
> +	s5k5baf_power_off(state);
> +
> +	ret = state->error;
> +	state->error = 0;

Probably makes sense to create a helper function that would get and
clear state->error, e.g. s5k5baf_error_get_clear() ?

> +
> +	mutex_unlock(&state->lock);
> +
> +	return ret;
> +}
> +
> +static const struct v4l2_subdev_ops s5k5baf_cis_subdev_ops = {
> +	.pad	= &s5k5baf_cis_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops s5k5baf_cis_subdev_internal_ops = {
> +	.open = s5k5baf_open,
> +};
> +
> +static const struct v4l2_subdev_internal_ops s5k5baf_subdev_internal_ops = {
> +	.registered = s5k5baf_registered,

The 'unregistered' callback is missing, it should undo anything what's 
done in s5k5baf_registered() or s5k5baf_registered() should be protected 
against multiple calls.

> +	.open = s5k5baf_open,
> +};
> +
> +static const struct v4l2_subdev_core_ops s5k5baf_core_ops = {
> +	.s_power = s5k5baf_set_power,
> +	.log_status = s5k5baf_log_status,

	.log_status = v4l2_ctrl_subdev_log_status,

> +};
> +
> +static const struct v4l2_subdev_ops s5k5baf_subdev_ops = {
> +	.core = &s5k5baf_core_ops,
> +	.pad = &s5k5baf_pad_ops,
> +	.video = &s5k5baf_video_ops,
> +};
> +
> +static void s5k5baf_configure_gpios(struct s5k5baf *state)
> +{
> +	static const char const *name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
> +	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
> +	struct s5k5baf_gpio *g = state->pdata.gpios;
> +	int ret, i;
> +
> +	if (state->error)
> +		return;
> +
> +	for (i = 0; i < GPIO_NUM; ++i) {
> +		int flags = GPIOF_EXPORT | GPIOF_DIR_OUT;
> +		if (g[i].level)
> +			flags |= GPIOF_INIT_HIGH;
> +		ret = devm_gpio_request_one(&c->dev, g[i].gpio, flags, name[i]);
> +		if (ret) {
> +			v4l2_err(c, "failed to request gpio %s\n", name[i]);
> +			state->error = ret;
> +			return;
> +		}
> +	}
> +	return;

Not needed, but anyway would be better to change the return value type to 'int'.

> +}
> +
> +static void s5k5baf_get_platform_data(struct s5k5baf *state, struct device *dev)
> +{
> +	struct device_node *node = dev->of_node;
> +	struct s5k5baf_platform_data *pd;
> +	struct device_node *node_ep;
> +	struct v4l2_of_endpoint ep;
> +	enum of_gpio_flags of_flags;
> +
> +	if (state->error)
> +		return;
> +
> +	if (!node) {
> +		pd = dev->platform_data;
> +		if (!pd) {
> +			dev_err(dev, "No platform data\n");
> +			state->error = -EINVAL;
> +		}
> +		state->pdata = *pd;
> +		return;
> +	}
> +
> +	pd = &state->pdata;
> +
> +	of_property_read_u32(node, "clock-frequency", &pd->mclk_frequency);
> +	pd->hflip = of_property_read_bool(node, "samsung,hflip");
> +	pd->vflip = of_property_read_bool(node, "samsung,vflip");
> +	pd->gpios[STBY].gpio = of_get_gpio_flags(node, STBY, &of_flags);
> +	pd->gpios[STBY].level = !(of_flags & OF_GPIO_ACTIVE_LOW);
> +	pd->gpios[RST].gpio = of_get_gpio_flags(node, RST, &of_flags);
> +	pd->gpios[RST].level = !(of_flags & OF_GPIO_ACTIVE_LOW);
> +
> +	node_ep = v4l2_of_get_next_endpoint(node, NULL);
> +	if (!node_ep) {
> +		dev_err(dev, "no endpoint defined\n");
> +		state->error = -EINVAL;
> +		return;
> +	}
> +	v4l2_of_parse_endpoint(node_ep, &ep);
> +	of_node_put(node_ep);
> +	pd->bus_type = ep.bus_type;
> +	if (pd->bus_type == V4L2_MBUS_CSI2)
> +		pd->nlanes = ep.bus.mipi_csi2.num_data_lanes;
> +}
> +
> +static void s5k5baf_configure_subdevs(struct s5k5baf *state,
> +				     struct i2c_client *c)
> +{
> +	struct v4l2_subdev *sd;
> +
> +	if (state->error)
> +		return;
> +
> +	sd = &state->cis_sd;
> +	v4l2_subdev_init(sd, &s5k5baf_cis_subdev_ops);
> +	sd->owner = c->driver->driver.owner;
> +	v4l2_set_subdevdata(sd, state);
> +	strlcpy(sd->name, "S5K5BAF-CIS", sizeof(sd->name));
> +
> +	sd->internal_ops = &s5k5baf_cis_subdev_internal_ops;
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	state->cis_pad.flags = MEDIA_PAD_FL_SOURCE;
> +	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	state->error = media_entity_init(&sd->entity, 1, &state->cis_pad, 0);
> +	if (state->error)
> +		goto err;
> +
> +	sd = &state->sd;
> +	v4l2_i2c_subdev_init(sd, c, &s5k5baf_subdev_ops);
> +	strlcpy(sd->name, "S5K5BAF-ISP", sizeof(sd->name));
> +
> +	sd->internal_ops = &s5k5baf_subdev_internal_ops;
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	state->pads[0].flags = MEDIA_PAD_FL_SINK;
> +	state->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> +	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
> +	state->error = media_entity_init(&sd->entity, 2, state->pads, 0);
> +	if (!state->error)
> +		return;
> +
> +	media_entity_cleanup(&state->cis_sd.entity);
> +err:
> +	dev_err(&c->dev, "cannot init media entity %s\n", sd->name);

Not sure if this error log is needed. Might be better to log this
upon return from this function, so 'goto' can be avoided.

> +}
> +
> +static void s5k5baf_configure_regulators(struct s5k5baf *state)
> +{
> +	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
> +	int i;
> +
> +	if (state->error)
> +		return;
> +
> +	for (i = 0; i < S5K5BAF_NUM_SUPPLIES; i++)
> +		state->supplies[i].supply = s5k5baf_supply_names[i];
> +
> +	state->error = devm_regulator_bulk_get(&c->dev, S5K5BAF_NUM_SUPPLIES,
> +				      state->supplies);
> +	if (state->error)
> +		v4l2_err(c, "failed to get regulators\n");
> +}
> +
> +static int s5k5baf_probe(struct i2c_client *c,
> +			const struct i2c_device_id *id)
> +{
> +	struct s5k5baf *state;
> +
> +	state = devm_kzalloc(&c->dev, sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	s5k5baf_get_platform_data(state, &c->dev);
> +	s5k5baf_configure_subdevs(state, c);
> +	s5k5baf_configure_gpios(state);
> +	s5k5baf_configure_regulators(state);
> +	s5k5baf_initialize_ctrls(state);
> +
> +	mutex_init(&state->lock);

Might make sense to do this as one of first steps, right after the 
private driver data structure allocation.

Some cleanup steps are missing here on the error paths, e.g.
media_entity_cleanup(&sd->entity);

What about changing the return value of all above functions used in 
probe() to 'int' so we can better track the error paths ?

> +	return state->error;
> +}
> +
> +static int s5k5baf_remove(struct i2c_client *c)
> +{
> +	struct v4l2_subdev *sd = i2c_get_clientdata(c);
> +	struct s5k5baf *state = to_s5k5baf(sd);
> +
> +	v4l2_device_unregister_subdev(sd);
> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> +	media_entity_cleanup(&sd->entity);
> +
> +	sd = &state->cis_sd;
> +	v4l2_device_unregister_subdev(sd);
> +	media_entity_cleanup(&sd->entity);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id s5k5baf_id[] = {
> +	{ DRIVER_NAME, 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, s5k5baf_id);
> +
> +static const struct of_device_id s5k5baf_of_match[] = {
> +	{ .compatible = "samsung," DRIVER_NAME },

Hmm, it looks a bit obfuscated to me, how about writing whole compatible 
string explicitly ?

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, s5k5baf_of_match);
> +
> +static struct i2c_driver s5k5baf_i2c_driver = {
> +	.driver = {
> +		.of_match_table = s5k5baf_of_match,
> +		.name = DRIVER_NAME
> +	},
> +	.probe		= s5k5baf_probe,
> +	.remove		= s5k5baf_remove,
> +	.id_table	= s5k5baf_id,
> +};
> +
> +module_i2c_driver(s5k5baf_i2c_driver);
> +
> +MODULE_DESCRIPTION("Samsung S5K5BAF(X) UXGA camera driver");
> +MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/media/s5k5baf.h b/include/media/s5k5baf.h
> new file mode 100644
> index 0000000..957e708
> --- /dev/null
> +++ b/include/media/s5k5baf.h
> @@ -0,0 +1,48 @@
> +/*
> + * S5K5BAF camera sensor driver header
> + *
> + * Copyright (C) 2011 Samsung Electronics Co., Ltd.
> + *
> + * 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.
> + */
> +
> +#ifndef S5K5BAF_H
> +#define S5K5BAF_H
> +
> +#include <media/v4l2-mediabus.h>
> +
> +/**
> + * struct s5k5baf_gpio - data structure describing a GPIO
> + * @gpio:  GPIO number
> + * @level: indicates active state of the @gpio
> + */
> +struct s5k5baf_gpio {
> +	int gpio;
> +	int level;
> +};
> +
> +/**
> + * struct s5k5baf_platform_data - s5k5baf driver platform data
> + * @set_power:   an additional callback to the board code, called
> + *               after enabling the regulators and before switching
> + *               the sensor off
> + * @mclk_frequency: sensor's master clock frequency in Hz
> + * @gpios:       GPIOs driving pins: STBY, RESET
> + * @nlanes:      maximum number of MIPI-CSI lanes used
> + * @hflip:       default horizontal image flip value, non zero to enable
> + * @vflip:       default vertical image flip value, non zero to enable
> + */
> +
> +struct s5k5baf_platform_data {
> +	u32 mclk_frequency;
> +	struct s5k5baf_gpio gpios[2];
> +	enum v4l2_mbus_type bus_type;
> +	u8 nlanes;
> +	u8 hflip:1;
> +	u8 vflip:1;
> +};
> +
> +#endif /* S5K5BAF_H */

Since we are not going to be using platform_data, I think this whole
header file could be removed for now. Let's not add a dead code. If it
happens someone ever needs platform_data I think it can be added then.


Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrzej Hajda April 19, 2013, 8:56 a.m. UTC | #2
Hi Sylwester,

Thank you for the review.
I will prepare v2 of the patch according to your comments.
My comments to your comments below...

On 17.04.2013 15:40, Sylwester Nawrocki wrote:
> Hi Andrzej,
>
> On 04/16/2013 03:38 PM, Andrzej Hajda wrote:
>> Driver for Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor
>> with embedded SoC ISP.
>> The driver exposes the sensor as two V4L2 subdevices:
>> - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format,
>>    no controls.
>> - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200,
>>    pre/post ISP cropping, downscaling via selection API, controls.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
> It's worth to note that this driver currently doesn't use the asynchronous
> sub-device probing API [1] but the intention is to switch to it once it's
> settled.
>
> [1] http://www.spinics.net/lists/linux-sh/msg18565.html
>
> A few more comments below...
>
>>   .../devicetree/bindings/media/samsung-s5k5baf.txt  |   50 +
>>   MAINTAINERS                                        |    8 +
>>   drivers/media/i2c/Kconfig                          |    7 +
>>   drivers/media/i2c/Makefile                         |    1 +
>>   drivers/media/i2c/s5k5baf.c                        | 1962 ++++++++++++++++++++
>>   include/media/s5k5baf.h                            |   48 +
>>   6 files changed, 2076 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
>>   create mode 100644 drivers/media/i2c/s5k5baf.c
>>   create mode 100644 include/media/s5k5baf.h
>>
>> diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
>> new file mode 100644
>> index 0000000..1099c1d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
>> @@ -0,0 +1,50 @@
>> +Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor with embedded SoC ISP
>> +-------------------------------------------------------------
>> +
>> +Required properties:
>> +
>> +- compatible	  : "samsung,s5k5baf";
>> +- reg		  : i2c slave address of the sensor;
>> +- vdda-supply	  : analog power supply 2.8V (2.6V to 3.0V);
>> +- vdd_reg-supply  : regulator input power 1.8V (1.7V to 1.9V)
> s/power/power supply ?
>
> Underscores should be avoided in the regulator supply names AFAIK, so
>
> s/vdd_reg/vddreg
>
>> +                    or 2.8V (2.6V to 3.0);
>> +- vddio-supply	  : I/O supply 1.8V (1.65V to 1.95V)
> s/supply/power supply ?
>
>> +                    or 2.8V (2.5V to 3.1V);
>> +- gpios		  : standby and reset gpios;
> You are not clear about the order, how about
>
> - gpios	: GPIOs connected to STDBYN and RSTN pins, in order: STBYN, RSTN;
>
> ?
>> +- clock-frequency : master clock frequency in Hz;
>> +
>> +Optional properties:
>> +
>> +- samsung,hflip	  : horizontal image flip
>> +- samsung,vflip	  : vertical image flip
> Optional clock properties are missing:
>
>   clocks : contains the sensor's master clock specifier;
>   clock-names : contains "mclk" entry;
>
> MCLK happens to be the clock input pin name in the datasheet.
>
>> +The device node should contain one 'port' child node with one child 'endpoint'
>> +node, according to the bindings defined in Documentation/devicetree/bindings/
>> +media/video-interfaces.txt. The following are properties specific to those nodes.
>> +
>> +endpoint node
>> +-------------
>> +
>> +- data-lanes	  : (optional) an array specifying active physical MIPI-CSI2
>> +		    data output lanes and their mapping to logical lanes; the
>> +		    array's content is unused, only its length is meaningful;
>> +
>> +Example:
>> +
>> +s5k5bafx@2d {
>> +	compatible = "samsung,s5k5baf";
>> +	reg = <0x2d>;
>> +	vdda-supply = <&cam_io_en_reg>;
>> +	vdd_reg-supply = <&vt_core_15v_reg>;
>> +	vddio-supply = <&vtcam_reg>;
>> +	gpios = <&gpl2 0 1>,
>> +		<&gpl2 1 1>;
>> +	clock-frequency = <24000000>;
>> +
>> +	port {
>> +		s5k5bafx_ep: endpoint {
>> +			remote-endpoint = <&csis1_ep>;
>> +			data-lanes = <1>;
>> +		};
>> +	};
>> +};
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 9e7ce8b..e487f7d 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -553,6 +553,13 @@ config VIDEO_S5K4ECGX
>>             This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M
>>             camera sensor with an embedded SoC image signal processor.
>>   
>> +config VIDEO_S5K5BAF
>> +	tristate "Samsung S5K5BAF sensor support"
>> +	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>> +	---help---
>> +	  This is a V4L2 sensor-level driver for Samsung S5K5BAF 2M
>> +	  camera sensor with an embedded SoC image signal processor.
>> +
>>   source "drivers/media/i2c/smiapp/Kconfig"
>>   
>>   config VIDEO_S5C73M3
>> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
>> new file mode 100644
>> index 0000000..0bae2d3
>> --- /dev/null
>> +++ b/drivers/media/i2c/s5k5baf.c
>> @@ -0,0 +1,1962 @@
>> +/*
>> + * Driver for Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor
>> + * with embedded SoC ISP.
>> + *
>> + * Copyright (C) 2013, Samsung Electronics Co., Ltd.
>> + * Andrzej Hajda <a.hajda@samsung.com>
>> + *
>> + * Based on S5K6AA driver authored by Sylwester Nawrocki
>> + * Sylwester Nawrocki <s.nawrocki@samsung.com>
> I would rephrase it to:
>
>    + * Based on S5K6AA driver authored by Sylwester Nawrocki
>    + * Copyright (C) 2013, Samsung Electronics Co., Ltd.
>
>> + * 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.
>> + */
>> +
>> +static const char * const s5k5baf_supply_names[] = {
>> +	"vdda",		/* Analog power supply 2.8V (2.6V to 3.0V) */
>> +	"vdd_reg",	/* Regulator input power 1.8V (1.7V to 1.9V)
> "vddreg"
>
>> +			   or 2.8V (2.6V to 3.0) */
>> +	"vddio",	/* I/O supply 1.8V (1.65V to 1.95V)
>> +			   or 2.8V (2.5V to 3.1V) */
>> +};
>> +#define S5K5BAF_NUM_SUPPLIES ARRAY_SIZE(s5k5baf_supply_names)
>> +struct s5k5baf_ctrls {
>> +	struct v4l2_ctrl_handler handler;
>> +	/* Auto / manual white balance cluster */
>> +	struct v4l2_ctrl *awb;
>> +	struct v4l2_ctrl *gain_red;
>> +	struct v4l2_ctrl *gain_blue;
>> +	struct v4l2_ctrl *gain_green;
>> +	/* Mirror cluster */
>> +	struct v4l2_ctrl *hflip;
>> +	struct v4l2_ctrl *vflip;
>> +	/* Auto exposure / manual exposure and gain cluster */
>> +	struct v4l2_ctrl *auto_exp;
>> +	struct v4l2_ctrl *exposure;
>> +	struct v4l2_ctrl *gain;
> Let's put each cluster in separate anonymous struct, so e.g.
>
> +	struct { /* Auto exposure / manual exposure and gain cluster */
> +		struct v4l2_ctrl *auto_exp;
> +		struct v4l2_ctrl *exposure;
> +		struct v4l2_ctrl *gain;
> +	};
>
>> +};
>> +
>> +/*
>> + * V4L2 subdev controls
>> + */
>> +static int s5k5baf_log_status(struct v4l2_subdev *sd)
>> +{
>> +	v4l2_ctrl_handler_log_status(sd->ctrl_handler, sd->name);
>> +	return 0;
>> +}
> This function is not needed, you could use now v4l2_ctrl_subdev_log_status()
> directly.
>
>> +#define V4L2_CID_RED_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1001)
>> +#define V4L2_CID_GREEN_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1002)
>> +#define V4L2_CID_BLUE_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1003)
> A range should be reserved for the private controls in
> include/uapi/linux/v4l2-controls.h and used as a base here, so control
> IDs of various drivers are not overlapping.
>
>> +/*
>> + * V4L2 subdev internal operations
>> + */
>> +static void s5k5baf_check_fw_revision(struct s5k5baf *state)
>> +{
>> +	u16 api_ver = 0, fw_rev = 0, s_id = 0;
>> +
>> +	api_ver = s5k5baf_read(state, REG_FW_APIVER);
>> +	fw_rev = s5k5baf_read(state, REG_FW_REVISION) & 0xff;
>> +	s_id = s5k5baf_read(state, REG_FW_SENSOR_ID);
>> +	if (state->error)
>> +		return;
>> +
>> +	v4l2_info(&state->sd, "FW API=%#x, revision=%#x sensor_id=%#x\n",
>> +		  api_ver, fw_rev, s_id);
>> +
>> +	if (api_ver == S5K5BAF_FW_APIVER)
>> +		return;
>> +
>> +	v4l2_err(&state->sd, "FW API version not supported\n");
>> +	state->error = -ENODEV;
> When we initially discussed it in private my intention was to use
> 'state->error' mainly to ease handling of multiple I2C write calls.
> I have a feeling that the code would be easier to follow if it would
> be returning here the error value explicitly.
I have different feelings :). In my opinion typical code is highly
polluted with error checking code.
In usual case for every line of function call we have at least
two lines of error checking. Ex.:

     ret = call(...);
     if (ret)
         return ret;

"state->error" pattern allows to significantly decrease number
of error checks without sacrificing code correctness.

By the way similar pattern is already used in struct v4l2_ctrl_handler.

If there is no strong objection about it I would like to keep this pattern.
>
>> +}
>> +
>> +static int s5k5baf_registered(struct v4l2_subdev *sd)
>> +{
>> +	struct s5k5baf *state = to_s5k5baf(sd);
>> +	int ret;
>> +
>> +	ret = v4l2_device_register_subdev(sd->v4l2_dev, &state->cis_sd);
>> +	if (ret) {
>> +		v4l2_err(sd, "failed to register subdev %s\n",
>> +			 state->cis_sd.name);
>> +		return ret;
>> +	}
>> +
>> +	ret = media_entity_create_link(&state->cis_sd.entity,
>> +			0, &state->sd.entity, 0,
>> +			MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
>> +
>> +	mutex_lock(&state->lock);
>> +
>> +	s5k5baf_power_on(state);
>> +	s5k5baf_hw_init(state);
>> +	s5k5baf_check_fw_revision(state);
>> +	s5k5baf_power_off(state);
>> +
>> +	ret = state->error;
>> +	state->error = 0;
> Probably makes sense to create a helper function that would get and
> clear state->error, e.g. s5k5baf_error_get_clear() ?
>
>> +
>> +	mutex_unlock(&state->lock);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct v4l2_subdev_ops s5k5baf_cis_subdev_ops = {
>> +	.pad	= &s5k5baf_cis_pad_ops,
>> +};
>> +
>> +static const struct v4l2_subdev_internal_ops s5k5baf_cis_subdev_internal_ops = {
>> +	.open = s5k5baf_open,
>> +};
>> +
>> +static const struct v4l2_subdev_internal_ops s5k5baf_subdev_internal_ops = {
>> +	.registered = s5k5baf_registered,
> The 'unregistered' callback is missing, it should undo anything what's
> done in s5k5baf_registered() or s5k5baf_registered() should be protected
> against multiple calls.
>
>> +	.open = s5k5baf_open,
>> +};
>> +
>> +static const struct v4l2_subdev_core_ops s5k5baf_core_ops = {
>> +	.s_power = s5k5baf_set_power,
>> +	.log_status = s5k5baf_log_status,
> 	.log_status = v4l2_ctrl_subdev_log_status,
>
>> +};
>> +
>> +static const struct v4l2_subdev_ops s5k5baf_subdev_ops = {
>> +	.core = &s5k5baf_core_ops,
>> +	.pad = &s5k5baf_pad_ops,
>> +	.video = &s5k5baf_video_ops,
>> +};
>> +
>> +static void s5k5baf_configure_gpios(struct s5k5baf *state)
>> +{
>> +	static const char const *name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
>> +	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
>> +	struct s5k5baf_gpio *g = state->pdata.gpios;
>> +	int ret, i;
>> +
>> +	if (state->error)
>> +		return;
>> +
>> +	for (i = 0; i < GPIO_NUM; ++i) {
>> +		int flags = GPIOF_EXPORT | GPIOF_DIR_OUT;
>> +		if (g[i].level)
>> +			flags |= GPIOF_INIT_HIGH;
>> +		ret = devm_gpio_request_one(&c->dev, g[i].gpio, flags, name[i]);
>> +		if (ret) {
>> +			v4l2_err(c, "failed to request gpio %s\n", name[i]);
>> +			state->error = ret;
>> +			return;
>> +		}
>> +	}
>> +	return;
> Not needed, but anyway would be better to change the return value type to 'int'.
>
>> +}
>> +
>> +static void s5k5baf_get_platform_data(struct s5k5baf *state, struct device *dev)
>> +{
>> +	struct device_node *node = dev->of_node;
>> +	struct s5k5baf_platform_data *pd;
>> +	struct device_node *node_ep;
>> +	struct v4l2_of_endpoint ep;
>> +	enum of_gpio_flags of_flags;
>> +
>> +	if (state->error)
>> +		return;
>> +
>> +	if (!node) {
>> +		pd = dev->platform_data;
>> +		if (!pd) {
>> +			dev_err(dev, "No platform data\n");
>> +			state->error = -EINVAL;
>> +		}
>> +		state->pdata = *pd;
>> +		return;
>> +	}
>> +
>> +	pd = &state->pdata;
>> +
>> +	of_property_read_u32(node, "clock-frequency", &pd->mclk_frequency);
>> +	pd->hflip = of_property_read_bool(node, "samsung,hflip");
>> +	pd->vflip = of_property_read_bool(node, "samsung,vflip");
>> +	pd->gpios[STBY].gpio = of_get_gpio_flags(node, STBY, &of_flags);
>> +	pd->gpios[STBY].level = !(of_flags & OF_GPIO_ACTIVE_LOW);
>> +	pd->gpios[RST].gpio = of_get_gpio_flags(node, RST, &of_flags);
>> +	pd->gpios[RST].level = !(of_flags & OF_GPIO_ACTIVE_LOW);
>> +
>> +	node_ep = v4l2_of_get_next_endpoint(node, NULL);
>> +	if (!node_ep) {
>> +		dev_err(dev, "no endpoint defined\n");
>> +		state->error = -EINVAL;
>> +		return;
>> +	}
>> +	v4l2_of_parse_endpoint(node_ep, &ep);
>> +	of_node_put(node_ep);
>> +	pd->bus_type = ep.bus_type;
>> +	if (pd->bus_type == V4L2_MBUS_CSI2)
>> +		pd->nlanes = ep.bus.mipi_csi2.num_data_lanes;
>> +}
>> +
>> +static void s5k5baf_configure_subdevs(struct s5k5baf *state,
>> +				     struct i2c_client *c)
>> +{
>> +	struct v4l2_subdev *sd;
>> +
>> +	if (state->error)
>> +		return;
>> +
>> +	sd = &state->cis_sd;
>> +	v4l2_subdev_init(sd, &s5k5baf_cis_subdev_ops);
>> +	sd->owner = c->driver->driver.owner;
>> +	v4l2_set_subdevdata(sd, state);
>> +	strlcpy(sd->name, "S5K5BAF-CIS", sizeof(sd->name));
>> +
>> +	sd->internal_ops = &s5k5baf_cis_subdev_internal_ops;
>> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +
>> +	state->cis_pad.flags = MEDIA_PAD_FL_SOURCE;
>> +	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
>> +	state->error = media_entity_init(&sd->entity, 1, &state->cis_pad, 0);
>> +	if (state->error)
>> +		goto err;
>> +
>> +	sd = &state->sd;
>> +	v4l2_i2c_subdev_init(sd, c, &s5k5baf_subdev_ops);
>> +	strlcpy(sd->name, "S5K5BAF-ISP", sizeof(sd->name));
>> +
>> +	sd->internal_ops = &s5k5baf_subdev_internal_ops;
>> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +
>> +	state->pads[0].flags = MEDIA_PAD_FL_SINK;
>> +	state->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>> +	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
>> +	state->error = media_entity_init(&sd->entity, 2, state->pads, 0);
>> +	if (!state->error)
>> +		return;
>> +
>> +	media_entity_cleanup(&state->cis_sd.entity);
>> +err:
>> +	dev_err(&c->dev, "cannot init media entity %s\n", sd->name);
> Not sure if this error log is needed. Might be better to log this
> upon return from this function, so 'goto' can be avoided.
But here we log exactly which subdev failed.
>
>> +}
>> +
>> +static void s5k5baf_configure_regulators(struct s5k5baf *state)
>> +{
>> +	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
>> +	int i;
>> +
>> +	if (state->error)
>> +		return;
>> +
>> +	for (i = 0; i < S5K5BAF_NUM_SUPPLIES; i++)
>> +		state->supplies[i].supply = s5k5baf_supply_names[i];
>> +
>> +	state->error = devm_regulator_bulk_get(&c->dev, S5K5BAF_NUM_SUPPLIES,
>> +				      state->supplies);
>> +	if (state->error)
>> +		v4l2_err(c, "failed to get regulators\n");
>> +}
>> +
>> +static int s5k5baf_probe(struct i2c_client *c,
>> +			const struct i2c_device_id *id)
>> +{
>> +	struct s5k5baf *state;
>> +
>> +	state = devm_kzalloc(&c->dev, sizeof(*state), GFP_KERNEL);
>> +	if (!state)
>> +		return -ENOMEM;
>> +
>> +	s5k5baf_get_platform_data(state, &c->dev);
>> +	s5k5baf_configure_subdevs(state, c);
>> +	s5k5baf_configure_gpios(state);
>> +	s5k5baf_configure_regulators(state);
>> +	s5k5baf_initialize_ctrls(state);
>> +
>> +	mutex_init(&state->lock);
> Might make sense to do this as one of first steps, right after the
> private driver data structure allocation.
>
> Some cleanup steps are missing here on the error paths, e.g.
> media_entity_cleanup(&sd->entity);
>
> What about changing the return value of all above functions used in
> probe() to 'int' so we can better track the error paths ?
As I explained before I would like to keep the 'state->error' pattern if 
possible.
Of course missing cleanup code will be added.
>
>> +	return state->error;
>> +}
>> +
>> +static int s5k5baf_remove(struct i2c_client *c)
>> +{
>> +	struct v4l2_subdev *sd = i2c_get_clientdata(c);
>> +	struct s5k5baf *state = to_s5k5baf(sd);
>> +
>> +	v4l2_device_unregister_subdev(sd);
>> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
>> +	media_entity_cleanup(&sd->entity);
>> +
>> +	sd = &state->cis_sd;
>> +	v4l2_device_unregister_subdev(sd);
>> +	media_entity_cleanup(&sd->entity);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct i2c_device_id s5k5baf_id[] = {
>> +	{ DRIVER_NAME, 0 },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(i2c, s5k5baf_id);
>> +
>> +static const struct of_device_id s5k5baf_of_match[] = {
>> +	{ .compatible = "samsung," DRIVER_NAME },
> Hmm, it looks a bit obfuscated to me, how about writing whole compatible
> string explicitly ?
>
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, s5k5baf_of_match);
>> +
>> +static struct i2c_driver s5k5baf_i2c_driver = {
>> +	.driver = {
>> +		.of_match_table = s5k5baf_of_match,
>> +		.name = DRIVER_NAME
>> +	},
>> +	.probe		= s5k5baf_probe,
>> +	.remove		= s5k5baf_remove,
>> +	.id_table	= s5k5baf_id,
>> +};
>> +
>> +module_i2c_driver(s5k5baf_i2c_driver);
>> +
>> +MODULE_DESCRIPTION("Samsung S5K5BAF(X) UXGA camera driver");
>> +MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/media/s5k5baf.h b/include/media/s5k5baf.h
>> new file mode 100644
>> index 0000000..957e708
>> --- /dev/null
>> +++ b/include/media/s5k5baf.h
>> @@ -0,0 +1,48 @@
>> +/*
>> + * S5K5BAF camera sensor driver header
>> + *
>> + * Copyright (C) 2011 Samsung Electronics Co., Ltd.
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef S5K5BAF_H
>> +#define S5K5BAF_H
>> +
>> +#include <media/v4l2-mediabus.h>
>> +
>> +/**
>> + * struct s5k5baf_gpio - data structure describing a GPIO
>> + * @gpio:  GPIO number
>> + * @level: indicates active state of the @gpio
>> + */
>> +struct s5k5baf_gpio {
>> +	int gpio;
>> +	int level;
>> +};
>> +
>> +/**
>> + * struct s5k5baf_platform_data - s5k5baf driver platform data
>> + * @set_power:   an additional callback to the board code, called
>> + *               after enabling the regulators and before switching
>> + *               the sensor off
>> + * @mclk_frequency: sensor's master clock frequency in Hz
>> + * @gpios:       GPIOs driving pins: STBY, RESET
>> + * @nlanes:      maximum number of MIPI-CSI lanes used
>> + * @hflip:       default horizontal image flip value, non zero to enable
>> + * @vflip:       default vertical image flip value, non zero to enable
>> + */
>> +
>> +struct s5k5baf_platform_data {
>> +	u32 mclk_frequency;
>> +	struct s5k5baf_gpio gpios[2];
>> +	enum v4l2_mbus_type bus_type;
>> +	u8 nlanes;
>> +	u8 hflip:1;
>> +	u8 vflip:1;
>> +};
>> +
>> +#endif /* S5K5BAF_H */
> Since we are not going to be using platform_data, I think this whole
> header file could be removed for now. Let's not add a dead code. If it
> happens someone ever needs platform_data I think it can be added then.
>
>
> Thanks,
> Sylwester
>

Thanks,
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
new file mode 100644
index 0000000..1099c1d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
@@ -0,0 +1,50 @@ 
+Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor with embedded SoC ISP
+-------------------------------------------------------------
+
+Required properties:
+
+- compatible	  : "samsung,s5k5baf";
+- reg		  : i2c slave address of the sensor;
+- vdda-supply	  : analog power supply 2.8V (2.6V to 3.0V);
+- vdd_reg-supply  : regulator input power 1.8V (1.7V to 1.9V)
+                    or 2.8V (2.6V to 3.0);
+- vddio-supply	  : I/O supply 1.8V (1.65V to 1.95V)
+                    or 2.8V (2.5V to 3.1V);
+- gpios		  : standby and reset gpios;
+- clock-frequency : master clock frequency in Hz;
+
+Optional properties:
+
+- samsung,hflip	  : horizontal image flip
+- samsung,vflip	  : vertical image flip
+
+The device node should contain one 'port' child node with one child 'endpoint'
+node, according to the bindings defined in Documentation/devicetree/bindings/
+media/video-interfaces.txt. The following are properties specific to those nodes.
+
+endpoint node
+-------------
+
+- data-lanes	  : (optional) an array specifying active physical MIPI-CSI2
+		    data output lanes and their mapping to logical lanes; the
+		    array's content is unused, only its length is meaningful;
+
+Example:
+
+s5k5bafx@2d {
+	compatible = "samsung,s5k5baf";
+	reg = <0x2d>;
+	vdda-supply = <&cam_io_en_reg>;
+	vdd_reg-supply = <&vt_core_15v_reg>;
+	vddio-supply = <&vtcam_reg>;
+	gpios = <&gpl2 0 1>,
+		<&gpl2 1 1>;
+	clock-frequency = <24000000>;
+
+	port {
+		s5k5bafx_ep: endpoint {
+			remote-endpoint = <&csis1_ep>;
+			data-lanes = <1>;
+		};
+	};
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index bbe872e..db4eb31 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6880,6 +6880,14 @@  L:	linux-media@vger.kernel.org
 S:	Supported
 F:	drivers/media/i2c/s5c73m3/*
 
+SAMSUNG S5K5BAF CAMERA DRIVER
+M:	Kyungmin Park <kyungmin.park@samsung.com>
+M:	Andrzej Hajda <a.hajda@samsung.com>
+L:	linux-media@vger.kernel.org
+S:	Supported
+F:	drivers/media/i2c/s5k5baf.c
+F:	include/media/s5k5baf.h
+
 SERIAL DRIVERS
 M:	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 L:	linux-serial@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9e7ce8b..e487f7d 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -553,6 +553,13 @@  config VIDEO_S5K4ECGX
           This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M
           camera sensor with an embedded SoC image signal processor.
 
+config VIDEO_S5K5BAF
+	tristate "Samsung S5K5BAF sensor support"
+	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+	---help---
+	  This is a V4L2 sensor-level driver for Samsung S5K5BAF 2M
+	  camera sensor with an embedded SoC image signal processor.
+
 source "drivers/media/i2c/smiapp/Kconfig"
 
 config VIDEO_S5C73M3
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 720f42d..6aeef24 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -64,6 +64,7 @@  obj-$(CONFIG_VIDEO_SR030PC30)	+= sr030pc30.o
 obj-$(CONFIG_VIDEO_NOON010PC30)	+= noon010pc30.o
 obj-$(CONFIG_VIDEO_S5K6AA)	+= s5k6aa.o
 obj-$(CONFIG_VIDEO_S5K4ECGX)	+= s5k4ecgx.o
+obj-$(CONFIG_VIDEO_S5K5BAF)	+= s5k5baf.o
 obj-$(CONFIG_VIDEO_S5C73M3)	+= s5c73m3/
 obj-$(CONFIG_VIDEO_ADP1653)	+= adp1653.o
 obj-$(CONFIG_VIDEO_AS3645A)	+= as3645a.o
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
new file mode 100644
index 0000000..0bae2d3
--- /dev/null
+++ b/drivers/media/i2c/s5k5baf.c
@@ -0,0 +1,1962 @@ 
+/*
+ * Driver for Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor
+ * with embedded SoC ISP.
+ *
+ * Copyright (C) 2013, Samsung Electronics Co., Ltd.
+ * Andrzej Hajda <a.hajda@samsung.com>
+ *
+ * Based on S5K6AA driver authored by Sylwester Nawrocki
+ * Sylwester Nawrocki <s.nawrocki@samsung.com>
+ *
+ * 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/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/media.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include <media/media-entity.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+#include <media/v4l2-mediabus.h>
+#include <media/v4l2-of.h>
+#include <media/s5k5baf.h>
+
+static int debug;
+module_param(debug, int, 0644);
+
+#define DRIVER_NAME			"S5K5BAF"
+
+#define S5K5BAF_OUT_WIDTH_DEF		640
+#define S5K5BAF_OUT_HEIGHT_DEF		480
+#define S5K5BAF_CIS_WIDTH		1600
+#define S5K5BAF_CIS_HEIGHT		1200
+#define S5K5BAF_WIN_WIDTH_MIN		8
+#define S5K5BAF_WIN_HEIGHT_MIN		8
+
+#define AHB_MSB_ADDR_PTR		0xfcfc
+
+/*
+ * Register interface pages (the most significant word of the address)
+ */
+#define PAGE_IF_HW			0xd000
+#define PAGE_IF_SW			0x7000
+
+/*
+ * H/W register Interface (PAGE_IF_HW)
+ */
+#define REG_SW_LOAD_COMPLETE		0x0014
+#define REG_CMDWR_PAGE			0x0028
+#define REG_CMDWR_ADDR			0x002a
+#define REG_CMDRD_PAGE			0x002c
+#define REG_CMDRD_ADDR			0x002e
+#define REG_CMD_BUF			0x0f12
+#define REG_SET_HOST_INT		0x1000
+#define REG_CLEAR_HOST_INT		0x1030
+#define REG_PATTERN_SET			0x3100
+#define REG_PATTERN_WIDTH		0x3118
+#define REG_PATTERN_HEIGHT		0x311a
+#define REG_PATTERN_PARAM		0x311c
+
+/*
+ * S/W register interface (PAGE_IF_SW)
+ */
+
+/* Firmware revision information */
+#define REG_FW_APIVER			0x012e
+#define  S5K5BAF_FW_APIVER		0x0001
+#define REG_FW_REVISION			0x0130
+#define REG_FW_SENSOR_ID		0x0152
+
+/* Initialization parameters */
+/* Master clock frequency in KHz */
+#define REG_I_INCLK_FREQ_L		0x01b8
+#define REG_I_INCLK_FREQ_H		0x01ba
+#define  MIN_MCLK_FREQ_KHZ		6000U
+#define  MAX_MCLK_FREQ_KHZ		48000U
+#define REG_I_USE_NPVI_CLOCKS		0x01c6
+#define  NPVI_CLOCKS			1
+#define REG_I_USE_NMIPI_CLOCKS		0x01c8
+#define  NMIPI_CLOCKS			1
+#define REG_I_BLOCK_INTERNAL_PLL_CALC	0x01ca
+
+/* Clock configurations, n = 0..2. REG_I_* frequency unit is 4 kHz. */
+#define REG_I_OPCLK_4KHZ(n)		((n) * 6 + 0x01cc)
+#define REG_I_MIN_OUTRATE_4KHZ(n)	((n) * 6 + 0x01ce)
+#define REG_I_MAX_OUTRATE_4KHZ(n)	((n) * 6 + 0x01d0)
+#define  SCLK_PVI_FREQ			24000
+#define  SCLK_MIPI_FREQ			48000
+#define  PCLK_MIN_FREQ			6000
+#define  PCLK_MAX_FREQ			48000
+#define REG_I_USE_REGS_API		0x01de
+#define REG_I_INIT_PARAMS_UPDATED	0x01e0
+#define REG_I_ERROR_INFO		0x01e2
+
+/* General purpose parameters */
+#define REG_USER_BRIGHTNESS		0x01e4
+#define REG_USER_CONTRAST		0x01e6
+#define REG_USER_SATURATION		0x01e8
+#define REG_USER_SHARPBLUR		0x01ea
+
+#define REG_G_SPEC_EFFECTS		0x01ee
+#define REG_G_ENABLE_PREV		0x01f0
+#define REG_G_ENABLE_PREV_CHG		0x01f2
+#define REG_G_NEW_CFG_SYNC		0x01f8
+#define REG_G_PREVREQ_IN_WIDTH		0x01fa
+#define REG_G_PREVREQ_IN_HEIGHT		0x01fc
+#define REG_G_PREVREQ_IN_XOFFS		0x01fe
+#define REG_G_PREVREQ_IN_YOFFS		0x0200
+#define REG_G_PREVZOOM_IN_WIDTH		0x020a
+#define REG_G_PREVZOOM_IN_HEIGHT	0x020c
+#define REG_G_PREVZOOM_IN_XOFFS		0x020e
+#define REG_G_PREVZOOM_IN_YOFFS		0x0210
+#define REG_G_INPUTS_CHANGE_REQ		0x021a
+#define REG_G_ACTIVE_PREV_CFG		0x021c
+#define REG_G_PREV_CFG_CHG		0x021e
+#define REG_G_PREV_OPEN_AFTER_CH	0x0220
+#define REG_G_PREV_CFG_ERROR		0x0222
+#define  CFG_ERROR_RANGE		0x0b
+#define REG_G_PREV_CFG_BYPASS_CHANGED	0x022a
+#define REG_G_ACTUAL_P_FR_TIME		0x023a
+#define REG_G_ACTUAL_P_OUT_RATE		0x023c
+#define REG_G_ACTUAL_C_FR_TIME		0x023e
+#define REG_G_ACTUAL_C_OUT_RATE		0x0240
+
+/* Preview control section. n = 0...4. */
+#define PREG(n, x)			((n) * 0x26 + x)
+#define REG_P_OUT_WIDTH(n)		PREG(n, 0x0242)
+#define REG_P_OUT_HEIGHT(n)		PREG(n, 0x0244)
+#define REG_P_FMT(n)			PREG(n, 0x0246)
+#define REG_P_MAX_OUT_RATE(n)		PREG(n, 0x0248)
+#define REG_P_MIN_OUT_RATE(n)		PREG(n, 0x024a)
+#define REG_P_PVI_MASK(n)		PREG(n, 0x024c)
+#define  PVI_MASK_MIPI			0x52
+#define REG_P_CLK_INDEX(n)		PREG(n, 0x024e)
+#define  CLK_PVI_INDEX			0
+#define  CLK_MIPI_INDEX			NPVI_CLOCKS
+#define REG_P_FR_RATE_TYPE(n)		PREG(n, 0x0250)
+#define  FR_RATE_DYNAMIC		0
+#define  FR_RATE_FIXED			1
+#define  FR_RATE_FIXED_ACCURATE		2
+#define REG_P_FR_RATE_Q_TYPE(n)		PREG(n, 0x0252)
+#define  FR_RATE_Q_DYNAMIC		0
+#define  FR_RATE_Q_BEST_FRRATE		1 /* Binning enabled */
+#define  FR_RATE_Q_BEST_QUALITY		2 /* Binning disabled */
+/* Frame period in 0.1 ms units */
+#define REG_P_MAX_FR_TIME(n)		PREG(n, 0x0254)
+#define REG_P_MIN_FR_TIME(n)		PREG(n, 0x0256)
+#define  S5K5BAF_MIN_FR_TIME		333  /* x100 us */
+#define  S5K5BAF_MAX_FR_TIME		6500 /* x100 us */
+/* The below 5 registers are for "device correction" values */
+#define REG_P_SATURATION(n)		PREG(n, 0x0258)
+#define REG_P_SHARP_BLUR(n)		PREG(n, 0x025a)
+#define REG_P_GLAMOUR(n)		PREG(n, 0x025c)
+#define REG_P_COLORTEMP(n)		PREG(n, 0x025e)
+#define REG_P_GAMMA_INDEX(n)		PREG(n, 0x0260)
+#define REG_P_PREV_MIRROR(n)		PREG(n, 0x0262)
+#define REG_P_CAP_MIRROR(n)		PREG(n, 0x0264)
+#define REG_P_CAP_ROTATION(n)		PREG(n, 0x0266)
+
+/* Extended image property controls */
+/* Exposure time in 10 us units */
+#define REG_SF_USR_EXPOSURE_L		0x03bc
+#define REG_SF_USR_EXPOSURE_H		0x03be
+#define REG_SF_USR_EXPOSURE_CHG		0x03c0
+#define REG_SF_USR_TOT_GAIN		0x03c2
+#define REG_SF_USR_TOT_GAIN_CHG		0x03c4
+#define REG_SF_RGAIN			0x03c6
+#define REG_SF_RGAIN_CHG		0x03c8
+#define REG_SF_GGAIN			0x03ca
+#define REG_SF_GGAIN_CHG		0x03cc
+#define REG_SF_BGAIN			0x03ce
+#define REG_SF_BGAIN_CHG		0x03d0
+#define REG_SF_WBGAIN_CHG		0x03d2
+#define REG_SF_FLICKER_QUANT		0x03d4
+#define REG_SF_FLICKER_QUANT_CHG	0x03d6
+
+/* Output interface (parallel/MIPI) setup */
+#define REG_OIF_EN_MIPI_LANES		0x03f2
+#define REG_OIF_EN_PACKETS		0x03f4
+#define  EN_PACKETS_CSI2		0xc3
+#define REG_OIF_CFG_CHG			0x03f6
+
+/* Auto-algorithms enable mask */
+#define REG_DBG_AUTOALG_EN		0x03f8
+#define  AALG_ALL_EN			BIT(0)
+#define  AALG_AE_EN			BIT(1)
+#define  AALG_DIVLEI_EN			BIT(2)
+#define  AALG_WB_EN			BIT(3)
+#define  AALG_USE_WB_FOR_ISP		BIT(4)
+#define  AALG_FLICKER_EN		BIT(5)
+#define  AALG_FIT_EN			BIT(6)
+#define  AALG_WRHW_EN			BIT(7)
+
+#define REG_PTR_CCM_HORIZON		0x06d0
+#define REG_PTR_CCM_INCANDESCENT	0x06d4
+#define REG_PTR_CCM_WARM_WHITE		0x06d8
+#define REG_PTR_CCM_COOL_WHITE		0x06dc
+#define REG_PTR_CCM_DL50		0x06e0
+#define REG_PTR_CCM_DL65		0x06e4
+#define REG_PTR_CCM_OUTDOOR		0x06ec
+
+#define REG_ARR_CCM(n)			(0x2800 + 36 * (n))
+
+static const char * const s5k5baf_supply_names[] = {
+	"vdda",		/* Analog power supply 2.8V (2.6V to 3.0V) */
+	"vdd_reg",	/* Regulator input power 1.8V (1.7V to 1.9V)
+			   or 2.8V (2.6V to 3.0) */
+	"vddio",	/* I/O supply 1.8V (1.65V to 1.95V)
+			   or 2.8V (2.5V to 3.1V) */
+};
+#define S5K5BAF_NUM_SUPPLIES ARRAY_SIZE(s5k5baf_supply_names)
+
+enum s5k5baf_gpio_id {
+	STBY,
+	RST,
+	GPIO_NUM,
+};
+
+struct s5k5baf_pixfmt {
+	enum v4l2_mbus_pixelcode code;
+	u32 colorspace;
+	/* REG_P_FMT(x) register value */
+	u16 reg_p_fmt;
+};
+
+struct s5k5baf_ctrls {
+	struct v4l2_ctrl_handler handler;
+	/* Auto / manual white balance cluster */
+	struct v4l2_ctrl *awb;
+	struct v4l2_ctrl *gain_red;
+	struct v4l2_ctrl *gain_blue;
+	struct v4l2_ctrl *gain_green;
+	/* Mirror cluster */
+	struct v4l2_ctrl *hflip;
+	struct v4l2_ctrl *vflip;
+	/* Auto exposure / manual exposure and gain cluster */
+	struct v4l2_ctrl *auto_exp;
+	struct v4l2_ctrl *exposure;
+	struct v4l2_ctrl *gain;
+};
+
+struct s5k5baf {
+	struct s5k5baf_platform_data pdata;
+	struct regulator_bulk_data supplies[S5K5BAF_NUM_SUPPLIES];
+
+	struct v4l2_subdev cis_sd;
+	struct media_pad cis_pad;
+
+	struct v4l2_subdev sd;
+	struct media_pad pads[2];
+
+	/* protects the struct members below */
+	struct mutex lock;
+
+	int error;
+
+	struct v4l2_rect crop_sink;
+	struct v4l2_rect compose;
+	struct v4l2_rect crop_source;
+	/* index to s5k5baf_formats array */
+	int pixfmt;
+	/* actual frame interval in 100us */
+	u16 fiv;
+	/* requested frame interval in 100us */
+	u16 req_fiv;
+
+	struct s5k5baf_ctrls ctrls;
+
+	unsigned int streaming:1;
+	unsigned int apply_cfg:1;
+	unsigned int apply_crop:1;
+	unsigned int power;
+};
+
+static const struct s5k5baf_pixfmt s5k5baf_formats[] = {
+	{ V4L2_MBUS_FMT_VYUY8_2X8,	V4L2_COLORSPACE_JPEG,	5 },
+	/* range 16-240 */
+	{ V4L2_MBUS_FMT_VYUY8_2X8,	V4L2_COLORSPACE_REC709,	6 },
+	{ V4L2_MBUS_FMT_RGB565_2X8_BE,	V4L2_COLORSPACE_JPEG,	0 },
+};
+
+static struct v4l2_rect s5k5baf_cis_rect = { 0, 0, S5K5BAF_CIS_WIDTH,
+				     S5K5BAF_CIS_HEIGHT };
+static struct v4l2_rect s5k5baf_def_rect = { 0, 0, S5K5BAF_OUT_WIDTH_DEF,
+				     S5K5BAF_OUT_HEIGHT_DEF };
+
+static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
+{
+	return &container_of(ctrl->handler, struct s5k5baf, ctrls.handler)->sd;
+}
+
+static inline bool s5k5baf_is_cis_subdev(struct v4l2_subdev *sd)
+{
+	return sd->entity.type == MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
+}
+
+static inline struct s5k5baf *to_s5k5baf(struct v4l2_subdev *sd)
+{
+	if (s5k5baf_is_cis_subdev(sd))
+		return container_of(sd, struct s5k5baf, cis_sd);
+	else
+		return container_of(sd, struct s5k5baf, sd);
+}
+
+static u16 s5k5baf_i2c_read(struct s5k5baf *state, u16 addr)
+{
+	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
+	u16 w, r;
+	struct i2c_msg msg[] = {
+		{.addr = c->addr, .flags = 0, .len = 2, .buf = (u8 *)&w},
+		{.addr = c->addr, .flags = I2C_M_RD, .len = 2, .buf = (u8 *)&r},
+	};
+	int ret;
+
+	if (state->error)
+		return 0;
+
+	w = htons(addr);
+	ret = i2c_transfer(c->adapter, msg, 2);
+	r = ntohs(r);
+
+	v4l2_dbg(3, debug, c, "i2c_read: 0x%04x : 0x%04x\n", addr, r);
+
+	if (ret != 2) {
+		v4l2_err(c, "i2c_read: error during transfer (%d)\n", ret);
+		state->error = ret;
+	}
+	return r;
+}
+
+static void s5k5baf_i2c_write(struct s5k5baf *state, u16 addr, u16 val)
+{
+	u8 buf[4] = { addr >> 8, addr & 0xFF, val >> 8, val & 0xFF };
+	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
+	int ret;
+
+	if (state->error)
+		return;
+
+	ret = i2c_master_send(c, buf, 4);
+	v4l2_dbg(3, debug, c, "i2c_write: 0x%04x : 0x%04x\n", addr, val);
+
+	if (ret != 4) {
+		v4l2_err(c, "i2c_write: error during transfer (%d)\n", ret);
+		state->error = ret;
+	}
+}
+
+static u16 s5k5baf_read(struct s5k5baf *state, u16 addr)
+{
+	s5k5baf_i2c_write(state, REG_CMDRD_ADDR, addr);
+	return s5k5baf_i2c_read(state, REG_CMD_BUF);
+}
+
+static void s5k5baf_write(struct s5k5baf *state, u16 addr, u16 val)
+{
+	s5k5baf_i2c_write(state, REG_CMDWR_ADDR, addr);
+	s5k5baf_i2c_write(state, REG_CMD_BUF, val);
+}
+
+static void s5k5baf_write_arr_seq(struct s5k5baf *state, u16 addr,
+				  u16 count, const u16 *seq)
+{
+	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
+	u16 buf[count + 1];
+	int ret, n;
+
+	s5k5baf_i2c_write(state, REG_CMDWR_ADDR, addr);
+	if (state->error)
+		return;
+
+	buf[0] = __constant_htons(REG_CMD_BUF);
+	for (n = 1; n <= count; ++n)
+		buf[n] = htons(*seq++);
+
+	n *= 2;
+	ret = i2c_master_send(c, (char *)buf, n);
+	v4l2_dbg(3, debug, c, "i2c_write_seq(count=%d): %*ph\n", count,
+		 min(2 * count, 64), seq - count);
+
+	if (ret != n) {
+		v4l2_err(c, "i2c_write_seq: error during transfer (%d)\n", ret);
+		state->error = ret;
+	}
+}
+
+#define s5k5baf_write_seq(state, addr, seq...) \
+	s5k5baf_write_arr_seq(state, addr, sizeof((char[]){ seq }), \
+			      (const u16 []){ seq });
+
+/* add items count at the beginning of the list */
+#define NSEQ(seq...) sizeof((char[]){ seq }), seq
+
+/*
+ * s5k5baf_write_nseq() - Writes sequences of values to sensor memory via i2c
+ * @nseq: sequence of u16 words in format:
+ *	(N, address, value[1]...value[N-1])*,0
+ * Ex.:
+ *	u16 seq[] = { NSEQ(0x4000, 1, 1), NSEQ(0x4010, 640, 480), 0 };
+ *	ret = s5k5baf_write_nseq(c, seq);
+ */
+static void s5k5baf_write_nseq(struct s5k5baf *state, const u16 *nseq)
+{
+	int count;
+
+	while ((count = *nseq++)) {
+		u16 addr = *nseq++;
+		--count;
+
+		s5k5baf_write_arr_seq(state, addr, count, nseq);
+		nseq += count;
+	}
+}
+
+static void s5k5baf_synchronize(struct s5k5baf *state, int timeout, u16 addr)
+{
+	unsigned long end = jiffies + msecs_to_jiffies(timeout);
+	u16 reg;
+
+	s5k5baf_write(state, addr, 1);
+	do {
+		reg = s5k5baf_read(state, addr);
+		if (state->error || !reg)
+			return;
+		usleep_range(5000, 10000);
+	} while (time_is_after_jiffies(end));
+
+	v4l2_err(&state->sd, "timeout on register synchronize (%#x)\n", addr);
+	state->error = -ETIMEDOUT;
+}
+
+static void s5k5baf_hw_patch(struct s5k5baf *state)
+{
+	static const u16 nseq_patch[] = {
+		NSEQ(0x1668,
+		0xb5fe, 0x0007, 0x683c, 0x687e, 0x1da5, 0x88a0, 0x2800, 0xd00b,
+		0x88a8, 0x2800, 0xd008, 0x8820, 0x8829, 0x4288, 0xd301, 0x1a40,
+		0xe000, 0x1a08, 0x9001, 0xe001, 0x2019, 0x9001, 0x4916, 0x466b,
+		0x8a48, 0x8118, 0x8a88, 0x8158, 0x4814, 0x8940, 0x0040, 0x2103,
+		0xf000, 0xf826, 0x88a1, 0x4288, 0xd908, 0x8828, 0x8030, 0x8868,
+		0x8070, 0x88a8, 0x6038, 0xbcfe, 0xbc08, 0x4718, 0x88a9, 0x4288,
+		0xd906, 0x8820, 0x8030, 0x8860, 0x8070, 0x88a0, 0x6038, 0xe7f2,
+		0x9801, 0xa902, 0xf000, 0xf812, 0x0033, 0x0029, 0x9a02, 0x0020,
+		0xf000, 0xf814, 0x6038, 0xe7e6, 0x1a28, 0x7000, 0x0d64, 0x7000,
+		0x4778, 0x46c0, 0xf004, 0xe51f, 0xa464, 0x0000, 0x4778, 0x46c0,
+		0xc000, 0xe59f, 0xff1c, 0xe12f, 0x6009, 0x0000, 0x4778, 0x46c0,
+		0xc000, 0xe59f, 0xff1c, 0xe12f, 0x622f, 0x0000),
+		NSEQ(0x2080,
+		0xb510, 0xf000, 0xf8f4, 0xbc10, 0xbc08, 0x4718, 0xb5f0, 0xb08b,
+		0x0006, 0x2000, 0x9004, 0x6835, 0x6874, 0x68b0, 0x900a, 0x68f0,
+		0x9009, 0x4f7d, 0x8979, 0x084a, 0x88a8, 0x88a3, 0x4298, 0xd300,
+		0x0018, 0xf000, 0xf907, 0x9007, 0x0021, 0x0028, 0xaa04, 0xf000,
+		0xf909, 0x9006, 0x88a8, 0x2800, 0xd102, 0x27ff, 0x1c7f, 0xe047,
+		0x88a0, 0x2800, 0xd101, 0x2700, 0xe042, 0x8820, 0x466b, 0x8198,
+		0x8860, 0x81d8, 0x8828, 0x8118, 0x8868, 0x8158, 0xa802, 0xc803,
+		0xf000, 0xf8f8, 0x9008, 0x8aba, 0x9808, 0x466b, 0x4342, 0x9202,
+		0x8820, 0x8198, 0x8860, 0x81d8, 0x980a, 0x9903, 0xf000, 0xf8ea,
+		0x9a02, 0x17d1, 0x0e09, 0x1889, 0x1209, 0x4288, 0xdd1f, 0x8820,
+		0x466b, 0x8198, 0x8860, 0x81d8, 0x980a, 0x9903, 0xf000, 0xf8da,
+		0x9001, 0x8828, 0x466b, 0x8118, 0x8868, 0x8158, 0x980a, 0x9902,
+		0xf000, 0xf8d0, 0x8ab9, 0x9a08, 0x4351, 0x17ca, 0x0e12, 0x1851,
+		0x120a, 0x9901, 0xf000, 0xf8b6, 0x0407, 0x0c3f, 0xe000, 0x2700,
+		0x8820, 0x466b, 0xaa05, 0x8198, 0x8860, 0x81d8, 0x8828, 0x8118,
+		0x8868, 0x8158, 0xa802, 0xc803, 0x003b, 0xf000, 0xf8bb, 0x88a1,
+		0x88a8, 0x003a, 0xf000, 0xf8be, 0x0004, 0xa804, 0xc803, 0x9a09,
+		0x9b07, 0xf000, 0xf8af, 0xa806, 0xc805, 0x0021, 0xf000, 0xf8b2,
+		0x6030, 0xb00b, 0xbcf0, 0xbc08, 0x4718, 0xb5f1, 0x9900, 0x680c,
+		0x493a, 0x694b, 0x698a, 0x4694, 0x69cd, 0x6a0e, 0x4f38, 0x42bc,
+		0xd800, 0x0027, 0x4937, 0x6b89, 0x0409, 0x0c09, 0x4a35, 0x1e92,
+		0x6bd2, 0x0412, 0x0c12, 0x429f, 0xd801, 0x0020, 0xe031, 0x001f,
+		0x434f, 0x0a3f, 0x42a7, 0xd301, 0x0018, 0xe02a, 0x002b, 0x434b,
+		0x0a1b, 0x42a3, 0xd303, 0x0220, 0xf000, 0xf88c, 0xe021, 0x0029,
+		0x4351, 0x0a09, 0x42a1, 0xd301, 0x0028, 0xe01a, 0x0031, 0x4351,
+		0x0a09, 0x42a1, 0xd304, 0x0220, 0x0011, 0xf000, 0xf87b, 0xe010,
+		0x491e, 0x8c89, 0x000a, 0x4372, 0x0a12, 0x42a2, 0xd301, 0x0030,
+		0xe007, 0x4662, 0x434a, 0x0a12, 0x42a2, 0xd302, 0x0220, 0xf000,
+		0xf869, 0x4b16, 0x4d18, 0x8d99, 0x1fca, 0x3af9, 0xd00a, 0x2001,
+		0x0240, 0x8468, 0x0220, 0xf000, 0xf85d, 0x9900, 0x6008, 0xbcf8,
+		0xbc08, 0x4718, 0x8d19, 0x8469, 0x9900, 0x6008, 0xe7f7, 0xb570,
+		0x2200, 0x490e, 0x480e, 0x2401, 0xf000, 0xf852, 0x0022, 0x490d,
+		0x480d, 0x2502, 0xf000, 0xf84c, 0x490c, 0x480d, 0x002a, 0xf000,
+		0xf847, 0xbc70, 0xbc08, 0x4718, 0x0d64, 0x7000, 0x0470, 0x7000,
+		0xa120, 0x0007, 0x0402, 0x7000, 0x14a0, 0x7000, 0x208d, 0x7000,
+		0x622f, 0x0000, 0x1669, 0x7000, 0x6445, 0x0000, 0x21ab, 0x7000,
+		0x2aa9, 0x0000, 0x4778, 0x46c0, 0xc000, 0xe59f, 0xff1c, 0xe12f,
+		0x5f49, 0x0000, 0x4778, 0x46c0, 0xc000, 0xe59f, 0xff1c, 0xe12f,
+		0x5fc7, 0x0000, 0x4778, 0x46c0, 0xc000, 0xe59f, 0xff1c, 0xe12f,
+		0x5457, 0x0000, 0x4778, 0x46c0, 0xc000, 0xe59f, 0xff1c, 0xe12f,
+		0x5fa3, 0x0000, 0x4778, 0x46c0, 0xc000, 0xe59f, 0xff1c, 0xe12f,
+		0x51f9, 0x0000, 0x4778, 0x46c0, 0xf004, 0xe51f, 0xa464, 0x0000,
+		0x4778, 0x46c0, 0xc000, 0xe59f, 0xff1c, 0xe12f, 0xa007, 0x0000,
+		0x6546, 0x2062, 0x3120, 0x3220, 0x3130, 0x0030, 0xe010, 0x0208,
+		0x0058, 0x0000),
+		0
+	};
+
+	s5k5baf_write_nseq(state, nseq_patch);
+}
+
+static void s5k5baf_hw_set_clocks(struct s5k5baf *state)
+{
+	unsigned long mclk = state->pdata.mclk_frequency / 1000;
+	u16 status;
+	static const u16 nseq_clk_cfg[] = {
+		NSEQ(REG_I_USE_NPVI_CLOCKS,
+		  NPVI_CLOCKS, NMIPI_CLOCKS, 0,
+		  SCLK_PVI_FREQ / 4, PCLK_MIN_FREQ / 4, PCLK_MAX_FREQ / 4,
+		  SCLK_MIPI_FREQ / 4, PCLK_MIN_FREQ / 4, PCLK_MAX_FREQ / 4),
+		NSEQ(REG_I_USE_REGS_API, 1),
+		0
+	};
+
+	s5k5baf_write_seq(state, REG_I_INCLK_FREQ_L, mclk & 0xffff, mclk >> 16);
+	s5k5baf_write_nseq(state, nseq_clk_cfg);
+
+	s5k5baf_synchronize(state, 250, REG_I_INIT_PARAMS_UPDATED);
+	status = s5k5baf_read(state, REG_I_ERROR_INFO);
+	if (!state->error && status) {
+		v4l2_err(&state->sd, "error configuring PLL (%d)\n", status);
+		state->error = -EINVAL;
+	}
+}
+
+static void s5k5baf_hw_set_ccm(struct s5k5baf *state)
+{
+	static const u16 nseq_cfg[] = {
+		NSEQ(REG_PTR_CCM_HORIZON,
+		REG_ARR_CCM(0), PAGE_IF_SW,
+		REG_ARR_CCM(1), PAGE_IF_SW,
+		REG_ARR_CCM(2), PAGE_IF_SW,
+		REG_ARR_CCM(3), PAGE_IF_SW,
+		REG_ARR_CCM(4), PAGE_IF_SW,
+		REG_ARR_CCM(5), PAGE_IF_SW),
+		NSEQ(REG_PTR_CCM_OUTDOOR,
+		REG_ARR_CCM(6), PAGE_IF_SW),
+		NSEQ(REG_ARR_CCM(0),
+		/* horizon */
+		0x010d, 0xffa7, 0xfff5, 0x003b, 0x00ef, 0xff38,
+		0xfe42, 0x0270, 0xff71, 0xfeed, 0x0198, 0x0198,
+		0xff95, 0xffa3, 0x0260, 0x00ec, 0xff33, 0x00f4,
+		/* incandescent */
+		0x010d, 0xffa7, 0xfff5, 0x003b, 0x00ef, 0xff38,
+		0xfe42, 0x0270, 0xff71, 0xfeed, 0x0198, 0x0198,
+		0xff95, 0xffa3, 0x0260, 0x00ec, 0xff33, 0x00f4,
+		/* warm white */
+		0x01ea, 0xffb9, 0xffdb, 0x0127, 0x0109, 0xff3c,
+		0xff2b, 0x021b, 0xff48, 0xff03, 0x0207, 0x0113,
+		0xffca, 0xff93, 0x016f, 0x0164, 0xff55, 0x0163,
+		/* cool white */
+		0x01ea, 0xffb9, 0xffdb, 0x0127, 0x0109, 0xff3c,
+		0xff2b, 0x021b, 0xff48, 0xff03, 0x0207, 0x0113,
+		0xffca, 0xff93, 0x016f, 0x0164, 0xff55, 0x0163,
+		/* daylight 5000K */
+		0x0194, 0xffad, 0xfffe, 0x00c5, 0x0103, 0xff5d,
+		0xfee3, 0x01ae, 0xff27, 0xff18, 0x018f, 0x00c8,
+		0xffe8, 0xffaa, 0x01c8, 0x0132, 0xff3e, 0x0100,
+		/* daylight 6500K */
+		0x0194, 0xffad, 0xfffe, 0x00c5, 0x0103, 0xff5d,
+		0xfee3, 0x01ae, 0xff27, 0xff18, 0x018f, 0x00c8,
+		0xffe8, 0xffaa, 0x01c8, 0x0132, 0xff3e, 0x0100,
+		/* outdoor */
+		0x01cc, 0xffc3, 0x0009, 0x00a2, 0x0106, 0xff3f,
+		0xfed8, 0x01fe, 0xff08, 0xfec7, 0x00f5, 0x0119,
+		0xffdf, 0x0024, 0x01a8, 0x0170, 0xffad, 0x011b),
+		0
+	};
+	s5k5baf_write_nseq(state, nseq_cfg);
+}
+
+static void s5k5baf_hw_set_cis(struct s5k5baf *state)
+{
+	static const u16 nseq_cfg[] = {
+		NSEQ(0xc202, 0x0700),
+		NSEQ(0xf260, 0x0001),
+		NSEQ(0xf414, 0x0030),
+		NSEQ(0xc204, 0x0100),
+		NSEQ(0xf402, 0x0092, 0x007f),
+		NSEQ(0xf700, 0x0040),
+		NSEQ(0xf708,
+		0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
+		0x0040, 0x0040, 0x0040, 0x0040, 0x0040,
+		0x0001, 0x0015, 0x0001, 0x0040),
+		NSEQ(0xf48a, 0x0048),
+		NSEQ(0xf10a, 0x008b),
+		NSEQ(0xf900, 0x0067),
+		NSEQ(0xf406, 0x0092, 0x007f, 0x0003, 0x0003, 0x0003),
+		NSEQ(0xf442, 0x0000, 0x0000),
+		NSEQ(0xf448, 0x0000),
+		NSEQ(0xf456, 0x0001, 0x0010, 0x0000),
+		NSEQ(0xf41a, 0x00ff, 0x0003, 0x0030),
+		NSEQ(0xf410, 0x0001, 0x0000),
+		NSEQ(0xf416, 0x0001),
+		NSEQ(0xf424, 0x0000),
+		NSEQ(0xf422, 0x0000),
+		NSEQ(0xf41e, 0x0000),
+		NSEQ(0xf428, 0x0000, 0x0000, 0x0000),
+		NSEQ(0xf430, 0x0000, 0x0000, 0x0008, 0x0005, 0x000f, 0x0001,
+		0x0040, 0x0040, 0x0010),
+		NSEQ(0xf4d6, 0x0090, 0x0000),
+		NSEQ(0xf47c, 0x000c, 0x0000),
+		NSEQ(0xf49a, 0x0008, 0x0000),
+		NSEQ(0xf4a2, 0x0008, 0x0000),
+		NSEQ(0xf4b2, 0x0013, 0x0000, 0x0013, 0x0000),
+		NSEQ(0xf4aa, 0x009b, 0x00fb, 0x009b, 0x00fb),
+		0
+	};
+
+	s5k5baf_i2c_write(state, REG_CMDWR_PAGE, PAGE_IF_HW);
+	s5k5baf_write_nseq(state, nseq_cfg);
+	s5k5baf_i2c_write(state, REG_CMDWR_PAGE, PAGE_IF_SW);
+}
+
+static void s5k5baf_hw_sync_cfg(struct s5k5baf *state)
+{
+	s5k5baf_write(state, REG_G_PREV_CFG_CHG, 1);
+	if (state->apply_crop) {
+		s5k5baf_write(state, REG_G_INPUTS_CHANGE_REQ, 1);
+		s5k5baf_write(state, REG_G_PREV_CFG_BYPASS_CHANGED, 1);
+	}
+	s5k5baf_synchronize(state, 500, REG_G_NEW_CFG_SYNC);
+
+}
+/* Set horizontal and vertical image flipping */
+static void s5k5baf_hw_set_mirror(struct s5k5baf *state, int horiz_flip)
+{
+	u16 vflip = state->ctrls.vflip->val ^ state->pdata.vflip;
+	u16 flip = (horiz_flip ^ state->pdata.hflip) | (vflip << 1);
+
+	s5k5baf_write(state, REG_P_PREV_MIRROR(0), flip);
+	if (state->streaming)
+		s5k5baf_hw_sync_cfg(state);
+}
+
+/* Configure auto/manual white balance and R/G/B gains */
+static void s5k5baf_hw_set_awb(struct s5k5baf *state, int awb)
+{
+	struct s5k5baf_ctrls *ctrls = &state->ctrls;
+	u16 reg;
+
+	reg = s5k5baf_read(state, REG_DBG_AUTOALG_EN);
+
+	if (!awb)
+		s5k5baf_write_seq(state, REG_SF_RGAIN,
+				  ctrls->gain_red->val, 1,
+				  ctrls->gain_green->val, 1,
+				  ctrls->gain_blue->val, 1);
+	reg = awb ? reg | AALG_WB_EN : reg & ~AALG_WB_EN;
+	s5k5baf_write(state, REG_DBG_AUTOALG_EN, reg);
+}
+
+/* Program FW with exposure time, 'exposure' in us units */
+static void s5k5baf_hw_set_user_exposure(struct s5k5baf *state, int exposure)
+{
+	unsigned int time = exposure / 10;
+
+	s5k5baf_write_seq(state, REG_SF_USR_EXPOSURE_L,
+			  time & 0xffff, time >> 16, 1);
+}
+
+static void s5k5baf_hw_set_user_gain(struct s5k5baf *state, int gain)
+{
+	s5k5baf_write_seq(state, REG_SF_USR_TOT_GAIN, gain, 1);
+}
+
+/* Set auto/manual exposure and total gain */
+static void s5k5baf_hw_set_auto_exposure(struct s5k5baf *state, int value)
+{
+	unsigned int exp_time = state->ctrls.exposure->val;
+	u16 auto_alg;
+
+	auto_alg = s5k5baf_read(state, REG_DBG_AUTOALG_EN);
+
+	if (value == V4L2_EXPOSURE_AUTO) {
+		auto_alg |= AALG_AE_EN | AALG_DIVLEI_EN;
+	} else {
+		s5k5baf_hw_set_user_exposure(state, exp_time);
+		s5k5baf_hw_set_user_gain(state, state->ctrls.gain->val);
+		auto_alg &= ~(AALG_AE_EN | AALG_DIVLEI_EN);
+	}
+
+	s5k5baf_write(state, REG_DBG_AUTOALG_EN, auto_alg);
+}
+
+static void s5k5baf_hw_set_anti_flicker(struct s5k5baf *state, int v)
+{
+	u16 auto_alg;
+
+	auto_alg = s5k5baf_read(state, REG_DBG_AUTOALG_EN);
+
+	if (v == V4L2_CID_POWER_LINE_FREQUENCY_AUTO) {
+		auto_alg |= AALG_FLICKER_EN;
+	} else {
+		auto_alg &= ~AALG_FLICKER_EN;
+		/* The V4L2_CID_LINE_FREQUENCY control values match
+		 * the register values */
+		s5k5baf_write_seq(state, REG_SF_FLICKER_QUANT, v, 1);
+	}
+
+	s5k5baf_write(state, REG_DBG_AUTOALG_EN, auto_alg);
+}
+
+static void s5k5baf_hw_set_colorfx(struct s5k5baf *state, int val)
+{
+	static const u16 colorfx[] = {
+		[V4L2_COLORFX_NONE] = 0,
+		[V4L2_COLORFX_BW] = 1,
+		[V4L2_COLORFX_NEGATIVE] = 2,
+		[V4L2_COLORFX_SEPIA] = 3,
+		[V4L2_COLORFX_SKY_BLUE] = 4,
+		[V4L2_COLORFX_SKETCH] = 5,
+	};
+
+	if (val >= ARRAY_SIZE(colorfx)) {
+		v4l2_err(&state->sd, "colorfx(%d) out of range(%d)\n",
+			 val, ARRAY_SIZE(colorfx));
+		state->error = -EINVAL;
+	} else {
+		s5k5baf_write(state, REG_G_SPEC_EFFECTS, colorfx[val]);
+	}
+}
+
+static int s5k5baf_find_pixfmt(struct v4l2_mbus_framefmt *mf)
+{
+	int i, c = -1;
+
+	for (i = 0; i < ARRAY_SIZE(s5k5baf_formats); i++) {
+		if (mf->colorspace != s5k5baf_formats[i].colorspace)
+			continue;
+		if (mf->code == s5k5baf_formats[i].code)
+			return i;
+		if (c < 0)
+			c = i;
+	}
+	return (c < 0) ? 0 : c;
+}
+
+static void s5k5baf_hw_set_video_bus(struct s5k5baf *state)
+{
+	struct s5k5baf_platform_data *pd = &state->pdata;
+	u16 en_packets;
+
+	switch (pd->bus_type) {
+	case V4L2_MBUS_CSI2:
+		en_packets = EN_PACKETS_CSI2;
+		break;
+	case V4L2_MBUS_PARALLEL:
+		en_packets = 0;
+		break;
+	default:
+		v4l2_err(&state->sd, "unknown video bus: %d\n", pd->bus_type);
+		state->error = -EINVAL;
+		return;
+	};
+
+	s5k5baf_write_seq(state, REG_OIF_EN_MIPI_LANES,
+			  pd->nlanes, en_packets, 1);
+}
+
+static u16 s5k5baf_get_cfg_error(struct s5k5baf *state)
+{
+	u16 err = s5k5baf_read(state, REG_G_PREV_CFG_ERROR);
+	if (err)
+		s5k5baf_write(state, REG_G_PREV_CFG_ERROR, 0);
+	return err;
+}
+
+static void s5k5baf_hw_set_fiv(struct s5k5baf *state, u16 fiv)
+{
+	s5k5baf_write(state, REG_P_MAX_FR_TIME(0), fiv);
+	s5k5baf_hw_sync_cfg(state);
+}
+
+static void s5k5baf_hw_find_min_fiv(struct s5k5baf *state)
+{
+	u16 err, fiv;
+	int n;
+
+	fiv = s5k5baf_read(state,  REG_G_ACTUAL_P_FR_TIME);
+	if (state->error)
+		return;
+
+	for (n = 5; n > 0; --n) {
+		s5k5baf_hw_set_fiv(state, fiv);
+		err = s5k5baf_get_cfg_error(state);
+		if (state->error)
+			return;
+		switch (err) {
+		case CFG_ERROR_RANGE:
+			++fiv;
+			break;
+		case 0:
+			state->fiv = fiv;
+			v4l2_info(&state->sd,
+				  "found valid frame interval: %d00us\n", fiv);
+			return;
+		default:
+			v4l2_err(&state->sd,
+				 "error setting frame interval: %d\n", err);
+			state->error = -EINVAL;
+		}
+	};
+	v4l2_err(&state->sd, "cannot find correct frame interval\n");
+	state->error = -ERANGE;
+}
+
+static void s5k5baf_hw_validate_cfg(struct s5k5baf *state)
+{
+	u16 err;
+
+	err = s5k5baf_get_cfg_error(state);
+	if (state->error)
+		return;
+
+	switch (err) {
+	case 0:
+		state->apply_cfg = 1;
+		return;
+	case CFG_ERROR_RANGE:
+		s5k5baf_hw_find_min_fiv(state);
+		if (!state->error)
+			state->apply_cfg = 1;
+		return;
+	default:
+		v4l2_err(&state->sd,
+			 "error setting format: %d\n", err);
+		state->error = -EINVAL;
+	}
+}
+
+static void s5k5baf_rescale(struct v4l2_rect *r, const struct v4l2_rect *v,
+			    const struct v4l2_rect *n,
+			    const struct v4l2_rect *d)
+{
+	r->left = v->left * n->width / d->width;
+	r->top = v->top * n->height / d->height;
+	r->width = v->width * n->width / d->width;
+	r->height = v->height * n->height / d->height;
+}
+
+static void s5k5baf_hw_set_crop_rects(struct s5k5baf *state)
+{
+	struct v4l2_rect *p, r;
+	u16 err;
+
+	p = &state->crop_sink;
+	s5k5baf_write_seq(state, REG_G_PREVREQ_IN_WIDTH, p->width, p->height,
+			  p->left, p->top);
+
+	s5k5baf_rescale(&r, &state->crop_source, &state->crop_sink,
+			&state->compose);
+	s5k5baf_write_seq(state, REG_G_PREVZOOM_IN_WIDTH, r.width, r.height,
+			  r.left, r.top);
+
+	s5k5baf_synchronize(state, 500, REG_G_INPUTS_CHANGE_REQ);
+	s5k5baf_synchronize(state, 500, REG_G_PREV_CFG_BYPASS_CHANGED);
+	err = s5k5baf_get_cfg_error(state);
+	if (state->error)
+		return;
+
+	switch (err) {
+	case 0:
+		break;
+	case CFG_ERROR_RANGE:
+		/* retry crop with frame interval set to max */
+		s5k5baf_hw_set_fiv(state, S5K5BAF_MAX_FR_TIME);
+		err = s5k5baf_get_cfg_error(state);
+		if (state->error)
+			return;
+		if (err) {
+			v4l2_err(&state->sd,
+				 "crop error on max frame interval: %d\n", err);
+			state->error = -EINVAL;
+		}
+		s5k5baf_hw_set_fiv(state, state->req_fiv);
+		s5k5baf_hw_validate_cfg(state);
+		break;
+	default:
+		v4l2_err(&state->sd, "crop error: %d\n", err);
+		state->error = -EINVAL;
+		return;
+	}
+
+	if (!state->apply_cfg)
+		return;
+
+	p = &state->crop_source;
+	s5k5baf_write_seq(state, REG_P_OUT_WIDTH(0), p->width, p->height);
+	s5k5baf_hw_set_fiv(state, state->req_fiv);
+	s5k5baf_hw_validate_cfg(state);
+}
+
+static void s5k5baf_hw_set_config(struct s5k5baf *state)
+{
+	u16 reg_fmt = s5k5baf_formats[state->pixfmt].reg_p_fmt;
+	struct v4l2_rect *r = &state->crop_source;
+
+	s5k5baf_write_seq(state, REG_P_OUT_WIDTH(0),
+			  r->width, r->height, reg_fmt,
+			  PCLK_MAX_FREQ >> 2, PCLK_MIN_FREQ >> 2,
+			  PVI_MASK_MIPI, CLK_MIPI_INDEX,
+			  FR_RATE_FIXED, FR_RATE_Q_DYNAMIC,
+			  state->req_fiv, S5K5BAF_MIN_FR_TIME);
+	s5k5baf_hw_sync_cfg(state);
+	s5k5baf_hw_validate_cfg(state);
+}
+
+
+static void s5k5baf_hw_set_test_pattern(struct s5k5baf *state, int id)
+{
+	s5k5baf_i2c_write(state, REG_PATTERN_WIDTH, 800);
+	s5k5baf_i2c_write(state, REG_PATTERN_HEIGHT, 511);
+	s5k5baf_i2c_write(state, REG_PATTERN_PARAM, 0);
+	s5k5baf_i2c_write(state, REG_PATTERN_SET, id);
+}
+
+static void s5k5baf_gpio_assert(struct s5k5baf *state, int id)
+{
+	struct s5k5baf_gpio *gpio = &state->pdata.gpios[id];
+
+	if (state->error)
+		return;
+	gpio_set_value(gpio->gpio, gpio->level);
+}
+
+static void s5k5baf_gpio_deassert(struct s5k5baf *state, int id)
+{
+	struct s5k5baf_gpio *gpio = &state->pdata.gpios[id];
+
+	if (state->error)
+		return;
+	gpio_set_value(gpio->gpio, !gpio->level);
+}
+
+static void s5k5baf_power_on(struct s5k5baf *state)
+{
+	state->error = regulator_bulk_enable(S5K5BAF_NUM_SUPPLIES,
+					       state->supplies);
+	if (state->error)
+		return;
+
+	s5k5baf_gpio_deassert(state, STBY);
+	usleep_range(50, 100);
+	s5k5baf_gpio_deassert(state, RST);
+}
+
+static void s5k5baf_power_off(struct s5k5baf *state)
+{
+	state->streaming = 0;
+	state->apply_cfg = 0;
+	state->apply_crop = 0;
+	s5k5baf_gpio_assert(state, RST);
+	s5k5baf_gpio_assert(state, STBY);
+	state->error = regulator_bulk_disable(S5K5BAF_NUM_SUPPLIES,
+						state->supplies);
+}
+
+static void s5k5baf_hw_init(struct s5k5baf *state)
+{
+	s5k5baf_i2c_write(state, AHB_MSB_ADDR_PTR, PAGE_IF_HW);
+	s5k5baf_i2c_write(state, REG_CLEAR_HOST_INT, 0);
+	s5k5baf_i2c_write(state, REG_SW_LOAD_COMPLETE, 1);
+	s5k5baf_i2c_write(state, REG_CMDRD_PAGE, PAGE_IF_SW);
+	s5k5baf_i2c_write(state, REG_CMDWR_PAGE, PAGE_IF_SW);
+}
+
+/*
+ * V4L2 subdev core and video operations
+ */
+
+static void s5k5baf_initialize_data(struct s5k5baf *state)
+{
+	state->crop_sink = s5k5baf_cis_rect;
+	state->compose = s5k5baf_def_rect;
+	state->crop_source = state->compose;
+	state->pixfmt = 0;
+	state->req_fiv = 10000 / 15;
+	state->fiv = state->req_fiv;
+}
+
+static int s5k5baf_set_power(struct v4l2_subdev *sd, int on)
+{
+	struct s5k5baf *state = to_s5k5baf(sd);
+	int ret;
+
+	mutex_lock(&state->lock);
+
+	if (!on == state->power) {
+		if (on) {
+			s5k5baf_initialize_data(state);
+			s5k5baf_power_on(state);
+			s5k5baf_hw_init(state);
+			s5k5baf_hw_patch(state);
+			s5k5baf_i2c_write(state, REG_SET_HOST_INT, 1);
+			s5k5baf_hw_set_clocks(state);
+			s5k5baf_hw_set_video_bus(state);
+			s5k5baf_hw_set_cis(state);
+			s5k5baf_hw_set_ccm(state);
+		} else {
+			s5k5baf_power_off(state);
+		}
+
+		if (!state->error)
+			state->power += on ? 1 : -1;
+	}
+
+	ret = state->error;
+	state->error = 0;
+	mutex_unlock(&state->lock);
+
+	if (!ret && on && state->power == 1)
+		ret = v4l2_ctrl_handler_setup(&state->ctrls.handler);
+
+	return ret;
+}
+
+static void s5k5baf_hw_set_stream(struct s5k5baf *state, int enable)
+{
+	s5k5baf_write_seq(state, REG_G_ENABLE_PREV, enable, 1);
+}
+
+static int s5k5baf_s_stream(struct v4l2_subdev *sd, int on)
+{
+	struct s5k5baf *state = to_s5k5baf(sd);
+	int ret;
+
+	if (state->streaming == !!on)
+		return 0;
+
+	mutex_lock(&state->lock);
+
+	if (!on) {
+		s5k5baf_hw_set_stream(state, 0);
+		goto out;
+	}
+
+	s5k5baf_hw_set_config(state);
+	s5k5baf_hw_set_stream(state, 1);
+	s5k5baf_i2c_write(state, 0xb0cc, 0x000b);
+
+	if (!state->error)
+		state->streaming = 1;
+
+out:
+	ret = state->error;
+	state->error = 0;
+	mutex_unlock(&state->lock);
+
+	return ret;
+}
+
+static int s5k5baf_g_frame_interval(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_frame_interval *fi)
+{
+	struct s5k5baf *state = to_s5k5baf(sd);
+
+	mutex_lock(&state->lock);
+	fi->interval.numerator = state->fiv;
+	fi->interval.denominator = 10000;
+	mutex_unlock(&state->lock);
+
+	return 0;
+}
+
+static void s5k5baf_set_frame_interval(struct s5k5baf *state,
+				       struct v4l2_subdev_frame_interval *fi)
+{
+	struct v4l2_fract *i = &fi->interval;
+
+	if (fi->interval.denominator == 0)
+		state->req_fiv = S5K5BAF_MAX_FR_TIME;
+	else
+		state->req_fiv = clamp_t(u32,
+					 i->numerator * 10000 / i->denominator,
+					 S5K5BAF_MIN_FR_TIME,
+					 S5K5BAF_MAX_FR_TIME);
+
+	state->fiv = state->req_fiv;
+	if (state->apply_cfg) {
+		s5k5baf_hw_set_fiv(state, state->req_fiv);
+		s5k5baf_hw_validate_cfg(state);
+	}
+	*i = (struct v4l2_fract){state->fiv, 10000};
+	if (state->fiv == state->req_fiv)
+		v4l2_info(&state->sd, "frame interval changed to %d00us\n",
+			  state->fiv);
+}
+
+static int s5k5baf_s_frame_interval(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_frame_interval *fi)
+{
+	struct s5k5baf *state = to_s5k5baf(sd);
+
+	mutex_lock(&state->lock);
+	s5k5baf_set_frame_interval(state, fi);
+	mutex_unlock(&state->lock);
+	return 0;
+}
+
+/*
+ * V4L2 subdev pad level and video operations
+ */
+static int s5k5baf_enum_frame_interval(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_fh *fh,
+			      struct v4l2_subdev_frame_interval_enum *fie)
+{
+	if (fie->index > S5K5BAF_MAX_FR_TIME - S5K5BAF_MIN_FR_TIME ||
+	    fie->pad != 0)
+		return -EINVAL;
+
+	v4l_bound_align_image(&fie->width, S5K5BAF_WIN_WIDTH_MIN,
+			      S5K5BAF_CIS_WIDTH, 1,
+			      &fie->height, S5K5BAF_WIN_HEIGHT_MIN,
+			      S5K5BAF_CIS_HEIGHT, 1, 0);
+
+	fie->interval.numerator = S5K5BAF_MIN_FR_TIME + fie->index;
+	fie->interval.denominator = 10000;
+
+	return 0;
+}
+
+static int s5k5baf_enum_mbus_code(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_fh *fh,
+				 struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->pad == 0) {
+		if (code->index > 0)
+			return -EINVAL;
+		code->code = V4L2_MBUS_FMT_FIXED;
+		return 0;
+	}
+
+	if (code->index >= ARRAY_SIZE(s5k5baf_formats))
+		return -EINVAL;
+
+	code->code = s5k5baf_formats[code->index].code;
+	return 0;
+}
+
+static int s5k5baf_enum_frame_size(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_fh *fh,
+				  struct v4l2_subdev_frame_size_enum *fse)
+{
+	int i;
+
+	if (fse->index > 0)
+		return -EINVAL;
+
+	if (fse->pad == 0) {
+		fse->code = V4L2_MBUS_FMT_FIXED;
+		fse->min_width = S5K5BAF_CIS_WIDTH;
+		fse->max_width = S5K5BAF_CIS_WIDTH;
+		fse->min_height = S5K5BAF_CIS_HEIGHT;
+		fse->max_height = S5K5BAF_CIS_HEIGHT;
+		return 0;
+	}
+
+	i = ARRAY_SIZE(s5k5baf_formats);
+	while (--i)
+		if (fse->code == s5k5baf_formats[i].code)
+			break;
+	fse->code = s5k5baf_formats[i].code;
+	fse->min_width = S5K5BAF_WIN_WIDTH_MIN;
+	fse->max_width = S5K5BAF_CIS_WIDTH;
+	fse->max_height = S5K5BAF_WIN_HEIGHT_MIN;
+	fse->min_height = S5K5BAF_CIS_HEIGHT;
+
+	return 0;
+}
+
+static void s5k5baf_try_cis_format(struct v4l2_mbus_framefmt *mf)
+{
+	mf->width = S5K5BAF_CIS_WIDTH;
+	mf->height = S5K5BAF_CIS_HEIGHT;
+	mf->code = V4L2_MBUS_FMT_FIXED;
+	mf->colorspace = V4L2_COLORSPACE_JPEG;
+	mf->field = V4L2_FIELD_NONE;
+}
+
+static int s5k5baf_try_isp_format(struct v4l2_mbus_framefmt *mf)
+{
+	int pixfmt;
+
+	v4l_bound_align_image(&mf->width, S5K5BAF_WIN_WIDTH_MIN,
+			      S5K5BAF_CIS_WIDTH, 1,
+			      &mf->height, S5K5BAF_WIN_HEIGHT_MIN,
+			      S5K5BAF_CIS_HEIGHT, 1, 0);
+
+	pixfmt = s5k5baf_find_pixfmt(mf);
+
+	mf->colorspace = s5k5baf_formats[pixfmt].colorspace;
+	mf->code = s5k5baf_formats[pixfmt].code;
+	mf->field = V4L2_FIELD_NONE;
+
+	return pixfmt;
+}
+
+static int s5k5baf_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
+			  struct v4l2_subdev_format *fmt)
+{
+	struct s5k5baf *state = to_s5k5baf(sd);
+	const struct s5k5baf_pixfmt *pixfmt;
+	struct v4l2_mbus_framefmt *mf;
+
+	memset(fmt->reserved, 0, sizeof(fmt->reserved));
+
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+		mf = v4l2_subdev_get_try_format(fh, fmt->pad);
+		fmt->format = *mf;
+		return 0;
+	}
+
+	mf = &fmt->format;
+	if (fmt->pad == 0) {
+		s5k5baf_try_cis_format(mf);
+		return 0;
+	}
+	mf->field = V4L2_FIELD_NONE;
+	mutex_lock(&state->lock);
+	pixfmt = &s5k5baf_formats[state->pixfmt];
+	mf->width = state->crop_source.width;
+	mf->height = state->crop_source.height;
+	mf->code = pixfmt->code;
+	mf->colorspace = pixfmt->colorspace;
+	mutex_unlock(&state->lock);
+
+	return 0;
+}
+
+static int s5k5baf_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
+			  struct v4l2_subdev_format *fmt)
+{
+	struct v4l2_mbus_framefmt *mf = &fmt->format;
+	struct s5k5baf *state = to_s5k5baf(sd);
+	const struct s5k5baf_pixfmt *pixfmt;
+	int ret = 0;
+
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+		*v4l2_subdev_get_try_format(fh, fmt->pad) = *mf;
+		return 0;
+	}
+
+	if (fmt->pad == 0) {
+		s5k5baf_try_cis_format(mf);
+		return 0;
+	}
+
+	mutex_lock(&state->lock);
+
+	if (state->streaming) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	state->pixfmt = s5k5baf_try_isp_format(mf);
+	pixfmt = &s5k5baf_formats[state->pixfmt];
+	mf->code = pixfmt->code;
+	mf->colorspace = pixfmt->colorspace;
+	mf->width = state->crop_source.width;
+	mf->height = state->crop_source.height;
+
+out:
+	mutex_unlock(&state->lock);
+
+	return ret;
+}
+
+enum selection_rect {R_CIS, R_CROP_SINK, R_COMPOSE, R_CROP_SOURCE, R_INVALID};
+
+static enum selection_rect s5k5baf_get_sel_rect(u32 pad, u32 target)
+{
+	switch (target) {
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		return pad ? R_COMPOSE : R_CIS;
+	case V4L2_SEL_TGT_CROP:
+		return pad ? R_CROP_SOURCE : R_CROP_SINK;
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		return pad ? R_INVALID : R_CROP_SINK;
+	case V4L2_SEL_TGT_COMPOSE:
+		return pad ? R_INVALID : R_COMPOSE;
+	default:
+		return R_INVALID;
+	}
+}
+
+static int s5k5baf_is_bound_tgt(u32 target)
+{
+	return (target == V4L2_SEL_TGT_CROP_BOUNDS ||
+		target == V4L2_SEL_TGT_COMPOSE_BOUNDS);
+}
+
+static int s5k5baf_get_selection(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_fh *fh,
+				 struct v4l2_subdev_selection *sel)
+{
+	static enum selection_rect rtype;
+	struct s5k5baf *state = to_s5k5baf(sd);
+
+	rtype = s5k5baf_get_sel_rect(sel->pad, sel->target);
+
+	switch (rtype) {
+	case R_INVALID:
+		return -EINVAL;
+	case R_CIS:
+		sel->r = s5k5baf_cis_rect;
+		return 0;
+	default:
+		break;
+	}
+
+	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
+		if (rtype == R_COMPOSE)
+			sel->r = *v4l2_subdev_get_try_compose(fh, sel->pad);
+		else
+			sel->r = *v4l2_subdev_get_try_crop(fh, sel->pad);
+		return 0;
+	}
+
+	mutex_lock(&state->lock);
+	switch (rtype) {
+	case R_CROP_SINK:
+		sel->r = state->crop_sink;
+		break;
+	case R_COMPOSE:
+		sel->r = state->compose;
+		break;
+	case R_CROP_SOURCE:
+		sel->r = state->crop_source;
+		break;
+	default:
+		break;
+	}
+	if (s5k5baf_is_bound_tgt(sel->target)) {
+		sel->r.left = 0;
+		sel->r.top = 0;
+	}
+	mutex_unlock(&state->lock);
+
+	return 0;
+}
+
+/* bounds range [start, start+len) to [0, max) and aligns to 2 */
+static void s5k5baf_bound_range(u32 *start, u32 *len, u32 max)
+{
+	if (*len > max)
+		*len = max;
+	if (*start + *len > max)
+		*start = max - *len;
+	*start &= ~1;
+	*len &= ~1;
+	if (*len < S5K5BAF_WIN_WIDTH_MIN)
+		*len = S5K5BAF_WIN_WIDTH_MIN;
+}
+
+static void s5k5baf_bound_rect(struct v4l2_rect *r, u32 width, u32 height)
+{
+	s5k5baf_bound_range(&r->left, &r->width, width);
+	s5k5baf_bound_range(&r->top, &r->height, height);
+}
+
+static void s5k5baf_set_rect_and_adjust(struct v4l2_rect **rects,
+					enum selection_rect first,
+					struct v4l2_rect *v)
+{
+	struct v4l2_rect *r, *br;
+	enum selection_rect i = first;
+
+	*rects[first] = *v;
+	do {
+		r = rects[i];
+		br = rects[i - 1];
+		s5k5baf_bound_rect(r, br->width, br->height);
+	} while (++i != R_INVALID);
+	*v = *rects[first];
+}
+
+static bool s5k5baf_cmp_rect(const struct v4l2_rect *r1,
+			     const struct v4l2_rect *r2)
+{
+	return !memcmp(r1, r2, sizeof(*r1));
+}
+
+static int s5k5baf_set_selection(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_fh *fh,
+				 struct v4l2_subdev_selection *sel)
+{
+	static enum selection_rect rtype;
+	struct s5k5baf *state = to_s5k5baf(sd);
+	struct v4l2_rect **rects;
+
+	rtype = s5k5baf_get_sel_rect(sel->pad, sel->target);
+	if (rtype == R_INVALID || s5k5baf_is_bound_tgt(sel->target))
+		return -EINVAL;
+
+	/* allow only scaling on compose */
+	if (rtype == R_COMPOSE) {
+		sel->r.left = 0;
+		sel->r.top = 0;
+	}
+
+	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
+		rects = (struct v4l2_rect * []) {
+				&s5k5baf_cis_rect,
+				v4l2_subdev_get_try_crop(fh, 0),
+				v4l2_subdev_get_try_compose(fh, 0),
+				v4l2_subdev_get_try_crop(fh, 1)
+			};
+		s5k5baf_set_rect_and_adjust(rects, rtype, &sel->r);
+		return 0;
+	}
+
+	rects = (struct v4l2_rect * []) {
+			&s5k5baf_cis_rect,
+			&state->crop_sink,
+			&state->compose,
+			&state->crop_source
+		};
+	mutex_lock(&state->lock);
+	if (state->streaming) {
+		/* adjust sel->r to avoid output resolution change */
+		if (rtype < R_CROP_SOURCE) {
+			if (sel->r.width < state->crop_source.width)
+				sel->r.width = state->crop_source.width;
+			if (sel->r.height < state->crop_source.height)
+				sel->r.height = state->crop_source.height;
+		} else {
+			sel->r.width = state->crop_source.width;
+			sel->r.height = state->crop_source.height;
+		}
+	}
+	s5k5baf_set_rect_and_adjust(rects, rtype, &sel->r);
+	if (!s5k5baf_cmp_rect(&state->crop_sink, &s5k5baf_cis_rect) ||
+	    !s5k5baf_cmp_rect(&state->compose, &s5k5baf_def_rect))
+		state->apply_crop = 1;
+	s5k5baf_hw_set_crop_rects(state);
+	mutex_unlock(&state->lock);
+
+	return state->error;
+}
+
+static const struct v4l2_subdev_pad_ops s5k5baf_cis_pad_ops = {
+	.enum_mbus_code		= s5k5baf_enum_mbus_code,
+	.enum_frame_size	= s5k5baf_enum_frame_size,
+	.get_fmt		= s5k5baf_get_fmt,
+	.set_fmt		= s5k5baf_set_fmt,
+};
+
+static const struct v4l2_subdev_pad_ops s5k5baf_pad_ops = {
+	.enum_mbus_code		= s5k5baf_enum_mbus_code,
+	.enum_frame_size	= s5k5baf_enum_frame_size,
+	.enum_frame_interval	= s5k5baf_enum_frame_interval,
+	.get_fmt		= s5k5baf_get_fmt,
+	.set_fmt		= s5k5baf_set_fmt,
+	.get_selection		= s5k5baf_get_selection,
+	.set_selection		= s5k5baf_set_selection,
+};
+
+static const struct v4l2_subdev_video_ops s5k5baf_video_ops = {
+	.g_frame_interval	= s5k5baf_g_frame_interval,
+	.s_frame_interval	= s5k5baf_s_frame_interval,
+	.s_stream		= s5k5baf_s_stream,
+};
+
+/*
+ * V4L2 subdev controls
+ */
+
+static int s5k5baf_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
+	struct s5k5baf *state = to_s5k5baf(sd);
+	int ret;
+
+	v4l2_dbg(1, debug, sd, "ctrl: %s, value: %d\n", ctrl->name, ctrl->val);
+
+	mutex_lock(&state->lock);
+
+	if (state->power == 0)
+		goto unlock;
+
+	switch (ctrl->id) {
+	case V4L2_CID_AUTO_WHITE_BALANCE:
+		s5k5baf_hw_set_awb(state, ctrl->val);
+		break;
+
+	case V4L2_CID_BRIGHTNESS:
+		s5k5baf_write(state, REG_USER_BRIGHTNESS, ctrl->val);
+		break;
+
+	case V4L2_CID_COLORFX:
+		s5k5baf_hw_set_colorfx(state, ctrl->val);
+		break;
+
+	case V4L2_CID_CONTRAST:
+		s5k5baf_write(state, REG_USER_CONTRAST, ctrl->val);
+		break;
+
+	case V4L2_CID_EXPOSURE_AUTO:
+		s5k5baf_hw_set_auto_exposure(state, ctrl->val);
+		break;
+
+	case V4L2_CID_HFLIP:
+		s5k5baf_hw_set_mirror(state, ctrl->val);
+		break;
+
+	case V4L2_CID_POWER_LINE_FREQUENCY:
+		s5k5baf_hw_set_anti_flicker(state, ctrl->val);
+		break;
+
+	case V4L2_CID_SATURATION:
+		s5k5baf_write(state, REG_USER_SATURATION, ctrl->val);
+		break;
+
+	case V4L2_CID_SHARPNESS:
+		s5k5baf_write(state, REG_USER_SHARPBLUR, ctrl->val);
+		break;
+
+	case V4L2_CID_WHITE_BALANCE_TEMPERATURE:
+		s5k5baf_write(state, REG_P_COLORTEMP(0), ctrl->val);
+		if (state->apply_cfg)
+			s5k5baf_hw_sync_cfg(state);
+		break;
+
+	case V4L2_CID_TEST_PATTERN:
+		s5k5baf_hw_set_test_pattern(state, ctrl->val);
+		break;
+	}
+unlock:
+	ret = state->error;
+	state->error = 0;
+	mutex_unlock(&state->lock);
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops s5k5baf_ctrl_ops = {
+	.s_ctrl	= s5k5baf_s_ctrl,
+};
+
+static int s5k5baf_log_status(struct v4l2_subdev *sd)
+{
+	v4l2_ctrl_handler_log_status(sd->ctrl_handler, sd->name);
+	return 0;
+}
+
+#define V4L2_CID_RED_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1001)
+#define V4L2_CID_GREEN_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1002)
+#define V4L2_CID_BLUE_GAIN	(V4L2_CTRL_CLASS_CAMERA | 0x1003)
+
+static const struct v4l2_ctrl_config s5k5baf_ctrls[] = {
+	{
+		.ops	= &s5k5baf_ctrl_ops,
+		.id	= V4L2_CID_RED_GAIN,
+		.type	= V4L2_CTRL_TYPE_INTEGER,
+		.name	= "Gain, Red",
+		.min	= 0,
+		.max	= 256,
+		.def	= 127,
+		.step	= 1,
+	}, {
+		.ops	= &s5k5baf_ctrl_ops,
+		.id	= V4L2_CID_GREEN_GAIN,
+		.type	= V4L2_CTRL_TYPE_INTEGER,
+		.name	= "Gain, Green",
+		.min	= 0,
+		.max	= 256,
+		.def	= 127,
+		.step	= 1,
+	}, {
+		.ops	= &s5k5baf_ctrl_ops,
+		.id	= V4L2_CID_BLUE_GAIN,
+		.type	= V4L2_CTRL_TYPE_INTEGER,
+		.name	= "Gain, Blue",
+		.min	= 0,
+		.max	= 256,
+		.def	= 127,
+		.step	= 1,
+	},
+};
+
+static const char * const s5k5baf_test_pattern_menu[] = {
+	"Disabled",
+	"Blank",
+	"Bars",
+	"Gradients",
+	"Textile",
+	"Textile2",
+	"Squares"
+};
+
+static void s5k5baf_initialize_ctrls(struct s5k5baf *state)
+{
+	const struct v4l2_ctrl_ops *ops = &s5k5baf_ctrl_ops;
+	struct s5k5baf_ctrls *ctrls = &state->ctrls;
+	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
+	int ret;
+
+	if (state->error)
+		return;
+
+	ret = v4l2_ctrl_handler_init(hdl, 16);
+	if (ret) {
+		v4l2_err(&state->sd, "cannot init ctrl handler (%d)\n", ret);
+		state->error = ret;
+		return;
+	}
+
+	/* Auto white balance cluster */
+	ctrls->awb = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_AUTO_WHITE_BALANCE,
+				       0, 1, 1, 1);
+	ctrls->gain_red = v4l2_ctrl_new_custom(hdl, &s5k5baf_ctrls[0], NULL);
+	ctrls->gain_green = v4l2_ctrl_new_custom(hdl, &s5k5baf_ctrls[1], NULL);
+	ctrls->gain_blue = v4l2_ctrl_new_custom(hdl, &s5k5baf_ctrls[2], NULL);
+	v4l2_ctrl_auto_cluster(4, &ctrls->awb, 0, false);
+
+	ctrls->hflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
+	ctrls->vflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_cluster(2, &ctrls->hflip);
+
+	ctrls->auto_exp = v4l2_ctrl_new_std_menu(hdl, ops,
+				V4L2_CID_EXPOSURE_AUTO,
+				V4L2_EXPOSURE_MANUAL, 0, V4L2_EXPOSURE_AUTO);
+	/* Exposure time: x 1 us */
+	ctrls->exposure = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE,
+					    0, 6000000U, 1, 100000U);
+	/* Total gain: 256 <=> 1x */
+	ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAIN,
+					0, 256, 1, 256);
+	v4l2_ctrl_auto_cluster(3, &ctrls->auto_exp, 0, false);
+
+	v4l2_ctrl_new_std_menu(hdl, ops, V4L2_CID_POWER_LINE_FREQUENCY,
+			       V4L2_CID_POWER_LINE_FREQUENCY_AUTO, 0,
+			       V4L2_CID_POWER_LINE_FREQUENCY_AUTO);
+
+	v4l2_ctrl_new_std_menu(hdl, ops, V4L2_CID_COLORFX,
+			       V4L2_COLORFX_SKY_BLUE, ~0x6f, V4L2_COLORFX_NONE);
+
+	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_WHITE_BALANCE_TEMPERATURE,
+			  0, 256, 1, 0);
+
+	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SATURATION, -127, 127, 1, 0);
+	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_BRIGHTNESS, -127, 127, 1, 0);
+	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_CONTRAST, -127, 127, 1, 0);
+	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SHARPNESS, -127, 127, 1, 0);
+
+	v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(s5k5baf_test_pattern_menu) - 1,
+				     0, 0, s5k5baf_test_pattern_menu);
+
+	if (hdl->error) {
+		v4l2_err(&state->sd, "error creating controls (%d)\n",
+			 hdl->error);
+		state->error = hdl->error;
+		v4l2_ctrl_handler_free(hdl);
+		return;
+	}
+
+	state->sd.ctrl_handler = hdl;
+}
+
+/*
+ * V4L2 subdev internal operations
+ */
+static int s5k5baf_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	struct v4l2_mbus_framefmt *mf;
+	struct v4l2_rect *r;
+
+	mf = v4l2_subdev_get_try_format(fh, 0);
+	s5k5baf_try_cis_format(mf);
+
+	if (s5k5baf_is_cis_subdev(sd))
+		return 0;
+
+	mf = v4l2_subdev_get_try_format(fh, 1);
+	mf->colorspace = s5k5baf_formats[0].colorspace;
+	mf->code = s5k5baf_formats[0].code;
+	mf->width = s5k5baf_def_rect.width;
+	mf->height = s5k5baf_def_rect.height;
+	mf->field = V4L2_FIELD_NONE;
+
+	*v4l2_subdev_get_try_crop(fh, 0) = s5k5baf_cis_rect;
+	r = v4l2_subdev_get_try_compose(fh, 0);
+	*r = s5k5baf_def_rect;
+	*v4l2_subdev_get_try_crop(fh, 1) = *r;
+
+	return 0;
+}
+
+static void s5k5baf_check_fw_revision(struct s5k5baf *state)
+{
+	u16 api_ver = 0, fw_rev = 0, s_id = 0;
+
+	api_ver = s5k5baf_read(state, REG_FW_APIVER);
+	fw_rev = s5k5baf_read(state, REG_FW_REVISION) & 0xff;
+	s_id = s5k5baf_read(state, REG_FW_SENSOR_ID);
+	if (state->error)
+		return;
+
+	v4l2_info(&state->sd, "FW API=%#x, revision=%#x sensor_id=%#x\n",
+		  api_ver, fw_rev, s_id);
+
+	if (api_ver == S5K5BAF_FW_APIVER)
+		return;
+
+	v4l2_err(&state->sd, "FW API version not supported\n");
+	state->error = -ENODEV;
+}
+
+static int s5k5baf_registered(struct v4l2_subdev *sd)
+{
+	struct s5k5baf *state = to_s5k5baf(sd);
+	int ret;
+
+	ret = v4l2_device_register_subdev(sd->v4l2_dev, &state->cis_sd);
+	if (ret) {
+		v4l2_err(sd, "failed to register subdev %s\n",
+			 state->cis_sd.name);
+		return ret;
+	}
+
+	ret = media_entity_create_link(&state->cis_sd.entity,
+			0, &state->sd.entity, 0,
+			MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+
+	mutex_lock(&state->lock);
+
+	s5k5baf_power_on(state);
+	s5k5baf_hw_init(state);
+	s5k5baf_check_fw_revision(state);
+	s5k5baf_power_off(state);
+
+	ret = state->error;
+	state->error = 0;
+
+	mutex_unlock(&state->lock);
+
+	return ret;
+}
+
+static const struct v4l2_subdev_ops s5k5baf_cis_subdev_ops = {
+	.pad	= &s5k5baf_cis_pad_ops,
+};
+
+static const struct v4l2_subdev_internal_ops s5k5baf_cis_subdev_internal_ops = {
+	.open = s5k5baf_open,
+};
+
+static const struct v4l2_subdev_internal_ops s5k5baf_subdev_internal_ops = {
+	.registered = s5k5baf_registered,
+	.open = s5k5baf_open,
+};
+
+static const struct v4l2_subdev_core_ops s5k5baf_core_ops = {
+	.s_power = s5k5baf_set_power,
+	.log_status = s5k5baf_log_status,
+};
+
+static const struct v4l2_subdev_ops s5k5baf_subdev_ops = {
+	.core = &s5k5baf_core_ops,
+	.pad = &s5k5baf_pad_ops,
+	.video = &s5k5baf_video_ops,
+};
+
+static void s5k5baf_configure_gpios(struct s5k5baf *state)
+{
+	static const char const *name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
+	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
+	struct s5k5baf_gpio *g = state->pdata.gpios;
+	int ret, i;
+
+	if (state->error)
+		return;
+
+	for (i = 0; i < GPIO_NUM; ++i) {
+		int flags = GPIOF_EXPORT | GPIOF_DIR_OUT;
+		if (g[i].level)
+			flags |= GPIOF_INIT_HIGH;
+		ret = devm_gpio_request_one(&c->dev, g[i].gpio, flags, name[i]);
+		if (ret) {
+			v4l2_err(c, "failed to request gpio %s\n", name[i]);
+			state->error = ret;
+			return;
+		}
+	}
+	return;
+}
+
+static void s5k5baf_get_platform_data(struct s5k5baf *state, struct device *dev)
+{
+	struct device_node *node = dev->of_node;
+	struct s5k5baf_platform_data *pd;
+	struct device_node *node_ep;
+	struct v4l2_of_endpoint ep;
+	enum of_gpio_flags of_flags;
+
+	if (state->error)
+		return;
+
+	if (!node) {
+		pd = dev->platform_data;
+		if (!pd) {
+			dev_err(dev, "No platform data\n");
+			state->error = -EINVAL;
+		}
+		state->pdata = *pd;
+		return;
+	}
+
+	pd = &state->pdata;
+
+	of_property_read_u32(node, "clock-frequency", &pd->mclk_frequency);
+	pd->hflip = of_property_read_bool(node, "samsung,hflip");
+	pd->vflip = of_property_read_bool(node, "samsung,vflip");
+	pd->gpios[STBY].gpio = of_get_gpio_flags(node, STBY, &of_flags);
+	pd->gpios[STBY].level = !(of_flags & OF_GPIO_ACTIVE_LOW);
+	pd->gpios[RST].gpio = of_get_gpio_flags(node, RST, &of_flags);
+	pd->gpios[RST].level = !(of_flags & OF_GPIO_ACTIVE_LOW);
+
+	node_ep = v4l2_of_get_next_endpoint(node, NULL);
+	if (!node_ep) {
+		dev_err(dev, "no endpoint defined\n");
+		state->error = -EINVAL;
+		return;
+	}
+	v4l2_of_parse_endpoint(node_ep, &ep);
+	of_node_put(node_ep);
+	pd->bus_type = ep.bus_type;
+	if (pd->bus_type == V4L2_MBUS_CSI2)
+		pd->nlanes = ep.bus.mipi_csi2.num_data_lanes;
+}
+
+static void s5k5baf_configure_subdevs(struct s5k5baf *state,
+				     struct i2c_client *c)
+{
+	struct v4l2_subdev *sd;
+
+	if (state->error)
+		return;
+
+	sd = &state->cis_sd;
+	v4l2_subdev_init(sd, &s5k5baf_cis_subdev_ops);
+	sd->owner = c->driver->driver.owner;
+	v4l2_set_subdevdata(sd, state);
+	strlcpy(sd->name, "S5K5BAF-CIS", sizeof(sd->name));
+
+	sd->internal_ops = &s5k5baf_cis_subdev_internal_ops;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+	state->cis_pad.flags = MEDIA_PAD_FL_SOURCE;
+	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
+	state->error = media_entity_init(&sd->entity, 1, &state->cis_pad, 0);
+	if (state->error)
+		goto err;
+
+	sd = &state->sd;
+	v4l2_i2c_subdev_init(sd, c, &s5k5baf_subdev_ops);
+	strlcpy(sd->name, "S5K5BAF-ISP", sizeof(sd->name));
+
+	sd->internal_ops = &s5k5baf_subdev_internal_ops;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+	state->pads[0].flags = MEDIA_PAD_FL_SINK;
+	state->pads[1].flags = MEDIA_PAD_FL_SOURCE;
+	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
+	state->error = media_entity_init(&sd->entity, 2, state->pads, 0);
+	if (!state->error)
+		return;
+
+	media_entity_cleanup(&state->cis_sd.entity);
+err:
+	dev_err(&c->dev, "cannot init media entity %s\n", sd->name);
+}
+
+static void s5k5baf_configure_regulators(struct s5k5baf *state)
+{
+	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
+	int i;
+
+	if (state->error)
+		return;
+
+	for (i = 0; i < S5K5BAF_NUM_SUPPLIES; i++)
+		state->supplies[i].supply = s5k5baf_supply_names[i];
+
+	state->error = devm_regulator_bulk_get(&c->dev, S5K5BAF_NUM_SUPPLIES,
+				      state->supplies);
+	if (state->error)
+		v4l2_err(c, "failed to get regulators\n");
+}
+
+static int s5k5baf_probe(struct i2c_client *c,
+			const struct i2c_device_id *id)
+{
+	struct s5k5baf *state;
+
+	state = devm_kzalloc(&c->dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	s5k5baf_get_platform_data(state, &c->dev);
+	s5k5baf_configure_subdevs(state, c);
+	s5k5baf_configure_gpios(state);
+	s5k5baf_configure_regulators(state);
+	s5k5baf_initialize_ctrls(state);
+
+	mutex_init(&state->lock);
+
+	return state->error;
+}
+
+static int s5k5baf_remove(struct i2c_client *c)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(c);
+	struct s5k5baf *state = to_s5k5baf(sd);
+
+	v4l2_device_unregister_subdev(sd);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
+	media_entity_cleanup(&sd->entity);
+
+	sd = &state->cis_sd;
+	v4l2_device_unregister_subdev(sd);
+	media_entity_cleanup(&sd->entity);
+
+	return 0;
+}
+
+static const struct i2c_device_id s5k5baf_id[] = {
+	{ DRIVER_NAME, 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, s5k5baf_id);
+
+static const struct of_device_id s5k5baf_of_match[] = {
+	{ .compatible = "samsung," DRIVER_NAME },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, s5k5baf_of_match);
+
+static struct i2c_driver s5k5baf_i2c_driver = {
+	.driver = {
+		.of_match_table = s5k5baf_of_match,
+		.name = DRIVER_NAME
+	},
+	.probe		= s5k5baf_probe,
+	.remove		= s5k5baf_remove,
+	.id_table	= s5k5baf_id,
+};
+
+module_i2c_driver(s5k5baf_i2c_driver);
+
+MODULE_DESCRIPTION("Samsung S5K5BAF(X) UXGA camera driver");
+MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/media/s5k5baf.h b/include/media/s5k5baf.h
new file mode 100644
index 0000000..957e708
--- /dev/null
+++ b/include/media/s5k5baf.h
@@ -0,0 +1,48 @@ 
+/*
+ * S5K5BAF camera sensor driver header
+ *
+ * Copyright (C) 2011 Samsung Electronics Co., Ltd.
+ *
+ * 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.
+ */
+
+#ifndef S5K5BAF_H
+#define S5K5BAF_H
+
+#include <media/v4l2-mediabus.h>
+
+/**
+ * struct s5k5baf_gpio - data structure describing a GPIO
+ * @gpio:  GPIO number
+ * @level: indicates active state of the @gpio
+ */
+struct s5k5baf_gpio {
+	int gpio;
+	int level;
+};
+
+/**
+ * struct s5k5baf_platform_data - s5k5baf driver platform data
+ * @set_power:   an additional callback to the board code, called
+ *               after enabling the regulators and before switching
+ *               the sensor off
+ * @mclk_frequency: sensor's master clock frequency in Hz
+ * @gpios:       GPIOs driving pins: STBY, RESET
+ * @nlanes:      maximum number of MIPI-CSI lanes used
+ * @hflip:       default horizontal image flip value, non zero to enable
+ * @vflip:       default vertical image flip value, non zero to enable
+ */
+
+struct s5k5baf_platform_data {
+	u32 mclk_frequency;
+	struct s5k5baf_gpio gpios[2];
+	enum v4l2_mbus_type bus_type;
+	u8 nlanes;
+	u8 hflip:1;
+	u8 vflip:1;
+};
+
+#endif /* S5K5BAF_H */