From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Mon, 17 Oct 2016 15:44:48 +0300 Subject: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller In-Reply-To: References: <1475672732-17111-1-git-send-email-bgolaszewski@baylibre.com> <4975084.EGQPv58AK6@avalon> Message-ID: <1615142.iysWriV8vb@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Tomi, On Monday 17 Oct 2016 15:29:23 Tomi Valkeinen wrote: > On 17/10/16 14:40, Laurent Pinchart wrote: > > On Monday 17 Oct 2016 10:33:58 Tomi Valkeinen wrote: > >> On 17/10/16 10:12, Sekhar Nori wrote: > >>> On Monday 17 October 2016 11:26 AM, Tomi Valkeinen wrote: > >>>> On 15/10/16 20:42, Sekhar Nori wrote: > >>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi > >>>>>> b/arch/arm/boot/dts/da850.dtsi > >>>>>> index f79e1b9..32908ae 100644 > >>>>>> --- a/arch/arm/boot/dts/da850.dtsi > >>>>>> +++ b/arch/arm/boot/dts/da850.dtsi > >>>>>> @@ -399,6 +420,14 @@ > >>>>>> <&edma0 0 1>; > >>>>>> dma-names = "tx", "rx"; > >>>>>> }; > >>>>>> + > >>>>>> + display: display at 213000 { > >>>>>> + compatible = "ti,am33xx-tilcdc", "ti,da850- tilcdc"; > >>>>> > >>>>> This should instead be: > >>>>> > >>>>> compatible = "ti,da850-tilcdc", "ti,am33xx-tilcdc"; > >>>>> > >>>>> as the closest match should appear first in the list. > >>>> > >>>> Actually I don't think that's correct. The LCDC on da850 is not > >>>> compatible with the LCDC on AM335x. I think it should be just > >>>> "ti,da850-tilcdc". > >>> > >>> So if "ti,am33xx-tilcdc" is used, the display wont work at all? If thats > >>> the case, I wonder how the patch passed testing. Bartosz? > >> > >> AM3 has "version 2" of LCDC, whereas DA850 is v1. They are quite > >> similar, but different. > >> > >> The driver gets the version number from LCDC's register, and acts based > >> on that, so afaik the compatible string doesn't really affect the > >> functionality (as long as it matches). > >> > >> But even if it works with the current driver, I don't think > >> "ti,am33xx-tilcdc" and "ti,da850-tilcdc" are compatible in the HW level. > > > > If the hardware provides IP revision information, how about just "ti,lcdc" > > ? > > Maybe, and I agree that's the "correct" way, but looking at the history, > it's not just once or twice when we've suddenly found out some > difference or bug or such in an IP revision, or the integration to a > SoC, that can't be found based on the IP revision. > > That's why I feel it's usually safer to have the SoC revision there in > the compatible string. > > That said, we have only a few different old SoCs with LCDC (compared to, > say, OMAP DSS) so in this case perhaps just "ti,lcdc" would be fine. You obviously know more than I do on this topic so I'll trust your opinion. If the version register isn't enough I'm fine with multiple compatible strings. -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Date: Mon, 17 Oct 2016 15:44:48 +0300 Message-ID: <1615142.iysWriV8vb@avalon> References: <1475672732-17111-1-git-send-email-bgolaszewski@baylibre.com> <4975084.EGQPv58AK6@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Tomi Valkeinen Cc: Mark Rutland , Karl Beldan , linux-devicetree , Kevin Hilman , Michael Turquette , Sekhar Nori , Russell King , linux-drm , LKML , Peter Ujfalusi , Bartosz Golaszewski , Rob Herring , Karl Beldan , Jyri Sarha , Maxime Ripard , Frank Rowand , arm-soc List-Id: devicetree@vger.kernel.org SGkgVG9taSwKCk9uIE1vbmRheSAxNyBPY3QgMjAxNiAxNToyOToyMyBUb21pIFZhbGtlaW5lbiB3 cm90ZToKPiBPbiAxNy8xMC8xNiAxNDo0MCwgTGF1cmVudCBQaW5jaGFydCB3cm90ZToKPiA+IE9u IE1vbmRheSAxNyBPY3QgMjAxNiAxMDozMzo1OCBUb21pIFZhbGtlaW5lbiB3cm90ZToKPiA+PiBP biAxNy8xMC8xNiAxMDoxMiwgU2VraGFyIE5vcmkgd3JvdGU6Cj4gPj4+IE9uIE1vbmRheSAxNyBP Y3RvYmVyIDIwMTYgMTE6MjYgQU0sIFRvbWkgVmFsa2VpbmVuIHdyb3RlOgo+ID4+Pj4gT24gMTUv MTAvMTYgMjA6NDIsIFNla2hhciBOb3JpIHdyb3RlOgo+ID4+Pj4+PiBkaWZmIC0tZ2l0IGEvYXJj aC9hcm0vYm9vdC9kdHMvZGE4NTAuZHRzaQo+ID4+Pj4+PiBiL2FyY2gvYXJtL2Jvb3QvZHRzL2Rh ODUwLmR0c2kKPiA+Pj4+Pj4gaW5kZXggZjc5ZTFiOS4uMzI5MDhhZSAxMDA2NDQKPiA+Pj4+Pj4g LS0tIGEvYXJjaC9hcm0vYm9vdC9kdHMvZGE4NTAuZHRzaQo+ID4+Pj4+PiArKysgYi9hcmNoL2Fy bS9ib290L2R0cy9kYTg1MC5kdHNpCj4gPj4+Pj4+IEBAIC0zOTksNiArNDIwLDE0IEBACj4gPj4+ Pj4+ICAJCQkJPCZlZG1hMCAwIDE+Owo+ID4+Pj4+PiAgCQkJZG1hLW5hbWVzID0gInR4IiwgInJ4 IjsKPiA+Pj4+Pj4gIAkJfTsKPiA+Pj4+Pj4gKwo+ID4+Pj4+PiArCQlkaXNwbGF5OiBkaXNwbGF5 QDIxMzAwMCB7Cj4gPj4+Pj4+ICsJCQljb21wYXRpYmxlID0gInRpLGFtMzN4eC10aWxjZGMiLCAi dGksZGE4NTAtCnRpbGNkYyI7Cj4gPj4+Pj4gCj4gPj4+Pj4gVGhpcyBzaG91bGQgaW5zdGVhZCBi ZToKPiA+Pj4+PiAKPiA+Pj4+PiBjb21wYXRpYmxlID0gInRpLGRhODUwLXRpbGNkYyIsICJ0aSxh bTMzeHgtdGlsY2RjIjsKPiA+Pj4+PiAKPiA+Pj4+PiBhcyB0aGUgY2xvc2VzdCBtYXRjaCBzaG91 bGQgYXBwZWFyIGZpcnN0IGluIHRoZSBsaXN0Lgo+ID4+Pj4gCj4gPj4+PiBBY3R1YWxseSBJIGRv bid0IHRoaW5rIHRoYXQncyBjb3JyZWN0LiBUaGUgTENEQyBvbiBkYTg1MCBpcyBub3QKPiA+Pj4+ IGNvbXBhdGlibGUgd2l0aCB0aGUgTENEQyBvbiBBTTMzNXguIEkgdGhpbmsgaXQgc2hvdWxkIGJl IGp1c3QKPiA+Pj4+ICJ0aSxkYTg1MC10aWxjZGMiLgo+ID4+PiAKPiA+Pj4gU28gaWYgInRpLGFt MzN4eC10aWxjZGMiIGlzIHVzZWQsIHRoZSBkaXNwbGF5IHdvbnQgd29yayBhdCBhbGw/IElmIHRo YXRzCj4gPj4+IHRoZSBjYXNlLCBJIHdvbmRlciBob3cgdGhlIHBhdGNoIHBhc3NlZCB0ZXN0aW5n LiBCYXJ0b3N6Pwo+ID4+IAo+ID4+IEFNMyBoYXMgInZlcnNpb24gMiIgb2YgTENEQywgd2hlcmVh cyBEQTg1MCBpcyB2MS4gVGhleSBhcmUgcXVpdGUKPiA+PiBzaW1pbGFyLCBidXQgZGlmZmVyZW50 Lgo+ID4+IAo+ID4+IFRoZSBkcml2ZXIgZ2V0cyB0aGUgdmVyc2lvbiBudW1iZXIgZnJvbSBMQ0RD J3MgcmVnaXN0ZXIsIGFuZCBhY3RzIGJhc2VkCj4gPj4gb24gdGhhdCwgc28gYWZhaWsgdGhlIGNv bXBhdGlibGUgc3RyaW5nIGRvZXNuJ3QgcmVhbGx5IGFmZmVjdCB0aGUKPiA+PiBmdW5jdGlvbmFs aXR5IChhcyBsb25nIGFzIGl0IG1hdGNoZXMpLgo+ID4+IAo+ID4+IEJ1dCBldmVuIGlmIGl0IHdv cmtzIHdpdGggdGhlIGN1cnJlbnQgZHJpdmVyLCBJIGRvbid0IHRoaW5rCj4gPj4gInRpLGFtMzN4 eC10aWxjZGMiIGFuZCAidGksZGE4NTAtdGlsY2RjIiBhcmUgY29tcGF0aWJsZSBpbiB0aGUgSFcg bGV2ZWwuCj4gPiAKPiA+IElmIHRoZSBoYXJkd2FyZSBwcm92aWRlcyBJUCByZXZpc2lvbiBpbmZv cm1hdGlvbiwgaG93IGFib3V0IGp1c3QgInRpLGxjZGMiCj4gPiA/Cj4KPiBNYXliZSwgYW5kIEkg YWdyZWUgdGhhdCdzIHRoZSAiY29ycmVjdCIgd2F5LCBidXQgbG9va2luZyBhdCB0aGUgaGlzdG9y eSwKPiBpdCdzIG5vdCBqdXN0IG9uY2Ugb3IgdHdpY2Ugd2hlbiB3ZSd2ZSBzdWRkZW5seSBmb3Vu ZCBvdXQgc29tZQo+IGRpZmZlcmVuY2Ugb3IgYnVnIG9yIHN1Y2ggaW4gYW4gSVAgcmV2aXNpb24s IG9yIHRoZSBpbnRlZ3JhdGlvbiB0byBhCj4gU29DLCB0aGF0IGNhbid0IGJlIGZvdW5kIGJhc2Vk IG9uIHRoZSBJUCByZXZpc2lvbi4KPiAKPiBUaGF0J3Mgd2h5IEkgZmVlbCBpdCdzIHVzdWFsbHkg c2FmZXIgdG8gaGF2ZSB0aGUgU29DIHJldmlzaW9uIHRoZXJlIGluCj4gdGhlIGNvbXBhdGlibGUg c3RyaW5nLgo+IAo+IFRoYXQgc2FpZCwgd2UgaGF2ZSBvbmx5IGEgZmV3IGRpZmZlcmVudCBvbGQg U29DcyB3aXRoIExDREMgKGNvbXBhcmVkIHRvLAo+IHNheSwgT01BUCBEU1MpIHNvIGluIHRoaXMg Y2FzZSBwZXJoYXBzIGp1c3QgInRpLGxjZGMiIHdvdWxkIGJlIGZpbmUuCgpZb3Ugb2J2aW91c2x5 IGtub3cgbW9yZSB0aGFuIEkgZG8gb24gdGhpcyB0b3BpYyBzbyBJJ2xsIHRydXN0IHlvdXIgb3Bp bmlvbi4gSWYgCnRoZSB2ZXJzaW9uIHJlZ2lzdGVyIGlzbid0IGVub3VnaCBJJ20gZmluZSB3aXRo IG11bHRpcGxlIGNvbXBhdGlibGUgc3RyaW5ncy4KCi0tIApSZWdhcmRzLAoKTGF1cmVudCBQaW5j aGFydAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJp LWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBz Oi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932624AbcJQMoz (ORCPT ); Mon, 17 Oct 2016 08:44:55 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:56325 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934675AbcJQMow (ORCPT ); Mon, 17 Oct 2016 08:44:52 -0400 From: Laurent Pinchart To: Tomi Valkeinen Cc: Sekhar Nori , Bartosz Golaszewski , Kevin Hilman , Michael Turquette , Rob Herring , Frank Rowand , Mark Rutland , Peter Ujfalusi , Russell King , Karl Beldan , LKML , arm-soc , linux-drm , linux-devicetree , Jyri Sarha , David Airlie , Maxime Ripard , Karl Beldan Subject: Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Date: Mon, 17 Oct 2016 15:44:48 +0300 Message-ID: <1615142.iysWriV8vb@avalon> User-Agent: KMail/4.14.10 (Linux/4.4.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: References: <1475672732-17111-1-git-send-email-bgolaszewski@baylibre.com> <4975084.EGQPv58AK6@avalon> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Tomi, On Monday 17 Oct 2016 15:29:23 Tomi Valkeinen wrote: > On 17/10/16 14:40, Laurent Pinchart wrote: > > On Monday 17 Oct 2016 10:33:58 Tomi Valkeinen wrote: > >> On 17/10/16 10:12, Sekhar Nori wrote: > >>> On Monday 17 October 2016 11:26 AM, Tomi Valkeinen wrote: > >>>> On 15/10/16 20:42, Sekhar Nori wrote: > >>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi > >>>>>> b/arch/arm/boot/dts/da850.dtsi > >>>>>> index f79e1b9..32908ae 100644 > >>>>>> --- a/arch/arm/boot/dts/da850.dtsi > >>>>>> +++ b/arch/arm/boot/dts/da850.dtsi > >>>>>> @@ -399,6 +420,14 @@ > >>>>>> <&edma0 0 1>; > >>>>>> dma-names = "tx", "rx"; > >>>>>> }; > >>>>>> + > >>>>>> + display: display@213000 { > >>>>>> + compatible = "ti,am33xx-tilcdc", "ti,da850- tilcdc"; > >>>>> > >>>>> This should instead be: > >>>>> > >>>>> compatible = "ti,da850-tilcdc", "ti,am33xx-tilcdc"; > >>>>> > >>>>> as the closest match should appear first in the list. > >>>> > >>>> Actually I don't think that's correct. The LCDC on da850 is not > >>>> compatible with the LCDC on AM335x. I think it should be just > >>>> "ti,da850-tilcdc". > >>> > >>> So if "ti,am33xx-tilcdc" is used, the display wont work at all? If thats > >>> the case, I wonder how the patch passed testing. Bartosz? > >> > >> AM3 has "version 2" of LCDC, whereas DA850 is v1. They are quite > >> similar, but different. > >> > >> The driver gets the version number from LCDC's register, and acts based > >> on that, so afaik the compatible string doesn't really affect the > >> functionality (as long as it matches). > >> > >> But even if it works with the current driver, I don't think > >> "ti,am33xx-tilcdc" and "ti,da850-tilcdc" are compatible in the HW level. > > > > If the hardware provides IP revision information, how about just "ti,lcdc" > > ? > > Maybe, and I agree that's the "correct" way, but looking at the history, > it's not just once or twice when we've suddenly found out some > difference or bug or such in an IP revision, or the integration to a > SoC, that can't be found based on the IP revision. > > That's why I feel it's usually safer to have the SoC revision there in > the compatible string. > > That said, we have only a few different old SoCs with LCDC (compared to, > say, OMAP DSS) so in this case perhaps just "ti,lcdc" would be fine. You obviously know more than I do on this topic so I'll trust your opinion. If the version register isn't enough I'm fine with multiple compatible strings. -- Regards, Laurent Pinchart