mbox series

[v7,0/5] Add Toshiba Visconti Video Input Interface driver

Message ID 20230714015059.18775-1-yuji2.ishikawa@toshiba.co.jp (mailing list archive)
Headers show
Series Add Toshiba Visconti Video Input Interface driver | expand

Message

Yuji Ishikawa July 14, 2023, 1:50 a.m. UTC
This series is the Video Input Interface driver
for Toshiba's ARM SoC, Visconti[0].
This provides DT binding documentation,
device driver, documentation and MAINTAINER files.

A visconti VIIF driver instance exposes
1 media control device file and 3 video device files
for a VIIF hardware.
Detailed HW/SW are described in documentation directory.
The VIIF hardware has CSI2 receiver,
image signal processor and DMAC inside.
The subdevice for image signal processor provides
vendor specific V4L2 controls.

The device driver depends on two other drivers under development;
clock framework driver and IOMMU driver.
Corresponding features will be added later.

Best regards,
Yuji

Changelog v2:
- Resend v1 because a patch exceeds size limit.

Changelog v3:
- Add documentation to describe SW and HW
- Adapted to media control framework
- Introduced ISP subdevice, capture device
- Remove private IOCTLs and add vendor specific V4L2 controls
- Change function name avoiding camelcase and uppercase letters

Changelog v4:
- Split patches because a patch exceeds size limit
- fix dt-bindings document
- stop specifying ID numbers for driver instance explicitly at device tree
- use pm_runtime to trigger initialization of HW
  along with open/close of device files.
- add a entry for a header file at MAINTAINERS file

Changelog v5:
- Fix coding style problem in viif.c (patch 2/6)

Changelog v6:
- add register definition of BUS-IF and MPU in dt-bindings
- add CSI2RX subdevice (separeted from ISP subdevice)
- change directory layout (moved to media/platform/toshiba/visconti)
- change source file layout (removed hwd_xxxx.c)
- pointer to userland memory is removed from uAPI parameters
- change register access (from struct style to macro style)
- remove unused macros

Changelog v7:
- remove redundant "bindings" from header and description text
- fix multiline text of "description"
- change "compatible" to "visconti5-viif"
- explicitly define allowed properties for port::endpoint
- remove unused variables
- update kerneldoc comments
- update references to headers

Yuji Ishikawa (5):
  dt-bindings: media: platform: visconti: Add Toshiba Visconti Video
    Input Interface
  media: platform: visconti: Add Toshiba Visconti Video Input Interface
    driver
  media: add V4L2 vendor specific control handlers
  documentation: media: add documentation for Toshiba Visconti Video
    Input Interface driver
  MAINTAINERS: Add entries for Toshiba Visconti Video Input Interface

 .../bindings/media/toshiba,visconti-viif.yaml |  108 +
 .../driver-api/media/drivers/index.rst        |    1 +
 .../media/drivers/visconti-viif.rst           |  462 +++
 MAINTAINERS                                   |    4 +
 drivers/media/platform/Kconfig                |    1 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/toshiba/Kconfig        |    6 +
 drivers/media/platform/toshiba/Makefile       |    2 +
 .../media/platform/toshiba/visconti/Kconfig   |   18 +
 .../media/platform/toshiba/visconti/Makefile  |    8 +
 .../media/platform/toshiba/visconti/viif.c    |  681 ++++
 .../media/platform/toshiba/visconti/viif.h    |  375 ++
 .../platform/toshiba/visconti/viif_capture.c  | 1485 +++++++
 .../platform/toshiba/visconti/viif_capture.h  |   22 +
 .../platform/toshiba/visconti/viif_common.c   |  199 +
 .../platform/toshiba/visconti/viif_common.h   |   38 +
 .../platform/toshiba/visconti/viif_controls.c | 3407 +++++++++++++++++
 .../platform/toshiba/visconti/viif_controls.h |   18 +
 .../platform/toshiba/visconti/viif_csi2rx.c   |  684 ++++
 .../platform/toshiba/visconti/viif_csi2rx.h   |   24 +
 .../toshiba/visconti/viif_csi2rx_regs.h       |  102 +
 .../platform/toshiba/visconti/viif_isp.c      | 1258 ++++++
 .../platform/toshiba/visconti/viif_isp.h      |   24 +
 .../platform/toshiba/visconti/viif_regs.h     |  716 ++++
 include/uapi/linux/v4l2-controls.h            |    6 +
 include/uapi/linux/visconti_viif.h            | 1800 +++++++++
 26 files changed, 11450 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
 create mode 100644 Documentation/driver-api/media/drivers/visconti-viif.rst
 create mode 100644 drivers/media/platform/toshiba/Kconfig
 create mode 100644 drivers/media/platform/toshiba/Makefile
 create mode 100644 drivers/media/platform/toshiba/visconti/Kconfig
 create mode 100644 drivers/media/platform/toshiba/visconti/Makefile
 create mode 100644 drivers/media/platform/toshiba/visconti/viif.c
 create mode 100644 drivers/media/platform/toshiba/visconti/viif.h
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_capture.c
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_capture.h
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_common.c
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_common.h
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_controls.c
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_controls.h
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_csi2rx.c
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_csi2rx.h
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_csi2rx_regs.h
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_isp.c
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_isp.h
 create mode 100644 drivers/media/platform/toshiba/visconti/viif_regs.h
 create mode 100644 include/uapi/linux/visconti_viif.h

Comments

Hans Verkuil Aug. 21, 2023, 12:58 p.m. UTC | #1
Hi Yuji,

On 14/07/2023 03:50, Yuji Ishikawa wrote:
> This series is the Video Input Interface driver
> for Toshiba's ARM SoC, Visconti[0].
> This provides DT binding documentation,
> device driver, documentation and MAINTAINER files.
> 
> A visconti VIIF driver instance exposes
> 1 media control device file and 3 video device files
> for a VIIF hardware.
> Detailed HW/SW are described in documentation directory.
> The VIIF hardware has CSI2 receiver,
> image signal processor and DMAC inside.
> The subdevice for image signal processor provides
> vendor specific V4L2 controls.
> 
> The device driver depends on two other drivers under development;
> clock framework driver and IOMMU driver.
> Corresponding features will be added later.

Trying to compile this series on top of our latest staging tree fails
due to v4l2-async changes that have been merged. So for v8 please
rebase to the staging tree.

I also got a few kerneldoc warnings:

drivers/media/platform/toshiba/visconti/viif.h:217: warning: Function parameter or member 'ops_lock' not described in 'isp_subdev'
drivers/media/platform/toshiba/visconti/viif.h:233: warning: Function parameter or member 'ops_lock' not described in 'csi2rx_subdev'
drivers/media/platform/toshiba/visconti/viif.h:254: warning: Function parameter or member 'post_enable_flag' not described in 'viif_l2_roi_path_info'

Regards,

	Hans

> 
> Best regards,
> Yuji
> 
> Changelog v2:
> - Resend v1 because a patch exceeds size limit.
> 
> Changelog v3:
> - Add documentation to describe SW and HW
> - Adapted to media control framework
> - Introduced ISP subdevice, capture device
> - Remove private IOCTLs and add vendor specific V4L2 controls
> - Change function name avoiding camelcase and uppercase letters
> 
> Changelog v4:
> - Split patches because a patch exceeds size limit
> - fix dt-bindings document
> - stop specifying ID numbers for driver instance explicitly at device tree
> - use pm_runtime to trigger initialization of HW
>   along with open/close of device files.
> - add a entry for a header file at MAINTAINERS file
> 
> Changelog v5:
> - Fix coding style problem in viif.c (patch 2/6)
> 
> Changelog v6:
> - add register definition of BUS-IF and MPU in dt-bindings
> - add CSI2RX subdevice (separeted from ISP subdevice)
> - change directory layout (moved to media/platform/toshiba/visconti)
> - change source file layout (removed hwd_xxxx.c)
> - pointer to userland memory is removed from uAPI parameters
> - change register access (from struct style to macro style)
> - remove unused macros
> 
> Changelog v7:
> - remove redundant "bindings" from header and description text
> - fix multiline text of "description"
> - change "compatible" to "visconti5-viif"
> - explicitly define allowed properties for port::endpoint
> - remove unused variables
> - update kerneldoc comments
> - update references to headers
> 
> Yuji Ishikawa (5):
>   dt-bindings: media: platform: visconti: Add Toshiba Visconti Video
>     Input Interface
>   media: platform: visconti: Add Toshiba Visconti Video Input Interface
>     driver
>   media: add V4L2 vendor specific control handlers
>   documentation: media: add documentation for Toshiba Visconti Video
>     Input Interface driver
>   MAINTAINERS: Add entries for Toshiba Visconti Video Input Interface
> 
>  .../bindings/media/toshiba,visconti-viif.yaml |  108 +
>  .../driver-api/media/drivers/index.rst        |    1 +
>  .../media/drivers/visconti-viif.rst           |  462 +++
>  MAINTAINERS                                   |    4 +
>  drivers/media/platform/Kconfig                |    1 +
>  drivers/media/platform/Makefile               |    1 +
>  drivers/media/platform/toshiba/Kconfig        |    6 +
>  drivers/media/platform/toshiba/Makefile       |    2 +
>  .../media/platform/toshiba/visconti/Kconfig   |   18 +
>  .../media/platform/toshiba/visconti/Makefile  |    8 +
>  .../media/platform/toshiba/visconti/viif.c    |  681 ++++
>  .../media/platform/toshiba/visconti/viif.h    |  375 ++
>  .../platform/toshiba/visconti/viif_capture.c  | 1485 +++++++
>  .../platform/toshiba/visconti/viif_capture.h  |   22 +
>  .../platform/toshiba/visconti/viif_common.c   |  199 +
>  .../platform/toshiba/visconti/viif_common.h   |   38 +
>  .../platform/toshiba/visconti/viif_controls.c | 3407 +++++++++++++++++
>  .../platform/toshiba/visconti/viif_controls.h |   18 +
>  .../platform/toshiba/visconti/viif_csi2rx.c   |  684 ++++
>  .../platform/toshiba/visconti/viif_csi2rx.h   |   24 +
>  .../toshiba/visconti/viif_csi2rx_regs.h       |  102 +
>  .../platform/toshiba/visconti/viif_isp.c      | 1258 ++++++
>  .../platform/toshiba/visconti/viif_isp.h      |   24 +
>  .../platform/toshiba/visconti/viif_regs.h     |  716 ++++
>  include/uapi/linux/v4l2-controls.h            |    6 +
>  include/uapi/linux/visconti_viif.h            | 1800 +++++++++
>  26 files changed, 11450 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
>  create mode 100644 Documentation/driver-api/media/drivers/visconti-viif.rst
>  create mode 100644 drivers/media/platform/toshiba/Kconfig
>  create mode 100644 drivers/media/platform/toshiba/Makefile
>  create mode 100644 drivers/media/platform/toshiba/visconti/Kconfig
>  create mode 100644 drivers/media/platform/toshiba/visconti/Makefile
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif.c
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif.h
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_capture.c
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_capture.h
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_common.c
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_common.h
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_controls.c
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_controls.h
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_csi2rx.c
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_csi2rx.h
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_csi2rx_regs.h
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_isp.c
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_isp.h
>  create mode 100644 drivers/media/platform/toshiba/visconti/viif_regs.h
>  create mode 100644 include/uapi/linux/visconti_viif.h
>
Yuji Ishikawa Aug. 30, 2023, 12:44 a.m. UTC | #2
Hello Hans,

> -----Original Message-----
> From: Hans Verkuil <hverkuil@xs4all.nl>
> Sent: Monday, August 21, 2023 9:59 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> <yuji2.ishikawa@toshiba.co.jp>; Sakari Ailus <sakari.ailus@iki.fi>; Laurent
> Pinchart <laurent.pinchart@ideasonboard.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; iwamatsu nobuhiro(岩松 信洋 ○DITC□DIT○
> OST) <nobuhiro1.iwamatsu@toshiba.co.jp>; Mark Brown
> <broonie@kernel.org>
> Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v7 0/5] Add Toshiba Visconti Video Input Interface driver
> 
> Hi Yuji,
> 
> On 14/07/2023 03:50, Yuji Ishikawa wrote:
> > This series is the Video Input Interface driver for Toshiba's ARM SoC,
> > Visconti[0].
> > This provides DT binding documentation, device driver, documentation
> > and MAINTAINER files.
> >
> > A visconti VIIF driver instance exposes
> > 1 media control device file and 3 video device files for a VIIF
> > hardware.
> > Detailed HW/SW are described in documentation directory.
> > The VIIF hardware has CSI2 receiver,
> > image signal processor and DMAC inside.
> > The subdevice for image signal processor provides vendor specific V4L2
> > controls.
> >
> > The device driver depends on two other drivers under development;
> > clock framework driver and IOMMU driver.
> > Corresponding features will be added later.
> 
> Trying to compile this series on top of our latest staging tree fails due to
> v4l2-async changes that have been merged. So for v8 please rebase to the
> staging tree.

All right. The v8 patchset will be rebased to media_stage.git .

> I also got a few kerneldoc warnings:
> 
> drivers/media/platform/toshiba/visconti/viif.h:217: warning: Function
> parameter or member 'ops_lock' not described in 'isp_subdev'
> drivers/media/platform/toshiba/visconti/viif.h:233: warning: Function
> parameter or member 'ops_lock' not described in 'csi2rx_subdev'
> drivers/media/platform/toshiba/visconti/viif.h:254: warning: Function
> parameter or member 'post_enable_flag' not described in 'viif_l2_roi_path_info'

I'll check for kerneldoc warnings and fix them.

Regards,
Yuji

> Regards,
> 
> 	Hans
> 
> >
> > Best regards,
> > Yuji
> >
> > Changelog v2:
> > - Resend v1 because a patch exceeds size limit.
> >
> > Changelog v3:
> > - Add documentation to describe SW and HW
> > - Adapted to media control framework
> > - Introduced ISP subdevice, capture device
> > - Remove private IOCTLs and add vendor specific V4L2 controls
> > - Change function name avoiding camelcase and uppercase letters
> >
> > Changelog v4:
> > - Split patches because a patch exceeds size limit
> > - fix dt-bindings document
> > - stop specifying ID numbers for driver instance explicitly at device
> > tree
> > - use pm_runtime to trigger initialization of HW
> >   along with open/close of device files.
> > - add a entry for a header file at MAINTAINERS file
> >
> > Changelog v5:
> > - Fix coding style problem in viif.c (patch 2/6)
> >
> > Changelog v6:
> > - add register definition of BUS-IF and MPU in dt-bindings
> > - add CSI2RX subdevice (separeted from ISP subdevice)
> > - change directory layout (moved to media/platform/toshiba/visconti)
> > - change source file layout (removed hwd_xxxx.c)
> > - pointer to userland memory is removed from uAPI parameters
> > - change register access (from struct style to macro style)
> > - remove unused macros
> >
> > Changelog v7:
> > - remove redundant "bindings" from header and description text
> > - fix multiline text of "description"
> > - change "compatible" to "visconti5-viif"
> > - explicitly define allowed properties for port::endpoint
> > - remove unused variables
> > - update kerneldoc comments
> > - update references to headers
> >
> > Yuji Ishikawa (5):
> >   dt-bindings: media: platform: visconti: Add Toshiba Visconti Video
> >     Input Interface
> >   media: platform: visconti: Add Toshiba Visconti Video Input Interface
> >     driver
> >   media: add V4L2 vendor specific control handlers
> >   documentation: media: add documentation for Toshiba Visconti Video
> >     Input Interface driver
> >   MAINTAINERS: Add entries for Toshiba Visconti Video Input Interface
> >
> >  .../bindings/media/toshiba,visconti-viif.yaml |  108 +
> >  .../driver-api/media/drivers/index.rst        |    1 +
> >  .../media/drivers/visconti-viif.rst           |  462 +++
> >  MAINTAINERS                                   |    4 +
> >  drivers/media/platform/Kconfig                |    1 +
> >  drivers/media/platform/Makefile               |    1 +
> >  drivers/media/platform/toshiba/Kconfig        |    6 +
> >  drivers/media/platform/toshiba/Makefile       |    2 +
> >  .../media/platform/toshiba/visconti/Kconfig   |   18 +
> >  .../media/platform/toshiba/visconti/Makefile  |    8 +
> >  .../media/platform/toshiba/visconti/viif.c    |  681 ++++
> >  .../media/platform/toshiba/visconti/viif.h    |  375 ++
> >  .../platform/toshiba/visconti/viif_capture.c  | 1485 +++++++
> >  .../platform/toshiba/visconti/viif_capture.h  |   22 +
> >  .../platform/toshiba/visconti/viif_common.c   |  199 +
> >  .../platform/toshiba/visconti/viif_common.h   |   38 +
> >  .../platform/toshiba/visconti/viif_controls.c | 3407
> +++++++++++++++++
> >  .../platform/toshiba/visconti/viif_controls.h |   18 +
> >  .../platform/toshiba/visconti/viif_csi2rx.c   |  684 ++++
> >  .../platform/toshiba/visconti/viif_csi2rx.h   |   24 +
> >  .../toshiba/visconti/viif_csi2rx_regs.h       |  102 +
> >  .../platform/toshiba/visconti/viif_isp.c      | 1258 ++++++
> >  .../platform/toshiba/visconti/viif_isp.h      |   24 +
> >  .../platform/toshiba/visconti/viif_regs.h     |  716 ++++
> >  include/uapi/linux/v4l2-controls.h            |    6 +
> >  include/uapi/linux/visconti_viif.h            | 1800 +++++++++
> >  26 files changed, 11450 insertions(+)  create mode 100644
> > Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
> >  create mode 100644
> > Documentation/driver-api/media/drivers/visconti-viif.rst
> >  create mode 100644 drivers/media/platform/toshiba/Kconfig
> >  create mode 100644 drivers/media/platform/toshiba/Makefile
> >  create mode 100644 drivers/media/platform/toshiba/visconti/Kconfig
> >  create mode 100644 drivers/media/platform/toshiba/visconti/Makefile
> >  create mode 100644 drivers/media/platform/toshiba/visconti/viif.c
> >  create mode 100644 drivers/media/platform/toshiba/visconti/viif.h
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_capture.c
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_capture.h
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_common.c
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_common.h
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_controls.c
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_controls.h
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_csi2rx.c
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_csi2rx.h
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_csi2rx_regs.h
> >  create mode 100644 drivers/media/platform/toshiba/visconti/viif_isp.c
> >  create mode 100644 drivers/media/platform/toshiba/visconti/viif_isp.h
> >  create mode 100644
> > drivers/media/platform/toshiba/visconti/viif_regs.h
> >  create mode 100644 include/uapi/linux/visconti_viif.h
> >