RE: [PATCH 7/7] firmware: imx: sm-misc: Dump syslog and system info

Peng Fan
2025-07-04T10:44:05Z

> Subject: Re: [PATCH 7/7] firmware: imx: sm-misc: Dump syslog and
> system info
>
> On Fri, Jul 04, 2025 at 06:28:02PM +0800, Peng Fan wrote:
> > On Wed, Jul 02, 2025 at 04:22:47PM +0100, Sudeep Holla wrote:
> > >On Fri, Jun 27, 2025 at 02:03:50PM +0800, Peng Fan wrote:
> > >> Add sysfs interface to read System Manager syslog and system
> info
> > >>
> > >> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > >> ---
> > >> drivers/firmware/imx/sm-misc.c | 97
> > >> ++++++++++++++++++++++++++++++++++++++++++
> > >> 1 file changed, 97 insertions(+)
> > >>
> > >> diff --git a/drivers/firmware/imx/sm-misc.c
> > >> b/drivers/firmware/imx/sm-misc.c index
> > >>
> fc3ee12c2be878e0285183e3381c9514a63d5142..55485a3c4a5c6151
> 02a377f41
> > >> 025a6911d746770 100644
> > >> --- a/drivers/firmware/imx/sm-misc.c
> > >> +++ b/drivers/firmware/imx/sm-misc.c
> > >> @@ -44,6 +44,100 @@ static int
> scmi_imx_misc_ctrl_notifier(struct notifier_block *nb,
> > >> return 0;
> > >> }
> > >>
> > >> +static ssize_t syslog_show(struct device *device, struct
> device_attribute *attr,
> > >> + char *buf)
> > >> +{
> > >> + struct scmi_imx_misc_sys_sleep_rec *rec;
> > >> + struct scmi_imx_misc_syslog *syslog;
> > >> + int ret;
> > >> + size_t len = 0;
> > >> +
> > >> + if (!ph)
> > >> + return 0;
> > >> +
> > >> + syslog = kmalloc(sizeof(*syslog), GFP_KERNEL);
> > >> + if (!syslog)
> > >> + return -ENOMEM;
> > >> +
> > >> + ret = imx_misc_ctrl_ops->misc_syslog(ph, sizeof(*syslog),
> syslog);
> > >> + if (ret) {
> > >> + kfree(syslog);
> > >> + return ret;
> > >> + }
> > >> +
> > >> + rec = &syslog->syssleeprecord;
> > >> +
> > >> + len += sysfs_emit_at(buf, len, "Wake Vector = %u\n", rec-
> >wakesource);
> > >> + len += sysfs_emit_at(buf, len, "Sys sleep mode = %u\n", rec-
> >syssleepmode);
> > >> + len += sysfs_emit_at(buf, len, "Sys sleep flags = 0x%08x\n", rec-
> >syssleepflags);
> > >> + len += sysfs_emit_at(buf, len, "MIX power status = 0x%08x\n",
> rec->mixpwrstat);
> > >> + len += sysfs_emit_at(buf, len, "MEM power status = 0x%08x\n",
> rec->mempwrstat);
> > >> + len += sysfs_emit_at(buf, len, "PLL power status = 0x%08x\n",
> rec->pllpwrstat);
> > >> + len += sysfs_emit_at(buf, len, "Sleep latency = %u\n", rec-
> >sleepentryusec);
> > >> + len += sysfs_emit_at(buf, len, "Wake latency = %u\n", rec-
> >sleepexitusec);
> > >> + len += sysfs_emit_at(buf, len, "Sleep count = %u\n",
> > >> +rec->sleepcnt);
> > >> +
> > >
> > >Why can't this be individual files and values ?
> >
> > The System manager firmware provides a command "syslog" to
> dump all
> > the information, I just follow same.
> >
> > With individual files, some values may not be consistent, because
> > system behaviour may change during reading two files.
> >
>
> You definitely need to convince Greg if you take this approach. I am
> sure he prefers single value sysfs files.

ok. Thanks for sharing me the info. I was not aware of this.
Let me follow single value for sysfs files in next version.

Thanks,
Peng.

>
> --
> Regards,
> Sudeep