[PATCH 0/2] firmware: arm_scmi/imx: Support dump syslog

[PATCH 0/2] firmware: arm_scmi/imx: Support dump syslog

[Peng Fan]

This is the patch 5 and 6 from patchset [1] with switching to using raw
dump, per check with Sudeep and NXP i.MX SM firmware owner

System Manager firmware provides API to dump system log information.
So add the interface for Linux to retrieve the information.

In patch 1, I drop the two structures compared to patch 5 in [1]:
struct scmi_imx_misc_sys_sleep_rec
struct scmi_imx_misc_syslog
No other changes in this patch.

In patch 2, I switched to use debugfs to do raw dump the syslog, compared
with patch 6 in [1].

[1] https://lore.kernel.org/arm-scmi/PAXPR04MB845937237E3C1AF5A2ABA8FA880CA@PAXPR04MB8459.eurprd04.prod.outlook.com/T/#m6ed303ac9c584c6e2ab39f89359f3131bdfcc9e5

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (2):
      firmware: arm_scmi: imx: Support getting syslog of MISC protocol
      firmware: imx: sm-misc: Dump syslog info

 .../firmware/arm_scmi/vendors/imx/imx-sm-misc.c    | 83 ++++++++++++++++++++++
 drivers/firmware/imx/sm-misc.c                     | 42 +++++++++++
 include/linux/scmi_imx_protocol.h                  |  2 +
 3 files changed, 127 insertions(+)
---
base-commit: 65dd046ef55861190ecde44c6d9fcde54b9fb77d
change-id: 20250910-sm-syslog-40a0dec6944b

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH 1/2] firmware: arm_scmi: imx: Support getting syslog of MISC protocol

[Peng Fan]

MISC protocol supports getting system log regarding system sleep latency,
wakeup interrupt and etc. Add the API for user to retrieve the information
from SM.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../firmware/arm_scmi/vendors/imx/imx-sm-misc.c    | 83 ++++++++++++++++++++++
 include/linux/scmi_imx_protocol.h                  |  2 +
 2 files changed, 85 insertions(+)

diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c
index 700a3f24f4efc153ca4a9ef1a9e50a7ece492a18..eae0b0562f6cf1931be612852ba2651f60820e6d 100644
--- a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c
+++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c
@@ -28,6 +28,7 @@ enum scmi_imx_misc_protocol_cmd {
 	SCMI_IMX_MISC_DISCOVER_BUILD_INFO = 0x6,
 	SCMI_IMX_MISC_CTRL_NOTIFY = 0x8,
 	SCMI_IMX_MISC_CFG_INFO_GET = 0xC,
+	SCMI_IMX_MISC_SYSLOG_GET = 0xD,
 	SCMI_IMX_MISC_BOARD_INFO = 0xE,
 };
 
@@ -89,6 +90,19 @@ struct scmi_imx_misc_cfg_info_out {
 	u8 cfgname[MISC_MAX_CFGNAME];
 };
 
+struct scmi_imx_misc_syslog_in {
+	__le32 flags;
+	__le32 index;
+};
+
+#define REMAINING(x)	le32_get_bits((x), GENMASK(31, 20))
+#define RETURNED(x)	le32_get_bits((x), GENMASK(11, 0))
+
+struct scmi_imx_misc_syslog_out {
+	__le32 numlogflags;
+	__le32 syslog[];
+};
+
 static int scmi_imx_misc_attributes_get(const struct scmi_protocol_handle *ph,
 					struct scmi_imx_misc_info *mi)
 {
@@ -371,10 +385,79 @@ static int scmi_imx_misc_cfg_info_get(const struct scmi_protocol_handle *ph)
 	return ret;
 }
 
+struct scmi_imx_misc_syslog_ipriv {
+	u32 *array;
+	u16 *size;
+};
+
+static void iter_misc_syslog_prepare_message(void *message, u32 desc_index,
+					     const void *priv)
+{
+	struct scmi_imx_misc_syslog_in *msg = message;
+
+	msg->flags = cpu_to_le32(0);
+	msg->index = cpu_to_le32(desc_index);
+}
+
+static int iter_misc_syslog_update_state(struct scmi_iterator_state *st,
+					 const void *response, void *priv)
+{
+	const struct scmi_imx_misc_syslog_out *r = response;
+	struct scmi_imx_misc_syslog_ipriv *p = priv;
+
+	st->num_returned = RETURNED(r->numlogflags);
+	st->num_remaining = REMAINING(r->numlogflags);
+	*p->size = st->num_returned + st->num_remaining;
+
+	return 0;
+}
+
+static int
+iter_misc_syslog_process_response(const struct scmi_protocol_handle *ph,
+				  const void *response,
+				  struct scmi_iterator_state *st, void *priv)
+{
+	const struct scmi_imx_misc_syslog_out *r = response;
+	struct scmi_imx_misc_syslog_ipriv *p = priv;
+
+	p->array[st->desc_index + st->loop_idx] =
+		le32_to_cpu(r->syslog[st->loop_idx]);
+
+	return 0;
+}
+
+static int scmi_imx_misc_syslog_get(const struct scmi_protocol_handle *ph, u16 *size,
+				    void *array)
+{
+	struct scmi_iterator_ops ops = {
+		.prepare_message = iter_misc_syslog_prepare_message,
+		.update_state = iter_misc_syslog_update_state,
+		.process_response = iter_misc_syslog_process_response,
+	};
+	struct scmi_imx_misc_syslog_ipriv ipriv = {
+		.array = array,
+		.size = size,
+	};
+	void *iter;
+
+	if (!array || !size || !*size)
+		return -EINVAL;
+
+	iter = ph->hops->iter_response_init(ph, &ops, *size, SCMI_IMX_MISC_SYSLOG_GET,
+					    sizeof(struct scmi_imx_misc_syslog_in),
+					    &ipriv);
+	if (IS_ERR(iter))
+		return PTR_ERR(iter);
+
+	/* If firmware return NOT SUPPORTED, propagate value to caller */
+	return ph->hops->iter_response_run(iter);
+}
+
 static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = {
 	.misc_ctrl_set = scmi_imx_misc_ctrl_set,
 	.misc_ctrl_get = scmi_imx_misc_ctrl_get,
 	.misc_ctrl_req_notify = scmi_imx_misc_ctrl_notify,
+	.misc_syslog = scmi_imx_misc_syslog_get,
 };
 
 static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph)
diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
index 27bd372cbfb142b6acb0b1cf4b82f061529d0d45..2407d7693b6ba1303e07629e45e2a7eaaa906fd3 100644
--- a/include/linux/scmi_imx_protocol.h
+++ b/include/linux/scmi_imx_protocol.h
@@ -59,6 +59,8 @@ struct scmi_imx_misc_proto_ops {
 			     u32 *num, u32 *val);
 	int (*misc_ctrl_req_notify)(const struct scmi_protocol_handle *ph,
 				    u32 ctrl_id, u32 evt_id, u32 flags);
+	int (*misc_syslog)(const struct scmi_protocol_handle *ph, u16 *size,
+			   void *array);
 };
 
 /* See LMM_ATTRIBUTES in imx95.rst */

-- 
2.37.1

Re: [PATCH 1/2] firmware: arm_scmi: imx: Support getting syslog of MISC protocol

[Frank Li]

On Wed, Sep 10, 2025 at 10:28:17PM +0800, Peng Fan wrote:
> MISC protocol supports getting system log regarding system sleep latency,
> wakeup interrupt and etc. Add the API for user to retrieve the information
> from SM.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../firmware/arm_scmi/vendors/imx/imx-sm-misc.c    | 83 ++++++++++++++++++++++
>  include/linux/scmi_imx_protocol.h                  |  2 +
>  2 files changed, 85 insertions(+)
>
...
> +
> +static int scmi_imx_misc_syslog_get(const struct scmi_protocol_handle *ph, u16 *size,
> +				    void *array)
> +{
> +	struct scmi_iterator_ops ops = {
> +		.prepare_message = iter_misc_syslog_prepare_message,
> +		.update_state = iter_misc_syslog_update_state,
> +		.process_response = iter_misc_syslog_process_response,
> +	};

Does it need const?  looks like also need static.

Frank
> +	struct scmi_imx_misc_syslog_ipriv ipriv = {
> +		.array = array,
> +		.size = size,
> +	};
> +	void *iter;
> +
> +	if (!array || !size || !*size)
> +		return -EINVAL;
> +
> +	iter = ph->hops->iter_response_init(ph, &ops, *size, SCMI_IMX_MISC_SYSLOG_GET,
> +					    sizeof(struct scmi_imx_misc_syslog_in),
> +					    &ipriv);
> +	if (IS_ERR(iter))
> +		return PTR_ERR(iter);
> +
> +	/* If firmware return NOT SUPPORTED, propagate value to caller */
> +	return ph->hops->iter_response_run(iter);
> +}
> +
>  static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = {
>  	.misc_ctrl_set = scmi_imx_misc_ctrl_set,
>  	.misc_ctrl_get = scmi_imx_misc_ctrl_get,
>  	.misc_ctrl_req_notify = scmi_imx_misc_ctrl_notify,
> +	.misc_syslog = scmi_imx_misc_syslog_get,
>  };
>
>  static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph)
> diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
> index 27bd372cbfb142b6acb0b1cf4b82f061529d0d45..2407d7693b6ba1303e07629e45e2a7eaaa906fd3 100644
> --- a/include/linux/scmi_imx_protocol.h
> +++ b/include/linux/scmi_imx_protocol.h
> @@ -59,6 +59,8 @@ struct scmi_imx_misc_proto_ops {
>  			     u32 *num, u32 *val);
>  	int (*misc_ctrl_req_notify)(const struct scmi_protocol_handle *ph,
>  				    u32 ctrl_id, u32 evt_id, u32 flags);
> +	int (*misc_syslog)(const struct scmi_protocol_handle *ph, u16 *size,
> +			   void *array);
>  };
>
>  /* See LMM_ATTRIBUTES in imx95.rst */
>
> --
> 2.37.1
>

RE: [PATCH 1/2] firmware: arm_scmi: imx: Support getting syslog of MISC protocol

[Peng Fan]

> Subject: Re: [PATCH 1/2] firmware: arm_scmi: imx: Support getting
> syslog of MISC protocol
[...] 
> > +{
> > +	struct scmi_iterator_ops ops = {
> > +		.prepare_message =
> iter_misc_syslog_prepare_message,
> > +		.update_state = iter_misc_syslog_update_state,
> > +		.process_response =
> iter_misc_syslog_process_response,
> > +	};
> 
> Does it need const?  looks like also need static.
> 

Current drivers/firmware/arm_scmi/drivers not use const
for this. Changes it to const will trigger build warning as I understand.

And many other places also not declare it as const, such as
pinctrl/perf/sensor.

We could take your suggestion as an improvement in a future
patch to convert them all to const.

I am not sure whether we need static for this.

Sudeep, Cristian,

Do you expect me to convert all the ops to const in this
patchset or could we defer to a future patch?

Thanks,
Peng.

> Frank
> > +	struct scmi_imx_misc_syslog_ipriv ipriv = {
> > +		.array = array,
> > +		.size = size,
> > +	};
> > +	void *iter;
> > +
> > +	if (!array || !size || !*size)
> > +		return -EINVAL;
> > +
> > +	iter = ph->hops->iter_response_init(ph, &ops, *size,
> SCMI_IMX_MISC_SYSLOG_GET,
> > +					    sizeof(struct
> scmi_imx_misc_syslog_in),
> > +					    &ipriv);
> > +	if (IS_ERR(iter))
> > +		return PTR_ERR(iter);
> > +
> > +	/* If firmware return NOT SUPPORTED, propagate value to
> caller */
> > +	return ph->hops->iter_response_run(iter);
> > +}
> > +
> >  static const struct scmi_imx_misc_proto_ops
> scmi_imx_misc_proto_ops = {
> >  	.misc_ctrl_set = scmi_imx_misc_ctrl_set,
> >  	.misc_ctrl_get = scmi_imx_misc_ctrl_get,
> >  	.misc_ctrl_req_notify = scmi_imx_misc_ctrl_notify,
> > +	.misc_syslog = scmi_imx_misc_syslog_get,
> >  };
> >
> >  static int scmi_imx_misc_protocol_init(const struct
> > scmi_protocol_handle *ph) diff --git
> > a/include/linux/scmi_imx_protocol.h
> > b/include/linux/scmi_imx_protocol.h
> > index
> >
> 27bd372cbfb142b6acb0b1cf4b82f061529d0d45..2407d7693b6ba130
> 3e07629e45e2
> > a7eaaa906fd3 100644
> > --- a/include/linux/scmi_imx_protocol.h
> > +++ b/include/linux/scmi_imx_protocol.h
> > @@ -59,6 +59,8 @@ struct scmi_imx_misc_proto_ops {
> >  			     u32 *num, u32 *val);
> >  	int (*misc_ctrl_req_notify)(const struct scmi_protocol_handle
> *ph,
> >  				    u32 ctrl_id, u32 evt_id, u32 flags);
> > +	int (*misc_syslog)(const struct scmi_protocol_handle *ph, u16
> *size,
> > +			   void *array);
> >  };
> >
> >  /* See LMM_ATTRIBUTES in imx95.rst */
> >
> > --
> > 2.37.1
> >

[PATCH 2/2] firmware: imx: sm-misc: Dump syslog info

[Peng Fan]

Add debugfs interface to read System Manager syslog info

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/sm-misc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/firmware/imx/sm-misc.c b/drivers/firmware/imx/sm-misc.c
index fc3ee12c2be878e0285183e3381c9514a63d5142..4678d76b7dd6907533b5131c15ff0edcb66f43b2 100644
--- a/drivers/firmware/imx/sm-misc.c
+++ b/drivers/firmware/imx/sm-misc.c
@@ -3,12 +3,15 @@
  * Copyright 2024 NXP
  */
 
+#include <linux/debugfs.h>
+#include <linux/device/devres.h>
 #include <linux/firmware/imx/sm.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/scmi_protocol.h>
 #include <linux/scmi_imx_protocol.h>
+#include <linux/seq_file.h>
 
 static const struct scmi_imx_misc_proto_ops *imx_misc_ctrl_ops;
 static struct scmi_protocol_handle *ph;
@@ -44,10 +47,43 @@ static int scmi_imx_misc_ctrl_notifier(struct notifier_block *nb,
 	return 0;
 }
 
+static int syslog_show(struct seq_file *file, void *priv)
+{
+	struct device *dev = file->private;
+	/* 4KB is large enough for syslog */
+	void *syslog __free(kfree) = kmalloc(SZ_4K, GFP_KERNEL);
+	/* syslog API use num words, not num bytes */
+	u16 size = SZ_4K / 4;
+	int ret;
+
+	if (!ph)
+		return -ENODEV;
+
+	ret = imx_misc_ctrl_ops->misc_syslog(ph, &size, syslog);
+	if (ret) {
+		if (size > SZ_4K / 4) {
+			dev_err(dev, "syslog size is larger than 4KB, please enlarge\n");
+			return ret;
+		}
+	}
+
+	seq_hex_dump(file, " ", DUMP_PREFIX_NONE, 16, sizeof(u32), syslog, size * 4, false);
+	seq_putc(file, '\n');
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(syslog);
+
+static void scmi_imx_misc_put(void *p)
+{
+	debugfs_remove((struct dentry *)p);
+}
+
 static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
 {
 	const struct scmi_handle *handle = sdev->handle;
 	struct device_node *np = sdev->dev.of_node;
+	struct dentry *scmi_imx_dentry;
 	u32 src_id, flags;
 	int ret, i, num;
 
@@ -98,6 +134,12 @@ static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
 		}
 	}
 
+	scmi_imx_dentry = debugfs_create_dir("scmi_imx", NULL);
+	if (!IS_ERR(scmi_imx_dentry))
+		debugfs_create_file("syslog", 0444, scmi_imx_dentry, &sdev->dev, &syslog_fops);
+
+	devm_add_action_or_reset(&sdev->dev, scmi_imx_misc_put, scmi_imx_dentry);
+
 	return 0;
 }
 

-- 
2.37.1

Re: [PATCH 2/2] firmware: imx: sm-misc: Dump syslog info

[Frank Li]

On Wed, Sep 10, 2025 at 10:28:18PM +0800, Peng Fan wrote:
> Add debugfs interface to read System Manager syslog info
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/firmware/imx/sm-misc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/firmware/imx/sm-misc.c b/drivers/firmware/imx/sm-misc.c
> index fc3ee12c2be878e0285183e3381c9514a63d5142..4678d76b7dd6907533b5131c15ff0edcb66f43b2 100644
> --- a/drivers/firmware/imx/sm-misc.c
> +++ b/drivers/firmware/imx/sm-misc.c
> @@ -3,12 +3,15 @@
>   * Copyright 2024 NXP
>   */
>
> +#include <linux/debugfs.h>
> +#include <linux/device/devres.h>
>  #include <linux/firmware/imx/sm.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/scmi_protocol.h>
>  #include <linux/scmi_imx_protocol.h>
> +#include <linux/seq_file.h>
>
>  static const struct scmi_imx_misc_proto_ops *imx_misc_ctrl_ops;
>  static struct scmi_protocol_handle *ph;
> @@ -44,10 +47,43 @@ static int scmi_imx_misc_ctrl_notifier(struct notifier_block *nb,
>  	return 0;
>  }
>
> +static int syslog_show(struct seq_file *file, void *priv)
> +{
> +	struct device *dev = file->private;
> +	/* 4KB is large enough for syslog */
> +	void *syslog __free(kfree) = kmalloc(SZ_4K, GFP_KERNEL);
> +	/* syslog API use num words, not num bytes */
> +	u16 size = SZ_4K / 4;
> +	int ret;
> +
> +	if (!ph)
> +		return -ENODEV;
> +
> +	ret = imx_misc_ctrl_ops->misc_syslog(ph, &size, syslog);
> +	if (ret) {
> +		if (size > SZ_4K / 4) {
> +			dev_err(dev, "syslog size is larger than 4KB, please enlarge\n");
> +			return ret;

suppose it is never happen, you pass down size to misc_syslog, it should
never write data more than size.

I am not sure what means of misc_syslog() return value. Generally it should
be how many data in pointer 'syslog' if return value > 0.

So seq_hex_dump() should use ret value. Then only dump validate data,
instead of the whole buffer.

Frank

> +		}
> +	}
> +
> +	seq_hex_dump(file, " ", DUMP_PREFIX_NONE, 16, sizeof(u32), syslog, size * 4, false);
> +	seq_putc(file, '\n');
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(syslog);
> +
> +static void scmi_imx_misc_put(void *p)
> +{
> +	debugfs_remove((struct dentry *)p);
> +}
> +
>  static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
>  {
>  	const struct scmi_handle *handle = sdev->handle;
>  	struct device_node *np = sdev->dev.of_node;
> +	struct dentry *scmi_imx_dentry;
>  	u32 src_id, flags;
>  	int ret, i, num;
>
> @@ -98,6 +134,12 @@ static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
>  		}
>  	}
>
> +	scmi_imx_dentry = debugfs_create_dir("scmi_imx", NULL);
> +	if (!IS_ERR(scmi_imx_dentry))
> +		debugfs_create_file("syslog", 0444, scmi_imx_dentry, &sdev->dev, &syslog_fops);
> +
> +	devm_add_action_or_reset(&sdev->dev, scmi_imx_misc_put, scmi_imx_dentry);
> +
>  	return 0;
>  }
>
>
> --
> 2.37.1
>

Re: [PATCH 2/2] firmware: imx: sm-misc: Dump syslog info

[Peng Fan]

Hi Frank,

On Wed, Sep 10, 2025 at 11:25:04AM -0400, Frank Li wrote:
>On Wed, Sep 10, 2025 at 10:28:18PM +0800, Peng Fan wrote:
[...]
>> +static int syslog_show(struct seq_file *file, void *priv)
>> +{
>> +	struct device *dev = file->private;
>> +	/* 4KB is large enough for syslog */
>> +	void *syslog __free(kfree) = kmalloc(SZ_4K, GFP_KERNEL);
>> +	/* syslog API use num words, not num bytes */
>> +	u16 size = SZ_4K / 4;
>> +	int ret;
>> +
>> +	if (!ph)
>> +		return -ENODEV;
>> +
>> +	ret = imx_misc_ctrl_ops->misc_syslog(ph, &size, syslog);
>> +	if (ret) {
>> +		if (size > SZ_4K / 4) {
>> +			dev_err(dev, "syslog size is larger than 4KB, please enlarge\n");
>> +			return ret;
>

size could be runtime updated with the returned syslog size.

>suppose it is never happen, you pass down size to misc_syslog, it should
>never write data more than size.

Right. But size could be updated even it not write data more than the input
size.

>
>I am not sure what means of misc_syslog() return value. Generally it should
>be how many data in pointer 'syslog' if return value > 0.
>
>So seq_hex_dump() should use ret value. Then only dump validate data,
>instead of the whole buffer.

size has been updated by misc_syslog, so it is not to dump whole buffer.

Thanks,
Peng

>
>Frank
>
>> +		}
>> +	}
>> +
>> +	seq_hex_dump(file, " ", DUMP_PREFIX_NONE, 16, sizeof(u32), syslog, size * 4, false);
>> +	seq_putc(file, '\n');
>> +
>> +	return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(syslog);
>> +
>> +static void scmi_imx_misc_put(void *p)
>> +{
>> +	debugfs_remove((struct dentry *)p);
>> +}
>> +
>>  static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
>>  {
>>  	const struct scmi_handle *handle = sdev->handle;
>>  	struct device_node *np = sdev->dev.of_node;
>> +	struct dentry *scmi_imx_dentry;
>>  	u32 src_id, flags;
>>  	int ret, i, num;
>>
>> @@ -98,6 +134,12 @@ static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
>>  		}
>>  	}
>>
>> +	scmi_imx_dentry = debugfs_create_dir("scmi_imx", NULL);
>> +	if (!IS_ERR(scmi_imx_dentry))
>> +		debugfs_create_file("syslog", 0444, scmi_imx_dentry, &sdev->dev, &syslog_fops);
>> +
>> +	devm_add_action_or_reset(&sdev->dev, scmi_imx_misc_put, scmi_imx_dentry);
>> +
>>  	return 0;
>>  }
>>
>>
>> --
>> 2.37.1
>>

Re: [PATCH 2/2] firmware: imx: sm-misc: Dump syslog info

[Dan Carpenter]

On Wed, Sep 10, 2025 at 10:28:18PM +0800, Peng Fan wrote:
>  static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
>  {
>  	const struct scmi_handle *handle = sdev->handle;
>  	struct device_node *np = sdev->dev.of_node;
> +	struct dentry *scmi_imx_dentry;
>  	u32 src_id, flags;
>  	int ret, i, num;
>  
> @@ -98,6 +134,12 @@ static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
>  		}
>  	}
>  
> +	scmi_imx_dentry = debugfs_create_dir("scmi_imx", NULL);
> +	if (!IS_ERR(scmi_imx_dentry))
> +		debugfs_create_file("syslog", 0444, scmi_imx_dentry, &sdev->dev, &syslog_fops);

You don't need the IS_ERR() check.  If debugfs_create_dir() fails then
just pass the error pointer to debugfs_create_file(), it's fine.

regards,
dan carpenter

> +
> +	devm_add_action_or_reset(&sdev->dev, scmi_imx_misc_put, scmi_imx_dentry);
> +
>  	return 0;
>  }
>  
> 
> -- 
> 2.37.1
> 

RE: [PATCH 2/2] firmware: imx: sm-misc: Dump syslog info

[Peng Fan]

> Subject: Re: [PATCH 2/2] firmware: imx: sm-misc: Dump syslog info
> 
> On Wed, Sep 10, 2025 at 10:28:18PM +0800, Peng Fan wrote:
> >  static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)  {
> >  	const struct scmi_handle *handle = sdev->handle;
> >  	struct device_node *np = sdev->dev.of_node;
> > +	struct dentry *scmi_imx_dentry;
> >  	u32 src_id, flags;
> >  	int ret, i, num;
> >
> > @@ -98,6 +134,12 @@ static int scmi_imx_misc_ctrl_probe(struct
> scmi_device *sdev)
> >  		}
> >  	}
> >
> > +	scmi_imx_dentry = debugfs_create_dir("scmi_imx", NULL);
> > +	if (!IS_ERR(scmi_imx_dentry))
> > +		debugfs_create_file("syslog", 0444, scmi_imx_dentry,
> &sdev->dev,
> > +&syslog_fops);
> 
> You don't need the IS_ERR() check.  If debugfs_create_dir() fails then
> just pass the error pointer to debugfs_create_file(), it's fine.

Hi Dan,

Thanks for raising this. I will fix in next version.

Thanks,
Peng.

> 
> regards,
> dan carpenter
> 
> > +
> > +	devm_add_action_or_reset(&sdev->dev, scmi_imx_misc_put,
> > +scmi_imx_dentry);
> > +
> >  	return 0;
> >  }
> >
> >
> > --
> > 2.37.1
> >

Re: [PATCH 2/2] firmware: imx: sm-misc: Dump syslog info

[kernel test robot]

Hi Peng,

kernel test robot noticed the following build errors:

[auto build test ERROR on 65dd046ef55861190ecde44c6d9fcde54b9fb77d]

url:    https://github.com/intel-lab-lkp/linux/commits/Peng-Fan/firmware-arm_scmi-imx-Support-getting-syslog-of-MISC-protocol/20250910-223316
base:   65dd046ef55861190ecde44c6d9fcde54b9fb77d
patch link:    https://lore.kernel.org/r/20250910-sm-syslog-v1-2-5b36f8f21da6%40nxp.com
patch subject: [PATCH 2/2] firmware: imx: sm-misc: Dump syslog info
config: i386-buildonly-randconfig-004-20250912 (https://download.01.org/0day-ci/archive/20250912/202509120758.omltrDMi-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250912/202509120758.omltrDMi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509120758.omltrDMi-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/firmware/imx/sm-misc.c:54:39: error: use of undeclared identifier 'SZ_4K'
      54 |         void *syslog __free(kfree) = kmalloc(SZ_4K, GFP_KERNEL);
         |                                              ^
>> drivers/firmware/imx/sm-misc.c:54:39: error: use of undeclared identifier 'SZ_4K'
>> drivers/firmware/imx/sm-misc.c:54:39: error: use of undeclared identifier 'SZ_4K'
   drivers/firmware/imx/sm-misc.c:56:13: error: use of undeclared identifier 'SZ_4K'
      56 |         u16 size = SZ_4K / 4;
         |                    ^
   drivers/firmware/imx/sm-misc.c:64:14: error: use of undeclared identifier 'SZ_4K'
      64 |                 if (size > SZ_4K / 4) {
         |                            ^
   drivers/firmware/imx/sm-misc.c:64:14: error: use of undeclared identifier 'SZ_4K'
   drivers/firmware/imx/sm-misc.c:64:14: error: use of undeclared identifier 'SZ_4K'
   7 errors generated.


vim +/SZ_4K +54 drivers/firmware/imx/sm-misc.c

    49	
    50	static int syslog_show(struct seq_file *file, void *priv)
    51	{
    52		struct device *dev = file->private;
    53		/* 4KB is large enough for syslog */
  > 54		void *syslog __free(kfree) = kmalloc(SZ_4K, GFP_KERNEL);
    55		/* syslog API use num words, not num bytes */
    56		u16 size = SZ_4K / 4;
    57		int ret;
    58	
    59		if (!ph)
    60			return -ENODEV;
    61	
    62		ret = imx_misc_ctrl_ops->misc_syslog(ph, &size, syslog);
    63		if (ret) {
    64			if (size > SZ_4K / 4) {
    65				dev_err(dev, "syslog size is larger than 4KB, please enlarge\n");
    66				return ret;
    67			}
    68		}
    69	
    70		seq_hex_dump(file, " ", DUMP_PREFIX_NONE, 16, sizeof(u32), syslog, size * 4, false);
    71		seq_putc(file, '\n');
    72	
    73		return 0;
    74	}
    75	DEFINE_SHOW_ATTRIBUTE(syslog);
    76	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki