Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c
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
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
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
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
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=1023&id=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
Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c
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
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
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
