* [PATCH v4 1/6] Documentation: firmware-guide: add NXP i.MX95 SCMI documentation
2024-05-24 8:56 [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan (OSS)
@ 2024-05-24 8:56 ` Peng Fan (OSS)
2024-06-17 16:21 ` Cristian Marussi
2024-05-24 8:56 ` [PATCH v4 2/6] dt-bindings: firmware: add i.MX95 SCMI Extension protocol Peng Fan (OSS)
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Peng Fan (OSS) @ 2024-05-24 8:56 UTC (permalink / raw)
To: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan
Cc: linux-doc, linux-kernel, imx, linux-arm-kernel, devicetree
From: Peng Fan <peng.fan@nxp.com>
Add NXP i.MX95 System Control Management Interface(SCMI) vendor
extensions protocol documentation.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Documentation/firmware-guide/index.rst | 10 +
Documentation/firmware-guide/nxp/imx95-scmi.rst | 877 ++++++++++++++++++++++++
Documentation/firmware-guide/nxp/index.rst | 10 +
3 files changed, 897 insertions(+)
diff --git a/Documentation/firmware-guide/index.rst b/Documentation/firmware-guide/index.rst
index 5355784ca0a2..8f66ae31337e 100644
--- a/Documentation/firmware-guide/index.rst
+++ b/Documentation/firmware-guide/index.rst
@@ -4,6 +4,9 @@
The Linux kernel firmware guide
===============================
+ACPI subsystem
+==============
+
This section describes the ACPI subsystem in Linux from firmware perspective.
.. toctree::
@@ -11,3 +14,10 @@ This section describes the ACPI subsystem in Linux from firmware perspective.
acpi/index
+NXP firmware
+============
+
+.. toctree::
+ :maxdepth: 1
+
+ nxp/index
diff --git a/Documentation/firmware-guide/nxp/imx95-scmi.rst b/Documentation/firmware-guide/nxp/imx95-scmi.rst
new file mode 100644
index 000000000000..bd87a961e4a5
--- /dev/null
+++ b/Documentation/firmware-guide/nxp/imx95-scmi.rst
@@ -0,0 +1,877 @@
+.. SPDX-License-Identifier: GPL-2.0
+.. include:: <isonum.txt>
+
+===============================================================================
+i.MX95 System Control and Management Interface(SCMI) Vendor Protocols Extension
+===============================================================================
+
+:Copyright: |copy| 2024 NXP
+
+:Author: Peng Fan <peng.fan@nxp.com>
+
+The System Manager (SM) is a low-level system function which runs on a System
+Control Processor (SCP) to support isolation and management of power domains,
+clocks, resets, sensors, pins, etc. on complex application processors. It often
+runs on a Cortex-M processor and provides an abstraction to many of the
+underlying features of the hardware. The primary purpose of the SM is to allow
+isolation between software running on different cores in the SoC. It does this
+by having exclusive access to critical resources such as those controlling
+power, clocks, reset, PMIC, etc. and then providing an RPC interface to those
+clients. This allows the SM to provide access control, arbitration, and
+aggregation policies for those shared critical resources.
+
+This document covers all the information necessary to understand, maintain,
+port, and deploy the SM on supported processors.
+
+The SM implements an interface compliant with the Arm SCMI 3.2 Specification
+with vendor specific extensions.
+
+SCMI_BBM: System Control and Management Interface Driver (BBM)
+==============================================================
+
+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.
+
+For most SoC, there is one on-chip RTC (e.g. in BBNSM) and this is RTC ID 0.
+Board code can add additional GPR and RTC.
+
+GPR are not aggregated. The RTC time is also not aggregated. Setting these
+sets for all so normally exclusive access would be granted to one agent for
+each. However, RTC alarms are maintained for each LM and the hardware is
+programmed with the next nearest alarm time. So only one agent in an LM should
+be given access rights to set an RTC alarm.
+
+Commands:
+_________
+
+PROTOCOL_VERSION
+~~~~~~~~~~~~~~~~
+
+message_id: 0x0
+protocol_id: 0x81
+
++---------------+--------------------------------------------------------------+
+|Return values |
++---------------+--------------------------------------------------------------+
+|Name |Description |
++---------------+--------------------------------------------------------------+
+|int32 status | See ARM SCMI Specification v3.2 section 4.1.4 for status |
+| | code definitions. |
++---------------+--------------------------------------------------------------+
+|uint32 version | For this revision of the specification, this value must be |
+| | 0x10000. |
++---------------+--------------------------------------------------------------+
+
+PROTOCOL_ATTRIBUTES
+~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x1
+protocol_id: 0x81
+
++---------------+--------------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status | See ARM SCMI Specification v3.2 section 4.1.4 for status |
+| | code definitions. |
++------------------+-----------------------------------------------------------+
+|uint32 attributes | Bits[31:8] Number of RTCs. |
+| | Bits[15:0] Number of persistent storage (GPR) words. |
++------------------+-----------------------------------------------------------+
+
+PROTOCOL_MESSAGE_ATTRIBUTES
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x2
+protocol_id: 0x81
+
++---------------+--------------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: in case the message is implemented and available |
+| |to use. |
+| |NOT_FOUND: if the message identified by message_id is |
+| |invalid or not implemented |
++------------------+-----------------------------------------------------------+
+|uint32 attributes |Flags that are associated with a specific function in the |
+| |protocol. For all functions in this protocol, this |
+| |parameter has a value of 0 |
++------------------+-----------------------------------------------------------+
+
+BBM_GPR_SET
+~~~~~~~~~~~
+
+message_id: 0x3
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of GPR to write |
++------------------+-----------------------------------------------------------+
+|uint32 value |32-bit value to write to the GPR |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if the GPR was successfully written. |
+| |NOT_FOUND: if the index is not valid. |
+| |DENIED: if the agent does not have permission to write |
+| |the specified GPR |
++------------------+-----------------------------------------------------------+
+
+BBM_GPR_GET
+~~~~~~~~~~~
+
+message_id: 0x4
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of GPR to read |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if the GPR was successfully written. |
+| |NOT_FOUND: if the index is not valid. |
+| |DENIED: if the agent does not have permission to write |
+| |the specified GPR. |
++------------------+-----------------------------------------------------------+
+|uint32 value |32-bit value read from the GPR |
++------------------+-----------------------------------------------------------+
+
+BBM_RTC_ATTRIBUTES
+~~~~~~~~~~~~~~~~~~
+
+message_id: 0x5
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of RTC |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: returned the attributes. |
+| |NOT_FOUND: Index is invalid. |
++------------------+-----------------------------------------------------------+
+|uint32 attributes |Bit[31:24] Bit width of RTC seconds. |
+| |Bit[23:16] Bit width of RTC ticks. |
+| |Bits[15:0] RTC ticks per second |
++------------------+-----------------------------------------------------------+
+|uint8 name[16] |Null-terminated ASCII string of up to 16 bytes in length |
+| |describing the RTC name |
++------------------+-----------------------------------------------------------+
+
+BBM_RTC_TIME_SET
+~~~~~~~~~~~~~~~~
+
+message_id: 0x6
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of RTC |
++------------------+-----------------------------------------------------------+
+|uint32 flags |Bits[31:1] Reserved, must be zero. |
+| |Bit[0] RTC time format: |
+| |Set to 1 if the time is in ticks. |
+| |Set to 0 if the time is in seconds |
++------------------+-----------------------------------------------------------+
+|uint32 time[2] |Lower word: Lower 32 bits of the time in seconds/ticks. |
+| |Upper word: Upper 32 bits of the time in seconds/ticks. |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: RTC time was successfully set. |
+| |NOT_FOUND: rtcId pertains to a non-existent RTC. |
+| |INVALID_PARAMETERS: time is not valid |
+| |(beyond the range of the RTC). |
+| |DENIED: the agent does not have permission to set the RTC. |
++------------------+-----------------------------------------------------------+
+
+BBM_RTC_TIME_GET
+~~~~~~~~~~~~~~~~
+
+message_id: 0x7
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of RTC |
++------------------+-----------------------------------------------------------+
+|uint32 flags |Bits[31:1] Reserved, must be zero. |
+| |Bit[0] RTC time format: |
+| |Set to 1 if the time is in ticks. |
+| |Set to 0 if the time is in seconds |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: RTC time was successfully set. |
+| |NOT_FOUND: rtcId pertains to a non-existent RTC. |
++------------------+-----------------------------------------------------------+
+|uint32 time[2] |Lower word: Lower 32 bits of the time in seconds/ticks. |
+| |Upper word: Upper 32 bits of the time in seconds/ticks. |
++------------------+-----------------------------------------------------------+
+
+BBM_RTC_ALARM_SET
+~~~~~~~~~~~~~~~~~
+
+message_id: 0x8
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of RTC |
++------------------+-----------------------------------------------------------+
+|uint32 flags |Bits[31:1] Reserved, must be zero. |
+| |Bit[0] RTC enable flag: |
+| |Set to 1 if the RTC alarm should be enabled. |
+| |Set to 0 if the RTC alarm should be disabled |
++------------------+-----------------------------------------------------------+
+|uint32 time[2] |Lower word: Lower 32 bits of the time in seconds. |
+| |Upper word: Upper 32 bits of the time in seconds. |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: RTC time was successfully set. |
+| |NOT_FOUND: rtcId pertains to a non-existent RTC. |
+| |INVALID_PARAMETERS: time is not valid |
+| |(beyond the range of the RTC). |
+| |DENIED: the agent does not have permission to set the RTC |
+| |alarm |
++------------------+-----------------------------------------------------------+
+
+BBM_BUTTON_GET
+~~~~~~~~~~~~~~
+
+message_id: 0x9
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if the button status was read. |
+| |Other value: ARM SCMI Specification v3.2 section 4.1.4. |
++------------------+-----------------------------------------------------------+
+|uint32 state |State of the ON/OFF button |
++------------------+-----------------------------------------------------------+
+
+BBM_RTC_NOTIFY
+~~~~~~~~~~~~~~
+
+message_id: 0xA
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of RTC |
++------------------+-----------------------------------------------------------+
+|uint32 flags |Notification flags |
+| |Bits[31:3] Reserved, must be zero. |
+| |Bit[2] Update enable: |
+| |Set to 1 to send notification. |
+| |Set to 0 if no notification. |
+| |Bit[1] Rollover enable: |
+| |Set to 1 to send notification. |
+| |Set to 0 if no notification. |
+| |Bit[0] Alarm enable: |
+| |Set to 1 to send notification. |
+| |Set to 0 if no notification |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: notification configuration was successfully |
+| |updated. |
+| |NOT_FOUND: rtcId pertains to a non-existent RTC. |
+| |DENIED: the agent does not have permission to request RTC |
+| |notifications. |
++------------------+-----------------------------------------------------------+
+
+BBM_BUTTON_NOTIFY
+~~~~~~~~~~~~~~~~~
+
+message_id: 0xB
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 flags |Notification flags |
+| |Bits[31:1] Reserved, must be zero. |
+| |Bit[0] Enable button: |
+| |Set to 1 to send notification. |
+| |Set to 0 if no notification |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: notification configuration was successfully |
+| |updated. |
+| |DENIED: the agent does not have permission to request |
+| |button notifications. |
++------------------+-----------------------------------------------------------+
+
+NEGOTIATE_PROTOCOL_VERSION
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x10
+protocol_id: 0x81
+
++--------------------+---------------------------------------------------------+
+|Parameters |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|uint32 version |The negotiated protocol version the agent intends to use |
++--------------------+---------------------------------------------------------+
+|Return values |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|int32 status |SUCCESS: if the negotiated protocol version is supported |
+| |by the platform. All commands, responses, and |
+| |notifications post successful return of this command must|
+| |comply with the negotiated version. |
+| |NOT_SUPPORTED: if the protocol version is not supported. |
++--------------------+---------------------------------------------------------+
+
+Notifications
+_____________
+
+BBM_RTC_EVENT
+~~~~~~~~~~~~~
+
+message_id: 0x0
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 flags |RTC events: |
+| |Bits[31:2] Reserved, must be zero. |
+| |Bit[1] RTC rollover notification: |
+| |1 RTC rollover detected. |
+| |0 no RTC rollover detected. |
+| |Bit[0] RTC alarm notification: |
+| |1 RTC alarm generated. |
+| |0 no RTC alarm generated. |
++------------------+-----------------------------------------------------------+
+
+BBM_BUTTON_EVENT
+~~~~~~~~~~~~~~~~
+
+message_id: 0x1
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 flags |RTC events: |
++------------------+-----------------------------------------------------------+
+| |Button events: |
+| |Bits[31:1] Reserved, must be zero. |
+| |Bit[0] Button notification: |
+| |1 button change detected. |
+| |0 no button change detected. |
++------------------+-----------------------------------------------------------+
+
+SCMI_MISC: System Control and Management Interface Driver (MISC)
+================================================================
+
+Provides misc. functions. 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.
+
+Commands:
+_________
+
+PROTOCOL_VERSION
+~~~~~~~~~~~~~~~~
+
+message_id: 0x0
+protocol_id: 0x84
+
++---------------+--------------------------------------------------------------+
+|Return values |
++---------------+--------------------------------------------------------------+
+|Name |Description |
++---------------+--------------------------------------------------------------+
+|int32 status | See ARM SCMI Specification v3.2 section 4.1.4 for status |
+| | code definitions. |
++---------------+--------------------------------------------------------------+
+|uint32 version | For this revision of the specification, this value must be |
+| | 0x10000. |
++---------------+--------------------------------------------------------------+
+
+PROTOCOL_ATTRIBUTES
+~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x1
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |See ARM SCMI Specification v3.2 section 4.1.4 for status |
+| |code definitions. |
++------------------+-----------------------------------------------------------+
+|uint32 attributes |Protocol attributes: |
+| |Bits[31:24] Reserved, must be zero. |
+| |Bits[23:16] Number of reasons. |
+| |Bits[15:0] Number of controls |
++------------------+-----------------------------------------------------------+
+
+PROTOCOL_MESSAGE_ATTRIBUTES
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x2
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: in case the message is implemented and available |
+| |to use. |
+| |NOT_FOUND: if the message identified by message_id is |
+| |invalid or not implemented |
++------------------+-----------------------------------------------------------+
+|uint32 attributes |Flags that are associated with a specific function in the |
+| |protocol. For all functions in this protocol, this |
+| |parameter has a value of 0 |
++------------------+-----------------------------------------------------------+
+
+MISC_CONTROL_SET
+~~~~~~~~~~~~~~~~
+
+message_id: 0x3
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of the control |
++------------------+-----------------------------------------------------------+
+|uint32 num |Size of the value data in words |
++------------------+-----------------------------------------------------------+
+|uint32 val[8] |value data array |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if the control was set successfully. |
+| |NOT_FOUND: if the index is not valid. |
+| |DENIED: if the agent does not have permission to set the |
+| |control |
++------------------+-----------------------------------------------------------+
+
+MISC_CONTROL_GET
+~~~~~~~~~~~~~~~~
+
+message_id: 0x4
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of the control |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if the control was set successfully. |
+| |NOT_FOUND: if the index is not valid. |
+| |DENIED: if the agent does not have permission to get the |
+| |control |
++------------------+-----------------------------------------------------------+
+|uint32 num |Size of the return data in words |
++------------------+-----------------------------------------------------------+
+|uint32 val[8] |value data array |
++------------------+-----------------------------------------------------------+
+
+MISC_CONTROL_ACTION
+~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x5
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of the control |
++------------------+-----------------------------------------------------------+
+|uint32 action |Action for the control |
++------------------+-----------------------------------------------------------+
+|uint32 numarg |Size of the argument data |
++------------------+-----------------------------------------------------------+
+|uint32 arg[8] |Argument data array |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if the action was set successfully. |
+| |NOT_FOUND: if the index is not valid. |
+| |DENIED: if the agent does not have permission to get the |
+| |control |
++------------------+-----------------------------------------------------------+
+|uint32 num |Size of the return data in words |
++------------------+-----------------------------------------------------------+
+|uint32 val[8] |value data array |
++------------------+-----------------------------------------------------------+
+
+MISC_DISCOVER_BUILD_INFO
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x6
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if the build info was got successfully. |
+| |NOT_SUPPORTED: if the data is not available. |
++------------------+-----------------------------------------------------------+
+|uint32 buildnum |Build number |
++------------------+-----------------------------------------------------------+
+|uint32 buildcommit|Most significant 32 bits of the git commit hash |
++------------------+-----------------------------------------------------------+
+|uint8 date[16] |Date of build. Null terminated ASCII string of up to 16 |
+| |bytes in length |
++------------------+-----------------------------------------------------------+
+|uint8 time[16] |Time of build. Null terminated ASCII string of up to 16 |
+| |bytes in length |
++------------------+-----------------------------------------------------------+
+
+MISC_ROM_PASSOVER_GET
+~~~~~~~~~~~~~~~~~~~~~
+
+This function is used to obtain the ROM passover data. The returned block of
+words is structured as defined in the ROM passover section in the SoC RM.
+
+message_id: 0x7
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if the data was got successfully. |
+| |NOT_SUPPORTED: if the data is not available. |
++------------------+-----------------------------------------------------------+
+|uint32 num |Size of the passover data in words |
++------------------+-----------------------------------------------------------+
+|uint32_t data[8] |Passover data array |
++------------------+-----------------------------------------------------------+
+
+MISC_CONTROL_NOTIFY
+~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x8
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 index |Index of control |
++------------------+-----------------------------------------------------------+
+|uint32 flags |Notification flags, varies by control |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: notification configuration was successfully |
+| |updated. |
+| |NOT_FOUND: control id not exists. |
+| |INVALID_PARAMETERS: if the input attributes flag specifies |
+| |unsupported or invalid configurations.. |
+| |DENIED: if the calling agent is not permitted to request |
+| |the notification. |
++------------------+-----------------------------------------------------------+
+
+MISC_REASON_ATTRIBUTES
+~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x9
+protocol_id: 0x84
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 reasonid |Identifier for the reason |
++------------------+-----------------------------------------------------------+
+|Return values |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|int32 status |SUCCESS: if valid reason attributes are returned |
+| |NOT_FOUND: if reasonId pertains to a non-existent reason. |
++------------------+-----------------------------------------------------------+
+|uint32 attributes |Reason attributes. This parameter has the following |
+| |format: Bits[31:0] Reserved, must be zero |
+| |Bits[15:0] Number of persistent storage (GPR) words. |
++------------------+-----------------------------------------------------------+
+|uint8 name[16] |Null-terminated ASCII string of up to 16 bytes in length |
+| |describing the reason |
++------------------+-----------------------------------------------------------+
+
+MISC_RESET_REASON
+~~~~~~~~~~~~~~~~~
+
+message_id: 0xA
+protocol_id: 0x84
+
++--------------------+---------------------------------------------------------+
+|Parameters |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|uint32 flags |Reason flags. This parameter has the following format: |
+| |Bits[31:1] Reserved, must be zero. |
+| |Bit[0] System: |
+| |Set to 1 to return the system reason. |
+| |Set to 0 to return the LM reason |
++--------------------+---------------------------------------------------------+
+|Return values |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|int32 status |SUCCESS: reset reason return |
++--------------------+---------------------------------------------------------+
+|uint32 bootflags |Boot reason flags. This parameter has the format: |
+| |Bits[31] Valid. |
+| |Set to 1 if the entire reason is valid. |
+| |Set to 0 if the entire reason is not valid. |
+| |Bits[30:29] Reserved, must be zero. |
+| |Bit[28] Valid origin: |
+| |Set to 1 if the origin field is valid. |
+| |Set to 0 if the origin field is not valid. |
+| |Bits[27:24] Origin. |
+| |Bit[23] Valid err ID: |
+| |Set to 1 if the error ID field is valid. |
+| |Set to 0 if the error ID field is not valid. |
+| |Bits[22:8] Error ID. |
+| |Bit[7:0] Reason |
++--------------------+---------------------------------------------------------+
+|uint32 shutdownflags|Shutdown reason flags. This parameter has the format: |
+| |Bits[31] Valid. |
+| |Set to 1 if the entire reason is valid. |
+| |Set to 0 if the entire reason is not valid. |
+| |Bits[30:29] Number of valid extended info words. |
+| |Bit[28] Valid origin: |
+| |Set to 1 if the origin field is valid. |
+| |Set to 0 if the origin field is not valid. |
+| |Bits[27:24] Origin. |
+| |Bit[23] Valid err ID: |
+| |Set to 1 if the error ID field is valid. |
+| |Set to 0 if the error ID field is not valid. |
+| |Bits[22:8] Error ID. |
+| |Bit[7:0] Reason |
++--------------------+---------------------------------------------------------+
+|uint32 extinfo[8] |Array of extended info words |
++--------------------+---------------------------------------------------------+
+
+MISC_SI_INFO
+~~~~~~~~~~~~
+
+message_id: 0xB
+protocol_id: 0x84
+
++--------------------+---------------------------------------------------------+
+|Return values |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|int32 status |SUCCESS: silicon info return |
++--------------------+---------------------------------------------------------+
+|uint32 deviceid |Silicon specific device ID |
++--------------------+---------------------------------------------------------+
+|uint32 sirev |Silicon specific revision |
++--------------------+---------------------------------------------------------+
+|uint32 partnum |Silicon specific part number |
++--------------------+---------------------------------------------------------+
+|uint8 siname[16] |Silicon name/revision. Null terminated ASCII string of up|
+| |to 16 bytes in length |
++--------------------+---------------------------------------------------------+
+
+MISC_CFG_INFO
+~~~~~~~~~~~~~
+
+message_id: 0xC
+protocol_id: 0x84
+
++--------------------+---------------------------------------------------------+
+|Return values |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|int32 status |SUCCESS: config name return |
+| |NOT_SUPPORTED: name not available |
++--------------------+---------------------------------------------------------+
+|uint32 msel |Mode selector value |
++--------------------+---------------------------------------------------------+
+|uint8 cfgname[16] |config file basename. Null terminated ASCII string of up |
+| |to 16 bytes in length |
++--------------------+---------------------------------------------------------+
+
+MISC_SYSLOG
+~~~~~~~~~~~
+
+message_id: 0xD
+protocol_id: 0x84
+
++--------------------+---------------------------------------------------------+
+|Parameters |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|uint32 flags |Device specific flags that might impact the data returned|
+| |or clearing of the data |
++--------------------+---------------------------------------------------------+
+|uint32 logindex |Index to the first log word. Will be the first element in|
+| |the return array |
++--------------------+---------------------------------------------------------+
+|Return values |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|int32 status |SUCCESS: reset reason return |
++--------------------+---------------------------------------------------------+
+|uint32 numLogflags |Descriptor for the log data returned by this call. |
+| |Bits[31:20] Number of remaining log words. |
+| |Bits[15:12] Reserved, must be zero. |
+| |Bits[11:0] Number of log words that are returned by this |
+| |call |
++--------------------+---------------------------------------------------------+
+|uint32 syslog[16] |Log data array |
++--------------------+---------------------------------------------------------+
+
+NEGOTIATE_PROTOCOL_VERSION
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x10
+protocol_id: 0x84
+
++--------------------+---------------------------------------------------------+
+|Parameters |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|uint32 version |The negotiated protocol version the agent intends to use |
++--------------------+---------------------------------------------------------+
+|Return values |
++--------------------+---------------------------------------------------------+
+|Name |Description |
++--------------------+---------------------------------------------------------+
+|int32 status |SUCCESS: if the negotiated protocol version is supported |
+| |by the platform. All commands, responses, and |
+| |notifications post successful return of this command must|
+| |comply with the negotiated version. |
+| |NOT_SUPPORTED: if the protocol version is not supported. |
++--------------------+---------------------------------------------------------+
+
+Notifications
+_____________
+
+MISC_CONTROL_EVENT
+~~~~~~~~~~~~~~~~~~
+
+message_id: 0x0
+protocol_id: 0x81
+
++------------------+-----------------------------------------------------------+
+|Parameters |
++------------------+-----------------------------------------------------------+
+|Name |Description |
++------------------+-----------------------------------------------------------+
+|uint32 ctrlid |Identifier for the control that caused the event. |
++------------------+-----------------------------------------------------------+
+|uint32 flags |Event flags, varies by control. |
++------------------+-----------------------------------------------------------+
+
diff --git a/Documentation/firmware-guide/nxp/index.rst b/Documentation/firmware-guide/nxp/index.rst
new file mode 100644
index 000000000000..b38c980a50c6
--- /dev/null
+++ b/Documentation/firmware-guide/nxp/index.rst
@@ -0,0 +1,10 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================
+NXP Firmware Support
+====================
+
+.. toctree::
+ :maxdepth: 1
+
+ imx95-scmi
--
2.37.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 1/6] Documentation: firmware-guide: add NXP i.MX95 SCMI documentation
2024-05-24 8:56 ` [PATCH v4 1/6] Documentation: firmware-guide: add NXP i.MX95 SCMI documentation Peng Fan (OSS)
@ 2024-06-17 16:21 ` Cristian Marussi
2024-06-20 3:10 ` Peng Fan
0 siblings, 1 reply; 18+ messages in thread
From: Cristian Marussi @ 2024-06-17 16:21 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan, linux-doc,
linux-kernel, imx, linux-arm-kernel, devicetree
On Fri, May 24, 2024 at 04:56:43PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add NXP i.MX95 System Control Management Interface(SCMI) vendor
> extensions protocol documentation.
>
Hi Peng,
thanks for adding this iMX SCMI vendor-protocols extensions docs; I am
not totally sure if we will end up placing this SCMI related stuff at this
level inside Documentation/firmware-guide, but these are details that can be
ironed out later...a few comments down below.
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> Documentation/firmware-guide/index.rst | 10 +
> Documentation/firmware-guide/nxp/imx95-scmi.rst | 877 ++++++++++++++++++++++++
> Documentation/firmware-guide/nxp/index.rst | 10 +
> 3 files changed, 897 insertions(+)
>
> diff --git a/Documentation/firmware-guide/index.rst b/Documentation/firmware-guide/index.rst
> index 5355784ca0a2..8f66ae31337e 100644
> --- a/Documentation/firmware-guide/index.rst
> +++ b/Documentation/firmware-guide/index.rst
> @@ -4,6 +4,9 @@
> The Linux kernel firmware guide
> ===============================
>
> +ACPI subsystem
> +==============
> +
> This section describes the ACPI subsystem in Linux from firmware perspective.
>
> .. toctree::
> @@ -11,3 +14,10 @@ This section describes the ACPI subsystem in Linux from firmware perspective.
>
> acpi/index
>
> +NXP firmware
> +============
> +
> +.. toctree::
> + :maxdepth: 1
> +
> + nxp/index
> diff --git a/Documentation/firmware-guide/nxp/imx95-scmi.rst b/Documentation/firmware-guide/nxp/imx95-scmi.rst
> new file mode 100644
> index 000000000000..bd87a961e4a5
> --- /dev/null
> +++ b/Documentation/firmware-guide/nxp/imx95-scmi.rst
> @@ -0,0 +1,877 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. include:: <isonum.txt>
> +
> +===============================================================================
> +i.MX95 System Control and Management Interface(SCMI) Vendor Protocols Extension
> +===============================================================================
> +
> +:Copyright: |copy| 2024 NXP
> +
> +:Author: Peng Fan <peng.fan@nxp.com>
> +
> +The System Manager (SM) is a low-level system function which runs on a System
> +Control Processor (SCP) to support isolation and management of power domains,
> +clocks, resets, sensors, pins, etc. on complex application processors. It often
> +runs on a Cortex-M processor and provides an abstraction to many of the
> +underlying features of the hardware. The primary purpose of the SM is to allow
> +isolation between software running on different cores in the SoC. It does this
> +by having exclusive access to critical resources such as those controlling
> +power, clocks, reset, PMIC, etc. and then providing an RPC interface to those
> +clients. This allows the SM to provide access control, arbitration, and
> +aggregation policies for those shared critical resources.
> +
> +This document covers all the information necessary to understand, maintain,
> +port, and deploy the SM on supported processors.
> +
> +The SM implements an interface compliant with the Arm SCMI 3.2 Specification
I would not specify the exact version, since the SCMI protocol is anyway
completely discoverable and in case you evolve your support you will
have to update endlessly the doc.
> +with vendor specific extensions.
with additional vendor specific extensions.
> +
> +SCMI_BBM: System Control and Management Interface Driver (BBM)
SCMI_BBM: System Control and Management BBM Vendor 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
what is an LM ? maybe a worth a note about it above in the intro
> +- Get notifications on RTC update, alarm, or rollover.
> +- Get notification on ON/OFF button activity.
> +
> +For most SoC, there is one on-chip RTC (e.g. in BBNSM) and this is RTC ID 0.
> +Board code can add additional GPR and RTC.
> +
> +GPR are not aggregated. The RTC time is also not aggregated. Setting these
> +sets for all so normally exclusive access would be granted to one agent for
> +each. However, RTC alarms are maintained for each LM and the hardware is
> +programmed with the next nearest alarm time. So only one agent in an LM should
> +be given access rights to set an RTC alarm.
> +
> +Commands:
> +_________
> +
> +PROTOCOL_VERSION
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x0
> +protocol_id: 0x81
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values |
> ++---------------+--------------------------------------------------------------+
> +|Name |Description |
> ++---------------+--------------------------------------------------------------+
> +|int32 status | See ARM SCMI Specification v3.2 section 4.1.4 for status |
> +| | code definitions. |
I understand that you want to mention a specific table in a specific
document for a well-defined version, BUT I would drop here and all over this
versioning and refs and just generically say
| See ARM SCMI Specification for status code definitions. |
to avoid all the future churn in keeping the refs updated here, since,
as said, all versions and features are discoverable and the Linux
kernel SCMI stack takes care usually to downgrade to match the detected
protocol version.
> ++---------------+--------------------------------------------------------------+
> +|uint32 version | For this revision of the specification, this value must be |
> +| | 0x10000. |
> ++---------------+--------------------------------------------------------------+
> +
> +PROTOCOL_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x1
> +protocol_id: 0x81
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status | See ARM SCMI Specification v3.2 section 4.1.4 for status |
> +| | code definitions. |
Ditto.
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes | Bits[31:8] Number of RTCs. |
> +| | Bits[15:0] Number of persistent storage (GPR) words. |
> ++------------------+-----------------------------------------------------------+
> +
> +PROTOCOL_MESSAGE_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x2
> +protocol_id: 0x81
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: in case the message is implemented and available |
> +| |to use. |
> +| |NOT_FOUND: if the message identified by message_id is |
> +| |invalid or not implemented |
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes |Flags that are associated with a specific function in the |
> +| |protocol. For all functions in this protocol, this |
> +| |parameter has a value of 0 |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_GPR_SET
> +~~~~~~~~~~~
> +
> +message_id: 0x3
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of GPR to write |
> ++------------------+-----------------------------------------------------------+
> +|uint32 value |32-bit value to write to the GPR |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if the GPR was successfully written. |
> +| |NOT_FOUND: if the index is not valid. |
> +| |DENIED: if the agent does not have permission to write |
> +| |the specified GPR |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_GPR_GET
> +~~~~~~~~~~~
> +
> +message_id: 0x4
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of GPR to read |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if the GPR was successfully written. |
read
> +| |NOT_FOUND: if the index is not valid. |
> +| |DENIED: if the agent does not have permission to write |
to read (if it make sense to deny the read...)
> +| |the specified GPR. |
> ++------------------+-----------------------------------------------------------+
> +|uint32 value |32-bit value read from the GPR |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_RTC_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x5
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of RTC |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: returned the attributes. |
> +| |NOT_FOUND: Index is invalid. |
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes |Bit[31:24] Bit width of RTC seconds. |
> +| |Bit[23:16] Bit width of RTC ticks. |
> +| |Bits[15:0] RTC ticks per second |
> ++------------------+-----------------------------------------------------------+
> +|uint8 name[16] |Null-terminated ASCII string of up to 16 bytes in length |
> +| |describing the RTC name |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_RTC_TIME_SET
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x6
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of RTC |
> ++------------------+-----------------------------------------------------------+
> +|uint32 flags |Bits[31:1] Reserved, must be zero. |
> +| |Bit[0] RTC time format: |
> +| |Set to 1 if the time is in ticks. |
> +| |Set to 0 if the time is in seconds |
> ++------------------+-----------------------------------------------------------+
> +|uint32 time[2] |Lower word: Lower 32 bits of the time in seconds/ticks. |
> +| |Upper word: Upper 32 bits of the time in seconds/ticks. |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: RTC time was successfully set. |
> +| |NOT_FOUND: rtcId pertains to a non-existent RTC. |
> +| |INVALID_PARAMETERS: time is not valid |
> +| |(beyond the range of the RTC). |
> +| |DENIED: the agent does not have permission to set the RTC. |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_RTC_TIME_GET
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x7
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of RTC |
> ++------------------+-----------------------------------------------------------+
> +|uint32 flags |Bits[31:1] Reserved, must be zero. |
> +| |Bit[0] RTC time format: |
> +| |Set to 1 if the time is in ticks. |
> +| |Set to 0 if the time is in seconds |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: RTC time was successfully set. |
get
> +| |NOT_FOUND: rtcId pertains to a non-existent RTC. |
> ++------------------+-----------------------------------------------------------+
> +|uint32 time[2] |Lower word: Lower 32 bits of the time in seconds/ticks. |
> +| |Upper word: Upper 32 bits of the time in seconds/ticks. |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_RTC_ALARM_SET
> +~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x8
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of RTC |
> ++------------------+-----------------------------------------------------------+
> +|uint32 flags |Bits[31:1] Reserved, must be zero. |
> +| |Bit[0] RTC enable flag: |
> +| |Set to 1 if the RTC alarm should be enabled. |
> +| |Set to 0 if the RTC alarm should be disabled |
> ++------------------+-----------------------------------------------------------+
> +|uint32 time[2] |Lower word: Lower 32 bits of the time in seconds. |
> +| |Upper word: Upper 32 bits of the time in seconds. |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: RTC time was successfully set. |
> +| |NOT_FOUND: rtcId pertains to a non-existent RTC. |
> +| |INVALID_PARAMETERS: time is not valid |
> +| |(beyond the range of the RTC). |
> +| |DENIED: the agent does not have permission to set the RTC |
> +| |alarm |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_BUTTON_GET
> +~~~~~~~~~~~~~~
> +
> +message_id: 0x9
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if the button status was read. |
> +| |Other value: ARM SCMI Specification v3.2 section 4.1.4. |
> ++------------------+-----------------------------------------------------------+
> +|uint32 state |State of the ON/OFF button |
> ++------------------+-----------------------------------------------------------+
How the states are codified ? 0/1 ? with the remaining bits reserevd ?
> +
> +BBM_RTC_NOTIFY
> +~~~~~~~~~~~~~~
> +
> +message_id: 0xA
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of RTC |
> ++------------------+-----------------------------------------------------------+
> +|uint32 flags |Notification flags |
> +| |Bits[31:3] Reserved, must be zero. |
> +| |Bit[2] Update enable: |
> +| |Set to 1 to send notification. |
> +| |Set to 0 if no notification. |
> +| |Bit[1] Rollover enable: |
> +| |Set to 1 to send notification. |
> +| |Set to 0 if no notification. |
> +| |Bit[0] Alarm enable: |
> +| |Set to 1 to send notification. |
> +| |Set to 0 if no notification |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: notification configuration was successfully |
> +| |updated. |
> +| |NOT_FOUND: rtcId pertains to a non-existent RTC. |
> +| |DENIED: the agent does not have permission to request RTC |
> +| |notifications. |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_BUTTON_NOTIFY
> +~~~~~~~~~~~~~~~~~
> +
> +message_id: 0xB
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 flags |Notification flags |
> +| |Bits[31:1] Reserved, must be zero. |
> +| |Bit[0] Enable button: |
> +| |Set to 1 to send notification. |
> +| |Set to 0 if no notification |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: notification configuration was successfully |
> +| |updated. |
> +| |DENIED: the agent does not have permission to request |
> +| |button notifications. |
> ++------------------+-----------------------------------------------------------+
> +
> +NEGOTIATE_PROTOCOL_VERSION
> +~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x10
> +protocol_id: 0x81
> +
> ++--------------------+---------------------------------------------------------+
> +|Parameters |
> ++--------------------+---------------------------------------------------------+
> +|Name |Description |
> ++--------------------+---------------------------------------------------------+
> +|uint32 version |The negotiated protocol version the agent intends to use |
> ++--------------------+---------------------------------------------------------+
> +|Return values |
> ++--------------------+---------------------------------------------------------+
> +|Name |Description |
> ++--------------------+---------------------------------------------------------+
> +|int32 status |SUCCESS: if the negotiated protocol version is supported |
> +| |by the platform. All commands, responses, and |
> +| |notifications post successful return of this command must|
> +| |comply with the negotiated version. |
> +| |NOT_SUPPORTED: if the protocol version is not supported. |
> ++--------------------+---------------------------------------------------------+
> +
> +Notifications
> +_____________
> +
> +BBM_RTC_EVENT
> +~~~~~~~~~~~~~
> +
> +message_id: 0x0
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 flags |RTC events: |
> +| |Bits[31:2] Reserved, must be zero. |
> +| |Bit[1] RTC rollover notification: |
> +| |1 RTC rollover detected. |
> +| |0 no RTC rollover detected. |
> +| |Bit[0] RTC alarm notification: |
> +| |1 RTC alarm generated. |
> +| |0 no RTC alarm generated. |
> ++------------------+-----------------------------------------------------------+
> +
> +BBM_BUTTON_EVENT
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x1
> +protocol_id: 0x81
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 flags |RTC events: |
> ++------------------+-----------------------------------------------------------+
> +| |Button events: |
> +| |Bits[31:1] Reserved, must be zero. |
> +| |Bit[0] Button notification: |
> +| |1 button change detected. |
> +| |0 no button change detected. |
> ++------------------+-----------------------------------------------------------+
> +
> +SCMI_MISC: System Control and Management Interface Driver (MISC)
SCMI_MISC: System Control and Management MISC Vendor Protocol
Miscellaneous
> +================================================================
> +
> +Provides misc. functions. This includes controls that are misc. settings/actions
miscellaneous
> +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.
> +
> +Commands:
> +_________
> +
> +PROTOCOL_VERSION
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x0
> +protocol_id: 0x84
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values |
> ++---------------+--------------------------------------------------------------+
> +|Name |Description |
> ++---------------+--------------------------------------------------------------+
> +|int32 status | See ARM SCMI Specification v3.2 section 4.1.4 for status |
> +| | code definitions. |
Ditto.
> ++---------------+--------------------------------------------------------------+
> +|uint32 version | For this revision of the specification, this value must be |
> +| | 0x10000. |
> ++---------------+--------------------------------------------------------------+
> +
> +PROTOCOL_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x1
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |See ARM SCMI Specification v3.2 section 4.1.4 for status |
> +| |code definitions. |
> ++------------------+-----------------------------------------------------------+
Ditto
> +|uint32 attributes |Protocol attributes: |
> +| |Bits[31:24] Reserved, must be zero. |
> +| |Bits[23:16] Number of reasons. |
here and all the occurences down below... is it 'reason' or 'region' ?
In case it is indeed 'reasons' I am not sure ot understand what is the
usage of reason IDs in the protoocol...
> +| |Bits[15:0] Number of controls |
> ++------------------+-----------------------------------------------------------+
> +
> +PROTOCOL_MESSAGE_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x2
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: in case the message is implemented and available |
> +| |to use. |
> +| |NOT_FOUND: if the message identified by message_id is |
> +| |invalid or not implemented |
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes |Flags that are associated with a specific function in the |
> +| |protocol. For all functions in this protocol, this |
> +| |parameter has a value of 0 |
> ++------------------+-----------------------------------------------------------+
> +
> +MISC_CONTROL_SET
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x3
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of the control |
> ++------------------+-----------------------------------------------------------+
> +|uint32 num |Size of the value data in words |
> ++------------------+-----------------------------------------------------------+
> +|uint32 val[8] |value data array |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if the control was set successfully. |
> +| |NOT_FOUND: if the index is not valid. |
> +| |DENIED: if the agent does not have permission to set the |
> +| |control |
> ++------------------+-----------------------------------------------------------+
> +
> +MISC_CONTROL_GET
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x4
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of the control |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if the control was set successfully. |
get
> +| |NOT_FOUND: if the index is not valid. |
> +| |DENIED: if the agent does not have permission to get the |
> +| |control |
> ++------------------+-----------------------------------------------------------+
> +|uint32 num |Size of the return data in words |
> ++------------------+-----------------------------------------------------------+
> +|uint32 val[8] |value data array |
> ++------------------+-----------------------------------------------------------+
> +
> +MISC_CONTROL_ACTION
> +~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x5
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of the control |
> ++------------------+-----------------------------------------------------------+
> +|uint32 action |Action for the control |
> ++------------------+-----------------------------------------------------------+
> +|uint32 numarg |Size of the argument data |
Max 8, it seems...please specify
> ++------------------+-----------------------------------------------------------+
> +|uint32 arg[8] |Argument data array |
arg[N] with N in [0, numarg -1] ?
... a bit of formatting issues too above...
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if the action was set successfully. |
> +| |NOT_FOUND: if the index is not valid. |
> +| |DENIED: if the agent does not have permission to get the |
> +| |control |
> ++------------------+-----------------------------------------------------------+
> +|uint32 num |Size of the return data in words |
max 8 ?
> ++------------------+-----------------------------------------------------------+
> +|uint32 val[8] |value data array |
> ++------------------+-----------------------------------------------------------+
val[N] with N in [0, num -1] as above ... I suppose
> +
> +MISC_DISCOVER_BUILD_INFO
> +~~~~~~~~~~~~~~~~~~~~~~~~
> +
Which build version this refers to ? the SM fw build version and metadata ?
> +message_id: 0x6
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if the build info was got successfully. |
> +| |NOT_SUPPORTED: if the data is not available. |
> ++------------------+-----------------------------------------------------------+
> +|uint32 buildnum |Build number |
> ++------------------+-----------------------------------------------------------+
> +|uint32 buildcommit|Most significant 32 bits of the git commit hash |
> ++------------------+-----------------------------------------------------------+
> +|uint8 date[16] |Date of build. Null terminated ASCII string of up to 16 |
> +| |bytes in length |
> ++------------------+-----------------------------------------------------------+
> +|uint8 time[16] |Time of build. Null terminated ASCII string of up to 16 |
> +| |bytes in length |
> ++------------------+-----------------------------------------------------------+
> +
> +MISC_ROM_PASSOVER_GET
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +This function is used to obtain the ROM passover data. The returned block of
> +words is structured as defined in the ROM passover section in the SoC RM.
> +
what is a ROM passover exactly ?
> +message_id: 0x7
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if the data was got successfully. |
> +| |NOT_SUPPORTED: if the data is not available. |
> ++------------------+-----------------------------------------------------------+
> +|uint32 num |Size of the passover data in words |
> ++------------------+-----------------------------------------------------------+
> +|uint32_t data[8] |Passover data array |
> ++------------------+-----------------------------------------------------------+
> +
> +MISC_CONTROL_NOTIFY
> +~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x8
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 index |Index of control |
> ++------------------+-----------------------------------------------------------+
> +|uint32 flags |Notification flags, varies by control |
So basically this is to somehow enable notifs on the specified index
BUT the flag field syntax is completely opaque and varies by the control type...
...how is this even used in the code ? there should be at least a bit
dedicatd to enable/disable right ?
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: notification configuration was successfully |
> +| |updated. |
> +| |NOT_FOUND: control id not exists. |
> +| |INVALID_PARAMETERS: if the input attributes flag specifies |
> +| |unsupported or invalid configurations.. |
> +| |DENIED: if the calling agent is not permitted to request |
> +| |the notification. |
> ++------------------+-----------------------------------------------------------+
> +
> +MISC_REASON_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~~~~
? as above said... REASON ?
> +
> +message_id: 0x9
> +protocol_id: 0x84
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|uint32 reasonid |Identifier for the reason |
> ++------------------+-----------------------------------------------------------+
> +|Return values |
> ++------------------+-----------------------------------------------------------+
> +|Name |Description |
> ++------------------+-----------------------------------------------------------+
> +|int32 status |SUCCESS: if valid reason attributes are returned |
> +| |NOT_FOUND: if reasonId pertains to a non-existent reason. |
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes |Reason attributes. This parameter has the following |
> +| |format: Bits[31:0] Reserved, must be zero |
> +| |Bits[15:0] Number of persistent storage (GPR) words. |
> ++------------------+-----------------------------------------------------------+
> +|uint8 name[16] |Null-terminated ASCII string of up to 16 bytes in length |
> +| |describing the reason |
> ++------------------+-----------------------------------------------------------+
> +
> +MISC_RESET_REASON
> +~~~~~~~~~~~~~~~~~
> +
> +message_id: 0xA
> +protocol_id: 0x84
> +
So is this a GET ? MISC_RESET_REASON_GET ?
> ++--------------------+---------------------------------------------------------+
> +|Parameters |
> ++--------------------+---------------------------------------------------------+
> +|Name |Description |
> ++--------------------+---------------------------------------------------------+
> +|uint32 flags |Reason flags. This parameter has the following format: |
> +| |Bits[31:1] Reserved, must be zero. |
> +| |Bit[0] System: |
> +| |Set to 1 to return the system reason. |
> +| |Set to 0 to return the LM reason |
> ++--------------------+---------------------------------------------------------+
> +|Return values |
> ++--------------------+---------------------------------------------------------+
> +|Name |Description |
> ++--------------------+---------------------------------------------------------+
> +|int32 status |SUCCESS: reset reason return |
> ++--------------------+---------------------------------------------------------+
> +|uint32 bootflags |Boot reason flags. This parameter has the format: |
> +| |Bits[31] Valid. |
> +| |Set to 1 if the entire reason is valid. |
> +| |Set to 0 if the entire reason is not valid. |
> +| |Bits[30:29] Reserved, must be zero. |
> +| |Bit[28] Valid origin: |
> +| |Set to 1 if the origin field is valid. |
> +| |Set to 0 if the origin field is not valid. |
> +| |Bits[27:24] Origin. |
> +| |Bit[23] Valid err ID: |
> +| |Set to 1 if the error ID field is valid. |
> +| |Set to 0 if the error ID field is not valid. |
> +| |Bits[22:8] Error ID. |
> +| |Bit[7:0] Reason |
> ++--------------------+---------------------------------------------------------+
> +|uint32 shutdownflags|Shutdown reason flags. This parameter has the format: |
> +| |Bits[31] Valid. |
> +| |Set to 1 if the entire reason is valid. |
> +| |Set to 0 if the entire reason is not valid. |
> +| |Bits[30:29] Number of valid extended info words. |
> +| |Bit[28] Valid origin: |
> +| |Set to 1 if the origin field is valid. |
> +| |Set to 0 if the origin field is not valid. |
> +| |Bits[27:24] Origin. |
> +| |Bit[23] Valid err ID: |
> +| |Set to 1 if the error ID field is valid. |
> +| |Set to 0 if the error ID field is not valid. |
> +| |Bits[22:8] Error ID. |
> +| |Bit[7:0] Reason |
> ++--------------------+---------------------------------------------------------+
> +|uint32 extinfo[8] |Array of extended info words |
> ++--------------------+---------------------------------------------------------+
> +
> +MISC_SI_INFO
> +~~~~~~~~~~~~
> +
MISC_SI_INFO_GET ?
> +message_id: 0xB
> +ROM passoverprotocol_id: 0x84
> +
> ++--------------------+---------------------------------------------------------+
> +|Return values |
> ++--------------------+---------------------------------------------------------+
> +|Name |Description |
> ++--------------------+---------------------------------------------------------+
> +|int32 status |SUCCESS: silicon info return |
> ++--------------------+---------------------------------------------------------+
> +|uint32 deviceid |Silicon specific device ID |
> ++--------------------+---------------------------------------------------------+
> +|uint32 sirev |Silicon specific revision |
> ++--------------------+---------------------------------------------------------+
> +|uint32 partnum |Silicon specific part number |
> ++--------------------+---------------------------------------------------------+
> +|uint8 siname[16] |Silicon name/revision. Null terminated ASCII string of up|
> +| |to 16 bytes in length |
> ++--------------------+---------------------------------------------------------+
> +
> +MISC_CFG_INFO
> +~~~~~~~~~~~~~
> +
MISC_CFG_INFO_GET
> +message_id: 0xC
> +protocol_id: 0x84
> +
> ++--------------------+---------------------------------------------------------+
> +|Return values |
> ++--------------------+---------------------------------------------------------+
> +|Name |Description |
> ++--------------------+---------------------------------------------------------+
> +|int32 status |SUCCESS: config name return |
> +| |NOT_SUPPORTED: name not available |
> ++--------------------+---------------------------------------------------------+
> +|uint32 msel |Mode selector value |
> ++--------------------+---------------------------------------------------------+
> +|uint8 cfgname[16] |config file basename. Null terminated ASCII string of up |
> +| |to 16 bytes in length |
> ++--------------------+---------------------------------------------------------+
> +
> +MISC_SYSLOG
> +~~~~~~~~~~~
> +
> +message_id: 0xD
> +protocol_id: 0x84
> +
> ++--------------------+---------------------------------------------------------+
> +|Parameters |
> ++--------------------+---------------------------------------------------------+
> +|Name |Description |
> ++--------------------+---------------------------------------------------------+
> +|uint32 flags |Device specific flags that might impact the data returned|
> +| |or clearing of the data |
> ++--------------------+---------------------------------------------------------+
> +|uint32 logindex |Index to the first log word. Will be the first element in|
> +| |the return array |
> ++--------------------+---------------------------------------------------------+
> +|Return values |
> ++--------------------+---------------------------------------------------------+
> +|Name |Description |
> ++--------------------+---------------------------------------------------------+
> +|int32 status |SUCCESS: reset reason return |
??
> ++--------------------+---------------------------------------------------------+
> +|uint32 numLogflags |Descriptor for the log data returned by this call. |
> +| |Bits[31:20] Number of remaining log words. |
> +| |Bits[15:12] Reserved, must be zero. |
> +| |Bits[11:0] Number of log words that are returned by this |
> +| |call |
> ++--------------------+---------------------------------------------------------+
> +|uint32 syslog[16] |Log data array |
> ++--------------------+---------------------------------------------------------+
This should be syslog[N} with N defined by bits[11:0] in numLogflags, by
the definition itself of multi-part SCMI command...the number of returned
entries is limited by the platform dinamically based on the max size that
the configure underlying transport allows....it could be more OR less
than 16...this way is seems that you always carry around 16 bytes max,
potentially with unused bytes if returned words are far less...
Thanks,
Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v4 1/6] Documentation: firmware-guide: add NXP i.MX95 SCMI documentation
2024-06-17 16:21 ` Cristian Marussi
@ 2024-06-20 3:10 ` Peng Fan
0 siblings, 0 replies; 18+ messages in thread
From: Peng Fan @ 2024-06-20 3:10 UTC (permalink / raw)
To: Cristian Marussi, Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
> Subject: Re: [PATCH v4 1/6] Documentation: firmware-guide: add NXP
> i.MX95 SCMI documentation
>
......
> > +The SM implements an interface compliant with the Arm SCMI 3.2
> Specification
>
> I would not specify the exact version, since the SCMI protocol is
> anyway
> completely discoverable and in case you evolve your support you will
> have to update endlessly the doc.
Sure. I will fix all in the patchset.
>
...
> > +- Set an alarm (per LM) in seconds
>
> what is an LM ? maybe a worth a note about it above in the intro
Logic Machine, it is i.MX SCMI firmware specific.
I will add in v5.
>
> > +- Get notifications on RTC update, alarm, or rollover.
> > +- Get notification on ON/OFF button activity.
....
> > ++---------------+--------------------------------------------------------------+
> > +|int32 status | See ARM SCMI Specification v3.2 section 4.1.4 for
> status |
> > +| | code definitions. |
>
> I understand that you want to mention a specific table in a specific
> document for a well-defined version, BUT I would drop here and all
> over this
> versioning and refs and just generically say
>
> | See ARM SCMI Specification for status code definitions.
> |
>
> to avoid all the future churn in keeping the refs updated here, since,
> as said, all versions and features are discoverable and the Linux
> kernel SCMI stack takes care usually to downgrade to match the
> detected
> protocol version.
Sure. I will fix in v5
>
>
> > ++---------------+--------------------------------------------------------------+
> > +|uint32 version | For this revision of the specification, this value
> must be |
> > +| | 0x10000. |
> > ++---------------+--------------------------------------------------------------+
> > +
......
> > +|int32 status |SUCCESS: if the button status was read.
> |
> > +| |Other value: ARM SCMI Specification v3.2 section 4.1.4.
> |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 state |State of the ON/OFF button |
> > ++------------------+-----------------------------------------------------------+
>
> How the states are codified ? 0/1 ? with the remaining bits reserevd ?
0 or 1 for now. other bits reserved.
>
> > +
> > +BBM_RTC_NOTIFY
> > +~~~~~~~~~~~~~~
> > +
.....
> > +|uint32 index |Index of the control |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 action |Action for the control
> |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 numarg |Size of the argument data
> |
>
> Max 8, it seems...please specify
>
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 arg[8] |Argument data array
> |
>
> arg[N] with N in [0, numarg -1] ?
>
> ... a bit of formatting issues too above...
>
Yeah. Fix in v5
> > ++------------------+-----------------------------------------------------------+
> > +|Return values |
> > ++------------------+-----------------------------------------------------------+
> > +|Name |Description |
> > ++------------------+-----------------------------------------------------------+
> > +|int32 status |SUCCESS: if the action was set successfully.
> |
> > +| |NOT_FOUND: if the index is not valid. |
> > +| |DENIED: if the agent does not have permission to get
> the |
> > +| |control |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 num |Size of the return data in words |
>
> max 8 ?
>
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 val[8] |value data array |
> > ++------------------+-----------------------------------------------------------+
>
> val[N] with N in [0, num -1] as above ... I suppose
>
Fix in v5.
> > +
> > +MISC_DISCOVER_BUILD_INFO
> > +~~~~~~~~~~~~~~~~~~~~~~~~
> > +
>
> Which build version this refers to ? the SM fw build version and
> metadata ?
>
Build date, commit hash and etc.
> > +message_id: 0x6
> > +protocol_id: 0x84
> > +
> > ++------------------+-----------------------------------------------------------+
> > +|Return values |
> > ++------------------+-----------------------------------------------------------+
> > +|Name |Description |
> > ++------------------+-----------------------------------------------------------+
> > +|int32 status |SUCCESS: if the build info was got successfully.
> |
> > +| |NOT_SUPPORTED: if the data is not available. |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 buildnum |Build number |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 buildcommit|Most significant 32 bits of the git commit
> hash |
> > ++------------------+-----------------------------------------------------------+
> > +|uint8 date[16] |Date of build. Null terminated ASCII string of up
> to 16 |
> > +| |bytes in length |
> > ++------------------+-----------------------------------------------------------+
> > +|uint8 time[16] |Time of build. Null terminated ASCII string of up
> to 16 |
> > +| |bytes in length |
> > ++------------------+-----------------------------------------------------------+
> > +
> > +MISC_ROM_PASSOVER_GET
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +This function is used to obtain the ROM passover data. The returned
> block of
> > +words is structured as defined in the ROM passover section in the
> SoC RM.
> > +
>
> what is a ROM passover exactly ?
>
It is ROM stored some info in RAM that could be used by SCMI firmware,
such ad boot device and etc.
> > +message_id: 0x7
> > +protocol_id: 0x84
> > +
> > ++------------------+-----------------------------------------------------------+
> > +|Return values |
> > ++------------------+-----------------------------------------------------------+
> > +|Name |Description |
> > ++------------------+-----------------------------------------------------------+
> > +|int32 status |SUCCESS: if the data was got successfully.
> |
> > +| |NOT_SUPPORTED: if the data is not available. |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 num |Size of the passover data in words |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32_t data[8] |Passover data array |
> > ++------------------+-----------------------------------------------------------+
> > +
> > +MISC_CONTROL_NOTIFY
> > +~~~~~~~~~~~~~~~~~~~
> > +
> > +message_id: 0x8
> > +protocol_id: 0x84
> > +
> > ++------------------+-----------------------------------------------------------+
> > +|Parameters |
> > ++------------------+-----------------------------------------------------------+
> > +|Name |Description |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 index |Index of control |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 flags |Notification flags, varies by control |
>
> So basically this is to somehow enable notifs on the specified index
> BUT the flag field syntax is completely opaque and varies by the
> control type...
> ...how is this even used in the code ? there should be at least a bit
> dedicatd to enable/disable right ?
Currently the flag is in board device tree. Our current usage is
board specific.
>
> > ++------------------+-----------------------------------------------------------+
> > +|Return values |
> > ++------------------+-----------------------------------------------------------+
> > +|Name |Description |
> > ++------------------+-----------------------------------------------------------+
> > +|int32 status |SUCCESS: notification configuration was
> successfully |
> > +| |updated. |
> > +| |NOT_FOUND: control id not exists. |
> > +| |INVALID_PARAMETERS: if the input attributes flag
> specifies |
> > +| |unsupported or invalid configurations.. |
> > +| |DENIED: if the calling agent is not permitted to request
> |
> > +| |the notification. |
> > ++------------------+-----------------------------------------------------------+
> > +
> > +MISC_REASON_ATTRIBUTES
> > +~~~~~~~~~~~~~~~~~~~~~~
>
> ? as above said... REASON ?
>
It is reset reason. Such as WDOG, FCCU and etc.
> > +
> > +message_id: 0x9
> > +protocol_id: 0x84
> > +
> > ++------------------+-----------------------------------------------------------+
> > +|Parameters |
> > ++------------------+-----------------------------------------------------------+
> > +|Name |Description |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 reasonid |Identifier for the reason |
> > ++------------------+-----------------------------------------------------------+
> > +|Return values |
> > ++------------------+-----------------------------------------------------------+
> > +|Name |Description |
> > ++------------------+-----------------------------------------------------------+
> > +|int32 status |SUCCESS: if valid reason attributes are returned
> |
> > +| |NOT_FOUND: if reasonId pertains to a non-existent
> reason. |
> > ++------------------+-----------------------------------------------------------+
> > +|uint32 attributes |Reason attributes. This parameter has the
> following |
> > +| |format: Bits[31:0] Reserved, must be zero |
> > +| |Bits[15:0] Number of persistent storage (GPR) words.
> |
> > ++------------------+-----------------------------------------------------------+
> > +|uint8 name[16] |Null-terminated ASCII string of up to 16 bytes in
> length |
> > +| |describing the reason |
> > ++------------------+-----------------------------------------------------------+
> > +
> > +MISC_RESET_REASON
> > +~~~~~~~~~~~~~~~~~
> > +
> > +message_id: 0xA
> > +protocol_id: 0x84
> > +
>
> So is this a GET ? MISC_RESET_REASON_GET ?
Yes.
>
> > ++--------------------+---------------------------------------------------------+
> > +|Parameters |
....
> > ++--------------------+---------------------------------------------------------+
> > +|int32 status |SUCCESS: reset reason return |
>
> ??
Fix in v5.
>
> > ++--------------------+---------------------------------------------------------+
> > +|uint32 numLogflags |Descriptor for the log data returned by this
> call. |
> > +| |Bits[31:20] Number of remaining log words. |
> > +| |Bits[15:12] Reserved, must be zero. |
> > +| |Bits[11:0] Number of log words that are returned by
> this |
> > +| |call |
> > ++--------------------+---------------------------------------------------------+
> > +|uint32 syslog[16] |Log data array |
> > ++--------------------+---------------------------------------------------------+
>
> This should be syslog[N} with N defined by bits[11:0] in numLogflags,
> by
> the definition itself of multi-part SCMI command...the number of
> returned
> entries is limited by the platform dinamically based on the max size
> that
> the configure underlying transport allows....it could be more OR less
> than 16...this way is seems that you always carry around 16 bytes max,
> potentially with unused bytes if returned words are far less...
I need check firmware design, but your question is valid. I will check
and fix in v5.
Thanks,
Peng.
>
> Thanks,
> Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 2/6] dt-bindings: firmware: add i.MX95 SCMI Extension protocol
2024-05-24 8:56 [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan (OSS)
2024-05-24 8:56 ` [PATCH v4 1/6] Documentation: firmware-guide: add NXP i.MX95 SCMI documentation Peng Fan (OSS)
@ 2024-05-24 8:56 ` Peng Fan (OSS)
2024-05-28 16:26 ` Rob Herring (Arm)
2024-05-24 8:56 ` [PATCH v4 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol Peng Fan (OSS)
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Peng Fan (OSS) @ 2024-05-24 8:56 UTC (permalink / raw)
To: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan
Cc: linux-doc, linux-kernel, imx, linux-arm-kernel, devicetree
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.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
.../devicetree/bindings/firmware/arm,scmi.yaml | 5 ++-
.../bindings/firmware/nxp,imx95-scmi.yaml | 43 ++++++++++++++++++++++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 7de2c29606e5..cead03fbe22a 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -22,6 +22,9 @@ description: |
[0] https://developer.arm.com/documentation/den0056/latest
+anyOf:
+ - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
+
properties:
$nodename:
const: scmi
@@ -278,7 +281,7 @@ properties:
required:
- reg
-additionalProperties: false
+unevaluatedProperties: false
$defs:
protocol-node:
diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
new file mode 100644
index 000000000000..1a95010a546b
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/nxp,imx95-scmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX95 System Control and Management Interface(SCMI) Vendor Protocols Extension
+
+maintainers:
+ - Peng Fan <peng.fan@nxp.com>
+
+properties:
+ protocol@81:
+ $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ const: 0x81
+
+ protocol@84:
+ $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ const: 0x84
+
+ nxp,ctrl-ids:
+ description:
+ Each entry consists of 2 integers, represents the ctrl id and the value
+ items:
+ items:
+ - description: the ctrl id index
+ enum: [0, 1, 2, 3, 4, 5, 6, 7, 0x8000, 0x8001, 0x8002, 0x8003,
+ 0x8004, 0x8005, 0x8006, 0x8007]
+ - description: the value assigned to the ctrl id
+ minItems: 1
+ maxItems: 16
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+
+additionalProperties: true
--
2.37.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 2/6] dt-bindings: firmware: add i.MX95 SCMI Extension protocol
2024-05-24 8:56 ` [PATCH v4 2/6] dt-bindings: firmware: add i.MX95 SCMI Extension protocol Peng Fan (OSS)
@ 2024-05-28 16:26 ` Rob Herring (Arm)
0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring (Arm) @ 2024-05-28 16:26 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Fabio Estevam, linux-kernel, Pengutronix Kernel Team, Peng Fan,
linux-arm-kernel, imx, devicetree, Jonathan Corbet, linux-doc,
Conor Dooley, Shawn Guo, Sascha Hauer, Cristian Marussi,
Sudeep Holla, Krzysztof Kozlowski
On Fri, 24 May 2024 16:56:44 +0800, 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.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> .../devicetree/bindings/firmware/arm,scmi.yaml | 5 ++-
> .../bindings/firmware/nxp,imx95-scmi.yaml | 43 ++++++++++++++++++++++
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol
2024-05-24 8:56 [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan (OSS)
2024-05-24 8:56 ` [PATCH v4 1/6] Documentation: firmware-guide: add NXP i.MX95 SCMI documentation Peng Fan (OSS)
2024-05-24 8:56 ` [PATCH v4 2/6] dt-bindings: firmware: add i.MX95 SCMI Extension protocol Peng Fan (OSS)
@ 2024-05-24 8:56 ` Peng Fan (OSS)
2024-06-17 16:48 ` Cristian Marussi
2024-05-24 8:56 ` [PATCH v4 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol Peng Fan (OSS)
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Peng Fan (OSS) @ 2024-05-24 8:56 UTC (permalink / raw)
To: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan
Cc: linux-doc, linux-kernel, imx, linux-arm-kernel, devicetree
From: Peng Fan <peng.fan@nxp.com>
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@nxp.com>
---
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"
+
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
@@ -16,6 +16,7 @@ scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
+obj-$(CONFIG_ARM_SCMI_PROTOCOL) += imx/
obj-$(CONFIG_ARM_SCMI_POWER_CONTROL) += scmi_power_control.o
diff --git a/drivers/firmware/arm_scmi/imx/Kconfig b/drivers/firmware/arm_scmi/imx/Kconfig
new file mode 100644
index 000000000000..4b6ac7febe8f
--- /dev/null
+++ b/drivers/firmware/arm_scmi/imx/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "ARM SCMI NXP i.MX Vendor Protocols"
+
+config IMX_SCMI_BBM_EXT
+ tristate "i.MX SCMI BBM EXTENSION"
+ depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
+ default y if ARCH_MXC
+ help
+ This enables i.MX System BBM control logic which supports RTC
+ and BUTTON.
+
+ This driver can also be built as a module.
+
+endmenu
diff --git a/drivers/firmware/arm_scmi/imx/Makefile b/drivers/firmware/arm_scmi/imx/Makefile
new file mode 100644
index 000000000000..a7dbdd20dbb9
--- /dev/null
+++ b/drivers/firmware/arm_scmi/imx/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
diff --git a/drivers/firmware/arm_scmi/imx/imx-sm-bbm.c b/drivers/firmware/arm_scmi/imx/imx-sm-bbm.c
new file mode 100644
index 000000000000..3f8321d247ae
--- /dev/null
+++ b/drivers/firmware/arm_scmi/imx/imx-sm-bbm.c
@@ -0,0 +1,380 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) NXP BBM Protocol
+ *
+ * Copyright 2024 NXP
+ */
+
+#define pr_fmt(fmt) "SCMI Notifications BBM - " fmt
+
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+#include "../protocols.h"
+#include "../notify.h"
+
+#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
+
+enum scmi_imx_bbm_protocol_cmd {
+ IMX_BBM_GPR_SET = 0x3,
+ IMX_BBM_GPR_GET = 0x4,
+ IMX_BBM_RTC_ATTRIBUTES = 0x5,
+ IMX_BBM_RTC_TIME_SET = 0x6,
+ IMX_BBM_RTC_TIME_GET = 0x7,
+ IMX_BBM_RTC_ALARM_SET = 0x8,
+ IMX_BBM_BUTTON_GET = 0x9,
+ IMX_BBM_RTC_NOTIFY = 0xA,
+ IMX_BBM_BUTTON_NOTIFY = 0xB,
+};
+
+#define GET_RTCS_NR(x) le32_get_bits((x), GENMASK(23, 16))
+#define GET_GPRS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+
+#define SCMI_IMX_BBM_NOTIFY_RTC_UPDATED BIT(2)
+#define SCMI_IMX_BBM_NOTIFY_RTC_ROLLOVER BIT(1)
+#define SCMI_IMX_BBM_NOTIFY_RTC_ALARM BIT(0)
+
+#define SCMI_IMX_BBM_RTC_ALARM_ENABLE_FLAG BIT(0)
+
+#define SCMI_IMX_BBM_NOTIFY_RTC_FLAG \
+ (SCMI_IMX_BBM_NOTIFY_RTC_UPDATED | SCMI_IMX_BBM_NOTIFY_RTC_ROLLOVER | \
+ SCMI_IMX_BBM_NOTIFY_RTC_ALARM)
+
+#define SCMI_IMX_BBM_EVENT_RTC_MASK GENMASK(31, 24)
+
+struct scmi_imx_bbm_info {
+ u32 version;
+ int nr_rtc;
+ int nr_gpr;
+};
+
+struct scmi_msg_imx_bbm_protocol_attributes {
+ __le32 attributes;
+};
+
+struct scmi_imx_bbm_set_time {
+ __le32 id;
+ __le32 flags;
+ __le32 value_low;
+ __le32 value_high;
+};
+
+struct scmi_imx_bbm_get_time {
+ __le32 id;
+ __le32 flags;
+};
+
+struct scmi_imx_bbm_alarm_time {
+ __le32 id;
+ __le32 flags;
+ __le32 value_low;
+ __le32 value_high;
+};
+
+struct scmi_msg_imx_bbm_rtc_notify {
+ __le32 rtc_id;
+ __le32 flags;
+};
+
+struct scmi_msg_imx_bbm_button_notify {
+ __le32 flags;
+};
+
+struct scmi_imx_bbm_notify_payld {
+ __le32 flags;
+};
+
+static int scmi_imx_bbm_attributes_get(const struct scmi_protocol_handle *ph,
+ struct scmi_imx_bbm_info *pi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_imx_bbm_protocol_attributes *attr;
+
+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ pi->nr_rtc = GET_RTCS_NR(attr->attributes);
+ pi->nr_gpr = GET_GPRS_NR(attr->attributes);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_notify(const struct scmi_protocol_handle *ph,
+ u32 src_id, int message_id, bool enable)
+{
+ int ret;
+ struct scmi_xfer *t;
+
+ if (message_id == IMX_BBM_RTC_NOTIFY) {
+ struct scmi_msg_imx_bbm_rtc_notify *rtc_notify;
+
+ ret = ph->xops->xfer_get_init(ph, message_id,
+ sizeof(*rtc_notify), 0, &t);
+ if (ret)
+ return ret;
+
+ rtc_notify = t->tx.buf;
+ rtc_notify->rtc_id = cpu_to_le32(0);
+ rtc_notify->flags =
+ cpu_to_le32(enable ? SCMI_IMX_BBM_NOTIFY_RTC_FLAG : 0);
+ } else if (message_id == IMX_BBM_BUTTON_NOTIFY) {
+ struct scmi_msg_imx_bbm_button_notify *button_notify;
+
+ ret = ph->xops->xfer_get_init(ph, message_id,
+ sizeof(*button_notify), 0, &t);
+ if (ret)
+ return ret;
+
+ button_notify = t->tx.buf;
+ button_notify->flags = cpu_to_le32(enable ? 1 : 0);
+ } else {
+ return -EINVAL;
+ }
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+ return ret;
+}
+
+static enum scmi_imx_bbm_protocol_cmd evt_2_cmd[] = {
+ IMX_BBM_RTC_NOTIFY,
+ IMX_BBM_BUTTON_NOTIFY
+};
+
+static int scmi_imx_bbm_set_notify_enabled(const struct scmi_protocol_handle *ph,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret, cmd_id;
+
+ if (evt_id >= ARRAY_SIZE(evt_2_cmd))
+ return -EINVAL;
+
+ cmd_id = evt_2_cmd[evt_id];
+ ret = scmi_imx_bbm_notify(ph, src_id, cmd_id, enable);
+ if (ret)
+ pr_debug("FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+ evt_id, src_id, ret);
+
+ return ret;
+}
+
+static void *scmi_imx_bbm_fill_custom_report(const struct scmi_protocol_handle *ph,
+ u8 evt_id, ktime_t timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_imx_bbm_notify_payld *p = payld;
+ struct scmi_imx_bbm_notif_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ return NULL;
+
+ if (evt_id == SCMI_EVENT_IMX_BBM_RTC) {
+ r->is_rtc = true;
+ r->is_button = false;
+ r->timestamp = timestamp;
+ r->rtc_id = le32_get_bits(p->flags, SCMI_IMX_BBM_EVENT_RTC_MASK);
+ r->rtc_evt = le32_get_bits(p->flags, SCMI_IMX_BBM_NOTIFY_RTC_FLAG);
+ dev_dbg(ph->dev, "RTC: %d evt: %x\n", r->rtc_id, r->rtc_evt);
+ *src_id = r->rtc_evt;
+ } else if (evt_id == SCMI_EVENT_IMX_BBM_BUTTON) {
+ r->is_rtc = false;
+ r->is_button = true;
+ r->timestamp = timestamp;
+ dev_dbg(ph->dev, "BBM Button\n");
+ *src_id = 0;
+ } else {
+ WARN_ON_ONCE(1);
+ return NULL;
+ }
+
+ return r;
+}
+
+static const struct scmi_event scmi_imx_bbm_events[] = {
+ {
+ .id = SCMI_EVENT_IMX_BBM_RTC,
+ .max_payld_sz = sizeof(struct scmi_imx_bbm_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_bbm_notif_report),
+ },
+ {
+ .id = SCMI_EVENT_IMX_BBM_BUTTON,
+ .max_payld_sz = sizeof(struct scmi_imx_bbm_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_bbm_notif_report),
+ },
+};
+
+static const struct scmi_event_ops scmi_imx_bbm_event_ops = {
+ .set_notify_enabled = scmi_imx_bbm_set_notify_enabled,
+ .fill_custom_report = scmi_imx_bbm_fill_custom_report,
+};
+
+static const struct scmi_protocol_events scmi_imx_bbm_protocol_events = {
+ .queue_sz = SCMI_PROTO_QUEUE_SZ,
+ .ops = &scmi_imx_bbm_event_ops,
+ .evts = scmi_imx_bbm_events,
+ .num_events = ARRAY_SIZE(scmi_imx_bbm_events),
+ .num_sources = 1,
+};
+
+static int scmi_imx_bbm_protocol_init(const struct scmi_protocol_handle *ph)
+{
+ u32 version;
+ int ret;
+ struct scmi_imx_bbm_info *binfo;
+
+ ret = ph->xops->version_get(ph, &version);
+ if (ret)
+ return ret;
+
+ dev_info(ph->dev, "NXP SM BBM Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ binfo = devm_kzalloc(ph->dev, sizeof(*binfo), GFP_KERNEL);
+ if (!binfo)
+ return -ENOMEM;
+
+ ret = scmi_imx_bbm_attributes_get(ph, binfo);
+ if (ret)
+ return ret;
+
+ return ph->set_priv(ph, binfo, version);
+}
+
+static int scmi_imx_bbm_rtc_time_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_set_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_SET, sizeof(*cfg), 0, &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(rtc_id);
+ cfg->flags = 0;
+ cfg->value_low = cpu_to_le32(lower_32_bits(sec));
+ cfg->value_high = cpu_to_le32(upper_32_bits(sec));
+
+ 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 = cpu_to_le32(lower_32_bits(sec));
+ cfg->value_high = cpu_to_le32(upper_32_bits(sec));
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_button_get(const struct scmi_protocol_handle *ph, u32 *state)
+{
+ struct scmi_xfer *t;
+ int ret;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_BUTTON_GET, 0, sizeof(u32), &t);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret)
+ *state = get_unaligned_le32(t->rx.buf);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static const struct scmi_imx_bbm_proto_ops scmi_imx_bbm_proto_ops = {
+ .rtc_time_get = scmi_imx_bbm_rtc_time_get,
+ .rtc_time_set = scmi_imx_bbm_rtc_time_set,
+ .rtc_alarm_set = scmi_imx_bbm_rtc_alarm_set,
+ .button_get = scmi_imx_bbm_button_get,
+};
+
+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",
+};
+
+module_scmi_protocol(scmi_imx_bbm);
diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
new file mode 100644
index 000000000000..e59aedaa4aec
--- /dev/null
+++ b/include/linux/scmi_imx_protocol.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * SCMI Message Protocol driver NXP extension header
+ *
+ * Copyright 2024 NXP.
+ */
+
+#ifndef _LINUX_SCMI_NXP_PROTOCOL_H
+#define _LINUX_SCMI_NXP_PROTOCOL_H
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/notifier.h>
+#include <linux/types.h>
+
+enum scmi_nxp_protocol {
+ SCMI_PROTOCOL_IMX_BBM = 0x81,
+};
+
+struct scmi_imx_bbm_proto_ops {
+ int (*rtc_time_set)(const struct scmi_protocol_handle *ph, u32 id,
+ uint64_t sec);
+ int (*rtc_time_get)(const struct scmi_protocol_handle *ph, u32 id,
+ u64 *val);
+ int (*rtc_alarm_set)(const struct scmi_protocol_handle *ph, u32 id,
+ u64 sec);
+ int (*button_get)(const struct scmi_protocol_handle *ph, u32 *state);
+};
+
+enum scmi_nxp_notification_events {
+ SCMI_EVENT_IMX_BBM_RTC = 0x0,
+ SCMI_EVENT_IMX_BBM_BUTTON = 0x1,
+};
+
+struct scmi_imx_bbm_notif_report {
+ bool is_rtc;
+ bool is_button;
+ ktime_t timestamp;
+ unsigned int rtc_id;
+ unsigned int rtc_evt;
+};
+#endif
--
2.37.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol
2024-05-24 8:56 ` [PATCH v4 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol Peng Fan (OSS)
@ 2024-06-17 16:48 ` Cristian Marussi
2024-06-20 3:13 ` Peng Fan
0 siblings, 1 reply; 18+ messages in thread
From: Cristian Marussi @ 2024-06-17 16:48 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan, linux-doc,
linux-kernel, imx, linux-arm-kernel, devicetree
On Fri, May 24, 2024 at 04:56:45PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> 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@nxp.com>
> ---
> 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.
> 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
> @@ -16,6 +16,7 @@ scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
> +obj-$(CONFIG_ARM_SCMI_PROTOCOL) += imx/
>
> obj-$(CONFIG_ARM_SCMI_POWER_CONTROL) += scmi_power_control.o
>
> diff --git a/drivers/firmware/arm_scmi/imx/Kconfig b/drivers/firmware/arm_scmi/imx/Kconfig
> new file mode 100644
> index 000000000000..4b6ac7febe8f
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/imx/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +menu "ARM SCMI NXP i.MX Vendor Protocols"
> +
> +config IMX_SCMI_BBM_EXT
> + tristate "i.MX SCMI BBM EXTENSION"
> + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
> + default y if ARCH_MXC
> + help
> + This enables i.MX System BBM control logic which supports RTC
> + and BUTTON.
> +
> + This driver can also be built as a module.
> +
> +endmenu
> diff --git a/drivers/firmware/arm_scmi/imx/Makefile b/drivers/firmware/arm_scmi/imx/Makefile
> new file mode 100644
> index 000000000000..a7dbdd20dbb9
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/imx/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
> diff --git a/drivers/firmware/arm_scmi/imx/imx-sm-bbm.c b/drivers/firmware/arm_scmi/imx/imx-sm-bbm.c
> new file mode 100644
> index 000000000000..3f8321d247ae
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/imx/imx-sm-bbm.c
> @@ -0,0 +1,380 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System Control and Management Interface (SCMI) NXP BBM Protocol
> + *
> + * Copyright 2024 NXP
> + */
> +
> +#define pr_fmt(fmt) "SCMI Notifications BBM - " fmt
> +
> +#include <linux/bits.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/scmi_imx_protocol.h>
> +
> +#include "../protocols.h"
> +#include "../notify.h"
> +
> +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
> +
> +enum scmi_imx_bbm_protocol_cmd {
> + IMX_BBM_GPR_SET = 0x3,
> + IMX_BBM_GPR_GET = 0x4,
> + IMX_BBM_RTC_ATTRIBUTES = 0x5,
> + IMX_BBM_RTC_TIME_SET = 0x6,
> + IMX_BBM_RTC_TIME_GET = 0x7,
> + IMX_BBM_RTC_ALARM_SET = 0x8,
> + IMX_BBM_BUTTON_GET = 0x9,
> + IMX_BBM_RTC_NOTIFY = 0xA,
> + IMX_BBM_BUTTON_NOTIFY = 0xB,
> +};
> +
> +#define GET_RTCS_NR(x) le32_get_bits((x), GENMASK(23, 16))
> +#define GET_GPRS_NR(x) le32_get_bits((x), GENMASK(15, 0))
> +
> +#define SCMI_IMX_BBM_NOTIFY_RTC_UPDATED BIT(2)
> +#define SCMI_IMX_BBM_NOTIFY_RTC_ROLLOVER BIT(1)
> +#define SCMI_IMX_BBM_NOTIFY_RTC_ALARM BIT(0)
> +
> +#define SCMI_IMX_BBM_RTC_ALARM_ENABLE_FLAG BIT(0)
> +
> +#define SCMI_IMX_BBM_NOTIFY_RTC_FLAG \
> + (SCMI_IMX_BBM_NOTIFY_RTC_UPDATED | SCMI_IMX_BBM_NOTIFY_RTC_ROLLOVER | \
> + SCMI_IMX_BBM_NOTIFY_RTC_ALARM)
> +
> +#define SCMI_IMX_BBM_EVENT_RTC_MASK GENMASK(31, 24)
> +
> +struct scmi_imx_bbm_info {
> + u32 version;
> + int nr_rtc;
> + int nr_gpr;
> +};
> +
> +struct scmi_msg_imx_bbm_protocol_attributes {
> + __le32 attributes;
> +};
> +
> +struct scmi_imx_bbm_set_time {
> + __le32 id;
> + __le32 flags;
> + __le32 value_low;
> + __le32 value_high;
> +};
> +
> +struct scmi_imx_bbm_get_time {
> + __le32 id;
> + __le32 flags;
> +};
> +
> +struct scmi_imx_bbm_alarm_time {
> + __le32 id;
> + __le32 flags;
> + __le32 value_low;
> + __le32 value_high;
> +};
> +
> +struct scmi_msg_imx_bbm_rtc_notify {
> + __le32 rtc_id;
> + __le32 flags;
> +};
> +
> +struct scmi_msg_imx_bbm_button_notify {
> + __le32 flags;
> +};
> +
> +struct scmi_imx_bbm_notify_payld {
> + __le32 flags;
> +};
> +
> +static int scmi_imx_bbm_attributes_get(const struct scmi_protocol_handle *ph,
> + struct scmi_imx_bbm_info *pi)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_imx_bbm_protocol_attributes *attr;
> +
> + ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr), &t);
> + if (ret)
> + return ret;
> +
> + attr = t->rx.buf;
> +
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + pi->nr_rtc = GET_RTCS_NR(attr->attributes);
> + pi->nr_gpr = GET_GPRS_NR(attr->attributes);
> + }
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int scmi_imx_bbm_notify(const struct scmi_protocol_handle *ph,
> + u32 src_id, int message_id, bool enable)
> +{
> + int ret;
> + struct scmi_xfer *t;
> +
> + if (message_id == IMX_BBM_RTC_NOTIFY) {
> + struct scmi_msg_imx_bbm_rtc_notify *rtc_notify;
> +
> + ret = ph->xops->xfer_get_init(ph, message_id,
> + sizeof(*rtc_notify), 0, &t);
> + if (ret)
> + return ret;
> +
> + rtc_notify = t->tx.buf;
> + rtc_notify->rtc_id = cpu_to_le32(0);
> + rtc_notify->flags =
> + cpu_to_le32(enable ? SCMI_IMX_BBM_NOTIFY_RTC_FLAG : 0);
> + } else if (message_id == IMX_BBM_BUTTON_NOTIFY) {
> + struct scmi_msg_imx_bbm_button_notify *button_notify;
> +
> + ret = ph->xops->xfer_get_init(ph, message_id,
> + sizeof(*button_notify), 0, &t);
> + if (ret)
> + return ret;
> +
> + button_notify = t->tx.buf;
> + button_notify->flags = cpu_to_le32(enable ? 1 : 0);
> + } else {
> + return -EINVAL;
> + }
> +
> + ret = ph->xops->do_xfer(ph, t);
> +
> + ph->xops->xfer_put(ph, t);
> + return ret;
> +}
> +
> +static enum scmi_imx_bbm_protocol_cmd evt_2_cmd[] = {
> + IMX_BBM_RTC_NOTIFY,
> + IMX_BBM_BUTTON_NOTIFY
> +};
> +
> +static int scmi_imx_bbm_set_notify_enabled(const struct scmi_protocol_handle *ph,
> + u8 evt_id, u32 src_id, bool enable)
> +{
> + int ret, cmd_id;
> +
> + if (evt_id >= ARRAY_SIZE(evt_2_cmd))
> + return -EINVAL;
> +
> + cmd_id = evt_2_cmd[evt_id];
> + ret = scmi_imx_bbm_notify(ph, src_id, cmd_id, enable);
> + if (ret)
> + pr_debug("FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
> + evt_id, src_id, ret);
> +
> + return ret;
> +}
> +
> +static void *scmi_imx_bbm_fill_custom_report(const struct scmi_protocol_handle *ph,
> + u8 evt_id, ktime_t timestamp,
> + const void *payld, size_t payld_sz,
> + void *report, u32 *src_id)
> +{
> + const struct scmi_imx_bbm_notify_payld *p = payld;
> + struct scmi_imx_bbm_notif_report *r = report;
> +
> + if (sizeof(*p) != payld_sz)
> + return NULL;
> +
> + if (evt_id == SCMI_EVENT_IMX_BBM_RTC) {
> + r->is_rtc = true;
> + r->is_button = false;
> + r->timestamp = timestamp;
> + r->rtc_id = le32_get_bits(p->flags, SCMI_IMX_BBM_EVENT_RTC_MASK);
> + r->rtc_evt = le32_get_bits(p->flags, SCMI_IMX_BBM_NOTIFY_RTC_FLAG);
> + dev_dbg(ph->dev, "RTC: %d evt: %x\n", r->rtc_id, r->rtc_evt);
> + *src_id = r->rtc_evt;
> + } else if (evt_id == SCMI_EVENT_IMX_BBM_BUTTON) {
> + r->is_rtc = false;
> + r->is_button = true;
> + r->timestamp = timestamp;
> + dev_dbg(ph->dev, "BBM Button\n");
> + *src_id = 0;
> + } else {
> + WARN_ON_ONCE(1);
> + return NULL;
> + }
> +
> + return r;
> +}
> +
> +static const struct scmi_event scmi_imx_bbm_events[] = {
> + {
> + .id = SCMI_EVENT_IMX_BBM_RTC,
> + .max_payld_sz = sizeof(struct scmi_imx_bbm_notify_payld),
> + .max_report_sz = sizeof(struct scmi_imx_bbm_notif_report),
> + },
> + {
> + .id = SCMI_EVENT_IMX_BBM_BUTTON,
> + .max_payld_sz = sizeof(struct scmi_imx_bbm_notify_payld),
> + .max_report_sz = sizeof(struct scmi_imx_bbm_notif_report),
> + },
> +};
> +
> +static const struct scmi_event_ops scmi_imx_bbm_event_ops = {
> + .set_notify_enabled = scmi_imx_bbm_set_notify_enabled,
> + .fill_custom_report = scmi_imx_bbm_fill_custom_report,
> +};
> +
> +static const struct scmi_protocol_events scmi_imx_bbm_protocol_events = {
> + .queue_sz = SCMI_PROTO_QUEUE_SZ,
> + .ops = &scmi_imx_bbm_event_ops,
> + .evts = scmi_imx_bbm_events,
> + .num_events = ARRAY_SIZE(scmi_imx_bbm_events),
> + .num_sources = 1,
> +};
> +
> +static int scmi_imx_bbm_protocol_init(const struct scmi_protocol_handle *ph)
> +{
> + u32 version;
> + int ret;
> + struct scmi_imx_bbm_info *binfo;
> +
> + ret = ph->xops->version_get(ph, &version);
> + if (ret)
> + return ret;
> +
> + dev_info(ph->dev, "NXP SM BBM Version %d.%d\n",
> + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
> +
> + binfo = devm_kzalloc(ph->dev, sizeof(*binfo), GFP_KERNEL);
> + if (!binfo)
> + return -ENOMEM;
> +
> + 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.
> +
> +static int scmi_imx_bbm_rtc_time_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_set_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_SET, sizeof(*cfg), 0, &t);
> + if (ret)
> + return ret;
> +
> + cfg = t->tx.buf;
> + cfg->id = cpu_to_le32(rtc_id);
> + cfg->flags = 0;
> + cfg->value_low = cpu_to_le32(lower_32_bits(sec));
> + cfg->value_high = cpu_to_le32(upper_32_bits(sec));
> +
> + 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 = cpu_to_le32(lower_32_bits(sec));
> + cfg->value_high = cpu_to_le32(upper_32_bits(sec));
> +
> + ret = ph->xops->do_xfer(ph, t);
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int scmi_imx_bbm_button_get(const struct scmi_protocol_handle *ph, u32 *state)
> +{
> + struct scmi_xfer *t;
> + int ret;
> +
> + ret = ph->xops->xfer_get_init(ph, IMX_BBM_BUTTON_GET, 0, sizeof(u32), &t);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret)
> + *state = get_unaligned_le32(t->rx.buf);
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static const struct scmi_imx_bbm_proto_ops scmi_imx_bbm_proto_ops = {
> + .rtc_time_get = scmi_imx_bbm_rtc_time_get,
> + .rtc_time_set = scmi_imx_bbm_rtc_time_set,
> + .rtc_alarm_set = scmi_imx_bbm_rtc_alarm_set,
> + .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@arm.com>
Thanks,
Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v4 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol
2024-06-17 16:48 ` Cristian Marussi
@ 2024-06-20 3:13 ` Peng Fan
0 siblings, 0 replies; 18+ messages in thread
From: Peng Fan @ 2024-06-20 3:13 UTC (permalink / raw)
To: Cristian Marussi, Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
> 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@nxp.com>
> >
> > 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@nxp.com>
> > ---
> > 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@arm.com>
Thanks for help reviewing the patches.
Thanks,
Peng.
>
> Thanks,
> Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol
2024-05-24 8:56 [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan (OSS)
` (2 preceding siblings ...)
2024-05-24 8:56 ` [PATCH v4 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol Peng Fan (OSS)
@ 2024-05-24 8:56 ` Peng Fan (OSS)
2024-06-17 18:14 ` Cristian Marussi
2024-05-24 8:56 ` [PATCH v4 5/6] firmware: imx: support i.MX95 BBM module Peng Fan (OSS)
` (2 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Peng Fan (OSS) @ 2024-05-24 8:56 UTC (permalink / raw)
To: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan
Cc: linux-doc, linux-kernel, imx, linux-arm-kernel, devicetree
From: Peng Fan <peng.fan@nxp.com>
i.MX95 System Manager(SM) firmware includes a SCMI vendor protocol, SCMI
MISC protocol which 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 General Purpose registers, Control Status Registers
owned by the SM.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/imx/Kconfig | 9 +
drivers/firmware/arm_scmi/imx/Makefile | 1 +
drivers/firmware/arm_scmi/imx/imx-sm-misc.c | 303 ++++++++++++++++++++++++++++
include/linux/scmi_imx_protocol.h | 22 ++
4 files changed, 335 insertions(+)
diff --git a/drivers/firmware/arm_scmi/imx/Kconfig b/drivers/firmware/arm_scmi/imx/Kconfig
index 4b6ac7febe8f..e9d015859eaa 100644
--- a/drivers/firmware/arm_scmi/imx/Kconfig
+++ b/drivers/firmware/arm_scmi/imx/Kconfig
@@ -11,4 +11,13 @@ config IMX_SCMI_BBM_EXT
This driver can also be built as a module.
+config IMX_SCMI_MISC_EXT
+ tristate "i.MX SCMI MISC EXTENSION"
+ depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
+ default y if ARCH_MXC
+ help
+ This enables i.MX System MISC control logic such as gpio expander
+ wakeup
+
+ This driver can also be built as a module.
endmenu
diff --git a/drivers/firmware/arm_scmi/imx/Makefile b/drivers/firmware/arm_scmi/imx/Makefile
index a7dbdd20dbb9..d3ee6d544924 100644
--- a/drivers/firmware/arm_scmi/imx/Makefile
+++ b/drivers/firmware/arm_scmi/imx/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
+obj-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o
diff --git a/drivers/firmware/arm_scmi/imx/imx-sm-misc.c b/drivers/firmware/arm_scmi/imx/imx-sm-misc.c
new file mode 100644
index 000000000000..9d0063299310
--- /dev/null
+++ b/drivers/firmware/arm_scmi/imx/imx-sm-misc.c
@@ -0,0 +1,303 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System control and Management Interface (SCMI) NXP MISC Protocol
+ *
+ * Copyright 2024 NXP
+ */
+
+#define pr_fmt(fmt) "SCMI Notifications MISC - " fmt
+
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+#include "../protocols.h"
+#include "../notify.h"
+
+#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
+
+enum scmi_imx_misc_protocol_cmd {
+ SCMI_IMX_MISC_CTRL_SET = 0x3,
+ SCMI_IMX_MISC_CTRL_GET = 0x4,
+ SCMI_IMX_MISC_CTRL_NOTIFY = 0x8,
+};
+
+struct scmi_imx_misc_info {
+ u32 version;
+ u32 nr_dev_ctrl;
+ u32 nr_brd_ctrl;
+ u32 nr_reason;
+};
+
+struct scmi_msg_imx_misc_protocol_attributes {
+ __le32 attributes;
+};
+
+#define GET_BRD_CTRLS_NR(x) le32_get_bits((x), GENMASK(31, 24))
+#define GET_REASONS_NR(x) le32_get_bits((x), GENMASK(23, 16))
+#define GET_DEV_CTRLS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+#define BRD_CTRL_START_ID BIT(15)
+
+struct scmi_imx_misc_ctrl_set_in {
+ __le32 id;
+ __le32 num;
+ __le32 value[MISC_MAX_VAL];
+};
+
+struct scmi_imx_misc_ctrl_notify_in {
+ __le32 ctrl_id;
+ __le32 flags;
+};
+
+struct scmi_imx_misc_ctrl_notify_payld {
+ __le32 ctrl_id;
+ __le32 flags;
+};
+
+struct scmi_imx_misc_ctrl_get_out {
+ __le32 num;
+ __le32 val[MISC_MAX_VAL];
+};
+
+static int scmi_imx_misc_attributes_get(const struct scmi_protocol_handle *ph,
+ struct scmi_imx_misc_info *mi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_imx_misc_protocol_attributes *attr;
+
+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0,
+ sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ mi->nr_dev_ctrl = GET_DEV_CTRLS_NR(attr->attributes);
+ mi->nr_brd_ctrl = GET_BRD_CTRLS_NR(attr->attributes);
+ mi->nr_reason = GET_REASONS_NR(attr->attributes);
+ dev_info(ph->dev, "i.MX MISC NUM DEV CTRL: %d, NUM BRD CTRL: %d,NUM Reason: %d\n",
+ mi->nr_dev_ctrl, mi->nr_brd_ctrl, mi->nr_reason);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_validate_id(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id)
+{
+ struct scmi_imx_misc_info *mi = ph->get_priv(ph);
+
+ if ((ctrl_id < BRD_CTRL_START_ID) && (ctrl_id > mi->nr_dev_ctrl))
+ return -EINVAL;
+ if (ctrl_id >= BRD_CTRL_START_ID + mi->nr_brd_ctrl)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int scmi_imx_misc_ctrl_notify(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 evt_id, u32 flags)
+{
+ struct scmi_imx_misc_ctrl_notify_in *in;
+ struct scmi_xfer *t;
+ int ret;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_NOTIFY,
+ sizeof(*in), 0, &t);
+ if (ret)
+ return ret;
+
+ in = t->tx.buf;
+ in->ctrl_id = cpu_to_le32(ctrl_id);
+ in->flags = cpu_to_le32(flags);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int
+scmi_imx_misc_ctrl_set_notify_enabled(const struct scmi_protocol_handle *ph,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ /* misc_ctrl_req_notify is for enablement */
+ if (enable)
+ return 0;
+
+ ret = scmi_imx_misc_ctrl_notify(ph, src_id, evt_id, 0);
+ if (ret)
+ dev_err(ph->dev, "FAIL_ENABLED - evt[%X] src[%d] - ret:%d\n",
+ evt_id, src_id, ret);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_get_num_sources(const struct scmi_protocol_handle *ph)
+{
+ return GENMASK(15, 0);
+}
+
+static void *
+scmi_imx_misc_ctrl_fill_custom_report(const struct scmi_protocol_handle *ph,
+ u8 evt_id, ktime_t timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_imx_misc_ctrl_notify_payld *p = payld;
+ struct scmi_imx_misc_ctrl_notify_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ return NULL;
+
+ r->timestamp = timestamp;
+ r->ctrl_id = p->ctrl_id;
+ r->flags = p->flags;
+ if (src_id)
+ *src_id = r->ctrl_id;
+ dev_dbg(ph->dev, "%s: ctrl_id: %d flags: %d\n", __func__,
+ r->ctrl_id, r->flags);
+
+ return r;
+}
+
+static const struct scmi_event_ops scmi_imx_misc_event_ops = {
+ .get_num_sources = scmi_imx_misc_ctrl_get_num_sources,
+ .set_notify_enabled = scmi_imx_misc_ctrl_set_notify_enabled,
+ .fill_custom_report = scmi_imx_misc_ctrl_fill_custom_report,
+};
+
+static const struct scmi_event scmi_imx_misc_events[] = {
+ {
+ .id = SCMI_EVENT_IMX_MISC_CONTROL,
+ .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
+ },
+};
+
+static struct scmi_protocol_events scmi_imx_misc_protocol_events = {
+ .queue_sz = SCMI_PROTO_QUEUE_SZ,
+ .ops = &scmi_imx_misc_event_ops,
+ .evts = scmi_imx_misc_events,
+ .num_events = ARRAY_SIZE(scmi_imx_misc_events),
+};
+
+static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph)
+{
+ struct scmi_imx_misc_info *minfo;
+ u32 version;
+ int ret;
+
+ ret = ph->xops->version_get(ph, &version);
+ if (ret)
+ return ret;
+
+ dev_info(ph->dev, "NXP SM MISC Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ minfo = devm_kzalloc(ph->dev, sizeof(*minfo), GFP_KERNEL);
+ if (!minfo)
+ return -ENOMEM;
+
+ ret = scmi_imx_misc_attributes_get(ph, minfo);
+ if (ret)
+ return ret;
+
+ return ph->set_priv(ph, minfo, version);
+}
+
+static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 *num, u32 *val)
+{
+ struct scmi_imx_misc_ctrl_get_out *out;
+ struct scmi_xfer *t;
+ int ret, i;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_GET, sizeof(u32),
+ 0, &t);
+ if (ret)
+ return ret;
+
+ put_unaligned_le32(ctrl_id, t->tx.buf);
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ out = t->rx.buf;
+ *num = le32_to_cpu(out->num);
+ for (i = 0; i < *num && i < MISC_MAX_VAL; i++)
+ val[i] = le32_to_cpu(out->val[i]);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_set(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 num, u32 *val)
+{
+ struct scmi_imx_misc_ctrl_set_in *in;
+ struct scmi_xfer *t;
+ int ret, i;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ if (num > MISC_MAX_VAL)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_SET, sizeof(*in),
+ 0, &t);
+ if (ret)
+ return ret;
+
+ in = t->tx.buf;
+ in->id = cpu_to_le32(ctrl_id);
+ in->num = cpu_to_le32(num);
+ for (i = 0; i < num; i++)
+ in->value[i] = cpu_to_le32(val[i]);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = {
+ .misc_ctrl_set = scmi_imx_misc_ctrl_set,
+ .misc_ctrl_get = scmi_imx_misc_ctrl_get,
+ .misc_ctrl_req_notify = scmi_imx_misc_ctrl_notify,
+};
+
+static const struct scmi_protocol scmi_imx_misc = {
+ .id = SCMI_PROTOCOL_IMX_MISC,
+ .owner = THIS_MODULE,
+ .instance_init = &scmi_imx_misc_protocol_init,
+ .ops = &scmi_imx_misc_proto_ops,
+ .events = &scmi_imx_misc_protocol_events,
+ .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
+ .vendor_id = "NXP",
+ .sub_vendor_id = "i.MX95 EVK",
+};
+module_scmi_protocol(scmi_imx_misc);
diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
index e59aedaa4aec..e9285abfc191 100644
--- a/include/linux/scmi_imx_protocol.h
+++ b/include/linux/scmi_imx_protocol.h
@@ -13,8 +13,14 @@
#include <linux/notifier.h>
#include <linux/types.h>
+#define SCMI_PAYLOAD_LEN 100
+
+#define SCMI_ARRAY(X, Y) ((SCMI_PAYLOAD_LEN - (X)) / sizeof(Y))
+#define MISC_MAX_VAL SCMI_ARRAY(8, uint32_t)
+
enum scmi_nxp_protocol {
SCMI_PROTOCOL_IMX_BBM = 0x81,
+ SCMI_PROTOCOL_IMX_MISC = 0x84,
};
struct scmi_imx_bbm_proto_ops {
@@ -30,6 +36,7 @@ struct scmi_imx_bbm_proto_ops {
enum scmi_nxp_notification_events {
SCMI_EVENT_IMX_BBM_RTC = 0x0,
SCMI_EVENT_IMX_BBM_BUTTON = 0x1,
+ SCMI_EVENT_IMX_MISC_CONTROL = 0x0,
};
struct scmi_imx_bbm_notif_report {
@@ -39,4 +46,19 @@ struct scmi_imx_bbm_notif_report {
unsigned int rtc_id;
unsigned int rtc_evt;
};
+
+struct scmi_imx_misc_ctrl_notify_report {
+ ktime_t timestamp;
+ unsigned int ctrl_id;
+ unsigned int flags;
+};
+
+struct scmi_imx_misc_proto_ops {
+ int (*misc_ctrl_set)(const struct scmi_protocol_handle *ph, u32 id,
+ u32 num, u32 *val);
+ int (*misc_ctrl_get)(const struct scmi_protocol_handle *ph, u32 id,
+ u32 *num, u32 *val);
+ int (*misc_ctrl_req_notify)(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 evt_id, u32 flags);
+};
#endif
--
2.37.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol
2024-05-24 8:56 ` [PATCH v4 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol Peng Fan (OSS)
@ 2024-06-17 18:14 ` Cristian Marussi
2024-06-20 3:20 ` Peng Fan
0 siblings, 1 reply; 18+ messages in thread
From: Cristian Marussi @ 2024-06-17 18:14 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan, linux-doc,
linux-kernel, imx, linux-arm-kernel, devicetree
On Fri, May 24, 2024 at 04:56:46PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> i.MX95 System Manager(SM) firmware includes a SCMI vendor protocol, SCMI
> MISC protocol which 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 General Purpose registers, Control Status Registers
> owned by the SM.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/arm_scmi/imx/Kconfig | 9 +
> drivers/firmware/arm_scmi/imx/Makefile | 1 +
> drivers/firmware/arm_scmi/imx/imx-sm-misc.c | 303 ++++++++++++++++++++++++++++
> include/linux/scmi_imx_protocol.h | 22 ++
> 4 files changed, 335 insertions(+)
>
Hi,
> diff --git a/drivers/firmware/arm_scmi/imx/Kconfig b/drivers/firmware/arm_scmi/imx/Kconfig
> index 4b6ac7febe8f..e9d015859eaa 100644
> --- a/drivers/firmware/arm_scmi/imx/Kconfig
> +++ b/drivers/firmware/arm_scmi/imx/Kconfig
> @@ -11,4 +11,13 @@ config IMX_SCMI_BBM_EXT
>
> This driver can also be built as a module.
>
> +config IMX_SCMI_MISC_EXT
> + tristate "i.MX SCMI MISC EXTENSION"
> + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
> + default y if ARCH_MXC
> + help
> + This enables i.MX System MISC control logic such as gpio expander
> + wakeup
> +
> + This driver can also be built as a module.
> endmenu
> diff --git a/drivers/firmware/arm_scmi/imx/Makefile b/drivers/firmware/arm_scmi/imx/Makefile
> index a7dbdd20dbb9..d3ee6d544924 100644
> --- a/drivers/firmware/arm_scmi/imx/Makefile
> +++ b/drivers/firmware/arm_scmi/imx/Makefile
> @@ -1,2 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
> +obj-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o
> diff --git a/drivers/firmware/arm_scmi/imx/imx-sm-misc.c b/drivers/firmware/arm_scmi/imx/imx-sm-misc.c
> new file mode 100644
> index 000000000000..9d0063299310
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/imx/imx-sm-misc.c
> @@ -0,0 +1,303 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System control and Management Interface (SCMI) NXP MISC Protocol
> + *
> + * Copyright 2024 NXP
> + */
> +
> +#define pr_fmt(fmt) "SCMI Notifications MISC - " fmt
> +
> +#include <linux/bits.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/scmi_imx_protocol.h>
> +
> +#include "../protocols.h"
> +#include "../notify.h"
> +
> +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
> +
> +enum scmi_imx_misc_protocol_cmd {
> + SCMI_IMX_MISC_CTRL_SET = 0x3,
> + SCMI_IMX_MISC_CTRL_GET = 0x4,
> + SCMI_IMX_MISC_CTRL_NOTIFY = 0x8,
> +};
> +
> +struct scmi_imx_misc_info {
> + u32 version;
> + u32 nr_dev_ctrl;
> + u32 nr_brd_ctrl;
> + u32 nr_reason;
> +};
> +
> +struct scmi_msg_imx_misc_protocol_attributes {
> + __le32 attributes;
> +};
> +
> +#define GET_BRD_CTRLS_NR(x) le32_get_bits((x), GENMASK(31, 24))
> +#define GET_REASONS_NR(x) le32_get_bits((x), GENMASK(23, 16))
> +#define GET_DEV_CTRLS_NR(x) le32_get_bits((x), GENMASK(15, 0))
> +#define BRD_CTRL_START_ID BIT(15)
> +
> +struct scmi_imx_misc_ctrl_set_in {
> + __le32 id;
> + __le32 num;
> + __le32 value[MISC_MAX_VAL];
> +};
> +
> +struct scmi_imx_misc_ctrl_notify_in {
> + __le32 ctrl_id;
> + __le32 flags;
> +};
> +
> +struct scmi_imx_misc_ctrl_notify_payld {
> + __le32 ctrl_id;
> + __le32 flags;
> +};
> +
> +struct scmi_imx_misc_ctrl_get_out {
> + __le32 num;
> + __le32 val[MISC_MAX_VAL];
> +};
> +
> +static int scmi_imx_misc_attributes_get(const struct scmi_protocol_handle *ph,
> + struct scmi_imx_misc_info *mi)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_imx_misc_protocol_attributes *attr;
> +
> + ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0,
> + sizeof(*attr), &t);
> + if (ret)
> + return ret;
> +
> + attr = t->rx.buf;
> +
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + mi->nr_dev_ctrl = GET_DEV_CTRLS_NR(attr->attributes);
> + mi->nr_brd_ctrl = GET_BRD_CTRLS_NR(attr->attributes);
> + mi->nr_reason = GET_REASONS_NR(attr->attributes);
> + dev_info(ph->dev, "i.MX MISC NUM DEV CTRL: %d, NUM BRD CTRL: %d,NUM Reason: %d\n",
> + mi->nr_dev_ctrl, mi->nr_brd_ctrl, mi->nr_reason);
> + }
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_validate_id(const struct scmi_protocol_handle *ph,
> + u32 ctrl_id)
> +{
> + struct scmi_imx_misc_info *mi = ph->get_priv(ph);
> +
> + if ((ctrl_id < BRD_CTRL_START_ID) && (ctrl_id > mi->nr_dev_ctrl))
> + return -EINVAL;
> + if (ctrl_id >= BRD_CTRL_START_ID + mi->nr_brd_ctrl)
> + return -EINVAL;
...are these conditions fine ? just checking because they seem a bit
odd...but I am certainly missing something...in case they are ok, is it
possible to add a comment explaining why those conds lead to -EINVAL ?
> +
> + return 0;
> +}
> +
> +static int scmi_imx_misc_ctrl_notify(const struct scmi_protocol_handle *ph,
> + u32 ctrl_id, u32 evt_id, u32 flags)
> +{
> + struct scmi_imx_misc_ctrl_notify_in *in;
> + struct scmi_xfer *t;
> + int ret;
> +
> + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_NOTIFY,
> + sizeof(*in), 0, &t);
> + if (ret)
> + return ret;
> +
> + in = t->tx.buf;
> + in->ctrl_id = cpu_to_le32(ctrl_id);
> + in->flags = cpu_to_le32(flags);
> +
> + ret = ph->xops->do_xfer(ph, t);
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int
> +scmi_imx_misc_ctrl_set_notify_enabled(const struct scmi_protocol_handle *ph,
> + u8 evt_id, u32 src_id, bool enable)
> +{
> + int ret;
> +
> + /* misc_ctrl_req_notify is for enablement */
> + if (enable)
> + return 0;
> +
> + ret = scmi_imx_misc_ctrl_notify(ph, src_id, evt_id, 0);
> + if (ret)
> + dev_err(ph->dev, "FAIL_ENABLED - evt[%X] src[%d] - ret:%d\n",
> + evt_id, src_id, ret);
> +
> + return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_get_num_sources(const struct scmi_protocol_handle *ph)
> +{
> + return GENMASK(15, 0);
> +}
This is statically defined at compile time..you dont need to provide
this method, which is just for discover number of possible event sources
at runtime....just drop it and use .num_sources in scmi_protocol_events
> +
> +static void *
> +scmi_imx_misc_ctrl_fill_custom_report(const struct scmi_protocol_handle *ph,
> + u8 evt_id, ktime_t timestamp,
> + const void *payld, size_t payld_sz,
> + void *report, u32 *src_id)
> +{
> + const struct scmi_imx_misc_ctrl_notify_payld *p = payld;
> + struct scmi_imx_misc_ctrl_notify_report *r = report;
> +
> + if (sizeof(*p) != payld_sz)
> + return NULL;
> +
> + r->timestamp = timestamp;
> + r->ctrl_id = p->ctrl_id;
> + r->flags = p->flags;
> + if (src_id)
> + *src_id = r->ctrl_id;
> + dev_dbg(ph->dev, "%s: ctrl_id: %d flags: %d\n", __func__,
> + r->ctrl_id, r->flags);
> +
> + return r;
> +}
> +
> +static const struct scmi_event_ops scmi_imx_misc_event_ops = {
> + .get_num_sources = scmi_imx_misc_ctrl_get_num_sources,
drop
> + .set_notify_enabled = scmi_imx_misc_ctrl_set_notify_enabled,
> + .fill_custom_report = scmi_imx_misc_ctrl_fill_custom_report,
> +};
> +
> +static const struct scmi_event scmi_imx_misc_events[] = {
> + {
> + .id = SCMI_EVENT_IMX_MISC_CONTROL,
> + .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
> + .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
> + },
> +};
> +
> +static struct scmi_protocol_events scmi_imx_misc_protocol_events = {
> + .queue_sz = SCMI_PROTO_QUEUE_SZ,
> + .ops = &scmi_imx_misc_event_ops,
> + .evts = scmi_imx_misc_events,
> + .num_events = ARRAY_SIZE(scmi_imx_misc_events),
.num_sources = MAX_MISC_CTRL_SOURCES, // GENMASK(15, 0)
> +};
> +
> +static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph)
> +{
> + struct scmi_imx_misc_info *minfo;
> + u32 version;
> + int ret;
> +
> + ret = ph->xops->version_get(ph, &version);
> + if (ret)
> + return ret;
> +
> + dev_info(ph->dev, "NXP SM MISC Version %d.%d\n",
> + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
> +
> + minfo = devm_kzalloc(ph->dev, sizeof(*minfo), GFP_KERNEL);
> + if (!minfo)
> + return -ENOMEM;
> +
> + ret = scmi_imx_misc_attributes_get(ph, minfo);
> + if (ret)
> + return ret;
> +
> + return ph->set_priv(ph, minfo, version);
> +}
Same as previous patch please move the init downb below near the
scmi_protocol struct right after the ops
> +
> +static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
> + u32 ctrl_id, u32 *num, u32 *val)
> +{
> + struct scmi_imx_misc_ctrl_get_out *out;
> + struct scmi_xfer *t;
> + int ret, i;
> +
> + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_GET, sizeof(u32),
> + 0, &t);
> + if (ret)
> + return ret;
> +
> + put_unaligned_le32(ctrl_id, t->tx.buf);
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + out = t->rx.buf;
> + *num = le32_to_cpu(out->num);
To stay even more safer, by guarding from malformed *num fields and just
bail out upfront with an error
if (*num >= MISC_MAX_VAL ||
*num * sizeof(__le32) > t->rx.len - sizeof(__le32))
and then just
for (i = 0; i < *num; i++)
> + for (i = 0; i < *num && i < MISC_MAX_VAL; i++)
> + val[i] = le32_to_cpu(out->val[i]);
> + }
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_set(const struct scmi_protocol_handle *ph,
> + u32 ctrl_id, u32 num, u32 *val)
> +{
> + struct scmi_imx_misc_ctrl_set_in *in;
> + struct scmi_xfer *t;
> + int ret, i;
> +
> + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> + if (ret)
> + return ret;
> +
> + if (num > MISC_MAX_VAL)
> + return -EINVAL;
> +
> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_SET, sizeof(*in),
> + 0, &t);
> + if (ret)
> + return ret;
> +
> + in = t->tx.buf;
> + in->id = cpu_to_le32(ctrl_id);
> + in->num = cpu_to_le32(num);
> + for (i = 0; i < num; i++)
> + in->value[i] = cpu_to_le32(val[i]);
> +
> + ret = ph->xops->do_xfer(ph, t);
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = {
> + .misc_ctrl_set = scmi_imx_misc_ctrl_set,
> + .misc_ctrl_get = scmi_imx_misc_ctrl_get,
> + .misc_ctrl_req_notify = scmi_imx_misc_ctrl_notify,
> +};
> +
> +static const struct scmi_protocol scmi_imx_misc = {
> + .id = SCMI_PROTOCOL_IMX_MISC,
> + .owner = THIS_MODULE,
> + .instance_init = &scmi_imx_misc_protocol_init,
> + .ops = &scmi_imx_misc_proto_ops,
> + .events = &scmi_imx_misc_protocol_events,
> + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
> + .vendor_id = "NXP",
> + .sub_vendor_id = "i.MX95 EVK",
> +};
> +module_scmi_protocol(scmi_imx_misc);
> diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
> index e59aedaa4aec..e9285abfc191 100644
> --- a/include/linux/scmi_imx_protocol.h
> +++ b/include/linux/scmi_imx_protocol.h
> @@ -13,8 +13,14 @@
> #include <linux/notifier.h>
> #include <linux/types.h>
>
> +#define SCMI_PAYLOAD_LEN 100
> +
> +#define SCMI_ARRAY(X, Y) ((SCMI_PAYLOAD_LEN - (X)) / sizeof(Y))
> +#define MISC_MAX_VAL SCMI_ARRAY(8, uint32_t)
>
You base all of this on this fixed payload length, but the payload
really depends on the configured underlying transport: you can use the
ph->hops->get_max_msg_size to retrieve the configured max payload length
for the platform you are running on....nad maybe bailout if the minimum
size required by your protocol is not available with the currently
configured transport...wouldnt't be more robust and reliable then
builtin fixing some payload ?
Thanks,
Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v4 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol
2024-06-17 18:14 ` Cristian Marussi
@ 2024-06-20 3:20 ` Peng Fan
0 siblings, 0 replies; 18+ messages in thread
From: Peng Fan @ 2024-06-20 3:20 UTC (permalink / raw)
To: Cristian Marussi, Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
> Subject: Re: [PATCH v4 4/6] firmware: arm_scmi: add initial support for
> i.MX MISC protocol
>
> On Fri, May 24, 2024 at 04:56:46PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX95 System Manager(SM) firmware includes a SCMI vendor
> protocol,
> > SCMI MISC protocol which 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 General Purpose
> > registers, Control Status Registers owned by the SM.
> >
.....
> > +
> > +static int scmi_imx_misc_ctrl_validate_id(const struct
> scmi_protocol_handle *ph,
> > + u32 ctrl_id)
> > +{
> > + struct scmi_imx_misc_info *mi = ph->get_priv(ph);
> > +
> > + if ((ctrl_id < BRD_CTRL_START_ID) && (ctrl_id > mi-
> >nr_dev_ctrl))
> > + return -EINVAL;
> > + if (ctrl_id >= BRD_CTRL_START_ID + mi->nr_brd_ctrl)
> > + return -EINVAL;
>
> ...are these conditions fine ?
Yes. they are correct.
just checking because they seem a bit
> odd...but I am certainly missing something...in case they are ok, is it
> possible to add a comment explaining why those conds lead to -
> EINVAL ?
We have a flag "MISC_CTRL_FLAG_BRD 0x8000U" to indicate it is
board ctrl or non-board ctrl
>
> > +
> > + return 0;
> > +}
> > +
> > +static int scmi_imx_misc_ctrl_notify(const struct
> scmi_protocol_handle *ph,
> > + u32 ctrl_id, u32 evt_id, u32 flags)
> {
> > + struct scmi_imx_misc_ctrl_notify_in *in;
> > + struct scmi_xfer *t;
> > + int ret;
> > +
> > + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ph->xops->xfer_get_init(ph,
> SCMI_IMX_MISC_CTRL_NOTIFY,
> > + sizeof(*in), 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + in = t->tx.buf;
> > + in->ctrl_id = cpu_to_le32(ctrl_id);
> > + in->flags = cpu_to_le32(flags);
> > +
> > + ret = ph->xops->do_xfer(ph, t);
> > +
> > + ph->xops->xfer_put(ph, t);
> > +
> > + return ret;
> > +}
> > +
> > +static int
> > +scmi_imx_misc_ctrl_set_notify_enabled(const struct
> scmi_protocol_handle *ph,
> > + u8 evt_id, u32 src_id, bool enable)
> {
> > + int ret;
> > +
> > + /* misc_ctrl_req_notify is for enablement */
> > + if (enable)
> > + return 0;
> > +
> > + ret = scmi_imx_misc_ctrl_notify(ph, src_id, evt_id, 0);
> > + if (ret)
> > + dev_err(ph->dev, "FAIL_ENABLED - evt[%X] src[%d] -
> ret:%d\n",
> > + evt_id, src_id, ret);
> > +
> > + return ret;
> > +}
> > +
> > +static int scmi_imx_misc_ctrl_get_num_sources(const struct
> > +scmi_protocol_handle *ph) {
> > + return GENMASK(15, 0);
> > +}
>
> This is statically defined at compile time..you dont need to provide this
> method, which is just for discover number of possible event sources at
> runtime....just drop it and use .num_sources in scmi_protocol_events
>
I see. Fix in v5.
> > +
> > +static void *
> > +scmi_imx_misc_ctrl_fill_custom_report(const struct
> scmi_protocol_handle *ph,
> > + u8 evt_id, ktime_t timestamp,
> > + const void *payld, size_t payld_sz,
> > + void *report, u32 *src_id)
> > +{
> > + const struct scmi_imx_misc_ctrl_notify_payld *p = payld;
> > + struct scmi_imx_misc_ctrl_notify_report *r = report;
> > +
> > + if (sizeof(*p) != payld_sz)
> > + return NULL;
> > +
> > + r->timestamp = timestamp;
> > + r->ctrl_id = p->ctrl_id;
> > + r->flags = p->flags;
> > + if (src_id)
> > + *src_id = r->ctrl_id;
> > + dev_dbg(ph->dev, "%s: ctrl_id: %d flags: %d\n", __func__,
> > + r->ctrl_id, r->flags);
> > +
> > + return r;
> > +}
> > +
> > +static const struct scmi_event_ops scmi_imx_misc_event_ops = {
> > + .get_num_sources = scmi_imx_misc_ctrl_get_num_sources,
> drop
>
> > + .set_notify_enabled = scmi_imx_misc_ctrl_set_notify_enabled,
> > + .fill_custom_report = scmi_imx_misc_ctrl_fill_custom_report,
> > +};
> > +
> > +static const struct scmi_event scmi_imx_misc_events[] = {
> > + {
> > + .id = SCMI_EVENT_IMX_MISC_CONTROL,
> > + .max_payld_sz = sizeof(struct
> scmi_imx_misc_ctrl_notify_payld),
> > + .max_report_sz = sizeof(struct
> scmi_imx_misc_ctrl_notify_report),
> > + },
> > +};
> > +
> > +static struct scmi_protocol_events scmi_imx_misc_protocol_events
> = {
> > + .queue_sz = SCMI_PROTO_QUEUE_SZ,
> > + .ops = &scmi_imx_misc_event_ops,
> > + .evts = scmi_imx_misc_events,
> > + .num_events = ARRAY_SIZE(scmi_imx_misc_events),
>
> .num_sources = MAX_MISC_CTRL_SOURCES, // GENMASK(15,
> 0)
>
> > +};
> > +
> > +static int scmi_imx_misc_protocol_init(const struct
> > +scmi_protocol_handle *ph) {
> > + struct scmi_imx_misc_info *minfo;
> > + u32 version;
> > + int ret;
> > +
> > + ret = ph->xops->version_get(ph, &version);
> > + if (ret)
> > + return ret;
> > +
> > + dev_info(ph->dev, "NXP SM MISC Version %d.%d\n",
> > + PROTOCOL_REV_MAJOR(version),
> PROTOCOL_REV_MINOR(version));
> > +
> > + minfo = devm_kzalloc(ph->dev, sizeof(*minfo), GFP_KERNEL);
> > + if (!minfo)
> > + return -ENOMEM;
> > +
> > + ret = scmi_imx_misc_attributes_get(ph, minfo);
> > + if (ret)
> > + return ret;
> > +
> > + return ph->set_priv(ph, minfo, version); }
>
> Same as previous patch please move the init downb below near the
> scmi_protocol struct right after the ops
>
Yeah. Fix in v5.
> > +
> > +static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle
> *ph,
> > + u32 ctrl_id, u32 *num, u32 *val) {
> > + struct scmi_imx_misc_ctrl_get_out *out;
> > + struct scmi_xfer *t;
> > + int ret, i;
> > +
> > + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_GET,
> sizeof(u32),
> > + 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + put_unaligned_le32(ctrl_id, t->tx.buf);
> > + ret = ph->xops->do_xfer(ph, t);
> > + if (!ret) {
> > + out = t->rx.buf;
> > + *num = le32_to_cpu(out->num);
>
> To stay even more safer, by guarding from malformed *num fields and
> just bail out upfront with an error
>
> if (*num >= MISC_MAX_VAL ||
> *num * sizeof(__le32) > t->rx.len - sizeof(__le32))
>
> and then just
>
> for (i = 0; i < *num; i++)
>
ok. looks good. I will fix in v5.
> > + for (i = 0; i < *num && i < MISC_MAX_VAL; i++)
> > + val[i] = le32_to_cpu(out->val[i]);
> > + }
> > +
> > + ph->xops->xfer_put(ph, t);
> > +
> > + return ret;
> > +}
> > +
> > +static int scmi_imx_misc_ctrl_set(const struct scmi_protocol_handle
> *ph,
> > + u32 ctrl_id, u32 num, u32 *val) {
> > + struct scmi_imx_misc_ctrl_set_in *in;
> > + struct scmi_xfer *t;
> > + int ret, i;
> > +
> > + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> > + if (ret)
> > + return ret;
> > +
> > + if (num > MISC_MAX_VAL)
> > + return -EINVAL;
> > +
> > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_SET,
> sizeof(*in),
> > + 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + in = t->tx.buf;
> > + in->id = cpu_to_le32(ctrl_id);
> > + in->num = cpu_to_le32(num);
> > + for (i = 0; i < num; i++)
> > + in->value[i] = cpu_to_le32(val[i]);
> > +
> > + ret = ph->xops->do_xfer(ph, t);
> > +
> > + ph->xops->xfer_put(ph, t);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct scmi_imx_misc_proto_ops
> scmi_imx_misc_proto_ops = {
> > + .misc_ctrl_set = scmi_imx_misc_ctrl_set,
> > + .misc_ctrl_get = scmi_imx_misc_ctrl_get,
> > + .misc_ctrl_req_notify = scmi_imx_misc_ctrl_notify, };
> > +
> > +static const struct scmi_protocol scmi_imx_misc = {
> > + .id = SCMI_PROTOCOL_IMX_MISC,
> > + .owner = THIS_MODULE,
> > + .instance_init = &scmi_imx_misc_protocol_init,
> > + .ops = &scmi_imx_misc_proto_ops,
> > + .events = &scmi_imx_misc_protocol_events,
> > + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
> > + .vendor_id = "NXP",
> > + .sub_vendor_id = "i.MX95 EVK",
> > +};
> > +module_scmi_protocol(scmi_imx_misc);
> > diff --git a/include/linux/scmi_imx_protocol.h
> > b/include/linux/scmi_imx_protocol.h
> > index e59aedaa4aec..e9285abfc191 100644
> > --- a/include/linux/scmi_imx_protocol.h
> > +++ b/include/linux/scmi_imx_protocol.h
> > @@ -13,8 +13,14 @@
> > #include <linux/notifier.h>
> > #include <linux/types.h>
> >
> > +#define SCMI_PAYLOAD_LEN 100
> > +
> > +#define SCMI_ARRAY(X, Y) ((SCMI_PAYLOAD_LEN - (X)) /
> sizeof(Y))
> > +#define MISC_MAX_VAL SCMI_ARRAY(8, uint32_t)
> >
> You base all of this on this fixed payload length, but the payload really
> depends on the configured underlying transport: you can use the
> ph->hops->get_max_msg_size to retrieve the configured max payload
> length
> for the platform you are running on....nad maybe bailout if the
> minimum size required by your protocol is not available with the
> currently configured transport...wouldnt't be more robust and reliable
> then builtin fixing some payload ?
Your point is valid. But I need check our firmware design and see
if feasible and update.
Thanks,
Peng.
>
> Thanks,
> Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 5/6] firmware: imx: support i.MX95 BBM module
2024-05-24 8:56 [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan (OSS)
` (3 preceding siblings ...)
2024-05-24 8:56 ` [PATCH v4 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol Peng Fan (OSS)
@ 2024-05-24 8:56 ` Peng Fan (OSS)
2024-06-18 9:53 ` Cristian Marussi
2024-05-24 8:56 ` [PATCH v4 6/6] firmware: imx: add i.MX95 MISC driver Peng Fan (OSS)
2024-06-17 9:42 ` [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan
6 siblings, 1 reply; 18+ messages in thread
From: Peng Fan (OSS) @ 2024-05-24 8:56 UTC (permalink / raw)
To: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan
Cc: linux-doc, linux-kernel, imx, linux-arm-kernel, devicetree
From: Peng Fan <peng.fan@nxp.com>
The BBM module provides RTC and BUTTON feature. To i.MX95, this module
is managed by System Manager. Linux could use i.MX SCMI BBM Extension
protocol to use RTC and BUTTON feature.
This driver is to use SCMI interface to get/set RTC, enable pwrkey.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/Makefile | 1 +
drivers/firmware/imx/sm-bbm.c | 314 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 315 insertions(+)
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index 8f9f04a513a8..fb20e22074e1 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
+obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
diff --git a/drivers/firmware/imx/sm-bbm.c b/drivers/firmware/imx/sm-bbm.c
new file mode 100644
index 000000000000..5e7083bf8fd3
--- /dev/null
+++ b/drivers/firmware/imx/sm-bbm.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 NXP.
+ */
+
+#include <linux/input.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+#include <linux/suspend.h>
+
+#define DEBOUNCE_TIME 30
+#define REPEAT_INTERVAL 60
+
+struct scmi_imx_bbm {
+ struct rtc_device *rtc_dev;
+ struct scmi_protocol_handle *ph;
+ const struct scmi_imx_bbm_proto_ops *ops;
+ struct notifier_block nb;
+ int keycode;
+ int keystate; /* 1:pressed */
+ bool suspended;
+ struct delayed_work check_work;
+ struct input_dev *input;
+};
+
+static int scmi_imx_bbm_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ u64 val;
+ int ret;
+
+ ret = bbnsm->ops->rtc_time_get(ph, 0, &val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ rtc_time64_to_tm(val, tm);
+
+ return 0;
+}
+
+static int scmi_imx_bbm_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ u64 val;
+ int ret;
+
+ val = rtc_tm_to_time64(tm);
+
+ ret = bbnsm->ops->rtc_time_set(ph, 0, val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ return 0;
+}
+
+static int scmi_imx_bbm_alarm_irq_enable(struct device *dev, unsigned int enable)
+{
+ return 0;
+}
+
+static int scmi_imx_bbm_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ struct rtc_time *alrm_tm = &alrm->time;
+ u64 val;
+ int ret;
+
+ val = rtc_tm_to_time64(alrm_tm);
+
+ ret = bbnsm->ops->rtc_alarm_set(ph, 0, val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ return 0;
+}
+
+static const struct rtc_class_ops smci_imx_bbm_rtc_ops = {
+ .read_time = scmi_imx_bbm_read_time,
+ .set_time = scmi_imx_bbm_set_time,
+ .set_alarm = scmi_imx_bbm_set_alarm,
+ .alarm_irq_enable = scmi_imx_bbm_alarm_irq_enable,
+};
+
+static void scmi_imx_bbm_pwrkey_check_for_events(struct work_struct *work)
+{
+ struct scmi_imx_bbm *bbnsm = container_of(work, struct scmi_imx_bbm, check_work.work);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ struct input_dev *input = bbnsm->input;
+ u32 state = 0;
+ int ret;
+
+ ret = bbnsm->ops->button_get(ph, &state);
+ if (ret) {
+ pr_err("%s: %d\n", __func__, ret);
+ return;
+ }
+
+ pr_debug("%s: state: %d, keystate %d\n", __func__, state, bbnsm->keystate);
+
+ /* only report new event if status changed */
+ if (state ^ bbnsm->keystate) {
+ bbnsm->keystate = state;
+ input_event(input, EV_KEY, bbnsm->keycode, state);
+ input_sync(input);
+ pm_relax(bbnsm->input->dev.parent);
+ pr_debug("EV_KEY: %x\n", bbnsm->keycode);
+ }
+
+ /* repeat check if pressed long */
+ if (state)
+ schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(REPEAT_INTERVAL));
+}
+
+static int scmi_imx_bbm_pwrkey_event(struct scmi_imx_bbm *bbnsm)
+{
+ struct input_dev *input = bbnsm->input;
+
+ schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(DEBOUNCE_TIME));
+
+ /*
+ * Directly report key event after resume to make no key press
+ * event is missed.
+ */
+ if (bbnsm->suspended) {
+ bbnsm->keystate = 1;
+ input_event(input, EV_KEY, bbnsm->keycode, 1);
+ input_sync(input);
+ bbnsm->suspended = false;
+ }
+
+ return 0;
+}
+
+static void scmi_imx_bbm_pwrkey_act(void *pdata)
+{
+ struct scmi_imx_bbm *bbnsm = pdata;
+
+ cancel_delayed_work_sync(&bbnsm->check_work);
+}
+
+static int scmi_imx_bbm_notifier(struct notifier_block *nb, unsigned long event, void *data)
+{
+ struct scmi_imx_bbm *bbnsm = container_of(nb, struct scmi_imx_bbm, nb);
+ struct scmi_imx_bbm_notif_report *r = data;
+
+ if (r->is_rtc)
+ rtc_update_irq(bbnsm->rtc_dev, 1, RTC_AF | RTC_IRQF);
+ if (r->is_button) {
+ pr_debug("BBM Button Power key pressed\n");
+ scmi_imx_bbm_pwrkey_event(bbnsm);
+ }
+
+ return 0;
+}
+
+static int scmi_imx_bbm_pwrkey_init(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct input_dev *input;
+ int ret;
+
+ if (device_property_read_u32(dev, "linux,code", &bbnsm->keycode)) {
+ bbnsm->keycode = KEY_POWER;
+ dev_warn(dev, "key code is not specified, using default KEY_POWER\n");
+ }
+
+ INIT_DELAYED_WORK(&bbnsm->check_work, scmi_imx_bbm_pwrkey_check_for_events);
+
+ input = devm_input_allocate_device(dev);
+ if (!input) {
+ dev_err(dev, "failed to allocate the input device for SCMI IMX BBM\n");
+ return -ENOMEM;
+ }
+
+ input->name = dev_name(dev);
+ input->phys = "bbnsm-pwrkey/input0";
+ input->id.bustype = BUS_HOST;
+
+ input_set_capability(input, EV_KEY, bbnsm->keycode);
+
+ ret = devm_add_action_or_reset(dev, scmi_imx_bbm_pwrkey_act, bbnsm);
+ if (ret) {
+ dev_err(dev, "failed to register remove action\n");
+ return ret;
+ }
+
+ bbnsm->input = input;
+
+ ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
+ SCMI_EVENT_IMX_BBM_BUTTON,
+ NULL, &bbnsm->nb);
+
+ if (ret)
+ dev_err(dev, "Failed to register BBM Button Events %d:", ret);
+
+ ret = input_register_device(input);
+ if (ret) {
+ dev_err(dev, "failed to register input device\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int scmi_imx_bbm_rtc_init(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ int ret;
+
+ bbnsm->rtc_dev = devm_rtc_allocate_device(dev);
+ if (IS_ERR(bbnsm->rtc_dev))
+ return PTR_ERR(bbnsm->rtc_dev);
+
+ bbnsm->rtc_dev->ops = &smci_imx_bbm_rtc_ops;
+ bbnsm->rtc_dev->range_min = 0;
+ bbnsm->rtc_dev->range_max = U32_MAX;
+
+ ret = devm_rtc_register_device(bbnsm->rtc_dev);
+ if (ret)
+ return ret;
+
+ bbnsm->nb.notifier_call = &scmi_imx_bbm_notifier;
+ return handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
+ SCMI_EVENT_IMX_BBM_RTC,
+ NULL, &bbnsm->nb);
+}
+
+static int scmi_imx_bbm_probe(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_protocol_handle *ph;
+ struct scmi_imx_bbm *bbnsm;
+ int ret;
+
+ if (!handle)
+ return -ENODEV;
+
+ bbnsm = devm_kzalloc(dev, sizeof(struct scmi_imx_bbm), GFP_KERNEL);
+ if (!bbnsm)
+ return -ENOMEM;
+
+ bbnsm->ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_BBM, &ph);
+ if (IS_ERR(bbnsm->ops))
+ return PTR_ERR(bbnsm->ops);
+
+ bbnsm->ph = ph;
+
+ device_init_wakeup(dev, true);
+
+ dev_set_drvdata(dev, bbnsm);
+
+ ret = scmi_imx_bbm_rtc_init(sdev);
+ if (ret) {
+ dev_err(dev, "rtc init failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = scmi_imx_bbm_pwrkey_init(sdev);
+ if (ret) {
+ dev_err(dev, "pwr init failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int __maybe_unused scmi_imx_bbm_suspend(struct device *dev)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+
+ bbnsm->suspended = true;
+
+ return 0;
+}
+
+static int __maybe_unused scmi_imx_bbm_resume(struct device *dev)
+{
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(scmi_imx_bbm_pm_ops, scmi_imx_bbm_suspend, scmi_imx_bbm_resume);
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_IMX_BBM, "imx-bbm" },
+ { },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_imx_bbm_driver = {
+ .driver = {
+ .pm = &scmi_imx_bbm_pm_ops,
+ },
+ .name = "scmi-imx-bbm",
+ .probe = scmi_imx_bbm_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_imx_bbm_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("IMX SM BBM driver");
+MODULE_LICENSE("GPL");
--
2.37.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 5/6] firmware: imx: support i.MX95 BBM module
2024-05-24 8:56 ` [PATCH v4 5/6] firmware: imx: support i.MX95 BBM module Peng Fan (OSS)
@ 2024-06-18 9:53 ` Cristian Marussi
2024-06-20 3:30 ` Peng Fan
0 siblings, 1 reply; 18+ messages in thread
From: Cristian Marussi @ 2024-06-18 9:53 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan, linux-doc,
linux-kernel, imx, linux-arm-kernel, devicetree
On Fri, May 24, 2024 at 04:56:47PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> The BBM module provides RTC and BUTTON feature. To i.MX95, this module
> is managed by System Manager. Linux could use i.MX SCMI BBM Extension
> protocol to use RTC and BUTTON feature.
>
> This driver is to use SCMI interface to get/set RTC, enable pwrkey.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/imx/Makefile | 1 +
> drivers/firmware/imx/sm-bbm.c | 314 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 315 insertions(+)
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index 8f9f04a513a8..fb20e22074e1 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
> +obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
> diff --git a/drivers/firmware/imx/sm-bbm.c b/drivers/firmware/imx/sm-bbm.c
> new file mode 100644
> index 000000000000..5e7083bf8fd3
> --- /dev/null
> +++ b/drivers/firmware/imx/sm-bbm.c
> @@ -0,0 +1,314 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2024 NXP.
> + */
> +
> +#include <linux/input.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/scmi_imx_protocol.h>
> +#include <linux/suspend.h>
> +
> +#define DEBOUNCE_TIME 30
> +#define REPEAT_INTERVAL 60
> +
> +struct scmi_imx_bbm {
> + struct rtc_device *rtc_dev;
> + struct scmi_protocol_handle *ph;
> + const struct scmi_imx_bbm_proto_ops *ops;
> + struct notifier_block nb;
> + int keycode;
> + int keystate; /* 1:pressed */
> + bool suspended;
> + struct delayed_work check_work;
> + struct input_dev *input;
> +};
You could pull out the *ops in a global like
static const struct scmi_imx_bbm_proto_ops *bbm_ops;
since the protocol ops handle are always the same you will end up
overwriting it with the same value if this driver is probed multiple
times (which is harmless)...you will get anyway a different *ph handle
to operate on that you are already storing...
..up to you really
> +
> +static int scmi_imx_bbm_read_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> + struct scmi_protocol_handle *ph = bbnsm->ph;
> + u64 val;
> + int ret;
> +
> + ret = bbnsm->ops->rtc_time_get(ph, 0, &val);
> + if (ret)
> + dev_err(dev, "%s: %d\n", __func__, ret);
> +
..so if you fail to get the time via SCMI you just carry on without caring ?
> + rtc_time64_to_tm(val, tm);
... converting some random val from the stack into tm ?
> +
> + return 0;
> +}
> +
> +static int scmi_imx_bbm_set_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> + struct scmi_protocol_handle *ph = bbnsm->ph;
> + u64 val;
> + int ret;
> +
> + val = rtc_tm_to_time64(tm);
> +
> + ret = bbnsm->ops->rtc_time_set(ph, 0, val);
> + if (ret)
> + dev_err(dev, "%s: %d\n", __func__, ret);
> +
same here why you dont report any error ?
> + return 0;
> +}
> +
> +static int scmi_imx_bbm_alarm_irq_enable(struct device *dev, unsigned int enable)
> +{
> + return 0;
> +}
> +
> +static int scmi_imx_bbm_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> + struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> + struct scmi_protocol_handle *ph = bbnsm->ph;
> + struct rtc_time *alrm_tm = &alrm->time;
> + u64 val;
> + int ret;
> +
> + val = rtc_tm_to_time64(alrm_tm);
> +
> + ret = bbnsm->ops->rtc_alarm_set(ph, 0, val);
> + if (ret)
> + dev_err(dev, "%s: %d\n", __func__, ret);
> +
same ...
....mmm... hang on ... this reminds me of something...
https://lore.kernel.org/linux-arm-kernel/ZdjgSxFx9YRP107y@pluto/
...I did exactly the same comments on V1 around these 2 BBM/MISC SCMI
drivers....but you never replied, addressed or disputed those issues.
You DID address other review/comments in protocols and DT in later
versions so I suppose you just forget these latter drivers reviewes
and just rebased on.
So, this review stops here for now, please address or reply to my V1
concerns on these last 2 BBM/MISC drivers.
Thanks,
Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v4 5/6] firmware: imx: support i.MX95 BBM module
2024-06-18 9:53 ` Cristian Marussi
@ 2024-06-20 3:30 ` Peng Fan
0 siblings, 0 replies; 18+ messages in thread
From: Peng Fan @ 2024-06-20 3:30 UTC (permalink / raw)
To: Cristian Marussi, Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
> Subject: Re: [PATCH v4 5/6] firmware: imx: support i.MX95 BBM
> module
>
> On Fri, May 24, 2024 at 04:56:47PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The BBM module provides RTC and BUTTON feature. To i.MX95, this
> module
> > is managed by System Manager. Linux could use i.MX SCMI BBM
> Extension
> > protocol to use RTC and BUTTON feature.
> >
> > This driver is to use SCMI interface to get/set RTC, enable pwrkey.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > drivers/firmware/imx/Makefile | 1 +
> > drivers/firmware/imx/sm-bbm.c | 314
> > ++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 315 insertions(+)
> >
> > diff --git a/drivers/firmware/imx/Makefile
> > b/drivers/firmware/imx/Makefile index 8f9f04a513a8..fb20e22074e1
> > 100644
> > --- a/drivers/firmware/imx/Makefile
> > +++ b/drivers/firmware/imx/Makefile
> > @@ -1,3 +1,4 @@
> > # SPDX-License-Identifier: GPL-2.0
> > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-
> irq.o rm.o imx-scu-soc.o
> > +obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
> > diff --git a/drivers/firmware/imx/sm-bbm.c
> > b/drivers/firmware/imx/sm-bbm.c new file mode 100644 index
> > 000000000000..5e7083bf8fd3
> > --- /dev/null
> > +++ b/drivers/firmware/imx/sm-bbm.c
> > @@ -0,0 +1,314 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2024 NXP.
> > + */
> > +
> > +#include <linux/input.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/rtc.h>
> > +#include <linux/scmi_protocol.h>
> > +#include <linux/scmi_imx_protocol.h>
> > +#include <linux/suspend.h>
> > +
> > +#define DEBOUNCE_TIME 30
> > +#define REPEAT_INTERVAL 60
> > +
> > +struct scmi_imx_bbm {
> > + struct rtc_device *rtc_dev;
> > + struct scmi_protocol_handle *ph;
> > + const struct scmi_imx_bbm_proto_ops *ops;
> > + struct notifier_block nb;
> > + int keycode;
> > + int keystate; /* 1:pressed */
> > + bool suspended;
> > + struct delayed_work check_work;
> > + struct input_dev *input;
> > +};
>
> You could pull out the *ops in a global like
>
> static const struct scmi_imx_bbm_proto_ops *bbm_ops;
>
> since the protocol ops handle are always the same you will end up
> overwriting it with the same value if this driver is probed multiple
> times (which is harmless)...you will get anyway a different *ph handle
> to operate on that you are already storing...
>
> ..up to you really
I prefer to keep as it is
>
> > +
> > +static int scmi_imx_bbm_read_time(struct device *dev, struct
> rtc_time
> > +*tm) {
> > + struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > + struct scmi_protocol_handle *ph = bbnsm->ph;
> > + u64 val;
> > + int ret;
> > +
> > + ret = bbnsm->ops->rtc_time_get(ph, 0, &val);
> > + if (ret)
> > + dev_err(dev, "%s: %d\n", __func__, ret);
> > +
>
> ..so if you fail to get the time via SCMI you just carry on without caring ?
>
> > + rtc_time64_to_tm(val, tm);
>
> ... converting some random val from the stack into tm ?
>
I will fix in v5, should return an error value.
> > +
> > + return 0;
> > +}
> > +
> > +static int scmi_imx_bbm_set_time(struct device *dev, struct
> rtc_time
> > +*tm) {
> > + struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > + struct scmi_protocol_handle *ph = bbnsm->ph;
> > + u64 val;
> > + int ret;
> > +
> > + val = rtc_tm_to_time64(tm);
> > +
> > + ret = bbnsm->ops->rtc_time_set(ph, 0, val);
> > + if (ret)
> > + dev_err(dev, "%s: %d\n", __func__, ret);
> > +
>
> same here why you dont report any error ?
>
Should return error. Fix in v5.
> > + return 0;
> > +}
> > +
> > +static int scmi_imx_bbm_alarm_irq_enable(struct device *dev,
> unsigned
> > +int enable) {
> > + return 0;
> > +}
> > +
> > +static int scmi_imx_bbm_set_alarm(struct device *dev, struct
> > +rtc_wkalrm *alrm) {
> > + struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > + struct scmi_protocol_handle *ph = bbnsm->ph;
> > + struct rtc_time *alrm_tm = &alrm->time;
> > + u64 val;
> > + int ret;
> > +
> > + val = rtc_tm_to_time64(alrm_tm);
> > +
> > + ret = bbnsm->ops->rtc_alarm_set(ph, 0, val);
> > + if (ret)
> > + dev_err(dev, "%s: %d\n", __func__, ret);
> > +
>
> same ...
>
> ....mmm... hang on ... this reminds me of something...
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> lore.kernel.org%2Flinux-arm-
> kernel%2FZdjgSxFx9YRP107y%40pluto%2F&data=05%7C02%7Cpeng.f
> an%40nxp.com%7C91000402f63d4e893d8208dc8f7c9399%7C686ea1
> d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638543012417198736
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=nK
> leYfoIRJtMN13TNQB9xYH8aX1tUYk9UI202tYl49I%3D&reserved=0
>
> ...I did exactly the same comments on V1 around these 2 BBM/MISC
> SCMI drivers....but you never replied, addressed or disputed those
> issues.
>
> You DID address other review/comments in protocols and DT in later
> versions so I suppose you just forget these latter drivers reviewes and
> just rebased on.
>
> So, this review stops here for now, please address or reply to my V1
> concerns on these last 2 BBM/MISC drivers.
My bad. I will re-collect all the comments and address in new version.
Thanks,
Peng.
>
> Thanks,
> Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 6/6] firmware: imx: add i.MX95 MISC driver
2024-05-24 8:56 [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan (OSS)
` (4 preceding siblings ...)
2024-05-24 8:56 ` [PATCH v4 5/6] firmware: imx: support i.MX95 BBM module Peng Fan (OSS)
@ 2024-05-24 8:56 ` Peng Fan (OSS)
2024-06-18 9:55 ` Cristian Marussi
2024-06-17 9:42 ` [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan
6 siblings, 1 reply; 18+ messages in thread
From: Peng Fan (OSS) @ 2024-05-24 8:56 UTC (permalink / raw)
To: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan
Cc: linux-doc, linux-kernel, imx, linux-arm-kernel, devicetree
From: Peng Fan <peng.fan@nxp.com>
The i.MX95 System manager exports SCMI MISC protocol for linux to do
various settings, such as set board gpio expander as wakeup source.
The driver is to add the support.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/Makefile | 1 +
drivers/firmware/imx/sm-misc.c | 108 ++++++++++++++++++++++++++++++++++++++++
include/linux/firmware/imx/sm.h | 33 ++++++++++++
3 files changed, 142 insertions(+)
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index fb20e22074e1..cb9c361d9b81 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
+obj-${CONFIG_IMX_SCMI_MISC_EXT} += sm-misc.o
diff --git a/drivers/firmware/imx/sm-misc.c b/drivers/firmware/imx/sm-misc.c
new file mode 100644
index 000000000000..22c1a5819425
--- /dev/null
+++ b/drivers/firmware/imx/sm-misc.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 NXP.
+ */
+
+#include <linux/firmware/imx/sm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+static const struct scmi_imx_misc_proto_ops *imx_misc_ctrl_ops;
+static struct scmi_protocol_handle *ph;
+struct notifier_block scmi_imx_misc_ctrl_nb;
+
+int scmi_imx_misc_ctrl_set(u32 id, u32 val)
+{
+ if (!ph)
+ return -EPROBE_DEFER;
+
+ return imx_misc_ctrl_ops->misc_ctrl_set(ph, id, 1, &val);
+};
+EXPORT_SYMBOL(scmi_imx_misc_ctrl_set);
+
+int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
+{
+ if (!ph)
+ return -EPROBE_DEFER;
+
+ return imx_misc_ctrl_ops->misc_ctrl_get(ph, id, num, val);
+}
+EXPORT_SYMBOL(scmi_imx_misc_ctrl_get);
+
+static int scmi_imx_misc_ctrl_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ return 0;
+}
+
+static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device_node *np = sdev->dev.of_node;
+ u32 src_id, flags;
+ int ret, i, num;
+
+ if (!handle)
+ return -ENODEV;
+
+ imx_misc_ctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_MISC, &ph);
+ if (IS_ERR(imx_misc_ctrl_ops))
+ return PTR_ERR(imx_misc_ctrl_ops);
+
+ scmi_imx_misc_ctrl_nb.notifier_call = &scmi_imx_misc_ctrl_notifier;
+ num = of_property_count_u32_elems(np, "nxp,ctrl-ids");
+ if (num % 2) {
+ dev_err(&sdev->dev, "Invalid wakeup-sources\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num; i += 2) {
+ ret = of_property_read_u32_index(np, "nxp,ctrl-ids", i, &src_id);
+ if (ret) {
+ dev_err(&sdev->dev, "Failed to read ctrl-id: %i\n", i);
+ continue;
+ }
+
+ ret = of_property_read_u32_index(np, "nxp,ctrl-ids", i + 1, &flags);
+ if (ret) {
+ dev_err(&sdev->dev, "Failed to read ctrl-id value: %d\n", i + 1);
+ continue;
+ }
+
+ ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_MISC,
+ SCMI_EVENT_IMX_MISC_CONTROL,
+ &src_id,
+ &scmi_imx_misc_ctrl_nb);
+ if (ret)
+ dev_err(&sdev->dev, "Failed to register scmi misc event: %d\n", src_id);
+ else {
+ ret = imx_misc_ctrl_ops->misc_ctrl_req_notify(ph, src_id,
+ SCMI_EVENT_IMX_MISC_CONTROL,
+ flags);
+ if (ret)
+ dev_err(&sdev->dev, "Failed to req notify: %d\n", src_id);
+ }
+ }
+
+ return 0;
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_IMX_MISC, "imx-misc-ctrl" },
+ { },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_imx_misc_ctrl_driver = {
+ .name = "scmi-imx-misc-ctrl",
+ .probe = scmi_imx_misc_ctrl_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_imx_misc_ctrl_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("IMX SM MISC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
new file mode 100644
index 000000000000..daad4bdf7d1c
--- /dev/null
+++ b/include/linux/firmware/imx/sm.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2024 NXP
+ */
+
+#ifndef _SCMI_IMX_H
+#define _SCMI_IMX_H
+
+#include <linux/bitfield.h>
+#include <linux/types.h>
+
+#define SCMI_IMX_CTRL_PDM_CLK_SEL 0 /* AON PDM clock sel */
+#define SCMI_IMX_CTRL_MQS1_SETTINGS 1 /* AON MQS settings */
+#define SCMI_IMX_CTRL_SAI1_MCLK 2 /* AON SAI1 MCLK */
+#define SCMI_IMX_CTRL_SAI3_MCLK 3 /* WAKE SAI3 MCLK */
+#define SCMI_IMX_CTRL_SAI4_MCLK 4 /* WAKE SAI4 MCLK */
+#define SCMI_IMX_CTRL_SAI5_MCLK 5 /* WAKE SAI5 MCLK */
+
+#if IS_ENABLED(CONFIG_IMX_SCMI_MISC_EXT)
+int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val);
+int scmi_imx_misc_ctrl_set(u32 id, u32 val);
+#else
+static inline int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int scmi_imx_misc_ctrl_set(u32 id, u32 val);
+{
+ return -EOPNOTSUPP;
+}
+#endif
+#endif
--
2.37.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 6/6] firmware: imx: add i.MX95 MISC driver
2024-05-24 8:56 ` [PATCH v4 6/6] firmware: imx: add i.MX95 MISC driver Peng Fan (OSS)
@ 2024-06-18 9:55 ` Cristian Marussi
0 siblings, 0 replies; 18+ messages in thread
From: Cristian Marussi @ 2024-06-18 9:55 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Jonathan Corbet, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peng Fan, linux-doc,
linux-kernel, imx, linux-arm-kernel, devicetree
On Fri, May 24, 2024 at 04:56:48PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> The i.MX95 System manager exports SCMI MISC protocol for linux to do
> various settings, such as set board gpio expander as wakeup source.
>
> The driver is to add the support.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/imx/Makefile | 1 +
> drivers/firmware/imx/sm-misc.c | 108 ++++++++++++++++++++++++++++++++++++++++
> include/linux/firmware/imx/sm.h | 33 ++++++++++++
> 3 files changed, 142 insertions(+)
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index fb20e22074e1..cb9c361d9b81 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -2,3 +2,4 @@
> obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
> obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
> +obj-${CONFIG_IMX_SCMI_MISC_EXT} += sm-misc.o
> diff --git a/drivers/firmware/imx/sm-misc.c b/drivers/firmware/imx/sm-misc.c
> new file mode 100644
> index 000000000000..22c1a5819425
> --- /dev/null
> +++ b/drivers/firmware/imx/sm-misc.c
> @@ -0,0 +1,108 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2024 NXP.
> + */
> +
> +#include <linux/firmware/imx/sm.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/scmi_imx_protocol.h>
> +
> +static const struct scmi_imx_misc_proto_ops *imx_misc_ctrl_ops;
> +static struct scmi_protocol_handle *ph;
Same comments as n V1 ... please have a look :P
Thanks,
Cristian
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion
2024-05-24 8:56 [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion Peng Fan (OSS)
` (5 preceding siblings ...)
2024-05-24 8:56 ` [PATCH v4 6/6] firmware: imx: add i.MX95 MISC driver Peng Fan (OSS)
@ 2024-06-17 9:42 ` Peng Fan
6 siblings, 0 replies; 18+ messages in thread
From: Peng Fan @ 2024-06-17 9:42 UTC (permalink / raw)
To: Peng Fan (OSS), Jonathan Corbet, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Sudeep Holla,
Cristian Marussi, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org
> Subject: [PATCH v4 0/6] firmware: support i.MX95 SCMI BBM/MISC
> Extenstion
Any comments for the driver and doc part?
Thanks,
Peng.
>
> 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: Jonathan Corbet <corbet@lwn.net>
> 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: Rob Herring <robh@kernel.org>
> To: Krzysztof Kozlowski <krzk+dt@kernel.org>
> To: Conor Dooley <conor+dt@kernel.org>
> To: Sudeep Holla <sudeep.holla@arm.com>
> To: Cristian Marussi <cristian.marussi@arm.com>
> To: Peng Fan <peng.fan@nxp.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
>
> 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 (6):
> Documentation: firmware-guide: add NXP i.MX95 SCMI
> documentation
> 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: imx: support i.MX95 BBM module
> firmware: imx: add i.MX95 MISC driver
>
> .../devicetree/bindings/firmware/arm,scmi.yaml | 5 +-
> .../bindings/firmware/nxp,imx95-scmi.yaml | 43 +
> Documentation/firmware-guide/index.rst | 10 +
> Documentation/firmware-guide/nxp/imx95-scmi.rst | 877
> +++++++++++++++++++++
> Documentation/firmware-guide/nxp/index.rst | 10 +
> 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 | 380 +++++++++
> drivers/firmware/arm_scmi/imx/imx-sm-misc.c | 303 +++++++
> drivers/firmware/imx/Makefile | 2 +
> drivers/firmware/imx/sm-bbm.c | 314 ++++++++
> drivers/firmware/imx/sm-misc.c | 108 +++
> include/linux/firmware/imx/sm.h | 33 +
> include/linux/scmi_imx_protocol.h | 64 ++
> 16 files changed, 2177 insertions(+), 1 deletion(-)
> ---
> base-commit: 81ec2bad50c0c1bd3c66389fda32a6f2cf922508
> change-id: 20240405-imx95-bbm-misc-v2-b5e9d24adc42
>
> Best regards,
> --
> Peng Fan <peng.fan@nxp.com>
^ permalink raw reply [flat|nested] 18+ messages in thread