2018-07-27 09:23:14

by Jia-Ju Bai

[permalink] [raw]
Subject: [PATCH] staging: fbtft: Replace mdelay() with msleep() and usleep_range()

reset() and init_display() are never called in atomic context.
They call mdelay() to busily wait, which is not necessary.
mdelay() can be replaced with msleep().

This is found by a static analysis tool named DCNS written by myself.

Signed-off-by: Jia-Ju Bai <[email protected]>
---
drivers/staging/fbtft/fb_agm1264k-fl.c | 2 +-
drivers/staging/fbtft/fb_bd663474.c | 6 +++---
drivers/staging/fbtft/fb_hx8340bn.c | 8 ++++----
drivers/staging/fbtft/fb_hx8347d.c | 10 +++++-----
drivers/staging/fbtft/fb_hx8353d.c | 4 ++--
drivers/staging/fbtft/fb_ili9163.c | 4 ++--
drivers/staging/fbtft/fb_ili9320.c | 8 ++++----
drivers/staging/fbtft/fb_ili9325.c | 8 ++++----
drivers/staging/fbtft/fb_ili9340.c | 2 +-
drivers/staging/fbtft/fb_ili9341.c | 6 +++---
drivers/staging/fbtft/fb_st7789v.c | 2 +-
drivers/staging/fbtft/fb_watterott.c | 4 ++--
12 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c b/drivers/staging/fbtft/fb_agm1264k-fl.c
index f6f30f5bf15a..8cead859e688 100644
--- a/drivers/staging/fbtft/fb_agm1264k-fl.c
+++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
@@ -87,7 +87,7 @@ static void reset(struct fbtft_par *par)
gpio_set_value(par->gpio.reset, 0);
udelay(20);
gpio_set_value(par->gpio.reset, 1);
- mdelay(120);
+ msleep(120);
}

/* Check if all necessary GPIOS defined */
diff --git a/drivers/staging/fbtft/fb_bd663474.c b/drivers/staging/fbtft/fb_bd663474.c
index a58c514f4721..59eb28fa0dc8 100644
--- a/drivers/staging/fbtft/fb_bd663474.c
+++ b/drivers/staging/fbtft/fb_bd663474.c
@@ -33,7 +33,7 @@ static int init_display(struct fbtft_par *par)

/* oscillator start */
write_reg(par, 0x000, 0x0001); /*oscillator 0: stop, 1: operation */
- mdelay(10);
+ usleep_range(10000, 11000);

/* Power settings */
write_reg(par, 0x100, 0x0000); /* power supply setup */
@@ -43,10 +43,10 @@ static int init_display(struct fbtft_par *par)
write_reg(par, 0x110, 0x009d);
write_reg(par, 0x111, 0x0022);
write_reg(par, 0x100, 0x0120);
- mdelay(20);
+ msleep(20);

write_reg(par, 0x100, 0x3120);
- mdelay(80);
+ msleep(80);
/* Display control */
write_reg(par, 0x001, 0x0100);
write_reg(par, 0x002, 0x0000);
diff --git a/drivers/staging/fbtft/fb_hx8340bn.c b/drivers/staging/fbtft/fb_hx8340bn.c
index d47dcf31fffb..0460597af95e 100644
--- a/drivers/staging/fbtft/fb_hx8340bn.c
+++ b/drivers/staging/fbtft/fb_hx8340bn.c
@@ -51,7 +51,7 @@ static int init_display(struct fbtft_par *par)
* is started, and panel scanning is started.
*/
write_reg(par, 0x11);
- mdelay(150);
+ msleep(150);

/* Undoc'd register? */
write_reg(par, 0xCA, 0x70, 0x00, 0xD9);
@@ -66,7 +66,7 @@ static int init_display(struct fbtft_par *par)

/* Drive ability setting */
write_reg(par, 0xC9, 0x90, 0x49, 0x10, 0x28, 0x28, 0x10, 0x00, 0x06);
- mdelay(20);
+ msleep(20);

/*
* SETPWCTR5: Set Power Control 5(B5h)
@@ -85,7 +85,7 @@ static int init_display(struct fbtft_par *par)
* for VGH and VGL voltage generation.
*/
write_reg(par, 0xB4, 0x33, 0x25, 0x4C);
- mdelay(10);
+ usleep_range(10000, 11000);

/*
* Interface Pixel Format (3Ah)
@@ -101,7 +101,7 @@ static int init_display(struct fbtft_par *par)
* Output from the Frame Memory is enabled.
*/
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
- mdelay(10);
+ usleep_range(10000, 11000);

return 0;
}
diff --git a/drivers/staging/fbtft/fb_hx8347d.c b/drivers/staging/fbtft/fb_hx8347d.c
index 0b605303813e..1d79190e5326 100644
--- a/drivers/staging/fbtft/fb_hx8347d.c
+++ b/drivers/staging/fbtft/fb_hx8347d.c
@@ -49,13 +49,13 @@ static int init_display(struct fbtft_par *par)
write_reg(par, 0x19, 0x01); /* start osc */
write_reg(par, 0x01, 0x00); /* wakeup */
write_reg(par, 0x1F, 0x88);
- mdelay(5);
+ usleep_range(5000, 6000);
write_reg(par, 0x1F, 0x80);
- mdelay(5);
+ usleep_range(5000, 6000);
write_reg(par, 0x1F, 0x90);
- mdelay(5);
+ usleep_range(5000, 6000);
write_reg(par, 0x1F, 0xD0);
- mdelay(5);
+ usleep_range(5000, 6000);

/* color selection */
write_reg(par, 0x17, 0x05); /* 65k */
@@ -65,7 +65,7 @@ static int init_display(struct fbtft_par *par)

/*display on */
write_reg(par, 0x28, 0x38);
- mdelay(40);
+ msleep(40);
write_reg(par, 0x28, 0x3C);

/* orientation */
diff --git a/drivers/staging/fbtft/fb_hx8353d.c b/drivers/staging/fbtft/fb_hx8353d.c
index 3e73b69b6a27..1ba63db42b83 100644
--- a/drivers/staging/fbtft/fb_hx8353d.c
+++ b/drivers/staging/fbtft/fb_hx8353d.c
@@ -20,7 +20,7 @@
static int init_display(struct fbtft_par *par)
{
par->fbtftops.reset(par);
- mdelay(150);
+ msleep(150);

/* SETEXTC */
write_reg(par, 0xB9, 0xFF, 0x83, 0x53);
@@ -42,7 +42,7 @@ static int init_display(struct fbtft_par *par)

/* SLPOUT - Sleep out & booster on */
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
- mdelay(150);
+ msleep(150);

/* DISPON - Display On */
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
diff --git a/drivers/staging/fbtft/fb_ili9163.c b/drivers/staging/fbtft/fb_ili9163.c
index fd3dd671509f..ea7f7261f74f 100644
--- a/drivers/staging/fbtft/fb_ili9163.c
+++ b/drivers/staging/fbtft/fb_ili9163.c
@@ -81,9 +81,9 @@ static int init_display(struct fbtft_par *par)
gpio_set_value(par->gpio.cs, 0); /* Activate chip */

write_reg(par, MIPI_DCS_SOFT_RESET); /* software reset */
- mdelay(500);
+ msleep(500);
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE); /* exit sleep */
- mdelay(5);
+ usleep_range(5000, 6000);
write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
/* default gamma curve 3 */
write_reg(par, MIPI_DCS_SET_GAMMA_CURVE, 0x02);
diff --git a/drivers/staging/fbtft/fb_ili9320.c b/drivers/staging/fbtft/fb_ili9320.c
index 501eee7dce4c..d89158e7a9df 100644
--- a/drivers/staging/fbtft/fb_ili9320.c
+++ b/drivers/staging/fbtft/fb_ili9320.c
@@ -94,25 +94,25 @@ static int init_display(struct fbtft_par *par)
write_reg(par, 0x0013, 0x0000);

/* Dis-charge capacitor power voltage */
- mdelay(200);
+ msleep(200);

/* SAP, BT[3:0], AP, DSTB, SLP, STB */
write_reg(par, 0x0010, 0x17B0);

/* R11h=0x0031 at VCI=3.3V DC1[2:0], DC0[2:0], VC[2:0] */
write_reg(par, 0x0011, 0x0031);
- mdelay(50);
+ msleep(50);

/* R12h=0x0138 at VCI=3.3V VREG1OUT voltage */
write_reg(par, 0x0012, 0x0138);
- mdelay(50);
+ msleep(50);

/* R13h=0x1800 at VCI=3.3V VDV[4:0] for VCOM amplitude */
write_reg(par, 0x0013, 0x1800);

/* R29h=0x0008 at VCI=3.3V VCM[4:0] for VCOMH */
write_reg(par, 0x0029, 0x0008);
- mdelay(50);
+ msleep(50);

/* GRAM horizontal Address */
write_reg(par, 0x0020, 0x0000);
diff --git a/drivers/staging/fbtft/fb_ili9325.c b/drivers/staging/fbtft/fb_ili9325.c
index d6b1d4be9ff4..50eeeb7afbc8 100644
--- a/drivers/staging/fbtft/fb_ili9325.c
+++ b/drivers/staging/fbtft/fb_ili9325.c
@@ -115,17 +115,17 @@ static int init_display(struct fbtft_par *par)
write_reg(par, 0x0011, 0x0007); /* DC1[2:0], DC0[2:0], VC[2:0] */
write_reg(par, 0x0012, 0x0000); /* VREG1OUT voltage */
write_reg(par, 0x0013, 0x0000); /* VDV[4:0] for VCOM amplitude */
- mdelay(200); /* Dis-charge capacitor power voltage */
+ msleep(200); /* Dis-charge capacitor power voltage */
write_reg(par, 0x0010, /* SAP, BT[3:0], AP, DSTB, SLP, STB */
BIT(12) | (bt << 8) | BIT(7) | BIT(4));
write_reg(par, 0x0011, 0x220 | vc); /* DC1[2:0], DC0[2:0], VC[2:0] */
- mdelay(50); /* Delay 50ms */
+ msleep(50); /* Delay 50ms */
write_reg(par, 0x0012, vrh); /* Internal reference voltage= Vci; */
- mdelay(50); /* Delay 50ms */
+ msleep(50); /* Delay 50ms */
write_reg(par, 0x0013, vdv << 8); /* Set VDV[4:0] for VCOM amplitude */
write_reg(par, 0x0029, vcm); /* Set VCM[5:0] for VCOMH */
write_reg(par, 0x002B, 0x000C); /* Set Frame Rate */
- mdelay(50); /* Delay 50ms */
+ msleep(50); /* Delay 50ms */
write_reg(par, 0x0020, 0x0000); /* GRAM horizontal Address */
write_reg(par, 0x0021, 0x0000); /* GRAM Vertical Address */

diff --git a/drivers/staging/fbtft/fb_ili9340.c b/drivers/staging/fbtft/fb_ili9340.c
index 430f21e50f4d..979f825930c3 100644
--- a/drivers/staging/fbtft/fb_ili9340.c
+++ b/drivers/staging/fbtft/fb_ili9340.c
@@ -72,7 +72,7 @@ static int init_display(struct fbtft_par *par)

write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);

- mdelay(120);
+ msleep(120);

write_reg(par, MIPI_DCS_SET_DISPLAY_ON);

diff --git a/drivers/staging/fbtft/fb_ili9341.c b/drivers/staging/fbtft/fb_ili9341.c
index a10e8c9de438..c384f96659ef 100644
--- a/drivers/staging/fbtft/fb_ili9341.c
+++ b/drivers/staging/fbtft/fb_ili9341.c
@@ -32,7 +32,7 @@ static int init_display(struct fbtft_par *par)

/* startup sequence for MI0283QT-9A */
write_reg(par, MIPI_DCS_SOFT_RESET);
- mdelay(5);
+ usleep_range(5000, 6000);
write_reg(par, MIPI_DCS_SET_DISPLAY_OFF);
/* --------------------------------------------------------- */
write_reg(par, 0xCF, 0x00, 0x83, 0x30);
@@ -58,9 +58,9 @@ static int init_display(struct fbtft_par *par)
write_reg(par, 0xB7, 0x07); /* entry mode set */
write_reg(par, 0xB6, 0x0A, 0x82, 0x27, 0x00);
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
- mdelay(100);
+ msleep(100);
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
- mdelay(20);
+ msleep(20);

return 0;
}
diff --git a/drivers/staging/fbtft/fb_st7789v.c b/drivers/staging/fbtft/fb_st7789v.c
index 7d7573a7b615..7033b82f2087 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -78,7 +78,7 @@ static int init_display(struct fbtft_par *par)
{
/* turn off sleep mode */
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
- mdelay(120);
+ msleep(120);

/* set pixel format to RGB-565 */
write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
index bfd1527f20f7..6994db5d9d36 100644
--- a/drivers/staging/fbtft/fb_watterott.c
+++ b/drivers/staging/fbtft/fb_watterott.c
@@ -156,9 +156,9 @@ static int init_display(struct fbtft_par *par)
}
write_reg(par, 0x00); /* make sure mode is set */

- mdelay(50);
+ msleep(50);
par->fbtftops.reset(par);
- mdelay(1000);
+ msleep(1000);
par->spi->mode = save_mode;
ret = spi_setup(par->spi);
if (ret) {
--
2.17.0



2018-07-27 10:36:15

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] staging: fbtft: Replace mdelay() with msleep() and usleep_range()

On Fri, Jul 27, 2018 at 12:21 PM, Jia-Ju Bai <[email protected]> wrote:
> reset() and init_display() are never called in atomic context.
> They call mdelay() to busily wait, which is not necessary.
> mdelay() can be replaced with msleep().

> gpio_set_value(par->gpio.reset, 0);
> udelay(20);
> gpio_set_value(par->gpio.reset, 1);
> - mdelay(120);
> + msleep(120);

I didn't look to the rest, but this one will be inconsistent after your patch.

The question here is why udelay() is needed, while mdelay() changed?

--
With Best Regards,
Andy Shevchenko

2018-07-27 11:44:38

by Jia-Ju Bai

[permalink] [raw]
Subject: Re: [PATCH] staging: fbtft: Replace mdelay() with msleep() and usleep_range()



On 2018/7/27 18:34, Andy Shevchenko wrote:
> On Fri, Jul 27, 2018 at 12:21 PM, Jia-Ju Bai <[email protected]> wrote:
>> reset() and init_display() are never called in atomic context.
>> They call mdelay() to busily wait, which is not necessary.
>> mdelay() can be replaced with msleep().
>> gpio_set_value(par->gpio.reset, 0);
>> udelay(20);
>> gpio_set_value(par->gpio.reset, 1);
>> - mdelay(120);
>> + msleep(120);
> I didn't look to the rest, but this one will be inconsistent after your patch.
>
> The question here is why udelay() is needed, while mdelay() changed?
>

I thought udelay() is used for short delay, so it is not very necessary
to change udelay() to usleep_range().
mdelay() is used for longer delay, so I changed it to msleep().

If you think udelay() should be also changed, I can send a new patch :)


Best wishes,
Jia-Ju Bai