From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ajay kumar Subject: Re: [PATCH 1/7] drm/exynos: dp: support hotplug detection via GPIO Date: Sat, 19 Apr 2014 01:03:07 +0530 Message-ID: References: <1397658786-26138-1-git-send-email-ajaykumar.rs@samsung.com> <1397658786-26138-2-git-send-email-ajaykumar.rs@samsung.com> <000001cf5adf$87cf7280$976e5780$%han@samsung.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1746825157==" Return-path: In-Reply-To: <000001cf5adf$87cf7280$976e5780$%han@samsung.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jingoo Han Cc: "linux-samsung-soc@vger.kernel.org" , Sean Paul , abrestic@chromium.org, sunil joshi , "dri-devel@lists.freedesktop.org" , a.hajda@samsung.com, Kyungmin Park , treding@nvidia.com, Prashanth G , Ajay Kumar , Rahul Sharma List-Id: linux-samsung-soc@vger.kernel.org --===============1746825157== Content-Type: multipart/alternative; boundary=001a113376fe1e1fac04f75638c5 --001a113376fe1e1fac04f75638c5 Content-Type: text/plain; charset=UTF-8 Hi Jingoo, On Fri, Apr 18, 2014 at 1:53 PM, Jingoo Han wrote: > On Wednesday, April 16, 2014 11:33 PM, Ajay Kumar wrote: > > > > From: Andrew Bresticker > > > > 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 > > Signed-off-by: Rahul Sharma > > Signed-off-by: Ajay Kumar > > --- > > .../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 > > #include > > #include > > +#include > > > > #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 --001a113376fe1e1fac04f75638c5 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
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 <a= brestic@chromium.org>
>
> Certain bridge chips use a GPIO to indicate the cable status instead > of the I_DP_HPD pin. =C2=A0This adds an optional device-tree property,=
> "samsung,hpd-gpio", to the exynos-dp controller which indica= tes 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 hotplu= g-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>
> ---
> =C2=A0.../devicetree/bindings/video/exynos_dp.txt =C2=A0 =C2=A0 =C2=A0= =C2=A0| =C2=A04 +++
> =C2=A0drivers/gpu/drm/exynos/exynos_dp_core.c =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0| 32 ++++++++++++++++++++--
> =C2=A0drivers/gpu/drm/exynos/exynos_dp_core.h =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0| =C2=A01 +
> =C2=A0drivers/gpu/drm/exynos/exynos_dp_reg.c =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 | 26 ++++++++++++++++--
> =C2=A04 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 @@
> =C2=A0#include <linux/device.h>
> =C2=A0#include <linux/io.h>
> =C2=A0#include <linux/delay.h>
> +#include <linux/gpio.h>
>
> =C2=A0#include "exynos_dp_core.h"
> =C2=A0#include "exynos_dp_reg.h"
> @@ -326,6 +327,9 @@ void exynos_dp_clear_hotplug_interrupts(struct exy= nos_dp_device *dp)
> =C2=A0{
> =C2=A0 =C2=A0 =C2=A0 u32 reg;
>
> + =C2=A0 =C2=A0 if (gpio_is_valid(dp->hpd_gpio))
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +
> =C2=A0 =C2=A0 =C2=A0 reg =3D HOTPLUG_CHG | HPD_LOST | PLUG;
> =C2=A0 =C2=A0 =C2=A0 writel(reg, dp->reg_base + EXYNOS_DP_COMMON_IN= T_STA_4);
>
> @@ -337,6 +341,9 @@ void exynos_dp_init_hpd(struct exynos_dp_device *d= p)
> =C2=A0{
> =C2=A0 =C2=A0 =C2=A0 u32 reg;
>
> + =C2=A0 =C2=A0 if (gpio_is_valid(dp->hpd_gpio))
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +
> =C2=A0 =C2=A0 =C2=A0 exynos_dp_clear_hotplug_interrupts(dp);
>
> =C2=A0 =C2=A0 =C2=A0 reg =3D readl(dp->reg_base + EXYNOS_DP_SYS_CTL= _3);
> @@ -348,6 +355,14 @@ enum dp_irq_type exynos_dp_get_irq_type(struct ex= ynos_dp_device *dp)
> =C2=A0{
> =C2=A0 =C2=A0 =C2=A0 u32 reg;
>
> + =C2=A0 =C2=A0 if (gpio_is_valid(dp->hpd_gpio)) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 reg =3D gpio_get_value(dp-= >hpd_gpio);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (reg)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return DP_IRQ_TYPE_HP_CABLE_IN;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 else
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return DP_IRQ_TYPE_HP_CABLE_OUT;
> + =C2=A0 =C2=A0 }
> +

Please keep the style. It enhances the readability.

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

Ok.
Then, it can be as bellows.

+ =C2=A0 =C2=A0 =C2=A0 if (gpio_is_valid(dp->hpd_gpio)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 reg =3D gpio_get_value(d= p->hpd_gpio);
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (reg)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return DP_IRQ_TYPE_HP_CABLE_IN;
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 else
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return DP_IRQ_TYPE_HP_CABLE_OUT;
+ =C2=A0 =C2=A0 =C2=A0 } else {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Parse= hotplug interrupt status register */
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 reg =3D readl(dp-&= gt;reg_base + EXYNOS_DP_COMMON_INT_STA_4);
+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (reg & PLUG)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return DP_IRQ_TYPE_HP_CABLE_IN;
+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (reg & HPD_LOST)<= br> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return DP_IRQ_TYPE_HP_CABLE_OUT;
+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (reg & HOTPLUG_CH= G)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return DP_IRQ_TYPE_HP_CHANGE;
+ =C2=A0 =C2=A0 =C2=A0 }

Will change this.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return DP_IRQ_TYPE_UNKNOWN;
}

Best regards,
Jingoo Han

> =C2=A0 =C2=A0 =C2=A0 /* Parse hotplug interrupt status register */
> =C2=A0 =C2=A0 =C2=A0 reg =3D 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)
> =C2=A0{
> =C2=A0 =C2=A0 =C2=A0 u32 reg;
>
> - =C2=A0 =C2=A0 reg =3D readl(dp->reg_base + EXYNOS_DP_SYS_CTL_3);<= br> > - =C2=A0 =C2=A0 if (reg & HPD_STATUS)
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return 0;
> + =C2=A0 =C2=A0 if (gpio_is_valid(dp->hpd_gpio)) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (gpio_get_value(dp->= hpd_gpio))
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return 0;
> + =C2=A0 =C2=A0 } else {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 reg =3D readl(dp->reg_b= ase + EXYNOS_DP_SYS_CTL_3);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (reg & HPD_STATUS)<= br> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return 0;
> + =C2=A0 =C2=A0 }
>
> =C2=A0 =C2=A0 =C2=A0 return -EINVAL;
> =C2=A0}
> --
> 1.8.1.2


Thanks = and Regrads,
Ajay Kumar
= --001a113376fe1e1fac04f75638c5-- --===============1746825157== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============1746825157==--