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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YCwFBNa2npYcEIQ+@kroah.com>
Date:   Tue, 16 Feb 2021 18:46:44 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Maciej Kwapulinski <maciej.kwapulinski@...ux.intel.com>
Cc:     Arnd Bergmann <arnd@...db.de>, Jonathan Corbet <corbet@....net>,
        Derek Kiernan <derek.kiernan@...inx.com>,
        Dragan Cvetic <dragan.cvetic@...inx.com>,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        Tomasz Jankowski <tomasz1.jankowski@...el.com>,
        Savo Novakovic <savox.novakovic@...el.com>,
        Jianxun Zhang <jianxun.zhang@...ux.intel.com>
Subject: Re: [PATCH v1 01/12] gna: add driver module

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