> Subject: Re: [PATCH v2 3/6] firmware: arm_scmi: add initial support for i.MX > BBM protocol > > 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. > > ..... > > +#include "notify.h" > > + > > +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000 > > + > > 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 Sorry for missing the previous comment. Will add some comments in the file. > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F% > 2Flore.kernel.org%2Flinux-arm- > kernel%2FZeGtoJ7ztSe8Kg8R%40bogus%2F%23t&data=05%7C02%7Cpeng.fa > n%40nxp.com%7C37b12c01b51f4329e9e308dc57f66153%7C686ea1d3bc2b > 4c6fa92cd99c5c301635%7C0%7C0%7C638481962901820964%7CUnknown > %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha > WwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=d2XRugSYyiFuUnE5R2Oz6p > xmXBaPC9lZ%2Bb%2FcMBuXeKo%3D&reserved=0 > > > +enum scmi_imx_bbm_protocol_cmd { > > + IMX_BBM_GPR_SET = 0x3, .... > > + cfg->flags = 0; > > + cfg->value_low = lower_32_bits(sec); > > + cfg->value_high = upper_32_bits(sec); > > Sorry I may have not been clear on this, but when I mentioned lower/upper > helpers I did not mean that they solved ALSO the endianity problem, so I > suppose that after having chunked your 64bits value in 2, you still want to > transmit it as 2 LE quantity....this is generally the expectation for SCMI > payloads...in this case any available documentation about the expected > command layout would have helped... Got it , will fix in v3. > > > + > > + ret = ph->xops->do_xfer(ph, t); > > + > > + ph->xops->xfer_put(ph, t); > > + > > + return ret; > > +} > > + > > +static int scmi_imx_bbm_rtc_time_get(const struct scmi_protocol_handle > *ph, > > + u32 rtc_id, u64 *value) > > +{ > > + struct scmi_imx_bbm_info *pi = ph->get_priv(ph); > > + struct scmi_imx_bbm_get_time *cfg; > > + struct scmi_xfer *t; > > + int ret; > > + > > + if (rtc_id >= pi->nr_rtc) > > + return -EINVAL; > > + > > + ret = ph->xops->xfer_get_init(ph, IMX_BBM_RTC_TIME_GET, > sizeof(*cfg), > > + sizeof(u64), &t); > > + if (ret) > > + return ret; > > + > > + cfg = t->tx.buf; > > + cfg->id = cpu_to_le32(rtc_id); > > + cfg->flags = 0; > > + > > + ret = ph->xops->do_xfer(ph, t); > > + if (!ret) > > + *value = get_unaligned_le64(t->rx.buf); > > + > > + ph->xops->xfer_put(ph, t); > > + > > + return ret; > > +} > > + > > +static int scmi_imx_bbm_rtc_alarm_set(const struct scmi_protocol_handle > *ph, > > + u32 rtc_id, u64 sec) > > +{ > > + struct scmi_imx_bbm_info *pi = ph->get_priv(ph); > > + struct scmi_imx_bbm_alarm_time *cfg; > > + struct scmi_xfer *t; > > + int ret; > > + > > + if (rtc_id >= pi->nr_rtc) > > + return -EINVAL; > > + > > + ret = ph->xops->xfer_get_init(ph, IMX_BBM_RTC_ALARM_SET, > sizeof(*cfg), 0, &t); > > + if (ret) > > + return ret; > > + > > + cfg = t->tx.buf; > > + cfg->id = cpu_to_le32(rtc_id); > > + cfg->flags = SCMI_IMX_BBM_RTC_ALARM_ENABLE_FLAG; > > + cfg->value_low = lower_32_bits(sec); > > + cfg->value_high = upper_32_bits(sec); > > Same. Fix in V3. Thanks, Peng > > Thanks, > Cristian
