Hi Mauro, On 9/6/24 8:15 AM, Mauro Carvalho Chehab wrote: > Em Sat, 27 Jul 2024 14:51:56 +0200 > Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> escreveu: > >> Using clamp_t() instead of min_t(max_t()) is easier to read. >> >> It also reduces the size of the preprocessed files by ~ 193 ko. >> (see [1] for a discussion about it) >> >> $ ls -l ia_css_eed1_8.host*.i >> 4829993 27 juil. 14:36 ia_css_eed1_8.host.old.i >> 4636649 27 juil. 14:42 ia_css_eed1_8.host.new.i >> >> [1]: https://lore.kernel.org/all/23bdb6fc8d884ceebeb6e8b8653b8cfe@xxxxxxxxxxxxxxxx/ >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> >> --- >> .../isp/kernels/eed1_8/ia_css_eed1_8.host.c | 24 +++++++++---------- >> 1 file changed, 12 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c >> index e4fc90f88e24..96c13ebc4331 100644 >> --- a/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c >> +++ b/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c >> @@ -172,25 +172,25 @@ ia_css_eed1_8_vmem_encode( >> base = shuffle_block * i; >> >> for (j = 0; j < IA_CSS_NUMBER_OF_DEW_ENHANCE_SEGMENTS; j++) { >> - to->e_dew_enh_x[0][base + j] = min_t(int, max_t(int, >> - from->dew_enhance_seg_x[j], 0), >> - 8191); >> - to->e_dew_enh_y[0][base + j] = min_t(int, max_t(int, >> - from->dew_enhance_seg_y[j], -8192), >> - 8191); >> + to->e_dew_enh_x[0][base + j] = clamp_t(int, >> + from->dew_enhance_seg_x[j], >> + 0, 8191); >> + to->e_dew_enh_y[0][base + j] = clamp_t(int, >> + from->dew_enhance_seg_y[j], >> + -8192, 8191); > > Such change introduces two warnings on smatch: > > drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c: drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c:177 ia_css_eed1_8_vmem_encode() warn: assigning (-8192) to unsigned variable 'to->e_dew_enh_y[0][base + j]' > drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c: drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c:182 ia_css_eed1_8_vmem_encode() warn: assigning (-8192) to unsigned variable 'to->e_dew_enh_a[0][base + j]' > > Should dew_enhance_seg_x and dew_enhance_seg_y be converted to signed? These already are s32, the problem is that e_dew_enh_a is of type t_vmem_elem which is: typedef u16 t_vmem_elem; And that type is used in a lot of places, so we cannot just change that. I guess we could add a t_signed_vmem_elem (s16) and use that for these vmem-arrays ? I tried fixing it like this: diff --git a/drivers/staging/media/atomisp/i2c/atomisp-t4ka3.c b/drivers/staging/media/atomisp/i2c/atomisp-t4ka3.c index 1e01d354152b..7c0195d15f53 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-t4ka3.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-t4ka3.c @@ -428,18 +428,13 @@ static int t4ka3_s_stream(struct v4l2_subdev *sd, int enable) goto error_unlock; } - ret = cci_multi_reg_write(sensor->regmap, t4ka3_init_config, - ARRAY_SIZE(t4ka3_init_config), NULL); - if (ret) - goto error_powerdown; - + cci_multi_reg_write(sensor->regmap, t4ka3_init_config, + ARRAY_SIZE(t4ka3_init_config), &ret); /* enable group hold */ - ret = cci_multi_reg_write(sensor->regmap, t4ka3_param_hold, - ARRAY_SIZE(t4ka3_param_hold), NULL); - if (ret) - goto error_powerdown; - - ret = cci_multi_reg_write(sensor->regmap, sensor->res->regs, sensor->res->regs_len, NULL); + cci_multi_reg_write(sensor->regmap, t4ka3_param_hold, + ARRAY_SIZE(t4ka3_param_hold), &ret); + cci_multi_reg_write(sensor->regmap, sensor->res->regs, + sensor->res->regs_len, &ret); if (ret) goto error_powerdown; diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c index b79d78e5b77f..c9043d516192 100644 --- a/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c +++ b/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c @@ -172,21 +172,21 @@ ia_css_eed1_8_vmem_encode( base = shuffle_block * i; for (j = 0; j < IA_CSS_NUMBER_OF_DEW_ENHANCE_SEGMENTS; j++) { - to->e_dew_enh_x[0][base + j] = clamp(from->dew_enhance_seg_x[j], - 0, 8191); - to->e_dew_enh_y[0][base + j] = clamp(from->dew_enhance_seg_y[j], - -8192, 8191); + to->e_dew_enh_x[0][base + j] = (u16)clamp(from->dew_enhance_seg_x[j], + 0, 8191); + to->e_dew_enh_y[0][base + j] = (u16)clamp(from->dew_enhance_seg_y[j], + -8192, 8191); } for (j = 0; j < (IA_CSS_NUMBER_OF_DEW_ENHANCE_SEGMENTS - 1); j++) { - to->e_dew_enh_a[0][base + j] = clamp(from->dew_enhance_seg_slope[j], - -8192, 8191); + to->e_dew_enh_a[0][base + j] = (u16)clamp(from->dew_enhance_seg_slope[j], + -8192, 8191); /* Convert dew_enhance_seg_exp to flag: * 0 -> 0 * 1...13 -> 1 */ - to->e_dew_enh_f[0][base + j] = clamp(from->dew_enhance_seg_exp[j], - 0, 13) > 0; + to->e_dew_enh_f[0][base + j] = (u16)clamp(from->dew_enhance_seg_exp[j], + 0, 13) > 0; } /* Hard-coded to 0, in order to be able to handle out of but smatch still complains after this... Regards, Hans
