Pratyush,

Tomi Valkeinen [off-list ref] wrote on Wed [2021-Mar-31 09:06:35 +0300]:

Hi,

On 30/03/2021 20:33, Pratyush Yadav wrote:

quoted

TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
capture over a CSI-2 bus.

The Cadence CSI2RX IP acts as a bridge between the TI specific parts and
the CSI-2 protocol parts. TI then has a wrapper on top of this bridge
called the SHIM layer. It takes in data from stream 0, repacks it, and
sends it to memory over PSI-L DMA.

This driver acts as the "front end" to V4L2 client applications. It
implements the required ioctls and buffer operations, passes the
necessary calls on to the bridge, programs the SHIM layer, and performs
DMA via the dmaengine API to finally return the data to a buffer
supplied by the application.

Signed-off-by: Pratyush Yadav <redacted>
---
  MAINTAINERS                               |   7 +
  drivers/media/platform/Kconfig            |  11 +
  drivers/media/platform/ti-vpe/Makefile    |   1 +
  drivers/media/platform/ti-vpe/ti-csi2rx.c | 964 ++++++++++++++++++++++
  4 files changed, 983 insertions(+)
  create mode 100644 drivers/media/platform/ti-vpe/ti-csi2rx.c

Some quick comments:

"ti-vpe" directory is not correct, this has nothing to do with VPE. That
said, the directory has already been abused by having CAL driver there,
perhaps we should rename the directory just to "ti". But if we do that,
I think we should have subdirs for cal, vpe and this new one.

I agree with Tomi here. This should create a ti directory under
media/platform and then add a directory under that specifically for this
driver/IP as a first step. Not sure what the correct name for that
directory should be but it should meaningful. As a follow on step then the
other drivers can be relocated to a proper directory structure.

"ti-csi2rx" is rather generic name. TI has had CSI-2 RX IPs before (CAL)
and probably will also have new ones in the future. If there's no clear
model name for the IP, as I think is the case here, it's probably best
to just use the SoC model in the name. E.g. the DSS on J7 is
"ti,j721e-dss".

This driver implements the legacy video API. I think it would be better
(and easier to maintain) to only implement the media-controller API,
unless you specifically need to support the legacy API for existing
userspace.

We just went through a major rework with CAL to make it media controller
compatible in order to be able to handle CSI2 virtual channels.
I think as this is a new driver/IP which perform the same type of service
it makes sense to make use the more current API instead of the legacy one.

 Tomi

Benoit

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy