All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomasz Figa <t.figa@samsung.com>
To: jg1.han@samsung.com
Cc: "Vikas Sajjan" <vikas.sajjan@linaro.org>,
	"linux-samsung-soc@vger.kernel.org"
	<linux-samsung-soc@vger.kernel.org>, 김국진 <kgene.kim@samsung.com>,
	"devicetree-discuss@lists.ozlabs.org"
	<devicetree-discuss@lists.ozlabs.org>,
	"patches@linaro.org" <patches@linaro.org>,
	"linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>,
	"rpurdie@rpsys.net" <rpurdie@rpsys.net>,
	"FlorianSchandinat@gmx.de" <FlorianSchandinat@gmx.de>,
	"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>
Subject: Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver
Date: Fri, 17 May 2013 12:29:35 +0000	[thread overview]
Message-ID: <6459883.FZan3akiXR@amdc1227> (raw)
In-Reply-To: <26364319.123371368669839202.JavaMail.weblogic@epml12>

Hi Jingoo,

On Thursday 16 of May 2013 02:03:59 한진구 wrote:
> Tuesday, May 14, 2013 11:17 PM, Vikas Sajjan wrote:
> 
> > 
> > Hi Vikas,
> > 
> > On Tuesday 14 of May 2013 18:25:51 Vikas Sajjan wrote:
> > 
> > >  Adds GPIO parsing functionality for "LCD backlight" and "LCD enable"
> > >  GPIO pins of exynos dp controller.
> > >
> > >
> > >
> > > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> > > ---
> > > 
> > >  drivers/video/exynos/exynos_dp_core.c |   45
> > > 
> > > +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
> > >
> > >
> > 
> > 
> > I don't think that Exynos DP driver is right place for such code.
> > Backlight
 and LCD drivers are responsible for backlight and LCD power
> > control using backlight and LCD subsystems.
> > 
> > IMHO the correct solution would be to either extend existing
> > backlight/lcd
> > drivers found in drivers/video/backlight to support direct GPIO control
> > and
 parse GPIO pins from device tree or create new gpio_bl and gpio_lcd
> > drivers.
> 
> Hi Vikas Sajian,
> 
> I agree with Tomasz Figa's opinion.
> Backlight/LCD framework should be used.
> eDP panel backlight on SMDK5210 board can be controlled by PWM;
> thus, pwm-backlight driver should be used.
> Also, eDP panel reset pin should be controlled by using
> platform-lcd driver.
> 
> 
> > 
> > CCing Richard, Florian and linux-fbdev.
> 
> 
> Also, I have been doing backlight reviews instead of Richard,
> please do CC'ing me.

OK. I used get_maintainers script, but it seems like the result was a bit off 
in this case. Will remember for future.

Best regards,
-- 
Tomasz Figa
Linux Kernel Developer
Samsung R&D Institute Poland
Samsung Electronics


WARNING: multiple messages have this Message-ID (diff)
From: Tomasz Figa <t.figa@samsung.com>
To: jg1.han@samsung.com
Cc: "Vikas Sajjan" <vikas.sajjan@linaro.org>,
	"linux-samsung-soc@vger.kernel.org"
	<linux-samsung-soc@vger.kernel.org>, 김국진 <kgene.kim@samsung.com>,
	"devicetree-discuss@lists.ozlabs.org"
	<devicetree-discuss@lists.ozlabs.org>,
	"patches@linaro.org" <patches@linaro.org>,
	"linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>,
	"rpurdie@rpsys.net" <rpurdie@rpsys.net>,
	"FlorianSchandinat@gmx.de" <FlorianSchandinat@gmx.de>,
	"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>
Subject: Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver
Date: Fri, 17 May 2013 14:29:35 +0200	[thread overview]
Message-ID: <6459883.FZan3akiXR@amdc1227> (raw)
In-Reply-To: <26364319.123371368669839202.JavaMail.weblogic@epml12>

Hi Jingoo,

On Thursday 16 of May 2013 02:03:59 한진구 wrote:
> Tuesday, May 14, 2013 11:17 PM, Vikas Sajjan wrote:
> 
> > 
> > Hi Vikas,
> > 
> > On Tuesday 14 of May 2013 18:25:51 Vikas Sajjan wrote:
> > 
> > >  Adds GPIO parsing functionality for "LCD backlight" and "LCD enable"
> > >  GPIO pins of exynos dp controller.
> > >
> > >
> > >
> > > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> > > ---
> > > 
> > >  drivers/video/exynos/exynos_dp_core.c |   45
> > > 
> > > +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
> > >
> > >
> > 
> > 
> > I don't think that Exynos DP driver is right place for such code.
> > Backlight
 and LCD drivers are responsible for backlight and LCD power
> > control using backlight and LCD subsystems.
> > 
> > IMHO the correct solution would be to either extend existing
> > backlight/lcd
> > drivers found in drivers/video/backlight to support direct GPIO control
> > and
 parse GPIO pins from device tree or create new gpio_bl and gpio_lcd
> > drivers.
> 
> Hi Vikas Sajian,
> 
> I agree with Tomasz Figa's opinion.
> Backlight/LCD framework should be used.
> eDP panel backlight on SMDK5210 board can be controlled by PWM;
> thus, pwm-backlight driver should be used.
> Also, eDP panel reset pin should be controlled by using
> platform-lcd driver.
> 
> 
> > 
> > CCing Richard, Florian and linux-fbdev.
> 
> 
> Also, I have been doing backlight reviews instead of Richard,
> please do CC'ing me.

OK. I used get_maintainers script, but it seems like the result was a bit off 
in this case. Will remember for future.

Best regards,
-- 
Tomasz Figa
Linux Kernel Developer
Samsung R&D Institute Poland
Samsung Electronics

  reply	other threads:[~2013-05-17 12:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-16  2:03 [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver 한진구
2013-05-16  2:04 ` 한진구
2013-05-17 12:29 ` Tomasz Figa [this message]
2013-05-17 12:29   ` Tomasz Figa
  -- strict thread matches above, loose matches on Subject: below --
2013-05-14 12:55 [PATCH 0/2] Add LCD backlight and LCD enable gpios pins to dp-controller DT node Vikas Sajjan
2013-05-14 12:55 ` [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver Vikas Sajjan
2013-05-14 14:16   ` Tomasz Figa
2013-05-14 14:16     ` Tomasz Figa
2013-05-14 12:55 ` [PATCH 2/2] ARM: dts: Add LCD backlight and LCD enable gpios pins to dp-controller DT node Vikas Sajjan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6459883.FZan3akiXR@amdc1227 \
    --to=t.figa@samsung.com \
    --cc=FlorianSchandinat@gmx.de \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=jg1.han@samsung.com \
    --cc=kgene.kim@samsung.com \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=patches@linaro.org \
    --cc=rpurdie@rpsys.net \
    --cc=vikas.sajjan@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.