linux-kernel - Re: [PATCH 1/3] drm/ssd130x: Fix reset timing for ssd132x

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: <87y0zdvxy2.fsf@minerva.mail-host-address-is-not-set>
Date: Tue, 14 Jan 2025 23:21:25 +0100
From: Javier Martinez Canillas <javierm@...hat.com>
To: John Keeping <jkeeping@...usicbrands.com>
Cc: John Keeping <jkeeping@...usicbrands.com>, Maarten Lankhorst
 <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] drm/ssd130x: Fix reset timing for ssd132x

John Keeping <jkeeping@...usicbrands.com> writes:

Hello John,

Thanks for your patches!

> The ssd132x family of chips require the result pulse to be at least
> 100us in length.  Increase the reset time to meet this requirement.
>

That's not what the datasheet says AFAIU. It says the following in the
"8.9 Power ON and OFF sequence" section.

Power ON sequence:

1. Power ON VDD.
2. After VDD become stable, set RES# pin LOW (logic LOW) for at least
   3us (t1) and then HIGH (logic HIGH).
3. After set RES# pin LOW (logic LOW), wait for at least 3us (t2).
   Then Power ON VCC.
4. After VCC become stable, send command AFh for display ON. SEG/COM
   will be ON after 100ms (tAF).

> Signed-off-by: John Keeping <jkeeping@...usicbrands.com>
> ---
>  drivers/gpu/drm/solomon/ssd130x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index b777690fd6607..2622172228361 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -363,7 +363,7 @@ static void ssd130x_reset(struct ssd130x_device *ssd130x)
>  
>  	/* Reset the screen */
>  	gpiod_set_value_cansleep(ssd130x->reset, 1);
> -	udelay(4);
> +	usleep_range(100, 1000);
>  	gpiod_set_value_cansleep(ssd130x->reset, 0);
>  	udelay(4);

That's why I think that the udelay(4) are correct here, since that will
make for the delay to be bigger than 3 usecs.

Now, is true that the mentioned 100ms (tAF) after sending an AFh command
might not happen. Since I see there's no delay after sending a display ON
command in ssd130x_encoder_atomic_enable():

  static void ssd130x_encoder_atomic_enable(struct drm_encoder *encoder,
                                            struct drm_atomic_state *state)
  {
..
           ssd130x_write_cmd(ssd130x, 1, SSD13XX_DISPLAY_ON);

Maybe the solution is to add the here?

Also, according to the timers-howto doc [0], the usleep_range() helper
should only be used for small msecs (in the 1~20ms range) and msleep()
used for larger msecs.

[0]: https://www.kernel.org/doc/Documentation/timers/timers-howto.rst

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat