[v7,13/13] V4L: Add driver for s5k4e5 image sensor
Commit Message
This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
Like s5k6a3, it is also another fimc-is firmware controlled
sensor. This minimal sensor driver doesn't do any I2C communications
as its done by ISP firmware. It can be updated if needed to a
regular sensor driver by adding the I2C communication.
Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
.../devicetree/bindings/media/i2c/s5k4e5.txt | 43 +++
drivers/media/i2c/Kconfig | 8 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/s5k4e5.c | 361 ++++++++++++++++++++
4 files changed, 413 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
create mode 100644 drivers/media/i2c/s5k4e5.c
Comments
On 08/21/2013 08:34 AM, Arun Kumar K wrote:
> This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
> Like s5k6a3, it is also another fimc-is firmware controlled
> sensor. This minimal sensor driver doesn't do any I2C communications
> as its done by ISP firmware. It can be updated if needed to a
> regular sensor driver by adding the I2C communication.
>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> .../devicetree/bindings/media/i2c/s5k4e5.txt | 43 +++
> drivers/media/i2c/Kconfig | 8 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/s5k4e5.c | 361 ++++++++++++++++++++
> 4 files changed, 413 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
> create mode 100644 drivers/media/i2c/s5k4e5.c
>
...
> diff --git a/drivers/media/i2c/s5k4e5.c b/drivers/media/i2c/s5k4e5.c
> new file mode 100644
> index 0000000..0a6ece6
> --- /dev/null
> +++ b/drivers/media/i2c/s5k4e5.c
> @@ -0,0 +1,361 @@
> +/*
> + * Samsung S5K4E5 image sensor driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Arun Kumar K <arun.kk@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define S5K4E5_SENSOR_MAX_WIDTH 2576
> +#define S5K4E5_SENSOR_MAX_HEIGHT 1930
> +
> +#define S5K4E5_SENSOR_ACTIVE_WIDTH 2560
> +#define S5K4E5_SENSOR_ACTIVE_HEIGHT 1920
> +
> +#define S5K4E5_SENSOR_MIN_WIDTH (32 + 16)
> +#define S5K4E5_SENSOR_MIN_HEIGHT (32 + 10)
> +
> +#define S5K4E5_DEF_WIDTH 1296
> +#define S5K4E5_DEF_HEIGHT 732
> +
> +#define S5K4E5_DRV_NAME "S5K4E5"
> +#define S5K4E5_CLK_NAME "mclk"
> +
> +#define S5K4E5_NUM_SUPPLIES 2
> +
> +#define S5K4E5_DEF_CLK_FREQ 24000000
> +
> +/**
> + * struct s5k4e5 - s5k4e5 sensor data structure
> + * @dev: pointer to this I2C client device structure
> + * @subdev: the image sensor's v4l2 subdev
> + * @pad: subdev media source pad
> + * @supplies: image sensor's voltage regulator supplies
> + * @gpio_reset: GPIO connected to the sensor's reset pin
> + * @lock: mutex protecting the structure's members below
> + * @format: media bus format at the sensor's source pad
> + */
> +struct s5k4e5 {
> + struct device *dev;
> + struct v4l2_subdev subdev;
> + struct media_pad pad;
> + struct regulator_bulk_data supplies[S5K4E5_NUM_SUPPLIES];
> + int gpio_reset;
> + struct mutex lock;
> + struct v4l2_mbus_framefmt format;
> + struct clk *clock;
> + u32 clock_frequency;
> +};
> +
> +static const char * const s5k4e5_supply_names[] = {
> + "svdda",
> + "svddio"
> +};
I'm no regulator expert, but shouldn't this list come from the DT or platform_data?
Or are these names specific to this sensor?
Regards,
Hans
--
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
Hi Hans,
On Wednesday 21 of August 2013 08:53:55 Hans Verkuil wrote:
> On 08/21/2013 08:34 AM, Arun Kumar K wrote:
> > This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
> > Like s5k6a3, it is also another fimc-is firmware controlled
> > sensor. This minimal sensor driver doesn't do any I2C communications
> > as its done by ISP firmware. It can be updated if needed to a
> > regular sensor driver by adding the I2C communication.
> >
> > Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> > Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > ---
> >
> > .../devicetree/bindings/media/i2c/s5k4e5.txt | 43 +++
> > drivers/media/i2c/Kconfig | 8 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/s5k4e5.c | 361
> > ++++++++++++++++++++ 4 files changed, 413 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/media/i2c/s5k4e5.txt create mode
> > 100644 drivers/media/i2c/s5k4e5.c
>
> ...
>
> > diff --git a/drivers/media/i2c/s5k4e5.c b/drivers/media/i2c/s5k4e5.c
> > new file mode 100644
> > index 0000000..0a6ece6
> > --- /dev/null
> > +++ b/drivers/media/i2c/s5k4e5.c
> > @@ -0,0 +1,361 @@
> > +/*
> > + * Samsung S5K4E5 image sensor driver
> > + *
> > + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> > + * Author: Arun Kumar K <arun.kk@samsung.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > modify + * it under the terms of the GNU General Public License
> > version 2 as + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <linux/gpio.h>
> > +#include <linux/i2c.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> > +#include <linux/videodev2.h>
> > +#include <media/v4l2-async.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define S5K4E5_SENSOR_MAX_WIDTH 2576
> > +#define S5K4E5_SENSOR_MAX_HEIGHT 1930
> > +
> > +#define S5K4E5_SENSOR_ACTIVE_WIDTH 2560
> > +#define S5K4E5_SENSOR_ACTIVE_HEIGHT 1920
> > +
> > +#define S5K4E5_SENSOR_MIN_WIDTH (32 + 16)
> > +#define S5K4E5_SENSOR_MIN_HEIGHT (32 + 10)
> > +
> > +#define S5K4E5_DEF_WIDTH 1296
> > +#define S5K4E5_DEF_HEIGHT 732
> > +
> > +#define S5K4E5_DRV_NAME "S5K4E5"
> > +#define S5K4E5_CLK_NAME "mclk"
> > +
> > +#define S5K4E5_NUM_SUPPLIES 2
> > +
> > +#define S5K4E5_DEF_CLK_FREQ 24000000
> > +
> > +/**
> > + * struct s5k4e5 - s5k4e5 sensor data structure
> > + * @dev: pointer to this I2C client device structure
> > + * @subdev: the image sensor's v4l2 subdev
> > + * @pad: subdev media source pad
> > + * @supplies: image sensor's voltage regulator supplies
> > + * @gpio_reset: GPIO connected to the sensor's reset pin
> > + * @lock: mutex protecting the structure's members below
> > + * @format: media bus format at the sensor's source pad
> > + */
> > +struct s5k4e5 {
> > + struct device *dev;
> > + struct v4l2_subdev subdev;
> > + struct media_pad pad;
> > + struct regulator_bulk_data supplies[S5K4E5_NUM_SUPPLIES];
> > + int gpio_reset;
> > + struct mutex lock;
> > + struct v4l2_mbus_framefmt format;
> > + struct clk *clock;
> > + u32 clock_frequency;
> > +};
> > +
> > +static const char * const s5k4e5_supply_names[] = {
> > + "svdda",
> > + "svddio"
> > +};
>
> I'm no regulator expert, but shouldn't this list come from the DT or
> platform_data? Or are these names specific to this sensor?
This is a list of regulator input (aka supply) names. In other words those
are usually names of pins of the consumer device (s5k4e5 chip in this
case) to which the regulators are connected. They are used as lookup keys
when looking up regulators, either from device tree or lookup tables.
Best regards,
Tomasz
--
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
On Wed 21 August 2013 09:58:28 Tomasz Figa wrote:
> Hi Hans,
>
> On Wednesday 21 of August 2013 08:53:55 Hans Verkuil wrote:
> > On 08/21/2013 08:34 AM, Arun Kumar K wrote:
> > > This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
> > > Like s5k6a3, it is also another fimc-is firmware controlled
> > > sensor. This minimal sensor driver doesn't do any I2C communications
> > > as its done by ISP firmware. It can be updated if needed to a
> > > regular sensor driver by adding the I2C communication.
> > >
> > > Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> > > Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > > ---
> > >
> > > .../devicetree/bindings/media/i2c/s5k4e5.txt | 43 +++
> > > drivers/media/i2c/Kconfig | 8 +
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/s5k4e5.c | 361
> > > ++++++++++++++++++++ 4 files changed, 413 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/media/i2c/s5k4e5.txt create mode
> > > 100644 drivers/media/i2c/s5k4e5.c
> >
> > ...
> >
> > > diff --git a/drivers/media/i2c/s5k4e5.c b/drivers/media/i2c/s5k4e5.c
> > > new file mode 100644
> > > index 0000000..0a6ece6
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/s5k4e5.c
> > > @@ -0,0 +1,361 @@
> > > +/*
> > > + * Samsung S5K4E5 image sensor driver
> > > + *
> > > + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> > > + * Author: Arun Kumar K <arun.kk@samsung.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > modify + * it under the terms of the GNU General Public License
> > > version 2 as + * published by the Free Software Foundation.
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/gpio.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_gpio.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/videodev2.h>
> > > +#include <media/v4l2-async.h>
> > > +#include <media/v4l2-subdev.h>
> > > +
> > > +#define S5K4E5_SENSOR_MAX_WIDTH 2576
> > > +#define S5K4E5_SENSOR_MAX_HEIGHT 1930
> > > +
> > > +#define S5K4E5_SENSOR_ACTIVE_WIDTH 2560
> > > +#define S5K4E5_SENSOR_ACTIVE_HEIGHT 1920
> > > +
> > > +#define S5K4E5_SENSOR_MIN_WIDTH (32 + 16)
> > > +#define S5K4E5_SENSOR_MIN_HEIGHT (32 + 10)
> > > +
> > > +#define S5K4E5_DEF_WIDTH 1296
> > > +#define S5K4E5_DEF_HEIGHT 732
> > > +
> > > +#define S5K4E5_DRV_NAME "S5K4E5"
> > > +#define S5K4E5_CLK_NAME "mclk"
> > > +
> > > +#define S5K4E5_NUM_SUPPLIES 2
> > > +
> > > +#define S5K4E5_DEF_CLK_FREQ 24000000
> > > +
> > > +/**
> > > + * struct s5k4e5 - s5k4e5 sensor data structure
> > > + * @dev: pointer to this I2C client device structure
> > > + * @subdev: the image sensor's v4l2 subdev
> > > + * @pad: subdev media source pad
> > > + * @supplies: image sensor's voltage regulator supplies
> > > + * @gpio_reset: GPIO connected to the sensor's reset pin
> > > + * @lock: mutex protecting the structure's members below
> > > + * @format: media bus format at the sensor's source pad
> > > + */
> > > +struct s5k4e5 {
> > > + struct device *dev;
> > > + struct v4l2_subdev subdev;
> > > + struct media_pad pad;
> > > + struct regulator_bulk_data supplies[S5K4E5_NUM_SUPPLIES];
> > > + int gpio_reset;
> > > + struct mutex lock;
> > > + struct v4l2_mbus_framefmt format;
> > > + struct clk *clock;
> > > + u32 clock_frequency;
> > > +};
> > > +
> > > +static const char * const s5k4e5_supply_names[] = {
> > > + "svdda",
> > > + "svddio"
> > > +};
> >
> > I'm no regulator expert, but shouldn't this list come from the DT or
> > platform_data? Or are these names specific to this sensor?
>
> This is a list of regulator input (aka supply) names. In other words those
> are usually names of pins of the consumer device (s5k4e5 chip in this
> case) to which the regulators are connected. They are used as lookup keys
> when looking up regulators, either from device tree or lookup tables.
How does that work if you have two of these sensors? E.g. in a stereo-camera?
Can the regulator subsystem map those pins to the correct regulators?
Again, sorry for my ignorance in this area as I've never used it. I just
want to make sure this information is stored in the right place.
Regards,
Hans
--
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
Hi Hans,
On Wed, Aug 21, 2013 at 1:54 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Wed 21 August 2013 09:58:28 Tomasz Figa wrote:
>> Hi Hans,
>>
>> On Wednesday 21 of August 2013 08:53:55 Hans Verkuil wrote:
>> > On 08/21/2013 08:34 AM, Arun Kumar K wrote:
>> > > This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
>> > > Like s5k6a3, it is also another fimc-is firmware controlled
>> > > sensor. This minimal sensor driver doesn't do any I2C communications
>> > > as its done by ISP firmware. It can be updated if needed to a
>> > > regular sensor driver by adding the I2C communication.
>> > >
>> > > Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>> > > Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> > > ---
>> > >
>> > > .../devicetree/bindings/media/i2c/s5k4e5.txt | 43 +++
>> > > drivers/media/i2c/Kconfig | 8 +
>> > > drivers/media/i2c/Makefile | 1 +
>> > > drivers/media/i2c/s5k4e5.c | 361
>> > > ++++++++++++++++++++ 4 files changed, 413 insertions(+)
>> > > create mode 100644
>> > > Documentation/devicetree/bindings/media/i2c/s5k4e5.txt create mode
>> > > 100644 drivers/media/i2c/s5k4e5.c
>> >
>> > ...
>> >
>> > > diff --git a/drivers/media/i2c/s5k4e5.c b/drivers/media/i2c/s5k4e5.c
>> > > new file mode 100644
>> > > index 0000000..0a6ece6
>> > > --- /dev/null
>> > > +++ b/drivers/media/i2c/s5k4e5.c
>> > > @@ -0,0 +1,361 @@
>> > > +/*
>> > > + * Samsung S5K4E5 image sensor driver
>> > > + *
>> > > + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
>> > > + * Author: Arun Kumar K <arun.kk@samsung.com>
>> > > + *
>> > > + * This program is free software; you can redistribute it and/or
>> > > modify + * it under the terms of the GNU General Public License
>> > > version 2 as + * published by the Free Software Foundation.
>> > > + */
>> > > +
>> > > +#include <linux/clk.h>
>> > > +#include <linux/delay.h>
>> > > +#include <linux/device.h>
>> > > +#include <linux/errno.h>
>> > > +#include <linux/gpio.h>
>> > > +#include <linux/i2c.h>
>> > > +#include <linux/kernel.h>
>> > > +#include <linux/module.h>
>> > > +#include <linux/of_gpio.h>
>> > > +#include <linux/pm_runtime.h>
>> > > +#include <linux/regulator/consumer.h>
>> > > +#include <linux/slab.h>
>> > > +#include <linux/videodev2.h>
>> > > +#include <media/v4l2-async.h>
>> > > +#include <media/v4l2-subdev.h>
>> > > +
>> > > +#define S5K4E5_SENSOR_MAX_WIDTH 2576
>> > > +#define S5K4E5_SENSOR_MAX_HEIGHT 1930
>> > > +
>> > > +#define S5K4E5_SENSOR_ACTIVE_WIDTH 2560
>> > > +#define S5K4E5_SENSOR_ACTIVE_HEIGHT 1920
>> > > +
>> > > +#define S5K4E5_SENSOR_MIN_WIDTH (32 + 16)
>> > > +#define S5K4E5_SENSOR_MIN_HEIGHT (32 + 10)
>> > > +
>> > > +#define S5K4E5_DEF_WIDTH 1296
>> > > +#define S5K4E5_DEF_HEIGHT 732
>> > > +
>> > > +#define S5K4E5_DRV_NAME "S5K4E5"
>> > > +#define S5K4E5_CLK_NAME "mclk"
>> > > +
>> > > +#define S5K4E5_NUM_SUPPLIES 2
>> > > +
>> > > +#define S5K4E5_DEF_CLK_FREQ 24000000
>> > > +
>> > > +/**
>> > > + * struct s5k4e5 - s5k4e5 sensor data structure
>> > > + * @dev: pointer to this I2C client device structure
>> > > + * @subdev: the image sensor's v4l2 subdev
>> > > + * @pad: subdev media source pad
>> > > + * @supplies: image sensor's voltage regulator supplies
>> > > + * @gpio_reset: GPIO connected to the sensor's reset pin
>> > > + * @lock: mutex protecting the structure's members below
>> > > + * @format: media bus format at the sensor's source pad
>> > > + */
>> > > +struct s5k4e5 {
>> > > + struct device *dev;
>> > > + struct v4l2_subdev subdev;
>> > > + struct media_pad pad;
>> > > + struct regulator_bulk_data supplies[S5K4E5_NUM_SUPPLIES];
>> > > + int gpio_reset;
>> > > + struct mutex lock;
>> > > + struct v4l2_mbus_framefmt format;
>> > > + struct clk *clock;
>> > > + u32 clock_frequency;
>> > > +};
>> > > +
>> > > +static const char * const s5k4e5_supply_names[] = {
>> > > + "svdda",
>> > > + "svddio"
>> > > +};
>> >
>> > I'm no regulator expert, but shouldn't this list come from the DT or
>> > platform_data? Or are these names specific to this sensor?
>>
>> This is a list of regulator input (aka supply) names. In other words those
>> are usually names of pins of the consumer device (s5k4e5 chip in this
>> case) to which the regulators are connected. They are used as lookup keys
>> when looking up regulators, either from device tree or lookup tables.
>
> How does that work if you have two of these sensors? E.g. in a stereo-camera?
> Can the regulator subsystem map those pins to the correct regulators?
>
> Again, sorry for my ignorance in this area as I've never used it. I just
> want to make sure this information is stored in the right place.
>
There are two regulator supplies needed for this sensor, and these pin names
are just used in the driver to differentiate them.
From the DT, we pass regulator node phandles with these properties
(supply names) which the driver uses.
Regards
Arun
--
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
On 08/21/2013 10:24 AM, Hans Verkuil wrote:
>>>> +static const char * const s5k4e5_supply_names[] = {
>>>> + "svdda",
>>>> + "svddio"
>>>> +};
>>>
>>> I'm no regulator expert, but shouldn't this list come from the DT or
>>> platform_data? Or are these names specific to this sensor?
>>
>> This is a list of regulator input (aka supply) names. In other words those
>> are usually names of pins of the consumer device (s5k4e5 chip in this
>> case) to which the regulators are connected. They are used as lookup keys
>> when looking up regulators, either from device tree or lookup tables.
>
> How does that work if you have two of these sensors? E.g. in a stereo-camera?
> Can the regulator subsystem map those pins to the correct regulators?
>
> Again, sorry for my ignorance in this area as I've never used it. I just
> want to make sure this information is stored in the right place.
The _voltage regulator supply names_ are fixed but _voltage regulator_
is matched with a consumer device based on the supply name and name of
the consumer device. See usage of struct regulator_consumer_supply, e.g.
in arch/arm/mach-s5pv210/mach-goni.c board file. This is an example of
non-dt system, and something that would presumably be created by a driver
like em28xx if it wanted to use that sensor. I.e. em28xx would first
need to create a voltage regulator device and then pass in a
struct regulator_init_data the list of voltage supply definitions for
the consumers to be able to use this regulator.
In case of device tree the voltage supplies are specified in
a DT node, which can be referenced by subsystems/drivers through
struct device::of_node.
reg_a: voltage-regulator-a {
compatible = "regulator-fixed";
regulator-name = "REG_5V_A";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
gpio = <...>;
...
};
reg_b: voltage-regulator-b {
compatible = "regulator-fixed";
regulator-name = "REG_3.3V_B";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
gpio = <...>;
...
};
s5k4e5@20 {
compatible = "samsung,s5k4e5";
reg = <0x20>;
...
svdda-supply = <®_a>;
svddio-supply = <®_b>;
...
};
The regulator supply names are part of name of the property defining
a voltage regulator for a device. Properties in form of
[supply_name]-supply are parsed by the regulator core when consumer
device driver calls regulator_get(). This way drivers don't need to
care whether the system is booted from Device Tree or not. They just
keep using the regulator API and the regulator supply lookup is done
the the core based on data in a board file or in device tree blob.
This is similar to the clock API operation, except that clkdev entries
are usually defined per SOC/MCU rather than per board.
I hope it helps. I looked yesterday at the em28xx driver. Do you happen
to know if there is a schematic for one of devices this driver supports ?
Sorry, I didn't dig to hard yet.
At first sight I thought it may look a bit problematic and require
significant amount of code to define regulators for the all supported
sensors by this driver, should it be made to work with sensors that
are currently known to be used only in embedded systems and use the
regulators API. However it should be as simple as defining at least one
regulator device and attaching regulator supply list definition for all
supported sensors. Thus not that scary at all. And the subdev drivers
can continue to use regulator API, without a need for any hacks making
it optional through e.g. platform_data flag. And IMO if the regulator
API is disabled currently by some x86 distros it should be enabled,
as long as some drivers need it.
--
Regards,
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
On 08/21/2013 11:13 AM, Sylwester Nawrocki wrote:
> On 08/21/2013 10:24 AM, Hans Verkuil wrote:
>>>>> +static const char * const s5k4e5_supply_names[] = {
>>>>> + "svdda",
>>>>> + "svddio"
>>>>> +};
>>>>
>>>> I'm no regulator expert, but shouldn't this list come from the DT or
>>>> platform_data? Or are these names specific to this sensor?
>>>
>>> This is a list of regulator input (aka supply) names. In other words those
>>> are usually names of pins of the consumer device (s5k4e5 chip in this
>>> case) to which the regulators are connected. They are used as lookup keys
>>> when looking up regulators, either from device tree or lookup tables.
>>
>> How does that work if you have two of these sensors? E.g. in a stereo-camera?
>> Can the regulator subsystem map those pins to the correct regulators?
>>
>> Again, sorry for my ignorance in this area as I've never used it. I just
>> want to make sure this information is stored in the right place.
>
> The _voltage regulator supply names_ are fixed but _voltage regulator_
> is matched with a consumer device based on the supply name and name of
> the consumer device. See usage of struct regulator_consumer_supply, e.g.
> in arch/arm/mach-s5pv210/mach-goni.c board file. This is an example of
> non-dt system, and something that would presumably be created by a driver
> like em28xx if it wanted to use that sensor. I.e. em28xx would first
> need to create a voltage regulator device and then pass in a
> struct regulator_init_data the list of voltage supply definitions for
> the consumers to be able to use this regulator.
>
>
> In case of device tree the voltage supplies are specified in
> a DT node, which can be referenced by subsystems/drivers through
> struct device::of_node.
>
> reg_a: voltage-regulator-a {
> compatible = "regulator-fixed";
> regulator-name = "REG_5V_A";
> regulator-min-microvolt = <5000000>;
> regulator-max-microvolt = <5000000>;
> gpio = <...>;
> ...
> };
>
> reg_b: voltage-regulator-b {
> compatible = "regulator-fixed";
> regulator-name = "REG_3.3V_B";
> regulator-min-microvolt = <3300000>;
> regulator-max-microvolt = <3300000>;
> gpio = <...>;
> ...
> };
>
> s5k4e5@20 {
> compatible = "samsung,s5k4e5";
> reg = <0x20>;
> ...
> svdda-supply = <®_a>;
> svddio-supply = <®_b>;
> ...
> };
I forgot to mention that each of the two sensors would of course have
its corresponding DT node, but since they both have same slave address
would likely be attached to separate I2C bus controllers, e.g.
/* I2C bus controller node */
i2c-gpio-0 {
compatible = "i2c-gpio";
gpios = <...>;
...
s5k4e5@20 {
compatible = "samsung,s5k4e5";
reg = <0x20>;
...
svdda-supply = <®_a>;
svddio-supply = <®_b>;
...
};
};
i2c-gpio-1 {
compatible = "i2c-gpio";
gpios = <...>;
...
s5k4e5@20 {
compatible = "samsung,s5k4e5";
reg = <0x20>;
...
svdda-supply = <®_a>;
svddio-supply = <®_b>;
...
};
};
So these sensors have same regulator supply names but different i2c_client
(device) names, since they are children of different I2C bus adapters.
> The regulator supply names are part of name of the property defining
> a voltage regulator for a device. Properties in form of
> [supply_name]-supply are parsed by the regulator core when consumer
> device driver calls regulator_get(). This way drivers don't need to
> care whether the system is booted from Device Tree or not. They just
> keep using the regulator API and the regulator supply lookup is done
> the the core based on data in a board file or in device tree blob.
>
> This is similar to the clock API operation, except that clkdev entries
> are usually defined per SOC/MCU rather than per board.
>
> I hope it helps. I looked yesterday at the em28xx driver. Do you happen
> to know if there is a schematic for one of devices this driver supports ?
> Sorry, I didn't dig to hard yet.
> At first sight I thought it may look a bit problematic and require
> significant amount of code to define regulators for the all supported
> sensors by this driver, should it be made to work with sensors that
> are currently known to be used only in embedded systems and use the
> regulators API. However it should be as simple as defining at least one
> regulator device and attaching regulator supply list definition for all
> supported sensors. Thus not that scary at all. And the subdev drivers
> can continue to use regulator API, without a need for any hacks making
> it optional through e.g. platform_data flag. And IMO if the regulator
> API is disabled currently by some x86 distros it should be enabled,
> as long as some drivers need it.
>
> --
> Regards,
> 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
On 08/21/2013 08:34 AM, Arun Kumar K wrote:
> This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
> Like s5k6a3, it is also another fimc-is firmware controlled
> sensor. This minimal sensor driver doesn't do any I2C communications
> as its done by ISP firmware. It can be updated if needed to a
> regular sensor driver by adding the I2C communication.
>
> Signed-off-by: Arun Kumar K<arun.kk@samsung.com>
> Reviewed-by: Sylwester Nawrocki<s.nawrocki@samsung.com>
> ---
> .../devicetree/bindings/media/i2c/s5k4e5.txt | 43 +++
> drivers/media/i2c/Kconfig | 8 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/s5k4e5.c | 361 ++++++++++++++++++++
> 4 files changed, 413 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
> create mode 100644 drivers/media/i2c/s5k4e5.c
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt b/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
> new file mode 100644
> index 0000000..5af462c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
> @@ -0,0 +1,43 @@
> +* Samsung S5K4E5 Raw Image Sensor
> +
> +S5K4E5 is a raw image sensor with maximum resolution of 2560x1920
> +pixels. Data transfer is carried out via MIPI CSI-2 port and controls
> +via I2C bus.
> +
> +Required Properties:
> +- compatible : must be "samsung,s5k4e5"
> +- reg : I2C device address
> +- gpios : reset gpio pin
I guess this should be "reset-gpios". How about changing description to:
- reset-gpios : specifier of a GPIO connected to the RESET pin;
?
> +- clocks : clock specifier for the clock-names property
Perhaps something along the lines of:
- clocks : "should contain the sensor's MCLK clock specifier, from
the common clock bindings"
?
> +- clock-names : must contain "mclk" entry
Is name of the clock input pin really MCLK ?
> +- svdda-supply : core voltage supply
> +- svddio-supply : I/O voltage supply
> +
> +Optional Properties:
> +- clock-frequency : operating frequency for the sensor
> + default value will be taken if not provided.
How about clarifying it a bit, as Stephen suggested, e.g.:
- clock-frequency : the frequency at which the "mclk" clock should be
configured to operate, in Hz; if this property is not
specified default 24 MHz value will be used.
> +The device node should be added to respective control bus controller
> +(e.g. I2C0) nodes and linked to the csis port node, using the common
> +video interfaces bindings, defined in video-interfaces.txt.
This seems misplaced, S5K4E5 image sensor has nothingto do with csis nodes.
How about something like this instead:
"The common video interfaces bindings (see video-interfaces.txt) should be
used to specify link to the image data receiver. The S5K6A3(YX) device
node should contain one 'port' child node with an 'endpoint' subnode.
Following properties are valid for the endpoint node:
..."
> +Example:
> +
> + i2c-isp@13130000 {
> + s5k4e5@20 {
> + compatible = "samsung,s5k4e5";
> + reg =<0x20>;
> + gpios =<&gpx1 2 1>;
> + clock-frequency =<24000000>;
> + clocks =<&clock 129>;
> + clock-names = "mclk";
> + svdda-supply =<...>;
> + svddio-supply =<...>;
> + port {
> + is_s5k4e5_ep: endpoint {
> + data-lanes =<1 2 3 4>;
> + remote-endpoint =<&csis0_ep>;
> + };
> + };
> + };
> + };
--
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
Hi Sylwester,
Will make the changes you suggested.
Will re-spin this entire series with some more minor fixes after more
rigorous testing.
Regards
Arun
On Fri, Sep 6, 2013 at 1:32 AM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
> On 08/21/2013 08:34 AM, Arun Kumar K wrote:
>>
>> This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
>> Like s5k6a3, it is also another fimc-is firmware controlled
>> sensor. This minimal sensor driver doesn't do any I2C communications
>> as its done by ISP firmware. It can be updated if needed to a
>> regular sensor driver by adding the I2C communication.
>>
>> Signed-off-by: Arun Kumar K<arun.kk@samsung.com>
>> Reviewed-by: Sylwester Nawrocki<s.nawrocki@samsung.com>
>> ---
>> .../devicetree/bindings/media/i2c/s5k4e5.txt | 43 +++
>> drivers/media/i2c/Kconfig | 8 +
>> drivers/media/i2c/Makefile | 1 +
>> drivers/media/i2c/s5k4e5.c | 361
>> ++++++++++++++++++++
>> 4 files changed, 413 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
>> create mode 100644 drivers/media/i2c/s5k4e5.c
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
>> b/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
>> new file mode 100644
>> index 0000000..5af462c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
>> @@ -0,0 +1,43 @@
>> +* Samsung S5K4E5 Raw Image Sensor
>> +
>> +S5K4E5 is a raw image sensor with maximum resolution of 2560x1920
>> +pixels. Data transfer is carried out via MIPI CSI-2 port and controls
>> +via I2C bus.
>> +
>> +Required Properties:
>> +- compatible : must be "samsung,s5k4e5"
>> +- reg : I2C device address
>> +- gpios : reset gpio pin
>
>
> I guess this should be "reset-gpios". How about changing description to:
>
> - reset-gpios : specifier of a GPIO connected to the RESET pin;
>
>
> ?
>>
>> +- clocks : clock specifier for the clock-names property
>
>
> Perhaps something along the lines of:
>
> - clocks : "should contain the sensor's MCLK clock specifier, from
> the common clock bindings"
>
> ?
>>
>> +- clock-names : must contain "mclk" entry
>
>
> Is name of the clock input pin really MCLK ?
>
>
>> +- svdda-supply : core voltage supply
>> +- svddio-supply : I/O voltage supply
>> +
>> +Optional Properties:
>> +- clock-frequency : operating frequency for the sensor
>> + default value will be taken if not provided.
>
>
> How about clarifying it a bit, as Stephen suggested, e.g.:
>
> - clock-frequency : the frequency at which the "mclk" clock should be
> configured to operate, in Hz; if this property is not
> specified default 24 MHz value will be used.
>
>
>> +The device node should be added to respective control bus controller
>> +(e.g. I2C0) nodes and linked to the csis port node, using the common
>> +video interfaces bindings, defined in video-interfaces.txt.
>
>
> This seems misplaced, S5K4E5 image sensor has nothingto do with csis nodes.
> How about something like this instead:
>
> "The common video interfaces bindings (see video-interfaces.txt) should be
> used to specify link to the image data receiver. The S5K6A3(YX) device
> node should contain one 'port' child node with an 'endpoint' subnode.
>
> Following properties are valid for the endpoint node:
> ..."
>
>
>> +Example:
>> +
>> + i2c-isp@13130000 {
>> + s5k4e5@20 {
>> + compatible = "samsung,s5k4e5";
>> + reg =<0x20>;
>> + gpios =<&gpx1 2 1>;
>> + clock-frequency =<24000000>;
>> + clocks =<&clock 129>;
>> + clock-names = "mclk";
>> + svdda-supply =<...>;
>> + svddio-supply =<...>;
>> + port {
>> + is_s5k4e5_ep: endpoint {
>> + data-lanes =<1 2 3 4>;
>> + remote-endpoint =<&csis0_ep>;
>> + };
>> + };
>> + };
>> + };
>
>
> --
> 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
Hi Sylwester,
On Fri, Sep 6, 2013 at 1:32 AM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
> On 08/21/2013 08:34 AM, Arun Kumar K wrote:
>>
>> This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
>> Like s5k6a3, it is also another fimc-is firmware controlled
>> sensor. This minimal sensor driver doesn't do any I2C communications
>> as its done by ISP firmware. It can be updated if needed to a
>> regular sensor driver by adding the I2C communication.
>>
>> Signed-off-by: Arun Kumar K<arun.kk@samsung.com>
>> Reviewed-by: Sylwester Nawrocki<s.nawrocki@samsung.com>
>> ---
>> .../devicetree/bindings/media/i2c/s5k4e5.txt | 43 +++
>> drivers/media/i2c/Kconfig | 8 +
>> drivers/media/i2c/Makefile | 1 +
>> drivers/media/i2c/s5k4e5.c | 361
>> ++++++++++++++++++++
>> 4 files changed, 413 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
>> create mode 100644 drivers/media/i2c/s5k4e5.c
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
>> b/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
>> new file mode 100644
>> index 0000000..5af462c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/s5k4e5.txt
>> @@ -0,0 +1,43 @@
>> +* Samsung S5K4E5 Raw Image Sensor
>> +
>> +S5K4E5 is a raw image sensor with maximum resolution of 2560x1920
>> +pixels. Data transfer is carried out via MIPI CSI-2 port and controls
>> +via I2C bus.
>> +
>> +Required Properties:
>> +- compatible : must be "samsung,s5k4e5"
>> +- reg : I2C device address
>> +- gpios : reset gpio pin
>
>
> I guess this should be "reset-gpios". How about changing description to:
>
> - reset-gpios : specifier of a GPIO connected to the RESET pin;
>
>
> ?
If I name it to reset-gpios, the function of_get_gpio_flags() in the driver
fails. This function searches for the entry with name "gpios". Is it still
recommended to use a custom name and parse it explicitly?
Regards
Arun
--
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
Hi Arun,
On 09/11/2013 07:10 AM, Arun Kumar K wrote:
> If I name it to reset-gpios, the function of_get_gpio_flags() in the driver
> fails. This function searches for the entry with name "gpios". Is it still
> recommended to use a custom name and parse it explicitly?
I guess so, you could just use of_get_named_gpio_flags().
Regards,
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
new file mode 100644
@@ -0,0 +1,43 @@
+* Samsung S5K4E5 Raw Image Sensor
+
+S5K4E5 is a raw image sensor with maximum resolution of 2560x1920
+pixels. Data transfer is carried out via MIPI CSI-2 port and controls
+via I2C bus.
+
+Required Properties:
+- compatible : must be "samsung,s5k4e5"
+- reg : I2C device address
+- gpios : reset gpio pin
+- clocks : clock specifier for the clock-names property
+- clock-names : must contain "mclk" entry
+- svdda-supply : core voltage supply
+- svddio-supply : I/O voltage supply
+
+Optional Properties:
+- clock-frequency : operating frequency for the sensor
+ default value will be taken if not provided.
+
+The device node should be added to respective control bus controller
+(e.g. I2C0) nodes and linked to the csis port node, using the common
+video interfaces bindings, defined in video-interfaces.txt.
+
+Example:
+
+ i2c-isp@13130000 {
+ s5k4e5@20 {
+ compatible = "samsung,s5k4e5";
+ reg = <0x20>;
+ gpios = <&gpx1 2 1>;
+ clock-frequency = <24000000>;
+ clocks = <&clock 129>;
+ clock-names = "mclk";
+ svdda-supply = <...>;
+ svddio-supply = <...>;
+ port {
+ is_s5k4e5_ep: endpoint {
+ data-lanes = <1 2 3 4>;
+ remote-endpoint = <&csis0_ep>;
+ };
+ };
+ };
+ };
@@ -572,6 +572,14 @@ config VIDEO_S5K6A3
This is a V4L2 sensor-level driver for Samsung S5K6A3 raw
camera sensor.
+config VIDEO_S5K4E5
+ tristate "Samsung S5K4E5 sensor support"
+ depends on MEDIA_CAMERA_SUPPORT
+ depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF
+ ---help---
+ This is a V4L2 sensor-level driver for Samsung S5K4E5 raw
+ camera sensor.
+
config VIDEO_S5K4ECGX
tristate "Samsung S5K4ECGX sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
@@ -65,6 +65,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
obj-$(CONFIG_VIDEO_NOON010PC30) += noon010pc30.o
obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
obj-$(CONFIG_VIDEO_S5K6A3) += s5k6a3.o
+obj-$(CONFIG_VIDEO_S5K4E5) += s5k4e5.o
obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o
obj-$(CONFIG_VIDEO_S5C73M3) += s5c73m3/
obj-$(CONFIG_VIDEO_ADP1653) += adp1653.o
new file mode 100644
@@ -0,0 +1,361 @@
+/*
+ * Samsung S5K4E5 image sensor driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Arun Kumar K <arun.kk@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/videodev2.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-subdev.h>
+
+#define S5K4E5_SENSOR_MAX_WIDTH 2576
+#define S5K4E5_SENSOR_MAX_HEIGHT 1930
+
+#define S5K4E5_SENSOR_ACTIVE_WIDTH 2560
+#define S5K4E5_SENSOR_ACTIVE_HEIGHT 1920
+
+#define S5K4E5_SENSOR_MIN_WIDTH (32 + 16)
+#define S5K4E5_SENSOR_MIN_HEIGHT (32 + 10)
+
+#define S5K4E5_DEF_WIDTH 1296
+#define S5K4E5_DEF_HEIGHT 732
+
+#define S5K4E5_DRV_NAME "S5K4E5"
+#define S5K4E5_CLK_NAME "mclk"
+
+#define S5K4E5_NUM_SUPPLIES 2
+
+#define S5K4E5_DEF_CLK_FREQ 24000000
+
+/**
+ * struct s5k4e5 - s5k4e5 sensor data structure
+ * @dev: pointer to this I2C client device structure
+ * @subdev: the image sensor's v4l2 subdev
+ * @pad: subdev media source pad
+ * @supplies: image sensor's voltage regulator supplies
+ * @gpio_reset: GPIO connected to the sensor's reset pin
+ * @lock: mutex protecting the structure's members below
+ * @format: media bus format at the sensor's source pad
+ */
+struct s5k4e5 {
+ struct device *dev;
+ struct v4l2_subdev subdev;
+ struct media_pad pad;
+ struct regulator_bulk_data supplies[S5K4E5_NUM_SUPPLIES];
+ int gpio_reset;
+ struct mutex lock;
+ struct v4l2_mbus_framefmt format;
+ struct clk *clock;
+ u32 clock_frequency;
+};
+
+static const char * const s5k4e5_supply_names[] = {
+ "svdda",
+ "svddio"
+};
+
+static inline struct s5k4e5 *sd_to_s5k4e5(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct s5k4e5, subdev);
+}
+
+static const struct v4l2_mbus_framefmt s5k4e5_formats[] = {
+ {
+ .code = V4L2_MBUS_FMT_SGRBG10_1X10,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .field = V4L2_FIELD_NONE,
+ }
+};
+
+static const struct v4l2_mbus_framefmt *find_sensor_format(
+ struct v4l2_mbus_framefmt *mf)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(s5k4e5_formats); i++)
+ if (mf->code == s5k4e5_formats[i].code)
+ return &s5k4e5_formats[i];
+
+ return &s5k4e5_formats[0];
+}
+
+static int s5k4e5_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_fh *fh,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ if (code->index >= ARRAY_SIZE(s5k4e5_formats))
+ return -EINVAL;
+
+ code->code = s5k4e5_formats[code->index].code;
+ return 0;
+}
+
+static void s5k4e5_try_format(struct v4l2_mbus_framefmt *mf)
+{
+ const struct v4l2_mbus_framefmt *fmt;
+
+ fmt = find_sensor_format(mf);
+ mf->code = fmt->code;
+ v4l_bound_align_image(&mf->width,
+ S5K4E5_SENSOR_MIN_WIDTH, S5K4E5_SENSOR_MAX_WIDTH, 0,
+ &mf->height,
+ S5K4E5_SENSOR_MIN_HEIGHT, S5K4E5_SENSOR_MAX_HEIGHT, 0,
+ 0);
+}
+
+static struct v4l2_mbus_framefmt *__s5k4e5_get_format(
+ struct s5k4e5 *sensor, struct v4l2_subdev_fh *fh,
+ u32 pad, enum v4l2_subdev_format_whence which)
+{
+ if (which == V4L2_SUBDEV_FORMAT_TRY)
+ return fh ? v4l2_subdev_get_try_format(fh, pad) : NULL;
+
+ return &sensor->format;
+}
+
+static int s5k4e5_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_fh *fh,
+ struct v4l2_subdev_format *fmt)
+{
+ struct s5k4e5 *sensor = sd_to_s5k4e5(sd);
+ struct v4l2_mbus_framefmt *mf;
+
+ s5k4e5_try_format(&fmt->format);
+
+ mf = __s5k4e5_get_format(sensor, fh, fmt->pad, fmt->which);
+ if (mf) {
+ mutex_lock(&sensor->lock);
+ if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+ *mf = fmt->format;
+ mutex_unlock(&sensor->lock);
+ }
+ return 0;
+}
+
+static int s5k4e5_get_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_fh *fh,
+ struct v4l2_subdev_format *fmt)
+{
+ struct s5k4e5 *sensor = sd_to_s5k4e5(sd);
+ struct v4l2_mbus_framefmt *mf;
+
+ mf = __s5k4e5_get_format(sensor, fh, fmt->pad, fmt->which);
+
+ mutex_lock(&sensor->lock);
+ fmt->format = *mf;
+ mutex_unlock(&sensor->lock);
+ return 0;
+}
+
+static struct v4l2_subdev_pad_ops s5k4e5_pad_ops = {
+ .enum_mbus_code = s5k4e5_enum_mbus_code,
+ .get_fmt = s5k4e5_get_fmt,
+ .set_fmt = s5k4e5_set_fmt,
+};
+
+static int s5k4e5_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ struct v4l2_mbus_framefmt *format = v4l2_subdev_get_try_format(fh, 0);
+
+ *format = s5k4e5_formats[0];
+ format->width = S5K4E5_DEF_WIDTH;
+ format->height = S5K4E5_DEF_HEIGHT;
+
+ return 0;
+}
+
+static const struct v4l2_subdev_internal_ops s5k4e5_sd_internal_ops = {
+ .open = s5k4e5_open,
+};
+
+static int s5k4e5_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct s5k4e5 *sensor = sd_to_s5k4e5(sd);
+ int gpio = sensor->gpio_reset;
+ int ret = 0;
+
+ if (on) {
+ sensor->clock = clk_get(sensor->dev, S5K4E5_CLK_NAME);
+ if (IS_ERR(sensor->clock))
+ return PTR_ERR(sensor->clock);
+
+ ret = clk_set_rate(sensor->clock, sensor->clock_frequency);
+ if (ret < 0)
+ goto clk_put;
+
+ ret = pm_runtime_get(sensor->dev);
+ if (ret < 0)
+ goto clk_put;
+
+ ret = regulator_bulk_enable(S5K4E5_NUM_SUPPLIES,
+ sensor->supplies);
+ if (ret < 0)
+ goto rpm_put;
+
+ ret = clk_prepare_enable(sensor->clock);
+ if (ret < 0)
+ goto reg_dis;
+
+ if (gpio_is_valid(gpio)) {
+ gpio_set_value(gpio, 1);
+ usleep_range(600, 800);
+ gpio_set_value(gpio, 0);
+ usleep_range(10000, 11000);
+ gpio_set_value(gpio, 1);
+ }
+
+ /* Delay needed for the sensor initialization */
+ msleep(20);
+ } else {
+ if (gpio_is_valid(gpio))
+ gpio_set_value(gpio, 0);
+
+ clk_disable_unprepare(sensor->clock);
+reg_dis:
+ regulator_bulk_disable(S5K4E5_NUM_SUPPLIES,
+ sensor->supplies);
+rpm_put:
+ pm_runtime_put(sensor->dev);
+clk_put:
+ clk_put(sensor->clock);
+ }
+ return ret;
+}
+
+static struct v4l2_subdev_core_ops s5k4e5_core_ops = {
+ .s_power = s5k4e5_s_power,
+};
+
+static struct v4l2_subdev_ops s5k4e5_subdev_ops = {
+ .core = &s5k4e5_core_ops,
+ .pad = &s5k4e5_pad_ops,
+};
+
+static int s5k4e5_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct s5k4e5 *sensor;
+ struct v4l2_subdev *sd;
+ int gpio, i, ret;
+
+ sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
+ if (!sensor)
+ return -ENOMEM;
+
+ mutex_init(&sensor->lock);
+ sensor->gpio_reset = -EINVAL;
+ sensor->clock = ERR_PTR(-EINVAL);
+ sensor->dev = dev;
+
+ gpio = of_get_gpio_flags(dev->of_node, 0, NULL);
+ if (gpio_is_valid(gpio)) {
+ ret = devm_gpio_request_one(dev, gpio, GPIOF_OUT_INIT_LOW,
+ S5K4E5_DRV_NAME);
+ if (ret < 0)
+ return ret;
+ }
+ sensor->gpio_reset = gpio;
+
+ if (of_property_read_u32(dev->of_node, "clock-frequency",
+ &sensor->clock_frequency)) {
+ /* Fallback to default value */
+ sensor->clock_frequency = S5K4E5_DEF_CLK_FREQ;
+ }
+
+ for (i = 0; i < S5K4E5_NUM_SUPPLIES; i++)
+ sensor->supplies[i].supply = s5k4e5_supply_names[i];
+
+ ret = devm_regulator_bulk_get(&client->dev, S5K4E5_NUM_SUPPLIES,
+ sensor->supplies);
+ if (ret < 0)
+ return ret;
+
+ /* Defer probing if the clock is not available yet */
+ sensor->clock = clk_get(dev, S5K4E5_CLK_NAME);
+ if (IS_ERR(sensor->clock))
+ return -EPROBE_DEFER;
+
+ sd = &sensor->subdev;
+ v4l2_i2c_subdev_init(sd, client, &s5k4e5_subdev_ops);
+ sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+ sensor->format.code = s5k4e5_formats[0].code;
+ sensor->format.width = S5K4E5_DEF_WIDTH;
+ sensor->format.height = S5K4E5_DEF_HEIGHT;
+
+ sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
+ ret = media_entity_init(&sd->entity, 1, &sensor->pad, 0);
+ if (ret < 0)
+ return ret;
+
+ pm_runtime_no_callbacks(dev);
+ pm_runtime_enable(dev);
+
+ ret = v4l2_async_register_subdev(sd);
+
+ /*
+ * Don't hold reference to the clock to avoid circular dependency
+ * between the subdev and the host driver, in case the host is
+ * a supplier of the clock.
+ * clk_get()/clk_put() will be called in s_power callback.
+ */
+ clk_put(sensor->clock);
+
+ return ret;
+}
+
+static int s5k4e5_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+
+ v4l2_async_unregister_subdev(sd);
+ media_entity_cleanup(&sd->entity);
+ return 0;
+}
+
+static const struct i2c_device_id s5k4e5_ids[] = {
+ { }
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id s5k4e5_of_match[] = {
+ { .compatible = "samsung,s5k4e5" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, s5k4e5_of_match);
+#endif
+
+static struct i2c_driver s5k4e5_driver = {
+ .driver = {
+ .of_match_table = of_match_ptr(s5k4e5_of_match),
+ .name = S5K4E5_DRV_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = s5k4e5_probe,
+ .remove = s5k4e5_remove,
+ .id_table = s5k4e5_ids,
+};
+
+module_i2c_driver(s5k4e5_driver);
+
+MODULE_DESCRIPTION("S5K4E5 image sensor subdev driver");
+MODULE_AUTHOR("Arun Kumar K <arun.kk@samsung.com>");
+MODULE_LICENSE("GPL v2");