From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756813Ab3BFAr4 (ORCPT ); Tue, 5 Feb 2013 19:47:56 -0500 Received: from mail-ie0-f181.google.com ([209.85.223.181]:56117 "EHLO mail-ie0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756089Ab3BFAru (ORCPT ); Tue, 5 Feb 2013 19:47:50 -0500 MIME-Version: 1.0 In-Reply-To: <5111A6D9.7090700@wwwdotorg.org> References: <1360107777-17490-1-git-send-email-seanpaul@chromium.org> <1360107777-17490-2-git-send-email-seanpaul@chromium.org> <5111A23F.50300@wwwdotorg.org> <5111A6D9.7090700@wwwdotorg.org> Date: Wed, 6 Feb 2013 09:47:49 +0900 X-Google-Sender-Auth: U8TFvuJis_KsVOepCl7PX5UzYm0 Message-ID: Subject: Re: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree From: Kyungmin Park To: Stephen Warren Cc: Sean Paul , Tomasz Stanislawski , "kgene.kim" , devicetree-discuss@lists.ozlabs.org, Linux Kernel Mailing List , dri-devel@lists.freedesktop.org, Tomasz Figa , InKi Dae , linux-samsung-soc@vger.kernel.org, Sylwester Nawrocki , Olof Johansson , linux-arm-kernel@lists.infradead.org, RAHUL SHARMA Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 6, 2013 at 9:42 AM, Stephen Warren wrote: > On 02/05/2013 05:37 PM, Sean Paul wrote: >> On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren wrote: >>> n 02/05/2013 04:42 PM, Sean Paul wrote: >>>> Use the compatible string in the device tree to determine which >>>> registers/functions to use in the HDMI driver. Also changes the >>>> references from v13 to 4210 and v14 to 4212 to reflect the IP >>>> block version instead of the HDMI version. >>> >>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >>> >>> Binding looks sane to me. >>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c >>> >>>> #ifdef CONFIG_OF >>>> static struct of_device_id hdmi_match_types[] = { >>>> { >>>> - .compatible = "samsung,exynos5-hdmi", >>>> - .data = (void *)HDMI_TYPE14, >>>> + .compatible = "samsung,exynos4-hdmi", >>>> }, { >>>> /* end node */ >>>> } >>> >>> Why not fill in all the "base" compatible values there (I think you need >>> this anyway so that DTs don't all have to be compatible with >>> samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* >>> values, then ... >>> >> >> At the moment, all DTs have to be compatible with exynos4-hdmi since >> it provides the base for the current driver. The driver uses 4210 and >> 4212 to differentiate between different register addresses and >> features, but most things are just exynos4-hdmi compatible. I would like to distinguish between 4210 and 4x12. since it has different features. e.g., HDMI v1.3 and v1.4. and I also want to use 4412 instead of 4212. there's no board to use 4212 at mainline until this time. Thank you, Kyungmin Park > > The DT nodes should include only the compatible values that the HW is > actually compatible with. If the HW isn't compatible with exynos4-hdmi, > that value shouldn't be in the compatible property, but instead whatever > the "base" value that the HW really is compatible with. The driver can > support multiple "base" compatible values from this table. > >>>> @@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev) >>> >>>> + >>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi")) >>>> + hdata->version |= HDMI_VER_EXYNOS4210; >>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi")) >>>> + hdata->version |= HDMI_VER_EXYNOS4212; >>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi")) >>>> + hdata->version |= HDMI_VER_EXYNOS5250; >>> >>> Instead of that, do roughly: >>> >>> match = of_match_device(hdmi_match_types, &pdev->dev); >>> if (match) >>> hdata->version |= (int)match->data; >>> >>> That way, it's all table-based. Any future additions to >>> hdmi_match_types[] won't require another if statement to be added to >>> probe(). >> >> I don't think it's that easy. of_match_device returns the first match >> from the device table, so I'd still need to iterate through the >> matches. I could still break this out into a table, but I don't think >> of_match_device is the right way to probe it. > > You shouldn't have to iterate over multiple matches. of_match_device() > is supposed to return the match for the first entry in the compatible > property, then if there was no match, move on to looking at the next > entry in the compatible property, etc. In practice, I think it's still > not implemented quite correctly for this, but you can make it work by > putting the newest compatible value first in the match table. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 From: kmpark@infradead.org (Kyungmin Park) Date: Wed, 6 Feb 2013 09:47:49 +0900 Subject: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree In-Reply-To: <5111A6D9.7090700@wwwdotorg.org> References: <1360107777-17490-1-git-send-email-seanpaul@chromium.org> <1360107777-17490-2-git-send-email-seanpaul@chromium.org> <5111A23F.50300@wwwdotorg.org> <5111A6D9.7090700@wwwdotorg.org> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Feb 6, 2013 at 9:42 AM, Stephen Warren wrote: > On 02/05/2013 05:37 PM, Sean Paul wrote: >> On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren wrote: >>> n 02/05/2013 04:42 PM, Sean Paul wrote: >>>> Use the compatible string in the device tree to determine which >>>> registers/functions to use in the HDMI driver. Also changes the >>>> references from v13 to 4210 and v14 to 4212 to reflect the IP >>>> block version instead of the HDMI version. >>> >>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >>> >>> Binding looks sane to me. >>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c >>> >>>> #ifdef CONFIG_OF >>>> static struct of_device_id hdmi_match_types[] = { >>>> { >>>> - .compatible = "samsung,exynos5-hdmi", >>>> - .data = (void *)HDMI_TYPE14, >>>> + .compatible = "samsung,exynos4-hdmi", >>>> }, { >>>> /* end node */ >>>> } >>> >>> Why not fill in all the "base" compatible values there (I think you need >>> this anyway so that DTs don't all have to be compatible with >>> samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* >>> values, then ... >>> >> >> At the moment, all DTs have to be compatible with exynos4-hdmi since >> it provides the base for the current driver. The driver uses 4210 and >> 4212 to differentiate between different register addresses and >> features, but most things are just exynos4-hdmi compatible. I would like to distinguish between 4210 and 4x12. since it has different features. e.g., HDMI v1.3 and v1.4. and I also want to use 4412 instead of 4212. there's no board to use 4212 at mainline until this time. Thank you, Kyungmin Park > > The DT nodes should include only the compatible values that the HW is > actually compatible with. If the HW isn't compatible with exynos4-hdmi, > that value shouldn't be in the compatible property, but instead whatever > the "base" value that the HW really is compatible with. The driver can > support multiple "base" compatible values from this table. > >>>> @@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev) >>> >>>> + >>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi")) >>>> + hdata->version |= HDMI_VER_EXYNOS4210; >>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi")) >>>> + hdata->version |= HDMI_VER_EXYNOS4212; >>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi")) >>>> + hdata->version |= HDMI_VER_EXYNOS5250; >>> >>> Instead of that, do roughly: >>> >>> match = of_match_device(hdmi_match_types, &pdev->dev); >>> if (match) >>> hdata->version |= (int)match->data; >>> >>> That way, it's all table-based. Any future additions to >>> hdmi_match_types[] won't require another if statement to be added to >>> probe(). >> >> I don't think it's that easy. of_match_device returns the first match >> from the device table, so I'd still need to iterate through the >> matches. I could still break this out into a table, but I don't think >> of_match_device is the right way to probe it. > > You shouldn't have to iterate over multiple matches. of_match_device() > is supposed to return the match for the first entry in the compatible > property, then if there was no match, move on to looking at the next > entry in the compatible property, etc. In practice, I think it's still > not implemented quite correctly for this, but you can make it work by > putting the newest compatible value first in the match table. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel