Re: [PATCH v2 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol

[Sudeep Holla <sudeep.holla@arm.com>]

On Tue, Apr 09, 2024 at 09:13:33AM +0000, Peng Fan wrote:
> Hi Sudeep,
>
> > Subject: Re: [PATCH v2 3/6] firmware: arm_scmi: add initial support for i.MX
> > BBM protocol
> >
> > On Mon, Apr 08, 2024 at 07:04:43PM +0100, Cristian Marussi wrote:
> > > On Fri, Apr 05, 2024 at 08:39:25PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > The i.MX BBM protocol is for managing i.MX BBM module which provides
> > > > RTC and BUTTON feature.
> > > >
> > >
> > > I appreciate that you added versioning but I think a bit of
> > > documentation about what the protocol and its comamnds purpose is
> > > still lacking, as asked by Sudeep previously
> > >
> > >
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > > .kernel.org%2Flinux-arm-
> > kernel%2FZeGtoJ7ztSe8Kg8R%40bogus%2F%23t&data=
> > >
> > 05%7C02%7Cpeng.fan%40nxp.com%7Ce92ff78b9126447afe9708dc587358d
> > 4%7C686e
> > >
> > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638482499632395762%7C
> > Unknown%7C
> > >
> > TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > CJXVC
> > >
> > I6Mn0%3D%7C0%7C%7C%7C&sdata=7QP%2BkkjHA3Sa0CdcbbObGG4kgYYK
> > XAGA2r%2F%2F
> > > x0MogqU%3D&reserved=0
> > >
> >
> > I have decided to ignore all these vendor protocol patches until they have
> > some documentation to understand what these protocol are for, what are the
> > commands, their input/output parameter details, any conditions are the
> > caller and callee,..etc very similar to SCMI spec.
>
> Where do you expect the documentation to be put?
>

To begin with, we need all these vendor protocols in a directory say
vendors/nxp under drivers/firmware/arm_scmi. It can be a simple text file
under that. We can see later if we need any more formal version elsewhere
but that shouldn't be a blocker for these changes.

> similar as scmi_protocol.h, put in scmi_imx_protcol.h?
> >
> > To start with can you please expand what is BBM or MISC protocol is ?
>
> ok. Sorry for missing your previous comment in v1. Let me write here briefly
> first.
>

Thanks

> The Battery Backup (BB) Domain contains the Battery Backed Security
> Module (BBSM) and the Battery Backed Non-Secure Module (BBNSM).
> BBM protocol is to manage i.MX BBSM and BBNSM. This protocol supports
> #define COMMAND_PROTOCOL_VERSION             0x0U
> #define COMMAND_PROTOCOL_ATTRIBUTES          0x1U
> #define COMMAND_PROTOCOL_MESSAGE_ATTRIBUTES  0x2U
> #define COMMAND_BBM_GPR_SET                  0x3U
> #define COMMAND_BBM_GPR_GET                  0x4U
> #define COMMAND_BBM_RTC_ATTRIBUTES           0x5U
> #define COMMAND_BBM_RTC_TIME_SET             0x6U
> #define COMMAND_BBM_RTC_TIME_GET             0x7U
> #define COMMAND_BBM_RTC_ALARM_SET            0x8U
> #define COMMAND_BBM_BUTTON_GET               0x9U
> #define COMMAND_BBM_RTC_NOTIFY               0xAU
> #define COMMAND_BBM_BUTTON_NOTIFY            0xBU
> #define COMMAND_NEGOTIATE_PROTOCOL_VERSION   0x10U
>

Hopefully description of each of these commands cover what GPR above means
really.

> For now in this patchset for linux, we only use RTC, and BUTTON
> for system wakeup
>
> For MISC protocol, it is for various misc things, such as discover
> build info, get rom passed data, get reset reason, get i.mx
> cfg name, control set(for gpio expander under m33 control and
> etc). The command as below:
> #define COMMAND_PROTOCOL_VERSION             0x0U
> #define COMMAND_PROTOCOL_ATTRIBUTES          0x1U
> #define COMMAND_PROTOCOL_MESSAGE_ATTRIBUTES  0x2U
> #define COMMAND_MISC_CONTROL_SET             0x3U
> #define COMMAND_MISC_CONTROL_GET             0x4U
> #define COMMAND_MISC_CONTROL_ACTION          0x5U
> #define COMMAND_MISC_DISCOVER_BUILD_INFO     0x6U
> #define COMMAND_MISC_ROM_PASSOVER_GET        0x7U
> #define COMMAND_MISC_CONTROL_NOTIFY          0x8U
> #define COMMAND_MISC_REASON_ATTRIBUTES       0x9U
> #define COMMAND_MISC_RESET_REASON            0xAU
> #define COMMAND_MISC_SI_INFO                 0xBU
> #define COMMAND_MISC_CFG_INFO                0xCU
> #define COMMAND_MISC_SYSLOG                  0xDU
> #define COMMAND_NEGOTIATE_PROTOCOL_VERSION   0x10U
>

And same here. Just as an example what BUILD_INFO ? There will be 10s if not
100s of different image in the system. What does this BUILD_INFO provide ?
And why is this important over version or release info ?

These are simple pointers, expect more questions like this if the document
is not self sufficient in explaining such details.

--
Regards,
Sudeep