RE: [PATCH v2 15/20] DSPBRIDGE: trivial cleanup and indentation for io_sm

Ramirez Luna, Omar
2009-12-01T18:16:33Z

>From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
>
>On Mon, Nov 30, 2009 at 11:54 PM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> Remove duplicate set of braces from if statement and reduce
>> indentation.
>
>> diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
>> index af31831..96a5aa6 100644
>> --- a/drivers/dsp/bridge/wmd/io_sm.c
>> +++ b/drivers/dsp/bridge/wmd/io_sm.c
>> @@ -115,7 +115,7 @@ struct IO_MGR {
>>        /* private extnd proc info; mmu setup */
>>        struct MGR_PROCESSOREXTINFO extProcInfo;
>>        struct CMM_OBJECT *hCmmMgr;     /* Shared Mem Mngr            */
>May be to fix commentary indentation?
>> -       struct work_struct io_workq;     /*workqueue */
>> +       struct work_struct io_workq;     /*workqueue */
>I didn't get why the space after /* still absent.

Removed on the next patch (trivial fix for multiline comments on io_sm)
- struct CMM_OBJECT *hCmmMgr; /* Shared Mem Mngr */
- struct work_struct io_workq; /*workqueue */
+ struct CMM_OBJECT *hCmmMgr; /* Shared Mem Mngr */
+ struct work_struct io_workq; /* workqueue */

>
>> @@ -319,7 +321,7 @@ DSP_STATUS WMD_IO_Destroy(struct IO_MGR *hIOMgr)
>>                SYNC_DeleteCS(hIOMgr->hCSObj);  /* Leak Fix. */
>>                /* Free this IO manager object: */
>>                MEM_FreeObject(hIOMgr);
>> -       } else
>> +       } else
>>                status = DSP_EHANDLE;
>Probably } else { ... } would be better. (1)
>

Agree

>> @@ -609,7 +608,7 @@ func_cont1:
>>                }
>>        }
>>
>> -        /* Copy remaining entries from CDB. All entries are 1 MB and should not
>> +       /* Copy remaining entries from CDB. All entries are 1 MB and should not
>>         * conflict with SHM entries on MPU or DSP side */
>In previous hunks you fixed * ... */. Why not here? (2)

trivial fix for multiline comments on io_sm, I tried to split the two patches because I thought there would be a lot of changes
0015-DSPBRIDGE-trivial-cleanup-and-indentation-for-io_sm.patch
0016-DSPBRIDGE-trivial-fix-for-multiline-comments-on-io_.patch

But I guess I can join them into one patch and resend (btw fixed on patch 16)

>
>> @@ -1002,15 +995,10 @@ void IO_DPC(IN OUT void *pRefData)
>>                        PrintDSPDebugTrace(pIOMgr);
>>        }
>>  #endif
>> -
>> -#ifndef DSP_TRACEBUF_DISABLED
>> -       PrintDSPDebugTrace(pIOMgr);
>> -#endif
>>  func_end:
>>        return;
>No comments about this changes. (Intuitively I understand, but...)
>
>> @@ -1028,29 +1016,28 @@ irqreturn_t IO_ISR(int irq, IN void *pRefData)
>>                }
>> -       } else
>> +       } else
>>                /* Ensure that, if WMD didn't claim it, the IRQ is shared. */
>>                DBC_Ensure(hIOMgr->fSharedIRQ);
>The same as in (1).

Agree

>
>> @@ -1380,10 +1365,10 @@ static void NotifyChnlComplete(struct CHNL_OBJECT *pChnl,
>>           !pChnl->pIOCompletions || !pChirp)
>>                goto func_end;
>>
>> -        /*  Note: we signal the channel event only if the queue of IO
>> -         *  completions is empty.  If it is not empty, the event is sure to be
>> -         *  signalled by the only IO completion list consumer:
>> -         *  WMD_CHNL_GetIOC().  */
>> +       /*  Note: we signal the channel event only if the queue of IO
>> +        *  completions is empty.  If it is not empty, the event is sure to be
>> +        *  signalled by the only IO completion list consumer:
>> +        *  WMD_CHNL_GetIOC().  */
>The same as in (2).
>

Fixed on patch 16, will join both of the patches and resend

>--
>With Best Regards,
>Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html