-
Notifications
You must be signed in to change notification settings - Fork 7.6k
drivers: ethernet: phy: add driver for ksz9131, update ksz8081 with interrupt support #90711
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
base: main
Are you sure you want to change the base?
Conversation
Add an entry to https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/drivers/build_all/ethernet/app.overlay so it builds in CI. |
6944d51
to
5055226
Compare
@pdgendt An entry for ksz9131 added to |
@TonyHan11 pls add PR description and address the things from sonarqube |
@maass-hamburg updated with removing the unused property, renaming |
5055226
to
cfd8c64
Compare
1ac4658
to
e7778f7
Compare
@maass-hamburg
|
ffe8e46
to
94224d7
Compare
Resolve the CI error by rebasing the commits (without changes). |
94224d7
to
35d32e6
Compare
reg_val |= PHY_KSZ9131_ICS_LINK_UP_IE_MASK | PHY_KSZ9131_ICS_LINK_DOWN_IE_MASK; | ||
|
||
/* Write settings to Interrupt Control/Status register */ | ||
ret = ksz9131_write(dev, 27, reg_val); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use PHY_KSZ9131_ICS_REG macro instead of magic number
mutual_capabilities = anar & anlpar; | ||
if (mutual_capabilities & MII_ADVERTISE_100_FULL) { | ||
state->speed = LINK_FULL_100BASE; | ||
} else if (mutual_capabilities & MII_ADVERTISE_100_HALF) { | ||
state->speed = LINK_HALF_100BASE; | ||
} else if (mutual_capabilities & MII_ADVERTISE_10_FULL) { | ||
state->speed = LINK_FULL_10BASE; | ||
} else if (mutual_capabilities & MII_ADVERTISE_10_HALF) { | ||
state->speed = LINK_HALF_10BASE; | ||
} else { | ||
ret = -EIO; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no gigabit?
goto done; | ||
} | ||
|
||
gpio_init_callback(&data->gpio_callback, phy_mc_ksz8081_interrupt_handler, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gpio_init_callback(&data->gpio_callback, phy_mc_ksz8081_interrupt_handler, | |
ret = gpio_init_callback(&data->gpio_callback, phy_mc_ksz8081_interrupt_handler, |
35d32e6
to
98ac8d9
Compare
@kartben Thank you for your comments. updated with the following changes:
|
/* Read AUTO-NEGOTIATION MASTER SLAVE STATUS REGISTER */ | ||
ret = ksz9131_read(dev, PHY_KSZ9131_MSS_REG, &mssr); | ||
if (ret < 0) { | ||
goto done; | ||
} else if (mssr & PHY_KSZ9131_MSS_ADVERTISE_1000_FULL) { | ||
state->speed = LINK_FULL_1000BASE; | ||
goto done; | ||
} else if (mssr & PHY_KSZ9131_MSS_ADVERTISE_1000_HALF) { | ||
state->speed = LINK_HALF_1000BASE; | ||
goto done; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- these are generic registers, use the deines from mii.h for that
- you have to read the status and the controll register to get the mutal_capabilities, like with the 100/10 registers later. take a look at phy_mii.c how to do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your comments.
According to the datasheet, 1000Mbps is related with the following bits:
5.2.1 BASIC CONTROL REGISTER
(index MII_BMCR) bit 13 and bit 6
if (bmsr & MII_BMCR_SPEED_MASK == MII_BMCR_SPEED_1000)
5.2.10 AUTO-NEGOTIATION MASTER SLAVE CONTROL REGISTER
(index MII_1KTCR) bit 9 and bit 8
9 1000BASE-T Full Duplex
0 = advertise PHY is not 1000BASE-T full duplex capable
1 = advertise PHY is 1000BASE-T full duplex capable
8 1000BASE-T Half Duplex
0 = advertise PHY is not 1000BASE-T half duplex capable
1 = advertise PHY is 1000BASE-T half duplex capable
5.2.11 AUTO-NEGOTIATION MASTER SLAVE STATUS REGISTER
(index MII_1KSTSR) bit 11 and bit 10
11 Link Partner Advertised 1000BASE-T Full Duplex Capability
0 = Link Partner is not capable of 1000BASE-T full duplex
1 = Link Partner is capable of 1000BASE-T full duplex
10 Link Partner Advertised 1000BASE-T Half Duplex Capability
0 = Link Partner is not capable of 1000BASE-T half duplex
1 = Link Partner is capable of 1000BASE-T half duplex
If it is acceptable with the following? Thanks.
/* Read AUTO-NEGOTIATION MASTER SLAVE CONTROL REGISTER */
ret = ksz9131_read(dev, MII_1KTCR, &mscr);
if (ret < 0) {
goto done;
}
/* Read AUTO-NEGOTIATION MASTER SLAVE STATUS REGISTER */
ret = ksz9131_read(dev, MII_1KSTSR, &mssr);
if (ret < 0) {
goto done;
}
if (mscr & MII_ADVERTISE_1000_FULL) {
if (mssr & PHY_KSZ9131_MSS_ADVERTISE_1000_FULL) {
state->speed = LINK_FULL_1000BASE;
goto done;
}else if (mscr & MII_ADVERTISE_1000_HALF) {
if (mssr & PHY_KSZ9131_MSS_ADVERTISE_1000_HALF) {
state->speed = LINK_HALF_1000BASE;
goto done;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer, if you would reuse the code of the phy_mii.c
if (data->gigabit_supported) {
if (phy_mii_reg_read(dev, MII_1KTCR, &c1kt_reg) < 0) {
return -EIO;
}
if (phy_mii_reg_read(dev, MII_1KSTSR, &s1kt_reg) < 0) {
return -EIO;
}
s1kt_reg = (uint16_t)(s1kt_reg >> MII_1KSTSR_OFFSET);
}
if (data->gigabit_supported &&
((c1kt_reg & s1kt_reg & MII_ADVERTISE_1000_FULL) != 0U)) {
data->state.speed = LINK_FULL_1000BASE;
} else if (data->gigabit_supported &&
((c1kt_reg & s1kt_reg & MII_ADVERTISE_1000_HALF) != 0U)) {
data->state.speed = LINK_HALF_1000BASE;
and just shift the MII_1KSTSR
register by 2.
Btw good way to only read the 100/10 Mbit/s regs, when gigabit doesn't match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thanks.
#define PHY_KSZ9131_MSS_REG 10 | ||
#define PHY_KSZ9131_MSS_ADVERTISE_1000_FULL BIT(11) | ||
#define PHY_KSZ9131_MSS_ADVERTISE_1000_HALF BIT(10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unneded
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No suitable macros found in mii.h for these 2 bits. No sure if it's special for KSZ9131.
8892b25
to
6f9aced
Compare
27c6e59
to
5a9a2c6
Compare
Add support for KSZ9131 (Gigabit Ethernet Transceiver with RGMII Support). As first starter, 100MBit/s mode is tested. https://www.microchip.com/en-us/product/ksz9131 Signed-off-by: Tony Han <[email protected]>
Add build tests for Microchip KSZ9131. Signed-off-by: Tony Han <[email protected]>
Enable Link-Up and Link-Down interrupts. On the interrupt handling the monitor work is scheduled to update the link status and calling corresponding callback routine. Signed-off-by: Tony Han <[email protected]>
Enable Link-Up and Link-Down interrupts. On the interrupt handling the monitor work is scheduled to update the link status and calling corresponding callback routine. Signed-off-by: Tony Han <[email protected]>
Read gigabit status from Master Slave Status Register. Signed-off-by: Tony Han <[email protected]>
|
This PR includes: