Hi Russell,

Thank you for the patch.

On Monday 31 Jul 2017 15:29:51 Russell King wrote:

Add a CEC driver for the dw-hdmi hardware.

Reviewed-by: Neil Armstrong <redacted>
Signed-off-by: Russell King <redacted>
---
 drivers/gpu/drm/bridge/synopsys/Kconfig       |   9 +
 drivers/gpu/drm/bridge/synopsys/Makefile      |   1 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 326 +++++++++++++++++++++++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h |  19 ++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     |  42 +++-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.h     |   1 +
 6 files changed, 397 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
 create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h

[snip]

[snip]

+static int dw_hdmi_cec_probe(struct platform_device *pdev)
+{
+	struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev);
+	struct dw_hdmi_cec *cec;
+	int ret;
+
+	if (!data)
+		return -ENXIO;
+
+	/*
+	 * Our device is just a convenience - we want to link to the real
+	 * hardware device here, so that userspace can see the association
+	 * between the HDMI hardware and its associated CEC chardev.
+	 */
+	cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
+	if (!cec)
+		return -ENOMEM;
+
+	cec->irq = data->irq;
+	cec->ops = data->ops;
+	cec->hdmi = data->hdmi;
+
+	platform_set_drvdata(pdev, cec);
+
+	dw_hdmi_write(cec, 0, HDMI_CEC_TX_CNT);
+	dw_hdmi_write(cec, ~0, HDMI_CEC_MASK);
+	dw_hdmi_write(cec, ~0, HDMI_IH_MUTE_CEC_STAT0);
+	dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY);
+
+	cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi",
+					 CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT 

|

+					 CEC_CAP_RC, CEC_MAX_LOG_ADDRS);
+	if (IS_ERR(cec->adap))
+		return PTR_ERR(cec->adap);
+
+	/* override the module pointer */
+	cec->adap->owner = THIS_MODULE;
+
+	ret = devm_add_action(&pdev->dev, dw_hdmi_cec_del, cec);
+	if (ret) {
+		cec_delete_adapter(cec->adap);
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, cec->irq,
+					dw_hdmi_cec_hardirq,
+					dw_hdmi_cec_thread, IRQF_SHARED,
+					"dw-hdmi-cec", cec->adap);
+	if (ret < 0)
+		return ret;
+
+	cec->notify = cec_notifier_get(pdev->dev.parent);
+	if (!cec->notify)
+		return -ENOMEM;
+
+	ret = cec_register_adapter(cec->adap, pdev->dev.parent);
+	if (ret < 0) {
+		cec_notifier_put(cec->notify);
+		return ret;
+	}
+
+	/*
+	 * CEC documentation says we must not call cec_delete_adapter
+	 * after a successful call to cec_register_adapter().
+	 */
+	devm_remove_action(&pdev->dev, dw_hdmi_cec_del, cec);

dw_hdmi_cec_del() is only used to clean up in the error path of the probe 
function. It would be simpler and less resource-consuming to add an error 
label to this function instead of using devm.

+
+	cec_register_cec_notifier(cec->adap, cec->notify);
+
+	return 0;
+}
+
+static int dw_hdmi_cec_remove(struct platform_device *pdev)
+{
+	struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
+
+	cec_unregister_adapter(cec->adap);
+	cec_notifier_put(cec->notify);
+
+	return 0;
+}
+
+static struct platform_driver dw_hdmi_cec_driver = {
+	.probe	= dw_hdmi_cec_probe,
+	.remove	= dw_hdmi_cec_remove,
+	.driver = {
+		.name = "dw-hdmi-cec",
+	},
+};
+module_platform_driver(dw_hdmi_cec_driver);

Is there a particular reason why this has to be a separate module instead of 
simply calling the CEC init/cleanup functions directly from the main dw-hdmi 
driver ?

+MODULE_AUTHOR("Russell King [off-list ref]");
+MODULE_DESCRIPTION("Synopsys Designware HDMI CEC driver for i.MX");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS(PLATFORM_MODULE_PREFIX "dw-hdmi-cec");

-- 
Regards,

Laurent Pinchart