Re: [PATCH] Staging: fbtft: fix space errors

Noralf Trønnes
2015-03-02T19:39:59Z

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.