[PR 17] GRETH: Fixed autonego timeout overflow problem

[Joel Sherrill]

On 03/20/2012 08:29 AM, Daniel Hellstrom wrote:
> Hello,
>
> Ralf and Thomas, you are correct. This patch was created a long time ago when 'struct rtems_clock_time_value' was used it was defined:
>
> typedef struct {
>     /** This is the seconds portion of a time of day. */
>     uint32_t    seconds;
>     /** This is the microseconds portion of a time of day. */
>     uint32_t    microseconds;
> } rtems_clock_time_value;
>
> Joel removed the structure in c5fd2cb6746ab2af2f38d74b17db07d39e6fb97a and 57674ef62e38b6c5566ea4c47fcd616aadb3ee5b, and introduced 'struct timeval' instead which is defined with signed tv_usec
> instead of unsigned microseconds. I have missed this during merging somewhere long time ago.
>
> Joel, can you ignore this patch and close the PR1465. I will update the spreadsheet.

Daniel...

I remembered coming across this code a while back while trying to
get rid of calls to the deprecated rtems_clock_set(). This driver
shouldn't be using wall time at all. It should be using interval time.

Can it be changed to either use rtems_clock_get_ticks_since_boot()
or rtems_clock_get_uptime()?

It really seems wrong that a driver could implicitly set the time of
day.

If you use rtems_clock_get_uptime(), you can use the internal
timespec math routines like _Timespec_Greater_than() to
help.

And yes.. those eventually need to have a public API wrapper
around them.


> Thanks for reviewing,
> Daniel
>
>
> On 03/20/2012 08:51 AM, Daniel Hellstrom wrote:
>> Hello Ralf,
>>
>> The problem was first reported by Joris, please see BUG 1465.
>>
>> -    msecs = (tnow.tv_sec-tstart.tv_sec)*1000+(tnow.tv_usec-tstart.tv_usec)/1000;
>> +   msecs = (int)(tnow.tv_sec-tstart.tv_sec)*1000+((int)tnow.tv_usec-(int)tstart.tv_usec)/1000;
>>
>> The seconds part is no problem. If (tstart.tv_usec>  tnow.tv_usec) will be negative, but since the variables are unsigned a underflow will result in a very long msecs timeout.
>>
>> Thanks,
>> Daniel
>>
>>
>> On 03/19/2012 11:26 PM, Ralf Corsepius wrote:
>>> On 03/19/2012 08:59 PM, Joel Sherrill wrote:
>>>> This looks ok to me.
>>> I am having strong doubts on this patch.
>>>
>>> Daniel, could you elaborate the problem you are trying to solve?
>>>
>>> Ralf
>>>
>>>
>>>> I would have thought we had some support routines to do this but alas.
>>>>
>>>> Please commit someone.
>>>> ________________________________________
>>>> From: rtems-devel-bounces at rtems.org [rtems-devel-bounces at rtems.org] On Behalf Of Daniel Hellstrom [daniel at gaisler.com]
>>>> Sent: Monday, February 06, 2012 8:15 AM
>>>> To: rtems-devel at rtems.org
>>>> Subject: [PR 17] GRETH: Fixed autonego timeout overflow problem
>>>>
>>>> Signed-off-by: Daniel Hellstrom<daniel at gaisler.com>
>>>> ---
>>>>    c/src/libchip/network/greth.c |    2 +-
>>>>    1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/c/src/libchip/network/greth.c b/c/src/libchip/network/greth.c
>>>> index aff4d0f..b0e70b4 100644
>>>> --- a/c/src/libchip/network/greth.c
>>>> +++ b/c/src/libchip/network/greth.c
>>>> @@ -339,7 +339,7 @@ greth_initialize_hardware (struct greth_softc *sc)
>>>>                while (!(((phystatus = read_mii(phyaddr, 1))>>   5)&   1)) {
>>>>                        if ( rtems_clock_get_tod_timeval(&tnow) != RTEMS_SUCCESSFUL )
>>>>                          printk("rtems_clock_get_tod_timeval failed\n\r");
>>>> -                    msecs = (tnow.tv_sec-tstart.tv_sec)*1000+(tnow.tv_usec-tstart.tv_usec)/1000;
>>>> +                    msecs = (int)(tnow.tv_sec-tstart.tv_sec)*1000+((int)tnow.tv_usec-(int)tstart.tv_usec)/1000;
>>>>                        if ( msecs>   GRETH_AUTONEGO_TIMEOUT_MS ){
>>>>                                sc->auto_neg_time = msecs;
>>>>                                sc->auto_neg = -1; /* Failed */
>>>> -- 
>>>> 1.7.0.4
>>>>
>>>> _______________________________________________
>>>> rtems-devel mailing list
>>>> rtems-devel at rtems.org
>>>> http://www.rtems.org/mailman/listinfo/rtems-devel
>>>>
>>>> _______________________________________________
>>>> rtems-devel mailing list
>>>> rtems-devel at rtems.org
>>>> http://www.rtems.org/mailman/listinfo/rtems-devel
>>>
>> _______________________________________________
>> rtems-devel mailing list
>> rtems-devel at rtems.org
>> http://www.rtems.org/mailman/listinfo/rtems-devel
>>
>>


-- 
Joel Sherrill, Ph.D.             Director of Research&   Development
joel.sherrill at OARcorp.com        On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
     Support Available             (256) 722-9985