ICU and timer channel?

[utzig]

I'm also trying to use the ICU driver and have a strange problem. The width is always the same as the period!

I backported the ICU driver to the 2.3.7 version which I'm using and I guess everything was done correctly. I checked that width is read from CCR1 and period from CCR2. I'm using TIM4_CH1 as input but I'm really quite confused by the documentation that mentions CH1 and CH2 as inputs to read both the low and high logic levels and always when I think of CH2 I think of an extra input pin (TIM4_CH2 in this case). That was my main doubt regarding the hardware implementation but from this thread it seams to be necessary to connect only one input pin.

I also checked that the value read matches the "width" of the input signal on the pin. So anyone have any idea what's happening to read both width and period with the same value?

[Giovanni]

Hi,

Only one input is used: CH1. This input is internally router to capture registers 1 and 2 that are used for measuring the two edges of the signal.

In the unstable version there is a PWM+ICU test application, the PWM generates a signal and the ICU measures it, can you try to run the test application?

Giovanni

[utzig]

Hi,

Just tried your suggestion and it worked correctly! At least now I know that the ICU driver backport I did is correct.

I will rebuild the original project and try to discover why it gave that strange results.

It took me sometime to "backport" the test code since some things had changed on the unstable version (PWMConfig, etc). Do you plan to release a new "stable" series based on the current unstable soon? Or do you suggest going with the unstable version?

[Giovanni]

Hi,

We hope to have a stable 2.4.0 based on the current unstable sometime between January and February, we are just finalizing details. I recommend to develop using the current unstable version, this way you will be compatible with the new stable.

Giovanni

[utzig]

Guys,

Just want to leave a follow-up here with things that might be useful to somebody else.

Last Tuesday I upgraded to the latest unstable version. After that I found out the original problem for me which was getting widths and periods with the same value. It was a quite stupid thing actually! I was using the timer frequency as in the example but was working with higher frequencies (in the KHz) so I didn't have precision enough on my timer. I just increased it to 1MHz.

I made some wireup on the circuit board I was working on to connect the input signal to TIM4_CH1 to make it compatible with the OS without changes. But on the original schematics this input was connected to TIM4_CH2. Today I had to struggle to make input capture work on CH2. I'd like to give some suggestions which might be helpful.

The first thing here is terminology. In the OS code you read:

os/hal/platforms/STM32/icu_lld.c [362] (SVN)

Code: Select all

/* CCMR1_CC1S = 01 = CH1 Input on TI1.
   CCMR1_CC2S = 10 = CH2 Input on TI1.*/


When I read CH1 and CH2 I always think of pin inputs. For me a better terminology would be to use TIx as used on the datasheets. On my code I used the following comments which at least for me look more clear.

Code: Select all

/* CCMR1_CC1S = 10 = IC1 is mapped on TI2.
   CCMR1_CC2S = 01 = IC2 is mapped on TI2. */
   ICUD4.tim->CCMR1 = TIM_CCMR1_CC1S_1 | TIM_CCMR1_CC2S_0;


Next thing is to change the trigger for TI2FP2:

Code: Select all

/* SMCR_TS  = 110, trigger on input TI2FP2.
   SMCR_SMS = 100, reset on rising edge. */
   ICUD4.tim->SMCR  = TIM_SMCR_TS_2 | TIM_SMCR_TS_1 | TIM_SMCR_SMS_2;


And then comes another catch! The example code given by Badger will actually not work because you must clear CCER before writing do CCMR1. This is indicated by this note in the datasheet:

Note: CC1S bits are writable only when the channel is OFF (CC1E = 0 in TIMx_CCER).

I guess it would not hurt to put a comment mentioning this like I did on my code as seen below!

So my final code looks like this:

Code: Select all

        icuStart(&ICUD4, &radio_config);

        /* Must be off to enable writing of CC1S and CC2S */
        ICUD4.tim->CCER = 0;

        /* CCMR1_CC1S = 10 = IC1 is mapped on TI2.
           CCMR1_CC2S = 01 = IC2 is mapped on TI2.*/
        ICUD4.tim->CCMR1 = TIM_CCMR1_CC1S_1 | TIM_CCMR1_CC2S_0;

        /* SMCR_TS  = 110, trigger on input TI2FP2.
           SMCR_SMS = 100, reset on rising edge.*/
        ICUD4.tim->SMCR  = TIM_SMCR_TS_2 | TIM_SMCR_TS_1 | TIM_SMCR_SMS_2;

        ICUD4.tim->CCER = TIM_CCER_CC1E | TIM_CCER_CC2E | TIM_CCER_CC2P;

        icuEnable(&ICUD4);


The last thing I had to do was to invert the CCRx registers for width and period (I'm not sure why yet, 'cause I just did it some minutes ago but it works!). So width is CCR1 and period is CCR2.

Fabio Utzig

[Giovanni]

Thanks for posting that, probably the ICU driver should be enhanced to be able to use alternate inputs.

Giovanni

[utzig]

I'm on a quite tight schedule right now but I'll give it a look by the end of the week. My idea would be to add a new field on the ICUConfig where the user passes the channel number (a #define). What do you think?

Also, looking at the SMCR description there are only option of triggering on TI1FP1 and TI2FP2, with no options for TI3FPx and TI4FPx. So, I'm quite sure there's no way to trig using channels 3 and 4 on PWM input capture mode.

And the reason for having to use "inverted" CCRx registers for width/period, as I mentioned before, is that for CH1 the trig is on FP1 and for CH2 on FP2.

Fabio Utzig

[Giovanni]

Hi,

Having an input selector field into the config structure would be the correct approach. There is no need to rush because I couldn't merge the code before 2.4.0 is released (trunk is frozen now).

I am not sure if inputs 3 and 4 could could be used by using the other capture registers but that would make the driver much heavier and less maintainable, probably not worth the effort.

(TODO) Another feature that I have to add is a callback on the counter overflow in order to signal a "missing input signal" event.

Giovanni

[utzig]

Hi Giovanni,

Attached is a patch that adds support for using channel 2 as input on icu driver.

I added an icuchannel_t field to ICUConfig where user can specify ICU_CHANNEL_1 or ICU_CHANNEL_2.

Code: Select all

const ICUConfig icu_config = {
  ICU_INPUT_ACTIVE_LOW,
  1000000,
  ICU_CHANNEL_2,
  width_cb,
  period_cb,
 };


As I mentioned before when using CH2, width and period are swapped CCR registers because the trigger is on TI2FP2. Another way of approaching this is using "inverted" logic for CC1Px which is what I have done here.

I only tested this on TIM4CH2 but since there's no change when using CH1, it should work as before.

I added an assert for correct channel only on icu_lld_start function. Although channel field is also used on icu_lld_enable I guess nobody will run that without running _start before. I also did not add anything related to using UIF flag as you mentioned on the TODO in your last message.

[Giovanni]

Hi,

Thanks for the patch, I will give it a try in the weekend.

Giovanni