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(). Yours, Linus Walleij