[i2c] Question about vt82c596 SMBus driver (Via I2c Bus driver)

Mortha, Prakash pmortha at escient.com
Tue Oct 7 23:02:36 CEST 2008

Hi Jean,

Here is my Signed-off-by line:
Signed-off-by: Prakash Mortha <pmortha at escient.com>

I tried to include this in the patch files too, but please modify it as

(This time also I am sending the patches as attachment as quilt send is
not working for me here - needs to configure mail transfer agent and
still trying to set it right).

Thanks a lot,

-----Original Message-----
From: Jean Delvare [mailto:khali at linux-fr.org] 
Sent: Tuesday, October 07, 2008 4:00 PM
To: Mortha, Prakash
Cc: Linux I2C
Subject: Re: Question about vt82c596 SMBus driver (Via I2c Bus driver)

Hi Prakash,

On Tue, 7 Oct 2008 11:19:46 -0400, Mortha, Prakash wrote:
> Please find attached the patches I have generated using quilt.
> this should meet the requirements.  (Please bear with me if this
> meet the requirements, as this is my first time to generate a official
> patch).

The patches meet the technical requirements, in that I was able to
apply them. However they are missing a proper subject and description,
as well as your Signed-off-by line. Again, please see section 12 (Sign
your work) of Documentation/SubmittingPatches for what it means.

For example, the header of your first patch could be something like:

* * * * *
From: Prakash Mortha <pmortha at escient.com>
Subject: i2c: Restore i2c_smbus_process_call function

Restore the i2c_smbus_process_call() as one driver (for the
Micronas MAP5401) will need it soon.
* * * * *

And then would come your Signed-off-by line. For your first
contribution, I want to make it easy for you and I'll be adding the
subject and description myself, but I _need_ your Signed-off-by line
before I can push both patches upstream.

> Thank you once again for your valuable inputs/comments. I tried
> implementing all your suggestions, except for the following:
> 1. I didn't modify the Documentation/i2c/smbus-protocol, as this
> document is already talking about the new function that I have worked
> on.

But it doesn't mention the name of the function implementing the
transaction. That's what I wanted you to add. But that's OK, I added it
myself. I also updated Documentation/i2c/writing-clients, which was
claiming that function i2c_smbus_process_call() had been removed from
the kernel.

> 2. I had to implicitly have the following lines of code in
> because for process call functionality the bus driver has to be in
> mode initially and then has to switch to Read mode. (With out these
> implicit lines of code i2c-viapro bus driver is returning immediately
> after write call as per the existing control flow.)
> 	if ( size == VT596_PROC_CALL)
> 	              read_write = I2C_SMBUS_READ;
> Please provide your suggestion if I could do it in a better way.

I was wondering about this. The "process call" is special in that it
doesn't have a read variant and a write variant. Instead, it is a
transaction combining write and read. So I was wondering if the
direction bit had any effect on the VIA chip. According to your tests,
it must be handled as a write operation at the device level. So, your
implementation is correct.

I had to fix a number of whitespace issues in the second patch. Next
time, I suggest that you run scripts/checkpatch.pl on your patch, it
will tell you about that kind of minor problems so that you can fix
them before sending the patch.

Jean Delvare
-------------- next part --------------
A non-text attachment was scrubbed...
Name: series
Type: application/octet-stream
Size: 14 bytes
Desc: series
Url : http://lists.lm-sensors.org/pipermail/i2c/attachments/20081007/52f4373d/attachment-0003.obj 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch1
Type: application/octet-stream
Size: 1445 bytes
Desc: patch1
Url : http://lists.lm-sensors.org/pipermail/i2c/attachments/20081007/52f4373d/attachment-0004.obj 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch2
Type: application/octet-stream
Size: 1850 bytes
Desc: patch2
Url : http://lists.lm-sensors.org/pipermail/i2c/attachments/20081007/52f4373d/attachment-0005.obj 

More information about the i2c mailing list