* Re: [PATCH v2] counter: stm32-lptimer-cnt: remove iio counter abi
[not found] <1611926542-2490-1-git-send-email-fabrice.gasnier@foss.st.com>
@ 2021-01-30 18:50 ` Jonathan Cameron
0 siblings, 0 replies; only message in thread
From: Jonathan Cameron @ 2021-01-30 18:50 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: vilhelm.gray, mchehab+huawei, lukas.bulwahn, linux-doc,
linux-arm-kernel, linux-kernel, alexandre.torgue, fabrice.gasnier,
olivier.moysan, linux-iio, linux-stm32
On Fri, 29 Jan 2021 14:22:22 +0100
Fabrice Gasnier <fabrice.gasnier@foss.st.com> wrote:
> Currently, the STM32 LP Timer counter driver registers into both IIO and
> counter subsystems, which is redundant.
>
> Remove the IIO counter ABI and IIO registration from the STM32 LP Timer
> counter driver since it's been superseded by the Counter subsystem
> as discussed in [1].
>
> Keep only the counter subsystem related part.
> Move a part of the ABI documentation into a driver comment.
>
> This also removes a duplicate ABI warning
> $ scripts/get_abi.pl validate
> ...
> /sys/bus/iio/devices/iio:deviceX/in_count0_preset is defined 2 times:
> ./Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:100
> ./Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:0
>
> [1] https://lkml.org/lkml/2021/1/19/347
>
> Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
The mod_devicetable.h include confused me for a bit, but then I realised
that iio.h includes of.h (which it almost certainly shouldn't).
I guess that's one to clean up at somepoint!
Applied to the togreg branch of iio.git and pushed out as testing.
Thanks,
Jonathan
> ---
> Changes in v2:
> - remove dependency on IIO subsustem, as spotted by William
> ---
> .../ABI/testing/sysfs-bus-iio-lptimer-stm32 | 62 -----
> drivers/counter/Kconfig | 2 +-
> drivers/counter/stm32-lptimer-cnt.c | 297 +++------------------
> 3 files changed, 37 insertions(+), 324 deletions(-)
> delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32
> deleted file mode 100644
> index 73498ff..00000000
> --- a/Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32
> +++ /dev/null
> @@ -1,62 +0,0 @@
> -What: /sys/bus/iio/devices/iio:deviceX/in_count0_preset
> -KernelVersion: 4.13
> -Contact: fabrice.gasnier@st.com
> -Description:
> - Reading returns the current preset value. Writing sets the
> - preset value. Encoder counts continuously from 0 to preset
> - value, depending on direction (up/down).
> -
> -What: /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
> -KernelVersion: 4.13
> -Contact: fabrice.gasnier@st.com
> -Description:
> - Reading returns the list possible quadrature modes.
> -
> -What: /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
> -KernelVersion: 4.13
> -Contact: fabrice.gasnier@st.com
> -Description:
> - Configure the device counter quadrature modes:
> -
> - - non-quadrature:
> - Encoder IN1 input servers as the count input (up
> - direction).
> -
> - - quadrature:
> - Encoder IN1 and IN2 inputs are mixed to get direction
> - and count.
> -
> -What: /sys/bus/iio/devices/iio:deviceX/in_count_polarity_available
> -KernelVersion: 4.13
> -Contact: fabrice.gasnier@st.com
> -Description:
> - Reading returns the list possible active edges.
> -
> -What: /sys/bus/iio/devices/iio:deviceX/in_count0_polarity
> -KernelVersion: 4.13
> -Contact: fabrice.gasnier@st.com
> -Description:
> - Configure the device encoder/counter active edge:
> -
> - - rising-edge
> - - falling-edge
> - - both-edges
> -
> - In non-quadrature mode, device counts up on active edge.
> -
> - In quadrature mode, encoder counting scenarios are as follows:
> -
> - +---------+----------+--------------------+--------------------+
> - | Active | Level on | IN1 signal | IN2 signal |
> - | edge | opposite +----------+---------+----------+---------+
> - | | signal | Rising | Falling | Rising | Falling |
> - +---------+----------+----------+---------+----------+---------+
> - | Rising | High -> | Down | - | Up | - |
> - | edge | Low -> | Up | - | Down | - |
> - +---------+----------+----------+---------+----------+---------+
> - | Falling | High -> | - | Up | - | Down |
> - | edge | Low -> | - | Down | - | Up |
> - +---------+----------+----------+---------+----------+---------+
> - | Both | High -> | Down | Up | Up | Down |
> - | edges | Low -> | Up | Down | Down | Up |
> - +---------+----------+----------+---------+----------+---------+
> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
> index 2de53ab..cbdf842 100644
> --- a/drivers/counter/Kconfig
> +++ b/drivers/counter/Kconfig
> @@ -41,7 +41,7 @@ config STM32_TIMER_CNT
>
> config STM32_LPTIMER_CNT
> tristate "STM32 LP Timer encoder counter driver"
> - depends on (MFD_STM32_LPTIMER || COMPILE_TEST) && IIO
> + depends on MFD_STM32_LPTIMER || COMPILE_TEST
> help
> Select this option to enable STM32 Low-Power Timer quadrature encoder
> and counter driver.
> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
> index fd6828e..9374396 100644
> --- a/drivers/counter/stm32-lptimer-cnt.c
> +++ b/drivers/counter/stm32-lptimer-cnt.c
> @@ -12,8 +12,8 @@
>
> #include <linux/bitfield.h>
> #include <linux/counter.h>
> -#include <linux/iio/iio.h>
> #include <linux/mfd/stm32-lptimer.h>
> +#include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/pinctrl/consumer.h>
> #include <linux/platform_device.h>
> @@ -107,249 +107,27 @@ static int stm32_lptim_setup(struct stm32_lptim_cnt *priv, int enable)
> return regmap_update_bits(priv->regmap, STM32_LPTIM_CFGR, mask, val);
> }
>
> -static int stm32_lptim_write_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int val, int val2, long mask)
> -{
> - struct stm32_lptim_cnt *priv = iio_priv(indio_dev);
> - int ret;
> -
> - switch (mask) {
> - case IIO_CHAN_INFO_ENABLE:
> - if (val < 0 || val > 1)
> - return -EINVAL;
> -
> - /* Check nobody uses the timer, or already disabled/enabled */
> - ret = stm32_lptim_is_enabled(priv);
> - if ((ret < 0) || (!ret && !val))
> - return ret;
> - if (val && ret)
> - return -EBUSY;
> -
> - ret = stm32_lptim_setup(priv, val);
> - if (ret)
> - return ret;
> - return stm32_lptim_set_enable_state(priv, val);
> -
> - default:
> - return -EINVAL;
> - }
> -}
> -
> -static int stm32_lptim_read_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int *val, int *val2, long mask)
> -{
> - struct stm32_lptim_cnt *priv = iio_priv(indio_dev);
> - u32 dat;
> - int ret;
> -
> - switch (mask) {
> - case IIO_CHAN_INFO_RAW:
> - ret = regmap_read(priv->regmap, STM32_LPTIM_CNT, &dat);
> - if (ret)
> - return ret;
> - *val = dat;
> - return IIO_VAL_INT;
> -
> - case IIO_CHAN_INFO_ENABLE:
> - ret = stm32_lptim_is_enabled(priv);
> - if (ret < 0)
> - return ret;
> - *val = ret;
> - return IIO_VAL_INT;
> -
> - case IIO_CHAN_INFO_SCALE:
> - /* Non-quadrature mode: scale = 1 */
> - *val = 1;
> - *val2 = 0;
> - if (priv->quadrature_mode) {
> - /*
> - * Quadrature encoder mode:
> - * - both edges, quarter cycle, scale is 0.25
> - * - either rising/falling edge scale is 0.5
> - */
> - if (priv->polarity > 1)
> - *val2 = 2;
> - else
> - *val2 = 1;
> - }
> - return IIO_VAL_FRACTIONAL_LOG2;
> -
> - default:
> - return -EINVAL;
> - }
> -}
> -
> -static const struct iio_info stm32_lptim_cnt_iio_info = {
> - .read_raw = stm32_lptim_read_raw,
> - .write_raw = stm32_lptim_write_raw,
> -};
> -
> -static const char *const stm32_lptim_quadrature_modes[] = {
> - "non-quadrature",
> - "quadrature",
> -};
> -
> -static int stm32_lptim_get_quadrature_mode(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan)
> -{
> - struct stm32_lptim_cnt *priv = iio_priv(indio_dev);
> -
> - return priv->quadrature_mode;
> -}
> -
> -static int stm32_lptim_set_quadrature_mode(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan,
> - unsigned int type)
> -{
> - struct stm32_lptim_cnt *priv = iio_priv(indio_dev);
> -
> - if (stm32_lptim_is_enabled(priv))
> - return -EBUSY;
> -
> - priv->quadrature_mode = type;
> -
> - return 0;
> -}
> -
> -static const struct iio_enum stm32_lptim_quadrature_mode_en = {
> - .items = stm32_lptim_quadrature_modes,
> - .num_items = ARRAY_SIZE(stm32_lptim_quadrature_modes),
> - .get = stm32_lptim_get_quadrature_mode,
> - .set = stm32_lptim_set_quadrature_mode,
> -};
> -
> -static const char * const stm32_lptim_cnt_polarity[] = {
> - "rising-edge", "falling-edge", "both-edges",
> -};
> -
> -static int stm32_lptim_cnt_get_polarity(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan)
> -{
> - struct stm32_lptim_cnt *priv = iio_priv(indio_dev);
> -
> - return priv->polarity;
> -}
> -
> -static int stm32_lptim_cnt_set_polarity(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan,
> - unsigned int type)
> -{
> - struct stm32_lptim_cnt *priv = iio_priv(indio_dev);
> -
> - if (stm32_lptim_is_enabled(priv))
> - return -EBUSY;
> -
> - priv->polarity = type;
> -
> - return 0;
> -}
> -
> -static const struct iio_enum stm32_lptim_cnt_polarity_en = {
> - .items = stm32_lptim_cnt_polarity,
> - .num_items = ARRAY_SIZE(stm32_lptim_cnt_polarity),
> - .get = stm32_lptim_cnt_get_polarity,
> - .set = stm32_lptim_cnt_set_polarity,
> -};
> -
> -static ssize_t stm32_lptim_cnt_get_ceiling(struct stm32_lptim_cnt *priv,
> - char *buf)
> -{
> - return snprintf(buf, PAGE_SIZE, "%u\n", priv->ceiling);
> -}
> -
> -static ssize_t stm32_lptim_cnt_set_ceiling(struct stm32_lptim_cnt *priv,
> - const char *buf, size_t len)
> -{
> - int ret;
> -
> - if (stm32_lptim_is_enabled(priv))
> - return -EBUSY;
> -
> - ret = kstrtouint(buf, 0, &priv->ceiling);
> - if (ret)
> - return ret;
> -
> - if (priv->ceiling > STM32_LPTIM_MAX_ARR)
> - return -EINVAL;
> -
> - return len;
> -}
> -
> -static ssize_t stm32_lptim_cnt_get_preset_iio(struct iio_dev *indio_dev,
> - uintptr_t private,
> - const struct iio_chan_spec *chan,
> - char *buf)
> -{
> - struct stm32_lptim_cnt *priv = iio_priv(indio_dev);
> -
> - return stm32_lptim_cnt_get_ceiling(priv, buf);
> -}
> -
> -static ssize_t stm32_lptim_cnt_set_preset_iio(struct iio_dev *indio_dev,
> - uintptr_t private,
> - const struct iio_chan_spec *chan,
> - const char *buf, size_t len)
> -{
> - struct stm32_lptim_cnt *priv = iio_priv(indio_dev);
> -
> - return stm32_lptim_cnt_set_ceiling(priv, buf, len);
> -}
> -
> -/* LP timer with encoder */
> -static const struct iio_chan_spec_ext_info stm32_lptim_enc_ext_info[] = {
> - {
> - .name = "preset",
> - .shared = IIO_SEPARATE,
> - .read = stm32_lptim_cnt_get_preset_iio,
> - .write = stm32_lptim_cnt_set_preset_iio,
> - },
> - IIO_ENUM("polarity", IIO_SEPARATE, &stm32_lptim_cnt_polarity_en),
> - IIO_ENUM_AVAILABLE("polarity", &stm32_lptim_cnt_polarity_en),
> - IIO_ENUM("quadrature_mode", IIO_SEPARATE,
> - &stm32_lptim_quadrature_mode_en),
> - IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_lptim_quadrature_mode_en),
> - {}
> -};
> -
> -static const struct iio_chan_spec stm32_lptim_enc_channels = {
> - .type = IIO_COUNT,
> - .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> - BIT(IIO_CHAN_INFO_ENABLE) |
> - BIT(IIO_CHAN_INFO_SCALE),
> - .ext_info = stm32_lptim_enc_ext_info,
> - .indexed = 1,
> -};
> -
> -/* LP timer without encoder (counter only) */
> -static const struct iio_chan_spec_ext_info stm32_lptim_cnt_ext_info[] = {
> - {
> - .name = "preset",
> - .shared = IIO_SEPARATE,
> - .read = stm32_lptim_cnt_get_preset_iio,
> - .write = stm32_lptim_cnt_set_preset_iio,
> - },
> - IIO_ENUM("polarity", IIO_SEPARATE, &stm32_lptim_cnt_polarity_en),
> - IIO_ENUM_AVAILABLE("polarity", &stm32_lptim_cnt_polarity_en),
> - {}
> -};
> -
> -static const struct iio_chan_spec stm32_lptim_cnt_channels = {
> - .type = IIO_COUNT,
> - .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> - BIT(IIO_CHAN_INFO_ENABLE) |
> - BIT(IIO_CHAN_INFO_SCALE),
> - .ext_info = stm32_lptim_cnt_ext_info,
> - .indexed = 1,
> -};
> -
> /**
> * enum stm32_lptim_cnt_function - enumerates LPTimer counter & encoder modes
> * @STM32_LPTIM_COUNTER_INCREASE: up count on IN1 rising, falling or both edges
> * @STM32_LPTIM_ENCODER_BOTH_EDGE: count on both edges (IN1 & IN2 quadrature)
> + *
> + * In non-quadrature mode, device counts up on active edge.
> + * In quadrature mode, encoder counting scenarios are as follows:
> + * +---------+----------+--------------------+--------------------+
> + * | Active | Level on | IN1 signal | IN2 signal |
> + * | edge | opposite +----------+---------+----------+---------+
> + * | | signal | Rising | Falling | Rising | Falling |
> + * +---------+----------+----------+---------+----------+---------+
> + * | Rising | High -> | Down | - | Up | - |
> + * | edge | Low -> | Up | - | Down | - |
> + * +---------+----------+----------+---------+----------+---------+
> + * | Falling | High -> | - | Up | - | Down |
> + * | edge | Low -> | - | Down | - | Up |
> + * +---------+----------+----------+---------+----------+---------+
> + * | Both | High -> | Down | Up | Up | Down |
> + * | edges | Low -> | Up | Down | Down | Up |
> + * +---------+----------+----------+---------+----------+---------+
> */
> enum stm32_lptim_cnt_function {
> STM32_LPTIM_COUNTER_INCREASE,
> @@ -484,7 +262,7 @@ static ssize_t stm32_lptim_cnt_ceiling_read(struct counter_device *counter,
> {
> struct stm32_lptim_cnt *const priv = counter->priv;
>
> - return stm32_lptim_cnt_get_ceiling(priv, buf);
> + return snprintf(buf, PAGE_SIZE, "%u\n", priv->ceiling);
> }
>
> static ssize_t stm32_lptim_cnt_ceiling_write(struct counter_device *counter,
> @@ -493,8 +271,22 @@ static ssize_t stm32_lptim_cnt_ceiling_write(struct counter_device *counter,
> const char *buf, size_t len)
> {
> struct stm32_lptim_cnt *const priv = counter->priv;
> + unsigned int ceiling;
> + int ret;
> +
> + if (stm32_lptim_is_enabled(priv))
> + return -EBUSY;
> +
> + ret = kstrtouint(buf, 0, &ceiling);
> + if (ret)
> + return ret;
> +
> + if (ceiling > STM32_LPTIM_MAX_ARR)
> + return -EINVAL;
> +
> + priv->ceiling = ceiling;
>
> - return stm32_lptim_cnt_set_ceiling(priv, buf, len);
> + return len;
> }
>
> static const struct counter_count_ext stm32_lptim_cnt_ext[] = {
> @@ -630,32 +422,19 @@ static int stm32_lptim_cnt_probe(struct platform_device *pdev)
> {
> struct stm32_lptimer *ddata = dev_get_drvdata(pdev->dev.parent);
> struct stm32_lptim_cnt *priv;
> - struct iio_dev *indio_dev;
> - int ret;
>
> if (IS_ERR_OR_NULL(ddata))
> return -EINVAL;
>
> - indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
> - if (!indio_dev)
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> return -ENOMEM;
>
> - priv = iio_priv(indio_dev);
> priv->dev = &pdev->dev;
> priv->regmap = ddata->regmap;
> priv->clk = ddata->clk;
> priv->ceiling = STM32_LPTIM_MAX_ARR;
>
> - /* Initialize IIO device */
> - indio_dev->name = dev_name(&pdev->dev);
> - indio_dev->dev.of_node = pdev->dev.of_node;
> - indio_dev->info = &stm32_lptim_cnt_iio_info;
> - if (ddata->has_encoder)
> - indio_dev->channels = &stm32_lptim_enc_channels;
> - else
> - indio_dev->channels = &stm32_lptim_cnt_channels;
> - indio_dev->num_channels = 1;
> -
> /* Initialize Counter device */
> priv->counter.name = dev_name(&pdev->dev);
> priv->counter.parent = &pdev->dev;
> @@ -673,10 +452,6 @@ static int stm32_lptim_cnt_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, priv);
>
> - ret = devm_iio_device_register(&pdev->dev, indio_dev);
> - if (ret)
> - return ret;
> -
> return devm_counter_register(&pdev->dev, &priv->counter);
> }
>
^ permalink raw reply [flat|nested] only message in thread