| Message ID | da6f2344396555034cf2476c8338b0ce1c56e0a7.camel@perches.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panel: Rocktech jh057n00900: Add terminating newlines to logging | expand |
Hi Joe, On Thu, Apr 04, 2019 at 08:06:09AM -0700, Joe Perches wrote: > These were missing '\n' terminations, add them. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > index 158a6d548068..d88ea8da2ec2 100644 > --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > @@ -123,7 +123,7 @@ static int jh057n_init_sequence(struct jh057n *ctx) > > ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > if (ret < 0) { > - DRM_DEV_ERROR(dev, "Failed to exit sleep mode"); > + DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n"); > return ret; > } > /* Panel is operational 120 msec after reset */ > @@ -132,7 +132,7 @@ static int jh057n_init_sequence(struct jh057n *ctx) > if (ret) > return ret; > > - DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done"); > + DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done\n"); > return 0; > } > > @@ -172,7 +172,7 @@ static int jh057n_prepare(struct drm_panel *panel) > if (ctx->prepared) > return 0; > > - DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel."); > + DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n"); > gpiod_set_value_cansleep(ctx->reset_gpio, 1); > usleep_range(20, 40); > gpiod_set_value_cansleep(ctx->reset_gpio, 0); > @@ -180,7 +180,8 @@ static int jh057n_prepare(struct drm_panel *panel) > > ret = jh057n_init_sequence(ctx); > if (ret < 0) { > - DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d", ret); > + DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d\n", > + ret); > return ret; > } > > @@ -212,7 +213,7 @@ static int jh057n_get_modes(struct drm_panel *panel) > > mode = drm_mode_duplicate(panel->drm, &default_mode); > if (!mode) { > - DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u", > + DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u\n", > default_mode.hdisplay, default_mode.vdisplay, > default_mode.vrefresh); > return -ENOMEM; > @@ -241,7 +242,7 @@ static int allpixelson_set(void *data, u64 val) > struct jh057n *ctx = data; > struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); > > - DRM_DEV_DEBUG_DRIVER(ctx->dev, "Setting all pixels on"); > + DRM_DEV_DEBUG_DRIVER(ctx->dev, "Setting all pixels on\n"); > dsi_generic_write_seq(dsi, ST7703_CMD_ALL_PIXEL_ON); > msleep(val * 1000); > /* Reset the panel to get video back */ > @@ -290,7 +291,7 @@ static int jh057n_probe(struct mipi_dsi_device *dsi) > > 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"); > + DRM_DEV_ERROR(dev, "cannot get reset gpio\n"); > return PTR_ERR(ctx->reset_gpio); > } > > @@ -315,12 +316,12 @@ static int jh057n_probe(struct mipi_dsi_device *dsi) > > ret = mipi_dsi_attach(dsi); > if (ret < 0) { > - DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?"); > + DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?\n"); > drm_panel_remove(&ctx->panel); > return ret; > } > > - DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready", > + DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready\n", > default_mode.hdisplay, default_mode.vdisplay, > default_mode.vrefresh, > mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes); > Reviewed-By: Guido Günther <agx@sigxcpu.org> Thanks, -- Guido P.S.: The cc: list is huge, is there a reason for going way past what get_maintainers puts out for this patch?
On Thu, 2019-04-04 at 17:48 +0200, Guido Günther wrote: > Hi Joe, [] > P.S.: The cc: list is huge, is there a reason for going way past what > get_maintainers puts out for this patch? No idea, I just used reply-all to your patch.
Hi Joe. On Thu, Apr 04, 2019 at 08:06:09AM -0700, Joe Perches wrote: > These were missing '\n' terminations, add them. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > index 158a6d548068..d88ea8da2ec2 100644 > --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > @@ -123,7 +123,7 @@ static int jh057n_init_sequence(struct jh057n *ctx) > > ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > if (ret < 0) { > - DRM_DEV_ERROR(dev, "Failed to exit sleep mode"); > + DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n"); I was under the impression that newlines was optional these days. Should we always use them with logging? I did not find any obvious clues in linux/printk.h Sam
On Thu, Apr 04, 2019 at 08:06:09AM -0700, Joe Perches wrote: > These were missing '\n' terminations, add them. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) Applied, thanks. Thierry
On Thu, Apr 04, 2019 at 05:48:50PM +0200, Guido Günther wrote: > Hi Joe, > On Thu, Apr 04, 2019 at 08:06:09AM -0700, Joe Perches wrote: > > These were missing '\n' terminations, add them. > > > > Signed-off-by: Joe Perches <joe@perches.com> > > --- > > drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 19 ++++++++++--------- > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > > index 158a6d548068..d88ea8da2ec2 100644 > > --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > > +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > > @@ -123,7 +123,7 @@ static int jh057n_init_sequence(struct jh057n *ctx) > > > > ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > > if (ret < 0) { > > - DRM_DEV_ERROR(dev, "Failed to exit sleep mode"); > > + DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n"); > > return ret; > > } > > /* Panel is operational 120 msec after reset */ > > @@ -132,7 +132,7 @@ static int jh057n_init_sequence(struct jh057n *ctx) > > if (ret) > > return ret; > > > > - DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done"); > > + DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done\n"); > > return 0; > > } > > > > @@ -172,7 +172,7 @@ static int jh057n_prepare(struct drm_panel *panel) > > if (ctx->prepared) > > return 0; > > > > - DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel."); > > + DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n"); > > gpiod_set_value_cansleep(ctx->reset_gpio, 1); > > usleep_range(20, 40); > > gpiod_set_value_cansleep(ctx->reset_gpio, 0); > > @@ -180,7 +180,8 @@ static int jh057n_prepare(struct drm_panel *panel) > > > > ret = jh057n_init_sequence(ctx); > > if (ret < 0) { > > - DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d", ret); > > + DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d\n", > > + ret); > > return ret; > > } > > > > @@ -212,7 +213,7 @@ static int jh057n_get_modes(struct drm_panel *panel) > > > > mode = drm_mode_duplicate(panel->drm, &default_mode); > > if (!mode) { > > - DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u", > > + DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u\n", > > default_mode.hdisplay, default_mode.vdisplay, > > default_mode.vrefresh); > > return -ENOMEM; > > @@ -241,7 +242,7 @@ static int allpixelson_set(void *data, u64 val) > > struct jh057n *ctx = data; > > struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); > > > > - DRM_DEV_DEBUG_DRIVER(ctx->dev, "Setting all pixels on"); > > + DRM_DEV_DEBUG_DRIVER(ctx->dev, "Setting all pixels on\n"); > > dsi_generic_write_seq(dsi, ST7703_CMD_ALL_PIXEL_ON); > > msleep(val * 1000); > > /* Reset the panel to get video back */ > > @@ -290,7 +291,7 @@ static int jh057n_probe(struct mipi_dsi_device *dsi) > > > > 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"); > > + DRM_DEV_ERROR(dev, "cannot get reset gpio\n"); > > return PTR_ERR(ctx->reset_gpio); > > } > > > > @@ -315,12 +316,12 @@ static int jh057n_probe(struct mipi_dsi_device *dsi) > > > > ret = mipi_dsi_attach(dsi); > > if (ret < 0) { > > - DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?"); > > + DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?\n"); > > drm_panel_remove(&ctx->panel); > > return ret; > > } > > > > - DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready", > > + DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready\n", > > default_mode.hdisplay, default_mode.vdisplay, > > default_mode.vrefresh, > > mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes); > > > > Reviewed-By: Guido Günther <agx@sigxcpu.org> Nit: the correct tag is "Reviewed-by:", note the lowercase "by". I only noticed because checkpatch complained, and fixed it up. Thierry
On Thu, 2019-04-04 at 19:00 +0200, Sam Ravnborg wrote: > Hi Joe. > > On Thu, Apr 04, 2019 at 08:06:09AM -0700, Joe Perches wrote: > > These were missing '\n' terminations, add them. > > > > Signed-off-by: Joe Perches <joe@perches.com> > > --- > > drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 19 ++++++++++--------- > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > > index 158a6d548068..d88ea8da2ec2 100644 > > --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > > +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > > @@ -123,7 +123,7 @@ static int jh057n_init_sequence(struct jh057n *ctx) > > > > ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > > if (ret < 0) { > > - DRM_DEV_ERROR(dev, "Failed to exit sleep mode"); > > + DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n"); > > I was under the impression that newlines was optional these days. > Should we always use them with logging? Yes. The general problem is KERN_CONT/pr_cont uses where interleaving is still possible from multiple threads. > I did not find any obvious clues in linux/printk.h I'll see about adding something one day to the Documentation. Likely in coding-style.rst.
Hi Joe. > > > > > > ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > > > if (ret < 0) { > > > - DRM_DEV_ERROR(dev, "Failed to exit sleep mode"); > > > + DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n"); > > > > I was under the impression that newlines was optional these days. > > Should we always use them with logging? > > Yes. > > The general problem is KERN_CONT/pr_cont uses where interleaving > is still possible from multiple threads. > > > I did not find any obvious clues in linux/printk.h > > I'll see about adding something one day to the Documentation. > Likely in coding-style.rst. Thanks for the clarification. A seperate doc on logging would be awesome. Sam
diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c index 158a6d548068..d88ea8da2ec2 100644 --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c @@ -123,7 +123,7 @@ static int jh057n_init_sequence(struct jh057n *ctx) ret = mipi_dsi_dcs_exit_sleep_mode(dsi); if (ret < 0) { - DRM_DEV_ERROR(dev, "Failed to exit sleep mode"); + DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n"); return ret; } /* Panel is operational 120 msec after reset */ @@ -132,7 +132,7 @@ static int jh057n_init_sequence(struct jh057n *ctx) if (ret) return ret; - DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done"); + DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done\n"); return 0; } @@ -172,7 +172,7 @@ static int jh057n_prepare(struct drm_panel *panel) if (ctx->prepared) return 0; - DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel."); + DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n"); gpiod_set_value_cansleep(ctx->reset_gpio, 1); usleep_range(20, 40); gpiod_set_value_cansleep(ctx->reset_gpio, 0); @@ -180,7 +180,8 @@ static int jh057n_prepare(struct drm_panel *panel) ret = jh057n_init_sequence(ctx); if (ret < 0) { - DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d", ret); + DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d\n", + ret); return ret; } @@ -212,7 +213,7 @@ static int jh057n_get_modes(struct drm_panel *panel) mode = drm_mode_duplicate(panel->drm, &default_mode); if (!mode) { - DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u", + DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u\n", default_mode.hdisplay, default_mode.vdisplay, default_mode.vrefresh); return -ENOMEM; @@ -241,7 +242,7 @@ static int allpixelson_set(void *data, u64 val) struct jh057n *ctx = data; struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); - DRM_DEV_DEBUG_DRIVER(ctx->dev, "Setting all pixels on"); + DRM_DEV_DEBUG_DRIVER(ctx->dev, "Setting all pixels on\n"); dsi_generic_write_seq(dsi, ST7703_CMD_ALL_PIXEL_ON); msleep(val * 1000); /* Reset the panel to get video back */ @@ -290,7 +291,7 @@ static int jh057n_probe(struct mipi_dsi_device *dsi) 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"); + DRM_DEV_ERROR(dev, "cannot get reset gpio\n"); return PTR_ERR(ctx->reset_gpio); } @@ -315,12 +316,12 @@ static int jh057n_probe(struct mipi_dsi_device *dsi) ret = mipi_dsi_attach(dsi); if (ret < 0) { - DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?"); + DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?\n"); drm_panel_remove(&ctx->panel); return ret; } - DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready", + DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready\n", default_mode.hdisplay, default_mode.vdisplay, default_mode.vrefresh, mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes);
These were missing '\n' terminations, add them. Signed-off-by: Joe Perches <joe@perches.com> --- drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)