[i2c] [patch 2.6.20-rc1 3/6] other i2c code stops using i2c_adapter.dev

[Jean Delvare]

David,

On Mon, 18 Dec 2006 13:19:50 -0800, David Brownell wrote:
> On Monday 18 December 2006 11:02 am, Jean Delvare wrote:
> 
> > > @@ -284,7 +287,8 @@ int i2c_del_adapter(struct i2c_adapter *
> > >  	/* free dynamically allocated bus id */
> > >  	idr_remove(&i2c_adapter_idr, adap->nr);
> > >  
> > > -	dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name);
> > > +	dev_dbg(dev, "adapter %s [%s] unregistered\n",
> > > +			adap->class_dev.class_id, adap->name);
> > 
> > You modify the format slightly here, and I am fine with that change,
> > but you should do the same at registration time for consistency.
> 
> OK ... moving that change from a later patch to this one.
> 
> Note that this is preserving content, since previously "adap->dev" was
> a synthetic name mostly equivalent to the class name, but "dev" uses
> the real hardware name.  So this fixes a diagnostic bug too:   previous
> text made it hard to associate the hardware and its bus ID.

Agreed, see my other post where I propose to introduce class_dbg() to
address the problem mode widely.

> > This change doesn't belong here. I agree that the debugging messages in
> > i2c-core are far from optimal, but we'd rather improve them after
> > fixing the i2c core rather before.
> 
> I was going for "as part of"; at some level, fixing core messages
> is just fixing core messages.  I'll revert that, but that makes it
> unlikely the problems with these messages will get fixed any time
> soon.  (Notice how someone debugging a problem in this area doesn't
> get enough information to sort out what's gone wrong when faults
> show up later in the sequence... and how the original messages
> reported the core successfully "registered" before even making the
> call to do so!!)

Yes, as I said I agree it's currently suboptimal, but given that all
this code is eventually going away, I see little benefit in trying to
change it now.

> > > @@ -452,7 +459,7 @@ out_unregister:
> > >  	wait_for_completion(&client->released);
> > >  out_list:
> > >  	list_del(&client->list);
> > > -	dev_err(&adapter->dev, "Failed to attach i2c client %s at 0x%02x "
> > > +	dev_err(&client->dev, "Failed to attach i2c client %s at 0x%02x "
> > >  		"(%d)\n", client->name, client->addr, res);
> > >  out_unlock:
> > >  	mutex_unlock(&adapter->clist_lock);
> > 
> > What's the rationale for changing the adapter-based messages above to
> > client-based ones?
> 
> They have to change anyway, and these are fundamentally messages about the
> client being registered, not the adapter.  Plus as I implied above:  in all

In the current (broken) model, not really, as the client doesn't exist
before being attached. In the new model, that'll be different.

> code, diagnostics get bugfixed so infrequently that I've learned that second
> passes virtually never happen.

In the i2c subsystem, I do care so they happen. See:
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=114fd18397eb0eacf51ac784f7d5c929b8499715
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b6d7b3d1b5a388b7e9af2629a9ecccedee064078
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7bef559455fc71f66f8573cc1aafe1dd33966c1c
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=86749e8512d2c37618dc5814ef41abbf168f291b
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b32d20dc8b187e03605f091dbde9a78676a2a642
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e8aafcb2bba1fe122907161701a167e38174c7a5

Just like I accept whitespace cleanups when sent separately, I'll
accept message fixes if sent separately after the core rewrite.

-- 
Jean Delvare