Re: [PATCH] [RFC] media: rcar-vin: don't wait for stop state on clock lane during start of CSI2

From: Jacopo Mondi <jacopo@jmondi.org>
To: Michael Rodin <mrodin@de.adit-jv.com>
Cc: niklas.soderlund@ragnatech.se, mchehab@kernel.org,
	p.zabel@pengutronix.de, linux-media@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	efriedrich@de.adit-jv.com, erosca@de.adit-jv.com,
	sudipi@jp.adit-jv.com, akiyama@nds-osk.co.jp
Subject: Re: [PATCH] [RFC] media: rcar-vin: don't wait for stop state on clock lane during start of CSI2
Date: Wed, 19 Feb 2020 18:24:56 +0100	[thread overview]
Message-ID: <20200219172456.hyo2aksvubxpoqrn@uno.localdomain> (raw)
In-Reply-To: <1582026251-21047-1-git-send-email-mrodin@de.adit-jv.com>

[-- Attachment #1: Type: text/plain, Size: 2435 bytes --]

Hello,

On Tue, Feb 18, 2020 at 12:44:11PM +0100, Michael Rodin wrote:
> The chapter 7.1 "D-PHY Physical Layer Option" of the CSI2 specification
> states that non-continuous clock behavior is optional, i.e. the Clock Lane
> can remain in high-speed mode between the transmission of data packets.
> Therefore waiting for the stop state (LP-11) on the Clock Lane is wrong and
> will cause timeouts when a CSI2 transmitter with continuous clock behavior
> is attached to R-Car CSI2 receiver. So wait only for the stop state on the
> Data Lanes.

Am I wrong or the desired behaviour should depend on the presence of
the clock-noncontinuous property in the CSI-2 input endpoint ?
If clock-noncontinuous is set, then wait for the clock lane to
enter stop state too, if not just wait for the data lanes to stop.

If this is correct, it will also require a change to the bindings and
that's the tricky part. So far the CSI-2 receiver behaved as the
clock-noncontinuous property was set (wait for both data and clock
lanes) and older dtb should continue to work under this assumption. If
you want to support devices with continuous clock then you have to require
the clock-noncontinuous property to be explicitly set to false, and
assume it's true if not specified. BUT clock-noncontinuous is a
boolean property, whose value depends on it's presence only. So I fear
we need to add a 'clock-continuous' flag to video-interfaces.txt,
parse it in the CSI-2 receiver driver, and then ignore the clock lane
stop state if and only if said property is specified.

Does this make sense ?

Thanks
   j

>
> Signed-off-by: Michael Rodin <mrodin@de.adit-jv.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index faa9fb2..6d1992a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -416,8 +416,7 @@ static int rcsi2_wait_phy_start(struct rcar_csi2 *priv)
>  	for (timeout = 0; timeout <= 20; timeout++) {
>  		const u32 lane_mask = (1 << priv->lanes) - 1;
>
> -		if ((rcsi2_read(priv, PHCLM_REG) & PHCLM_STOPSTATECKL)  &&
> -		    (rcsi2_read(priv, PHDLM_REG) & lane_mask) == lane_mask)
> +		if ((rcsi2_read(priv, PHDLM_REG) & lane_mask) == lane_mask)
>  			return 0;
>
>  		usleep_range(1000, 2000);
> --
> 2.7.4
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]