Re: [PATCH] staging: comedi: amplc_pci224: Convert macro GAT_CONFIG to static inline function

[SIMRAN SINGHAL]

On Sun, Mar 12, 2017 at 7:26 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Mar 10, 2017 at 06:12:31PM +0530, simran singhal wrote:
>> Convert macro GAT_CONFIG to static inline function as static inline
>> functions are preferred over macros. This change is possible since the
>> arguments at all call sites have the same type.
>>
>> This was done using Coccinelle:
>>
>> @r@
>> expression e;
>> @@
>> - #define GAT_CONFIG(chan, src) e
>> + static inline unsigned int pci230_gat_config(unsigned int chan,
>> +                                              unsigned int src)
>
> Where did you get the name from?
>
Actually the name should be pci224_gat_config as when I checked the other
drivers like amplc_pci230.c then its using pci230_gat_config so according to
what other drivers are using I should use pci224_gat_config.

I will correct this and will resend it.

>> +{
>> +       return ((chan & 3) << 3) | (src & 7);
>> +}
>>
>> @r1@
>> expression dev,reg,chan,src;
>> @@
>> -GAT_CONFIG(chan, src)
>> +pci230_gat_config(chan, src)
>>
>> Also, the comment describing the macro has been removed manually.
>>
>> Signed-off-by: simran singhal <singhalsimran0@xxxxxxxxx>
>> ---
>>  drivers/staging/comedi/drivers/amplc_pci224.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
>> index 2e6decf..46026f5 100644
>> --- a/drivers/staging/comedi/drivers/amplc_pci224.c
>> +++ b/drivers/staging/comedi/drivers/amplc_pci224.c
>> @@ -216,8 +216,12 @@
>>  #define GAT_GND              1       /* GND (i.e. disabled) */
>>  #define GAT_EXT              2       /* reserved (external gate input) */
>>  #define GAT_NOUTNM2  3       /* inverted output of channel-2 modulo total */
>> -/* Macro to construct gate input configuration register value. */
>> -#define GAT_CONFIG(chan, src)        ((((chan) & 3) << 3) | ((src) & 7))
>> +
>> +static inline unsigned int pci230_gat_config(unsigned int chan,
>> +                                          unsigned int src)
>> +{
>> +     return ((chan & 3) << 3) | (src & 7);
>> +}
>>
>>  /*
>>   * Summary of CLK_OUTNM1 and GAT_NOUTNM2 connections for PCI224 and PCI234:
>> @@ -817,10 +821,12 @@ static void pci224_ao_start_pacer(struct comedi_device *dev,
>>        * source.
>>        */
>>       /* Make sure Z2-0 is gated on.  */
>> -     outb(GAT_CONFIG(0, GAT_VCC), devpriv->iobase1 + PCI224_ZGAT_SCE);
>> +     outb(pci230_gat_config(0, GAT_VCC),
>> +          devpriv->iobase1 + PCI224_ZGAT_SCE);
>
> Why put this on 2 lines?

I'll check this again and if their will be no problem in putting this
on one line then
I'll resend it with the change.

>
>>       /* Cascading with Z2-2. */
>>       /* Make sure Z2-2 is gated on.  */
>> -     outb(GAT_CONFIG(2, GAT_VCC), devpriv->iobase1 + PCI224_ZGAT_SCE);
>> +     outb(pci230_gat_config(2, GAT_VCC),
>> +          devpriv->iobase1 + PCI224_ZGAT_SCE);
>
> Same here?
>
> thanks,
>
> greg k-h