[PATCH SCP] module/scmi_pin_control: allow variable length responses

[PATCH SCP] module/scmi_pin_control: allow variable length responses

[Dan Carpenter]

The PINCTRL_LIST_ASSOCIATIONS and PINCTRL_SETTINGS_GET commands can
return variable length responses.  Originally, they were hard coded as
returning only one response.  Change the length to -1 and change
scmi_message_validation() to skip the length checking for lengths when
it's -1.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 module/scmi/src/mod_scmi.c                         | 5 ++++-
 module/scmi_pin_control/src/mod_scmi_pin_control.c | 6 ++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/module/scmi/src/mod_scmi.c b/module/scmi/src/mod_scmi.c
index dfaf441fbae5..da4ecc09e13b 100644
--- a/module/scmi/src/mod_scmi.c
+++ b/module/scmi/src/mod_scmi.c
@@ -434,8 +434,11 @@ static int scmi_message_validation(
         return (int)SCMI_NOT_FOUND;
     }
 
-    if (payload_size != payload_size_table[message_id]) {
+    if (payload_size_table[message_id] != -1 &&
+        payload_size != payload_size_table[message_id]) {
         /* Incorrect payload size or message is not supported */
+        FWK_LOG_ERR("invalid message size: message_id=%ld payload_size=%ld vs %ld\n",
+		    message_id, payload_size, payload_size_table[message_id]);
         return (int)SCMI_PROTOCOL_ERROR;
     }
 
diff --git a/module/scmi_pin_control/src/mod_scmi_pin_control.c b/module/scmi_pin_control/src/mod_scmi_pin_control.c
index 058c89f9512f..2a1095dba9b4 100644
--- a/module/scmi_pin_control/src/mod_scmi_pin_control.c
+++ b/module/scmi_pin_control/src/mod_scmi_pin_control.c
@@ -99,12 +99,10 @@ static const size_t payload_size_table[MOD_SCMI_PIN_CONTROL_COMMAND_COUNT] = {
         (unsigned int)sizeof(struct scmi_protocol_message_attributes_a2p),
     [MOD_SCMI_PIN_CONTROL_ATTRIBUTES] =
         (unsigned int)sizeof(struct scmi_pin_control_attributes_a2p),
-    [MOD_SCMI_PIN_CONTROL_LIST_ASSOCIATIONS] =
-        (unsigned int)sizeof(struct scmi_pin_control_list_associations_a2p),
-    [MOD_SCMI_PIN_CONTROL_SETTINGS_GET] =
-        (unsigned int)sizeof(struct scmi_pin_control_settings_get_a2p),
     [MOD_SCMI_PIN_CONTROL_SETTINGS_CONFIGURE] =
         (unsigned int)sizeof(struct scmi_pin_control_settings_configure_a2p),
+    [MOD_SCMI_PIN_CONTROL_LIST_ASSOCIATIONS] = -1,
+    [MOD_SCMI_PIN_CONTROL_SETTINGS_GET] = -1,
     [MOD_SCMI_PIN_CONTROL_REQUEST] =
         (unsigned int)sizeof(struct scmi_pin_control_request_a2p),
     [MOD_SCMI_PIN_CONTROL_RELEASE] =
-- 
2.47.2

Re: [PATCH SCP] module/scmi_pin_control: allow variable length responses

[Khaled Ali Ahmed]

Good morning Dan,
I am sending this email to add the whole attached group in the CC, also to list more details about the implementation of the Dynamic length setting configuration.
This takes into account the current code base, also the "scmi_message_validation" scmi API has been updated, and a new macro has been introduced in the mod_scmi.h "MOD_SCMI_PROTOCOL_DAYNAMIC_PAYLOAD_SIZE".

Thanks, Dan, in advance

________________________________________
From: Dan Carpenter <dan.carpenter@linaro.org>
Sent: Monday, April 28, 2025 10:27 AM
To: Khaled Ali Ahmed <Khaled.AliAhmed@arm.com>
Cc: arm-scmi@vger.kernel.org <arm-scmi@vger.kernel.org>; Girish Pathak <Girish.Pathak@arm.com>
Subject: [PATCH SCP] module/scmi_pin_control: allow variable length responses

The PINCTRL_LIST_ASSOCIATIONS and PINCTRL_SETTINGS_GET commands can
return variable length responses.  Originally, they were hard coded as
returning only one response.  Change the length to -1 and change
scmi_message_validation() to skip the length checking for lengths when
it's -1.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 module/scmi/src/mod_scmi.c                         | 5 ++++-
 module/scmi_pin_control/src/mod_scmi_pin_control.c | 6 ++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/module/scmi/src/mod_scmi.c b/module/scmi/src/mod_scmi.c
index dfaf441fbae5..da4ecc09e13b 100644
--- a/module/scmi/src/mod_scmi.c
+++ b/module/scmi/src/mod_scmi.c
@@ -434,8 +434,11 @@ static int scmi_message_validation(
         return (int)SCMI_NOT_FOUND;
     }

-    if (payload_size != payload_size_table[message_id]) {
+    if (payload_size_table[message_id] != -1 &&
+        payload_size != payload_size_table[message_id]) {
         /* Incorrect payload size or message is not supported */
+        FWK_LOG_ERR("invalid message size: message_id=%ld payload_size=%ld vs %ld\n",
+                   message_id, payload_size, payload_size_table[message_id]);
         return (int)SCMI_PROTOCOL_ERROR;
     }

diff --git a/module/scmi_pin_control/src/mod_scmi_pin_control.c b/module/scmi_pin_control/src/mod_scmi_pin_control.c
index 058c89f9512f..2a1095dba9b4 100644
--- a/module/scmi_pin_control/src/mod_scmi_pin_control.c
+++ b/module/scmi_pin_control/src/mod_scmi_pin_control.c
@@ -99,12 +99,10 @@ static const size_t payload_size_table[MOD_SCMI_PIN_CONTROL_COMMAND_COUNT] = {
         (unsigned int)sizeof(struct scmi_protocol_message_attributes_a2p),
     [MOD_SCMI_PIN_CONTROL_ATTRIBUTES] =
         (unsigned int)sizeof(struct scmi_pin_control_attributes_a2p),
-    [MOD_SCMI_PIN_CONTROL_LIST_ASSOCIATIONS] =
-        (unsigned int)sizeof(struct scmi_pin_control_list_associations_a2p),
-    [MOD_SCMI_PIN_CONTROL_SETTINGS_GET] =
-        (unsigned int)sizeof(struct scmi_pin_control_settings_get_a2p),
     [MOD_SCMI_PIN_CONTROL_SETTINGS_CONFIGURE] =
         (unsigned int)sizeof(struct scmi_pin_control_settings_configure_a2p),
+    [MOD_SCMI_PIN_CONTROL_LIST_ASSOCIATIONS] = -1,
+    [MOD_SCMI_PIN_CONTROL_SETTINGS_GET] = -1,
     [MOD_SCMI_PIN_CONTROL_REQUEST] =
         (unsigned int)sizeof(struct scmi_pin_control_request_a2p),
     [MOD_SCMI_PIN_CONTROL_RELEASE] =
--
2.47.2
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH SCP] module/scmi_pin_control: allow variable length responses

[Dan Carpenter]

On Mon, Apr 28, 2025 at 10:30:33AM +0000, Khaled Ali Ahmed wrote:
> Good morning Dan,
> I am sending this email to add the whole attached group in the CC, also
> to list more details about the implementation of the Dynamic length
>  setting configuration.
> This takes into account the current code base, also the
> "scmi_message_validation" scmi API has been updated, and a new macro has
> been introduced in the mod_scmi.h "MOD_SCMI_PROTOCOL_DAYNAMIC_PAYLOAD_SIZE".
> 
> Thanks, Dan, in advance

Great thanks.  That works.

I'm getting some sort of crash in scmi_clock_init().  The "data" pointer is
not a valid pointer.  Apparently, it's 0x133a80.  I don't know if that's a
known issue or if it's from a mistake I made forward porting my patches.
I've disabled that for now because I only need pinctrl.

regards,
dan carpenter

Re: [PATCH SCP] module/scmi_pin_control: allow variable length responses

[Khaled Ali Ahmed]

Good morning Dan,
If the data pointer in scmi_clock_init() is corrupted, it means that the scmi_clock_config was not done in the right way, or it is not implemented. It is okay to go on without scmi_clock from pinctrl perspective, but let me know more about the test you are working on it I may help.

Thanks, Dan, so much
Best regards.

________________________________________
From: Dan Carpenter <dan.carpenter@linaro.org>
Sent: Monday, April 28, 2025 6:32 PM
To: Khaled Ali Ahmed <Khaled.AliAhmed@arm.com>
Cc: arm-scmi@vger.kernel.org <arm-scmi@vger.kernel.org>; Girish Pathak <Girish.Pathak@arm.com>
Subject: Re: [PATCH SCP] module/scmi_pin_control: allow variable length responses

On Mon, Apr 28, 2025 at 10:30:33AM +0000, Khaled Ali Ahmed wrote:
> Good morning Dan,
> I am sending this email to add the whole attached group in the CC, also
> to list more details about the implementation of the Dynamic length
>  setting configuration.
> This takes into account the current code base, also the
> "scmi_message_validation" scmi API has been updated, and a new macro has
> been introduced in the mod_scmi.h "MOD_SCMI_PROTOCOL_DAYNAMIC_PAYLOAD_SIZE".
>
> Thanks, Dan, in advance

Great thanks.  That works.

I'm getting some sort of crash in scmi_clock_init().  The "data" pointer is
not a valid pointer.  Apparently, it's 0x133a80.  I don't know if that's a
known issue or if it's from a mistake I made forward porting my patches.
I've disabled that for now because I only need pinctrl.

regards,
dan carpenter
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.