Re: [PATCH v2] platform/mellanox: Add BlueField-3 support in the tmfifo driver

From: Hans de Goede
Date: Mon Nov 07 2022 - 07:55:25 EST


Hi,

On 10/18/22 15:33, Liming Sun wrote:
> BlueField-3 uses the same control registers in tmfifo access but
> at different addresses. This commit replaces the offset reference
> with pointers, and set up these pointers in the probe functions
> accordingly.
>
> Signed-off-by: Liming Sun <limings@xxxxxxxxxx>
> Reviewed-by: David Thompson <davthompson@xxxxxxxxxx>
> Reviewed-by: Vadim Pasternak <vadimp@xxxxxxxxxx>

Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> ---
> v1->v2:
> Resend with updated reviewer list
> v1: Initial version
> ---
> drivers/platform/mellanox/mlxbf-tmfifo-regs.h | 10 +++
> drivers/platform/mellanox/mlxbf-tmfifo.c | 86 ++++++++++++++-----
> 2 files changed, 74 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/platform/mellanox/mlxbf-tmfifo-regs.h b/drivers/platform/mellanox/mlxbf-tmfifo-regs.h
> index e4f0d2eda714..44fb8c5b1484 100644
> --- a/drivers/platform/mellanox/mlxbf-tmfifo-regs.h
> +++ b/drivers/platform/mellanox/mlxbf-tmfifo-regs.h
> @@ -60,4 +60,14 @@
> #define MLXBF_TMFIFO_RX_CTL__MAX_ENTRIES_RMASK GENMASK_ULL(8, 0)
> #define MLXBF_TMFIFO_RX_CTL__MAX_ENTRIES_MASK GENMASK_ULL(40, 32)
>
> +/* BF3 register offsets within resource 0. */
> +#define MLXBF_TMFIFO_RX_DATA_BF3 0x0000
> +#define MLXBF_TMFIFO_TX_DATA_BF3 0x1000
> +
> +/* BF3 register offsets within resource 1. */
> +#define MLXBF_TMFIFO_RX_STS_BF3 0x0000
> +#define MLXBF_TMFIFO_RX_CTL_BF3 0x0008
> +#define MLXBF_TMFIFO_TX_STS_BF3 0x0100
> +#define MLXBF_TMFIFO_TX_CTL_BF3 0x0108
> +
> #endif /* !defined(__MLXBF_TMFIFO_REGS_H__) */
> diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c b/drivers/platform/mellanox/mlxbf-tmfifo.c
> index 1ae3c56b66b0..91a077c35b8b 100644
> --- a/drivers/platform/mellanox/mlxbf-tmfifo.c
> +++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
> @@ -47,6 +47,9 @@
> /* Message with data needs at least two words (for header & data). */
> #define MLXBF_TMFIFO_DATA_MIN_WORDS 2
>
> +/* ACPI UID for BlueField-3. */
> +#define TMFIFO_BF3_UID 1
> +
> struct mlxbf_tmfifo;
>
> /**
> @@ -136,12 +139,26 @@ struct mlxbf_tmfifo_irq_info {
> int index;
> };
>
> +/**
> + * mlxbf_tmfifo_io - Structure of the TmFifo IO resource (for both rx & tx)
> + * @ctl: control register offset (TMFIFO_RX_CTL / TMFIFO_TX_CTL)
> + * @sts: status register offset (TMFIFO_RX_STS / TMFIFO_TX_STS)
> + * @data: data register offset (TMFIFO_RX_DATA / TMFIFO_TX_DATA)
> + */
> +struct mlxbf_tmfifo_io {
> + void __iomem *ctl;
> + void __iomem *sts;
> + void __iomem *data;
> +};
> +
> /**
> * mlxbf_tmfifo - Structure of the TmFifo
> * @vdev: array of the virtual devices running over the TmFifo
> * @lock: lock to protect the TmFifo access
> - * @rx_base: mapped register base address for the Rx FIFO
> - * @tx_base: mapped register base address for the Tx FIFO
> + * @res0: mapped resource block 0
> + * @res1: mapped resource block 1
> + * @rx: rx io resource
> + * @tx: tx io resource
> * @rx_fifo_size: number of entries of the Rx FIFO
> * @tx_fifo_size: number of entries of the Tx FIFO
> * @pend_events: pending bits for deferred events
> @@ -155,8 +172,10 @@ struct mlxbf_tmfifo_irq_info {
> struct mlxbf_tmfifo {
> struct mlxbf_tmfifo_vdev *vdev[MLXBF_TMFIFO_VDEV_MAX];
> struct mutex lock; /* TmFifo lock */
> - void __iomem *rx_base;
> - void __iomem *tx_base;
> + void __iomem *res0;
> + void __iomem *res1;
> + struct mlxbf_tmfifo_io rx;
> + struct mlxbf_tmfifo_io tx;
> int rx_fifo_size;
> int tx_fifo_size;
> unsigned long pend_events;
> @@ -472,7 +491,7 @@ static int mlxbf_tmfifo_get_rx_avail(struct mlxbf_tmfifo *fifo)
> {
> u64 sts;
>
> - sts = readq(fifo->rx_base + MLXBF_TMFIFO_RX_STS);
> + sts = readq(fifo->rx.sts);
> return FIELD_GET(MLXBF_TMFIFO_RX_STS__COUNT_MASK, sts);
> }
>
> @@ -489,7 +508,7 @@ static int mlxbf_tmfifo_get_tx_avail(struct mlxbf_tmfifo *fifo, int vdev_id)
> else
> tx_reserve = 1;
>
> - sts = readq(fifo->tx_base + MLXBF_TMFIFO_TX_STS);
> + sts = readq(fifo->tx.sts);
> count = FIELD_GET(MLXBF_TMFIFO_TX_STS__COUNT_MASK, sts);
> return fifo->tx_fifo_size - tx_reserve - count;
> }
> @@ -525,7 +544,7 @@ static void mlxbf_tmfifo_console_tx(struct mlxbf_tmfifo *fifo, int avail)
> /* Write header. */
> hdr.type = VIRTIO_ID_CONSOLE;
> hdr.len = htons(size);
> - writeq(*(u64 *)&hdr, fifo->tx_base + MLXBF_TMFIFO_TX_DATA);
> + writeq(*(u64 *)&hdr, fifo->tx.data);
>
> /* Use spin-lock to protect the 'cons->tx_buf'. */
> spin_lock_irqsave(&fifo->spin_lock[0], flags);
> @@ -542,7 +561,7 @@ static void mlxbf_tmfifo_console_tx(struct mlxbf_tmfifo *fifo, int avail)
> memcpy((u8 *)&data + seg, cons->tx_buf.buf,
> sizeof(u64) - seg);
> }
> - writeq(data, fifo->tx_base + MLXBF_TMFIFO_TX_DATA);
> + writeq(data, fifo->tx.data);
>
> if (size >= sizeof(u64)) {
> cons->tx_buf.tail = (cons->tx_buf.tail + sizeof(u64)) %
> @@ -573,7 +592,7 @@ static void mlxbf_tmfifo_rxtx_word(struct mlxbf_tmfifo_vring *vring,
>
> /* Read a word from FIFO for Rx. */
> if (is_rx)
> - data = readq(fifo->rx_base + MLXBF_TMFIFO_RX_DATA);
> + data = readq(fifo->rx.data);
>
> if (vring->cur_len + sizeof(u64) <= len) {
> /* The whole word. */
> @@ -595,7 +614,7 @@ static void mlxbf_tmfifo_rxtx_word(struct mlxbf_tmfifo_vring *vring,
>
> /* Write the word into FIFO for Tx. */
> if (!is_rx)
> - writeq(data, fifo->tx_base + MLXBF_TMFIFO_TX_DATA);
> + writeq(data, fifo->tx.data);
> }
>
> /*
> @@ -617,7 +636,7 @@ static void mlxbf_tmfifo_rxtx_header(struct mlxbf_tmfifo_vring *vring,
> /* Read/Write packet header. */
> if (is_rx) {
> /* Drain one word from the FIFO. */
> - *(u64 *)&hdr = readq(fifo->rx_base + MLXBF_TMFIFO_RX_DATA);
> + *(u64 *)&hdr = readq(fifo->rx.data);
>
> /* Skip the length 0 packets (keepalive). */
> if (hdr.len == 0)
> @@ -661,7 +680,7 @@ static void mlxbf_tmfifo_rxtx_header(struct mlxbf_tmfifo_vring *vring,
> hdr.type = (vring->vdev_id == VIRTIO_ID_NET) ?
> VIRTIO_ID_NET : VIRTIO_ID_CONSOLE;
> hdr.len = htons(vring->pkt_len - hdr_len);
> - writeq(*(u64 *)&hdr, fifo->tx_base + MLXBF_TMFIFO_TX_DATA);
> + writeq(*(u64 *)&hdr, fifo->tx.data);
> }
>
> vring->cur_len = hdr_len;
> @@ -1157,7 +1176,7 @@ static void mlxbf_tmfifo_set_threshold(struct mlxbf_tmfifo *fifo)
> u64 ctl;
>
> /* Get Tx FIFO size and set the low/high watermark. */
> - ctl = readq(fifo->tx_base + MLXBF_TMFIFO_TX_CTL);
> + ctl = readq(fifo->tx.ctl);
> fifo->tx_fifo_size =
> FIELD_GET(MLXBF_TMFIFO_TX_CTL__MAX_ENTRIES_MASK, ctl);
> ctl = (ctl & ~MLXBF_TMFIFO_TX_CTL__LWM_MASK) |
> @@ -1166,17 +1185,17 @@ static void mlxbf_tmfifo_set_threshold(struct mlxbf_tmfifo *fifo)
> ctl = (ctl & ~MLXBF_TMFIFO_TX_CTL__HWM_MASK) |
> FIELD_PREP(MLXBF_TMFIFO_TX_CTL__HWM_MASK,
> fifo->tx_fifo_size - 1);
> - writeq(ctl, fifo->tx_base + MLXBF_TMFIFO_TX_CTL);
> + writeq(ctl, fifo->tx.ctl);
>
> /* Get Rx FIFO size and set the low/high watermark. */
> - ctl = readq(fifo->rx_base + MLXBF_TMFIFO_RX_CTL);
> + ctl = readq(fifo->rx.ctl);
> fifo->rx_fifo_size =
> FIELD_GET(MLXBF_TMFIFO_RX_CTL__MAX_ENTRIES_MASK, ctl);
> ctl = (ctl & ~MLXBF_TMFIFO_RX_CTL__LWM_MASK) |
> FIELD_PREP(MLXBF_TMFIFO_RX_CTL__LWM_MASK, 0);
> ctl = (ctl & ~MLXBF_TMFIFO_RX_CTL__HWM_MASK) |
> FIELD_PREP(MLXBF_TMFIFO_RX_CTL__HWM_MASK, 1);
> - writeq(ctl, fifo->rx_base + MLXBF_TMFIFO_RX_CTL);
> + writeq(ctl, fifo->rx.ctl);
> }
>
> static void mlxbf_tmfifo_cleanup(struct mlxbf_tmfifo *fifo)
> @@ -1197,8 +1216,15 @@ static int mlxbf_tmfifo_probe(struct platform_device *pdev)
> struct virtio_net_config net_config;
> struct device *dev = &pdev->dev;
> struct mlxbf_tmfifo *fifo;
> + u64 dev_id;
> int i, rc;
>
> + rc = acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &dev_id);
> + if (rc) {
> + dev_err(dev, "Cannot retrieve UID\n");
> + return rc;
> + }
> +
> fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
> if (!fifo)
> return -ENOMEM;
> @@ -1209,14 +1235,30 @@ static int mlxbf_tmfifo_probe(struct platform_device *pdev)
> mutex_init(&fifo->lock);
>
> /* Get the resource of the Rx FIFO. */
> - fifo->rx_base = devm_platform_ioremap_resource(pdev, 0);
> - if (IS_ERR(fifo->rx_base))
> - return PTR_ERR(fifo->rx_base);
> + fifo->res0 = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(fifo->res0))
> + return PTR_ERR(fifo->res0);
>
> /* Get the resource of the Tx FIFO. */
> - fifo->tx_base = devm_platform_ioremap_resource(pdev, 1);
> - if (IS_ERR(fifo->tx_base))
> - return PTR_ERR(fifo->tx_base);
> + fifo->res1 = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(fifo->res1))
> + return PTR_ERR(fifo->res1);
> +
> + if (dev_id == TMFIFO_BF3_UID) {
> + fifo->rx.ctl = fifo->res1 + MLXBF_TMFIFO_RX_CTL_BF3;
> + fifo->rx.sts = fifo->res1 + MLXBF_TMFIFO_RX_STS_BF3;
> + fifo->rx.data = fifo->res0 + MLXBF_TMFIFO_RX_DATA_BF3;
> + fifo->tx.ctl = fifo->res1 + MLXBF_TMFIFO_TX_CTL_BF3;
> + fifo->tx.sts = fifo->res1 + MLXBF_TMFIFO_TX_STS_BF3;
> + fifo->tx.data = fifo->res0 + MLXBF_TMFIFO_TX_DATA_BF3;
> + } else {
> + fifo->rx.ctl = fifo->res0 + MLXBF_TMFIFO_RX_CTL;
> + fifo->rx.sts = fifo->res0 + MLXBF_TMFIFO_RX_STS;
> + fifo->rx.data = fifo->res0 + MLXBF_TMFIFO_RX_DATA;
> + fifo->tx.ctl = fifo->res1 + MLXBF_TMFIFO_TX_CTL;
> + fifo->tx.sts = fifo->res1 + MLXBF_TMFIFO_TX_STS;
> + fifo->tx.data = fifo->res1 + MLXBF_TMFIFO_TX_DATA;
> + }
>
> platform_set_drvdata(pdev, fifo);
>