On Wed, Jan 23, 2019 at 02:00:43AM +0200, Oded Gabbay wrote:
> This patch adds the habanalabs skeleton driver. The driver does nothing at
> this stage except very basic operations. It contains the minimal code to
> insmod and rmmod the driver and to create a /dev/hlX file per PCI device.
> 
> Signed-off-by: Oded Gabbay <oded.gab...@gmail.com>
> ---
>  drivers/misc/Kconfig                          |   1 +
>  drivers/misc/Makefile                         |   1 +
>  drivers/misc/habanalabs/Kconfig               |  22 ++
>  drivers/misc/habanalabs/Makefile              |   7 +
>  drivers/misc/habanalabs/device.c              | 331 ++++++++++++++++
>  drivers/misc/habanalabs/habanalabs.h          | 149 +++++++
>  drivers/misc/habanalabs/habanalabs_drv.c      | 366 ++++++++++++++++++
>  .../habanalabs/include/habanalabs_device_if.h | 125 ++++++
>  8 files changed, 1002 insertions(+)
>  create mode 100644 drivers/misc/habanalabs/Kconfig
>  create mode 100644 drivers/misc/habanalabs/Makefile
>  create mode 100644 drivers/misc/habanalabs/device.c
>  create mode 100644 drivers/misc/habanalabs/habanalabs.h
>  create mode 100644 drivers/misc/habanalabs/habanalabs_drv.c
>  create mode 100644 drivers/misc/habanalabs/include/habanalabs_device_if.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f417b06e11c5..fecab53c4f21 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -535,4 +535,5 @@ source "drivers/misc/echo/Kconfig"
>  source "drivers/misc/cxl/Kconfig"
>  source "drivers/misc/ocxl/Kconfig"
>  source "drivers/misc/cardreader/Kconfig"
> +source "drivers/misc/habanalabs/Kconfig"
>  endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index e39ccbbc1b3a..ae77dfd790a4 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -59,3 +59,4 @@ obj-$(CONFIG_PCI_ENDPOINT_TEST)     += pci_endpoint_test.o
>  obj-$(CONFIG_OCXL)           += ocxl/
>  obj-y                                += cardreader/
>  obj-$(CONFIG_PVPANIC)        += pvpanic.o
> +obj-$(CONFIG_HABANA_AI)              += habanalabs/
> diff --git a/drivers/misc/habanalabs/Kconfig b/drivers/misc/habanalabs/Kconfig
> new file mode 100644
> index 000000000000..b7f38a14caf5
> --- /dev/null
> +++ b/drivers/misc/habanalabs/Kconfig
> @@ -0,0 +1,22 @@
> +#
> +# HabanaLabs AI accelerators driver
> +#
> +
> +config HABANA_AI
> +     tristate "HabanaAI accelerators (habanalabs)"
> +     depends on PCI
> +     select FRAME_VECTOR
> +     help
> +       Enables PCIe card driver for Habana's AI Processors (AIP) that are
> +       designed to accelerate Deep Learning inference and training workloads.
> +
> +       The driver manages the PCIe devices and provides IOCTL interface for
> +       the user to submit workloads to the devices.
> +
> +       The user-space interface is described in
> +       include/uapi/misc/habanalabs.h
> +
> +       If unsure, say N.
> +
> +       To compile this driver as a module, choose M here: the
> +       module will be called habanalabs.
> diff --git a/drivers/misc/habanalabs/Makefile 
> b/drivers/misc/habanalabs/Makefile
> new file mode 100644
> index 000000000000..b41433a09e02
> --- /dev/null
> +++ b/drivers/misc/habanalabs/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# Makefile for HabanaLabs AI accelerators driver
> +#
> +
> +obj-m        := habanalabs.o
> +
> +habanalabs-y := habanalabs_drv.o device.o
> \ No newline at end of file
> diff --git a/drivers/misc/habanalabs/device.c 
> b/drivers/misc/habanalabs/device.c
> new file mode 100644
> index 000000000000..376b55eb73d4
> --- /dev/null
> +++ b/drivers/misc/habanalabs/device.c
> @@ -0,0 +1,331 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2016-2018 HabanaLabs, Ltd.
> + * All Rights Reserved.
> + */
> +
> +#include "habanalabs.h"
> +
> +#include <linux/fs.h>
> +#include <linux/kthread.h>
> +#include <linux/sched/signal.h>
> +
> +static void hpriv_release(struct kref *ref)
> +{
> +     struct hl_fpriv *hpriv;
> +     struct hl_device *hdev;
> +
> +     hpriv = container_of(ref, struct hl_fpriv, refcount);
> +
> +     hdev = hpriv->hdev;
> +
> +     put_pid(hpriv->taskpid);
> +
> +     kfree(hpriv);
> +}
> +
> +void hl_hpriv_get(struct hl_fpriv *hpriv)
> +{
> +     kref_get(&hpriv->refcount);
> +}
> +
> +void hl_hpriv_put(struct hl_fpriv *hpriv)
> +{
> +     kref_put(&hpriv->refcount, hpriv_release);
> +}
> +
> +/**
> + * hl_device_release - release function for habanalabs device
> + *
> + * @inode: pointer to inode structure
> + * @filp: pointer to file structure
> + *
> + * Called when process closes an habanalabs device
> + */

It's nice to see docs coming along with the codei
I have some comments for the formatting.

kernel-doc won't be happy about missing return value descriptions, and
although they are sometimes redundant or too obvious their absence makes
'make V=1 htmldocs' really noisy.

In general, it would be nice if you could link hanabnalabs driver
kernel-doc somewhere in Documentation/ run 'make V=1 htmldocs'.

> +static int hl_device_release(struct inode *inode, struct file *filp)
> +{
> +     struct hl_fpriv *hpriv = filp->private_data;
> +
> +     filp->private_data = NULL;
> +
> +     hl_hpriv_put(hpriv);
> +
> +     return 0;
> +}
> +
> +static const struct file_operations hl_ops = {
> +     .owner = THIS_MODULE,
> +     .open = hl_device_open,
> +     .release = hl_device_release
> +};
> +
> +/**
> + * device_setup_cdev - setup cdev and device for habanalabs device
> + *
> + * @hdev: pointer to habanalabs device structure
> + * @hclass: pointer to the class object of the device
> + * @minor: minor number of the specific device
> + * @fpos : file operations to install for this device
> + *
> + * Create a cdev and a Linux device for habanalabs's device. Need to be
> + * called at the end of the habanalabs device initialization process,
> + * because this function exposes the device to the user
> + */
> +static int device_setup_cdev(struct hl_device *hdev, struct class *hclass,
> +                             int minor, const struct file_operations *fops)
> +{
> +     int err, devno = MKDEV(hdev->major, minor);
> +     struct cdev *hdev_cdev = &hdev->cdev;
> +     char name[8];
> +
> +     sprintf(name, "hl%d", hdev->id);
> +
> +     cdev_init(hdev_cdev, fops);
> +     hdev_cdev->owner = THIS_MODULE;
> +     err = cdev_add(hdev_cdev, devno, 1);
> +     if (err) {
> +             pr_err("habanalabs: Failed to add char device %s", name);
> +             goto err_cdev_add;
> +     }
> +
> +     hdev->dev = device_create(hclass, NULL, devno, NULL, "%s", name);
> +     if (IS_ERR(hdev->dev)) {
> +             pr_err("habanalabs: Failed to create device %s\n", name);
> +             err = PTR_ERR(hdev->dev);
> +             goto err_device_create;
> +     }
> +
> +     dev_set_drvdata(hdev->dev, hdev);
> +
> +     return 0;
> +
> +err_device_create:
> +     cdev_del(hdev_cdev);
> +err_cdev_add:
> +     return err;
> +}
> +
> +/**
> + * device_early_init - do some early initialization for the habanalabs device
> + *
> + * @hdev: pointer to habanalabs device structure
> + *
> + * Install the relevant function pointers and call the early_init function,
> + * if such a function exists
> + */
> +static int device_early_init(struct hl_device *hdev)
> +{
> +     switch (hdev->asic_type) {
> +     case ASIC_GOYA:
> +             sprintf(hdev->asic_name, "GOYA");
> +             break;
> +     default:
> +             dev_err(hdev->dev, "Unrecognized ASIC type %d\n",
> +                     hdev->asic_type);
> +             return -EINVAL;
> +     }
> +
> +     return 0;
> +}
> +
> +/**
> + * device_early_fini - finalize all that was done in device_early_fini

                                                                    ^init
> + *
> + * @hdev: pointer to habanalabs device structure
> + *
> + */
> +static void device_early_fini(struct hl_device *hdev)
> +{
> +}
> +
> +/**
> + * hl_device_suspend - initiate device suspend
> + *
> + * @hdev: pointer to habanalabs device structure
> + *
> + * Puts the hw in the suspend state (all asics).
> + * Returns 0 for success or an error on failure.

Should be Return: or Returns: for kernel-doc to understand it.

> + * Called at driver suspend.

This probably should be marked as Context:

> + */
> +int hl_device_suspend(struct hl_device *hdev)
> +{
> +     pci_save_state(hdev->pdev);
> +
> +     /* Shut down the device */
> +     pci_disable_device(hdev->pdev);
> +     pci_set_power_state(hdev->pdev, PCI_D3hot);
> +
> +     return 0;
> +}
> +
> +/**
> + * hl_device_resume - initiate device resume
> + *
> + * @hdev: pointer to habanalabs device structure
> + *
> + * Bring the hw back to operating state (all asics).
> + * Returns 0 for success or an error on failure.
> + * Called at driver resume.

Same comments as for the previous functions.

> + */
> +int hl_device_resume(struct hl_device *hdev)
> +{
> +     int rc;
> +
> +     pci_set_power_state(hdev->pdev, PCI_D0);
> +     pci_restore_state(hdev->pdev);
> +     rc = pci_enable_device(hdev->pdev);
> +     if (rc) {
> +             dev_err(hdev->dev,
> +                     "Failed to enable PCI device in resume\n");
> +             return rc;
> +     }
> +
> +     return 0;
> +}
> +
> +/**
> + * hl_device_init - main initialization function for habanalabs device
> + *
> + * @hdev: pointer to habanalabs device structure
> + *
> + * Allocate an id for the device, do early initialization and then call the
> + * ASIC specific initialization functions. Finally, create the cdev and the
> + * Linux device to expose it to the user
> + */
> +int hl_device_init(struct hl_device *hdev, struct class *hclass)
> +{
> +     int rc;
> +
> +     /* Create device */
> +     rc = device_setup_cdev(hdev, hclass, hdev->id, &hl_ops);
> +
> +     if (rc)
> +             goto out_disabled;
> +
> +     /* Initialize ASIC function pointers and perform early init */
> +     rc = device_early_init(hdev);
> +     if (rc)
> +             goto release_device;
> +
> +     dev_notice(hdev->dev,
> +             "Successfully added device to habanalabs driver\n");
> +
> +     return 0;
> +
> +release_device:
> +     device_destroy(hclass, hdev->dev->devt);
> +     cdev_del(&hdev->cdev);
> +out_disabled:
> +     hdev->disabled = true;
> +     if (hdev->pdev)
> +             dev_err(&hdev->pdev->dev,
> +                     "Failed to initialize hl%d. Device is NOT usable !!!\n",
> +                     hdev->id);
> +     else
> +             pr_err("habanalabs: Failed to initialize hl%d. Device is NOT 
> usable !!!\n",
> +                     hdev->id);

Maybe three exclamation marks would be too much?

> +
> +     return rc;
> +}
> +
> +/**
> + * hl_device_fini - main tear-down function for habanalabs device
> + *
> + * @hdev: pointer to habanalabs device structure
> + *
> + * Destroy the device, call ASIC fini functions and release the id
> + */
> +void hl_device_fini(struct hl_device *hdev)
> +{
> +     dev_info(hdev->dev, "Removing device\n");
> +
> +     /* Mark device as disabled */
> +     hdev->disabled = true;
> +
> +     device_early_fini(hdev);
> +
> +     /* Hide device from user */
> +     device_destroy(hdev->dev->class, hdev->dev->devt);
> +     cdev_del(&hdev->cdev);
> +
> +     pr_info("habanalabs: removed device successfully\n");
> +}
> +
> +/**
> + * hl_poll_timeout_memory - Periodically poll a host memory address
> + *                              until it is not zero or a timeout occurs
> + * @hdev: pointer to habanalabs device structure
> + * @addr: Address to poll
> + * @timeout_us: timeout in us
> + * @val: Variable to read the value into
> + *
> + * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
> + * case, the last read value at @addr is stored in @val. Must not
> + * be called from atomic context if sleep_us or timeout_us are used.
> + *
> + * The function sleeps for 100us with timeout value of
> + * timeout_us
> + */
> +int hl_poll_timeout_memory(struct hl_device *hdev, u64 addr,
> +                             u32 timeout_us, u32 *val)
> +{
> +     /*
> +      * pReturnVal is defined as volatile because it points to HOST memory,
> +      * which is being written to by the device. Therefore, we can't use
> +      * locks to synchronize it and it is not a memory-mapped register space
> +      */
> +     volatile u32 *pReturnVal = (volatile u32 *) addr;
> +     ktime_t timeout = ktime_add_us(ktime_get(), timeout_us);
> +
> +     might_sleep();
> +
> +     for (;;) {
> +             *val = *pReturnVal;
> +             if (*val)
> +                     break;
> +             if (ktime_compare(ktime_get(), timeout) > 0) {
> +                     *val = *pReturnVal;
> +                     break;
> +             }
> +             usleep_range((100 >> 2) + 1, 100);
> +     }
> +
> +     return (*val ? 0 : -ETIMEDOUT);
> +}
> +
> +/**
> + * hl_poll_timeout_devicememory - Periodically poll a device memory address
> + *                                until it is not zero or a timeout occurs
> + * @hdev: pointer to habanalabs device structure
> + * @addr: Device address to poll
> + * @timeout_us: timeout in us
> + * @val: Variable to read the value into
> + *
> + * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
> + * case, the last read value at @addr is stored in @val. Must not
> + * be called from atomic context if sleep_us or timeout_us are used.
> + *
> + * The function sleeps for 100us with timeout value of
> + * timeout_us
> + */
> +int hl_poll_timeout_device_memory(struct hl_device *hdev, void __iomem *addr,
> +                             u32 timeout_us, u32 *val)
> +{
> +     ktime_t timeout = ktime_add_us(ktime_get(), timeout_us);
> +
> +     might_sleep();
> +
> +     for (;;) {
> +             *val = readl(addr);
> +             if (*val)
> +                     break;
> +             if (ktime_compare(ktime_get(), timeout) > 0) {
> +                     *val = readl(addr);
> +                     break;
> +             }
> +             usleep_range((100 >> 2) + 1, 100);
> +     }
> +
> +     return (*val ? 0 : -ETIMEDOUT);
> +}
> diff --git a/drivers/misc/habanalabs/habanalabs.h 
> b/drivers/misc/habanalabs/habanalabs.h
> new file mode 100644
> index 000000000000..7e1b088b677c
> --- /dev/null
> +++ b/drivers/misc/habanalabs/habanalabs.h
> @@ -0,0 +1,149 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright 2016-2018 HabanaLabs, Ltd.
> + * All Rights Reserved.
> + *
> + */
> +
> +#ifndef HABANALABSP_H_
> +#define HABANALABSP_H_
> +
> +#include "include/habanalabs_device_if.h"
> +
> +#include <linux/pci.h>
> +#include <linux/types.h>
> +#include <linux/cdev.h>
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/dma-fence.h>
> +#include <linux/hashtable.h>
> +#include <linux/hwmon.h>
> +
> +#define HL_NAME                              "habanalabs"
> +
> +struct hl_device;
> +
> +
> +
> +
> +
> +

Too many blank lines, IMHO.

> +/*
> + * ASICs
> + */
> +
> +/**
> + * enum hl_asic_type - supported ASIC types.
> + * @ASIC_AUTO_DETECT: ASIC type will be automatically set.
> + * @ASIC_GOYA: Goya device.
> + * @ASIC_LAST: last ASIC type.
> + */
> +enum hl_asic_type {
> +     ASIC_AUTO_DETECT,
> +     ASIC_GOYA,
> +     ASIC_LAST
> +};
> +
> +
> +
> +
> +
> +/*
> + * FILE PRIVATE STRUCTURE
> + */
> +
> +/**
> + * struct hl_fpriv - process information stored in FD private data.
> + * @hdev: habanalabs device structure.
> + * @filp: pointer to the given file structure.
> + * @taskpid: current process ID.
> + * @refcount: number of related contexts.
> + */
> +struct hl_fpriv {
> +     struct hl_device        *hdev;
> +     struct file             *filp;
> +     struct pid              *taskpid;
> +     struct kref             refcount;
> +};
> +
> +
> +
> +
> +/*
> + * DEVICES
> + */
> +
> +/* Theoretical limit only. A single host can only contain up to 4 or 8 PCIe
> + * x16 cards. In extereme cases, there are hosts that can accommodate 16 
> cards
> + */
> +#define HL_MAX_MINORS        256
> +
> +/**
> + * struct hl_device - habanalabs device structure.
> + * @pdev: pointer to PCI device, can be NULL in case of simulator device.
> + * @cdev: related char device.
> + * @dev: realted kernel basic device structure.
> + * @asic_name: ASIC specific nmae.
> + * @asic_type: ASIC specific type.
> + * @major: habanalabs KMD major.
> + * @id: device minor.
> + * @disabled: is device disabled.
> + */
> +struct hl_device {
> +     struct pci_dev                  *pdev;
> +     struct cdev                     cdev;
> +     struct device                   *dev;
> +     char                            asic_name[16];
> +     enum hl_asic_type               asic_type;
> +     u32                             major;
> +     u16                             id;
> +     u8                              disabled;
> +};
> +
> +/*
> + * IOCTLs
> + */
> +
> +/**
> + * typedef hl_ioctl_t - typedef for ioctl function in the driver
> + * @hpriv: pointer to the FD's private data, which contains state of
> + *           user process
> + * @data: pointer to the input/output arguments structure of the IOCTL
> + *
> + * Return: 0 for success, negative value for error
> + */
> +typedef int hl_ioctl_t(struct hl_fpriv *hpriv, void *data);
> +
> +/**
> + * struct hl_ioctl_desc - describes an IOCTL entry of the driver.
> + * @cmd: the IOCTL code as created by the kernel macros.
> + * @func: pointer to the driver's function that should be called for this 
> IOCTL.
> + */
> +struct hl_ioctl_desc {
> +     unsigned int cmd;
> +     hl_ioctl_t *func;
> +};
> +
> +
> +
> +
> +
> +/*
> + * Kernel module functions that can be accessed by entire module
> + */
> +
> +int hl_device_open(struct inode *inode, struct file *filp);
> +int create_hdev(struct hl_device **dev, struct pci_dev *pdev,
> +             enum hl_asic_type asic_type, int minor);
> +void destroy_hdev(struct hl_device *hdev);
> +int hl_poll_timeout_memory(struct hl_device *hdev, u64 addr, u32 timeout_us,
> +                             u32 *val);
> +int hl_poll_timeout_device_memory(struct hl_device *hdev, void __iomem *addr,
> +                             u32 timeout_us, u32 *val);
> +
> +int hl_device_init(struct hl_device *hdev, struct class *hclass);
> +void hl_device_fini(struct hl_device *hdev);
> +int hl_device_suspend(struct hl_device *hdev);
> +int hl_device_resume(struct hl_device *hdev);
> +
> +#endif /* HABANALABSP_H_ */
> diff --git a/drivers/misc/habanalabs/habanalabs_drv.c 
> b/drivers/misc/habanalabs/habanalabs_drv.c
> new file mode 100644
> index 000000000000..15217975327b
> --- /dev/null
> +++ b/drivers/misc/habanalabs/habanalabs_drv.c
> @@ -0,0 +1,366 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2016-2018 HabanaLabs, Ltd.
> + * All Rights Reserved.
> + *
> + * Author: Oded Gabbay <oded.gab...@gmail.com>
> + *
> + */
> +
> +#include "habanalabs.h"
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kthread.h>
> +
> +#include <linux/fs.h>
> +
> +#define HL_DRIVER_AUTHOR     "HabanaLabs Kernel Driver Team"
> +
> +#define HL_DRIVER_DESC               "Driver for HabanaLabs's AI 
> Accelerators"
> +
> +MODULE_AUTHOR(HL_DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(HL_DRIVER_DESC);
> +MODULE_LICENSE("GPL v2");
> +
> +static int hl_major;
> +static struct class *hl_class;
> +DEFINE_IDR(hl_devs_idr);
> +DEFINE_MUTEX(hl_devs_idr_lock);
> +
> +#define PCI_VENDOR_ID_HABANALABS     0x1da3
> +
> +#define PCI_IDS_GOYA                 0x0001
> +
> +static struct pci_device_id ids[] = {
> +     { PCI_DEVICE(PCI_VENDOR_ID_HABANALABS, PCI_IDS_GOYA), },
> +     { 0, }
> +};
> +MODULE_DEVICE_TABLE(pci, ids);
> +
> +/**
> + * get_asic_type - translate device id to asic type
> + *
> + * @device: id of the PCI device
> + * @asic_type: pointer that will be filled by the asic type
> + *
> + * Translate device id to asic type.
> + * In case of unidentified device, return -1
> + */
> +static int get_asic_type(u16 device, enum hl_asic_type *asic_type)

This can simply return the hl_asic_type, see also a comment in
create_hdev(().

> +{
> +     int rc = 0;
> +
> +     switch (device) {
> +     case PCI_IDS_GOYA:
> +             *asic_type = ASIC_GOYA;
> +             break;
> +     default:
> +             *asic_type = rc = -1;
> +             break;
> +     }
> +
> +     return rc;
> +}
> +
> +/**
> + * hl_device_open - open function for habanalabs device
> + *
> + * @inode: pointer to inode structure
> + * @filp: pointer to file structure
> + *
> + * Called when process opens an habanalabs device.
> + */
> +int hl_device_open(struct inode *inode, struct file *filp)
> +{
> +     struct hl_device *hdev;
> +     struct hl_fpriv *hpriv;
> +
> +     mutex_lock(&hl_devs_idr_lock);
> +     hdev = idr_find(&hl_devs_idr, iminor(inode));
> +     mutex_unlock(&hl_devs_idr_lock);
> +
> +     if (!hdev) {
> +             pr_err("habanalabs: Couldn't find device %d:%d\n",
> +                     imajor(inode), iminor(inode));
> +             return -ENXIO;
> +     }
> +
> +     hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
> +     if (!hpriv)
> +             return -ENOMEM;
> +
> +     hpriv->hdev = hdev;
> +     filp->private_data = hpriv;
> +     hpriv->filp = filp;
> +     kref_init(&hpriv->refcount);
> +     nonseekable_open(inode, filp);
> +
> +     hpriv->taskpid = find_get_pid(current->pid);
> +
> +     return 0;
> +}
> +
> +/**
> + * create_hdev - create habanalabs device instance
> + *
> + * @dev: will hold the pointer to the new habanalabs device structure
> + * @pdev: pointer to the pci device
> + * @asic_type: in case of simulator device, which device is it
> + * @minor: in case of simulator device, the minor of the device
> + *
> + * Allocate memory for habanalabs device and initialize basic fields
> + * Identify the ASIC type
> + * Allocate ID (minor) for the device (only for real devices)
> + */
> +int create_hdev(struct hl_device **dev, struct pci_dev *pdev,
> +             enum hl_asic_type asic_type, int minor)
> +{
> +     struct hl_device *hdev;
> +     int rc;
> +
> +     *dev = NULL;
> +
> +     hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
> +     if (!hdev) {
> +             if (pdev)
> +                     dev_err(&pdev->dev,
> +                             "Not enough memory for habanalabs device\n");
> +             else
> +                     pr_err("habanalabs: Not enough memory for  device\n");
> +
> +             return -ENOMEM;
> +     }
> +
> +     hdev->major = hl_major;
> +
> +     hdev->disabled = true;
> +     hdev->pdev = pdev; /* can be NULL in case of simulator device */
> +
> +     if (asic_type == ASIC_AUTO_DETECT) {
> +             rc = get_asic_type(pdev->device, &hdev->asic_type);

You can just make it 

                &hdev->asic_type = get_asic_type(pdev->device);

> +             if (rc) {
> +                     dev_err(&pdev->dev, "Unsupported ASIC\n");
> +                     rc = -ENODEV;
> +                     goto free_hdev;
> +             }
> +     } else {
> +             hdev->asic_type = asic_type;
> +     }

In the current version create_hdev() is always called with
ASIC_AUTO_DETECT, what are the usecases for other types?

> +
> +     mutex_lock(&hl_devs_idr_lock);
> +
> +     if (minor == -1) {
> +             rc = idr_alloc(&hl_devs_idr, hdev, 0, HL_MAX_MINORS,
> +                             GFP_KERNEL);
> +     } else {
> +             idr_replace(&hl_devs_idr, hdev, minor);

idr_replace can fail, can't it?

> +             rc = minor;
> +     }
> +
> +     mutex_unlock(&hl_devs_idr_lock);
> +
> +     if (rc < 0) {
> +             if (rc == -ENOSPC) {
> +                     pr_err("habanalabs: too many devices in the system\n");
> +                     rc = -EBUSY;
> +             }
> +             goto free_hdev;
> +     }
> +
> +     hdev->id = rc;
> +
> +     *dev = hdev;
> +
> +     return 0;
> +
> +free_hdev:
> +     kfree(hdev);
> +     return rc;
> +}
> +
> +/**
> + * destroy_hdev - destroy habanalabs device instance
> + *
> + * @dev: pointer to the habanalabs device structure
> + *
> + */
> +void destroy_hdev(struct hl_device *hdev)
> +{
> +     /* Remove device from the device list */
> +     mutex_lock(&hl_devs_idr_lock);
> +     idr_remove(&hl_devs_idr, hdev->id);
> +     mutex_unlock(&hl_devs_idr_lock);
> +
> +     kfree(hdev);
> +}
> +
> +static int hl_pmops_suspend(struct device *dev)
> +{
> +     struct pci_dev *pdev = to_pci_dev(dev);
> +     struct hl_device *hdev = pci_get_drvdata(pdev);
> +
> +     pr_debug("habanalabs: Going to suspend PCI device\n");
> +
> +     if (!hdev) {
> +             pr_err("habanalabs: device pointer is NULL in suspend\n");
> +             return 0;
> +     }
> +
> +     return hl_device_suspend(hdev);
> +}
> +
> +static int hl_pmops_resume(struct device *dev)
> +{
> +     struct pci_dev *pdev = to_pci_dev(dev);
> +     struct hl_device *hdev = pci_get_drvdata(pdev);
> +
> +     pr_debug("habanalabs: Going to resume PCI device\n");
> +
> +     if (!hdev) {
> +             pr_err("habanalabs: device pointer is NULL in resume\n");
> +             return 0;
> +     }
> +
> +     return hl_device_resume(hdev);
> +}
> +
> +/**
> + * hl_pci_probe - probe PCI habanalabs devices
> + *
> + * @pdev: pointer to pci device
> + * @id: pointer to pci device id structure
> + *
> + * Standard PCI probe function for habanalabs device.
> + * Create a new habanalabs device and initialize it according to the
> + * device's type
> + */
> +static int hl_pci_probe(struct pci_dev *pdev,
> +                             const struct pci_device_id *id)
> +{
> +     struct hl_device *hdev;
> +     int rc;
> +
> +     dev_info(&pdev->dev, HL_NAME
> +              " device found [%04x:%04x] (rev %x)\n",
> +              (int)pdev->vendor, (int)pdev->device, (int)pdev->revision);
> +
> +     rc = create_hdev(&hdev, pdev, ASIC_AUTO_DETECT, -1);
> +     if (rc)
> +             return rc;
> +
> +     pci_set_drvdata(pdev, hdev);
> +
> +     rc = hl_device_init(hdev, hl_class);
> +     if (rc) {
> +             dev_err(&pdev->dev, "Fatal error during habanalabs device 
> init\n");
> +             rc = -ENODEV;
> +             goto disable_device;
> +     }
> +
> +     return 0;
> +
> +disable_device:
> +     pci_set_drvdata(pdev, NULL);
> +     destroy_hdev(hdev);
> +
> +     return rc;
> +}
> +
> +/**
> + * hl_pci_remove - remove PCI habanalabs devices
> + *
> + * @pdev: pointer to pci device
> + *
> + * Standard PCI remove function for habanalabs device
> + */
> +static void hl_pci_remove(struct pci_dev *pdev)
> +{
> +     struct hl_device *hdev;
> +
> +     hdev = pci_get_drvdata(pdev);
> +     if (!hdev)
> +             return;
> +
> +     hl_device_fini(hdev);
> +     pci_set_drvdata(pdev, NULL);
> +
> +     destroy_hdev(hdev);
> +}
> +
> +static const struct dev_pm_ops hl_pm_ops = {
> +     .suspend = hl_pmops_suspend,
> +     .resume = hl_pmops_resume,
> +};
> +
> +static struct pci_driver hl_pci_driver = {
> +     .name = HL_NAME,
> +     .id_table = ids,
> +     .probe = hl_pci_probe,
> +     .remove = hl_pci_remove,
> +     .driver.pm = &hl_pm_ops,
> +};
> +
> +/**
> + * hl_init - Initialize the habanalabs kernel driver
> + *
> + */
> +static int __init hl_init(void)
> +{
> +     int rc;
> +     dev_t dev;
> +
> +     pr_info("habanalabs: loading driver\n");
> +
> +     rc = alloc_chrdev_region(&dev, 0, HL_MAX_MINORS, HL_NAME);
> +     if (rc < 0) {
> +             pr_err("habanalabs: unable to get major\n");
> +             return rc;
> +     }
> +
> +     hl_major = MAJOR(dev);
> +
> +     hl_class = class_create(THIS_MODULE, HL_NAME);
> +     if (IS_ERR(hl_class)) {
> +             pr_err("habanalabs: failed to allocate class\n");
> +             rc = PTR_ERR(hl_class);
> +             goto remove_major;
> +     }
> +
> +     rc = pci_register_driver(&hl_pci_driver);
> +     if (rc) {
> +             pr_err("habanalabs: failed to register pci device\n");
> +             goto remove_class;
> +     }
> +
> +     pr_debug("habanalabs: driver loaded\n");
> +
> +     return 0;
> +
> +remove_class:
> +     class_destroy(hl_class);
> +remove_major:
> +     unregister_chrdev_region(MKDEV(hl_major, 0), HL_MAX_MINORS);
> +     return rc;
> +}
> +
> +/**
> + * hl_exit - Release all resources of the habanalabs kernel driver
> + *
> + */
> +static void __exit hl_exit(void)
> +{
> +     pci_unregister_driver(&hl_pci_driver);
> +
> +     class_destroy(hl_class);
> +     unregister_chrdev_region(MKDEV(hl_major, 0), HL_MAX_MINORS);
> +
> +     idr_destroy(&hl_devs_idr);
> +
> +     pr_debug("habanalabs: driver removed\n");
> +}
> +
> +module_init(hl_init);
> +module_exit(hl_exit);
> diff --git a/drivers/misc/habanalabs/include/habanalabs_device_if.h 
> b/drivers/misc/habanalabs/include/habanalabs_device_if.h
> new file mode 100644
> index 000000000000..9dbb7077eabd
> --- /dev/null
> +++ b/drivers/misc/habanalabs/include/habanalabs_device_if.h
> @@ -0,0 +1,125 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright 2016-2018 HabanaLabs, Ltd.
> + * All Rights Reserved.
> + *
> + */
> +
> +#ifndef HABANALABS_DEVICE_IF_H
> +#define HABANALABS_DEVICE_IF_H
> +
> +#include <linux/types.h>
> +
> +/*
> + * PRIMARY QUEUE
> + */
> +
> +struct hl_bd {
> +     __u64   ptr;
> +     __u32   len;
> +     union {
> +             struct {
> +                     __u32   repeat:16;
> +                     __u32   res1:8;
> +                     __u32   repeat_valid:1;
> +                     __u32   res2:7;
> +             };
> +             __u32   ctl;
> +     };
> +};
> +
> +#define HL_BD_SIZE                   sizeof(struct hl_bd)
> +
> +/*
> + * BD_CTL_REPEAT_VALID tells the CP whether the repeat field in the BD CTL is
> + * valid. 1 means the repeat field is valid, 0 means not-valid,
> + * i.e. repeat == 1
> + */
> +#define BD_CTL_REPEAT_VALID_SHIFT    24
> +#define BD_CTL_REPEAT_VALID_MASK     0x01000000
> +
> +#define BD_CTL_SHADOW_INDEX_SHIFT    0
> +#define BD_CTL_SHADOW_INDEX_MASK     0x00000FFF
> +
> +/*
> + * COMPLETION QUEUE
> + */
> +
> +struct hl_cq_entry {
> +     __u32   data;
> +};
> +
> +#define HL_CQ_ENTRY_SIZE             sizeof(struct hl_cq_entry)
> +
> +#define CQ_ENTRY_READY_SHIFT                 31
> +#define CQ_ENTRY_READY_MASK                  0x80000000
> +
> +#define CQ_ENTRY_SHADOW_INDEX_VALID_SHIFT    30
> +#define CQ_ENTRY_SHADOW_INDEX_VALID_MASK     0x40000000
> +
> +#define CQ_ENTRY_SHADOW_INDEX_SHIFT          BD_CTL_SHADOW_INDEX_SHIFT
> +#define CQ_ENTRY_SHADOW_INDEX_MASK           BD_CTL_SHADOW_INDEX_MASK
> +
> +/*
> + * EVENT QUEUE
> + */
> +
> +struct hl_eq_header {
> +     __u32 reserved;
> +     union {
> +             struct {
> +                     __u32 ctx_id :10;
> +                     __u32:6;
> +                     __u32 opcode :10;
> +                     __u32:5;
> +                     __u32 ready :1;
> +             };
> +             __u32 ctl;
> +     };
> +};
> +
> +struct hl_eq_entry {
> +     struct hl_eq_header hdr;
> +     __u64 data[7];
> +};
> +
> +#define HL_EQ_ENTRY_SIZE             sizeof(struct hl_eq_entry)
> +
> +#define EQ_CTL_READY_SHIFT           31
> +#define EQ_CTL_READY_MASK            0x80000000
> +
> +#define EQ_CTL_EVENT_TYPE_SHIFT              16
> +#define EQ_CTL_EVENT_TYPE_MASK               0x03FF0000
> +
> +enum pq_init_status {
> +     PQ_INIT_STATUS_NA = 0,
> +     PQ_INIT_STATUS_READY_FOR_CP,
> +     PQ_INIT_STATUS_READY_FOR_HOST
> +};
> +
> +/*
> + * ArmCP info
> + */
> +
> +#define VERSION_MAX_LEN                      128
> +#define ARMCP_MAX_SENSORS            128
> +
> +struct armcp_sensor {
> +     __u32 type;
> +     __u32 flags;
> +};
> +
> +/* must be aligned to 4 bytes */
> +struct armcp_info {
> +     struct armcp_sensor sensors[ARMCP_MAX_SENSORS];
> +     __u8 kernel_version[VERSION_MAX_LEN];
> +     __u32 reserved[3];
> +     __u32 cpld_version;
> +     __u32 infineon_version;
> +     __u8 fuse_version[VERSION_MAX_LEN];
> +     __u8 thermal_version[VERSION_MAX_LEN];
> +     __u8 armcp_version[VERSION_MAX_LEN];
> +     __u64 dram_size;
> +};
> +
> +#endif /* HABANALABS_DEVICE_IF_H */
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

Reply via email to