Re: [PATCH v2] staging: atomisp: add a driver for ov5648 camera sensor
From: Andy Shevchenko
Date: Sun Oct 01 2017 - 06:33:02 EST
On Sun, Sep 24, 2017 at 5:59 PM, Devid Antonio Floni
<d.filoni@xxxxxxxxxx> wrote:
> The ov5648 5-megapixel camera sensor from OmniVision supports up to 2592x1944
> resolution and MIPI CSI-2 interface. Output format is raw sRGB/Bayer with
> 10 bits per colour (SGRBG10_1X10).
>
> This patch is a port of ov5648 driver after applying following
> 01org/ProductionKernelQuilts patches:
> - 0004-ov2680-ov5648-Fork-lift-source-from-CTS.patch
> - 0005-ov2680-ov5648-gminification.patch
> - 0006-ov5648-Focus-support.patch
> - 0007-Fix-resolution-issues-on-rear-camera.patch
> - 0008-ov2680-ov5648-Enabled-the-set_exposure-functions.patch
> - 0010-IRDA-cameras-mode-list-cleanup-unification.patch
> - 0012-ov5648-Add-1296x972-binned-mode.patch
> - 0014-ov5648-Adapt-to-Atomisp2-Gmin-VCM-framework.patch
> - 0015-dw9714-Gmin-Atomisp-specific-VCM-driver.patch
> - 0017-ov5648-Fix-deadlock-on-I2C-error.patch
> - 0018-gc2155-Fix-deadlock-on-error.patch
> - 0019-ov5648-Add-1280x960-binned-mode.patch
> - 0020-ov5648-Make-1280x960-as-default-video-resolution.patch
> - 0021-MALATA-Fix-testCameraToSurfaceTextureMetadata-CTS.patch
> - 0023-OV5648-Added-5MP-video-resolution.patch
>
> New changes introduced during the port:
> - Rename entity types to entity functions
> - Replace v4l2_subdev_fh by v4l2_subdev_pad_config
> - Make use of media_bus_format enum
> - Rename media_entity_init function to media_entity_pads_init
> - Replace try_mbus_fmt by set_fmt
> - Replace s_mbus_fmt by set_fmt
> - Replace g_mbus_fmt by get_fmt
> - Use s_ctrl/g_volatile_ctrl instead of ctrl core ops
> - Update gmin platform API path
> - Constify acpi_device_id
> - Add "INT5648" value to acpi_device_id
> - Fix some checkpatch errors and warnings
> - Remove FSF's mailing address from the sample GPL notice
>
> Changes in v2:
> - Fix indentation
> - Add atomisp prefix to Kconfig option
I discussed with Sakari about my review and we agreed that two things
that I have mentioned (converting to smbus calls and regulator
framework) would be a material for future changes.
Other than that, please, address the rest of comments and we will be fine.
You may also refer to my last patch series WRT atomisp driver where I
tried to address my own comments to the rest of the code.
--
With Best Regards,
Andy Shevchenko