Re: [PATCH v9 1/3] media: dt-bindings: media: add bindings for Rockchip CIF

[Paul Kocialkowski]

Hi Conor,

On Wed 01 Nov 23, 15:44, Conor Dooley wrote:
> On Tue, Oct 31, 2023 at 10:51:07AM +0100, Paul Kocialkowski wrote:
> > Hi Rob,
> > 
> > On Mon 30 Oct 23, 14:32, Rob Herring wrote:
> > > On Mon, Oct 30, 2023 at 01:25:12PM +0100, Mehdi Djait wrote:
> > > > Add a documentation for the Rockchip Camera Interface
> > > > binding.
> > > > 
> > > > Signed-off-by: Mehdi Djait <mehdi.djait@xxxxxxxxxxx>
> > > > ---
> > > > v8=>v9:
> > > > dropped the "Reviewed-by: Rob Herring <robh@xxxxxxxxxx>"
> > > > because of the following changes:
> > > > - changed the compatible to rk3066-cif: rk3066 is the earliest Rockchip SoC
> > > >   that uses cif and it is the first model starting the RK30 lineup.
> > > 
> > > Is px30 compatible with rk3066? It's not clear because you didn't add 
> > > rk3066 support. If not compatible, then add rk3066 when you have a user. 
> > > If it is compatible, then you should have a fallback for px30.
> > 
> > Just to clarify here: we haven't checked that rk3066 uses the exact same
> > programming model as px30 (so there should be no fallback compatible), but it
> > is definitely the same unit (in a different version).
> 
> If you aren't sure about the programming model, are you sure about the
> number of clocks, resets etc that it has? Same IP in a different SoC
> could be integrated differently, right?

Yes absolutely there might be differences in these areas. But we can always add
conditionals in the binding to take care of different cases later.

My main concern is to avoid giving px30 too much importance just because it
happens to be the first platform supported by the Linux driver.

Cheers,

Paul

> > Since the yaml binding document will apply to all generations of the unit,
> > the name of the file should be the first generation (rk3066) instead of the
> > px30 which is just one of the many iterations of the unit.
> > 
> > It would be both confusing and irrelevant to pick px30 just because it happens
> > to be the first generation supported in the Linux driver (and described in
> > the binding).



-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: PGP signature