* [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver
@ 2018-12-22 17:12 Akinobu Mita
2018-12-22 17:12 ` [PATCH 01/12] media: i2c: mt9m001: copy mt9m001 soc_camera " Akinobu Mita
` (12 more replies)
0 siblings, 13 replies; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Rob Herring, Guennadi Liakhovetski, Sakari Ailus,
Mauro Carvalho Chehab
This patchset converts soc_camera mt9m001 driver to a standard subdev
sensor driver.
Akinobu Mita (12):
media: i2c: mt9m001: copy mt9m001 soc_camera sensor driver
media: i2c: mt9m001: dt: add binding for mt9m001
media: mt9m001: convert to SPDX license identifer
media: mt9m001: add of_match_table
media: mt9m001: introduce multi_reg_write()
media: mt9m001: switch s_power callback to runtime PM
media: mt9m001: remove remaining soc_camera specific code
media: mt9m001: add media controller support
media: mt9m001: register to V4L2 asynchronous subdevice framework
media: mt9m001: support log_status ioctl and event interface
media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with
V4L2_SUBDEV_FORMAT_TRY
media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls
.../devicetree/bindings/media/i2c/mt9m001.txt | 37 +
drivers/media/i2c/Kconfig | 9 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/mt9m001.c | 900 +++++++++++++++++++++
4 files changed, 947 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9m001.txt
create mode 100644 drivers/media/i2c/mt9m001.c
Cc: Rob Herring <robh@kernel.org>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 01/12] media: i2c: mt9m001: copy mt9m001 soc_camera sensor driver
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 02/12] media: i2c: mt9m001: dt: add binding for mt9m001 Akinobu Mita
` (11 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
Copy the soc_camera based driver in v4l2 sensor driver directory.
This commit just copies the original file without modifying it.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/Kconfig | 7 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/mt9m001.c | 757 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 765 insertions(+)
create mode 100644 drivers/media/i2c/mt9m001.c
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index d933f68..0efc038 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -843,6 +843,13 @@ config VIDEO_VS6624
To compile this driver as a module, choose M here: the
module will be called vs6624.
+config VIDEO_MT9M001
+ tristate "mt9m001 support"
+ depends on SOC_CAMERA && I2C
+ help
+ This driver supports MT9M001 cameras from Micron, monochrome
+ and colour models.
+
config VIDEO_MT9M032
tristate "MT9M032 camera sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 04cbbfa..5806bd1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_VIDEO_OV7740) += ov7740.o
obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
obj-$(CONFIG_VIDEO_OV9655) += ov9655.o
obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
+obj-$(CONFIG_VIDEO_MT9M001) += mt9m001.o
obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o
obj-$(CONFIG_VIDEO_MT9M111) += mt9m111.o
obj-$(CONFIG_VIDEO_MT9P031) += mt9p031.o
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
new file mode 100644
index 0000000..a1a85ff
--- /dev/null
+++ b/drivers/media/i2c/mt9m001.c
@@ -0,0 +1,757 @@
+/*
+ * Driver for MT9M001 CMOS Image Sensor from Micron
+ *
+ * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
+ *
+ * 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/videodev2.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/log2.h>
+#include <linux/module.h>
+
+#include <media/soc_camera.h>
+#include <media/drv-intf/soc_mediabus.h>
+#include <media/v4l2-clk.h>
+#include <media/v4l2-subdev.h>
+#include <media/v4l2-ctrls.h>
+
+/*
+ * mt9m001 i2c address 0x5d
+ * The platform has to define struct i2c_board_info objects and link to them
+ * from struct soc_camera_host_desc
+ */
+
+/* mt9m001 selected register addresses */
+#define MT9M001_CHIP_VERSION 0x00
+#define MT9M001_ROW_START 0x01
+#define MT9M001_COLUMN_START 0x02
+#define MT9M001_WINDOW_HEIGHT 0x03
+#define MT9M001_WINDOW_WIDTH 0x04
+#define MT9M001_HORIZONTAL_BLANKING 0x05
+#define MT9M001_VERTICAL_BLANKING 0x06
+#define MT9M001_OUTPUT_CONTROL 0x07
+#define MT9M001_SHUTTER_WIDTH 0x09
+#define MT9M001_FRAME_RESTART 0x0b
+#define MT9M001_SHUTTER_DELAY 0x0c
+#define MT9M001_RESET 0x0d
+#define MT9M001_READ_OPTIONS1 0x1e
+#define MT9M001_READ_OPTIONS2 0x20
+#define MT9M001_GLOBAL_GAIN 0x35
+#define MT9M001_CHIP_ENABLE 0xF1
+
+#define MT9M001_MAX_WIDTH 1280
+#define MT9M001_MAX_HEIGHT 1024
+#define MT9M001_MIN_WIDTH 48
+#define MT9M001_MIN_HEIGHT 32
+#define MT9M001_COLUMN_SKIP 20
+#define MT9M001_ROW_SKIP 12
+
+/* MT9M001 has only one fixed colorspace per pixelcode */
+struct mt9m001_datafmt {
+ u32 code;
+ enum v4l2_colorspace colorspace;
+};
+
+/* Find a data format by a pixel code in an array */
+static const struct mt9m001_datafmt *mt9m001_find_datafmt(
+ u32 code, const struct mt9m001_datafmt *fmt,
+ int n)
+{
+ int i;
+ for (i = 0; i < n; i++)
+ if (fmt[i].code == code)
+ return fmt + i;
+
+ return NULL;
+}
+
+static const struct mt9m001_datafmt mt9m001_colour_fmts[] = {
+ /*
+ * Order important: first natively supported,
+ * second supported with a GPIO extender
+ */
+ {MEDIA_BUS_FMT_SBGGR10_1X10, V4L2_COLORSPACE_SRGB},
+ {MEDIA_BUS_FMT_SBGGR8_1X8, V4L2_COLORSPACE_SRGB},
+};
+
+static const struct mt9m001_datafmt mt9m001_monochrome_fmts[] = {
+ /* Order important - see above */
+ {MEDIA_BUS_FMT_Y10_1X10, V4L2_COLORSPACE_JPEG},
+ {MEDIA_BUS_FMT_Y8_1X8, V4L2_COLORSPACE_JPEG},
+};
+
+struct mt9m001 {
+ struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
+ struct {
+ /* exposure/auto-exposure cluster */
+ struct v4l2_ctrl *autoexposure;
+ struct v4l2_ctrl *exposure;
+ };
+ struct v4l2_rect rect; /* Sensor window */
+ struct v4l2_clk *clk;
+ const struct mt9m001_datafmt *fmt;
+ const struct mt9m001_datafmt *fmts;
+ int num_fmts;
+ unsigned int total_h;
+ unsigned short y_skip_top; /* Lines to skip at the top */
+};
+
+static struct mt9m001 *to_mt9m001(const struct i2c_client *client)
+{
+ return container_of(i2c_get_clientdata(client), struct mt9m001, subdev);
+}
+
+static int reg_read(struct i2c_client *client, const u8 reg)
+{
+ return i2c_smbus_read_word_swapped(client, reg);
+}
+
+static int reg_write(struct i2c_client *client, const u8 reg,
+ const u16 data)
+{
+ return i2c_smbus_write_word_swapped(client, reg, data);
+}
+
+static int reg_set(struct i2c_client *client, const u8 reg,
+ const u16 data)
+{
+ int ret;
+
+ ret = reg_read(client, reg);
+ if (ret < 0)
+ return ret;
+ return reg_write(client, reg, ret | data);
+}
+
+static int reg_clear(struct i2c_client *client, const u8 reg,
+ const u16 data)
+{
+ int ret;
+
+ ret = reg_read(client, reg);
+ if (ret < 0)
+ return ret;
+ return reg_write(client, reg, ret & ~data);
+}
+
+static int mt9m001_init(struct i2c_client *client)
+{
+ int ret;
+
+ dev_dbg(&client->dev, "%s\n", __func__);
+
+ /*
+ * We don't know, whether platform provides reset, issue a soft reset
+ * too. This returns all registers to their default values.
+ */
+ ret = reg_write(client, MT9M001_RESET, 1);
+ if (!ret)
+ ret = reg_write(client, MT9M001_RESET, 0);
+
+ /* Disable chip, synchronous option update */
+ if (!ret)
+ ret = reg_write(client, MT9M001_OUTPUT_CONTROL, 0);
+
+ return ret;
+}
+
+static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+ /* Switch to master "normal" mode or stop sensor readout */
+ if (reg_write(client, MT9M001_OUTPUT_CONTROL, enable ? 2 : 0) < 0)
+ return -EIO;
+ return 0;
+}
+
+static int mt9m001_set_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_selection *sel)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ struct v4l2_rect rect = sel->r;
+ const u16 hblank = 9, vblank = 25;
+ int ret;
+
+ if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE ||
+ sel->target != V4L2_SEL_TGT_CROP)
+ return -EINVAL;
+
+ if (mt9m001->fmts == mt9m001_colour_fmts)
+ /*
+ * Bayer format - even number of rows for simplicity,
+ * but let the user play with the top row.
+ */
+ rect.height = ALIGN(rect.height, 2);
+
+ /* Datasheet requirement: see register description */
+ rect.width = ALIGN(rect.width, 2);
+ rect.left = ALIGN(rect.left, 2);
+
+ soc_camera_limit_side(&rect.left, &rect.width,
+ MT9M001_COLUMN_SKIP, MT9M001_MIN_WIDTH, MT9M001_MAX_WIDTH);
+
+ soc_camera_limit_side(&rect.top, &rect.height,
+ MT9M001_ROW_SKIP, MT9M001_MIN_HEIGHT, MT9M001_MAX_HEIGHT);
+
+ mt9m001->total_h = rect.height + mt9m001->y_skip_top + vblank;
+
+ /* Blanking and start values - default... */
+ ret = reg_write(client, MT9M001_HORIZONTAL_BLANKING, hblank);
+ if (!ret)
+ ret = reg_write(client, MT9M001_VERTICAL_BLANKING, vblank);
+
+ /*
+ * The caller provides a supported format, as verified per
+ * call to .set_fmt(FORMAT_TRY).
+ */
+ if (!ret)
+ ret = reg_write(client, MT9M001_COLUMN_START, rect.left);
+ if (!ret)
+ ret = reg_write(client, MT9M001_ROW_START, rect.top);
+ if (!ret)
+ ret = reg_write(client, MT9M001_WINDOW_WIDTH, rect.width - 1);
+ if (!ret)
+ ret = reg_write(client, MT9M001_WINDOW_HEIGHT,
+ rect.height + mt9m001->y_skip_top - 1);
+ if (!ret && v4l2_ctrl_g_ctrl(mt9m001->autoexposure) == V4L2_EXPOSURE_AUTO)
+ ret = reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h);
+
+ if (!ret)
+ mt9m001->rect = rect;
+
+ return ret;
+}
+
+static int mt9m001_get_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_selection *sel)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+
+ if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+ return -EINVAL;
+
+ switch (sel->target) {
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ sel->r.left = MT9M001_COLUMN_SKIP;
+ sel->r.top = MT9M001_ROW_SKIP;
+ sel->r.width = MT9M001_MAX_WIDTH;
+ sel->r.height = MT9M001_MAX_HEIGHT;
+ return 0;
+ case V4L2_SEL_TGT_CROP:
+ sel->r = mt9m001->rect;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mt9m001_get_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ struct v4l2_mbus_framefmt *mf = &format->format;
+
+ if (format->pad)
+ return -EINVAL;
+
+ mf->width = mt9m001->rect.width;
+ mf->height = mt9m001->rect.height;
+ mf->code = mt9m001->fmt->code;
+ mf->colorspace = mt9m001->fmt->colorspace;
+ mf->field = V4L2_FIELD_NONE;
+
+ return 0;
+}
+
+static int mt9m001_s_fmt(struct v4l2_subdev *sd,
+ const struct mt9m001_datafmt *fmt,
+ struct v4l2_mbus_framefmt *mf)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ struct v4l2_subdev_selection sel = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .target = V4L2_SEL_TGT_CROP,
+ .r.left = mt9m001->rect.left,
+ .r.top = mt9m001->rect.top,
+ .r.width = mf->width,
+ .r.height = mf->height,
+ };
+ int ret;
+
+ /* No support for scaling so far, just crop. TODO: use skipping */
+ ret = mt9m001_set_selection(sd, NULL, &sel);
+ if (!ret) {
+ mf->width = mt9m001->rect.width;
+ mf->height = mt9m001->rect.height;
+ mt9m001->fmt = fmt;
+ mf->colorspace = fmt->colorspace;
+ }
+
+ return ret;
+}
+
+static int mt9m001_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+ struct v4l2_mbus_framefmt *mf = &format->format;
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ const struct mt9m001_datafmt *fmt;
+
+ if (format->pad)
+ return -EINVAL;
+
+ v4l_bound_align_image(&mf->width, MT9M001_MIN_WIDTH,
+ MT9M001_MAX_WIDTH, 1,
+ &mf->height, MT9M001_MIN_HEIGHT + mt9m001->y_skip_top,
+ MT9M001_MAX_HEIGHT + mt9m001->y_skip_top, 0, 0);
+
+ if (mt9m001->fmts == mt9m001_colour_fmts)
+ mf->height = ALIGN(mf->height - 1, 2);
+
+ fmt = mt9m001_find_datafmt(mf->code, mt9m001->fmts,
+ mt9m001->num_fmts);
+ if (!fmt) {
+ fmt = mt9m001->fmt;
+ mf->code = fmt->code;
+ }
+
+ mf->colorspace = fmt->colorspace;
+
+ if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+ return mt9m001_s_fmt(sd, fmt, mf);
+ cfg->try_fmt = *mf;
+ return 0;
+}
+
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+static int mt9m001_g_register(struct v4l2_subdev *sd,
+ struct v4l2_dbg_register *reg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+ if (reg->reg > 0xff)
+ return -EINVAL;
+
+ reg->size = 2;
+ reg->val = reg_read(client, reg->reg);
+
+ if (reg->val > 0xffff)
+ return -EIO;
+
+ return 0;
+}
+
+static int mt9m001_s_register(struct v4l2_subdev *sd,
+ const struct v4l2_dbg_register *reg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+ if (reg->reg > 0xff)
+ return -EINVAL;
+
+ if (reg_write(client, reg->reg, reg->val) < 0)
+ return -EIO;
+
+ return 0;
+}
+#endif
+
+static int mt9m001_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+
+ return soc_camera_set_power(&client->dev, ssdd, mt9m001->clk, on);
+}
+
+static int mt9m001_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct mt9m001 *mt9m001 = container_of(ctrl->handler,
+ struct mt9m001, hdl);
+ s32 min, max;
+
+ switch (ctrl->id) {
+ case V4L2_CID_EXPOSURE_AUTO:
+ min = mt9m001->exposure->minimum;
+ max = mt9m001->exposure->maximum;
+ mt9m001->exposure->val =
+ (524 + (mt9m001->total_h - 1) * (max - min)) / 1048 + min;
+ break;
+ }
+ return 0;
+}
+
+static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct mt9m001 *mt9m001 = container_of(ctrl->handler,
+ struct mt9m001, hdl);
+ struct v4l2_subdev *sd = &mt9m001->subdev;
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct v4l2_ctrl *exp = mt9m001->exposure;
+ int data;
+
+ switch (ctrl->id) {
+ case V4L2_CID_VFLIP:
+ if (ctrl->val)
+ data = reg_set(client, MT9M001_READ_OPTIONS2, 0x8000);
+ else
+ data = reg_clear(client, MT9M001_READ_OPTIONS2, 0x8000);
+ if (data < 0)
+ return -EIO;
+ return 0;
+
+ case V4L2_CID_GAIN:
+ /* See Datasheet Table 7, Gain settings. */
+ if (ctrl->val <= ctrl->default_value) {
+ /* Pack it into 0..1 step 0.125, register values 0..8 */
+ unsigned long range = ctrl->default_value - ctrl->minimum;
+ data = ((ctrl->val - (s32)ctrl->minimum) * 8 + range / 2) / range;
+
+ dev_dbg(&client->dev, "Setting gain %d\n", data);
+ data = reg_write(client, MT9M001_GLOBAL_GAIN, data);
+ if (data < 0)
+ return -EIO;
+ } else {
+ /* Pack it into 1.125..15 variable step, register values 9..67 */
+ /* We assume qctrl->maximum - qctrl->default_value - 1 > 0 */
+ unsigned long range = ctrl->maximum - ctrl->default_value - 1;
+ unsigned long gain = ((ctrl->val - (s32)ctrl->default_value - 1) *
+ 111 + range / 2) / range + 9;
+
+ if (gain <= 32)
+ data = gain;
+ else if (gain <= 64)
+ data = ((gain - 32) * 16 + 16) / 32 + 80;
+ else
+ data = ((gain - 64) * 7 + 28) / 56 + 96;
+
+ dev_dbg(&client->dev, "Setting gain from %d to %d\n",
+ reg_read(client, MT9M001_GLOBAL_GAIN), data);
+ data = reg_write(client, MT9M001_GLOBAL_GAIN, data);
+ if (data < 0)
+ return -EIO;
+ }
+ return 0;
+
+ case V4L2_CID_EXPOSURE_AUTO:
+ if (ctrl->val == V4L2_EXPOSURE_MANUAL) {
+ unsigned long range = exp->maximum - exp->minimum;
+ unsigned long shutter = ((exp->val - (s32)exp->minimum) * 1048 +
+ range / 2) / range + 1;
+
+ dev_dbg(&client->dev,
+ "Setting shutter width from %d to %lu\n",
+ reg_read(client, MT9M001_SHUTTER_WIDTH), shutter);
+ if (reg_write(client, MT9M001_SHUTTER_WIDTH, shutter) < 0)
+ return -EIO;
+ } else {
+ const u16 vblank = 25;
+
+ mt9m001->total_h = mt9m001->rect.height +
+ mt9m001->y_skip_top + vblank;
+ if (reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h) < 0)
+ return -EIO;
+ }
+ return 0;
+ }
+ return -EINVAL;
+}
+
+/*
+ * Interface active, can use i2c. If it fails, it can indeed mean, that
+ * this wasn't our capture interface, so, we wait for the right one
+ */
+static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
+ struct i2c_client *client)
+{
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ s32 data;
+ unsigned long flags;
+ int ret;
+
+ ret = mt9m001_s_power(&mt9m001->subdev, 1);
+ if (ret < 0)
+ return ret;
+
+ /* Enable the chip */
+ data = reg_write(client, MT9M001_CHIP_ENABLE, 1);
+ dev_dbg(&client->dev, "write: %d\n", data);
+
+ /* Read out the chip version register */
+ data = reg_read(client, MT9M001_CHIP_VERSION);
+
+ /* must be 0x8411 or 0x8421 for colour sensor and 8431 for bw */
+ switch (data) {
+ case 0x8411:
+ case 0x8421:
+ mt9m001->fmts = mt9m001_colour_fmts;
+ break;
+ case 0x8431:
+ mt9m001->fmts = mt9m001_monochrome_fmts;
+ break;
+ default:
+ dev_err(&client->dev,
+ "No MT9M001 chip detected, register read %x\n", data);
+ ret = -ENODEV;
+ goto done;
+ }
+
+ mt9m001->num_fmts = 0;
+
+ /*
+ * This is a 10bit sensor, so by default we only allow 10bit.
+ * The platform may support different bus widths due to
+ * different routing of the data lines.
+ */
+ if (ssdd->query_bus_param)
+ flags = ssdd->query_bus_param(ssdd);
+ else
+ flags = SOCAM_DATAWIDTH_10;
+
+ if (flags & SOCAM_DATAWIDTH_10)
+ mt9m001->num_fmts++;
+ else
+ mt9m001->fmts++;
+
+ if (flags & SOCAM_DATAWIDTH_8)
+ mt9m001->num_fmts++;
+
+ mt9m001->fmt = &mt9m001->fmts[0];
+
+ dev_info(&client->dev, "Detected a MT9M001 chip ID %x (%s)\n", data,
+ data == 0x8431 ? "C12STM" : "C12ST");
+
+ ret = mt9m001_init(client);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to initialise the camera\n");
+ goto done;
+ }
+
+ /* mt9m001_init() has reset the chip, returning registers to defaults */
+ ret = v4l2_ctrl_handler_setup(&mt9m001->hdl);
+
+done:
+ mt9m001_s_power(&mt9m001->subdev, 0);
+ return ret;
+}
+
+static void mt9m001_video_remove(struct soc_camera_subdev_desc *ssdd)
+{
+ if (ssdd->free_bus)
+ ssdd->free_bus(ssdd);
+}
+
+static int mt9m001_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+
+ *lines = mt9m001->y_skip_top;
+
+ return 0;
+}
+
+static const struct v4l2_ctrl_ops mt9m001_ctrl_ops = {
+ .g_volatile_ctrl = mt9m001_g_volatile_ctrl,
+ .s_ctrl = mt9m001_s_ctrl,
+};
+
+static const struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+ .g_register = mt9m001_g_register,
+ .s_register = mt9m001_s_register,
+#endif
+ .s_power = mt9m001_s_power,
+};
+
+static int mt9m001_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+
+ if (code->pad || code->index >= mt9m001->num_fmts)
+ return -EINVAL;
+
+ code->code = mt9m001->fmts[code->index].code;
+ return 0;
+}
+
+static int mt9m001_g_mbus_config(struct v4l2_subdev *sd,
+ struct v4l2_mbus_config *cfg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+
+ /* MT9M001 has all capture_format parameters fixed */
+ cfg->flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
+ V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
+ V4L2_MBUS_DATA_ACTIVE_HIGH | V4L2_MBUS_MASTER;
+ cfg->type = V4L2_MBUS_PARALLEL;
+ cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
+
+ return 0;
+}
+
+static int mt9m001_s_mbus_config(struct v4l2_subdev *sd,
+ const struct v4l2_mbus_config *cfg)
+{
+ const struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ unsigned int bps = soc_mbus_get_fmtdesc(mt9m001->fmt->code)->bits_per_sample;
+
+ if (ssdd->set_bus_param)
+ return ssdd->set_bus_param(ssdd, 1 << (bps - 1));
+
+ /*
+ * Without board specific bus width settings we only support the
+ * sensors native bus width
+ */
+ return bps == 10 ? 0 : -EINVAL;
+}
+
+static const struct v4l2_subdev_video_ops mt9m001_subdev_video_ops = {
+ .s_stream = mt9m001_s_stream,
+ .g_mbus_config = mt9m001_g_mbus_config,
+ .s_mbus_config = mt9m001_s_mbus_config,
+};
+
+static const struct v4l2_subdev_sensor_ops mt9m001_subdev_sensor_ops = {
+ .g_skip_top_lines = mt9m001_g_skip_top_lines,
+};
+
+static const struct v4l2_subdev_pad_ops mt9m001_subdev_pad_ops = {
+ .enum_mbus_code = mt9m001_enum_mbus_code,
+ .get_selection = mt9m001_get_selection,
+ .set_selection = mt9m001_set_selection,
+ .get_fmt = mt9m001_get_fmt,
+ .set_fmt = mt9m001_set_fmt,
+};
+
+static const struct v4l2_subdev_ops mt9m001_subdev_ops = {
+ .core = &mt9m001_subdev_core_ops,
+ .video = &mt9m001_subdev_video_ops,
+ .sensor = &mt9m001_subdev_sensor_ops,
+ .pad = &mt9m001_subdev_pad_ops,
+};
+
+static int mt9m001_probe(struct i2c_client *client,
+ const struct i2c_device_id *did)
+{
+ struct mt9m001 *mt9m001;
+ struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+ struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ int ret;
+
+ if (!ssdd) {
+ dev_err(&client->dev, "MT9M001 driver needs platform data\n");
+ return -EINVAL;
+ }
+
+ if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
+ dev_warn(&adapter->dev,
+ "I2C-Adapter doesn't support I2C_FUNC_SMBUS_WORD\n");
+ return -EIO;
+ }
+
+ mt9m001 = devm_kzalloc(&client->dev, sizeof(struct mt9m001), GFP_KERNEL);
+ if (!mt9m001)
+ return -ENOMEM;
+
+ v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
+ v4l2_ctrl_handler_init(&mt9m001->hdl, 4);
+ v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+ V4L2_CID_GAIN, 0, 127, 1, 64);
+ mt9m001->exposure = v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+ V4L2_CID_EXPOSURE, 1, 255, 1, 255);
+ /*
+ * Simulated autoexposure. If enabled, we calculate shutter width
+ * ourselves in the driver based on vertical blanking and frame width
+ */
+ mt9m001->autoexposure = v4l2_ctrl_new_std_menu(&mt9m001->hdl,
+ &mt9m001_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+ V4L2_EXPOSURE_AUTO);
+ mt9m001->subdev.ctrl_handler = &mt9m001->hdl;
+ if (mt9m001->hdl.error)
+ return mt9m001->hdl.error;
+
+ v4l2_ctrl_auto_cluster(2, &mt9m001->autoexposure,
+ V4L2_EXPOSURE_MANUAL, true);
+
+ /* Second stage probe - when a capture adapter is there */
+ mt9m001->y_skip_top = 0;
+ mt9m001->rect.left = MT9M001_COLUMN_SKIP;
+ mt9m001->rect.top = MT9M001_ROW_SKIP;
+ mt9m001->rect.width = MT9M001_MAX_WIDTH;
+ mt9m001->rect.height = MT9M001_MAX_HEIGHT;
+
+ mt9m001->clk = v4l2_clk_get(&client->dev, "mclk");
+ if (IS_ERR(mt9m001->clk)) {
+ ret = PTR_ERR(mt9m001->clk);
+ goto eclkget;
+ }
+
+ ret = mt9m001_video_probe(ssdd, client);
+ if (ret) {
+ v4l2_clk_put(mt9m001->clk);
+eclkget:
+ v4l2_ctrl_handler_free(&mt9m001->hdl);
+ }
+
+ return ret;
+}
+
+static int mt9m001_remove(struct i2c_client *client)
+{
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+
+ v4l2_clk_put(mt9m001->clk);
+ v4l2_device_unregister_subdev(&mt9m001->subdev);
+ v4l2_ctrl_handler_free(&mt9m001->hdl);
+ mt9m001_video_remove(ssdd);
+
+ return 0;
+}
+
+static const struct i2c_device_id mt9m001_id[] = {
+ { "mt9m001", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mt9m001_id);
+
+static struct i2c_driver mt9m001_i2c_driver = {
+ .driver = {
+ .name = "mt9m001",
+ },
+ .probe = mt9m001_probe,
+ .remove = mt9m001_remove,
+ .id_table = mt9m001_id,
+};
+
+module_i2c_driver(mt9m001_i2c_driver);
+
+MODULE_DESCRIPTION("Micron MT9M001 Camera driver");
+MODULE_AUTHOR("Guennadi Liakhovetski <kernel@pengutronix.de>");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 02/12] media: i2c: mt9m001: dt: add binding for mt9m001
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
2018-12-22 17:12 ` [PATCH 01/12] media: i2c: mt9m001: copy mt9m001 soc_camera " Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2019-01-03 21:12 ` Rob Herring
2018-12-22 17:12 ` [PATCH 03/12] media: mt9m001: convert to SPDX license identifer Akinobu Mita
` (10 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Rob Herring, Guennadi Liakhovetski, Sakari Ailus,
Mauro Carvalho Chehab
Add device tree binding documentation for the MT9M001 CMOS image sensor.
Cc: Rob Herring <robh@kernel.org>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
.../devicetree/bindings/media/i2c/mt9m001.txt | 37 ++++++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9m001.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9m001.txt b/Documentation/devicetree/bindings/media/i2c/mt9m001.txt
new file mode 100644
index 0000000..794b787
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/mt9m001.txt
@@ -0,0 +1,37 @@
+MT9M001: 1/2-Inch Megapixel Digital Image Sensor
+
+The MT9M001 is an SXGA-format with a 1/2-inch CMOS active-pixel digital
+image sensor. It is programmable through a simple two-wire serial
+interface.
+
+Required Properties:
+
+- compatible: shall be "onnn,mt9m001".
+- clocks: reference to the master clock into sensor
+
+Optional Properties:
+
+- reset-gpios: GPIO handle which is connected to the reset pin of the chip.
+ Active low.
+- standby-gpios: GPIO handle which is connected to the standby pin of the chip.
+ Active high.
+
+For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+ &i2c1 {
+ mt9m001@5d {
+ compatible = "onnn,mt9m001";
+ reg = <0x5d>;
+ reset-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
+ standby-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+ clocks = <&camera_clk>;
+ port {
+ mt9m001_out: endpoint {
+ remote-endpoint = <&vcap_in>;
+ };
+ };
+ };
+ };
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 03/12] media: mt9m001: convert to SPDX license identifer
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
2018-12-22 17:12 ` [PATCH 01/12] media: i2c: mt9m001: copy mt9m001 soc_camera " Akinobu Mita
2018-12-22 17:12 ` [PATCH 02/12] media: i2c: mt9m001: dt: add binding for mt9m001 Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2019-01-07 11:37 ` Sakari Ailus
2018-12-22 17:12 ` [PATCH 04/12] media: mt9m001: add of_match_table Akinobu Mita
` (9 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
Replace GPL license statements with SPDX license identifiers (GPL-2.0).
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/mt9m001.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index a1a85ff..65ff59d 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -1,11 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Driver for MT9M001 CMOS Image Sensor from Micron
*
* Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
- *
- * 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/videodev2.h>
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 04/12] media: mt9m001: add of_match_table
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (2 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 03/12] media: mt9m001: convert to SPDX license identifer Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 05/12] media: mt9m001: introduce multi_reg_write() Akinobu Mita
` (8 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
Add of_match_table for the MT9M001 CMOS image sensor.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/mt9m001.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index 65ff59d..2d800ca 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -738,9 +738,16 @@ static const struct i2c_device_id mt9m001_id[] = {
};
MODULE_DEVICE_TABLE(i2c, mt9m001_id);
+static const struct of_device_id mt9m001_of_match[] = {
+ { .compatible = "onnn,mt9m001", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mt9m001_of_match);
+
static struct i2c_driver mt9m001_i2c_driver = {
.driver = {
.name = "mt9m001",
+ .of_match_table = mt9m001_of_match,
},
.probe = mt9m001_probe,
.remove = mt9m001_remove,
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 05/12] media: mt9m001: introduce multi_reg_write()
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (3 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 04/12] media: mt9m001: add of_match_table Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 06/12] media: mt9m001: switch s_power callback to runtime PM Akinobu Mita
` (7 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
Introduce multi_reg_write() to write multiple registers to the device and
use it where possible.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/mt9m001.c | 88 +++++++++++++++++++++++++++++----------------
1 file changed, 57 insertions(+), 31 deletions(-)
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index 2d800ca..c0180fdc 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -47,6 +47,8 @@
#define MT9M001_MIN_HEIGHT 32
#define MT9M001_COLUMN_SKIP 20
#define MT9M001_ROW_SKIP 12
+#define MT9M001_DEFAULT_HBLANK 9
+#define MT9M001_DEFAULT_VBLANK 25
/* MT9M001 has only one fixed colorspace per pixelcode */
struct mt9m001_datafmt {
@@ -137,25 +139,65 @@ static int reg_clear(struct i2c_client *client, const u8 reg,
return reg_write(client, reg, ret & ~data);
}
+struct mt9m001_reg {
+ u8 reg;
+ u16 data;
+};
+
+static int multi_reg_write(struct i2c_client *client,
+ const struct mt9m001_reg *regs, int num)
+{
+ int i;
+
+ for (i = 0; i < num; i++) {
+ int ret = reg_write(client, regs[i].reg, regs[i].data);
+
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int mt9m001_init(struct i2c_client *client)
{
- int ret;
+ const struct mt9m001_reg init_regs[] = {
+ /*
+ * Issue a soft reset. This returns all registers to their
+ * default values.
+ */
+ { MT9M001_RESET, 1 },
+ { MT9M001_RESET, 0 },
+ /* Disable chip, synchronous option update */
+ { MT9M001_OUTPUT_CONTROL, 0 }
+ };
dev_dbg(&client->dev, "%s\n", __func__);
- /*
- * We don't know, whether platform provides reset, issue a soft reset
- * too. This returns all registers to their default values.
- */
- ret = reg_write(client, MT9M001_RESET, 1);
- if (!ret)
- ret = reg_write(client, MT9M001_RESET, 0);
+ return multi_reg_write(client, init_regs, ARRAY_SIZE(init_regs));
+}
- /* Disable chip, synchronous option update */
- if (!ret)
- ret = reg_write(client, MT9M001_OUTPUT_CONTROL, 0);
+static int mt9m001_apply_selection(struct v4l2_subdev *sd,
+ struct v4l2_rect *rect)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ const struct mt9m001_reg regs[] = {
+ /* Blanking and start values - default... */
+ { MT9M001_HORIZONTAL_BLANKING, MT9M001_DEFAULT_HBLANK },
+ { MT9M001_VERTICAL_BLANKING, MT9M001_DEFAULT_VBLANK },
+ /*
+ * The caller provides a supported format, as verified per
+ * call to .set_fmt(FORMAT_TRY).
+ */
+ { MT9M001_COLUMN_START, rect->left },
+ { MT9M001_ROW_START, rect->top },
+ { MT9M001_WINDOW_WIDTH, rect->width - 1 },
+ { MT9M001_WINDOW_HEIGHT,
+ rect->height + mt9m001->y_skip_top - 1 },
+ };
- return ret;
+ return multi_reg_write(client, regs, ARRAY_SIZE(regs));
}
static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
@@ -175,7 +217,6 @@ static int mt9m001_set_selection(struct v4l2_subdev *sd,
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct mt9m001 *mt9m001 = to_mt9m001(client);
struct v4l2_rect rect = sel->r;
- const u16 hblank = 9, vblank = 25;
int ret;
if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE ||
@@ -199,26 +240,11 @@ static int mt9m001_set_selection(struct v4l2_subdev *sd,
soc_camera_limit_side(&rect.top, &rect.height,
MT9M001_ROW_SKIP, MT9M001_MIN_HEIGHT, MT9M001_MAX_HEIGHT);
- mt9m001->total_h = rect.height + mt9m001->y_skip_top + vblank;
+ mt9m001->total_h = rect.height + mt9m001->y_skip_top +
+ MT9M001_DEFAULT_VBLANK;
- /* Blanking and start values - default... */
- ret = reg_write(client, MT9M001_HORIZONTAL_BLANKING, hblank);
- if (!ret)
- ret = reg_write(client, MT9M001_VERTICAL_BLANKING, vblank);
- /*
- * The caller provides a supported format, as verified per
- * call to .set_fmt(FORMAT_TRY).
- */
- if (!ret)
- ret = reg_write(client, MT9M001_COLUMN_START, rect.left);
- if (!ret)
- ret = reg_write(client, MT9M001_ROW_START, rect.top);
- if (!ret)
- ret = reg_write(client, MT9M001_WINDOW_WIDTH, rect.width - 1);
- if (!ret)
- ret = reg_write(client, MT9M001_WINDOW_HEIGHT,
- rect.height + mt9m001->y_skip_top - 1);
+ ret = mt9m001_apply_selection(sd, &rect);
if (!ret && v4l2_ctrl_g_ctrl(mt9m001->autoexposure) == V4L2_EXPOSURE_AUTO)
ret = reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h);
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 06/12] media: mt9m001: switch s_power callback to runtime PM
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (4 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 05/12] media: mt9m001: introduce multi_reg_write() Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
[not found] ` <20190107100034.po3jsnc3rdj37l4x@kekkonen.localdomain>
2018-12-22 17:12 ` [PATCH 07/12] media: mt9m001: remove remaining soc_camera specific code Akinobu Mita
` (6 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
Switch s_power() callback to runtime PM framework. This also removes
soc_camera specific power management code and introduces reset and standby
gpios instead.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/mt9m001.c | 242 ++++++++++++++++++++++++++++++++------------
1 file changed, 178 insertions(+), 64 deletions(-)
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index c0180fdc..f20188a 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -5,6 +5,10 @@
* Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
*/
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/pm_runtime.h>
#include <linux/videodev2.h>
#include <linux/slab.h>
#include <linux/i2c.h>
@@ -13,7 +17,6 @@
#include <media/soc_camera.h>
#include <media/drv-intf/soc_mediabus.h>
-#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-ctrls.h>
@@ -92,8 +95,12 @@ struct mt9m001 {
struct v4l2_ctrl *autoexposure;
struct v4l2_ctrl *exposure;
};
+ bool streaming;
+ struct mutex mutex;
struct v4l2_rect rect; /* Sensor window */
- struct v4l2_clk *clk;
+ struct clk *clk;
+ struct gpio_desc *standby_gpio;
+ struct gpio_desc *reset_gpio;
const struct mt9m001_datafmt *fmt;
const struct mt9m001_datafmt *fmts;
int num_fmts;
@@ -177,8 +184,7 @@ static int mt9m001_init(struct i2c_client *client)
return multi_reg_write(client, init_regs, ARRAY_SIZE(init_regs));
}
-static int mt9m001_apply_selection(struct v4l2_subdev *sd,
- struct v4l2_rect *rect)
+static int mt9m001_apply_selection(struct v4l2_subdev *sd)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct mt9m001 *mt9m001 = to_mt9m001(client);
@@ -190,11 +196,11 @@ static int mt9m001_apply_selection(struct v4l2_subdev *sd,
* The caller provides a supported format, as verified per
* call to .set_fmt(FORMAT_TRY).
*/
- { MT9M001_COLUMN_START, rect->left },
- { MT9M001_ROW_START, rect->top },
- { MT9M001_WINDOW_WIDTH, rect->width - 1 },
+ { MT9M001_COLUMN_START, mt9m001->rect.left },
+ { MT9M001_ROW_START, mt9m001->rect.top },
+ { MT9M001_WINDOW_WIDTH, mt9m001->rect.width - 1 },
{ MT9M001_WINDOW_HEIGHT,
- rect->height + mt9m001->y_skip_top - 1 },
+ mt9m001->rect.height + mt9m001->y_skip_top - 1 },
};
return multi_reg_write(client, regs, ARRAY_SIZE(regs));
@@ -203,11 +209,50 @@ static int mt9m001_apply_selection(struct v4l2_subdev *sd,
static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ int ret = 0;
- /* Switch to master "normal" mode or stop sensor readout */
- if (reg_write(client, MT9M001_OUTPUT_CONTROL, enable ? 2 : 0) < 0)
- return -EIO;
- return 0;
+ mutex_lock(&mt9m001->mutex);
+
+ if (mt9m001->streaming == enable)
+ goto done;
+
+ if (enable) {
+ ret = pm_runtime_get_sync(&client->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(&client->dev);
+ goto done;
+ }
+
+ ret = mt9m001_apply_selection(sd);
+ if (ret) {
+ pm_runtime_put(&client->dev);
+ goto done;
+ }
+
+ ret = __v4l2_ctrl_handler_setup(&mt9m001->hdl);
+ if (ret) {
+ pm_runtime_put(&client->dev);
+ goto done;
+ }
+
+ /* Switch to master "normal" mode */
+ ret = reg_write(client, MT9M001_OUTPUT_CONTROL, 2);
+ if (ret < 0) {
+ pm_runtime_put(&client->dev);
+ goto done;
+ }
+ } else {
+ /* Switch to master stop sensor readout */
+ reg_write(client, MT9M001_OUTPUT_CONTROL, 0);
+ pm_runtime_put(&client->dev);
+ }
+
+ mt9m001->streaming = enable;
+done:
+ mutex_unlock(&mt9m001->mutex);
+
+ return ret;
}
static int mt9m001_set_selection(struct v4l2_subdev *sd,
@@ -217,7 +262,6 @@ static int mt9m001_set_selection(struct v4l2_subdev *sd,
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct mt9m001 *mt9m001 = to_mt9m001(client);
struct v4l2_rect rect = sel->r;
- int ret;
if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE ||
sel->target != V4L2_SEL_TGT_CROP)
@@ -243,15 +287,9 @@ static int mt9m001_set_selection(struct v4l2_subdev *sd,
mt9m001->total_h = rect.height + mt9m001->y_skip_top +
MT9M001_DEFAULT_VBLANK;
+ mt9m001->rect = rect;
- ret = mt9m001_apply_selection(sd, &rect);
- if (!ret && v4l2_ctrl_g_ctrl(mt9m001->autoexposure) == V4L2_EXPOSURE_AUTO)
- ret = reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h);
-
- if (!ret)
- mt9m001->rect = rect;
-
- return ret;
+ return 0;
}
static int mt9m001_get_selection(struct v4l2_subdev *sd,
@@ -395,13 +433,34 @@ static int mt9m001_s_register(struct v4l2_subdev *sd,
}
#endif
-static int mt9m001_s_power(struct v4l2_subdev *sd, int on)
+static int mt9m001_power_on(struct mt9m001 *mt9m001)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
- struct mt9m001 *mt9m001 = to_mt9m001(client);
+ int ret = clk_prepare_enable(mt9m001->clk);
+
+ if (ret)
+ return ret;
+
+ if (mt9m001->standby_gpio) {
+ gpiod_set_value_cansleep(mt9m001->standby_gpio, 0);
+ usleep_range(1000, 2000);
+ }
+
+ if (mt9m001->reset_gpio) {
+ gpiod_set_value_cansleep(mt9m001->reset_gpio, 1);
+ usleep_range(1000, 2000);
+ gpiod_set_value_cansleep(mt9m001->reset_gpio, 0);
+ usleep_range(1000, 2000);
+ }
- return soc_camera_set_power(&client->dev, ssdd, mt9m001->clk, on);
+ return 0;
+}
+
+static int mt9m001_power_off(struct mt9m001 *mt9m001)
+{
+ gpiod_set_value_cansleep(mt9m001->standby_gpio, 1);
+ clk_disable_unprepare(mt9m001->clk);
+
+ return 0;
}
static int mt9m001_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -429,16 +488,18 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct v4l2_ctrl *exp = mt9m001->exposure;
int data;
+ int ret;
+
+ if (!pm_runtime_get_if_in_use(&client->dev))
+ return 0;
switch (ctrl->id) {
case V4L2_CID_VFLIP:
if (ctrl->val)
- data = reg_set(client, MT9M001_READ_OPTIONS2, 0x8000);
+ ret = reg_set(client, MT9M001_READ_OPTIONS2, 0x8000);
else
- data = reg_clear(client, MT9M001_READ_OPTIONS2, 0x8000);
- if (data < 0)
- return -EIO;
- return 0;
+ ret = reg_clear(client, MT9M001_READ_OPTIONS2, 0x8000);
+ break;
case V4L2_CID_GAIN:
/* See Datasheet Table 7, Gain settings. */
@@ -448,9 +509,7 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
data = ((ctrl->val - (s32)ctrl->minimum) * 8 + range / 2) / range;
dev_dbg(&client->dev, "Setting gain %d\n", data);
- data = reg_write(client, MT9M001_GLOBAL_GAIN, data);
- if (data < 0)
- return -EIO;
+ ret = reg_write(client, MT9M001_GLOBAL_GAIN, data);
} else {
/* Pack it into 1.125..15 variable step, register values 9..67 */
/* We assume qctrl->maximum - qctrl->default_value - 1 > 0 */
@@ -467,11 +526,9 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
dev_dbg(&client->dev, "Setting gain from %d to %d\n",
reg_read(client, MT9M001_GLOBAL_GAIN), data);
- data = reg_write(client, MT9M001_GLOBAL_GAIN, data);
- if (data < 0)
- return -EIO;
+ ret = reg_write(client, MT9M001_GLOBAL_GAIN, data);
}
- return 0;
+ break;
case V4L2_CID_EXPOSURE_AUTO:
if (ctrl->val == V4L2_EXPOSURE_MANUAL) {
@@ -482,19 +539,22 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
dev_dbg(&client->dev,
"Setting shutter width from %d to %lu\n",
reg_read(client, MT9M001_SHUTTER_WIDTH), shutter);
- if (reg_write(client, MT9M001_SHUTTER_WIDTH, shutter) < 0)
- return -EIO;
+ ret = reg_write(client, MT9M001_SHUTTER_WIDTH, shutter);
} else {
- const u16 vblank = 25;
-
mt9m001->total_h = mt9m001->rect.height +
- mt9m001->y_skip_top + vblank;
- if (reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h) < 0)
- return -EIO;
+ mt9m001->y_skip_top + MT9M001_DEFAULT_VBLANK;
+ ret = reg_write(client, MT9M001_SHUTTER_WIDTH,
+ mt9m001->total_h);
}
- return 0;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
}
- return -EINVAL;
+
+ pm_runtime_put(&client->dev);
+
+ return ret;
}
/*
@@ -509,10 +569,6 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
unsigned long flags;
int ret;
- ret = mt9m001_s_power(&mt9m001->subdev, 1);
- if (ret < 0)
- return ret;
-
/* Enable the chip */
data = reg_write(client, MT9M001_CHIP_ENABLE, 1);
dev_dbg(&client->dev, "write: %d\n", data);
@@ -571,7 +627,6 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
ret = v4l2_ctrl_handler_setup(&mt9m001->hdl);
done:
- mt9m001_s_power(&mt9m001->subdev, 0);
return ret;
}
@@ -601,7 +656,6 @@ static const struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
.g_register = mt9m001_g_register,
.s_register = mt9m001_s_register,
#endif
- .s_power = mt9m001_s_power,
};
static int mt9m001_enum_mbus_code(struct v4l2_subdev *sd,
@@ -700,6 +754,20 @@ static int mt9m001_probe(struct i2c_client *client,
if (!mt9m001)
return -ENOMEM;
+ mt9m001->clk = devm_clk_get(&client->dev, NULL);
+ if (IS_ERR(mt9m001->clk))
+ return PTR_ERR(mt9m001->clk);
+
+ mt9m001->standby_gpio = devm_gpiod_get_optional(&client->dev, "standby",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(mt9m001->standby_gpio))
+ return PTR_ERR(mt9m001->standby_gpio);
+
+ mt9m001->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(mt9m001->reset_gpio))
+ return PTR_ERR(mt9m001->reset_gpio);
+
v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
v4l2_ctrl_handler_init(&mt9m001->hdl, 4);
v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
@@ -722,6 +790,9 @@ static int mt9m001_probe(struct i2c_client *client,
v4l2_ctrl_auto_cluster(2, &mt9m001->autoexposure,
V4L2_EXPOSURE_MANUAL, true);
+ mutex_init(&mt9m001->mutex);
+ mt9m001->hdl.lock = &mt9m001->mutex;
+
/* Second stage probe - when a capture adapter is there */
mt9m001->y_skip_top = 0;
mt9m001->rect.left = MT9M001_COLUMN_SKIP;
@@ -729,18 +800,30 @@ static int mt9m001_probe(struct i2c_client *client,
mt9m001->rect.width = MT9M001_MAX_WIDTH;
mt9m001->rect.height = MT9M001_MAX_HEIGHT;
- mt9m001->clk = v4l2_clk_get(&client->dev, "mclk");
- if (IS_ERR(mt9m001->clk)) {
- ret = PTR_ERR(mt9m001->clk);
- goto eclkget;
- }
+ ret = mt9m001_power_on(mt9m001);
+ if (ret)
+ goto error_hdl_free;
+
+ pm_runtime_get_noresume(&client->dev);
+ pm_runtime_set_active(&client->dev);
+ pm_runtime_enable(&client->dev);
ret = mt9m001_video_probe(ssdd, client);
- if (ret) {
- v4l2_clk_put(mt9m001->clk);
-eclkget:
- v4l2_ctrl_handler_free(&mt9m001->hdl);
- }
+ if (ret)
+ goto error_power_off;
+
+ pm_runtime_put_sync(&client->dev);
+
+ return 0;
+
+error_power_off:
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+ pm_runtime_put_noidle(&client->dev);
+ mt9m001_power_off(mt9m001);
+error_hdl_free:
+ v4l2_ctrl_handler_free(&mt9m001->hdl);
+ mutex_destroy(&mt9m001->mutex);
return ret;
}
@@ -750,10 +833,17 @@ static int mt9m001_remove(struct i2c_client *client)
struct mt9m001 *mt9m001 = to_mt9m001(client);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
- v4l2_clk_put(mt9m001->clk);
v4l2_device_unregister_subdev(&mt9m001->subdev);
+ pm_runtime_get_sync(&client->dev);
+
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+ pm_runtime_put_noidle(&client->dev);
+ mt9m001_power_off(mt9m001);
+
v4l2_ctrl_handler_free(&mt9m001->hdl);
mt9m001_video_remove(ssdd);
+ mutex_destroy(&mt9m001->mutex);
return 0;
}
@@ -764,6 +854,29 @@ static const struct i2c_device_id mt9m001_id[] = {
};
MODULE_DEVICE_TABLE(i2c, mt9m001_id);
+static int __maybe_unused mt9m001_runtime_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+
+ return mt9m001_power_on(mt9m001);
+}
+
+static int __maybe_unused mt9m001_runtime_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+
+ mt9m001_power_off(mt9m001);
+
+ return 0;
+}
+
+static const struct dev_pm_ops mt9m001_pm_ops = {
+ SET_RUNTIME_PM_OPS(mt9m001_runtime_suspend,
+ mt9m001_runtime_resume, NULL)
+};
+
static const struct of_device_id mt9m001_of_match[] = {
{ .compatible = "onnn,mt9m001", },
{ /* sentinel */ },
@@ -773,6 +886,7 @@ MODULE_DEVICE_TABLE(of, mt9m001_of_match);
static struct i2c_driver mt9m001_i2c_driver = {
.driver = {
.name = "mt9m001",
+ .pm = &mt9m001_pm_ops,
.of_match_table = mt9m001_of_match,
},
.probe = mt9m001_probe,
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 07/12] media: mt9m001: remove remaining soc_camera specific code
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (5 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 06/12] media: mt9m001: switch s_power callback to runtime PM Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2019-01-07 11:29 ` Sakari Ailus
2018-12-22 17:12 ` [PATCH 08/12] media: mt9m001: add media controller support Akinobu Mita
` (5 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
Remove remaining soc_camera specific code and drop soc_camera dependency
from this driver.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/Kconfig | 2 +-
drivers/media/i2c/mt9m001.c | 84 ++++++++-------------------------------------
2 files changed, 15 insertions(+), 71 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 0efc038..4bdf043 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -845,7 +845,7 @@ config VIDEO_VS6624
config VIDEO_MT9M001
tristate "mt9m001 support"
- depends on SOC_CAMERA && I2C
+ depends on I2C && VIDEO_V4L2
help
This driver supports MT9M001 cameras from Micron, monochrome
and colour models.
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index f20188a..eb5c4ed 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -15,15 +15,12 @@
#include <linux/log2.h>
#include <linux/module.h>
-#include <media/soc_camera.h>
-#include <media/drv-intf/soc_mediabus.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
/*
* mt9m001 i2c address 0x5d
- * The platform has to define struct i2c_board_info objects and link to them
- * from struct soc_camera_host_desc
*/
/* mt9m001 selected register addresses */
@@ -278,11 +275,15 @@ static int mt9m001_set_selection(struct v4l2_subdev *sd,
rect.width = ALIGN(rect.width, 2);
rect.left = ALIGN(rect.left, 2);
- soc_camera_limit_side(&rect.left, &rect.width,
- MT9M001_COLUMN_SKIP, MT9M001_MIN_WIDTH, MT9M001_MAX_WIDTH);
+ rect.width = clamp_t(u32, rect.width, MT9M001_MIN_WIDTH,
+ MT9M001_MAX_WIDTH);
+ rect.left = clamp_t(u32, rect.left, MT9M001_COLUMN_SKIP,
+ MT9M001_COLUMN_SKIP + MT9M001_MAX_WIDTH - rect.width);
- soc_camera_limit_side(&rect.top, &rect.height,
- MT9M001_ROW_SKIP, MT9M001_MIN_HEIGHT, MT9M001_MAX_HEIGHT);
+ rect.height = clamp_t(u32, rect.height, MT9M001_MIN_HEIGHT,
+ MT9M001_MAX_HEIGHT);
+ rect.top = clamp_t(u32, rect.top, MT9M001_ROW_SKIP,
+ MT9M001_ROW_SKIP + MT9M001_MAX_HEIGHT - rect.width);
mt9m001->total_h = rect.height + mt9m001->y_skip_top +
MT9M001_DEFAULT_VBLANK;
@@ -561,12 +562,10 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
* Interface active, can use i2c. If it fails, it can indeed mean, that
* this wasn't our capture interface, so, we wait for the right one
*/
-static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
- struct i2c_client *client)
+static int mt9m001_video_probe(struct i2c_client *client)
{
struct mt9m001 *mt9m001 = to_mt9m001(client);
s32 data;
- unsigned long flags;
int ret;
/* Enable the chip */
@@ -581,9 +580,11 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
case 0x8411:
case 0x8421:
mt9m001->fmts = mt9m001_colour_fmts;
+ mt9m001->num_fmts = ARRAY_SIZE(mt9m001_colour_fmts);
break;
case 0x8431:
mt9m001->fmts = mt9m001_monochrome_fmts;
+ mt9m001->num_fmts = ARRAY_SIZE(mt9m001_monochrome_fmts);
break;
default:
dev_err(&client->dev,
@@ -592,26 +593,6 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
goto done;
}
- mt9m001->num_fmts = 0;
-
- /*
- * This is a 10bit sensor, so by default we only allow 10bit.
- * The platform may support different bus widths due to
- * different routing of the data lines.
- */
- if (ssdd->query_bus_param)
- flags = ssdd->query_bus_param(ssdd);
- else
- flags = SOCAM_DATAWIDTH_10;
-
- if (flags & SOCAM_DATAWIDTH_10)
- mt9m001->num_fmts++;
- else
- mt9m001->fmts++;
-
- if (flags & SOCAM_DATAWIDTH_8)
- mt9m001->num_fmts++;
-
mt9m001->fmt = &mt9m001->fmts[0];
dev_info(&client->dev, "Detected a MT9M001 chip ID %x (%s)\n", data,
@@ -630,12 +611,6 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
return ret;
}
-static void mt9m001_video_remove(struct soc_camera_subdev_desc *ssdd)
-{
- if (ssdd->free_bus)
- ssdd->free_bus(ssdd);
-}
-
static int mt9m001_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -675,41 +650,18 @@ static int mt9m001_enum_mbus_code(struct v4l2_subdev *sd,
static int mt9m001_g_mbus_config(struct v4l2_subdev *sd,
struct v4l2_mbus_config *cfg)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
-
/* MT9M001 has all capture_format parameters fixed */
cfg->flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
V4L2_MBUS_DATA_ACTIVE_HIGH | V4L2_MBUS_MASTER;
cfg->type = V4L2_MBUS_PARALLEL;
- cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
return 0;
}
-static int mt9m001_s_mbus_config(struct v4l2_subdev *sd,
- const struct v4l2_mbus_config *cfg)
-{
- const struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
- struct mt9m001 *mt9m001 = to_mt9m001(client);
- unsigned int bps = soc_mbus_get_fmtdesc(mt9m001->fmt->code)->bits_per_sample;
-
- if (ssdd->set_bus_param)
- return ssdd->set_bus_param(ssdd, 1 << (bps - 1));
-
- /*
- * Without board specific bus width settings we only support the
- * sensors native bus width
- */
- return bps == 10 ? 0 : -EINVAL;
-}
-
static const struct v4l2_subdev_video_ops mt9m001_subdev_video_ops = {
.s_stream = mt9m001_s_stream,
.g_mbus_config = mt9m001_g_mbus_config,
- .s_mbus_config = mt9m001_s_mbus_config,
};
static const struct v4l2_subdev_sensor_ops mt9m001_subdev_sensor_ops = {
@@ -736,21 +688,15 @@ static int mt9m001_probe(struct i2c_client *client,
{
struct mt9m001 *mt9m001;
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
- struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
int ret;
- if (!ssdd) {
- dev_err(&client->dev, "MT9M001 driver needs platform data\n");
- return -EINVAL;
- }
-
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
dev_warn(&adapter->dev,
"I2C-Adapter doesn't support I2C_FUNC_SMBUS_WORD\n");
return -EIO;
}
- mt9m001 = devm_kzalloc(&client->dev, sizeof(struct mt9m001), GFP_KERNEL);
+ mt9m001 = devm_kzalloc(&client->dev, sizeof(*mt9m001), GFP_KERNEL);
if (!mt9m001)
return -ENOMEM;
@@ -808,7 +754,7 @@ static int mt9m001_probe(struct i2c_client *client,
pm_runtime_set_active(&client->dev);
pm_runtime_enable(&client->dev);
- ret = mt9m001_video_probe(ssdd, client);
+ ret = mt9m001_video_probe(client);
if (ret)
goto error_power_off;
@@ -831,7 +777,6 @@ static int mt9m001_probe(struct i2c_client *client,
static int mt9m001_remove(struct i2c_client *client)
{
struct mt9m001 *mt9m001 = to_mt9m001(client);
- struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
v4l2_device_unregister_subdev(&mt9m001->subdev);
pm_runtime_get_sync(&client->dev);
@@ -842,7 +787,6 @@ static int mt9m001_remove(struct i2c_client *client)
mt9m001_power_off(mt9m001);
v4l2_ctrl_handler_free(&mt9m001->hdl);
- mt9m001_video_remove(ssdd);
mutex_destroy(&mt9m001->mutex);
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 08/12] media: mt9m001: add media controller support
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (6 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 07/12] media: mt9m001: remove remaining soc_camera specific code Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 09/12] media: mt9m001: register to V4L2 asynchronous subdevice framework Akinobu Mita
` (4 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
Create a source pad and set the media controller type to the sensor.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/Kconfig | 2 ++
drivers/media/i2c/mt9m001.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 4bdf043..5e30ad3 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -846,6 +846,8 @@ config VIDEO_VS6624
config VIDEO_MT9M001
tristate "mt9m001 support"
depends on I2C && VIDEO_V4L2
+ depends on MEDIA_CAMERA_SUPPORT
+ depends on MEDIA_CONTROLLER
help
This driver supports MT9M001 cameras from Micron, monochrome
and colour models.
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index eb5c4ed..e31fb7d 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -103,6 +103,7 @@ struct mt9m001 {
int num_fmts;
unsigned int total_h;
unsigned short y_skip_top; /* Lines to skip at the top */
+ struct media_pad pad;
};
static struct mt9m001 *to_mt9m001(const struct i2c_client *client)
@@ -758,6 +759,12 @@ static int mt9m001_probe(struct i2c_client *client,
if (ret)
goto error_power_off;
+ mt9m001->pad.flags = MEDIA_PAD_FL_SOURCE;
+ mt9m001->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(&mt9m001->subdev.entity, 1, &mt9m001->pad);
+ if (ret)
+ goto error_power_off;
+
pm_runtime_put_sync(&client->dev);
return 0;
@@ -781,6 +788,8 @@ static int mt9m001_remove(struct i2c_client *client)
v4l2_device_unregister_subdev(&mt9m001->subdev);
pm_runtime_get_sync(&client->dev);
+ media_entity_cleanup(&mt9m001->subdev.entity);
+
pm_runtime_disable(&client->dev);
pm_runtime_set_suspended(&client->dev);
pm_runtime_put_noidle(&client->dev);
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 09/12] media: mt9m001: register to V4L2 asynchronous subdevice framework
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (7 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 08/12] media: mt9m001: add media controller support Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2019-01-07 11:27 ` Sakari Ailus
2018-12-22 17:12 ` [PATCH 10/12] media: mt9m001: support log_status ioctl and event interface Akinobu Mita
` (3 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
Register a sub-device to the asynchronous subdevice framework, and also
create subdevice device node.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/Kconfig | 2 +-
drivers/media/i2c/mt9m001.c | 9 ++++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 5e30ad3..a6d8416 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -845,7 +845,7 @@ config VIDEO_VS6624
config VIDEO_MT9M001
tristate "mt9m001 support"
- depends on I2C && VIDEO_V4L2
+ depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
depends on MEDIA_CONTROLLER
help
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index e31fb7d..b4deec3 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -716,6 +716,7 @@ static int mt9m001_probe(struct i2c_client *client,
return PTR_ERR(mt9m001->reset_gpio);
v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
+ mt9m001->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
v4l2_ctrl_handler_init(&mt9m001->hdl, 4);
v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
V4L2_CID_VFLIP, 0, 1, 1, 0);
@@ -765,10 +766,16 @@ static int mt9m001_probe(struct i2c_client *client,
if (ret)
goto error_power_off;
+ ret = v4l2_async_register_subdev(&mt9m001->subdev);
+ if (ret)
+ goto error_entity_cleanup;
+
pm_runtime_put_sync(&client->dev);
return 0;
+error_entity_cleanup:
+ media_entity_cleanup(&mt9m001->subdev.entity);
error_power_off:
pm_runtime_disable(&client->dev);
pm_runtime_set_suspended(&client->dev);
@@ -785,9 +792,9 @@ static int mt9m001_remove(struct i2c_client *client)
{
struct mt9m001 *mt9m001 = to_mt9m001(client);
- v4l2_device_unregister_subdev(&mt9m001->subdev);
pm_runtime_get_sync(&client->dev);
+ v4l2_async_unregister_subdev(&mt9m001->subdev);
media_entity_cleanup(&mt9m001->subdev.entity);
pm_runtime_disable(&client->dev);
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 10/12] media: mt9m001: support log_status ioctl and event interface
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (8 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 09/12] media: mt9m001: register to V4L2 asynchronous subdevice framework Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 11/12] media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY Akinobu Mita
` (2 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
This adds log_status ioctl and event interface for mt9m001's v4l2 controls.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/mt9m001.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index b4deec3..a5b94d7 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -18,6 +18,7 @@
#include <media/v4l2-subdev.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
/*
* mt9m001 i2c address 0x5d
@@ -628,6 +629,9 @@ static const struct v4l2_ctrl_ops mt9m001_ctrl_ops = {
};
static const struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
+ .log_status = v4l2_ctrl_subdev_log_status,
+ .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+ .unsubscribe_event = v4l2_event_subdev_unsubscribe,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = mt9m001_g_register,
.s_register = mt9m001_s_register,
@@ -716,7 +720,8 @@ static int mt9m001_probe(struct i2c_client *client,
return PTR_ERR(mt9m001->reset_gpio);
v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
- mt9m001->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+ mt9m001->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE |
+ V4L2_SUBDEV_FL_HAS_EVENTS;
v4l2_ctrl_handler_init(&mt9m001->hdl, 4);
v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
V4L2_CID_VFLIP, 0, 1, 1, 0);
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 11/12] media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (9 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 10/12] media: mt9m001: support log_status ioctl and event interface Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2019-01-07 11:30 ` Sakari Ailus
2018-12-22 17:12 ` [PATCH 12/12] media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls Akinobu Mita
2019-01-07 9:36 ` [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Sakari Ailus
12 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
The VIDIOC_SUBDEV_G_FMT ioctl for this driver doesn't recognize
V4L2_SUBDEV_FORMAT_TRY and always works as if V4L2_SUBDEV_FORMAT_ACTIVE
is specified.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/mt9m001.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index a5b94d7..f4afbc9 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -331,6 +331,12 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd,
if (format->pad)
return -EINVAL;
+ if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+ mf = v4l2_subdev_get_try_format(sd, cfg, 0);
+ format->format = *mf;
+ return 0;
+ }
+
mf->width = mt9m001->rect.width;
mf->height = mt9m001->rect.height;
mf->code = mt9m001->fmt->code;
@@ -638,6 +644,26 @@ static const struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
#endif
};
+static int mt9m001_init_cfg(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
+ struct v4l2_mbus_framefmt *try_fmt =
+ v4l2_subdev_get_try_format(sd, cfg, 0);
+
+ try_fmt->width = mt9m001->rect.width;
+ try_fmt->height = mt9m001->rect.height;
+ try_fmt->code = mt9m001->fmt->code;
+ try_fmt->colorspace = mt9m001->fmt->colorspace;
+ try_fmt->field = V4L2_FIELD_NONE;
+ try_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ try_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
+ try_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+ return 0;
+}
+
static int mt9m001_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
@@ -674,6 +700,7 @@ static const struct v4l2_subdev_sensor_ops mt9m001_subdev_sensor_ops = {
};
static const struct v4l2_subdev_pad_ops mt9m001_subdev_pad_ops = {
+ .init_cfg = mt9m001_init_cfg,
.enum_mbus_code = mt9m001_enum_mbus_code,
.get_selection = mt9m001_get_selection,
.set_selection = mt9m001_set_selection,
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 12/12] media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (10 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 11/12] media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY Akinobu Mita
@ 2018-12-22 17:12 ` Akinobu Mita
2019-01-07 11:32 ` Sakari Ailus
2019-01-07 9:36 ` [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Sakari Ailus
12 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2018-12-22 17:12 UTC (permalink / raw)
To: linux-media, devicetree
Cc: Akinobu Mita, Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
This driver doesn't set all members of mbus format field when the
VIDIOC_SUBDEV_{S,G}_FMT ioctls are called.
This is detected by v4l2-compliance.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/media/i2c/mt9m001.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index f4afbc9..82b89d5 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -342,6 +342,9 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd,
mf->code = mt9m001->fmt->code;
mf->colorspace = mt9m001->fmt->colorspace;
mf->field = V4L2_FIELD_NONE;
+ mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ mf->quantization = V4L2_QUANTIZATION_DEFAULT;
+ mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
return 0;
}
@@ -402,6 +405,10 @@ static int mt9m001_set_fmt(struct v4l2_subdev *sd,
}
mf->colorspace = fmt->colorspace;
+ mf->field = V4L2_FIELD_NONE;
+ mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ mf->quantization = V4L2_QUANTIZATION_DEFAULT;
+ mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
return mt9m001_s_fmt(sd, fmt, mf);
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 02/12] media: i2c: mt9m001: dt: add binding for mt9m001
2018-12-22 17:12 ` [PATCH 02/12] media: i2c: mt9m001: dt: add binding for mt9m001 Akinobu Mita
@ 2019-01-03 21:12 ` Rob Herring
2019-01-05 15:09 ` Akinobu Mita
0 siblings, 1 reply; 31+ messages in thread
From: Rob Herring @ 2019-01-03 21:12 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, devicetree, Guennadi Liakhovetski, Sakari Ailus,
Mauro Carvalho Chehab
On Sun, Dec 23, 2018 at 02:12:44AM +0900, Akinobu Mita wrote:
> Add device tree binding documentation for the MT9M001 CMOS image sensor.
>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> .../devicetree/bindings/media/i2c/mt9m001.txt | 37 ++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9m001.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/mt9m001.txt b/Documentation/devicetree/bindings/media/i2c/mt9m001.txt
> new file mode 100644
> index 0000000..794b787
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/mt9m001.txt
> @@ -0,0 +1,37 @@
> +MT9M001: 1/2-Inch Megapixel Digital Image Sensor
> +
> +The MT9M001 is an SXGA-format with a 1/2-inch CMOS active-pixel digital
> +image sensor. It is programmable through a simple two-wire serial
> +interface.
I2C?
> +
> +Required Properties:
> +
> +- compatible: shall be "onnn,mt9m001".
> +- clocks: reference to the master clock into sensor
> +
> +Optional Properties:
> +
> +- reset-gpios: GPIO handle which is connected to the reset pin of the chip.
> + Active low.
> +- standby-gpios: GPIO handle which is connected to the standby pin of the chip.
> + Active high.
> +
> +For further reading on port node refer to
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
You still need to state how many ports/endpoints and what they are.
> +
> +Example:
> +
> + &i2c1 {
> + mt9m001@5d {
camera-sensor@5d
> + compatible = "onnn,mt9m001";
> + reg = <0x5d>;
> + reset-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
> + standby-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
> + clocks = <&camera_clk>;
> + port {
> + mt9m001_out: endpoint {
> + remote-endpoint = <&vcap_in>;
> + };
> + };
> + };
> + };
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 02/12] media: i2c: mt9m001: dt: add binding for mt9m001
2019-01-03 21:12 ` Rob Herring
@ 2019-01-05 15:09 ` Akinobu Mita
0 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2019-01-05 15:09 UTC (permalink / raw)
To: Rob Herring
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Sakari Ailus, Mauro Carvalho Chehab
2019年1月4日(金) 6:12 Rob Herring <robh@kernel.org>:
>
> On Sun, Dec 23, 2018 at 02:12:44AM +0900, Akinobu Mita wrote:
> > Add device tree binding documentation for the MT9M001 CMOS image sensor.
> >
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> > .../devicetree/bindings/media/i2c/mt9m001.txt | 37 ++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9m001.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/mt9m001.txt b/Documentation/devicetree/bindings/media/i2c/mt9m001.txt
> > new file mode 100644
> > index 0000000..794b787
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/mt9m001.txt
> > @@ -0,0 +1,37 @@
> > +MT9M001: 1/2-Inch Megapixel Digital Image Sensor
> > +
> > +The MT9M001 is an SXGA-format with a 1/2-inch CMOS active-pixel digital
> > +image sensor. It is programmable through a simple two-wire serial
> > +interface.
>
> I2C?
Sounds good.
> > +
> > +Required Properties:
> > +
> > +- compatible: shall be "onnn,mt9m001".
> > +- clocks: reference to the master clock into sensor
> > +
> > +Optional Properties:
> > +
> > +- reset-gpios: GPIO handle which is connected to the reset pin of the chip.
> > + Active low.
> > +- standby-gpios: GPIO handle which is connected to the standby pin of the chip.
> > + Active high.
> > +
> > +For further reading on port node refer to
> > +Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> You still need to state how many ports/endpoints and what they are.
I'll write the following description that is copied from
Documentation/devicetree/bindings/media/i2c/mt9m111.txt.
"The device node must contain one 'port' child node with one 'endpoint' child
sub-node for its digital output video port, in accordance with the video
interface bindings defined in:
Documentation/devicetree/bindings/media/video-interfaces.txt"
> > +
> > +Example:
> > +
> > + &i2c1 {
> > + mt9m001@5d {
>
> camera-sensor@5d
OK.
> > + compatible = "onnn,mt9m001";
> > + reg = <0x5d>;
> > + reset-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
> > + standby-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
> > + clocks = <&camera_clk>;
> > + port {
> > + mt9m001_out: endpoint {
> > + remote-endpoint = <&vcap_in>;
> > + };
> > + };
> > + };
> > + };
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
` (11 preceding siblings ...)
2018-12-22 17:12 ` [PATCH 12/12] media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls Akinobu Mita
@ 2019-01-07 9:36 ` Sakari Ailus
2019-01-07 9:48 ` Sakari Ailus
12 siblings, 1 reply; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 9:36 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, devicetree, Rob Herring, Guennadi Liakhovetski,
Mauro Carvalho Chehab
Hi Mita-san,
On Sun, Dec 23, 2018 at 02:12:42AM +0900, Akinobu Mita wrote:
> This patchset converts soc_camera mt9m001 driver to a standard subdev
> sensor driver.
>
> Akinobu Mita (12):
> media: i2c: mt9m001: copy mt9m001 soc_camera sensor driver
> media: i2c: mt9m001: dt: add binding for mt9m001
> media: mt9m001: convert to SPDX license identifer
> media: mt9m001: add of_match_table
> media: mt9m001: introduce multi_reg_write()
> media: mt9m001: switch s_power callback to runtime PM
> media: mt9m001: remove remaining soc_camera specific code
> media: mt9m001: add media controller support
> media: mt9m001: register to V4L2 asynchronous subdevice framework
> media: mt9m001: support log_status ioctl and event interface
> media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with
> V4L2_SUBDEV_FORMAT_TRY
> media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls
>
> .../devicetree/bindings/media/i2c/mt9m001.txt | 37 +
> drivers/media/i2c/Kconfig | 9 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/mt9m001.c | 900 +++++++++++++++++++++
> 4 files changed, 947 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9m001.txt
> create mode 100644 drivers/media/i2c/mt9m001.c
>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
There's something wrong somewhere --- I can't see the set on the LMML.
Could you resend it, please (or v2 to address Rob's comments)? I don't
think the number of recipients is too big, so I hope this was a transient
problem somehere that shouldn't have happened. Hmm.
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver
2019-01-07 9:36 ` [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Sakari Ailus
@ 2019-01-07 9:48 ` Sakari Ailus
0 siblings, 0 replies; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 9:48 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, devicetree, Rob Herring, Guennadi Liakhovetski,
Mauro Carvalho Chehab
On Mon, Jan 07, 2019 at 11:36:03AM +0200, Sakari Ailus wrote:
> Hi Mita-san,
>
> On Sun, Dec 23, 2018 at 02:12:42AM +0900, Akinobu Mita wrote:
> > This patchset converts soc_camera mt9m001 driver to a standard subdev
> > sensor driver.
> >
> > Akinobu Mita (12):
> > media: i2c: mt9m001: copy mt9m001 soc_camera sensor driver
> > media: i2c: mt9m001: dt: add binding for mt9m001
> > media: mt9m001: convert to SPDX license identifer
> > media: mt9m001: add of_match_table
> > media: mt9m001: introduce multi_reg_write()
> > media: mt9m001: switch s_power callback to runtime PM
> > media: mt9m001: remove remaining soc_camera specific code
> > media: mt9m001: add media controller support
> > media: mt9m001: register to V4L2 asynchronous subdevice framework
> > media: mt9m001: support log_status ioctl and event interface
> > media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with
> > V4L2_SUBDEV_FORMAT_TRY
> > media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls
> >
> > .../devicetree/bindings/media/i2c/mt9m001.txt | 37 +
> > drivers/media/i2c/Kconfig | 9 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/mt9m001.c | 900 +++++++++++++++++++++
> > 4 files changed, 947 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9m001.txt
> > create mode 100644 drivers/media/i2c/mt9m001.c
> >
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
>
> There's something wrong somewhere --- I can't see the set on the LMML.
> Could you resend it, please (or v2 to address Rob's comments)? I don't
> think the number of recipients is too big, so I hope this was a transient
> problem somehere that shouldn't have happened. Hmm.
Oh well, I can see my own reply as the set. Please ignore.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 09/12] media: mt9m001: register to V4L2 asynchronous subdevice framework
2018-12-22 17:12 ` [PATCH 09/12] media: mt9m001: register to V4L2 asynchronous subdevice framework Akinobu Mita
@ 2019-01-07 11:27 ` Sakari Ailus
2019-01-07 14:09 ` Akinobu Mita
0 siblings, 1 reply; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 11:27 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, devicetree, Guennadi Liakhovetski, Mauro Carvalho Chehab
Hi Mita-san,
On Sun, Dec 23, 2018 at 02:12:51AM +0900, Akinobu Mita wrote:
> Register a sub-device to the asynchronous subdevice framework, and also
> create subdevice device node.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> drivers/media/i2c/Kconfig | 2 +-
> drivers/media/i2c/mt9m001.c | 9 ++++++++-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 5e30ad3..a6d8416 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -845,7 +845,7 @@ config VIDEO_VS6624
>
> config VIDEO_MT9M001
> tristate "mt9m001 support"
> - depends on I2C && VIDEO_V4L2
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
VIDEO_V4L2_SUBDEV_API depends on MEDIA_CONTROLLER, so MEDIA_CONTROLLER
below can be removed.
> depends on MEDIA_CAMERA_SUPPORT
> depends on MEDIA_CONTROLLER
> help
> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> index e31fb7d..b4deec3 100644
> --- a/drivers/media/i2c/mt9m001.c
> +++ b/drivers/media/i2c/mt9m001.c
> @@ -716,6 +716,7 @@ static int mt9m001_probe(struct i2c_client *client,
> return PTR_ERR(mt9m001->reset_gpio);
>
> v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
> + mt9m001->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
|=
Otherwise you lose flags set by v4l2_i2c_subdev_init().
> v4l2_ctrl_handler_init(&mt9m001->hdl, 4);
> v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
> V4L2_CID_VFLIP, 0, 1, 1, 0);
> @@ -765,10 +766,16 @@ static int mt9m001_probe(struct i2c_client *client,
> if (ret)
> goto error_power_off;
>
> + ret = v4l2_async_register_subdev(&mt9m001->subdev);
> + if (ret)
> + goto error_entity_cleanup;
> +
> pm_runtime_put_sync(&client->dev);
>
> return 0;
>
> +error_entity_cleanup:
> + media_entity_cleanup(&mt9m001->subdev.entity);
> error_power_off:
> pm_runtime_disable(&client->dev);
> pm_runtime_set_suspended(&client->dev);
> @@ -785,9 +792,9 @@ static int mt9m001_remove(struct i2c_client *client)
> {
> struct mt9m001 *mt9m001 = to_mt9m001(client);
>
> - v4l2_device_unregister_subdev(&mt9m001->subdev);
> pm_runtime_get_sync(&client->dev);
>
> + v4l2_async_unregister_subdev(&mt9m001->subdev);
> media_entity_cleanup(&mt9m001->subdev.entity);
>
> pm_runtime_disable(&client->dev);
--
Kind regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/12] media: mt9m001: remove remaining soc_camera specific code
2018-12-22 17:12 ` [PATCH 07/12] media: mt9m001: remove remaining soc_camera specific code Akinobu Mita
@ 2019-01-07 11:29 ` Sakari Ailus
2019-01-07 14:13 ` Akinobu Mita
0 siblings, 1 reply; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 11:29 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, devicetree, Guennadi Liakhovetski, Mauro Carvalho Chehab
Hi Mita-san,
On Sun, Dec 23, 2018 at 02:12:49AM +0900, Akinobu Mita wrote:
> Remove remaining soc_camera specific code and drop soc_camera dependency
> from this driver.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> drivers/media/i2c/Kconfig | 2 +-
> drivers/media/i2c/mt9m001.c | 84 ++++++++-------------------------------------
> 2 files changed, 15 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 0efc038..4bdf043 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -845,7 +845,7 @@ config VIDEO_VS6624
>
> config VIDEO_MT9M001
> tristate "mt9m001 support"
> - depends on SOC_CAMERA && I2C
> + depends on I2C && VIDEO_V4L2
> help
> This driver supports MT9M001 cameras from Micron, monochrome
> and colour models.
> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> index f20188a..eb5c4ed 100644
> --- a/drivers/media/i2c/mt9m001.c
> +++ b/drivers/media/i2c/mt9m001.c
> @@ -15,15 +15,12 @@
> #include <linux/log2.h>
> #include <linux/module.h>
>
> -#include <media/soc_camera.h>
> -#include <media/drv-intf/soc_mediabus.h>
> #include <media/v4l2-subdev.h>
> #include <media/v4l2-ctrls.h>
While at it, could you order the list alphabetically? The same applies to
further patches changing included files.
> +#include <media/v4l2-device.h>
>
> /*
> * mt9m001 i2c address 0x5d
> - * The platform has to define struct i2c_board_info objects and link to them
> - * from struct soc_camera_host_desc
> */
>
> /* mt9m001 selected register addresses */
> @@ -278,11 +275,15 @@ static int mt9m001_set_selection(struct v4l2_subdev *sd,
> rect.width = ALIGN(rect.width, 2);
> rect.left = ALIGN(rect.left, 2);
>
> - soc_camera_limit_side(&rect.left, &rect.width,
> - MT9M001_COLUMN_SKIP, MT9M001_MIN_WIDTH, MT9M001_MAX_WIDTH);
> + rect.width = clamp_t(u32, rect.width, MT9M001_MIN_WIDTH,
> + MT9M001_MAX_WIDTH);
> + rect.left = clamp_t(u32, rect.left, MT9M001_COLUMN_SKIP,
> + MT9M001_COLUMN_SKIP + MT9M001_MAX_WIDTH - rect.width);
>
> - soc_camera_limit_side(&rect.top, &rect.height,
> - MT9M001_ROW_SKIP, MT9M001_MIN_HEIGHT, MT9M001_MAX_HEIGHT);
> + rect.height = clamp_t(u32, rect.height, MT9M001_MIN_HEIGHT,
> + MT9M001_MAX_HEIGHT);
> + rect.top = clamp_t(u32, rect.top, MT9M001_ROW_SKIP,
> + MT9M001_ROW_SKIP + MT9M001_MAX_HEIGHT - rect.width);
>
> mt9m001->total_h = rect.height + mt9m001->y_skip_top +
> MT9M001_DEFAULT_VBLANK;
> @@ -561,12 +562,10 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
> * Interface active, can use i2c. If it fails, it can indeed mean, that
> * this wasn't our capture interface, so, we wait for the right one
> */
> -static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
> - struct i2c_client *client)
> +static int mt9m001_video_probe(struct i2c_client *client)
> {
> struct mt9m001 *mt9m001 = to_mt9m001(client);
> s32 data;
> - unsigned long flags;
> int ret;
>
> /* Enable the chip */
> @@ -581,9 +580,11 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
> case 0x8411:
> case 0x8421:
> mt9m001->fmts = mt9m001_colour_fmts;
> + mt9m001->num_fmts = ARRAY_SIZE(mt9m001_colour_fmts);
> break;
> case 0x8431:
> mt9m001->fmts = mt9m001_monochrome_fmts;
> + mt9m001->num_fmts = ARRAY_SIZE(mt9m001_monochrome_fmts);
> break;
> default:
> dev_err(&client->dev,
> @@ -592,26 +593,6 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
> goto done;
> }
>
> - mt9m001->num_fmts = 0;
> -
> - /*
> - * This is a 10bit sensor, so by default we only allow 10bit.
> - * The platform may support different bus widths due to
> - * different routing of the data lines.
> - */
> - if (ssdd->query_bus_param)
> - flags = ssdd->query_bus_param(ssdd);
> - else
> - flags = SOCAM_DATAWIDTH_10;
> -
> - if (flags & SOCAM_DATAWIDTH_10)
> - mt9m001->num_fmts++;
> - else
> - mt9m001->fmts++;
> -
> - if (flags & SOCAM_DATAWIDTH_8)
> - mt9m001->num_fmts++;
> -
> mt9m001->fmt = &mt9m001->fmts[0];
>
> dev_info(&client->dev, "Detected a MT9M001 chip ID %x (%s)\n", data,
> @@ -630,12 +611,6 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
> return ret;
> }
>
> -static void mt9m001_video_remove(struct soc_camera_subdev_desc *ssdd)
> -{
> - if (ssdd->free_bus)
> - ssdd->free_bus(ssdd);
> -}
> -
> static int mt9m001_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> @@ -675,41 +650,18 @@ static int mt9m001_enum_mbus_code(struct v4l2_subdev *sd,
> static int mt9m001_g_mbus_config(struct v4l2_subdev *sd,
> struct v4l2_mbus_config *cfg)
> {
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> -
> /* MT9M001 has all capture_format parameters fixed */
> cfg->flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> V4L2_MBUS_DATA_ACTIVE_HIGH | V4L2_MBUS_MASTER;
> cfg->type = V4L2_MBUS_PARALLEL;
> - cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
>
> return 0;
> }
>
> -static int mt9m001_s_mbus_config(struct v4l2_subdev *sd,
> - const struct v4l2_mbus_config *cfg)
> -{
> - const struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> - struct mt9m001 *mt9m001 = to_mt9m001(client);
> - unsigned int bps = soc_mbus_get_fmtdesc(mt9m001->fmt->code)->bits_per_sample;
> -
> - if (ssdd->set_bus_param)
> - return ssdd->set_bus_param(ssdd, 1 << (bps - 1));
> -
> - /*
> - * Without board specific bus width settings we only support the
> - * sensors native bus width
> - */
> - return bps == 10 ? 0 : -EINVAL;
> -}
> -
> static const struct v4l2_subdev_video_ops mt9m001_subdev_video_ops = {
> .s_stream = mt9m001_s_stream,
> .g_mbus_config = mt9m001_g_mbus_config,
> - .s_mbus_config = mt9m001_s_mbus_config,
> };
>
> static const struct v4l2_subdev_sensor_ops mt9m001_subdev_sensor_ops = {
> @@ -736,21 +688,15 @@ static int mt9m001_probe(struct i2c_client *client,
> {
> struct mt9m001 *mt9m001;
> struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> int ret;
>
> - if (!ssdd) {
> - dev_err(&client->dev, "MT9M001 driver needs platform data\n");
> - return -EINVAL;
> - }
> -
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
> dev_warn(&adapter->dev,
> "I2C-Adapter doesn't support I2C_FUNC_SMBUS_WORD\n");
> return -EIO;
> }
>
> - mt9m001 = devm_kzalloc(&client->dev, sizeof(struct mt9m001), GFP_KERNEL);
> + mt9m001 = devm_kzalloc(&client->dev, sizeof(*mt9m001), GFP_KERNEL);
> if (!mt9m001)
> return -ENOMEM;
>
> @@ -808,7 +754,7 @@ static int mt9m001_probe(struct i2c_client *client,
> pm_runtime_set_active(&client->dev);
> pm_runtime_enable(&client->dev);
>
> - ret = mt9m001_video_probe(ssdd, client);
> + ret = mt9m001_video_probe(client);
> if (ret)
> goto error_power_off;
>
> @@ -831,7 +777,6 @@ static int mt9m001_probe(struct i2c_client *client,
> static int mt9m001_remove(struct i2c_client *client)
> {
> struct mt9m001 *mt9m001 = to_mt9m001(client);
> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>
> v4l2_device_unregister_subdev(&mt9m001->subdev);
> pm_runtime_get_sync(&client->dev);
> @@ -842,7 +787,6 @@ static int mt9m001_remove(struct i2c_client *client)
> mt9m001_power_off(mt9m001);
>
> v4l2_ctrl_handler_free(&mt9m001->hdl);
> - mt9m001_video_remove(ssdd);
> mutex_destroy(&mt9m001->mutex);
>
> return 0;
> --
> 2.7.4
>
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 11/12] media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY
2018-12-22 17:12 ` [PATCH 11/12] media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY Akinobu Mita
@ 2019-01-07 11:30 ` Sakari Ailus
2019-01-07 14:15 ` Akinobu Mita
0 siblings, 1 reply; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 11:30 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, devicetree, Guennadi Liakhovetski, Mauro Carvalho Chehab
Hi Mita-san,
On Sun, Dec 23, 2018 at 02:12:53AM +0900, Akinobu Mita wrote:
> The VIDIOC_SUBDEV_G_FMT ioctl for this driver doesn't recognize
> V4L2_SUBDEV_FORMAT_TRY and always works as if V4L2_SUBDEV_FORMAT_ACTIVE
> is specified.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> drivers/media/i2c/mt9m001.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> index a5b94d7..f4afbc9 100644
> --- a/drivers/media/i2c/mt9m001.c
> +++ b/drivers/media/i2c/mt9m001.c
> @@ -331,6 +331,12 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd,
> if (format->pad)
> return -EINVAL;
>
> + if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> + mf = v4l2_subdev_get_try_format(sd, cfg, 0);
> + format->format = *mf;
> + return 0;
> + }
> +
> mf->width = mt9m001->rect.width;
> mf->height = mt9m001->rect.height;
> mf->code = mt9m001->fmt->code;
> @@ -638,6 +644,26 @@ static const struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
> #endif
> };
>
> +static int mt9m001_init_cfg(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> + struct mt9m001 *mt9m001 = to_mt9m001(client);
> + struct v4l2_mbus_framefmt *try_fmt =
> + v4l2_subdev_get_try_format(sd, cfg, 0);
> +
> + try_fmt->width = mt9m001->rect.width;
> + try_fmt->height = mt9m001->rect.height;
> + try_fmt->code = mt9m001->fmt->code;
> + try_fmt->colorspace = mt9m001->fmt->colorspace;
The initial configuration set here should reflect the default, not current
configuration. This appears to refer to the current one.
> + try_fmt->field = V4L2_FIELD_NONE;
> + try_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + try_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> + try_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> + return 0;
> +}
> +
> static int mt9m001_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_pad_config *cfg,
> struct v4l2_subdev_mbus_code_enum *code)
> @@ -674,6 +700,7 @@ static const struct v4l2_subdev_sensor_ops mt9m001_subdev_sensor_ops = {
> };
>
> static const struct v4l2_subdev_pad_ops mt9m001_subdev_pad_ops = {
> + .init_cfg = mt9m001_init_cfg,
> .enum_mbus_code = mt9m001_enum_mbus_code,
> .get_selection = mt9m001_get_selection,
> .set_selection = mt9m001_set_selection,
> --
> 2.7.4
>
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 12/12] media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls
2018-12-22 17:12 ` [PATCH 12/12] media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls Akinobu Mita
@ 2019-01-07 11:32 ` Sakari Ailus
2019-01-07 14:18 ` Akinobu Mita
0 siblings, 1 reply; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 11:32 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, devicetree, Guennadi Liakhovetski, Mauro Carvalho Chehab
Hi Mita-san,
On Sun, Dec 23, 2018 at 02:12:54AM +0900, Akinobu Mita wrote:
> This driver doesn't set all members of mbus format field when the
> VIDIOC_SUBDEV_{S,G}_FMT ioctls are called.
>
> This is detected by v4l2-compliance.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> drivers/media/i2c/mt9m001.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> index f4afbc9..82b89d5 100644
> --- a/drivers/media/i2c/mt9m001.c
> +++ b/drivers/media/i2c/mt9m001.c
> @@ -342,6 +342,9 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd,
> mf->code = mt9m001->fmt->code;
> mf->colorspace = mt9m001->fmt->colorspace;
> mf->field = V4L2_FIELD_NONE;
> + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + mf->quantization = V4L2_QUANTIZATION_DEFAULT;
> + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
Instead of setting the fields individually, would it be feasible to just
assign mt9m001->fmt to mf?
>
> return 0;
> }
> @@ -402,6 +405,10 @@ static int mt9m001_set_fmt(struct v4l2_subdev *sd,
> }
>
> mf->colorspace = fmt->colorspace;
> + mf->field = V4L2_FIELD_NONE;
> + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + mf->quantization = V4L2_QUANTIZATION_DEFAULT;
> + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
Ditto.
>
> if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> return mt9m001_s_fmt(sd, fmt, mf);
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 03/12] media: mt9m001: convert to SPDX license identifer
2018-12-22 17:12 ` [PATCH 03/12] media: mt9m001: convert to SPDX license identifer Akinobu Mita
@ 2019-01-07 11:37 ` Sakari Ailus
2019-01-07 14:20 ` Akinobu Mita
0 siblings, 1 reply; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 11:37 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, devicetree, Guennadi Liakhovetski, Mauro Carvalho Chehab
Hi Mita-san,
On Sun, Dec 23, 2018 at 02:12:45AM +0900, Akinobu Mita wrote:
> Replace GPL license statements with SPDX license identifiers (GPL-2.0).
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> drivers/media/i2c/mt9m001.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> index a1a85ff..65ff59d 100644
> --- a/drivers/media/i2c/mt9m001.c
> +++ b/drivers/media/i2c/mt9m001.c
> @@ -1,11 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Driver for MT9M001 CMOS Image Sensor from Micron
> *
> * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> - *
> - * 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/videodev2.h>
The MODULE_LICENSE macro at the end of the file lists "GPL" as the license,
i.e. including later versions. I'm not sure what was the intention
originally. I guess it's safer to stick to 2.0, at least unless the
original author is able to shed some light into this.
Guennadi?
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 06/12] media: mt9m001: switch s_power callback to runtime PM
[not found] ` <20190107100034.po3jsnc3rdj37l4x@kekkonen.localdomain>
@ 2019-01-07 14:07 ` Akinobu Mita
2019-01-07 14:10 ` Sakari Ailus
0 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2019-01-07 14:07 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
2019年1月7日(月) 19:00 Sakari Ailus <sakari.ailus@linux.intel.com>:
>
> Hi Mita-san,
>
> Thanks for the patchset.
>
> On Sun, Dec 23, 2018 at 02:12:48AM +0900, Akinobu Mita wrote:
> > Switch s_power() callback to runtime PM framework. This also removes
> > soc_camera specific power management code and introduces reset and standby
> > gpios instead.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> > drivers/media/i2c/mt9m001.c | 242 ++++++++++++++++++++++++++++++++------------
> > 1 file changed, 178 insertions(+), 64 deletions(-)
> >
> > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > index c0180fdc..f20188a 100644
> > --- a/drivers/media/i2c/mt9m001.c
> > +++ b/drivers/media/i2c/mt9m001.c
> > @@ -5,6 +5,10 @@
> > * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> > */
> >
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/videodev2.h>
> > #include <linux/slab.h>
> > #include <linux/i2c.h>
> > @@ -13,7 +17,6 @@
> >
> > #include <media/soc_camera.h>
> > #include <media/drv-intf/soc_mediabus.h>
> > -#include <media/v4l2-clk.h>
> > #include <media/v4l2-subdev.h>
> > #include <media/v4l2-ctrls.h>
> >
> > @@ -92,8 +95,12 @@ struct mt9m001 {
> > struct v4l2_ctrl *autoexposure;
> > struct v4l2_ctrl *exposure;
> > };
> > + bool streaming;
> > + struct mutex mutex;
> > struct v4l2_rect rect; /* Sensor window */
> > - struct v4l2_clk *clk;
> > + struct clk *clk;
> > + struct gpio_desc *standby_gpio;
> > + struct gpio_desc *reset_gpio;
> > const struct mt9m001_datafmt *fmt;
> > const struct mt9m001_datafmt *fmts;
> > int num_fmts;
> > @@ -177,8 +184,7 @@ static int mt9m001_init(struct i2c_client *client)
> > return multi_reg_write(client, init_regs, ARRAY_SIZE(init_regs));
> > }
> >
> > -static int mt9m001_apply_selection(struct v4l2_subdev *sd,
> > - struct v4l2_rect *rect)
> > +static int mt9m001_apply_selection(struct v4l2_subdev *sd)
> > {
> > struct i2c_client *client = v4l2_get_subdevdata(sd);
> > struct mt9m001 *mt9m001 = to_mt9m001(client);
> > @@ -190,11 +196,11 @@ static int mt9m001_apply_selection(struct v4l2_subdev *sd,
> > * The caller provides a supported format, as verified per
> > * call to .set_fmt(FORMAT_TRY).
> > */
> > - { MT9M001_COLUMN_START, rect->left },
> > - { MT9M001_ROW_START, rect->top },
> > - { MT9M001_WINDOW_WIDTH, rect->width - 1 },
> > + { MT9M001_COLUMN_START, mt9m001->rect.left },
> > + { MT9M001_ROW_START, mt9m001->rect.top },
> > + { MT9M001_WINDOW_WIDTH, mt9m001->rect.width - 1 },
> > { MT9M001_WINDOW_HEIGHT,
> > - rect->height + mt9m001->y_skip_top - 1 },
> > + mt9m001->rect.height + mt9m001->y_skip_top - 1 },
> > };
> >
> > return multi_reg_write(client, regs, ARRAY_SIZE(regs));
> > @@ -203,11 +209,50 @@ static int mt9m001_apply_selection(struct v4l2_subdev *sd,
> > static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
> > {
> > struct i2c_client *client = v4l2_get_subdevdata(sd);
> > + struct mt9m001 *mt9m001 = to_mt9m001(client);
> > + int ret = 0;
> >
> > - /* Switch to master "normal" mode or stop sensor readout */
> > - if (reg_write(client, MT9M001_OUTPUT_CONTROL, enable ? 2 : 0) < 0)
> > - return -EIO;
> > - return 0;
> > + mutex_lock(&mt9m001->mutex);
> > +
> > + if (mt9m001->streaming == enable)
> > + goto done;
> > +
> > + if (enable) {
> > + ret = pm_runtime_get_sync(&client->dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(&client->dev);
> > + goto done;
>
> How about adding another label for calling pm_runtime_put()? The error
> handling is the same in all cases. You can also use pm_runtime_put()
> instead of pm_runtime_put_noidle() here; there's no harm done.
There are two ways that I can think of. Which one do you prefer?
(1)
done:
mutex_unlock(&mt9m001->mutex);
return 0;
enable_error:
pm_runtime_put(&client->dev);
mutex_unlock(&mt9m001->mutex);
return ret;
}
(2)
done:
if (ret && enable)
pm_runtime_put(&client->dev);
mutex_unlock(&mt9m001->mutex);
return ret;
}
> > + }
> > +
> > + ret = mt9m001_apply_selection(sd);
> > + if (ret) {
> > + pm_runtime_put(&client->dev);
> > + goto done;
> > + }
> > +
> > + ret = __v4l2_ctrl_handler_setup(&mt9m001->hdl);
> > + if (ret) {
> > + pm_runtime_put(&client->dev);
> > + goto done;
> > + }
> > +
> > + /* Switch to master "normal" mode */
> > + ret = reg_write(client, MT9M001_OUTPUT_CONTROL, 2);
> > + if (ret < 0) {
> > + pm_runtime_put(&client->dev);
> > + goto done;
> > + }
> > + } else {
> > + /* Switch to master stop sensor readout */
> > + reg_write(client, MT9M001_OUTPUT_CONTROL, 0);
> > + pm_runtime_put(&client->dev);
> > + }
> > +
> > + mt9m001->streaming = enable;
> > +done:
> > + mutex_unlock(&mt9m001->mutex);
> > +
> > + return ret;
> > }
> >
> > static int mt9m001_set_selection(struct v4l2_subdev *sd,
> > @@ -217,7 +262,6 @@ static int mt9m001_set_selection(struct v4l2_subdev *sd,
> > struct i2c_client *client = v4l2_get_subdevdata(sd);
> > struct mt9m001 *mt9m001 = to_mt9m001(client);
> > struct v4l2_rect rect = sel->r;
> > - int ret;
> >
> > if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE ||
> > sel->target != V4L2_SEL_TGT_CROP)
> > @@ -243,15 +287,9 @@ static int mt9m001_set_selection(struct v4l2_subdev *sd,
> > mt9m001->total_h = rect.height + mt9m001->y_skip_top +
> > MT9M001_DEFAULT_VBLANK;
> >
> > + mt9m001->rect = rect;
> >
> > - ret = mt9m001_apply_selection(sd, &rect);
> > - if (!ret && v4l2_ctrl_g_ctrl(mt9m001->autoexposure) == V4L2_EXPOSURE_AUTO)
> > - ret = reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h);
> > -
> > - if (!ret)
> > - mt9m001->rect = rect;
> > -
> > - return ret;
> > + return 0;
> > }
> >
> > static int mt9m001_get_selection(struct v4l2_subdev *sd,
> > @@ -395,13 +433,34 @@ static int mt9m001_s_register(struct v4l2_subdev *sd,
> > }
> > #endif
> >
> > -static int mt9m001_s_power(struct v4l2_subdev *sd, int on)
> > +static int mt9m001_power_on(struct mt9m001 *mt9m001)
> > {
> > - struct i2c_client *client = v4l2_get_subdevdata(sd);
> > - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> > - struct mt9m001 *mt9m001 = to_mt9m001(client);
> > + int ret = clk_prepare_enable(mt9m001->clk);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + if (mt9m001->standby_gpio) {
> > + gpiod_set_value_cansleep(mt9m001->standby_gpio, 0);
> > + usleep_range(1000, 2000);
> > + }
> > +
> > + if (mt9m001->reset_gpio) {
> > + gpiod_set_value_cansleep(mt9m001->reset_gpio, 1);
> > + usleep_range(1000, 2000);
> > + gpiod_set_value_cansleep(mt9m001->reset_gpio, 0);
> > + usleep_range(1000, 2000);
> > + }
> >
> > - return soc_camera_set_power(&client->dev, ssdd, mt9m001->clk, on);
> > + return 0;
> > +}
> > +
> > +static int mt9m001_power_off(struct mt9m001 *mt9m001)
> > +{
> > + gpiod_set_value_cansleep(mt9m001->standby_gpio, 1);
> > + clk_disable_unprepare(mt9m001->clk);
> > +
> > + return 0;
> > }
> >
> > static int mt9m001_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> > @@ -429,16 +488,18 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
> > struct i2c_client *client = v4l2_get_subdevdata(sd);
> > struct v4l2_ctrl *exp = mt9m001->exposure;
> > int data;
> > + int ret;
> > +
> > + if (!pm_runtime_get_if_in_use(&client->dev))
> > + return 0;
> >
> > switch (ctrl->id) {
> > case V4L2_CID_VFLIP:
> > if (ctrl->val)
> > - data = reg_set(client, MT9M001_READ_OPTIONS2, 0x8000);
> > + ret = reg_set(client, MT9M001_READ_OPTIONS2, 0x8000);
> > else
> > - data = reg_clear(client, MT9M001_READ_OPTIONS2, 0x8000);
> > - if (data < 0)
> > - return -EIO;
> > - return 0;
> > + ret = reg_clear(client, MT9M001_READ_OPTIONS2, 0x8000);
> > + break;
> >
> > case V4L2_CID_GAIN:
> > /* See Datasheet Table 7, Gain settings. */
> > @@ -448,9 +509,7 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
> > data = ((ctrl->val - (s32)ctrl->minimum) * 8 + range / 2) / range;
> >
> > dev_dbg(&client->dev, "Setting gain %d\n", data);
> > - data = reg_write(client, MT9M001_GLOBAL_GAIN, data);
> > - if (data < 0)
> > - return -EIO;
> > + ret = reg_write(client, MT9M001_GLOBAL_GAIN, data);
> > } else {
> > /* Pack it into 1.125..15 variable step, register values 9..67 */
> > /* We assume qctrl->maximum - qctrl->default_value - 1 > 0 */
> > @@ -467,11 +526,9 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
> >
> > dev_dbg(&client->dev, "Setting gain from %d to %d\n",
> > reg_read(client, MT9M001_GLOBAL_GAIN), data);
> > - data = reg_write(client, MT9M001_GLOBAL_GAIN, data);
> > - if (data < 0)
> > - return -EIO;
> > + ret = reg_write(client, MT9M001_GLOBAL_GAIN, data);
> > }
> > - return 0;
> > + break;
> >
> > case V4L2_CID_EXPOSURE_AUTO:
> > if (ctrl->val == V4L2_EXPOSURE_MANUAL) {
> > @@ -482,19 +539,22 @@ static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
> > dev_dbg(&client->dev,
> > "Setting shutter width from %d to %lu\n",
> > reg_read(client, MT9M001_SHUTTER_WIDTH), shutter);
> > - if (reg_write(client, MT9M001_SHUTTER_WIDTH, shutter) < 0)
> > - return -EIO;
> > + ret = reg_write(client, MT9M001_SHUTTER_WIDTH, shutter);
> > } else {
> > - const u16 vblank = 25;
> > -
> > mt9m001->total_h = mt9m001->rect.height +
> > - mt9m001->y_skip_top + vblank;
> > - if (reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h) < 0)
> > - return -EIO;
> > + mt9m001->y_skip_top + MT9M001_DEFAULT_VBLANK;
> > + ret = reg_write(client, MT9M001_SHUTTER_WIDTH,
> > + mt9m001->total_h);
> > }
> > - return 0;
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + break;
> > }
> > - return -EINVAL;
> > +
> > + pm_runtime_put(&client->dev);
> > +
> > + return ret;
> > }
> >
> > /*
> > @@ -509,10 +569,6 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
> > unsigned long flags;
> > int ret;
> >
> > - ret = mt9m001_s_power(&mt9m001->subdev, 1);
> > - if (ret < 0)
> > - return ret;
> > -
> > /* Enable the chip */
> > data = reg_write(client, MT9M001_CHIP_ENABLE, 1);
> > dev_dbg(&client->dev, "write: %d\n", data);
> > @@ -571,7 +627,6 @@ static int mt9m001_video_probe(struct soc_camera_subdev_desc *ssdd,
> > ret = v4l2_ctrl_handler_setup(&mt9m001->hdl);
> >
> > done:
> > - mt9m001_s_power(&mt9m001->subdev, 0);
> > return ret;
> > }
> >
> > @@ -601,7 +656,6 @@ static const struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
> > .g_register = mt9m001_g_register,
> > .s_register = mt9m001_s_register,
> > #endif
> > - .s_power = mt9m001_s_power,
> > };
> >
> > static int mt9m001_enum_mbus_code(struct v4l2_subdev *sd,
> > @@ -700,6 +754,20 @@ static int mt9m001_probe(struct i2c_client *client,
> > if (!mt9m001)
> > return -ENOMEM;
> >
> > + mt9m001->clk = devm_clk_get(&client->dev, NULL);
> > + if (IS_ERR(mt9m001->clk))
> > + return PTR_ERR(mt9m001->clk);
> > +
> > + mt9m001->standby_gpio = devm_gpiod_get_optional(&client->dev, "standby",
> > + GPIOD_OUT_LOW);
> > + if (IS_ERR(mt9m001->standby_gpio))
> > + return PTR_ERR(mt9m001->standby_gpio);
> > +
> > + mt9m001->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
> > + GPIOD_OUT_LOW);
> > + if (IS_ERR(mt9m001->reset_gpio))
> > + return PTR_ERR(mt9m001->reset_gpio);
> > +
> > v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
> > v4l2_ctrl_handler_init(&mt9m001->hdl, 4);
> > v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
> > @@ -722,6 +790,9 @@ static int mt9m001_probe(struct i2c_client *client,
> > v4l2_ctrl_auto_cluster(2, &mt9m001->autoexposure,
> > V4L2_EXPOSURE_MANUAL, true);
> >
> > + mutex_init(&mt9m001->mutex);
> > + mt9m001->hdl.lock = &mt9m001->mutex;
> > +
> > /* Second stage probe - when a capture adapter is there */
> > mt9m001->y_skip_top = 0;
> > mt9m001->rect.left = MT9M001_COLUMN_SKIP;
> > @@ -729,18 +800,30 @@ static int mt9m001_probe(struct i2c_client *client,
> > mt9m001->rect.width = MT9M001_MAX_WIDTH;
> > mt9m001->rect.height = MT9M001_MAX_HEIGHT;
> >
> > - mt9m001->clk = v4l2_clk_get(&client->dev, "mclk");
> > - if (IS_ERR(mt9m001->clk)) {
> > - ret = PTR_ERR(mt9m001->clk);
> > - goto eclkget;
> > - }
> > + ret = mt9m001_power_on(mt9m001);
> > + if (ret)
> > + goto error_hdl_free;
> > +
> > + pm_runtime_get_noresume(&client->dev);
> > + pm_runtime_set_active(&client->dev);
> > + pm_runtime_enable(&client->dev);
>
> You could replace ...get_noresume() + ...put_sync() below with a single
> pm_runtime_idle() call.
OK.
> >
> > ret = mt9m001_video_probe(ssdd, client);
> > - if (ret) {
> > - v4l2_clk_put(mt9m001->clk);
> > -eclkget:
> > - v4l2_ctrl_handler_free(&mt9m001->hdl);
> > - }
> > + if (ret)
> > + goto error_power_off;
> > +
> > + pm_runtime_put_sync(&client->dev);
> > +
> > + return 0;
> > +
> > +error_power_off:
> > + pm_runtime_disable(&client->dev);
> > + pm_runtime_set_suspended(&client->dev);
> > + pm_runtime_put_noidle(&client->dev);
> > + mt9m001_power_off(mt9m001);
>
> A newline would be nice here.
OK.
> > +error_hdl_free:
> > + v4l2_ctrl_handler_free(&mt9m001->hdl);
> > + mutex_destroy(&mt9m001->mutex);
> >
> > return ret;
> > }
> > @@ -750,10 +833,17 @@ static int mt9m001_remove(struct i2c_client *client)
> > struct mt9m001 *mt9m001 = to_mt9m001(client);
> > struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> >
> > - v4l2_clk_put(mt9m001->clk);
> > v4l2_device_unregister_subdev(&mt9m001->subdev);
> > + pm_runtime_get_sync(&client->dev);
> > +
> > + pm_runtime_disable(&client->dev);
> > + pm_runtime_set_suspended(&client->dev);
> > + pm_runtime_put_noidle(&client->dev);
> > + mt9m001_power_off(mt9m001);
> > +
> > v4l2_ctrl_handler_free(&mt9m001->hdl);
> > mt9m001_video_remove(ssdd);
> > + mutex_destroy(&mt9m001->mutex);
> >
> > return 0;
> > }
> > @@ -764,6 +854,29 @@ static const struct i2c_device_id mt9m001_id[] = {
> > };
> > MODULE_DEVICE_TABLE(i2c, mt9m001_id);
> >
> > +static int __maybe_unused mt9m001_runtime_resume(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct mt9m001 *mt9m001 = to_mt9m001(client);
> > +
> > + return mt9m001_power_on(mt9m001);
>
> How about changing the argument of mt9m001_power_o{n,ff} to struct device
> so you could remove these two? I think the original names are fine.
Sounds good.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 09/12] media: mt9m001: register to V4L2 asynchronous subdevice framework
2019-01-07 11:27 ` Sakari Ailus
@ 2019-01-07 14:09 ` Akinobu Mita
0 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2019-01-07 14:09 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
2019年1月7日(月) 20:27 Sakari Ailus <sakari.ailus@linux.intel.com>:
>
> Hi Mita-san,
>
> On Sun, Dec 23, 2018 at 02:12:51AM +0900, Akinobu Mita wrote:
> > Register a sub-device to the asynchronous subdevice framework, and also
> > create subdevice device node.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> > drivers/media/i2c/Kconfig | 2 +-
> > drivers/media/i2c/mt9m001.c | 9 ++++++++-
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 5e30ad3..a6d8416 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -845,7 +845,7 @@ config VIDEO_VS6624
> >
> > config VIDEO_MT9M001
> > tristate "mt9m001 support"
> > - depends on I2C && VIDEO_V4L2
> > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>
> VIDEO_V4L2_SUBDEV_API depends on MEDIA_CONTROLLER, so MEDIA_CONTROLLER
> below can be removed.
OK.
> > depends on MEDIA_CAMERA_SUPPORT
> > depends on MEDIA_CONTROLLER
> > help
> > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > index e31fb7d..b4deec3 100644
> > --- a/drivers/media/i2c/mt9m001.c
> > +++ b/drivers/media/i2c/mt9m001.c
> > @@ -716,6 +716,7 @@ static int mt9m001_probe(struct i2c_client *client,
> > return PTR_ERR(mt9m001->reset_gpio);
> >
> > v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
> > + mt9m001->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
>
> |=
>
> Otherwise you lose flags set by v4l2_i2c_subdev_init().
Oops. Thanks for spotting.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 06/12] media: mt9m001: switch s_power callback to runtime PM
2019-01-07 14:07 ` Akinobu Mita
@ 2019-01-07 14:10 ` Sakari Ailus
0 siblings, 0 replies; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 14:10 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
Hi Mita-san,
On Mon, Jan 07, 2019 at 11:07:18PM +0900, Akinobu Mita wrote:
> 2019年1月7日(月) 19:00 Sakari Ailus <sakari.ailus@linux.intel.com>:
> >
> > Hi Mita-san,
> >
> > Thanks for the patchset.
> >
> > On Sun, Dec 23, 2018 at 02:12:48AM +0900, Akinobu Mita wrote:
> > > Switch s_power() callback to runtime PM framework. This also removes
> > > soc_camera specific power management code and introduces reset and standby
> > > gpios instead.
> > >
> > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > > ---
> > > drivers/media/i2c/mt9m001.c | 242 ++++++++++++++++++++++++++++++++------------
> > > 1 file changed, 178 insertions(+), 64 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > > index c0180fdc..f20188a 100644
> > > --- a/drivers/media/i2c/mt9m001.c
> > > +++ b/drivers/media/i2c/mt9m001.c
> > > @@ -5,6 +5,10 @@
> > > * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> > > */
> > >
> > > +#include <linux/clk.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/pm_runtime.h>
> > > #include <linux/videodev2.h>
> > > #include <linux/slab.h>
> > > #include <linux/i2c.h>
> > > @@ -13,7 +17,6 @@
> > >
> > > #include <media/soc_camera.h>
> > > #include <media/drv-intf/soc_mediabus.h>
> > > -#include <media/v4l2-clk.h>
> > > #include <media/v4l2-subdev.h>
> > > #include <media/v4l2-ctrls.h>
> > >
> > > @@ -92,8 +95,12 @@ struct mt9m001 {
> > > struct v4l2_ctrl *autoexposure;
> > > struct v4l2_ctrl *exposure;
> > > };
> > > + bool streaming;
> > > + struct mutex mutex;
> > > struct v4l2_rect rect; /* Sensor window */
> > > - struct v4l2_clk *clk;
> > > + struct clk *clk;
> > > + struct gpio_desc *standby_gpio;
> > > + struct gpio_desc *reset_gpio;
> > > const struct mt9m001_datafmt *fmt;
> > > const struct mt9m001_datafmt *fmts;
> > > int num_fmts;
> > > @@ -177,8 +184,7 @@ static int mt9m001_init(struct i2c_client *client)
> > > return multi_reg_write(client, init_regs, ARRAY_SIZE(init_regs));
> > > }
> > >
> > > -static int mt9m001_apply_selection(struct v4l2_subdev *sd,
> > > - struct v4l2_rect *rect)
> > > +static int mt9m001_apply_selection(struct v4l2_subdev *sd)
> > > {
> > > struct i2c_client *client = v4l2_get_subdevdata(sd);
> > > struct mt9m001 *mt9m001 = to_mt9m001(client);
> > > @@ -190,11 +196,11 @@ static int mt9m001_apply_selection(struct v4l2_subdev *sd,
> > > * The caller provides a supported format, as verified per
> > > * call to .set_fmt(FORMAT_TRY).
> > > */
> > > - { MT9M001_COLUMN_START, rect->left },
> > > - { MT9M001_ROW_START, rect->top },
> > > - { MT9M001_WINDOW_WIDTH, rect->width - 1 },
> > > + { MT9M001_COLUMN_START, mt9m001->rect.left },
> > > + { MT9M001_ROW_START, mt9m001->rect.top },
> > > + { MT9M001_WINDOW_WIDTH, mt9m001->rect.width - 1 },
> > > { MT9M001_WINDOW_HEIGHT,
> > > - rect->height + mt9m001->y_skip_top - 1 },
> > > + mt9m001->rect.height + mt9m001->y_skip_top - 1 },
> > > };
> > >
> > > return multi_reg_write(client, regs, ARRAY_SIZE(regs));
> > > @@ -203,11 +209,50 @@ static int mt9m001_apply_selection(struct v4l2_subdev *sd,
> > > static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
> > > {
> > > struct i2c_client *client = v4l2_get_subdevdata(sd);
> > > + struct mt9m001 *mt9m001 = to_mt9m001(client);
> > > + int ret = 0;
> > >
> > > - /* Switch to master "normal" mode or stop sensor readout */
> > > - if (reg_write(client, MT9M001_OUTPUT_CONTROL, enable ? 2 : 0) < 0)
> > > - return -EIO;
> > > - return 0;
> > > + mutex_lock(&mt9m001->mutex);
> > > +
> > > + if (mt9m001->streaming == enable)
> > > + goto done;
> > > +
> > > + if (enable) {
> > > + ret = pm_runtime_get_sync(&client->dev);
> > > + if (ret < 0) {
> > > + pm_runtime_put_noidle(&client->dev);
> > > + goto done;
> >
> > How about adding another label for calling pm_runtime_put()? The error
> > handling is the same in all cases. You can also use pm_runtime_put()
> > instead of pm_runtime_put_noidle() here; there's no harm done.
>
> There are two ways that I can think of. Which one do you prefer?
>
> (1)
> done:
> mutex_unlock(&mt9m001->mutex);
>
> return 0;
>
> enable_error:
> pm_runtime_put(&client->dev);
> mutex_unlock(&mt9m001->mutex);
>
> return ret;
> }
>
> (2)
> done:
> if (ret && enable)
> pm_runtime_put(&client->dev);
>
> mutex_unlock(&mt9m001->mutex);
>
> return ret;
> }
I'd prefer the first; it's cleaner. I might call the new label e.g.
put_unlock as that describes what it does.
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/12] media: mt9m001: remove remaining soc_camera specific code
2019-01-07 11:29 ` Sakari Ailus
@ 2019-01-07 14:13 ` Akinobu Mita
0 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2019-01-07 14:13 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
2019年1月7日(月) 20:29 Sakari Ailus <sakari.ailus@linux.intel.com>:
>
> Hi Mita-san,
>
> On Sun, Dec 23, 2018 at 02:12:49AM +0900, Akinobu Mita wrote:
> > Remove remaining soc_camera specific code and drop soc_camera dependency
> > from this driver.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> > drivers/media/i2c/Kconfig | 2 +-
> > drivers/media/i2c/mt9m001.c | 84 ++++++++-------------------------------------
> > 2 files changed, 15 insertions(+), 71 deletions(-)
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 0efc038..4bdf043 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -845,7 +845,7 @@ config VIDEO_VS6624
> >
> > config VIDEO_MT9M001
> > tristate "mt9m001 support"
> > - depends on SOC_CAMERA && I2C
> > + depends on I2C && VIDEO_V4L2
> > help
> > This driver supports MT9M001 cameras from Micron, monochrome
> > and colour models.
> > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > index f20188a..eb5c4ed 100644
> > --- a/drivers/media/i2c/mt9m001.c
> > +++ b/drivers/media/i2c/mt9m001.c
> > @@ -15,15 +15,12 @@
> > #include <linux/log2.h>
> > #include <linux/module.h>
> >
> > -#include <media/soc_camera.h>
> > -#include <media/drv-intf/soc_mediabus.h>
> > #include <media/v4l2-subdev.h>
> > #include <media/v4l2-ctrls.h>
>
> While at it, could you order the list alphabetically? The same applies to
> further patches changing included files.
OK. I'll prepare another separate patch.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 11/12] media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY
2019-01-07 11:30 ` Sakari Ailus
@ 2019-01-07 14:15 ` Akinobu Mita
0 siblings, 0 replies; 31+ messages in thread
From: Akinobu Mita @ 2019-01-07 14:15 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
2019年1月7日(月) 20:30 Sakari Ailus <sakari.ailus@linux.intel.com>:
>
> Hi Mita-san,
>
> On Sun, Dec 23, 2018 at 02:12:53AM +0900, Akinobu Mita wrote:
> > The VIDIOC_SUBDEV_G_FMT ioctl for this driver doesn't recognize
> > V4L2_SUBDEV_FORMAT_TRY and always works as if V4L2_SUBDEV_FORMAT_ACTIVE
> > is specified.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> > drivers/media/i2c/mt9m001.c | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > index a5b94d7..f4afbc9 100644
> > --- a/drivers/media/i2c/mt9m001.c
> > +++ b/drivers/media/i2c/mt9m001.c
> > @@ -331,6 +331,12 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd,
> > if (format->pad)
> > return -EINVAL;
> >
> > + if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> > + mf = v4l2_subdev_get_try_format(sd, cfg, 0);
> > + format->format = *mf;
> > + return 0;
> > + }
> > +
> > mf->width = mt9m001->rect.width;
> > mf->height = mt9m001->rect.height;
> > mf->code = mt9m001->fmt->code;
> > @@ -638,6 +644,26 @@ static const struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
> > #endif
> > };
> >
> > +static int mt9m001_init_cfg(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(sd);
> > + struct mt9m001 *mt9m001 = to_mt9m001(client);
> > + struct v4l2_mbus_framefmt *try_fmt =
> > + v4l2_subdev_get_try_format(sd, cfg, 0);
> > +
> > + try_fmt->width = mt9m001->rect.width;
> > + try_fmt->height = mt9m001->rect.height;
> > + try_fmt->code = mt9m001->fmt->code;
> > + try_fmt->colorspace = mt9m001->fmt->colorspace;
>
> The initial configuration set here should reflect the default, not current
> configuration. This appears to refer to the current one.
You are right. I'll fix this.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 12/12] media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls
2019-01-07 11:32 ` Sakari Ailus
@ 2019-01-07 14:18 ` Akinobu Mita
2019-01-07 15:16 ` Sakari Ailus
0 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2019-01-07 14:18 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
2019年1月7日(月) 20:32 Sakari Ailus <sakari.ailus@linux.intel.com>:
>
> Hi Mita-san,
>
> On Sun, Dec 23, 2018 at 02:12:54AM +0900, Akinobu Mita wrote:
> > This driver doesn't set all members of mbus format field when the
> > VIDIOC_SUBDEV_{S,G}_FMT ioctls are called.
> >
> > This is detected by v4l2-compliance.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> > drivers/media/i2c/mt9m001.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > index f4afbc9..82b89d5 100644
> > --- a/drivers/media/i2c/mt9m001.c
> > +++ b/drivers/media/i2c/mt9m001.c
> > @@ -342,6 +342,9 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd,
> > mf->code = mt9m001->fmt->code;
> > mf->colorspace = mt9m001->fmt->colorspace;
> > mf->field = V4L2_FIELD_NONE;
> > + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > + mf->quantization = V4L2_QUANTIZATION_DEFAULT;
> > + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>
> Instead of setting the fields individually, would it be feasible to just
> assign mt9m001->fmt to mf?
The data type of mt9m001->fmt is struct mt9m001_datafmt, so it can't
simply assign to mf.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 03/12] media: mt9m001: convert to SPDX license identifer
2019-01-07 11:37 ` Sakari Ailus
@ 2019-01-07 14:20 ` Akinobu Mita
2019-01-07 15:15 ` Sakari Ailus
0 siblings, 1 reply; 31+ messages in thread
From: Akinobu Mita @ 2019-01-07 14:20 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
2019年1月7日(月) 20:37 Sakari Ailus <sakari.ailus@linux.intel.com>:
>
> Hi Mita-san,
>
> On Sun, Dec 23, 2018 at 02:12:45AM +0900, Akinobu Mita wrote:
> > Replace GPL license statements with SPDX license identifiers (GPL-2.0).
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> > drivers/media/i2c/mt9m001.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > index a1a85ff..65ff59d 100644
> > --- a/drivers/media/i2c/mt9m001.c
> > +++ b/drivers/media/i2c/mt9m001.c
> > @@ -1,11 +1,8 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > /*
> > * Driver for MT9M001 CMOS Image Sensor from Micron
> > *
> > * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> > - *
> > - * 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/videodev2.h>
>
> The MODULE_LICENSE macro at the end of the file lists "GPL" as the license,
> i.e. including later versions. I'm not sure what was the intention
> originally. I guess it's safer to stick to 2.0, at least unless the
> original author is able to shed some light into this.
I've come across the same thought, and I found the following conversation.
https://lkml.org/lkml/2018/6/24/457
So I think MODULE_LICENSE() mismatch can be resolved in the future.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 03/12] media: mt9m001: convert to SPDX license identifer
2019-01-07 14:20 ` Akinobu Mita
@ 2019-01-07 15:15 ` Sakari Ailus
0 siblings, 0 replies; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 15:15 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
On Mon, Jan 07, 2019 at 11:20:29PM +0900, Akinobu Mita wrote:
> 2019年1月7日(月) 20:37 Sakari Ailus <sakari.ailus@linux.intel.com>:
> >
> > Hi Mita-san,
> >
> > On Sun, Dec 23, 2018 at 02:12:45AM +0900, Akinobu Mita wrote:
> > > Replace GPL license statements with SPDX license identifiers (GPL-2.0).
> > >
> > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > > ---
> > > drivers/media/i2c/mt9m001.c | 5 +----
> > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > > index a1a85ff..65ff59d 100644
> > > --- a/drivers/media/i2c/mt9m001.c
> > > +++ b/drivers/media/i2c/mt9m001.c
> > > @@ -1,11 +1,8 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > /*
> > > * Driver for MT9M001 CMOS Image Sensor from Micron
> > > *
> > > * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> > > - *
> > > - * 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/videodev2.h>
> >
> > The MODULE_LICENSE macro at the end of the file lists "GPL" as the license,
> > i.e. including later versions. I'm not sure what was the intention
> > originally. I guess it's safer to stick to 2.0, at least unless the
> > original author is able to shed some light into this.
>
> I've come across the same thought, and I found the following conversation.
>
> https://lkml.org/lkml/2018/6/24/457
>
> So I think MODULE_LICENSE() mismatch can be resolved in the future.
I don't see a reason to postpone it. Strictly speaking, for GPL 2+ we'd
need an approval from anyone who has submitted patches to the file. For GPL
v2 it'd be just a patch that fixes the MODULE_LICENSE.
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 12/12] media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls
2019-01-07 14:18 ` Akinobu Mita
@ 2019-01-07 15:16 ` Sakari Ailus
0 siblings, 0 replies; 31+ messages in thread
From: Sakari Ailus @ 2019-01-07 15:16 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, open list:OPEN FIRMWARE AND...,
Guennadi Liakhovetski, Mauro Carvalho Chehab
On Mon, Jan 07, 2019 at 11:18:47PM +0900, Akinobu Mita wrote:
> 2019年1月7日(月) 20:32 Sakari Ailus <sakari.ailus@linux.intel.com>:
> >
> > Hi Mita-san,
> >
> > On Sun, Dec 23, 2018 at 02:12:54AM +0900, Akinobu Mita wrote:
> > > This driver doesn't set all members of mbus format field when the
> > > VIDIOC_SUBDEV_{S,G}_FMT ioctls are called.
> > >
> > > This is detected by v4l2-compliance.
> > >
> > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > > ---
> > > drivers/media/i2c/mt9m001.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > > index f4afbc9..82b89d5 100644
> > > --- a/drivers/media/i2c/mt9m001.c
> > > +++ b/drivers/media/i2c/mt9m001.c
> > > @@ -342,6 +342,9 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd,
> > > mf->code = mt9m001->fmt->code;
> > > mf->colorspace = mt9m001->fmt->colorspace;
> > > mf->field = V4L2_FIELD_NONE;
> > > + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > > + mf->quantization = V4L2_QUANTIZATION_DEFAULT;
> > > + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> >
> > Instead of setting the fields individually, would it be feasible to just
> > assign mt9m001->fmt to mf?
>
> The data type of mt9m001->fmt is struct mt9m001_datafmt, so it can't
> simply assign to mf.
Oh, good point. There are still three places in the driver where you're
setting effectively the same fields; if there is a way to reasonably
address that, it'd be nice to do that. Up to you.
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2019-01-07 15:17 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-22 17:12 [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Akinobu Mita
2018-12-22 17:12 ` [PATCH 01/12] media: i2c: mt9m001: copy mt9m001 soc_camera " Akinobu Mita
2018-12-22 17:12 ` [PATCH 02/12] media: i2c: mt9m001: dt: add binding for mt9m001 Akinobu Mita
2019-01-03 21:12 ` Rob Herring
2019-01-05 15:09 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 03/12] media: mt9m001: convert to SPDX license identifer Akinobu Mita
2019-01-07 11:37 ` Sakari Ailus
2019-01-07 14:20 ` Akinobu Mita
2019-01-07 15:15 ` Sakari Ailus
2018-12-22 17:12 ` [PATCH 04/12] media: mt9m001: add of_match_table Akinobu Mita
2018-12-22 17:12 ` [PATCH 05/12] media: mt9m001: introduce multi_reg_write() Akinobu Mita
2018-12-22 17:12 ` [PATCH 06/12] media: mt9m001: switch s_power callback to runtime PM Akinobu Mita
[not found] ` <20190107100034.po3jsnc3rdj37l4x@kekkonen.localdomain>
2019-01-07 14:07 ` Akinobu Mita
2019-01-07 14:10 ` Sakari Ailus
2018-12-22 17:12 ` [PATCH 07/12] media: mt9m001: remove remaining soc_camera specific code Akinobu Mita
2019-01-07 11:29 ` Sakari Ailus
2019-01-07 14:13 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 08/12] media: mt9m001: add media controller support Akinobu Mita
2018-12-22 17:12 ` [PATCH 09/12] media: mt9m001: register to V4L2 asynchronous subdevice framework Akinobu Mita
2019-01-07 11:27 ` Sakari Ailus
2019-01-07 14:09 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 10/12] media: mt9m001: support log_status ioctl and event interface Akinobu Mita
2018-12-22 17:12 ` [PATCH 11/12] media: mt9m001: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY Akinobu Mita
2019-01-07 11:30 ` Sakari Ailus
2019-01-07 14:15 ` Akinobu Mita
2018-12-22 17:12 ` [PATCH 12/12] media: mt9m001: set all mbus format field when G_FMT and S_FMT ioctls Akinobu Mita
2019-01-07 11:32 ` Sakari Ailus
2019-01-07 14:18 ` Akinobu Mita
2019-01-07 15:16 ` Sakari Ailus
2019-01-07 9:36 ` [PATCH 00/12] media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver Sakari Ailus
2019-01-07 9:48 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).