linux-kernel - Re: [PATCH] Staging: fbtft: fix space errors

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] [day] [month] [year] [list]

Message-ID: <54F4BC85.8080402@tronnes.org>
Date:	Mon, 02 Mar 2015 20:39:49 +0100
From:	Noralf Trønnes <noralf@...nnes.org>
To:	Andrey Skvortsov <Andrej.Skvortzov@...il.com>,
	Joe Perches <joe@...ches.com>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	devel@...verdev.osuosl.org, gregkh@...uxfoundation.org,
	Matteo Semenzato <mattew8898@...il.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Staging: fbtft: fix space errors


Den 02.03.2015 20:21, skrev Andrey Skvortsov:
> On 02 Mar, Joe Perches wrote:
>> On Mon, 2015-03-02 at 12:37 +0300, Dan Carpenter wrote:
>>> On Sat, Feb 28, 2015 at 06:59:19AM -0800, Joe Perches wrote:
>>>> If you're really going to change these, please
>>>> remove the unnecessary \ line continuations
>>>> indent the blocks properly and group the blocks
>>>> more intelligibly.  Maybe something like;
>>>>
>>>> static const int st7735r_init[] = {
>>>> 	-1, 0x01,
>> []
>>>> 	-2, 10,
>>>> 	-3
>>>> };
>>> What's the logic here?  Why are we putting the negatives first?
>> Those are delimiters.  see fbtft-core.c:fbtft_init_display().
>>
>> As far as I understand:
>>
>> -1, start of block
>> -2, millisecond delay after block write
>> -3, end of blocks
>>
>> Beyond that, I don't much care.
>> I just prefer intelligible over apparently random.
> If it is correct, then it would be better to replace these magic numbers with meaningful
> defines.
>

I agree with that in principal, but I'm not convinced that there is any
point in cleaning up this. It should be reworked in another way.
One problem with these drivers is that they are controller centric.
They have a default init sequence matching some, but not all display panels.
The result of this is that in the Device Tree case, the (ugly) init sequence
is passed in through a DT property string. This will never be accepted by
the DT guys. Register init seq. in any form in DT seem to be generally not
accepted. So I guess the drivers have to be display centric instead.
Each having hardcoded the register init sequence with necessary delays.

I have made an attempt to address this in a RFC I posted earlier today:

staging: fbtft: minimize coupling to the fbdev subsystem
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-March/065985.html


Noralf.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/