On Wed, Mar 27, 2019 at 08:11:20AM +0000, Sidong Yang wrote: > Moved code to configure sync to where check enable_sync option before. > There is no need to check enable_sync twice. Configuring sync should be > executed immediately after enabling sync. > > Signed-off-by: Sidong Yang <realwakka@xxxxxxxxx> > --- > drivers/staging/pi433/pi433_if.c | 17 +++++++---------- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c > index 53928af696a6..0a48d6cb9547 100644 > --- a/drivers/staging/pi433/pi433_if.c > +++ b/drivers/staging/pi433/pi433_if.c > @@ -318,10 +318,17 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg) > return ret; > } > > + /* configure sync, if enabled */ This comment is obvious. Just delete it. > if (tx_cfg->enable_sync == OPTION_ON) { > ret = rf69_enable_sync(dev->spi); > if (ret < 0) > return ret; > + ret = rf69_set_sync_size(dev->spi, tx_cfg->sync_length); > + if (ret < 0) > + return ret; > + ret = rf69_set_sync_values(dev->spi, tx_cfg->sync_pattern); > + if (ret < 0) > + return ret; It's weird that we enable sync before we et the size or sync values... regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel