Add teensy41 example #2468
Add teensy41 example #2468
Conversation
There was a problem hiding this comment.
So, in the future, we can move PHY detection and "init" + "up" functions to one or more PHY drivers, and these MAC drivers will call a function here, passing a pointer to the actual function that will interact with the MAC controller to get the PHY data.
This will avoid a lot of similar code with same function in most drivers.
| @@ -27,7 +27,7 @@ static void timer_fn(void *arg) { | |||
| gpio_toggle(LED); // Blink LED | |||
| struct mg_tcpip_if *ifp = arg; // And show | |||
| const char *names[] = {"down", "up", "req", "ready"}; // network stats | |||
| MG_INFO(("Ethernet: %s, IP: %M, rx:%u, tx:%u, dr:%u, er:%u", | |||
| MG_INFO(("Ethernet 2: %s, IP: %M, rx:%u, tx:%u, dr:%u, er:%u", | |||
There was a problem hiding this comment.
Ethernet 2 ?
(and this does not belong here)
There was a problem hiding this comment.
Ah, that is testing leftover, removed
|
|
||
| static uint32_t eth_read_phy(uint8_t addr, uint8_t reg) { | ||
| static uint16_t rt10xx_phy_read(uint8_t addr, uint8_t reg) { |
There was a problem hiding this comment.
Most drivers have an "eth_write_phy"
Yes, the new name is taxonomy friendlier, I like it
There was a problem hiding this comment.
I was thinking that in (very unlikely situation) we have 2 drivers enabled - then eth_read_phy () would clash. So the main reason was a protection against that
src/drivers/rt1020.c
Outdated
| @@ -52,16 +52,16 @@ uint8_t s_txbuf[ETH_DESC_CNT][ETH_PKT_SIZE] __attribute__((aligned((64U)))); | |||
|
|
|||
| static struct mg_tcpip_if *s_ifp; // MIP interface | |||
|
|
|||
| enum { PHY_ADDR = 2, PHY_BCR = 0, PHY_BSR = 1, PHY_PC1R = 30, PHY_PC2R = 31 }; | |||
| enum { PHY_BCR = 0, PHY_BSR = 1, PHY_ID2 = 2, PHY_ID3 = 3 }; | |||
There was a problem hiding this comment.
proper register names are ID1 and ID2
src/drivers/rt1020.c
Outdated
| uint16_t phy_id2 = rt10xx_phy_read(d->phy_addr, PHY_ID2); | ||
| uint16_t phy_id3 = rt10xx_phy_read(d->phy_addr, PHY_ID3); | ||
| MG_INFO(("PHY ID: %04x %04x", phy_id2, phy_id3)); | ||
| // 2000 a140 - TI DP83825I | ||
| // 0007 c0fx - LAN8720 | ||
| // 0022 1561 - KSZ8081RNB | ||
|
|
||
| if (phy_id2 == 0x22) { // KSZ8081RNB, like EVK-RTxxxx boards | ||
| rt10xx_phy_write(d->phy_addr, 31, BIT(15) | BIT(8) | BIT(7)); // PC2R | ||
| // PHY_PC1R = 30, PHY_PC2R = 31 | ||
| } else if (phy_id2 == 0x2000) { // DP83825I, like Teensy4.1 | ||
| rt10xx_phy_write(d->phy_addr, 23, 0x81); // 50Mhz clock input | ||
| rt10xx_phy_write(d->phy_addr, 24, 0x280); // LED status, active high | ||
| } else { // Default to LAN8720 | ||
| MG_INFO(("Defauling to LAN8720 PHY...")); | ||
| } |
There was a problem hiding this comment.
We should store a PHY ID on ifp or mif, as this will be used later and we should avoid asking and deciding again
Actions here, are "init" and will be done by all drivers (with non built-in PHY), though each one will have its own way to access the PHY.
There was a problem hiding this comment.
Specifically here, the "up" function is called infrequently so it's not a big deal to query the ID.
Also, PHY is kinda Ethernet specific..
Let's for time being keep it this way, to keep things as loosely coupled as possible.
If we have to optimise in the future - let's think then..
src/drivers/rt1020.c
Outdated
| // uint8_t pc2r = d->phy_pc2r; | ||
| // if (pc2r == 0) pc2r = PHY_PC2R; | ||
| // rt10xx_phy_write(d->phy_addr, pc2r, BIT(15) | BIT(8) | BIT(7)); | ||
| // if (d->phy_ledcr) rt10xx_phy_write(d->phy_addr, d->phy_ledcr, 0x280); | ||
|
|
src/drivers/rt1020.c
Outdated
| uint32_t tcr = ENET->TCR |= BIT(2); // Full-duplex | ||
| uint32_t rcr = ENET->RCR &= ~BIT(9); // 100M | ||
| if (rt10xx_phy_read(d->phy_addr, PHY_ID2)) { // KSZ8081RNB ? | ||
| uint32_t pc1r = rt10xx_phy_read(d->phy_addr, 30); // Read PC2R | ||
| if ((pc1r & 3) == 1) rcr |= BIT(9); // 10M | ||
| if ((pc1r & BIT(2)) == 0) tcr &= ~BIT(2); // Half-duplex | ||
| } |
There was a problem hiding this comment.
Here we must check PHY ID again. In fact, this code, as is, will not run at 10baseT nor half-duplex on the Teensy.
For the DP83825, the register to read in order to get this info is PHYSTS (Offset = 0x10)
Actions here, are "up" and will be done by all drivers (with non built-in PHY), though each one will have its own way to access the PHY.
There was a problem hiding this comment.
Ack. I'll leave this for you!
No description provided.