Re: [PATCH 3/3] drm/panel: add panel driver for Xinpeng XPP055C272 panels

[Sam Ravnborg <sam@ravnborg.org>]

Hi Heiko.

> Am Samstag, 14. Dezember 2019, 09:17:30 CET schrieb Sam Ravnborg:
> > > +#define dsi_generic_write_seq(dsi, cmd, seq...) do {			\
> > > +		static const u8 d[] = { seq };				\
> > > +		int ret;						\
> > > +		ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d));	\
> > > +		if (ret < 0)						\
> > > +			return ret;					\
> > > +	} while (0)
> > This macro return an error code if a write fails.
> > 
> > > +
> > > +static int xpp055c272_init_sequence(struct xpp055c272 *ctx)
> > > +{
> > > +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> > > +	struct device *dev = ctx->dev;
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * Init sequence was supplied by the panel vendor without much
> > > +	 * documentation.
> > > +	 */
> > > +	dsi_generic_write_seq(dsi, XPP055C272_CMD_SETEXTC, 0xf1, 0x12, 0x83);
> > But all uses of the macro here ignore the error.
> 
> hmm, am I way off track here?
> 
> 	dsi_generic_write_seq(dsi, XPP055C272_CMD_SETEXTC, 0xf1, 0x12, 0x83);
> 	dsi_generic_write_seq(dsi, XPP055C272_CMD_SETMIPI,
> 			      0x33, 0x81, 0x05, 0xf9, 0x0e, 0x0e, 0x00, 0x00,
> 			      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x44, 0x25,
> 			      0x00, 0x91, 0x0a, 0x00, 0x00, 0x02, 0x4f, 0x01,
> 			      0x00, 0x00, 0x37);
> 	...
> 
> should just expand to
> 
> do {
> 		static const u8 d[] = { 0xf1, 0x12, 0x83 };
> 		int ret;
> 		ret = mipi_dsi_dcs_write(dsi, XPP055C272_CMD_SETEXTC, d, ARRAY_SIZE(d));
> 		if (ret < 0)
> 			return ret;
> } while (0)
> do {
> 		static const u8 d[] = { 0x33, 0x81, 0x05, 0xf9, 0x0e, 0x0e, 0x00, 0x00,
> 			      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x44, 0x25,
> 			      0x00, 0x91, 0x0a, 0x00, 0x00, 0x02, 0x4f, 0x01,
> 			      0x00, 0x00, 0x37 };
> 		int ret;
> 		ret = mipi_dsi_dcs_write(dsi, XPP055C272_CMD_SETMIPI, d, ARRAY_SIZE(d));
> 		if (ret < 0)
> 			return ret;
> } while (0)
> ...
> 
> so every write instance will actually return an error if it happens and not
> continue on with the next init item.
The idea was that if a write returned an error then do not even attempt
more writes. So if a write fails we do not loose the original error
code, assuming subsequent write would also fail.

I have looked through all the panel drivers now, and the majority does
not check if the write goes wrong.
So following the pattern on the other panels you can also decide to just
ignore the return value of mipi_dsi_dcs_write() rahter than trying to
invent the check I tried to explain.

	Sam