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

Andre Heider
2020-12-02T12:37:47Z

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?
- 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
>