staging: dgnc: replace usleep_range with udelay

PATCH Superseded Aishwarya Pant 2 participants 4 messages Mar 1, 2017 Staging

Share & tools

Aishwarya Pant

author

2017-03-01 10:36 UTC #0 lore raw

PATCH v3 1/1 staging: dgnc: replace usleep_range with udelay

udelay is impelmented using a busy-wait loop and consumes CPU cycles while usleep_range is implemented using interrupts.cls_flush_uart_write() is called after a channel lock is acquired i.e. an atomic context. Hence delay in this method should use udelay instead of usleep_range.

Signed-off-by Aishwarya Pant <[email protected]>

---
Changes in v3:
- Add a comment in code udelay to ignore checkpatch warning
Changes in v2:
- Edit commit message to explain why udelay should be used

 drivers/staging/dgnc/dgnc_cls.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/staging/dgnc/dgnc_cls.c +5 -1

index 28a3e16..ec190e5 100644
@@ -609,7 +609,11 @@ static void cls_flush_uart_write(struct channel_t *ch)
 
 	writeb((UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_XMIT),
 	       &ch->ch_cls_uart->isr_fcr);
-	usleep_range(10, 20);
+
+	/* Ignore checkpatch warning to replace udelay by usleep_range
+	 * as this function is called under an atomic context
+	 */
+	udelay(10);
 
 	ch->ch_flags |= (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 }

Signature · 1 line

2.7.4

Julia Lawall

2017-03-01 10:56 UTC #1 lore raw

Quoted 21 lines · Aishwarya Pant

udelay is impelmented using a busy-wait loop and consumes CPU cycles while usleep_range is implemented using interrupts.cls_flush_uart_write() is called after a channel lock is acquired i.e. an atomic context. Hence delay in this method should use udelay instead of usleep_range.
Signed-off-by Aishwarya Pant <[email protected]>
---
Changes in v3:
- Add a comment in code udelay to ignore checkpatch warning
Changes in v2:
- Edit commit message to explain why udelay should be used
 drivers/staging/dgnc/dgnc_cls.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/staging/dgnc/dgnc_cls.c +0 -0
index 28a3e16..ec190e5 100644

drivers/staging/dgnc/dgnc_cls.c static void cls_flush_uart_write(struct channel_t *ch) · line 616

Quoted 9 lines

drivers/staging/dgnc/dgnc_cls.c +5 -1

@@ -609,7 +609,11 @@ static void cls_flush_uart_write(struct channel_t *ch)
 
 	writeb((UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_XMIT),
 	       &ch->ch_cls_uart->isr_fcr);
-	usleep_range(10, 20);
+
+	/* Ignore checkpatch warning to replace udelay by usleep_range
+	 * as this function is called under an atomic context
+	 */
+	udelay(10);

I think this doesn't follow the proper block comment format?

Anyway, perhaps something like "called in atomic context so usleep_range not used" would be sufficient.

julia

Quoted 13 lines

drivers/staging/dgnc/dgnc_cls.c +0 -1
 	ch->ch_flags |= (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 }
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170301103315.GA4448%40aishwarya.
For more options, visit https://groups.google.com/d/optout.

Aishwarya Pant

author

2017-03-01 11:12 UTC #2 lore raw

Quoted 26 lines · Julia Lawall

Quoted 22 lines · Aishwarya Pant
udelay is impelmented using a busy-wait loop and consumes CPU cycles while usleep_range is implemented using interrupts.cls_flush_uart_write() is called after a channel lock is acquired i.e. an atomic context. Hence delay in this method should use udelay instead of usleep_range.
Signed-off-by Aishwarya Pant <[email protected]>
---
Changes in v3:
- Add a comment in code udelay to ignore checkpatch warning
Changes in v2:
- Edit commit message to explain why udelay should be used
 drivers/staging/dgnc/dgnc_cls.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/staging/dgnc/dgnc_cls.c +0 -0
index 28a3e16..ec190e5 100644
@@ -609,7 +609,11 @@ static void cls_flush_uart_write(struct channel_t *ch)
 

Quoted 11 lines

drivers/staging/dgnc/dgnc_cls.c static void cls_flush_uart_write(struct channel_t *ch) · line 616
Quoted 8 lines
drivers/staging/dgnc/dgnc_cls.c +5 -1
 	writeb((UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_XMIT),
 	       &ch->ch_cls_uart->isr_fcr);
-	usleep_range(10, 20);
+
+	/* Ignore checkpatch warning to replace udelay by usleep_range
+	 * as this function is called under an atomic context
+	 */
+	udelay(10);
I think this doesn't follow the proper block comment format?

I looked through the coding style docs and found the correct format for multi-line comments-
/*
* Some comments spanning over
* multiple lines
*/

What is convention for single line comments?

Quoted 18 lines

Anyway, perhaps something like "called in atomic context so usleep_range not used" would be sufficient.
julia
Quoted 13 lines
drivers/staging/dgnc/dgnc_cls.c +0 -1
 	ch->ch_flags |= (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 }
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170301103315.GA4448%40aishwarya.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-03-01 13:07 UTC #3 lore raw

Quoted 39 lines · Aishwarya Pant

Quoted 26 lines · Julia Lawall
Quoted 22 lines · Aishwarya Pant
udelay is impelmented using a busy-wait loop and consumes CPU cycles while usleep_range is implemented using interrupts.cls_flush_uart_write() is called after a channel lock is acquired i.e. an atomic context. Hence delay in this method should use udelay instead of usleep_range.
Signed-off-by Aishwarya Pant <[email protected]>
---
Changes in v3:
- Add a comment in code udelay to ignore checkpatch warning
Changes in v2:
- Edit commit message to explain why udelay should be used
 drivers/staging/dgnc/dgnc_cls.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/staging/dgnc/dgnc_cls.c +0 -0
index 28a3e16..ec190e5 100644
@@ -609,7 +609,11 @@ static void cls_flush_uart_write(struct channel_t *ch)
 
Quoted 11 lines
drivers/staging/dgnc/dgnc_cls.c static void cls_flush_uart_write(struct channel_t *ch) · line 616
Quoted 8 lines
drivers/staging/dgnc/dgnc_cls.c +5 -1
 	writeb((UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_XMIT),
 	       &ch->ch_cls_uart->isr_fcr);
-	usleep_range(10, 20);
+
+	/* Ignore checkpatch warning to replace udelay by usleep_range
+	 * as this function is called under an atomic context
+	 */
+	udelay(10);
I think this doesn't follow the proper block comment format?

Quoted 8 lines

I looked through the coding style docs and found the correct format for multi-line comments-
/*
* Some comments spanning over
* multiple lines
*/
What is convention for single line comments?

/* text */

julia

Quoted 20 lines

Quoted 19 lines
Anyway, perhaps something like "called in atomic context so usleep_range not used" would be sufficient.
julia
Quoted 13 lines
drivers/staging/dgnc/dgnc_cls.c +0 -1
 	ch->ch_flags |= (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 }
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170301103315.GA4448%40aishwarya.
For more options, visit https://groups.google.com/d/optout.