Am 07.08.2018 um 19:32 schrieb Ben Whitten: > diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c > index 5342b61..49958f0 100644 > --- a/drivers/net/lora/sx1301.c > +++ b/drivers/net/lora/sx1301.c [...] > @@ -76,8 +287,29 @@ struct sx1301_priv { > struct gpio_desc *rst_gpio; > u8 cur_page; > struct spi_controller *radio_a_ctrl, *radio_b_ctrl; > + struct regmap *regmap; > + struct regmap_field *regmap_fields[ARRAY_SIZE(sx1301_reg_fields)]; > }; > > +static int sx1301_field_read(struct sx1301_priv *priv, > + enum sx1301_fields field_id) > +{ > + int ret; > + int val; > + > + ret = regmap_field_read(priv->regmap_fields[field_id], &val); > + if (ret) > + return ret; > + > + return val;
This strikes me as a bad idea. Please keep returning the value by pointer, so that we can clearly distinguish from error values. > +} > + > +static int sx1301_field_write(struct sx1301_priv *priv, > + enum sx1301_fields field_id, u8 val) > +{ > + return regmap_field_write(priv->regmap_fields[field_id], val); > +} > + > static int sx1301_read_burst(struct spi_device *spi, u8 reg, u8 *val, size_t > len) > { > u8 addr = reg & 0x7f; [snip] It looks to me as if both of those static functions are unused in this patch? Please keep things bisectable. Regards, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg)