netdev - Re: [PATCH v1 1/2] net: phy: at803x: support qca8081 1G chip type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <68ec9a1e-81d3-93b9-d68f-47df07db8965@quicinc.com>
Date: Wed, 5 Jul 2023 16:35:28 +0800
From: Jie Luo <quic_luoj@...cinc.com>
To: Andrew Lunn <andrew@...n.ch>
CC: <hkallweit1@...il.com>, <linux@...linux.org.uk>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 1/2] net: phy: at803x: support qca8081 1G chip type



On 7/5/2023 7:24 AM, Andrew Lunn wrote:
> On Tue, Jul 04, 2023 at 05:00:15PM +0800, Luo Jie wrote:
>> The qca8081 1G chip version does not support 2.5 capability, which
>> is distinguished from qca8081 2.5G chip according to the bit0 of
>> register mmd7.0x901d.
>>
>> The fast retrain and master slave seed configs are only needed when
>> the 2.5G capability is supported.
>>
>> Switch to use genphy_c45_pma_read_abilities for .get_features API.
> 
> It is better to have lots of small patches, each doing one thing. If
> something regresses, a git bisect gives a much finer idea where the
> problem is. It is also easier to review small patches with good commit
> messages.
> 
> So please break this patch up.

Ok, i will split this patch to small patches, thanks for the suggestion.

> 
>> -	/* Configure lower ramdom seed to make phy linked as slave mode */
>> -	ret = qca808x_phy_ms_random_seed_set(phydev);
>> -	if (ret)
>> -		return ret;
>> +		/* Configure lower ramdom seed to make phy linked as slave mode */
>> +		ret = qca808x_phy_ms_random_seed_set(phydev);
>> +		if (ret)
>> +			return ret;
> 
> Shouldn't this depend on how MDIO_MMD_AN, MDIO_AN_T1_ADV_L bit
> MDIO_AN_T1_ADV_M_MST is set? Maybe the user wants it to prefer master
> rather than slave?
> 
> I know you are just trying to move code around, but it does seem like
> a good time to also improve the code.
> 
> FYI: net-next is closed at the moment. Officially you should post as
> RFC, or wait until it opens again.
> 
>    Andrew

Hi Andrew,
The master/slave configuration/status is only existed in MII reg9, 10 on 
qca8081 PHY, which is not existed in MDIO_MMD_AN or MDIO_MMD_PMAPMD, i 
will improve this code according to the user's prefer master/slave 
configuration in the next patch series.

Thanks for the reminder of close window, i will upload the patches after 
the open of next window.