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

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

 




Hi Hans,

On Wed, Aug 21, 2013 at 1:54 PM, Hans Verkuil <hverkuil@xxxxxxxxx> 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@xxxxxxxxxxx>
>> > > Reviewed-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
>> > > ---
>> > >
>> > >  .../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@xxxxxxxxxxx>
>> > > + *
>> > > + * 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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux