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 <s...@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 > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel