Date:   Wed, 25 Jan 2023 09:36:36 -0500
From:   Brent Pappas <bpappas@pappasbrent.com>
To:     Dan Carpenter <error27@gmail.com>
Cc:     sakari.ailus@linux.intel.com, bingbu.cao@intel.com,
        tian.shu.qiu@intel.com, mchehab@kernel.org,
        gregkh@linuxfoundation.org, linux-media@vger.kernel.org,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mdeia: ipu3: ipu33-mmu: Replace macro IPU3_ADDR2PTE()
 with a function
Message-ID: <Y9E+dGgQXFUQnIb8@pappasbrent.com>
References: <20230124135554.13787-1-bpappas@pappasbrent.com>
 <Y8/uTYK7qmYD5MSA@kadam>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <Y8/uTYK7qmYD5MSA@kadam>
Precedence: bulk

Hi Dan,

> When you say "Linux coding style standards" what exactly does that mean?

I am specifically referring to this line of
Documentation/process/coding-style.rst, from the section "Macros, Enums,
and RTL":

> Generally, inline functions are preferable to macros resembling
> functions.

This is the first reason I chose this specific macro.
IPU3_ADD2PTE() would behave the same as a function, so based on my
reading of coding-style.rst, I thought it would be appropriate to
proprose turning it into a function.

Full disclosure, I am university student, and my current research
project is on creating a static analysis framework for finding macros
that can be easily turned into functions.
I want this project to have an impact on widely-used code,
and so I have been using this framework to find such macros in Linux.
That is why I have recently been submitting patches to turn macros into
functions.
So the second reason I chose this macro was because my framework
identifies it as transformable.

> This code is in the middle of a big section full of macros.  Why did you
> pick this particular macro?  Now it doesn't mirror the IPU3_PTE2ADDR()
> so this patch hurts readability.

The reason why I did not try to turn the macro IPU3_PTE2ADDR() into a
function is that it is never invoked, and my framework does not identify
uninvoked macros as transformable.

There are more macros in drivers/staging that I think could be turned
into functions, and I would like to continue submitting patches to do
so.
However, if you would rather I change the way I am doing this,
or that I stop submitting these sorts of patches altogether,
please let me know.

Thank you,
Brent