[PATCH 6/6] dma/imx-sdma: check whether event_id0 < 32 when set event_mask

[Eric Miao]

On Wed, Jan 11, 2012 at 9:35 PM, Shawn Guo <shawn.guo at linaro.org> wrote:
> On Wed, Jan 11, 2012 at 09:09:11PM +0800, Richard Zhao wrote:
>> On Wed, Jan 11, 2012 at 09:16:17PM +0800, Shawn Guo wrote:
>> > On Wed, Jan 11, 2012 at 02:37:08PM +0800, Eric Miao wrote:
>> > > I think Richard has made the issue quite clear here, the original
>> >
>> > Yes, he has made it clear, but only after I asked for more comments,
>> > not with the empty commit message.
>> >
>> > > code does seem to have some problems even to me, who do not
>> > > understand the very details of the SDMA:
>> > >
>> > > -                       sdmac->event_mask0 = 1 << sdmac->event_id0;
>> > > -                       sdmac->event_mask1 = 1 << (sdmac->event_id0 - 32);
>> > >
>> > > 1. if sdmac->event_id0 >= 32, which will cause event_mask0 to be incorrect
>> > > 2. if sdmac->event_id < 32, sdmac->event_mask1 will be incorrect
>> > >
>> > My testing tells this is not the case.  The event_mask0 will be 0 in
>> > case 1) and event_mask1 will be 0 in case 2), which is quite what we
>> > expect.  And I do not believe you will see any functionality bug with
>> > the existing code.
>> Please see my mail mentioned "we shoud not let it depends on gcc's
>> behavior."
>
> In this case, I would rather believe that the author is smart enough
> to write the code intentionally based on his good understanding on
> behavior of arm-gcc.

Either using the tricky gcc behavior which _will_ vary along version changes,
or using "hard to understand code" is BAD idea. We don't write good code
depending on author's "smartness", we write code so that it's understandable
and maintainable.