| Message ID | 20221006135440.3680563-2-Raju.Rangoju@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | amd-xgbe: Miscellaneous fixes | expand |
On 10/6/22 08:54, Raju Rangoju wrote: > Yellow carp devices disables the CDR workaround path, > receiver reset cycle is not needed in such cases. > Hence, avoid issuing rrc on Yellow carp platforms. > > Fixes: 47f164deab22 ("amd-xgbe: Add PCI device support") That is the wrong Fixes: tag. Yellow Carp support was added with commit dbb6c58b5a61 ("net: amd-xgbe: Add Support for Yellow Carp Ethernet device") However, the changes to allow updating the version data were made with 6f60ecf233f9 ("net: amd-xgbe: Disable the CDR workaround path for Yellow Carp Devices") so that is the tag most likely needed should you want this to be able to go to stable. With a change to the Fixes: tag: Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com> > --- > drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 5 +++++ > drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 2 +- > drivers/net/ethernet/amd/xgbe/xgbe.h | 1 + > 3 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c > index 2af3da4b2d05..f409d7bd1f1e 100644 > --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c > @@ -285,6 +285,9 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > /* Yellow Carp devices do not need cdr workaround */ > pdata->vdata->an_cdr_workaround = 0; > + > + /* Yellow Carp devices do not need rrc */ > + pdata->vdata->enable_rrc = 0; > } else { > pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF; > pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT; > @@ -483,6 +486,7 @@ static struct xgbe_version_data xgbe_v2a = { > .tx_desc_prefetch = 5, > .rx_desc_prefetch = 5, > .an_cdr_workaround = 1, > + .enable_rrc = 1, > }; > > static struct xgbe_version_data xgbe_v2b = { > @@ -498,6 +502,7 @@ static struct xgbe_version_data xgbe_v2b = { > .tx_desc_prefetch = 5, > .rx_desc_prefetch = 5, > .an_cdr_workaround = 1, > + .enable_rrc = 1, > }; > > static const struct pci_device_id xgbe_pci_table[] = { > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c > index 2156600641b6..19b943eba560 100644 > --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c > @@ -2640,7 +2640,7 @@ static int xgbe_phy_link_status(struct xgbe_prv_data *pdata, int *an_restart) > } > > /* No link, attempt a receiver reset cycle */ > - if (phy_data->rrc_count++ > XGBE_RRC_FREQUENCY) { > + if (pdata->vdata->enable_rrc && phy_data->rrc_count++ > XGBE_RRC_FREQUENCY) { > phy_data->rrc_count = 0; > xgbe_phy_rrc(pdata); > } > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h > index b875c430222e..49d23abce73d 100644 > --- a/drivers/net/ethernet/amd/xgbe/xgbe.h > +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h > @@ -1013,6 +1013,7 @@ struct xgbe_version_data { > unsigned int tx_desc_prefetch; > unsigned int rx_desc_prefetch; > unsigned int an_cdr_workaround; > + unsigned int enable_rrc; > }; > > struct xgbe_prv_data {
On Thu, 6 Oct 2022 09:32:34 -0500 Tom Lendacky wrote: > On 10/6/22 08:54, Raju Rangoju wrote: > > Yellow carp devices disables the CDR workaround path, > > receiver reset cycle is not needed in such cases. > > Hence, avoid issuing rrc on Yellow carp platforms. Not entirely clear why this is a Fix, TBH. What harm comes from doing the reset? You need to describe the user-observable harm if the patch is a fix. > > Fixes: 47f164deab22 ("amd-xgbe: Add PCI device support") > > That is the wrong Fixes: tag. Yellow Carp support was added with commit > > dbb6c58b5a61 ("net: amd-xgbe: Add Support for Yellow Carp Ethernet device") > > However, the changes to allow updating the version data were made with > > 6f60ecf233f9 ("net: amd-xgbe: Disable the CDR workaround path for Yellow Carp Devices") > > so that is the tag most likely needed should you want this to be able to > go to stable. FWIW the Fixes tag should point to the commit where the bug is introduced, not where the patch will apply. The automation will figure out where it applies. > With a change to the Fixes: tag: > > Acked-by: Tom Lendacky <thomas.lendacky@amd.com> These devices are only present on SoCs? Changing global data during probe looks odd.
On 10/7/2022 5:56 AM, Jakub Kicinski wrote: > On Thu, 6 Oct 2022 09:32:34 -0500 Tom Lendacky wrote: >> On 10/6/22 08:54, Raju Rangoju wrote: >>> Yellow carp devices disables the CDR workaround path, >>> receiver reset cycle is not needed in such cases. >>> Hence, avoid issuing rrc on Yellow carp platforms. > > Not entirely clear why this is a Fix, TBH. > > What harm comes from doing the reset? You need to describe > the user-observable harm if the patch is a fix. Link stability issues are noticed if RRC is issued on Yellow carp platforms. Since the CDR workaround is disabled on these platforms, the Receiver Reset Cycle is not needed. > >>> Fixes: 47f164deab22 ("amd-xgbe: Add PCI device support") >> >> That is the wrong Fixes: tag. Yellow Carp support was added with commit >> >> dbb6c58b5a61 ("net: amd-xgbe: Add Support for Yellow Carp Ethernet device") >> >> However, the changes to allow updating the version data were made with >> >> 6f60ecf233f9 ("net: amd-xgbe: Disable the CDR workaround path for Yellow Carp Devices") >> >> so that is the tag most likely needed should you want this to be able to >> go to stable. > > FWIW the Fixes tag should point to the commit where the bug is > introduced, not where the patch will apply. The automation will > figure out where it applies. > >> With a change to the Fixes: tag: >> >> Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > > These devices are only present on SoCs? Changing global data during > probe looks odd. >
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c index 2af3da4b2d05..f409d7bd1f1e 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c @@ -285,6 +285,9 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) /* Yellow Carp devices do not need cdr workaround */ pdata->vdata->an_cdr_workaround = 0; + + /* Yellow Carp devices do not need rrc */ + pdata->vdata->enable_rrc = 0; } else { pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF; pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT; @@ -483,6 +486,7 @@ static struct xgbe_version_data xgbe_v2a = { .tx_desc_prefetch = 5, .rx_desc_prefetch = 5, .an_cdr_workaround = 1, + .enable_rrc = 1, }; static struct xgbe_version_data xgbe_v2b = { @@ -498,6 +502,7 @@ static struct xgbe_version_data xgbe_v2b = { .tx_desc_prefetch = 5, .rx_desc_prefetch = 5, .an_cdr_workaround = 1, + .enable_rrc = 1, }; static const struct pci_device_id xgbe_pci_table[] = { diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c index 2156600641b6..19b943eba560 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c @@ -2640,7 +2640,7 @@ static int xgbe_phy_link_status(struct xgbe_prv_data *pdata, int *an_restart) } /* No link, attempt a receiver reset cycle */ - if (phy_data->rrc_count++ > XGBE_RRC_FREQUENCY) { + if (pdata->vdata->enable_rrc && phy_data->rrc_count++ > XGBE_RRC_FREQUENCY) { phy_data->rrc_count = 0; xgbe_phy_rrc(pdata); } diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h index b875c430222e..49d23abce73d 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe.h +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h @@ -1013,6 +1013,7 @@ struct xgbe_version_data { unsigned int tx_desc_prefetch; unsigned int rx_desc_prefetch; unsigned int an_cdr_workaround; + unsigned int enable_rrc; }; struct xgbe_prv_data {
Yellow carp devices disables the CDR workaround path, receiver reset cycle is not needed in such cases. Hence, avoid issuing rrc on Yellow carp platforms. Fixes: 47f164deab22 ("amd-xgbe: Add PCI device support") Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com> --- drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 5 +++++ drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 2 +- drivers/net/ethernet/amd/xgbe/xgbe.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)