| Message ID | 20240329190224.3613471-9-jonas@kwiboo.se |
|---|---|
| State | Superseded |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | rockchip: rk35xx: Miscellaneous fixes and updates | expand |
Hi Jonas, On 3/29/24 20:01, Jonas Karlman wrote: > Imply support for GbE PHY status parsing and configuration when support > for onboard ethernet is enabled. > > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > --- > arch/arm/mach-rockchip/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig > index d518913f8a37..d85b59a92da2 100644 > --- a/arch/arm/mach-rockchip/Kconfig > +++ b/arch/arm/mach-rockchip/Kconfig > @@ -316,6 +316,7 @@ config ROCKCHIP_RK3568 > imply MISC_INIT_R > imply MMC_HS200_SUPPORT if MMC_SDHCI_ROCKCHIP > imply OF_LIBFDT_OVERLAY > + imply PHY_GIGE if DWC_ETH_QOS_ROCKCHIP Is this really something we expect most devices to use? We have two products based on RK3588, none use it. If I'm not mistaken, Rock5B doesn't as well as Orange Pi 5 Plus, RK3588 EVB, Rock5A, (likely not the Edgeble as well since they have 2.5Gbps connectors), NanoPC T6, IndieDroid Nova, Cool Pi 4B, Cool Pi CM5 EVB, NanoPi R6S, Rockchip Toybrick TB-RK3588X. So, I'm not sure it's worth it making it the default? (Even though we could remove it from the defconfig manually). Wouldn't this make more sense in your generic defconfigs? Cheers, Quentin
Hi Quentin, On 2024-04-02 13:11, Quentin Schulz wrote: > Hi Jonas, > > On 3/29/24 20:01, Jonas Karlman wrote: >> Imply support for GbE PHY status parsing and configuration when support >> for onboard ethernet is enabled. >> >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >> --- >> arch/arm/mach-rockchip/Kconfig | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig >> index d518913f8a37..d85b59a92da2 100644 >> --- a/arch/arm/mach-rockchip/Kconfig >> +++ b/arch/arm/mach-rockchip/Kconfig >> @@ -316,6 +316,7 @@ config ROCKCHIP_RK3568 >> imply MISC_INIT_R >> imply MMC_HS200_SUPPORT if MMC_SDHCI_ROCKCHIP >> imply OF_LIBFDT_OVERLAY >> + imply PHY_GIGE if DWC_ETH_QOS_ROCKCHIP > > Is this really something we expect most devices to use? > > We have two products based on RK3588, none use it. If I'm not mistaken, > Rock5B doesn't as well as Orange Pi 5 Plus, RK3588 EVB, Rock5A, (likely > not the Edgeble as well since they have 2.5Gbps connectors), NanoPC T6, > IndieDroid Nova, Cool Pi 4B, Cool Pi CM5 EVB, NanoPi R6S, Rockchip > Toybrick TB-RK3588X. > > So, I'm not sure it's worth it making it the default? (Even though we > could remove it from the defconfig manually). Wouldn't this make more > sense in your generic defconfigs? The PHY_GIGE option is only used to control if miiphy_speed() and miiphy_duplex() should use MII_STAT1000 reg to determine speed/duplex. This patch only imply this option if a board use on-board gmac and have DWC_ETH_QOS_ROCKCHIP enabled. Mostly this only help the "mii info" command to show 1000baseT instead of max 100baseT. I can drop this if you think it will cause an issue for any board? Regards, Jonas > > Cheers, > Quentin
Hi Jonas, On 4/2/24 14:54, Jonas Karlman wrote: > Hi Quentin, > > On 2024-04-02 13:11, Quentin Schulz wrote: >> Hi Jonas, >> >> On 3/29/24 20:01, Jonas Karlman wrote: >>> Imply support for GbE PHY status parsing and configuration when support >>> for onboard ethernet is enabled. >>> >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >>> --- >>> arch/arm/mach-rockchip/Kconfig | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig >>> index d518913f8a37..d85b59a92da2 100644 >>> --- a/arch/arm/mach-rockchip/Kconfig >>> +++ b/arch/arm/mach-rockchip/Kconfig >>> @@ -316,6 +316,7 @@ config ROCKCHIP_RK3568 >>> imply MISC_INIT_R >>> imply MMC_HS200_SUPPORT if MMC_SDHCI_ROCKCHIP >>> imply OF_LIBFDT_OVERLAY >>> + imply PHY_GIGE if DWC_ETH_QOS_ROCKCHIP >> >> Is this really something we expect most devices to use? >> >> We have two products based on RK3588, none use it. If I'm not mistaken, >> Rock5B doesn't as well as Orange Pi 5 Plus, RK3588 EVB, Rock5A, (likely >> not the Edgeble as well since they have 2.5Gbps connectors), NanoPC T6, >> IndieDroid Nova, Cool Pi 4B, Cool Pi CM5 EVB, NanoPi R6S, Rockchip >> Toybrick TB-RK3588X. >> >> So, I'm not sure it's worth it making it the default? (Even though we >> could remove it from the defconfig manually). Wouldn't this make more >> sense in your generic defconfigs? > > The PHY_GIGE option is only used to control if miiphy_speed() and > miiphy_duplex() should use MII_STAT1000 reg to determine speed/duplex. > > This patch only imply this option if a board use on-board gmac and have > DWC_ETH_QOS_ROCKCHIP enabled. > > Mostly this only help the "mii info" command to show 1000baseT instead > of max 100baseT. > > I can drop this if you think it will cause an issue for any board? > No no, this is fine thanks for the correction. I don't know what happened for my brain to read this as "enable support for integrated PHY by default" (c.f. the list of boards I listed not having integrated PHYs). Sorry for the noise, Reviewed-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> Thanks, Quentin
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index d518913f8a37..d85b59a92da2 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -316,6 +316,7 @@ config ROCKCHIP_RK3568 imply MISC_INIT_R imply MMC_HS200_SUPPORT if MMC_SDHCI_ROCKCHIP imply OF_LIBFDT_OVERLAY + imply PHY_GIGE if DWC_ETH_QOS_ROCKCHIP imply RNG_ROCKCHIP imply ROCKCHIP_COMMON_BOARD imply ROCKCHIP_OTP @@ -347,6 +348,7 @@ config ROCKCHIP_RK3588 imply MISC_INIT_R imply MMC_HS200_SUPPORT if MMC_SDHCI_ROCKCHIP imply OF_LIBFDT_OVERLAY + imply PHY_GIGE if DWC_ETH_QOS_ROCKCHIP imply RNG_ROCKCHIP imply ROCKCHIP_COMMON_BOARD imply ROCKCHIP_OTP
Imply support for GbE PHY status parsing and configuration when support for onboard ethernet is enabled. Signed-off-by: Jonas Karlman <jonas@kwiboo.se> --- arch/arm/mach-rockchip/Kconfig | 2 ++ 1 file changed, 2 insertions(+)