mbox series

[0/3] media: De-stage imx7-mipi-csis.c

Message ID 20220211180216.290133-1-jacopo@jmondi.org
Headers show
Series media: De-stage imx7-mipi-csis.c | expand

Message

Jacopo Mondi Feb. 11, 2022, 6:02 p.m. UTC
The imx7-mipi-csis.c driver has no obvious problems and can be destaged.

Move it to a newly created drivers/media/platform/imx directory and plumb
the Kconfig and build system.

To make it more controversial, I'm annoyed by having the SoC identifier in the
driver file name, as the same IP the driver controls is found on i.MX7 as well
as i.MX8 SoCs.

I'm not sure how it will look like when more CSI-2 receiver drivers will be
de-staged. Currently the situation is a bit confusing, but I think, looking at
the compatibles for each driver that it might be doable to remove the SoC
identifiers from driver names (although I'm sure it has been attempted in the
past).

Anyway, I'm mostly interested in 1/3 to be able to move the driver out of
staging and start adding support for other i.MX8 SoC revisions on top.

Series based on the most recent media/master tree.

Thanks
   j

Jacopo Mondi (3):
  media: imx: De-stage imx7-mipi-csis
  media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
  media: imx: Remove reference to i.MX7 from driver

 MAINTAINERS                                   |  2 +-
 drivers/media/platform/Kconfig                |  1 +
 drivers/media/platform/Makefile               |  1 +
 drivers/media/platform/imx/Kconfig            | 23 +++++++++++++++++++
 drivers/media/platform/imx/Makefile           |  1 +
 .../platform/imx/imx-mipi-csis.c}             | 10 +++++---
 drivers/staging/media/imx/Makefile            |  1 -
 7 files changed, 34 insertions(+), 5 deletions(-)
 create mode 100644 drivers/media/platform/imx/Kconfig
 create mode 100644 drivers/media/platform/imx/Makefile
 rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (99%)

--
2.35.0

Comments

Laurent Pinchart Feb. 14, 2022, 9:51 a.m. UTC | #1
Hi Alexander,

On Mon, Feb 14, 2022 at 10:43:59AM +0100, Alexander Stein wrote:
> Am Freitag, 11. Februar 2022, 19:02:14 CET schrieb Jacopo Mondi:
> > The imx7-mipi-csis driver is in a good state and can be staged.
> > 
> > Move the imx7-mipi-csis.c driver to the newly created
> > drivers/media/platform/imx directory and plumb the related
> > options in Kconfig and in Makefile.
> 
> Please note that there is (at least) one pending patch at [1] which is crucial 
> for my setup.
>
> Also what about calculation for clk_settle in mipi_csis_calculate_params()? Is 
> it really ok to leave it at 0?

It should ideally be computed based on the timing parameters of the
transmitter, but until we figure out how to do so, I don't think it
blocks de-staging the driver. Moving to drivers/media/ doesn't mean
everything has to be perfect, we can continue improving the driver after
the move.

> Despite that I can't say if that driver is ready to be moved out of staging or 
> not.
> 
> Laurent, do you have any preference in which order they should be applied?

Not really, I'm fine with either order.

> [1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/
> 20220211142752.779952-6-alexander.stein@ew.tq-group.com/
> 
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  MAINTAINERS                                   |  2 +-
> >  drivers/media/platform/Kconfig                |  1 +
> >  drivers/media/platform/Makefile               |  1 +
> >  drivers/media/platform/imx/Kconfig            | 23 +++++++++++++++++++
> >  drivers/media/platform/imx/Makefile           |  1 +
> >  .../platform}/imx/imx7-mipi-csis.c            |  0
> >  drivers/staging/media/imx/Makefile            |  1 -
> >  7 files changed, 27 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/media/platform/imx/Kconfig
> >  create mode 100644 drivers/media/platform/imx/Makefile
> >  rename drivers/{staging/media => media/platform}/imx/imx7-mipi-csis.c
> > (100%)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 83d27b57016f..5bdb8c881b0b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11891,8 +11891,8 @@ T:	git git://linuxtv.org/media_tree.git
> >  F:	Documentation/admin-guide/media/imx7.rst
> >  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> >  F:	Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > +F:	drivers/media/platform/imx/imx7-mipi-csis.c
> >  F:	drivers/staging/media/imx/imx7-media-csi.c
> > -F:	drivers/staging/media/imx/imx7-mipi-csis.c
> > 
> >  MEDIA DRIVERS FOR HELENE
> >  M:	Abylay Ospan <aospan@netup.ru>
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index 9fbdba0fd1e7..d9eeccffea69 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -171,6 +171,7 @@ source "drivers/media/platform/xilinx/Kconfig"
> >  source "drivers/media/platform/rcar-vin/Kconfig"
> >  source "drivers/media/platform/atmel/Kconfig"
> >  source "drivers/media/platform/sunxi/Kconfig"
> > +source "drivers/media/platform/imx/Kconfig"
> > 
> >  config VIDEO_TI_CAL
> >  	tristate "TI CAL (Camera Adaptation Layer) driver"
> > diff --git a/drivers/media/platform/Makefile
> > b/drivers/media/platform/Makefile index 28eb4aadbf45..a9466c854610 100644
> > --- a/drivers/media/platform/Makefile
> > +++ b/drivers/media/platform/Makefile
> > @@ -20,6 +20,7 @@ obj-y	+= ti-vpe/
> >  obj-$(CONFIG_VIDEO_MX2_EMMAPRP)		+= mx2_emmaprp.o
> >  obj-$(CONFIG_VIDEO_CODA)		+= coda/
> > 
> > +obj-$(CONFIG_VIDEO_IMX)			+= imx/
> >  obj-$(CONFIG_VIDEO_IMX_PXP)		+= imx-pxp.o
> >  obj-$(CONFIG_VIDEO_IMX8_JPEG)		+= imx-jpeg/
> > 
> > diff --git a/drivers/media/platform/imx/Kconfig
> > b/drivers/media/platform/imx/Kconfig new file mode 100644
> > index 000000000000..0cf35733040c
> > --- /dev/null
> > +++ b/drivers/media/platform/imx/Kconfig
> > @@ -0,0 +1,23 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +menuconfig VIDEO_IMX
> > +	bool "V4L2 capture drivers for NXP i.MX devices"
> > +	depends on VIDEO_DEV && VIDEO_V4L2 && (ARCH_MXC || COMPILE_TEST)
> > +	help
> > +	  Say yes here to enable support for capture drivers on i.MX SoCs.
> > +	  Support for the single SoC features are selectable in the sub-
> menu
> > +	  options.
> > +
> > +if VIDEO_IMX
> > +
> > +config VIDEO_IMX_MIPI_CSIS
> > +	tristate "MIPI CSI-2 CSIS receiver found on i.MX7 and i.MX8 models"
> > +	select MEDIA_CONTROLLER
> > +	select V4L2_FWNODE
> > +	select VIDEO_V4L2_SUBDEV_API
> > +	default n
> > +	help
> > +	  Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
> > +	  v3.3/v3.6 found on some i.MX7 and i.MX8 SoCs.
> > +
> > +endif # VIDEO_IMX
> > diff --git a/drivers/media/platform/imx/Makefile
> > b/drivers/media/platform/imx/Makefile new file mode 100644
> > index 000000000000..ee272234c8d7
> > --- /dev/null
> > +++ b/drivers/media/platform/imx/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx7-mipi-csis.o
> > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
> > b/drivers/media/platform/imx/imx7-mipi-csis.c similarity index 100%
> > rename from drivers/staging/media/imx/imx7-mipi-csis.c
> > rename to drivers/media/platform/imx/imx7-mipi-csis.c
> > diff --git a/drivers/staging/media/imx/Makefile
> > b/drivers/staging/media/imx/Makefile index 19c2fc54d424..d82be898145b
> > 100644
> > --- a/drivers/staging/media/imx/Makefile
> > +++ b/drivers/staging/media/imx/Makefile
> > @@ -15,5 +15,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-media-csi.o
> >  obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
> > 
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> > -obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
Fabio Estevam Feb. 14, 2022, 11:40 a.m. UTC | #2
Hi Jacopo,

On Mon, Feb 14, 2022 at 6:22 AM Jacopo Mondi <jacopo@jmondi.org> wrote:

> I found nothing there that applies to this driver.
> Have I missed any point ?

You are right. After reading the comments in the TODO I see it does
not apply to this driver.

Sorry about the noise.