Re: [PATCH v3] staging: atomisp: add a driver for ov5648 camera sensor

2017-10-03 Thread Andy Shevchenko
On Mon, 2017-10-02 at 21:30 +0200, Devid Antonio Filoni 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
> 
> "INT5648" ACPI device id can be found in following production
> hardware:
> BIOS Information
> Vendor: LENOVO
> Version: 1HCN40WW
> Release Date: 11/04/2016
> ...
> BIOS Revision: 0.40
> Firmware Revision: 0.0
> ...
> System Information
> Manufacturer: LENOVO
> Product Name: 80SG
> Version: MIIX 310-10ICR
> ...
> SKU Number: LENOVO_MT_80SG_BU_idea_FM_MIIX 310-10ICR
> Family: IDEAPAD
> ...
> 
> Device DSDT excerpt:
> Device (CA00)
> {
> Name (_ADR, Zero)  // _ADR: Address
> Name (_HID, "INT5648")  // _HID: Hardware ID
> Name (_CID, "INT5648")  // _CID: Compatible ID
> Name (_SUB, "INTL")  // _SUB: Subsystem ID
> Name (_DDN, "ov5648")  // _DDN: DOS Device Name
> ...
> 
> I was not able to properly test this patch on my Lenovo Miix 310 due
> to other
> issues with atomisp,

Can you elaborate this a bit?

>  the output is the same as ov2680 driver (OVTI2680)
> which is very similar.
> 

Other than above the patch looks good enough for staging, though still
requires a lot of clean up work.

Reviewed-by: Andy Shevchenko 

> Signed-off-by: Devid Antonio Filoni 
> ---
> Changes in v2:
>  - Fix indentation
>  - Add atomisp prefix to Kconfig option
> 
> Changes in v3:
>  - Use module_i2c_driver() macro
>  - Switch i2c drivers to use ->probe_new()
>  - Remove unused ->gpio_ctrl() callback
>  - Remove unused ->power_ctrl() callback
>  - Remove unneeded header inclusions
>  - Sort header inclusions alphabetically
>  - Replace kzalloc with devm_kzalloc
>  - Remove "XXOV5648" acpi_device_id, we don't know if it's used in any
> production device
>  - Use reverse XMAS tree declarations
>  - Fix comments style
>  - Remove __func__ from all dev_{dbg,info,err}() calls
>  - Add missing new line chars in all dev_{dbg,info,err}() calls
>  - Remove useless dev_{dbg,err}() calls
>  - Fix all checkpatch.pl issues
> 
>  drivers/staging/media/atomisp/i2c/Kconfig  |   12 +
>  drivers/staging/media/atomisp/i2c/Makefile |1 +
>  drivers/staging/media/atomisp/i2c/atomisp-ov5648.c | 1787
> 
>  drivers/staging/media/atomisp/i2c/ov5648.h |  828 +
>  4 files changed, 2628 insertions(+)
>  create mode 100644 drivers/staging/media/atomisp/i2c/atomisp-ov5648.c
>  create mode 100644 drivers/staging/media/atomisp/i2c/ov5648.h
> 
> diff --git a/drivers/staging/media/atomisp/i2c/Kconfig
> b/drivers/staging/media/atomisp/i2c/Kconfig
> index a76f17d..6bd849d 100644
> --- a/drivers/staging/media/atomisp/i2c/Kconfig
> +++ b/drivers/staging/media/atomisp/i2c/Kconfig
> @@ -83,6 +83,18 @@ config VIDEO_ATOMISP_OV2680
>  
>It currently only works with the atomisp driver.
>  
> +config VIDEO_ATOMISP_OV5648
> + tristate "Omnivision OV5648 sensor support"
> + depends on ACPI
> + depends on I2C && VIDEO_V4L2
> + ---help---
> +  This is a Video4Linux2 sensor-level driver for the
> Omnivision
> +  OV5648 raw camera.
> +
> +  ov5648 is a 5M raw sensor.
> +
> +  It currently only works with the 

[PATCH v3] staging: atomisp: add a driver for ov5648 camera sensor

2017-10-02 Thread Devid Antonio Filoni
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

"INT5648" ACPI device id can be found in following production hardware:
BIOS Information
Vendor: LENOVO
Version: 1HCN40WW
Release Date: 11/04/2016
...
BIOS Revision: 0.40
Firmware Revision: 0.0
...
System Information
Manufacturer: LENOVO
Product Name: 80SG
Version: MIIX 310-10ICR
...
SKU Number: LENOVO_MT_80SG_BU_idea_FM_MIIX 310-10ICR
Family: IDEAPAD
...

Device DSDT excerpt:
Device (CA00)
{
Name (_ADR, Zero)  // _ADR: Address
Name (_HID, "INT5648")  // _HID: Hardware ID
Name (_CID, "INT5648")  // _CID: Compatible ID
Name (_SUB, "INTL")  // _SUB: Subsystem ID
Name (_DDN, "ov5648")  // _DDN: DOS Device Name
...

I was not able to properly test this patch on my Lenovo Miix 310 due to other
issues with atomisp, the output is the same as ov2680 driver (OVTI2680)
which is very similar.

Signed-off-by: Devid Antonio Filoni 
---
Changes in v2:
 - Fix indentation
 - Add atomisp prefix to Kconfig option

Changes in v3:
 - Use module_i2c_driver() macro
 - Switch i2c drivers to use ->probe_new()
 - Remove unused ->gpio_ctrl() callback
 - Remove unused ->power_ctrl() callback
 - Remove unneeded header inclusions
 - Sort header inclusions alphabetically
 - Replace kzalloc with devm_kzalloc
 - Remove "XXOV5648" acpi_device_id, we don't know if it's used in any 
production device
 - Use reverse XMAS tree declarations
 - Fix comments style
 - Remove __func__ from all dev_{dbg,info,err}() calls
 - Add missing new line chars in all dev_{dbg,info,err}() calls
 - Remove useless dev_{dbg,err}() calls
 - Fix all checkpatch.pl issues

 drivers/staging/media/atomisp/i2c/Kconfig  |   12 +
 drivers/staging/media/atomisp/i2c/Makefile |1 +
 drivers/staging/media/atomisp/i2c/atomisp-ov5648.c | 1787 

 drivers/staging/media/atomisp/i2c/ov5648.h |  828 +
 4 files changed, 2628 insertions(+)
 create mode 100644 drivers/staging/media/atomisp/i2c/atomisp-ov5648.c
 create mode 100644 drivers/staging/media/atomisp/i2c/ov5648.h

diff --git a/drivers/staging/media/atomisp/i2c/Kconfig 
b/drivers/staging/media/atomisp/i2c/Kconfig
index a76f17d..6bd849d 100644
--- a/drivers/staging/media/atomisp/i2c/Kconfig
+++ b/drivers/staging/media/atomisp/i2c/Kconfig
@@ -83,6 +83,18 @@ config VIDEO_ATOMISP_OV2680
 
 It currently only works with the atomisp driver.
 
+config VIDEO_ATOMISP_OV5648
+   tristate "Omnivision OV5648 sensor support"
+   depends on ACPI
+   depends on I2C && VIDEO_V4L2
+   ---help---
+This is a Video4Linux2 sensor-level driver for the Omnivision
+OV5648 raw camera.
+
+ov5648 is a 5M raw sensor.
+
+It currently only works with the atomisp driver.
+
 #
 # Kconfig for flash drivers
 #
diff --git a/drivers/staging/media/atomisp/i2c/Makefile 
b/drivers/staging/media/atomisp/i2c/Makefile
index f8b3505..bb9cf7b 100644
--- a/drivers/staging/media/atomisp/i2c/Makefile
+++ b/drivers/staging/media/atomisp/i2c/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_VIDEO_ATOMISP_MT9M114)+= atomisp-mt9m114.o
 obj-$(CONFIG_VIDEO_ATOMISP_GC2235) += atomisp-gc2235.o
 obj-$(CONFIG_VIDEO_ATOMISP_OV2722) += atomisp-ov2722.o