Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 3/3] serial: core: Update uart_poll_timeout() function to return unsigned long
@ 2023-10-25 14:26 Vamshi Gajjela
  2023-10-26  8:56 ` Jiri Slaby
  0 siblings, 1 reply; 6+ messages in thread
From: Vamshi Gajjela @ 2023-10-25 14:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, ilpo.jarvinen
  Cc: linux-serial, linux-kernel, manugautam, Subhash Jadavani,
	Channa Kadabi, VAMSHI GAJJELA

From: VAMSHI GAJJELA <vamshigajjela@google.com>

The function uart_fifo_timeout() returns an unsigned long value, which is
the number of jiffies. Therefore, the function uart_poll_timeout() has been
modified to use an unsigned long type for timeout values instead of an int
and to avoid truncation.
The return type of the function uart_poll_timeout() has also been changed
from int to unsigned long to be consistent with the type of timeout values.

Signed-off-by: VAMSHI GAJJELA <vamshigajjela@google.com>
---
v2:
- unsigned long instead of unsigned int
- added () after function name in short log
- updated description
 include/linux/serial_core.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index bb6f073bc159..6916a1d7e477 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -773,9 +773,9 @@ static inline unsigned long uart_fifo_timeout(struct uart_port *port)
 }
 
 /* Base timer interval for polling */
-static inline int uart_poll_timeout(struct uart_port *port)
+static inline unsigned long uart_poll_timeout(struct uart_port *port)
 {
-	int timeout = uart_fifo_timeout(port);
+	unsigned long timeout = uart_fifo_timeout(port);
 
 	return timeout > 6 ? (timeout / 2 - 2) : 1;
 }
-- 
2.42.0.758.gaed0368e0e-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] serial: core: Update uart_poll_timeout() function to return unsigned long
  2023-10-25 14:26 [PATCH v2 3/3] serial: core: Update uart_poll_timeout() function to return unsigned long Vamshi Gajjela
@ 2023-10-26  8:56 ` Jiri Slaby
  2023-10-26  9:24   ` Ilpo Järvinen
  0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2023-10-26  8:56 UTC (permalink / raw)
  To: Vamshi Gajjela, Greg Kroah-Hartman, ilpo.jarvinen
  Cc: linux-serial, linux-kernel, manugautam, Subhash Jadavani,
	Channa Kadabi

On 25. 10. 23, 16:26, Vamshi Gajjela wrote:
> From: VAMSHI GAJJELA <vamshigajjela@google.com>
> 
> The function uart_fifo_timeout() returns an unsigned long value, which is
> the number of jiffies. Therefore, the function uart_poll_timeout() has been
> modified to use an unsigned long type for timeout values instead of an int
> and to avoid truncation.

Hi,

there is no truncation possible, right?

> The return type of the function uart_poll_timeout() has also been changed
> from int to unsigned long to be consistent with the type of timeout values.
> 
> Signed-off-by: VAMSHI GAJJELA <vamshigajjela@google.com>
> ---
> v2:
> - unsigned long instead of unsigned int
> - added () after function name in short log
> - updated description
>   include/linux/serial_core.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index bb6f073bc159..6916a1d7e477 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -773,9 +773,9 @@ static inline unsigned long uart_fifo_timeout(struct uart_port *port)
>   }
>   
>   /* Base timer interval for polling */
> -static inline int uart_poll_timeout(struct uart_port *port)
> +static inline unsigned long uart_poll_timeout(struct uart_port *port)
>   {
> -	int timeout = uart_fifo_timeout(port);
> +	unsigned long timeout = uart_fifo_timeout(port);
>   
>   	return timeout > 6 ? (timeout / 2 - 2) : 1;
>   }

-- 
js
suse labs


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] serial: core: Update uart_poll_timeout() function to return unsigned long
  2023-10-26  8:56 ` Jiri Slaby
@ 2023-10-26  9:24   ` Ilpo Järvinen
  2023-10-26  9:49     ` Jiri Slaby
  2023-10-26 12:28     ` VAMSHI GAJJELA
  0 siblings, 2 replies; 6+ messages in thread
From: Ilpo Järvinen @ 2023-10-26  9:24 UTC (permalink / raw)
  To: Jiri Slaby, Vamshi Gajjela
  Cc: Greg Kroah-Hartman, linux-serial, LKML, manugautam,
	Subhash Jadavani, Channa Kadabi

On Thu, 26 Oct 2023, Jiri Slaby wrote:

> On 25. 10. 23, 16:26, Vamshi Gajjela wrote:
> > From: VAMSHI GAJJELA <vamshigajjela@google.com>
> > 
> > The function uart_fifo_timeout() returns an unsigned long value, which is
> > the number of jiffies. Therefore, the function uart_poll_timeout() has been
> > modified to use an unsigned long type for timeout values instead of an int
> > and to avoid truncation.
> 
> Hi,
> 
> there is no truncation possible, right?

That's very likely true (I didn't run the calculations), thus it's correct 
to not have Fixes tag. It's more about having consistent typing since 
we're talking about jiffies, so unsigned long as usual.

> > The return type of the function uart_poll_timeout() has also been changed
> > from int to unsigned long to be consistent with the type of timeout values.

Don't write changes you make in the patch in the past tense. Just say:

Change the return type of uart_poll_timeout() from ...

The comment also applies to the other sentence above this one.

-- 
 i.

> > Signed-off-by: VAMSHI GAJJELA <vamshigajjela@google.com>
> > ---
> > v2:
> > - unsigned long instead of unsigned int
> > - added () after function name in short log
> > - updated description
> >   include/linux/serial_core.h | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > index bb6f073bc159..6916a1d7e477 100644
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -773,9 +773,9 @@ static inline unsigned long uart_fifo_timeout(struct
> > uart_port *port)
> >   }
> >     /* Base timer interval for polling */
> > -static inline int uart_poll_timeout(struct uart_port *port)
> > +static inline unsigned long uart_poll_timeout(struct uart_port *port)
> >   {
> > -	int timeout = uart_fifo_timeout(port);
> > +	unsigned long timeout = uart_fifo_timeout(port);
> >     	return timeout > 6 ? (timeout / 2 - 2) : 1;
> >   }
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] serial: core: Update uart_poll_timeout() function to return unsigned long
  2023-10-26  9:24   ` Ilpo Järvinen
@ 2023-10-26  9:49     ` Jiri Slaby
  2023-10-26 12:30       ` VAMSHI GAJJELA
  2023-10-26 12:28     ` VAMSHI GAJJELA
  1 sibling, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2023-10-26  9:49 UTC (permalink / raw)
  To: Ilpo Järvinen, Vamshi Gajjela
  Cc: Greg Kroah-Hartman, linux-serial, LKML, manugautam,
	Subhash Jadavani, Channa Kadabi

On 26. 10. 23, 11:24, Ilpo Järvinen wrote:
> On Thu, 26 Oct 2023, Jiri Slaby wrote:
> 
>> On 25. 10. 23, 16:26, Vamshi Gajjela wrote:
>>> From: VAMSHI GAJJELA <vamshigajjela@google.com>
>>>
>>> The function uart_fifo_timeout() returns an unsigned long value, which is
>>> the number of jiffies. Therefore, the function uart_poll_timeout() has been
>>> modified to use an unsigned long type for timeout values instead of an int
>>> and to avoid truncation.
>>
>> Hi,
>>
>> there is no truncation possible, right?
> 
> That's very likely true (I didn't run the calculations), thus it's correct
> to not have Fixes tag.

Good. I would remove that "and to avoid truncation" completely from the 
commit log. I would only say that we are getting things consistent.

"avoid truncation" can lead to -> we should backport it to stable. And 
neither is true.

> It's more about having consistent typing since
> we're talking about jiffies, so unsigned long as usual.

Yeah and that's of course fine by me.

thanks,
-- 
js
suse labs


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] serial: core: Update uart_poll_timeout() function to return unsigned long
  2023-10-26  9:24   ` Ilpo Järvinen
  2023-10-26  9:49     ` Jiri Slaby
@ 2023-10-26 12:28     ` VAMSHI GAJJELA
  1 sibling, 0 replies; 6+ messages in thread
From: VAMSHI GAJJELA @ 2023-10-26 12:28 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Jiri Slaby, Greg Kroah-Hartman, linux-serial, LKML, manugautam,
	Subhash Jadavani, Channa Kadabi

On Thu, Oct 26, 2023 at 2:54 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Thu, 26 Oct 2023, Jiri Slaby wrote:
>
> > On 25. 10. 23, 16:26, Vamshi Gajjela wrote:
> > > From: VAMSHI GAJJELA <vamshigajjela@google.com>
> > >
> > > The function uart_fifo_timeout() returns an unsigned long value, which is
> > > the number of jiffies. Therefore, the function uart_poll_timeout() has been
> > > modified to use an unsigned long type for timeout values instead of an int
> > > and to avoid truncation.
> >
> > Hi,
> >
> > there is no truncation possible, right?
>
> That's very likely true (I didn't run the calculations), thus it's correct
> to not have Fixes tag. It's more about having consistent typing since
> we're talking about jiffies, so unsigned long as usual.
>
> > > The return type of the function uart_poll_timeout() has also been changed
> > > from int to unsigned long to be consistent with the type of timeout values.
>
> Don't write changes you make in the patch in the past tense. Just say:
>
> Change the return type of uart_poll_timeout() from ...
>
> The comment also applies to the other sentence above this one.
Sure, will update commit message.
>
> --
>  i.
>
> > > Signed-off-by: VAMSHI GAJJELA <vamshigajjela@google.com>
> > > ---
> > > v2:
> > > - unsigned long instead of unsigned int
> > > - added () after function name in short log
> > > - updated description
> > >   include/linux/serial_core.h | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > > index bb6f073bc159..6916a1d7e477 100644
> > > --- a/include/linux/serial_core.h
> > > +++ b/include/linux/serial_core.h
> > > @@ -773,9 +773,9 @@ static inline unsigned long uart_fifo_timeout(struct
> > > uart_port *port)
> > >   }
> > >     /* Base timer interval for polling */
> > > -static inline int uart_poll_timeout(struct uart_port *port)
> > > +static inline unsigned long uart_poll_timeout(struct uart_port *port)
> > >   {
> > > -   int timeout = uart_fifo_timeout(port);
> > > +   unsigned long timeout = uart_fifo_timeout(port);
> > >             return timeout > 6 ? (timeout / 2 - 2) : 1;
> > >   }
> >
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] serial: core: Update uart_poll_timeout() function to return unsigned long
  2023-10-26  9:49     ` Jiri Slaby
@ 2023-10-26 12:30       ` VAMSHI GAJJELA
  0 siblings, 0 replies; 6+ messages in thread
From: VAMSHI GAJJELA @ 2023-10-26 12:30 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Ilpo Järvinen, Greg Kroah-Hartman, linux-serial, LKML,
	manugautam, Subhash Jadavani, Channa Kadabi

On Thu, Oct 26, 2023 at 3:19 PM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 26. 10. 23, 11:24, Ilpo Järvinen wrote:
> > On Thu, 26 Oct 2023, Jiri Slaby wrote:
> >
> >> On 25. 10. 23, 16:26, Vamshi Gajjela wrote:
> >>> From: VAMSHI GAJJELA <vamshigajjela@google.com>
> >>>
> >>> The function uart_fifo_timeout() returns an unsigned long value, which is
> >>> the number of jiffies. Therefore, the function uart_poll_timeout() has been
> >>> modified to use an unsigned long type for timeout values instead of an int
> >>> and to avoid truncation.
> >>
> >> Hi,
> >>
> >> there is no truncation possible, right?
> >
> > That's very likely true (I didn't run the calculations), thus it's correct
> > to not have Fixes tag.
>
> Good. I would remove that "and to avoid truncation" completely from the
> commit log. I would only say that we are getting things consistent.
>
> "avoid truncation" can lead to -> we should backport it to stable. And
> neither is true.
I will drop "and to avoid truncation".
>
> > It's more about having consistent typing since
> > we're talking about jiffies, so unsigned long as usual.
>
> Yeah and that's of course fine by me.
>
> thanks,
> --
> js
> suse labs
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-26 12:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-25 14:26 [PATCH v2 3/3] serial: core: Update uart_poll_timeout() function to return unsigned long Vamshi Gajjela
2023-10-26  8:56 ` Jiri Slaby
2023-10-26  9:24   ` Ilpo Järvinen
2023-10-26  9:49     ` Jiri Slaby
2023-10-26 12:30       ` VAMSHI GAJJELA
2023-10-26 12:28     ` VAMSHI GAJJELA

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox