linux-kernel - Re: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

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: <YG3wRJnootNdV0cc@kroah.com>
Date:   Wed, 7 Apr 2021 19:47:48 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     min.li.xe@...esas.com
Cc:     derek.kiernan@...inx.com, dragan.cvetic@...inx.com, arnd@...db.de,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization
 Management Unit (SMU) support

On Wed, Apr 07, 2021 at 01:33:35PM -0400, min.li.xe@...esas.com wrote:
> --- /dev/null
> +++ b/drivers/misc/rsmu_cdev.c
> @@ -0,0 +1,266 @@
> +// SPDX-License-Identifier: GPL-2.0+

Do you really meen "+" here?  (sorry, have to ask.)

> +/*
> + * This driver is developed for the IDT ClockMatrix(TM) and 82P33xxx families
> + * of timing and synchronization devices. It will be used by Renesas PTP Clock
> + * Manager for Linux (pcm4l) software to provide support to GNSS assisted
> + * partial timing support (APTS) and other networking timing functions.
> + *
> + * Please note it must work with Renesas MFD driver to access device through
> + * I2C/SPI.
> + *
> + * Copyright (C) 2019 Integrated Device Technology, Inc., a Renesas Company.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

You should not need this as it's a driver, so you should only use the
dev_dbg() family of print message functions, no need for pr_*() calls.

> +
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/mfd/rsmu.h>
> +#include <uapi/linux/rsmu.h>
> +#include "rsmu_cdev.h"
> +
> +#define DRIVER_NAME	"rsmu"

KBUILD_MODNAME instead?  But you really only need this in one place, so
no need for it at all, just use KBUILD_MODNAME here:

> +static struct platform_driver rsmu_driver = {
> +	.driver = {
> +		.name = DRIVER_NAME,

Right there.

> +	},
> +	.probe = rsmu_probe,
> +	.remove =  rsmu_remove,
> +	.id_table = rsmu_id_table,
> +};
> +
> +static int __init rsmu_init(void)
> +{
> +	int err;
> +
> +	err = platform_driver_register(&rsmu_driver);
> +	if (err < 0)
> +		pr_err("Unabled to register %s platform driver", DRIVER_NAME);
> +
> +	return err;
> +}
> +
> +static void __exit rsmu_exit(void)
> +{
> +	platform_driver_unregister(&rsmu_driver);
> +}
> +
> +module_init(rsmu_init);
> +module_exit(rsmu_exit);

module_platform_driver() instead?

thanks,

greg k-h