Hi, On Sat, Aug 15, 2020 at 12:46:51PM +0200, Sam Ravnborg wrote: > On Sat, Aug 15, 2020 at 12:40:22PM +0200, Guido Günther wrote: > > Hi Sam, > > On Sat, Aug 15, 2020 at 12:02:30PM +0200, Sam Ravnborg wrote: > > > Hi Guido. > > > > > > > +static int mantix_probe(struct mipi_dsi_device *dsi) > > > > +{ > > > > + struct device *dev = &dsi->dev; > > > > + struct mantix *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\n"); > > > > + 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->avdd = devm_regulator_get(dev, "avdd"); > > > > + if (IS_ERR(ctx->avdd)) { > > > > + ret = PTR_ERR(ctx->avdd); > > > > + if (ret != -EPROBE_DEFER) > > > > + DRM_DEV_ERROR(dev, > > > > + "Failed to request avdd > > > > regulator: %d\n", > > > > + ret); > > > > + return ret; > > > > + } > > > > > > Consider to use the recently added dev_err_probe() here and below. > > > Note: Not part of drm-misc-next yet - but hopefully after -rc1 > > > when a backmerge is done. > > > > In fact I did decided against it since i was told that missing dev_* and > > DRM_* logging shouldn't be done. So is that o.k. nowadays? > s/missing/mixing/ > > I often request that logging is consistent - so I recognize the > argument. > > For panel/* I have not made up my mind what I think is the best > approach. The DRM_DEV_* and DRM_* logging do not add much value. > So I have been tempted several times to convert all logging in > panel/ to dev_* and pr_* (when no struct device * is available). > That would also avoid that we mix up logging. > > We have drm_* logging - but they require a valid drm_device * which we > do not have in the the panel drivers. So they are ruled out here. > > Do you have any opinions/comments on this?
I think for panel drivers DRM_* does not give any bonus so moving to {dev,pr}_* sounds good. I just wonder if other drm parts don't need `dev_drm_err_probe()` (or similar) anyway. But then maybe dyn_debug is enough nowadays to not need DRM_DEV_DEBUG_* either? Cheers, -- Guido > > Sam > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel