i2c-mpc clocking scheme
All,
is anybody working on some improvements regarding configurable I2C frequency inside the i2c-mpc driver ?
If not - would anybody be intersted in getting this done, i.e. configurable via device tree ?
regards,
Andre
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
Quoted 7 lines · Andre Schwarz
All,
is anybody working on some improvements regarding configurable I2C frequency inside the i2c-mpc driver ?
If not - would anybody be intersted in getting this done, i.e. configurable via device tree ?
Maybe I'm missing something, but U-Boot configures the I2C bus speed. It does this because the algorithm is specific to the SOC itself. For example, the 8544 is different from the 8548. It would be a mess to duplicate this code in the kernel.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 8 lines · Timur Tabi
Quoted 5 lines · Andre Schwarz
All,
is anybody working on some improvements regarding configurable I2C frequency inside the i2c-mpc driver ?
Quoted 10 lines
Quoted 3 lines
If not - would anybody be intersted in getting this done, i.e. configurable via device tree ?
Maybe I'm missing something, but U-Boot configures the I2C bus speed. It does this because the algorithm is specific to the SOC itself. For example, the 8544 is different from the 8548. It would be a mess to duplicate this code in the kernel.
You're right regarding U-Boot, but the i2c-mpc driver overwrites the "frequency divider register" on all chips.
I'm not happy with a fixed 0x3f @ MPC5200 which results in 65kHz ... :-(
Have a look at line 163 in drivers/i2c/busses/i2c-mpc.c
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
Quoted 19 lines · Andre Schwarz
Quoted 8 lines · Timur Tabi
Quoted 5 lines · Andre Schwarz
All,
is anybody working on some improvements regarding configurable I2C frequency inside the i2c-mpc driver ?
Quoted 10 lines
Quoted 3 lines
If not - would anybody be intersted in getting this done, i.e. configurable via device tree ?
Maybe I'm missing something, but U-Boot configures the I2C bus speed. It does this because the algorithm is specific to the SOC itself. For example, the 8544 is different from the 8548. It would be a mess to duplicate this code in the kernel.
Quoted 6 lines
You're right regarding U-Boot, but the i2c-mpc driver overwrites the "frequency divider register" on all chips.
I'm not happy with a fixed
0x3f@ MPC5200 which results in 65kHz ... :-(Have a look at line 163 in
drivers/i2c/busses/i2c-mpc.c
Seems like it should keep the clock registers at what u-boot set them too.
Quoted 1 line · Trent Piepho
Seems like it should keep the clock registers at what u-boot set them too.
Or we could have U-Boot put the i2c clock frequency into the I2C node, and let the driver program the hardware again. That would keep the ugliness in U-Boot.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 10 lines · Timur Tabi
Quoted 2 lines · Trent Piepho
Seems like it should keep the clock registers at what u-boot set them too.
Or we could have U-Boot put the i2c clock frequency into the I2C node, and let the driver program the hardware again. That would keep the ugliness in U-Boot.
Wouldn't it be easier to omit frequency re-programming at all ? Maybe configurable for non U-Boot users ...
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
Quoted 2 lines · André Schwarz
Wouldn't it be easier to omit frequency re-programming at all ? Maybe configurable for non U-Boot users ...
Well, the real problem is that Linux is ignoring what the boot loader has done.
This is bad, regardless as to which boot loader you're using.
The question is, do we give Linux the capability to program the I2C bus speed, or is this something that the boot loader should do? Unfortunately, the current Linux code means that this issue is not properly architected.
You mentioned people who don't use U-Boot. Do they use some other boot loader?
Or no boot loader at all? Do other boot loaders program the I2C bus speed?
The reason I ask is that I want to know whether it's okay for Linux to ignore the FDR and DFSRR registers.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 8 lines · Andr� Schwarz
Quoted 7 lines · Timur Tabi
Quoted 3 lines · Trent Piepho
Seems like it should keep the clock registers at what u-boot set them too.
Quoted 8 lines
Quoted 6 lines
Or we could have U-Boot put the i2c clock frequency into the I2C node, and let
the driver program the hardware again. That would keep the ugliness in U-Boot.Wouldn't it be easier to omit frequency re-programming at all ? Maybe configurable for non U-Boot users ...
That's what I'm thinking too. Calculating the settings for a given bus frequency, even with the system specific source clock provided by u-boot, involves either a complex algorithm or a big ugly black box table that's even larger than the algorithm. I also think it's different for 8xxx and 52xx.
On the other hand just keeping the clock the same doesn't require any code at all.
U-boot could pass in "bus-frequency" to let software know the speed of the I2C bus from Linux. Seems like a standard property for bus nodes.
There could be a "current-speed" property that tells linux to keep the registers the same, in case we have to worry about u-boot not programming the i2c clock. I think the serial drivers have something like this.
Quoted 2 lines · Trent Piepho
U-boot could pass in "bus-frequency" to let software know the speed of the I2C bus from Linux. Seems like a standard property for bus nodes.
clock-frequency is standard, though it should probably be the input frequency rather than the bus frequency, in case the OS really does want to change it (maybe making the bus run faster when accessing faster devices).
Quoted 2 lines
There could be a "current-speed" property that tells linux to keep the registers the same,
That would be a bit different from the way it's used in serial nodes, where current-speed is simply a description of the baud rate that corresponds to the current divider setting. I'm not sure that it makes as much sense for i2c, as you don't have the shared state on the other end that depends on maintaining the exact same speed.
When does the guest really care what the specific i2c bus frequency is, if it's not going to change it?
-Scott
Quoted 7 lines · Scott Wood
Quoted 2 lines · Trent Piepho
U-boot could pass in "bus-frequency" to let software know the speed of the I2C bus from Linux. Seems like a standard property for bus nodes.
clock-frequency is standard, though it should probably be the input frequency rather than the bus frequency, in case the OS really does want to change it (maybe making the bus run faster when accessing faster devices).
For a bus device like an i2c controller, you really have two clocks. The input clock the controller runs from and the speed it runs the bus at. One could say that one clock is for the device node and the other clock is for the device's sub-nodes.
Linux doesn't have an API for setting I2C bus speed. If it did, then adding support for it to i2c-mpc if there was demand would really be another patch. Right now the problem is that Linux programs the I2C controller stupidly and undoes u-boot's (better) settings.
Quoted 8 lines
Quoted 2 lines
There could be a "current-speed" property that tells linux to keep the registers the same,
That would be a bit different from the way it's used in serial nodes, where current-speed is simply a description of the baud rate that corresponds to the current divider setting. I'm not sure that it makes as much sense for i2c, as you don't have the shared state on the other end that depends on maintaining the exact same speed.
The Linux code could use current-speed to know if it should program the registers. I.e., if current-speed is present and non-zero, then leave the frequency registers alone. Otherwise u-boot or whatever might not have programmed the I2C controller and the driver can do what it's doing now.
Quoted 2 lines
When does the guest really care what the specific i2c bus frequency is, if it's not going to change it?
I don't know of a real reason. Maybe an I2C device where the clock speed makes a difference? Maximum polling rate or something? Is there reason the CPU clock and the CCB frequency need to be in the device tree?
Quoted 4 lines · Trent Piepho
For a bus device like an i2c controller, you really have two clocks. The input clock the controller runs from and the speed it runs the bus at. One could say that one clock is for the device node and the other clock is for the device's sub-nodes.
We could add a property to each I2C device nodes that lists the maximum speed that this supports. Then the I2C driver could find the smallest of these speeds, and program the I2C controller for that speed.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 4 lines · Trent Piepho
The Linux code could use current-speed to know if it should program the registers. I.e., if current-speed is present and non-zero, then leave the frequency registers alone. Otherwise u-boot or whatever might not have programmed the I2C controller and the driver can do what it's doing now.
I suppose. I was thinking that Linux could just check to see whether the current divider value appears to be valid, but it seems that all values including zero can be valid. :-(
Quoted 5 lines
Quoted 1 line
When does the guest really care what the specific i2c bus frequency is, if it's not going to change it?
I don't know of a real reason. Maybe an I2C device where the clock speed makes a difference? Maximum polling rate or something? Is there reason the CPU clock and the CCB frequency need to be in the device tree?
I'm fine with including it for informational purposes, it just doesn't seem quite as necessary.
-Scott
Quoted 11 lines · Timur Tabi
Quoted 7 lines · Andre Schwarz
All,
is anybody working on some improvements regarding configurable I2C frequency inside the i2c-mpc driver ?
If not - would anybody be intersted in getting this done, i.e. configurable via device tree ?
Maybe I'm missing something, but U-Boot configures the I2C bus speed.
i2c-mpc sets the bus clock in kernel with mpc_i2c_setclock. The bus speed in kernel is indepedent from u-boot.
Quoted 4 lines
It does this because the algorithm is specific to the SOC itself. For example, the 8544 is different from the 8548. It would be a mess to duplicate this code in the kernel.
I did try to write some mechanismen to make the i2c bus speed configurable for mpc5200. I found it quite hard to find some algorithm to calculate the right values in the Timing table of the I2C bus controller. Anyhow I gave it up after starring at the datasheet for some time. ;-) I can't speak for other platforms, which are supported by i2c-mpc. Anyway, I do think it might be interesting to have configurable i2c bus speed.
cheers
Luotao Fu
Signature · 3 lines
Dipl.-Ing. Luotao Fu | Phone: +49-5121-206917-5004 Pengutronix - Linux Solutions for Science and Industry Entwicklungszentrum Nord http://www.pengutronix.de