linux-kernel - Re: [RFC PATCH v10 1/2] media: iris: introduce helper module to select video driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dl5gxjxvajoq3tszujqrew7ynvepufcr2qj7ztj7ksgv2fxtdt@zaeqtxuwbayb>
Date: Wed, 29 Jan 2025 13:27:32 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Dikshita Agarwal <quic_dikshita@...cinc.com>
Cc: quic_vgarodia@...cinc.com, quic_abhinavk@...cinc.com, 
	mchehab@...nel.org, robh@...nel.org, krzk+dt@...nel.org, p.zabel@...gutronix.de, 
	hverkuil@...all.nl, sebastian.fricke@...labora.com, bryan.odonoghue@...aro.org, 
	neil.armstrong@...aro.org, nicolas@...fresne.ca, u.kleine-koenig@...libre.com, 
	stefan.schmidt@...aro.org, lujianhua000@...il.com, linux-arm-msm@...r.kernel.org, 
	linux-media@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	krzysztof.kozlowski@...aro.org, johan@...nel.org
Subject: Re: [RFC PATCH v10 1/2] media: iris: introduce helper module to
 select video driver

On Wed, Jan 29, 2025 at 03:23:22PM +0530, Dikshita Agarwal wrote:
> 
> 
> On 1/28/2025 9:44 PM, Dmitry Baryshkov wrote:
> > On Tue, Jan 28, 2025 at 01:34:28PM +0530, Dikshita Agarwal wrote:
> >> Introduce a helper module with a kernel param to select between
> >> venus and iris drivers for platforms supported by both drivers.
> >>
> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
> >> ---
> >>  drivers/media/platform/qcom/Makefile          |  1 +
> >>  drivers/media/platform/qcom/iris/iris_core.h  |  1 +
> >>  drivers/media/platform/qcom/iris/iris_probe.c |  3 +
> >>  drivers/media/platform/qcom/venus/core.c      |  5 ++
> >>  .../platform/qcom/video_drv_helper/Makefile   |  4 ++
> >>  .../qcom/video_drv_helper/video_drv_helper.c  | 70 +++++++++++++++++++
> >>  .../qcom/video_drv_helper/video_drv_helper.h  | 11 +++
> >>  7 files changed, 95 insertions(+)
> >>  create mode 100644 drivers/media/platform/qcom/video_drv_helper/Makefile
> >>  create mode 100644 drivers/media/platform/qcom/video_drv_helper/video_drv_helper.c
> >>  create mode 100644 drivers/media/platform/qcom/video_drv_helper/video_drv_helper.h
> >>
> >> diff --git a/drivers/media/platform/qcom/Makefile b/drivers/media/platform/qcom/Makefile
> >> index ea2221a202c0..15accde3bd67 100644
> >> --- a/drivers/media/platform/qcom/Makefile
> >> +++ b/drivers/media/platform/qcom/Makefile
> >> @@ -2,3 +2,4 @@
> >>  obj-y += camss/
> >>  obj-y += iris/
> >>  obj-y += venus/
> >> +obj-y += video_drv_helper/
> >> diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
> >> index 37fb4919fecc..7108e751ff88 100644
> >> --- a/drivers/media/platform/qcom/iris/iris_core.h
> >> +++ b/drivers/media/platform/qcom/iris/iris_core.h
> >> @@ -107,5 +107,6 @@ struct iris_core {
> >>  
> >>  int iris_core_init(struct iris_core *core);
> >>  void iris_core_deinit(struct iris_core *core);
> >> +extern bool video_drv_should_bind(struct device *dev, bool is_iris_driver);
> > 
> > s/extern //g
> > 
> >>  
> >>  #endif
> >> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> >> index 954cc7c0cc97..276461ade811 100644
> >> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> >> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> >> @@ -196,6 +196,9 @@ static int iris_probe(struct platform_device *pdev)
> >>  	u64 dma_mask;
> >>  	int ret;
> >>  
> >> +	if (!video_drv_should_bind(&pdev->dev, true))
> >> +		return -ENODEV;
> >> +
> >>  	core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);
> >>  	if (!core)
> >>  		return -ENOMEM;
> >> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> >> index 77d48578ecd2..b38be7812efe 100644
> >> --- a/drivers/media/platform/qcom/venus/core.c
> >> +++ b/drivers/media/platform/qcom/venus/core.c
> >> @@ -369,12 +369,17 @@ static int venus_add_dynamic_nodes(struct venus_core *core)
> >>  static void venus_remove_dynamic_nodes(struct venus_core *core) {}
> >>  #endif
> >>  
> >> +extern bool video_drv_should_bind(struct device *dev, bool is_iris_driver);
> > 
> > Use #include instead.
> > 
> >> +
> >>  static int venus_probe(struct platform_device *pdev)
> >>  {
> >>  	struct device *dev = &pdev->dev;
> >>  	struct venus_core *core;
> >>  	int ret;
> >>  
> >> +	if (!video_drv_should_bind(&pdev->dev, false))
> >> +		return -ENODEV;
> >> +
> >>  	core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
> >>  	if (!core)
> >>  		return -ENOMEM;
> >> diff --git a/drivers/media/platform/qcom/video_drv_helper/Makefile b/drivers/media/platform/qcom/video_drv_helper/Makefile
> >> new file mode 100644
> >> index 000000000000..82567e0392fb
> >> --- /dev/null
> >> +++ b/drivers/media/platform/qcom/video_drv_helper/Makefile
> >> @@ -0,0 +1,4 @@
> >> +# Makefile for Video driver helper
> >> +
> >> +obj-m := video_drv_helper.o
> > 
> > Always built as a module? And what if iris or venus are built into the
> > kernel?
> iris and venus are always built as module,

This is not correct.

> and if we are adding the
> dependency of this module on IRIS && VENUS then this can't be Y I think.

It surely can. Moreover, if one doesn't enable both Iris and Venus, this
module is completely unnecessary.

> > Add a normal Kconfig symbol, tristate, no Kconfig string. Use depends on
> > IRIS && VENUS (and maybe default y) to let it be built only if both
> > drivers are enabled.

-- 
With best wishes
Dmitry