linux-kernel - Re: [PATCHv3 3/3] fb: backlight: HX8357: Add HX8369 support

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]

Message-ID: <51FA56FF.8090308@ti.com>
Date:	Thu, 1 Aug 2013 15:39:27 +0300
From:	Tomi Valkeinen <tomi.valkeinen@...com>
To:	Maxime Ripard <maxime.ripard@...e-electrons.com>
CC:	<plagnioj@...osoft.com>,
	Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
	<hector.palacios@...i.com>,
	Thomas Petazzoni <thomas@...e-electrons.com>,
	<linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-fbdev@...r.kernel.org>
Subject: Re: [PATCHv3 3/3] fb: backlight: HX8357: Add HX8369 support

Hi,

On 01/08/13 11:40, Maxime Ripard wrote:
> From: Alexandre Belloni <alexandre.belloni@...e-electrons.com>
> 
> Add support for the Himax HX8369 controller as it is quite similar to the
> hx8357.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@...e-electrons.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@...e-electrons.com>
> Acked-by: Jingoo Han <jg1.han@...sung.com>
> ---
>  drivers/video/backlight/hx8357.c | 219 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 204 insertions(+), 15 deletions(-)

I don't have comments about this patch as such, but hx8357.c in general:

- The commands used look like MIPI DCS commands. We have defined those
in include/video/mipi_display.h.

- The command arrays could be const

- I don't like command arrays. Rather than having array for
"HX8357_SET_COLUMN_ADDRESS, 0x00, 0x00, 0x01, 0x3f,", I'd much more like
a function hx8357_set_column_address(int x1, int x2) or such.

- Looking at the driver makes me think it resembles very much the panel
driver(s) we have for OMAP. If only CDF was ready... ;)

Those said, I don't see a problem with applying this. We could clean up
later those things mentioned above, but maybe it's better to do that
when moving to CDF.

For this and the two other patches:

Acked-by: Tomi Valkeinen <tomi.valkeinen@...com>

I can apply these to my 3.12-fbdev branch some day soon, if
Jean-Christophe hasn't come back yet (I'd normally rather deal only with
trivial patches, and leave the rest to Jean-Christophe).

 Tomi



Download attachment "signature.asc" of type "application/pgp-signature" (902 bytes)