Hi Bingbu, Thank you for the patch. I'm happy to see this series landing on the mailing list. I'm starting the review with the easy pieces :-) On Thu, Apr 13, 2023 at 06:04:27PM +0800, bingbu.cao@xxxxxxxxx wrote: > From: Bingbu Cao <bingbu.cao@xxxxxxxxx> > > Add IPU6 support in Kconfig and Makefile, with this patch you can > build the Intel IPU6 and input system modules by select the > CONFIG_VIDEO_INTEL_IPU6 in config. > > Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx> > --- > drivers/media/pci/Kconfig | 1 + > drivers/media/pci/intel/Makefile | 3 ++- > drivers/media/pci/intel/ipu6/Kconfig | 15 +++++++++++++++ > drivers/media/pci/intel/ipu6/Makefile | 23 +++++++++++++++++++++++ > 4 files changed, 41 insertions(+), 1 deletion(-) > create mode 100644 drivers/media/pci/intel/ipu6/Kconfig > create mode 100644 drivers/media/pci/intel/ipu6/Makefile > > diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig > index 480194543d05..38fb484f5c8e 100644 > --- a/drivers/media/pci/Kconfig > +++ b/drivers/media/pci/Kconfig > @@ -74,6 +74,7 @@ config VIDEO_PCI_SKELETON > when developing new drivers. > > source "drivers/media/pci/intel/ipu3/Kconfig" > +source "drivers/media/pci/intel/ipu6/Kconfig" > > endif #MEDIA_PCI_SUPPORT > endif #PCI > diff --git a/drivers/media/pci/intel/Makefile b/drivers/media/pci/intel/Makefile > index 0b4236c4db49..de2b73fef890 100644 > --- a/drivers/media/pci/intel/Makefile > +++ b/drivers/media/pci/intel/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0-only > # > -# Makefile for the IPU3 cio2 and ImGU drivers > +# Makefile for the Intel IPU drivers > # > > obj-y += ipu3/ > +obj-$(CONFIG_VIDEO_INTEL_IPU6) += ipu6/ > diff --git a/drivers/media/pci/intel/ipu6/Kconfig b/drivers/media/pci/intel/ipu6/Kconfig > new file mode 100644 > index 000000000000..c88ef2f40f6d > --- /dev/null > +++ b/drivers/media/pci/intel/ipu6/Kconfig > @@ -0,0 +1,15 @@ > +config VIDEO_INTEL_IPU6 As there will (hopefully) be a driver for the processing system in the future, should this Kconfig symbol be named with a reference to the input system already (and the help text be updated accordingly) ? The name "IPU6" covers both. Or do you envision that we would have a single Kconfig symbol to select both drivers ? > + tristate "Intel IPU6 driver" > + depends on ACPI || COMPILE_TEST > + depends on MEDIA_SUPPORT > + depends on MEDIA_PCI_SUPPORT > + depends on X86_64 Do you use any x86-64 API, or could this be depends on X86_64 || COMPILE_TEST ? > + select IOMMU_IOVA > + select VIDEOBUF2_DMA_CONTIG > + select V4L2_FWNODE > + help > + This is the 6th Gen Intel Image Processing Unit, found in Intel SoCs > + and used for capturing images and video from camera sensors. > + > + To compile this driver, say Y here! It contains 2 modules - > + intel_ipu6 and intel_ipu6_isys. > diff --git a/drivers/media/pci/intel/ipu6/Makefile b/drivers/media/pci/intel/ipu6/Makefile > new file mode 100644 > index 000000000000..6a6339c84ef4 > --- /dev/null > +++ b/drivers/media/pci/intel/ipu6/Makefile > @@ -0,0 +1,23 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +intel-ipu6-objs += ipu6.o \ > + ipu6-bus.o \ > + ipu6-dma.o \ > + ipu6-mmu.o \ > + ipu6-buttress.o \ > + ipu6-cpd.o \ > + ipu6-fw-com.o > + > +obj-$(CONFIG_VIDEO_INTEL_IPU6) += intel-ipu6.o > + > +intel-ipu6-isys-objs += ipu6-isys.o \ > + ipu6-isys-csi2.o \ > + ipu6-fw-isys.o \ > + ipu6-isys-video.o \ > + ipu6-isys-queue.o \ > + ipu6-isys-subdev.o \ > + ipu6-isys-mcd-phy.o \ > + ipu6-isys-jsl-phy.o \ > + ipu6-isys-dwc-phy.o > + > +obj-$(CONFIG_VIDEO_INTEL_IPU6) += intel-ipu6-isys.o -- Regards, Laurent Pinchart