Messages in this thread | | | Date | Thu, 21 May 2020 17:23:40 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver |
| |
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
| |