[PATCH 0/4] Use just one DTS file for all Espressobin variants

Philipp Tomsich
2020-12-02T14:20:10Z

> On 02.12.2020, at 13:37, Andre Heider <a.heider@gmail.com> wrote:
>
> On 02/12/2020 11:35, Stefan Roese wrote:
>> On 02.12.20 10:12, Pali Roh?r wrote:
>>> On Wednesday 02 December 2020 09:09:15 Stefan Roese wrote:
>>>> On 02.12.20 01:33, Pali Roh?r wrote:
>>>>> On Wednesday 25 November 2020 19:20:06 Pali Roh?r wrote:
>>>>>> This patch series change Espressobin code to use in U-Boot just one DTS
>>>>>> file for all Espressobin variants. Therefore DT compatible string
>>>>>> globalscale,espressobin-emmc is not used anymore as it is not needed.
>>>>>>
>>>>>> It means that setup and compilation of U-Boot for Espressobin is less
>>>>>> complicated and more simple. As there is no need to check for HW details
>>>>>> and just one U-Boot binary would work for all Espressobin variants.
>>>>>>
>>>>>> First two patches just revert previous eMMC support and next two patches
>>>>>> add support for eMMC in way that just one DTS file is used and fdtfile
>>>>>> env variable is correctly set for any Espressobin variant.
>>>>>>
>>>>>> We have tested that fdtfile env variable is correctly set on Espressobin
>>>>>> variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
>>>>>> Also that eMMC is working on Espressobin variant with eMMC.
>>>>>
>>>>> Stefan, could you please review this patch series?
>>>>
>>>> I like the approach in general to simplify things. One comment though:
>>>>
>>>> AFAICT, Linux uses multiple dts/dtsi files for espressobin. So your
>>>> approach to move to one single file contradicts the (planned after
>>>> comphy conversion) move to the Linux dts/dtsi files.
>>>
>>> After comphy conversion we can use e.g. Linux dtsi file and create one
>>> main U-Boot dts file which would contain all nodes enabled and in U-Boot
>>> code disable nodes which are not present/relevant. This patch series
>>> allows to detect all variants v5, v7, with emmc, without emmc; so we can
>>> reconstruct dts file at U-Boot runtime. In Linux we also simplified dts
>>> files as much as possible, so all options are in common dtsi file and
>>> only variant relevant changes (enable/disable nodes) are in dts files.
>> I see. So let's please wait for Andre, if he has some comments on this.
>> Thanks,
>> Stefan
>>>>> Andre, are you fine with these changes? I would like to get your
>>>>> acknowledgment or review comment what needs to be changed or improved as
>>>>> this patch series basically rework your code (which is first reverted
>>>>> and them implemented in different way).
>>>>
>>>> Yes. Andre please also comment on this.
>
> This is a nice simplification. Pali had this idea before, and I hesitated because of two questions I don't have the answer to:
> - Can we just try to detect an emmc chip on an unpopulated board without any drawbacks?

All eMMC host controllers I know, will eventually return a timeout on the first command(s) to the eMMC.
So the main drawback should be wasted runtime that will slow down the boot sequence.

> - What about power consumption? Does this unnecessarily waste power on non-emmc boards because something is still powered up?
>
> But if noone sees an issue with that I don't have any objections.
>
> I think this set could be simplified though. Instead of the first two reverts, just remove the non-emmc dts and rename the emmc dts. That's less churn and keeps the dtsi for future syncing with linux.
>
> Thanks,
> Andre
>
>>>>
>>>> Thanks,
>>>> Stefan
>>>>
>>>>>> Pali Roh?r (4):
>>>>>> Revert "arm64: dts: armada-3720-espressobin: split common parts to
>>>>>> .dtsi"
>>>>>> Revert "arm64: dts: a3720: add support for espressobin with populated
>>>>>> emmc"
>>>>>> arm: mvebu: Espressobin: Add support for emmc into dts file
>>>>>> arm: mvebu: Espressobin: Detect presence of emmc at runtime
>>>>>>
>>>>>> arch/arm/dts/Makefile | 1 -
>>>>>> arch/arm/dts/armada-3720-espressobin-emmc.dts | 44 -----
>>>>>> arch/arm/dts/armada-3720-espressobin.dts | 186 +++++++++++++++++-
>>>>>> arch/arm/dts/armada-3720-espressobin.dtsi | 167 ----------------
>>>>>> board/Marvell/mvebu_armada-37xx/board.c | 6 +-
>>>>>> doc/README.marvell | 7 +-
>>>>>> 6 files changed, 186 insertions(+), 225 deletions(-)
>>>>>> delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>>>>>> delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>>>>>>
>>>>>> --
>>>>>> 2.20.1
>>>>>>
>>>>
>>>>
>>>> Viele Gr??e,
>>>> Stefan
>>>>
>>>> --
>>>> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
>>>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>>>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
>> Viele Gr??e,
>> Stefan