Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-28 Thread Jonathan Cameron 
On 24/01/17 14:37, Fabrice Gasnier wrote:
> On 01/22/2017 01:55 PM, Jonathan Cameron wrote:
>> On 19/01/17 13:34, Fabrice Gasnier wrote:
>>> STM32 ADC has external timer trigger sources. Use stm32 timer triggers
>>> API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
>>> validate a trigger can be used.
>>> This also provides correct trigger selection value (e.g. extsel).
>>>
>>> Signed-off-by: Fabrice Gasnier 
>> Looks good. Observations inline.
>>> ---
>>>  drivers/iio/adc/Kconfig |  2 ++
>>>  drivers/iio/adc/stm32-adc.c | 60 
>>> +
>>>  2 files changed, 62 insertions(+)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 33341f4..9a7b090 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -447,6 +447,8 @@ config STM32_ADC_CORE
>>>  depends on OF
>>>  depends on REGULATOR
>>>  select IIO_BUFFER
>>> +select MFD_STM32_TIMERS
>>> +select IIO_STM32_TIMER_TRIGGER
>>>  select IIO_TRIGGERED_BUFFER
>>>  help
>>>Select this option to enable the core driver for STMicroelectronics
>>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>>> index 8d0b74b..30708bc 100644
>>> --- a/drivers/iio/adc/stm32-adc.c
>>> +++ b/drivers/iio/adc/stm32-adc.c
>>> @@ -23,6 +23,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -122,6 +123,35 @@ enum stm32_adc_exten {
>>>  STM32_EXTEN_HWTRIG_BOTH_EDGES,
>>>  };
>>>
>>> +/* extsel - trigger mux selection value */
>>> +enum stm32_adc_extsel {
>>> +STM32_EXT0,
>>> +STM32_EXT1,
>>> +STM32_EXT2,
>>> +STM32_EXT3,
>>> +STM32_EXT4,
>>> +STM32_EXT5,
>>> +STM32_EXT6,
>>> +STM32_EXT7,
>>> +STM32_EXT8,
>>> +STM32_EXT9,
>>> +STM32_EXT10,
>>> +STM32_EXT11,
>>> +STM32_EXT12,
>>> +STM32_EXT13,
>>> +STM32_EXT14,
>>> +STM32_EXT15,
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_trig_info - ADC trigger info
>>> + * @name:name of the trigger, corresponding to its source
>>> + * @extsel:trigger selection
>>> + */
>>> +struct stm32_adc_trig_info {
>>> +const char *name;
>>> +enum stm32_adc_extsel extsel;
>>> +};
>>>
>>>  /**
>>>   * struct stm32_adc - private data of each ADC IIO instance
>>> @@ -218,6 +248,26 @@ struct stm32_adc_regs {
>>>  { STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
>>>  };
>>>
>>> +/* STM32F4 external trigger sources for all instances */
>>> +static struct stm32_adc_trig_info stm32f4_adc_timer_trigs[] = {
>>> +{ TIM1_CH1, STM32_EXT0 },
>>> +{ TIM1_CH2, STM32_EXT1 },
>>> +{ TIM1_CH3, STM32_EXT2 },
>>> +{ TIM2_CH2, STM32_EXT3 },
>>> +{ TIM2_CH3, STM32_EXT4 },
>>> +{ TIM2_CH4, STM32_EXT5 },
>>> +{ TIM2_TRGO, STM32_EXT6 },
>>> +{ TIM3_CH1, STM32_EXT7 },
>>> +{ TIM3_TRGO, STM32_EXT8 },
>>> +{ TIM4_CH4, STM32_EXT9 },
>>> +{ TIM5_CH1, STM32_EXT10 },
>>> +{ TIM5_CH2, STM32_EXT11 },
>>> +{ TIM5_CH3, STM32_EXT12 },
>>> +{ TIM8_CH1, STM32_EXT13 },
>>> +{ TIM8_TRGO, STM32_EXT14 },
>>> +{}, /* sentinel */
>>> +};
>>> +
>>>  /**
>>>   * STM32 ADC registers access routines
>>>   * @adc: stm32 adc instance
>>> @@ -362,6 +412,16 @@ static int stm32_adc_conf_scan_seq(struct iio_dev 
>>> *indio_dev,
>>>  static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
>>>   struct iio_trigger *trig)
>>>  {
>>> +int i;
>> Ah. This makes more sense than patch 1 on it's own did.
>>> +
>>> +/* lookup triggers registered by stm32 timer trigger driver */
>>> +for (i = 0; stm32f4_adc_timer_trigs[i].name; i++) {
>>> +if (is_stm32_timer_trigger(trig) &&
>>> +!strcmp(stm32f4_adc_timer_trigs[i].name, trig->name)) {
>>> +return stm32f4_adc_timer_trigs[i].extsel;
>> Good. The combination of the first check and the name match should make this 
>> safe
>> against those triggers that can be assigned arbitrary names.
> 
> Do you wish I add a comment about it ?
Nope. I just thought it was nicely done ;)
> 
> Best Regards,
> Fabrice
> 
>>> +}
>>> +}
>>> +
>>>  return -EINVAL;
>>>  }
>>>
>>>
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-28 Thread Jonathan Cameron 
On 24/01/17 14:37, Fabrice Gasnier wrote:
> On 01/22/2017 01:55 PM, Jonathan Cameron wrote:
>> On 19/01/17 13:34, Fabrice Gasnier wrote:
>>> STM32 ADC has external timer trigger sources. Use stm32 timer triggers
>>> API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
>>> validate a trigger can be used.
>>> This also provides correct trigger selection value (e.g. extsel).
>>>
>>> Signed-off-by: Fabrice Gasnier 
>> Looks good. Observations inline.
>>> ---
>>>  drivers/iio/adc/Kconfig |  2 ++
>>>  drivers/iio/adc/stm32-adc.c | 60 
>>> +
>>>  2 files changed, 62 insertions(+)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 33341f4..9a7b090 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -447,6 +447,8 @@ config STM32_ADC_CORE
>>>  depends on OF
>>>  depends on REGULATOR
>>>  select IIO_BUFFER
>>> +select MFD_STM32_TIMERS
>>> +select IIO_STM32_TIMER_TRIGGER
>>>  select IIO_TRIGGERED_BUFFER
>>>  help
>>>Select this option to enable the core driver for STMicroelectronics
>>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>>> index 8d0b74b..30708bc 100644
>>> --- a/drivers/iio/adc/stm32-adc.c
>>> +++ b/drivers/iio/adc/stm32-adc.c
>>> @@ -23,6 +23,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -122,6 +123,35 @@ enum stm32_adc_exten {
>>>  STM32_EXTEN_HWTRIG_BOTH_EDGES,
>>>  };
>>>
>>> +/* extsel - trigger mux selection value */
>>> +enum stm32_adc_extsel {
>>> +STM32_EXT0,
>>> +STM32_EXT1,
>>> +STM32_EXT2,
>>> +STM32_EXT3,
>>> +STM32_EXT4,
>>> +STM32_EXT5,
>>> +STM32_EXT6,
>>> +STM32_EXT7,
>>> +STM32_EXT8,
>>> +STM32_EXT9,
>>> +STM32_EXT10,
>>> +STM32_EXT11,
>>> +STM32_EXT12,
>>> +STM32_EXT13,
>>> +STM32_EXT14,
>>> +STM32_EXT15,
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_trig_info - ADC trigger info
>>> + * @name:name of the trigger, corresponding to its source
>>> + * @extsel:trigger selection
>>> + */
>>> +struct stm32_adc_trig_info {
>>> +const char *name;
>>> +enum stm32_adc_extsel extsel;
>>> +};
>>>
>>>  /**
>>>   * struct stm32_adc - private data of each ADC IIO instance
>>> @@ -218,6 +248,26 @@ struct stm32_adc_regs {
>>>  { STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
>>>  };
>>>
>>> +/* STM32F4 external trigger sources for all instances */
>>> +static struct stm32_adc_trig_info stm32f4_adc_timer_trigs[] = {
>>> +{ TIM1_CH1, STM32_EXT0 },
>>> +{ TIM1_CH2, STM32_EXT1 },
>>> +{ TIM1_CH3, STM32_EXT2 },
>>> +{ TIM2_CH2, STM32_EXT3 },
>>> +{ TIM2_CH3, STM32_EXT4 },
>>> +{ TIM2_CH4, STM32_EXT5 },
>>> +{ TIM2_TRGO, STM32_EXT6 },
>>> +{ TIM3_CH1, STM32_EXT7 },
>>> +{ TIM3_TRGO, STM32_EXT8 },
>>> +{ TIM4_CH4, STM32_EXT9 },
>>> +{ TIM5_CH1, STM32_EXT10 },
>>> +{ TIM5_CH2, STM32_EXT11 },
>>> +{ TIM5_CH3, STM32_EXT12 },
>>> +{ TIM8_CH1, STM32_EXT13 },
>>> +{ TIM8_TRGO, STM32_EXT14 },
>>> +{}, /* sentinel */
>>> +};
>>> +
>>>  /**
>>>   * STM32 ADC registers access routines
>>>   * @adc: stm32 adc instance
>>> @@ -362,6 +412,16 @@ static int stm32_adc_conf_scan_seq(struct iio_dev 
>>> *indio_dev,
>>>  static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
>>>   struct iio_trigger *trig)
>>>  {
>>> +int i;
>> Ah. This makes more sense than patch 1 on it's own did.
>>> +
>>> +/* lookup triggers registered by stm32 timer trigger driver */
>>> +for (i = 0; stm32f4_adc_timer_trigs[i].name; i++) {
>>> +if (is_stm32_timer_trigger(trig) &&
>>> +!strcmp(stm32f4_adc_timer_trigs[i].name, trig->name)) {
>>> +return stm32f4_adc_timer_trigs[i].extsel;
>> Good. The combination of the first check and the name match should make this 
>> safe
>> against those triggers that can be assigned arbitrary names.
> 
> Do you wish I add a comment about it ?
Nope. I just thought it was nicely done ;)
> 
> Best Regards,
> Fabrice
> 
>>> +}
>>> +}
>>> +
>>>  return -EINVAL;
>>>  }
>>>
>>>
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-24 Thread Fabrice Gasnier 

On 01/22/2017 01:55 PM, Jonathan Cameron wrote:

On 19/01/17 13:34, Fabrice Gasnier wrote:

STM32 ADC has external timer trigger sources. Use stm32 timer triggers
API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
validate a trigger can be used.
This also provides correct trigger selection value (e.g. extsel).

Signed-off-by: Fabrice Gasnier 

Looks good. Observations inline.

---
 drivers/iio/adc/Kconfig |  2 ++
 drivers/iio/adc/stm32-adc.c | 60 +
 2 files changed, 62 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 33341f4..9a7b090 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -447,6 +447,8 @@ config STM32_ADC_CORE
depends on OF
depends on REGULATOR
select IIO_BUFFER
+   select MFD_STM32_TIMERS
+   select IIO_STM32_TIMER_TRIGGER
select IIO_TRIGGERED_BUFFER
help
  Select this option to enable the core driver for STMicroelectronics
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 8d0b74b..30708bc 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -122,6 +123,35 @@ enum stm32_adc_exten {
STM32_EXTEN_HWTRIG_BOTH_EDGES,
 };

+/* extsel - trigger mux selection value */
+enum stm32_adc_extsel {
+   STM32_EXT0,
+   STM32_EXT1,
+   STM32_EXT2,
+   STM32_EXT3,
+   STM32_EXT4,
+   STM32_EXT5,
+   STM32_EXT6,
+   STM32_EXT7,
+   STM32_EXT8,
+   STM32_EXT9,
+   STM32_EXT10,
+   STM32_EXT11,
+   STM32_EXT12,
+   STM32_EXT13,
+   STM32_EXT14,
+   STM32_EXT15,
+};
+
+/**
+ * struct stm32_adc_trig_info - ADC trigger info
+ * @name:  name of the trigger, corresponding to its source
+ * @extsel:trigger selection
+ */
+struct stm32_adc_trig_info {
+   const char *name;
+   enum stm32_adc_extsel extsel;
+};

 /**
  * struct stm32_adc - private data of each ADC IIO instance
@@ -218,6 +248,26 @@ struct stm32_adc_regs {
{ STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
 };

+/* STM32F4 external trigger sources for all instances */
+static struct stm32_adc_trig_info stm32f4_adc_timer_trigs[] = {
+   { TIM1_CH1, STM32_EXT0 },
+   { TIM1_CH2, STM32_EXT1 },
+   { TIM1_CH3, STM32_EXT2 },
+   { TIM2_CH2, STM32_EXT3 },
+   { TIM2_CH3, STM32_EXT4 },
+   { TIM2_CH4, STM32_EXT5 },
+   { TIM2_TRGO, STM32_EXT6 },
+   { TIM3_CH1, STM32_EXT7 },
+   { TIM3_TRGO, STM32_EXT8 },
+   { TIM4_CH4, STM32_EXT9 },
+   { TIM5_CH1, STM32_EXT10 },
+   { TIM5_CH2, STM32_EXT11 },
+   { TIM5_CH3, STM32_EXT12 },
+   { TIM8_CH1, STM32_EXT13 },
+   { TIM8_TRGO, STM32_EXT14 },
+   {}, /* sentinel */
+};
+
 /**
  * STM32 ADC registers access routines
  * @adc: stm32 adc instance
@@ -362,6 +412,16 @@ static int stm32_adc_conf_scan_seq(struct iio_dev 
*indio_dev,
 static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
 struct iio_trigger *trig)
 {
+   int i;

Ah. This makes more sense than patch 1 on it's own did.

+
+   /* lookup triggers registered by stm32 timer trigger driver */
+   for (i = 0; stm32f4_adc_timer_trigs[i].name; i++) {
+   if (is_stm32_timer_trigger(trig) &&
+   !strcmp(stm32f4_adc_timer_trigs[i].name, trig->name)) {
+   return stm32f4_adc_timer_trigs[i].extsel;

Good. The combination of the first check and the name match should make this 
safe
against those triggers that can be assigned arbitrary names.


Do you wish I add a comment about it ?

Best Regards,
Fabrice


+   }
+   }
+
return -EINVAL;
 }

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-24 Thread Fabrice Gasnier 

On 01/22/2017 01:55 PM, Jonathan Cameron wrote:

On 19/01/17 13:34, Fabrice Gasnier wrote:

STM32 ADC has external timer trigger sources. Use stm32 timer triggers
API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
validate a trigger can be used.
This also provides correct trigger selection value (e.g. extsel).

Signed-off-by: Fabrice Gasnier 

Looks good. Observations inline.

---
 drivers/iio/adc/Kconfig |  2 ++
 drivers/iio/adc/stm32-adc.c | 60 +
 2 files changed, 62 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 33341f4..9a7b090 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -447,6 +447,8 @@ config STM32_ADC_CORE
depends on OF
depends on REGULATOR
select IIO_BUFFER
+   select MFD_STM32_TIMERS
+   select IIO_STM32_TIMER_TRIGGER
select IIO_TRIGGERED_BUFFER
help
  Select this option to enable the core driver for STMicroelectronics
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 8d0b74b..30708bc 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -122,6 +123,35 @@ enum stm32_adc_exten {
STM32_EXTEN_HWTRIG_BOTH_EDGES,
 };

+/* extsel - trigger mux selection value */
+enum stm32_adc_extsel {
+   STM32_EXT0,
+   STM32_EXT1,
+   STM32_EXT2,
+   STM32_EXT3,
+   STM32_EXT4,
+   STM32_EXT5,
+   STM32_EXT6,
+   STM32_EXT7,
+   STM32_EXT8,
+   STM32_EXT9,
+   STM32_EXT10,
+   STM32_EXT11,
+   STM32_EXT12,
+   STM32_EXT13,
+   STM32_EXT14,
+   STM32_EXT15,
+};
+
+/**
+ * struct stm32_adc_trig_info - ADC trigger info
+ * @name:  name of the trigger, corresponding to its source
+ * @extsel:trigger selection
+ */
+struct stm32_adc_trig_info {
+   const char *name;
+   enum stm32_adc_extsel extsel;
+};

 /**
  * struct stm32_adc - private data of each ADC IIO instance
@@ -218,6 +248,26 @@ struct stm32_adc_regs {
{ STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
 };

+/* STM32F4 external trigger sources for all instances */
+static struct stm32_adc_trig_info stm32f4_adc_timer_trigs[] = {
+   { TIM1_CH1, STM32_EXT0 },
+   { TIM1_CH2, STM32_EXT1 },
+   { TIM1_CH3, STM32_EXT2 },
+   { TIM2_CH2, STM32_EXT3 },
+   { TIM2_CH3, STM32_EXT4 },
+   { TIM2_CH4, STM32_EXT5 },
+   { TIM2_TRGO, STM32_EXT6 },
+   { TIM3_CH1, STM32_EXT7 },
+   { TIM3_TRGO, STM32_EXT8 },
+   { TIM4_CH4, STM32_EXT9 },
+   { TIM5_CH1, STM32_EXT10 },
+   { TIM5_CH2, STM32_EXT11 },
+   { TIM5_CH3, STM32_EXT12 },
+   { TIM8_CH1, STM32_EXT13 },
+   { TIM8_TRGO, STM32_EXT14 },
+   {}, /* sentinel */
+};
+
 /**
  * STM32 ADC registers access routines
  * @adc: stm32 adc instance
@@ -362,6 +412,16 @@ static int stm32_adc_conf_scan_seq(struct iio_dev 
*indio_dev,
 static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
 struct iio_trigger *trig)
 {
+   int i;

Ah. This makes more sense than patch 1 on it's own did.

+
+   /* lookup triggers registered by stm32 timer trigger driver */
+   for (i = 0; stm32f4_adc_timer_trigs[i].name; i++) {
+   if (is_stm32_timer_trigger(trig) &&
+   !strcmp(stm32f4_adc_timer_trigs[i].name, trig->name)) {
+   return stm32f4_adc_timer_trigs[i].extsel;

Good. The combination of the first check and the name match should make this 
safe
against those triggers that can be assigned arbitrary names.


Do you wish I add a comment about it ?

Best Regards,
Fabrice


+   }
+   }
+
return -EINVAL;
 }

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-22 Thread Jonathan Cameron 
On 19/01/17 13:34, Fabrice Gasnier wrote:
> STM32 ADC has external timer trigger sources. Use stm32 timer triggers
> API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
> validate a trigger can be used.
> This also provides correct trigger selection value (e.g. extsel).
> 
> Signed-off-by: Fabrice Gasnier 
Looks good. Observations inline.
> ---
>  drivers/iio/adc/Kconfig |  2 ++
>  drivers/iio/adc/stm32-adc.c | 60 
> +
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 33341f4..9a7b090 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -447,6 +447,8 @@ config STM32_ADC_CORE
>   depends on OF
>   depends on REGULATOR
>   select IIO_BUFFER
> + select MFD_STM32_TIMERS
> + select IIO_STM32_TIMER_TRIGGER
>   select IIO_TRIGGERED_BUFFER
>   help
> Select this option to enable the core driver for STMicroelectronics
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 8d0b74b..30708bc 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -122,6 +123,35 @@ enum stm32_adc_exten {
>   STM32_EXTEN_HWTRIG_BOTH_EDGES,
>  };
>  
> +/* extsel - trigger mux selection value */
> +enum stm32_adc_extsel {
> + STM32_EXT0,
> + STM32_EXT1,
> + STM32_EXT2,
> + STM32_EXT3,
> + STM32_EXT4,
> + STM32_EXT5,
> + STM32_EXT6,
> + STM32_EXT7,
> + STM32_EXT8,
> + STM32_EXT9,
> + STM32_EXT10,
> + STM32_EXT11,
> + STM32_EXT12,
> + STM32_EXT13,
> + STM32_EXT14,
> + STM32_EXT15,
> +};
> +
> +/**
> + * struct stm32_adc_trig_info - ADC trigger info
> + * @name:name of the trigger, corresponding to its source
> + * @extsel:  trigger selection
> + */
> +struct stm32_adc_trig_info {
> + const char *name;
> + enum stm32_adc_extsel extsel;
> +};
>  
>  /**
>   * struct stm32_adc - private data of each ADC IIO instance
> @@ -218,6 +248,26 @@ struct stm32_adc_regs {
>   { STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
>  };
>  
> +/* STM32F4 external trigger sources for all instances */
> +static struct stm32_adc_trig_info stm32f4_adc_timer_trigs[] = {
> + { TIM1_CH1, STM32_EXT0 },
> + { TIM1_CH2, STM32_EXT1 },
> + { TIM1_CH3, STM32_EXT2 },
> + { TIM2_CH2, STM32_EXT3 },
> + { TIM2_CH3, STM32_EXT4 },
> + { TIM2_CH4, STM32_EXT5 },
> + { TIM2_TRGO, STM32_EXT6 },
> + { TIM3_CH1, STM32_EXT7 },
> + { TIM3_TRGO, STM32_EXT8 },
> + { TIM4_CH4, STM32_EXT9 },
> + { TIM5_CH1, STM32_EXT10 },
> + { TIM5_CH2, STM32_EXT11 },
> + { TIM5_CH3, STM32_EXT12 },
> + { TIM8_CH1, STM32_EXT13 },
> + { TIM8_TRGO, STM32_EXT14 },
> + {}, /* sentinel */
> +};
> +
>  /**
>   * STM32 ADC registers access routines
>   * @adc: stm32 adc instance
> @@ -362,6 +412,16 @@ static int stm32_adc_conf_scan_seq(struct iio_dev 
> *indio_dev,
>  static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
>struct iio_trigger *trig)
>  {
> + int i;
Ah. This makes more sense than patch 1 on it's own did.
> +
> + /* lookup triggers registered by stm32 timer trigger driver */
> + for (i = 0; stm32f4_adc_timer_trigs[i].name; i++) {
> + if (is_stm32_timer_trigger(trig) &&
> + !strcmp(stm32f4_adc_timer_trigs[i].name, trig->name)) {
> + return stm32f4_adc_timer_trigs[i].extsel;
Good. The combination of the first check and the name match should make this 
safe
against those triggers that can be assigned arbitrary names.
> + }
> + }
> +
>   return -EINVAL;
>  }
>  
>

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-22 Thread Jonathan Cameron 
On 19/01/17 13:34, Fabrice Gasnier wrote:
> STM32 ADC has external timer trigger sources. Use stm32 timer triggers
> API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
> validate a trigger can be used.
> This also provides correct trigger selection value (e.g. extsel).
> 
> Signed-off-by: Fabrice Gasnier 
Looks good. Observations inline.
> ---
>  drivers/iio/adc/Kconfig |  2 ++
>  drivers/iio/adc/stm32-adc.c | 60 
> +
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 33341f4..9a7b090 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -447,6 +447,8 @@ config STM32_ADC_CORE
>   depends on OF
>   depends on REGULATOR
>   select IIO_BUFFER
> + select MFD_STM32_TIMERS
> + select IIO_STM32_TIMER_TRIGGER
>   select IIO_TRIGGERED_BUFFER
>   help
> Select this option to enable the core driver for STMicroelectronics
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 8d0b74b..30708bc 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -122,6 +123,35 @@ enum stm32_adc_exten {
>   STM32_EXTEN_HWTRIG_BOTH_EDGES,
>  };
>  
> +/* extsel - trigger mux selection value */
> +enum stm32_adc_extsel {
> + STM32_EXT0,
> + STM32_EXT1,
> + STM32_EXT2,
> + STM32_EXT3,
> + STM32_EXT4,
> + STM32_EXT5,
> + STM32_EXT6,
> + STM32_EXT7,
> + STM32_EXT8,
> + STM32_EXT9,
> + STM32_EXT10,
> + STM32_EXT11,
> + STM32_EXT12,
> + STM32_EXT13,
> + STM32_EXT14,
> + STM32_EXT15,
> +};
> +
> +/**
> + * struct stm32_adc_trig_info - ADC trigger info
> + * @name:name of the trigger, corresponding to its source
> + * @extsel:  trigger selection
> + */
> +struct stm32_adc_trig_info {
> + const char *name;
> + enum stm32_adc_extsel extsel;
> +};
>  
>  /**
>   * struct stm32_adc - private data of each ADC IIO instance
> @@ -218,6 +248,26 @@ struct stm32_adc_regs {
>   { STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
>  };
>  
> +/* STM32F4 external trigger sources for all instances */
> +static struct stm32_adc_trig_info stm32f4_adc_timer_trigs[] = {
> + { TIM1_CH1, STM32_EXT0 },
> + { TIM1_CH2, STM32_EXT1 },
> + { TIM1_CH3, STM32_EXT2 },
> + { TIM2_CH2, STM32_EXT3 },
> + { TIM2_CH3, STM32_EXT4 },
> + { TIM2_CH4, STM32_EXT5 },
> + { TIM2_TRGO, STM32_EXT6 },
> + { TIM3_CH1, STM32_EXT7 },
> + { TIM3_TRGO, STM32_EXT8 },
> + { TIM4_CH4, STM32_EXT9 },
> + { TIM5_CH1, STM32_EXT10 },
> + { TIM5_CH2, STM32_EXT11 },
> + { TIM5_CH3, STM32_EXT12 },
> + { TIM8_CH1, STM32_EXT13 },
> + { TIM8_TRGO, STM32_EXT14 },
> + {}, /* sentinel */
> +};
> +
>  /**
>   * STM32 ADC registers access routines
>   * @adc: stm32 adc instance
> @@ -362,6 +412,16 @@ static int stm32_adc_conf_scan_seq(struct iio_dev 
> *indio_dev,
>  static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
>struct iio_trigger *trig)
>  {
> + int i;
Ah. This makes more sense than patch 1 on it's own did.
> +
> + /* lookup triggers registered by stm32 timer trigger driver */
> + for (i = 0; stm32f4_adc_timer_trigs[i].name; i++) {
> + if (is_stm32_timer_trigger(trig) &&
> + !strcmp(stm32f4_adc_timer_trigs[i].name, trig->name)) {
> + return stm32f4_adc_timer_trigs[i].extsel;
Good. The combination of the first check and the name match should make this 
safe
against those triggers that can be assigned arbitrary names.
> + }
> + }
> +
>   return -EINVAL;
>  }
>  
>

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-21 Thread Jonathan Cameron 
On 19/01/17 23:31, kbuild test robot wrote:
> Hi Fabrice,
> 
> [auto build test ERROR on next-20170119]
> [also build test ERROR on v4.10-rc4]
> [cannot apply to iio/togreg robh/for-next v4.9-rc8 v4.9-rc7 v4.9-rc6]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Fabrice-Gasnier/Add-support-for-triggered-buffer-mode-to-STM32-ADC/20170120-062214
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
There is a precursor patch set mentioned in the cover letter. Not sure there is
a sensible automated way to deal with that!

Jonathan
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/iio/adc/stm32-adc.c:26:49: fatal error: 
>>> linux/iio/timer/stm32-timer-trigger.h: No such file or directory
> #include 
> ^
>compilation terminated.
> 
> vim +26 drivers/iio/adc/stm32-adc.c
> 
> 20 */
> 21
> 22#include 
> 23#include 
> 24#include 
> 25#include 
>   > 26#include 
> 27#include 
> 28#include 
> 29#include 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
>

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-21 Thread Jonathan Cameron 
On 19/01/17 23:31, kbuild test robot wrote:
> Hi Fabrice,
> 
> [auto build test ERROR on next-20170119]
> [also build test ERROR on v4.10-rc4]
> [cannot apply to iio/togreg robh/for-next v4.9-rc8 v4.9-rc7 v4.9-rc6]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Fabrice-Gasnier/Add-support-for-triggered-buffer-mode-to-STM32-ADC/20170120-062214
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
There is a precursor patch set mentioned in the cover letter. Not sure there is
a sensible automated way to deal with that!

Jonathan
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/iio/adc/stm32-adc.c:26:49: fatal error: 
>>> linux/iio/timer/stm32-timer-trigger.h: No such file or directory
> #include 
> ^
>compilation terminated.
> 
> vim +26 drivers/iio/adc/stm32-adc.c
> 
> 20 */
> 21
> 22#include 
> 23#include 
> 24#include 
> 25#include 
>   > 26#include 
> 27#include 
> 28#include 
> 29#include 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
>

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-19 Thread kbuild test robot 
Hi Fabrice,

[auto build test ERROR on next-20170119]
[also build test ERROR on v4.10-rc4]
[cannot apply to iio/togreg robh/for-next v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Fabrice-Gasnier/Add-support-for-triggered-buffer-mode-to-STM32-ADC/20170120-062214
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/iio/adc/stm32-adc.c:26:49: fatal error: 
>> linux/iio/timer/stm32-timer-trigger.h: No such file or directory
#include 
^
   compilation terminated.

vim +26 drivers/iio/adc/stm32-adc.c

20   */
21  
22  #include 
23  #include 
24  #include 
25  #include 
  > 26  #include 
27  #include 
28  #include 
29  #include 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers

2017-01-19 Thread kbuild test robot 
Hi Fabrice,

[auto build test ERROR on next-20170119]
[also build test ERROR on v4.10-rc4]
[cannot apply to iio/togreg robh/for-next v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Fabrice-Gasnier/Add-support-for-triggered-buffer-mode-to-STM32-ADC/20170120-062214
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/iio/adc/stm32-adc.c:26:49: fatal error: 
>> linux/iio/timer/stm32-timer-trigger.h: No such file or directory
#include 
^
   compilation terminated.

vim +26 drivers/iio/adc/stm32-adc.c

20   */
21  
22  #include 
23  #include 
24  #include 
25  #include 
  > 26  #include 
27  #include 
28  #include 
29  #include 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip