Re: [PATCH v5 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors

From: Jonathan Cameron
Date: Mon Feb 15 2021 - 06:09:16 EST

Next message: Petr Mladek: "Re: [PATCH 0/3][v4] add support for never printing hashed addresses"
Previous message: Cornelia Huck: "Re: [PATCH] vfio/type1: Use follow_pte()"
In reply to: Cristian Marussi: "Re: [PATCH v5 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors"
Next in thread: Sudeep Holla: "Re: [PATCH v5 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, 15 Feb 2021 09:25:26 +0000
Cristian Marussi <cristian.marussi@xxxxxxx> wrote:

> On Fri, Feb 12, 2021 at 07:18:58PM +0000, Jonathan Cameron wrote:
> > On Wed, 10 Feb 2021 21:46:19 +0000
> > Cristian Marussi <cristian.marussi@xxxxxxx> wrote:
> >
> > > Hi Jyoti,
> > >
> > > On Wed, Feb 10, 2021 at 11:19:35AM -0800, Jyoti Bhayana wrote:
> > > > Hi Cristian,
> > > >
> > > > Thanks for the feedback. Regarding registering callbacks at the probe time
> > > > instead of .preenable, I have tried it before but I think due to some
> > > > issues(don't remember it now maybe on the platform side) I kept it at the
> > > > .preenable level.
> > > >
> > > > But you are right, that it will be nice to move it at the probe level
> > > > instead. I will try again and test if it works and would move it at the
> > > > probe level. Regarding the unregistering of the notifier, is it required at
> > > > the remove of iio driver or scmi driver will take care of it?
> > > > Because if I add unregister at the iio driver remove level, I would have to
> > > > iterate all the sensors again and unregister them.
> > >
> > > Yes you are right if you move callbacks registration once for all to the
> > > .probe step you'll have to unregister them all in a .remove.
> > >
> > > BUT I think instead you should stick with your current solution given
> > > it's working fine anyway and it's supported by the notification
> > > framework and also for another reason I'm going to explain down below
> > > (which is also the reason why I asked you this at first :D)
> > >
> > > As you may remember I'm refactoring all the SCMI internals in a separate
> > > series to ease modularization and vendor protocols support, and that will
> > > lead also to some changes in the SCMI driver interface that you use:
> > > amongst other things one interesting addition will be a new devres managed
> > > notification registration method, something like:
> > >
> > > handle->notify_ops->devm_register_notifier(sdev, ...);
> > >
> > > With such method you could just move your registration to the .probe
> > > step and just forget about it, without the need to add any unregistration
> > > in the .remove step, since the core will take care to remove all the
> > > callbacks at driver unloading time.
> > >
> > > Now, this series, which is here if you want to have a look:
> > >
> > > https://lore.kernel.org/linux-arm-kernel/20210202221555.41167-1-cristian.marussi@xxxxxxx/
> > >
> > > is already taking care to port any existent SCMI driver to the new interface,
> > > so when your IIODEV SCMI driver will be finally queued somewhere for merge, I
> > > can in turn rebase my series on yours and take care to port your driver too to
> > > the new interface applying the changes above in the context of my series.
> > > (and ask you to review of course :D)
> >
> > I'm guessing you probably want this driver in an immutable branch then
> > rather than having to wait another cycle for it to tick through to a
> > a sensible upstream?
> >
>
> Hi Jonathan
>
> the above series (still pending a final review from Sudeep) is targeted
> at 5.13 at this point and usually it'd be queued via Sudeep for-next/scmi
> which in turn goes via the ARM soc branch.
>
> Having said that, I'm not really familiar enough with this sort of clashes
> to know how they should be properly handled, so I'll stick to what you
> and Sudeep would think it's better :D (..and I'm pinging him to have a say)
>
> Thanks
>
> Cristian
Hi Cristian,

So this driver will also be 5.13 material now (merge window for IIO effectively
closes 1-2 weeks before Linus opens the main one).

The way we normally handle cases like this where we likely to have dependencies
on a patch set from two separate directions is to do what is known as an
immutable branch. This is a branch that would probably be based on 5.12-rc1
containing just this driver.

Then both trees, in this case IIO and scmi merge that branch. The magic
of git then means that when Linus gets the eventual pull requests for
the two trees, the git IDs and content will be the same and the history
of that particular set of files will be cleanly maintained.

This happens quite a lot for certain parts of the kernel because there are
a lot of cross dependencies.

@Sudeep, that work for you? Have to wait for 5.12-rc1 though to give
us a sensible base.

Jonathan

>
> > Jonathan
> >
> > >
> > > I'm saying that is better if you keep your series as it is for now
> > > (old interface + .preenable/.postdisable regs/unregs) because, as said,
> > > with the new interface the devm_ methods will ease the registration
> > > @probe time, and also especially because the new interface is not (and
> > > most probably won't) be part of the v5.4 backport that you are testing
> > > against: so if you stick with your current solution you'll have a
> > > working patch easily backportable now, and once queued I'll port it to
> > > the interface using devm_ (so simplifying it)
> > >
> > > In this context, it would be indeed important to know if in general moving
> > > registration to the probe phase (which should be fine by the spec) poses
> > > any kind of problem. (and that's reason why asked it)
> > >
> > > Hope to have been clear despite the flood of words :D
> > >
> > > Thanks
> > >
> > > Cristian
> > >
> > > >
> > > > Thanks,
> > > > Jyoti
> > > >
> > > > On Tue, Feb 9, 2021 at 3:56 AM Cristian Marussi <cristian.marussi@xxxxxxx>
> > > > wrote:
> > > >
> > > > > Hi Jyoti
> > > > >
> > > > > some minor things down below.
> > > > >
> > > > > Other than that, FWIW about the SCMI side of this:
> > > > >
> > > > > Reviewed-by: Cristian Marussi <cristian.marussi@xxxxxxx>
> > > > >
> > > > > Thanks
> > > > >
> > > > > Cristian
> > > > >
> > > > > On Mon, Feb 08, 2021 at 09:19:18PM +0000, Jyoti Bhayana wrote:
> > > > > > This change provides ARM SCMI Protocol based IIO device.
> > > > > > This driver provides support for Accelerometer and Gyroscope using
> > > > > > SCMI Sensor Protocol extensions added in the SCMIv3.0 ARM specification
> > > > > >
> > > > > > Signed-off-by: Jyoti Bhayana <jbhayana@xxxxxxxxxx>
> > > > > > ---
> > > > > > MAINTAINERS | 6 +
> > > > > > drivers/firmware/arm_scmi/driver.c | 2 +-
> > > > > > drivers/iio/common/Kconfig | 1 +
> > > > > > drivers/iio/common/Makefile | 1 +
> > > > > > drivers/iio/common/scmi_sensors/Kconfig | 18 +
> > > > > > drivers/iio/common/scmi_sensors/Makefile | 5 +
> > > > > > drivers/iio/common/scmi_sensors/scmi_iio.c | 673 +++++++++++++++++++++
> > > > > > 7 files changed, 705 insertions(+), 1 deletion(-)
> > > > > > create mode 100644 drivers/iio/common/scmi_sensors/Kconfig
> > > > > > create mode 100644 drivers/iio/common/scmi_sensors/Makefile
> > > > > > create mode 100644 drivers/iio/common/scmi_sensors/scmi_iio.c
> > > > > >
> > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > index b516bb34a8d5..ccf37d43ab41 100644
> > > > > > --- a/MAINTAINERS
> > > > > > +++ b/MAINTAINERS
> > > > > > @@ -8567,6 +8567,12 @@ S: Maintained
> > > > > > F:
> > > > > Documentation/devicetree/bindings/iio/multiplexer/io-channel-mux.txt
> > > > > > F: drivers/iio/multiplexer/iio-mux.c
> > > > > >
> > > > > > +IIO SCMI BASED DRIVER
> > > > > > +M: Jyoti Bhayana <jbhayana@xxxxxxxxxx>
> > > > > > +L: linux-iio@xxxxxxxxxxxxxxx
> > > > > > +S: Maintained
> > > > > > +F: drivers/iio/common/scmi_sensors/scmi_iio.c
> > > > > > +
> > > > > > IIO SUBSYSTEM AND DRIVERS
> > > > > > M: Jonathan Cameron <jic23@xxxxxxxxxx>
> > > > > > R: Lars-Peter Clausen <lars@xxxxxxxxxx>
> > > > > > diff --git a/drivers/firmware/arm_scmi/driver.c
> > > > > b/drivers/firmware/arm_scmi/driver.c
> > > > > > index 5392e1fc6b4e..248313bbd473 100644
> > > > > > --- a/drivers/firmware/arm_scmi/driver.c
> > > > > > +++ b/drivers/firmware/arm_scmi/driver.c
> > > > > > @@ -741,7 +741,7 @@ static struct scmi_prot_devnames devnames[] = {
> > > > > > { SCMI_PROTOCOL_SYSTEM, { "syspower" },},
> > > > > > { SCMI_PROTOCOL_PERF, { "cpufreq" },},
> > > > > > { SCMI_PROTOCOL_CLOCK, { "clocks" },},
> > > > > > - { SCMI_PROTOCOL_SENSOR, { "hwmon" },},
> > > > > > + { SCMI_PROTOCOL_SENSOR, { "hwmon", "iiodev" },},
> > > > > > { SCMI_PROTOCOL_RESET, { "reset" },},
> > > > > > { SCMI_PROTOCOL_VOLTAGE, { "regulator" },},
> > > > > > };
> > > > > > diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> > > > > > index 2b9ee9161abd..0334b4954773 100644
> > > > > > --- a/drivers/iio/common/Kconfig
> > > > > > +++ b/drivers/iio/common/Kconfig
> > > > > > @@ -6,5 +6,6 @@
> > > > > > source "drivers/iio/common/cros_ec_sensors/Kconfig"
> > > > > > source "drivers/iio/common/hid-sensors/Kconfig"
> > > > > > source "drivers/iio/common/ms_sensors/Kconfig"
> > > > > > +source "drivers/iio/common/scmi_sensors/Kconfig"
> > > > > > source "drivers/iio/common/ssp_sensors/Kconfig"
> > > > > > source "drivers/iio/common/st_sensors/Kconfig"
> > > > > > diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
> > > > > > index 4bc30bb548e2..fad40e1e1718 100644
> > > > > > --- a/drivers/iio/common/Makefile
> > > > > > +++ b/drivers/iio/common/Makefile
> > > > > > @@ -11,5 +11,6 @@
> > > > > > obj-y += cros_ec_sensors/
> > > > > > obj-y += hid-sensors/
> > > > > > obj-y += ms_sensors/
> > > > > > +obj-y += scmi_sensors/
> > > > > > obj-y += ssp_sensors/
> > > > > > obj-y += st_sensors/
> > > > > > diff --git a/drivers/iio/common/scmi_sensors/Kconfig
> > > > > b/drivers/iio/common/scmi_sensors/Kconfig
> > > > > > new file mode 100644
> > > > > > index 000000000000..67e084cbb1ab
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/iio/common/scmi_sensors/Kconfig
> > > > > > @@ -0,0 +1,18 @@
> > > > > > +#
> > > > > > +# IIO over SCMI
> > > > > > +#
> > > > > > +# When adding new entries keep the list in alphabetical order
> > > > > > +
> > > > > > +menu "IIO SCMI Sensors"
> > > > > > +
> > > > > > +config IIO_SCMI
> > > > > > + tristate "IIO SCMI"
> > > > > > + depends on ARM_SCMI_PROTOCOL
> > > > > > + select IIO_BUFFER
> > > > > > + select IIO_KFIFO_BUF
> > > > > > + help
> > > > > > + Say yes here to build support for IIO SCMI Driver.
> > > > > > + This provides ARM SCMI Protocol based IIO device.
> > > > > > + This driver provides support for accelerometer and gyroscope
> > > > > > + sensors available on SCMI based platforms.
> > > > > > +endmenu
> > > > > > diff --git a/drivers/iio/common/scmi_sensors/Makefile
> > > > > b/drivers/iio/common/scmi_sensors/Makefile
> > > > > > new file mode 100644
> > > > > > index 000000000000..f13140a2575a
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/iio/common/scmi_sensors/Makefile
> > > > > > @@ -0,0 +1,5 @@
> > > > > > +# SPDX - License - Identifier : GPL - 2.0 - only
> > > > > > +#
> > > > > > +# Makefile for the IIO over SCMI
> > > > > > +#
> > > > > > +obj-$(CONFIG_IIO_SCMI) += scmi_iio.o
> > > > > > diff --git a/drivers/iio/common/scmi_sensors/scmi_iio.c
> > > > > b/drivers/iio/common/scmi_sensors/scmi_iio.c
> > > > > > new file mode 100644
> > > > > > index 000000000000..093b1fc24e27
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/iio/common/scmi_sensors/scmi_iio.c
> > > > > > @@ -0,0 +1,673 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +
> > > > > > +/*
> > > > > > + * System Control and Management Interface(SCMI) based IIO sensor driver
> > > > > > + *
> > > > > > + * Copyright (C) 2021 Google LLC
> > > > > > + */
> > > > > > +
> > > > > > +#include <linux/delay.h>
> > > > > > +#include <linux/err.h>
> > > > > > +#include <linux/iio/buffer.h>
> > > > > > +#include <linux/iio/iio.h>
> > > > > > +#include <linux/iio/kfifo_buf.h>
> > > > > > +#include <linux/iio/sysfs.h>
> > > > > > +#include <linux/kernel.h>
> > > > > > +#include <linux/kthread.h>
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/scmi_protocol.h>
> > > > > > +#include <linux/time.h>
> > > > > > +#include <linux/types.h>
> > > > > > +
> > > > > > +#define SCMI_IIO_NUM_OF_AXIS 3
> > > > > > +
> > > > > > +struct scmi_iio_priv {
> > > > > > + struct scmi_handle *handle;
> > > > > > + const struct scmi_sensor_info *sensor_info;
> > > > > > + struct iio_dev *indio_dev;
> > > > > > + /* adding one additional channel for timestamp */
> > > > > > + long long iio_buf[SCMI_IIO_NUM_OF_AXIS + 1];
> > > > > > + struct notifier_block sensor_update_nb;
> > > > > > + u32 *freq_avail;
> > > > > > +};
> > > > > > +
> > > > > > +static int scmi_iio_sensor_update_cb(struct notifier_block *nb,
> > > > > > + unsigned long event, void *data)
> > > > > > +{
> > > > > > + struct scmi_sensor_update_report *sensor_update = data;
> > > > > > + struct iio_dev *scmi_iio_dev;
> > > > > > + struct scmi_iio_priv *sensor;
> > > > > > + s8 tstamp_scale;
> > > > > > + u64 time, time_ns;
> > > > > > + int i;
> > > > > > +
> > > > > > + if (sensor_update->readings_count == 0)
> > > > > > + return NOTIFY_DONE;
> > > > > > +
> > > > > > + sensor = container_of(nb, struct scmi_iio_priv, sensor_update_nb);
> > > > > > +
> > > > > > + for (i = 0; i < sensor_update->readings_count; i++)
> > > > > > + sensor->iio_buf[i] = sensor_update->readings[i].value;
> > > > > > +
> > > > > > + if (!sensor->sensor_info->timestamped) {
> > > > > > + time_ns = ktime_to_ns(sensor_update->timestamp);
> > > > > > + } else {
> > > > > > + /*
> > > > > > + * All the axes are supposed to have the same value for
> > > > > timestamp.
> > > > > > + * We are just using the values from the Axis 0 here.
> > > > > > + */
> > > > > > + time = sensor_update->readings[0].timestamp;
> > > > > > +
> > > > > > + /*
> > > > > > + * Timestamp returned by SCMI is in seconds and is equal
> > > > > to
> > > > > > + * time * power-of-10 multiplier(tstamp_scale) seconds.
> > > > > > + * Converting the timestamp to nanoseconds below.
> > > > > > + */
> > > > > > + tstamp_scale = sensor->sensor_info->tstamp_scale +
> > > > > > + const_ilog2(NSEC_PER_SEC) / const_ilog2(10);
> > > > > > + if (tstamp_scale < 0)
> > > > > > + time_ns =
> > > > > > + div64_u64(time, int_pow(10,
> > > > > abs(tstamp_scale)));
> > > > > > + else
> > > > > > + time_ns = time * int_pow(10, tstamp_scale);
> > > > > > + }
> > > > > > +
> > > > > > + scmi_iio_dev = sensor->indio_dev;
> > > > > > + iio_push_to_buffers_with_timestamp(scmi_iio_dev, sensor->iio_buf,
> > > > > > + time_ns);
> > > > > > + return NOTIFY_OK;
> > > > > > +}
> > > > > > +
> > > > > > +static int scmi_iio_buffer_preenable(struct iio_dev *iio_dev)
> > > > > > +{
> > > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev);
> > > > > > + u32 sensor_id = sensor->sensor_info->id;
> > > > > > + u32 sensor_config = 0;
> > > > > > + int err;
> > > > > > +
> > > > > > + if (sensor->sensor_info->timestamped)
> > > > > > + sensor_config |=
> > > > > FIELD_PREP(SCMI_SENS_CFG_TSTAMP_ENABLED_MASK,
> > > > > > + SCMI_SENS_CFG_TSTAMP_ENABLE);
> > > > > > +
> > > > > > + sensor_config |= FIELD_PREP(SCMI_SENS_CFG_SENSOR_ENABLED_MASK,
> > > > > > + SCMI_SENS_CFG_SENSOR_ENABLE);
> > > > > > +
> > > > > > + err =
> > > > > sensor->handle->notify_ops->register_event_notifier(sensor->handle,
> > > > > > + SCMI_PROTOCOL_SENSOR, SCMI_EVENT_SENSOR_UPDATE,
> > > > > > + &sensor_id, &sensor->sensor_update_nb);
> > > > > > + if (err) {
> > > > > > + dev_err(&iio_dev->dev,
> > > > > > + "Error in registering sensor update notifier for
> > > > > sensor %s err %d",
> > > > > > + sensor->sensor_info->name, err);
> > > > > > + return err;
> > > > > > + }
> > > > > > +
> > > > > > + err = sensor->handle->sensor_ops->config_set(sensor->handle,
> > > > > > + sensor->sensor_info->id, sensor_config);
> > > > > > + if (err) {
> > > > > > +
> > > > > sensor->handle->notify_ops->unregister_event_notifier(sensor->handle,
> > > > > > + SCMI_PROTOCOL_SENSOR,
> > > > > > + SCMI_EVENT_SENSOR_UPDATE, &sensor_id,
> > > > > > + &sensor->sensor_update_nb);
> > > > > > + dev_err(&iio_dev->dev, "Error in enabling sensor %s err
> > > > > %d",
> > > > > > + sensor->sensor_info->name, err);
> > > > > > + }
> > > > > > +
> > > > > > + return err;
> > > > > > +}
> > > > > > +
> > > > > > +static int scmi_iio_buffer_postdisable(struct iio_dev *iio_dev)
> > > > > > +{
> > > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev);
> > > > > > + u32 sensor_id = sensor->sensor_info->id;
> > > > > > + u32 sensor_config = 0;
> > > > > > + int err;
> > > > > > +
> > > > > > + sensor_config |= FIELD_PREP(SCMI_SENS_CFG_SENSOR_ENABLED_MASK,
> > > > > > + SCMI_SENS_CFG_SENSOR_DISABLE);
> > > > > > +
> > > > > > + err =
> > > > > sensor->handle->notify_ops->unregister_event_notifier(sensor->handle,
> > > > > > + SCMI_PROTOCOL_SENSOR, SCMI_EVENT_SENSOR_UPDATE,
> > > > > > + &sensor_id, &sensor->sensor_update_nb);
> > > > > > + if (err) {
> > > > > > + dev_err(&iio_dev->dev,
> > > > > > + "Error in unregistering sensor update notifier for
> > > > > sensor %s err %d",
> > > > > > + sensor->sensor_info->name, err);
> > > > > > + return err;
> > > > > > + }
> > > > > > +
> > > > > > + err = sensor->handle->sensor_ops->config_set(sensor->handle,
> > > > > sensor_id,
> > > > > > + sensor_config);
> > > > > > + if (err) {
> > > > > > + dev_err(&iio_dev->dev,
> > > > > > + "Error in disabling sensor %s with err %d",
> > > > > > + sensor->sensor_info->name, err);
> > > > > > + }
> > > > > > +
> > > > > > + return err;
> > > > > > +}
> > > > > > +
> > > > > > +static const struct iio_buffer_setup_ops scmi_iio_buffer_ops = {
> > > > > > + .preenable = scmi_iio_buffer_preenable,
> > > > > > + .postdisable = scmi_iio_buffer_postdisable,
> > > > > > +};
> > > > >
> > > > > This is just a question, I'm not suggesting to change anything here at
> > > > > this point to be clear, since it works just fine as it is.
> > > > >
> > > > > Following up a previous email, given these are called on enable/disable
> > > > > by sysfs, is there a specific reason why you configure here, inside
> > > > > these ops, also timestamping and callbacks i.e. each time the sensor is
> > > > > turned on/off by sysfs ? ... instead of just, as an example, enabling
> > > > > in _preenable the sensor while registering callbacks and enabling
> > > > > timestamping once for all earlier during probe phase ?
> > > > > (likewise for _postdisable -> remove)
> > > > >
> > > > > AFAIU the spec says notifications are emitted for sensors which has
> > > > > requested them (via SENSOR_CONTINUOUS_UPDATE_NOTIFY) BUT only if the
> > > > > sensor is enabled as a whole (via proper CONFIG_SET as you do), so
> > > > > that enabling/disabling the sensor as a whole should result in starting/
> > > > > stopping the notification flow without the need of unregistering the
> > > > > callbacks everytime. (same goes with the timestamping)
> > > > >
> > > > > In other words, I would expect the sensor to maintain its state (on the
> > > > > platform side) even when going through enable/disable cycles, so that
> > > > > it 'remembers' that timestamping/notifications were enabled across an
> > > > > on/off.
> > > > >
> > > > > This would reduce the number of SCMI messages exchanges between the
> > > > > kernel and the platform and should be supported by both, but as said,
> > > > > it's more of a question for the future, not necessarily for this series.
> > > > >
> > > > > > +
> > > > >
> > > > > [snip]
> > > > >
> > > > > > +static int scmi_iio_set_sampling_freq_avail(struct iio_dev *iio_dev)
> > > > > > +{
> > > > > > + u64 cur_interval_ns, low_interval_ns, high_interval_ns,
> > > > > step_size_ns,
> > > > > > + hz, uhz;
> > > > > > + unsigned int cur_interval, low_interval, high_interval, step_size;
> > > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev);
> > > > > > + int i;
> > > > > > +
> > > > > > + sensor->freq_avail =
> > > > > > + devm_kzalloc(&iio_dev->dev,
> > > > > > + sizeof(*sensor->freq_avail) *
> > > > > > + (sensor->sensor_info->intervals.count
> > > > > * 2),
> > > > > > + GFP_KERNEL);
> > > > > > + if (!sensor->freq_avail)
> > > > > > + return -ENOMEM;
> > > > > > +
> > > > > > + if (sensor->sensor_info->intervals.segmented) {
> > > > > > + low_interval = sensor->sensor_info->intervals
> > > > > > + .desc[SCMI_SENS_INTVL_SEGMENT_LOW];
> > > > > > + low_interval_ns =
> > > > > scmi_iio_convert_interval_to_ns(low_interval);
> > > > > > + convert_ns_to_freq(low_interval_ns, &hz, &uhz);
> > > > > > + sensor->freq_avail[0] = hz;
> > > > > > + sensor->freq_avail[1] = uhz;
> > > > > > +
> > > > > > + step_size = sensor->sensor_info->intervals
> > > > > > + .desc[SCMI_SENS_INTVL_SEGMENT_STEP];
> > > > > > + step_size_ns = scmi_iio_convert_interval_to_ns(step_size);
> > > > > > + convert_ns_to_freq(step_size_ns, &hz, &uhz);
> > > > > > + sensor->freq_avail[2] = hz;
> > > > > > + sensor->freq_avail[3] = uhz;
> > > > > > +
> > > > > > + high_interval = sensor->sensor_info->intervals
> > > > > > +
> > > > > .desc[SCMI_SENS_INTVL_SEGMENT_HIGH];
> > > > > > + high_interval_ns =
> > > > > > + scmi_iio_convert_interval_to_ns(high_interval);
> > > > > > + convert_ns_to_freq(high_interval_ns, &hz, &uhz);
> > > > > > + sensor->freq_avail[4] = hz;
> > > > > > + sensor->freq_avail[5] = uhz;
> > > > > > + } else {
> > > > > > + for (i = 0; i < sensor->sensor_info->intervals.count; i++)
> > > > > {
> > > > > > + cur_interval =
> > > > > sensor->sensor_info->intervals.desc[i];
> > > > > > + cur_interval_ns =
> > > > > > +
> > > > > scmi_iio_convert_interval_to_ns(cur_interval);
> > > > > > + convert_ns_to_freq(cur_interval_ns, &hz, &uhz);
> > > > > > + sensor->freq_avail[i * 2] = hz;
> > > > > > + sensor->freq_avail[i * 2 + 1] = uhz;
> > > > > > + }
> > > > > > + }
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int scmi_iio_buffers_setup(struct iio_dev *scmi_iiodev)
> > > > > > +{
> > > > > > + struct iio_buffer *buffer;
> > > > > > +
> > > > > > + buffer = devm_iio_kfifo_allocate(&scmi_iiodev->dev);
> > > > > > + if (!buffer)
> > > > > > + return -ENOMEM;
> > > > > > +
> > > > > > + iio_device_attach_buffer(scmi_iiodev, buffer);
> > > > > > + scmi_iiodev->modes |= INDIO_BUFFER_SOFTWARE;
> > > > > > + scmi_iiodev->setup_ops = &scmi_iio_buffer_ops;
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +struct iio_dev *scmi_alloc_iiodev(struct device *dev,
> > > > > > + struct scmi_handle *handle,
> > > > > > + const struct scmi_sensor_info
> > > > > *sensor_info)
> > > > > > +{
> > > > > > + struct iio_chan_spec *iio_channels;
> > > > > > + struct scmi_iio_priv *sensor;
> > > > > > + enum iio_modifier modifier;
> > > > > > + enum iio_chan_type type;
> > > > > > + struct iio_dev *iiodev;
> > > > > > + int i, ret;
> > > > > > +
> > > > > > + iiodev = devm_iio_device_alloc(dev, sizeof(*sensor));
> > > > > > + if (!iiodev)
> > > > > > + return ERR_PTR(-ENOMEM);
> > > > > > +
> > > > > > + iiodev->modes = INDIO_DIRECT_MODE;
> > > > > > + iiodev->dev.parent = dev;
> > > > > > + sensor = iio_priv(iiodev);
> > > > > > + sensor->handle = handle;
> > > > > > + sensor->sensor_info = sensor_info;
> > > > > > + sensor->sensor_update_nb.notifier_call = scmi_iio_sensor_update_cb;
> > > > > > + sensor->indio_dev = iiodev;
> > > > > > +
> > > > > > + /* adding one additional channel for timestamp */
> > > > > > + iiodev->num_channels = sensor_info->num_axis + 1;
> > > > > > + iiodev->name = sensor_info->name;
> > > > > > + iiodev->info = &scmi_iio_info;
> > > > > > +
> > > > > > + iio_channels =
> > > > > > + devm_kzalloc(dev,
> > > > > > + sizeof(*iio_channels) *
> > > > > (iiodev->num_channels),
> > > > > > + GFP_KERNEL);
> > > > > > + if (!iio_channels)
> > > > > > + return ERR_PTR(-ENOMEM);
> > > > > > +
> > > > > > + scmi_iio_set_sampling_freq_avail(iiodev);
> > > > >
> > > > > You don't check this for retval, and it could fail with -ENOMEM.
> > > > >
> > > > > > +
> > > > > > + for (i = 0; i < sensor_info->num_axis; i++) {
> > > > > > + ret = scmi_iio_get_chan_type(sensor_info->axis[i].type,
> > > > > &type);
> > > > > > + if (ret < 0)
> > > > > > + return ERR_PTR(ret);
> > > > > > +
> > > > > > + ret = scmi_iio_get_chan_modifier(sensor_info->axis[i].name,
> > > > > > + &modifier);
> > > > > > + if (ret < 0)
> > > > > > + return ERR_PTR(ret);
> > > > > > +
> > > > > > + scmi_iio_set_data_channel(&iio_channels[i], type, modifier,
> > > > > > + sensor_info->axis[i].id);
> > > > > > + }
> > > > > > +
> > > > > > + scmi_iio_set_timestamp_channel(&iio_channels[i], i);
> > > > > > + iiodev->channels = iio_channels;
> > > > > > + return iiodev;
> > > > > > +}
> > > > > > +
> > > > >
> >

Next message: Petr Mladek: "Re: [PATCH 0/3][v4] add support for never printing hashed addresses"
Previous message: Cornelia Huck: "Re: [PATCH] vfio/type1: Use follow_pte()"
In reply to: Cristian Marussi: "Re: [PATCH v5 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors"
Next in thread: Sudeep Holla: "Re: [PATCH v5 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]