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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html