[libcamera-devel] [PATCH 04/31] libcamera: ipa: Remove unused IPA control types

[Kieran Bingham]

Hola,

On 29/02/2020 16:42, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
> 
> The ipa_control_range_data structure is only used to document the IPA
> control serialization format, but isn't used in code at all as the
> ControlRange entries are directly serialized to a byte stream buffer.
> This applies to the ipa_control_value_data structure that is solely used
> by ipa_control_range_data.
> 
> Expand the IPA control serialization format documentation to describe

That feels like it needs a bit of a reword:

"Expand the documentation of the IPA control serialization format to
describe..."


> the layout of control range data in words and diagrams instead of

"of the control range data"?

"of control range-data"?

> through a C structure. Remove the unused structures as a result.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  include/ipa/ipa_controls.h     | 11 ----------
>  src/libcamera/ipa_controls.cpp | 38 ++++++++++++++--------------------
>  2 files changed, 16 insertions(+), 33 deletions(-)
> 
> diff --git a/include/ipa/ipa_controls.h b/include/ipa/ipa_controls.h
> index de3a017b0179..426d99689de2 100644
> --- a/include/ipa/ipa_controls.h
> +++ b/include/ipa/ipa_controls.h
> @@ -36,17 +36,6 @@ struct ipa_control_range_entry {
>  	uint32_t padding[1];
>  };
>  
> -union ipa_control_value_data {
> -	bool b;
> -	int32_t i32;
> -	int64_t i64;
> -};
> -
> -struct ipa_control_range_data {
> -	union ipa_control_value_data min;
> -	union ipa_control_value_data max;
> -};
> -
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/src/libcamera/ipa_controls.cpp b/src/libcamera/ipa_controls.cpp
> index ed12830c0d9e..6ea71bc6dc46 100644
> --- a/src/libcamera/ipa_controls.cpp
> +++ b/src/libcamera/ipa_controls.cpp
> @@ -100,8 +100,22 @@
>   *
>   * Entries are described by the ipa_control_range_entry structure. They contain
>   * the numerical ID and type of the control. The control range data is stored
> - * in the data section as described by the ipa_control_range_data structure.
> - * The ipa_control_range_entry::offset field stores the offset from the
> + * in the data section as described by the following diagram.
> + *
> + * ~~~~
> + *           +-------------------------+       .
> + *         / | ...                     |       | entry[n].offset
> + *         | +-------------------------+ <-----´
> + *    Data | | minimum value (#n)      | \
> + * section | +-------------------------+ | Entry #n
> + *         | | maximum value (#n)      | /
> + *         | +-------------------------+
> + *         \ | ...                     |
> + *           +-------------------------+
> + * ~~~~
> + *
> + * The minimum and maximum value are stored in the platform's native data
> + * format. The ipa_control_range_entry::offset field stores the offset from the
>   * beginning of the data section to the range data.
>   *
>   * Range data in the data section shall be stored in the same order as the
> @@ -164,23 +178,3 @@
>   * \var ipa_control_range_entry::padding
>   * Padding bytes (shall be set to 0)
>   */
> -
> -/**
> - * \union ipa_control_value_data
> - * \brief Serialized control value
> - * \var ipa_control_value_data::b
> - * Value for ControlTypeBool controls
> - * \var ipa_control_value_data::i32
> - * Value for ControlTypeInteger32 controls
> - * \var ipa_control_value_data::i64
> - * Value for ControlTypeInteger64 controls
> - */
> -
> -/**
> - * \struct ipa_control_range_data
> - * \brief Serialized control range
> - * \var ipa_control_range_data::min
> - * The control minimum value
> - * \var ipa_control_range_data::max
> - * The control maximum value
> - */
> 

-- 
Regards
--
Kieran