| Message ID | 20210915120240.21572-6-p.yadav@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CSI2RX support on J721E | expand |
Hi Pratyush, Thank you for the patch. On Wed, Sep 15, 2021 at 05:32:34PM +0530, Pratyush Yadav wrote: > 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. No issue with that. Hopefully we'll support multiple streams for real in the not too distant future :-) > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > > (no changes since v1) > > 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 3730e8beee48..edd56c5f2e89 100644 > --- a/drivers/media/platform/cadence/cdns-csi2rx.c > +++ b/drivers/media/platform/cadence/cdns-csi2rx.c > @@ -48,7 +48,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) > @@ -286,8 +285,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,
diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c index 3730e8beee48..edd56c5f2e89 100644 --- a/drivers/media/platform/cadence/cdns-csi2rx.c +++ b/drivers/media/platform/cadence/cdns-csi2rx.c @@ -48,7 +48,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) @@ -286,8 +285,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,
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> --- (no changes since v1) drivers/media/platform/cadence/cdns-csi2rx.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)