linux-kernel - Re: [PATCH v8 0/2] Add driver for Sharp Memory LCD

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]

Message-ID: <q53inyaxyvfib7okxzazepxzarqmq4rubbasumvvx2woioyp42@fbtn4poujsyh>
Date: Thu, 3 Oct 2024 11:27:43 +0200
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Alex Lanzano <lanzano.alex@...il.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Mehdi Djait <mehdi.djait@...tlin.com>, skhan@...uxfoundation.org, 
	linux-kernel-mentees@...ts.linuxfoundation.org, dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v8 0/2] Add driver for Sharp Memory LCD

Hello Alex,

On Wed, Oct 02, 2024 at 10:33:13PM -0400, Alex Lanzano wrote:
> On Wed, Oct 02, 2024 at 09:56:38AM GMT, Uwe Kleine-König wrote:
> > On Tue, Oct 01, 2024 at 11:37:35PM -0400, Alex Lanzano wrote:
> > > Changes in v8:
> > > - Addressed review comments from Uwe
> > >     - Replace pwm_get_state with pwm_init_state
> > >     - Use pwm_set_relative_duty_cycle instead of manually setting period and duty cycle
> > 
> > You didn't explicitly mention that it's fine if the PWM doesn't emit the
> > inactive state when you call pwm_disable(). You're code should continue
> > to work if you drop all calls to pwm_disable().
> > 
> > Ideally you mention that in a code comment to make others reading your
> > code understand that.
> 
> Sorry about that! The intent of the code is to stop the pwm from outputing
> when the display is disabled since the signal is no longer needed. If
> it's best to emit the inactive state rather than calling pwm_disable()
> I'm fine with making that change.

Calling pwm_disable() is best iff you don't care about the output any
more. If however you rely on it to emit the inactive level,
pwm_disable() is wrong. I don't know enough about your display to judge
from here.

The code to disable the display looks (simplified) as follows:

	if (smd->enable_gpio)
		gpiod_set_value(smd->enable_gpio, 0);

	pwm_disable(smd->pwm_vcom_signal);

so maybe the logic you need is:

	if (smd->enable_gpio) {
		gpiod_set_value(smd->enable_gpio, 0);

		/*
		 * In the presence of a GPIO to disable the display the
		 * behaviour of the PWM doesn't matter and we can
		 * just disable it.
		 */
		pwm_disable(smd->pwm_vcom_signal);
	} else {
		struct pwm_state state;

		/*
		 * However without a GPIO driving the display's output
		 * enable pin the PWM must emit the inactive level,
		 * which isn't guaranteed when calling pwm_disable(), so
		 * configure it for duty_cycle = 0.
		 */
		 pwm_init_state(smd->pwm_vcom_signal, &state);
		 state.duty_cycle = 0;
		 state.enabled = true;
		 pwm_apply_might_sleep(smd->pwm_vcom_signal, &state);
	}

?

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)