On Tue, Jan 09, 2018 at 09:04:50PM +0100, Valentin Vidic wrote:
> 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?
>
It's often better to avoid switch statements.
int rf69_set_dagc(struct spi_device *spi, enum dagc dagc)
{
static const int dagc_reg[] = {
[normalMode] = DAGC_NORMAL,
[improve] = DAGC_IMPROVED_LOWBETA0,
[improve4LowModulationIndex] = DAGC_IMPROVED_LOWBETA1,
};
if (dagc >= ARRAY_SIZE(dagc_reg))
return -EINVAL;
return rf69_write_reg(spi, REG_TESTDAGC, dagc_reg[dagc]);
}
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
