Hi, On Fri, Jul 2, 2021 at 6:58 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > 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 v2->v3: > - Make the macro actually return the error value if need be, by > putting a single ret; at the end of the macro. (Neat trick from > StackOverflow!) > - Switch the site where I switched mipi_dbi_command() to > mipi_dbi_command_buf() back to what it was. > - Print the failed command in the error message. > - Put the dbi in (parens) since drivers/gpu/drm/tiny/st7586.c was > passing &dbidev->dbi as parameter to mipi_dbi_command() > and this would expand to > struct device *dev = &&dbidev->dbi->spi->dev > which can't be parsed but > struct device *dev = &(&dbidev->dbi)->spi-dev; > should work. I hope. > 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. > --- > include/drm/drm_mipi_dbi.h | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) This seems reasonable to me. There's really no reasonable case I can think of where someone would want to handle the error without it spitting something in the logs. If someone ever comes up with a need for that then we can always add a variant that avoids the logging. ;-) With the commit message fixed as per Noralf: Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
