linux-kernel - Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Oct 2016 15:29:23 +0300
From:   Tomi Valkeinen <tomi.valkeinen@...com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
CC:     Sekhar Nori <nsekhar@...com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Kevin Hilman <khilman@...libre.com>,
        Michael Turquette <mturquette@...libre.com>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Mark Rutland <mark.rutland@....com>,
        Peter Ujfalusi <peter.ujfalusi@...com>,
        Russell King <linux@...linux.org.uk>,
        Karl Beldan <karl.beldan@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        arm-soc <linux-arm-kernel@...ts.infradead.org>,
        linux-drm <dri-devel@...ts.freedesktop.org>,
        linux-devicetree <devicetree@...r.kernel.org>,
        Jyri Sarha <jsarha@...com>, David Airlie <airlied@...ux.ie>,
        Maxime Ripard <maxime.ripard@...e-electrons.com>,
        Karl Beldan <kbeldan@...libre.com>
Subject: Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller

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@...000 {
>>>>>> +			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



Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)