Re: [PATCH v11 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver

From: Andy Shevchenko
Date: Wed May 20 2020 - 07:49:30 EST


On Wed, May 20, 2020 at 02:37:13PM +0300, Avi Fishman wrote:
> On Wed, May 20, 2020 at 1:24 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> >
> > On Wed, May 20, 2020 at 12:51:12PM +0300, Tali Perry wrote:
> > > Add Nuvoton NPCM BMC I2C controller driver.
> >
> > ...
> >
> > > +#ifdef CONFIG_DEBUG_FS
> >
> > Why?!
>
> It is made to save code size if CONFIG_DEBUG_FS is not defined?

Nope (in cases I have commented on). Try again.

> We see a lot of kernel code that is doing it.

Cargo cult, okay. So, somebody should try to understand what they are doing.

> So could you elaborate what is the problem?

Problem 1: ugly code.
Problem 2: some of the code is not guarded (seems never been tested with disabled debugfs).
Problem 3: it's not needed.

> > > +#include <linux/debugfs.h>
> > > +#endif

--
With Best Regards,
Andy Shevchenko