On 08/16/2024, Krzysztof Kozlowski wrote: > On 16/08/2024 10:09, Liu Ying wrote: >> NXP i.MX93 mediamix blk-ctrl contains one DISPLAY_MUX register which >> configures parallel display format by using the "PARALLEL_DISP_FORMAT" >> field. Add a DRM bridge driver to support the display format configuration. >> >> Signed-off-by: Liu Ying <victor.liu@xxxxxxx> >> --- > > ... > >> + >> +static int imx93_pdfc_bridge_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct imx93_pdfc *pdfc; >> + int ret; >> + >> + pdfc = devm_kzalloc(dev, sizeof(*pdfc), GFP_KERNEL); >> + if (!pdfc) >> + return -ENOMEM; >> + >> + pdfc->regmap = syscon_node_to_regmap(dev->of_node->parent); >> + if (IS_ERR(pdfc->regmap)) { >> + ret = PTR_ERR(pdfc->regmap); >> + if (ret != -EPROBE_DEFER) >> + DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret); >> + return ret; > > Nope, you just open-coded dev_err_probe. Syntax is - return > dev_err_probe(). if you need wrapper for DRM, add such. Will use dev_err_probe(). > >> + } >> + >> + pdfc->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0); >> + if (IS_ERR(pdfc->next_bridge)) { >> + ret = PTR_ERR(pdfc->next_bridge); >> + if (ret != -EPROBE_DEFER) >> + DRM_DEV_ERROR(dev, "failed to get next bridge: %d\n", ret); >> + return ret; > > Ditto Will use dev_err_probe(). > > >> + } >> + > > ... > >> +MODULE_DESCRIPTION("NXP i.MX93 parallel display format configuration driver"); >> +MODULE_AUTHOR("Liu Ying <victor.liu@xxxxxxx>"); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_ALIAS("platform:" DRIVER_NAME); > > Which other driver needs this platform alias? Quote include/linux/module.h: " /* For userspace: you can also call me... */ #define MODULE_ALIAS(_alias) MODULE_INFO(alias, _alias) " Anything wrong with using MODULE_ALIAS() here? > > Best regards, > Krzysztof > -- Regards, Liu Ying