Re: [PATCH v3 5/5] ARM: dts: rockchip: set PWM delay backlight settings for Minnie

[Pavel Machek]

Hi!

> 2017-07-20 10:10 GMT+02:00 Pavel Machek <pavel@xxxxxx>:
> > On Mon 2017-07-17 23:28:11, Enric Balletbo i Serra wrote:
> >> The minnie devices comes with an AUO B101EAN01 panel which is different
> >> from default veyron devices, thus the power on/off timing sequence is
> >> slightly different. The datasheet specifies a pwm delay of 200 ms, so
> >> update the PMW delay proprieties accordingly.
> >
> > Wait a wait a moment!
> >
> >> --- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
> >> +++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
> >> @@ -123,6 +123,8 @@
> >>                       240 241 242 243 244 245 246 247
> >>                       248 249 250 251 252 253 254 255>;
> >>       power-supply = <&backlight_regulator>;
> >> +     post-pwm-on-delay-us = <20000>;
> >
> > -us = <20 000>;
> >
> > This is not 200 msec.
> >
> 
> Oops, good catch.
> 
> > Plus, it is quite anti-social to do udelay(200 000).
> >
> > Plus, it is very anti-socifal to use udelay_range(200msec,
> > 400msec).
> >
> > Whoever told you udelay_range is good thing to use -- it is not, and
> > it is certainly not worth making user wait 200msec more!
> >
> 
> Checked again some datasheets and seems that or not require a delay or
> the delays are 10ms+, so according to [1] I'll use msleep instead.
> 
> [1] https://www.kernel.org/doc/Documentation/timers/timers-howto.txt

Ok, msleep makes sense.

But in such case, you probably want to specify delays in the
miliseconds in the device tree... or at least carefully round the
values up.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachment: signature.asc
Description: Digital signature