Re: [PATCH v13 4/7] media: platform: visconti: Add Toshiba Visconti CSI-2 Receiver driver

[Frank Li <Frank.li@nxp.com>]

On Mon, Oct 20, 2025 at 06:13:37AM +0000, yuji2.ishikawa@toshiba.co.jp wrote:
> Hello Frank,
>
> Thank you for review comments.
>
> > -----Original Message-----
> > From: Frank Li <Frank.li@nxp.com>
> > Sent: Thursday, October 16, 2025 1:45 PM
> > To: ishikawa yuji(石川 悠司 □ＡＩＤＣ○ＥＡ開)
> > <yuji2.ishikawa@toshiba.co.jp>
> > Cc: iwamatsu nobuhiro(岩松 信洋 □ＤＩＴＣ○ＣＰＴ)
> > <nobuhiro.iwamatsu.x90@mail.toshiba>; Mauro Carvalho Chehab
> > <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Philipp Zabel
> > <p.zabel@pengutronix.de>; linux-media@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v13 4/7] media: platform: visconti: Add Toshiba Visconti
> > CSI-2 Receiver driver
> >
> > On Thu, Oct 16, 2025 at 11:24:41AM +0900, Yuji Ishikawa wrote:
> > > Add support to MIPI CSI-2 Receiver on Toshiba Visconti ARM SoCs.
> > > This driver is used with Visconti Video Input Interface driver.
> > >
> > > Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp>
> > > ---
> > > Changelog v12:
> > > - Separate CSI2RX driver and made it independent driver
> > > - viif_csi2rx subdevice driver (in v11 patch) was removed.
> > > - dictionary order at Kconfig and Makefile
> > >
> > > Changelog v13:
> > > - wrap one line at 80 characters
> > > - change banner comment style
> > > - update comment style; spacing at the start and end, capitalize first
> > > letter
> > > - add support for clock and reset framework
> > > - add debugfs to pass debug and status information
> > > - add entries to MAINTAINERS file
> > > - Kconfig: add a blank line just after License Identifier.
> > > - update references to header files
> > > - remove redundant inline qualifier
> > > - shorten function/variable names: visconti_csi2rx -> viscsi2
> > > - simplify dphy_write and dphy read operations
> > > - remove osc_freq_target from struct csi2rx_dphy_hs_info, which is always
> > the same value.
> > > - add comment about MASK register's behavior (reversed polarity)
> > > - use v4l2_get_link_freq() instead of get_pixelclock()
> > > - set driver name according to module name: visconti_csi2rx_dev ->
> > > visconti-csi2rx
> > > - check error before setting priv->irq in probe()
> > > - check error at fmt_for_mbus_code()
> > > - add callback for ioctl(VIDIOC_ENUM_FRAMESIZES)
> > > - improve viscsi2_parse_dt() by assuming bus_type is CSI2_DPHY
> > > - use dev_err_ratelimited() for irq handler
> > > - bugfix on fmt_for_mbus_code(): in case unsupported mbus_code is
> > > given
> > > - add goto based error handling sequence to viscsi2_parse_dt()
> > > - specify default value of colorspace, ycbcr_enc, quantization and
> > > xfer_func of sink/src_fmt
> > > - specify sensor at enable_streams() using previously set ID, instead
> > > of checking remote pad every time
> > > - remove U suffix on numeric value
> > > - use unsigned int variable for loop index
> > > - remove redundant casting
> > > - use GENMASK instead of literal
> > > - remove unused constants
> > > - remove unused visconti_csi2rx_video_ops
> > > ---
> > >  MAINTAINERS                                        |   1 +
> > >  drivers/media/platform/Kconfig                     |   1 +
> > >  drivers/media/platform/Makefile                    |   1 +
> > >  drivers/media/platform/toshiba/Kconfig             |   6 +
> > >  drivers/media/platform/toshiba/Makefile            |   3 +
> > >  drivers/media/platform/toshiba/visconti/Kconfig    |  17 +
> > >  drivers/media/platform/toshiba/visconti/Makefile   |   8 +
> > >  .../media/platform/toshiba/visconti/csi2rx_drv.c   | 954
> > +++++++++++++++++++++
> > >  8 files changed, 991 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS index
> > > c17c7ddba5af..ce973791b367 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -25986,6 +25986,7 @@ L:	linux-media@vger.kernel.org
> > >  S:	Maintained
> > >  F:
> > 	Documentation/devicetree/bindings/media/toshiba,visconti5-csi2.ya
> > ml
> > >  F:
> > 	Documentation/devicetree/bindings/media/toshiba,visconti5-viif.yam
> > l
> > > +F:	drivers/media/platform/toshiba/visconti/
> > >
> > >  TOSHIBA WMI HOTKEYS DRIVER
> > >  M:	Azael Avalos <coproscefalo@gmail.com>
> > > diff --git a/drivers/media/platform/Kconfig
> > > b/drivers/media/platform/Kconfig index 9287faafdce5..d5265aa16c88
> > > 100644
> > > --- a/drivers/media/platform/Kconfig
> > > +++ b/drivers/media/platform/Kconfig
> > > @@ -87,6 +87,7 @@ source "drivers/media/platform/st/Kconfig"
> > >  source "drivers/media/platform/sunxi/Kconfig"
> > >  source "drivers/media/platform/synopsys/Kconfig"
> > >  source "drivers/media/platform/ti/Kconfig"
> > > +source "drivers/media/platform/toshiba/Kconfig"
> > >  source "drivers/media/platform/verisilicon/Kconfig"
> > >  source "drivers/media/platform/via/Kconfig"
> > >  source "drivers/media/platform/xilinx/Kconfig"
> > > diff --git a/drivers/media/platform/Makefile
> > > b/drivers/media/platform/Makefile index 6fd7db0541c7..09e67ecb9559
> > > 100644
> > > --- a/drivers/media/platform/Makefile
> > > +++ b/drivers/media/platform/Makefile
> > > @@ -30,6 +30,7 @@ obj-y += st/
> > >  obj-y += sunxi/
> > >  obj-y += synopsys/
> > >  obj-y += ti/
> > > +obj-y += toshiba/
> > >  obj-y += verisilicon/
> > >  obj-y += via/
> > >  obj-y += xilinx/
> > > diff --git a/drivers/media/platform/toshiba/Kconfig
> > > b/drivers/media/platform/toshiba/Kconfig
> > > new file mode 100644
> > > index 000000000000..f02983f4fc97
> > > --- /dev/null
> > > +++ b/drivers/media/platform/toshiba/Kconfig
> > > @@ -0,0 +1,6 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +comment "Toshiba media platform drivers"
> > > +
> > > +source "drivers/media/platform/toshiba/visconti/Kconfig"
> > > +
> > > diff --git a/drivers/media/platform/toshiba/Makefile
> > > b/drivers/media/platform/toshiba/Makefile
> > > new file mode 100644
> > > index 000000000000..dd89a9a35704
> > > --- /dev/null
> > > +++ b/drivers/media/platform/toshiba/Makefile
> > > @@ -0,0 +1,3 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +obj-y += visconti/
> > > diff --git a/drivers/media/platform/toshiba/visconti/Kconfig
> > > b/drivers/media/platform/toshiba/visconti/Kconfig
> > > new file mode 100644
> > > index 000000000000..aa0b63f9f008
> > > --- /dev/null
> > > +++ b/drivers/media/platform/toshiba/visconti/Kconfig
> > > @@ -0,0 +1,17 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +config VIDEO_VISCONTI_CSI2RX
> > > +	tristate "Visconti MIPI CSI-2 Receiver driver"
> > > +	depends on V4L_PLATFORM_DRIVERS
> > > +	depends on VIDEO_DEV && OF
> > > +	depends on ARCH_VISCONTI || COMPILE_TEST
> > > +	select MEDIA_CONTROLLER
> > > +	select VIDEO_V4L2_SUBDEV_API
> > > +	select V4L2_FWNODE
> > > +	help
> > > +	  Support for Toshiba Visconti MIPI CSI-2 receiver,
> > > +	  which is used with Visconti Camera Interface driver.
> > > +
> > > +	  This driver yields 1 subdevice node for a hardware instance.
> > > +	  To compile this driver as a module, choose M here: the
> > > +	  module will be called visconti-csi2rx.
> > > diff --git a/drivers/media/platform/toshiba/visconti/Makefile
> > > b/drivers/media/platform/toshiba/visconti/Makefile
> > > new file mode 100644
> > > index 000000000000..62a029376134
> > > --- /dev/null
> > > +++ b/drivers/media/platform/toshiba/visconti/Makefile
> > > @@ -0,0 +1,8 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Makefile for the Visconti video input device driver #
> > > +
> > > +visconti-csi2rx-objs = csi2rx_drv.o
> > > +
> > > +obj-$(CONFIG_VIDEO_VISCONTI_CSI2RX) += visconti-csi2rx.o
> > > diff --git a/drivers/media/platform/toshiba/visconti/csi2rx_drv.c
> > > b/drivers/media/platform/toshiba/visconti/csi2rx_drv.c
> > > new file mode 100644
> > > index 000000000000..53d112432a86
> > > --- /dev/null
> > > +++ b/drivers/media/platform/toshiba/visconti/csi2rx_drv.c
> > > @@ -0,0 +1,954 @@
> > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > > +/*
> > > + * Toshiba Visconti Video Capture Support
> > > + *
> > > + * (C) Copyright 2025 TOSHIBA CORPORATION
> > > + * (C) Copyright 2025 Toshiba Electronic Devices & Storage
> > > +Corporation  */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/debugfs.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/io.h>
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/property.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/seq_file.h>
> > > +
> > > +#include <media/mipi-csi2.h>
> > > +#include <media/v4l2-common.h>
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +#include <media/v4l2-subdev.h>
> > > +
> > > +/* CSI2HOST register space */
> > > +#define REG_CSI2RX_NLANES	 0x4
> > > +#define REG_CSI2RX_RESETN	 0x8
> > > +#define REG_CSI2RX_INT_ST_MAIN	 0xc
> > > +#define REG_CSI2RX_DATA_IDS_1	 0x10
> > > +#define REG_CSI2RX_DATA_IDS_2	 0x14
> > > +#define REG_CSI2RX_PHY_SHUTDOWNZ 0x40
> > > +#define REG_CSI2RX_PHY_RSTZ	 0x44
> > > +
> > > +/* Access to dphy external registers */ #define
> > > +REG_CSI2RX_PHY_TESTCTRL0 0x50
> > > +#define BIT_TESTCTRL0_CLK_0	 0
> > > +#define BIT_TESTCTRL0_CLK_1	 BIT(1)
> > > +
> > > +#define REG_CSI2RX_PHY_TESTCTRL1 0x54
> > > +#define BIT_TESTCTRL1_ADDR	 BIT(16)
> > > +#define MASK_TESTCTRL1_DOUT	 GENMASK(15, 8)
> > > +
> > > +#define REG_CSI2RX_INT_ST_PHY_FATAL  0xe0 #define
> > > +REG_CSI2RX_INT_MSK_PHY_FATAL 0xe4
> > > +#define MASK_PHY_FATAL_ALL	     0x0000000f
> > > +
> > > +#define REG_CSI2RX_INT_ST_PKT_FATAL  0xf0 #define
> > > +REG_CSI2RX_INT_MSK_PKT_FATAL 0xf4
> > > +#define MASK_PKT_FATAL_ALL	     0x0001000f
> > > +
> > > +#define REG_CSI2RX_INT_ST_FRAME_FATAL  0x100 #define
> > > +REG_CSI2RX_INT_MSK_FRAME_FATAL 0x104
> > > +#define MASK_FRAME_FATAL_ALL	       0x000f0f0f
> > > +
> > > +#define REG_CSI2RX_INT_ST_PHY  0x110
> > > +#define REG_CSI2RX_INT_MSK_PHY 0x114
> > > +#define MASK_PHY_ERROR_ALL     0x000f000f
> > > +
> > > +#define REG_CSI2RX_INT_ST_PKT  0x120
> > > +#define REG_CSI2RX_INT_MSK_PKT 0x124
> > > +#define MASK_PKT_ERROR_ALL     0x000f000f
> > > +
> > > +#define REG_CSI2RX_INT_ST_LINE	0x130
> > > +#define REG_CSI2RX_INT_MSK_LINE 0x134
> > > +#define MASK_LINE_ERROR_ALL	0x00ff00ff
> >
> >
> > Look like it is dwc CSI2RX controller. Can we work out a common dwc CSI2RX
> > driver to avoid every duplicate the same code
> >
> > A attempt at
> > https://lore.kernel.org/imx/20250821-95_cam-v3-20-c9286fbb34b9@nxp.com
> > /
> >
> > The above is the base on stage's imx6. we try to find a path to workout a
> > common dwc csi2rx.
> >
> > Frank
> >
>
> Yes, it is DWC CSI2RX controller.
> It's an interesting idea to have a common dwc driver.
> Let me check if CSI2RX and PHY drivers work well with Visconti's hardware.
> Please let me know the base repository to apply patches.

It's base next-20250821.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tag/?h=next-20250821

Frank
>
> Regards,
> Yuji Ishikawa
>