Re: [PATCH v7 13/13] V4L: Add driver for s5k4e5 image sensor

2013-09-11 Thread Sylwester Nawrocki
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 13/13] V4L: Add driver for s5k4e5 image sensor

2013-09-10 Thread Arun Kumar K
Hi Sylwester,

On Fri, Sep 6, 2013 at 1:32 AM, Sylwester Nawrocki
sylvester.nawro...@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 Karun...@samsung.com
 Reviewed-by: Sylwester Nawrockis.nawro...@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 000..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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 13/13] V4L: Add driver for s5k4e5 image sensor

2013-09-05 Thread Sylwester Nawrocki

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 Karun...@samsung.com
Reviewed-by: Sylwester Nawrockis.nawro...@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 000..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@1313 {
+   s5k4e5@20 {
+   compatible = samsung,s5k4e5;
+   reg =0x20;
+   gpios =gpx1 2 1;
+   clock-frequency =2400;
+   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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 13/13] V4L: Add driver for s5k4e5 image sensor

2013-09-05 Thread Arun Kumar K
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.nawro...@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 Karun...@samsung.com
 Reviewed-by: Sylwester Nawrockis.nawro...@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 000..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@1313 {
 +   s5k4e5@20 {
 +   compatible = samsung,s5k4e5;
 +   reg =0x20;
 +   gpios =gpx1 2 1;
 +   clock-frequency =2400;
 +   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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 13/13] V4L: Add driver for s5k4e5 image sensor

2013-08-21 Thread Hans Verkuil
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...@samsung.com
 Reviewed-by: Sylwester Nawrocki s.nawro...@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 000..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...@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_HEIGHT732
 +
 +#define S5K4E5_DRV_NAME  S5K4E5
 +#define S5K4E5_CLK_NAME  mclk
 +
 +#define S5K4E5_NUM_SUPPLIES  2
 +
 +#define S5K4E5_DEF_CLK_FREQ  2400
 +
 +/**
 + * 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 13/13] V4L: Add driver for s5k4e5 image sensor

2013-08-21 Thread Tomasz Figa
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...@samsung.com
  Reviewed-by: Sylwester Nawrocki s.nawro...@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 000..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...@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_WIDTH2576
  +#define S5K4E5_SENSOR_MAX_HEIGHT   1930
  +
  +#define S5K4E5_SENSOR_ACTIVE_WIDTH 2560
  +#define S5K4E5_SENSOR_ACTIVE_HEIGHT1920
  +
  +#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_NAMES5K4E5
  +#define S5K4E5_CLK_NAMEmclk
  +
  +#define S5K4E5_NUM_SUPPLIES2
  +
  +#define S5K4E5_DEF_CLK_FREQ2400
  +
  +/**
  + * 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 13/13] V4L: Add driver for s5k4e5 image sensor

2013-08-21 Thread Hans Verkuil
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...@samsung.com
   Reviewed-by: Sylwester Nawrocki s.nawro...@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 000..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...@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_HEIGHT732
   +
   +#define S5K4E5_DRV_NAME  S5K4E5
   +#define S5K4E5_CLK_NAME  mclk
   +
   +#define S5K4E5_NUM_SUPPLIES  2
   +
   +#define S5K4E5_DEF_CLK_FREQ  2400
   +
   +/**
   + * 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 13/13] V4L: Add driver for s5k4e5 image sensor

2013-08-21 Thread Arun Kumar K
Hi Hans,

On Wed, Aug 21, 2013 at 1:54 PM, Hans Verkuil hverk...@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...@samsung.com
   Reviewed-by: Sylwester Nawrocki s.nawro...@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 000..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...@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_HEIGHT732
   +
   +#define S5K4E5_DRV_NAME  S5K4E5
   +#define S5K4E5_CLK_NAME  mclk
   +
   +#define S5K4E5_NUM_SUPPLIES  2
   +
   +#define S5K4E5_DEF_CLK_FREQ  2400
   +
   +/**
   + * 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html