On Fri, Mar 17, 2023 at 10:16:06AM +0100, Luca Ceresoli wrote:
> Hi Tomi, Wolfram,
> On Wed, 22 Feb 2023 15:29:00 +0200
> Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx> wrote:
> > From: Luca Ceresoli <luca@xxxxxxxxxxxxxxxx>
...
> > Signed-off-by: Luca Ceresoli <luca@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
>
> Wolfram, I think Tomi improved this work as much as currently possible
> and this patch now looks extremely good to me. I wish we had this in
> mainline soon. Does it make sense for me to send a Reviewed-by tag,
> given I already have a S-o-b one?
I believe documentation is in favour that standalone SoB suffice.
Especially when you are the author (From line) the Rb makes no
sense to me.
...
> > +/**
> > + * struct i2c_atr - The I2C ATR instance
> > + * @parent: The parent &struct i2c_adapter
> > + * @dev: The device that owns the I2C ATR instance
> > + * @ops: &struct i2c_atr_ops
> > + * @priv: Private driver data, set with i2c_atr_set_driver_data()
> > + * @algo: The &struct i2c_algorithm for adapters
> > + * @lock: Lock for the I2C bus segment (see &struct i2c_lock_operations)
> > + * @max_adapters: Maximum number of adapters this I2C ATR can have
> > + * @adapter: Array of adapters
> > + */
> > +struct i2c_atr {
> > + struct i2c_adapter *parent;
> > + struct device *dev;
> > + const struct i2c_atr_ops *ops;
> > +
> > + void *priv;
> > +
> > + struct i2c_algorithm algo;
> > + /* lock for the I2C bus segment (see struct i2c_lock_operations) */
>
> This comment is identical to the one in the kerneldoc comments just
> above, I'd just remove it.
>
> > + struct mutex lock;
> > + int max_adapters;
> > +
> > + struct notifier_block i2c_nb;
>
> Undocumented?
`kernel-doc -v` should actually catch this up.
--
With Best Regards,
Andy Shevchenko
