Messages in this thread | | | Date | Mon, 20 Feb 2012 04:02:34 +0100 | From | Jean-Christophe PLAGNIOL-VILLARD <> | Subject | Re: [PATCH 04/18] ARM: at91: make ST (System Timer) soc independent |
| |
On 12:52 Mon 20 Feb , Ryan Mallon wrote: > On 20/02/12 12:38, Jean-Christophe PLAGNIOL-VILLARD wrote: > > > On 11:22 Mon 20 Feb , Ryan Mallon wrote: > >> On 18/02/12 04:49, Nicolas Ferre wrote: > >> > >>> From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > >>> > >>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > >>> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com> > >>> --- > >>> arch/arm/mach-at91/at91rm9200.c | 4 +- > >>> arch/arm/mach-at91/at91rm9200_time.c | 37 ++++++++++++++++---------- > >>> arch/arm/mach-at91/generic.h | 1 + > >>> arch/arm/mach-at91/include/mach/at91_st.h | 32 +++++++++++++++------- > >>> arch/arm/mach-at91/include/mach/at91rm9200.h | 2 +- > >>> drivers/watchdog/at91rm9200_wdt.c | 8 +++--- > >>> 6 files changed, 53 insertions(+), 31 deletions(-) > >>> > >>> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c > >> > >> Hi Jean, Nicolas, > >> > >> Patch looks mostly good, couple of points below. > >> > >> ~Ryan > >> > >> <snip> > >> > >>> /* Cancel any pending alarm; flush any pending IRQ */ > >>> - at91_sys_write(AT91_ST_RTAR, alm); > >>> - (void) at91_sys_read(AT91_ST_SR); > >>> + at91_st_write(AT91_ST_RTAR, alm); > >>> + (void) at91_st_read(AT91_ST_SR); > >> > >> > >> Can we please remove the (void) casting of the return value when making > >> this change, especially since at91_st_read is now a macro which doesn't > >> even have a return value. Same in a few other places. > > modification done by script and it's no the scope of this patch > > > That isn't an excuse to leave incorrect code there. It is a simple fix. no (void) in c means you don't care of the return so basically it's right
Best Regards, J.
| |