| Message ID | fdfedf6dd0196afd887049877eae9fce0fe63eb2.1698353854.git.oleksii_moisieiev@epam.com |
|---|---|
| State | New |
| Headers | show |
| Series | firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support | expand |
On Fri, Oct 27, 2023 at 06:28:09AM +0000, Oleksii Moisieiev wrote: > Current SCMI implementation supports only receiving arrays from the > SCMI server and provides helpers to process received data. It uses > msg_max_size value to determine maximum message size that can be > transmitted via selected protocol. When sending arrays to SCMI server > this value should be checked by the Client driver to prevent > overflowing protocol buffers. > That's why scmi_get_max_msg_size call was introduced. > Hi Oleksii, indeed given the new variable sized v3.2 SCMI requests (instead of responses) this common helper is now needed for the protocols to be able to properly size and chunk their command requests, BUT this is a new core helper that has potentially to be available to any future protocol so it has to be exposed as a common helpers in helpers_ops (like iterators or extended_name helpers), if NOT this common method won't be available to protocols when compiled as distinct loadable modules (vendor-modules can be defined and built as LKM) Thanks, Cristian
Hi Cristian, Just found an interesting note in the PINCTRL_CONFIG_SET command description: The maximum value of this field is limited by the transport used. The agent needs to specify this field such that the entire command can be accommodated within the transport chosen. Furthermore, I observed the absence of a skip_configs parameter. From my understanding, this implies that the maximum number of configurations should not exceed the msg_max_size allowed by the protocol, enabling the transmission of only one message to the SCMI server at a time. Given this constraint, it seems we might not require additional helper functions. We could potentially just verify against msg_max_size. Best regards, Oleksii On 02.11.23 09:29, Cristian Marussi wrote: > On Fri, Oct 27, 2023 at 06:28:09AM +0000, Oleksii Moisieiev wrote: >> Current SCMI implementation supports only receiving arrays from the >> SCMI server and provides helpers to process received data. It uses >> msg_max_size value to determine maximum message size that can be >> transmitted via selected protocol. When sending arrays to SCMI server >> this value should be checked by the Client driver to prevent >> overflowing protocol buffers. >> That's why scmi_get_max_msg_size call was introduced. >> > > Hi Oleksii, > > indeed given the new variable sized v3.2 SCMI requests (instead of > responses) this common helper is now needed for the protocols to be > able to properly size and chunk their command requests, BUT this is > a new core helper that has potentially to be available to any future > protocol so it has to be exposed as a common helpers in helpers_ops > (like iterators or extended_name helpers), if NOT this common method > won't be available to protocols when compiled as distinct loadable > modules (vendor-modules can be defined and built as LKM) > > Thanks, > Cristian >
On Thu, Nov 02, 2023 at 01:57:24PM +0000, Oleksii Moisieiev wrote: > Hi Cristian, > Hi, > Just found an interesting note in the PINCTRL_CONFIG_SET command > description: > > The maximum value of this field is limited by > the transport used. The agent needs to specify > this field such that the entire command can be > accommodated within the transport chosen. > Yes I am aware of that. > Furthermore, I observed the absence of a skip_configs parameter. > > From my understanding, this implies that the maximum number of > configurations should not exceed the msg_max_size allowed by the > protocol, enabling the transmission of only one message to the SCMI > server at a time. > Yes that is correct, my understanding is that the transmitter is in charge of building a message whose payload can fit into the maximum message size allowed by the underlying configured transport. > Given this constraint, it seems we might not require additional helper > functions. We could potentially just verify against msg_max_size. > Indeed for that reason the scmi_get_max_msg_size that you introduced is just enough since it allows you to peek into the transport and get the max_msg_size...the misunderstanding is around the fact that I was simply meaning that you should plug it into some new helper_ops so that yo can call it like: max_msg = ph->hops->get_max_msg_size(ph); (like iterators or get_extended_name) Because in this way you could use it also when the protocol is build as a loadable module, thing that now it is possible only for vendor defined protocols, but we could also easily switch all the base protocols to be selectable via Kconfig and =m in the future (if ever) Your helper is fine by itself it is just that it cannot be called by a protocol defined to loaded as a module because the symbol is not exported and, indeed, we introduced the ph->hops thing just for this reason, i.e. to have a set of common protocol utilities that can be called even from loadable modules protocols without the need to export every single symbol. The reference to iterators and extended_name was misleading probably...my bad...or I am still missing something :D Thanks, Cristian
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index c46dc5215af7..3db97f59bc59 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -286,6 +286,9 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle, int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer, unsigned int timeout_ms); + +int scmi_get_max_msg_size(const struct scmi_protocol_handle *ph); + #ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX extern const struct scmi_desc scmi_mailbox_desc; #endif diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 729201d8f935..f15e9b2b21f3 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1152,6 +1152,22 @@ int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo, return ret; } +/** + * scmi_get_max_msg_size - An helper to get currently configured + * maximum message size. + * + * @ph: SCMI protocol handle + * + * Return: Maximum message size for the current protocol. + */ +int scmi_get_max_msg_size(const struct scmi_protocol_handle *ph) +{ + const struct scmi_protocol_instance *pi = ph_to_pi(ph); + struct scmi_info *info = handle_to_scmi_info(pi->handle); + + return info->desc->max_msg_size; +} + /** * do_xfer() - Do one transfer *
Current SCMI implementation supports only receiving arrays from the SCMI server and provides helpers to process received data. It uses msg_max_size value to determine maximum message size that can be transmitted via selected protocol. When sending arrays to SCMI server this value should be checked by the Client driver to prevent overflowing protocol buffers. That's why scmi_get_max_msg_size call was introduced. Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> --- drivers/firmware/arm_scmi/common.h | 3 +++ drivers/firmware/arm_scmi/driver.c | 16 ++++++++++++++++ 2 files changed, 19 insertions(+)