Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

[Tomasz Figa]

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.

CCing Richard, Florian and linux-fbdev.

Best regards,
Tomasz

Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

[한진구]

VHVlc2RheSwgTWF5IDE0LCAyMDEzIDExOjE3IFBNLCBWaWthcyBTYWpqYW4gd3JvdGU6DQo+IA0K
PiBIaSBWaWthcywNCj4gDQo+IE9uIFR1ZXNkYXkgMTQgb2YgTWF5IDIwMTMgMTg6MjU6NTEgVmlr
YXMgU2FqamFuIHdyb3RlOg0KPiA+ICBBZGRzIEdQSU8gcGFyc2luZyBmdW5jdGlvbmFsaXR5IGZv
ciAiTENEIGJhY2tsaWdodCIgYW5kICJMQ0QgZW5hYmxlIg0KPiA+ICBHUElPIHBpbnMgb2YgZXh5
bm9zIGRwIGNvbnRyb2xsZXIuDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBWaWthcyBTYWpqYW4g
PHZpa2FzLnNhamphbkBsaW5hcm8ub3JnPg0KPiA+IC0tLQ0KPiA+ICBkcml2ZXJzL3ZpZGVvL2V4
eW5vcy9leHlub3NfZHBfY29yZS5jIHwgICA0NQ0KPiA+ICsrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKyAxIGZpbGUgY2hhbmdlZCwgNDUgaW5zZXJ0aW9ucygrKQ0KPiA+DQo+IA0KPiBJ
IGRvbid0IHRoaW5rIHRoYXQgRXh5bm9zIERQIGRyaXZlciBpcyByaWdodCBwbGFjZSBmb3Igc3Vj
aCBjb2RlLiBCYWNrbGlnaHQNCj4gYW5kIExDRCBkcml2ZXJzIGFyZSByZXNwb25zaWJsZSBmb3Ig
YmFja2xpZ2h0IGFuZCBMQ0QgcG93ZXIgY29udHJvbCB1c2luZw0KPiBiYWNrbGlnaHQgYW5kIExD
RCBzdWJzeXN0ZW1zLg0KPiANCj4gSU1ITyB0aGUgY29ycmVjdCBzb2x1dGlvbiB3b3VsZCBiZSB0
byBlaXRoZXIgZXh0ZW5kIGV4aXN0aW5nIGJhY2tsaWdodC9sY2QNCj4gZHJpdmVycyBmb3VuZCBp
biBkcml2ZXJzL3ZpZGVvL2JhY2tsaWdodCB0byBzdXBwb3J0IGRpcmVjdCBHUElPIGNvbnRyb2wg
YW5kDQo+IHBhcnNlIEdQSU8gcGlucyBmcm9tIGRldmljZSB0cmVlIG9yIGNyZWF0ZSBuZXcgZ3Bp
b19ibCBhbmQgZ3Bpb19sY2QgZHJpdmVycy4NCg0KSGkgVmlrYXMgU2FqaWFuLA0KDQpJIGFncmVl
IHdpdGggVG9tYXN6IEZpZ2EncyBvcGluaW9uLg0KQmFja2xpZ2h0L0xDRCBmcmFtZXdvcmsgc2hv
dWxkIGJlIHVzZWQuDQplRFAgcGFuZWwgYmFja2xpZ2h0IG9uIFNNREs1MjEwIGJvYXJkIGNhbiBi
ZSBjb250cm9sbGVkIGJ5IFBXTTsNCnRodXMsIHB3bS1iYWNrbGlnaHQgZHJpdmVyIHNob3VsZCBi
ZSB1c2VkLg0KQWxzbywgZURQIHBhbmVsIHJlc2V0IHBpbiBzaG91bGQgYmUgY29udHJvbGxlZCBi
eSB1c2luZw0KcGxhdGZvcm0tbGNkIGRyaXZlci4NCg0KPiANCj4gQ0NpbmcgUmljaGFyZCwgRmxv
cmlhbiBhbmQgbGludXgtZmJkZXYuDQoNCkFsc28sIEkgaGF2ZSBiZWVuIGRvaW5nIGJhY2tsaWdo
dCByZXZpZXdzIGluc3RlYWQgb2YgUmljaGFyZCwNCnBsZWFzZSBkbyBDQydpbmcgbWUuDQoNCkJl
c3QgcmVnYXJkcywNCkppbmdvbyBIYW4NCg0KPiANCj4gQmVzdCByZWdhcmRzLA0KPiBUb21hc3oN
Cg=

Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

[Tomasz Figa]

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