Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <95b5a18c-8f31-81d6-c878-17d32ae8d7e6@xs4all.nl>
Date:   Thu, 1 Sep 2022 08:34:02 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.1.2
Subject: Re: [PATCH] media: v4l2-tpg: Use 'unsigned int' instead of just
 'unsigned'.
Content-Language: en-US
To:     Jilin Yuan <yuanjilin@cdjrlc.com>, mchehab@kernel.org,
        jacopo@jmondi.org, laurent.pinchart@ideasonboard.com
Cc:     linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220901011151.22735-1-yuanjilin@cdjrlc.com>
From:   Hans Verkuil <hverkuil-cisco@xs4all.nl>
In-Reply-To: <20220901011151.22735-1-yuanjilin@cdjrlc.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Jilin,

On 9/1/22 03:11, Jilin Yuan wrote:
> 'unsigned int' should be clearer than 'unsigned'.
> 
> Signed-off-by: Jilin Yuan <yuanjilin@cdjrlc.com>
> ---
>  drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

I don't get it, just 8 changes? If I do a 'git grep unsigned drivers/media/common/v4l2-tpg'
I get 128 hits, and include/media/tpg/v4l2-tpg.h has another 52.

What's the point of changing just a few of these instances instead of doing
them all?

In any case, patches like this just create a lot of noise and take away too
much code reviewer bandwidth for very little gain.

Speaking for the media subsystem I will reject patches like this, it's not
worth the effort.

Regards,

	Hans

> 
> diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> index 9b7bcdce6e44..10ed9e65eecc 100644
> --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> @@ -86,7 +86,7 @@ void tpg_set_font(const u8 *f)
>  }
>  EXPORT_SYMBOL_GPL(tpg_set_font);
>  
> -void tpg_init(struct tpg_data *tpg, unsigned w, unsigned h)
> +void tpg_init(struct tpg_data *tpg, unsigned int w, unsigned int h)
>  {
>  	memset(tpg, 0, sizeof(*tpg));
>  	tpg->scaled_width = tpg->src_width = w;
> @@ -109,15 +109,15 @@ void tpg_init(struct tpg_data *tpg, unsigned w, unsigned h)
>  }
>  EXPORT_SYMBOL_GPL(tpg_init);
>  
> -int tpg_alloc(struct tpg_data *tpg, unsigned max_w)
> +int tpg_alloc(struct tpg_data *tpg, unsigned int max_w)
>  {
> -	unsigned pat;
> -	unsigned plane;
> +	unsigned int pat;
> +	unsigned int plane;
>  
>  	tpg->max_line_width = max_w;
>  	for (pat = 0; pat < TPG_MAX_PAT_LINES; pat++) {
>  		for (plane = 0; plane < TPG_MAX_PLANES; plane++) {
> -			unsigned pixelsz = plane ? 2 : 4;
> +			unsigned int pixelsz = plane ? 2 : 4;
>  
>  			tpg->lines[pat][plane] =
>  				vzalloc(array3_size(max_w, 2, pixelsz));
> @@ -132,7 +132,7 @@ int tpg_alloc(struct tpg_data *tpg, unsigned max_w)
>  		}
>  	}
>  	for (plane = 0; plane < TPG_MAX_PLANES; plane++) {
> -		unsigned pixelsz = plane ? 2 : 4;
> +		unsigned int pixelsz = plane ? 2 : 4;
>  
>  		tpg->contrast_line[plane] =
>  			vzalloc(array_size(pixelsz, max_w));
> @@ -153,8 +153,8 @@ EXPORT_SYMBOL_GPL(tpg_alloc);
>  
>  void tpg_free(struct tpg_data *tpg)
>  {
> -	unsigned pat;
> -	unsigned plane;
> +	unsigned int pat;
> +	unsigned int plane;
>  
>  	for (pat = 0; pat < TPG_MAX_PAT_LINES; pat++)
>  		for (plane = 0; plane < TPG_MAX_PLANES; plane++) {