Re: [PATCH v1 01/12] gna: add driver module

[Greg Kroah-Hartman]

On Tue, Feb 16, 2021 at 05:05:14PM +0100, Maciej Kwapulinski wrote:
> --- /dev/null
> +++ b/drivers/misc/gna/gna_driver.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright(c) 2017-2021 Intel Corporation
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME " " fmt

You are a driver, you should never need a pr_* call, so this should not
be needed.  You should always just use dev_* instead.

> --- /dev/null
> +++ b/drivers/misc/gna/gna_driver.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright(c) 2017-2021 Intel Corporation */
> +
> +#ifndef __GNA_DRIVER_H__
> +#define __GNA_DRIVER_H__
> +
> +#include <linux/kernel.h>
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +
> +#define GNA_DRV_NAME	"gna"

Way too generic, no one knows what "gna" is.

> +#define GNA_DRV_VER	"1.2.0"

As Andy said, this means nothing within the kernel (or really, outside
the kernel either), so please drop.

> +
> +#define GNA_MAX_DEVICES		16

Why 16?

And if that's all, then just use the misc device api, that saves you so
much overhead and mess and you don't have to worry about sysfs and
classes or anything like that at all.

thanks,

greg k-h