Am 09.01.2018 um 21:04 schrieb Valentin Vidic:
> On Sun, Dec 24, 2017 at 02:42:57PM +0100, Marcus Wolf wrote:
>>>  int rf69_set_dagc(struct spi_device *spi, enum dagc dagc)
>>>  {
>>>     switch (dagc) {
>>> -   case normalMode:                 return rf69_write_reg(spi, 
>>> REG_TESTDAGC, DAGC_NORMAL);
>>> -   case improve:                    return rf69_write_reg(spi, 
>>> REG_TESTDAGC, DAGC_IMPROVED_LOWBETA0);
>>> -   case improve4LowModulationIndex: return rf69_write_reg(spi, 
>>> REG_TESTDAGC, DAGC_IMPROVED_LOWBETA1);
>>> +   case normalMode:
>>> +           return rf69_write_reg(spi, REG_TESTDAGC, DAGC_NORMAL);
>>> +   case improve:
>>> +           return rf69_write_reg(spi, REG_TESTDAGC, 
>>> DAGC_IMPROVED_LOWBETA0);
>>> +   case improve4LowModulationIndex:
>>> +           return rf69_write_reg(spi, REG_TESTDAGC, 
>>> DAGC_IMPROVED_LOWBETA1);
>>>     default:
>>>             dev_dbg(&spi->dev, "set: illegal input param");
>>>             return -EINVAL;
>>>
>>
>> Hi Michael,
>>
>> first of all thank you for your effort :-)
>>
>> For me, the readability is reduced with this patch.
>>
>> But that's just my opinion/favour...
> 
> Would something like this be any better for these simple switch
> statements?
> 
> int rf69_set_dagc(struct spi_device *spi, enum dagc dagc)
> {
>    int dagc_value;
> 
>    switch (dagc) {
>    case normalMode:   dagc_value = DAGC_NORMAL;
>    case improve:      dagc_value = DAGC_IMPROVED_LOWBETA0;
>    ...
>    default:
>             dev_dbg(&spi->dev, "set: illegal input param");
>             return -EINVAL;
>    }
> 
>    return rf69_write_reg(spi, REG_TESTDAGC, dagc_value);
> }
> 

Hi Valentin,

I'd like such code very much. Marcin Ciupak already made such a proposal
but most probably the mainline changed to fast so he couldn't place his
patch...

If you would like to refactor the rf69.c in that way, I would be very happy.
Hope others will like this, too.

Thanks,

Marcus

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to