2017-01-03 10:23 GMT+01:00 Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx>: > 2017-01-02 19:22 GMT+01:00 Jonathan Cameron <jic23@xxxxxxxxxx>: >> On 02/01/17 08:46, Benjamin Gaignard wrote: >>> 2016-12-30 22:12 GMT+01:00 Jonathan Cameron <jic23@xxxxxxxxxx>: >>>> On 09/12/16 14:15, Benjamin Gaignard wrote: >>>>> Timers IPs can be used to generate triggers for other IPs like >>>>> DAC, ADC or other timers. >>>>> Each trigger may result of timer internals signals like counter enable, >>>>> reset or edge, this configuration could be done through "master_mode" >>>>> device attribute. >>>>> >>>>> A timer device could be triggered by other timers, we use the trigger >>>>> name and is_stm32_iio_timer_trigger() function to distinguish them >>>>> and configure IP input switch. >>>>> >>>>> Timer may also decide on which event (edge, level) they could >>>>> be activated by a trigger, this configuration is done by writing in >>>>> "slave_mode" device attribute. >>>>> >>>>> Since triggers could also be used by DAC or ADC their names are defined >>>>> in include/ nux/iio/timer/stm32-timer-trigger.h so those IPs will be able >>>>> to configure themselves in valid_trigger function >>>>> >>>>> Trigger have a "sampling_frequency" attribute which allow to configure >>>>> timer sampling frequency without using PWM interface >>>>> >>>>> version 5: >>>>> - simplify tables of triggers >>>>> - only create an IIO device when needed >>>>> >>>>> version 4: >>>>> - get triggers configuration from "reg" in DT >>>>> - add tables of triggers >>>>> - sampling frequency is enable/disable when writing in trigger >>>>> sampling_frequency attribute >>>>> - no more use of interruptions >>>>> >>>>> version 3: >>>>> - change compatible to "st,stm32-timer-trigger" >>>>> - fix attributes access right >>>>> - use string instead of int for master_mode and slave_mode >>>>> - document device attributes in sysfs-bus-iio-timer-stm32 >>>>> >>>>> version 2: >>>>> - keep only one compatible >>>>> - use st,input-triggers-names and st,output-triggers-names >>>>> to know which triggers are accepted and/or create by the device >>>> Firstly, sorry it has taken me so long to get back to this. >>>> >>>> I'm still not keen on this use of iio_device elements just to act as >>>> glue between triggers. I think we need to work out a more light weight >>>> way to do this. As you are only using them for validation and to provide >>>> somewhere to hang the control attibutes off, there is nothing stopping us >>>> moving that over to the iio_trigger instead which would avoid the messy >>>> duality going on here. >>> >>> I have add an iio_device because each hardware can generate multiple >>> triggers (up to 5: trgo, ch 1...4) and slave_mode attribute will impact all the >>> triggers of a device. For me it was making sense to centralize that in an >>> iio_device rather than having an attribute "shared" (from hardware >>> point of view) >>> on multiple triggers. >>> Since master_mode attribute is only used by trgo and not impact ch1...4 >>> triggers I will move it to trigger instead of the iio_device. >>> >>> I also wanted to be able to connect triggers on a iio_device as I >>> could do for an >>> ADC with a command like 'echo "tim1_trgo" > iio_deviceX/trigger/current_trigger' >> This is interesting, but with a bit of refactoring I would think it would >> be possible to share some of that code thus allowing non IIO devices to >> bind to triggers. Ultimately I want to be able to bind a trigger to >> a trigger - I appreciate here the topology is more limited than that >> so some complexity comes in. >> >> My gut feeling is that representing that topology explicitly is hard >> to do in a remotely general way, but lets try it and see. >> We run into this sort of interdependency issue between different bits of >> the hardware all the time. Setting a value somewhere effects the configuration >> elsewhere - often the best plan is to just let that happen and leave it up to >> userspace to check for changes if it cares. > > okay > >>> If I change that to parent_trigger attribute it change this behavior >>> and I will have to >>> duplicated what is done in iio_trigger_write_current() to find and >>> validate triggers. >> I get the reasoning, but we still end up with something represented >> by an IIO device that isn't providing any channels at all. It's simply >> using some of the infrastructure. To my mind it is 'something else' >> and should be represented as such. I have no problem at all with >> you registering additional elements in /sysfs/bus/iio/ to represent >> these shared elements - we already have drivers that do that to >> provide some centralized infrastructure (e.g. the sysfs-trigger) > > My hardware block are timers maybe I can add a channel type "IIO_TIMER" > and declare a channel with info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ) > so I will be able to write/read sampling frequency on IIO device. > If it isn't possible to implement IIO_TIMER I will simply drop device part of my driver until we find a solution because I would like to upstream at least what is need to ADC/DAC. >> I'm worried about the scope spread we get for an IIO device otherwise. >> They serve a well defined purpose at the moment, and that isn't what >> is happening here. >> >> So my gut feeling is we are better deliberately not representing the >> inter dependence and claiming all triggers we are creating are >> independent. That way we can have a nice generic infrastructure >> that will work in all cases (be it pushing the sanity checking to >> userspace). >> >> So each trigger has direct access to what controls it. Changing anything >> can effect other triggers in weird ways. >> >> I'm finding it hard to see anything else generalizing sufficiently >> as we'll always get cases where we can't represent the topology without >> diving into the complexity of something like the media controller >> framework. >> >> Jonathan >>> >>>> I might still be missing something though! >>>> >>>> You would only I think need 3 attributes >>>> >>>> parrent_trigger >>>> and something like your master_mode and slave_mode attributes. >>>> >>>> The parrent_trigger would need some validation etc, but if we keep it >>>> within this driver initially that won't be hard to do. Checking the device >>>> parent matches will do most of it. >>>> >>>> Jonathan >>>>> >>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxx> >>>>> --- >>>>> .../ABI/testing/sysfs-bus-iio-timer-stm32 | 55 +++ >>>>> drivers/iio/Kconfig | 2 +- >>>>> drivers/iio/Makefile | 1 + >>>>> drivers/iio/timer/Kconfig | 13 + >>>>> drivers/iio/timer/Makefile | 1 + >>>>> drivers/iio/timer/stm32-timer-trigger.c | 466 +++++++++++++++++++++ >>>>> drivers/iio/trigger/Kconfig | 1 - >>>>> include/linux/iio/timer/stm32-timer-trigger.h | 62 +++ >>>>> 8 files changed, 599 insertions(+), 2 deletions(-) >>>>> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 >>>>> create mode 100644 drivers/iio/timer/Kconfig >>>>> create mode 100644 drivers/iio/timer/Makefile >>>>> create mode 100644 drivers/iio/timer/stm32-timer-trigger.c >>>>> create mode 100644 include/linux/iio/timer/stm32-timer-trigger.h >>>>> >>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 >>>>> new file mode 100644 >>>>> index 0000000..26583dd >>>>> --- /dev/null >>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 >>>>> @@ -0,0 +1,55 @@ >>>>> +What: /sys/bus/iio/devices/iio:deviceX/master_mode_available >>>>> +KernelVersion: 4.10 >>>>> +Contact: benjamin.gaignard@xxxxxx >>>>> +Description: >>>>> + Reading returns the list possible master modes which are: >>>>> + - "reset" : The UG bit from the TIMx_EGR register is used as trigger output (TRGO). >>>>> + - "enable" : The Counter Enable signal CNT_EN is used as trigger output. >>>>> + - "update" : The update event is selected as trigger output. >>>>> + For instance a master timer can then be used as a prescaler for a slave timer. >>>>> + - "compare_pulse" : The trigger output send a positive pulse when the CC1IF flag is to be set. >>>>> + - "OC1REF" : OC1REF signal is used as trigger output. >>>>> + - "OC2REF" : OC2REF signal is used as trigger output. >>>>> + - "OC3REF" : OC3REF signal is used as trigger output. >>>>> + - "OC4REF" : OC4REF signal is used as trigger output. >>>>> + >>>>> +What: /sys/bus/iio/devices/iio:deviceX/master_mode >>>>> +KernelVersion: 4.10 >>>>> +Contact: benjamin.gaignard@xxxxxx >>>>> +Description: >>>>> + Reading returns the current master modes. >>>>> + Writing set the master mode >>>>> + >>>>> +What: /sys/bus/iio/devices/iio:deviceX/slave_mode_available >>>>> +KernelVersion: 4.10 >>>>> +Contact: benjamin.gaignard@xxxxxx >>>>> +Description: >>>>> + Reading returns the list possible slave modes which are: >>>>> + - "disabled" : The prescaler is clocked directly by the internal clock. >>>>> + - "encoder_1" : Counter counts up/down on TI2FP1 edge depending on TI1FP2 level. >>>>> + - "encoder_2" : Counter counts up/down on TI1FP2 edge depending on TI2FP1 level. >>>>> + - "encoder_3" : Counter counts up/down on both TI1FP1 and TI2FP2 edges depending >>>>> + on the level of the other input. >>>>> + - "reset" : Rising edge of the selected trigger input reinitializes the counter >>>>> + and generates an update of the registers. >>>>> + - "gated" : The counter clock is enabled when the trigger input is high. >>>>> + The counter stops (but is not reset) as soon as the trigger becomes low. >>>>> + Both start and stop of the counter are controlled. >>>>> + - "trigger" : The counter starts at a rising edge of the trigger TRGI (but it is not >>>>> + reset). Only the start of the counter is controlled. >>>>> + - "external_clock": Rising edges of the selected trigger (TRGI) clock the counter. >>>>> + >>>>> +What: /sys/bus/iio/devices/iio:deviceX/slave_mode >>>>> +KernelVersion: 4.10 >>>>> +Contact: benjamin.gaignard@xxxxxx >>>>> +Description: >>>>> + Reading returns the current slave mode. >>>>> + Writing set the slave mode >>>>> + >>>>> +What: /sys/bus/iio/devices/triggerX/sampling_frequency >>>>> +KernelVersion: 4.10 >>>>> +Contact: benjamin.gaignard@xxxxxx >>>>> +Description: >>>>> + Reading returns the current sampling frequency. >>>>> + Writing an value different of 0 set and start sampling. >>>>> + Writing 0 stop sampling. >>>>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig >>>>> index 6743b18..2de2a80 100644 >>>>> --- a/drivers/iio/Kconfig >>>>> +++ b/drivers/iio/Kconfig >>>>> @@ -90,5 +90,5 @@ source "drivers/iio/potentiometer/Kconfig" >>>>> source "drivers/iio/pressure/Kconfig" >>>>> source "drivers/iio/proximity/Kconfig" >>>>> source "drivers/iio/temperature/Kconfig" >>>>> - >>>>> +source "drivers/iio/timer/Kconfig" >>>>> endif # IIO >>>>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile >>>>> index 87e4c43..b797c08 100644 >>>>> --- a/drivers/iio/Makefile >>>>> +++ b/drivers/iio/Makefile >>>>> @@ -32,4 +32,5 @@ obj-y += potentiometer/ >>>>> obj-y += pressure/ >>>>> obj-y += proximity/ >>>>> obj-y += temperature/ >>>>> +obj-y += timer/ >>>>> obj-y += trigger/ >>>>> diff --git a/drivers/iio/timer/Kconfig b/drivers/iio/timer/Kconfig >>>>> new file mode 100644 >>>>> index 0000000..e3c21f2 >>>>> --- /dev/null >>>>> +++ b/drivers/iio/timer/Kconfig >>>>> @@ -0,0 +1,13 @@ >>>>> +# >>>>> +# Timers drivers >>>>> + >>>>> +menu "Timers" >>>>> + >>>>> +config IIO_STM32_TIMER_TRIGGER >>>>> + tristate "STM32 Timer Trigger" >>>>> + depends on (ARCH_STM32 && OF && MFD_STM32_TIMERS) || COMPILE_TEST >>>>> + select IIO_TRIGGERED_EVENT >>>>> + help >>>>> + Select this option to enable STM32 Timer Trigger >>>>> + >>>>> +endmenu >>>>> diff --git a/drivers/iio/timer/Makefile b/drivers/iio/timer/Makefile >>>>> new file mode 100644 >>>>> index 0000000..4ad95ec9 >>>>> --- /dev/null >>>>> +++ b/drivers/iio/timer/Makefile >>>>> @@ -0,0 +1 @@ >>>>> +obj-$(CONFIG_IIO_STM32_TIMER_TRIGGER) += stm32-timer-trigger.o >>>>> diff --git a/drivers/iio/timer/stm32-timer-trigger.c b/drivers/iio/timer/stm32-timer-trigger.c >>>>> new file mode 100644 >>>>> index 0000000..8d16e8f >>>>> --- /dev/null >>>>> +++ b/drivers/iio/timer/stm32-timer-trigger.c >>>>> @@ -0,0 +1,466 @@ >>>>> +/* >>>>> + * Copyright (C) STMicroelectronics 2016 >>>>> + * >>>>> + * Author: Benjamin Gaignard <benjamin.gaignard@xxxxxx> >>>>> + * >>>>> + * License terms: GNU General Public License (GPL), version 2 >>>>> + */ >>>>> + >>>>> +#include <linux/iio/iio.h> >>>>> +#include <linux/iio/sysfs.h> >>>>> +#include <linux/iio/timer/stm32-timer-trigger.h> >>>>> +#include <linux/iio/trigger.h> >>>>> +#include <linux/iio/triggered_event.h> >>>>> +#include <linux/interrupt.h> >>>>> +#include <linux/mfd/stm32-timers.h> >>>>> +#include <linux/module.h> >>>>> +#include <linux/platform_device.h> >>>>> + >>>>> +#define MAX_TRIGGERS 6 >>>>> +#define MAX_VALIDS 5 >>>>> + >>>>> +/* List the triggers created by each timer */ >>>>> +static const void *triggers_table[][MAX_TRIGGERS] = { >>>>> + { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,}, >>>>> + { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4,}, >>>>> + { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4,}, >>>>> + { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4,}, >>>>> + { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4,}, >>>>> + { TIM6_TRGO,}, >>>>> + { TIM7_TRGO,}, >>>>> + { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,}, >>>>> + { TIM9_TRGO, TIM9_CH1, TIM9_CH2,}, >>>>> + { TIM12_TRGO, TIM12_CH1, TIM12_CH2,}, >>>>> +}; >>>>> + >>>>> +/* List the triggers accepted by each timer */ >>>>> +static const void *valids_table[][MAX_VALIDS] = { >>>>> + { TIM5_TRGO, TIM2_TRGO, TIM4_TRGO, TIM3_TRGO,}, >>>>> + { TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,}, >>>>> + { TIM1_TRGO, TIM8_TRGO, TIM5_TRGO, TIM4_TRGO,}, >>>>> + { TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,}, >>>>> + { TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,}, >>>>> + { }, /* timer 6 */ >>>>> + { }, /* timer 7 */ >>>>> + { TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,}, >>>>> + { TIM2_TRGO, TIM3_TRGO,}, >>>>> + { TIM4_TRGO, TIM5_TRGO,}, >>>>> +}; >>>>> + >>>>> +struct stm32_timer_trigger { >>>>> + struct device *dev; >>>>> + struct regmap *regmap; >>>>> + struct clk *clk; >>>>> + u32 max_arr; >>>>> + const void *triggers; >>>>> + const void *valids; >>>>> +}; >>>>> + >>>>> +static int stm32_timer_start(struct stm32_timer_trigger *priv, >>>>> + unsigned int frequency) >>>>> +{ >>>>> + unsigned long long prd, div; >>>>> + int prescaler = 0; >>>>> + u32 ccer, cr1; >>>>> + >>>>> + /* Period and prescaler values depends of clock rate */ >>>>> + div = (unsigned long long)clk_get_rate(priv->clk); >>>>> + >>>>> + do_div(div, frequency); >>>>> + >>>>> + prd = div; >>>>> + >>>>> + /* >>>>> + * Increase prescaler value until we get a result that fit >>>>> + * with auto reload register maximum value. >>>>> + */ >>>>> + while (div > priv->max_arr) { >>>>> + prescaler++; >>>>> + div = prd; >>>>> + do_div(div, (prescaler + 1)); >>>>> + } >>>>> + prd = div; >>>>> + >>>>> + if (prescaler > MAX_TIM_PSC) { >>>>> + dev_err(priv->dev, "prescaler exceeds the maximum value\n"); >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + /* Check if nobody else use the timer */ >>>>> + regmap_read(priv->regmap, TIM_CCER, &ccer); >>>>> + if (ccer & TIM_CCER_CCXE) >>>>> + return -EBUSY; >>>>> + >>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1); >>>>> + if (!(cr1 & TIM_CR1_CEN)) >>>>> + clk_enable(priv->clk); >>>>> + >>>>> + regmap_write(priv->regmap, TIM_PSC, prescaler); >>>>> + regmap_write(priv->regmap, TIM_ARR, prd - 1); >>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE); >>>>> + >>>>> + /* Force master mode to update mode */ >>>>> + regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS, 0x20); >>>>> + >>>>> + /* Make sure that registers are updated */ >>>>> + regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG); >>>>> + >>>>> + /* Enable controller */ >>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static void stm32_timer_stop(struct stm32_timer_trigger *priv) >>>>> +{ >>>>> + u32 ccer, cr1; >>>>> + >>>>> + regmap_read(priv->regmap, TIM_CCER, &ccer); >>>>> + if (ccer & TIM_CCER_CCXE) >>>>> + return; >>>>> + >>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1); >>>>> + if (cr1 & TIM_CR1_CEN) >>>>> + clk_disable(priv->clk); >>>>> + >>>>> + /* Stop timer */ >>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0); >>>>> + regmap_write(priv->regmap, TIM_PSC, 0); >>>>> + regmap_write(priv->regmap, TIM_ARR, 0); >>>>> +} >>>>> + >>>>> +static ssize_t stm32_tt_store_frequency(struct device *dev, >>>>> + struct device_attribute *attr, >>>>> + const char *buf, size_t len) >>>>> +{ >>>>> + struct iio_trigger *trig = to_iio_trigger(dev); >>>>> + struct stm32_timer_trigger *priv = iio_trigger_get_drvdata(trig); >>>>> + unsigned int freq; >>>>> + int ret; >>>>> + >>>>> + ret = kstrtouint(buf, 10, &freq); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + if (freq == 0) { >>>>> + stm32_timer_stop(priv); >>>>> + } else { >>>>> + ret = stm32_timer_start(priv, freq); >>>>> + if (ret) >>>>> + return ret; >>>>> + } >>>>> + >>>>> + return len; >>>>> +} >>>>> + >>>>> +static ssize_t stm32_tt_read_frequency(struct device *dev, >>>>> + struct device_attribute *attr, char *buf) >>>>> +{ >>>>> + struct iio_trigger *trig = to_iio_trigger(dev); >>>>> + struct stm32_timer_trigger *priv = iio_trigger_get_drvdata(trig); >>>>> + u32 psc, arr, cr1; >>>>> + unsigned long long freq = 0; >>>>> + >>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1); >>>>> + regmap_read(priv->regmap, TIM_PSC, &psc); >>>>> + regmap_read(priv->regmap, TIM_ARR, &arr); >>>>> + >>>>> + if (psc && arr && (cr1 & TIM_CR1_CEN)) { >>>>> + freq = (unsigned long long)clk_get_rate(priv->clk); >>>>> + do_div(freq, psc); >>>>> + do_div(freq, arr); >>>>> + } >>>>> + >>>>> + return sprintf(buf, "%d\n", (unsigned int)freq); >>>>> +} >>>>> + >>>>> +static IIO_DEV_ATTR_SAMP_FREQ(0660, >>>>> + stm32_tt_read_frequency, >>>>> + stm32_tt_store_frequency); >>>>> + >>>>> +static struct attribute *stm32_trigger_attrs[] = { >>>>> + &iio_dev_attr_sampling_frequency.dev_attr.attr, >>>>> + NULL, >>>>> +}; >>>>> + >>>>> +static const struct attribute_group stm32_trigger_attr_group = { >>>>> + .attrs = stm32_trigger_attrs, >>>>> +}; >>>>> + >>>>> +static const struct attribute_group *stm32_trigger_attr_groups[] = { >>>>> + &stm32_trigger_attr_group, >>>>> + NULL, >>>>> +}; >>>>> + >>>>> +static char *master_mode_table[] = { >>>>> + "reset", >>>>> + "enable", >>>>> + "update", >>>>> + "compare_pulse", >>>>> + "OC1REF", >>>>> + "OC2REF", >>>>> + "OC3REF", >>>>> + "OC4REF" >>>>> +}; >>>>> + >>>>> +static ssize_t stm32_tt_show_master_mode(struct device *dev, >>>>> + struct device_attribute *attr, >>>>> + char *buf) >>>>> +{ >>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev); >>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev); >>>>> + u32 cr2; >>>>> + >>>>> + regmap_read(priv->regmap, TIM_CR2, &cr2); >>>>> + cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT; >>>>> + >>>>> + return snprintf(buf, PAGE_SIZE, "%s\n", master_mode_table[cr2]); >>>>> +} >>>>> + >>>>> +static ssize_t stm32_tt_store_master_mode(struct device *dev, >>>>> + struct device_attribute *attr, >>>>> + const char *buf, size_t len) >>>>> +{ >>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev); >>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev); >>>>> + int i; >>>>> + >>>>> + for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) { >>>>> + if (!strncmp(master_mode_table[i], buf, >>>>> + strlen(master_mode_table[i]))) { >>>>> + regmap_update_bits(priv->regmap, TIM_CR2, >>>>> + TIM_CR2_MMS, i << TIM_CR2_MMS_SHIFT); >>>>> + return len; >>>>> + } >>>>> + } >>>>> + >>>>> + return -EINVAL; >>>>> +} >>>>> + >>>>> +static IIO_CONST_ATTR(master_mode_available, >>>>> + "reset enable update compare_pulse OC1REF OC2REF OC3REF OC4REF"); >>>>> + >>>>> +static IIO_DEVICE_ATTR(master_mode, 0660, >>>>> + stm32_tt_show_master_mode, >>>>> + stm32_tt_store_master_mode, >>>>> + 0); >>>>> + >>>>> +static char *slave_mode_table[] = { >>>>> + "disabled", >>>>> + "encoder_1", >>>>> + "encoder_2", >>>>> + "encoder_3", >>>>> + "reset", >>>>> + "gated", >>>>> + "trigger", >>>>> + "external_clock", >>>>> +}; >>>>> + >>>>> +static ssize_t stm32_tt_show_slave_mode(struct device *dev, >>>>> + struct device_attribute *attr, >>>>> + char *buf) >>>>> +{ >>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev); >>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev); >>>>> + u32 smcr; >>>>> + >>>>> + regmap_read(priv->regmap, TIM_SMCR, &smcr); >>>>> + smcr &= TIM_SMCR_SMS; >>>>> + >>>>> + return snprintf(buf, PAGE_SIZE, "%s\n", slave_mode_table[smcr]); >>>>> +} >>>>> + >>>>> +static ssize_t stm32_tt_store_slave_mode(struct device *dev, >>>>> + struct device_attribute *attr, >>>>> + const char *buf, size_t len) >>>>> +{ >>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev); >>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev); >>>>> + int i; >>>>> + >>>>> + for (i = 0; i < ARRAY_SIZE(slave_mode_table); i++) { >>>>> + if (!strncmp(slave_mode_table[i], buf, >>>>> + strlen(slave_mode_table[i]))) { >>>>> + regmap_update_bits(priv->regmap, >>>>> + TIM_SMCR, TIM_SMCR_SMS, i); >>>>> + return len; >>>>> + } >>>>> + } >>>>> + >>>>> + return -EINVAL; >>>>> +} >>>>> + >>>>> +static IIO_CONST_ATTR(slave_mode_available, >>>>> +"disabled encoder_1 encoder_2 encoder_3 reset gated trigger external_clock"); >>>>> + >>>>> +static IIO_DEVICE_ATTR(slave_mode, 0660, >>>>> + stm32_tt_show_slave_mode, >>>>> + stm32_tt_store_slave_mode, >>>>> + 0); >>>>> + >>>>> +static struct attribute *stm32_timer_attrs[] = { >>>>> + &iio_dev_attr_master_mode.dev_attr.attr, >>>>> + &iio_const_attr_master_mode_available.dev_attr.attr, >>>>> + &iio_dev_attr_slave_mode.dev_attr.attr, >>>>> + &iio_const_attr_slave_mode_available.dev_attr.attr, >>>>> + NULL, >>>>> +}; >>>>> + >>>>> +static const struct attribute_group stm32_timer_attr_group = { >>>>> + .attrs = stm32_timer_attrs, >>>>> +}; >>>>> + >>>>> +static const struct iio_trigger_ops timer_trigger_ops = { >>>>> + .owner = THIS_MODULE, >>>>> +}; >>>>> + >>>>> +static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv) >>>>> +{ >>>>> + int ret; >>>>> + const char * const *cur = priv->triggers; >>>>> + >>>>> + while (cur && *cur) { >>>>> + struct iio_trigger *trig; >>>>> + >>>>> + trig = devm_iio_trigger_alloc(priv->dev, "%s", *cur); >>>>> + if (!trig) >>>>> + return -ENOMEM; >>>>> + >>>>> + trig->dev.parent = priv->dev->parent; >>>>> + trig->ops = &timer_trigger_ops; >>>>> + trig->dev.groups = stm32_trigger_attr_groups; >>>>> + iio_trigger_set_drvdata(trig, priv); >>>>> + >>>>> + ret = devm_iio_trigger_register(priv->dev, trig); >>>>> + if (ret) >>>>> + return ret; >>>>> + cur++; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/** >>>>> + * is_stm32_timer_trigger >>>>> + * @trig: trigger to be checked >>>>> + * >>>>> + * return true if the trigger is a valid stm32 iio timer trigger >>>>> + * either return false >>>>> + */ >>>>> +bool is_stm32_timer_trigger(struct iio_trigger *trig) >>>>> +{ >>>>> + return (trig->ops == &timer_trigger_ops); >>>>> +} >>>>> +EXPORT_SYMBOL(is_stm32_timer_trigger); >>>>> + >>>>> +static int stm32_validate_trigger(struct iio_dev *indio_dev, >>>>> + struct iio_trigger *trig) >>>>> +{ >>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev); >>>>> + const char * const *cur = priv->valids; >>>>> + unsigned int i = 0; >>>>> + >>>>> + if (!is_stm32_timer_trigger(trig)) >>>>> + return -EINVAL; >>>>> + >>>>> + while (cur && *cur) { >>>>> + if (!strncmp(trig->name, *cur, strlen(trig->name))) { >>>>> + regmap_update_bits(priv->regmap, >>>>> + TIM_SMCR, TIM_SMCR_TS, >>>>> + i << TIM_SMCR_TS_SHIFT); >>>>> + return 0; >>>>> + } >>>>> + cur++; >>>>> + i++; >>>>> + } >>>>> + >>>>> + return -EINVAL; >>>>> +} >>>>> + >>>>> +static const struct iio_info stm32_trigger_info = { >>>>> + .driver_module = THIS_MODULE, >>>>> + .validate_trigger = stm32_validate_trigger, >>>>> + .attrs = &stm32_timer_attr_group, >>>>> +}; >>>>> + >>>>> +static struct stm32_timer_trigger *stm32_setup_iio_device(struct device *dev) >>>>> +{ >>>>> + struct iio_dev *indio_dev; >>>>> + int ret; >>>>> + >>>>> + indio_dev = devm_iio_device_alloc(dev, >>>>> + sizeof(struct stm32_timer_trigger)); >>>>> + if (!indio_dev) >>>>> + return NULL; >>>>> + >>>>> + indio_dev->name = dev_name(dev); >>>>> + indio_dev->dev.parent = dev; >>>>> + indio_dev->info = &stm32_trigger_info; >>>>> + indio_dev->modes = INDIO_EVENT_TRIGGERED; >>>>> + indio_dev->num_channels = 0; >>>>> + indio_dev->dev.of_node = dev->of_node; >>>>> + >>>>> + ret = devm_iio_device_register(dev, indio_dev); >>>>> + if (ret) >>>>> + return NULL; >>>>> + >>>>> + return iio_priv(indio_dev); >>>>> +} >>>>> + >>>>> +static int stm32_timer_trigger_probe(struct platform_device *pdev) >>>>> +{ >>>>> + struct device *dev = &pdev->dev; >>>>> + struct stm32_timer_trigger *priv; >>>>> + struct stm32_timers *ddata = dev_get_drvdata(pdev->dev.parent); >>>>> + unsigned int index; >>>>> + int ret; >>>>> + >>>>> + if (of_property_read_u32(dev->of_node, "reg", &index)) >>>>> + return -EINVAL; >>>>> + >>>>> + if (index >= ARRAY_SIZE(triggers_table)) >>>>> + return -EINVAL; >>>>> + >>>>> + /* Create an IIO device only if we have triggers to be validated */ >>>>> + if (*valids_table[index]) >>>>> + priv = stm32_setup_iio_device(dev); >>>> >>>> I still don't like this. Really feels like we shouldn't be creating an >>>> iio device with all the bagage that carries just to allow us to do the >>>> trigger trees. We ought to have a much more light weight solution for this >>>> functionality - we aren't typically even using the interrupt tree stuff >>>> that the triggers for devices are all really about. >>>> >>>> A simpler approach of allowing each trigger the option of a parent seems like >>>> it would be cleaner. Could be done entirely within this driver in the first >>>> instance. Basically it would just look like your master and slave attributes >>>> but have those under triggerX not iio:deviceX. >>>> >>>> We can work out how to make it more generic later - including perhaps the >>>> option to trigger from triggers outside this driver, using some parallel >>>> infrastructure to the device triggering. >>>> >>>> >>>>> + else >>>>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >>>>> + >>>>> + if (!priv) >>>>> + return -ENOMEM; >>>>> + >>>>> + priv->dev = dev; >>>>> + priv->regmap = ddata->regmap; >>>>> + priv->clk = ddata->clk; >>>>> + priv->max_arr = ddata->max_arr; >>>>> + priv->triggers = triggers_table[index]; >>>>> + priv->valids = valids_table[index]; >>>>> + >>>>> + ret = stm32_setup_iio_triggers(priv); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + platform_set_drvdata(pdev, priv); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static const struct of_device_id stm32_trig_of_match[] = { >>>>> + { .compatible = "st,stm32-timer-trigger", }, >>>>> + { /* end node */ }, >>>>> +}; >>>>> +MODULE_DEVICE_TABLE(of, stm32_trig_of_match); >>>>> + >>>>> +static struct platform_driver stm32_timer_trigger_driver = { >>>>> + .probe = stm32_timer_trigger_probe, >>>>> + .driver = { >>>>> + .name = "stm32-timer-trigger", >>>>> + .of_match_table = stm32_trig_of_match, >>>>> + }, >>>>> +}; >>>>> +module_platform_driver(stm32_timer_trigger_driver); >>>>> + >>>>> +MODULE_ALIAS("platform: stm32-timer-trigger"); >>>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 Timer Trigger driver"); >>>>> +MODULE_LICENSE("GPL v2"); >>>>> diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig >>>>> index 809b2e7..f2af4fe 100644 >>>>> --- a/drivers/iio/trigger/Kconfig >>>>> +++ b/drivers/iio/trigger/Kconfig >>>>> @@ -46,5 +46,4 @@ config IIO_SYSFS_TRIGGER >>>>> >>>>> To compile this driver as a module, choose M here: the >>>>> module will be called iio-trig-sysfs. >>>>> - >>>> Clean this up. >>> >>> ok >>> >>>>> endmenu >>>>> diff --git a/include/linux/iio/timer/stm32-timer-trigger.h b/include/linux/iio/timer/stm32-timer-trigger.h >>>>> new file mode 100644 >>>>> index 0000000..55535ae >>>>> --- /dev/null >>>>> +++ b/include/linux/iio/timer/stm32-timer-trigger.h >>>>> @@ -0,0 +1,62 @@ >>>>> +/* >>>>> + * Copyright (C) STMicroelectronics 2016 >>>>> + * >>>>> + * Author: Benjamin Gaignard <benjamin.gaignard@xxxxxx> >>>>> + * >>>>> + * License terms: GNU General Public License (GPL), version 2 >>>>> + */ >>>>> + >>>>> +#ifndef _STM32_TIMER_TRIGGER_H_ >>>>> +#define _STM32_TIMER_TRIGGER_H_ >>>>> + >>>>> +#define TIM1_TRGO "tim1_trgo" >>>>> +#define TIM1_CH1 "tim1_ch1" >>>>> +#define TIM1_CH2 "tim1_ch2" >>>>> +#define TIM1_CH3 "tim1_ch3" >>>>> +#define TIM1_CH4 "tim1_ch4" >>>>> + >>>>> +#define TIM2_TRGO "tim2_trgo" >>>>> +#define TIM2_CH1 "tim2_ch1" >>>>> +#define TIM2_CH2 "tim2_ch2" >>>>> +#define TIM2_CH3 "tim2_ch3" >>>>> +#define TIM2_CH4 "tim2_ch4" >>>>> + >>>>> +#define TIM3_TRGO "tim3_trgo" >>>>> +#define TIM3_CH1 "tim3_ch1" >>>>> +#define TIM3_CH2 "tim3_ch2" >>>>> +#define TIM3_CH3 "tim3_ch3" >>>>> +#define TIM3_CH4 "tim3_ch4" >>>>> + >>>>> +#define TIM4_TRGO "tim4_trgo" >>>>> +#define TIM4_CH1 "tim4_ch1" >>>>> +#define TIM4_CH2 "tim4_ch2" >>>>> +#define TIM4_CH3 "tim4_ch3" >>>>> +#define TIM4_CH4 "tim4_ch4" >>>>> + >>>>> +#define TIM5_TRGO "tim5_trgo" >>>>> +#define TIM5_CH1 "tim5_ch1" >>>>> +#define TIM5_CH2 "tim5_ch2" >>>>> +#define TIM5_CH3 "tim5_ch3" >>>>> +#define TIM5_CH4 "tim5_ch4" >>>>> + >>>>> +#define TIM6_TRGO "tim6_trgo" >>>>> + >>>>> +#define TIM7_TRGO "tim7_trgo" >>>>> + >>>>> +#define TIM8_TRGO "tim8_trgo" >>>>> +#define TIM8_CH1 "tim8_ch1" >>>>> +#define TIM8_CH2 "tim8_ch2" >>>>> +#define TIM8_CH3 "tim8_ch3" >>>>> +#define TIM8_CH4 "tim8_ch4" >>>>> + >>>>> +#define TIM9_TRGO "tim9_trgo" >>>>> +#define TIM9_CH1 "tim9_ch1" >>>>> +#define TIM9_CH2 "tim9_ch2" >>>>> + >>>>> +#define TIM12_TRGO "tim12_trgo" >>>>> +#define TIM12_CH1 "tim12_ch1" >>>>> +#define TIM12_CH2 "tim12_ch2" >>>>> + >>>>> +bool is_stm32_timer_trigger(struct iio_trigger *trig); >>>>> + >>>>> +#endif >>>>> >>>> >>> >>> >>> >> > > > > -- > Benjamin Gaignard > > Graphic Study Group > > Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: Facebook | Twitter | Blog -- Benjamin Gaignard Graphic Study Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html