Re: [PATCH v10 1/8] i2c: add I2C Address Translator (ATR) support

[Andy Shevchenko]

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