2022-04-16 00:25:25

by Arnaud Ferraris

[permalink] [raw]
Subject: [PATCH 0/2] iio: stk3310: Export near level property for proximity sensor

Userspace tools like iio-sensor-proxy need to be instructed the value from
which they should consider an object is "near". This threshold can be
exported through the sysfs ABI based on the "proximity-near-level"
device-tree property.

This patchset implements this property for the stk3310 driver and adds the
necessary bits to export its value to userspace. It is based on similar
changes applied to the vcnl4000 and ltr501 drivers.

Arnaud Ferraris (2):
dt-bindings: iio: light: stk33xx: Add proximity-near-level
iio: stk3310: Export near level property for proximity sensor

.../bindings/iio/light/stk33xx.yaml | 6 +++++
drivers/iio/light/stk3310.c | 26 +++++++++++++++++++
2 files changed, 32 insertions(+)

--
2.35.1


2022-04-16 00:32:35

by Arnaud Ferraris

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: iio: light: stk33xx: Add proximity-near-level

This allows exporting the value from which userspace should assert an
object is "near".

Signed-off-by: Arnaud Ferraris <[email protected]>
---
Documentation/devicetree/bindings/iio/light/stk33xx.yaml | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
index f92bf7b2b7f0..f6e22dc9814a 100644
--- a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
+++ b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
@@ -13,6 +13,9 @@ maintainers:
description: |
Ambient light and proximity sensor over an i2c interface.

+allOf:
+ - $ref: ../common.yaml#
+
properties:
compatible:
enum:
@@ -26,6 +29,8 @@ properties:
interrupts:
maxItems: 1

+ proximity-near-level: true
+
required:
- compatible
- reg
@@ -44,6 +49,7 @@ examples:
stk3310@48 {
compatible = "sensortek,stk3310";
reg = <0x48>;
+ proximity-near-level = <25>;
interrupt-parent = <&gpio1>;
interrupts = <5 IRQ_TYPE_LEVEL_LOW>;
};
--
2.35.1

2022-04-16 02:31:51

by Arnaud Ferraris

[permalink] [raw]
Subject: [PATCH 2/2] iio: stk3310: Export near level property for proximity sensor

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 <[email protected]>
---
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;
+
mutex_init(&data->lock);

ret = stk3310_regmap_init(data);
--
2.35.1

2022-04-16 19:33:10

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 2/2] iio: stk3310: Export near level property for proximity sensor

On Fri, 15 Apr 2022 10:50:18 +0200
Arnaud Ferraris <[email protected]> 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 <[email protected]>
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);

2022-04-22 21:09:55

by Arnaud Ferraris

[permalink] [raw]
Subject: Re: [PATCH 2/2] iio: stk3310: Export near level property for proximity sensor

Hi Jonathan,

Le 16/04/2022 à 18:26, Jonathan Cameron a écrit :
> On Fri, 15 Apr 2022 10:50:18 +0200
> Arnaud Ferraris <[email protected]> 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 <[email protected]>
> 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);
>