From mboxrd@z Thu Jan 1 00:00:00 1970 From: stefan@agner.ch (Stefan Agner) Date: Tue, 29 Mar 2016 00:11:13 -0700 Subject: [PATCH v2 6/8] drm/fsl-dcu: add TCON driver In-Reply-To: <2234894.ri36D5MRrN@ws-stein> References: <1459216802-32094-1-git-send-email-stefan@agner.ch> <1459216802-32094-7-git-send-email-stefan@agner.ch> <2234894.ri36D5MRrN@ws-stein> Message-ID: <57c86de8eca0aa16a5cdfc488977ca4e@agner.ch> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2016-03-28 23:45, Alexander Stein wrote: > Hi, > > some comments below. > > On Monday 28 March 2016 19:00:00, Stefan Agner wrote: >> Add driver for the TCON (timing controller) module. The TCON module >> is a separate module attached after the DCU (display controller >> unit). Each DCU instance has its own, directly connected TCON >> instance. The DCU's RGB and timing signals are passing through >> the TCON module. TCON can provide timing signals for raw TFT panels >> or operate in a bypass mode which leaves all signals unaltered. >> >> The driver currently only supports the bypass mode. >> >> Signed-off-by: Stefan Agner > >> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt >> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt >> @@ -14,6 +14,7 @@ Required properties: >> Optional properties: >> - clocks: Second handle for pixel clock. >> - clock-names: Second name "pix" for pixel clock. >> +- fsl,tcon: The phandle to the timing controller node. > > Maybe a comment this is (currently) only for Vybrid, but not LS1021A. > IMHO, such references to SoCs in a peripheral binding is somewhat misplaced. If we would start doing this, we would end up in lots of SoC references, and nobody knows whether they are actually accurate and up to date... And I think we should add tcon to LS1021a, more on that below... >> --- /dev/null >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c >> [...] >> +struct fsl_tcon *fsl_tcon_init(struct device *dev) >> +{ >> + struct fsl_tcon *tcon; >> + struct device_node *np; >> + int ret; >> + >> + tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL); >> + if (!tcon) >> + return NULL; >> + >> + np = of_parse_phandle(dev->of_node, "fsl,tcon", 0); >> + if (!np) { >> + dev_warn(dev, "Couldn't find the tcon node\n"); >> + return NULL; >> + } > > Maybe check for device tree node before allocating struct fsl_tcon? Also this > should not be a warning, as on LS1021A this is to be expected. Agreed, definitely the smarter sequence. Afact LS1021a has also a TCON. At least in Jianwei Wangs initial patches it was part of the driver, see: https://lkml.org/lkml/2015/7/13/242 Not sure how that driver can work without TCON support on LS1021a, maybe the bootloader sets the TCON to bypass? Since I do not have a LS1021a, I hesitated to just add it to the LS1021s dt's, but it should be fairly straight forward. -- Stefan From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Agner Subject: Re: [PATCH v2 6/8] drm/fsl-dcu: add TCON driver Date: Tue, 29 Mar 2016 00:11:13 -0700 Message-ID: <57c86de8eca0aa16a5cdfc488977ca4e@agner.ch> References: <1459216802-32094-1-git-send-email-stefan@agner.ch> <1459216802-32094-7-git-send-email-stefan@agner.ch> <2234894.ri36D5MRrN@ws-stein> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <2234894.ri36D5MRrN@ws-stein> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Alexander Stein Cc: meng.yi@nxp.com, pawel.moll@arm.com, alison.wang@freescale.com, daniel.vetter@ffwll.ch, mturquette@baylibre.com, ijc+devicetree@hellion.org.uk, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, robh+dt@kernel.org, kernel@pengutronix.de, galak@codeaurora.org, mark.rutland@arm.com, shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org T24gMjAxNi0wMy0yOCAyMzo0NSwgQWxleGFuZGVyIFN0ZWluIHdyb3RlOgo+IEhpLAo+IAo+IHNv bWUgY29tbWVudHMgYmVsb3cuCj4gCj4gT24gTW9uZGF5IDI4IE1hcmNoIDIwMTYgMTk6MDA6MDAs IFN0ZWZhbiBBZ25lciB3cm90ZToKPj4gQWRkIGRyaXZlciBmb3IgdGhlIFRDT04gKHRpbWluZyBj b250cm9sbGVyKSBtb2R1bGUuIFRoZSBUQ09OIG1vZHVsZQo+PiBpcyBhIHNlcGFyYXRlIG1vZHVs ZSBhdHRhY2hlZCBhZnRlciB0aGUgRENVIChkaXNwbGF5IGNvbnRyb2xsZXIKPj4gdW5pdCkuIEVh Y2ggRENVIGluc3RhbmNlIGhhcyBpdHMgb3duLCBkaXJlY3RseSBjb25uZWN0ZWQgVENPTgo+PiBp bnN0YW5jZS4gVGhlIERDVSdzIFJHQiBhbmQgdGltaW5nIHNpZ25hbHMgYXJlIHBhc3NpbmcgdGhy b3VnaAo+PiB0aGUgVENPTiBtb2R1bGUuIFRDT04gY2FuIHByb3ZpZGUgdGltaW5nIHNpZ25hbHMg Zm9yIHJhdyBURlQgcGFuZWxzCj4+IG9yIG9wZXJhdGUgaW4gYSBieXBhc3MgbW9kZSB3aGljaCBs ZWF2ZXMgYWxsIHNpZ25hbHMgdW5hbHRlcmVkLgo+Pgo+PiBUaGUgZHJpdmVyIGN1cnJlbnRseSBv bmx5IHN1cHBvcnRzIHRoZSBieXBhc3MgbW9kZS4KPj4KPj4gU2lnbmVkLW9mZi1ieTogU3RlZmFu IEFnbmVyIDxzdGVmYW5AYWduZXIuY2g+Cj4gCj4+IC0tLSBhL0RvY3VtZW50YXRpb24vZGV2aWNl dHJlZS9iaW5kaW5ncy9kaXNwbGF5L2ZzbCxkY3UudHh0Cj4+ICsrKyBiL0RvY3VtZW50YXRpb24v ZGV2aWNldHJlZS9iaW5kaW5ncy9kaXNwbGF5L2ZzbCxkY3UudHh0Cj4+IEBAIC0xNCw2ICsxNCw3 IEBAIFJlcXVpcmVkIHByb3BlcnRpZXM6Cj4+ICBPcHRpb25hbCBwcm9wZXJ0aWVzOgo+PiAgLSBj bG9ja3M6CQlTZWNvbmQgaGFuZGxlIGZvciBwaXhlbCBjbG9jay4KPj4gIC0gY2xvY2stbmFtZXM6 CQlTZWNvbmQgbmFtZSAicGl4IiBmb3IgcGl4ZWwgY2xvY2suCj4+ICstIGZzbCx0Y29uOgkJVGhl IHBoYW5kbGUgdG8gdGhlIHRpbWluZyBjb250cm9sbGVyIG5vZGUuCj4gCj4gTWF5YmUgYSBjb21t ZW50IHRoaXMgaXMgKGN1cnJlbnRseSkgb25seSBmb3IgVnlicmlkLCBidXQgbm90IExTMTAyMUEu Cj4gCgpJTUhPLCBzdWNoIHJlZmVyZW5jZXMgdG8gU29DcyBpbiBhIHBlcmlwaGVyYWwgYmluZGlu ZyBpcyBzb21ld2hhdAptaXNwbGFjZWQuIElmIHdlIHdvdWxkIHN0YXJ0IGRvaW5nIHRoaXMsIHdl IHdvdWxkIGVuZCB1cCBpbiBsb3RzIG9mIFNvQwpyZWZlcmVuY2VzLCBhbmQgbm9ib2R5IGtub3dz IHdoZXRoZXIgdGhleSBhcmUgYWN0dWFsbHkgYWNjdXJhdGUgYW5kIHVwCnRvIGRhdGUuLi4gQW5k IEkgdGhpbmsgd2Ugc2hvdWxkIGFkZCB0Y29uIHRvIExTMTAyMWEsIG1vcmUgb24gdGhhdApiZWxv dy4uLgoKPj4gLS0tIC9kZXYvbnVsbAo+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZnNsLWRjdS9m c2xfdGNvbi5jCj4+IFsuLi5dCj4+ICtzdHJ1Y3QgZnNsX3Rjb24gKmZzbF90Y29uX2luaXQoc3Ry dWN0IGRldmljZSAqZGV2KQo+PiArewo+PiArCXN0cnVjdCBmc2xfdGNvbiAqdGNvbjsKPj4gKwlz dHJ1Y3QgZGV2aWNlX25vZGUgKm5wOwo+PiArCWludCByZXQ7Cj4+ICsKPj4gKwl0Y29uID0gZGV2 bV9remFsbG9jKGRldiwgc2l6ZW9mKCp0Y29uKSwgR0ZQX0tFUk5FTCk7Cj4+ICsJaWYgKCF0Y29u KQo+PiArCQlyZXR1cm4gTlVMTDsKPj4gKwo+PiArCW5wID0gb2ZfcGFyc2VfcGhhbmRsZShkZXYt Pm9mX25vZGUsICJmc2wsdGNvbiIsIDApOwo+PiArCWlmICghbnApIHsKPj4gKwkJZGV2X3dhcm4o ZGV2LCAiQ291bGRuJ3QgZmluZCB0aGUgdGNvbiBub2RlXG4iKTsKPj4gKwkJcmV0dXJuIE5VTEw7 Cj4+ICsJfQo+IAo+IE1heWJlIGNoZWNrIGZvciBkZXZpY2UgdHJlZSBub2RlIGJlZm9yZSBhbGxv Y2F0aW5nIHN0cnVjdCBmc2xfdGNvbj8gQWxzbyB0aGlzIAo+IHNob3VsZCBub3QgYmUgYSB3YXJu aW5nLCBhcyBvbiBMUzEwMjFBIHRoaXMgaXMgdG8gYmUgZXhwZWN0ZWQuCgpBZ3JlZWQsIGRlZmlu aXRlbHkgdGhlIHNtYXJ0ZXIgc2VxdWVuY2UuCgpBZmFjdCBMUzEwMjFhIGhhcyBhbHNvIGEgVENP Ti4gQXQgbGVhc3QgaW4gSmlhbndlaSBXYW5ncyBpbml0aWFsIHBhdGNoZXMKaXQgd2FzIHBhcnQg b2YgdGhlIGRyaXZlciwgc2VlOgpodHRwczovL2xrbWwub3JnL2xrbWwvMjAxNS83LzEzLzI0MgoK Tm90IHN1cmUgaG93IHRoYXQgZHJpdmVyIGNhbiB3b3JrIHdpdGhvdXQgVENPTiBzdXBwb3J0IG9u IExTMTAyMWEsIG1heWJlCnRoZSBib290bG9hZGVyIHNldHMgdGhlIFRDT04gdG8gYnlwYXNzPwoK U2luY2UgSSBkbyBub3QgaGF2ZSBhIExTMTAyMWEsIEkgaGVzaXRhdGVkIHRvIGp1c3QgYWRkIGl0 IHRvIHRoZSBMUzEwMjFzCmR0J3MsIGJ1dCBpdCBzaG91bGQgYmUgZmFpcmx5IHN0cmFpZ2h0IGZv cndhcmQuCgotLQpTdGVmYW4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0 b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJp LWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755972AbcC2HOZ (ORCPT ); Tue, 29 Mar 2016 03:14:25 -0400 Received: from mail.kmu-office.ch ([178.209.48.109]:37828 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752560AbcC2HOX (ORCPT ); Tue, 29 Mar 2016 03:14:23 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Tue, 29 Mar 2016 00:11:13 -0700 From: Stefan Agner To: Alexander Stein Cc: dri-devel@lists.freedesktop.org, shawnguo@kernel.org, kernel@pengutronix.de, airlied@linux.ie, daniel.vetter@ffwll.ch, jianwei.wang.chn@gmail.com, alison.wang@freescale.com, meng.yi@nxp.com, mturquette@baylibre.com, sboyd@codeaurora.org, mark.rutland@arm.com, robh+dt@kernel.org, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 6/8] drm/fsl-dcu: add TCON driver In-Reply-To: <2234894.ri36D5MRrN@ws-stein> References: <1459216802-32094-1-git-send-email-stefan@agner.ch> <1459216802-32094-7-git-send-email-stefan@agner.ch> <2234894.ri36D5MRrN@ws-stein> Message-ID: <57c86de8eca0aa16a5cdfc488977ca4e@agner.ch> User-Agent: Roundcube Webmail/1.1.3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-03-28 23:45, Alexander Stein wrote: > Hi, > > some comments below. > > On Monday 28 March 2016 19:00:00, Stefan Agner wrote: >> Add driver for the TCON (timing controller) module. The TCON module >> is a separate module attached after the DCU (display controller >> unit). Each DCU instance has its own, directly connected TCON >> instance. The DCU's RGB and timing signals are passing through >> the TCON module. TCON can provide timing signals for raw TFT panels >> or operate in a bypass mode which leaves all signals unaltered. >> >> The driver currently only supports the bypass mode. >> >> Signed-off-by: Stefan Agner > >> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt >> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt >> @@ -14,6 +14,7 @@ Required properties: >> Optional properties: >> - clocks: Second handle for pixel clock. >> - clock-names: Second name "pix" for pixel clock. >> +- fsl,tcon: The phandle to the timing controller node. > > Maybe a comment this is (currently) only for Vybrid, but not LS1021A. > IMHO, such references to SoCs in a peripheral binding is somewhat misplaced. If we would start doing this, we would end up in lots of SoC references, and nobody knows whether they are actually accurate and up to date... And I think we should add tcon to LS1021a, more on that below... >> --- /dev/null >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c >> [...] >> +struct fsl_tcon *fsl_tcon_init(struct device *dev) >> +{ >> + struct fsl_tcon *tcon; >> + struct device_node *np; >> + int ret; >> + >> + tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL); >> + if (!tcon) >> + return NULL; >> + >> + np = of_parse_phandle(dev->of_node, "fsl,tcon", 0); >> + if (!np) { >> + dev_warn(dev, "Couldn't find the tcon node\n"); >> + return NULL; >> + } > > Maybe check for device tree node before allocating struct fsl_tcon? Also this > should not be a warning, as on LS1021A this is to be expected. Agreed, definitely the smarter sequence. Afact LS1021a has also a TCON. At least in Jianwei Wangs initial patches it was part of the driver, see: https://lkml.org/lkml/2015/7/13/242 Not sure how that driver can work without TCON support on LS1021a, maybe the bootloader sets the TCON to bypass? Since I do not have a LS1021a, I hesitated to just add it to the LS1021s dt's, but it should be fairly straight forward. -- Stefan