Re: [PATCH v11 03/12] [media] exynos5-fimc-is: Add common driver header files

2013-11-06 Thread Sakari Ailus
Hi Sylwester and Arun,

On Wed, Nov 06, 2013 at 12:23:07PM +0100, Sylwester Nawrocki wrote:
> Hi,
> 
> On 05/11/13 14:16, Arun Kumar K wrote:
> >>> +struct is_common_reg {
> >>> + u32 hicmd;
> >>> + u32 hic_sensorid;
> >>> + u32 hic_param[4];
> >>> +
> >>> + u32 reserved1[3];
> [...]
> >>> + u32 meta_iflag;
> >>> + u32 meta_sensor_id;
> >>> + u32 meta_param1;
> >>> +
> >>> + u32 reserved9[1];
> >>> +
> >>> + u32 fcount;
> >>
> >> If these structs define an interface that's not used by the driver only it
> >> might be a good idea to use __packed to ensure no padding is added.
> >>
> > 
> > The same structure is used as is in the firmware code and so it is retained
> > in the driver.
> 
> I agree it makes sense to use __packed attribute to ensure no padding is
> added by the compiler. The firmware source and the driver will likely be 
> compiled with different toolchains, and in both cases we should ensure
> no padding is added.

Agreed.

> >>> diff --git a/drivers/media/platform/exynos5-is/fimc-is-metadata.h 
> >>> b/drivers/media/platform/exynos5-is/fimc-is-metadata.h
> >>> new file mode 100644
> >>> index 000..02367c4
> >>> --- /dev/null
> >>> +++ b/drivers/media/platform/exynos5-is/fimc-is-metadata.h
> >>> @@ -0,0 +1,767 @@
> [..]
> >>> +enum metadata_mode {
> >>> + METADATA_MODE_NONE,
> >>> + METADATA_MODE_FULL
> >>> +};
> >>> +
> >>> +struct camera2_request_ctl {
> >>> + uint32_tid;
> >>> + enum metadata_mode  metadatamode;
> >>> + uint8_t outputstreams[16];
> >>> + uint32_tframecount;
> >>> +};
> >>> +
> >>> +struct camera2_request_dm {
> >>> + uint32_tid;
> >>> + enum metadata_mode  metadatamode;
> >>> + uint32_tframecount;
> >>> +};
> [...]
> >>> +struct camera2_lens_ctl {
> >>> + uint32_tfocus_distance;
> >>> + float   aperture;
> >>
> >> Floating point numbers? Really? :-)
> >>
> > 
> > Yes as mentioned, the same structure is used by the firmware and
> > so it is used as is in the kernel.
> 
> These floating numbers are pretty painful, but I don't think they can
> be avoided unless the firmware is changed. I hope there is no need to 
> touch those in the kernel.
> 
> There are already precedents of using floating point numbers in driver's
> public interface, e.g. some gpu/drm drivers. 

As long as you can somehow ensure these will never end up to FPU registers,
I think that should be fine. Just copying the struct elsewhere using
memcpy() will be good, I believe.

> I noticed there is another issue in this firmware/kernel interface, i.e. 
> some data structures contain enums in them, e.g.
> 
> struct camera2_lens_ctl {
>   uint32_tfocus_distance;
>   float   aperture;
>   float   focal_length;
>   float   filter_density;
>   enum optical_stabilization_mode optical_stabilization_mode;
> };
> 
> It looks like a mistake in the interface design, as size of an enum is
> implementation specific.
> 
> I guess size of those enum types is supposed to be 4 bytes ? Presumably
> you should, e.g. use fixed data type like uin32_t or __u32 instead of those 
> enums. It looks pretty fragile as it is now.

Good point; I agree.

> In addition all those data structures should be declared with __packed
> attribute, to ensure a specific data structure layout and to avoid 
> an unexpected padding.
> 
> >> diff --git a/drivers/media/platform/exynos5-is/fimc-is-param.h 
> >> b/drivers/media/platform/exynos5-is/fimc-is-param.h
> >> new file mode 100644
> >> index 000..015cc13
> >> --- /dev/null
> >> +++ b/drivers/media/platform/exynos5-is/fimc-is-param.h
> > ...
> >> +struct param_control {
> >> +  u32 cmd;
> > 
> > You use uint32_t in some other headers. It's not wrong to use both C99 and
> > Linux types but I'd try to stick to either one.
> 
> I tend to agree with that, it's probably better to use one convention, u32
> for kernel internal structures and __u32 for any public interfaces. I don't
> think it is e requirement but would be nice to keep it more consistent.
> 
> Even if we wanted to keep the firmware defined data structures in sync with
> the Linux driver, there are already some Linux types used within the firmware
> interface. if I understood things correctly.

I guess it wouldn't hurt to use uint32_t there instead of u32 (and __u32).
entirely up to you.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v11 03/12] [media] exynos5-fimc-is: Add common driver header files

2013-11-06 Thread Arun Kumar K
Hi Sylwester,

On Wed, Nov 6, 2013 at 4:53 PM, Sylwester Nawrocki
 wrote:
> Hi,
>
> On 05/11/13 14:16, Arun Kumar K wrote:
 +struct is_common_reg {
 + u32 hicmd;
 + u32 hic_sensorid;
 + u32 hic_param[4];
 +
 + u32 reserved1[3];
> [...]
 + u32 meta_iflag;
 + u32 meta_sensor_id;
 + u32 meta_param1;
 +
 + u32 reserved9[1];
 +
 + u32 fcount;
>>>
>>> If these structs define an interface that's not used by the driver only it
>>> might be a good idea to use __packed to ensure no padding is added.
>>>
>>
>> The same structure is used as is in the firmware code and so it is retained
>> in the driver.
>
> I agree it makes sense to use __packed attribute to ensure no padding is
> added by the compiler. The firmware source and the driver will likely be
> compiled with different toolchains, and in both cases we should ensure
> no padding is added.
>

Yes the toolchains are different and ideally the firmware also should
use a __packed
attribute which possibly is not happening.

 diff --git a/drivers/media/platform/exynos5-is/fimc-is-metadata.h 
 b/drivers/media/platform/exynos5-is/fimc-is-metadata.h
 new file mode 100644
 index 000..02367c4
 --- /dev/null
 +++ b/drivers/media/platform/exynos5-is/fimc-is-metadata.h
 @@ -0,0 +1,767 @@
> [..]
 +enum metadata_mode {
 + METADATA_MODE_NONE,
 + METADATA_MODE_FULL
 +};
 +
 +struct camera2_request_ctl {
 + uint32_tid;
 + enum metadata_mode  metadatamode;
 + uint8_t outputstreams[16];
 + uint32_tframecount;
 +};
 +
 +struct camera2_request_dm {
 + uint32_tid;
 + enum metadata_mode  metadatamode;
 + uint32_tframecount;
 +};
> [...]
 +struct camera2_lens_ctl {
 + uint32_tfocus_distance;
 + float   aperture;
>>>
>>> Floating point numbers? Really? :-)
>>>
>>
>> Yes as mentioned, the same structure is used by the firmware and
>> so it is used as is in the kernel.
>
> These floating numbers are pretty painful, but I don't think they can
> be avoided unless the firmware is changed. I hope there is no need to
> touch those in the kernel.
>
> There are already precedents of using floating point numbers in driver's
> public interface, e.g. some gpu/drm drivers.
>

Ok

> I noticed there is another issue in this firmware/kernel interface, i.e.
> some data structures contain enums in them, e.g.
>
> struct camera2_lens_ctl {
> uint32_tfocus_distance;
> float   aperture;
> float   focal_length;
> float   filter_density;
> enum optical_stabilization_mode optical_stabilization_mode;
> };
>
> It looks like a mistake in the interface design, as size of an enum is
> implementation specific.
>
> I guess size of those enum types is supposed to be 4 bytes ? Presumably
> you should, e.g. use fixed data type like uin32_t or __u32 instead of those
> enums. It looks pretty fragile as it is now.
>

Yes its better to use 4byte data structures. Will change that.

> In addition all those data structures should be declared with __packed
> attribute, to ensure a specific data structure layout and to avoid
> an unexpected padding.
>

Ok.

>>> diff --git a/drivers/media/platform/exynos5-is/fimc-is-param.h 
>>> b/drivers/media/platform/exynos5-is/fimc-is-param.h
>>> new file mode 100644
>>> index 000..015cc13
>>> --- /dev/null
>>> +++ b/drivers/media/platform/exynos5-is/fimc-is-param.h
>> ...
>>> +struct param_control {
>>> +u32 cmd;
>>
>> You use uint32_t in some other headers. It's not wrong to use both C99 and
>> Linux types but I'd try to stick to either one.
>
> I tend to agree with that, it's probably better to use one convention, u32
> for kernel internal structures and __u32 for any public interfaces. I don't
> think it is e requirement but would be nice to keep it more consistent.
>

Ok

> Even if we wanted to keep the firmware defined data structures in sync with
> the Linux driver, there are already some Linux types used within the firmware
> interface. if I understood things correctly.
>
>>> +u32 bypass;
>>> +u32 buffer_address;
>>> +u32 buffer_number;
>>> +/* 0: continuous, 1: single */
>>> +u32 run_mode;
>>> +u32 reserved[PARAMETER_MAX_MEMBER - 6];
>>> +u32 err;
>>> +};
>
> Can you please address those issues in follow up patches ?
> I will be sending these patches for inclusion in the media tree,
> I would prefer to avoid keeping it on the ML for more than those
> 7 months already passed.
>

Ok will address these comments in follow up patches.

Thanks & Regards
Arun
--
To unsubscribe from this list: sen

Re: [PATCH v11 03/12] [media] exynos5-fimc-is: Add common driver header files

2013-11-06 Thread Sylwester Nawrocki
Hi,

On 05/11/13 14:16, Arun Kumar K wrote:
>>> +struct is_common_reg {
>>> + u32 hicmd;
>>> + u32 hic_sensorid;
>>> + u32 hic_param[4];
>>> +
>>> + u32 reserved1[3];
[...]
>>> + u32 meta_iflag;
>>> + u32 meta_sensor_id;
>>> + u32 meta_param1;
>>> +
>>> + u32 reserved9[1];
>>> +
>>> + u32 fcount;
>>
>> If these structs define an interface that's not used by the driver only it
>> might be a good idea to use __packed to ensure no padding is added.
>>
> 
> The same structure is used as is in the firmware code and so it is retained
> in the driver.

I agree it makes sense to use __packed attribute to ensure no padding is
added by the compiler. The firmware source and the driver will likely be 
compiled with different toolchains, and in both cases we should ensure
no padding is added.

>>> diff --git a/drivers/media/platform/exynos5-is/fimc-is-metadata.h 
>>> b/drivers/media/platform/exynos5-is/fimc-is-metadata.h
>>> new file mode 100644
>>> index 000..02367c4
>>> --- /dev/null
>>> +++ b/drivers/media/platform/exynos5-is/fimc-is-metadata.h
>>> @@ -0,0 +1,767 @@
[..]
>>> +enum metadata_mode {
>>> + METADATA_MODE_NONE,
>>> + METADATA_MODE_FULL
>>> +};
>>> +
>>> +struct camera2_request_ctl {
>>> + uint32_tid;
>>> + enum metadata_mode  metadatamode;
>>> + uint8_t outputstreams[16];
>>> + uint32_tframecount;
>>> +};
>>> +
>>> +struct camera2_request_dm {
>>> + uint32_tid;
>>> + enum metadata_mode  metadatamode;
>>> + uint32_tframecount;
>>> +};
[...]
>>> +struct camera2_lens_ctl {
>>> + uint32_tfocus_distance;
>>> + float   aperture;
>>
>> Floating point numbers? Really? :-)
>>
> 
> Yes as mentioned, the same structure is used by the firmware and
> so it is used as is in the kernel.

These floating numbers are pretty painful, but I don't think they can
be avoided unless the firmware is changed. I hope there is no need to 
touch those in the kernel.

There are already precedents of using floating point numbers in driver's
public interface, e.g. some gpu/drm drivers. 

I noticed there is another issue in this firmware/kernel interface, i.e. 
some data structures contain enums in them, e.g.

struct camera2_lens_ctl {
uint32_tfocus_distance;
float   aperture;
float   focal_length;
float   filter_density;
enum optical_stabilization_mode optical_stabilization_mode;
};

It looks like a mistake in the interface design, as size of an enum is
implementation specific.

I guess size of those enum types is supposed to be 4 bytes ? Presumably
you should, e.g. use fixed data type like uin32_t or __u32 instead of those 
enums. It looks pretty fragile as it is now.

In addition all those data structures should be declared with __packed
attribute, to ensure a specific data structure layout and to avoid 
an unexpected padding.

>> diff --git a/drivers/media/platform/exynos5-is/fimc-is-param.h 
>> b/drivers/media/platform/exynos5-is/fimc-is-param.h
>> new file mode 100644
>> index 000..015cc13
>> --- /dev/null
>> +++ b/drivers/media/platform/exynos5-is/fimc-is-param.h
> ...
>> +struct param_control {
>> +u32 cmd;
> 
> You use uint32_t in some other headers. It's not wrong to use both C99 and
> Linux types but I'd try to stick to either one.

I tend to agree with that, it's probably better to use one convention, u32
for kernel internal structures and __u32 for any public interfaces. I don't
think it is e requirement but would be nice to keep it more consistent.

Even if we wanted to keep the firmware defined data structures in sync with
the Linux driver, there are already some Linux types used within the firmware
interface. if I understood things correctly.

>> +u32 bypass;
>> +u32 buffer_address;
>> +u32 buffer_number;
>> +/* 0: continuous, 1: single */
>> +u32 run_mode;
>> +u32 reserved[PARAMETER_MAX_MEMBER - 6];
>> +u32 err;
>> +};

Can you please address those issues in follow up patches ?
I will be sending these patches for inclusion in the media tree,
I would prefer to avoid keeping it on the ML for more than those
7 months already passed.

--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v11 03/12] [media] exynos5-fimc-is: Add common driver header files

2013-11-05 Thread Arun Kumar K
Hi Sakari,

Thank you for the review.

On Tue, Nov 5, 2013 at 6:21 PM, Sakari Ailus  wrote:
> Hi Arun,
>
> On Tue, Nov 05, 2013 at 11:42:34AM +0530, Arun Kumar K wrote:
>> This patch adds all the common header files used by the fimc-is
>> driver. It includes the commands for interfacing with the firmware
>> and error codes from IS firmware, metadata and command parameter
>> definitions.
>>
>> Signed-off-by: Arun Kumar K 
>> Signed-off-by: Kilyeon Im 
>> Reviewed-by: Sylwester Nawrocki 
>> ---
>>  drivers/media/platform/exynos5-is/fimc-is-cmd.h|  187 
>>  drivers/media/platform/exynos5-is/fimc-is-err.h|  257 +
>>  .../media/platform/exynos5-is/fimc-is-metadata.h   |  767 +
>>  drivers/media/platform/exynos5-is/fimc-is-param.h  | 1159 
>> 
>>  4 files changed, 2370 insertions(+)
>>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-cmd.h
>>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-err.h
>>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-metadata.h
>>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-param.h
>>
>> diff --git a/drivers/media/platform/exynos5-is/fimc-is-cmd.h 
>> b/drivers/media/platform/exynos5-is/fimc-is-cmd.h
>> new file mode 100644
>> index 000..6250280
>> --- /dev/null
>> +++ b/drivers/media/platform/exynos5-is/fimc-is-cmd.h
>> @@ -0,0 +1,187 @@
>> +/*

[snip]

>> +struct is_common_reg {
>> + u32 hicmd;
>> + u32 hic_sensorid;
>> + u32 hic_param[4];
>> +
>> + u32 reserved1[3];
>> +
>> + u32 ihcmd_iflag;
>> + u32 ihcmd;
>> + u32 ihc_sensorid;
>> + u32 ihc_param[4];
>> +
>> + u32 reserved2[3];
>> +
>> + u32 isp_bayer_iflag;
>> + u32 isp_bayer_sensor_id;
>> + u32 isp_bayer_param[2];
>> +
>> + u32 reserved3[4];
>> +
>> + u32 scc_iflag;
>> + u32 scc_sensor_id;
>> + u32 scc_param[3];
>> +
>> + u32 reserved4[3];
>> +
>> + u32 dnr_iflag;
>> + u32 dnr_sensor_id;
>> + u32 dnr_param[2];
>> +
>> + u32 reserved5[4];
>> +
>> + u32 scp_iflag;
>> + u32 scp_sensor_id;
>> + u32 scp_param[3];
>> +
>> + u32 reserved6[1];
>> +
>> + u32 isp_yuv_iflag;
>> + u32 isp_yuv_sensor_id;
>> + u32 isp_yuv_param[2];
>> +
>> + u32 reserved7[1];
>> +
>> + u32 shot_iflag;
>> + u32 shot_sensor_id;
>> + u32 shot_param[2];
>> +
>> + u32 reserved8[1];
>> +
>> + u32 meta_iflag;
>> + u32 meta_sensor_id;
>> + u32 meta_param1;
>> +
>> + u32 reserved9[1];
>> +
>> + u32 fcount;
>
> If these structs define an interface that's not used by the driver only it
> might be a good idea to use __packed to ensure no padding is added.
>

The same structure is used as is in the firmware code and so it is retained
in the driver.

>> +};
>> +
>> +struct is_mcuctl_reg {
>> + u32 mcuctl;
>> + u32 bboar;
>> +
>> + u32 intgr0;
>> + u32 intcr0;
>> + u32 intmr0;
>> + u32 intsr0;
>> + u32 intmsr0;
>> +
>> + u32 intgr1;
>> + u32 intcr1;
>> + u32 intmr1;
>> + u32 intsr1;
>> + u32 intmsr1;
>> +
>> + u32 intcr2;
>> + u32 intmr2;
>> + u32 intsr2;
>> + u32 intmsr2;
>> +
>> + u32 gpoctrl;
>> + u32 cpoenctlr;
>> + u32 gpictlr;
>> +
>> + u32 pad[0xD];
>> +
>> + struct is_common_reg common_reg;
>> +};
>> +#endif
> ...
>> diff --git a/drivers/media/platform/exynos5-is/fimc-is-metadata.h 
>> b/drivers/media/platform/exynos5-is/fimc-is-metadata.h
>> new file mode 100644
>> index 000..02367c4
>> --- /dev/null
>> +++ b/drivers/media/platform/exynos5-is/fimc-is-metadata.h
>> @@ -0,0 +1,767 @@
>> +/*
>> + * Samsung EXYNOS5 FIMC-IS (Imaging Subsystem) driver
>> + *
>> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
>> + * Kil-yeon Lim 
>> + * Arun Kumar K 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef FIMC_IS_METADATA_H_
>> +#define FIMC_IS_METADATA_H_
>> +
>> +struct rational {
>> + uint32_t num;
>> + uint32_t den;
>> +};
>> +
>> +#define CAMERA2_MAX_AVAILABLE_MODE   21
>> +#define CAMERA2_MAX_FACES16
>> +
>> +/*
>> + * Controls/dynamic metadata
>> + */
>> +
>> +enum metadata_mode {
>> + METADATA_MODE_NONE,
>> + METADATA_MODE_FULL
>> +};
>> +
>> +struct camera2_request_ctl {
>> + uint32_tid;
>> + enum metadata_mode  metadatamode;
>> + uint8_t outputstreams[16];
>> + uint32_tframecount;
>> +};
>> +
>> +struct camera2_request_dm {
>> + uint32_tid;
>> + enum metadata_mode  metadatamode;
>> + uint32_tframecount;
>> +};
>> +
>> +
>> +
>> +enum optical_stabilization_mode {
>> + OPTICAL_STABILIZATION_MODE_OFF,
>> + OPTICAL_STABILIZATION_MODE_ON
>> +};
>> +
>> +enum lens_facing {
>> + LENS_FACING_B

Re: [PATCH v11 03/12] [media] exynos5-fimc-is: Add common driver header files

2013-11-05 Thread Sakari Ailus
Hi Arun,

On Tue, Nov 05, 2013 at 11:42:34AM +0530, Arun Kumar K wrote:
> This patch adds all the common header files used by the fimc-is
> driver. It includes the commands for interfacing with the firmware
> and error codes from IS firmware, metadata and command parameter
> definitions.
> 
> Signed-off-by: Arun Kumar K 
> Signed-off-by: Kilyeon Im 
> Reviewed-by: Sylwester Nawrocki 
> ---
>  drivers/media/platform/exynos5-is/fimc-is-cmd.h|  187 
>  drivers/media/platform/exynos5-is/fimc-is-err.h|  257 +
>  .../media/platform/exynos5-is/fimc-is-metadata.h   |  767 +
>  drivers/media/platform/exynos5-is/fimc-is-param.h  | 1159 
> 
>  4 files changed, 2370 insertions(+)
>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-cmd.h
>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-err.h
>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-metadata.h
>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-param.h
> 
> diff --git a/drivers/media/platform/exynos5-is/fimc-is-cmd.h 
> b/drivers/media/platform/exynos5-is/fimc-is-cmd.h
> new file mode 100644
> index 000..6250280
> --- /dev/null
> +++ b/drivers/media/platform/exynos5-is/fimc-is-cmd.h
> @@ -0,0 +1,187 @@
> +/*
> + * Samsung Exynos5 SoC series FIMC-IS driver
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd
> + * Kil-yeon Lim 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef FIMC_IS_CMD_H
> +#define FIMC_IS_CMD_H
> +
> +#define IS_COMMAND_VER 122 /* IS COMMAND VERSION 1.22 */
> +
> +enum is_cmd {
> + /* HOST -> IS */
> + HIC_PREVIEW_STILL = 0x1,
> + HIC_PREVIEW_VIDEO,
> + HIC_CAPTURE_STILL,
> + HIC_CAPTURE_VIDEO,
> + HIC_PROCESS_START,
> + HIC_PROCESS_STOP,
> + HIC_STREAM_ON,
> + HIC_STREAM_OFF,
> + HIC_SHOT,
> + HIC_GET_STATIC_METADATA,
> + HIC_SET_CAM_CONTROL,
> + HIC_GET_CAM_CONTROL,
> + HIC_SET_PARAMETER,
> + HIC_GET_PARAMETER,
> + HIC_SET_A5_MEM_ACCESS,
> + RESERVED2,
> + HIC_GET_STATUS,
> + /* SENSOR PART*/
> + HIC_OPEN_SENSOR,
> + HIC_CLOSE_SENSOR,
> + HIC_SIMMIAN_INIT,
> + HIC_SIMMIAN_WRITE,
> + HIC_SIMMIAN_READ,
> + HIC_POWER_DOWN,
> + HIC_GET_SET_FILE_ADDR,
> + HIC_LOAD_SET_FILE,
> + HIC_MSG_CONFIG,
> + HIC_MSG_TEST,
> + /* IS -> HOST */
> + IHC_GET_SENSOR_NUMBER = 0x1000,
> + /* Parameter1 : Address of space to copy a setfile */
> + /* Parameter2 : Space szie */
> + IHC_SET_SHOT_MARK,
> + /* PARAM1 : a frame number */
> + /* PARAM2 : confidence level(smile 0~100) */
> + /* PARMA3 : confidence level(blink 0~100) */
> + IHC_SET_FACE_MARK,
> + /* PARAM1 : coordinate count */
> + /* PARAM2 : coordinate buffer address */
> + IHC_FRAME_DONE,
> + /* PARAM1 : frame start number */
> + /* PARAM2 : frame count */
> + IHC_AA_DONE,
> + IHC_NOT_READY,
> + IHC_FLASH_READY
> +};
> +
> +enum is_reply {
> + ISR_DONE= 0x2000,
> + ISR_NDONE
> +};
> +
> +enum is_scenario_id {
> + ISS_PREVIEW_STILL,
> + ISS_PREVIEW_VIDEO,
> + ISS_CAPTURE_STILL,
> + ISS_CAPTURE_VIDEO,
> + ISS_END
> +};
> +
> +enum is_subscenario_id {
> + ISS_SUB_SCENARIO_STILL,
> + ISS_SUB_SCENARIO_VIDEO,
> + ISS_SUB_SCENARIO_SCENE1,
> + ISS_SUB_SCENARIO_SCENE2,
> + ISS_SUB_SCENARIO_SCENE3,
> + ISS_SUB_END
> +};
> +
> +struct is_setfile_header_element {
> + u32 binary_addr;
> + u32 binary_size;
> +};
> +
> +struct is_setfile_header {
> + struct is_setfile_header_element isp[ISS_END];
> + struct is_setfile_header_element drc[ISS_END];
> + struct is_setfile_header_element fd[ISS_END];
> +};
> +
> +struct is_common_reg {
> + u32 hicmd;
> + u32 hic_sensorid;
> + u32 hic_param[4];
> +
> + u32 reserved1[3];
> +
> + u32 ihcmd_iflag;
> + u32 ihcmd;
> + u32 ihc_sensorid;
> + u32 ihc_param[4];
> +
> + u32 reserved2[3];
> +
> + u32 isp_bayer_iflag;
> + u32 isp_bayer_sensor_id;
> + u32 isp_bayer_param[2];
> +
> + u32 reserved3[4];
> +
> + u32 scc_iflag;
> + u32 scc_sensor_id;
> + u32 scc_param[3];
> +
> + u32 reserved4[3];
> +
> + u32 dnr_iflag;
> + u32 dnr_sensor_id;
> + u32 dnr_param[2];
> +
> + u32 reserved5[4];
> +
> + u32 scp_iflag;
> + u32 scp_sensor_id;
> + u32 scp_param[3];
> +
> + u32 reserved6[1];
> +
> + u32 isp_yuv_iflag;
> + u32 isp_yuv_sensor_id;
> + u32 isp_yuv_param[2];
> +
> + u32 reserved7[1];
> +
> + u32 shot_iflag;
> + u32 shot_sensor_id;
> + u32 shot_param[2];
> +
> + u32 reserved8[1];
> +
> + u32 meta_iflag;
> + u32 meta_sensor_id;
> + u32 meta_param1;
> +
> + u32 reserved9[1];
> +
> + u32 fcount;

If thes