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

Imx8mm compatibility #126

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion libethdrivers/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ mark_as_advanced(
)
add_config_library(ethdrivers "${configure_string}")

if("${KernelPlatform}" MATCHES "imx8mq-evk")
if(("${KernelPlatform}" MATCHES "imx8mq-evk") OR ("${KernelPlatform}" MATCHES "imx8mm-evk"))
Copy link
Member

Choose a reason for hiding this comment

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

I just wonder, was there any reason we say MATCHES here and not STREQ? We could simply use the flags here also if this selects well define platforms: if(KernelPlatformImx8mq-evk OR KernelPlatformImx8mm-evk)

# Re-use the imx6 sources
set(PlatPrefix "imx6")
else()
Expand Down
2 changes: 2 additions & 0 deletions libethdrivers/include/ethdrivers/imx6.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@

#pragma once

struct eth_driver;

/* nothing here */
3 changes: 0 additions & 3 deletions libethdrivers/src/lwip.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,6 @@ static void lwip_rx_complete(void *iface, unsigned int num_bufs, void **cookies,
}
}

// PKT_DEBUG(printf("Receiving packet\n"));
// PKT_DEBUG(print_packet(COL_RX, p->payload, len));

#if ETH_PAD_SIZE
pbuf_header(p, ETH_PAD_SIZE); /* reclaim the padding word */
#endif
Expand Down
22 changes: 16 additions & 6 deletions libethdrivers/src/plat/imx6/enet.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
#include <stdlib.h>
#include <assert.h>

#ifdef CONFIG_PLAT_IMX8MQ_EVK
#define CCM_PADDR 0x30380000
#if (defined(CONFIG_PLAT_IMX8MQ_EVK) || defined(CONFIG_PLAT_IMX8MM_EVK))
#define CCM_PADDR 0x30380000 // from: root/arch/arm/include/asm/arch-imx8m/imx-regs-imx8mm.h
Copy link
Member

Choose a reason for hiding this comment

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

What is "root"?

#define CCM_SIZE 0x10000
#endif

Expand Down Expand Up @@ -314,7 +314,7 @@ static struct clock mdc_clk = {
};


#ifdef CONFIG_PLAT_IMX8MQ_EVK
#if (defined(CONFIG_PLAT_IMX8MQ_EVK) || defined(CONFIG_PLAT_IMX8MM_EVK))

static freq_t _enet_clk_get_freq(clk_t *clk)
{
Expand Down Expand Up @@ -456,6 +456,12 @@ void enet_disable(struct enet *enet)
regs->ecr &= ~ECR_ETHEREN;
}

int enet_get_mask(struct enet *enet)
{
assert(enet);
return enet_get_regs(enet)->eimr;
}

void enet_set_mac(struct enet *enet, uint64_t mac)
{
enet_regs_t *regs = enet_get_regs(enet);
Expand Down Expand Up @@ -558,7 +564,7 @@ struct enet *enet_init(void *mapped_peripheral, uintptr_t tx_phys,
enet_clk_ptr = clk_get_clock(clk_sys, CLK_ENET);
clk_set_freq(enet_clk_ptr, ENET_FREQ);

#elif defined(CONFIG_PLAT_IMX8MQ_EVK)
#elif defined(CONFIG_PLAT_IMX8MQ_EVK) || defined(CONFIG_PLAT_IMX8MM_EVK)

enet_clk_ptr = &enet_clk;
// TODO Implement an actual clock driver for the imx8mq
Expand Down Expand Up @@ -606,8 +612,12 @@ struct enet *enet_init(void *mapped_peripheral, uintptr_t tx_phys,
regs->gaur = 0;
regs->galr = 0;

#if defined(CONFIG_PLAT_IMX8MQ_EVK) || defined(CONFIG_PLAT_IMX8MM_EVK)
/* For iMX.8 we assume that the bootloader has already correctly
* configured the MAC address */
/* Set MAC and pause frame type field */
enet_set_mac(enet, mac);
#endif

/* Configure pause frames (continues into MAC registers...) */
regs->opd = PAUSE_OPCODE_FIELD << 16;
Expand Down Expand Up @@ -688,9 +698,9 @@ void enet_print_mib(struct enet *enet)
volatile struct mib_regs *mib = &regs->mib;
regs->mibc |= MIBC_DIS;

printf("Ethernet Counter regs\n");
/*printf("Ethernet Counter regs\n");
dump_regs((uint32_t *)mib, sizeof(*mib));
printf("\n");
printf("\n");*/

printf("-----------------------------\n");
printf("RX Frames RX OK: %d/%d\n", mib->ieee_r_frame_ok,
Expand Down
2 changes: 2 additions & 0 deletions libethdrivers/src/plat/imx6/enet.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ int enet_tx_enabled(struct enet *enet);
void enet_rx_enable(struct enet *enet);
int enet_rx_enabled(struct enet *enet);

int enet_get_mask(struct enet *enet);

void enet_set_mdcclk(struct enet *enet, uint32_t fout);
uint32_t enet_get_mdcclk(struct enet *imx_eth);

Expand Down
62 changes: 38 additions & 24 deletions libethdrivers/src/plat/imx6/imx6.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
#include "uboot/micrel.h"
#include "unimplemented.h"

// Temporary fix: set to imx8mq mac
#define DEFAULT_MAC "\x00\x04\x9f\x05\x93\xdf"

#define IRQ_MASK (NETIRQ_RXF | NETIRQ_TXF | NETIRQ_EBERR)
#define BUF_SIZE MAX_PKT_SIZE
#define DMA_ALIGN 32
Expand Down Expand Up @@ -185,6 +188,7 @@ static void fill_rx_bufs(imx6_eth_driver_t *dev)
* small because CONFIG_LIB_ETHDRIVER_NUM_PREALLOCATED_BUFFERS
* is less than CONFIG_LIB_ETHDRIVER_RX_DESC_COUNT.
*/
ZF_LOGF("There are no buffers left.");
break;
}
uint16_t stat = RXD_EMPTY;
Expand Down Expand Up @@ -388,7 +392,6 @@ static void complete_tx(imx6_eth_driver_t *dev)
unsigned int cnt = 0;

while (head != ring->tail) {

if (0 == cnt) {
cnt = dev->tx_lengths[head];
if ((0 == cnt) || (cnt > dev->tx.cnt)) {
Expand All @@ -409,10 +412,11 @@ static void complete_tx(imx6_eth_driver_t *dev)
/* If this buffer was not sent, we can't release any buffer. */
if (d->stat & TXD_READY) {
assert(dev->enet);
/* give it another chance */
if (!enet_tx_enabled(dev->enet)) {
enet_tx_enable(dev->enet);
}
return;
if (d->stat & TXD_READY) return;
Copy link
Member

Choose a reason for hiding this comment

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

please use

Suggested change
if (d->stat & TXD_READY) return;
if (d->stat & TXD_READY) {
return;
}

}

/* Go to next buffer, handle roll-over. */
Expand All @@ -427,7 +431,6 @@ static void complete_tx(imx6_eth_driver_t *dev)
/* give the buffer back */
cb_complete(cb_cookie, cookie);
}

}

/* The only reason to arrive here is when head equals tails. If cnt is not
Expand Down Expand Up @@ -461,18 +464,21 @@ static void handle_irq(struct eth_driver *driver, int irq)
assert(enet);

uint32_t e = enet_clr_events(enet, IRQ_MASK);
if (e & NETIRQ_TXF) {
complete_tx(dev);
}
if (e & NETIRQ_RXF) {
complete_rx(dev);
fill_rx_bufs(dev);
}
if (e & NETIRQ_EBERR) {
ZF_LOGE("Error: System bus/uDMA");
// ethif_print_state(netif_get_eth_driver(netif));
assert(0);
while (1);
while (e & IRQ_MASK) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we merge this as a potential bug fix in a separate PR? Looping to consume all pending interrupt seems a good general pattern and I can't remember any reason not to do it here.

if (e & NETIRQ_TXF) {
complete_tx(dev);
}
if (e & NETIRQ_RXF) {
complete_rx(dev);
fill_rx_bufs(dev);
}
if (e & NETIRQ_EBERR) {
ZF_LOGE("Error: System bus/uDMA");
//ethif_print_state(netif_get_eth_driver(netif));
assert(0);
while (1);
}
e = enet_clr_events(enet, IRQ_MASK);
}
}

Expand Down Expand Up @@ -554,6 +560,7 @@ static int raw_tx(struct eth_driver *driver, unsigned int num, uintptr_t *phys,
tail_new = 0;
stat |= TXD_WRAP;
}
ZF_LOGW("Inserting buffer %p of length %d into ring", *phys, *len);
update_ring_slot(ring, idx, *phys++, *len++, stat);
}

Expand Down Expand Up @@ -636,6 +643,18 @@ static int init_device(imx6_eth_driver_t *dev, const nic_config_t *nic_config)

int ret;

#if defined(CONFIG_PLAT_IMX8MQ_EVK)
/* Don't attempt to obtain the MAC address for iMX8MQ.
*
* The code to obtain the MAC address assumes an iMX6 compatible
* OCOTP which the iMX8 does not have.
*
* Instead, we assume that the bootloader has already configured the
* hardware MAC address. */
uint64_t mac = 0;
ZF_LOGI("using MAC configured by bootloader");
#else
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should make this a conditional else, so we handle all known platform explicitly and don't have a default behavior? Ir would fail compilation for unknown platforms then and requires explicit action.

/* this works for imx8mm however */
uint64_t mac = obtain_mac(nic_config, &(dev->eth_drv.io_ops.io_mapper));
if (0 == mac) {
ZF_LOGE("Failed to obtain a MAC");
Expand All @@ -649,6 +668,7 @@ static int init_device(imx6_eth_driver_t *dev, const nic_config_t *nic_config)
(uint8_t)(mac >> 16),
(uint8_t)(mac >> 8),
(uint8_t)(mac));
#endif

ret = setup_desc_ring(dev, &(dev->rx));
if (ret) {
Expand Down Expand Up @@ -736,9 +756,9 @@ static int init_device(imx6_eth_driver_t *dev, const nic_config_t *nic_config)
/* Initialise the phy library */
miiphy_init();
/* Initialise the phy */
#if defined(CONFIG_PLAT_SABRE) || defined(CONFIG_PLAT_WANDQ)
#if defined(CONFIG_PLAT_SABRE) || defined(CONFIG_PLAT_WANDQ) || defined(CONFIG_PLAT_IMX8MQ_EVK)
phy_micrel_init();
#elif defined(CONFIG_PLAT_NITROGEN6SX)
#elif defined(CONFIG_PLAT_NITROGEN6SX) || defined(CONFIG_PLAT_IMX8MM_EVK)
phy_atheros_init();
#else
#error "unsupported board"
Expand Down Expand Up @@ -824,12 +844,6 @@ static int allocate_irq_callback(ps_irq_t irq, unsigned curr_num,
assert(token);
imx6_eth_driver_t *dev = (imx6_eth_driver_t *)token;

unsigned target_num = config_set(CONFIG_PLAT_IMX8MQ_EVK) ? 2 : 0;
if (curr_num != target_num) {
ZF_LOGW("Ignoring interrupt #%d with value %d", curr_num, irq);
return 0;
}

dev->irq_id = ps_irq_register(
&(dev->eth_drv.io_ops.irq_ops),
irq,
Expand Down Expand Up @@ -951,7 +965,6 @@ int ethif_imx_init_module(ps_io_ops_t *io_ops, const char *device_path)
ret = -ENODEV;
goto error;
}

return 0;

error:
Expand All @@ -975,6 +988,7 @@ static const char *compatible_strings[] = {
"fsl,imx6q-fec",
"fsl,imx6sx-fec",
"fsl,imx8mq-fec",
"fsl,imx8mm-fec",
NULL
};

Expand Down
11 changes: 8 additions & 3 deletions libethdrivers/src/plat/imx6/ocotp_ctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
#define IMX6_OCOTP_PADDR 0x021BC000
#define IMX6_OCOTP_SIZE 0x00004000

#elif defined(CONFIG_PLAT_IMX8MQ_EVK)

#elif defined(CONFIG_PLAT_IMX8MQ_EVK) || defined(CONFIG_PLAT_IMX8MM_EVK)
#define IMX6_OCOTP_PADDR 0x30350000
#define IMX6_OCOTP_SIZE 0x00010000

Expand Down Expand Up @@ -211,10 +210,16 @@ uint64_t ocotp_get_mac(struct ocotp *ocotp, unsigned int id)
uint64_t mac = 0;

switch (id) {
#if defined(CONFIG_PLAT_IMX8MM_EVK) || defined(CONFIG_IMX8MQ_EVK)
case 0:
mac = ((uint64_t)((uint16_t)regs->res37[4]) << 32) | regs->res37[0];
Copy link
Member

Choose a reason for hiding this comment

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

Please give the registers a name in the definition, reading a MAC from reserved registers is a bit odd.

break;
#else
case 0:
/* 0x0000<aa><bb><cc><dd><ee><ff> */
mac = ((uint64_t)((uint16_t)regs->mac1) << 32) | regs->mac0;
break;
#endif

#ifdef CONFIG_PLAT_IMX6SX

Expand All @@ -237,7 +242,7 @@ uint64_t ocotp_get_mac(struct ocotp *ocotp, unsigned int id)
return 0;
}

ZF_LOGI("OCOTP MAC #%u: %02x:%02x:%02x:%02x:%02x:%02x",
ZF_LOGW("OCOTP MAC #%u: %02x:%02x:%02x:%02x:%02x:%02x",
id, (uint8_t)(mac >> 40), (uint8_t)(mac >> 32),
(uint8_t)(mac >> 24), (uint8_t)(mac >> 16), (uint8_t)(mac >> 8),
(uint8_t)mac);
Expand Down
8 changes: 5 additions & 3 deletions libethdrivers/src/plat/imx6/uboot/fec_mxc.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,11 @@ struct phy_device *fec_init(unsigned int phy_mask, struct enet *enet,
ZF_LOGW("SABRE: unexpected PHY with ID 0x%x", phydev->phy_id);
}

#elif defined(CONFIG_PLAT_NITROGEN6SX)

if (0x004dd072 == phydev->phy_id) {
#elif defined(CONFIG_PLAT_NITROGEN6SX) || defined(CONFIG_PLAT_IMX8MM_EVK)
/* imx6sx and imx8mm seem to use a very similar phy device.
The AR8031 (used in imx8mm) also has the SmartEEE feature but not sure if
this is necessary. */
if (0x004dd072 == phydev->phy_id || 0x004dd074 == phydev->phy_id) {
/* Disable Ar803x PHY SmartEEE feature, it causes link status glitches
* that result in the ethernet link going down and up.
*/
Expand Down
Loading