linux-kernel - Re: [PATCH RFC v2 04/11] ARM: dts: omap4: Add device tree entry for SGX GPU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 12 Jan 2024 18:33:58 +0100
From: H. Nikolaus Schaller <hns@...delico.com>
To: Andrew Davis <afd@...com>
Cc: Frank Binns <frank.binns@...tec.com>,
 Donald Robson <donald.robson@...tec.com>,
 Matt Coster <matt.coster@...tec.com>,
 Adam Ford <aford173@...il.com>,
 Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Chen-Yu Tsai <wens@...e.org>,
 Jernej Skrabec <jernej.skrabec@...il.com>,
 Samuel Holland <samuel@...lland.org>,
 Benoît Cousson <bcousson@...libre.com>,
 Tony Lindgren <tony@...mide.com>,
 Nishanth Menon <nm@...com>,
 Vignesh Raghavendra <vigneshr@...com>,
 Tero Kristo <kristo@...nel.org>,
 Paul Cercueil <paul@...pouillou.net>,
 dri-devel@...ts.freedesktop.org,
 devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org,
 linux-sunxi@...ts.linux.dev,
 linux-omap@...r.kernel.org,
 linux-mips@...r.kernel.org
Subject: Re: [PATCH RFC v2 04/11] ARM: dts: omap4: Add device tree entry for
 SGX GPU

Hi,
I just comment on this example, but it applies almost the same for all other .dtsi changes.

> Am 08.01.2024 um 19:32 schrieb Andrew Davis <afd@...com>:
> 
> Add SGX GPU device entry to base OMAP4 dtsi file.
> 
> Signed-off-by: Andrew Davis <afd@...com>
> ---
> arch/arm/boot/dts/ti/omap/omap4.dtsi | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/ti/omap/omap4.dtsi b/arch/arm/boot/dts/ti/omap/omap4.dtsi
> index 2bbff9032be3e..559b2bfe4ca7c 100644
> --- a/arch/arm/boot/dts/ti/omap/omap4.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/omap4.dtsi
> @@ -501,10 +501,11 @@ sgx_module: target-module@...00000 {
> #size-cells = <1>;
> ranges = <0 0x56000000 0x2000000>;
> 
> - /*
> - * Closed source PowerVR driver, no child device
> - * binding or driver in mainline
> - */
> + gpu@0 {

I wonder why we don't add a "gpu:" label here.

Almost all other subsystem nodes have one (e.g. emif:, aes:, dss:, dsi:, hdmi:, etc.),
obviously for convenience when using a .dtsi file.

It would allow a board-specific DTS to easily add status = "disabled" to avoid driver
probing or disabling the GPU (e.g. if there is no display).

> + compatible = "ti,omap4430-gpu", "img,powervr-sgx540";

It still appears to me that the "img,powervr-sgx540" (or similar) entry is redundant
information.

I have experimentally updated our openpvrsgx driver and we do not have any use for
this information (at least in the kernel driver):

https://github.com/goldelico/letux-kernel/commit/f2f7cb3b858ef255f52f2b82a8bb34c047337afe

It shows how easy it is to derive the sgx version and revision number if we ever
need it inside the driver.

So if you want to keep a reference to powervr, it would suffice to have

> + compatible = "ti,omap4430-gpu", "img,powervr-sgx";

Otherwise your device tree entries compile fine and seem to work (at least in
a cursory test on PandaBoard ES).

> + reg = <0x0 0x2000000>; /* 32MB */
> + interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> + };
> };

BR and thanks,
Nikolaus