[i2c] i2c-remove-redundant-i2c_client-list.patch
Jean Delvare
khali at linux-fr.org
Tue Jan 8 20:57:42 CET 2008
On Tue, 8 Jan 2008 10:52:31 -0800, David Brownell wrote:
> On Tuesday 08 January 2008, Jean Delvare wrote:
> > > Good point. I didn't like this part much, and that function can
> > > have other uses. I updated kobj_to_i2c_client() to use it too.
> >
> > I wouldn't change kobj_to_i2c_client(). Drivers using it already know
> > that their kobj is an i2c_client, so there's no need to check for that,
> > and the use cases I've seen are in runtime paths that should be fast, I
> > don't want to slow them down without a good reason.
>
> Hmm, Mr. Grep says all the users of that are in drivers/i2c/chips ...
> basically mapping an eeprom binary attribute's kobj to the i2c_client
> of the base device. I have to say that not only would I not call
> those "should be fast" paths (how often does anyone read eeproms??),
Sorry, I expressed myself incorrectly, I didn't mean to intend that
these were hot paths, just that this was run-time code and not
initialization code, which means that the speed matters a little more.
But the key argument here is that the current code is not broken so I
couldn't see the rationale for touching it.
> but I also have no problem using
>
> client = to_i2c_client(container_of(kobj, struct device, kobj));
>
> instead of a fancy kobj_to_i2c_client() ... in fact, I would never
> have thought to look for any kobj_to_*() utilities. Although Mr. Grep
> tellse me there are a few others floating around; back it out if you
> feel strongly about it.
I seem to remember that kobj_to_i2c_client() was originally declared
inside one of these drivers by the driver author and I suggested to
move it to i2c.h.
> > - return device_for_each_child(&adapter->dev, &addr, i2cdev_check);
> > + return i2c_for_each_client(adapter, &addr, i2cdev_check);
> >
> > ...
> >
> > Admittedly it will slow down things a bit as each iteration of the loop
> > will have one additional level of indirection. This makes the calling
> > code somewhat simpler though. Whether it is worth the additional
> > runtime cost, I just don't know. What do you think?
>
> My bias in such cases is towards interfaces that force less policy.
> So in this case that would be to expose i2_verify_client() instead
> of an i2c-specific iterator. The fact that it's got lower callstack
> overhead is a bonus. :)
OK, fine with me, I'll forget about it then.
--
Jean Delvare
More information about the i2c
mailing list