2022-09-17 20:21:27

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v3 5/9] i2c-mlxbf.c: support lock mechanism


> + /* Try to acquire the smbus gw lock before any reads of the GW register since
> + * a read sets the lock.
> + */

Please use kernel multiline comment style.

> @@ -792,6 +824,8 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
> priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_FSM);
> }

Dunno if this is taste, but I think it is easier to follow the locking
when we have central acquire and release points for it. So, I'd suggest
to add something like

out_unlock:

here and then goto to this place from the other locations.

What do you think?

>
> + mlxbf_i2c_smbus_master_unlock(priv);
> +
> return ret;


Attachments:
(No filename) (659.00 B)
signature.asc (849.00 B)
Download all attachments

2022-09-19 14:08:06

by Asmaa Mnebhi

[permalink] [raw]
Subject: RE: [PATCH v3 5/9] i2c-mlxbf.c: support lock mechanism


> + /* Try to acquire the smbus gw lock before any reads of the GW register since
> + * a read sets the lock.
> + */

Please use kernel multiline comment style.

Done.
> @@ -792,6 +824,8 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
> priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_FSM);
> }

Dunno if this is taste, but I think it is easier to follow the locking when we have central acquire and release points for it. So, I'd suggest to add something like

out_unlock:

here and then goto to this place from the other locations.

What do you think?

Sure!
>
> + mlxbf_i2c_smbus_master_unlock(priv);
> +
> return ret;