Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Vladimir Vid <vladimir.vid@sartura.hr>
To:     pali@kernel.org, a.heider@gmail.com
Cc:     devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, tmn505@gmail.com,
        sebastian.hesselbarth@gmail.com, gregory.clement@bootlin.com,
        andrew@lunn.ch, jason@lakedaemon.net, robh+dt@kernel.org,
        Vladimir Vid <vladimir.vid@sartura.hr>
Subject: Re: [PATCH] arm64: dts: marvell: add DT for ESPRESSObin-Ultra
Date:   Fri, 23 Oct 2020 18:18:02 +0200
Message-Id: <20201023161802.265575-1-vladimir.vid@sartura.hr>
In-Reply-To: <20201009152710.316047-1-vladimir.vid@sartura.hr>
References: <20201009152710.316047-1-vladimir.vid@sartura.hr>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Fri, Oct 9, 2020 at 5:37 PM Pali Rohár wrote:
> Cannot you in this case include armada-3720-espressobin.dtsi and write
> only differences into DTS ultra file to reduce definitions?

> Currently there are pending patches for espressobin DTS files to
> simplify them and use common include file as much as possible to
> de-duplicate same parts.

Updated in v2.

> Looks like this is incorrect as on Armada 3720 at the beginning of the
> nor must be stored Marvell wtmi/secure firmware which is running on
> Cortex M3 processor. For sure there cannot be u-boot. Also on A3720 is
> u-boot loaded by ARM trusted firmware therefore I doubt that u-boot is
> stored at offset zero.

> I guess that on offset zero is stored concatenated binary of Marvell
> secure firmware for M3, ARM trusted firmware (for main CPU) and U-Boot.
> Can you verify it? In this case I would just call this partition as
> "firmware" to indicate what is stored here.

Correct. Concatenated binary you get from building u-boot and ATF, which
includes wtmi, is flashed to offset zero.
I agree that firmware is more appropriate name for the partition,
updated in v2.


On Mon, Oct 12, 2020 at 9:08 AM Andre Heider wrote:
> the other espressobin boards use "Globalscale Marvell ESPRESSOBin
> Board", so maybe "Globalscale Marvell ESPRESSOBin Ultra Board"?

Updated in v2.

>> > +                     partition@3e0000 {
>> > +                             label = "hw-info";
> any idea what's in there?

SPI is used insted of the EEPROM to save some hardware related info,
currently there are MAC addresses and serial number on that partition.

>> > +&usb3 {
>> > +     status = "okay";
>> > +     usb-phy = <&usb3_phy>;
>
> That node comes from armada-37xx.dtsi and has:
>
>        phys = <&comphy0 0>, <&usb2_utmi_otg_phy>;
>        phy-names = "usb3-phy", "usb2-utmi-otg-phy";
>
> Are those supposed to be mixed?

No, I don't think they should. I originally added dts for the 4.19
and there weren't usb2 related nodes there.
I'm disabling usb3 for the moment until I see what's going on,
as I'm currently working on adding u-boot support and a couple
of other things.

For the other comments, I think they are resolved by including
the espressobin dtsi instead of armada-37xx.