Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760620AbbBIKkN (ORCPT ); Mon, 9 Feb 2015 05:40:13 -0500 Received: from mailgw1.uni-kl.de ([131.246.120.220]:55886 "EHLO mailgw1.uni-kl.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759464AbbBIKkK (ORCPT ); Mon, 9 Feb 2015 05:40:10 -0500 Message-ID: <1423478275.2541.5.camel@ag-ott-thomas.physik.uni-kl.de> Subject: Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree From: Thomas =?ISO-8859-1?Q?Niederpr=FCm?= Reply-To: niederp@physik.uni-kl.de To: Noralf =?ISO-8859-1?Q?Tr=F8nnes?= Cc: Maxime Ripard , linux-fbdev@vger.kernel.org, plagnioj@jcrosoft.com, tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org Date: Mon, 09 Feb 2015 11:37:55 +0100 In-Reply-To: <54D62D09.8080404@tronnes.org> References: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de> <1423261694-5939-3-git-send-email-niederp@physik.uni-kl.de> <20150207104225.GM2079@lukather> <20150207155933.4f1d0998@maestro.intranet> <54D62D09.8080404@tronnes.org> Organization: TU Kaiserslautern Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5556 Lines: 121 Am Samstag, den 07.02.2015, 16:19 +0100 schrieb Noralf Trønnes: > Hi, > > Den 07.02.2015 15:59, skrev Thomas Niederprüm: > > Am Sat, 7 Feb 2015 11:42:25 +0100 > > schrieb Maxime Ripard : > > > >> Hi, > >> > >> On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de > >> wrote: > >>> From: Thomas Niederprüm > >>> > >>> This patches unifies the init code for the ssd130X chips and > >>> adds device tree bindings to describe the hardware configuration > >>> of the used controller. This gets rid of the magic bit values > >>> used in the init code so far. > >>> > >>> Signed-off-by: Thomas Niederprüm > >>> --- > >>> .../devicetree/bindings/video/ssd1307fb.txt | 11 + > >>> drivers/video/fbdev/ssd1307fb.c | 243 > >>> ++++++++++++++------- 2 files changed, 174 insertions(+), 80 > >>> deletions(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt > >>> b/Documentation/devicetree/bindings/video/ssd1307fb.txt index > >>> 7a12542..1230f68 100644 --- > >>> a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++ > >>> b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@ -15,6 > >>> +15,17 @@ Required properties: > >>> Optional properties: > >>> - reset-active-low: Is the reset gpio is active on physical low? > >>> + - solomon,segment-remap: Invert the order of data column to > >>> segment mapping > >>> + - solomon,offset: Map the display start line to one of COM0 - > >>> COM63 > >>> + - solomon,contrast: Set initial contrast of the display > >>> + - solomon,prechargep1: Set the duration of the precharge period > >>> phase1 > >>> + - solomon,prechargep2: Set the duration of the precharge period > >>> phase2 > >>> + - solomon,com-alt: Enable/disable alternate COM pin configuration > >>> + - solomon,com-lrremap: Enable/disable left-right remap of COM > >>> pins > >>> + - solomon,com-invdir: Invert COM scan direction > >>> + - solomon,vcomh: Set VCOMH regulator voltage > >>> + - solomon,dclk-div: Set display clock divider > >>> + - solomon,dclk-frq: Set display clock frequency > >> I'm sorry, but this is the wrong approach, for at least two reasons: > >> you broke all existing users of that driver, which is a clear no-go, > > Unfortunately this is true. The problem is that the SSD130X controllers > > allow for a very versatile wiring of the display to the controller. > > It's over to the manufacturer of the OLED module (disp+controller) to > > decide how it's actually wired and during device initialization the > > driver has to take care to configure the SSD130X controller according > > to that wiring. If the driver fails to do so you will end up having > > your display showing garbage. Unfortunately the current sate of the > > initialization code of the ssd1307fb driver is not very flexible in that > > respect. Taking a look at the initialization code for the ssd1306 shows > > that it was written with one very special display module in mind. Most > > of the magic bit values set there are non-default values according to > > the datasheet. The result is that the driver works with that one > > particular display module but many other (differently wired) display > > modules using a ssd1306 controller won't work without changing the > > hardcoded magic bit values. > > > > My idea here was to set all configuration to the default values (as > > given in the datasheet) unless it is overwritten by DT. Of course, > > without a change in DT, this breaks the driver for all existing users. > > The only alternative would be to set the current values as default. > > Somehow this feels wrong to me as these values look arbitrary when you > > don't know what exact display module they were set for. But if you > > insist, I will change the default values. > > > >> and the DT itself should not contain any direct mapping of the > >> registers. > >> > > I think I don't get what you mean here. Is it because I do no sanity > > checks of the numbers set in DT? I was just looking for a way to hand > > over the information about the wiring of display to the driver. How > > would you propose to solve this? > > I have the exact same challenge with the staging/fbtft drivers. Even though we are facing the same problem here, yours is much harder than mine, since I have much more knowledge about the controller. Therefor I have no need to completely expose the registers for initialization. I just define all configurable options that the controller has in DT and let the driver take care to write the corresponding register values during initialization. Of course this needs the exact knowledge of the configuration options of the controller as well as register addresses of these options in the driver. So I have the fear that this approach does not scale for a driver handling different controllers. > I have asked about this on the DT list a couple of days ago, but no > answer yet: > > Can I do register initialization from Device Tree? > http://www.spinics.net/lists/devicetree/msg68174.html > > > Regards, > Noralf Trønnes > -- Thomas Niederprüm TU Kaiserslautern FB Physik (AG Ott) Erwin-Schrödinger-Str. 46/431 67663 Kaiserslautern Tel.: 0631 205 2387 Fax: 0631 205 3906 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/