Stephen, On Tue, Feb 12, 2013 at 11:10 AM, Stephen Warren <swar...@wwwdotorg.org> wrote: > On 02/12/2013 10:40 AM, Tom Warren wrote: >> Laxman, >> >> On Tue, Feb 12, 2013 at 5:02 AM, Laxman Dewangan <ldewan...@nvidia.com> >> wrote: >>> On Friday 08 February 2013 11:34 PM, Stephen Warren wrote: >>>> >>>> On 02/08/2013 10:25 AM, Tom Warren wrote: >>>>> >>>>> T114, like T30, does not have a separate/different DVC (power I2C) >>>>> controller like T20 - all 5 I2C controllers are identical, but >>>>> I2C5 is used to designate the controller intended for power >>>>> control (PWR_I2C in the schematics). PWR_I2C is set to 400KHz. >>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts >>>>> b/board/nvidia/dts/tegra114-dalmore.dts >>>>> + aliases { >>>>> + }; >>>>> + >>>> >>>> There's no point adding an empty aliases node here. Feel free to fix >>>> that up when you apply it rather than reposting if you want. >>>> >>>> I'd like too see Laxman sign-off on the "*2" question he had earlier >>>> before actually checking this in. >>>> >>> We do not require *2 as the i2c clock divider is DIVU16 type. There was bug >>> in early code on kernel also which we fixed in dowstream long back. Possibly >>> uboot have not fixed this yet. >> >> Yeah, the Tegra I2C driver in U-Boot has always doubled the rate >> before calling the clock set routine. > > Perhaps it's related to some dividers being U7.1 (i.e. the LSB of the > divider represents 1/2 not 1)? However, as Laxman points out, this isn't > the case for I2C clocks; they have a U16 divider, so the LSB represents > 1 not 1/2. > >> But the important thing is that >> the actual I2C clock is 100KHz (or 400KHz for I2C5) as measured at the >> test points on the board. >> >> I'll look into the Tegra U-Boot clock routine idiosyncrasies later >> when I get some more bandwidth. > > Just a few minutes of investigation points at clk_get_divider() being buggy. > > It assumes that all dividers have a built-in *2 clock multiplier on the > front of them: > > u64 divider = parent_rate * 2; > > (the name for that variable is wrong; it should be something more like > parent_rate or divider_input_rate) > > This (presence of a *2 pre-multiplier) is true for U7.1 dividers, since > this is required for the LSB of the divider to represent 1/2 not 1. > Hence, I assume that e.g. the SPI driver doesn't do this "* 2" on the > clock rate; it actually has a U7.1 divider. > > However, this is not true for U16 dividers, since the HW doesn't need to > multiply up the rate before dividing, since the LSB is 1 not 1/2. > > The solution here is to fix clk_get_divider() to return both a field > width and a flag (or width) indicating whether a fractional part of the > divider is present, and then pass that on to adjust_periph_pll() so that > it can only optionally perform this initial "* 2". > > So at least that explains it. Yeah, I just started looking at it before lunch, and saw the same thing.
I'll see about fixing it now so I can put the T114 I2C patches to bed. Thanks > > I would strongly recommend just fixing this now. However, if you don't > please do file a bug so that we don't forget about this. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot