linux-kernel - Re: [PATCH v2 07/14] staging: iio: ad7746: Remove unused defines

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:   Mon, 16 Apr 2018 11:50:20 -0300
From:   Hernán Gonzalez <hernan@...guardiasur.com.ar>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     knaack.h@....de, lars@...afoo.de,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        gregkh@...uxfoundation.org, Michael.Hennerich@...log.com,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 07/14] staging: iio: ad7746: Remove unused defines

You're right, got confused from the macro defined in the .c file. I'll
leave this alone on the next series
Thanks!

On Sun, Apr 15, 2018 at 12:12 PM, Jonathan Cameron <jic23@...nel.org> wrote:
> On Fri, 13 Apr 2018 13:36:44 -0300
> Hernán Gonzalez <hernan@...guardiasur.com.ar> wrote:
>
>> Signed-off-by: Hernán Gonzalez <hernan@...guardiasur.com.ar>
>> ---
>>  drivers/staging/iio/cdc/ad7746.c | 7 -------
>>  drivers/staging/iio/cdc/ad7746.h | 5 -----
>>  2 files changed, 12 deletions(-)
>>
>> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
>> index f53612a..d39ab34 100644
>> --- a/drivers/staging/iio/cdc/ad7746.c
>> +++ b/drivers/staging/iio/cdc/ad7746.c
>> @@ -25,7 +25,6 @@
>>   * AD7746 Register Definition
>>   */
>>
>> -#define AD7746_REG_STATUS            0
>>  #define AD7746_REG_CAP_DATA_HIGH     1
>>  #define AD7746_REG_VT_DATA_HIGH              4
>>  #define AD7746_REG_CAP_SETUP         7
>> @@ -38,12 +37,6 @@
>>  #define AD7746_REG_CAP_GAINH         15
>>  #define AD7746_REG_VOLT_GAINH                17
>>
>> -/* Status Register Bit Designations (AD7746_REG_STATUS) */
>> -#define AD7746_STATUS_EXCERR         BIT(3)
>> -#define AD7746_STATUS_RDY            BIT(2)
>> -#define AD7746_STATUS_RDYVT          BIT(1)
>> -#define AD7746_STATUS_RDYCAP         BIT(0)
>> -
> There is a pretty strong argument that the driver 'should' be
> checking the status register...
>
> I would leave it the defines here.  When they are acting
> as 'documentation' of the register layout of a device, they
> do little harm and can be very useful.
>
>>  /* Capacitive Channel Setup Register Bit Designations (AD7746_REG_CAP_SETUP) */
>>  #define AD7746_CAPSETUP_CAPEN                BIT(7)
>>  #define AD7746_CAPSETUP_CIN2         BIT(6) /* AD7746 only */
>> diff --git a/drivers/staging/iio/cdc/ad7746.h b/drivers/staging/iio/cdc/ad7746.h
>> index ea8572d..2fbcee8 100644
>> --- a/drivers/staging/iio/cdc/ad7746.h
>> +++ b/drivers/staging/iio/cdc/ad7746.h
>> @@ -13,11 +13,6 @@
>>   * TODO: struct ad7746_platform_data needs to go into include/linux/iio
>>   */
>>
>> -#define AD7466_EXCLVL_0              0 /* +-VDD/8 */
>> -#define AD7466_EXCLVL_1              1 /* +-VDD/4 */
>> -#define AD7466_EXCLVL_2              2 /* +-VDD * 3/8 */
>> -#define AD7466_EXCLVL_3              3 /* +-VDD/2 */
>> -
>
> Think about what these are for.... They aren't unused
> if you are actually using platform data on a given oard.
>
>>  struct ad7746_platform_data {
>>       unsigned char exclvl;   /*Excitation Voltage Level */
>>       bool exca_en;           /* enables EXCA pin as the excitation output */
>