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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tomi,

On Tue, 13 Feb 2024 at 07:28, Tomi Valkeinen
<tomi.valkeinen@xxxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> 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.

>
> >>        - 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,
Naush

>
>   Tomi
>




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux