| Message ID | 20220415085018.35063-3-arnaud.ferraris@collabora.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: stk3310: Export near level property for proximity sensor | expand |
On Fri, 15 Apr 2022 10:50:18 +0200 Arnaud Ferraris <arnaud.ferraris@collabora.com> wrote: > This makes the value from which an object should be considered "near" > available to userspace. This hardware-dependent value should be set > in the device-tree. > > Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com> Hi Arnaud, Minor request to slightly modify how you do this inline. Otherwise looks good to me. Thanks, Jonathan > --- > drivers/iio/light/stk3310.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c > index 1d02dfbc29d1..7792456323ef 100644 > --- a/drivers/iio/light/stk3310.c > +++ b/drivers/iio/light/stk3310.c > @@ -106,6 +106,7 @@ struct stk3310_data { > struct mutex lock; > bool als_enabled; > bool ps_enabled; > + uint32_t ps_near_level; > u64 timestamp; > struct regmap *regmap; > struct regmap_field *reg_state; > @@ -135,6 +136,25 @@ static const struct iio_event_spec stk3310_events[] = { > }, > }; > > +static ssize_t stk3310_read_near_level(struct iio_dev *indio_dev, > + uintptr_t priv, > + const struct iio_chan_spec *chan, > + char *buf) > +{ > + struct stk3310_data *data = iio_priv(indio_dev); > + > + return sprintf(buf, "%u\n", data->ps_near_level); > +} > + > +static const struct iio_chan_spec_ext_info stk3310_ext_info[] = { > + { > + .name = "nearlevel", > + .shared = IIO_SEPARATE, > + .read = stk3310_read_near_level, > + }, > + { /* sentinel */ } > +}; > + > static const struct iio_chan_spec stk3310_channels[] = { > { > .type = IIO_LIGHT, > @@ -151,6 +171,7 @@ static const struct iio_chan_spec stk3310_channels[] = { > BIT(IIO_CHAN_INFO_INT_TIME), > .event_spec = stk3310_events, > .num_event_specs = ARRAY_SIZE(stk3310_events), > + .ext_info = stk3310_ext_info, > } > }; > > @@ -581,6 +602,11 @@ static int stk3310_probe(struct i2c_client *client, > data = iio_priv(indio_dev); > data->client = client; > i2c_set_clientdata(client, indio_dev); > + > + if (device_property_read_u32(&client->dev, "proximity-near-level", > + &data->ps_near_level)) > + data->ps_near_level = 0; Prefer this pattern. data->ps_near_level = 0; device_property_read_u32(&client->dev, "proximity-near-level", &data->ps_near_level); taking advantage of the fact that the output won't be set unless the property read succeeds. > + > mutex_init(&data->lock); > > ret = stk3310_regmap_init(data);
Hi Jonathan, Le 16/04/2022 à 18:26, Jonathan Cameron a écrit : > On Fri, 15 Apr 2022 10:50:18 +0200 > Arnaud Ferraris <arnaud.ferraris@collabora.com> wrote: > >> This makes the value from which an object should be considered "near" >> available to userspace. This hardware-dependent value should be set >> in the device-tree. >> >> Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com> > Hi Arnaud, > > Minor request to slightly modify how you do this inline. > Otherwise looks good to me. > > Thanks, > > Jonathan > >> --- >> drivers/iio/light/stk3310.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c >> index 1d02dfbc29d1..7792456323ef 100644 >> --- a/drivers/iio/light/stk3310.c >> +++ b/drivers/iio/light/stk3310.c >> @@ -106,6 +106,7 @@ struct stk3310_data { >> struct mutex lock; >> bool als_enabled; >> bool ps_enabled; >> + uint32_t ps_near_level; >> u64 timestamp; >> struct regmap *regmap; >> struct regmap_field *reg_state; >> @@ -135,6 +136,25 @@ static const struct iio_event_spec stk3310_events[] = { >> }, >> }; >> >> +static ssize_t stk3310_read_near_level(struct iio_dev *indio_dev, >> + uintptr_t priv, >> + const struct iio_chan_spec *chan, >> + char *buf) >> +{ >> + struct stk3310_data *data = iio_priv(indio_dev); >> + >> + return sprintf(buf, "%u\n", data->ps_near_level); >> +} >> + >> +static const struct iio_chan_spec_ext_info stk3310_ext_info[] = { >> + { >> + .name = "nearlevel", >> + .shared = IIO_SEPARATE, >> + .read = stk3310_read_near_level, >> + }, >> + { /* sentinel */ } >> +}; >> + >> static const struct iio_chan_spec stk3310_channels[] = { >> { >> .type = IIO_LIGHT, >> @@ -151,6 +171,7 @@ static const struct iio_chan_spec stk3310_channels[] = { >> BIT(IIO_CHAN_INFO_INT_TIME), >> .event_spec = stk3310_events, >> .num_event_specs = ARRAY_SIZE(stk3310_events), >> + .ext_info = stk3310_ext_info, >> } >> }; >> >> @@ -581,6 +602,11 @@ static int stk3310_probe(struct i2c_client *client, >> data = iio_priv(indio_dev); >> data->client = client; >> i2c_set_clientdata(client, indio_dev); >> + >> + if (device_property_read_u32(&client->dev, "proximity-near-level", >> + &data->ps_near_level)) >> + data->ps_near_level = 0; > > Prefer this pattern. > > data->ps_near_level = 0; > device_property_read_u32(&client->dev, "proximity-near-level", > &data->ps_near_level); > taking advantage of the fact that the output won't be set unless > the property read succeeds. That's a good suggestion indeed! We can even get rid of the initial assignment as the struct is zero-initialized on alloc, will send a v2 in a moment. Thanks, Arnaud > >> + >> mutex_init(&data->lock); >> >> ret = stk3310_regmap_init(data); >
diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c index 1d02dfbc29d1..7792456323ef 100644 --- a/drivers/iio/light/stk3310.c +++ b/drivers/iio/light/stk3310.c @@ -106,6 +106,7 @@ struct stk3310_data { struct mutex lock; bool als_enabled; bool ps_enabled; + uint32_t ps_near_level; u64 timestamp; struct regmap *regmap; struct regmap_field *reg_state; @@ -135,6 +136,25 @@ static const struct iio_event_spec stk3310_events[] = { }, }; +static ssize_t stk3310_read_near_level(struct iio_dev *indio_dev, + uintptr_t priv, + const struct iio_chan_spec *chan, + char *buf) +{ + struct stk3310_data *data = iio_priv(indio_dev); + + return sprintf(buf, "%u\n", data->ps_near_level); +} + +static const struct iio_chan_spec_ext_info stk3310_ext_info[] = { + { + .name = "nearlevel", + .shared = IIO_SEPARATE, + .read = stk3310_read_near_level, + }, + { /* sentinel */ } +}; + static const struct iio_chan_spec stk3310_channels[] = { { .type = IIO_LIGHT, @@ -151,6 +171,7 @@ static const struct iio_chan_spec stk3310_channels[] = { BIT(IIO_CHAN_INFO_INT_TIME), .event_spec = stk3310_events, .num_event_specs = ARRAY_SIZE(stk3310_events), + .ext_info = stk3310_ext_info, } }; @@ -581,6 +602,11 @@ static int stk3310_probe(struct i2c_client *client, data = iio_priv(indio_dev); data->client = client; i2c_set_clientdata(client, indio_dev); + + if (device_property_read_u32(&client->dev, "proximity-near-level", + &data->ps_near_level)) + data->ps_near_level = 0; + mutex_init(&data->lock); ret = stk3310_regmap_init(data);
This makes the value from which an object should be considered "near" available to userspace. This hardware-dependent value should be set in the device-tree. Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com> --- drivers/iio/light/stk3310.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)