Hi Laurent, On Tue, Nov 22, 2022 at 8:28 PM Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > > Hi Laurent, > > Thank you for the review. > > On Tue, Nov 22, 2022 at 2:00 AM Laurent Pinchart > <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > > > > Hi Prabhakar, > > > > Thank you for the patch. > > > > On Wed, Nov 02, 2022 at 12:43:29AM +0000, Prabhakar wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > > > Add v4l driver for Renesas RZ/G2L Camera data Receiving Unit. > > > > > > Based on a patch in the BSP by Hien Huynh > > > <hien.huynh.px@xxxxxxxxxxx> > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > --- > > > -v4 -> v5 > > > * Made sure we call pre_streamon/post_streamoff around s_stream > > > * Made sure to cleanup on error path in s_stream > > > * Dropped pre_streamon/post_streamoff callbacks from IP subdev > > > * Moved the CRU start/stop configuration to IP subdev to avoid > > > recursively calling pre_streamon/post_streamoff callbacks. > > > > > > -v3 -> v4 > > > * Undoing the configuration in case s_stream(1) fails > > > * Made sure we call post_streamoff in the error path of s_stream(1) > > > > > > v2 -> v3 > > > * Switched to PM runtime > > > * Modeled CRU IP block as a subdev > > > * Dropped explicitly selecting VIDEO_RZG2L_CSI2 for VIDEO_RZG2L_CRU config > > > * Replaced v4l2_dev_to_cru macro with inline function notifier_to_cru() > > > * Dropped id parameter from rvin_mc_parse_of() > > > * Renamed rzg2l_cru_csi2_init() -> rzg2l_cru_media_init() > > > * Used dev_err_probe() in rzg2l_cru_probe() > > > * Replaced devm_reset_control_get() -> devm_reset_control_get_exclusive() > > > * Prefixed HW_BUFFER_MAX and HW_BUFFER_DEFAULT macros with RZG2L_CRU_ > > > * Moved asserting presetn signal from rzg2l_cru_dma_register() to rzg2l_cru_start_streaming_vq() > > > * Dropped VB2_READ from VB2 io_modes > > > * Used dev_dbg() in rzg2l_cru_video_register() and rzg2l_cru_video_unregister() > > > * Got rid of rzg2l_cru_notify() > > > * Dropped V4L2_CAP_READWRITE from device caps > > > * Introduced rzg2l_cru_v4l2_init() for initialization. > > > * Got rid v4l2_pipeline_pm_get() and used PM in ov5645 sensor driver. Patch posted > > > https://patchwork.linuxtv.org/project/linux-media/patch/20220927201634.750141-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/ > > > > > > v1 -> v2 > > > * No change > > > > > > RFC v2 -> v1 > > > * Moved the driver to renesas folder > > > * Fixed review comments pointed by Jacopo > > > > > > RFC v1 -> RFC v2 > > > * Dropped group > > > * Dropped CSI subdev and implemented as new driver > > > * Dropped "mc_" from function names > > > * Moved the driver to renesas folder > > > --- > > > .../media/platform/renesas/rzg2l-cru/Kconfig | 16 + > > > .../media/platform/renesas/rzg2l-cru/Makefile | 3 + > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 370 ++++++ > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 169 +++ > > > .../platform/renesas/rzg2l-cru/rzg2l-ip.c | 283 +++++ > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 1086 +++++++++++++++++ > > > 6 files changed, 1927 insertions(+) > > > create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c > > > create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h > > > create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig > > > index 57c40bb499df..b39818c1f053 100644 > > > --- a/drivers/media/platform/renesas/rzg2l-cru/Kconfig > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig > > > @@ -15,3 +15,19 @@ config VIDEO_RZG2L_CSI2 > > > > > > To compile this driver as a module, choose M here: the > > > module will be called rzg2l-csi2. > > > + > > > +config VIDEO_RZG2L_CRU > > > + tristate "RZ/G2L Camera Receiving Unit (CRU) Driver" > > > + depends on ARCH_RENESAS || COMPILE_TEST > > > + depends on V4L_PLATFORM_DRIVERS > > > + depends on VIDEO_DEV && OF > > > + select MEDIA_CONTROLLER > > > + select V4L2_FWNODE > > > + select VIDEOBUF2_DMA_CONTIG > > > + select VIDEO_V4L2_SUBDEV_API > > > + help > > > + Support for Renesas RZ/G2L (and alike SoC's) Camera Receiving > > > + Unit (CRU) driver. > > > + > > > + To compile this driver as a module, choose M here: the > > > + module will be called rzg2l-cru. > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile > > > index 91ea97a944e6..c4db2632874f 100644 > > > --- a/drivers/media/platform/renesas/rzg2l-cru/Makefile > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile > > > @@ -1,3 +1,6 @@ > > > # SPDX-License-Identifier: GPL-2.0 > > > > > > obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o > > > + > > > +rzg2l-cru-objs = rzg2l-core.o rzg2l-ip.o rzg2l-video.o > > > +obj-$(CONFIG_VIDEO_RZG2L_CRU) += rzg2l-cru.o > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c > > > new file mode 100644 > > > index 000000000000..7a92fcfee84c > > > --- /dev/null > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c > > > @@ -0,0 +1,370 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Driver for Renesas RZ/G2L CRU > > > + * > > > + * Copyright (C) 2022 Renesas Electronics Corp. > > > + * > > > + * Based on Renesas R-Car VIN > > > + * Copyright (C) 2011-2013 Renesas Solutions Corp. > > > + * Copyright (C) 2013 Cogent Embedded, Inc., <source@xxxxxxxxxxxxxxxxxx> > > > + * Copyright (C) 2008 Magnus Damm > > > + */ > > > + > > > +#include <linux/clk.h> > > > +#include <linux/module.h> > > > +#include <linux/mod_devicetable.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > +#include <linux/of_graph.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/pm_runtime.h> > > > + > > > +#include <media/v4l2-fwnode.h> > > > +#include <media/v4l2-mc.h> > > > + > > > +#include "rzg2l-cru.h" > > > + > > > +static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n) > > > +{ > > > + return container_of(n, struct rzg2l_cru_dev, notifier); > > > +} > > > + > > > +static int rzg2l_cru_csi2_link_notify(struct media_link *link, u32 flags, > > > + unsigned int notification) > > > +{ > > > + struct media_entity *entity; > > > + struct rzg2l_cru_dev *cru; > > > + int ret; > > > + > > > + ret = v4l2_pipeline_link_notify(link, flags, notification); > > > + if (ret) > > > + return ret; > > > + > > > + /* Only care about link enablement for CRU nodes. */ > > > + if (!(flags & MEDIA_LNK_FL_ENABLED)) > > > + return 0; > > > + > > > + cru = container_of(link->graph_obj.mdev, struct rzg2l_cru_dev, mdev); > > > + /* > > > + * Don't allow link changes if any entity in the graph is > > > + * streaming, modifying the CHSEL register fields can disrupt > > > + * running streams. > > > + */ > > > + media_device_for_each_entity(entity, &cru->mdev) > > > + if (media_entity_is_streaming(entity)) > > > + return -EBUSY; > > > + > > > + mutex_lock(&cru->mdev_lock); > > > + if (media_pad_remote_pad_first(&cru->vdev.entity.pads[0])) > > > + ret = -EMLINK; > > > + mutex_unlock(&cru->mdev_lock); > > > + > > > + return ret; > > > +} > > > + > > > +static const struct media_device_ops rzg2l_cru_media_ops = { > > > + .link_notify = rzg2l_cru_csi2_link_notify, > > > +}; > > > + > > > +/* ----------------------------------------------------------------------------- > > > + * Group async notifier > > > + */ > > > + > > > +static int rzg2l_cru_group_notify_complete(struct v4l2_async_notifier *notifier) > > > +{ > > > + struct rzg2l_cru_dev *cru = notifier_to_cru(notifier); > > > + struct media_entity *source, *sink; > > > + int ret; > > > + > > > + ret = media_device_register(&cru->mdev); > > > + if (ret) > > > + return ret; > > > > I'd move the v4l2_device_register() call here, as it's the V4L2 > > counterpart of the media device, and handling them together would be > > best. > > > OK, but now that we plan to get rid of rzg2l_cru_csi2_link_notify() > I'll move this call into rzg2l_cru_dma_register(). > I misread the comment earlier, I will get rid of rzg2l_cru_csi2_link_notify() and use v4l2_pipeline_link_notify() instead. And I will also move the media_device_register() into rzg2l_cru_dma_register(). Cheers, Prabhakar
