linux-kernel - Re: [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments

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: <20180306222530.GA2604@himanshu-Vostro-3559>
Date:   Wed, 7 Mar 2018 03:55:30 +0530
From:   Himanshu Jha <himanshujha199640@...il.com>
To:     Shreeya Patel <shreeya.patel23498@...il.com>
Cc:     jic23@...nel.org, lars@...afoo.de, Michael.Hennerich@...log.com,
        knaack.h@....de, pmeerw@...erw.net, gregkh@...uxfoundation.org,
        linux-iio@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org, daniel.baluta@...il.com
Subject: Re: [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments

Hi Shreeya,

> I was just going through your patch for giving myself 
> a habit of reading patches.

Great!

> I see here that there are many comments which are necessary
> have also been removed.
> Jonathan told that some of the names do not explain
> much about the how registers are related to the orientation.
> So it is necessary for some comments to be there here.
> 
> I saw your next patch too, in which you are changing some of the
> names for betterment, but again, that doesn't cover everything which
> was told by Jonathan.
> 
> Here is the link to the patch where Jonathan had given detailed 
> description
> 
> Just sharing this information so in case if Jonathan agrees with
> this then he will not have to explain it all again :)
> 
> https://lkml.org/lkml/2018/3/3/104

Well, the naming of macros is debatable as Jonathan pointed out[1] and
the unusual/odd naming pointed to you was for rotation registers

#define ADIS16209_ROT_OUT_REG		0x10

I renamed the unusual/odd naming macros for eg
#define ADIS16209_DIAG_STAT_FLASH_UPT_BIT

to

#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT

as it didn't look like a failure bit address for which I consulted Jonathan to which
he agreed. Also, *DIAG_STAT* seems like a good name since these status registers
are serving the purpose of diagnosing the device behavior. Again, GLOB_CMD stands
for Global Command register for controlling the deivce operation such as
Fatory Reset, Software Reset, etc.

See, it is difficult to point the perfect names than the suitable ones!
And let's just leave these *bikeshedding* ;-)

-- 
Thanks
Himanshu Jha