linux-kernel - Re: [PATCH v3 02/14] i2c-octeon: Cleanup i2c-octeon driver

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:	Sat, 12 Mar 2016 16:35:00 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Jan Glauber <jglauber@...ium.com>
Cc:	linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
	David Daney <ddaney@...iumnetworks.com>
Subject: Re: [PATCH v3 02/14] i2c-octeon: Cleanup i2c-octeon driver

On Mon, Mar 07, 2016 at 04:10:45PM +0100, Jan Glauber wrote:
> Cleanup only without functional change.

I like most of the changes, but there are still some functional changes
left.

> -static int octeon_i2c_stop(struct octeon_i2c *i2c)
> +/* send STOP to the bus */
> +static void octeon_i2c_stop(struct octeon_i2c *i2c)
>  {
>  	u8 data;
>  
> @@ -266,11 +259,8 @@ static int octeon_i2c_stop(struct octeon_i2c *i2c)
>  
>  	data = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
>  
> -	if (data != STAT_IDLE) {
> +	if (data != STAT_IDLE)
>  		dev_err(i2c->dev, "%s: bad status(0x%x)\n", __func__, data);
> -		return -EIO;
> -	}
> -	return 0;

Why this change? I don't know what SW_TWSI_EOP_TWSI_STAT tells, but this
is surely not a cleanup.

>  	octeon_i2c_stop(i2c);
>  
> -	return (ret != 0) ? ret : num;
> +	return ret ? -EAGAIN : num;

This is also not a cleanup and looks wrong. -EAGAIN is for lost
arbitration only.

>  
> -static struct of_device_id octeon_i2c_match[] = {
> -	{
> -		.compatible = "cavium,octeon-3860-twsi",
> -	},
> +static const struct of_device_id octeon_i2c_match[] = {
> +	{	.compatible = "cavium,octeon-3860-twsi",	},

Nit: I'd prefer no tabs within the curly braces.

Thanks,

   Wolfram


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)