Re: [PATCH v10 3/6] media: platform: synopsys: Add support for HDMI input driver

On 2/26/25 11:47, Hans Verkuil wrote:
...

quoted

+static int hdmirx_register_cec(struct snps_hdmirx_dev *hdmirx_dev,
+			       struct platform_device *pdev)
+{
+	struct device *dev = hdmirx_dev->dev;
+	struct hdmirx_cec_data cec_data;
+	int irq;
+
+	irq = platform_get_irq_byname(pdev, "cec");
+	if (irq < 0) {
+		dev_err_probe(dev, irq, "failed to get cec irq\n");
+		return irq;
+	}
+
+	hdmirx_dev->cec_notifier = cec_notifier_conn_register(dev, NULL, NULL);

I never really paid attention to this, but you don't need to use the cec-notifier
API. The notifier API is needed if an independent CEC device has to be associated
with an independent HDMI interface, and needs to be notified whenever the
physical address changes.

This is a single IP combining both HDMI and CEC, so the HDMI part calls
cec_s_phys_addr_from_edid()/cec_phys_addr_invalidate() directly whenever the EDID
changes.

So just drop the include of cec-notifier.h and anything related to this.

Ack

...

quoted

+	hdmirx_dev->infoframes = v4l2_debugfs_if_alloc(hdmirx_dev->debugfs_dir,
+						       V4L2_DEBUGFS_IF_AVI, hdmirx_dev,
+						       hdmirx_debugfs_if_read);
+
+	return 0;
+
+err_unreg_video_dev:
+	video_unregister_device(&hdmirx_dev->stream.vdev);

Replace this with vb2_video_unregister_device(). Sorry, I missed this in my previous reviews.

quoted

+err_unreg_v4l2_dev:
+	v4l2_device_unregister(&hdmirx_dev->v4l2_dev);
+err_hdl:
+	v4l2_ctrl_handler_free(&hdmirx_dev->hdl);
+err_pm:
+	hdmirx_disable(dev);
+
+	return ret;
+}
+
+static void hdmirx_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct snps_hdmirx_dev *hdmirx_dev = dev_get_drvdata(dev);
+
+	v4l2_debugfs_if_free(hdmirx_dev->infoframes);
+	debugfs_remove_recursive(hdmirx_dev->debugfs_dir);
+
+	snps_hdmirx_cec_unregister(hdmirx_dev->cec);
+	cec_notifier_conn_unregister(hdmirx_dev->cec_notifier);
+
+	hdmirx_disable_irq(dev);
+
+	video_unregister_device(&hdmirx_dev->stream.vdev);

vb2_video_unregister_device().

This function ensures that, if streaming was in progress and the driver was forcibly
removed, streaming is stopped gracefully and safely.

Ack

-- 
Best regards,
Dmitry