From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yu-Chien Peter Lin Date: Thu, 29 Dec 2022 15:42:35 +0000 Subject: [PATCH 1/2] lib: utils: Add fdt_add_cpu_idle_states() helper function In-Reply-To: <6f14e6af-058c-58d1-bd56-af68814b8463@sholland.org> References: <20221228191125.37584-1-samuel@sholland.org> <6f14e6af-058c-58d1-bd56-af68814b8463@sholland.org> Message-ID: List-Id: To: opensbi@lists.infradead.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Wed, Dec 28, 2022 at 11:40:23PM -0600, Samuel Holland wrote: > On 12/29/22 06:33, Yu-Chien Peter Lin wrote: > > Hi Samuel, > > > > On Wed, Dec 28, 2022 at 01:11:23PM -0600, Samuel Holland wrote: > >> Since the availability and latency properties of CPU idle states depend > >> on the specific SBI HSM implementation, it is appropriate that the idle > >> states are added to the devicetree at runtime by that implementation. > >> > >> This helper function adds a platform-provided array of idle states to > >> the devicetree, following the SBI idle state binding. It makes some > >> assumptions for simplicity, but these could be relaxed if needed. > >> > >> Signed-off-by: Samuel Holland > >> --- > >> > >> include/sbi_utils/fdt/fdt_fixup.h | 23 +++++++++ > >> lib/utils/fdt/fdt_fixup.c | 80 +++++++++++++++++++++++++++++++ > >> 2 files changed, 103 insertions(+) > >> > >> diff --git a/include/sbi_utils/fdt/fdt_fixup.h b/include/sbi_utils/fdt/fdt_fixup.h > >> index fb076ba..cab3f0f 100644 > >> --- a/include/sbi_utils/fdt/fdt_fixup.h > >> +++ b/include/sbi_utils/fdt/fdt_fixup.h > >> @@ -9,6 +9,29 @@ > >> #ifndef __FDT_FIXUP_H__ > >> #define __FDT_FIXUP_H__ > >> > >> +struct sbi_cpu_idle_state { > >> + const char *name; > >> + uint32_t suspend_param; > >> + bool local_timer_stop; > >> + uint32_t entry_latency_us; > >> + uint32_t exit_latency_us; > >> + uint32_t min_residency_us; > >> + uint32_t wakeup_latency_us; > >> +}; > >> + > >> +/** > >> + * Add CPU idle states to cpu nodes in the DT > >> + * > >> + * Add information about CPU idle states to the devicetree. This function > >> + * assumes that CPU idle states are not already present in the devicetree, and > >> + * that all CPU states are equally applicable to all CPUs. > >> + * > >> + * @param fdt: device tree blob > >> + * @param states: array of idle state descriptions, ending with empty element > >> + * @return zero on success and -ve on failure > >> + */ > >> +int fdt_add_cpu_idle_states(void *dtb, const struct sbi_cpu_idle_state *state); > >> + > >> /** > >> * Fix up the CPU node in the device tree > >> * > >> diff --git a/lib/utils/fdt/fdt_fixup.c b/lib/utils/fdt/fdt_fixup.c > >> index 41f6cbb..d9aa0b2 100644 > >> --- a/lib/utils/fdt/fdt_fixup.c > >> +++ b/lib/utils/fdt/fdt_fixup.c > >> @@ -18,6 +18,86 @@ > >> #include > >> #include > >> > >> +int fdt_add_cpu_idle_states(void *fdt, const struct sbi_cpu_idle_state *state) > >> +{ > >> + int cpu_node, cpus_node, err, idle_states_node; > >> + uint32_t count, phandle; > >> + > >> + err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + 1024); > >> + if (err < 0) > >> + return err; > >> + > >> + err = fdt_find_max_phandle(fdt, &phandle); > >> + phandle++; > >> + if (err < 0) > >> + return err; > >> + > >> + cpus_node = fdt_path_offset(fdt, "/cpus"); > >> + if (cpus_node < 0) > >> + return cpus_node; > >> + > >> + /* Create the idle-states node and its child nodes. */ > >> + idle_states_node = fdt_add_subnode(fdt, cpus_node, "idle-states"); > >> + if (idle_states_node < 0) > >> + return idle_states_node; > > > > It breaks platform final init when the ?idle-states? already exists: > > Right, that is a documented limitation. I would expect there to be a > single source of idle state information -- wherever the HSM > implementation is. So my suggestion would be to remove idle-states from > the static devicetree if you plan to use this function. If that is not > desired, could you explain your scenario in a bit more detail? Understood, then I have no more questions. Thanks, Peter Lin > > Regards, > Samuel >