On Tue, 4 Jun 2024 01:03:23 +0200 Antoni Pokusinski <apokusinski01@xxxxxxxxx> wrote: > Hello, thanks for the review. I have a one question though: > > >> @@ -134,6 +259,10 @@ static int si7020_probe(struct i2c_client *client) > >> indio_dev->channels = si7020_channels; > >> indio_dev->num_channels = ARRAY_SIZE(si7020_channels); > >> > >> + /* Default register values */ > >> + data->user_reg = 0x3A; > > >Can you build that up from fields with names? > >I don't like a magic value where I have no idea what some of the bits are. > > In the datasheet it's said that all the bits that are set by default in > the `user_reg` are "reserved" and have no meaning for now. So perhaps > mentioning this in a simple comment would be sufficient here? Yes and grr at the datasheet author / hardware designer. Would be unusual to have reserved and set bits but I guess maybe... More likely chicken bits for features that don't work ;) Jonathan
