On Tue, Mar 29, 2011 at 07:02, Sergei Shtylyov wrote:
> On 28-03-2011 20:58, Mike Frysinger wrote:
>> +static int bfin_musb_channel_program(struct dma_channel *channel,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â u16 *packet_sz, u8 *mode,
>
> Â Are you going to change 'packet_sz'? That's interesting... :-)
i dont need to, i was just making everything an option because i
assume i cant foresee everyone's requirement. if changing packet_sz
doesnt make sense, i can drop the pointer.
>> + Â Â Â if (ANOMALY_05000450) {
>
> Â {} not needed; you could also collapse this into single *if*.
i prefer to keep the anomaly checking sep from the rest of the logic.
keeps things cleaner imo.
>> + Â Â Â Â Â Â Â if (musb_channel->transmit && *mode == 1)
>> + Â Â Â Â Â Â Â Â Â Â Â *len = *len - (*len % *packet_sz);
>
> Â Parens not needed. And why not:
>
> Â Â Â Â*len -= *len % *packet_sz;
>
> We're coding in C, after all. :-)
yes, but rather than force people to remember the more obscure
operator precedence, just about anyone can look at my version and
instantly digest what it's doing.
>> +    int   (*channel_program)(struct dma_channel *channel,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â u16 *packet_sz, u8 *mode,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â dma_addr_t *dma_addr, u32 *len);
>
> Â Sigh. I don't think channel_program is a good name -- the function doesn't
> actually program anything.
i was just trying to make it obvious that this correlates directly to
the existing channel_program func. i could call it
"pre_channel_program" or something else ... i dont particularly care.
suggest a better name :P.
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
