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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
