From: Simon Horman <horms@kernel.org>
To: Raju Rangoju <Raju.Rangoju@amd.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, Shyam-sundar.S-k@amd.com,
Sudheesh Mavila <sudheesh.mavila@amd.com>
Subject: Re: [PATCH net-next 2/2] amd-xgbe: Add support for AMD Crater ethernet device
Date: Thu, 19 Oct 2023 13:03:10 +0200 [thread overview]
Message-ID: <20231019110310.GB2100445@kernel.org> (raw)
In-Reply-To: <20231018144450.2061125-3-Raju.Rangoju@amd.com>
On Wed, Oct 18, 2023 at 08:14:50PM +0530, Raju Rangoju wrote:
> The AMD Crater device has new window settings for the XPCS access, add
> support to adopt to the new window settings. There is a hardware bug
> where in the BAR1 registers cannot be accessed directly. As a fallback
> mechanism, access these PCS registers through indirect access via SMN.
>
> Co-developed-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
> Signed-off-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
Hi Sudheesh,
some minor feedback from my side.
> ---
> drivers/net/ethernet/amd/xgbe/xgbe-common.h | 5 ++++
> drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 33 +++++++++++++++++----
> drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 32 +++++++++++++++-----
> drivers/net/ethernet/amd/xgbe/xgbe.h | 6 ++++
> 4 files changed, 63 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> index 3b70f6737633..e1f70f0528ef 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> @@ -900,6 +900,11 @@
> #define PCS_V2_RV_WINDOW_SELECT 0x1064
> #define PCS_V2_YC_WINDOW_DEF 0x18060
> #define PCS_V2_YC_WINDOW_SELECT 0x18064
> +#define PCS_V2_RN_WINDOW_DEF 0xF8078
> +#define PCS_V2_RN_WINDOW_SELECT 0xF807c
> +
> +#define PCS_RN_SMN_BASE_ADDR 0x11E00000
> +#define PCS_RN_PORT_ADDR_SIZE 0x100000
>
> /* PCS register entry bit positions and sizes */
> #define PCS_V2_WINDOW_DEF_OFFSET_INDEX 6
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> index f393228d41c7..da8ec218282f 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> @@ -1176,8 +1176,17 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
> offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>
> spin_lock_irqsave(&pdata->xpcs_lock, flags);
> - XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> - mmd_data = XPCS16_IOREAD(pdata, offset);
> + if (pdata->vdata->is_crater) {
> + amd_smn_write(0,
> + (pdata->xphy_base + pdata->xpcs_window_sel_reg),
> + index);
> + amd_smn_read(0, pdata->xphy_base + offset, &mmd_data);
> + mmd_data = (offset % ALIGNMENT_VAL) ?
> + ((mmd_data >> 16) & 0xffff) : (mmd_data & 0xffff);
I wonder if it would be nice to use FIELD_GET() here...
> + } else {
> + XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> + mmd_data = XPCS16_IOREAD(pdata, offset);
> + }
> spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
>
> return mmd_data;
> @@ -1186,8 +1195,8 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
> static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
> int mmd_reg, int mmd_data)
> {
> + unsigned int mmd_address, index, offset, crtr_mmd_data;
> unsigned long flags;
> - unsigned int mmd_address, index, offset;
>
> if (mmd_reg & XGBE_ADDR_C45)
> mmd_address = mmd_reg & ~XGBE_ADDR_C45;
> @@ -1208,8 +1217,22 @@ static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
> offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>
> spin_lock_irqsave(&pdata->xpcs_lock, flags);
> - XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> - XPCS16_IOWRITE(pdata, offset, mmd_data);
> + if (pdata->vdata->is_crater) {
> + amd_smn_write(0, (pdata->xphy_base + pdata->xpcs_window_sel_reg), index);
> + amd_smn_read(0, pdata->xphy_base + offset, &crtr_mmd_data);
> + if (offset % ALIGNMENT_VAL) {
> + crtr_mmd_data &= ~GENMASK(31, 16);
> + crtr_mmd_data |= (mmd_data << 16);
> + } else {
> + crtr_mmd_data &= ~GENMASK(15, 0);
> + crtr_mmd_data |= (mmd_data);
> + }
... and FIELD_PREP() here.
> + amd_smn_write(0, (pdata->xphy_base + pdata->xpcs_window_sel_reg), index);
> + amd_smn_write(0, (pdata->xphy_base + offset), crtr_mmd_data);
> + } else {
> + XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> + XPCS16_IOWRITE(pdata, offset, mmd_data);
> + }
> spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
> }
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> index a17359d43b45..90ad520d3c29 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> @@ -279,15 +279,21 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> pdata->xpcs_window_def_reg = PCS_V2_RV_WINDOW_DEF;
> pdata->xpcs_window_sel_reg = PCS_V2_RV_WINDOW_SELECT;
> } else if (rdev && (rdev->vendor == PCI_VENDOR_ID_AMD) &&
> - (rdev->device == 0x14b5)) {
> - pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
> - pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
> -
> - /* Yellow Carp devices do not need cdr workaround */
> + ((rdev->device == 0x14b5) || (rdev->device == 0x1630))) {
> + /* Yellow Carp and Crater devices
> + * do not need cdr workaround and RRC
> + */
> pdata->vdata->an_cdr_workaround = 0;
> -
> - /* Yellow Carp devices do not need rrc */
> pdata->vdata->enable_rrc = 0;
> +
> + if (rdev->device == 0x1630) {
Not strictly related to this patch, but I am wondering if we could create
#defines for magic numbers like this one.
> + pdata->xpcs_window_def_reg = PCS_V2_RN_WINDOW_DEF;
> + pdata->xpcs_window_sel_reg = PCS_V2_RN_WINDOW_SELECT;
> + pdata->vdata->is_crater = true;
Is 'is_crater' necessary?
pdata has a pointer to the pci_dev, AFAICT.
> + } else {
> + pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
> + pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
> + }
> } else {
> pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF;
> pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT;
> @@ -295,7 +301,17 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> pci_dev_put(rdev);
>
> /* Configure the PCS indirect addressing support */
> - reg = XPCS32_IOREAD(pdata, pdata->xpcs_window_def_reg);
> + if (pdata->vdata->is_crater) {
> + reg = XP_IOREAD(pdata, XP_PROP_0);
> + pdata->xphy_base = PCS_RN_SMN_BASE_ADDR +
> + (PCS_RN_PORT_ADDR_SIZE *
> + XP_GET_BITS(reg, XP_PROP_0, PORT_ID));
> + if (netif_msg_probe(pdata))
> + dev_dbg(dev, "xphy_base = %#08x\n", pdata->xphy_base);
> + amd_smn_read(0, pdata->xphy_base + (pdata->xpcs_window_def_reg), ®);
> + } else {
> + reg = XPCS32_IOREAD(pdata, pdata->xpcs_window_def_reg);
> + }
> pdata->xpcs_window = XPCS_GET_BITS(reg, PCS_V2_WINDOW_DEF, OFFSET);
> pdata->xpcs_window <<= 6;
> pdata->xpcs_window_size = XPCS_GET_BITS(reg, PCS_V2_WINDOW_DEF, SIZE);
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
> index ad136ed493ed..a161fac35643 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
> @@ -133,6 +133,7 @@
> #include <linux/dcache.h>
> #include <linux/ethtool.h>
> #include <linux/list.h>
> +#include <asm/amd_nb.h>
>
> #define XGBE_DRV_NAME "amd-xgbe"
> #define XGBE_DRV_DESC "AMD 10 Gigabit Ethernet Driver"
> @@ -305,6 +306,9 @@
> /* MDIO port types */
> #define XGMAC_MAX_C22_PORT 3
>
> + /* offset alignment */
> +#define ALIGNMENT_VAL 4
> +
> /* Link mode bit operations */
> #define XGBE_ZERO_SUP(_ls) \
> ethtool_link_ksettings_zero_link_mode((_ls), supported)
> @@ -1046,6 +1050,7 @@ struct xgbe_version_data {
> unsigned int rx_desc_prefetch;
> unsigned int an_cdr_workaround;
> unsigned int enable_rrc;
> + bool is_crater;
> };
>
> struct xgbe_prv_data {
> @@ -1056,6 +1061,7 @@ struct xgbe_prv_data {
> struct device *dev;
> struct platform_device *phy_platdev;
> struct device *phy_dev;
> + unsigned int xphy_base;
>
> /* Version related data */
> struct xgbe_version_data *vdata;
> --
> 2.25.1
>
>
next prev parent reply other threads:[~2023-10-19 11:03 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-18 14:44 [PATCH net-next 0/2] amd-xgbe: add support for AMD Crater Raju Rangoju
2023-10-18 14:44 ` [PATCH net-next 1/2] amd-xgbe: add support for new pci device id 0x1641 Raju Rangoju
2023-10-18 14:44 ` [PATCH net-next 2/2] amd-xgbe: Add support for AMD Crater ethernet device Raju Rangoju
2023-10-19 11:03 ` Simon Horman [this message]
2023-10-19 21:47 ` Tom Lendacky
2023-10-22 4:57 ` kernel test robot
2023-10-19 21:10 ` [PATCH net-next 0/2] amd-xgbe: add support for AMD Crater 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=20231019110310.GB2100445@kernel.org \
--to=horms@kernel.org \
--cc=Raju.Rangoju@amd.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sudheesh.mavila@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.