Date:   Fri, 17 Mar 2023 14:11:52 +0200
From:   Andy Shevchenko <andriy.shevchenko@intel.com>
To:     Luca Ceresoli <luca.ceresoli@bootlin.com>
Cc:     Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
        Wolfram Sang <wsa@kernel.org>, linux-media@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-i2c@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Matti Vaittinen <Matti.Vaittinen@fi.rohmeurope.com>,
        Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Peter Rosin <peda@axentia.se>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Sakari Ailus <sakari.ailus@linux.intel.com>,
        Michael Tretter <m.tretter@pengutronix.de>,
        Hans Verkuil <hverkuil@xs4all.nl>,
        Mike Pagano <mpagano@gentoo.org>,
        Krzysztof =?utf-8?Q?Ha=C5=82asa?= <khalasa@piap.pl>,
        Marek Vasut <marex@denx.de>,
        Satish Nagireddy <satish.nagireddy@getcruise.com>,
        Luca Ceresoli <luca@lucaceresoli.net>
Subject: Re: [PATCH v10 1/8] i2c: add I2C Address Translator (ATR) support
Message-ID: <ZBRZCHM4rI9Csvib@smile.fi.intel.com>
References: <20230222132907.594690-1-tomi.valkeinen@ideasonboard.com>
 <20230222132907.594690-2-tomi.valkeinen@ideasonboard.com>
 <20230317101606.69602bba@booty>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230317101606.69602bba@booty>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

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@ideasonboard.com> wrote:
> > From: Luca Ceresoli <luca@lucaceresoli.net>

...

> > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 
> 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