Hi, On Fri, Mar 01, 2019 at 11:39:06PM +0100, Sam Ravnborg wrote:
Hi Guido.
Thanks for addressing review comments in first round. Just a few nits in this follow-up. With these nits addressed: Reviewed-by: Sam Ravnborg sam@ravnborg.org
Thanks! I made all the suggested changes in v3, on top of that I prepended 'panel-rockteck-' to the driver name to be more consistent with other drivers. -- Guido
On Fri, Mar 01, 2019 at 02:02:04PM +0100, Guido Günther wrote:
+#include <video/display_timing.h>
This include file is, as far as I could tell, no longer used and can be dropped.
+static int jh057n_get_modes(struct drm_panel *panel) +{
- struct drm_display_mode *mode;
- mode = drm_mode_duplicate(panel->drm, &default_mode);
- if (!mode) {
DRM_ERROR("Failed to add mode %ux%u@%u",default_mode.hdisplay, default_mode.vdisplay,default_mode.vrefresh);return -ENOMEM;Use DRM_DEV_ERROR() You can find dev via: panel->base.drm-dev
+static int jh057n_probe(struct mipi_dsi_device *dsi) +{
- struct device *dev = &dsi->dev;
- struct jh057n *ctx;
- int ret;
- ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
- if (!ctx)
return -ENOMEM;- ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(ctx->reset_gpio)) {
DRM_DEV_ERROR(dev, "cannot get reset gpio");return PTR_ERR(ctx->reset_gpio);- }
- mipi_dsi_set_drvdata(dsi, ctx);
- ctx->dev = dev;
- dsi->lanes = 4;
- dsi->format = MIPI_DSI_FMT_RGB888;
- dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;- ctx->backlight = devm_of_find_backlight(dev);
- if (IS_ERR(ctx->backlight))
return PTR_ERR(ctx->backlight);- drm_panel_init(&ctx->panel);
- ctx->panel.dev = dev;
- ctx->panel.funcs = &jh057n_drm_funcs;
- drm_panel_add(&ctx->panel);
- ret = mipi_dsi_attach(dsi);
- if (ret < 0) {
DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?");drm_panel_remove(&ctx->panel);return ret;- }
- DRM_INFO(DRV_NAME "_panel %ux%u@%u %ubpp dsi %udl - ready",
default_mode.hdisplay, default_mode.vdisplay,default_mode.vrefresh,mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes);You already have dev, so use DRM_DEV_INFO() and drop DRV_NAME
+static const struct of_device_id jh057n_of_match[] = {
- { .compatible = "rocktech,jh057n00900" },
- { /* Sentinel */ }
Lower case 's' (sorry, likely my bad)
- .probe = jh057n_probe,
- .remove = jh057n_remove,
- .shutdown = jh057n_shutdown,
- .driver = {
.name = DRV_NAME "_panel",Drop "_panel" postfix. Other drivers do not use it.
Sam