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

[H. Nikolaus Schaller]

Hi,

> Am 15.01.2024 um 09:25 schrieb Maxime Ripard <mripard at kernel.org>:
> 
> Hi,
> 
> On Fri, Jan 12, 2024 at 06:33:58PM +0100, H. Nikolaus Schaller wrote:
>>> Am 08.01.2024 um 19:32 schrieb Andrew Davis <afd at ti.com>:
>>> 
>>> Add SGX GPU device entry to base OMAP4 dtsi file.
>>> 
>>> Signed-off-by: Andrew Davis <afd at ti.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 at 56000000 {
>>> #size-cells = <1>;
>>> ranges = <0 0x56000000 0x2000000>;
>>> 
>>> - /*
>>> - * Closed source PowerVR driver, no child device
>>> - * binding or driver in mainline
>>> - */
>>> + gpu at 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).
> 
> There's no reason to disable it in the DT: the hardware block would
> still be there and it's rendering to memory so it still could be useful.

Well, if you know that the board does not have a dm3730 but a dm3725 without
GPU it is better to disable the GPU completely instead of loading the driver
and make it detect by some internal bits that it has no GPU on the SoC.

> If there's no display on the board and you really don't want the GPU
> driver, then you can disable the driver or block the module loading, but
> it should be a distro / package / user decision, not a DT / kernel one
> still.

The same holds for aes: dss: dsi: hdmi: etc. If they are not used by some
board file, they don't change a single bit of the DTB [1] which IMHO would
be of reasonable concern to question additional labels.

BR and thanks,
Nikolaus

[1] https://devicetree-specification.readthedocs.io/en/stable/source-language.html
"Labels are only used in the devicetree source format and are not encoded into the
DTB binary."