linux-kernel - Re: [PATCH v4 3/5] vimc: move duplicated IS_SRC and IS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 15 Sep 2019 16:25:44 -0300
From:   Helen Koike <helen.koike@...labora.com>
To:     Shuah Khan <skhan@...uxfoundation.org>, mchehab@...nel.org,
        andrealmeid@...labora.com, dafna.hirschfeld@...labora.com,
        hverkuil-cisco@...all.nl
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/5] vimc: move duplicated IS_SRC and IS_SINK to common
 header

Hi Shuah,

On 9/6/19 11:42 PM, Shuah Khan wrote:
> Move duplicated IS_SRC and IS_SINK dfines to common header. Rename
> them to VIMC_IS_SRC and VIM_IS_SINK.
> 
> Signed-off-by: Shuah Khan <skhan@...uxfoundation.org>
> ---
>  drivers/media/platform/vimc/vimc-common.h  |  4 ++++
>  drivers/media/platform/vimc/vimc-debayer.c | 11 ++++-------
>  drivers/media/platform/vimc/vimc-scaler.c  |  8 +++-----
>  3 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
> index 87ee84f78322..236412ad7548 100644
> --- a/drivers/media/platform/vimc/vimc-common.h
> +++ b/drivers/media/platform/vimc/vimc-common.h
> @@ -27,6 +27,10 @@
>  
>  #define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp)
>  
> +/* Source and sink pad checks */
> +#define VIMC_IS_SRC(pad)	(pad)
> +#define VIMC_IS_SINK(pad)	(!(pad))

This is true now, but it might not be true in the future.
In the output video device (that was sent by André but not yet upstream) for instance, only have a single
source pad (which I suppose the index will be 0), and this macro won't be true.

Maybe we could check pad flags in sd->entity->pads[index].flags ?

Thanks
Helen

> +
>  /**
>   * struct vimc_colorimetry_clamp - Adjust colorimetry parameters
>   *
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> index b38b55f51a24..37f3767db469 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -22,9 +22,6 @@ MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n"
>  	"stays in the center of the window, otherwise the next odd number "
>  	"is considered");
>  
> -#define IS_SINK(pad) (!pad)
> -#define IS_SRC(pad)  (pad)
> -
>  enum vimc_deb_rgb_colors {
>  	VIMC_DEB_RED = 0,
>  	VIMC_DEB_GREEN = 1,
> @@ -157,7 +154,7 @@ static int vimc_deb_enum_mbus_code(struct v4l2_subdev *sd,
>  				   struct v4l2_subdev_mbus_code_enum *code)
>  {
>  	/* We only support one format for source pads */
> -	if (IS_SRC(code->pad)) {
> +	if (VIMC_IS_SRC(code->pad)) {
>  		struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
>  
>  		if (code->index)
> @@ -183,7 +180,7 @@ static int vimc_deb_enum_frame_size(struct v4l2_subdev *sd,
>  	if (fse->index)
>  		return -EINVAL;
>  
> -	if (IS_SINK(fse->pad)) {
> +	if (VIMC_IS_SINK(fse->pad)) {
>  		const struct vimc_deb_pix_map *vpix =
>  			vimc_deb_pix_map_by_code(fse->code);
>  
> @@ -213,7 +210,7 @@ static int vimc_deb_get_fmt(struct v4l2_subdev *sd,
>  		      vdeb->sink_fmt;
>  
>  	/* Set the right code for the source pad */
> -	if (IS_SRC(fmt->pad))
> +	if (VIMC_IS_SRC(fmt->pad))
>  		fmt->format.code = vdeb->src_code;
>  
>  	return 0;
> @@ -260,7 +257,7 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd,
>  	 * Do not change the format of the source pad,
>  	 * it is propagated from the sink
>  	 */
> -	if (IS_SRC(fmt->pad)) {
> +	if (VIMC_IS_SRC(fmt->pad)) {
>  		fmt->format = *sink_fmt;
>  		/* TODO: Add support for other formats */
>  		fmt->format.code = vdeb->src_code;
> diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
> index 05db5070e268..a5a0855ad9cd 100644
> --- a/drivers/media/platform/vimc/vimc-scaler.c
> +++ b/drivers/media/platform/vimc/vimc-scaler.c
> @@ -16,8 +16,6 @@ static unsigned int sca_mult = 3;
>  module_param(sca_mult, uint, 0000);
>  MODULE_PARM_DESC(sca_mult, " the image size multiplier");
>  
> -#define IS_SINK(pad)	(!pad)
> -#define IS_SRC(pad)	(pad)
>  #define MAX_ZOOM	8
>  
>  struct vimc_sca_device {
> @@ -93,7 +91,7 @@ static int vimc_sca_enum_frame_size(struct v4l2_subdev *sd,
>  	fse->min_width = VIMC_FRAME_MIN_WIDTH;
>  	fse->min_height = VIMC_FRAME_MIN_HEIGHT;
>  
> -	if (IS_SINK(fse->pad)) {
> +	if (VIMC_IS_SINK(fse->pad)) {
>  		fse->max_width = VIMC_FRAME_MAX_WIDTH;
>  		fse->max_height = VIMC_FRAME_MAX_HEIGHT;
>  	} else {
> @@ -116,7 +114,7 @@ static int vimc_sca_get_fmt(struct v4l2_subdev *sd,
>  			 vsca->sink_fmt;
>  
>  	/* Scale the frame size for the source pad */
> -	if (IS_SRC(format->pad)) {
> +	if (VIMC_IS_SRC(format->pad)) {
>  		format->format.width = vsca->sink_fmt.width * sca_mult;
>  		format->format.height = vsca->sink_fmt.height * sca_mult;
>  	}
> @@ -165,7 +163,7 @@ static int vimc_sca_set_fmt(struct v4l2_subdev *sd,
>  	 * Do not change the format of the source pad,
>  	 * it is propagated from the sink
>  	 */
> -	if (IS_SRC(fmt->pad)) {
> +	if (VIMC_IS_SRC(fmt->pad)) {
>  		fmt->format = *sink_fmt;
>  		fmt->format.width = sink_fmt->width * sca_mult;
>  		fmt->format.height = sink_fmt->height * sca_mult;
>