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 Content-Length: 2513 Lines: 66 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