Hi,
checking some patches thru www.kernel.org
/drivers/video/omap2/dss/dsi.c
I noticed in many places that if timeout happens,
while "read/status/check" is not done after the last Nth udelay.
Why not "check" after the last udelay?
Lot of different logics for udelays/timeouts:
i=0;
while( checking(x) ) {
udelay(1);
i++;
if ( i > N ) {
/* timeout /*
}
} /* timeout after N+1 checking(x)'s and N+1 udelay(1)'s,
no checking(x) done after last udelay */
i=0;
while( checking(x) ) {
udelay(1);
if ( i++ > N ) {
/* timeout /*
}
} /* timeout after N+2 checking(x)'s and N+2 udelay(1)'s,
no checking(x) done after last udelay */
i=0;
while( checking(x) ) {
if ( i++ > N ) {
/* timeout /*
}
udelay(1);
} /* timeout after N+3 checking(x)'s and N+2 udelay(1)'s,
checking(x) done!! after last udelay */
/* My version */
i=0;
while( checking(x) ) {
if ( ++i > N ) {
/* timeout /*
}
udelay(1);
} /* timeout after N+1 checking(x)'s and N udelay(1)'s,
checking(x) done!! after last udelay,
is this what is intended? This is my understanding. */
Suggested modifications:
..........
static int dsi_pll_power(enum dsi_pll_power_state state)
{
int t = 0;
REG_FLD_MOD(DSI_CLK_CTRL, state, 31, 30); /* PLL_PWR_CMD */
/* PLL_PWR_STATUS */
while (FLD_GET(dsi_read_reg(DSI_CLK_CTRL), 29, 28) != state) {
- udelay(1);
- if (t++ > 1000) {
+ if (++t > 1000) {
DSSERR("Failed to set DSI PLL power mode to %d\n",
state);
return -ENODEV;
}
+ udelay(1);
}
return 0;
}
.....
static int dsi_complexio_power(enum dsi_complexio_power_state state)
{
int t = 0;
/* PWR_CMD */
REG_FLD_MOD(DSI_COMPLEXIO_CFG1, state, 28, 27);
/* PWR_STATUS */
while (FLD_GET(dsi_read_reg(DSI_COMPLEXIO_CFG1), 26, 25) != state) {
- udelay(1);
- if (t++ > 1000) {
+ if (++t > 1000) {
DSSERR("failed to set complexio power state to "
"%d\n", state);
return -ENODEV;
}
+ udelay(1);
}
return 0;
}
.....
static int dsi_update_screen_l4(struct omap_dss_device *dssdev,
int x, int y, int w, int h)
{
/* Note: supports only 24bit colors in 32bit container */
int first = 1;
int fifo_stalls = 0;
<SNIP>
#if 1
/* using fifo not empty */
/* TX_FIFO_NOT_EMPTY */
while (FLD_GET(dsi_read_reg(DSI_VC_CTRL(0)), 5, 5)) {
- udelay(1);
fifo_stalls++;
if (fifo_stalls > 0xfffff) {
DSSERR("fifo stalls overflow, pixels left %d\n",
pixels_left);
dsi_if_enable(0);
return -EIO;
}
+ udelay(1);
}
#elif 1
<SNIP>
.....
static int _dsi_wait_reset(void)
{
int i = 0;
while (REG_GET(DSI_SYSSTATUS, 0, 0) == 0) {
- if (i++ > 5) {
+ if (++i > 5) {
DSSERR("soft reset failed\n");
return -ENODEV;
}
udelay(1);
}
return 0;
}
.....
Sorry if this mail is completely pointless,
I just like to see consistent looking code,
that shows you what the intent of the coder is.
Reported-by: "Juha Leppanen" <[email protected]>
Signed-off-by: "Juha Leppanen" <[email protected]>
No reply needed to me,
my spam filter will probably eat your reply.
Happy hacking,
Mr. Juha Leppanen
Kuopio, Finland
....................................................................
Luukku Plus -paketilla pääset eroon tila- ja turvallisuusongelmista.
Hanki Luukku Plus ja helpotat elämääsi. http://www.mtv3.fi/luukku
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
