Linux-OMAP Archive on lore.kernel.org
 help / color / Atom feed
From: Maxime Ripard <maxime@cerno.tech>
To: Jonathan Bakker <xc-racer2@live.ca>
Cc: "Krzysztof Kozlowski" <krzk@kernel.org>,
	"Paul Cercueil" <paul@crapouillou.net>,
	"H. Nikolaus Schaller" <hns@goldelico.com>,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Benoît Cousson" <bcousson@baylibre.com>,
	"Tony Lindgren" <tony@atomide.com>,
	"Ralf Baechle" <ralf@linux-mips.org>,
	"Paul Burton" <paulburton@kernel.org>,
	"James Hogan" <jhogan@kernel.org>,
	"Kukjin Kim" <kgene@kernel.org>, "Chen-Yu Tsai" <wens@csie.org>,
	"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
	"Philipp Rossak" <embed3d@gmail.com>,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	openpvrsgx-devgroup@letux.org, letux-kernel@openphoenux.org,
	kernel@pyra-handheld.com, linux-mips@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org
Subject: Re: [PATCH v7 08/12] arm: dts: s5pv210: Add node for SGX 540
Date: Wed, 29 Apr 2020 10:47:23 +0200
Message-ID: <20200429084723.jxybvsakniinvivt@gilmour.lan> (raw)
In-Reply-To: <BN6PR04MB0660044B5B1D45BE4CBCD2AAA3AC0@BN6PR04MB0660.namprd04.prod.outlook.com>


[-- Attachment #1: Type: text/plain, Size: 3614 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-24 20:34 [PATCH v7 00/12] ARM/MIPS: DTS: add child nodes describing the PVRSGX GPU present in some OMAP SoC and JZ4780 (and many more) H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs H. Nikolaus Schaller
2020-04-24 20:43   ` H. Nikolaus Schaller
2020-04-26 13:11   ` Paul Cercueil
2020-05-02 20:26     ` H. Nikolaus Schaller
2020-05-03 12:52       ` Paul Cercueil
2020-05-03 13:31         ` H. Nikolaus Schaller
2020-05-03 14:18           ` Paul Cercueil
2020-05-03 15:01             ` Tony Lindgren
2020-05-15  7:58               ` H. Nikolaus Schaller
2020-05-03 16:41             ` H. Nikolaus Schaller
2020-05-15  7:18               ` H. Nikolaus Schaller
2020-04-26 19:36   ` Philipp Rossak
2020-04-26 20:59     ` H. Nikolaus Schaller
2020-05-05 15:53   ` Rob Herring
2020-05-15  7:59     ` H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 02/12] ARM: DTS: am33xx: add sgx gpu child node H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 03/12] ARM: DTS: am3517: " H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 04/12] ARM: DTS: omap34xx: " H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 05/12] ARM: DTS: omap36xx: " H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 06/12] ARM: DTS: omap4: " H. Nikolaus Schaller
2020-04-26 12:50   ` Paul Cercueil
2020-04-28  7:59     ` H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 07/12] ARM: DTS: omap5: " H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 08/12] arm: dts: s5pv210: Add node for SGX 540 H. Nikolaus Schaller
2020-04-26 12:56   ` Paul Cercueil
2020-04-26 14:57     ` Jonathan Bakker
2020-04-27 15:46       ` Krzysztof Kozlowski
2020-04-28 21:39         ` Jonathan Bakker
2020-04-28 22:58           ` Jonathan Bakker
2020-04-29  8:47             ` Maxime Ripard [this message]
2020-04-29 12:26             ` Paul Cercueil
2020-04-24 20:34 ` [PATCH v7 09/12] ARM: dts: sun6i: a31: add sgx gpu child node H. Nikolaus Schaller
2020-04-26 12:53   ` Paul Cercueil
2020-04-26 19:31     ` Philipp Rossak
2020-04-24 20:34 ` [PATCH v7 10/12] ARM: dts: sun6i: a31s: " H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 11/12] ARM: dts: sun8i: a83t: " H. Nikolaus Schaller
2020-04-24 20:34 ` [PATCH v7 12/12] MIPS: DTS: jz4780: add sgx gpu node H. Nikolaus Schaller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429084723.jxybvsakniinvivt@gilmour.lan \
    --to=maxime@cerno.tech \
    --cc=airlied@linux.ie \
    --cc=bcousson@baylibre.com \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=embed3d@gmail.com \
    --cc=hns@goldelico.com \
    --cc=jhogan@kernel.org \
    --cc=kernel@pyra-handheld.com \
    --cc=kgene@kernel.org \
    --cc=krzk@kernel.org \
    --cc=letux-kernel@openphoenux.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=openpvrsgx-devgroup@letux.org \
    --cc=paul@crapouillou.net \
    --cc=paulburton@kernel.org \
    --cc=ralf@linux-mips.org \
    --cc=robh+dt@kernel.org \
    --cc=tony@atomide.com \
    --cc=tsbogend@alpha.franken.de \
    --cc=wens@csie.org \
    --cc=xc-racer2@live.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-OMAP Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-omap/0 linux-omap/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-omap linux-omap/ https://lore.kernel.org/linux-omap \
		linux-omap@vger.kernel.org
	public-inbox-index linux-omap

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-omap


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git