linux-kernel - Re: [v2,1/1] i2c: designware: Add SMBus Quick Command support

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: <Z_yxVJVc69ljaDZe@smile.fi.intel.com>
Date: Mon, 14 Apr 2025 09:55:16 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: ende.tan@...rfivetech.com
Cc: linux-i2c@...r.kernel.org, jarkko.nikula@...ux.intel.com,
	mika.westerberg@...ux.intel.com, jsd@...ihalf.com,
	andi.shyti@...nel.org, linux-kernel@...r.kernel.org,
	leyfoon.tan@...rfivetech.com, endeneer@...il.com
Subject: Re: [v2,1/1] i2c: designware: Add SMBus Quick Command support

On Sat, Apr 12, 2025 at 05:34:14PM +0800, ende.tan@...rfivetech.com wrote:
> From: Tan En De <ende.tan@...rfivetech.com>
> 
> Add support for SMBus Quick Read and Quick Write commands.

it's interesting how you made a versioning. Just run
`git format-patch -v<X>...` where <X> is the number of version you want,
it will make it properly in the Subject.

...

> +	/* i2c-core-smbus.c: Only I2C_SMBUS_QUICK has msg[0].len = 0 */

Please, remove filenames from the code, better to refer to the actual functions
as func(). This helps also to grep for all usages in case of renaming.

...

> +		/* i2c-core-smbus.c: Only I2C_SMBUS_QUICK has msg[0].len = 0 */

Ditto.

...

> +			regmap_write(
> +				dev->map, DW_IC_DATA_CMD,

Something wrong with the indentation. And you have plenty of room on the
previous line.

> +				*buf | DW_IC_DATA_CMD_STOP |
> +				((msgs[dev->msg_write_idx].flags & I2C_M_RD) ?
> +				DW_IC_DATA_CMD_CMD : 0)
> +			);

...

> -	dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY;
> +	dev->functionality = I2C_FUNC_10BIT_ADDR |
> +			     I2C_FUNC_SMBUS_QUICK |
> +			     DW_IC_DEFAULT_FUNCTIONALITY;

Ditto.

-- 
With Best Regards,
Andy Shevchenko