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

From: Noralf TrÃnnes
Date: Mon Mar 02 2015 - 14:40:04 EST



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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/