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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 15 Dec 2019 09:29:16 +0100
From:   Sam Ravnborg <sam@...nborg.org>
To:     Heiko Stübner <heiko@...ech.de>
Cc:     thierry.reding@...il.com, mark.rutland@....com,
        devicetree@...r.kernel.org,
        Heiko Stuebner <heiko.stuebner@...obroma-systems.com>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        robh+dt@...nel.org, christoph.muellner@...obroma-systems.com
Subject: Re: [PATCH 3/3] drm/panel: add panel driver for Xinpeng XPP055C272
 panels

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