linux-kernel - Re: [PATCH 2/3] i2c: mux: pca9541: namespace cleanup

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:   Wed, 21 Mar 2018 06:53:12 +0100
From:   Peter Rosin <peda@...ntia.se>
To:     Vladimir Zapolskiy <vz@...ia.com>, linux-kernel@...r.kernel.org
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Wolfram Sang <wsa@...-dreams.de>,
        Ken Chen <chen.kenyy@...entec.com>, joel@....id.au,
        linux-i2c@...r.kernel.org
Subject: Re: [PATCH 2/3] i2c: mux: pca9541: namespace cleanup

On 2018-03-21 00:24, Vladimir Zapolskiy wrote:
> Hi Peter,
> 
> On 03/20/2018 11:31 AM, Peter Rosin wrote:
>> In preparation for PCA9641 support, convert the mybus and busoff macros
>> to functions, and in the process prefix them with pca9541_. Also prefix
>> remaining chip specific macros with PCA9541_.
>>
>> Signed-off-by: Peter Rosin <peda@...ntia.se>
>> ---
>>  drivers/i2c/muxes/i2c-mux-pca9541.c | 26 +++++++++++++++++++-------
>>  1 file changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
>> index ad168125d23d..47685eb4e0e9 100644
>> --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
>> +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
>> @@ -59,10 +59,8 @@
>>  #define PCA9541_ISTAT_MYTEST	BIT(6)
>>  #define PCA9541_ISTAT_NMYTEST	BIT(7)
>>  
>> -#define BUSON		(PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
>> -#define MYBUS		(PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
>> -#define mybus(x)	(!((x) & MYBUS) || ((x) & MYBUS) == MYBUS)
>> -#define busoff(x)	(!((x) & BUSON) || ((x) & BUSON) == BUSON)
>> +#define PCA9541_BUSON	(PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
>> +#define PCA9541_MYBUS	(PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
>>  
>>  /* arbitration timeouts, in jiffies */
>>  #define ARB_TIMEOUT	(HZ / 8)	/* 125 ms until forcing bus ownership */
>> @@ -93,6 +91,20 @@ static const struct of_device_id pca9541_of_match[] = {
>>  MODULE_DEVICE_TABLE(of, pca9541_of_match);
>>  #endif
>>  
>> +static int pca9541_mybus(int ctl)
> 
> static inline?

No, "inline" is only used in header files in the kernel. The compiler
is free to inline whatever function it likes anyway, and in this case
we do not know better than the compiler. We don't care either. At least,
that is my understanding of the situation regarding the "inline"
keyword.

> 
>> +{
>> +	if (!(ctl & PCA9541_MYBUS))
>> +		return 1;
>> +	return (ctl & PCA9541_MYBUS) == PCA9541_MYBUS;
>> +}
>> +
>> +static int pca9541_busoff(int ctl)
> 
> static inline?
> 
>> +{
>> +	if (!(ctl & PCA9541_BUSON))
>> +		return 1;
>> +	return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
>> +}
> 
> Reviewed-by: Vladimir Zapolskiy <vz@...ia.com>

Thanks!

Cheers,
Peter

> 
> --
> With best wishes,
> Vladimir
>