Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions drivers/counter/Kconfig.ti_am3352_eqep
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,15 @@ config COUNTER_TI_AM3352_EQEP
select PINCTRL
help
Enables the TI EQEP counter driver.

if COUNTER_TI_AM3352_EQEP
config COUNTER_TI_AM3352_EQEP_VARIANT_AM13
bool "TI EQEP AM13E variant (QEPSRCSEL, Input X-BAR, PWREN)"
depends on SOC_SERIES_AM13E
default y
select MUX
select SYSCON
help
Enable AM13E EQEP support: QEPSRCSEL, mux-states X-BAR routing,
and SYSCTL PWREN via ti,pwren.
endif
228 changes: 208 additions & 20 deletions drivers/counter/counter_ti_am3352_eqep.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,25 @@
#include <zephyr/drivers/clock_control.h>
#include <zephyr/logging/log.h>

#ifdef CONFIG_CLOCK_CONTROL_TISCI
#include <zephyr/drivers/clock_control/tisci_clock_control.h>
#endif

#ifdef CONFIG_CLOCK_CONTROL_MSPM0
#include <zephyr/drivers/clock_control/mspm0_clock_control.h>
#endif

#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
#include <zephyr/drivers/mux.h>
#include <zephyr/drivers/syscon.h>
#include <zephyr/dt-bindings/counter/ti_am3352_eqep.h>
#include <zephyr/kernel.h>

#include <soc.h>

#define TI_EQEP_SYSCTL_PWREN_KEY (0x26U << 24)
#endif

LOG_MODULE_REGISTER(ti_eqep);

#define DT_DRV_COMPAT ti_am3352_eqep
Expand Down Expand Up @@ -39,6 +58,10 @@ struct ti_eqep_regs {
uint8_t RESERVED_4[0x4]; /**< Reserved, offset: 0x3A - 0x3E */
volatile uint16_t QCTLAT; /**< QEP Capture Timer Latch Register, offset: 0x3E */
volatile uint16_t QCPRDLAT; /**< QEP Capture Period Latch Register, offset: 0x40 */
#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
uint8_t RESERVED_5[0x2A]; /**< Reserved, offset: 0x42 - 0x6C */
volatile uint32_t QEPSRCSEL; /**< QEP Source Select, offset: 0x6C */
#endif
};

/* Quadrature Decoder Control Register */
Expand All @@ -47,14 +70,16 @@ struct ti_eqep_regs {
#define TI_EQEP_QDECCTL_SWAP BIT(10)

/* QEP Control Register */
#define TI_EQEP_QEPCTL_PCRM GENMASK(13, 12)
#define TI_EQEP_QEPCTL_PCRM_MAX (0x1)
#define TI_EQEP_QEPCTL_SWI BIT(7)
#define TI_EQEP_QEPCTL_SEL BIT(6)
#define TI_EQEP_QEPCTL_IEL GENMASK(5, 4)
#define TI_EQEP_QEPCTL_QPEN BIT(3)
#define TI_EQEP_QEPCTL_QCLM BIT(2)
#define TI_EQEP_QEPCTL_UTE BIT(1)
#define TI_EQEP_QEPCTL_FREE_SOFT GENMASK(15, 14)
#define TI_EQEP_QEPCTL_FREE_SOFT_RUNFREE 2
#define TI_EQEP_QEPCTL_PCRM GENMASK(13, 12)
#define TI_EQEP_QEPCTL_PCRM_MAX (0x1)
#define TI_EQEP_QEPCTL_SWI BIT(7)
#define TI_EQEP_QEPCTL_SEL BIT(6)
#define TI_EQEP_QEPCTL_IEL GENMASK(5, 4)
#define TI_EQEP_QEPCTL_QPEN BIT(3)
#define TI_EQEP_QEPCTL_QCLM BIT(2)
#define TI_EQEP_QEPCTL_UTE BIT(1)

/* Position Compare Control Register */
#define TI_EQEP_QPOSCTL_PCSHDW BIT(15)
Expand All @@ -70,6 +95,12 @@ struct ti_eqep_regs {
#define TI_EQEP_QCAPCTL_CCPS GENMASK(6, 4)
#define TI_EQEP_QCAPCTL_UPPS GENMASK(3, 0)

#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
#define TI_EQEP_QEPSRCSEL_QEPASEL GENMASK(3, 0)
#define TI_EQEP_QEPSRCSEL_QEPBSEL GENMASK(7, 4)
#define TI_EQEP_QEPSRCSEL_QEPISEL GENMASK(11, 8)
#endif

/* Interrupt Types - used in multiple registers */
#define TI_EQEP_INT_UTOI BIT(11) /* Timeout Event */
#define TI_EQEP_INT_IELI BIT(10) /* Index Event */
Expand All @@ -84,6 +115,13 @@ struct ti_eqep_regs {
#define DEV_DATA(dev) ((struct ti_eqep_data *)(dev)->data)
#define DEV_REGS(dev) ((struct ti_eqep_regs *)DEVICE_MMIO_NAMED_GET(dev, base))

#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
struct ti_eqep_mux_entry {
const struct device *dev;
const struct mux_state *state;
};
#endif

struct ti_eqep_cfg {
struct counter_config_info info;

Expand All @@ -92,6 +130,18 @@ struct ti_eqep_cfg {
clock_control_subsys_t clock_subsys;
const struct pinctrl_dev_config *pcfg;
void (*irq_config_func)();
#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
const struct ti_eqep_mux_entry *mux_entries;
uint8_t mux_entries_count;
bool has_src_select;
uint8_t qepa_src;
uint8_t qepb_src;
uint8_t qepi_src;
const struct device *pwren_syscon;
uint32_t pwren_offset;
uint8_t pwren_bit;
bool has_pwren;
#endif
};

struct ti_eqep_data {
Expand Down Expand Up @@ -129,8 +179,9 @@ static void ti_eqep_reset_counter(const struct device *dev, uint32_t top_value)
regs->QPOSINIT = top_value;
}

/* initialize counter */
/* SWI is sticky; pulse it so later QEPCTL writes do not re-init. */
regs->QEPCTL |= TI_EQEP_QEPCTL_SWI;
regs->QEPCTL &= ~TI_EQEP_QEPCTL_SWI;
}

static int ti_eqep_start(const struct device *dev)
Expand Down Expand Up @@ -400,12 +451,49 @@ static int ti_eqep_set_guard_period(const struct device *dev, uint32_t ticks, ui
return 0;
}

#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
static int ti_eqep_pwren_enable(const struct ti_eqep_cfg *cfg)
{
uint32_t bit;
uint32_t mask;
int ret;

if (!cfg->has_pwren) {
return 0;
}

if (!device_is_ready(cfg->pwren_syscon)) {
LOG_ERR("SYSCTL (pwren) device not ready");
return -ENODEV;
}

bit = BIT(cfg->pwren_bit);
mask = bit | TI_EQEP_SYSCTL_PWREN_KEY;
ret = syscon_update_bits(cfg->pwren_syscon, cfg->pwren_offset, mask, mask);
if (ret < 0) {
LOG_ERR("failed to set SYSCTL PWREN bit %u: %d", cfg->pwren_bit, ret);
return ret;
}

msp_delay_peripheral_startup();

return 0;
}
#endif /* CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13 */

static int ti_eqep_init(const struct device *dev)
{
const struct ti_eqep_cfg *cfg = DEV_CFG(dev);
struct ti_eqep_regs *regs;
int ret;

#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
ret = ti_eqep_pwren_enable(cfg);
if (ret < 0) {
return ret;
}
#endif

DEVICE_MMIO_NAMED_MAP(dev, base, K_MEM_CACHE_NONE);
regs = DEV_REGS(dev);

Expand All @@ -415,6 +503,36 @@ static int ti_eqep_init(const struct device *dev)
return ret;
}

#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
for (uint8_t i = 0; i < cfg->mux_entries_count; i++) {
const struct ti_eqep_mux_entry *entry = &cfg->mux_entries[i];

if (!device_is_ready(entry->dev)) {
LOG_ERR("xbar device not ready");
return -ENODEV;
}

ret = mux_state_apply(entry->dev, entry->state);
if (ret < 0) {
LOG_ERR("failed to apply mux state %d: %d", i, ret);
return ret;
}
}

if (cfg->has_src_select) {
regs->QEPSRCSEL = (regs->QEPSRCSEL &
~(TI_EQEP_QEPSRCSEL_QEPASEL | TI_EQEP_QEPSRCSEL_QEPBSEL |
TI_EQEP_QEPSRCSEL_QEPISEL)) |
FIELD_PREP(TI_EQEP_QEPSRCSEL_QEPASEL, cfg->qepa_src) |
FIELD_PREP(TI_EQEP_QEPSRCSEL_QEPBSEL, cfg->qepb_src) |
FIELD_PREP(TI_EQEP_QEPSRCSEL_QEPISEL, cfg->qepi_src);
}
#endif /* CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13 */

/* FREE_SOFT=run-free: keep QUTMR running under debugger halt. */
regs->QEPCTL = (regs->QEPCTL & ~TI_EQEP_QEPCTL_FREE_SOFT) |
FIELD_PREP(TI_EQEP_QEPCTL_FREE_SOFT, TI_EQEP_QEPCTL_FREE_SOFT_RUNFREE);
Comment on lines +532 to +534

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

3. Debug-halt timer behaviour changes on all socs 🐞 Bug ≡ Correctness

ti_eqep_init now always rewrites QEPCTL.FREE_SOFT to run-free, outside the
CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13 guard. Existing K3/AM3352 users therefore also get the
new debugger-halt behaviour, where position and unit timers keep running while the core is halted,
even though the change was meant for AM13.
Agent Prompt
## Issue description
The FREE_SOFT run-free setting applies to all SoCs that use the ti,am3352-eqep driver, not only AM13. This changes debug-halt behaviour on existing platforms.

## Fix Focus Areas
- drivers/counter/counter_ti_am3352_eqep.c[532-534]

## Recommended Fix
Move the FREE_SOFT write inside the AM13 variant block, or make it configurable (Kconfig or DT property). If the change is meant for all SoCs, say so in the commit message.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


/* irq connect */
cfg->irq_config_func();

Expand All @@ -435,15 +553,21 @@ static void ti_eqep_isr(const struct device *dev)
struct ti_eqep_regs *regs = DEV_REGS(dev);
struct ti_eqep_data *data = DEV_DATA(dev);
uint16_t flg = regs->INTFLG;
uint16_t unknown = flg & ~(TI_EQEP_INT_PCOI | TI_EQEP_INT_PCUI | TI_EQEP_INT_PCMI |
TI_EQEP_INT_SELI | TI_EQEP_INT_IELI | TI_EQEP_INT_UTOI |
TI_EQEP_INT_GLOB);
Comment on lines +556 to +558

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Isr logs errors for flags that were never enabled 🐞 Bug ◔ Observability

The ISR now computes unknown = flg & ~(known) from the raw INTFLG without masking by INTEN,
and it runs on every interrupt rather than only when no known flag matched. Status flags the driver
never enables, such as quadrature direction change (TI_EQEP_INT_QDCI) and phase or position
errors, can be latched when an overflow, compare, index or timeout interrupt fires; each such
interrupt then emits LOG_ERR("unknown interrupt ...") from ISR context.
Agent Prompt
## Issue description
`ti_eqep_isr` reports any latched `INTFLG` bit outside the known set as an error on every interrupt. That includes status flags like QDC that latch without being enabled, so error logs get spammed from ISR context.

## Fix Focus Areas
- drivers/counter/counter_ti_am3352_eqep.c[556-558]
- drivers/counter/counter_ti_am3352_eqep.c[637-641]

## Recommended Fix
Compute `unknown = flg & regs->INTEN & ~(known | TI_EQEP_INT_GLOB)` so only enabled-but-unhandled sources are logged. Clear the remaining non-enabled latched flags silently, or leave them alone. Consider `LOG_DBG` for the silent case.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


/* Service every pending flag; multiple events can be set together. */
if (flg & (TI_EQEP_INT_PCOI | TI_EQEP_INT_PCUI)) {
if (data->top_callback) {
data->top_callback(dev, data->top_user_data);
}

/* clear overflow/underflow */
regs->INTCLR |= (TI_EQEP_INT_PCOI | TI_EQEP_INT_PCUI);
} else if (flg & TI_EQEP_INT_PCMI) {
}

if (flg & TI_EQEP_INT_PCMI) {
enum ti_eqep_alarm_channel chan = TI_EQEP_ALARM_CHAN_COMPARE;
counter_alarm_callback_t cb = data->alarm_callback[chan];
void *user_data = data->alarm_user_data[chan];
Expand All @@ -460,7 +584,9 @@ static void ti_eqep_isr(const struct device *dev)

/* clear compare interrupt */
regs->INTCLR |= TI_EQEP_INT_PCMI;
} else if (flg & TI_EQEP_INT_SELI) {
}

if (flg & TI_EQEP_INT_SELI) {
enum ti_eqep_alarm_channel chan = TI_EQEP_ALARM_CHAN_STROBE;
counter_alarm_callback_t cb = data->alarm_callback[chan];
void *user_data = data->alarm_user_data[chan];
Expand All @@ -474,7 +600,9 @@ static void ti_eqep_isr(const struct device *dev)

/* clear strobe interrupt */
regs->INTCLR |= TI_EQEP_INT_SELI;
} else if (flg & TI_EQEP_INT_IELI) {
}

if (flg & TI_EQEP_INT_IELI) {
enum ti_eqep_alarm_channel chan = TI_EQEP_ALARM_CHAN_INDEX;
counter_alarm_callback_t cb = data->alarm_callback[chan];
void *user_data = data->alarm_user_data[chan];
Expand All @@ -488,7 +616,9 @@ static void ti_eqep_isr(const struct device *dev)

/* clear index interrupt */
regs->INTCLR |= TI_EQEP_INT_IELI;
} else if (flg & TI_EQEP_INT_UTOI) {
}

if (flg & TI_EQEP_INT_UTOI) {
enum ti_eqep_alarm_channel chan = TI_EQEP_ALARM_CHAN_TIMEOUT;
counter_alarm_callback_t cb = data->alarm_callback[chan];
void *user_data = data->alarm_user_data[chan];
Expand All @@ -502,10 +632,12 @@ static void ti_eqep_isr(const struct device *dev)

/* clear timeout interrupt */
regs->INTCLR |= TI_EQEP_INT_UTOI;
} else {
LOG_ERR("unknown interrupt %u encountered, clearing", flg);
}

if (unknown != 0U) {
LOG_ERR("unknown interrupt %u encountered, clearing", unknown);

regs->INTCLR |= flg;
regs->INTCLR |= unknown;
}

/* clear global interrupt */
Expand Down Expand Up @@ -639,15 +771,69 @@ int z_impl_ti_eqep_get_latched_capture_values(const struct device *dev, uint32_t
), (COND_CODE_1(CONFIG_CLOCK_CONTROL_ARM_SCMI, \
(static const clock_control_subsys_t ti_eqep_clk_subsys_##n = \
(clock_control_subsys_t)DT_INST_PHA(n, clocks, name); \
), (BUILD_ASSERT(0, "Unsupported clock controller");))))
), (COND_CODE_1(CONFIG_CLOCK_CONTROL_MSPM0, \
(static const struct mspm0_sys_clock ti_eqep_mspm0_sys_clock_##n = \
MSPM0_CLOCK_SUBSYS_FN(n); \
static const clock_control_subsys_t ti_eqep_clk_subsys_##n = \
(clock_control_subsys_t)&ti_eqep_mspm0_sys_clock_##n; \
), (BUILD_ASSERT(0, "Unsupported clock controller");))))))

#ifdef CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13
#define TI_EQEP_MUX_ENTRY(node_id, prop, idx) \
{ \
.dev = MUX_STATE_DT_DEV_GET_BY_IDX(node_id, idx), \
.state = MUX_STATE_DT_GET_BY_IDX(node_id, idx), \
}

#define TI_EQEP_MUX_ENTRIES_DEFINE(n) \
IF_ENABLED(DT_INST_NODE_HAS_PROP(n, mux_states), \
(MUX_STATE_DT_INST_SPEC_DEFINE_ALL(n); \
static const struct ti_eqep_mux_entry ti_eqep_mux_entries_##n[] = { \
DT_INST_FOREACH_PROP_ELEM_SEP(n, mux_states, TI_EQEP_MUX_ENTRY, \
(,))};))

#define TI_EQEP_MUX_ENTRIES_INIT(n) \
COND_CODE_1(DT_INST_NODE_HAS_PROP(n, mux_states), \
(.mux_entries = ti_eqep_mux_entries_##n, \
.mux_entries_count = ARRAY_SIZE(ti_eqep_mux_entries_##n),), ())

#define TI_EQEP_HAS_SRC_SELECT(n) \
UTIL_OR(DT_INST_NODE_HAS_PROP(n, mux_states), \
UTIL_OR(DT_INST_NODE_HAS_PROP(n, ti_qepa_src), \
UTIL_OR(DT_INST_NODE_HAS_PROP(n, ti_qepb_src), \
DT_INST_NODE_HAS_PROP(n, ti_qepi_src))))

#define TI_EQEP_SRC_SELECT_INIT(n) \
COND_CODE_1(TI_EQEP_HAS_SRC_SELECT(n), \
(.has_src_select = true, \
.qepa_src = DT_INST_PROP_OR(n, ti_qepa_src, TI_EQEP_SRC_INPUT_XBAR), \
.qepb_src = DT_INST_PROP_OR(n, ti_qepb_src, TI_EQEP_SRC_INPUT_XBAR), \
.qepi_src = DT_INST_PROP_OR(n, ti_qepi_src, TI_EQEP_SRC_INPUT_XBAR),), ())

#define TI_EQEP_PWREN_INIT(n) \
COND_CODE_1(DT_INST_NODE_HAS_PROP(n, ti_pwren), \
(.has_pwren = true, \
.pwren_syscon = DEVICE_DT_GET(DT_INST_PHANDLE(n, ti_pwren)), \
.pwren_offset = DT_INST_PHA(n, ti_pwren, offset), \
.pwren_bit = DT_INST_PHA(n, ti_pwren, bit),), ())
#else /* !CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13 */
#define TI_EQEP_MUX_ENTRIES_DEFINE(n)
#define TI_EQEP_MUX_ENTRIES_INIT(n)
#define TI_EQEP_SRC_SELECT_INIT(n)
#define TI_EQEP_PWREN_INIT(n)
#endif /* CONFIG_COUNTER_TI_AM3352_EQEP_VARIANT_AM13 */

#define TI_EQEP_INIT(n) \
PINCTRL_DT_INST_DEFINE(n); \
TI_EQEP_DEFINE_CLK_SUBSYS(n); \
TI_EQEP_MUX_ENTRIES_DEFINE(n); \
static void ti_eqep_irq_config_func_##n(void) \
{ \
IRQ_CONNECT(DT_INST_IRQN(n), DT_INST_IRQ(n, priority), ti_eqep_isr, \
DEVICE_DT_INST_GET(n), DT_INST_IRQ(n, flags)); \
IRQ_CONNECT( \
DT_INST_IRQN(n), DT_INST_IRQ(n, priority), ti_eqep_isr, \
DEVICE_DT_INST_GET(n), \
COND_CODE_1(DT_INST_IRQ_HAS_CELL(n, flags), \
(DT_INST_IRQ(n, flags)), (0))); \
irq_enable(DT_INST_IRQN(n)); \
} \
static struct ti_eqep_cfg ti_eqep_config_##n = { \
Expand All @@ -661,7 +847,9 @@ int z_impl_ti_eqep_get_latched_capture_values(const struct device *dev, uint32_t
.clock_subsys = ti_eqep_clk_subsys_##n, \
.irq_config_func = ti_eqep_irq_config_func_##n, \
.pcfg = PINCTRL_DT_INST_DEV_CONFIG_GET(n), \
}; \
TI_EQEP_MUX_ENTRIES_INIT(n) \
TI_EQEP_SRC_SELECT_INIT(n) \
TI_EQEP_PWREN_INIT(n)}; \
\
static struct ti_eqep_data ti_eqep_data_##n; \
\
Expand Down
Loading
Loading