drivers: firmware: scmi: add more protocols and APIs #77437

yangbolu1991 · 2024-08-23T02:04:33Z

This patch-set is to add more SCMI clock protocol APIs and power domain protocol driver.
These are essential for platform to configure clock and control power domain.
These had been verified on NXP i.MX95 platforms.

Thank you very much.

LaurentiuM1234 · 2024-08-23T12:44:59Z

thanks for your contribution! does #74920 depend on this? if not, I propose we handle these new additions after that gets merged. As things stand now we've no twister tests for SCMI (since there's no platform using it) so it's more prone to issues such as #77344

yangbolu1991 · 2024-08-26T02:29:00Z

@LaurentiuM1234 Thanks.
Logically, this is an independent change to add more APIs/protocol for SCMI.
As we know, setting clock parent and power domain control are not supported in SCMI, which should be basic features.

Actually I will send NETC support PR, which needs this to set clock and power up NETCMIX domain. That will include,

new i.mx netc drivers
imx95 platform netc support.

So, not sure if the PR will be huge making all changes together with this in one PR.
But I'm ok for either.

THanks.

GrygiriiS · 2024-09-02T14:32:53Z

Hi @LaurentiuM1234, @yangbolu1991

First of all I'd like to thank you for your tremendous work adding SCMI support in Zephyr.

Regarding this PR - it's probably would be better to split it. At least patch which fixes code formatting.
Actually CLK update vs PD management API introduce - also looks like two different functional changes (especially taking into account that SCMI POWER_DOMAIN driver probably should also be added) .

Below is just FYI.
We also worked on Zephyr SCMI implementation on raspberry pi 5 [1], but, of course, now we will migrate on already existing solution (Our solution implements SMC/HVC transport only). But what you might find interesting/useful is that we added SHELL interface and SCMI sample application which allows to test SCMI - at least manually.

[1] xen-troops#124

LaurentiuM1234 · 2024-09-09T08:08:41Z

Below is just FYI. We also worked on Zephyr SCMI implementation on raspberry pi 5 [1], but, of course, now we will migrate on already existing solution (Our solution implements SMC/HVC transport only). But what you might find interesting/useful is that we added SHELL interface and SCMI sample application which allows to test SCMI - at least manually.

[1] xen-troops#124

great to see more platforms using the SCMI support! Do you plan on also upstreaming the patches to Zephyr? I think it would be good to have another upstream platform using SCMI so we could have more coverage. Also, IMO, the sample is a very good idea since we're lacking in that department.

GrygiriiS · 2024-09-10T16:46:14Z

@LaurentiuM1234 Hope to have smth this week. I was able to port on top of current code, need to look at it one more time. Current state in https://github.com/GrygiriiS/zephyr/commits/zm_rpi5_dev_scmi/

yangbolu1991 · 2024-11-08T04:45:49Z

@LaurentiuM1234 @GrygiriiS

Since I had create PR for i.MX95 NETC feature support, which depends on this PR, I updated this to v2. Changes,

Dropped clang-format patch for existed code.

Thank you very much.

LaurentiuM1234

Sorry for the (very) long review time. Some comments below. Also, is there a particular reason you didn't implement the clock_control_set_rate() function? Should be useful for #81113.

LaurentiuM1234 · 2024-12-02T13:24:34Z

drivers/firmware/scmi/power.c

+	reply.len = sizeof(status);
+	reply.content = &status;
+
+	ret = scmi_send_message(proto, &msg, &reply);


This function is synchronous so if someone sets the ASYNC bit it will come in contradiction with it. For now, since we don't support ASYNC messages, maybe we can return an error if someone attempts to call this function with that bit set? Or perhaps force it to 0 and print a warning?

The scmi driver should be standard. I think it's not good to limit only SYNC flag here.
Actually, if ASYNC is used, I think the reply from system manager will be error, right?

we're limited to SYNC because that's the only option the support allows. Allowing people to set the ASYNC flag and still using SYNC messaging behind the scenes is not the way IMO. This deserves a comment at least.

EDIT: could add a comment to the flags field of the affected structures. People will have to look up their definitions anyways when using them, thus making sure that they know the ASYNC flag will be ignored.

LaurentiuM1234 · 2024-12-02T13:26:44Z

drivers/firmware/scmi/power.c

+#include <zephyr/drivers/firmware/scmi/power.h>
+#include <string.h>
+
+DT_SCMI_PROTOCOL_DEFINE_NODEV(DT_INST(0, arm_scmi_power), NULL);


A note on this: not exactly ideal to put this here as we'd normally have a generic power domain driver using this, but since Zephyr doesn't really support DT nodes providing multiple PDs it becomes a bit tricky. For now, I'd say it's fine to go with this.

Okay, thanks:)

LaurentiuM1234 · 2024-12-02T13:31:39Z

drivers/firmware/scmi/clk.c

+	reply.len = sizeof(status);
+	reply.content = &status;
+
+	ret = scmi_send_message(proto, &msg, &reply);


Same comment as for scmi_power_state_set().

LaurentiuM1234 · 2024-12-02T13:38:24Z

drivers/firmware/scmi/clk.c

+	return 0;
+}
+
+int scmi_clock_parent_set(struct scmi_protocol *proto, struct scmi_clock_parent_config *cfg)


Seems kinda odd to encase clock ID and parent ID in a structure here and not in scmi_clock_parent_get()? I'd suggest going with the same approach as scmi_clock_parent_get() (i.e: don't encase the parameters in a structure unless we have > 2?)

Agree, I can't understand why I encased the parameters too.
Thanks.

yangbolu1991 · 2024-12-11T03:29:55Z

Sorry for the (very) long review time. Some comments below. Also, is there a particular reason you didn't implement the clock_control_set_rate() function? Should be useful for #81113.

I think the common clock control driver is missing function like set_parent.
It's not able to directly use set_rate since we have to set parent clock before that.
So, to quick support NXP platform, I use scmi APIs directly.
Thanks.

LaurentiuM1234 · 2024-12-15T22:49:43Z

BTW, could you please also add the PD protocol to the list of supported protocols in https://github.com/zephyrproject-rtos/zephyr/blob/main/doc/hardware/arch/arm-scmi.rst? Thanks!

Added more APIs for ARM SCMI clock management protocol. - scmi_clock_rate_set - scmi_clock_parent_get - scmi_clock_parent_set Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>

Added helpers for ARM SCMI power dmomain protocol. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>

yangbolu1991 · 2024-12-16T09:38:03Z

Updated to v3. Changes include,

Returned not supported for ASYNC flag for clk/power API. I had some misunderstand before.
Converted to use clk_id/parent_id as scmi_clock_parent_set() arguments.
Added power domain protocol in zephyr scmi doc.

Thanks a lot.

LaurentiuM1234

good with me @dleach02

mmahadevan108 · 2025-01-15T16:36:44Z

@LaurentiuM1234 , would you like to add a change PR to make yourself the maintainer for ARM SCMI?

yangbolu1991 force-pushed the main_scmi branch from a38a026 to d99a945 Compare August 23, 2024 04:32

LaurentiuM1234 self-requested a review August 23, 2024 12:42

GrygiriiS mentioned this pull request Sep 12, 2024

firmware: scmi: add base proto, scmi sample, smc/hvc transport and reset proto #78293

Closed

yangbolu1991 mentioned this pull request Nov 8, 2024

Enable NETC support for i.MX95 EKV M7 #81113

Open

yangbolu1991 force-pushed the main_scmi branch from d99a945 to 2b21c18 Compare November 8, 2024 04:42

LaurentiuM1234 reviewed Dec 2, 2024

View reviewed changes

dleach02 self-assigned this Dec 4, 2024

yangbolu1991 added 2 commits December 16, 2024 17:09

drivers: firmware: scmi: add more APIs of clock management protocol

28de671

Added more APIs for ARM SCMI clock management protocol. - scmi_clock_rate_set - scmi_clock_parent_get - scmi_clock_parent_set Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>

drivers: firmware: scmi: add power domain protocol

adb6606

Added helpers for ARM SCMI power dmomain protocol. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>

yangbolu1991 force-pushed the main_scmi branch from 2b21c18 to adb6606 Compare December 16, 2024 09:34

LaurentiuM1234 approved these changes Dec 16, 2024

View reviewed changes

mmahadevan108 requested review from mmahadevan108, decsny and wearyzen January 15, 2025 15:58

mmahadevan108 approved these changes Jan 15, 2025

View reviewed changes

dleach02 approved these changes Jan 15, 2025

View reviewed changes

kartben merged commit 7c57fec into zephyrproject-rtos:main Jan 15, 2025
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: firmware: scmi: add more protocols and APIs #77437

drivers: firmware: scmi: add more protocols and APIs #77437

yangbolu1991 commented Aug 23, 2024

LaurentiuM1234 commented Aug 23, 2024

yangbolu1991 commented Aug 26, 2024

GrygiriiS commented Sep 2, 2024

LaurentiuM1234 commented Sep 9, 2024

GrygiriiS commented Sep 10, 2024

yangbolu1991 commented Nov 8, 2024

LaurentiuM1234 left a comment

LaurentiuM1234 Dec 2, 2024

yangbolu1991 Dec 11, 2024

LaurentiuM1234 Dec 15, 2024 •

edited

Loading

LaurentiuM1234 Dec 2, 2024

yangbolu1991 Dec 11, 2024

LaurentiuM1234 Dec 2, 2024

LaurentiuM1234 Dec 2, 2024

yangbolu1991 Dec 11, 2024

yangbolu1991 commented Dec 11, 2024

LaurentiuM1234 commented Dec 15, 2024

yangbolu1991 commented Dec 16, 2024

LaurentiuM1234 left a comment

mmahadevan108 commented Jan 15, 2025

drivers: firmware: scmi: add more protocols and APIs #77437

drivers: firmware: scmi: add more protocols and APIs #77437

Conversation

yangbolu1991 commented Aug 23, 2024

LaurentiuM1234 commented Aug 23, 2024

yangbolu1991 commented Aug 26, 2024

GrygiriiS commented Sep 2, 2024

LaurentiuM1234 commented Sep 9, 2024

GrygiriiS commented Sep 10, 2024

yangbolu1991 commented Nov 8, 2024

LaurentiuM1234 left a comment

Choose a reason for hiding this comment

LaurentiuM1234 Dec 2, 2024

Choose a reason for hiding this comment

yangbolu1991 Dec 11, 2024

Choose a reason for hiding this comment

LaurentiuM1234 Dec 15, 2024 • edited Loading

Choose a reason for hiding this comment

LaurentiuM1234 Dec 2, 2024

Choose a reason for hiding this comment

yangbolu1991 Dec 11, 2024

Choose a reason for hiding this comment

LaurentiuM1234 Dec 2, 2024

Choose a reason for hiding this comment

LaurentiuM1234 Dec 2, 2024

Choose a reason for hiding this comment

yangbolu1991 Dec 11, 2024

Choose a reason for hiding this comment

yangbolu1991 commented Dec 11, 2024

LaurentiuM1234 commented Dec 15, 2024

yangbolu1991 commented Dec 16, 2024

LaurentiuM1234 left a comment

Choose a reason for hiding this comment

mmahadevan108 commented Jan 15, 2025

LaurentiuM1234 Dec 15, 2024 •

edited

Loading