linux-kernel - Re: [PATCH 1/2] media: zoran: add spaces around '<<'

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]

Message-ID: <5b187aca-8793-bfcb-e52a-c1e03c94b5c9@xs4all.nl>
Date:   Fri, 9 Apr 2021 17:48:15 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Mitali Borkar <mitaliborkar810@...il.com>
Cc:     clabbe@...libre.com, mchehab@...nel.org,
        gregkh@...uxfoundation.org, linux-media@...r.kernel.org,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        outreachy-kernel@...glegroups.com, mitali_s@...iitr.ac.in
Subject: Re: [PATCH 1/2] media: zoran: add spaces around '<<'

On 09/04/2021 17:11, Mitali Borkar wrote:
> On Fri, Apr 09, 2021 at 09:23:22AM +0200, Hans Verkuil wrote:
>> Hi Mitali,
>>
>> On 08/04/2021 22:38, Mitali Borkar wrote:
>>> Added spaces around '<<' operator to improve readability and meet linux
>>> kernel coding style.
>>> Reported by checkpatch
>>>
>>> Signed-off-by: Mitali Borkar <mitaliborkar810@...il.com>
>>> ---
>>>  drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
>>> index 71b651add35a..a2a75fd9f535 100644
>>> --- a/drivers/staging/media/zoran/zr36057.h
>>> +++ b/drivers/staging/media/zoran/zr36057.h
>>> @@ -30,13 +30,13 @@
>>>  #define ZR36057_VFESPFR_HOR_DCM          14
>>>  #define ZR36057_VFESPFR_VER_DCM          8
>>>  #define ZR36057_VFESPFR_DISP_MODE        6
>>> -#define ZR36057_VFESPFR_YUV422          (0<<3)
>>> -#define ZR36057_VFESPFR_RGB888          (1<<3)
>>> -#define ZR36057_VFESPFR_RGB565          (2<<3)
>>> -#define ZR36057_VFESPFR_RGB555          (3<<3)
>>> -#define ZR36057_VFESPFR_ERR_DIF          (1<<2)
>>> -#define ZR36057_VFESPFR_PACK24          (1<<1)
>>> -#define ZR36057_VFESPFR_LITTLE_ENDIAN    (1<<0)
>>> +#define ZR36057_VFESPFR_YUV422          (0 << 3)
>>> +#define ZR36057_VFESPFR_RGB888          (1 << 3)
>>> +#define ZR36057_VFESPFR_RGB565          (2 << 3)
>>> +#define ZR36057_VFESPFR_RGB555          (3 << 3)
>>> +#define ZR36057_VFESPFR_ERR_DIF          (1 << 2)
>>> +#define ZR36057_VFESPFR_PACK24          (1 << 1)
>>> +#define ZR36057_VFESPFR_LITTLE_ENDIAN    (1 << 0)
>>>  
>>>  #define ZR36057_VDTR            0x00c	/* Video Display "Top" Register */
>>>  
>>>
>>
>> I looked at that header and it is very messy.
>>
>> Can you make two new patches? The first aligns every nicely, e.g. this:
>>
>> #define ZR36057_VFEHCR          0x000   /* Video Front End, Horizontal Configuration Register */
>> #define ZR36057_VFEHCR_HS_POL             BIT(30)
>> #define ZR36057_VFEHCR_H_START           10
>> #define ZR36057_VFEHCR_H_END            0
>> #define ZR36057_VFEHCR_HMASK            0x3ff
>>
>> should become:
>>
>> /* Video Front End, Horizontal Configuration Register */
>> #define ZR36057_VFEHCR			0x000
>> #define ZR36057_VFEHCR_HS_POL		BIT(30)
>> #define ZR36057_VFEHCR_H_START		10
>> #define ZR36057_VFEHCR_H_END		0
>> #define ZR36057_VFEHCR_HMASK		0x3ff
>>
>> Same for all the other register blocks. Use tabs to do the alignment
>> instead of spaces, as is currently the case.
>>
> Okay Sir, will do this.
> 
>> The second patch can replace the (0<<3) etc. to BIT(0).
>>
> Sir may I know how to write (2<<3) in BIT() form? Like I am still
> learning and not sure how to convert this. Should it be BIT(2)? But this
> is only possible for (1<<nr), PLease help me out. 

The << notation is kept for that. So this would be kept as-is:

#define ZR36057_VFESPFR_YUV422          (0 << 3)
#define ZR36057_VFESPFR_RGB888          (1 << 3)
#define ZR36057_VFESPFR_RGB565          (2 << 3)
#define ZR36057_VFESPFR_RGB555          (3 << 3)

BIT() is only used for single bits.

Regards,

	Hans

> 
> Thanks.
> 
>> That would be a nice cleanup of this rather messy header.
>>
>> Thanks!
>>
>> 	Hans