Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-13 Thread Jean Delvare
Hi Jon,

On Sat, 12 Jan 2008 11:00:31 -0500, Jon Smirl wrote:
 On 1/12/08, Jean Delvare wrote:
  What I meant is that the translation from Open Firmware device name to
  Linux device name could happen in different ways. Making module aliases
  out of the is one possibility but this is not the only one.
 
  I am curious why the translation could not happen offline. As I
  understand it, you're getting the device names from these .dts files.
  However you're not parsing them in the kernel directly, are you? I
  presume that you have some tool that converts these files into C code
  that the kernel can use? This conversion tool could translate the names.
 
 Those dts files are for embedded devices that were specifically
 developed for Linux. All of the PowerPC Macs in the world have a
 device tree in ROM that was developed by Apple following the Open
 Firmware standard. Same thing for Sun boxes, but I'm not working on
 those.

OK. So basically we have to handle two different cases here, trees that
come from the .dts files and trees that are read from ROMs, right?
Does this mean that .dts files are compiled to some binary format to
look like what is in the ROMs? Is there kernel code that parses this?
Please explain how both types are handled by the kernel. I need to
understand how this works before I can decide where the OF names -
Linux names translation can happen.

 The kernel has an existing mechanism for handling translations like
 these, the alias scheme.

That we agree on. My concern here is that you want to replace the Linux
names of i2c devices by OF names, without realizing that the Linux
names have a use outside of the kernel. We can't just replace them like
that, it would break some user-space applications. That's the reason
why I believe that it would make more sense to translate from OF names
to Linux names early in the process, so that the kernel, and thus
user-space applications, always handle and see the Linux names,
independently of the platform. I'm asking questions in order to figure
out whether and how this could be achieved.

 Currently developers add entries to the table in their private builds
 for the i2c devices they are using. Another way to avoid adding a
 table entry is to create a platform device in the platform code. But
 this support is being extended to audio codecs too. There are hundreds
 of audio codecs.
 
 The whole purpose of this code is to dynamically load the correct i2c
 and audio drivers by reading the device tree instead of having static
 i2s/codec devices for every possible platform combination.

I2C driver autoloading is already implemented, and works. Just not the
way you expected, but it works.

Replacing this mechanism with standard aliases is IMHO a good idea, it
makes the code cleaner and also more similar to what the rest of the
kernel does, which is always nice.

However, having a module aliasing mechanism for i2c drivers does NOT
require that OF names are used. We could implement aliasing using Linux
device names. Note that I have no problem with using OF names for
aliasing, however it should not break applications that currently know
the I2C devices by their Linux name.

-- 
Jean Delvare
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-12 Thread Jean Delvare
On Fri, 11 Jan 2008 15:16:57 -0500, Jon Smirl wrote:
 On 1/11/08, Jean Delvare [EMAIL PROTECTED] wrote:
  Secondly, it promotes OF device names as acceptable aliases. This I
  don't think I agree with. While I see some value in moving the OF name
  - Linux name translation to the drivers themselves (even though I
  don't see this as a mandatory move either), this doesn't imply that OF
  names should be used as aliases. I don't like the idea that different
  architectures will name the same device differently in a visible way.
  This could easily break user-space code that makes assumptions on the
  device names (libsensors comes to mind.) So, I think that this part
  will need some more discussion.
 
 They're aliases.  On the x86 my e1000 Ethernet driver loads using this
 alias name:
 pci:v8086d1010sv*sd*bc*sc*i*
 In fact, the e1000 driver has 63 alias names in addition to e1000
 
 But it's still the e1000 driver after it is loaded.
 [EMAIL PROTECTED]:/home/linux/drivers/net/e1000$ lsmod | grep e1000
 e1000 115968  0
 
 Loading a I2C driver with an OF alias name is not going to change the
 module name after it is loaded. In fact, once the module is in memory
 there's no way to tell what name was used to load it.

Of course. That's not what I was worried about... what I was worried
about is something your patch set doesn't do but I misread the code and
I thought it was doing. I'll read it again before I make more comments
on this.

 OF device names are set by the Open Firmware committee. It is not
 reasonable to force the Linux names back into Open Firmware since this
 would force the other operating systems using Open Firmware to adopt
 the Linux names.

I never meant to force the Linux names into Open Firmware. It wouldn't
make sense especially when the Linux names are invented by random
contributors with no specific rules, and can even change over time.

What I meant is that the translation from Open Firmware device name to
Linux device name could happen in different ways. Making module aliases
out of the is one possibility but this is not the only one.

I am curious why the translation could not happen offline. As I
understand it, you're getting the device names from these .dts files.
However you're not parsing them in the kernel directly, are you? I
presume that you have some tool that converts these files into C code
that the kernel can use? This conversion tool could translate the names.

 This issue hasn't been visible before since there was a global table
 in the PowerPC code mapping all known Open Firmware names into linux
 names. Keeping this as a global table doesn't scale. The mapping needs
 to be done by each device individually.

Looking at your patch set, I see only 11 entries in the table (in
arch/powerpc/sysdev/fsl_soc.c) that patch #2 deletes. Are there more in
other files? I'm asking because 11 entries hardly qualifies as doesn't
scale. I sure hope that you're not doing all this for the sole purpose
of getting rid of this 11-element table.

-- 
Jean Delvare
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-12 Thread Jon Smirl
On 1/12/08, Jean Delvare [EMAIL PROTECTED] wrote:
 On Fri, 11 Jan 2008 15:16:57 -0500, Jon Smirl wrote:
  On 1/11/08, Jean Delvare [EMAIL PROTECTED] wrote:
   Secondly, it promotes OF device names as acceptable aliases. This I
   don't think I agree with. While I see some value in moving the OF name
   - Linux name translation to the drivers themselves (even though I
   don't see this as a mandatory move either), this doesn't imply that OF
   names should be used as aliases. I don't like the idea that different
   architectures will name the same device differently in a visible way.
   This could easily break user-space code that makes assumptions on the
   device names (libsensors comes to mind.) So, I think that this part
   will need some more discussion.
 
  They're aliases.  On the x86 my e1000 Ethernet driver loads using this
  alias name:
  pci:v8086d1010sv*sd*bc*sc*i*
  In fact, the e1000 driver has 63 alias names in addition to e1000
 
  But it's still the e1000 driver after it is loaded.
  [EMAIL PROTECTED]:/home/linux/drivers/net/e1000$ lsmod | grep e1000
  e1000 115968  0
 
  Loading a I2C driver with an OF alias name is not going to change the
  module name after it is loaded. In fact, once the module is in memory
  there's no way to tell what name was used to load it.

 Of course. That's not what I was worried about... what I was worried
 about is something your patch set doesn't do but I misread the code and
 I thought it was doing. I'll read it again before I make more comments
 on this.

  OF device names are set by the Open Firmware committee. It is not
  reasonable to force the Linux names back into Open Firmware since this
  would force the other operating systems using Open Firmware to adopt
  the Linux names.

 I never meant to force the Linux names into Open Firmware. It wouldn't
 make sense especially when the Linux names are invented by random
 contributors with no specific rules, and can even change over time.

 What I meant is that the translation from Open Firmware device name to
 Linux device name could happen in different ways. Making module aliases
 out of the is one possibility but this is not the only one.

 I am curious why the translation could not happen offline. As I
 understand it, you're getting the device names from these .dts files.
 However you're not parsing them in the kernel directly, are you? I
 presume that you have some tool that converts these files into C code
 that the kernel can use? This conversion tool could translate the names.

Those dts files are for embedded devices that were specifically
developed for Linux. All of the PowerPC Macs in the world have a
device tree in ROM that was developed by Apple following the Open
Firmware standard. Same thing for Sun boxes, but I'm not working on
those.

The kernel has an existing mechanism for handling translations like
these, the alias scheme.


  This issue hasn't been visible before since there was a global table
  in the PowerPC code mapping all known Open Firmware names into linux
  names. Keeping this as a global table doesn't scale. The mapping needs
  to be done by each device individually.

 Looking at your patch set, I see only 11 entries in the table (in
 arch/powerpc/sysdev/fsl_soc.c) that patch #2 deletes. Are there more in
 other files? I'm asking because 11 entries hardly qualifies as doesn't
 scale. I sure hope that you're not doing all this for the sole purpose
 of getting rid of this 11-element table.

Currently developers add entries to the table in their private builds
for the i2c devices they are using. Another way to avoid adding a
table entry is to create a platform device in the platform code. But
this support is being extended to audio codecs too. There are hundreds
of audio codecs.

The whole purpose of this code is to dynamically load the correct i2c
and audio drivers by reading the device tree instead of having static
i2s/codec devices for every possible platform combination.


-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-11 Thread Jean Delvare
Hi Jon,

On Wed, 19 Dec 2007 23:41:36 -0500, Jon Smirl wrote:
 Since copying i2c-mpc.c to maintain support for the ppc architecture seems to 
 be an issue; instead rework i2c-mpc.c to use CONFIG_PPC_MERGE #ifdefs to 
 support both the ppc and powerpc architecture. When ppc is deleted in six 
 months these #ifdefs will need to be removed.
 
 Another rework of the i2c for powerpc device tree patch. This version 
 implements standard alias naming only on the powerpc platform and only for 
 the device tree names. The old naming mechanism of 
 i2c_client.name,driver_name is left in place and not changed for non-powerpc 
 platforms. This patch is fully capable of dynamically loading the i2c 
 modules. You can modprobe in the i2c-mpc driver and the i2c modules described 
 in the device tree will be automatically loaded. Modules also work if 
 compiled in.
 
 The follow on patch to module-init-tools is also needed since the i2c 
 subsystem has never implemented dynamic loading.
 
 The following series implements standard linux module aliasing for i2c 
 modules on arch=powerpc. It then converts the mpc i2c driver from being a 
 platform driver to an open firmware one. I2C device names are picked up from 
 the device tree. Module aliasing is used to translate from device tree names 
 into to linux kernel names. Several i2c drivers are updated to use the new 
 aliasing. 

Now that I have read all the previous versions of this patch series
and, more importantly, all objections that were raised on the way, I
can start reviewing the latest iteration of your patches. I'll also do
some testing, although I have no powerpc stuff here, but at least I
want to make sure that there are no regressions introduced by your
patches on x86.

-- 
Jean Delvare
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-11 Thread Jon Smirl
On 1/11/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Hi Jon,

 On Wed, 19 Dec 2007 23:41:36 -0500, Jon Smirl wrote:
  Since copying i2c-mpc.c to maintain support for the ppc architecture seems 
  to be an issue; instead rework i2c-mpc.c to use CONFIG_PPC_MERGE #ifdefs to 
  support both the ppc and powerpc architecture. When ppc is deleted in six 
  months these #ifdefs will need to be removed.
 
  Another rework of the i2c for powerpc device tree patch. This version 
  implements standard alias naming only on the powerpc platform and only for 
  the device tree names. The old naming mechanism of 
  i2c_client.name,driver_name is left in place and not changed for 
  non-powerpc platforms. This patch is fully capable of dynamically loading 
  the i2c modules. You can modprobe in the i2c-mpc driver and the i2c modules 
  described in the device tree will be automatically loaded. Modules also 
  work if compiled in.
 
  The follow on patch to module-init-tools is also needed since the i2c 
  subsystem has never implemented dynamic loading.
 
  The following series implements standard linux module aliasing for i2c 
  modules on arch=powerpc. It then converts the mpc i2c driver from being a 
  platform driver to an open firmware one. I2C device names are picked up 
  from the device tree. Module aliasing is used to translate from device tree 
  names into to linux kernel names. Several i2c drivers are updated to use 
  the new aliasing.

 Now that I have read all the previous versions of this patch series
 and, more importantly, all objections that were raised on the way, I
 can start reviewing the latest iteration of your patches. I'll also do
 some testing, although I have no powerpc stuff here, but at least I
 want to make sure that there are no regressions introduced by your
 patches on x86.


Various people were worried about x86. Around version 15 I altered the
patches so that they only impacted PowerPC. If they impact x86 in
current form that is a bug.

When x86 is ready for it I do think dynamic module loading should be
implemented there also.

 --
 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-11 Thread Jean Delvare
On Fri, 11 Jan 2008 10:52:56 -0500, Jon Smirl wrote:
 On 1/11/08, Jean Delvare wrote:
  Now that I have read all the previous versions of this patch series
  and, more importantly, all objections that were raised on the way, I
  can start reviewing the latest iteration of your patches. I'll also do
  some testing, although I have no powerpc stuff here, but at least I
  want to make sure that there are no regressions introduced by your
  patches on x86.
 
 
 Various people were worried about x86. Around version 15 I altered the
 patches so that they only impacted PowerPC. If they impact x86 in
 current form that is a bug.
 
 When x86 is ready for it I do think dynamic module loading should be
 implemented there also.

I agree, and I am doing some testing on x86 to make sure that your
patch will work fine there as well once we decide to go that way.

Your patch set really contains two different parts which should be
clearly identified and discussed separately. Firstly, it lets i2c
drivers export module aliases so that the rest of the world knows which
devices they support. This part I think everybody agrees is needed, so
that platform code no longer needs to specify the driver name for every
I2C device.

Secondly, it promotes OF device names as acceptable aliases. This I
don't think I agree with. While I see some value in moving the OF name
- Linux name translation to the drivers themselves (even though I
don't see this as a mandatory move either), this doesn't imply that OF
names should be used as aliases. I don't like the idea that different
architectures will name the same device differently in a visible way.
This could easily break user-space code that makes assumptions on the
device names (libsensors comes to mind.) So, I think that this part
will need some more discussion.

-- 
Jean Delvare
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-11 Thread Jon Smirl
On 1/11/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Secondly, it promotes OF device names as acceptable aliases. This I
 don't think I agree with. While I see some value in moving the OF name
 - Linux name translation to the drivers themselves (even though I
 don't see this as a mandatory move either), this doesn't imply that OF
 names should be used as aliases. I don't like the idea that different
 architectures will name the same device differently in a visible way.
 This could easily break user-space code that makes assumptions on the
 device names (libsensors comes to mind.) So, I think that this part
 will need some more discussion.

They're aliases.  On the x86 my e1000 Ethernet driver loads using this
alias name:
pci:v8086d1010sv*sd*bc*sc*i*
In fact, the e1000 driver has 63 alias names in addition to e1000

But it's still the e1000 driver after it is loaded.
[EMAIL PROTECTED]:/home/linux/drivers/net/e1000$ lsmod | grep e1000
e1000 115968  0

Loading a I2C driver with an OF alias name is not going to change the
module name after it is loaded. In fact, once the module is in memory
there's no way to tell what name was used to load it.

OF device names are set by the Open Firmware committee. It is not
reasonable to force the Linux names back into Open Firmware since this
would force the other operating systems using Open Firmware to adopt
the Linux names.

This issue hasn't been visible before since there was a global table
in the PowerPC code mapping all known Open Firmware names into linux
names. Keeping this as a global table doesn't scale. The mapping needs
to be done by each device individually.


 --
 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-11 Thread Jochen Friedrich
Hi Jon,

 The following series implements standard linux module aliasing for i2c 
 modules on arch=powerpc. It then converts the mpc i2c driver from being a 
 platform driver to an open firmware one. I2C device names are picked up 
 from the device tree. Module aliasing is used to translate from device tree 
 names into to linux kernel names. Several i2c drivers are updated to use 
 the new aliasing.

 Now that I have read all the previous versions of this patch series
 and, more importantly, all objections that were raised on the way, I
 can start reviewing the latest iteration of your patches. I'll also do
 some testing, although I have no powerpc stuff here, but at least I
 want to make sure that there are no regressions introduced by your
 patches on x86.

 Various people were worried about x86. Around version 15 I altered the
 patches so that they only impacted PowerPC. If they impact x86 in
 current form that is a bug.

I can only second this. The latest version of i2c-cpm
(http://patchwork.ozlabs.org/linuxppc/patch?person=1023id=15902) makes
use of this patch, as well. On the dbox2, loading and unloading of
modules in any order just works fine.

Thanks,
Jochen

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev