On 14:16 Mon 20 Feb , Ryan Mallon wrote: > On 20/02/12 14:02, Jean-Christophe PLAGNIOL-VILLARD wrote: > > > 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@xxxxxxxxxxxx> > >>>>> > >>>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@xxxxxxxxxxxx> > >>>>> Acked-by: Nicolas Ferre <nicolas.ferre@xxxxxxxxx> > >>>>> --- > >>>>> 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 > > > Because of the way the __raw_writel is defined you are casting the > result of an assignment, basically you are doing this: > > int foo, bar; > > (void)(foo = bar); > > Which is pointless. Don't make excuses for silly, redundant code. Fix > it, please. except it's a read it has nothing to do with that Best Regards, J. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/