Hi Cedric, On 03/17/2017 04:35 PM, M'boumba Cedric Madianga wrote: >> Sorry I don't understand. >> The value you use from the DT and the one calculated from the setup/hold/high/low value >> with the algorithm I developed will set the same values. > > With the ST tool, I could set the following values: > I2C speed mode (Master, Fast Mode, Fast Mode Plus) > I2C speed frequency > I2C clock source frequency > I2C rise time > I2C fall time > > The values set in the DT in this patchset is 0x40202537 for the > following input values: > I2C speed mode = Master mode > I2C speed frequency= 100 kHz > I2C clock source frequency = 48 MHz > I2C rise time = 25 ns > I2C fall time = 10 ns > > If I keep all the above values and just change I2C rise time with 100 > ns, I will have TIMINGR value at 0x10805E89 > >> >> The main difference is that you won't need the ST tool to calculate these, and even better >> you can provide generic binding for whatever APB frequency the I2C peripheral is running >> on. > > As, I2C rise/fall time have some impacts in I2C timings value, the > question is: it is very relevant to let customer control these > parameters ? Actually, you could specify a different rise time in DT if it's relevant for a specific design, this is why you have the following DT attributes : - i2c-scl-falling-time-ns - i2c-scl-internal-delay-ns - i2c-scl-rising-time-ns - i2c-sda-falling-time-ns > If the answer is NO, I agree with you, it is better to use your > formula and remove this property from DT. > If the answer is YES, I think we should keep ST tool. Note that the ST tool calculations are tied to the clock source frequency, so either you provide a table for all the possible clock source frequencies or calculate dynamically. Having a single parameter for a single frequency is, from my point of view, not acceptable. And, I don't think it's a military secret to have (at least) a simplified algorithm from ST... since you have very detailed explanations in the public manuals ! > >> Having the STM32L4 running mainline won't be hard, maybe useless, but not hard. >> My point is that you can somehow consider the STM32F0/F1/F4 I2C IP to be "v1" or "gen1", >> and the L0/L4/F6 (and H7 ?) to be "v2" or "gen2" then prepend a generic compatible to >> have something like : >> compatible = "st,stm32-i2c-v2", "st,stm32f7-i2c" > > This is not the strategy chosen by the company. > They decided to push all driver with ip_name-stm32.c if the driver is > common for all Soc. > If it not the case, the suffix to be used is the name of the first > supported SoC that introduced the IP in the mainline kernel. OK, but I think the I2C and DT maintainers are also involved in these kind of decisions. They should also give their advice. Neil > > BR, > Cedric > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
