Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0f3990de-7c72-99d8-5a93-3b7eaa066e49@quicinc.com>
Date:   Fri, 30 Jun 2023 14:54:02 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH 3/3] net: phy: at803x: add qca8081 fifo reset on the link
 down
Content-Language: en-US
To:     Andrew Lunn <andrew@lunn.ch>
CC:     <hkallweit1@gmail.com>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <linux@armlinux.org.uk>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20230629034846.30600-1-quic_luoj@quicinc.com>
 <20230629034846.30600-4-quic_luoj@quicinc.com>
 <e1cf3666-fecc-4272-b91b-5921ada45ade@lunn.ch>
From:   Jie Luo <quic_luoj@quicinc.com>
In-Reply-To: <e1cf3666-fecc-4272-b91b-5921ada45ade@lunn.ch>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 6/29/2023 9:23 PM, Andrew Lunn wrote:
>> +static int qca808x_fifo_reset(struct phy_device *phydev)
>> +{
>> +	/* Reset serdes fifo on link down, Release serdes fifo on link up,
>> +	 * the serdes address is phy address added by 1.
>> +	 */
>> +	return mdiobus_c45_modify_changed(phydev->mdio.bus, phydev->mdio.addr + 1,
>> +			MDIO_MMD_PMAPMD, QCA8081_PHY_SERDES_MMD1_FIFO_CTRL,
>> +			QCA8081_PHY_FIFO_RSTN, phydev->link ? QCA8081_PHY_FIFO_RSTN : 0);
> 
> In polling mode, this is going to be called once per second. Do you
> really want to be setting that register all the time? Consider using
> the link_change_notify callback.
> 
> Also, can you tell us more about this SERDES device on the bus. I just
> want to make sure this is not a PCS and should have its own driver.
> 
>       Andrew
Hi Andrew,
Thanks for the review.
yes, we can use the link_change_notify, since the fifo reset is needed 
on the link changed, i will update the patch to use link_change_notify.

SERDES device is the block converts data between serial data and 
parallel interfaces in each direction, which is the SGMII interface in 
qca8081 PHY, it's address is always the PHY address added by 1 in 
qca8081 PHY.