Hi Joel.
Replying to Noralf's mail as I lost the original mail.
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c new file mode 100644 index 000000000000..e2d1d7497352 --- /dev/null +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: GPL-2.0+ +// Copyright 2018 IBM Corporation
+#include <linux/clk.h> +#include <linux/reset.h> +#include <linux/regmap.h>
+#include <drm/drm_crtc_helper.h> +#include <drm/drm_device.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_simple_kms_helper.h> +#include <drm/drm_gem_framebuffer_helper.h> +#include <drm/drm_panel.h>
I prefer includes sorted alphabetically, but no requirement.
Please sort them as Noralf suggest, as this makes it much more obvious when one is adding a duplicate header.
- priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
- if (IS_ERR(priv->rst)) {
dev_err(&pdev->dev,"missing or invalid reset controller device tree entry");
Add error code to your dev_err() calls, so one later better can tell what was wrong if river fails to load. Same goes for other dev_xxx calls in this function / dirver. (Most dev_xxx have the return code, only a few seems to miss it)
+static const struct of_device_id aspeed_gfx_match[] = {
- { .compatible = "aspeed,ast2400-gfx" },
- { .compatible = "aspeed,ast2500-gfx" },
- { }
Many drivers put a /* sentinel */ comment inside the empty {}
With the few things suggested above considered this is Reviewed-by: Sam Ravnborg sam@ravnborg.org
Sam