Re: [PATCH v5 04/11] media: bcm2835-unicam: Add support for CCP2/CSI2 camera interface

[Sakari Ailus]

On Mon, Jul 03, 2023 at 01:28:03AM +0300, Laurent Pinchart wrote:
> On Sun, Jul 02, 2023 at 10:20:29PM +0000, Sakari Ailus wrote:
> > On Mon, Jul 03, 2023 at 01:01:38AM +0300, Laurent Pinchart wrote:
> > > > > > > > > If the hardware doesn't support lane remapping for CCP2, then that should
> > > > > > > > > be reflected in DT bindings, i.e. data-lanes isn't relevant. There's no
> > > > > > > > > need to check that here.
> > > > > > > > 
> > > > > > > > Should the above check for CSI-2 be dropped as well then ?
> > > > > > > 
> > > > > > > Same for CSI-2, too: if there's nothing to configure there (lane remapping)
> > > > > > > there's no need to validate that part of the DT either.
> > > > > > 
> > > > > > OK, I'll drop that.
> > > > > 
> > > > > Actually, I'm wondering if it would make sense to tell the parsing
> > > > > functions whether lane reordering is supported or not. The checks could
> > > > > then be moved to the framework. What do you think ?
> > > > 
> > > > I'm not sure how useful this check would be in the first place: if you have
> > > > hardware that can reorder the lanes, the framework doesn't know what to
> > > > check there (if anything) and otherwise there's little point in the
> > > > entire check.
> > > 
> > > Isn't it good to tell users that something is wrong instead of accepting
> > > the invalid configuration and let them wonder why the device isn't
> > > working ? Users in this case would be system integrators, not end
> > > users, but we have lots of debugging information in the kernel aimed for
> > > them already.
> > 
> > In which of the two cases above the framework could do something useful
> > there? For devices where you can reorder the lanes or for those where you
> > can't?
> 
> For devices where you can't, to detect DT that reorders lanes.

I'd suggest to ignore that. The data-lanes property is only used to tell
the number of lanes in that case, even bindings just specify the number of
entries in general.

The parser notifies about that, though, if the numbers clash, using a debug
messages --- since it's not an error (or even worth warning about).

-- 
Sakari Ailus