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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 2 Aug 2013 10:28:56 +0200
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	Tomi Valkeinen <tomi.valkeinen@...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 Tomi,

On Thu, Aug 01, 2013 at 03:39:27PM +0300, Tomi Valkeinen wrote:
> 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.

Aaah, yes indeed. Thanks for pointing that out.

> - The command arrays could be const

True.

> - 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.

Hmm, Ok.

> - 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.

Yes, CDF would be really great for this :)

Alexandre and I will try to find some time to do the cleanups you
requested.

> 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).

Great, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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