On Thu, May 21, 2020 at 02:09:09PM +0300, Tali Perry wrote:

Add Nuvoton NPCM BMC I2C controller driver.

Thanks. My comments below.
After addressing them, FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

+	/* Frequency larger than 1 MHZ is not supported */

1 MHZ -> 1MHz

...

+#ifdef CONFIG_DEBUG_FS

Again, why is this here?

Have you checked debugfs.h for !CONFIG_DEBUG_FS case?

+/* i2c debugfs directory: used to keep health monitor of i2c devices */
+static struct dentry *npcm_i2c_debugfs_dir;
+
+static void i2c_init_debugfs(struct platform_device *pdev, struct npcm_i2c *bus)
+{
+	struct dentry *d;
+
+	if (!npcm_i2c_debugfs_dir)
+		return;
+
+	d = debugfs_create_dir(dev_name(&pdev->dev), npcm_i2c_debugfs_dir);
+	if (IS_ERR_OR_NULL(d))
+		return;
+
+	debugfs_create_u64("ber_cnt", 0444, d, &bus->ber_cnt);
+	debugfs_create_u64("nack_cnt", 0444, d, &bus->nack_cnt);
+	debugfs_create_u64("rec_succ_cnt", 0444, d, &bus->rec_succ_cnt);
+	debugfs_create_u64("rec_fail_cnt", 0444, d, &bus->rec_fail_cnt);
+	debugfs_create_u64("timeout_cnt", 0444, d, &bus->timeout_cnt);
+
+	bus->debugfs = d;
+}

+#else
+static void i2c_init_debugfs(struct platform_device *pdev, struct npcm_i2c *bus)
+{
+}

This is completely redundant.

+#endif

...

+#ifdef CONFIG_DEBUG_FS

Ditto.

+static int __init npcm_i2c_init(void)
+{
+	struct dentry *dir;
+
+	dir = debugfs_create_dir("i2c", NULL);
+	if (IS_ERR_OR_NULL(dir))
+		return 0;
+
+	npcm_i2c_debugfs_dir = dir;
+	return 0;
+}
+
+static void __exit npcm_i2c_exit(void)
+{
+	debugfs_remove_recursive(npcm_i2c_debugfs_dir);
+}
+
+module_init(npcm_i2c_init);
+module_exit(npcm_i2c_exit);
+#endif

...

+MODULE_VERSION("0.1.3");

Module version is defined by kernel commit hash. But it's up to you and
subsystem maintainer to decide.

-- 
With Best Regards,
Andy Shevchenko