Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking

From: Wolfram Sang <wsa@the-dreams.de>
To: Peter Rosin <peda@axentia.se>
Cc: linux-kernel@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>,
	Peter Korsgaard <peter.korsgaard@barco.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Jonathan Cameron <jic23@kernel.org>,
	Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald <pmeerw@pmeerw.net>,
	Antti Palosaari <crope@iki.fi>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Grant Likely <grant.likely@linaro.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kalle Valo <kvalo@codeaurora.org>, Jiri Slaby <jslaby@suse.com>,
	Daniel Baluta <daniel.baluta@intel.com>,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	Adriana Reus <adriana.reus@intel.com>,
	Matt Ranostay <matt.ranostay@intel.com>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Terry Heo <terryheo@google.com>, Arnd Bergmann <arnd@arndb.de>,
	Tommi Rantala <tt.rantala@gmail.com>,
	Crestez Dan Leonard <leonard.crestez@intel.com>,
	linux-i2c@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-iio@vger.kernel.org, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, Peter Rosin <peda@lysator.liu.se>
Subject: Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
Date: Wed, 4 May 2016 14:07:28 +0200	[thread overview]
Message-ID: <20160504120728.GA2120@tetsubishi> (raw)
In-Reply-To: <0b4136b2-e555-9bc0-9003-898d686de7a1@axentia.se>

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]

Hi Peter,

thanks for the detailed explanation!

> So maybe there should be only one flag, e.g. I2C_LOCK_ROOT_ADAPTER?
> I.e. perhaps leave the future for later?

I think this makes the current code easier understandable at this point,
but I'll leave the decision to you. I am fine with both. Maybe a few
words of explanation would be good if you want to keep both flags.

> Hmmm, I just now realized that you were not really suggesting any
> changes other than to the commit message. Oh well, I can perhaps
> rephrase some of the above in the commit message if you think that
> we should not unnecessarily touch the code at this point...

Yes, updated commit description is enough for me now. If you want to
change to one flag, we should do it incrementally. I think I can apply
this as a fixup until around rc3 I'd say.

> > I think this kerneldoc should be moved to i2c_lock_adapter and/or
> > i2c_lock_bus() which are now in i2c.h. This is what users will use, not
> > this static, adapter-specific implementation. I think it is enough to
> > have a comment here explaining what is special in handling adapters.
> 
> Yes, I was not really satisfied with having documentation on static
> functions. But if I move it, there is no natural home for the current
> i2c_trylock_adapter docs, and I'd hate killing documentation that
> still applies. Do you have a suggestion? Maybe keep that one doc at
> the static i2c_trylock_adapter for now and move it to ->trylock_bus
> when someone decides to write kerneldoc for struct i2c_adapter?

Well, because I think redundancy is acceptable when it comes to
documentation, how about keeping the chunks you already have and copy an
adapted one over to the functions in i2c.h?

Regards,

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]