On 12/05/2022 01:49, Chris Packham wrote:
>
>>> + spi_clock: spi-clock {
>>> + compatible = "fixed-clock";
>>> + #clock-cells = <0>;
>>> + clock-frequency = <200000000>;
>>> + };
>> My questions about these clocks are still unanswered. Why do you define
>> fixed-clocks. Aren't these part of clock controller?
>
> Not one that I have any information on. Marvell don't put it in their
> customer facing documentation so I can only speculate. The 25MHz
> oscillator goes into the chip, from there I guess that it is fed in some
> fashion to both the CPU block (CnM in Marvell speak) and to the switch
> block. Where exactly it gets divided into these individual peripheral
> clocks I don't really know.
Hm, so it seems you do not have a proper clock-controller (or cannot
create one). OK then, but these are silly stubs, you know. :)
>
>>> + };
>>> +};
>>> diff --git a/arch/arm64/boot/dts/marvell/rd-ac5x.dts b/arch/arm64/boot/dts/marvell/rd-ac5x.dts
>>> new file mode 100644
>>> index 000000000000..7ac87413e023
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/marvell/rd-ac5x.dts
>>> @@ -0,0 +1,90 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Device Tree For AC5X.
>>> + *
>>> + * Copyright (C) 2021 Marvell
>>> + *
>>> + */
>>> +/*
>>> + * Device Tree file for Marvell Alleycat 5X development board
>>> + * This board file supports the B configuration of the board
>>> + */
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "armada-98dx2530.dtsi"
>>> +
>>> +/ {
>>> + model = "Marvell RD-AC5X Board";
>>> + compatible = "marvell,ac5x", "marvell,ac5";
>> From the bindings I understood ac5x is a SoC, not board. If ac5x is a
>> board, not a SoC, then compatible should be rather "marvell,rd-ac5x".
>
> So If I understand the convention the full compatible would be:
>
> compatible = "marvell,rd-ac5x", "marvell,ac5x", "marvell,ac5";
Yes, this looks correct.
>
>>
>>> +
>>> + memory@0 {
>>> + device_type = "memory";
>>> + reg = <0x2 0x00000000 0x0 0x40000000>;
>>> + };
>>> +};
>>> +
>>> +&mdio {
>>> + phy0: ethernet-phy@0 {
>>> + reg = <0>;
>>> + };
>>> +};
>>> +
>>> +&i2c0 {
>>> + status = "okay";
>>> +};
>>> +
>>> +&i2c1 {
>>> + status = "okay";
>>> +};
>>> +
>>> +ð0 {
>>> + status = "okay";
>>> + phy-handle = <&phy0>;
>>> +};
>>> +
>>> +&usb0 {
>>> + status = "okay";
>>> +};
>>> +
>>> +&usb1 {
>>> + status = "okay";
>>> +};
>>> +
>>> +&spi0 {
>>> + status = "okay";
>>> +
>>> + spiflash0: flash@0 {
>>> + compatible = "jedec,spi-nor";
>>> + spi-max-frequency = <50000000>;
>>> + spi-tx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
>>> + spi-rx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
>>> + reg = <0>;
>>> +
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> +
>>> + partition@0 {
>>> + label = "spi_flash_part0";
>>> + reg = <0x0 0x800000>;
>>> + };
>>> +
>>> + parition@1 {
>>> + label = "spi_flash_part1";
>>> + reg = <0x800000 0x700000>;
>>> + };
>>> +
>>> + parition@2 {
>>> + label = "spi_flash_part2";
>>> + reg = <0xF00000 0x100000>;
>>> + };
>>> + };
>>> +};
>>> +
>>> +&usb1 {
>>> + compatible = "chipidea,usb2";
>> Why compatible is defined per board?
>
> That came from the Marvell SDK. On some boards this is used as a
> device/OTG interface. But regardless it should have one in the SoC dtsi.
Yes, please.
> As for why they used the "chipidea,usb2" compatible I have no idea. I'll
> remove this and add the correct compatible to the SoC.
They could reuse some other block. Pretty often for such cases there is
a dedicated compatible and fallback, e.g.:
Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
Best regards,
Krzysztof
