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";
However, that scheme is mostly used when SoC vendor license IP cores
from third parties. Here, while the SoC is indeed manufactured by
Broadcom, it's a Raspberry Pi-specific SoC.
I don't have a personal preference.
> > >> - const: raspberrypi,pispbe
> > >>
> > >> do in this case ?
> > >>
> > >> Also, let's see what RPi think as they are certainly more informed
> > >> than me on what a good revision-specific match could be
> > >
> > > I am fine with auto-detection, though.
> > >
> > > ...
> > >
> > >>>> +
> > >>>> +examples:
> > >>>> + - |
> > >>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > >>>> +
> > >>>> + rpi1 {
> > >>>
> > >>> soc {
> > >>>
> > >>
> > >> Are you sure ? This will only ever live in the 'rp1' node.
> > >
> > > What is "rp1" node? Does not look like a generic name.
> >
> > I don't think this is right. RP1 is a separate chip, an IO controller,
> > on raspberrypi 5. BE is not in the RP1.
>
> Oops, missed this. Yes, I think it should be the "soc" node.
--
Regards,
Laurent Pinchart
