| Message ID | 20240718-imx95-bbm-misc-v2-v6-0-18f008e16e9d@nxp.com |
|---|---|
| Headers | show |
| Series | firmware: support i.MX95 SCMI BBM/MISC Extenstion | expand |
> Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI > Extension protocol > > On 18/07/2024 09:41, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Add i.MX SCMI Extension protocols bindings for: > > - Battery Backed Module(BBM) Protocol > > This contains persistent storage (GPR), an RTC, and the ON/OFF > button. > > The protocol can also provide access to similar functions > implemented via > > external board components. > > - MISC Protocol. > > This includes controls that are misc settings/actions that must be > exposed > > from the SM to agents. They are device specific and are usually > define to > > access bit fields in various mix block control modules, IOMUX_GPR, > and > > other GPR/CSR owned by the SM. > > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org> > > Why quotes? Which tools did you use? I could not recall why have this. I will drop and resend the patchset. Thanks, Peng. > > Best regards, > Krzysztof >
> Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI > Extension protocol > > On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 > SCMI > > > Extension protocol > > > > > > On 18/07/2024 09:41, Peng Fan (OSS) wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > Add i.MX SCMI Extension protocols bindings for: > > > > - Battery Backed Module(BBM) Protocol > > > > This contains persistent storage (GPR), an RTC, and the ON/OFF > > > button. > > > > The protocol can also provide access to similar functions > > > implemented via > > > > external board components. > > > > - MISC Protocol. > > > > This includes controls that are misc settings/actions that must > > > > be > > > exposed > > > > from the SM to agents. They are device specific and are usually > > > define to > > > > access bit fields in various mix block control modules, > > > > IOMUX_GPR, > > > and > > > > other GPR/CSR owned by the SM. > > > > > > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org> > > > > > > Why quotes? Which tools did you use? > > > > I could not recall why have this. I will drop and resend the patchset. > > > > Resend only if you have any other comments addressed, no spin just > for this one please. Sorry, I pushed the button too quick to send out v7(just correct this R-b tag and rebased to linux-next) before checking my inbox. Regards, Peng. > > -- > Regards, > Sudeep
On Wed, Jul 31, 2024 at 12:49:59PM +0000, Peng Fan wrote: > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI > > Extension protocol > > > > On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote: > > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 > > SCMI > > > > Extension protocol > > > > > > > > On 18/07/2024 09:41, Peng Fan (OSS) wrote: > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > > > Add i.MX SCMI Extension protocols bindings for: > > > > > - Battery Backed Module(BBM) Protocol > > > > > This contains persistent storage (GPR), an RTC, and the ON/OFF > > > > button. > > > > > The protocol can also provide access to similar functions > > > > implemented via > > > > > external board components. > > > > > - MISC Protocol. > > > > > This includes controls that are misc settings/actions that must > > > > > be > > > > exposed > > > > > from the SM to agents. They are device specific and are usually > > > > define to > > > > > access bit fields in various mix block control modules, > > > > > IOMUX_GPR, > > > > and > > > > > other GPR/CSR owned by the SM. > > > > > > > > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org> > > > > > > > > Why quotes? Which tools did you use? > > > > > > I could not recall why have this. I will drop and resend the patchset. > > > > > > > Resend only if you have any other comments addressed, no spin just > > for this one please. > > Sorry, I pushed the button too quick to send out v7(just correct > this R-b tag and rebased to linux-next) before checking my inbox. > Just makes me wonder if I should wait for 3-4 pings + 2-3 weeks after the last version of any of your patch set without any version bump before I can look at it. Otherwise it is quite impossible to match your speed of patch respinning and the whole review process gets complicated to follow. Also it is bit sad that you thought it needs to be re-spinned just for this which can be easily fixed when applying. Also I haven't even started looking at this series for the reason I mentioned above. -- Regards, Sudeep
Hi Sudeep, > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI > Extension protocol > > On Wed, Jul 31, 2024 at 12:49:59PM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 > SCMI > > > Extension protocol > > > > > > On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote: > > > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 > > > SCMI > > > > > Extension protocol > > > > > > > > > > On 18/07/2024 09:41, Peng Fan (OSS) wrote: > > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > > > > > Add i.MX SCMI Extension protocols bindings for: > > > > > > - Battery Backed Module(BBM) Protocol > > > > > > This contains persistent storage (GPR), an RTC, and the > > > > > > ON/OFF > > > > > button. > > > > > > The protocol can also provide access to similar functions > > > > > implemented via > > > > > > external board components. > > > > > > - MISC Protocol. > > > > > > This includes controls that are misc settings/actions that > > > > > > must be > > > > > exposed > > > > > > from the SM to agents. They are device specific and are > > > > > > usually > > > > > define to > > > > > > access bit fields in various mix block control modules, > > > > > > IOMUX_GPR, > > > > > and > > > > > > other GPR/CSR owned by the SM. > > > > > > > > > > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org> > > > > > > > > > > Why quotes? Which tools did you use? > > > > > > > > I could not recall why have this. I will drop and resend the > patchset. > > > > > > > > > > Resend only if you have any other comments addressed, no spin > just > > > for this one please. > > > > Sorry, I pushed the button too quick to send out v7(just correct this > > R-b tag and rebased to linux-next) before checking my inbox. > > > > Just makes me wonder if I should wait for 3-4 pings + 2-3 weeks after > the last version of any of your patch set without any version bump > before I can look at it. Otherwise it is quite impossible to match your > speed of patch respinning and the whole review process gets > complicated to follow. I think you might be busy. So just after addressing Cristian's comments, I post a new version. Then I think Cristian's R-b is good enough for the patch to be queued into your tree. So I did not wait for your reply on previous patchset. I usually wait more than one week, near two weeks. If no comments, I will ping. > > Also it is bit sad that you thought it needs to be re-spinned just for this > which can be easily fixed when applying. Also I haven't even started > looking at this series for the reason I mentioned above. I did not intend to bring trouble for your reviewing. For future scmi related patches that I do, I will wait for two weeks to collect comments. If no comments, I will post a ping(if you have a patchwork queue to check, that would be better). I will wait for your reply before post a new version. But if Cristian's comments are enough for me to do new version after two weeks time window, that would also be good. I hope the upper approach is good for you. Thanks, Peng. > > -- > Regards, > Sudeep
> Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI > Extension protocol > > On Wed, Jul 31, 2024 at 12:49:59PM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 > SCMI > > > Extension protocol > > > > > > On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote: > > > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 > > > SCMI > > > > > Extension protocol > > > > > > > > > > On 18/07/2024 09:41, Peng Fan (OSS) wrote: > > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > > > > > Add i.MX SCMI Extension protocols bindings for: > > > > > > - Battery Backed Module(BBM) Protocol > > > > > > This contains persistent storage (GPR), an RTC, and the > > > > > > ON/OFF > > > > > button. > > > > > > The protocol can also provide access to similar functions > > > > > implemented via > > > > > > external board components. > > > > > > - MISC Protocol. > > > > > > This includes controls that are misc settings/actions that > > > > > > must be > > > > > exposed > > > > > > from the SM to agents. They are device specific and are > > > > > > usually > > > > > define to > > > > > > access bit fields in various mix block control modules, > > > > > > IOMUX_GPR, > > > > > and > > > > > > other GPR/CSR owned by the SM. > > > > > > > > > > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org> > > > > > > > > > > Why quotes? Which tools did you use? > > > > > > > > I could not recall why have this. I will drop and resend the > patchset. > > > > > > > > > > Resend only if you have any other comments addressed, no spin > just > > > for this one please. > > > > Sorry, I pushed the button too quick to send out v7(just correct this > > R-b tag and rebased to linux-next) before checking my inbox. > > > I just rechecked the whole series patch version history from the cover-letter. And I have to respond again to your reply. > Just makes me wonder if I should wait for 3-4 pings + 2-3 weeks after > the last version of any of your patch set without any version bump > before I can look at it. I hope not. I think I not did rapid version respin. Otherwise it is quite impossible to match your > speed of patch respinning and the whole review process gets > complicated to follow. I'd argue for this. If you have read the cover-letter. https://lore.kernel.org/all/20240731-imx95-bbm-misc-v2-v7-0-a41394365602@nxp.com/ The patch version timeline is as below: v1: 2024-2-2 v2: 2024-4-5 v3: 2024-4-12 v4: 2024-5-24 v5: 2024-6-21 v6: 2024-7-18 v7: 2024-7-31 v2->v3 is 1 week, I admit this is short period. as you said, you not look into this patchset. It is almost 6 months, I not think it is a rapid patch version respin except that I did a quick update in v7 with just a minor R-b tag update after I reply in . > > Also it is bit sad that you thought it needs to be re-spinned just for this > which can be easily fixed when applying. I admit it is not good to just update R-b with a new version. But.. Also I haven't even started > looking at this series for the reason I mentioned above. > It is 6 months.. if just because I missed your 20mins reply, you think the whole patchset should be delayed or else, that is not fair, there must be some misunderstanding here. Thanks, Peng. > -- > Regards, > Sudeep
i.MX95 System Manager Firmware source: https://github.com/nxp-imx/imx-sm To generate html from the repo: make html i.MX95 System Manager Firmware support vendor extension protocol: - Battery Backed Module(BBM) Protocol This protocol is intended provide access to the battery-backed module. This contains persistent storage (GPR), an RTC, and the ON/OFF button. The protocol can also provide access to similar functions implemented via external board components. The BBM protocol provides functions to: - Describe the protocol version. - Discover implementation attributes. - Read/write GPR - Discover the RTCs available in the system. - Read/write the RTC time in seconds and ticks - Set an alarm (per LM) in seconds - Get notifications on RTC update, alarm, or rollover. - Get notification on ON/OFF button activity. - MISC Protocol for misc settings This includes controls that are misc settings/actions that must be exposed from the SM to agents. They are device specific and are usually define to access bit fields in various mix block control modules, IOMUX_GPR, and other GPR/CSR owned by the SM. This protocol supports the following functions: - Describe the protocol version. - Discover implementation attributes. - Set/Get a control. - Initiate an action on a control. - Obtain platform (i.e. SM) build information. - Obtain ROM passover data. - Read boot/shutdown/reset information for the LM or the system. This patchset is to support the two protocols and users that use the protocols. The upper protocol infomation is also included in patch 1 Signed-off-by: Peng Fan <peng.fan@nxp.com> To: Sudeep Holla <sudeep.holla@arm.com> To: Cristian Marussi <cristian.marussi@arm.com> To: Rob Herring <robh@kernel.org> To: Krzysztof Kozlowski <krzk+dt@kernel.org> To: Conor Dooley <conor+dt@kernel.org> To: Shawn Guo <shawnguo@kernel.org> To: Sascha Hauer <s.hauer@pengutronix.de> To: Pengutronix Kernel Team <kernel@pengutronix.de> To: Fabio Estevam <festevam@gmail.com> To: Peng Fan <peng.fan@nxp.com> To: Alexandre Belloni <alexandre.belloni@bootlin.com> To: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: arm-scmi@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: imx@lists.linux.dev Cc: linux-rtc@vger.kernel.org Cc: linux-input@vger.kernel.org Changes in v6: - Add R-b from Cristian for patch 2,3,4,5,6 - Add a new function parameter 'bool enable' to rtc_alarm_set in patch 2 - Drop dev_err per RTC maintainer, move devm_rtc_register to function end in patch 6 - Address Cristian's comment to documentation. Also moved the documentation to patch 3, which adds the imx.rst under drivers/firmware/arm_scmi/imx - Add remove hook to cancel_delayed_work_sync in patch 7 - Link to v5: https://lore.kernel.org/r/20240621-imx95-bbm-misc-v2-v5-0-b85a6bf778cb@nxp.com Changes in v5: - Collected missing comments in v1, I not intend to miss any, and sorry if I make something wrong. - Update the documentation per Cristian's comments. Not sure we need a new directory for firmware stuff, not firmware-guide direcotyr. - Added R-b for patch 3 "firmware: arm_scmi: add initial support for i.MX BBM protocol" - For patch 4, added comments in scmi_imx_misc_ctrl_validate_id, use num_sources in scmi_protocol_events, move scmi_imx_misc_protocol_init near init, use get_max_msg_size and drop MISC_MAX_VAL. - Separate the sm-bbm.c into rtc and key drivers with each has its own notifiy callback, put the driver in rtc and input directory, handle error return, add kconfig for each driver, use to_delayed_work, use READ/WRITE_ONCE, still keep ops as private, device_init_wakeup set false if failure. - For patch 5, Add kconfig for sm-misc.c. Only support one instance, so add a check ops in probe. - Link to v4: https://lore.kernel.org/r/20240524-imx95-bbm-misc-v2-v4-0-dc456995d590@nxp.com Changes in v4: - Rebased to next-20240520 - Added vendor/sub-vendor, currently the sub-vendor is "i.MX95 EVK", this may not be proper, I will check with firmware owner on this to seen any update. please still help review other parts of the patchset. - Added constrain value in binding doc, change the property name from nxp,wakeup-sources to nxp,ctrl-ids to match firmware definition. - Put i.MX code under new directory imx/ - Change the misc event from three to one, the code in previous patchset was wrong. - Link to v3: https://lore.kernel.org/r/20240412-imx95-bbm-misc-v2-v3-0-4380a4070980@nxp.com Changes in v3: - Update cover letter and patch commit log to include more information. - Add documentation for BBM and MISC protocols under Documentation/firmware-guide/nxp. Not sure if this is a good place. - Fix the bindings, hope I have addressed the issues. Drop imx,scmi.yaml. Add nxp,imx95-scmi.yaml for NXP vendor protocol properties. Add constraints, add nxp prefix for NXP vendor properties. Use anyOf in arm,scmi.yaml to ref vendor yaml. - Use cpu_to_le32 per Cristian - Link to v2: https://lore.kernel.org/r/20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com Changes in v2: - Sorry for late update since v1. - Add a new patch 1 - Address imx,scmi.yaml issues - Address comments for imx-sm-bbm.c and imx-sm-misc.c - I not add vendor id since related patches not landed in linux-next. - Link to v1: https://lore.kernel.org/r/20240202-imx95-bbm-misc-v1-0-3cb743020933@nxp.com --- Peng Fan (7): dt-bindings: firmware: add i.MX95 SCMI Extension protocol firmware: arm_scmi: add initial support for i.MX BBM protocol firmware: arm_scmi: add initial support for i.MX MISC protocol firmware: arm_scmi: add NXP i.MX95 SCMI documentation firmware: imx: add i.MX95 MISC driver rtc: support i.MX95 BBM RTC input: keyboard: support i.MX95 BBM module .../devicetree/bindings/firmware/arm,scmi.yaml | 5 +- .../bindings/firmware/nxp,imx95-scmi.yaml | 43 + drivers/firmware/arm_scmi/Kconfig | 2 + drivers/firmware/arm_scmi/Makefile | 1 + drivers/firmware/arm_scmi/imx/Kconfig | 23 + drivers/firmware/arm_scmi/imx/Makefile | 3 + drivers/firmware/arm_scmi/imx/imx-sm-bbm.c | 379 +++++++++ drivers/firmware/arm_scmi/imx/imx-sm-misc.c | 315 ++++++++ drivers/firmware/arm_scmi/imx/imx95.rst | 886 +++++++++++++++++++++ drivers/firmware/imx/Kconfig | 11 + drivers/firmware/imx/Makefile | 1 + drivers/firmware/imx/sm-misc.c | 119 +++ drivers/input/keyboard/Kconfig | 11 + drivers/input/keyboard/Makefile | 1 + drivers/input/keyboard/imx-sm-bbm-key.c | 236 ++++++ drivers/rtc/Kconfig | 8 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-imx-sm-bbm.c | 162 ++++ include/linux/firmware/imx/sm.h | 33 + include/linux/scmi_imx_protocol.h | 59 ++ 20 files changed, 2298 insertions(+), 1 deletion(-) --- base-commit: 6f051fea82cb6d99f0dc4db6cbe44d64eb066adf change-id: 20240405-imx95-bbm-misc-v2-b5e9d24adc42 Best regards,