> Subject: Re: [PATCH v2 3/6] firmware: arm_scmi: add initial support for i.MX > BBM protocol > > 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@xxxxxxx> > > > > > > > > > > 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%2F > > > > > lore%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C6a989d0fb4c1454e94 > 3608 > > > > > dc5882c563%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6384 > 825658 > > > > > 71932293%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > V2luM > > > > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=OCvYPh3l94 > 0aQ > > > > 7mbAgrfEJMhYmix9%2FUUOA6kZyZJYWQ%3D&reserved=0 > > > > .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. ok, will add more comment in the patch for the commands. For GPR, there are some general purpose registers in BBM module that could survive during power cycle, so users could set their own value and use. > > > 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 ? BUILD_INFO here is to expose the commit hash and patch numbers. This is firmware developer's decision to add this command, I could not say it is less or more important. > > These are simple pointers, expect more questions like this if the document is > not self sufficient in explaining such details. My plan is to add only the commands that this patch use, not add all the commands. So in v3, there will be doc only for the commands included, hope this is ok. Thanks, Peng. > > -- > Regards, > Sudeep
