Den 02.07.2021 12.04, skrev Linus Walleij:
> The macro mipi_dbi_command() does not report errors unless you wrap it
> in another macro to do the error reporting.
>
> Report a rate-limited error so we know what is going on.
>
> Drop the only user in DRM using mipi_dbi_command() and actually checking
> the error explicitly, let it use mipi_dbi_command_buf() directly
> instead.
>
> After this any code wishing to send command arrays can rely on
> mipi_dbi_command() providing an appropriate error message if something
> goes wrong.
>
> Suggested-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> Suggested-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> ChangeLog v1->v2:
> - Fish out the struct device * from the DBI SPI client and use
> that to print the errors associated with the SPI device.
> ---
> drivers/gpu/drm/drm_mipi_dbi.c | 2 +-
> include/drm/drm_mipi_dbi.h | 6 +++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index 3854fb9798e9..c7c1b75df190 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -645,7 +645,7 @@ static int mipi_dbi_poweron_reset_conditional(struct mipi_dbi_dev *dbidev, bool
> return 1;
>
> mipi_dbi_hw_reset(dbi);
> - ret = mipi_dbi_command(dbi, MIPI_DCS_SOFT_RESET);
> + ret = mipi_dbi_command_buf(dbi, MIPI_DCS_SOFT_RESET, NULL, 0);
> if (ret) {
> DRM_DEV_ERROR(dev, "Failed to send reset command (%d)\n", ret);
> if (dbidev->regulator)
> diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
> index f543d6e3e822..f00cb9690cf2 100644
> --- a/include/drm/drm_mipi_dbi.h
> +++ b/include/drm/drm_mipi_dbi.h
> @@ -183,7 +183,11 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
> #define mipi_dbi_command(dbi, cmd, seq...) \
> ({ \
> const u8 d[] = { seq }; \
> - mipi_dbi_command_stackbuf(dbi, cmd, d, ARRAY_SIZE(d)); \
> + struct device *dev = &dbi->spi->dev; \
> + int ret; \
> + ret = mipi_dbi_command_stackbuf(dbi, cmd, d, ARRAY_SIZE(d)); \
> + if (ret) \
> + dev_err_ratelimited(dev, "error %d when sending command\n", ret); \
Nit: Printing the failing command would have been useful, like you did
in the driver macro.
> })
I would have preferred if mipi_dbi_command could have returned the error
code. This indicates that it should be possible:
https://stackoverflow.com/questions/3532621/using-and-returning-output-in-c-macro
But I can live with this, but if drivers want to start checking the
error code we might have to rethink this.
But this works as things are now:
Reviewed-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
