On Mon, 19 Dec 2022 19:17:03 +0100, Heiner Kallweit wrote:
> This patch factors out non-block pre/post processing to a new function
> i801_simple_transaction(), complementing existing function
> i801_block_transaction(). This makes i801_access() better readable.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
> v2:
> - rename new function to i801_simple_transaction
> - code style fixes
> ---
> drivers/i2c/busses/i2c-i801.c | 92 +++++++++++++++++++++--------------
> 1 file changed, 55 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 882cf5135..d934d410b 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -732,6 +732,59 @@ static void i801_set_hstadd(struct i801_priv *priv, u8 addr, char read_write)
> outb_p((addr << 1) | (read_write & 0x01), SMBHSTADD(priv));
> }
>
> +/* Single value transaction function */
> +static int i801_simple_transaction(struct i801_priv *priv, union i2c_smbus_data *data,
> + char read_write, int command)
> +{
> + int xact, ret;
> +
> + switch (command) {
I did not notice during my initial review, but this parameter name is
confusing, and this becomes even more obvious with patch 7 later. What
you name "command" here is named "size" in function i801_access(). And
what is named "command" there, you will name "hstcmd" in this function
in patch 7. I would prefer to keep the same names (and parameter order)
throughout the function stack for consistency.
Rest is OK.
--
Jean Delvare
SUSE L3 Support
