Hi Sarah, thanks for your patch! On Tue, Jun 13, 2023 at 5:20 PM Sarah Walker <sarah.walker@xxxxxxxxxx> wrote: > Add the device tree binding documentation for the Series AXE GPU used in > TI AM62 SoCs. > > Signed-off-by: Sarah Walker <sarah.walker@xxxxxxxxxx> > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - ti,am62-gpu > + - const: img,powervr-seriesaxe I don't see why you need to restrict the bindings to just the stuff you happen to be writing Linux drivers for right now. Add all of them! There is this out-of-tree binding: https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/commit/0ddd308a78926782b8a72f75c74b91417ceb9779 With these two on top: https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/commit/053346e1933932815066f16ebf6e6bda45d67548 https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/commit/1cb62c4cdcad096d438ee7d1d51f6001998acee3 They are indeed out-of-tree, but H. Nikolaus is a well-known and respected contributor to the kernel, and I'm pretty sure he would be contributing these upstream if he had the time and incentive. To me it seems much more like you should talk to Nikolaus about submitting these patches initially, and then add Rogue support with patches on top of it. It could be a nice series with just DT bindings. I see that your binding uses a power domain rather than a regulator (sgx-supply) for power and that is probably a better approach but other than that the openpvrsgx binding looks more complete and to the point? It will also help them to get these bindings settled so they can merge all of the DTS patches adding the SGX block to misc platforms in the kernel upstream so they can focus their work on the actual driver. When I look at the PowerVR wikipedia page: https://en.wikipedia.org/wiki/PowerVR there is no correspondence between the product names there and "img,powervr-seriesaxe" as used here. I think you need to explain if these are internal product names or what is going on, and what the relationship is to the marketing names, it could be part of the description simply, so that people know what string to use somewhat intuitively. Maybe all the strings in the out-of-tree documentation are just wrong because internal code names need to be used? Yours, Linus Walleij
