Re: [PATCH 2/4] dt-bindings: media: Add bindings for raspberrypi,rp1-cfe

[Naushir Patuck]

On Tue, 19 Mar 2024 at 17:05, Tomi Valkeinen
<tomi.valkeinen@xxxxxxxxxxxxxxxx> wrote:
>
> On 19/03/2024 17:32, Naushir Patuck wrote:
> > On Tue, 19 Mar 2024 at 14:03, Tomi Valkeinen
> > <tomi.valkeinen@xxxxxxxxxxxxxxxx> wrote:
> >>
> >> On 19/03/2024 15:05, Naushir Patuck wrote:
> >>> On Tue, 19 Mar 2024 at 13:02, Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >>>>
> >>>> On 19/03/2024 13:57, Naushir Patuck wrote:
> >>>>>>>>
> >>>>>>>> See writing bindings. Compatibles should be SoC specific. In some cases
> >>>>>>>> generic fallbacks make sense, in some note. But don't just choose
> >>>>>>>> "generic fallback" because you want. Provide rationale.
> >>>>>>>
> >>>>>>> If the compatible is SoC specific, I suppose "raspberrypi,rp1-cfe"
> >>>>>>> would be the correct string.
> >>>>>>
> >>>>>> Sure, but then please think what if rp1 is on Rpi6, called exactly the
> >>>>>> same (rp1), with some minor differences? Could it be?
> >>>>>
> >>>>> Yes, this is definitely possible.  In such cases, I would expect the
> >>>>> hardware to have a version register that would be queried by the
> >>>>> driver to adjust for minor differences, and the compatible string
> >>>>> remains the same.  Does that seem reasonable?
> >>>>
> >>>> The "would expect" is concerning. The register(s) must be there already,
> >>>> with proper value.
> >>>>
> >>>
> >>> A version register already exists in the current hardware, so we will
> >>> update it to identify future hardware revisions.
> >>
> >> But that's a version register for the FE block, not for the whole
> >> module, right? Are you suggesting that you'll make sure the FE version
> >> will be changed every time anything in the bigger CFE block is changed,
> >> and thus the FE version would also reflect the whole CFE version?
> >
> > Yes, we will update the FE versioning when either CSI2 / FE blocks are updated.
> >
> >>
> >> Can there be versions without the FE block, with just the CSI-2 parts?
> >
> > There is no version register just in the CSI2 block in isolation, so
> > this is not possible.
>
> I meant could there be a future RPx with only the CSI-2 parts on it, no
> FE? In which case we would not have any register for the version. But
> then, that would be a rather big change, so a different compatible
> string would be in order.
>
> So, while it's not exactly a perfect version register, I think it will
> work fine, assuming the HW people will actually increase the version
> also for changes outside FE.
>
> >>
> >> Also, I'm still wondering about the RP1 part there in the compatible
> >> string. Is it necessary? The CFE is located in the RP1 co-processor, but
> >> is that relevant?
> >>
> >> Is there a versioning for the whole RP1 chip? Maybe it's going to the
> >> wrong direction if we use the board/SoC for this compatible name, as
> >> it's actually the RP1 where the CFE is located in, not the SoC.
> >>
> >
> > I don't really know the conversion required to answer this one.
> > Logically CFE is on RP1, so it makes sense to me to have "rp1" in the
> > string, but I will follow the judgment of the maintainers.
>
> Well, my thinking here was that if we have a register from which to read
> the version, and Raspberry Pi would create a new co-processor, RP2, with
> the same CFE. Would we then have "raspberrypi,rp1-cfe" and
> "raspberrypi,rp2-cfe", even if there are no changes? Or would a plain
> "raspberrypi,cfe" do for both?
>
> In other words, if we don't need the "rp1" for versioning purposes,
> should it then be dropped?

I agree with the above, you've convinced me that "raspberrypi,cfe"
might be the more appropriate string, or a convincing argument for
that to be a fallback string.

Naush

>
> On the other hand, maybe it is safer to just keep the "rp1" there anyway...
>
>   Tomi
>