[PATCH 08/19] rockchip: rk35xx: Imply support for GbE PHY

[Quentin Schulz]

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 at 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 at theobroma-systems.com>

Thanks,
Quentin