Messages in this thread | | | Date | Fri, 1 Mar 2019 23:39:06 +0100 | From | Sam Ravnborg <> | Subject | Re: [PATCH v2 3/3] drm/panel: Add Rocktech jh057n00900 panel driver |
| |
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>
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
| |