| Message ID | cover.1584380360.git.agx@sigxcpu.org |
|---|---|
| Headers | show |
| Series | iio: vcnl4000: Export near level property for proximity sensor | expand |
On 3/16/20 6:46 PM, Guido Günther wrote: > [...] > +static ssize_t vcnl4000_read_near_level(struct iio_dev *indio_dev, > + uintptr_t priv, > + const struct iio_chan_spec *chan, > + char *buf) > +{ > + struct vcnl4000_data *data = iio_priv(indio_dev); > + > + return sprintf(buf, "%u\n", data->near_level); > +} > + > +static const struct iio_chan_spec_ext_info vcnl4000_ext_info[] = { > + { > + .name = "near_level", Generally having properties with a underscore in them breaks generic parsing of the property name by userspace applications. This is because we use underscores to separate different components (type, modifier, etc.) of the attribute from each other. Do you think calling this "nearlevel" would work? I know there are existing bad examples of properties that use an underscore, but we should try to limit introducing new ones. > + .shared = IIO_SEPARATE, > + .read = vcnl4000_read_near_level, > + }, > + { /* sentinel */ } > +}; > + > static const struct iio_chan_spec vcnl4000_channels[] = { > { > .type = IIO_LIGHT, > @@ -350,6 +371,7 @@ static const struct iio_chan_spec vcnl4000_channels[] = { > }, { > .type = IIO_PROXIMITY, > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .ext_info = vcnl4000_ext_info, > } > }; > > @@ -439,6 +461,10 @@ static int vcnl4000_probe(struct i2c_client *client, > dev_dbg(&client->dev, "%s Ambient light/proximity sensor, Rev: %02x\n", > data->chip_spec->prod, data->rev); > > + if (device_property_read_u32(&client->dev, "near-level", > + &data->near_level) < 0) > + data->near_level = 0; > + > indio_dev->dev.parent = &client->dev; > indio_dev->info = &vcnl4000_info; > indio_dev->channels = vcnl4000_channels;
Hi, On Mon, Mar 16, 2020 at 07:23:01PM +0100, Lars-Peter Clausen wrote: > On 3/16/20 6:46 PM, Guido Günther wrote: > > [...] > > +static ssize_t vcnl4000_read_near_level(struct iio_dev *indio_dev, > > + uintptr_t priv, > > + const struct iio_chan_spec *chan, > > + char *buf) > > +{ > > + struct vcnl4000_data *data = iio_priv(indio_dev); > > + > > + return sprintf(buf, "%u\n", data->near_level); > > +} > > + > > +static const struct iio_chan_spec_ext_info vcnl4000_ext_info[] = { > > + { > > + .name = "near_level", > > Generally having properties with a underscore in them breaks generic parsing > of the property name by userspace applications. This is because we use > underscores to separate different components (type, modifier, etc.) of the > attribute from each other. > > Do you think calling this "nearlevel" would work? That works as well. I'll change that for v3. For my education: Is the type, modifier policy written down somewhere (similar to https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/leds/leds-class.rst#n44 )? Cheers, -- Guido > > I know there are existing bad examples of properties that use an underscore, > but we should try to limit introducing new ones. > > > + .shared = IIO_SEPARATE, > > + .read = vcnl4000_read_near_level, > > + }, > > + { /* sentinel */ } > > +}; > > + > > static const struct iio_chan_spec vcnl4000_channels[] = { > > { > > .type = IIO_LIGHT, > > @@ -350,6 +371,7 @@ static const struct iio_chan_spec vcnl4000_channels[] = { > > }, { > > .type = IIO_PROXIMITY, > > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > + .ext_info = vcnl4000_ext_info, > > } > > }; > > @@ -439,6 +461,10 @@ static int vcnl4000_probe(struct i2c_client *client, > > dev_dbg(&client->dev, "%s Ambient light/proximity sensor, Rev: %02x\n", > > data->chip_spec->prod, data->rev); > > + if (device_property_read_u32(&client->dev, "near-level", > > + &data->near_level) < 0) > > + data->near_level = 0; > > + > > indio_dev->dev.parent = &client->dev; > > indio_dev->info = &vcnl4000_info; > > indio_dev->channels = vcnl4000_channels; > >
On 3/17/20 1:05 PM, Guido Günther wrote: > Hi, > On Mon, Mar 16, 2020 at 07:23:01PM +0100, Lars-Peter Clausen wrote: >> On 3/16/20 6:46 PM, Guido Günther wrote: >>> [...] >>> +static ssize_t vcnl4000_read_near_level(struct iio_dev *indio_dev, >>> + uintptr_t priv, >>> + const struct iio_chan_spec *chan, >>> + char *buf) >>> +{ >>> + struct vcnl4000_data *data = iio_priv(indio_dev); >>> + >>> + return sprintf(buf, "%u\n", data->near_level); >>> +} >>> + >>> +static const struct iio_chan_spec_ext_info vcnl4000_ext_info[] = { >>> + { >>> + .name = "near_level", >> Generally having properties with a underscore in them breaks generic parsing >> of the property name by userspace applications. This is because we use >> underscores to separate different components (type, modifier, etc.) of the >> attribute from each other. >> >> Do you think calling this "nearlevel" would work? > That works as well. I'll change that for v3. > > For my education: Is the type, modifier policy written down somewhere > (similar to > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/leds/leds-class.rst#n44 > )? Good point, this is quite badly documented at the moment. The only thing I could find is this presentation by Daniel https://events.static.linuxfound.org/sites/events/files/slides/lceu15_baluta.pdf#page=9 - Lars
On Mon, Mar 16, 2020 at 7:47 PM Guido Günther <agx@sigxcpu.org> wrote: > > When an object can be considered close to the sensor is hardware > dependent. Allowing to configure the property via device tree > allows to configure this device specific value. > > This is useful for e.g. iio-sensor-proxy to indicate to userspace > if an object is close to the sensor. ... > @@ -342,6 +343,26 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = { > }, > }; > > + No need for this blank line. > +static ssize_t vcnl4000_read_near_level(struct iio_dev *indio_dev, > + uintptr_t priv, > + const struct iio_chan_spec *chan, > + char *buf) ... > + if (device_property_read_u32(&client->dev, "near-level", > + &data->near_level) < 0) It doesn't return > 0. So, you may drop that and put everything to one line I think. > + data->near_level = 0;