Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Freescale Kinetis MCUs, kinetis_common #2265

Merged
merged 1 commit into from
Feb 5, 2015
Merged

Support for Freescale Kinetis MCUs, kinetis_common #2265

merged 1 commit into from
Feb 5, 2015

Conversation

jfischer-no
Copy link
Contributor

@jfischer-no jfischer-no added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Jan 8, 2015
NVIC_SetPriority(TIMER_0_IRQ_CHAN, TIMER_IRQ_PRIO);
/* select timer, use channel 0 as prescaler */
timer = TIMER_0_DEV;
timer->MCR = PIT_MCR_FRZ_MASK;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we freeze the timer here? I can not find an instance below where the FRZ flag is turned off again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows the timers to be stopped when the device enters the Debug mode. (MKW2xD RM).

ups, PIT code is a little neglected. I wanted to remove it, because it does not fit in function to riot. Maybe I will find time to rewrite it on the weekend.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I mixed up the FRZ and MDIS flags.

@jnohlgard
Copy link
Member

I will do a more thorough review during the weekend or beginning of the next week. The code seems to work on my K60 board as well as @jfischer-phytec-iot 's KW22 board.

@jfischer-no
Copy link
Contributor Author

During the porting to kw01z128 I had some problems with identifiers of IRQs (lptmr). It can be resolved if one uses the latest CMSIS compliant header filer. (Contact your FAE to get a newest 😄 )

I currently work at the UART code, there are 3 slightly different UARTs for kinetis MCUs.

}
else if (dev->S1 & UART_S1_TDRE_MASK) {
if (config[uartnum].tx_cb(config[uartnum].arg) == 0) {
dev->C2 |= (1 << UART_C2_TIE_SHIFT);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic seem wrong. TX interrupt is supposed to be disabled if return value is zero. dev->C2 &= ~(1 << UART_C2_TIE_SHIFT); (or shorter: dev->C2 &= ~(UART_C2_TIE_MASK);)

See also: http://riot-os.org/api/group__driver__periph__uart.html#ga2aa1686d28caae1aa74baa3d047e8330

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, corrected.

@jnohlgard
Copy link
Member

I have been working on this with my K60 port during the day, so far I have successfully used the following modules:

  • cpuid
  • random (See jfischer-phytec-iot/RIOT#3 )
  • uart
  • hwtimer (See jfischer-phytec-iot/RIOT#3 )

Still some more work to be done on my part for testing the rest of the stuff.

@jnohlgard
Copy link
Member

@Farthen Regarding PIT timers: The timers are always running at the Bus clock frequency, in order to allow for variable frequency it is possible to use one timer as a divider/counter and chain two timers together. What the timer module does is that it counts ticks_per_usec ticks on one timer, and every time that timer overflows, the second timer decreases by one, effectively making the second timer run at 1 MHz. This is why we can only use two timers even though the hardware has four timer/counters.

@jnohlgard
Copy link
Member

@jfischer-phytec-iot Looking at the SPI driver. I think it can be merged as is, but I would like to suggest some changes to the configuration. I would like to use the two CTAR registers in a different way, in order to be able to communicate with multiple devices with different signal polarities or timings without having to reinitialize the driver over and over.

I realize that currently there is no configuration at all, everything is hard coded. I will open a PR later if I find a way to provide a suitable API for the change...

@jnohlgard
Copy link
Member

@Farthen Please write your comments in the "Files Changed" tab, that way the comments and a code snippet will be inlined in the discussion thread for the PR on #2265
I agree that the text in the code is wrong, it should be either 32.768 kHz or 32768 Hz.

@jnohlgard jnohlgard added this to the Release NEXT MAJOR milestone Jan 13, 2015
@jfischer-no jfischer-no added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Jan 15, 2015

switch (LPTIMER_CLKSRC) {
case LPTIMER_CLKSRC_MCGIRCLK:
/* Select MCGIRCLK as clock source */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should enable MCGIRCLK first, as this is not the default.

MCG->C1 |= MCG_C1_IRCLKEN_MASK;

// For fast internal clock
MCG->C2 |= MCG_C2_IRCS_MASK;
while(!(MCG->S & MCG_S_IRCST_MASK));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that is a option for MCG driver or your cpu_clock_init, not for LPTMR driver.

@jnohlgard
Copy link
Member

@jfischer-phytec-iot Could you rebase kinetis_common to latest master?

(Since #2290 and #2323 have been merged)

*/

/**
* @addtogroup cpu_kinetis_common Common Drivers for Freescale Kinetis MCUs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to @ingroup cpu_kinetis_common, put the textual description of the group in the @defgroup command, create a doc.txt similar to what some other CPU directories have.

This applies for all other uses of @addtogroup cpu_kinetis_common below as well. (I won't put a comment on every one, they are too many)

case ADC_RES_10BIT:
case ADC_RES_12BIT:
case ADC_RES_14BIT:
case ADC_RES_16BIT:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For each case, add:

adc_config[dev].max_value = (1 << numbits) - 1;

@jnohlgard
Copy link
Member

I noticed some issues with the ADC driver when using it on the K60. Some of the analog pins on the K60 are not configurable through the pin mux and do not have a corresponding PORTx->PCR entry. I will add support for ADC_x_CHy_PORT == 0 to tell the driver to skip pin mux configuration.
The reason that I want these pins so bad is that they are also the ADC pins which have the greatest analog functionality, such as programmable gain and differential input.

@jfischer-phytec-iot I will take care of the issues in the ADC driver and open a PR against your branch (this PR) when I have fixed it. See above for my other comments on the other kinetis_common drivers.

@jfischer-no
Copy link
Contributor Author

@gebart ok

@jnohlgard
Copy link
Member

@jfischer-phytec-iot
ADC refactor PR: https://github.com/jfischer-phytec-iot/RIOT/pull/12

KINETIS_MCG_FEI, /**< FLL Engaged Internal Mode */
} kinetis_mcg_mode_t;

uint32_t cpufreq;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace by an extern declaration (if this is indeed used in any other files than mcg.c):

extern uint32_t cpufreq;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cpufreq is a zombie, it was planned that the mcg driver calculates current cpu frequency, but because of lack of time it has not been done. I will remove it for now.

@jnohlgard
Copy link
Member

@jfischer-phytec-iot
Could you rebase on latest master?

@jnohlgard jnohlgard added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Feb 3, 2015
break;
}

adc_config[dev].max_value = (1 << adc_config[dev].bits);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to subtract 1 here.. Please change this to:

adc_config[dev].max_value = (1 << adc_config[dev].bits) - 1;

@jnohlgard
Copy link
Member

I believe this is ready for merge after squashing and fixing the off by one error in adc.c (caused by me, oops!)

@jnohlgard jnohlgard removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Feb 4, 2015
  add peripheral drivers for Freescale Kinetis MCUs:
    adc driver
    cpuid driver
    gpio driver
    hwtimer_arch driver (hwtimer used Low Power Timer)
    i2c driver (master mode only)
    mcg driver
    pwm driver
    random_rnga driver
    random_rngb driver
    rtc driver
    spi driver
    timer driver (timer used Periodic Interrupt Timer)
    uart driver
  add doc.txt (configuration examples)

  random_rnga: Update RNGA driver in preparation for RNGB driver.
  random_rngb: Add RNGB driver.
  spi: refactor SPI to work for multiple CTARS, add spi_acquire, spi_release
  gpio: Add gpio_irq_enable, gpio_irq_disable. Refactor GPIO.
  gpio: Add gpio_irq_enable, gpio_irq_disable.
  gpio: Refactor ISR functions to work with all GPIOs (0-31) and all ports (PORTA-PORTH)
  adc: Refactor ADC, add calibration and scaling.
    Added integer scaling of results in adc_map.
    Handle precision setting in adc_init.
    Set ADC clock divider depending on module clock.
    Add ADC_1 as a possible device.
    Add ADC calibration procedure according to K60 ref manual.
    Handle ADC pins which are not part of the pin function mux.
  Signed-off-by: Joakim Gebart <[email protected]>
@jfischer-no jfischer-no removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Feb 4, 2015
@jnohlgard
Copy link
Member

ACK

jnohlgard pushed a commit that referenced this pull request Feb 5, 2015
Support for Freescale Kinetis MCUs, kinetis_common
@jnohlgard jnohlgard merged commit 890262e into RIOT-OS:master Feb 5, 2015
@OlegHahm
Copy link
Member

OlegHahm commented Feb 5, 2015

Great! Congrats and thanks for the effort.

@jfischer-no jfischer-no deleted the pr@kinetis_common branch February 5, 2015 09:19
@jfischer-no jfischer-no restored the pr@kinetis_common branch February 9, 2015 13:15
@OlegHahm OlegHahm modified the milestones: Release NEXT MAJOR, Release 2015.06 Apr 29, 2015
@jfischer-no jfischer-no deleted the pr@kinetis_common branch July 6, 2015 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants