Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp3202231pxb; Sun, 31 Jan 2021 07:41:11 -0800 (PST) X-Google-Smtp-Source: ABdhPJwp18NGyv3k56v2o97iI/3WZmz9eODlE4wgcVexkG7pJ9e9Df3BnZ4I3a4IrvbG6EYWR79E X-Received: by 2002:a05:6402:110b:: with SMTP id u11mr15160414edv.57.1612107671664; Sun, 31 Jan 2021 07:41:11 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1612107671; cv=none; d=google.com; s=arc-20160816; b=gWv68O8keQIjbD6OGtDdj3qpi7X4z1hLMD4LPihAc310aIJvfeOpmFnQsL0Z5uoGmN j5SVPJ1QNarXS3Ak7fzfxs8yXK0XJUsgBAf4SYwjRtpGWV1zxPwUwjJ7VLh9QA6glk4h V+e3xkQqI0je1lAggAHjzZMCiwRhfUhl5v3uWr6PAa51kEzJt+hB1P57hEG54OB+EXZg 4Bejn/1Oqs9lltnXTFQftMvlyy8pPuC0WVrH9JD0oK9CiOIX/ayS1FjJC0V+RllbND5P YdsTKydJtUxMHfv1TJFYUtNfMr7ZedClK/fTiIGQyNXlI/Po6V8xjnOYMM2AP2LpeW0m Fkrw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date; bh=ab8FG3S5WCL2HcEhJ9yN+AlLP6trHFR0s9hAv96TiEQ=; b=MjHCWNM9fKDXRZYRnYxLQD1dvDV0HYnaDScWfZJpMEZLC6itkK/MgEQjSC4/pA6ZD6 tcLQm2ZtILAibJ3TwXus0070sxMTHr9sSS/rHF8HVMkL+cx9bxNJzkpQfmGe4KWoxRKs tOD1DQcX5ixR/bDHVnVjto/PRHPPJ57k97G+Sx1hWKOXo8u1kFvTMaUoQAU4P5Ih/zR4 lLBYjwmIp3tDxz2vDvKg1pibmfzqlqSlJ1KMp6DbusQK5cJtBsfw22BMoL5XSH1R0dU5 vzYUAjqL6pqIswvHuUlhl36g2uUYTYcDvtbWm+UadA635GoTiHqBtIMJlAJ7gihNbzcv nF/g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id zd10si8727898ejb.77.2021.01.31.07.40.46; Sun, 31 Jan 2021 07:41:11 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231653AbhAaM5b convert rfc822-to-8bit (ORCPT + 99 others); Sun, 31 Jan 2021 07:57:31 -0500 Received: from mail.kernel.org ([198.145.29.99]:54146 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231625AbhAaL5D (ORCPT ); Sun, 31 Jan 2021 06:57:03 -0500 Received: from archlinux (cpc108967-cmbg20-2-0-cust86.5-4.cable.virginm.net [81.101.6.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 8227660233; Sun, 31 Jan 2021 11:56:04 +0000 (UTC) Date: Sun, 31 Jan 2021 11:55:58 +0000 From: Jonathan Cameron To: Cristian Marussi Cc: Jyoti Bhayana , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Mauro Carvalho Chehab , "David S. Miller" , Rob Herring , Lukas Bulwahn , linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org, Sudeep Holla , Enrico Granata , Mikhail Golubev , Igor Skalkin , Peter Hilber , Ankit Arora Subject: Re: [RFC PATCH v3 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors Message-ID: <20210131115558.3e24afca@archlinux> In-Reply-To: <20210130212141.GA7790@e120937-lin> References: <20210121232147.1849509-1-jbhayana@google.com> <20210121232147.1849509-2-jbhayana@google.com> <20210122133740.GB22231@e120937-lin> <20210130212141.GA7790@e120937-lin> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 30 Jan 2021 21:21:41 +0000 Cristian Marussi wrote: > Hi Jyoti, > > On Fri, Jan 29, 2021 at 02:43:13PM -0800, Jyoti Bhayana wrote: > > Hi Christian, > > > > I have addressed your feedback in v4 of the patch. Please find below > > the answers to some of your questions: > > > The initial sensor_config ORed here is NOT initialized nor zeroed. > > > > There is no need to initialize as it is already initialized to 0 when defined. > > > > True for the sensor_config in scmi_iio_buffer_postdisable(), BUT it is > not indeed zero-initialized at definition time in the other similar > function scmi_iio_buffer_preenable(), that is what the above comment was > referred to. > > > > being not so familiar with IIO, may I ask when and how much frequently > > > these ops are called ? (given they include a register/unregister + > > > enable/disable every time) > > > > These are called whenever the sensor is enabled/disabled by writing > > to buffer/enable sysfs node > > > > Ok, thanks. > > > >AFAIU this is called during IIO init and derives a modifier from the name > > >of the sensors which are passed by the platform fw with sensor descriptors, > > >so I would not trust them to be weell formed (:D) and try to be picky about > > >*name as it was user-input to validate. > > > > The SCMI specification has mentioned the naming convention for the > > axis as below . > > "A NULL terminated UTF-8 format string with the sensor axis name, of > > up to 16 bytes. It is recommended that the name ends with ‘_’ followed > > by the axis of the sensor in uppercase. For example, the name for the > > x-axis of a triaxial accelerometer could be “acc_X” or “_X” > > > > If there is any other better way to get the modifier, please let me know. > > > > What I was trying to say was simply that, despite what the spec says, > you cannot trust the fw/platform produced values (fw buggy ? data on channel > corrupted ? malicious pkt injections ? fuzzing ? ) and so you should be picky > and validate them also against any possible out-of-spec data, since the Kernel > must not crash even if presented with unreasonable out-of-spec garbage-like > data, thing that indeed you did in v4 by adding the check for NULL against > name param. :D (the other case of a malformed name like "acc_" seemed to > me handled properly already by the kernel strcmp) > > > > why not a break to a final 'return ret;' (with ret properly set) ? > > > > According to Jonathan, direct returns are preferred. > > > > Ok > > > >Not familiar with IIO, but is it fine to setup ops and modes AFTER > > >having attached the buffer to the scmi_iiodev ? > > >Is is not 'racy-possible' that the buffer is already operational without > > >any ops immediately after being attached ? > > > > I have looked at other IIO drivers and they all do it this way. > > > > Ok. There is a good logical flow in setting the ops before attaching, but in reality it doesn't matter because all that attach is actually doing is connecting up some internals for the iio_device_register() to act on later. Until that's called no access to the buffer ops can be made (there are no userspace or inkernel interfaces available). Jonathan > > Thanks, I'll have a look at v4. > > Cristian > > > Thanks, > > Jyoti > > > > > > > > > > > > > > > > On Fri, Jan 22, 2021 at 5:37 AM Cristian Marussi > > wrote: > > > > > > Hi Jyoti, > > > > > > a few remarks below. > > > > > > On Thu, Jan 21, 2021 at 11:21:47PM +0000, Jyoti Bhayana wrote: > > > > This change provides ARM SCMI Protocol based IIO device. > > > > This driver provides support for Accelerometer and Gyroscope using > > > > new SCMI Sensor Protocol defined by the upcoming SCMIv3.0 > > > > > > I'd say: > > > > > > new SCMI Sensor Protocol extension added by the upcoming SCMIv3.0 > > > > > > given that SCMI Sensor existed already in SCMIv2.0 > > > > > > > ARM specification > > > > > > > > Signed-off-by: Jyoti Bhayana > > > > --- > > > > MAINTAINERS | 6 + > > > > 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 | 736 +++++++++++++++++++++ > > > > 6 files changed, 767 insertions(+) > > > > 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 > > > > +L: linux-iio@vger.kernel.org > > > > +S: Maintained > > > > +F: drivers/iio/common/scmi_sensors/scmi_iio.c > > > > + > > > > IIO SUBSYSTEM AND DRIVERS > > > > M: Jonathan Cameron > > > > R: Lars-Peter Clausen > > > > 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..3b76cc54511c > > > > --- /dev/null > > > > +++ b/drivers/iio/common/scmi_sensors/scmi_iio.c > > > > @@ -0,0 +1,736 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > + > > > > +/* > > > > + * System Control and Management Interface(SCMI) based IIO sensor driver > > > > + * > > > > + * Copyright (C) 2020 Google LLC > > > > + */ > > > > + > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > + > > > > +#define ilog10(x) (ilog2(x) / const_ilog2(10)) > > > > +#define UHZ_PER_HZ 1000000UL > > > > +#define ODR_EXPAND(odr, uodr) (((odr) * 1000000ULL) + (uodr)) > > > > +#define MAX_NUM_OF_CHANNELS 4 > > > > +#define H32(x) (((x) & 0xFFFFFFFF00000000LL) >> 32) > > > > +#define L32(x) ((x) & 0xFFFFFFFFLL) > > > > > > Probaly you want to use GENMASK here to define the masks, and not sure > > > but maybe there are already similar macros in linux to extract > > > upper/lower 32 bits. > > > > > > > + > > > > +struct scmi_iio_priv { > > > > + struct scmi_handle *handle; > > > > + const struct scmi_sensor_info *sensor_info; > > > > + struct iio_dev *indio_dev; > > > > + long long iio_buf[MAX_NUM_OF_CHANNELS]; > > > > + struct notifier_block sensor_update_nb; > > > > + u32 *freq_avail; > > > > + /* > > > > + * range_avail = [minRange resolution maxRange] > > > > + * with IIO val type as IIO_VAL_FRACTIONAL. > > > > + * Hence, array of size 6. > > > > + */ > > > > + int range_avail[6]; > > > > +}; > > > > + > > > > +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 = iio_get_time_ns(scmi_iio_dev); > > > > > > This scmi_iio_dev is on the stack and still NOT initialized here right ? > > > > > > Moreover, if this is meant to attach a timestamp of arrival also to > > > NON-timestamped sensor notifications, note that, if you want, scmi_sensor_update_report > > > contains a 'timestamp' (as all SCMI notif reports) ktime field (ns) which > > > represents the time (ns from boot with ktime_get_boottime()) of reception > > > of this notification taken by the SCMI core as soon as it arrives in the > > > RX ISR. (so in the OSPM Linux world) > > > > > > If you grab the timestamp here with iio_get_time_ns() you are indeed > > > getting a timestamp inside the notification callback so at the end of > > > the notifications processing chain, i.e. in a deferred worker processing > > > this queue of events and in turn calling this function, so potentially > > > you're timestamping a lot later than when the event arrived really in > > > the OSPM world (especially on a loaded system); it is more the time of > > > arrival in IIO framework. > > > > > > On the other side timestamped events handled in the else-branch down > > > below carry a timestamp set by the SCMI platform fw early on when the > > > event has been detected by the sensor in the real world. > > > > > > Not sure how these values are used at the end, but just to let you know > > > this detail if it was not apparent. > > > > > > > + } 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 + > > > > + ilog10(NSEC_PER_SEC); > > > > + 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; > > > > + int err; > > > > + > > > > + if (sensor->sensor_info->timestamped) > > > > + sensor_config |= FIELD_PREP(SCMI_SENS_CFG_TSTAMP_ENABLED_MASK, > > > > + SCMI_SENS_CFG_TSTAMP_ENABLE); > > > > > > The initial sensor_config ORed here is NOT initialized nor zeroed. > > > > + > > > > + 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) > > > > + dev_err(&iio_dev->dev, "Error in enabling sensor %s err %d", > > > > + sensor->sensor_info->name, err); > > > > > > Here you do not unregister the notification above here when you fail > > > the config_set. > > > If you happen to retry this code path again you'll end up registering > > > the same notifier_block sensor_update_nb() in the same notification chain > > > (same event samme sensor_id), which is broken and probably triggers a WARN > > > from the core kernel notification chains machinery too. > > > > > > In an upcoming series on SCMI core I'll introduce a devm_ variant for > > > SCMI notification registration, but as of now you have to properly unregister > > > on error paths. > > > > > > > + > > > > + 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); > > > > > > This alignment here is a bit awkaward. > > > > > > > + 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, > > > > +}; > > > > + > > > > > > Being not so familiar with IIO, may I ask when annd ho wmuch frequently > > > these ops are called ? (given they include a register/unregister + > > > enable/disable every time) > > > > > > > +static int scmi_iio_read_avail(struct iio_dev *iio_dev, > > > > + struct iio_chan_spec const *chan, > > > > + const int **vals, int *type, int *length, > > > > + long mask) > > > > +{ > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev); > > > > + > > > > + switch (mask) { > > > > + case IIO_CHAN_INFO_SAMP_FREQ: > > > > + *vals = sensor->freq_avail; > > > > + *type = IIO_VAL_INT_PLUS_MICRO; > > > > + *length = sensor->sensor_info->intervals.count * 2; > > > > + if (sensor->sensor_info->intervals.segmented) > > > > + return IIO_AVAIL_RANGE; > > > > + else > > > > + return IIO_AVAIL_LIST; > > > > + case IIO_CHAN_INFO_RAW: > > > > + *vals = sensor->range_avail; > > > > + *type = IIO_VAL_FRACTIONAL; > > > > + *length = ARRAY_SIZE(sensor->range_avail); > > > > + return IIO_AVAIL_RANGE; > > > > + default: > > > > + return -EINVAL; > > > > + } > > > > +} > > > > + > > > > +static int scmi_iio_set_odr_val(struct iio_dev *iio_dev, int val, int val2) > > > > +{ > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev); > > > > + u64 sec, mult, uHz; > > > > + u32 sensor_config; > > > > + > > > > + int err = sensor->handle->sensor_ops->config_get(sensor->handle, > > > > + sensor->sensor_info->id, &sensor_config); > > > > + if (err) { > > > > + dev_err(&iio_dev->dev, > > > > + "Error in getting sensor config for sensor %s err %d", > > > > + sensor->sensor_info->name, err); > > > > + return err; > > > > + } > > > > + > > > > + uHz = ODR_EXPAND(val, val2); > > > > + > > > > + /* > > > > + * The seconds field in the sensor interval in SCMI is 16 bits long > > > > + * Therefore seconds = 1/Hz <= 0xFFFF. As floating point calculations are > > > > + * discouraged in the kernel driver code, to calculate the scale factor (sf) > > > > + * (1* 1000000 * sf)/uHz <= 0xFFFF. Therefore, sf <= (uHz * 0xFFFF)/1000000 > > > > + */ > > > > + mult = ilog10(((u64)uHz * 0xFFFF) / UHZ_PER_HZ); > > > > + > > > > + sec = div64_u64(int_pow(10, mult) * UHZ_PER_HZ, uHz); > > > > + if (sec == 0) { > > > > + dev_err(&iio_dev->dev, > > > > + "Trying to set invalid sensor update value for sensor %s", > > > > + sensor->sensor_info->name); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + sensor_config &= ~SCMI_SENS_CFG_UPDATE_SECS_MASK; > > > > + sensor_config |= FIELD_PREP(SCMI_SENS_CFG_UPDATE_SECS_MASK, sec); > > > > + sensor_config &= ~SCMI_SENS_CFG_UPDATE_EXP_MASK; > > > > + sensor_config |= FIELD_PREP(SCMI_SENS_CFG_UPDATE_EXP_MASK, -mult); > > > > + > > > > + if (sensor->sensor_info->timestamped) { > > > > + sensor_config &= ~SCMI_SENS_CFG_TSTAMP_ENABLED_MASK; > > > > + sensor_config |= FIELD_PREP(SCMI_SENS_CFG_TSTAMP_ENABLED_MASK, > > > > + SCMI_SENS_CFG_TSTAMP_ENABLE); > > > > + } > > > > + > > > > + sensor_config &= ~SCMI_SENS_CFG_ROUND_MASK; > > > > + sensor_config |= > > > > + FIELD_PREP(SCMI_SENS_CFG_ROUND_MASK, SCMI_SENS_CFG_ROUND_AUTO); > > > > + > > > > + err = sensor->handle->sensor_ops->config_set(sensor->handle, > > > > + sensor->sensor_info->id, sensor_config); > > > > + if (err) > > > > + dev_err(&iio_dev->dev, > > > > + "Error in setting sensor update interval for sensor %s value %u err %d", > > > > + sensor->sensor_info->name, sensor_config, err); > > > > + > > > > + return err; > > > > +} > > > > + > > > > +static int scmi_iio_write_raw(struct iio_dev *iio_dev, > > > > + struct iio_chan_spec const *chan, int val, > > > > + int val2, long mask) > > > > +{ > > > > + int err; > > > > + > > > > + switch (mask) { > > > > + case IIO_CHAN_INFO_SAMP_FREQ: > > > > + mutex_lock(&iio_dev->mlock); > > > > + err = scmi_iio_set_odr_val(iio_dev, val, val2); > > > > + mutex_unlock(&iio_dev->mlock); > > > > + return err; > > > > + default: > > > > + return -EINVAL; > > > > + } > > > > +} > > > > + > > > > +static u64 scmi_iio_convert_interval_to_ns(u32 val) > > > > +{ > > > > + u64 sensor_update_interval = > > > > + SCMI_SENS_INTVL_GET_SECS(val) * NSEC_PER_SEC; > > > > + u64 sensor_interval_mult; > > > > + int mult; > > > > + > > > > + mult = SCMI_SENS_INTVL_GET_EXP(val); > > > > + if (mult < 0) { > > > > + sensor_interval_mult = int_pow(10, abs(mult)); > > > > + sensor_update_interval = > > > > + sensor_update_interval / sensor_interval_mult; > > > > + } else { > > > > + sensor_interval_mult = int_pow(10, mult); > > > > + sensor_update_interval = > > > > + sensor_update_interval * sensor_interval_mult; > > > > + } > > > > + return sensor_update_interval; > > > > +} > > > > + > > > > +static void convert_ns_to_freq(u64 interval_ns, u64 *hz, u64 *uhz) > > > > +{ > > > > + u64 rem; > > > > + > > > > + *hz = div64_u64_rem(NSEC_PER_SEC, interval_ns, &rem); > > > > + *uhz = (rem * 1000000UL) / interval_ns; > > > > +} > > > > + > > > > +static int scmi_iio_get_odr_val(struct iio_dev *iio_dev, int *val, int *val2) > > > > +{ > > > > + u64 sensor_update_interval, sensor_interval_mult, hz, uhz; > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev); > > > > + u32 sensor_config; > > > > + int mult; > > > > + > > > > + int err = sensor->handle->sensor_ops->config_get(sensor->handle, > > > > + sensor->sensor_info->id, &sensor_config); > > > > + if (err) { > > > > + dev_err(&iio_dev->dev, > > > > + "Error in getting sensor config for sensor %s err %d", > > > > + sensor->sensor_info->name, err); > > > > + return err; > > > > + } > > > > + > > > > + sensor_update_interval = > > > > + SCMI_SENS_CFG_GET_UPDATE_SECS(sensor_config) * NSEC_PER_SEC; > > > > + > > > > + mult = SCMI_SENS_CFG_GET_UPDATE_EXP(sensor_config); > > > > + if (mult < 0) { > > > > + sensor_interval_mult = int_pow(10, abs(mult)); > > > > + sensor_update_interval = > > > > + sensor_update_interval / sensor_interval_mult; > > > > + } else { > > > > + sensor_interval_mult = int_pow(10, mult); > > > > + sensor_update_interval = > > > > + sensor_update_interval * sensor_interval_mult; > > > > + } > > > > + > > > > + convert_ns_to_freq(sensor_update_interval, &hz, &uhz); > > > > + *val = hz; > > > > + *val2 = uhz; > > > > + return 0; > > > > +} > > > > + > > > > +static int scmi_iio_read_raw(struct iio_dev *iio_dev, > > > > + struct iio_chan_spec const *ch, int *val, > > > > + int *val2, long mask) > > > > +{ > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev); > > > > + s8 scale; > > > > + int ret; > > > > + > > > > + switch (mask) { > > > > + case IIO_CHAN_INFO_SCALE: > > > > + scale = sensor->sensor_info->axis[ch->scan_index].scale; > > > > + if (scale < 0) { > > > > + *val = 1; > > > > + *val2 = int_pow(10, abs(scale)); > > > > + return IIO_VAL_FRACTIONAL; > > > > + } > > > > + *val = int_pow(10, scale); > > > > + return IIO_VAL_INT; > > > > + case IIO_CHAN_INFO_SAMP_FREQ: > > > > + ret = scmi_iio_get_odr_val(iio_dev, val, val2); > > > > + return ret ? ret : IIO_VAL_INT_PLUS_MICRO; > > > > + default: > > > > + return -EINVAL; > > > > + } > > > > +} > > > > + > > > > +static const struct iio_info scmi_iio_info = { > > > > + .read_raw = scmi_iio_read_raw, > > > > + .read_avail = scmi_iio_read_avail, > > > > + .write_raw = scmi_iio_write_raw, > > > > +}; > > > > + > > > > +static void scmi_iio_set_timestamp_channel(struct iio_chan_spec *iio_chan, > > > > + int scan_index) > > > > +{ > > > > + iio_chan->type = IIO_TIMESTAMP; > > > > + iio_chan->channel = -1; > > > > + iio_chan->scan_index = scan_index; > > > > + iio_chan->scan_type.sign = 'u'; > > > > + iio_chan->scan_type.realbits = 64; > > > > + iio_chan->scan_type.storagebits = 64; > > > > +} > > > > + > > > > +static void scmi_iio_set_data_channel(struct iio_chan_spec *iio_chan, > > > > + enum iio_chan_type type, > > > > + enum iio_modifier mod, int scan_index) > > > > +{ > > > > + iio_chan->type = type; > > > > + iio_chan->modified = 1; > > > > + iio_chan->channel2 = mod; > > > > + iio_chan->info_mask_separate = BIT(IIO_CHAN_INFO_SCALE); > > > > + iio_chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ); > > > > + iio_chan->info_mask_shared_by_type_available = > > > > + BIT(IIO_CHAN_INFO_SAMP_FREQ) | BIT(IIO_CHAN_INFO_RAW); > > > > + iio_chan->scan_index = scan_index; > > > > + iio_chan->scan_type.sign = 's'; > > > > + iio_chan->scan_type.realbits = 64; > > > > + iio_chan->scan_type.storagebits = 64; > > > > + iio_chan->scan_type.endianness = IIO_LE; > > > > +} > > > > + > > > > +static int scmi_iio_get_chan_modifier(const char *name, > > > > + enum iio_modifier *modifier) > > > > +{ > > > > > > AFAIU this is called during IIO init and derives a modifier from the name > > > of the sensors which are passed by the platform fw with sensor descriptors, > > > so I would not trust them to be weell formed (:D) and try to be picky about > > > *name as it was user-input to validate. > > > > > > > + char *pch; > > > > + > > > > + pch = strrchr(name, '_'); > > > > > > What happens if name is NULL ? > > > > > > > + if (!pch) > > > > + return -EINVAL; > > > > + > > > > + if (strcmp(pch + 1, "X") == 0) { > > > > + *modifier = IIO_MOD_X; > > > > + return 0; > > > > + } else if (strcmp(pch + 1, "Y") == 0) { > > > > + *modifier = IIO_MOD_Y; > > > > + return 0; > > > > + } else if (strcmp(pch + 1, "Z") == 0) { > > > > + *modifier = IIO_MOD_Z; > > > > + return 0; > > > > + } else { > > > > + return -EINVAL; > > > > + } > > > > > > Moroever, being a single char compare cannot this be done with a switch ? > > > > > > char mod = *(pch + 1); > > > > > > switch(mod) { > > > case 'X': > > > *modifier = IIO_MOD_X; > > > break > > > ... > > > > > > > +} > > > > + > > > > +static int scmi_iio_get_chan_type(u8 scmi_type, enum iio_chan_type *iio_type) > > > > +{ > > > > + switch (scmi_type) { > > > > + case METERS_SEC_SQUARED: > > > > + *iio_type = IIO_ACCEL; > > > > + return 0; > > > why not a break to a final 'return ret;' (with ret properly set) ? > > > > > > > + case RADIANS_SEC: > > > > + *iio_type = IIO_ANGL_VEL; > > > > + return 0; > > > > + default: > > > > + return -EINVAL; > > > > + } > > > > +} > > > > + > > > > +static int scmi_iio_get_sensor_max_range(struct iio_dev *iio_dev, int *val, > > > > + int *val2) > > > > +{ > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev); > > > > + int max_range_high, max_range_low; > > > > + long long max_range; > > > > + > > > > + /* > > > > + * All the axes are supposed to have the same value for max range. > > > > + * We are just using the values from the Axis 0 here. > > > > + */ > > > > + if (sensor->sensor_info->axis[0].extended_attrs) { > > > > + max_range = sensor->sensor_info->axis[0].attrs.max_range; > > > > + max_range_high = H32(max_range); > > > > + max_range_low = L32(max_range); > > > > + > > > > + /* > > > > + * As IIO Val types have no provision for 64 bit values, > > > > + * and currently there are no known sensors using 64 bit > > > > + * for the range, this driver only supports sensor with > > > > + * 32 bit range value. > > > > + */ > > > > + if (max_range_high != 0) > > > > + return -EINVAL; > > > > + > > > > + *val = max_range_low; > > > > + *val2 = 1; > > > > + } > > > > + return 0; > > > > +} > > > > + > > > > +static void scmi_iio_get_sensor_resolution(struct iio_dev *iio_dev, int *val, > > > > + int *val2) > > > > +{ > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev); > > > > + > > > > + /* > > > > + * All the axes are supposed to have the same value for resolution > > > > + * and exponent. We are just using the values from the Axis 0 here. > > > > + */ > > > > + if (sensor->sensor_info->axis[0].extended_attrs) { > > > > + uint resolution = sensor->sensor_info->axis[0].resolution; > > > > + s8 exponent = sensor->sensor_info->axis[0].exponent; > > > > + s8 scale = sensor->sensor_info->axis[0].scale; > > > > + > > > > + /* > > > > + * To provide the raw value for the resolution to the userspace, > > > > + * need to divide the resolution exponent by the sensor scale > > > > + */ > > > > + exponent = exponent - scale; > > > > + if (exponent >= 0) { > > > > + *val = resolution * int_pow(10, exponent); > > > > + *val2 = 1; > > > > + } else { > > > > + *val = resolution; > > > > + *val2 = int_pow(10, abs(exponent)); > > > > + } > > > > + } > > > > +} > > > > + > > > > +static int scmi_iio_get_sensor_min_range(struct iio_dev *iio_dev, int *val, > > > > + int *val2) > > > > +{ > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev); > > > > + int min_range_high, min_range_low; > > > > + long long min_range; > > > > + > > > > + /* > > > > + * All the axes are supposed to have the same value for min range. > > > > + * We are just using the values from the Axis 0 here. > > > > + */ > > > > + if (sensor->sensor_info->axis[0].extended_attrs) { > > > > + min_range = sensor->sensor_info->axis[0].attrs.min_range; > > > > + min_range_high = H32(min_range); > > > > + min_range_low = L32(min_range); > > > > + > > > > + /* > > > > + * As IIO Val types have no provision for 64 bit values, > > > > + * and currently there are no known sensors using 64 bit > > > > + * for the range, this driver only supports sensor with > > > > + * 32 bit range value. > > > > + */ > > > > + if (min_range_high != 0xFFFFFFFF) > > > > + return -EINVAL; > > > > + > > > > + *val = min_range_low; > > > > + *val2 = 1; > > > > + } > > > > + return 0; > > > > +} > > > > + > > > > +static int scmi_iio_set_sensor_range_avail(struct iio_dev *iio_dev) > > > > +{ > > > > + struct scmi_iio_priv *sensor = iio_priv(iio_dev); > > > > + int ret; > > > > + > > > > + ret = scmi_iio_get_sensor_min_range(iio_dev, &sensor->range_avail[0], > > > > + &sensor->range_avail[1]); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + scmi_iio_get_sensor_resolution(iio_dev, &sensor->range_avail[2], > > > > + &sensor->range_avail[3]); > > > > + ret = scmi_iio_get_sensor_max_range(iio_dev, &sensor->range_avail[4], > > > > + &sensor->range_avail[5]); > > > > + return ret; > > > > +} > > > > + > > > > +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(u32) * (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; > > > > > > Not familiar with IIO, but is it fine to setup ops and modes AFTER > > > having attached the buffer to the scmi_iiodev ? > > > > > > Is is not 'racy-possible' that the buffer is already operational without > > > any ops immediately after being attached ? > > > > > > > + return 0; > > > > +} > > > > + > > > > +static int scmi_alloc_iiodev(struct device *dev, struct scmi_handle *handle, > > > > + const struct scmi_sensor_info *sensor_info, > > > > + struct iio_dev **scmi_iio_dev) > > > > +{ > > > > + 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 -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 -ENOMEM; > > > > + > > > > + scmi_iio_set_sampling_freq_avail(iiodev); > > > > + > > > > + ret = scmi_iio_set_sensor_range_avail(iiodev); > > > > + if (ret) { > > > > + dev_err(dev, "Error while setting the sensor %s range %d", > > > > + sensor_info->name, ret); > > > > + return ret; > > > > + } > > > > + > > > > + 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 ret; > > > > + > > > > + ret = scmi_iio_get_chan_modifier(sensor_info->axis[i].name, > > > > + &modifier); > > > > + if (ret < 0) > > > > + return 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; > > > > + *scmi_iio_dev = iiodev; > > > > + return ret; > > > > +} > > > > + > > > > +static int scmi_iio_dev_probe(struct scmi_device *sdev) > > > > +{ > > > > + const struct scmi_sensor_info *sensor_info; > > > > + struct scmi_handle *handle = sdev->handle; > > > > + struct device *dev = &sdev->dev; > > > > + struct iio_dev *scmi_iio_dev; > > > > + u16 nr_sensors; > > > > + int err, i; > > > > + > > > > + if (!handle || !handle->sensor_ops || !handle->sensor_ops->count_get || > > > > + !handle->sensor_ops->info_get || !handle->sensor_ops->config_get || > > > > + !handle->sensor_ops->config_set) { > > > > > > In all other SCMI driver we just check for: > > > > > > !handle && !handle->sensor_ops > > > > > > it should be enough, since all the ops are defined if the stack you are > > > using supports SCMIv3.0, if not this driver would not even compile since > > > some SCMIv3.0 Sensor Extension sensor_ops would be missing as a whole > > > inside the struct itself. > > > > > > Thanks > > > > > > Cristian > > > > > > > + dev_err(dev, "SCMI device has no sensor interface\n"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + nr_sensors = handle->sensor_ops->count_get(handle); > > > > + if (!nr_sensors) { > > > > + dev_dbg(dev, "0 sensors found via SCMI bus\n"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + dev_dbg(dev, "%d sensors found via SCMI bus\n", nr_sensors); > > > > + > > > > + for (i = 0; i < nr_sensors; i++) { > > > > + sensor_info = handle->sensor_ops->info_get(handle, i); > > > > + if (!sensor_info) { > > > > + dev_err(dev, "SCMI sensor %d has missing info\n", i); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + /* Skipping scalar sensor,as this driver only supports accel and gyro */ > > > > + if (sensor_info->num_axis == 0) > > > > + continue; > > > > + > > > > + err = scmi_alloc_iiodev(dev, handle, sensor_info, > > > > + &scmi_iio_dev); > > > > + if (err < 0) { > > > > + dev_err(dev, > > > > + "failed to allocate IIO device for sensor %s: %d\n", > > > > + sensor_info->name, err); > > > > + return err; > > > > + } > > > > + > > > > + err = scmi_iio_buffers_setup(scmi_iio_dev); > > > > + if (err < 0) { > > > > + dev_err(dev, > > > > + "IIO buffer setup error at sensor %s: %d\n", > > > > + sensor_info->name, err); > > > > + return err; > > > > + } > > > > + > > > > + err = devm_iio_device_register(dev, scmi_iio_dev); > > > > + if (err) { > > > > + dev_err(dev, > > > > + "IIO device registration failed at sensor %s: %d\n", > > > > + sensor_info->name, err); > > > > + return err; > > > > + } > > > > + } > > > > + return err; > > > > +} > > > > + > > > > +static const struct scmi_device_id scmi_id_table[] = { > > > > + { SCMI_PROTOCOL_SENSOR, "iiodev" }, > > > > + {}, > > > > +}; > > > > + > > > > +MODULE_DEVICE_TABLE(scmi, scmi_id_table); > > > > + > > > > +static struct scmi_driver scmi_iiodev_driver = { > > > > + .name = "scmi-sensor-iiodev", > > > > + .probe = scmi_iio_dev_probe, > > > > + .id_table = scmi_id_table, > > > > +}; > > > > + > > > > +module_scmi_driver(scmi_iiodev_driver); > > > > + > > > > +MODULE_AUTHOR("Jyoti Bhayana "); > > > > +MODULE_DESCRIPTION("SCMI IIO Driver"); > > > > +MODULE_LICENSE("GPL v2"); > > > > -- > > > > 2.30.0.280.ga3ce27912f-goog > > > > >