Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Fri, 3 Nov 2023 14:01:13 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Luo Jie <quic_luoj@quicinc.com>
Cc:     hkallweit1@gmail.com, linux@armlinux.org.uk, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: phy: at803x: add QCA8084 ethernet phy support
Message-ID: <806fb6b6-d9b6-457b-b079-48f8b958cc5a@lunn.ch>
References: <20231103123538.15735-1-quic_luoj@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231103123538.15735-1-quic_luoj@quicinc.com>
Precedence: bulk

>  #define QCA8081_PHY_ID				0x004dd101
> +#define QCA8081_PHY_MASK			0xffffff00

That is an unusual mask. Please check it is correct. All you should
need its PHY_ID_MATCH_EXACT, PHY_ID_MATCH_MODEL, PHY_ID_MATCH_VENDOR.

> @@ -1767,6 +1781,20 @@ static int qca808x_config_init(struct phy_device *phydev)
>  {
>  	int ret;
>  
> +	if (phydev->phy_id == QCA8084_PHY_ID) {
> +		/* Invert ADC clock edge */
> +		ret = at803x_debug_reg_mask(phydev, QCA8084_ADC_CLK_SEL,
> +					    QCA8084_ADC_CLK_SEL_ACLK,
> +					    FIELD_PREP(QCA8084_ADC_CLK_SEL_ACLK,
> +						       QCA8084_ADC_CLK_SEL_ACLK_FALL));
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Adjust MSE threshold value to avoid link issue with some link partner */
> +		return phy_write_mmd(phydev, MDIO_MMD_PMAPMD,
> +				QCA8084_MSE_THRESHOLD, QCA8084_MSE_THRESHOLD_2P5G_VAL);
> +	}
> +

Please add a qca8084_config_init() and use that from the phy_driver
structure.

>  	/* Active adc&vga on 802.3az for the link 1000M and 100M */
>  	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_ADDR_CLD_CTRL7,
>  			QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
> @@ -1958,6 +1986,11 @@ static int qca808x_cable_test_start(struct phy_device *phydev)
>  	phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807a, 0xc060);
>  	phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807e, 0xb060);
>  
> +	if (phydev->phy_id == QCA8084_PHY_ID) {
> +		phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8075, 0xa060);
> +		phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807f, 0x1eb0);
> +	}
> +

Please add a comment what this is doing.

>  }, {
>  	/* Qualcomm QCA8081 */
> -	PHY_ID_MATCH_EXACT(QCA8081_PHY_ID),
> -	.name			= "Qualcomm QCA8081",
> +	.phy_id			= QCA8081_PHY_ID,
> +	.phy_id_mask		= QCA8081_PHY_MASK,
> +	.name			= "Qualcomm QCA808X",

Please add a new entry for the 8084. 

       Andrew