Re: [PATCH v1] i2c: microchip-corei2c: add smbus support

[Conor Dooley <conor@kernel.org>]

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

On Mon, May 05, 2025 at 10:04:27PM +0200, Andi Shyti wrote:
> Hi,
> 
> On Wed, Apr 30, 2025 at 05:06:09PM +0530, Mukesh Kumar Savaliya wrote:
> > On 4/30/2025 4:53 PM, Conor Dooley wrote:
> > > From: prashanth kumar burujukindi <prashanthkumar.burujukindi@microchip.com>
> > > 
> > > In this driver the supported SMBUS commands are smbus_quick,
> > > smbus_byte, smbus_byte_data, smbus_word_data and smbus_block_data.
> > > 
> > Write completely in imperative mood. something like :
> > 
> > Add support for SMBUS commands in driver
> > 
> > Add support for SMBUS commands: smbus_quick, smbus_byte, smbus_byte_data,
> > smbus_word_data, and smbus_block_data.
> 
> yes, I agree that the original commit log is a bit lazy written :-)

I don't personally think the suggested wording makes any meaningful
difference, but I can rework it if required.

> > Also mention below limitations here .

I actually removed them from the commit message, since they're not
limitations just what was and was not tested. I can put them back too
if that's needed.

> > SMBUS block read is supported by the controller but has not been tested due
> > to lack of hardware. However, SMBUS I2C block read has been tested.
> 
> Smbus i2c block has not been tested? If so, can we leave it out?
> What is the interest to keep it in?

What's the interest in adding any feature? Someone might want to use it.
We did not have a piece of hardware that uses it, so didn't do testing
of that specific command, but a customer may well want to so we included
it. Again, if you think removing it is the play, I can do that.

Cheers,
Conor.


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