Decouple system timer and drivers: I2C #15415
Conversation
pizi-nordic
requested review from
Mani-Sadhasivam,
MaureenHelm,
nashif and
vanti
as code owners
pizi-nordic
force-pushed
the
decouple-system-timer-and-clock-i2c
branch
from
8dd9175
to
5ad9de5
Compare
There was a problem hiding this comment.
Although, I don't disagree with the change (and in general with this kind of patches), I wonder if the "clock-frequency" is actually going to be "copied" in all DTS driver nodes, for each platform, e.g. SPIs, UARTs, etc.
If, so, I wonder if there a way to "get" the value of this clock-frequency, somehow, and not hard-code it in every peripheral node. I understand that different peripherals might have different clock sources, so until we have a proper clock management, this, all , might be hard, so I am just wondering.
|
We should have a "clock" node which references the clock source for the peripheral. But this needs both entries in DTS as well as better DTS infrastructure (ideally we should have something like DT_I2C_0_CLOCK_0_FREQUENCY extracted from clock node.). |
| @@ -18,6 +18,12 @@ properties: | |||
| generation: define | |||
| category: required | |||
|
|
|||
| "cc32xx,clock-frequency" : | |||
There was a problem hiding this comment.
What about naming it ip-clock-frequency? And consistently using IP_ also in the fixup label? DT_I2C_0_CLOCK_FREQUENCY looks like something generated for the clock-frequency property.
There was a problem hiding this comment.
No problem for me. Any other opinions/suggestions on that?
There was a problem hiding this comment.
We did something on sifive, looks like we used input-frequency. But I'm not keen on propagating that everywhere
I agree that this is the real solution, as shown in examples like this one: In the meantime we can go with one of the options here |
The i2c_cc32xx driver used system clock frequency as a base for I2C clock frequency calculation. This commit corrects that by obtaining the needed value from DTS. Please note, that for I2C devices the clock-frequency property specifies SCK frequency, instead of frequency of the clock driving peripheral. To solve that problem, a new property was added. Signed-off-by: Piotr Zięcik <[email protected]>
pizi-nordic
force-pushed
the
decouple-system-timer-and-clock-i2c
branch
from
5ad9de5
to
6896de9
Compare
| "cc32xx,clock-frequency" : | ||
| type: int | ||
| category: required | ||
| description: Clock frequency the SPI peripheral is being driven at |
| @@ -18,6 +18,12 @@ properties: | |||
| generation: define | |||
| category: required | |||
|
|
|||
| "cc32xx,clock-frequency" : | |||
There was a problem hiding this comment.
We did something on sifive, looks like we used input-frequency. But I'm not keen on propagating that everywhere
|
@pizi-nordic was the thread of discussion about how to handle 'clock' vs 'clock-frequency' for i2c here or somewhere else? |
|
@galak: Here: #15415 (comment). |
During our work in timer area, we found that system clock frequency is used incorrectly in several drivers.
For example, it is used to obtain bus frequency needed to calculate speed of I2C interfaces.
If we select different system timer device, all these calculations become invalid.
This PR fixes the problem by obtaining all needed information directly from the DTS.
Brings us closer to: #15363
Please note, that for I2C devices the clock-frequency property specifies SCK frequency, instead of frequency of the clock driving peripheral. To solve that problem, a new property was added.
(I used https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-ocores.txt as a reference)