-
Notifications
You must be signed in to change notification settings - Fork 16
Fix soc init #98
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?
Fix soc init #98
Conversation
Detect the SOC type and variant. RTL8372 vs RTL8373 and also is it as non-N/N variant of the SOC. Even if the machine profile is wrong the hardware will be initilised on the detected type.
Without this fix N-type SOC devices like RTL8372N, RTL8383N and also the 4-port PHY RTL8224N, don't have a functional Serdes. Although the SOC sees a link, there is no packet flow on both SFP-port nor RTL8224 ports. Added helper functions to read/write to the RTL8224. RTL8224 has the same register layout so we can use the same register defines as for the main SOC.
|
@feelfree69 , @TylerDurden-23 could you test this? I do not have one of those -N devices. |
|
@logicog Works as before - the issue with the 1000 Base-T module in slot 1 persists. |
logicog
left a comment
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.
This is truly impressive. The indirections to write the SDS registers of the RTL8224 give me a headache, but they looks all correct! I will test also with my non-n device and an RTL8371B.
rtl837x_phy.c
Outdated
|
|
||
| // Phy ID of the external RTL8224 PHY. | ||
| #define RTL8224_PHY_ID 0x00 | ||
| #define RTL8224_DEV_ID 0x1e |
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.
In phy.h is already a definition:
#define PHY_SDS_CTRL 30
This is also the name of that page in the RTL8221 datasheet, the RTL8224 is the Quad version of that.
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 like to rename it, because it is Vendor specific register.
In the linux kernel they call it https://elixir.bootlin.com/linux/v6.18.6/source/include/uapi/linux/mdio.h#L28
#define MDIO_MMD_VEND1 30 /* Vendor specific 1 */
#define MDIO_MMD_VEND2 31 /* Vendor specific 2 */We can use the same naming convention.
What do you think?
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 see that we are already using a simulair name
phy.h
14:#define PHY_MMD_PMAPMD 1
15:#define PHY_MMD_AN 7
17:#define PHY_MMD_CTRL 31
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.
The kernel calls it MDIO_MMD_VEND1 because it supports a dozen different PHY vendors who support MMD, so it is not clear what this page does for an arbitrary vendor. But the ones we will encounter will always be Realtek, they use always their own ecosystem, because they have their own variant of MDIO that supports up to 63 PHYs. The PHY_MMD_CTRL is based on what they call that page in their datasheets. What about PHY_MMD_SDS ?
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.
It is a vendor specific register so the vendor can do that he want, and also use a different layout for every chip.
PHY_MMD_SDS is valid for rtl8221 but not for rtl8224.
Looking into rtl8221-datasheet and it doesn't have the same layout as rtl8224.
So I am think some other options
PHY_MMD31How they also call these registersMMD 31PHY_MMD31_VEND2Make it more distinctive.PHY_MMD_1E,PHY_MMD1EIn hex
| write_char('N'); | ||
| } | ||
| write_char('\n'); | ||
| if (machine.isRTL8373 != machine_detected.isRTL8373) { |
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.
Do we still need machine.isRTL8373 if we anyway detect it? What about an RTL8371B?
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.
This provides an extra check to see if you have at least the right profile select that can fit on the SOC.
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.
But still how does the 8371 relates to the other chips?
Do we need add support for it of can it just use the RTL8373-settings?
Maybe we need at some extra documentation about this chip.
Sorry, my 8173 is also not an -N device. |
But still how does the 8173 relates to the other chips? Can you still test it to see if it still works? |
|
Sorry, typo. My switch is an ordinary 8+1 type with RTL8373. |
Remove the unused array `rtl8224_cb`. Rename `rtl8224_ca` to `rtl8224_sds0_setttings`.
|
Some new patches on the linux kernel malinglist about this issue. |
|
Looking at the SDK, the RTL8371B is just an older version of the RTL8372, probably a different PHY. The PHYs have a tendency to continuously evolve with time while keeping their basic performance characteristics. That is also why they have so many PHY patches in the SDK. This seems to solve interoperability issues with other PHYs. |
So demagic the `phy_(, 0x1f, )` to `phy_(, PHY_MMD31, )`.
So demagic the `phy_(, 0x1e, )` to `phy_(, PHY_MMD30, )`. Use `rg -tc phy | rg -i 0x1e` to find to most locations.
|
I tested this on the 2 Non-N devices MACHINE_KP_9000_6XHML_X2 and MACHINE_KP_9000_9XH_X_EU and everything seems to work, in particular the 10GBit SFP+ ports: However, My HG0402G V1.1 with an RTL8371B does not show any serial output, although the device seems to boot, and even brings up the SFP port. Probably this is unrelated... |
Fix the N-chips serdes swap issues.
Detect and init the chip based on detected type instead of machine profile.
Added helper functions to read/write to/from rtl8224.
rtl8224_write_reg_u16()andrtl8224_read_reg_u16copy ofphy_read()/phy_write().Without the copy, the compiler error-out because we are out-of-ram.
Now I don't use any additional ram.