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);
}

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

Reply via email to