LKML Archive on
help / color / mirror / Atom feed
From: Jean Delvare <>
To: Jochen Friedrich <>
Cc: Scott Wood <>,
	Kumar Gala <>,
	Vitaly Bordug <>,,
	linuxppc-dev list <>,
Subject: Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2  controllers
Date: Sat, 23 Feb 2008 13:43:35 +0100	[thread overview]
Message-ID: <20080223134335.0f5c7d79@hyperion.delvare> (raw)
In-Reply-To: <>

Hi Jochen,

On Fri, 22 Feb 2008 12:16:16 +0100, Jochen Friedrich wrote:
> Hi Jean,
> >> +/*
> >> + * Wait for patch from Jon Smirl
> >> + * #include "powerpc-common.h"
> >> + */
> > 
> > It doesn't make sense to merge this comment upstream.
> I know you don't like the patch from Jon Smirl and you also explained your reasons.
> Fortunately, I2c no longer uses numeric device IDs but names. So what are the alternatives?
> 1. modify the I2c subsystem to accept OF names additionally to I2c names (proposed by Jon smirl).

The problem I have with this is that it breaks compatibility. The chip
name is not only used for device/driver matching, it is also exported
to userspace as a sysfs attribute ("name"). Applications might rely on
it. At least libsensors does.

One way to work around the problem would be to use separate fields for
device/driver matching and the "name" attribute. However, this will add
some complexity to the i2c-core code and cost some memory as well, so
it is far from perfect.

> 2. record the I2c name in the dts tree, either as seperate tag (like linux,i2c-name="<i2c-name>")
>    or as additional compatible entry (like compatible="...", "linux,<i2c-name>").

This would work, and it would require almost no change to the current
i2c-core code, but I thought that these dts files were not under our

The problem I see with this approach is that the name translation would
have to be done for each dts file, rather than just once for each device
type. This means more work, but maybe this can be done if at least part
of it is automated. I admit that I never considered this option because
I considered the dts files as read-only. If this assumption was
incorrect, then maybe this is the best solution. Can you please
elaborate on the details of this proposal?

> 3. use a glue layer with a translation map.

This is what we do at the moment (see i2c_devices in
arch/powerpc/sysdev/fsl_soc.c). Jon Smirl is worried that it won't
scale well though, and I can't disagree.

> What is the preferred way to do this?

I still have to think about it. I didn't have much time to work on this
during the last 2 weeks, hopefully I will fine some time to experiment
again soon. As I underlined before, my patch set affects no less than 5
subsystems with different needs and expectations, it's no trivial

Jean Delvare

  reply	other threads:[~2008-02-23 12:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-31 12:54 Jochen Friedrich
2008-02-21 12:05 ` Jean Delvare
2008-02-22 11:16   ` Jochen Friedrich
2008-02-23 12:43     ` Jean Delvare [this message]
2008-03-25 18:46       ` Jochen Friedrich
2008-03-26  0:41         ` Stephen Rothwell
2008-02-23 21:28     ` Olof Johansson
2008-02-24 15:16       ` Jochen Friedrich
2008-02-24 18:15         ` Olof Johansson
2008-02-25 16:48           ` Jochen Friedrich
2008-02-24 16:19     ` Jon Smirl

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080223134335.0f5c7d79@hyperion.delvare \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2  controllers' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).