Re: [PATCH v6 08/12] arm: dts: s5pv210: Add G3D node

From: Krzysztof Kozlowski
Date: Wed Apr 15 2020 - 09:44:30 EST


On Wed, Apr 15, 2020 at 02:50:31PM +0200, H. Nikolaus Schaller wrote:
>
> > Am 15.04.2020 um 13:49 schrieb Krzysztof Kozlowski <krzk@xxxxxxxxxx>:
> >
> > On Wed, 15 Apr 2020 at 10:36, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote:
> >>
> >> From: Jonathan Bakker <xc-racer2@xxxxxxx>
> >>
> >> to add support for SGX540 GPU.
> >
> > Do not continue the subject in commit msg like it is one sentence.
> > These are two separate sentences, so commit msg starts with capital
> > letter and it is sentence by itself.
> >
> >> Signed-off-by: Jonathan Bakker <xc-racer2@xxxxxxx>
> >> Signed-off-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
> >> ---
> >> arch/arm/boot/dts/s5pv210.dtsi | 15 +++++++++++++++
> >> 1 file changed, 15 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
> >> index 2ad642f51fd9..e7fc709c0cca 100644
> >> --- a/arch/arm/boot/dts/s5pv210.dtsi
> >> +++ b/arch/arm/boot/dts/s5pv210.dtsi
> >> @@ -512,6 +512,21 @@ vic3: interrupt-controller@f2300000 {
> >> #interrupt-cells = <1>;
> >> };
> >>
> >> + g3d: g3d@f3000000 {
> >> + compatible = "samsung,s5pv210-sgx540-120";
> >> + reg = <0xf3000000 0x10000>;
> >> + interrupt-parent = <&vic2>;
> >> + interrupts = <10>;
> >> + clock-names = "sclk";
> >> + clocks = <&clocks CLK_G3D>;
> >
> > Not part of bindings, please remove or add to the bindings.
>
> Well, the bindings should describe what is common for all SoC
> and they are quite different in what they need in addition.
>
> Thererfore we have no "additionalProperties: false" in the
> bindings [PATCH v6 01/12].

If these properties are needed for Exynos-specific implementation, they
should be in the bindings. If they are not needed, they should not be
here.

Take a look at midgard bindings for example. This is a nice starting
point for these here.

Best regards,
Krzysztof