diff mbox series

[v8,11/16] media: cadence: csi2rx: Fix stream data configuration

Message ID 20230731-upstream_csi-v8-11-fb7d3661c2c9@ti.com
State New
Headers
Series CSI2RX support on J721E and AM62 |

Commit Message

Jai Luthra July 31, 2023, 8:29 a.m. UTC 
  From: Pratyush Yadav <p.yadav@ti.com>

Firstly, there is no VC_EN bit present in the STREAM_DATA_CFG register.
Bit 31 is part of the VL_SELECT field. Remove it completely.

Secondly, it makes little sense to enable ith virtual channel for ith
stream. Sure, there might be a use-case that demands it. But there might
also be a use case that demands all streams to use the 0th virtual
channel. Prefer this case over the former because it is less arbitrary
and also makes it very clear what the limitations of the current driver
is instead of giving a false impression that multiple virtual channels
are supported.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
Signed-off-by: Jai Luthra <j-luthra@ti.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
v7->v8: No change

 drivers/media/platform/cadence/cdns-csi2rx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Tomi Valkeinen Aug. 1, 2023, 2:17 p.m. UTC | #1 
On 31/07/2023 11:29, Jai Luthra wrote:
> From: Pratyush Yadav <p.yadav@ti.com>
> 
> Firstly, there is no VC_EN bit present in the STREAM_DATA_CFG register.
> Bit 31 is part of the VL_SELECT field. Remove it completely.
> 
> Secondly, it makes little sense to enable ith virtual channel for ith
> stream. Sure, there might be a use-case that demands it. But there might
> also be a use case that demands all streams to use the 0th virtual
> channel. Prefer this case over the former because it is less arbitrary
> and also makes it very clear what the limitations of the current driver
> is instead of giving a false impression that multiple virtual channels
> are supported.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> Signed-off-by: Jai Luthra <j-luthra@ti.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> v7->v8: No change
> 
>   drivers/media/platform/cadence/cdns-csi2rx.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
> index a17ef88dff82..cde7fd6463e1 100644
> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
> @@ -49,7 +49,6 @@
>   #define CSI2RX_STREAM_STATUS_RDY			BIT(31)
>   
>   #define CSI2RX_STREAM_DATA_CFG_REG(n)		(CSI2RX_STREAM_BASE(n) + 0x008)
> -#define CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT		BIT(31)
>   #define CSI2RX_STREAM_DATA_CFG_VC_SELECT(n)		BIT((n) + 16)
>   
>   #define CSI2RX_STREAM_CFG_REG(n)		(CSI2RX_STREAM_BASE(n) + 0x00c)
> @@ -275,8 +274,11 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>   		writel(CSI2RX_STREAM_CFG_FIFO_MODE_LARGE_BUF,
>   		       csi2rx->base + CSI2RX_STREAM_CFG_REG(i));
>   
> -		writel(CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT |
> -		       CSI2RX_STREAM_DATA_CFG_VC_SELECT(i),
> +		/*
> +		 * Enable one virtual channel. When multiple virtual channels
> +		 * are supported this will have to be changed.
> +		 */
> +		writel(CSI2RX_STREAM_DATA_CFG_VC_SELECT(0),
>   		       csi2rx->base + CSI2RX_STREAM_DATA_CFG_REG(i));
>   
>   		writel(CSI2RX_STREAM_CTRL_START,
> 

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi
diff mbox series

Patch

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index a17ef88dff82..cde7fd6463e1 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -49,7 +49,6 @@ 
 #define CSI2RX_STREAM_STATUS_RDY			BIT(31)
 
 #define CSI2RX_STREAM_DATA_CFG_REG(n)		(CSI2RX_STREAM_BASE(n) + 0x008)
-#define CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT		BIT(31)
 #define CSI2RX_STREAM_DATA_CFG_VC_SELECT(n)		BIT((n) + 16)
 
 #define CSI2RX_STREAM_CFG_REG(n)		(CSI2RX_STREAM_BASE(n) + 0x00c)
@@ -275,8 +274,11 @@  static int csi2rx_start(struct csi2rx_priv *csi2rx)
 		writel(CSI2RX_STREAM_CFG_FIFO_MODE_LARGE_BUF,
 		       csi2rx->base + CSI2RX_STREAM_CFG_REG(i));
 
-		writel(CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT |
-		       CSI2RX_STREAM_DATA_CFG_VC_SELECT(i),
+		/*
+		 * Enable one virtual channel. When multiple virtual channels
+		 * are supported this will have to be changed.
+		 */
+		writel(CSI2RX_STREAM_DATA_CFG_VC_SELECT(0),
 		       csi2rx->base + CSI2RX_STREAM_DATA_CFG_REG(i));
 
 		writel(CSI2RX_STREAM_CTRL_START,