linux-kernel - Re: [PATCH RFC 01/10] dt-bindings: gpu: Add PowerVR Series5 SGX GPUs

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]

Message-Id: <D8AB6CC4-DCA5-40DD-A311-94A16FF59254@goldelico.com>
Date:   Thu, 7 Dec 2023 11:33:53 +0100
From:   "H. Nikolaus Schaller" <hns@...delico.com>
To:     Maxime Ripard <mripard@...nel.org>
Cc:     Andrew Davis <afd@...com>, 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>,
        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 01/10] dt-bindings: gpu: Add PowerVR Series5 SGX GPUs

Hi Maxime,

> Am 07.12.2023 um 10:20 schrieb Maxime Ripard <mripard@...nel.org>:
> 
> On Tue, Dec 05, 2023 at 02:50:08PM +0100, H. Nikolaus Schaller wrote:
>> Hi,
>> 
>>> Am 05.12.2023 um 14:29 schrieb Maxime Ripard <mripard@...nel.org>:
>>> 
>>> Hi,
>>> 
>>> On Tue, Dec 05, 2023 at 09:18:58AM +0100, H. Nikolaus Schaller wrote:
>>>>> Am 05.12.2023 um 07:57 schrieb Maxime Ripard <mripard@...nel.org>:
>>>>> 
>>>>> On Mon, Dec 04, 2023 at 12:22:36PM -0600, Andrew Davis wrote:
>>>>>> The Imagination PowerVR Series5 "SGX" GPU is part of several SoCs from
>>>>>> multiple vendors. Describe how the SGX GPU is integrated in these SoC,
>>>>>> including register space and interrupts. Clocks, reset, and power domain
>>>>>> information is SoC specific.
>>>>>> 
>>>>>> Signed-off-by: Andrew Davis <afd@...com>
>>>>>> ---
>>>>>> .../devicetree/bindings/gpu/img,powervr.yaml  | 69 +++++++++++++++++--
>>>>>> 1 file changed, 63 insertions(+), 6 deletions(-)
>>>>> 
>>>>> I think it would be best to have a separate file for this, img,sgx.yaml
>>>>> maybe?
>>>> 
>>>> Why?
>>> 
>>> Because it's more convenient?
>> 
>> Is it?
> 
> It's for a separate architecture, with a separate driver, maintained out
> of tree by a separate community, with a separate set of requirements as
> evidenced by the other thread. And that's all fine in itself, but
> there's very little reason to put these two bindings in the same file.
> 
> We could also turn this around, why is it important that it's in the
> same file?

Same vendor. And enough similarity in architectures, even a logical sequence
of development of versions (SGX = Version 5, Rogue = Version 6+) behind.
(SGX and Rogue seem to be just trade names for their architecture development).

AFAIK bindings should describe hardware and not communities or drivers
or who is currently maintaining it. The latter can change, the first not.

> 
>>>> The whole family of IMG GPUs is PowerVR and SGX and Rogue are generations 5 and 6++:
>>>> 
>>>> https://en.wikipedia.org/wiki/PowerVR
>>> 
>>> That's not really relevant as far as bindings go.
>> 
>> But maybe for choosing binding file names. Well they are machine readable
>> but sometimes humans work with them.
> 
> Heh. It's something that can also be easily grepped,

Yes, arbitrarily introduced confusion can always be resolved by search engines
and makes them necessary and more and more advanced :)

> and the name is
> never going to reflect all the compatibles in a binding so it's what
> you'll end up doing anyway. But feel free to suggest another name to
> avoid the confusion.

Well,

1. rename img,powervr.yaml => img,powervr-rogue.yaml
2. new file img,powervr-sgx.yaml

to have at least a systematic approach here.

>>> We have multiple
>>> binding files for devices of the same generation, or single bindings
>>> covering multiple generations.
>>> 
>>> The important part is that every compatible is documented. It doesn't
>>> really matter how or where.
>> 
>> Yes, and that is why I would find it more convenient to have a single
>> "img,powervr.yaml" for all variations unless it becomes filled with
>> unrelated stuff (which isn't as far as I see).
> 
> Again, hard disagree there.

I am fine with that. I just advocate to follow the KISS principle.

Same vendor, similar purpose, similar architecture => single bindings file
as Andrew proposed.

BR,
Nikolaus