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 devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel