Re: [RFC 4/5] media: i2c: max9286: Fetch PIXEL_RATE in s_stream

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jacopo,

On Mon, Aug 23, 2021 at 09:20:08AM +0200, Jacopo Mondi wrote:
> On Mon, Aug 23, 2021 at 05:17:05AM +0300, Laurent Pinchart wrote:
> > On Tue, Aug 17, 2021 at 09:27:02AM +0200, Jacopo Mondi wrote:
> > > The max9286 driver needs to fetch the remote serializer PIXEL_RATE
> > > control value in order to compute its own one, as the sum of the
> >
> > s/its own one/its own/ (or its own pixel rate)
> >
> > > values reported by the connected subdevices.
> > >
> > > Currently the control is verified to be present at notifier's bound
> > > time, which requires the serializer driver to register the control at
> > > probe time. As the serializer driver might need to register the control
> > > later, by adding the control handler of its connected sensor, post-pone
> > > the max9286 check for the control availability at start stream time.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  drivers/media/i2c/max9286.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > > index 1b92d18a1f94..98fc90339a9e 100644
> > > --- a/drivers/media/i2c/max9286.c
> > > +++ b/drivers/media/i2c/max9286.c
> > > @@ -595,7 +595,7 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> > >  	max9286_check_config_link(priv, priv->source_mask);
> > >  	max9286_configure_i2c(priv, false);
> > >
> > > -	return max9286_set_pixelrate(priv);
> > > +	return 0;
> > >  }
> > >
> > >  static void max9286_notify_unbind(struct v4l2_async_notifier *notifier,
> > > @@ -674,6 +674,10 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
> > >  	int ret;
> > >
> > >  	if (enable) {
> > > +		ret = max9286_set_pixelrate(priv);
> > > +		if (ret)
> > > +			return ret;
> >
> > That's very likely not going to work. The CSI-2 receiver connected to
> > the max9286 will need to know the pixel rate as part of its
> > initialization sequence, before calling .s_stream(1) on the max9286.
> 
> How so ? The R-Car CSI-2 receiver feteches the pixel rate at s_stream
> time, I thought it was customary to do so. What is it needed for before
> streamon time ?

At stream on time is usually fine, but the CSI-2 receiver usually needs
to be configured before starting the source, not after.

> > > +
> > >  		/*
> > >  		 * The frame sync between cameras is transmitted across the
> > >  		 * reverse channel as GPIO. We must open all channels while

-- 
Regards,

Laurent Pinchart