From: Raju Rangoju <Raju.Rangoju@amd•com>
To: Tom Lendacky <thomas.lendacky@amd•com>,
Shyam-sundar.S-k@amd•com, davem@davemloft•net, kuba@kernel•org
Cc: netdev@vger•kernel.org, rrangoju@amd•com
Subject: Re: [PATCH net 2/3] amd-xgbe: enable PLL_CTL for fixed PHY modes only
Date: Thu, 6 Oct 2022 22:53:53 +0530 [thread overview]
Message-ID: <8a7c989d-1541-bb69-89b6-e18398ecaed4@amd.com> (raw)
In-Reply-To: <88a61eba-a779-96ae-8210-d31e73ed73a1@amd.com>
Hi Tom,
On 10/6/2022 8:17 PM, Tom Lendacky wrote:
> On 10/6/22 08:54, Raju Rangoju wrote:
>> PLL control setting(HW RRCM) is needed only in fixed PHY configuration
>> to fix the peer-peer issues. Without the PLL control setting, the link
>> up takes longer time in a fixed phy configuration.
>>
>> Driver implements SW RRCM for Autoneg On configuration, hence PLL
>> control setting (HW RRCM) is not needed for AN On configuration, and
>> can be skipped.
>>
>> Also, PLL re-initialization is not needed for PHY Power Off and RRCM
>> commands. Otherwise, they lead to mailbox errors. Added the changes
>> accordingly.
>>
>> Fixes: daf182d360e5 ("net: amd-xgbe: Toggle PLL settings during rate
>> change")
>> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd•com>
>> ---
>> drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 21 +++++++++++++--------
>> drivers/net/ethernet/amd/xgbe/xgbe.h | 10 ++++++++++
>> 2 files changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>> b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>> index 19b943eba560..23fbd89a29df 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>> @@ -1979,13 +1979,16 @@ static void xgbe_phy_rx_reset(struct
>> xgbe_prv_data *pdata)
>> static void xgbe_phy_pll_ctrl(struct xgbe_prv_data *pdata, bool enable)
>> {
>> - XMDIO_WRITE_BITS(pdata, MDIO_MMD_PMAPMD, MDIO_VEND2_PMA_MISC_CTRL0,
>> - XGBE_PMA_PLL_CTRL_MASK,
>> - enable ? XGBE_PMA_PLL_CTRL_ENABLE
>> - : XGBE_PMA_PLL_CTRL_DISABLE);
>> + /* PLL_CTRL feature needs to be enabled for fixed PHY modes
>> (Non-Autoneg) only */
>> + if (pdata->phy.autoneg == AUTONEG_DISABLE) {
>> + XMDIO_WRITE_BITS(pdata, MDIO_MMD_PMAPMD,
>> MDIO_VEND2_PMA_MISC_CTRL0,
>> + XGBE_PMA_PLL_CTRL_MASK,
>> + enable ? XGBE_PMA_PLL_CTRL_ENABLE
>> + : XGBE_PMA_PLL_CTRL_DISABLE);
>> - /* Wait for command to complete */
>> - usleep_range(100, 200);
>> + /* Wait for command to complete */
>> + usleep_range(100, 200);
>> + }
>
> Rather than indent all this, just add an if that returns at the beginning
> of the function, e.g.:
>
> /* PLL_CTRL feature needs to be enabled for fixed PHY modes
> (Non-Autoneg) only */
> if (pdata->phy.autoneg != AUTONEG_DISABLE)
> return;
Sure, I'll add the above changes.
>
> Now a general question... is this going to force Autoneg ON to end up
> always going through the RRC path now where it may not have before? In
> other words, there's now an auto-negotiation delay?
As per the databook, Receiver power state change (HW RRCM) is not
allowed during the CL73 / CL72. FW is already implementing SW RRCM for
Autoneg ON, and no additional changes are needed in driver for AN ON
path. However, HW RRCM(PLL_CTRL) is needed in Fixed PHY configs.
>
>> }
>> static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata,
>> @@ -2029,8 +2032,10 @@ static void xgbe_phy_perform_ratechange(struct
>> xgbe_prv_data *pdata,
>> xgbe_phy_rx_reset(pdata);
>> reenable_pll:
>> - /* Enable PLL re-initialization */
>> - xgbe_phy_pll_ctrl(pdata, true);
>> + /* Enable PLL re-initialization, not needed for PHY Power Off cmd */
>
> Comment should also include the RRC command...
>
>> + if (cmd != XGBE_MAILBOX_CMD_POWER_OFF &&
>> + cmd != XGBE_MAILBOX_CMD_RRCM)
>> + xgbe_phy_pll_ctrl(pdata, true);
>> }
>> static void xgbe_phy_rrc(struct xgbe_prv_data *pdata)
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h
>> b/drivers/net/ethernet/amd/xgbe/xgbe.h
>> index 49d23abce73d..c7865681790c 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe.h
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
>> @@ -611,6 +611,16 @@ enum xgbe_mdio_mode {
>> XGBE_MDIO_MODE_CL45,
>> };
>> +enum XGBE_MAILBOX_CMD {
>> + XGBE_MAILBOX_CMD_POWER_OFF = 0,
>> + XGBE_MAILBOX_CMD_SET_1G = 1,
>> + XGBE_MAILBOX_CMD_SET_2_5G = 2,
>> + XGBE_MAILBOX_CMD_SET_10G_SFI = 3,
>> + XGBE_MAILBOX_CMD_SET_10G_KR = 4,
>> + XGBE_MAILBOX_CMD_RRCM = 5,
>> + XGBE_MAILBOX_CMD_UNKNOWN = 6
>> +};
>
> If you're going to add an enum for the commands, then you should apply
> them everywhere. Creating this enum and updating all the command locations
> should be a pre-patch to this patch.
>
> Thanks,
> Tom
>
>> +
>> struct xgbe_phy {
>> struct ethtool_link_ksettings lks;
next prev parent reply other threads:[~2022-10-06 17:24 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-06 13:54 [PATCH net 0/3] amd-xgbe: Miscellaneous fixes Raju Rangoju
2022-10-06 13:54 ` [PATCH net 1/3] amd-xgbe: Yellow carp devices do not need rrc Raju Rangoju
2022-10-06 14:32 ` Tom Lendacky
2022-10-07 0:26 ` Jakub Kicinski
2022-10-08 18:27 ` Raju Rangoju
2022-10-06 13:54 ` [PATCH net 2/3] amd-xgbe: enable PLL_CTL for fixed PHY modes only Raju Rangoju
2022-10-06 14:47 ` Tom Lendacky
2022-10-06 17:23 ` Raju Rangoju [this message]
2022-10-06 13:54 ` [PATCH net 3/3] amd-xgbe: fix the SFP compliance codes check for DAC cables Raju Rangoju
2022-10-06 15:00 ` Tom Lendacky
[not found] ` <9a8552c2-0a23-ea1d-9d1c-56b7a6c75487@amd.com>
2022-10-06 19:04 ` Tom Lendacky
[not found] ` <77d8ea25-3557-cd6f-f732-20a046b29661@amd.com>
2022-10-07 14:27 ` Tom Lendacky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8a7c989d-1541-bb69-89b6-e18398ecaed4@amd.com \
--to=raju.rangoju@amd$(echo .)com \
--cc=Shyam-sundar.S-k@amd$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=rrangoju@amd$(echo .)com \
--cc=thomas.lendacky@amd$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox