Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 7 Apr 2021 19:47:48 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     min.li.xe@renesas.com
Cc:     derek.kiernan@xilinx.com, dragan.cvetic@xilinx.com, arnd@arndb.de,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization
 Management Unit (SMU) support
Message-ID: <YG3wRJnootNdV0cc@kroah.com>
References: <1617816815-3785-1-git-send-email-min.li.xe@renesas.com>
 <1617816815-3785-2-git-send-email-min.li.xe@renesas.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1617816815-3785-2-git-send-email-min.li.xe@renesas.com>
Precedence: bulk

On Wed, Apr 07, 2021 at 01:33:35PM -0400, min.li.xe@renesas.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