| Message ID | 20240424024215.1624299-1-yuji2.ishikawa@toshiba.co.jp |
|---|---|
| Headers | show |
| Series | Add Toshiba Visconti Video Input Interface driver | expand |
Hi Yuji, On 24/04/2024 04:42, Yuji Ishikawa wrote: > This series is the Video Input Interface driver > for Toshiba's ARM SoC, Visconti. > This provides DT binding documentation, > device driver, documentation and MAINTAINER files. > > A visconti VIIF driver instance exposes > 1 media control device file, 3 video device files for capture > and 2 video device files for controlling image signal processor. > 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 I commented on patches 3 and 4. I also ran this series through my build scripts, and it did found a few small warnings: kerneldoc: WARNINGS: drivers/media/platform/toshiba/visconti/viif.h:386: warning: Excess struct member 'subdevs' description in 'viif_device' drivers/media/platform/toshiba/visconti/viif.h:386: warning: Excess struct member 'asds' description in 'viif_device' drivers/media/platform/toshiba/visconti/viif.h:386: warning: Excess struct member 'sd' description in 'viif_device' drivers/media/platform/toshiba/visconti/viif_common.h:30: warning: Function parameter or struct member 'bayer_pattern' not described in 'viif_mbus_format' drivers/media/platform/toshiba/visconti/viif_common.h:30: warning: Function parameter or struct member 'is_bayer' not described in 'viif_mbus_format' Should be trivial to fix. Regards, Hans > > 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 > > Changelog v8: > - rename bindings description file > - remove/simplify items in bindings > - update operations around v4l2_async_notifier > - use v4l2_async_connection instead of v4l2_async_subdev > - use dev_err_probe() > - better error handling at probe > - remove redundant mutex > - add V4L2_CTRL_TYPE_VISCONTI_ISP constant > > Changelog v9: > - dictionary ordering of dt-bindings properties > - applied sparce checker > - call div64_u64 for 64bit division > - rebase to media_staging tree > - fix warning for cast between ptr and dma_addr_t > > Changelog v10: > - add an independent entry in MAINTAINERS > - add paddings to uAPI structs > - use parameter buffer to control ISP (instead of vendor specific controls) > > Yuji Ishikawa (6): > dt-bindings: media: platform: visconti: Add Toshiba Visconti Video > Input Interface > media: videodev2.h: add visconti viif meta buffer format > media: platform: visconti: Add Toshiba Visconti Video Input Interface > driver > media: platform: visconti: add streaming interface for ISP parameters > and status > documentation: media: add documentation for Toshiba Visconti Video > Input Interface driver > MAINTAINERS: Add entries for Toshiba Visconti Video Input Interface > > .../admin-guide/media/v4l-drivers.rst | 1 + > .../admin-guide/media/visconti-viif.dot | 18 + > .../admin-guide/media/visconti-viif.rst | 252 ++ > .../media/toshiba,visconti5-viif.yaml | 105 + > .../userspace-api/media/v4l/meta-formats.rst | 1 + > .../media/v4l/metafmt-visconti-viif.rst | 48 + > MAINTAINERS | 11 + > 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 | 19 + > .../media/platform/toshiba/visconti/Makefile | 8 + > .../media/platform/toshiba/visconti/viif.c | 664 ++++++ > .../media/platform/toshiba/visconti/viif.h | 398 ++++ > .../platform/toshiba/visconti/viif_capture.c | 1472 ++++++++++++ > .../platform/toshiba/visconti/viif_capture.h | 22 + > .../platform/toshiba/visconti/viif_common.c | 239 ++ > .../platform/toshiba/visconti/viif_common.h | 40 + > .../platform/toshiba/visconti/viif_csi2rx.c | 657 ++++++ > .../platform/toshiba/visconti/viif_csi2rx.h | 24 + > .../toshiba/visconti/viif_csi2rx_regs.h | 102 + > .../platform/toshiba/visconti/viif_isp.c | 1191 ++++++++++ > .../platform/toshiba/visconti/viif_isp.h | 24 + > .../platform/toshiba/visconti/viif_params.c | 2026 +++++++++++++++++ > .../platform/toshiba/visconti/viif_params.h | 19 + > .../platform/toshiba/visconti/viif_regs.h | 721 ++++++ > .../platform/toshiba/visconti/viif_stats.c | 334 +++ > .../platform/toshiba/visconti/viif_stats.h | 14 + > include/uapi/linux/videodev2.h | 4 + > include/uapi/linux/visconti_viif.h | 1921 ++++++++++++++++ > 31 files changed, 10345 insertions(+) > create mode 100644 Documentation/admin-guide/media/visconti-viif.dot > create mode 100644 Documentation/admin-guide/media/visconti-viif.rst > create mode 100644 Documentation/devicetree/bindings/media/toshiba,visconti5-viif.yaml > create mode 100644 Documentation/userspace-api/media/v4l/metafmt-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_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_params.c > create mode 100644 drivers/media/platform/toshiba/visconti/viif_params.h > create mode 100644 drivers/media/platform/toshiba/visconti/viif_regs.h > create mode 100644 drivers/media/platform/toshiba/visconti/viif_stats.c > create mode 100644 drivers/media/platform/toshiba/visconti/viif_stats.h > create mode 100644 include/uapi/linux/visconti_viif.h >
Hello Hans, Thank you for your review. > -----Original Message----- > From: Hans Verkuil <hverkuil@xs4all.nl> > Sent: Friday, May 31, 2024 8:02 PM > To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開) > <yuji2.ishikawa@toshiba.co.jp>; Laurent Pinchart > <laurent.pinchart@ideasonboard.com>; Mauro Carvalho Chehab > <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Rafael J . > Wysocki <rafael.j.wysocki@intel.com>; iwamatsu nobuhiro(岩松 信洋 ○ＤＩ > ＴＣ□ＤＩＴ○ＯＳＴ) <nobuhiro1.iwamatsu@toshiba.co.jp> > Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v10 0/6] Add Toshiba Visconti Video Input Interface driver > > Hi Yuji, > > On 24/04/2024 04:42, Yuji Ishikawa wrote: > > This series is the Video Input Interface driver for Toshiba's ARM SoC, > > Visconti. > > This provides DT binding documentation, device driver, documentation > > and MAINTAINER files. > > > > A visconti VIIF driver instance exposes > > 1 media control device file, 3 video device files for capture and 2 > > video device files for controlling image signal processor. > > 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 > > I commented on patches 3 and 4. > > I also ran this series through my build scripts, and it did found a few small > warnings: > > kerneldoc: WARNINGS: > > drivers/media/platform/toshiba/visconti/viif.h:386: warning: Excess struct > member 'subdevs' description in 'viif_device' > drivers/media/platform/toshiba/visconti/viif.h:386: warning: Excess struct > member 'asds' description in 'viif_device' > drivers/media/platform/toshiba/visconti/viif.h:386: warning: Excess struct > member 'sd' description in 'viif_device' > drivers/media/platform/toshiba/visconti/viif_common.h:30: warning: Function > parameter or struct member 'bayer_pattern' not described in 'viif_mbus_format' > drivers/media/platform/toshiba/visconti/viif_common.h:30: warning: Function > parameter or struct member 'is_bayer' not described in 'viif_mbus_format' > > Should be trivial to fix. > I'll fix kerneldoc comments. > Regards, > > Hans > Best regards, Yuji
This series is the Video Input Interface driver for Toshiba's ARM SoC, Visconti. This provides DT binding documentation, device driver, documentation and MAINTAINER files. A visconti VIIF driver instance exposes 1 media control device file, 3 video device files for capture and 2 video device files for controlling image signal processor. 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 Changelog v8: - rename bindings description file - remove/simplify items in bindings - update operations around v4l2_async_notifier - use v4l2_async_connection instead of v4l2_async_subdev - use dev_err_probe() - better error handling at probe - remove redundant mutex - add V4L2_CTRL_TYPE_VISCONTI_ISP constant Changelog v9: - dictionary ordering of dt-bindings properties - applied sparce checker - call div64_u64 for 64bit division - rebase to media_staging tree - fix warning for cast between ptr and dma_addr_t Changelog v10: - add an independent entry in MAINTAINERS - add paddings to uAPI structs - use parameter buffer to control ISP (instead of vendor specific controls) Yuji Ishikawa (6): dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface media: videodev2.h: add visconti viif meta buffer format media: platform: visconti: Add Toshiba Visconti Video Input Interface driver media: platform: visconti: add streaming interface for ISP parameters and status documentation: media: add documentation for Toshiba Visconti Video Input Interface driver MAINTAINERS: Add entries for Toshiba Visconti Video Input Interface .../admin-guide/media/v4l-drivers.rst | 1 + .../admin-guide/media/visconti-viif.dot | 18 + .../admin-guide/media/visconti-viif.rst | 252 ++ .../media/toshiba,visconti5-viif.yaml | 105 + .../userspace-api/media/v4l/meta-formats.rst | 1 + .../media/v4l/metafmt-visconti-viif.rst | 48 + MAINTAINERS | 11 + 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 | 19 + .../media/platform/toshiba/visconti/Makefile | 8 + .../media/platform/toshiba/visconti/viif.c | 664 ++++++ .../media/platform/toshiba/visconti/viif.h | 398 ++++ .../platform/toshiba/visconti/viif_capture.c | 1472 ++++++++++++ .../platform/toshiba/visconti/viif_capture.h | 22 + .../platform/toshiba/visconti/viif_common.c | 239 ++ .../platform/toshiba/visconti/viif_common.h | 40 + .../platform/toshiba/visconti/viif_csi2rx.c | 657 ++++++ .../platform/toshiba/visconti/viif_csi2rx.h | 24 + .../toshiba/visconti/viif_csi2rx_regs.h | 102 + .../platform/toshiba/visconti/viif_isp.c | 1191 ++++++++++ .../platform/toshiba/visconti/viif_isp.h | 24 + .../platform/toshiba/visconti/viif_params.c | 2026 +++++++++++++++++ .../platform/toshiba/visconti/viif_params.h | 19 + .../platform/toshiba/visconti/viif_regs.h | 721 ++++++ .../platform/toshiba/visconti/viif_stats.c | 334 +++ .../platform/toshiba/visconti/viif_stats.h | 14 + include/uapi/linux/videodev2.h | 4 + include/uapi/linux/visconti_viif.h | 1921 ++++++++++++++++ 31 files changed, 10345 insertions(+) create mode 100644 Documentation/admin-guide/media/visconti-viif.dot create mode 100644 Documentation/admin-guide/media/visconti-viif.rst create mode 100644 Documentation/devicetree/bindings/media/toshiba,visconti5-viif.yaml create mode 100644 Documentation/userspace-api/media/v4l/metafmt-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_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_params.c create mode 100644 drivers/media/platform/toshiba/visconti/viif_params.h create mode 100644 drivers/media/platform/toshiba/visconti/viif_regs.h create mode 100644 drivers/media/platform/toshiba/visconti/viif_stats.c create mode 100644 drivers/media/platform/toshiba/visconti/viif_stats.h create mode 100644 include/uapi/linux/visconti_viif.h