[PATCH] Updated legacy code in arm edb7312

[Vipul Nayyar]

Hello,

I'll keep that in mind to closely look for warnings & errors while sending patches. But I might also add, that warnings for unused functions might be present in my patches, since you suggested to remove unused functions, if they're affected by change in API methods. Well, from what I saw, the warnings for unused functions encountered are not due to the work I'm doing. So, either I can stop touching those functions(& a couple of warnings will stay there), or remove all of these inter-related functions from the file. What say ?

I'll remove the clock_driver_get_nanoseconds_since_last_tick() function & the Clock_driver_nanoseconds_since_last_tick macro  in edb7312. Is there a need to modify the clockdrv_shell.h file ?

Regards 
Vipul Nayyar 



________________________________
 From: Sebastian Huber <sebastian.huber at embedded-brains.de>
To: rtems-devel at rtems.org 
Sent: Monday, 15 July 2013 3:38 PM
Subject: Re: [PATCH] Updated legacy code in arm edb7312
 

Hello Vipul,

your patches are getting better, but please review them carefully before you 
submit them and fix all obvious problems:

gmake[6]: Entering directory 
`/scratch/git-build/b-edb7312/arm-rtems4.11/c/edb7312/lib/libbsp/arm/edb7312'
arm-rtems4.11-gcc --pipe -B../../../../../.././lib/ 
-B../../../../../.././edb7312/lib/ -specs bsp_specs -qrtems -DHAVE_CONFIG_H 
-isystem ../../../../../.././edb7312/lib/include   -O2 -g -Wall 
-Wmissing-prototypes -Wimplicit-function-declaration -Wstrict-prototypes 
-Wnested-externs -MT clockdrv.o -MD -MP -MF .deps/clockdrv.Tpo -c -o clockdrv.o 
`test -f 'clock/clockdrv.c' || echo 
'/home/sh/rtems-4.11/c/src/lib/libbsp/arm/edb7312/'`clock/clockdrv.c
/home/sh/rtems-4.11/c/src/lib/libbsp/arm/edb7312/clock/clockdrv.c:74:10: 
warning: no previous prototype for 
'clock_driver_get_nanoseconds_since_last_tick' [-Wmissing-prototypes]
  uint32_t clock_driver_get_nanoseconds_since_last_tick(void)
           ^

The next patch from you with compiler warnings is the last patch I review from you.

If you are unsure, how to deal with a warning, then ask on the list, but don't 
expect that reviewers solve the problems for you.

In this particular case you can completely remove the nanoseconds support.  See 
also clockdrv_shell.h:

[...]
   #if defined(Clock_driver_nanoseconds_since_last_tick)
     rtems_clock_set_nanoseconds_extension(
       Clock_driver_nanoseconds_since_last_tick
     );
   #endif
[...]

So this nanoseconds handler is optional.  All BSPs defining

uint32_t clock_driver_get_nanoseconds_since_last_tick(void)
{
   return 0;
}

have this problem.

On 07/15/2013 11:32 AM, Vipul Nayyar wrote:
> ---
>   c/src/lib/libbsp/arm/edb7312/clock/clockdrv.c  |  3 +--
>   c/src/lib/libbsp/arm/edb7312/network/network.c | 12 +++++-------
>   2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/c/src/lib/libbsp/arm/edb7312/clock/clockdrv.c b/c/src/lib/libbsp/arm/edb7312/clock/clockdrv.c
> index af0448ff7..761387f 100644
> --- a/c/src/lib/libbsp/arm/edb7312/clock/clockdrv.c
> +++ b/c/src/lib/libbsp/arm/edb7312/clock/clockdrv.c
> @@ -18,8 +18,7 @@
>     #define CLOCK_DRIVER_USE_FAST_IDLE
>   #endif
>
> -void Clock_isr(rtems_irq_hdl_param arg);
> -uint32_t clock_driver_get_nanoseconds_since_last_tick(void);
> +void Clock_isr(void * arg);
>
>   #define Clock_driver_support_at_tick()                \
>     do {                                                \
> diff --git a/c/src/lib/libbsp/arm/edb7312/network/network.c b/c/src/lib/libbsp/arm/edb7312/network/network.c
> index ce139ec..468fb61 100644
> --- a/c/src/lib/libbsp/arm/edb7312/network/network.c
> +++ b/c/src/lib/libbsp/arm/edb7312/network/network.c
> @@ -7,14 +7,13 @@
>   #define CS8900_BASE 0x20000300
>   unsigned int bsp_cs8900_io_base = 0;
>   unsigned int bsp_cs8900_memory_base = 0;
> -cs8900_device *g_cs;
> -void cs8900_isr(rtems_irq_hdl_param unused);
> +static void cs8900_isr(void *);
>
>   char g_enetbuf[1520];
>
> -void cs8900_isr(rtems_irq_hdl_param unused)
> +static void cs8900_isr(void *arg)
>   {
> -    cs8900_interrupt(BSP_EINT3, g_cs);
> +    cs8900_interrupt(BSP_EINT3, arg);
>   }
>
>   /* cs8900_io_set_reg - set one of the I/O addressed registers */
> @@ -64,14 +63,13 @@ unsigned short cs8900_mem_get_reg (cs8900_device *cs, unsigned long reg)
>   void cs8900_attach_interrupt (cs8900_device *cs)
>   {
>       rtems_status_code status = RTEMS_SUCCESSFUL;
> -    g_cs = cs;
>
>       status = rtems_interrupt_handler_install(
>           BSP_EINT3,
>           "Network",
>           RTEMS_INTERRUPT_UNIQUE,
>           cs8900_isr,
> -        NULL
> +        cs
>       );
>       assert(status == RTEMS_SUCCESSFUL);
>   }
> @@ -83,7 +81,7 @@ void cs8900_detach_interrupt (cs8900_device *cs)
>       status = rtems_interrupt_handler_remove(
>           BSP_EINT3,
>           cs8900_isr,
> -        NULL
> +        cs
>       );
>       assert(status == RTEMS_SUCCESSFUL);
>   }
>


-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
_______________________________________________
rtems-devel mailing list
rtems-devel at rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20130715/a0d284e7/attachment-0001.html>