linux-kernel - Re: [PATCH v4 2/3] media: synopsys: add driver for the designware mipi csi-2 receiver

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: <03547127-6903-4925-bb49-c77a377bf177@collabora.com>
Date: Fri, 16 Jan 2026 12:52:54 +0100
From: Michael Riesch <michael.riesch@...labora.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: Chaoyi Chen <chaoyi.chen@...k-chips.com>,
 Kever Yang <kever.yang@...k-chips.com>, Frank Li <Frank.li@....com>,
 Mehdi Djait <mehdi.djait@...ux.intel.com>,
 Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Hans Verkuil <hverkuil@...nel.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Collabora Kernel Team <kernel@...labora.com>, linux-media@...r.kernel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] media: synopsys: add driver for the designware
 mipi csi-2 receiver

Hi Sakari,

Thanks for your review!

On 1/16/26 11:10, Sakari Ailus wrote:
> Hi Michael,
> 
> Thanks for the update. A few minor comments below...
> 
> On Thu, Jan 15, 2026 at 07:26:08PM +0100, Michael Riesch via B4 Relay wrote:
>> From: Michael Riesch <michael.riesch@...labora.com>
>>
>> The Synopsys DesignWare MIPI CSI-2 Receiver is a CSI-2 bridge with
>> one input port and one output port. It receives the data with the
>> help of an external MIPI PHY (C-PHY or D-PHY) and passes it to e.g.,
>> the Rockchip Video Capture (VICAP) block on recent Rockchip SoCs.
>>
>> Add a V4L2 subdevice driver for this unit.
>>
>> Signed-off-by: Michael Riesch <michael.riesch@...fvision.net>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
>> Reviewed-by: Mehdi Djait <mehdi.djait@...ux.intel.com>
>> Signed-off-by: Michael Riesch <michael.riesch@...labora.com>
>> ---
>>  MAINTAINERS                                        |   1 +
>>  drivers/media/platform/synopsys/Kconfig            |   1 +
>>  drivers/media/platform/synopsys/Makefile           |   1 +
>>  .../media/platform/synopsys/dw-mipi-csi2/Kconfig   |  17 +
>>  .../media/platform/synopsys/dw-mipi-csi2/Makefile  |   2 +
>>  .../platform/synopsys/dw-mipi-csi2/dw-mipi-csi2.c  | 735 +++++++++++++++++++++
> 
> How about dw-mipi-csi2rx (or dw-csi2rx)? There might be a tx chip, too...

Heiko and I had an offlist name bikeshedding discussion, and he already
responded to that concern. Please find the rationale for this there.

> 
>>  [...]
>> +static int dw_mipi_csi2_disable_streams(struct v4l2_subdev *sd,
>> +					struct v4l2_subdev_state *state,
>> +					u32 pad, u64 streams_mask)
>> +{
>> +	struct dw_mipi_csi2_device *csi2 = to_csi2(sd);
>> +	struct v4l2_subdev *remote_sd;
>> +	struct media_pad *sink_pad, *remote_pad;
>> +	struct device *dev = csi2->dev;
>> +	u64 mask;
>> +	int ret;
>> +
>> +	sink_pad = &sd->entity.pads[DW_MIPI_CSI2_PAD_SINK];
>> +	remote_pad = media_pad_remote_pad_first(sink_pad);
>> +	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
>> +
>> +	mask = v4l2_subdev_state_xlate_streams(state, DW_MIPI_CSI2_PAD_SINK,
>> +					       DW_MIPI_CSI2_PAD_SRC,
>> +					       &streams_mask);
>> +
>> +	ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
>> +
>> +	dw_mipi_csi2_stop(csi2);
>> +
>> +	pm_runtime_put_sync(dev);
> 
> How about just pm_runtime_put()?

Ah right, for some reason I used the _sync variant in the error path of
_enable_streams and used it here, too.

But I can change both instances to plain _put.


> [...]
>> +static struct platform_driver dw_mipi_csi2_drv = {
>> +	.driver = {
>> +		   .name = "dw-mipi-csi2",
>> +		   .of_match_table = dw_mipi_csi2_of_match,
>> +		   .pm = &dw_mipi_csi2_pm_ops,
> 
> Just tabs, please.

Oops! Not sure how this could happen. Sorry for that, will fix!

I'll send out v5 soon.

Best regards,
Michael

> 
>> +	},
>> +	.probe = dw_mipi_csi2_probe,
>> +	.remove = dw_mipi_csi2_remove,
>> +};
>> +module_platform_driver(dw_mipi_csi2_drv);
>> +
>> +MODULE_DESCRIPTION("Synopsys DesignWare MIPI CSI-2 Receiver platform driver");
>> +MODULE_LICENSE("GPL");
>>
>