Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1501942

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

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Sekhar Nori <nsekhar@ti.com> - 2016-10-15 19:50 +0200
  Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-10-15 21:50 +0200
  Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Tomi Valkeinen <tomi.valkeinen@ti.com> - 2016-10-17 08:00 +0200
    Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Sekhar Nori <nsekhar@ti.com> - 2016-10-17 09:20 +0200
      Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-10-17 09:30 +0200
      Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Tomi Valkeinen <tomi.valkeinen@ti.com> - 2016-10-17 09:40 +0200
        Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-10-17 13:50 +0200
          Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Tomi Valkeinen <tomi.valkeinen@ti.com> - 2016-10-17 14:40 +0200
            Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-10-17 14:50 +0200
            Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-10-17 16:10 +0200
              Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Sekhar Nori <nsekhar@ti.com> - 2016-10-20 12:10 +0200
                Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Tomi Valkeinen <tomi.valkeinen@ti.com> - 2016-10-20 12:30 +0200
                Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller Sekhar Nori <nsekhar@ti.com> - 2016-10-20 12:40 +0200

csiph-web