Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752375AbdCBM5F (ORCPT ); Thu, 2 Mar 2017 07:57:05 -0500 Received: from mail-wr0-f182.google.com ([209.85.128.182]:34421 "EHLO mail-wr0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750892AbdCBM4j (ORCPT ); Thu, 2 Mar 2017 07:56:39 -0500 Subject: Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL To: =?UTF-8?Q?Andreas_F=c3=a4rber?= References: <1488365164-22861-1-git-send-email-narmstrong@baylibre.com> <1488365164-22861-4-git-send-email-narmstrong@baylibre.com> Cc: sboyd@codeaurora.org, khilman@baylibre.com, carlo@caione.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org From: Neil Armstrong Organization: Baylibre Message-ID: Date: Thu, 2 Mar 2017 13:47:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4107 Lines: 102 Hi Andreas, On 03/02/2017 01:31 PM, Andreas F?rber wrote: > Hi Neil, > > Am 01.03.2017 um 11:46 schrieb Neil Armstrong: >> The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs. > > First of all, any reason you're upper-casing Mali in the commit message? > ARM doesn't. No reason, only a type, indeed it was lower-casing on the v1. Will fix in v2. > >> >> The node is simply added in the meson-gxbb.dtsi file. > > The GXBB part looks fine on a quick look. > >> >> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this >> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific >> dtsi files. > > This part is slightly confusing though. > > What exactly is the GXL vs. GXM difference that this can't be handled by > overriding node properties compatible/interrupts/clocks? I am missing a > GXM patch in this series as rationale for doing it this way. > > In particular I am wondering whether the whole GXM-inherits-from-GXL > concept is flawed and should be adjusted if this leads to secondary > .dtsi files like this: My proposal would be to instead create a > meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit > the current common parts from, then the Mali bits can simply go into > meson-gxl.dtsi without extra #includes needed in S905X and S905D. While > it's slightly more work to split once again, I think it would be cleaner. The GXL and GXM differences are very small : - They share the same clock tree - They share the same pinctrl and even the same pinout (S905D and S912 are pin-to-pin compatible) - They share all the peripherals The only changes are : - Enhanced video encoding and decoding support, this will need a family-specific compatible when pushed - Slightly differences in the Video Processing Unit, this is why I introduced family-specific compatibles - A secondary Cortex-A53 cluster - A secondary SCPI cpufreq clock entry - A different Mali core, but with the same interrupts (less but they share the same lower interrupts), clocks and memory space This is why it was decided to have a sub-dtsi, having a secondary dtsi will simply copy 99% of the GXL dtsi, but surely we could also have an intermediate dtsi but for boards I'm ok with it, but less for a SoC dtsi, since it could lead to some confusion. Finally, yes I could have added the mali node to the GXL dtsi, but the midgard Mali dt-bindings are not upstream and the family is too big and recent enough to consider having stable bindings for now. Nevertheless, nothing is final, this gxl-mali.dtsi could be merged into the GXL dtsi in the future when we have proper dt-bindings and a real support of the T820 Mali on the S912. Kevin, what's your thought about this ? Neil > Regards, > Andreas > >> >> Signed-off-by: Neil Armstrong >> --- >> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 37 ++++++++++++++++++++ >> arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi | 43 ++++++++++++++++++++++++ >> arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi | 1 + >> arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi | 1 + >> 4 files changed, 82 insertions(+) >> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi > [...] >> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi >> index 615308e..5a90e30 100644 >> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi >> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi >> @@ -42,6 +42,7 @@ >> */ >> >> #include "meson-gxl.dtsi" >> +#include "meson-gxl-mali.dtsi" >> >> / { >> compatible = "amlogic,s905d", "amlogic,meson-gxl"; >> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi >> index 08237ee..0f78d83 100644 >> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi >> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi >> @@ -42,6 +42,7 @@ >> */ >> >> #include "meson-gxl.dtsi" >> +#include "meson-gxl-mali.dtsi" >> >> / { >> compatible = "amlogic,s905x", "amlogic,meson-gxl"; >