Hi Linus, Thank you for your feedback (comments below). On Fri, 2023-08-18 at 11:36 +0200, Linus Walleij wrote: > Hi Sarah, > > thanks for your patch! > > Patches adding device tree bindings need to be CC:ed to > devicetree@xxxxxxxxxxxxxxx > and the DT binding maintainers, I have added it for now. Thank you - it looks like something went wrong when the patch was sent. > > On Wed, Aug 16, 2023 at 10:26 AM Sarah Walker <sarah.walker@xxxxxxxxxx> wrote: > > > Add the device tree binding documentation for the Series AXE GPU used in > > TI AM62 SoCs. > > > > Co-developed-by: Frank Binns <frank.binns@xxxxxxxxxx> > > Signed-off-by: Frank Binns <frank.binns@xxxxxxxxxx> > > Signed-off-by: Sarah Walker <sarah.walker@xxxxxxxxxx> > (...) > > +properties: > > + compatible: > > + items: > > + - enum: > > + - ti,am62-gpu > > + - const: img,powervr-seriesaxe > > Should there not at least be a dash there? > > img,powervr-series-axe? > > It is spelled in two words in the commit message, > Series AXE not SeriesAXE? We've now changed the string to address your earlier feedback (see below). > > Moreover, if this pertains to the AXE-1-16 and AXE-2-16 it is kind of a wildcard > and we usually don't do that, I would use the exact version instead, > such as: > const: img,powervr-axe-1-16 > any reason not to do this? The exact GPU model/revision is fully discoverable via a register. We saw the same is also true for Mali Bifrost, where they have a single string covering multiple models [1], so we took the same approach. We'll add a comment in v6 along the lines of the one in the Mali Bifrost bindings. > > I asked about the relationship between these strings and the product > designations earlier I think :/ Sorry about that, I honestly thought we'd addressed that bit of feedback by changing the compatibility string, but clearly we hadn't :( Thank you for catching this. We've now changed the string to "img,img-axe" to align with the marketing name, along with updating the commit message and various other places to refer to PowerVR and IMG GPUs (the DRM driver supporting both). Thanks Frank [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml?h=v6.5#n29
