resolve some remarks after code review.

Author: fiepfiep

arch/arm/boot/dts/overlays/README

@@ -3511,7 +3511,6 @@ Params:
                                 (default 16).
 
 
-
 Name:   mira220
 Info:   ams mira220 camera module.
         Uses Unicam 1, which is the standard camera connector on most Pi
@@ -3527,7 +3526,6 @@ Params: rotation                Mounting rotation of the camera sensor (0 or
                                 Compute Module (CSI0, i2c_vc, and cam0_reg).
 
 
-
 Name:   mlx90640
 Info:   Overlay for i2c connected mlx90640 thermal camera
 Load:   dtoverlay=mlx90640
@@ -3649,7 +3647,6 @@ Params: rotation                Mounting rotation of the camera sensor (0 or
                                 Appears not to work on Pi3.
 
 
-
 Name:   papirus
 Info:   PaPiRus ePaper Screen by Pi Supply (both HAT and pHAT)
 Load:   dtoverlay=papirus,<param>=<val>

drivers/media/i2c/Kconfig

@@ -149,20 +149,6 @@ config VIDEO_HI847
           To compile this driver as a module, choose M here: the
           module will be called hi847.
 
-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_IMX208
 	tristate "Sony IMX208 sensor support"
 	help
@@ -352,6 +338,20 @@ config VIDEO_IMX708
 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

@@ -49,7 +49,6 @@ 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_MIRA220)	+= mira220.o
 obj-$(CONFIG_VIDEO_IMX208) += imx208.o
 obj-$(CONFIG_VIDEO_IMX214) += imx214.o
 obj-$(CONFIG_VIDEO_IMX219) += imx219.o
@@ -79,6 +78,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

@@ -35,9 +35,9 @@
 #define MIRA220_PIXEL_ARRAY_WIDTH 1600U
 #define MIRA220_PIXEL_ARRAY_HEIGHT 1400U
 
+/* mira220 does not support analog gain. */
 #define MIRA220_ANALOG_GAIN_MIN 1
-#define MIRA220_ANALOG_GAIN_MAX \
-	1 /* Fixed analog gain to 1, to avoid unexpected behavior. */
+#define MIRA220_ANALOG_GAIN_MAX 1  
 #define MIRA220_ANALOG_GAIN_STEP 1
 #define MIRA220_ANALOG_GAIN_DEFAULT MIRA220_ANALOG_GAIN_MIN
 
@@ -95,11 +95,6 @@
 #define MIRA220_XCLR_MIN_DELAY_US 100000
 #define MIRA220_XCLR_DELAY_RANGE_US 30
 
-// Outdated. See below.
-// pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample
-// 1.0Gb/s * 2 * 2 / 12 = 357913941
-// #define MIRA220_PIXEL_RATE		(357913941)
-
 // Mira220 PIXEL_RATE is derived from ROW_LENGTH. See datasheet Section 9.2.
 // ROW_LENGTH is set by registers: 0x102B, 0x102C. Unit is number of CLK_IN cycles.
 // PIXEL_RATE = 1000000000 * WIDTH / (ROW_LENGTH * CLK_IN_PERIOD_NS)
@@ -146,16 +141,12 @@
 #define MIRA220_TEST_PATTERN_DISABLE 0x00
 #define MIRA220_TEST_PATTERN_VERTICAL_GRADIENT 0x01
 
-/* Embedded metadata stream structure */
-#define MIRA220_EMBEDDED_LINE_WIDTH 16384
-#define MIRA220_NUM_EMBEDDED_LINES 1
-
 /* From Jetson driver */
 #define MIRA220_DEFAULT_LINE_LENGTH (0xA80)
 #define MIRA220_DEFAULT_PIXEL_CLOCK (160)
 #define MIRA220_DEFAULT_FRAME_LENGTH (0x07C0) //TODO REMOVE THESE
 
-enum pad_types { IMAGE_PAD, METADATA_PAD, NUM_PADS };
+enum pad_types { IMAGE_PAD, NUM_PADS };
 
 struct mira220_reg {
 	u16 address;
@@ -1369,8 +1360,6 @@ static int mira220_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 	struct mira220 *mira220 = to_mira220(sd);
 	struct v4l2_mbus_framefmt *try_fmt_img =
 		v4l2_subdev_state_get_format(fh->state, IMAGE_PAD);
-	struct v4l2_mbus_framefmt *try_fmt_meta =
-		v4l2_subdev_state_get_format(fh->state, METADATA_PAD);
 	struct v4l2_rect *try_crop;
 
 	mutex_lock(&mira220->mutex);
@@ -1382,12 +1371,6 @@ static int mira220_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 		mira220, supported_modes[0].code);
 	try_fmt_img->field = V4L2_FIELD_NONE;
 
-	/* TODO(jalv): Initialize try_fmt for the embedded metadata pad */
-	try_fmt_meta->width = MIRA220_EMBEDDED_LINE_WIDTH;
-	try_fmt_meta->height = MIRA220_NUM_EMBEDDED_LINES;
-	try_fmt_meta->code = MEDIA_BUS_FMT_SENSOR_DATA;
-	try_fmt_meta->field = V4L2_FIELD_NONE;
-
 	/* Initialize try_crop rectangle. */
 	try_crop = v4l2_subdev_state_get_crop(fh->state, 0);
 	try_crop->top = supported_modes[0].crop.top;
@@ -1527,15 +1510,7 @@ static int mira220_enum_frame_size(struct v4l2_subdev *sd,
 		fse->max_width = fse->min_width;
 		fse->min_height = supported_modes[fse->index].height;
 		fse->max_height = fse->min_height;
-	} else {
-		if (fse->code != MEDIA_BUS_FMT_SENSOR_DATA || fse->index > 0)
-			return -EINVAL;
-
-		fse->min_width = MIRA220_EMBEDDED_LINE_WIDTH;
-		fse->max_width = fse->min_width;
-		fse->min_height = MIRA220_NUM_EMBEDDED_LINES;
-		fse->max_height = fse->min_height;
-	}
+	} 
 
 	return 0;
 }
@@ -1559,13 +1534,6 @@ static void mira220_update_image_pad_format(struct mira220 *mira220,
 	mira220_reset_colorspace(&fmt->format);
 }
 
-static void mira220_update_metadata_pad_format(struct v4l2_subdev_format *fmt)
-{
-	fmt->format.width = MIRA220_EMBEDDED_LINE_WIDTH;
-	fmt->format.height = MIRA220_NUM_EMBEDDED_LINES;
-	fmt->format.code = MEDIA_BUS_FMT_SENSOR_DATA;
-	fmt->format.field = V4L2_FIELD_NONE;
-}
 
 static int __mira220_get_pad_format(struct mira220 *mira220,
 				    struct v4l2_subdev_state *sd_state,
@@ -1590,9 +1558,7 @@ static int __mira220_get_pad_format(struct mira220 *mira220,
 			fmt->format.code =
 				mira220_validate_format_code_or_default(
 					mira220, mira220->fmt.code);
-		} else {
-			mira220_update_metadata_pad_format(fmt);
-		}
+		} 
 	}
 
 	return 0;
@@ -1686,10 +1652,7 @@ static int mira220_set_pad_format(struct v4l2_subdev *sd,
 			framefmt = v4l2_subdev_state_get_format(sd_state,
 								fmt->pad);
 			*framefmt = fmt->format;
-		} else {
-			/* Only one embedded data mode is supported */
-			mira220_update_metadata_pad_format(fmt);
-		}
+		} 
 	}
 
 	mutex_unlock(&mira220->mutex);
@@ -2130,53 +2093,6 @@ static void mira220_free_controls(struct mira220 *mira220)
 	mutex_destroy(&mira220->mutex);
 }
 
-static int mira220_check_hwcfg(struct device *dev)
-{
-	struct fwnode_handle *endpoint;
-	struct v4l2_fwnode_endpoint ep_cfg = { .bus_type =
-						       V4L2_MBUS_CSI2_DPHY };
-	int ret = -EINVAL;
-
-	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
-	if (!endpoint) {
-		dev_err(dev, "endpoint node not found\n");
-		return -EINVAL;
-	}
-
-	if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg)) {
-		dev_err(dev, "could not parse endpoint\n");
-		goto error_out;
-	}
-
-	/* Check the number of MIPI CSI2 data lanes */
-	if (ep_cfg.bus.mipi_csi2.num_data_lanes != 2) {
-		dev_err(dev, "only 2 data lanes are currently supported\n");
-		goto error_out;
-	}
-
-	/* Check the link frequency set in device tree */
-	if (!ep_cfg.nr_of_link_frequencies) {
-		dev_err(dev, "link-frequency property not found in DT\n");
-		goto error_out;
-	}
-
-	if (ep_cfg.nr_of_link_frequencies != 1 ||
-	    ep_cfg.link_frequencies[0] != MIRA220_DEFAULT_LINK_FREQ) {
-		dev_err(dev, "Link frequency not supported: %lld\n",
-			ep_cfg.link_frequencies[0]);
-		goto error_out;
-	}
-
-	// TODO(jalv): Check device tree configuration and make sure it is supported by the driver
-	ret = 0;
-
-error_out:
-	v4l2_fwnode_endpoint_free(&ep_cfg);
-	fwnode_handle_put(endpoint);
-
-	return ret;
-}
-
 static int mira220_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -2192,10 +2108,6 @@ static int mira220_probe(struct i2c_client *client)
 	v4l2_i2c_subdev_init(&mira220->sd, client, &mira220_subdev_ops);
 	mira220->regmap = devm_cci_regmap_init_i2c(client, 16);
 
-	/* Check the hardware configuration in device tree */
-	if (mira220_check_hwcfg(dev))
-		return -EINVAL;
-
 	/* Parse device tree to check if dtoverlay has param skip-reg-upload=1 */
 	/* Set default TBD I2C device address to LED I2C Address*/
 
@@ -2250,7 +2162,6 @@ static int mira220_probe(struct i2c_client *client)
 
 	/* Initialize source pads */
 	mira220->pad[IMAGE_PAD].flags = MEDIA_PAD_FL_SOURCE;
-	mira220->pad[METADATA_PAD].flags = MEDIA_PAD_FL_SOURCE;
 
 
 	/* Initialize default format */