Re: [PATCH 2/2] arm64: dts: marvell: add support for Methode eDPU

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 10/05/2022 14:43, Robert Marko wrote:
> On Tue, May 10, 2022 at 1:46 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 10/05/2022 13:41, Robert Marko wrote:
>>> On Tue, May 10, 2022 at 12:20 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 09/05/2022 13:00, Robert Marko wrote:
>>>>> Methode eDPU is an Armada 3720 powered board based on the Methode uDPU.
>>>>>
>>>>> They feature the same CPU, RAM, and storage as well as the form factor.
>>>>>
>>>>> However, eDPU only has one SFP slot plus a copper G.hn port.
>>>>>
>>>>> In order to reduce duplication, split the uDPU DTS into a common one.
>>>>>
>>>>> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
>>>>> ---
>>>>>  arch/arm64/boot/dts/marvell/Makefile          |   1 +
>>>>>  .../boot/dts/marvell/armada-3720-eDPU.dts     |  14 ++
>>>>>  .../boot/dts/marvell/armada-3720-uDPU.dts     | 148 +---------------
>>>>>  .../boot/dts/marvell/armada-3720-uDPU.dtsi    | 163 ++++++++++++++++++
>>>>>  4 files changed, 179 insertions(+), 147 deletions(-)
>>>>>  create mode 100644 arch/arm64/boot/dts/marvell/armada-3720-eDPU.dts
>>>>>  create mode 100644 arch/arm64/boot/dts/marvell/armada-3720-uDPU.dtsi
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
>>>>> index 1c794cdcb8e6..104d7d7e8215 100644
>>>>> --- a/arch/arm64/boot/dts/marvell/Makefile
>>>>> +++ b/arch/arm64/boot/dts/marvell/Makefile
>>>>> @@ -1,6 +1,7 @@
>>>>>  # SPDX-License-Identifier: GPL-2.0
>>>>>  # Mvebu SoC Family
>>>>>  dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-db.dtb
>>>>> +dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-eDPU.dtb
>>>>>  dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin.dtb
>>>>>  dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin-emmc.dtb
>>>>>  dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin-ultra.dtb
>>>>> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-eDPU.dts b/arch/arm64/boot/dts/marvell/armada-3720-eDPU.dts
>>>>> new file mode 100644
>>>>> index 000000000000..6b573a6854cc
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/marvell/armada-3720-eDPU.dts
>>>>> @@ -0,0 +1,14 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>> +
>>>>> +/dts-v1/;
>>>>> +
>>>>> +#include "armada-3720-uDPU.dtsi"
>>>>> +
>>>>> +/ {
>>>>> +     model = "Methode eDPU Board";
>>>>> +     compatible = "methode,edpu", "marvell,armada3720";
>>>>
>>>> You need also bindings for the board compatible. Someone should convert
>>>> the Documentation/devicetree/bindings/arm/marvell/armada-37xx.txt to YAML.
>>>
>>> Ok, I can convert the SoC compatibles at least for now.
>>> Any advice you can give me on how the handle the Espressobin boards
>>> having multiple board-specific compatibles?
>>> For example, Espressobin V7 has:
>>> "globalscale,espressobin-v7", "globalscale,espressobin"
>>>
>>
>> Documentation/devicetree/bindings/arm/fsl.yaml
> 
> Thanks, now it makes sense.
> 
>>
>>>>
>>>>> +};
>>>>> +> +  sfp_eth1: sfp-eth1 {
>>>>
>>>> Generic node names, please.
>>>
>>> Can you give me an example of what would be appropriate here because the SFP
>>> bindings example utilizes the same naming scheme as used here?
>>
>> "sfp" if you have only one sfp node.
> 
> There are 2 SFP nodes in total, that is why they are named according
> to the ethernet controller
> to which they are connected.
> uDPU has 2 SFP slots while eDPU has 1, so one was moved to uDPU DTS.

Ah, then let it be. sfp-1 and sfp-2 would also work, but that's not
important.


Best regards,
Krzysztof