On 24/01/2023 16:05, Naushir Patuck wrote: > From: Nick Hollinghurst <nick.hollinghurst@xxxxxxxxxxxxxxx> > > The imx708 is a 12MP MIPI sensor with a 16:9 aspect ratio, here using > two CSI-2 lanes. It is a "quad Bayer" sensor with all 3 modes offering > 10-bit output: > > 12MP: 4608x2592 up to 14.35fps (full FoV) > 1080p: 2304x1296 up to 56.02fps (full FoV) > 720p: 1536x864 up to 120.12fps (cropped) > Thank you for your patch. There is something to discuss/improve. > + > +static int imx708_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct imx708 *imx708; > + int ret; > + > + imx708 = devm_kzalloc(&client->dev, sizeof(*imx708), GFP_KERNEL); > + if (!imx708) > + return -ENOMEM; > + > + v4l2_i2c_subdev_init(&imx708->sd, client, &imx708_subdev_ops); > + > + /* Check the hardware configuration in device tree */ > + if (imx708_check_hwcfg(dev)) > + return -EINVAL; > + > + /* Get system clock (xclk) */ > + imx708->xclk = devm_clk_get(dev, NULL); > + if (IS_ERR(imx708->xclk)) { > + dev_err(dev, "failed to get xclk\n"); return dev_err_probe(). > + return PTR_ERR(imx708->xclk); > + } > + > + imx708->xclk_freq = clk_get_rate(imx708->xclk); > + if (imx708->xclk_freq != IMX708_XCLK_FREQ) { > + dev_err(dev, "xclk frequency not supported: %d Hz\n", > + imx708->xclk_freq); > + return -EINVAL; > + } > + > + ret = imx708_get_regulators(imx708); > + if (ret) { > + dev_err(dev, "failed to get regulators\n"); return dev_err_probe(). > + return ret; > + } > + > + /* Request optional enable pin */ > + imx708->reset_gpio = devm_gpiod_get_optional(dev, "reset", > + GPIOD_OUT_HIGH); > + > + /* > + * The sensor must be powered for imx708_identify_module() > + * to be able to read the CHIP_ID register > + */ > + ret = imx708_power_on(dev); > + if (ret) > + return ret; > + > + ret = imx708_identify_module(imx708); > + if (ret) > + goto error_power_off; > + > + /* Initialize default format */ > + imx708_set_default_format(imx708); > + > + /* > + * Enable runtime PM with autosuspend. As the device has been powered > + * manually, mark it as active, and increase the usage count without > + * resuming the device. > + */ > + pm_runtime_set_active(dev); > + pm_runtime_get_noresume(dev); > + pm_runtime_enable(dev); > + pm_runtime_set_autosuspend_delay(dev, 1000); > + pm_runtime_use_autosuspend(dev); > + > + /* This needs the pm runtime to be registered. */ > + ret = imx708_init_controls(imx708); > + if (ret) > + goto error_power_off; > + > + /* Initialize subdev */ > + imx708->sd.internal_ops = &imx708_internal_ops; > + imx708->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | > + V4L2_SUBDEV_FL_HAS_EVENTS; > + imx708->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; > + > + /* Initialize source pad */ > + imx708->pad.flags = MEDIA_PAD_FL_SOURCE; > + > + ret = media_entity_pads_init(&imx708->sd.entity, 1, &imx708->pad); > + if (ret) { > + dev_err(dev, "failed to init entity pads: %d\n", ret); return dev_err_probe(). > + goto error_handler_free; > + } > + > + ret = v4l2_async_register_subdev_sensor(&imx708->sd); > + if (ret < 0) { > + dev_err(dev, "failed to register sensor sub-device: %d\n", ret); return dev_err_probe(). Best regards, Krzysztof