On Mon, Sep 28, 2020 at 05:11:55PM -0700, Scott Branden wrote:
> Add initial version of Broadcom VK driver to enumerate PCI device IDs
> of Valkyrie and Viper device IDs.
> 
> VK based cards provide real-time high performance, high throughput,
> low latency offload compute engine operations.
> They are used for multiple parallel offload tasks as:
> audio, video and image processing and crypto operations.

As we have had this come up for other offload engine devices, we need a
pointer to the userspace code for this as well, so that we can properly
review the whole thing together.  Otherwise this is just one half, the
other is a black-box that is pretty impossible to understand :)


> 
> Further commits add additional features to driver beyond probe/remove.
> 
> Signed-off-by: Scott Branden <scott.bran...@broadcom.com>
> ---
>  drivers/misc/Kconfig             |   1 +
>  drivers/misc/Makefile            |   1 +
>  drivers/misc/bcm-vk/Kconfig      |  15 ++++
>  drivers/misc/bcm-vk/Makefile     |   8 ++
>  drivers/misc/bcm-vk/bcm_vk.h     |  29 +++++++
>  drivers/misc/bcm-vk/bcm_vk_dev.c | 137 +++++++++++++++++++++++++++++++
>  6 files changed, 191 insertions(+)
>  create mode 100644 drivers/misc/bcm-vk/Kconfig
>  create mode 100644 drivers/misc/bcm-vk/Makefile
>  create mode 100644 drivers/misc/bcm-vk/bcm_vk.h
>  create mode 100644 drivers/misc/bcm-vk/bcm_vk_dev.c
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index ce136d685d14..9d42b5def81b 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -469,6 +469,7 @@ source "drivers/misc/genwqe/Kconfig"
>  source "drivers/misc/echo/Kconfig"
>  source "drivers/misc/cxl/Kconfig"
>  source "drivers/misc/ocxl/Kconfig"
> +source "drivers/misc/bcm-vk/Kconfig"
>  source "drivers/misc/cardreader/Kconfig"
>  source "drivers/misc/habanalabs/Kconfig"
>  source "drivers/misc/uacce/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index c7bd01ac6291..766837e4b961 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_ECHO)          += echo/
>  obj-$(CONFIG_CXL_BASE)               += cxl/
>  obj-$(CONFIG_PCI_ENDPOINT_TEST)      += pci_endpoint_test.o
>  obj-$(CONFIG_OCXL)           += ocxl/
> +obj-$(CONFIG_BCM_VK)         += bcm-vk/
>  obj-y                                += cardreader/
>  obj-$(CONFIG_PVPANIC)        += pvpanic.o
>  obj-$(CONFIG_HABANA_AI)              += habanalabs/
> diff --git a/drivers/misc/bcm-vk/Kconfig b/drivers/misc/bcm-vk/Kconfig
> new file mode 100644
> index 000000000000..2272e47655ed
> --- /dev/null
> +++ b/drivers/misc/bcm-vk/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Broadcom VK device
> +#
> +config BCM_VK
> +     tristate "Support for Broadcom VK Accelerators"
> +     depends on PCI_MSI
> +     help
> +       Select this option to enable support for Broadcom
> +       VK Accelerators.  VK is used for performing
> +       specific offload processing.
> +       This driver enables userspace programs to access these
> +       accelerators via /dev/bcm-vk.N devices.
> +
> +       If unsure, say N.
> diff --git a/drivers/misc/bcm-vk/Makefile b/drivers/misc/bcm-vk/Makefile
> new file mode 100644
> index 000000000000..f8a7ac4c242f
> --- /dev/null
> +++ b/drivers/misc/bcm-vk/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for Broadcom VK driver
> +#
> +
> +obj-$(CONFIG_BCM_VK) += bcm_vk.o
> +bcm_vk-objs := \
> +     bcm_vk_dev.o
> diff --git a/drivers/misc/bcm-vk/bcm_vk.h b/drivers/misc/bcm-vk/bcm_vk.h
> new file mode 100644
> index 000000000000..9152785199ab
> --- /dev/null
> +++ b/drivers/misc/bcm-vk/bcm_vk.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2018-2020 Broadcom.
> + */
> +
> +#ifndef BCM_VK_H
> +#define BCM_VK_H
> +
> +#include <linux/pci.h>
> +
> +#define DRV_MODULE_NAME              "bcm-vk"
> +
> +/* VK device supports a maximum of 3 bars */
> +#define MAX_BAR      3
> +
> +enum pci_barno {
> +     BAR_0 = 0,
> +     BAR_1,
> +     BAR_2
> +};
> +
> +#define BCM_VK_NUM_TTY 2
> +
> +struct bcm_vk {
> +     struct pci_dev *pdev;
> +     void __iomem *bar[MAX_BAR];
> +};
> +
> +#endif
> diff --git a/drivers/misc/bcm-vk/bcm_vk_dev.c 
> b/drivers/misc/bcm-vk/bcm_vk_dev.c
> new file mode 100644
> index 000000000000..bb24efb1b9fb
> --- /dev/null
> +++ b/drivers/misc/bcm-vk/bcm_vk_dev.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2018-2020 Broadcom.
> + */
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/pci_regs.h>
> +
> +#include "bcm_vk.h"
> +
> +#define PCI_DEVICE_ID_VALKYRIE       0x5e87
> +#define PCI_DEVICE_ID_VIPER  0x5e88
> +
> +/* MSIX usages */
> +#define VK_MSIX_MSGQ_MAX             3
> +#define VK_MSIX_NOTF_MAX             1
> +#define VK_MSIX_TTY_MAX                      BCM_VK_NUM_TTY
> +#define VK_MSIX_IRQ_MAX                      (VK_MSIX_MSGQ_MAX + 
> VK_MSIX_NOTF_MAX + \
> +                                      VK_MSIX_TTY_MAX)
> +#define VK_MSIX_IRQ_MIN_REQ             (VK_MSIX_MSGQ_MAX + VK_MSIX_NOTF_MAX)
> +
> +/* Number of bits set in DMA mask*/
> +#define BCM_VK_DMA_BITS                      64
> +
> +static int bcm_vk_probe(struct pci_dev *pdev, const struct pci_device_id 
> *ent)
> +{
> +     int err;
> +     int i;
> +     int irq;
> +     struct bcm_vk *vk;
> +     struct device *dev = &pdev->dev;
> +
> +     vk = kzalloc(sizeof(*vk), GFP_KERNEL);
> +     if (!vk)
> +             return -ENOMEM;
> +
> +     err = pci_enable_device(pdev);
> +     if (err) {
> +             dev_err(dev, "Cannot enable PCI device\n");
> +             return err;

You just leaked memory :(

That was a short review, I didn't get all that far...

> +     }
> +     vk->pdev = pci_dev_get(pdev);
> +
> +     err = pci_request_regions(pdev, DRV_MODULE_NAME);
> +     if (err) {
> +             dev_err(dev, "Cannot obtain PCI resources\n");
> +             goto err_disable_pdev;
> +     }
> +
> +     /* make sure DMA is good */
> +     err = dma_set_mask_and_coherent(&pdev->dev,
> +                                     DMA_BIT_MASK(BCM_VK_DMA_BITS));
> +     if (err) {
> +             dev_err(dev, "failed to set DMA mask\n");
> +             goto err_disable_pdev;
> +     }
> +
> +     pci_set_master(pdev);
> +     pci_set_drvdata(pdev, vk);
> +
> +     irq = pci_alloc_irq_vectors(pdev,
> +                                 1,
> +                                 VK_MSIX_IRQ_MAX,
> +                                 PCI_IRQ_MSI | PCI_IRQ_MSIX);
> +
> +     if (irq < VK_MSIX_IRQ_MIN_REQ) {
> +             dev_err(dev, "failed to get min %d MSIX interrupts, irq(%d)\n",
> +                     VK_MSIX_IRQ_MIN_REQ, irq);
> +             err = (irq >= 0) ? -EINVAL : irq;
> +             goto err_disable_pdev;
> +     }
> +
> +     dev_info(dev, "Number of IRQs %d allocated - requested(%d).\n",
> +              irq, VK_MSIX_IRQ_MAX);

Drivers are quiet when all works properly.

> +
> +     for (i = 0; i < MAX_BAR; i++) {
> +             /* multiple by 2 for 64 bit BAR mapping */
> +             vk->bar[i] = pci_ioremap_bar(pdev, i * 2);
> +             if (!vk->bar[i]) {
> +                     dev_err(dev, "failed to remap BAR%d\n", i);
> +                     goto err_iounmap;
> +             }
> +     }
> +
> +     return 0;
> +
> +err_iounmap:
> +     for (i = 0; i < MAX_BAR; i++) {
> +             if (vk->bar[i])
> +                     pci_iounmap(pdev, vk->bar[i]);
> +     }
> +     pci_release_regions(pdev);
> +
> +err_disable_pdev:
> +     pci_free_irq_vectors(pdev);
> +     pci_disable_device(pdev);
> +     pci_dev_put(pdev);
> +
> +     return err;

Again, memory is leaked :(

thanks,

greg k-h

Reply via email to