| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a1368383-8e5e-72ab-32f9-efe9715aaccb@st.com>
Date: Mon, 26 Jun 2017 18:41:36 +0200
From: Fabrice Gasnier <fabrice.gasnier@...com>
To: Jonathan Cameron <jic23@...nel.org>
CC: <lee.jones@...aro.org>, <benjamin.gaignard@...aro.org>,
<thierry.reding@...il.com>, <robh+dt@...nel.org>,
<mark.rutland@....com>, <alexandre.torgue@...com>,
<mcoquelin.stm32@...il.com>, <benjamin.gaignard@...com>,
<linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <linux-pwm@...r.kernel.org>
Subject: Re: [PATCH v2 6/8] iio: trigger: Add STM32 LPTimer trigger driver
On 06/24/2017 10:13 PM, Jonathan Cameron wrote:
> On Wed, 21 Jun 2017 16:30:13 +0200
> Fabrice Gasnier <fabrice.gasnier@...com> wrote:
>
>> Add support for LPTIMx_OUT triggers that can be found on some STM32
>> devices. These triggers can be used then by ADC or DAC.
>> Typical usage is to configure LPTimer as PWM output (via pwm-stm32-lp)
>> and have synchronised analog conversions with these triggers.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@...com>
> Given this can't be used as a trigger for other devices (no exposed
> interrupt?) I'd expect to see a validate_device callback provided for
> the trigger ops. That would prevent other devices trying to use it.
Hi Jonathan,
This is something I had in mind also earlier. Only thing is...
Basically, this is limiting: when trigger poll happens on device side
(e.g. ADC), another device could use same trigger. But I admit this
looks like corner case.
I'll add it in next version, with additional patch for ADC part to
validate it's a valid device (No DAC yet).
I think I'll use INDIO_HARDWARE_TRIGGERED mode:
- in adc driver: indio_dev->modes |= INDIO_HARDWARE_TRIGGERED;
- in lptimer: if (indio_dev->modes & INDIO_HARDWARE_TRIGGERED)...
>
> Otherwise, looks good.
Many thanks for your review.
Best Regards,
Fabrice
>
> Jonathan
>> ---
>> Changes in v2:
>> - s/Low Power/Low-Power
>> - update few comments
>> ---
>> drivers/iio/trigger/Kconfig | 11 +++
>> drivers/iio/trigger/Makefile | 1 +
>> drivers/iio/trigger/stm32-lptimer-trigger.c | 110 ++++++++++++++++++++++++++
>> include/linux/iio/timer/stm32-lptim-trigger.h | 24 ++++++
>> 4 files changed, 146 insertions(+)
>> create mode 100644 drivers/iio/trigger/stm32-lptimer-trigger.c
>> create mode 100644 include/linux/iio/timer/stm32-lptim-trigger.h
>>
>> diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
>> index e4d4e63..a633d2c 100644
>> --- a/drivers/iio/trigger/Kconfig
>> +++ b/drivers/iio/trigger/Kconfig
>> @@ -24,6 +24,17 @@ config IIO_INTERRUPT_TRIGGER
>> To compile this driver as a module, choose M here: the
>> module will be called iio-trig-interrupt.
>>
>> +config IIO_STM32_LPTIMER_TRIGGER
>> + tristate "STM32 Low-Power Timer Trigger"
>> + depends on MFD_STM32_LPTIMER || COMPILE_TEST
>> + help
>> + Select this option to enable STM32 Low-Power Timer Trigger.
>> + This can be used as trigger source for STM32 internal ADC
>> + and/or DAC.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called stm32-lptimer-trigger.
>> +
>> config IIO_STM32_TIMER_TRIGGER
>> tristate "STM32 Timer Trigger"
>> depends on (ARCH_STM32 && OF && MFD_STM32_TIMERS) || COMPILE_TEST
>> diff --git a/drivers/iio/trigger/Makefile b/drivers/iio/trigger/Makefile
>> index 5c4ecd3..0a72a2a 100644
>> --- a/drivers/iio/trigger/Makefile
>> +++ b/drivers/iio/trigger/Makefile
>> @@ -6,6 +6,7 @@
>>
>> obj-$(CONFIG_IIO_HRTIMER_TRIGGER) += iio-trig-hrtimer.o
>> obj-$(CONFIG_IIO_INTERRUPT_TRIGGER) += iio-trig-interrupt.o
>> +obj-$(CONFIG_IIO_STM32_LPTIMER_TRIGGER) += stm32-lptimer-trigger.o
>> obj-$(CONFIG_IIO_STM32_TIMER_TRIGGER) += stm32-timer-trigger.o
>> obj-$(CONFIG_IIO_SYSFS_TRIGGER) += iio-trig-sysfs.o
>> obj-$(CONFIG_IIO_TIGHTLOOP_TRIGGER) += iio-trig-loop.o
>> diff --git a/drivers/iio/trigger/stm32-lptimer-trigger.c b/drivers/iio/trigger/stm32-lptimer-trigger.c
>> new file mode 100644
>> index 0000000..bcb9aa2
>> --- /dev/null
>> +++ b/drivers/iio/trigger/stm32-lptimer-trigger.c
>> @@ -0,0 +1,110 @@
>> +/*
>> + * STM32 Low-Power Timer Trigger driver
>> + *
>> + * Copyright (C) STMicroelectronics 2017
>> + *
>> + * Author: Fabrice Gasnier <fabrice.gasnier@...com>.
>> + *
>> + * License terms: GNU General Public License (GPL), version 2
>> + *
>> + * Inspired by Benjamin Gaignard's stm32-timer-trigger driver
>> + */
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/timer/stm32-lptim-trigger.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/mfd/stm32-lptimer.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +
>> +/* List Low-Power Timer triggers */
>> +static const char * const stm32_lptim_triggers[] = {
>> + LPTIM1_OUT,
>> + LPTIM2_OUT,
>> + LPTIM3_OUT,
>> +};
>> +
>> +struct stm32_lptim_trigger {
>> + struct device *dev;
>> + const char *trg;
>> +};
>> +
>> +static const struct iio_trigger_ops stm32_lptim_trigger_ops = {
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +/**
>> + * is_stm32_lptim_trigger
>> + * @trig: trigger to be checked
>> + *
>> + * return true if the trigger is a valid STM32 IIO Low-Power Timer Trigger
>> + * either return false
>> + */
>> +bool is_stm32_lptim_trigger(struct iio_trigger *trig)
>> +{
>> + return (trig->ops == &stm32_lptim_trigger_ops);
>> +}
>> +EXPORT_SYMBOL(is_stm32_lptim_trigger);
>> +
>> +static int stm32_lptim_setup_trig(struct stm32_lptim_trigger *priv)
>> +{
>> + struct iio_trigger *trig;
>> +
>> + trig = devm_iio_trigger_alloc(priv->dev, "%s", priv->trg);
>> + if (!trig)
>> + return -ENOMEM;
>> +
>> + trig->dev.parent = priv->dev->parent;
>> + trig->ops = &stm32_lptim_trigger_ops;
>> + iio_trigger_set_drvdata(trig, priv);
>> +
>> + return devm_iio_trigger_register(priv->dev, trig);
>> +}
>> +
>> +static int stm32_lptim_trigger_probe(struct platform_device *pdev)
>> +{
>> + struct stm32_lptim_trigger *priv;
>> + u32 index;
>> + int ret;
>> +
>> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + if (of_property_read_u32(pdev->dev.of_node, "reg", &index))
>> + return -EINVAL;
>> +
>> + if (index >= ARRAY_SIZE(stm32_lptim_triggers))
>> + return -EINVAL;
>> +
>> + priv->dev = &pdev->dev;
>> + priv->trg = stm32_lptim_triggers[index];
>> +
>> + ret = stm32_lptim_setup_trig(priv);
>> + if (ret)
>> + return ret;
>> +
>> + platform_set_drvdata(pdev, priv);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id stm32_lptim_trig_of_match[] = {
>> + { .compatible = "st,stm32-lptimer-trigger", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_lptim_trig_of_match);
>> +
>> +static struct platform_driver stm32_lptim_trigger_driver = {
>> + .probe = stm32_lptim_trigger_probe,
>> + .driver = {
>> + .name = "stm32-lptimer-trigger",
>> + .of_match_table = stm32_lptim_trig_of_match,
>> + },
>> +};
>> +module_platform_driver(stm32_lptim_trigger_driver);
>> +
>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@...com>");
>> +MODULE_ALIAS("platform:stm32-lptimer-trigger");
>> +MODULE_DESCRIPTION("STMicroelectronics STM32 LPTIM trigger driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/iio/timer/stm32-lptim-trigger.h b/include/linux/iio/timer/stm32-lptim-trigger.h
>> new file mode 100644
>> index 0000000..cb795b1
>> --- /dev/null
>> +++ b/include/linux/iio/timer/stm32-lptim-trigger.h
>> @@ -0,0 +1,24 @@
>> +/*
>> + * Copyright (C) STMicroelectronics 2017
>> + *
>> + * Author: Fabrice Gasnier <fabrice.gasnier@...com>
>> + *
>> + * License terms: GNU General Public License (GPL), version 2
>> + */
>> +
>> +#ifndef _STM32_LPTIM_TRIGGER_H_
>> +#define _STM32_LPTIM_TRIGGER_H_
>> +
>> +#define LPTIM1_OUT "lptim1_out"
>> +#define LPTIM2_OUT "lptim2_out"
>> +#define LPTIM3_OUT "lptim3_out"
>> +
>> +#if IS_ENABLED(CONFIG_IIO_STM32_LPTIMER_TRIGGER)
>> +bool is_stm32_lptim_trigger(struct iio_trigger *trig);
>> +#else
>> +static inline bool is_stm32_lptim_trigger(struct iio_trigger *trig)
>> +{
>> + return false;
>> +}
>> +#endif
>> +#endif
>
Powered by blists - more mailing lists