From: ppokorny@penguincomputing.com (Philip Pokorny)
To: LM Sensors <sensors@Stimpy.netroedge.com>,
LKML <linux-kernel@vger.kernel.org>
Cc: Greg KH <greg@kroah.com>
Subject: [RFC] I2C: Remove the i2c_client id field
Date: Thu, 19 May 2005 06:25:27 +0000 [thread overview]
Message-ID: <41D0942B.8020109@penguincomputing.com> (raw)
In-Reply-To: <20041227230402.272fafd0.khali@linux-fr.org>
So only the drives I wrote use the ID in a meaningful way?
What do you propose to replace the ID value in the debug messages with?
Ideally it would be the things you mention that uniquely identify the
chip in question (bus number and address)
How hard are those values to get at? Do we have to chase possibly NULL
pointers?
:v)
Jean Delvare wrote:
>Hi Greg, hi all,
>
>While porting various hardware monitoring drivers to Linux 2.6 and
>otherwise working on i2c drivers in 2.6, I found that the i2c_client
>structure has an "id" field (of type int) which is mostly unused. I am
>not exactly sure why it was introduced in the first place, and since the
>i2c subsystem code was significantly reworked since, it might not
>actually matter.
>
>Most hardware monitoring drivers allocate a unique (per driver) id
>through an incremented static global variable, and never use it. Some
>(lm85 and most notably adm1026) use the value in debug messages. I saw
>various video drivers appending the id value to the client name between
>square brackets, while others would set the id field to -1 and then
>leave it alone. The i2c core itself doesn't use this field.
>
>Using this field to identify a client doesn't make much sense to me, for
>the following reasons:
>
>1* A client is already uniquely identified by the combination of the
>number of the bus it sits on and the address it is located at on this
>bus.
>
>2* With the implementation described above, the id will possibly change
>depending on which i2c bus drivers are loaded and the order they were
>loaded in. As a consequence, you can't rely on its value from
>user-space, and its usability in kernel-space isn't obvious either.
>
>3* As a matter of fact, no driver in the kernel tree uses this field
>except for debugging (and even then with no obvious benefit), with only
>a few exceptions where I could easily change the code so it wouldn't
>need this field anymore.
>
>Thus, I propose that we simply get rid of this field, so as to save some
>memory space and kill some useless code. If anyone really ever needs to
>carry some sort of id attached to an i2c_client structure, this is
>private data and can be added to whatever structure the data field is
>pointing to for this particular driver.
>
>Unless someone objects with valid reasons, I am going to send patches to
>kill the i2c_client id field. I have everything ready, but don't know
>exactly how I should send them. The difficulty comes from the relatively
>large number of affected drivers (50) and the fact that they spread over
>very different subsystems (the big ones are i2c and media/video, plus
>half a dozen drivers in acorn/char, macintoch and sound).
>
>Greg, can you tell me if you would take such a patch, and how I would
>have to split it for you to accept it?
>
>Thanks,
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Philip Pokorny <ppokorny@penguincomputing.com>
To: LM Sensors <sensors@Stimpy.netroedge.com>,
LKML <linux-kernel@vger.kernel.org>
Cc: Greg KH <greg@kroah.com>
Subject: Re: [RFC] I2C: Remove the i2c_client id field
Date: Mon, 27 Dec 2004 15:00:59 -0800 [thread overview]
Message-ID: <41D0942B.8020109@penguincomputing.com> (raw)
In-Reply-To: <20041227230402.272fafd0.khali@linux-fr.org>
So only the drives I wrote use the ID in a meaningful way?
What do you propose to replace the ID value in the debug messages with?
Ideally it would be the things you mention that uniquely identify the
chip in question (bus number and address)
How hard are those values to get at? Do we have to chase possibly NULL
pointers?
:v)
Jean Delvare wrote:
>Hi Greg, hi all,
>
>While porting various hardware monitoring drivers to Linux 2.6 and
>otherwise working on i2c drivers in 2.6, I found that the i2c_client
>structure has an "id" field (of type int) which is mostly unused. I am
>not exactly sure why it was introduced in the first place, and since the
>i2c subsystem code was significantly reworked since, it might not
>actually matter.
>
>Most hardware monitoring drivers allocate a unique (per driver) id
>through an incremented static global variable, and never use it. Some
>(lm85 and most notably adm1026) use the value in debug messages. I saw
>various video drivers appending the id value to the client name between
>square brackets, while others would set the id field to -1 and then
>leave it alone. The i2c core itself doesn't use this field.
>
>Using this field to identify a client doesn't make much sense to me, for
>the following reasons:
>
>1* A client is already uniquely identified by the combination of the
>number of the bus it sits on and the address it is located at on this
>bus.
>
>2* With the implementation described above, the id will possibly change
>depending on which i2c bus drivers are loaded and the order they were
>loaded in. As a consequence, you can't rely on its value from
>user-space, and its usability in kernel-space isn't obvious either.
>
>3* As a matter of fact, no driver in the kernel tree uses this field
>except for debugging (and even then with no obvious benefit), with only
>a few exceptions where I could easily change the code so it wouldn't
>need this field anymore.
>
>Thus, I propose that we simply get rid of this field, so as to save some
>memory space and kill some useless code. If anyone really ever needs to
>carry some sort of id attached to an i2c_client structure, this is
>private data and can be added to whatever structure the data field is
>pointing to for this particular driver.
>
>Unless someone objects with valid reasons, I am going to send patches to
>kill the i2c_client id field. I have everything ready, but don't know
>exactly how I should send them. The difficulty comes from the relatively
>large number of affected drivers (50) and the fact that they spread over
>very different subsystems (the big ones are i2c and media/video, plus
>half a dozen drivers in acorn/char, macintoch and sound).
>
>Greg, can you tell me if you would take such a patch, and how I would
>have to split it for you to accept it?
>
>Thanks,
>
>
next prev parent reply other threads:[~2005-05-19 6:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-12-27 22:04 [RFC] I2C: Remove the i2c_client id field Jean Delvare
2005-05-19 6:25 ` Jean Delvare
2004-12-27 23:00 ` Philip Pokorny [this message]
2005-05-19 6:25 ` Philip Pokorny
2004-12-28 10:42 ` Jean Delvare
2005-05-19 6:25 ` Jean Delvare
2004-12-28 16:36 ` Philip Pokorny
2005-05-19 6:25 ` Philip Pokorny
2004-12-28 17:22 ` Jean Delvare
2005-05-19 6:25 ` Jean Delvare
2005-01-06 23:12 ` Greg KH
2005-05-19 6:25 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=41D0942B.8020109@penguincomputing.com \
--to=ppokorny@penguincomputing.com \
--cc=greg@kroah.com \
--cc=linux-kernel@vger.kernel.org \
--cc=sensors@Stimpy.netroedge.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.