public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Noralf Trønnes" <noralf@tronnes.org>
To: Andrey Skvortsov <Andrej.Skvortzov@gmail.com>,
	Joe Perches <joe@perches.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
	Matteo Semenzato <mattew8898@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Staging: fbtft: fix space errors
Date: Mon, 02 Mar 2015 20:39:49 +0100	[thread overview]
Message-ID: <54F4BC85.8080402@tronnes.org> (raw)
In-Reply-To: <20150302192120.GA4136@nest>


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.



  reply	other threads:[~2015-03-02 19:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-28 14:44 [PATCH] Staging: fbtft: fix space errors Matteo Semenzato
2015-02-28 14:59 ` Joe Perches
2015-03-02  9:37   ` Dan Carpenter
2015-03-02 14:20     ` Joe Perches
2015-03-02 19:21       ` Andrey Skvortsov
2015-03-02 19:39         ` Noralf Trønnes [this message]
2015-02-28 16:04 ` [PATCH V2] " Matteo Semenzato
  -- strict thread matches above, loose matches on Subject: below --
2015-02-23 16:24 [PATCH] " Matteo Semenzato
2015-03-02  1:16 ` Greg KH
2015-02-22  8:50 Matteo Semenzato

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54F4BC85.8080402@tronnes.org \
    --to=noralf@tronnes.org \
    --cc=Andrej.Skvortzov@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mattew8898@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox