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@xxxxxx>:
>
> Add SGX GPU device entry to base OMAP4 dtsi file.
>
> Signed-off-by: Andrew Davis <afd@xxxxxx>
> ---
> 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@56000000 {
> #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
