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, 6 May 2020 17:22:18 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Aishwarya Ramakrishnan <aishwaryarj100@gmail.com>
Cc:     Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Marco Felsch <m.felsch@pengutronix.de>,
        Ondrej Jirman <megous@megous.com>,
        =?iso-8859-1?Q?Myl=E8ne?= Josserand 
        <mylene.josserand@bootlin.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Input: edt-ft5x06: Use DEFINE_DEBUGFS_ATTRIBUTE to
 define debugfs fops
Message-ID: <20200506152218.GA3447721@kroah.com>
References: <20200505153325.20113-1-aishwaryarj100@gmail.com>
 <20200506150623.3841-1-raishwar@visteon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200506150623.3841-1-raishwar@visteon.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, May 06, 2020 at 08:36:22PM +0530, Aishwarya Ramakrishnan wrote:
> From: Aishwarya Ramakrishnan <aishwaryarj100@gmail.com>
> 
> On Tue, May 5, 2020 at 11:49 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > On Tue, May 05, 2020 at 09:03:24PM +0530, Aishwarya Ramakrishnan wrote:
> >> It is more clear to use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs file
> >> operation rather than DEFINE_SIMPLE_ATTRIBUTE.
> 
> > No it is not, why do you think so?
> 
> This change is suggested by Coccinelle software.
> Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci

Not a good thing, I do not know why that was added :(

> > The two defines do different things, that is why we have 2 different
> > defines.  You can not just replace one with the other without
> > understanding why one was used and not the other one.
> 
> > Did you test this change to verify that everything still works
> > properly?  Why is it needed to be changed at all?
> 
> DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> imposes some significant overhead as compared to
> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().

What kind of overhead?  Is it required?

> But I missed to use debugfs_create_file_unsafe() function in the patch.

Yeah, don't use the unsafe stuff unless you know what is happening here
please.

greg k-h