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

[Christophe JAILLET]

Le 23/02/2025 à 18:30, Dmitry Osipenko a écrit :
> From: Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx>
>
> Add initial support for the Synopsys DesignWare HDMI RX
> Controller Driver used by Rockchip RK3588. The driver
> supports:
>   - HDMI 1.4b and 2.0 modes (HDMI 4k@60Hz)
>   - RGB888, YUV422, YUV444 and YCC420 pixel formats
>   - CEC
>   - EDID configuration
>
> The hardware also has Audio and HDCP capabilities, but these are
> not yet supported by the driver.
>
> Co-developed-by: Dingxian Wen <shawn.wen@xxxxxxxxxxxxxx>
> Signed-off-by: Dingxian Wen <shawn.wen@xxxxxxxxxxxxxx>
> Signed-off-by: Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>

Hi,

> +	hdmirx_dev->dev = dev;
> +	dev_set_drvdata(dev, hdmirx_dev);
> +
> +	ret = hdmirx_parse_dt(hdmirx_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = hdmirx_setup_irq(hdmirx_dev, pdev);
> +	if (ret)
> +		return ret;

From here, should of_reserved_mem_device_release() be called in the 
error handling path, as done in the remove function?

> +
> +	hdmirx_dev->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(hdmirx_dev->regs))
> +		return dev_err_probe(dev, PTR_ERR(hdmirx_dev->regs),
> +				     "failed to remap regs resource\n");

...

> +static const struct of_device_id hdmirx_id[] = {
> +	{ .compatible = "rockchip,rk3588-hdmirx-ctrler" },
> +	{ },

Unneeded trailing comma after a terminator.

> +};
> +MODULE_DEVICE_TABLE(of, hdmirx_id);

...

> +	ret = cec_register_adapter(cec->adap, cec->dev);
> +	if (ret < 0) {
> +		dev_err(cec->dev, "cec register adapter failed\n");
> +		cec_unregister_adapter(cec->adap);

Is it needed to call cec_unregister_adapter() when 
cec_register_adapter() fails?

> +		return NULL;
> +	}
> +
> +	irqs = CECTX_LINE_ERR | CECTX_NACK | CECRX_EOM | CECTX_DONE;
> +	hdmirx_cec_write(cec, CEC_INT_MASK_N, irqs);
> +

CJ