Hi Jingoo,


On Fri, Apr 18, 2014 at 1:53 PM, Jingoo Han <jg1.han@samsung.com> wrote:
On Wednesday, April 16, 2014 11:33 PM, Ajay Kumar wrote:
>
> From: Andrew Bresticker <abrestic@chromium.org>
>
> Certain bridge chips use a GPIO to indicate the cable status instead
> of the I_DP_HPD pin.  This adds an optional device-tree property,
> "samsung,hpd-gpio", to the exynos-dp controller which indicates that
> the specified GPIO should be used for hotplug detection.
> The GPIO is then set up as an edge-triggered interrupt where the
> rising edge indicates hotplug-in and the falling edge indicates hotplug-out.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
>  .../devicetree/bindings/video/exynos_dp.txt        |  4 +++
>  drivers/gpu/drm/exynos/exynos_dp_core.c            | 32 ++++++++++++++++++++--
>  drivers/gpu/drm/exynos/exynos_dp_core.h            |  1 +
>  drivers/gpu/drm/exynos/exynos_dp_reg.c             | 26 ++++++++++++++++--
>  4 files changed, 57 insertions(+), 6 deletions(-)
>

[.....]

> --- a/drivers/gpu/drm/exynos/exynos_dp_reg.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_reg.c
> @@ -13,6 +13,7 @@
>  #include <linux/device.h>
>  #include <linux/io.h>
>  #include <linux/delay.h>
> +#include <linux/gpio.h>
>
>  #include "exynos_dp_core.h"
>  #include "exynos_dp_reg.h"
> @@ -326,6 +327,9 @@ void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp)
>  {
>       u32 reg;
>
> +     if (gpio_is_valid(dp->hpd_gpio))
> +             return;
> +
>       reg = HOTPLUG_CHG | HPD_LOST | PLUG;
>       writel(reg, dp->reg_base + EXYNOS_DP_COMMON_INT_STA_4);
>
> @@ -337,6 +341,9 @@ void exynos_dp_init_hpd(struct exynos_dp_device *dp)
>  {
>       u32 reg;
>
> +     if (gpio_is_valid(dp->hpd_gpio))
> +             return;
> +
>       exynos_dp_clear_hotplug_interrupts(dp);
>
>       reg = readl(dp->reg_base + EXYNOS_DP_SYS_CTL_3);
> @@ -348,6 +355,14 @@ enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp)
>  {
>       u32 reg;
>
> +     if (gpio_is_valid(dp->hpd_gpio)) {
> +             reg = gpio_get_value(dp->hpd_gpio);
> +             if (reg)
> +                     return DP_IRQ_TYPE_HP_CABLE_IN;
> +             else
> +                     return DP_IRQ_TYPE_HP_CABLE_OUT;
> +     }
> +

Please keep the style. It enhances the readability.

if (gpio_is_valid(dp->hpd_gpio)) {
        ...
} else {
        ...
}

Ok.
Then, it can be as bellows.

+       if (gpio_is_valid(dp->hpd_gpio)) {
+               reg = gpio_get_value(dp->hpd_gpio);
+               if (reg)
+                       return DP_IRQ_TYPE_HP_CABLE_IN;
+               else
+                       return DP_IRQ_TYPE_HP_CABLE_OUT;
+       } else {
+               /* Parse hotplug interrupt status register */
+               reg = readl(dp->reg_base + EXYNOS_DP_COMMON_INT_STA_4);
+
+               if (reg & PLUG)
+                       return DP_IRQ_TYPE_HP_CABLE_IN;
+
+               if (reg & HPD_LOST)
+                       return DP_IRQ_TYPE_HP_CABLE_OUT;
+
+               if (reg & HOTPLUG_CHG)
+                       return DP_IRQ_TYPE_HP_CHANGE;
+       }

Will change this.
        return DP_IRQ_TYPE_UNKNOWN;
}

Best regards,
Jingoo Han

>       /* Parse hotplug interrupt status register */
>       reg = readl(dp->reg_base + EXYNOS_DP_COMMON_INT_STA_4);
>
> @@ -402,9 +417,14 @@ int exynos_dp_get_plug_in_status(struct exynos_dp_device *dp)
>  {
>       u32 reg;
>
> -     reg = readl(dp->reg_base + EXYNOS_DP_SYS_CTL_3);
> -     if (reg & HPD_STATUS)
> -             return 0;
> +     if (gpio_is_valid(dp->hpd_gpio)) {
> +             if (gpio_get_value(dp->hpd_gpio))
> +                     return 0;
> +     } else {
> +             reg = readl(dp->reg_base + EXYNOS_DP_SYS_CTL_3);
> +             if (reg & HPD_STATUS)
> +                     return 0;
> +     }
>
>       return -EINVAL;
>  }
> --
> 1.8.1.2


Thanks and Regrads,
Ajay Kumar