Re: [PATCH edk2-platforms 14/27] Silicon/NXP: Add i.MX6 GPT and EPIT timer headers

[Leif Lindholm <leif.lindholm@linaro.org>]

On Fri, Sep 21, 2018 at 08:26:05AM +0000, Chris Co wrote:
> This adds the definitions for the NXP i.MX6 General Purpose Timer
> and the Enhanced Periodic Interrupt Timer modules.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Silicon/NXP/iMX6Pkg/Include/common_epit.h | 118 +++++++++
>  Silicon/NXP/iMX6Pkg/Include/common_gpt.h  | 271 ++++++++++++++++++++
>  2 files changed, 389 insertions(+)
> 
> diff --git a/Silicon/NXP/iMX6Pkg/Include/common_epit.h b/Silicon/NXP/iMX6Pkg/Include/common_epit.h
> new file mode 100644
> index 000000000000..485d6ccbc51e
> --- /dev/null
> +++ b/Silicon/NXP/iMX6Pkg/Include/common_epit.h

Rename to CamelCase?

> @@ -0,0 +1,118 @@
> +/** @file
> +*
> +*  Provides definitions for the EPIT (Enhanced Periodic Interrupt Timer)
> +*  module that are common to Freescale SoCs.
> +*
> +*  Copyright (c) 2018 Microsoft Corporation. All rights reserved.
> +*  Copyright (c) 2004-2010, Freescale Semiconductor, Inc. All Rights Reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD License
> +*  which accompanies this distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#ifndef __COMMON_EPIT_H
> +#define __COMMON_EPIT_H

COMMON is a bit too common. Can we at least stick an IMX on the front?

This is really also a problem for all of the structs and #defines
below as well.

> +
> +typedef struct {
> +  UINT32 CR;
> +  UINT32 SR;
> +  UINT32 LR;
> +  UINT32 CMPR;
> +  UINT32 CNT;
> +} CSP_EPIT_REG, *PCSP_EPIT_REG;

I'm really not a fan of typedef pointers. Please drop the *PSCP one.

> +
> +#define EPIT_CR_OFFSET          0x0000
> +#define EPIT_SR_OFFSET          0x0004
> +#define EPIT_LR_OFFSET          0x0008
> +#define EPIT_CMPR_OFFSET        0x000C
> +#define EPIT_CNR_OFFSET         0x0010
> +
> +#define EPIT_CR_EN_LSH          0

What is LSH?

> +#define EPIT_CR_ENMOD_LSH       1
> +#define EPIT_CR_OCIEN_LSH       2
> +#define EPIT_CR_RLD_LSH         3
> +#define EPIT_CR_PRESCALAR_LSH   4
> +#define EPIT_CR_SWR_LSH         16
> +#define EPIT_CR_IOVW_LSH        17
> +#define EPIT_CR_DBGEN_LSH       18
> +#define EPIT_CR_WAITEN_LSH      19
> +#define EPIT_CR_DOZEN_LSH       20
> +#define EPIT_CR_STOPEN_LSH      21
> +#define EPIT_CR_OM_LSH          22
> +#define EPIT_CR_CLKSRC_LSH      24
> +
> +#define EPIT_SR_OCIF_LSH        0
> +#define EPIT_LR_LOAD_LSH        0
> +#define EPIT_CMPR_COMPARE_LSH   0
> +#define EPIT_CNT_COUNT_LSH      0
> +
> +#define EPIT_CR_EN_WID          1

What is WID?

> +#define EPIT_CR_ENMOD_WID       1
> +#define EPIT_CR_OCIEN_WID       2
> +#define EPIT_CR_RLD_WID         1
> +#define EPIT_CR_PRESCALAR_WID   12
> +#define EPIT_CR_SWR_WID         1
> +#define EPIT_CR_IOVW_WID        1
> +#define EPIT_CR_DBGEN_WID       1
> +#define EPIT_CR_WAITEN_WID      1
> +#define EPIT_CR_DOZEN_WID       1
> +#define EPIT_CR_STOPEN_WID      1
> +#define EPIT_CR_OM_WID          2
> +#define EPIT_CR_CLKSRC_WID      2
> +
> +#define EPIT_SR_OCIF_WID        1
> +#define EPIT_LR_LOAD_WID        32
> +#define EPIT_CMPR_COMPARE_WID   32
> +#define EPIT_CNT_COUNT_WID      32
> +
> +// CR
> +#define EPIT_CR_EN_DISABLE          0
> +#define EPIT_CR_EN_ENABLE           1
> +
> +#define EPIT_CR_ENMOD_RESUME        0
> +#define EPIT_CR_ENMOD_LOAD          1
> +
> +#define EPIT_CR_OCIEN_DISABLE       0
> +#define EPIT_CR_OCIEN_ENABLE        1
> +
> +#define EPIT_CR_RLD_ROLLOVER        0
> +#define EPIT_CR_RLD_RELOAD          1
> +
> +#define EPIT_CR_SWR_NORESET         0
> +#define EPIT_CR_SWR_RESET           1
> +
> +#define EPIT_CR_IOVW_NOOVR          0
> +#define EPIT_CR_IOVW_OVR            1
> +
> +#define EPIT_CR_DBGEN_INACTIVE      0
> +#define EPIT_CR_DBGEN_ACTIVE        1
> +
> +#define EPIT_CR_WAITEN_DISABLE      0
> +#define EPIT_CR_WAITEN_ENABLE       1
> +
> +#define EPIT_CR_DOZEN_DISABLE       0
> +#define EPIT_CR_DOZEN_ENABLE        1
> +
> +#define EPIT_CR_STOPEN_DISABLE      0
> +#define EPIT_CR_STOPEN_ENABLE       1
> +
> +#define EPIT_CR_OM_DICONNECT        0
> +#define EPIT_CR_OM_TOGGLE           1
> +#define EPIT_CR_OM_CLEAR            2
> +#define EPIT_CR_OM_SET              3
> +
> +#define EPIT_CR_CLKSRC_OFF          0
> +#define EPIT_CR_CLKSRC_IPGCLK       1
> +#define EPIT_CR_CLKSRC_HIGHFREQ     2   // High freq is sourcing from PERCLK
> +#define EPIT_CR_CLKSRC_CKIL         3
> +
> +// CNT
> +#define EPIT_CNT_COUNT_MAX          0xFFFFFFFF

MAX_UINT32?

> +
> +#endif // __COMMON_EPIT_H
> diff --git a/Silicon/NXP/iMX6Pkg/Include/common_gpt.h b/Silicon/NXP/iMX6Pkg/Include/common_gpt.h
> new file mode 100644
> index 000000000000..7fdfc25d819f
> --- /dev/null
> +++ b/Silicon/NXP/iMX6Pkg/Include/common_gpt.h

Rename to CamelCase.

> @@ -0,0 +1,271 @@
> +/** @file
> +*
> +*  Provides definitions for the GPT (General Purpose Timer) module
> +*  that are common to Freescale SoCs.
> +*
> +*  Copyright (c) 2018 Microsoft Corporation. All rights reserved.
> +*  Copyright (c) 2004-2010, Freescale Semiconductor, Inc. All Rights Reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD License
> +*  which accompanies this distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#ifndef __COMMON_GPT_H
> +#define __COMMON_GPT_H

+IMX.

> +
> +typedef struct {
> +  UINT32 CR;
> +  UINT32 PR;
> +  UINT32 SR;
> +  UINT32 IR;
> +  UINT32 OCR1;
> +  UINT32 OCR2;
> +  UINT32 OCR3;
> +  UINT32 ICR1;
> +  UINT32 ICR2;
> +  UINT32 CNT;
> +} CSP_GPT_REGS, *PCSP_GPT_REGS;

Drop *PCSP.

> +
> +#define GPT_CR_OFFSET          0x0000
> +#define GPT_PR_OFFSET          0x0004
> +#define GPT_SR_OFFSET          0x0008
> +#define GPT_IR_OFFSET          0x000C
> +#define GPT_OCR1_OFFSET        0x0010
> +#define GPT_OCR2_OFFSET        0x0014
> +#define GPT_OCR3_OFFSET        0x0018
> +#define GPT_ICR1_OFFSET        0x001C
> +#define GPT_ICR2_OFFSET        0x0020
> +#define GPT_CNT_OFFSET         0x0024
> +
> +
> +#define GPT_CR_EN_LSH          0

What is LSH?

> +#define GPT_CR_ENMOD_LSH       1
> +#define GPT_CR_DBGEN_LSH       2
> +#define GPT_CR_WAITEN_LSH      3
> +#define GPT_CR_STOPEN_LSH      5
> +#define GPT_CR_CLKSRC_LSH      6
> +#define GPT_CR_FRR_LSH         9
> +#if defined(CPU_IMX6SX) || defined(CPU_IMX6SDL)
> +#define GPT_CR_EN_24M_LSH      10
> +#endif
> +#define GPT_CR_SWR_LSH         15
> +#define GPT_CR_IM1_LSH         16
> +#define GPT_CR_IM2_LSH         18
> +#define GPT_CR_OM1_LSH         20
> +#define GPT_CR_OM2_LSH         23
> +#define GPT_CR_OM3_LSH         26
> +#define GPT_CR_FO1_LSH         29
> +#define GPT_CR_FO2_LSH         30
> +#define GPT_CR_FO3_LSH         31
> +
> +#define GPT_PR_PRESCALER_LSH   0
> +
> +#define GPT_SR_OF1_LSH         0
> +#define GPT_SR_OF2_LSH         1
> +#define GPT_SR_OF3_LSH         2
> +#define GPT_SR_IF1_LSH         3
> +#define GPT_SR_IF2_LSH         4
> +#define GPT_SR_ROV_LSH         5
> +
> +#define GPT_IR_OF1IE_LSH       0
> +#define GPT_IR_OF2IE_LSH       1
> +#define GPT_IR_OF3IE_LSH       2
> +#define GPT_IR_IF1IE_LSH       3
> +#define GPT_IR_IF2IE_LSH       4
> +#define GPT_IR_ROVIE_LSH       5
> +
> +#define GPT_OCR1_COMP_LSH      0
> +#define GPT_OCR2_COMP_LSH      0
> +#define GPT_OCR3_COMP_LSH      0
> +#define GPT_ICR1_CAPT_LSH      0
> +#define GPT_ICR2_CAPT_LSH      0
> +#define GPT_CNT_COUNT_LSH      0
> +
> +#define GPT_CR_EN_WID          1

What is WID?

/
    Leif

> +#define GPT_CR_ENMOD_WID       1
> +#define GPT_CR_DBGEN_WID       1
> +#define GPT_CR_WAITEN_WID      1
> +#define GPT_CR_STOPEN_WID      1
> +#define GPT_CR_CLKSRC_WID      3
> +#define GPT_CR_FRR_WID         1
> +#if defined(CPU_IMX6SX) || defined(CPU_IMX6SDL)
> +#define GPT_CR_EN_24M_WID      1
> +#endif
> +#define GPT_CR_SWR_WID         1
> +#define GPT_CR_IM1_WID         2
> +#define GPT_CR_IM2_WID         2
> +#define GPT_CR_OM1_WID         3
> +#define GPT_CR_OM2_WID         3
> +#define GPT_CR_OM3_WID         3
> +#define GPT_CR_FO1_WID         1
> +#define GPT_CR_FO2_WID         1
> +#define GPT_CR_FO3_WID         1
> +
> +#define GPT_PR_PRESCALER_WID   12
> +
> +#define GPT_SR_OF1_WID         1
> +#define GPT_SR_OF2_WID         1
> +#define GPT_SR_OF3_WID         1
> +#define GPT_SR_IF1_WID         1
> +#define GPT_SR_IF2_WID         1
> +#define GPT_SR_ROV_WID         1
> +
> +#define GPT_IR_OF1IE_WID       1
> +#define GPT_IR_OF2IE_WID       1
> +#define GPT_IR_OF3IE_WID       1
> +#define GPT_IR_IF1IE_WID       1
> +#define GPT_IR_IF2IE_WID       1
> +#define GPT_IR_ROVIE_WID       1
> +
> +#define GPT_OCR1_COMP_WID      32
> +#define GPT_OCR2_COMP_WID      32
> +#define GPT_OCR3_COMP_WID      32
> +#define GPT_ICR1_CAPT_WID      32
> +#define GPT_ICR2_CAPT_WID      32
> +#define GPT_CNT_COUNT_WID      32
> +
> +
> +//------------------------------------------------------------------------------
> +// REGISTER BIT WRITE VALUES
> +//------------------------------------------------------------------------------
> +
> +// GPTCR
> +#define GPT_CR_EN_ENABLE                1 // GPT enabled
> +#define GPT_CR_EN_DISABLE               0 // GPT disabled
> +
> +#define GPT_CR_ENMOD_RESET              1 // GPT counter reset to
> +                                          //   0 when disabled
> +#define GPT_CR_ENMOD_RETAIN             0 // GPT counter retains
> +                                          //   value when disabled
> +
> +#define GPT_CR_DBGEN_ENABLE             1 // GPT enabled in debug mode
> +#define GPT_CR_DBGEN_DISABLE            0 // GPT disabled in debug mode
> +
> +#define GPT_CR_WAITEN_ENABLE            1 // GPT enabled in wait mode
> +#define GPT_CR_WAITEN_DISABLE           0 // GPT disabled in wait mode
> +
> +#define GPT_CR_STOPEN_ENABLE            1 // GPT enabled in stopdoze mode
> +#define GPT_CR_STOPEN_DISABLE           0 // GPT disabled in stopoze mode
> +
> +#if defined(CPU_IMX6DQ) || defined (CPU_IMX6DQP)
> +#define GPT_CR_CLKSRC_NOCLK             0 // No clock to GPT
> +#define GPT_CR_CLKSRC_IPGCLK            1 // ipg_clk is the clock source
> +#define GPT_CR_CLKSRC_HIGHFREQ          2 // ipg_clk_highfreq
> +#define GPT_CR_CLKSRC_EXTCLK            3 // ipp_gpt_clkin (external clock
> +// from pad) is the clock source
> +#define GPT_CR_CLKSRC_CLK32K            4 // ipg_clk_32k is clock source
> +#define GPT_CR_CLKSRC_CLK8M             5 // crystal oscillator divided by 8 is clock source
> +#define GPT_CR_CLKSRC_CLK24M            7 // crystal oscillator (24 Mhz) is clock source
> +#elif defined(CPU_IMX6SDL)
> +#define GPT_CR_CLKSRC_NOCLK             0 // No clock to GPT
> +#define GPT_CR_CLKSRC_IPGCLK            1 // Peripheral Clock
> +#define GPT_CR_CLKSRC_HIGHFREQ          2 // High Frequency Reference Clock
> +#define GPT_CR_CLKSRC_EXTCLK            3 // External Clock (CLKIN)
> +#define GPT_CR_CLKSRC_LOWFREQ           4 // Low Frequency Reference Clock
> +#define GPT_CR_CLKSRC_CLK24M            5 // Crystal oscillator as Reference Clock
> +#elif defined(CPU_IMX6SX)
> +#define GPT_CR_CLKSRC_NOCLK             0 // No clock to GPT
> +#define GPT_CR_CLKSRC_PERIPHCLK         1 // Peripheral Clock
> +#define GPT_CR_CLKSRC_HIGHFREQ          2 // High Frequency Reference Clock
> +#define GPT_CR_CLKSRC_EXTCLK            3 // External Clock (CLKIN)
> +#define GPT_CR_CLKSRC_LOWFREQ           4 // Low Frequency Reference Clock
> +#define GPT_CR_CLKSRC_CLK24M            5 // Crystal oscillator as Reference Clock
> +#else
> +#error CPU Preprocessor Flag Not Defined
> +#endif
> +
> +#define GPT_CR_FRR_FREERUN              1 // Freerun mode (counter
> +                                          //   continues after compare)
> +#define GPT_CR_FRR_RESTART              0 // Restart mode (counter set
> +                                          //   to zero after compare)
> +#if defined(CPU_IMX6SX) || defined(CPU_IMX6SDL)
> +#define GPT_CR_EN_24M_DISABLE           0  // 24M clock disabled
> +#define GPT_CR_EN_24M_ENABLE            1  // 24M clock enabled
> +#endif
> +
> +#define GPT_CR_SWR_RESET                1 // Self-clearing software reset
> +#define GPT_CR_SWR_NORESET              0 // Do not activate software reset
> +
> +#define GPT_CR_IM1_DISABLE              0 // Capture Disabled
> +#define GPT_CR_IM1_EDGE_RISE            1 // Capture on rising edge
> +#define GPT_CR_IM1_EDGE_FALL            2 // Capture on falling edge
> +#define GPT_CR_IM1_EDGE_BOTH            3 // Capture on both edges
> +
> +#define GPT_CR_IM2_DISABLE              0 // Capture Disabled
> +#define GPT_CR_IM2_EDGE_RISE            1 // Capture on rising edge
> +#define GPT_CR_IM2_EDGE_FALL            2 // Capture on falling edge
> +#define GPT_CR_IM2_EDGE_BOTH            3 // Capture on both edges
> +
> +#define GPT_CR_OM1_DISABLE              0 // Compare generates no response
> +#define GPT_CR_OM1_TOGGLE               1 // Compare toggles output pin
> +#define GPT_CR_OM1_CLEAR                2 // Compare clears output pin
> +#define GPT_CR_OM1_SET                  3 // Compare sets output pin
> +#define GPT_CR_OM1_PULSE                4 // Compare event generates a
> +                                          //   single count duration pulse
> +                                          //   on output pin
> +
> +#define GPT_CR_OM2_DISABLE              0 // Compare generates no response
> +#define GPT_CR_OM2_TOGGLE               1 // Compare toggles output pin
> +#define GPT_CR_OM2_CLEAR                2 // Compare clears output pin
> +#define GPT_CR_OM2_SET                  3 // Compare sets output pin
> +#define GPT_CR_OM2_PULSE                4 // Compare event generates a
> +                                          //   single count duration pulse
> +                                          //   on output pin
> +
> +#define GPT_CR_OM3_DISABLE              0 // Compare generates no response
> +#define GPT_CR_OM3_TOGGLE               1 // Compare toggles output pin
> +#define GPT_CR_OM3_CLEAR                2 // Compare clears output pin
> +#define GPT_CR_OM3_SET                  3 // Compare sets output pin
> +#define GPT_CR_OM3_PULSE                4 // Compare event generates a
> +                                          //   single count duration pulse
> +                                          //   on output pin
> +
> +#define GPT_CR_FO1_FORCE                1 // Force pin action programmed
> +                                          //   for output compare 1 pin.
> +                                          //   Pin is self-negating.
> +#define GPT_CR_FO1_NOFORCE              0 // Do not force pin
> +
> +#define GPT_CR_FO2_FORCE                1 // Force pin action programmed
> +                                          //   for output compare 1 pin
> +                                          //   Pin is self-negating.
> +#define GPT_CR_FO2_NOFORCE              0 // Do not force pin
> +
> +#define GPT_CR_FO3_FORCE                1 // Force pin action programmed
> +                                          //   for output compare 1 pin
> +                                          //   Pin is self-negating.
> +#define GPT_CR_FO3_NOFORCE              0 // Do not force pin
> +
> +// GPTSR
> +#define GPT_SR_OF1_STATUS_CLEAR         1 // Output compare 1 status clear
> +#define GPT_SR_OF2_STATUS_CLEAR         1 // Output compare 2 status clear
> +#define GPT_SR_OF3_STATUS_CLEAR         1 // Output compare 3 status clear
> +#define GPT_SR_IF1_STATUS_CLEAR         1 // Input capture 1 status clear
> +#define GPT_SR_IF2_STATUS_CLEAR         1 // Input capture 2 status clear
> +#define GPT_SR_ROV_STATUS_CLEAR         1 // Rollover status clear
> +
> +// GPTIR
> +#define GPT_IR_OF1IE_INT_ENABLE         1 // Output compare 1 int enabled
> +#define GPT_IR_OF1IE_INT_DISABLE        0 // Output compare 1 int disabled
> +
> +#define GPT_IR_OF2IE_INT_ENABLE         1 // Output compare 2 int enabled
> +#define GPT_IR_OF2IE_INT_DISABLE        0 // Output compare 2 int disabled
> +
> +#define GPT_IR_OF3IE_INT_ENABLE         1 // Output compare 3 int enabled
> +#define GPT_IR_OF3IE_INT_DISABLE        0 // Output compare 3 int disabled
> +
> +#define GPT_IR_IF1IE_INT_ENABLE         1 // Input capture 1 int enabled
> +#define GPT_IR_IF1IE_INT_DISABLE        0 // Input capture 1 int disabled
> +
> +#define GPT_IR_IF2IE_INT_ENABLE         1 // Input capture 2 int enabled
> +#define GPT_IR_IF2IE_INT_DISABLE        0 // Input capture 2 int disabled
> +
> +#define GPT_IR_ROVIE_INT_ENABLE         1 // Rollover int enabled
> +#define GPT_IR_ROVIE_INT_DISABLE        0 // Rollover int disabled
> +
> +#endif // __COMMON_GPT_H
> -- 
> 2.16.2.gvfs.1.33.gf5370f1
>