linux-kernel - Re: [PATCH v2] pwm-backlight: fix the panel power sequence

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 30 Oct 2015 11:34:31 +0100
From:	Philipp Zabel <p.zabel@...gutronix.de>
To:	YH Huang <yh.huang@...iatek.com>
Cc:	linux-pwm@...r.kernel.org, linux-fbdev@...r.kernel.org,
	Jingoo Han <jingoohan1@...il.com>,
	Tomi Valkeinen <tomi.valkeinen@...com>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	linux-kernel@...r.kernel.org,
	Thierry Reding <thierry.reding@...il.com>,
	linux-mediatek@...ts.infradead.org,
	Sascha Hauer <kernel@...gutronix.de>,
	Matthias Brugger <matthias.bgg@...il.com>,
	yingjoe.chen@...iatek.com, Lee Jones <lee.jones@...aro.org>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] pwm-backlight: fix the panel power sequence

Am Freitag, den 30.10.2015, 15:41 +0800 schrieb YH Huang:
> > That won't work if the gpio is still configured as input. How about I
> > add the GPIOD_ASIS change to my patch you remove that and the above from
> > yours?
> 
> I revise these two lines 
> if (pb->enable_gpio)
> 	gpiod_direction_output(pb->enable_gpio, 0);
> into
> if (pb->enable_gpio) {
> 	if(gpiod_get_value(pb->enable_gpio) == 0)
> 		gpiod_direction_output(pb->enable_gpio, 0);
> 	else
> 		gpiod_direction_output(pb->enable_gpio, 1);
> }

If the GPIO is still configured as an input, the return value of
gpiod_get_value could be random.

> I am not sure what "phandle" is working for.

The reasoning is that devices where there is no phandle link pointing to
the backlight (for example from a simple-panel node), we should keep the
current default behaviour (enable during probe).

> My change is like your patch but remove the "phandle related" and
> "gpiod_get_direction(pb->enable_gpio) == GPIOF_DIR_OUT".
> Do we really need these?

See above, I wanted to be careful not to change existing behavior in
unexpected ways.

> Also, do you think it is right that I do the "pwm_enable(pb->pwm);"
> before "gpiod_set_value(pb->enable_gpio, 1);" in power on function?

That is unclear to me, because I'm sure that depends on the actual
constant current LED driver used.
I suspect EN and PWM pins are completely independent on a lot of them,
so enabling EN last should be ok to work around the problem that the PWM
pin state might not be well defined when the PWM is disabled.

For example, the STCS1A datasheet doesn't mention any order for enabling
EN and PWM. In the LP8545 datasheet, the modes of operation state
diagram suggests that enabling EN first is the proper way, on the other
hand the PWM interface characteristics table also lists a startup delay
for a rising EN edge while PWM is already active.

But I'm not sure that there aren't any LED drivers that somehow have a
problem with this order.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/