[PATCH] Staging: fwserial: Convert macro into an inline function

[PATCH] Staging: fwserial: Convert macro into an inline function

[Madhumitha Prabakaran]

Convert macro into an inline function to make codebase better

Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
 drivers/staging/fwserial/fwserial.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index a1b90ea7fcb8..aec0f19597a9 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -19,7 +19,10 @@
 
 #include "fwserial.h"
 
-#define be32_to_u64(hi, lo)  ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo))
+inline u64 be32_to_u64(__be32 hi, __be32 lo)
+{
+	return ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo));
+}
 
 #define LINUX_VENDOR_ID   0xd00d1eU  /* same id used in card root directory   */
 #define FWSERIAL_VERSION  0x00e81cU  /* must be unique within LINUX_VENDOR_ID */
-- 
2.17.1

Re: [PATCH] Staging: fwserial: Convert macro into an inline function

[Greg KH]

On Wed, Mar 06, 2019 at 04:48:09PM -0600, Madhumitha Prabakaran wrote:
> Convert macro into an inline function to make codebase better
> 
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
>  drivers/staging/fwserial/fwserial.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
> index a1b90ea7fcb8..aec0f19597a9 100644
> --- a/drivers/staging/fwserial/fwserial.c
> +++ b/drivers/staging/fwserial/fwserial.c
> @@ -19,7 +19,10 @@
>  
>  #include "fwserial.h"
>  
> -#define be32_to_u64(hi, lo)  ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo))
> +inline u64 be32_to_u64(__be32 hi, __be32 lo)
> +{
> +	return ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo));
> +}

For some reason I thought the kernel had a function that already did
this, but it doesn't, which is really odd.  Anyway, that's something
that could be done in the future, as this is done all over the place in
different drivers.

thanks,

greg k-h

Re: [Outreachy kernel] Re: [PATCH] Staging: fwserial: Convert macro into an inline function

[Julia Lawall]

On Thu, 7 Mar 2019, Greg KH wrote:

> On Wed, Mar 06, 2019 at 04:48:09PM -0600, Madhumitha Prabakaran wrote:
> > Convert macro into an inline function to make codebase better
> >
> > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> > ---
> >  drivers/staging/fwserial/fwserial.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
> > index a1b90ea7fcb8..aec0f19597a9 100644
> > --- a/drivers/staging/fwserial/fwserial.c
> > +++ b/drivers/staging/fwserial/fwserial.c
> > @@ -19,7 +19,10 @@
> >
> >  #include "fwserial.h"
> >
> > -#define be32_to_u64(hi, lo)  ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo))
> > +inline u64 be32_to_u64(__be32 hi, __be32 lo)
> > +{
> > +	return ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo));
> > +}
>
> For some reason I thought the kernel had a function that already did
> this, but it doesn't, which is really odd.  Anyway, that's something
> that could be done in the future, as this is done all over the place in
> different drivers.

Actually, as far as I can see, only two occurrences, in
drivers/target/sbp/sbp_target.c and
drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c

Perhaps there are some other variants that are more popular.

julia

Re: [Outreachy kernel] Re: [PATCH] Staging: fwserial: Convert macro into an inline function

[Greg KH]

On Thu, Mar 07, 2019 at 09:08:53PM +0100, Julia Lawall wrote:
> 
> 
> On Thu, 7 Mar 2019, Greg KH wrote:
> 
> > On Wed, Mar 06, 2019 at 04:48:09PM -0600, Madhumitha Prabakaran wrote:
> > > Convert macro into an inline function to make codebase better
> > >
> > > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> > > ---
> > >  drivers/staging/fwserial/fwserial.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
> > > index a1b90ea7fcb8..aec0f19597a9 100644
> > > --- a/drivers/staging/fwserial/fwserial.c
> > > +++ b/drivers/staging/fwserial/fwserial.c
> > > @@ -19,7 +19,10 @@
> > >
> > >  #include "fwserial.h"
> > >
> > > -#define be32_to_u64(hi, lo)  ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo))
> > > +inline u64 be32_to_u64(__be32 hi, __be32 lo)
> > > +{
> > > +	return ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo));
> > > +}
> >
> > For some reason I thought the kernel had a function that already did
> > this, but it doesn't, which is really odd.  Anyway, that's something
> > that could be done in the future, as this is done all over the place in
> > different drivers.
> 
> Actually, as far as I can see, only two occurrences, in
> drivers/target/sbp/sbp_target.c and
> drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c
> 
> Perhaps there are some other variants that are more popular.

The file include/linux/io-64-nonatomic-hi-lo.h open-codes this 4
different times, as does include/linux/ml5/device.h and then there is
HILO_GEN() which tries to do the right thing for this type of macro, but
it's specific to a single driver.

Odds are it is also open-coded in many other drivers as well :(

thanks,

greg k-h

Re: [Outreachy kernel] Re: [PATCH] Staging: fwserial: Convert macro into an inline function

[Julia Lawall]

On Thu, 7 Mar 2019, Greg KH wrote:

> On Thu, Mar 07, 2019 at 09:08:53PM +0100, Julia Lawall wrote:
> >
> >
> > On Thu, 7 Mar 2019, Greg KH wrote:
> >
> > > On Wed, Mar 06, 2019 at 04:48:09PM -0600, Madhumitha Prabakaran wrote:
> > > > Convert macro into an inline function to make codebase better
> > > >
> > > > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> > > > ---
> > > >  drivers/staging/fwserial/fwserial.c | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
> > > > index a1b90ea7fcb8..aec0f19597a9 100644
> > > > --- a/drivers/staging/fwserial/fwserial.c
> > > > +++ b/drivers/staging/fwserial/fwserial.c
> > > > @@ -19,7 +19,10 @@
> > > >
> > > >  #include "fwserial.h"
> > > >
> > > > -#define be32_to_u64(hi, lo)  ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo))
> > > > +inline u64 be32_to_u64(__be32 hi, __be32 lo)
> > > > +{
> > > > +	return ((u64)be32_to_cpu(hi) << 32 | be32_to_cpu(lo));
> > > > +}
> > >
> > > For some reason I thought the kernel had a function that already did
> > > this, but it doesn't, which is really odd.  Anyway, that's something
> > > that could be done in the future, as this is done all over the place in
> > > different drivers.
> >
> > Actually, as far as I can see, only two occurrences, in
> > drivers/target/sbp/sbp_target.c and
> > drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c
> >
> > Perhaps there are some other variants that are more popular.
>
> The file include/linux/io-64-nonatomic-hi-lo.h open-codes this 4
> different times, as does include/linux/ml5/device.h and then there is

Ah, I only looked at .c files :)

julia

> HILO_GEN() which tries to do the right thing for this type of macro, but
> it's specific to a single driver.
>
> Odds are it is also open-coded in many other drivers as well :(
>
> thanks,
>
> greg k-h
>