[PATCH] mtd: rawnand: gpmi: add debugfs entry for BCH geometry

[PATCH] mtd: rawnand: gpmi: add debugfs entry for BCH geometry

[han.xu]

From: Han Xu <han.xu@nxp.com>

Export the BCH geometry parameters via debugfs to aid debugging and
provide the necessary information for legacy kobs-ng tool. The debugfs
directory "gpmi-nand" is created under the root debugfs tree, exposing:

bch_geometry  - blob containing the struct bch_geometry fields
raw_mode      - flag indicating raw mode status

The implementation is guarded with #ifdef CONFIG_DEBUG_FS to avoid
build failures when debugfs is disabled.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index c1f766cb225aa..3ceb8df7f8f93 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -7,6 +7,7 @@
  */
 #include <linux/cleanup.h>
 #include <linux/clk.h>
+#include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/sched/task_stack.h>
@@ -732,6 +733,26 @@ static int common_nfc_set_geometry(struct gpmi_nand_data *this)
 	return err;
 }
 
+#ifdef CONFIG_DEBUG_FS
+static struct debugfs_blob_wrapper dbg_bch_geo;
+static void bch_create_debugfs(struct gpmi_nand_data *this)
+{
+	struct bch_geometry *bch_geo = &this->bch_geometry;
+	struct dentry *dbg_root;
+
+	dbg_root = debugfs_create_dir("gpmi-nand", NULL);
+	dbg_bch_geo.data = (void *)bch_geo;
+	dbg_bch_geo.size = sizeof(struct bch_geometry);
+	debugfs_create_blob("bch_geometry", 0444, dbg_root, &dbg_bch_geo);
+	/* create raw mode flag */
+	debugfs_create_file_full("raw_mode", 0444, dbg_root, NULL, NULL, NULL);
+}
+#else
+static void bch_create_debugfs(struct gpmi_nand_data *this)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
 /* Configures the geometry for BCH.  */
 static int bch_set_geometry(struct gpmi_nand_data *this)
 {
@@ -2282,6 +2303,9 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
 	if (ret)
 		return ret;
 
+	/* save BCH geometry to debugfs if CONFIG_DEBUG_FS is enabled */
+	bch_create_debugfs(this);
+
 	/* Init the nand_ecc_ctrl{} */
 	ecc->read_page	= gpmi_ecc_read_page;
 	ecc->write_page	= gpmi_ecc_write_page;

---
base-commit: f97bdc8ec1dc7b33781a702eeba55326c206be56
change-id: 20260727-nand_debugfs-74d6d6ba52df

Best regards,
--  
Han Xu <han.xu@nxp.com>

Re: [PATCH] mtd: rawnand: gpmi: add debugfs entry for BCH geometry

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing cleanup of debugfs entries leads to a Use-After-Free on device unbind or module unload.
- [Medium] The use of a static global variable for the debugfs blob wrapper and a hardcoded directory name breaks support for multiple device instances.
- [Medium] The `raw_mode` debugfs file is created with NULL file operations, making it completely non-functional.
--

--- Patch [1]: [PATCH] mtd: rawnand: gpmi: add debugfs entry for BCH geometry ---
commit 687d6ba8c3ad7f3c6c8dbd852caa00f732194bfb
Author: Han Xu <han.xu@nxp.com>

mtd: rawnand: gpmi: add debugfs entry for BCH geometry

Export the BCH geometry parameters via debugfs to aid debugging and
provide the necessary information for legacy kobs-ng tool. The debugfs
directory "gpmi-nand" is created under the root debugfs tree, exposing
the bch_geometry and raw_mode. The implementation is guarded with
#ifdef CONFIG_DEBUG_FS to avoid build failures.

> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index c1f766cb225aa..3ceb8df7f8f93 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c

[ ... ]

> @@ -732,6 +733,26 @@ static int common_nfc_set_geometry(struct gpmi_nand_data *this)
>  	return err;
>  }
>  
> +#ifdef CONFIG_DEBUG_FS
> +static struct debugfs_blob_wrapper dbg_bch_geo;

[Severity: Medium]
Since this blob wrapper is declared as a static global variable, what happens
if there are multiple instances of the GPMI NAND controller probed?

It looks like the second instance will overwrite the global .data pointer
with its own geometry pointer, causing all reads from the debugfs file to
return the geometry of the most recently probed device.

> +static void bch_create_debugfs(struct gpmi_nand_data *this)
> +{
> +	struct bch_geometry *bch_geo = &this->bch_geometry;
> +	struct dentry *dbg_root;
> +
> +	dbg_root = debugfs_create_dir("gpmi-nand", NULL);

[Severity: High]
Does this code leak the debugfs directory and files when the driver is
unbound or the module is unloaded?

It appears that gpmi_nand_remove() lacks a corresponding call to
debugfs_remove() or debugfs_remove_recursive() to tear down this tree.
Since bch_geo points to memory within gpmi_nand_data which is managed
by devres, can this lead to a use-after-free if userspace reads the
debugfs file after the device is unbound?

Additionally, hardcoding the debugfs directory name to "gpmi-nand" might
cause creation conflicts if multiple controller instances are present.

> +	dbg_bch_geo.data = (void *)bch_geo;
> +	dbg_bch_geo.size = sizeof(struct bch_geometry);
> +	debugfs_create_blob("bch_geometry", 0444, dbg_root, &dbg_bch_geo);
> +	/* create raw mode flag */
> +	debugfs_create_file_full("raw_mode", 0444, dbg_root, NULL, NULL, NULL);

[Severity: Medium]
Are we passing NULL for the file operations here?

If the fops parameter is NULL, the debugfs core assigns
debugfs_noop_file_operations, which simply returns EOF (0 bytes) when
read. Does this make the raw_mode debugfs file completely non-functional
instead of exposing the intended boolean flag?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-nand_debugfs-v1-1-7cba1ceab5ee@nxp.com?part=1

Re: [PATCH] mtd: rawnand: gpmi: add debugfs entry for BCH geometry

[Miquel Raynal]

Hi Han,

On 27/07/2026 at 16:09:08 -05, han.xu@oss.nxp.com wrote:

> From: Han Xu <han.xu@nxp.com>
>
> Export the BCH geometry parameters via debugfs to aid debugging and
> provide the necessary information for legacy kobs-ng tool. The debugfs
> directory "gpmi-nand" is created under the root debugfs tree, exposing:
>
> bch_geometry  - blob containing the struct bch_geometry fields
> raw_mode      - flag indicating raw mode status
>
> The implementation is guarded with #ifdef CONFIG_DEBUG_FS to avoid
> build failures when debugfs is disabled.
>
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index c1f766cb225aa..3ceb8df7f8f93 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -7,6 +7,7 @@
>   */
>  #include <linux/cleanup.h>
>  #include <linux/clk.h>
> +#include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/slab.h>
>  #include <linux/sched/task_stack.h>
> @@ -732,6 +733,26 @@ static int common_nfc_set_geometry(struct gpmi_nand_data *this)
>  	return err;
>  }
>  
> +#ifdef CONFIG_DEBUG_FS
> +static struct debugfs_blob_wrapper dbg_bch_geo;

Why do you use a static variable for that? This should be dynamically
allocated otherwise if you ever have more than one device you'll have
strange results.

> +static void bch_create_debugfs(struct gpmi_nand_data *this)
> +{
> +	struct bch_geometry *bch_geo = &this->bch_geometry;
> +	struct dentry *dbg_root;
> +
> +	dbg_root = debugfs_create_dir("gpmi-nand", NULL);
> +	dbg_bch_geo.data = (void *)bch_geo;
> +	dbg_bch_geo.size = sizeof(struct bch_geometry);
> +	debugfs_create_blob("bch_geometry", 0444, dbg_root, &dbg_bch_geo);
> +	/* create raw mode flag */
> +	debugfs_create_file_full("raw_mode", 0444, dbg_root, NULL, NULL,
> NULL);

I am not sure about the _full suffix, bug debufs_create_file() is enough
here, provided that you give a tangible variable to expose.

> +}
> +#else
> +static void bch_create_debugfs(struct gpmi_nand_data *this)
> +{
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +
>  /* Configures the geometry for BCH.  */
>  static int bch_set_geometry(struct gpmi_nand_data *this)
>  {
> @@ -2282,6 +2303,9 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
>  	if (ret)
>  		return ret;
>  
> +	/* save BCH geometry to debugfs if CONFIG_DEBUG_FS is enabled */
> +	bch_create_debugfs(this);

You need to call debugfs_remove_recursive() in the removal path.

> +
>  	/* Init the nand_ecc_ctrl{} */
>  	ecc->read_page	= gpmi_ecc_read_page;
>  	ecc->write_page	= gpmi_ecc_write_page;
>
> ---
> base-commit: f97bdc8ec1dc7b33781a702eeba55326c206be56
> change-id: 20260727-nand_debugfs-74d6d6ba52df
>
> Best regards,

Thanks,
Miquèl