> 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@xxxxxxx>
> >
> > 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@xxxxxxx>
> > ---
> > 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@xxxxxxx>
Thanks for help reviewing the patches.
Thanks,
Peng.
>
> Thanks,
> Cristian
