[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<AM6PR04MB59419D538B85A3FF0C79821088C82@AM6PR04MB5941.eurprd04.prod.outlook.com>
Date: Thu, 20 Jun 2024 03:13:30 +0000
From: Peng Fan <peng.fan@....com>
To: Cristian Marussi <cristian.marussi@....com>, "Peng Fan (OSS)"
<peng.fan@....nxp.com>
CC: Jonathan Corbet <corbet@....net>, Shawn Guo <shawnguo@...nel.org>, Sascha
Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team
<kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, Sudeep Holla
<sudeep.holla@....com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>
Subject: RE: [PATCH v4 3/6] firmware: arm_scmi: add initial support for i.MX
BBM protocol
> Subject: Re: [PATCH v4 3/6] firmware: arm_scmi: add initial support for
> i.MX BBM protocol
>
> On Fri, May 24, 2024 at 04:56:45PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@....com>
> >
> > i.MX95 has a battery-backed module(BBM), which has persistent
> storage
> > (GPR), an RTC, and the ON/OFF button. The System Manager(SM)
> firmware
> > use SCMI vendor protocol(SCMI BBM) to let agent be able to use GPR,
> > RTC and ON/OFF button.
> >
> > Signed-off-by: Peng Fan <peng.fan@....com>
> > ---
> > drivers/firmware/arm_scmi/Kconfig | 2 +
> > drivers/firmware/arm_scmi/Makefile | 1 +
> > drivers/firmware/arm_scmi/imx/Kconfig | 14 ++
> > drivers/firmware/arm_scmi/imx/Makefile | 2 +
> > drivers/firmware/arm_scmi/imx/imx-sm-bbm.c | 380
> +++++++++++++++++++++++++++++
> > include/linux/scmi_imx_protocol.h | 42 ++++
> > 6 files changed, 441 insertions(+)
> >
> > diff --git a/drivers/firmware/arm_scmi/Kconfig
> > b/drivers/firmware/arm_scmi/Kconfig
> > index aa5842be19b2..79846cbaf71b 100644
> > --- a/drivers/firmware/arm_scmi/Kconfig
> > +++ b/drivers/firmware/arm_scmi/Kconfig
> > @@ -180,4 +180,6 @@ config ARM_SCMI_POWER_CONTROL
> > called scmi_power_control. Note this may needed early in
> boot to catch
> > early shutdown/reboot SCMI requests.
> >
> > +source "drivers/firmware/arm_scmi/imx/Kconfig"
> > +
>
> It could be that we fold all the Vendor drivers under
> drivers/firmware/arm_scmi/vendors once it is merged...but we will
> take care of this reowrk/refctor...still not sure about this details.
ok. Sudeep may comment more on this.
>
> > endmenu
> > diff --git a/drivers/firmware/arm_scmi/Makefile
> > b/drivers/firmware/arm_scmi/Makefile
> > index fd59f58ce8a2..fb9407fef60c 100644
> > --- a/drivers/firmware/arm_scmi/Makefile
> > +++ b/drivers/firmware/arm_scmi/Makefile
.....
> > +
> > + ret = scmi_imx_bbm_attributes_get(ph, binfo);
> > + if (ret)
> > + return ret;
> > +
> > + return ph->set_priv(ph, binfo, version); }
>
> I would move this init down below, right before the scmi_imx_bbm and
> after the proto_ops definition, for consistency and readability.
Yeah. Fix in v5.
>
> > +
....
> > + .button_get = scmi_imx_bbm_button_get, };
> > +
>
> ...just here the init
>
> > +static const struct scmi_protocol scmi_imx_bbm = {
> > + .id = SCMI_PROTOCOL_IMX_BBM,
> > + .owner = THIS_MODULE,
> > + .instance_init = &scmi_imx_bbm_protocol_init,
> > + .ops = &scmi_imx_bbm_proto_ops,
> > + .events = &scmi_imx_bbm_protocol_events,
> > + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
> > + .vendor_id = "NXP",
> > + .sub_vendor_id = "i.MX95 EVK",
> > +};
> > +
>
> Beside this, LGTM.
>
> Reviewed-by: Cristian Marussi <cristian.marussi@....com>
Thanks for help reviewing the patches.
Thanks,
Peng.
>
> Thanks,
> Cristian
Powered by blists - more mailing lists