Hi,
On 1/20/23 19:26, Brent Pappas wrote:
> Replace the macros SUBWORD() and INV_SUBWORD() with functions to comply
> with Linux coding style standards.
>
> Signed-off-by: Brent Pappas <bpappas@xxxxxxxxxxxxxxx>
Thank you.
I have added this to my personal git tree now and I will include
this in the atomisp driver pull-req which I will send to the
media-subsystem maintainer in a couple of weeks.
Regards,
Hans
> ---
> I am not sure if it would better to inline SUBWORD() or turn it into
> a function.
> On the one hand, SUBWORD() is only invoked once, so it may be better to
> to inline it.
> On the other hand, the macro defined beside it, INV_SUBWORD() should be
> turned into to a function because it is invoked multiple times, so it
> may make sense to turn SUBWORD() into a function as well.
> I have opted to turn SUBWORD() into a function.
>
> .../pci/hive_isp_css_common/host/vmem.c | 21 +++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c
> index 6620f091442f..316abfb72a83 100644
> --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c
> +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c
> @@ -28,10 +28,19 @@ typedef hive_uedge *hive_wide;
> /* Copied from SDK: sim_semantics.c */
>
> /* subword bits move like this: MSB[____xxxx____]LSB -> MSB[00000000xxxx]LSB */
> -#define SUBWORD(w, start, end) (((w) & (((1ULL << ((end) - 1)) - 1) << 1 | 1)) >> (start))
> +static inline hive_uedge
> +subword(hive_uedge w, unsigned int start, unsigned int end)
> +{
> + return (w & (((1ULL << (end - 1)) - 1) << 1 | 1)) >> start;
> +}
>
> /* inverse subword bits move like this: MSB[xxxx____xxxx]LSB -> MSB[xxxx0000xxxx]LSB */
> -#define INV_SUBWORD(w, start, end) ((w) & (~(((1ULL << ((end) - 1)) - 1) << 1 | 1) | ((1ULL << (start)) - 1)))
> +static inline hive_uedge
> +inv_subword(hive_uedge w, unsigned int start, unsigned int end)
> +{
> + return w & (~(((1ULL << (end - 1)) - 1) << 1 | 1) |
> + ((1ULL << start) - 1));
> +}
>
> #define uedge_bits (8 * sizeof(hive_uedge))
> #define move_lower_bits(target, target_bit, src, src_bit) move_subword(target, target_bit, src, 0, src_bit)
> @@ -50,18 +59,18 @@ move_subword(
> unsigned int start_bit = target_bit % uedge_bits;
> unsigned int subword_width = src_end - src_start;
>
> - hive_uedge src_subword = SUBWORD(src, src_start, src_end);
> + hive_uedge src_subword = subword(src, src_start, src_end);
>
> if (subword_width + start_bit > uedge_bits) { /* overlap */
> hive_uedge old_val1;
> - hive_uedge old_val0 = INV_SUBWORD(target[start_elem], start_bit, uedge_bits);
> + hive_uedge old_val0 = inv_subword(target[start_elem], start_bit, uedge_bits);
>
> target[start_elem] = old_val0 | (src_subword << start_bit);
> - old_val1 = INV_SUBWORD(target[start_elem + 1], 0,
> + old_val1 = inv_subword(target[start_elem + 1], 0,
> subword_width + start_bit - uedge_bits);
> target[start_elem + 1] = old_val1 | (src_subword >> (uedge_bits - start_bit));
> } else {
> - hive_uedge old_val = INV_SUBWORD(target[start_elem], start_bit,
> + hive_uedge old_val = inv_subword(target[start_elem], start_bit,
> start_bit + subword_width);
>
> target[start_elem] = old_val | (src_subword << start_bit);
