Re: [PATCH 6/8] media: dt-bindings: Add bindings for Raspberry Pi PiSP Back End

[Rob Herring <robh@kernel.org>]

On Tue, Feb 13, 2024 at 12:44:05PM +0200, Laurent Pinchart wrote:
> On Tue, Feb 13, 2024 at 08:35:39AM +0000, Naushir Patuck wrote:
> > On Tue, 13 Feb 2024 at 07:28, Tomi Valkeinen wrote:
> > > On 12/02/2024 11:05, Krzysztof Kozlowski wrote:
> > > > On 12/02/2024 09:50, Jacopo Mondi wrote:
> > > >
> > > >>>> +properties:
> > > >>>> +  compatible:
> > > >>>> +    const: raspberrypi,pispbe
> > > >>>
> > > >>> Nothing more specific? No model name, no version? It's quite generic
> > > >>> compatible which in general should not be allowed. I would assume that
> > > >>> at least version of Pi could denote some sort of a model... unless
> > > >>> version is detectable?
> > > >>
> > > >> The driver matches on a version register and that should be enough to
> > > >> handle quirks which are specific to an IP revision in the driver
> > > >> itself.
> > > >>
> > > >> Considering how minimal the integration with the SoC is (one clock, one
> > > >> interrupt and one optional iommu reference) even if we'll get future
> > > >> revisions of the SoC I don't think there will be any need to match on
> > > >> a dedicated compatible for bindings-validation purposes.
> > > >>
> > > >> However I understand that to be future-proof it's good practice to
> > > >> allow a more flexible scheme, so we can have a generic fallback and a
> > > >> revision-specific entry.
> > > >>
> > > >> Would
> > > >>
> > > >>    compatible:
> > > >>      items:
> > > >>        - enum:
> > > >>          - raspberrypi,pipspbe-bcm2712
> > > >
> > > > bcm2712 is manufactured by Broadcom, not Raspberry Pi, so it should be
> > > > rather Pi model?
> > >
> > > Indeed, this is something I don't get. If the BE is in the bcm2712, is
> > > it not a broadcom IP? Why is raspberrypi in the compatible name at all?
> > >
> > > Naush, Dave?
> > 
> > The Backend (and Frontend) IP are both owned solely by Raspberry Pi,
> > and the BE is instantiated on the BCM2712.  So I think "raspberry" in
> > the compatible string is correct here.
> 
> Following what we do with other SoCs, we could have
> 
> 	compatible = "brcm,pispbe-bcm2712", "raspberrypi,pispbe";

Nit: brcm,bcm7212-pispbe

Otherwise looks fine to me.

Rob