Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965049AbcJQOBl (ORCPT ); Mon, 17 Oct 2016 10:01:41 -0400 Received: from mail-it0-f46.google.com ([209.85.214.46]:34926 "EHLO mail-it0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933215AbcJQOBY (ORCPT ); Mon, 17 Oct 2016 10:01:24 -0400 MIME-Version: 1.0 In-Reply-To: References: <1475672732-17111-1-git-send-email-bgolaszewski@baylibre.com> <65fde145-0820-794d-d345-621f66cdacc0@ti.com> <7a2ffcd0-fe1d-c887-53b7-7cb5e1e61222@ti.com> <4975084.EGQPv58AK6@avalon> From: Bartosz Golaszewski Date: Mon, 17 Oct 2016 16:01:21 +0200 Message-ID: Subject: Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller To: Tomi Valkeinen Cc: Laurent Pinchart , Sekhar Nori , 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 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2475 Lines: 64 2016-10-17 14:29 GMT+02:00 Tomi Valkeinen : > On 17/10/16 14:40, Laurent Pinchart wrote: >> Hello, >> >> 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. > > Tomi > I Sekhar is ok with this, I'll send a follow-up patch for that. Thanks, Bartosz