On 2/23/25 20:54, Christophe JAILLET wrote:
> 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?
Indeed, I'll make it to use devm.
>> + 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?
Yes, it's confusing, but unregister is needed to free the adapter
properly, it's prepared to do it. Thanks for the review.
--
Best regards,
Dmitry
