Add a driver for the NIR-enhanced Mira220 1600x1400 global shutter image sensor.
PATCH v3 2/2 Add a driver for the NIR-enhanced Mira220 1600x1400 global shutter image sensor.
This driver implements 12b, 10b and 8b RAW RGB color format. The output datarate is
1500Mbit/s, 2 lane. Framerate is up to 90 fps.
Note: this sensor does not support analog gain.
---
Changes in v3:
- Add people to mailing list
- Improve commit description
Changes in v2:
- Fix checkpatch warnings
--- drivers/media/i2c/Kconfig | 39 + drivers/media/i2c/Makefile | 3 + drivers/media/i2c/mira220.c | 1973 +++++++++++++++++++++++++++++++++++ 3 files changed, 2015 insertions(+) create mode 100644 drivers/media/i2c/mira220.c
drivers/media/i2c/Kconfig +39 -0
@@ -127,6 +127,31 @@ config VIDEO_HI847 To compile this driver as a module, choose M here: the module will be called hi847. +config VIDEO_PONCHA110 + tristate "ams PONCHA110 sensor support" + depends on I2C && VIDEO_DEV + select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API + select V4L2_FWNODE + help + This is a Video4Linux2 sensor driver for the ams + PONCHA110 camera. + + To compile this driver as a module, choose M here: the + module will be called poncha110. + +config VIDEO_PONCHA110COLOR + tristate "ams PONCHA110COLOR sensor support" + depends on I2C && VIDEO_DEV + select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API + select V4L2_FWNODE + help + This is a Video4Linux2 sensor driver for the ams + PONCHA110 camera. + + To compile this driver as a module, choose M here: the + module will be called poncha110. config VIDEO_IMX208 tristate "Sony IMX208 sensor support" help
@@ -269,6 +294,20 @@ config VIDEO_IMX415 config VIDEO_MAX9271_LIB tristate +config VIDEO_MIRA220 + tristate "ams MIRA220 sensor support" + depends on I2C && VIDEO_DEV + select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API + select V4L2_CCI_I2C + select V4L2_FWNODE + help + This is a Video4Linux2 sensor driver for the ams + MIRA220 camera. + + To compile this driver as a module, choose M here: the + module will be called mira220. + config VIDEO_MT9M001 tristate "mt9m001 support" help
drivers/media/i2c/Makefile +3 -0
@@ -45,6 +45,8 @@ obj-$(CONFIG_VIDEO_HI556) += hi556.o obj-$(CONFIG_VIDEO_HI846) += hi846.o obj-$(CONFIG_VIDEO_HI847) += hi847.o obj-$(CONFIG_VIDEO_I2C) += video-i2c.o +obj-$(CONFIG_VIDEO_PONCHA110) += poncha110.o +obj-$(CONFIG_VIDEO_PONCHA110COLOR) += poncha110color.o obj-$(CONFIG_VIDEO_IMX208) += imx208.o obj-$(CONFIG_VIDEO_IMX214) += imx214.o obj-$(CONFIG_VIDEO_IMX219) += imx219.o
@@ -70,6 +72,7 @@ obj-$(CONFIG_VIDEO_MAX9271_LIB) += max9271.o obj-$(CONFIG_VIDEO_MAX9286) += max9286.o obj-$(CONFIG_VIDEO_MAX96714) += max96714.o obj-$(CONFIG_VIDEO_MAX96717) += max96717.o +obj-$(CONFIG_VIDEO_MIRA220) += mira220.o obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o obj-$(CONFIG_VIDEO_MSP3400) += msp3400.o obj-$(CONFIG_VIDEO_MT9M001) += mt9m001.o
drivers/media/i2c/mira220.c new file +1973 -0
1,973 lines not on this page · raw message
Signature · 1 line
2.39.5
Quoted 5 lines · Philippe Baetens
This driver implements 12b, 10b and 8b RAW RGB color format. The output datarate is
1500Mbit/s, 2 lane. Framerate is up to 90 fps.
Note: this sensor does not support analog gain.
Quoted 7 lines
Signed-off-by philippe baetens <[email protected]>---
Changes in v3:
- Add people to mailing list
- Improve commit description
Changes in v2:
- Fix checkpatch warnings
Subject: missing prefixes
Subject: no full stop, this is not a sentence. Look at git log how this is created.
Quoted 12 lines
--- drivers/media/i2c/Kconfig | 39 + drivers/media/i2c/Makefile | 3 + drivers/media/i2c/mira220.c | 1973 +++++++++++++++++++++++++++++++++++ 3 files changed, 2015 insertions(+) create mode 100644 drivers/media/i2c/mira220.cdrivers/media/i2c/Kconfig +0
-0
drivers/media/i2c/Kconfig config VIDEO_HI847 · line 130
Quoted 4 lines
drivers/media/i2c/Kconfig +1
-0@@ -127,6 +127,31 @@ config VIDEO_HI847 To compile this driver as a module, choose M here: the module will be called hi847. +config VIDEO_PONCHA110
What is this?
drivers/media/i2c/Kconfig config VIDEO_HI847 · line 143
Quoted 13 lines
drivers/media/i2c/Kconfig +13
-0+ tristate "ams PONCHA110 sensor support" + depends on I2C && VIDEO_DEV + select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API + select V4L2_FWNODE + help + This is a Video4Linux2 sensor driver for the ams + PONCHA110 camera. + + To compile this driver as a module, choose M here: the + module will be called poncha110. + +config VIDEO_PONCHA110COLOR
Hm?
Quoted 40 lines
drivers/media/i2c/Kconfig +25
-0+ tristate "ams PONCHA110COLOR sensor support" + depends on I2C && VIDEO_DEV + select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API + select V4L2_FWNODE + help + This is a Video4Linux2 sensor driver for the ams + PONCHA110 camera. + + To compile this driver as a module, choose M here: the + module will be called poncha110. config VIDEO_IMX208 tristate "Sony IMX208 sensor support" help@@ -269,6 +294,20 @@ config VIDEO_IMX415 config VIDEO_MAX9271_LIB tristate +config VIDEO_MIRA220 + tristate "ams MIRA220 sensor support" + depends on I2C && VIDEO_DEV + select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API + select V4L2_CCI_I2C + select V4L2_FWNODE + help + This is a Video4Linux2 sensor driver for the ams + MIRA220 camera. + + To compile this driver as a module, choose M here: the + module will be called mira220. + config VIDEO_MT9M001 tristate "mt9m001 support" helpdrivers/media/i2c/Makefile +0
-0
drivers/media/i2c/Makefile obj-$(CONFIG_VIDEO_HI556) += hi556.o · line 49
Quoted 5 lines
drivers/media/i2c/Makefile +2
-0@@ -45,6 +45,8 @@ obj-$(CONFIG_VIDEO_HI556) += hi556.o obj-$(CONFIG_VIDEO_HI846) += hi846.o obj-$(CONFIG_VIDEO_HI847) += hi847.o obj-$(CONFIG_VIDEO_I2C) += video-i2c.o +obj-$(CONFIG_VIDEO_PONCHA110) += poncha110.o +obj-$(CONFIG_VIDEO_PONCHA110COLOR) += poncha110color.o
What?
...
drivers/media/i2c/Makefile obj-$(CONFIG_VIDEO_HI556) += hi556.o · line 51
Quoted 2 lines
drivers/media/i2c/Makefile +2
-0+ +struct mira220 {
Type declarations go before variable definitions.
Quoted 43 lines
drivers/media/i2c/Makefile +43
-0+ struct v4l2_subdev sd; + struct media_pad pad; + + struct v4l2_mbus_framefmt fmt; + + struct clk *xclk; /* system clock to MIRA220 */ + u32 xclk_freq; + + struct regulator_bulk_data supplies[MIRA220_NUM_SUPPLIES]; + + struct v4l2_ctrl_handler ctrl_handler; + struct v4l2_ctrl *pixel_rate; + struct v4l2_ctrl *vflip; + struct v4l2_ctrl *hflip; + struct v4l2_ctrl *vblank; + struct v4l2_ctrl *hblank; + struct v4l2_ctrl *exposure; + struct v4l2_ctrl *gain; + + /* Current mode */ + const struct mira220_mode *mode; + + struct mutex mutex; //comment + /* mutex ensures correct initialization */ + + struct regmap *regmap; +}; + +static inline struct mira220 *to_mira220(struct v4l2_subdev *_sd) +{ + return container_of(_sd, struct mira220, sd); +} + +/* Power/clock management functions */ +static int mira220_power_on(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct mira220 *mira220 = to_mira220(sd); + int ret = -EINVAL; + + ret = regulator_bulk_enable(MIRA220_NUM_SUPPLIES, mira220->supplies); + if (ret) {
drivers/media/i2c/Makefile obj-$(CONFIG_VIDEO_HI556) += hi556.o
Quoted 10 lines
drivers/media/i2c/Makefile +10
-0+ dev_err(&client->dev, "%s: failed to enable regulators\n", + __func__); + goto reg_off; + } + ret = clk_prepare_enable(mira220->xclk); + if (ret) { + dev_err(&client->dev, "%s: failed to enable clock\n", __func__); + goto clk_off; + } + fsleep(MIRA220_XCLR_MIN_DELAY_US);
No. Look at your probe code, this makes no sense.
drivers/media/i2c/Makefile
Quoted 7 lines
drivers/media/i2c/Makefile +7
-0+ + return 0; + +clk_off: + clk_disable_unprepare(mira220->xclk); +reg_off: + ret = regulator_bulk_disable(MIRA220_NUM_SUPPLIES, mira220->supplies);
So you return 0 on error?
Why are you disabling regulators which were not enabled? This is wrong - bug.
Quoted 16 lines
drivers/media/i2c/Makefile +16
-0+ return ret; +} + +static int mira220_power_off(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct mira220 *mira220 = to_mira220(sd); + (void)mira220; + + clk_disable_unprepare(mira220->xclk); + regulator_bulk_disable(MIRA220_NUM_SUPPLIES, mira220->supplies); + + return 0; +} +
...
Quoted 8 lines
drivers/media/i2c/Makefile +8
-0+ +static int mira220_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct mira220 *mira220; + int ret; + + mira220 = devm_kzalloc(&client->dev, sizeof(*mira220), GFP_KERNEL);
drivers/media/i2c/Makefile
Quoted 10 lines
drivers/media/i2c/Makefile +10
-0+ if (!mira220) + return -ENOMEM; + + v4l2_i2c_subdev_init(&mira220->sd, client, &mira220_subdev_ops); + mira220->sd.internal_ops = &mira220_internal_ops; + + mira220->regmap = devm_cci_regmap_init_i2c(client, 16); + if (IS_ERR(mira220->regmap)) + return dev_err_probe(dev, PTR_ERR(mira220->regmap), + "failed to initialize CCI\n");
Blank line
drivers/media/i2c/Makefile
Quoted 5 lines
drivers/media/i2c/Makefile +5
-0+ /* Get system clock (xclk) */ + mira220->xclk = devm_clk_get(dev, NULL); + if (IS_ERR(mira220->xclk)) { + dev_err(dev, "failed to get xclk\n"); + return PTR_ERR(mira220->xclk);
Syntax is: return dev_err_probe()
drivers/media/i2c/Makefile
Quoted 6 lines
drivers/media/i2c/Makefile +6
-0+ } + mira220->xclk_freq = clk_get_rate(mira220->xclk); + if (mira220->xclk_freq != MIRA220_SUPPORTED_XCLK_FREQ) { + dev_err(dev, "xclk frequency not supported: %d Hz\n", + mira220->xclk_freq); + return -EINVAL;
return dev_err_probe
drivers/media/i2c/Makefile
Quoted 6 lines
drivers/media/i2c/Makefile +6
-0+ } + + ret = mira220_get_regulators(mira220); + if (ret) { + dev_err(dev, "failed to get regulators\n"); + return ret;
Same
drivers/media/i2c/Makefile
Quoted 3 lines
drivers/media/i2c/Makefile +3
-0+ } + + fsleep(10000);
Uh... Why? This sleep makes no sense at all.
drivers/media/i2c/Makefile
Quoted 7 lines
drivers/media/i2c/Makefile +7
-0+ + // The sensor must be powered for mira220_identify_module() + // to be able to read the CHIP_ID register + + ret = mira220_power_on(dev); + if (ret) + return ret;
You just slept here!
drivers/media/i2c/Makefile
Quoted 2 lines
drivers/media/i2c/Makefile +2
-0+ + fsleep(100000);
Oh... so entire probe is delayed by 100 ms + 10 ms (!!!). This is just some arbitrary and not suspicious delay. Maybe you miss regulator ramps, but 100 ms here is just wrong.
Quoted 75 lines
drivers/media/i2c/Makefile +75
-0+ + ret = mira220_identify_module(mira220); + if (ret) + goto error_power_off; + + /* Set default mode to max resolution */ + mira220->mode = &supported_modes[0]; + + ret = mira220_init_controls(mira220); + if (ret) + goto error_power_off; + + /* Initialize subdev */ + mira220->sd.internal_ops = &mira220_internal_ops; + mira220->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | + V4L2_SUBDEV_FL_HAS_EVENTS; + mira220->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; + + /* Initialize source pads */ + mira220->pad.flags = MEDIA_PAD_FL_SOURCE; + + ret = media_entity_pads_init(&mira220->sd.entity, 1, &mira220->pad); + if (ret) { + dev_err_probe(dev, ret, "failed to init entity pads\n"); + goto error_handler_free; + } + + mira220->sd.state_lock = mira220->ctrl_handler.lock; + ret = v4l2_subdev_init_finalize(&mira220->sd); + if (ret < 0) { + dev_err_probe(dev, ret, "subdev init error\n"); + goto error_media_entity; + } + + ret = v4l2_async_register_subdev_sensor(&mira220->sd); + if (ret < 0) { + dev_err_probe(dev, ret, + "failed to register sensor sub-device\n"); + goto error_subdev_cleanup; + } + + /* Enable runtime PM and turn off the device */ + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + pm_runtime_idle(dev); + + return 0; + +error_subdev_cleanup: + v4l2_subdev_cleanup(&mira220->sd); + +error_media_entity: + media_entity_cleanup(&mira220->sd.entity); + +error_handler_free: + mira220_free_controls(mira220); + +error_power_off: + mira220_power_off(dev); + + return ret; +} + +static void mira220_remove(struct i2c_client *client) +{ + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct mira220 *mira220 = to_mira220(sd); + + v4l2_async_unregister_subdev(sd); + media_entity_cleanup(&sd->entity); + mira220_free_controls(mira220); + + pm_runtime_disable(&client->dev); + if (!pm_runtime_status_suspended(&client->dev)) + mira220_power_off(&client->dev);
drivers/media/i2c/Makefile
Quoted 10 lines
drivers/media/i2c/Makefile +10
-0+ pm_runtime_set_suspended(&client->dev); +} + +static const struct dev_pm_ops mira220_pm_ops = { + SET_RUNTIME_PM_OPS(mira220_power_off, mira220_power_on, NULL) +}; + +static const struct of_device_id mira220_dt_ids[] = { + { .compatible = "ams,mira220" }, + { /* sentinel */ } };
}; is always in new line.
drivers/media/i2c/Makefile
Quoted 2 lines
drivers/media/i2c/Makefile +2
-0+MODULE_DEVICE_TABLE(of, mira220_dt_ids); +
Best regards,
Krzysztof