[PATCH 0/7] debugfs: Add and use debugfs_create_xul()

[PATCH 0/7] debugfs: Add and use debugfs_create_xul()

[Geert Uytterhoeven]

	Hi all,

The existing debugfs_create_ulong() function supports objects of
type "unsigned long", which are 32-bit or 64-bit depending on the
platform, in decimal form.  To format objects in hexadecimal, various
debugfs_create_x*() functions exist, but all of them take fixed-size
types.

To work around this, some drivers call one of debugfs_create_x{32,64}(),
depending on the size of unsigned long.
Other driver just cast the value pointer to "u32 *" or "u64 *",
introducing portability bugs or data leaks in the process.

Hence this patch series adds a debugfs helper for "unsigned long"
objects in hexadecimal format, and converts drivers to make use of it.
It also contains two cleanups removing superfluous casts, which I added
to this series to avoid conflicts.

Thanks for your comments!

Geert Uytterhoeven (7):
  debugfs: Add debugfs_create_xul() for hexadecimal unsigned long
  mac80211: Use debugfs_create_xul() helper
  net: caif: Fix debugfs on 64-bit platforms
  mmc: atmel-mci: Fix debugfs on 64-bit platforms
  mmc: atmel-mci: Remove superfluous cast in debugfs_create_u32() call
  mmc: dw_mmc: Fix debugfs on 64-bit platforms
  mmc: dw_mmc: Remove superfluous cast in debugfs_create_u32() call

 drivers/mmc/host/atmel-mci.c   | 10 +++++-----
 drivers/mmc/host/dw_mmc.c      | 10 +++++-----
 drivers/net/caif/caif_serial.c |  4 ++--
 include/linux/debugfs.h        | 10 ++++++++++
 net/mac80211/debugfs_sta.c     | 17 +++--------------
 5 files changed, 25 insertions(+), 26 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/7] debugfs: Add debugfs_create_xul() for hexadecimal unsigned long

[Geert Uytterhoeven]

The existing debugfs_create_ulong() function supports objects of
type "unsigned long", which are 32-bit or 64-bit depending on the
platform, in decimal form.  To format objects in hexadecimal, various
debugfs_create_x*() functions exist, but all of them take fixed-size
types.

Add a debugfs helper for "unsigned long" objects in hexadecimal format.
This avoids the need for users to open-code the same, or introduce
bugs when casting the value pointer to "u32 *" or "u64 *" to call
debugfs_create_x{32,64}().

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 include/linux/debugfs.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 33690949b45d6904..d7b2aebcc277d65e 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -356,4 +356,14 @@ static inline ssize_t debugfs_write_file_bool(struct file *file,
 
 #endif
 
+static inline void debugfs_create_xul(const char *name, umode_t mode,
+				      struct dentry *parent,
+				      unsigned long *value)
+{
+	if (sizeof(*value) == sizeof(u32))
+		debugfs_create_x32(name, mode, parent, (u32 *)value);
+	else
+		debugfs_create_x64(name, mode, parent, (u64 *)value);
+}
+
 #endif
-- 
2.17.1

Re: [PATCH 1/7] debugfs: Add debugfs_create_xul() for hexadecimal unsigned long

[Greg Kroah-Hartman]

On Mon, Oct 21, 2019 at 04:37:36PM +0200, Geert Uytterhoeven wrote:
> The existing debugfs_create_ulong() function supports objects of
> type "unsigned long", which are 32-bit or 64-bit depending on the
> platform, in decimal form.  To format objects in hexadecimal, various
> debugfs_create_x*() functions exist, but all of them take fixed-size
> types.
> 
> Add a debugfs helper for "unsigned long" objects in hexadecimal format.
> This avoids the need for users to open-code the same, or introduce
> bugs when casting the value pointer to "u32 *" or "u64 *" to call
> debugfs_create_x{32,64}().
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  include/linux/debugfs.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> index 33690949b45d6904..d7b2aebcc277d65e 100644
> --- a/include/linux/debugfs.h
> +++ b/include/linux/debugfs.h
> @@ -356,4 +356,14 @@ static inline ssize_t debugfs_write_file_bool(struct file *file,
>  
>  #endif
>  
> +static inline void debugfs_create_xul(const char *name, umode_t mode,
> +				      struct dentry *parent,
> +				      unsigned long *value)
> +{
> +	if (sizeof(*value) == sizeof(u32))
> +		debugfs_create_x32(name, mode, parent, (u32 *)value);
> +	else
> +		debugfs_create_x64(name, mode, parent, (u64 *)value);
> +}

Looks sane, but can you add some kernel-doc comments here so that we can
pull it into the debugfs documentation?  Also there is debugfs
documentation in Documentation/filesystems/ so maybe also add this
there?  I am going to be overhauling the debugfs documentation "soon"
but it's at the lower part of my todo list, so it will take a while,
might as well keep it up to date with new stuff added like this so that
people don't get lost.

thanks,

greg k-h

Re: [PATCH 1/7] debugfs: Add debugfs_create_xul() for hexadecimal unsigned long

[Geert Uytterhoeven]

Hi Greg,

On Mon, Oct 21, 2019 at 4:45 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Oct 21, 2019 at 04:37:36PM +0200, Geert Uytterhoeven wrote:
> > The existing debugfs_create_ulong() function supports objects of
> > type "unsigned long", which are 32-bit or 64-bit depending on the
> > platform, in decimal form.  To format objects in hexadecimal, various
> > debugfs_create_x*() functions exist, but all of them take fixed-size
> > types.
> >
> > Add a debugfs helper for "unsigned long" objects in hexadecimal format.
> > This avoids the need for users to open-code the same, or introduce
> > bugs when casting the value pointer to "u32 *" or "u64 *" to call
> > debugfs_create_x{32,64}().
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  include/linux/debugfs.h | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> > index 33690949b45d6904..d7b2aebcc277d65e 100644
> > --- a/include/linux/debugfs.h
> > +++ b/include/linux/debugfs.h
> > @@ -356,4 +356,14 @@ static inline ssize_t debugfs_write_file_bool(struct file *file,
> >
> >  #endif
> >
> > +static inline void debugfs_create_xul(const char *name, umode_t mode,
> > +                                   struct dentry *parent,
> > +                                   unsigned long *value)
> > +{
> > +     if (sizeof(*value) == sizeof(u32))
> > +             debugfs_create_x32(name, mode, parent, (u32 *)value);
> > +     else
> > +             debugfs_create_x64(name, mode, parent, (u64 *)value);
> > +}
>
> Looks sane, but can you add some kernel-doc comments here so that we can
> pull it into the debugfs documentation?  Also there is debugfs

Sure, will do.

> documentation in Documentation/filesystems/ so maybe also add this
> there?  I am going to be overhauling the debugfs documentation "soon"
> but it's at the lower part of my todo list, so it will take a while,
> might as well keep it up to date with new stuff added like this so that
> people don't get lost.

Right. Currently it already lacks debugfs_create_ulong(). Will add both
debugfs_create_ulong() and debugfs_create_xul().

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/7] debugfs: Add debugfs_create_xul() for hexadecimal unsigned long

[Joe Perches]

On Mon, 2019-10-21 at 16:37 +0200, Geert Uytterhoeven wrote:
> The existing debugfs_create_ulong() function supports objects of
> type "unsigned long", which are 32-bit or 64-bit depending on the
> platform, in decimal form.  To format objects in hexadecimal, various
> debugfs_create_x*() functions exist, but all of them take fixed-size
> types.
> 
> Add a debugfs helper for "unsigned long" objects in hexadecimal format.
> This avoids the need for users to open-code the same, or introduce
> bugs when casting the value pointer to "u32 *" or "u64 *" to call
> debugfs_create_x{32,64}().
[]
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
[]
> @@ -356,4 +356,14 @@ static inline ssize_t debugfs_write_file_bool(struct file *file,
>  
>  #endif
>  
> +static inline void debugfs_create_xul(const char *name, umode_t mode,
> +				      struct dentry *parent,
> +				      unsigned long *value)
> +{
> +	if (sizeof(*value) == sizeof(u32))
> +		debugfs_create_x32(name, mode, parent, (u32 *)value);
> +	else
> +		debugfs_create_x64(name, mode, parent, (u64 *)value);

trivia: the casts are unnecessary.

This might be more sensible using #ifdef

static inline void debugfs_create_xul(const char *name, umode_t mode,
				      struct dentry *parent,
				      unsigned long *value)
{
#if BITS_PER_LONG == 64
	debugfs_create_x64(name, mode, parent, value);
#else
	debugfs_create_x32(name, mode, parent, value);
#endif
}

Re: [PATCH 1/7] debugfs: Add debugfs_create_xul() for hexadecimal unsigned long

[Geert Uytterhoeven]

Hi Joe,

On Mon, Oct 21, 2019 at 5:37 PM Joe Perches <joe@perches.com> wrote:
> On Mon, 2019-10-21 at 16:37 +0200, Geert Uytterhoeven wrote:
> > The existing debugfs_create_ulong() function supports objects of
> > type "unsigned long", which are 32-bit or 64-bit depending on the
> > platform, in decimal form.  To format objects in hexadecimal, various
> > debugfs_create_x*() functions exist, but all of them take fixed-size
> > types.
> >
> > Add a debugfs helper for "unsigned long" objects in hexadecimal format.
> > This avoids the need for users to open-code the same, or introduce
> > bugs when casting the value pointer to "u32 *" or "u64 *" to call
> > debugfs_create_x{32,64}().
> []
> > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> []
> > @@ -356,4 +356,14 @@ static inline ssize_t debugfs_write_file_bool(struct file *file,
> >
> >  #endif
> >
> > +static inline void debugfs_create_xul(const char *name, umode_t mode,
> > +                                   struct dentry *parent,
> > +                                   unsigned long *value)
> > +{
> > +     if (sizeof(*value) == sizeof(u32))
> > +             debugfs_create_x32(name, mode, parent, (u32 *)value);
> > +     else
> > +             debugfs_create_x64(name, mode, parent, (u64 *)value);
>
> trivia: the casts are unnecessary.

They are necessary, in both calls (so using #ifdef as suggested below
won't help):

    include/linux/debugfs.h:375:42: error: passing argument 4 of
‘debugfs_create_x32’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
       debugfs_create_x32(name, mode, parent, value);
                                              ^~~~~
    include/linux/debugfs.h:114:6: note: expected ‘u32 * {aka unsigned
int *}’ but argument is of type ‘long unsigned int *’
     void debugfs_create_x32(const char *name, umode_t mode, struct
dentry *parent,
          ^~~~~~~~~~~~~~~~~~
    include/linux/debugfs.h:377:42: error: passing argument 4 of
‘debugfs_create_x64’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
       debugfs_create_x64(name, mode, parent, value);
                                              ^~~~~
    include/linux/debugfs.h:116:6: note: expected ‘u64 * {aka long
long unsigned int *}’ but argument is of type ‘long unsigned int *’
     void debugfs_create_x64(const char *name, umode_t mode, struct
dentry *parent,
          ^~~~~~~~~~~~~~~~~~

> This might be more sensible using #ifdef
>
> static inline void debugfs_create_xul(const char *name, umode_t mode,
>                                       struct dentry *parent,
>                                       unsigned long *value)
> {
> #if BITS_PER_LONG == 64
>         debugfs_create_x64(name, mode, parent, value);
> #else
>         debugfs_create_x32(name, mode, parent, value);
> #endif
> }

... at the expense of the compiler checking only one branch.

Just like "if (IS_ENABLED(CONFIG_<foo>)" (when possible) is preferred
over "#ifdef CONFIG_<foo>" because of compile-coverage, I think using
"if" here is better than using "#if".

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/7] debugfs: Add debugfs_create_xul() for hexadecimal unsigned long

[Joe Perches]

On Tue, 2019-10-22 at 10:03 +0200, Geert Uytterhoeven wrote:
> Hi Joe,

Hey again Geert.

> On Mon, Oct 21, 2019 at 5:37 PM Joe Perches <joe@perches.com> wrote:
> > On Mon, 2019-10-21 at 16:37 +0200, Geert Uytterhoeven wrote:
> > > The existing debugfs_create_ulong() function supports objects of
> > > type "unsigned long", which are 32-bit or 64-bit depending on the
> > > platform, in decimal form.  To format objects in hexadecimal, various
> > > debugfs_create_x*() functions exist, but all of them take fixed-size
> > > types.
> > > 
> > > Add a debugfs helper for "unsigned long" objects in hexadecimal format.
> > > This avoids the need for users to open-code the same, or introduce
> > > bugs when casting the value pointer to "u32 *" or "u64 *" to call
> > > debugfs_create_x{32,64}().
> > []
> > > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> > []
> > > @@ -356,4 +356,14 @@ static inline ssize_t debugfs_write_file_bool(struct file *file,
> > > 
> > >  #endif
> > > 
> > > +static inline void debugfs_create_xul(const char *name, umode_t mode,
> > > +                                   struct dentry *parent,
> > > +                                   unsigned long *value)
> > > +{
> > > +     if (sizeof(*value) == sizeof(u32))
> > > +             debugfs_create_x32(name, mode, parent, (u32 *)value);
> > > +     else
> > > +             debugfs_create_x64(name, mode, parent, (u64 *)value);
> > 
> > trivia: the casts are unnecessary.
> 
> They are necessary, in both calls (so using #ifdef as suggested below
> won't help):

Silly thinko, (I somehow thought the compiler would
eliminate the code after the branch not taken, but
of course it has to compile it first...  oops)
though the #ifdef should work.

> > This might be more sensible using #ifdef
> > 
> > static inline void debugfs_create_xul(const char *name, umode_t mode,
> >                                       struct dentry *parent,
> >                                       unsigned long *value)
> > {
> > #if BITS_PER_LONG == 64
> >         debugfs_create_x64(name, mode, parent, value);
> > #else
> >         debugfs_create_x32(name, mode, parent, value);
> > #endif
> > }
> 
> ... at the expense of the compiler checking only one branch.
> 
> Just like "if (IS_ENABLED(CONFIG_<foo>)" (when possible) is preferred
> over "#ifdef CONFIG_<foo>" because of compile-coverage, I think using
> "if" here is better than using "#if".

True if all compilers will always eliminate the unused branch.

Re: [PATCH 1/7] debugfs: Add debugfs_create_xul() for hexadecimal unsigned long

[Greg Kroah-Hartman]

On Tue, Oct 22, 2019 at 02:07:34AM -0700, Joe Perches wrote:
> On Tue, 2019-10-22 at 10:03 +0200, Geert Uytterhoeven wrote:
> > Hi Joe,
> 
> Hey again Geert.
> 
> > On Mon, Oct 21, 2019 at 5:37 PM Joe Perches <joe@perches.com> wrote:
> > > On Mon, 2019-10-21 at 16:37 +0200, Geert Uytterhoeven wrote:
> > > > The existing debugfs_create_ulong() function supports objects of
> > > > type "unsigned long", which are 32-bit or 64-bit depending on the
> > > > platform, in decimal form.  To format objects in hexadecimal, various
> > > > debugfs_create_x*() functions exist, but all of them take fixed-size
> > > > types.
> > > > 
> > > > Add a debugfs helper for "unsigned long" objects in hexadecimal format.
> > > > This avoids the need for users to open-code the same, or introduce
> > > > bugs when casting the value pointer to "u32 *" or "u64 *" to call
> > > > debugfs_create_x{32,64}().
> > > []
> > > > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> > > []
> > > > @@ -356,4 +356,14 @@ static inline ssize_t debugfs_write_file_bool(struct file *file,
> > > > 
> > > >  #endif
> > > > 
> > > > +static inline void debugfs_create_xul(const char *name, umode_t mode,
> > > > +                                   struct dentry *parent,
> > > > +                                   unsigned long *value)
> > > > +{
> > > > +     if (sizeof(*value) == sizeof(u32))
> > > > +             debugfs_create_x32(name, mode, parent, (u32 *)value);
> > > > +     else
> > > > +             debugfs_create_x64(name, mode, parent, (u64 *)value);
> > > 
> > > trivia: the casts are unnecessary.
> > 
> > They are necessary, in both calls (so using #ifdef as suggested below
> > won't help):
> 
> Silly thinko, (I somehow thought the compiler would
> eliminate the code after the branch not taken, but
> of course it has to compile it first...  oops)
> though the #ifdef should work.
> 
> > > This might be more sensible using #ifdef
> > > 
> > > static inline void debugfs_create_xul(const char *name, umode_t mode,
> > >                                       struct dentry *parent,
> > >                                       unsigned long *value)
> > > {
> > > #if BITS_PER_LONG == 64
> > >         debugfs_create_x64(name, mode, parent, value);
> > > #else
> > >         debugfs_create_x32(name, mode, parent, value);
> > > #endif
> > > }
> > 
> > ... at the expense of the compiler checking only one branch.
> > 
> > Just like "if (IS_ENABLED(CONFIG_<foo>)" (when possible) is preferred
> > over "#ifdef CONFIG_<foo>" because of compile-coverage, I think using
> > "if" here is better than using "#if".
> 
> True if all compilers will always eliminate the unused branch.

Good ones will, we don't care about bad ones :)

[PATCH 2/7] mac80211: Use debugfs_create_xul() helper

[Geert Uytterhoeven]

Use the new debugfs_create_xul() helper instead of open-coding the same
operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 net/mac80211/debugfs_sta.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index c8ad20c28c438dab..ca34dcdac8c0bd4d 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -928,12 +928,7 @@ STA_OPS(he_capa);
 		sta->debugfs_dir, sta, &sta_ ##name## _ops);
 
 #define DEBUGFS_ADD_COUNTER(name, field)				\
-	if (sizeof(sta->field) == sizeof(u32))				\
-		debugfs_create_u32(#name, 0400, sta->debugfs_dir,	\
-			(u32 *) &sta->field);				\
-	else								\
-		debugfs_create_u64(#name, 0400, sta->debugfs_dir,	\
-			(u64 *) &sta->field);
+	debugfs_create_ulong(#name, 0400, sta->debugfs_dir, &sta->field);
 
 void ieee80211_sta_debugfs_add(struct sta_info *sta)
 {
@@ -978,14 +973,8 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
 				    NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
 		DEBUGFS_ADD(airtime);
 
-	if (sizeof(sta->driver_buffered_tids) == sizeof(u32))
-		debugfs_create_x32("driver_buffered_tids", 0400,
-				   sta->debugfs_dir,
-				   (u32 *)&sta->driver_buffered_tids);
-	else
-		debugfs_create_x64("driver_buffered_tids", 0400,
-				   sta->debugfs_dir,
-				   (u64 *)&sta->driver_buffered_tids);
+	debugfs_create_xul("driver_buffered_tids", 0400, sta->debugfs_dir,
+			   &sta->driver_buffered_tids);
 
 	drv_sta_add_debugfs(local, sdata, &sta->sta, sta->debugfs_dir);
 }
-- 
2.17.1

[PATCH 3/7] net: caif: Fix debugfs on 64-bit platforms

[Geert Uytterhoeven]

"ser_device.state" is "unsigned long", i.e. 32-bit or 64-bit, depending
on the platform.  Hence casting its address to "u32 *", and calling
debugfs_create_x32() breaks operation on 64-bit platforms.

Fix this by using the new debugfs_create_xul() helper instead.

Fixes: 9b27105b4a44c54b ("net-caif-driver: add CAIF serial driver (ldisc)")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/net/caif/caif_serial.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 40b079162804fb0c..bd40b114d6cd7214 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -102,8 +102,8 @@ static inline void debugfs_init(struct ser_device *ser, struct tty_struct *tty)
 	debugfs_create_blob("last_rx_msg", 0400, ser->debugfs_tty_dir,
 			    &ser->rx_blob);
 
-	debugfs_create_x32("ser_state", 0400, ser->debugfs_tty_dir,
-			   (u32 *)&ser->state);
+	debugfs_create_xul("ser_state", 0400, ser->debugfs_tty_dir,
+			   &ser->state);
 
 	debugfs_create_x8("tty_status", 0400, ser->debugfs_tty_dir,
 			  &ser->tty_status);
-- 
2.17.1

[PATCH 4/7] mmc: atmel-mci: Fix debugfs on 64-bit platforms

[Geert Uytterhoeven]

"atmel_mci.pending_events" and "atmel_mci.completed_events" are
"unsigned long", i.e. 32-bit or 64-bit, depending on the platform.
Hence casting their addresses to "u32 *", and calling
debugfs_create_x32() breaks operation on 64-bit platforms.

Fix this by using the new debugfs_create_xul() helper instead.

Fixes: deec9ae31e607955 ("atmel-mci: debugfs support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/mmc/host/atmel-mci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index c26fbe5f22221d95..ef2eb9e7c75a32a1 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -584,10 +584,10 @@ static void atmci_init_debugfs(struct atmel_mci_slot *slot)
 	debugfs_create_file("regs", S_IRUSR, root, host, &atmci_regs_fops);
 	debugfs_create_file("req", S_IRUSR, root, slot, &atmci_req_fops);
 	debugfs_create_u32("state", S_IRUSR, root, (u32 *)&host->state);
-	debugfs_create_x32("pending_events", S_IRUSR, root,
-			   (u32 *)&host->pending_events);
-	debugfs_create_x32("completed_events", S_IRUSR, root,
-			   (u32 *)&host->completed_events);
+	debugfs_create_xul("pending_events", S_IRUSR, root,
+			   &host->pending_events);
+	debugfs_create_xul("completed_events", S_IRUSR, root,
+			   &host->completed_events);
 }
 
 #if defined(CONFIG_OF)
-- 
2.17.1

[PATCH 5/7] mmc: atmel-mci: Remove superfluous cast in debugfs_create_u32() call

[Geert Uytterhoeven]

"atmel_mci.state" is an enum, which is compatible with u32, so there is
no need to cast its address, preventing further compiler checks.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/mmc/host/atmel-mci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index ef2eb9e7c75a32a1..34c992d875298932 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -583,7 +583,7 @@ static void atmci_init_debugfs(struct atmel_mci_slot *slot)
 
 	debugfs_create_file("regs", S_IRUSR, root, host, &atmci_regs_fops);
 	debugfs_create_file("req", S_IRUSR, root, slot, &atmci_req_fops);
-	debugfs_create_u32("state", S_IRUSR, root, (u32 *)&host->state);
+	debugfs_create_u32("state", S_IRUSR, root, &host->state);
 	debugfs_create_xul("pending_events", S_IRUSR, root,
 			   &host->pending_events);
 	debugfs_create_xul("completed_events", S_IRUSR, root,
-- 
2.17.1

[PATCH 6/7] mmc: dw_mmc: Fix debugfs on 64-bit platforms

[Geert Uytterhoeven]

"dw_mci.pending_events" and "dw_mci.completed_events" are "unsigned
long", i.e. 32-bit or 64-bit, depending on the platform.  Hence casting
their addresses to "u32 *", and calling debugfs_create_x32() breaks
operation on 64-bit platforms.

Fix this by using the new debugfs_create_xul() helper instead.

Fixes: f95f3850f7a9e1d4 ("mmc: dw_mmc: Add Synopsys DesignWare mmc host driver.")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/mmc/host/dw_mmc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bf0048ebbda356a7..b4c4a9cd6365f122 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -177,10 +177,10 @@ static void dw_mci_init_debugfs(struct dw_mci_slot *slot)
 	debugfs_create_file("regs", S_IRUSR, root, host, &dw_mci_regs_fops);
 	debugfs_create_file("req", S_IRUSR, root, slot, &dw_mci_req_fops);
 	debugfs_create_u32("state", S_IRUSR, root, (u32 *)&host->state);
-	debugfs_create_x32("pending_events", S_IRUSR, root,
-			   (u32 *)&host->pending_events);
-	debugfs_create_x32("completed_events", S_IRUSR, root,
-			   (u32 *)&host->completed_events);
+	debugfs_create_xul("pending_events", S_IRUSR, root,
+			   &host->pending_events);
+	debugfs_create_xul("completed_events", S_IRUSR, root,
+			   &host->completed_events);
 }
 #endif /* defined(CONFIG_DEBUG_FS) */
 
-- 
2.17.1

[PATCH 7/7] mmc: dw_mmc: Remove superfluous cast in debugfs_create_u32() call

[Geert Uytterhoeven]

"dw_mci.state" is an enum, which is compatible with u32, so there is no
need to cast its address, preventing further compiler checks.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b4c4a9cd6365f122..fc9d4d000f97e434 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -176,7 +176,7 @@ static void dw_mci_init_debugfs(struct dw_mci_slot *slot)
 
 	debugfs_create_file("regs", S_IRUSR, root, host, &dw_mci_regs_fops);
 	debugfs_create_file("req", S_IRUSR, root, slot, &dw_mci_req_fops);
-	debugfs_create_u32("state", S_IRUSR, root, (u32 *)&host->state);
+	debugfs_create_u32("state", S_IRUSR, root, &host->state);
 	debugfs_create_xul("pending_events", S_IRUSR, root,
 			   &host->pending_events);
 	debugfs_create_xul("completed_events", S_IRUSR, root,
-- 
2.17.1

Re: [PATCH 0/7] debugfs: Add and use debugfs_create_xul()

[Ulf Hansson]

On Mon, 21 Oct 2019 at 16:37, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
>         Hi all,
>
> The existing debugfs_create_ulong() function supports objects of
> type "unsigned long", which are 32-bit or 64-bit depending on the
> platform, in decimal form.  To format objects in hexadecimal, various
> debugfs_create_x*() functions exist, but all of them take fixed-size
> types.
>
> To work around this, some drivers call one of debugfs_create_x{32,64}(),
> depending on the size of unsigned long.
> Other driver just cast the value pointer to "u32 *" or "u64 *",
> introducing portability bugs or data leaks in the process.
>
> Hence this patch series adds a debugfs helper for "unsigned long"
> objects in hexadecimal format, and converts drivers to make use of it.
> It also contains two cleanups removing superfluous casts, which I added
> to this series to avoid conflicts.
>
> Thanks for your comments!

The mmc changes looks good to me. I guess we could funnel via Greg's
"debugfs" tree?

In such case, feel free to add my ack for the mmc changes.

Kind regards
Uffe

>
> Geert Uytterhoeven (7):
>   debugfs: Add debugfs_create_xul() for hexadecimal unsigned long
>   mac80211: Use debugfs_create_xul() helper
>   net: caif: Fix debugfs on 64-bit platforms
>   mmc: atmel-mci: Fix debugfs on 64-bit platforms
>   mmc: atmel-mci: Remove superfluous cast in debugfs_create_u32() call
>   mmc: dw_mmc: Fix debugfs on 64-bit platforms
>   mmc: dw_mmc: Remove superfluous cast in debugfs_create_u32() call
>
>  drivers/mmc/host/atmel-mci.c   | 10 +++++-----
>  drivers/mmc/host/dw_mmc.c      | 10 +++++-----
>  drivers/net/caif/caif_serial.c |  4 ++--
>  include/linux/debugfs.h        | 10 ++++++++++
>  net/mac80211/debugfs_sta.c     | 17 +++--------------
>  5 files changed, 25 insertions(+), 26 deletions(-)
>
> --
> 2.17.1
>
> Gr{oetje,eeting}s,
>
>                                                 Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                                             -- Linus Torvalds