Re: [PATCH 1/2] i2c-i801: Handle multiple instances instead of keeping global state

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

On Sat, 30 Oct 2010 19:47:58 -0400, David Woodhouse wrote:
> On Sun, 2010-10-31 at 00:39 +0100, Ben Dooks wrote:
> > On Sat, Oct 30, 2010 at 07:34:23PM -0400, David Woodhouse wrote:
> > > On Sat, 2010-10-30 at 18:24 +0200, Jean Delvare wrote:
> > > > An explanation why this change is needed would be nice.
> > > 
> > > Um, does it really need explaining? It's really poor form to keep driver
> > > state in global variables rather than per-instance, even if you *don't*
> > > actually have more than one device.

This is actually a good second half for an explanation paragraph.

> > I always like to fill it in, it makes it easier for lazy folks who can't
> > be bothered to read the patch itself.
> 
> The explanation fits in one line:
>   "Handle multiple instances instead of keeping global state"

No, this isn't en explanation. This is a statement of _what_ your patch
does. This is needed but not sufficient. What is additionally requested
from you is an explanation _why_ it does it.

> I could try adding a paragraph explaining further that this patch will
> make the driver handle multiple instances of the device, by keeping its
> state per-device instead of in global variables. But that seems
> somewhat... redundant.

You want to explain that the assumption of the original driver author
that there can be only one supported device with one SMBus interface on
a given system no longer holds true for new devices to be released soon.

Thanks,
-- 
Jean Delvare