[PATCH] staging: dgnc: dgnc_cls.c: usleep_range is preferred over udelay

[PATCH] staging: dgnc: dgnc_cls.c: usleep_range is preferred over udelay

[Anchal Jain]

According to Documentation/timers/timers-howto.txt"
udelay() is only called once from a place where sleeping is allowed.
We can replace it with a call to usleep_range()
with a reasonable upper limit.

Signed-off-by: Anchal Jain <anchalj109@gmail.com>
---
 drivers/staging/dgnc/dgnc_cls.c      |  6 +++---
 drivers/staging/wilc1000/linux_mon.c | 11 +++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
index 4e1e0dc6..1b07255 100644
--- a/drivers/staging/dgnc/dgnc_cls.c
+++ b/drivers/staging/dgnc/dgnc_cls.c
@@ -410,7 +410,7 @@ static void cls_assert_modem_signals(struct channel_t *ch)
 	writeb(out, &ch->ch_cls_uart->mcr);
 
 	/* Give time for the UART to actually drop the signals */
-	udelay(10);
+	usleep_range(10, 20);
 }
 
 static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
@@ -632,7 +632,7 @@ static void cls_flush_uart_read(struct channel_t *ch)
 	 * Presumably, this is a bug in this UART.
 	 */
 
-	udelay(10);
+	usleep_delay(10, 20);
 }
 
 /*
@@ -1104,7 +1104,7 @@ static void cls_uart_init(struct channel_t *ch)
 
 	writeb(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
 	       &ch->ch_cls_uart->isr_fcr);
-	udelay(10);
+	usleep_delay(10, 20);
 
 	ch->ch_flags |= (CH_FIFO_ENABLED | CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 
diff --git a/drivers/staging/wilc1000/linux_mon.c b/drivers/staging/wilc1000/linux_mon.c
index 242f82f..22f7684 100644
--- a/drivers/staging/wilc1000/linux_mon.c
+++ b/drivers/staging/wilc1000/linux_mon.c
@@ -59,9 +59,15 @@ void WILC_WFI_monitor_rx(u8 *buff, u32 size)
 
 	/* Get WILC header */
 	memcpy(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
+<<<<<<< HEAD
 	/*
 	 * The packet offset field contain info about what type of management
 	 * the frame we are dealing with and ack status
+=======
+
+	/* The packet offset field conain info about what type of management
+	 * frame we are dealing with and ack status
+>>>>>>> staging: wilc1000: Fix lines over 80 characters
 	 */
 	pkt_offset = GET_PKT_OFFSET(header);
 
@@ -128,8 +134,13 @@ struct tx_complete_mon_data {
 static void mgmt_tx_complete(void *priv, int status)
 {
 	struct tx_complete_mon_data *pv_data = priv;
+<<<<<<< HEAD
 	/*
 	 * in case of fully hosting mode, the freeing will be done
+=======
+
+	/* in case of fully hosting mode, the freeing will be done
+>>>>>>> staging: wilc1000: Fix lines over 80 characters
 	 * in response to the cfg packet
 	 */
 	kfree(pv_data->buff);
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] staging: dgnc: dgnc_cls.c: usleep_range is preferred over udelay

[Julia Lawall]

On Tue, 13 Sep 2016, Anchal Jain wrote:

> According to Documentation/timers/timers-howto.txt"
> udelay() is only called once from a place where sleeping is allowed.
> We can replace it with a call to usleep_range()
> with a reasonable upper limit.
>
> Signed-off-by: Anchal Jain <anchalj109@gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_cls.c      |  6 +++---
>  drivers/staging/wilc1000/linux_mon.c | 11 +++++++++++
>  2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
> index 4e1e0dc6..1b07255 100644
> --- a/drivers/staging/dgnc/dgnc_cls.c
> +++ b/drivers/staging/dgnc/dgnc_cls.c
> @@ -410,7 +410,7 @@ static void cls_assert_modem_signals(struct channel_t *ch)
>  	writeb(out, &ch->ch_cls_uart->mcr);
>
>  	/* Give time for the UART to actually drop the signals */
> -	udelay(10);
> +	usleep_range(10, 20);
>  }
>
>  static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
> @@ -632,7 +632,7 @@ static void cls_flush_uart_read(struct channel_t *ch)
>  	 * Presumably, this is a bug in this UART.
>  	 */
>
> -	udelay(10);
> +	usleep_delay(10, 20);
>  }
>
>  /*
> @@ -1104,7 +1104,7 @@ static void cls_uart_init(struct channel_t *ch)
>
>  	writeb(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
>  	       &ch->ch_cls_uart->isr_fcr);
> -	udelay(10);
> +	usleep_delay(10, 20);
>
>  	ch->ch_flags |= (CH_FIFO_ENABLED | CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
>
> diff --git a/drivers/staging/wilc1000/linux_mon.c b/drivers/staging/wilc1000/linux_mon.c
> index 242f82f..22f7684 100644
> --- a/drivers/staging/wilc1000/linux_mon.c
> +++ b/drivers/staging/wilc1000/linux_mon.c
> @@ -59,9 +59,15 @@ void WILC_WFI_monitor_rx(u8 *buff, u32 size)
>
>  	/* Get WILC header */
>  	memcpy(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> +<<<<<<< HEAD

This looks like git noise, and should not be in the patch.

julia


>  	/*
>  	 * The packet offset field contain info about what type of management
>  	 * the frame we are dealing with and ack status
> +=======
> +
> +	/* The packet offset field conain info about what type of management
> +	 * frame we are dealing with and ack status
> +>>>>>>> staging: wilc1000: Fix lines over 80 characters
>  	 */
>  	pkt_offset = GET_PKT_OFFSET(header);
>
> @@ -128,8 +134,13 @@ struct tx_complete_mon_data {
>  static void mgmt_tx_complete(void *priv, int status)
>  {
>  	struct tx_complete_mon_data *pv_data = priv;
> +<<<<<<< HEAD
>  	/*
>  	 * in case of fully hosting mode, the freeing will be done
> +=======
> +
> +	/* in case of fully hosting mode, the freeing will be done
> +>>>>>>> staging: wilc1000: Fix lines over 80 characters
>  	 * in response to the cfg packet
>  	 */
>  	kfree(pv_data->buff);
> --
> 1.9.1
>
> --
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160913075211.GA6959%40life-desktop.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] staging: dgnc: dgnc_cls.c: usleep_range is preferred over udelay

[Arnd Bergmann]

On Tuesday, September 13, 2016 1:22:26 PM CEST Anchal Jain wrote:
> According to Documentation/timers/timers-howto.txt"
> udelay() is only called once from a place where sleeping is allowed.
> We can replace it with a call to usleep_range()
> with a reasonable upper limit.
> 
> Signed-off-by: Anchal Jain <anchalj109@gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_cls.c      |  6 +++---
>  drivers/staging/wilc1000/linux_mon.c | 11 +++++++++++
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
> index 4e1e0dc6..1b07255 100644
> --- a/drivers/staging/dgnc/dgnc_cls.c
> +++ b/drivers/staging/dgnc/dgnc_cls.c
> @@ -410,7 +410,7 @@ static void cls_assert_modem_signals(struct channel_t *ch)
>  	writeb(out, &ch->ch_cls_uart->mcr);
>  
>  	/* Give time for the UART to actually drop the signals */
> -	udelay(10);
> +	usleep_range(10, 20);
>  }
>  

This gets called from cls_copy_data_from_queue_to_uart() under a spinlock.

> diff --git a/drivers/staging/wilc1000/linux_mon.c b/drivers/staging/wilc1000/linux_mon.c
> index 242f82f..22f7684 100644
> --- a/drivers/staging/wilc1000/linux_mon.c
> +++ b/drivers/staging/wilc1000/linux_mon.c
> @@ -59,9 +59,15 @@ void WILC_WFI_monitor_rx(u8 *buff, u32 size)
>  
>  	/* Get WILC header */
>  	memcpy(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> +<<<<<<< HEAD
>  	/*
>  	 * The packet offset field contain info about what type of management
>  	 * the frame we are dealing with and ack status
> +=======

Clearly doesn't belong into the patch.

	Arnd

Re: [Outreachy kernel] [PATCH] staging: dgnc: dgnc_cls.c: usleep_range is preferred over udelay

[Anchal Jain]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

> This gets called from cls_copy_data_from_queue_to_uart() under a spinlock.

According to the documentation "Documentation/timers/timers-howto.txt".
In this documentation no explanation of "spinlock". What is that meaning?

On Tue, Sep 13, 2016 at 1:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday, September 13, 2016 1:22:26 PM CEST Anchal Jain wrote:
> > According to Documentation/timers/timers-howto.txt"
> > udelay() is only called once from a place where sleeping is allowed.
> > We can replace it with a call to usleep_range()
> > with a reasonable upper limit.
> >
> > Signed-off-by: Anchal Jain <anchalj109@gmail.com>
> > ---
> >  drivers/staging/dgnc/dgnc_cls.c      |  6 +++---
> >  drivers/staging/wilc1000/linux_mon.c | 11 +++++++++++
> >  2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/dgnc/dgnc_cls.c
> b/drivers/staging/dgnc/dgnc_cls.c
> > index 4e1e0dc6..1b07255 100644
> > --- a/drivers/staging/dgnc/dgnc_cls.c
> > +++ b/drivers/staging/dgnc/dgnc_cls.c
> > @@ -410,7 +410,7 @@ static void cls_assert_modem_signals(struct
> channel_t *ch)
> >       writeb(out, &ch->ch_cls_uart->mcr);
> >
> >       /* Give time for the UART to actually drop the signals */
> > -     udelay(10);
> > +     usleep_range(10, 20);
> >  }
> >
>
> This gets called from cls_copy_data_from_queue_to_uart() under a spinlock.
>
> > diff --git a/drivers/staging/wilc1000/linux_mon.c
> b/drivers/staging/wilc1000/linux_mon.c
> > index 242f82f..22f7684 100644
> > --- a/drivers/staging/wilc1000/linux_mon.c
> > +++ b/drivers/staging/wilc1000/linux_mon.c
> > @@ -59,9 +59,15 @@ void WILC_WFI_monitor_rx(u8 *buff, u32 size)
> >
> >       /* Get WILC header */
> >       memcpy(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> > +<<<<<<< HEAD
> >       /*
> >        * The packet offset field contain info about what type of
> management
> >        * the frame we are dealing with and ack status
> > +=======
>
> Clearly doesn't belong into the patch.
>
>         Arnd
>

[-- Attachment #2: Type: text/html, Size: 2893 bytes --]

Re: [Outreachy kernel] [PATCH] staging: dgnc: dgnc_cls.c: usleep_range is preferred over udelay

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3150 bytes --]



On Tue, 13 Sep 2016, Anchal Jain wrote:

> > This gets called from cls_copy_data_from_queue_to_uart() under a spinlock.
>
> According to the documentation "Documentation/timers/timers-howto.txt".
> In this documentation no explanation of "spinlock". What is that meaning?

A spinlock is a certain kind of lock.  It could be taken with a function
like spin_lock or spin_lock_irqsave.  Probably you can find more
information with google.

julia

>
> On Tue, Sep 13, 2016 at 1:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>       On Tuesday, September 13, 2016 1:22:26 PM CEST Anchal Jain
>       wrote:
>       > According to Documentation/timers/timers-howto.txt"
>       > udelay() is only called once from a place where sleeping is
>       allowed.
>       > We can replace it with a call to usleep_range()
>       > with a reasonable upper limit.
>       >
>       > Signed-off-by: Anchal Jain <anchalj109@gmail.com>
>       > ---
>       >  drivers/staging/dgnc/dgnc_cls.c      |  6 +++---
>       >  drivers/staging/wilc1000/linux_mon.c | 11 +++++++++++
>       >  2 files changed, 14 insertions(+), 3 deletions(-)
>       >
>       > diff --git a/drivers/staging/dgnc/dgnc_cls.c
>       b/drivers/staging/dgnc/dgnc_cls.c
>       > index 4e1e0dc6..1b07255 100644
>       > --- a/drivers/staging/dgnc/dgnc_cls.c
>       > +++ b/drivers/staging/dgnc/dgnc_cls.c
>       > @@ -410,7 +410,7 @@ static void
>       cls_assert_modem_signals(struct channel_t *ch)
>       >       writeb(out, &ch->ch_cls_uart->mcr);
>       >
>       >       /* Give time for the UART to actually drop the signals
>       */
>       > -     udelay(10);
>       > +     usleep_range(10, 20);
>       >  }
>       >
>
>       This gets called from cls_copy_data_from_queue_to_uart() under a
>       spinlock.
>
>       > diff --git a/drivers/staging/wilc1000/linux_mon.c
>       b/drivers/staging/wilc1000/linux_mon.c
>       > index 242f82f..22f7684 100644
>       > --- a/drivers/staging/wilc1000/linux_mon.c
>       > +++ b/drivers/staging/wilc1000/linux_mon.c
>       > @@ -59,9 +59,15 @@ void WILC_WFI_monitor_rx(u8 *buff, u32
>       size)
>       >
>       >       /* Get WILC header */
>       >       memcpy(&header, (buff - HOST_HDR_OFFSET),
>       HOST_HDR_OFFSET);
>       > +<<<<<<< HEAD
>       >       /*
>       >        * The packet offset field contain info about what type
>       of management
>       >        * the frame we are dealing with and ack status
>       > +=======
>
>       Clearly doesn't belong into the patch.
>
>               Arnd
>
>
> --
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/CAAbeOSv0%2BHCx28Ug1eqEn
> FsEjT3sMuP6aMUR%2BJ2G_gKFuYYcaw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

Re: [Outreachy kernel] [PATCH] staging: dgnc: dgnc_cls.c: usleep_range is preferred over udelay

[Arnd Bergmann]

On Tuesday, September 13, 2016 1:54:57 PM CEST Anchal Jain wrote:
> > This gets called from cls_copy_data_from_queue_to_uart() under a spinlock.
> 
> According to the documentation "Documentation/timers/timers-howto.txt".
> In this documentation no explanation of "spinlock". What is that meaning?

Holding a spinlock is one of the things that imply "atomic context",
which is mentioned there. Other examples would be

- being called from an interrupt handler
- disabling interrupts
- disabling preemption

	Arnd

Re: [Outreachy kernel] [PATCH] staging: dgnc: dgnc_cls.c: usleep_range is preferred over udelay

[Anchal Jain]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

Thank you, Now I understand. I will send the next version of the patch soon.

On Tue, Sep 13, 2016 at 4:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday, September 13, 2016 1:54:57 PM CEST Anchal Jain wrote:
> > > This gets called from cls_copy_data_from_queue_to_uart() under a
> spinlock.
> >
> > According to the documentation "Documentation/timers/timers-howto.txt".
> > In this documentation no explanation of "spinlock". What is that meaning?
>
> Holding a spinlock is one of the things that imply "atomic context",
> which is mentioned there. Other examples would be
>
> - being called from an interrupt handler
> - disabling interrupts
> - disabling preemption
>
>         Arnd
>

[-- Attachment #2: Type: text/html, Size: 1168 bytes --]