Re: [PATCH 01/05] dmaengine: shdma: Remove sh_dmae_slave_chan_id enum

[Magnus Damm <magnus.damm@gmail.com>]

Hey Guennadi,

On Fri, Mar 19, 2010 at 4:30 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Fri, 19 Mar 2010, Magnus Damm wrote:
>> --- 0001/arch/sh/include/asm/dmaengine.h
>> +++ work/arch/sh/include/asm/dmaengine.h      2010-03-18 23:30:46.000000000 +0900
>> @@ -17,7 +17,7 @@
>>
>>  #define SH_DMAC_MAX_CHANNELS 6
>>
>> -enum sh_dmae_slave_chan_id {
>> +enum {
>>       SHDMA_SLAVE_SCIF0_TX,
>>       SHDMA_SLAVE_SCIF0_RX,
>>       SHDMA_SLAVE_SCIF1_TX,
>> @@ -38,7 +38,7 @@ enum sh_dmae_slave_chan_id {
>>  };
>>
>>  struct sh_dmae_slave_config {
>> -     enum sh_dmae_slave_chan_id      slave_id;
>> +     unsigned int                    slave_id;
>
> I think, INT_MAX DMA slave IDs will be enough;) So, you could just use an
> int to save typing and to reserve -1 (or -EINVAL) as an invalid DMA ID. As
> I am currencly realising, it can be useful, e.g., if you want to enable
> DMA only on Rx and not on Tx of some device. This, of course, throughout
> this patch.

Reserving some number sounds like a good plan, but exactly what is the
best is a different question. If I'm allowed to nitpick then I think
its wasteful to use one bit for the sign when only needing one value
to mark unused. So I'd prefer to go with unsigned and 0 as unused mark
and the rest as valid ids.

>> --- 0001/drivers/dma/shdma.c
>> +++ work/drivers/dma/shdma.c  2010-03-18 23:30:46.000000000 +0900
>> @@ -266,7 +266,7 @@ static struct sh_desc *sh_dmae_get_desc(
>>  }
>>
>>  static struct sh_dmae_slave_config *sh_dmae_find_slave(
>> -     struct sh_dmae_chan *sh_chan, enum sh_dmae_slave_chan_id slave_id)
>> +     struct sh_dmae_chan *sh_chan, struct sh_dmae_slave *param)
>
> This doesn't seem to be needed, at least, not in this patch.

Well, I have to change the function to something, no? =)

Cheers,

/ magnus