[v7,08/12] arm: dts: s5pv210: Add node for SGX 540
diff mbox series

Message ID 3fd18c747426e15fd1f3500b9c4adce2db9ddd0c.1587760454.git.hns@goldelico.com
State Changes Requested
Headers show
Series
  • ARM/MIPS: DTS: add child nodes describing the PVRSGX GPU present in some OMAP SoC and JZ4780 (and many more)
Related show

Commit Message

H. Nikolaus Schaller April 24, 2020, 8:34 p.m. UTC
From: Jonathan Bakker <xc-racer2@live.ca>

All s5pv210 devices have a PowerVR SGX 540 (revision 120) attached.

There is no external regulator for it so it can be enabled by default.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/s5pv210.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Paul Cercueil April 26, 2020, 12:56 p.m. UTC | #1
Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> All s5pv210 devices have a PowerVR SGX 540 (revision 120) attached.
> 
> There is no external regulator for it so it can be enabled by default.
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  arch/arm/boot/dts/s5pv210.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/s5pv210.dtsi 
> b/arch/arm/boot/dts/s5pv210.dtsi
> index 2ad642f51fd9..abbdda205c1b 100644
> --- a/arch/arm/boot/dts/s5pv210.dtsi
> +++ b/arch/arm/boot/dts/s5pv210.dtsi
> @@ -512,6 +512,19 @@ vic3: interrupt-controller@f2300000 {
>  			#interrupt-cells = <1>;
>  		};
> 
> +		gpu: gpu@f3000000 {
> +			compatible = "samsung,s5pv210-sgx540-120";
> +			reg = <0xf3000000 0x10000>;
> +			interrupt-parent = <&vic2>;
> +			interrupts = <10>;
> +			clock-names = "core";
> +			clocks = <&clocks CLK_G3D>;
> +
> +			assigned-clocks = <&clocks MOUT_G3D>, <&clocks DOUT_G3D>;
> +			assigned-clock-rates = <0>, <66700000>;
> +			assigned-clock-parents = <&clocks MOUT_MPLL>;

What are these clocks for, and why are they reparented / reclocked?

Shouldn't they be passed to 'clocks' as well?

-Paul

> +		};
> +
>  		fimd: fimd@f8000000 {
>  			compatible = "samsung,s5pv210-fimd";
>  			interrupt-parent = <&vic2>;
> --
> 2.25.1
>
Jonathan Bakker April 26, 2020, 2:57 p.m. UTC | #2
Hi Paul,

On 2020-04-26 5:56 a.m., Paul Cercueil wrote:
> 
> 
> Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>> From: Jonathan Bakker <xc-racer2@live.ca>
>>
>> All s5pv210 devices have a PowerVR SGX 540 (revision 120) attached.
>>
>> There is no external regulator for it so it can be enabled by default.
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>>  arch/arm/boot/dts/s5pv210.dtsi | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
>> index 2ad642f51fd9..abbdda205c1b 100644
>> --- a/arch/arm/boot/dts/s5pv210.dtsi
>> +++ b/arch/arm/boot/dts/s5pv210.dtsi
>> @@ -512,6 +512,19 @@ vic3: interrupt-controller@f2300000 {
>>              #interrupt-cells = <1>;
>>          };
>>
>> +        gpu: gpu@f3000000 {
>> +            compatible = "samsung,s5pv210-sgx540-120";
>> +            reg = <0xf3000000 0x10000>;
>> +            interrupt-parent = <&vic2>;
>> +            interrupts = <10>;
>> +            clock-names = "core";
>> +            clocks = <&clocks CLK_G3D>;
>> +
>> +            assigned-clocks = <&clocks MOUT_G3D>, <&clocks DOUT_G3D>;
>> +            assigned-clock-rates = <0>, <66700000>;
>> +            assigned-clock-parents = <&clocks MOUT_MPLL>;
> 
> What are these clocks for, and why are they reparented / reclocked?
> 
> Shouldn't they be passed to 'clocks' as well?
> 
> -Paul
> 

The G3D clock system can have multiple parents, and for stable operation
it's recommended to use the MPLL clock as the parent (which in turn
is actually a mux as well).  MOUT_G3D is simply the mux for CLK_G3D
(SGX core clock), DOUT_G3D is the divider.  DOUT_G3D could equally be CLK_G3D
(and probably should be, for readability) as CLK_G3D is simply the gate and
DOUT_G3D is the divider for it.

The SGX clock layout on S5PV210 looks something like this:

        MOUT_MPLL -----------> MOUT_G3D ---> DOUT_G3D  ---> CLK_G3D
(selectable parent clock)      (mux)    ---> (divider) ---> (gate)

This is fairly common for older Samsung SoCs, eg having a look at
arch/arm/boot/dts/exynos4210-universal_c210.dts you can see that
the FIMC clocks are parented to MPLL and have a rate set.

>> +        };
>> +
>>          fimd: fimd@f8000000 {
>>              compatible = "samsung,s5pv210-fimd";
>>              interrupt-parent = <&vic2>;
>> -- 
>> 2.25.1
>>
> 
> 

Thanks,
Jonathan
Krzysztof Kozlowski April 27, 2020, 3:46 p.m. UTC | #3
On Sun, Apr 26, 2020 at 07:57:12AM -0700, Jonathan Bakker wrote:
> Hi Paul,
> 
> On 2020-04-26 5:56 a.m., Paul Cercueil wrote:
> > 
> > 
> > Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
> >> From: Jonathan Bakker <xc-racer2@live.ca>
> >>
> >> All s5pv210 devices have a PowerVR SGX 540 (revision 120) attached.
> >>
> >> There is no external regulator for it so it can be enabled by default.
> >>
> >> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> >> ---
> >>  arch/arm/boot/dts/s5pv210.dtsi | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
> >> index 2ad642f51fd9..abbdda205c1b 100644
> >> --- a/arch/arm/boot/dts/s5pv210.dtsi
> >> +++ b/arch/arm/boot/dts/s5pv210.dtsi
> >> @@ -512,6 +512,19 @@ vic3: interrupt-controller@f2300000 {
> >>              #interrupt-cells = <1>;
> >>          };
> >>
> >> +        gpu: gpu@f3000000 {
> >> +            compatible = "samsung,s5pv210-sgx540-120";

This should not pass the bindings check because you missed last
compatibles.

> >> +            reg = <0xf3000000 0x10000>;
> >> +            interrupt-parent = <&vic2>;
> >> +            interrupts = <10>;
> >> +            clock-names = "core";
> >> +            clocks = <&clocks CLK_G3D>;
> >> +
> >> +            assigned-clocks = <&clocks MOUT_G3D>, <&clocks DOUT_G3D>;
> >> +            assigned-clock-rates = <0>, <66700000>;
> >> +            assigned-clock-parents = <&clocks MOUT_MPLL>;
> > 
> > What are these clocks for, and why are they reparented / reclocked?
> > 
> > Shouldn't they be passed to 'clocks' as well?
> > 
> > -Paul
> > 
> 
> The G3D clock system can have multiple parents, and for stable operation
> it's recommended to use the MPLL clock as the parent (which in turn
> is actually a mux as well).  MOUT_G3D is simply the mux for CLK_G3D
> (SGX core clock), DOUT_G3D is the divider.  DOUT_G3D could equally be CLK_G3D
> (and probably should be, for readability) as CLK_G3D is simply the gate and
> DOUT_G3D is the divider for it.

Good point, it should be CLK_G3D instead of DOUT.  Can you fix this as
well?

Best regards,
Krzysztof
Jonathan Bakker April 28, 2020, 9:39 p.m. UTC | #4
Hi Krzysztof,

On 2020-04-27 8:46 a.m., Krzysztof Kozlowski wrote:
> On Sun, Apr 26, 2020 at 07:57:12AM -0700, Jonathan Bakker wrote:
>> Hi Paul,
>>
>> On 2020-04-26 5:56 a.m., Paul Cercueil wrote:
>>>
>>>
>>> Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>>>> From: Jonathan Bakker <xc-racer2@live.ca>
>>>>
>>>> All s5pv210 devices have a PowerVR SGX 540 (revision 120) attached.
>>>>
>>>> There is no external regulator for it so it can be enabled by default.
>>>>
>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>> ---
>>>>  arch/arm/boot/dts/s5pv210.dtsi | 13 +++++++++++++
>>>>  1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
>>>> index 2ad642f51fd9..abbdda205c1b 100644
>>>> --- a/arch/arm/boot/dts/s5pv210.dtsi
>>>> +++ b/arch/arm/boot/dts/s5pv210.dtsi
>>>> @@ -512,6 +512,19 @@ vic3: interrupt-controller@f2300000 {
>>>>              #interrupt-cells = <1>;
>>>>          };
>>>>
>>>> +        gpu: gpu@f3000000 {
>>>> +            compatible = "samsung,s5pv210-sgx540-120";
> 
> This should not pass the bindings check because you missed last
> compatibles.
> 

Thanks for pointing that out, I'll add it and make sure it passes the bindings check.

>>>> +            reg = <0xf3000000 0x10000>;
>>>> +            interrupt-parent = <&vic2>;
>>>> +            interrupts = <10>;
>>>> +            clock-names = "core";
>>>> +            clocks = <&clocks CLK_G3D>;
>>>> +
>>>> +            assigned-clocks = <&clocks MOUT_G3D>, <&clocks DOUT_G3D>;
>>>> +            assigned-clock-rates = <0>, <66700000>;
>>>> +            assigned-clock-parents = <&clocks MOUT_MPLL>;
>>>
>>> What are these clocks for, and why are they reparented / reclocked?
>>>
>>> Shouldn't they be passed to 'clocks' as well?
>>>
>>> -Paul
>>>
>>
>> The G3D clock system can have multiple parents, and for stable operation
>> it's recommended to use the MPLL clock as the parent (which in turn
>> is actually a mux as well).  MOUT_G3D is simply the mux for CLK_G3D
>> (SGX core clock), DOUT_G3D is the divider.  DOUT_G3D could equally be CLK_G3D
>> (and probably should be, for readability) as CLK_G3D is simply the gate and
>> DOUT_G3D is the divider for it.
> 
> Good point, it should be CLK_G3D instead of DOUT.  Can you fix this as
> well?

Yep, will do.  Nikolaus, I'll send you an updated patch to include.

> 
> Best regards,
> Krzysztof
> 

Thanks,
Jonathan
Jonathan Bakker April 28, 2020, 10:58 p.m. UTC | #5
Hi all,

On 2020-04-28 2:39 p.m., Jonathan Bakker wrote:
> Hi Krzysztof,
> 
> On 2020-04-27 8:46 a.m., Krzysztof Kozlowski wrote:
>> On Sun, Apr 26, 2020 at 07:57:12AM -0700, Jonathan Bakker wrote:
>>> Hi Paul,
>>>
>>> On 2020-04-26 5:56 a.m., Paul Cercueil wrote:
>>>>
>>>>
>>>> Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>>>>> From: Jonathan Bakker <xc-racer2@live.ca>
>>>>>
>>>>> All s5pv210 devices have a PowerVR SGX 540 (revision 120) attached.
>>>>>
>>>>> There is no external regulator for it so it can be enabled by default.
>>>>>
>>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>>> ---
>>>>>  arch/arm/boot/dts/s5pv210.dtsi | 13 +++++++++++++
>>>>>  1 file changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
>>>>> index 2ad642f51fd9..abbdda205c1b 100644
>>>>> --- a/arch/arm/boot/dts/s5pv210.dtsi
>>>>> +++ b/arch/arm/boot/dts/s5pv210.dtsi
>>>>> @@ -512,6 +512,19 @@ vic3: interrupt-controller@f2300000 {
>>>>>              #interrupt-cells = <1>;
>>>>>          };
>>>>>
>>>>> +        gpu: gpu@f3000000 {
>>>>> +            compatible = "samsung,s5pv210-sgx540-120";
>>
>> This should not pass the bindings check because you missed last
>> compatibles.
>>
> 
> Thanks for pointing that out, I'll add it and make sure it passes the bindings check.
> 
>>>>> +            reg = <0xf3000000 0x10000>;
>>>>> +            interrupt-parent = <&vic2>;
>>>>> +            interrupts = <10>;
>>>>> +            clock-names = "core";
>>>>> +            clocks = <&clocks CLK_G3D>;
>>>>> +
>>>>> +            assigned-clocks = <&clocks MOUT_G3D>, <&clocks DOUT_G3D>;
>>>>> +            assigned-clock-rates = <0>, <66700000>;
>>>>> +            assigned-clock-parents = <&clocks MOUT_MPLL>;
>>>>
>>>> What are these clocks for, and why are they reparented / reclocked?
>>>>
>>>> Shouldn't they be passed to 'clocks' as well?
>>>>
>>>> -Paul
>>>>
>>>
>>> The G3D clock system can have multiple parents, and for stable operation
>>> it's recommended to use the MPLL clock as the parent (which in turn
>>> is actually a mux as well).  MOUT_G3D is simply the mux for CLK_G3D
>>> (SGX core clock), DOUT_G3D is the divider.  DOUT_G3D could equally be CLK_G3D
>>> (and probably should be, for readability) as CLK_G3D is simply the gate and
>>> DOUT_G3D is the divider for it.
>>
>> Good point, it should be CLK_G3D instead of DOUT.  Can you fix this as
>> well?
> 
> Yep, will do.  Nikolaus, I'll send you an updated patch to include.
> 

How are assigned-clocks handled in the yaml DT schema?  When running make dtbs_check,
I end up with messages such as

arch/arm/boot/dts/s5pv210-aquila.dt.yaml: gpu@f3000000: 'assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+'

Do they need to explicitly be listed as valid entries?

Thanks,
Jonathan
Maxime Ripard April 29, 2020, 8:47 a.m. UTC | #6
Hi,

On Tue, Apr 28, 2020 at 03:58:03PM -0700, Jonathan Bakker wrote:
> On 2020-04-28 2:39 p.m., Jonathan Bakker wrote:
> > On 2020-04-27 8:46 a.m., Krzysztof Kozlowski wrote:
> >> On Sun, Apr 26, 2020 at 07:57:12AM -0700, Jonathan Bakker wrote:
> >>> Hi Paul,
> >>>
> >>> On 2020-04-26 5:56 a.m., Paul Cercueil wrote:
> >>>>
> >>>>
> >>>> Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
> >>>>> From: Jonathan Bakker <xc-racer2@live.ca>
> >>>>>
> >>>>> All s5pv210 devices have a PowerVR SGX 540 (revision 120) attached.
> >>>>>
> >>>>> There is no external regulator for it so it can be enabled by default.
> >>>>>
> >>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> >>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> >>>>> ---
> >>>>>  arch/arm/boot/dts/s5pv210.dtsi | 13 +++++++++++++
> >>>>>  1 file changed, 13 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
> >>>>> index 2ad642f51fd9..abbdda205c1b 100644
> >>>>> --- a/arch/arm/boot/dts/s5pv210.dtsi
> >>>>> +++ b/arch/arm/boot/dts/s5pv210.dtsi
> >>>>> @@ -512,6 +512,19 @@ vic3: interrupt-controller@f2300000 {
> >>>>>              #interrupt-cells = <1>;
> >>>>>          };
> >>>>>
> >>>>> +        gpu: gpu@f3000000 {
> >>>>> +            compatible = "samsung,s5pv210-sgx540-120";
> >>
> >> This should not pass the bindings check because you missed last
> >> compatibles.
> >>
> > 
> > Thanks for pointing that out, I'll add it and make sure it passes the bindings check.
> > 
> >>>>> +            reg = <0xf3000000 0x10000>;
> >>>>> +            interrupt-parent = <&vic2>;
> >>>>> +            interrupts = <10>;
> >>>>> +            clock-names = "core";
> >>>>> +            clocks = <&clocks CLK_G3D>;
> >>>>> +
> >>>>> +            assigned-clocks = <&clocks MOUT_G3D>, <&clocks DOUT_G3D>;
> >>>>> +            assigned-clock-rates = <0>, <66700000>;
> >>>>> +            assigned-clock-parents = <&clocks MOUT_MPLL>;
> >>>>
> >>>> What are these clocks for, and why are they reparented / reclocked?
> >>>>
> >>>> Shouldn't they be passed to 'clocks' as well?
> >>>>
> >>>> -Paul
> >>>>
> >>>
> >>> The G3D clock system can have multiple parents, and for stable operation
> >>> it's recommended to use the MPLL clock as the parent (which in turn
> >>> is actually a mux as well).  MOUT_G3D is simply the mux for CLK_G3D
> >>> (SGX core clock), DOUT_G3D is the divider.  DOUT_G3D could equally be CLK_G3D
> >>> (and probably should be, for readability) as CLK_G3D is simply the gate and
> >>> DOUT_G3D is the divider for it.
> >>
> >> Good point, it should be CLK_G3D instead of DOUT.  Can you fix this as
> >> well?
> > 
> > Yep, will do.  Nikolaus, I'll send you an updated patch to include.
> > 
> 
> How are assigned-clocks handled in the yaml DT schema?  When running make dtbs_check,
> I end up with messages such as
> 
> arch/arm/boot/dts/s5pv210-aquila.dt.yaml: gpu@f3000000: 'assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> Do they need to explicitly be listed as valid entries?

If you have additionalProperties set to false, yes. But you should really
consider not using them in the first place, since they provide no guarantee on
whether the setup you did in the DT will remain during the life of the system.

I'm not sure why it's needed on that SoC in particular, but this should be fixed
in the driver itself (either the clock or the GPU driver).

Maxime
Paul Cercueil April 29, 2020, 12:26 p.m. UTC | #7
Hi Jonathan,

Le mar. 28 avril 2020 à 15:58, Jonathan Bakker <xc-racer2@live.ca> a 
écrit :
> Hi all,
> 
> On 2020-04-28 2:39 p.m., Jonathan Bakker wrote:
>>  Hi Krzysztof,
>> 
>>  On 2020-04-27 8:46 a.m., Krzysztof Kozlowski wrote:
>>>  On Sun, Apr 26, 2020 at 07:57:12AM -0700, Jonathan Bakker wrote:
>>>>  Hi Paul,
>>>> 
>>>>  On 2020-04-26 5:56 a.m., Paul Cercueil wrote:
>>>>> 
>>>>> 
>>>>>  Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller 
>>>>> <hns@goldelico.com> a écrit :
>>>>>>  From: Jonathan Bakker <xc-racer2@live.ca>
>>>>>> 
>>>>>>  All s5pv210 devices have a PowerVR SGX 540 (revision 120) 
>>>>>> attached.
>>>>>> 
>>>>>>  There is no external regulator for it so it can be enabled by 
>>>>>> default.
>>>>>> 
>>>>>>  Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>>>>  Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>>>>  ---
>>>>>>   arch/arm/boot/dts/s5pv210.dtsi | 13 +++++++++++++
>>>>>>   1 file changed, 13 insertions(+)
>>>>>> 
>>>>>>  diff --git a/arch/arm/boot/dts/s5pv210.dtsi 
>>>>>> b/arch/arm/boot/dts/s5pv210.dtsi
>>>>>>  index 2ad642f51fd9..abbdda205c1b 100644
>>>>>>  --- a/arch/arm/boot/dts/s5pv210.dtsi
>>>>>>  +++ b/arch/arm/boot/dts/s5pv210.dtsi
>>>>>>  @@ -512,6 +512,19 @@ vic3: interrupt-controller@f2300000 {
>>>>>>               #interrupt-cells = <1>;
>>>>>>           };
>>>>>> 
>>>>>>  +        gpu: gpu@f3000000 {
>>>>>>  +            compatible = "samsung,s5pv210-sgx540-120";
>>> 
>>>  This should not pass the bindings check because you missed last
>>>  compatibles.
>>> 
>> 
>>  Thanks for pointing that out, I'll add it and make sure it passes 
>> the bindings check.
>> 
>>>>>>  +            reg = <0xf3000000 0x10000>;
>>>>>>  +            interrupt-parent = <&vic2>;
>>>>>>  +            interrupts = <10>;
>>>>>>  +            clock-names = "core";
>>>>>>  +            clocks = <&clocks CLK_G3D>;
>>>>>>  +
>>>>>>  +            assigned-clocks = <&clocks MOUT_G3D>, <&clocks 
>>>>>> DOUT_G3D>;
>>>>>>  +            assigned-clock-rates = <0>, <66700000>;
>>>>>>  +            assigned-clock-parents = <&clocks MOUT_MPLL>;
>>>>> 
>>>>>  What are these clocks for, and why are they reparented / 
>>>>> reclocked?
>>>>> 
>>>>>  Shouldn't they be passed to 'clocks' as well?
>>>>> 
>>>>>  -Paul
>>>>> 
>>>> 
>>>>  The G3D clock system can have multiple parents, and for stable 
>>>> operation
>>>>  it's recommended to use the MPLL clock as the parent (which in 
>>>> turn
>>>>  is actually a mux as well).  MOUT_G3D is simply the mux for 
>>>> CLK_G3D
>>>>  (SGX core clock), DOUT_G3D is the divider.  DOUT_G3D could 
>>>> equally be CLK_G3D
>>>>  (and probably should be, for readability) as CLK_G3D is simply 
>>>> the gate and
>>>>  DOUT_G3D is the divider for it.
>>> 
>>>  Good point, it should be CLK_G3D instead of DOUT.  Can you fix 
>>> this as
>>>  well?
>> 
>>  Yep, will do.  Nikolaus, I'll send you an updated patch to include.
>> 
> 
> How are assigned-clocks handled in the yaml DT schema?  When running 
> make dtbs_check,
> I end up with messages such as
> 
> arch/arm/boot/dts/s5pv210-aquila.dt.yaml: gpu@f3000000: 
> 'assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' 
> do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> Do they need to explicitly be listed as valid entries?

The assigned-* can also be moved inside the node of the clocks 
provider. I would say it makes more sense to have them there.

-Paul

Patch
diff mbox series

diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
index 2ad642f51fd9..abbdda205c1b 100644
--- a/arch/arm/boot/dts/s5pv210.dtsi
+++ b/arch/arm/boot/dts/s5pv210.dtsi
@@ -512,6 +512,19 @@  vic3: interrupt-controller@f2300000 {
 			#interrupt-cells = <1>;
 		};
 
+		gpu: gpu@f3000000 {
+			compatible = "samsung,s5pv210-sgx540-120";
+			reg = <0xf3000000 0x10000>;
+			interrupt-parent = <&vic2>;
+			interrupts = <10>;
+			clock-names = "core";
+			clocks = <&clocks CLK_G3D>;
+
+			assigned-clocks = <&clocks MOUT_G3D>, <&clocks DOUT_G3D>;
+			assigned-clock-rates = <0>, <66700000>;
+			assigned-clock-parents = <&clocks MOUT_MPLL>;
+		};
+
 		fimd: fimd@f8000000 {
 			compatible = "samsung,s5pv210-fimd";
 			interrupt-parent = <&vic2>;