On 4/26/24 4:42 PM, Conor Dooley wrote:
On Fri, Apr 26, 2024 at 04:12:00PM +0200, Thomas Bonnefille wrote:
On 4/25/24 6:35 PM, Conor Dooley wrote:
On Thu, Apr 25, 2024 at 09:51:26AM -0500, Rob Herring wrote:
On Thu, 25 Apr 2024 10:21:31 +0200, Thomas Bonnefille wrote:
This adds I2C support in the device tree of the T-Head TH1520 RISCV-SoC
and a default configuration for the BeagleV-Ahead. It appears that the
TH1520 I2C is already supported in the upstream kernel through the
Synopsis Designware I2C adapter driver.
As there is no clock driver for this board as of today, this patch
series uses a fixed-clock named i2c_ic_clk.
There is also no pinctrl driver yet so pinmux must be handled manually
for now.
It also fixes the order of the nodes in the device tree to comply with
device-tree coding-style.
Thomas Bonnefille (4):
dt-bindings: i2c: dw: Document compatible thead,th1520-i2c
riscv: boot: dts: thead: Fix node ordering in TH1520 device tree
riscv: dts: thead: Add TH1520 I2C nodes
riscv: dts: thead: Enable I2C on the BeagleV-Ahead
.../bindings/i2c/snps,designware-i2c.yaml | 12 ++
.../boot/dts/thead/th1520-beaglev-ahead.dts | 22 ++++
arch/riscv/boot/dts/thead/th1520.dtsi | 120 ++++++++++++++----
3 files changed, 127 insertions(+), 27 deletions(-)
--
2.44.0
My bot found new DTB warnings on the .dts files added or changed in this
series.
Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.
If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:
pip3 install dtschema --upgrade
New warnings running 'make CHECK_DTBS=y thead/th1520-beaglev-ahead.dtb' for 20240425082138.374445-1-thomas.bonnefille@xxxxxxxxxxx:
arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dtb: i2c-clock: clock-frequency:0:0: 50000000 is greater than the maximum of 5000000
The bot is not freaking out here, 50 MHz is indeed more than 5 MHz :)
5MHz is the maximum clock-frequency, ie. the I2C bus frequency.
This is actually set to 100kHz for I2C0 in the DT:
&i2c0 {
status = "okay";
clock-frequency = <100000>; <----
...
};
50MHz is the "fixed-clock" frequency, that is the clock feeding the I2C
IP block:
i2c_ic_clk: i2c-clock {
compatible = "fixed-clock";
clock-frequency = <50000000>; <-----
#clock-cells = <0>;
};
My guess is that the bot confused the clock-frequency parameter for the bus
clock (SCL) with the i2c-ic-clock value for the controller itself during the
checks.
Do you agree with this or am I misunderstanding the error ?
If I lower the fixed-clock frequency to eg. 100kHz, the error is gone. But I
guess the 5MHz limit should probably not apply to the input clock?
Heh, I know why that's happening - it's your node name.
The pattern for i2c controllers is "^i2c(@.*)?":
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/i2c/i2c-controller.yaml
Rob co-incidentally (or maybe not) put out a patch for fixed-frequency
clock names, suggesting using clock as a prefix:
https://lore.kernel.org/all/20240425183810.3079069-1-robh@xxxxxxxxxx/
If we switched to that format, I believe your problem goes away.
Cheers,
Conor.
Indeed it was the node name, when I switched the name of the fixed clock
to clock-i2c-ic, the error was gone, thank you. :)
But i2c_ic_clk shouldn't match "^i2c(@.*)?", one of my teammates
suggested that the error may instead come from this line
https://elixir.bootlin.com/linux/latest/source/scripts/dtc/checks.c#L1023.
But as Drew is working on new iterations of the TH1520's clock driver I
will just delete this fixed clock and publish a second version depending
on his patch.
Thank you,
Thomas
