> -----Original Message----- > From: Andrei Warkentin [mailto:andreiw@xxxxxxxxxxxx] > Sent: Tuesday, February 15, 2011 2:40 PM > To: Santosh Shilimkar > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > omap@xxxxxxxxxxxxxxx; Kevin Hilman; tony@xxxxxxxxxxx; Catalin > Marinas > Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way > operation can cause data corruption > > On Tue, Feb 15, 2011 at 1:14 AM, Santosh Shilimkar > <santosh.shilimkar@xxxxxx> wrote: > >> -----Original Message----- > >> From: Santosh Shilimkar [mailto:santosh.shilimkar@xxxxxx] > >> Sent: Monday, February 14, 2011 10:39 AM > >> To: Andrei Warkentin > >> Cc: linux-omap@xxxxxxxxxxxxxxx; Kevin Hilman; tony@xxxxxxxxxxx; > >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Catalin Marinas > >> Subject: RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way > >> operation can cause data corruption > >> > > [....] > > > > Why set by default to NULL, why not have a normal l2x0_set_debug > which > just does a write to L2X0_DEBUG_CTRL, and then just invoke the > function pointer when you wish to manipulate the DCR? That way you > don't need the runtime check, and it's just clearer, I think. > I though about it. There more changes in the file and hence I avoided it. This can be done though. > Also, why not do something like - > .... > do_stuff(); > #ifdef CONFIG_NEED_ERRATA_1234 > do_errata_stuff(); > #endif > do_more_stuff(); > ... > Which makes code completely ugly. > instead of - > > ... > #ifdef CONFIG_NEED_ERRATA_1234 > do_some_stuf() { > bar(); > } > #else > { > do_some_stuff() { > } > // nothing > } > We have already discussed this. The code becomes ugly. If you are interested in the reasoning, please check archives. Russell and Catalin has suggested above. If you understand the errata in first place, you could understand the comment. I let Catalin, Russell comment on it more, but unnecessary CONFIG options and polluting every function with #If, else checks don't make sense. Rest of your comments are related to this. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
