Hi, On Thu, Jun 10, 2021 at 3:39 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > On Fri, Jun 11, 2021 at 12:30 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > > > + mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, 0x0a); > > > > I would still prefer it if there was some type of error checking since > > SPI commands can fail and could potentially fail silently. What about > > at least this (untested): > > > > #define db7430_dbi_cmd(_db, _cmd, _seq...) \ > > do { > > int _ret = mipi_dbi_command(_db->dbi, _cmd, _seq); > > if (_ret) > > dev_warn(_db->dev, "DBI cmd %d failed (%d)\n", _cmd, _ret); > > } while (0) > > > > Then at least you know _something_ will show up in the logs if there's > > a transfer failure instead of silence? > > > > If you truly don't want the error checking then I guess I won't > > insist, but it feels like the kind of thing that will bite someone > > eventually... In any case, I'm happy to add this now (especially since > > the DBI stuff is Acked now). > > This looks more like something that should be done in > mipi_dbi_command() in include/drm/drm_mipi_dbi.h > which claims: > > * Returns: > * Zero on success, negative error code on failure. > */ > > But no it does not return anything: > > #define mipi_dbi_command(dbi, cmd, seq...) \ > ({ \ > const u8 d[] = { seq }; \ > mipi_dbi_command_stackbuf(dbi, cmd, d, ARRAY_SIZE(d)); \ > }) > > I'll fix up the include and apply then we can think about > what to do with mipi_dbi_command(). Are you sure that doesn't work? Isn't the return value of a macro the last expression? In this case the return value of mipi_dbi_command_stackbuf() should just flow through. -Doug