* Re: [PATCH 5.4] Common clock: To list active consumers of clocks [not found] <CAEXpiVQihEadxsNodarz2-wxSAipfpzEaA8zKpnozszC+weYTQ@mail.gmail.com> @ 2022-06-10 19:40 ` Stephen Boyd [not found] ` <CAOUcgRfB-r3aERYeLumExgpTVzsDsBuyOWT+nCJ_OfOv1g0L9g@mail.gmail.com> ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Stephen Boyd @ 2022-06-10 19:40 UTC (permalink / raw) To: Vishal Badole, linux-clk, linux-kernel, mturquette Cc: chinmoyghosh2001, mintupatel89 Why "5.4" in the subject? Quoting Vishal Badole (2022-05-31 11:28:35) > This feature lists the name of clocks and their consumer devices. This > debug feature can be used to check the active clocks and the devices who > have enabled them. > > for example: > debian@beaglebone:~$ cat /sys/kernel/debug/clk/clk_devices_name > clock_name devices_name > ---------------- > ------------------------------------------------------------------------- > l4-wkup-clkctrl:0008:0 44e07000.target-module > l4ls-clkctrl:0074:0 4804c000.target-module > l4ls-clkctrl:0058:0 48311fe0.target-module > l4-rtc-clkctrl:0000:0 44e3e074.target-module > clk_32768_ck > 44e3e000.rtc > l4ls-clkctrl:00d8:0 480c8000.target-module > cpsw-125mhz-clkctrl:0014:0 4a101200.target-module > > Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> > Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > Signed-off-by: Mintu Patel <mintupatel89@gmail.com> > Signed-off-by: Vimal Kumar <vimal.kumar32@gmail.com> Where are the Co-developed-by tags? Also, your SoB should be last. > --- > drivers/clk/Kconfig | 8 ++++ > drivers/clk/clk.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 127 insertions(+), 1 deletion(-) The patch is malformed. Not sure what's happening with your MUA. > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index c44247d..549cdda 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -23,6 +23,14 @@ config COMMON_CLK > menu "Common Clock Framework" > depends on COMMON_CLK > > +config DEBUG_CLK_CONSUMER > + bool "Debug feature to list clocks and their active consumers" Don't think we need a new config for this. Just add it to the existing CONFIG_DEBUGFS area. > + depends on DEBUG_FS && COMMON_CLK > + help > + Clock consumer debug feature supports for clock debugging. Chose y > + to get debug entry in file system to list clocks and their active > + consumer devices. > + > config COMMON_CLK_WM831X > tristate "Clock driver for WM831x/2x PMICs" > depends on MFD_WM831X > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 13332f8..dccbd35 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -105,6 +105,84 @@ struct clk { > struct hlist_node clks_node; > }; > > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > +/*Linked List Node*/ > +struct clk_dev_list { > + struct list_head list; > + const char *clk_name; > + const char *dev_name; > +}; > + > +/*Declare and init the head node of the linked list*/ > +LIST_HEAD(head_node); > + > +static void clk_dev_entry(struct clk *clk_ptr) > +{ > + struct clk_dev_list *new_node_ptr = NULL; > + struct clk_dev_list *temp_node_ptr = NULL; > + int repeat_count = 0; > + static bool is_first_node; > + const char *clk_name_ptr = NULL; > + const char *dev_name_ptr = NULL; > + > + if (clk_ptr->dev) { > + dev_name_ptr = dev_name(clk_ptr->dev); > + > + clk_name_ptr = clk_ptr->core->name; > + > + if (is_first_node) { > + /* Iterate the list to check duplicate entry */ > + list_for_each_entry(temp_node_ptr, &head_node, list) { > + if (temp_node_ptr->clk_name == clk_name_ptr && > + temp_node_ptr->dev_name == dev_name_ptr) { > + repeat_count++; > + break; > + } > + } > + } > + > + is_first_node = 1; > + > + if (!repeat_count && clk_ptr->core->enable_count) { > + /*Creating Node*/ > + new_node_ptr = kmalloc(sizeof(*new_node_ptr), > + GFP_KERNEL); > + if (!new_node_ptr) > + return; > + > + /*Assgin the data that is received*/ Typo in Assign. > + new_node_ptr->clk_name = clk_name_ptr; > + new_node_ptr->dev_name = dev_name_ptr; > + > + /*Init the list within the struct*/ > + INIT_LIST_HEAD(&new_node_ptr->list); > + > + /*Add Node to Linked List*/ > + list_add_tail(&new_node_ptr->list, &head_node); > + } > + } > +} > + > +/* Function to remove the clk and device entry */ > +static void clk_dev_dentry(struct clk *clk) > +{ > + struct clk_dev_list *temp_node_ptr = NULL; > + struct clk_dev_list *cur_node_ptr = NULL; > + > + if (clk->dev) { > + /* Go through the list and free the memory */ > + list_for_each_entry_safe(cur_node_ptr, temp_node_ptr, > + &head_node, list) { > + if (cur_node_ptr->clk_name == clk->core->name && > + cur_node_ptr->dev_name == dev_name(clk->dev)) { > + list_del(&cur_node_ptr->list); > + kfree(cur_node_ptr); > + } > + } > + } > +} > +#endif > + > /*** runtime pm ***/ > static int clk_pm_runtime_get(struct clk_core *core) > { > @@ -1020,6 +1098,9 @@ void clk_disable(struct clk *clk) > return; > > clk_core_disable_lock(clk->core); > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > + clk_dev_dentry(clk); > +#endif > } > EXPORT_SYMBOL_GPL(clk_disable); > > @@ -1181,10 +1262,21 @@ EXPORT_SYMBOL_GPL(clk_restore_context); > */ > int clk_enable(struct clk *clk) > { > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > + int ret = 0; > +#endif > if (!clk) > return 0; > > +#ifndef CONFIG_DEBUG_CLK_CONSUMER > return clk_core_enable_lock(clk->core); > +#else > + ret = clk_core_enable_lock(clk->core); > + if (!ret) > + clk_dev_entry(clk); > + > + return ret; > +#endif Not sure what this is doing. > } > EXPORT_SYMBOL_GPL(clk_enable); > > @@ -2986,6 +3078,29 @@ static void clk_dump_one(struct seq_file *s, struct > clk_core *c, int level) > clk_core_get_scaled_duty_cycle(c, 100000)); > } > > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > +static int clk_devices_show(struct seq_file *s, void *data) > +{ > + struct clk_dev_list *clk_dev_node; > + > + seq_puts(s, " clock_name > devices_name\n"); > + seq_puts(s, > "-------------------------------------------------------------------------\n"); > + > + clk_prepare_lock(); > + > + /*Traversing Linked List and Print its Members*/ > + list_for_each_entry(clk_dev_node, &head_node, list) { It's hard to read but we already have a list of all clk consumers for a clk_hw pointer, see clk_core_link_consumer(). And we stash the device consuming it with clk_hw_create_clk(). That should be sufficient. > + seq_printf(s, "%35s %35s\n", clk_dev_node->clk_name, > + clk_dev_node->dev_name); > + } > + > + clk_prepare_unlock(); > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(clk_devices); > +#endif > + > static void clk_dump_subtree(struct seq_file *s, struct clk_core *c, int > level) > { > struct clk_core *child; > @@ -3256,7 +3371,10 @@ static int __init clk_debug_init(void) > &clk_summary_fops); > debugfs_create_file("clk_orphan_dump", 0444, rootdir, &orphan_list, > &clk_dump_fops); > - > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > + debugfs_create_file("clk_devices_name", 0444, rootdir, NULL, Call it 'clk_consumers' please. > + &clk_devices_fops); ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CAOUcgRfB-r3aERYeLumExgpTVzsDsBuyOWT+nCJ_OfOv1g0L9g@mail.gmail.com>]
* Re: [PATCH 5.4] Common clock: To list active consumers of clocks [not found] ` <CAOUcgRfB-r3aERYeLumExgpTVzsDsBuyOWT+nCJ_OfOv1g0L9g@mail.gmail.com> @ 2022-06-15 19:23 ` <Vishal Badole> 0 siblings, 0 replies; 17+ messages in thread From: <Vishal Badole> @ 2022-06-15 19:23 UTC (permalink / raw) To: Stephen Boyd Cc: linux-clk, linux-kernel, mturquette, mintu patel, chinmoy ghosh Hi Stephen, Thanks for reviewing and giving your valueable review points for this patch. Shortly I will attach a revised patch based on your review points. Thanks, Vishal On Tue, Jun 14, 2022 at 11:25:01PM +0530, chinmoy ghosh wrote: > Agreed here . > I feel , if dev_id <https://elixir.bootlin.com/linux/latest/C/ident/dev_id> > is not from dts property then dont think we can easily find out consumers > from clk_hw_create_clk.Depends on how the legacy driver maintained the BSP > code. > Check the next . > > Thanks > Chinmoy > > On Sat, Jun 11, 2022 at 1:10 AM Stephen Boyd <sboyd@kernel.org> wrote: > > > Why "5.4" in the subject? > > > > Quoting Vishal Badole (2022-05-31 11:28:35) > > > This feature lists the name of clocks and their consumer devices. This > > > debug feature can be used to check the active clocks and the devices who > > > have enabled them. > > > > > > for example: > > > debian@beaglebone:~$ cat /sys/kernel/debug/clk/clk_devices_name > > > clock_name devices_name > > > ---------------- > > > ------------------------------------------------------------------------- > > > l4-wkup-clkctrl:0008:0 44e07000.target-module > > > l4ls-clkctrl:0074:0 > > 4804c000.target-module > > > l4ls-clkctrl:0058:0 > > 48311fe0.target-module > > > l4-rtc-clkctrl:0000:0 > > 44e3e074.target-module > > > clk_32768_ck > > > 44e3e000.rtc > > > l4ls-clkctrl:00d8:0 > > 480c8000.target-module > > > cpsw-125mhz-clkctrl:0014:0 4a101200.target-module > > > > > > Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> > > > Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > > > Signed-off-by: Mintu Patel <mintupatel89@gmail.com> > > > Signed-off-by: Vimal Kumar <vimal.kumar32@gmail.com> > > > > Where are the Co-developed-by tags? Also, your SoB should be last. > > > > > --- > > > drivers/clk/Kconfig | 8 ++++ > > > drivers/clk/clk.c | 120 > > +++++++++++++++++++++++++++++++++++++++++++++++++++- > > > 2 files changed, 127 insertions(+), 1 deletion(-) > > > > The patch is malformed. Not sure what's happening with your MUA. > > > > > > > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > > > index c44247d..549cdda 100644 > > > --- a/drivers/clk/Kconfig > > > +++ b/drivers/clk/Kconfig > > > @@ -23,6 +23,14 @@ config COMMON_CLK > > > menu "Common Clock Framework" > > > depends on COMMON_CLK > > > > > > +config DEBUG_CLK_CONSUMER > > > + bool "Debug feature to list clocks and their active consumers" > > > > Don't think we need a new config for this. Just add it to the existing > > CONFIG_DEBUGFS area. > > > > > + depends on DEBUG_FS && COMMON_CLK > > > + help > > > + Clock consumer debug feature supports for clock debugging. Chose y > > > + to get debug entry in file system to list clocks and their active > > > + consumer devices. > > > + > > > config COMMON_CLK_WM831X > > > tristate "Clock driver for WM831x/2x PMICs" > > > depends on MFD_WM831X > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > index 13332f8..dccbd35 100644 > > > --- a/drivers/clk/clk.c > > > +++ b/drivers/clk/clk.c > > > @@ -105,6 +105,84 @@ struct clk { > > > struct hlist_node clks_node; > > > }; > > > > > > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > > > +/*Linked List Node*/ > > > +struct clk_dev_list { > > > + struct list_head list; > > > + const char *clk_name; > > > + const char *dev_name; > > > +}; > > > + > > > +/*Declare and init the head node of the linked list*/ > > > +LIST_HEAD(head_node); > > > + > > > +static void clk_dev_entry(struct clk *clk_ptr) > > > +{ > > > + struct clk_dev_list *new_node_ptr = NULL; > > > + struct clk_dev_list *temp_node_ptr = NULL; > > > + int repeat_count = 0; > > > + static bool is_first_node; > > > + const char *clk_name_ptr = NULL; > > > + const char *dev_name_ptr = NULL; > > > + > > > + if (clk_ptr->dev) { > > > + dev_name_ptr = dev_name(clk_ptr->dev); > > > + > > > + clk_name_ptr = clk_ptr->core->name; > > > + > > > + if (is_first_node) { > > > + /* Iterate the list to check duplicate entry */ > > > + list_for_each_entry(temp_node_ptr, &head_node, list) { > > > + if (temp_node_ptr->clk_name == clk_name_ptr && > > > + temp_node_ptr->dev_name == dev_name_ptr) { > > > + repeat_count++; > > > + break; > > > + } > > > + } > > > + } > > > + > > > + is_first_node = 1; > > > + > > > + if (!repeat_count && clk_ptr->core->enable_count) { > > > + /*Creating Node*/ > > > + new_node_ptr = kmalloc(sizeof(*new_node_ptr), > > > + GFP_KERNEL); > > > + if (!new_node_ptr) > > > + return; > > > + > > > + /*Assgin the data that is received*/ > > > > Typo in Assign. > > > > > + new_node_ptr->clk_name = clk_name_ptr; > > > + new_node_ptr->dev_name = dev_name_ptr; > > > + > > > + /*Init the list within the struct*/ > > > + INIT_LIST_HEAD(&new_node_ptr->list); > > > + > > > + /*Add Node to Linked List*/ > > > + list_add_tail(&new_node_ptr->list, &head_node); > > > + } > > > + } > > > +} > > > + > > > +/* Function to remove the clk and device entry */ > > > +static void clk_dev_dentry(struct clk *clk) > > > +{ > > > + struct clk_dev_list *temp_node_ptr = NULL; > > > + struct clk_dev_list *cur_node_ptr = NULL; > > > + > > > + if (clk->dev) { > > > + /* Go through the list and free the memory */ > > > + list_for_each_entry_safe(cur_node_ptr, temp_node_ptr, > > > + &head_node, list) { > > > + if (cur_node_ptr->clk_name == clk->core->name && > > > + cur_node_ptr->dev_name == dev_name(clk->dev)) { > > > + list_del(&cur_node_ptr->list); > > > + kfree(cur_node_ptr); > > > + } > > > + } > > > + } > > > +} > > > +#endif > > > + > > > /*** runtime pm ***/ > > > static int clk_pm_runtime_get(struct clk_core *core) > > > { > > > @@ -1020,6 +1098,9 @@ void clk_disable(struct clk *clk) > > > return; > > > > > > clk_core_disable_lock(clk->core); > > > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > > > + clk_dev_dentry(clk); > > > +#endif > > > } > > > EXPORT_SYMBOL_GPL(clk_disable); > > > > > > @@ -1181,10 +1262,21 @@ EXPORT_SYMBOL_GPL(clk_restore_context); > > > */ > > > int clk_enable(struct clk *clk) > > > { > > > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > > > + int ret = 0; > > > +#endif > > > if (!clk) > > > return 0; > > > > > > +#ifndef CONFIG_DEBUG_CLK_CONSUMER > > > return clk_core_enable_lock(clk->core); > > > +#else > > > + ret = clk_core_enable_lock(clk->core); > > > + if (!ret) > > > + clk_dev_entry(clk); > > > + > > > + return ret; > > > +#endif > > > > Not sure what this is doing. > > > > > } > > > EXPORT_SYMBOL_GPL(clk_enable); > > > > > > @@ -2986,6 +3078,29 @@ static void clk_dump_one(struct seq_file *s, > > struct > > > clk_core *c, int level) > > > clk_core_get_scaled_duty_cycle(c, 100000)); > > > } > > > > > > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > > > +static int clk_devices_show(struct seq_file *s, void *data) > > > +{ > > > + struct clk_dev_list *clk_dev_node; > > > + > > > + seq_puts(s, " clock_name > > > devices_name\n"); > > > + seq_puts(s, > > > > > "-------------------------------------------------------------------------\n"); > > > + > > > + clk_prepare_lock(); > > > + > > > + /*Traversing Linked List and Print its Members*/ > > > + list_for_each_entry(clk_dev_node, &head_node, list) { > > > > It's hard to read but we already have a list of all clk consumers for a > > clk_hw pointer, see clk_core_link_consumer(). And we stash the device > > consuming it with clk_hw_create_clk(). That should be sufficient. > > > > > > > + seq_printf(s, "%35s %35s\n", clk_dev_node->clk_name, > > > + clk_dev_node->dev_name); > > > + } > > > + > > > + clk_prepare_unlock(); > > > + > > > + return 0; > > > +} > > > +DEFINE_SHOW_ATTRIBUTE(clk_devices); > > > +#endif > > > + > > > static void clk_dump_subtree(struct seq_file *s, struct clk_core *c, int > > > level) > > > { > > > struct clk_core *child; > > > @@ -3256,7 +3371,10 @@ static int __init clk_debug_init(void) > > > &clk_summary_fops); > > > debugfs_create_file("clk_orphan_dump", 0444, rootdir, &orphan_list, > > > &clk_dump_fops); > > > - > > > +#ifdef CONFIG_DEBUG_CLK_CONSUMER > > > + debugfs_create_file("clk_devices_name", 0444, rootdir, NULL, > > > > Call it 'clk_consumers' please. > > > > > + &clk_devices_fops); > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Common clock: To list active consumers of clocks 2022-06-10 19:40 ` [PATCH 5.4] Common clock: To list active consumers of clocks Stephen Boyd [not found] ` <CAOUcgRfB-r3aERYeLumExgpTVzsDsBuyOWT+nCJ_OfOv1g0L9g@mail.gmail.com> @ 2022-06-22 16:32 ` <Vishal Badole> 2022-06-22 19:37 ` Randy Dunlap 2022-06-22 17:02 ` <Vishal Badole> 2 siblings, 1 reply; 17+ messages in thread From: <Vishal Badole> @ 2022-06-22 16:32 UTC (permalink / raw) To: Stephen Boyd, mturquette, inux-clk, linux-kernel Cc: chinmoyghosh2001, mintupatel89 From eba241016ea868b841473ba73ece16173a6b5aee Mon Sep 17 00:00:00 2001 From: Vishal Badole <badolevishal1116@gmail.com> Date: Tue, 31 May 2022 21:23:34 +0530 Subject: [PATCH] Common clock: To list active consumers of clocks This feature lists the name of clocks and their consumer devices. Using this feature user can easily check which device is using a perticular clock. for example: debian@beaglebone:~$ cat /sys/kernel/debug/clk/clk_devices_name clock_name devices_name ------------------------------------------------------------------------- l4-wkup-clkctrl:0008:0 44e07000.target-module l4ls-clkctrl:0074:0 4804c000.target-module l4ls-clkctrl:0058:0 48311fe0.target-module l4-rtc-clkctrl:0000:0 44e3e074.target-module clk_32768_ck 44e3e000.rtc l4ls-clkctrl:00d8:0 480c8000.target-module cpsw-125mhz-clkctrl:0014:0 4a101200.target-module Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> Signed-off-by: Mintu Patel <mintupatel89@gmail.com> Signed-off-by: Vimal Kumar <vimal.kumar32@gmail.com> --- drivers/clk/Kconfig | 8 ++++ drivers/clk/clk.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index c44247d..549cdda 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -23,6 +23,14 @@ config COMMON_CLK menu "Common Clock Framework" depends on COMMON_CLK +config DEBUG_CLK_CONSUMER + bool "Debug feature to list clocks and their active consumers" + depends on DEBUG_FS && COMMON_CLK + help + Clock consumer debug feature supports for clock debugging. Chose y + to get debug entry in file system to list clocks and their active + consumer devices. + config COMMON_CLK_WM831X tristate "Clock driver for WM831x/2x PMICs" depends on MFD_WM831X diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 13332f8..dccbd35 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -105,6 +105,84 @@ struct clk { struct hlist_node clks_node; }; +#ifdef CONFIG_DEBUG_CLK_CONSUMER +/*Linked List Node*/ +struct clk_dev_list { + struct list_head list; + const char *clk_name; + const char *dev_name; +}; + +/*Declare and init the head node of the linked list*/ +LIST_HEAD(head_node); + +static void clk_dev_entry(struct clk *clk_ptr) +{ + struct clk_dev_list *new_node_ptr = NULL; + struct clk_dev_list *temp_node_ptr = NULL; + int repeat_count = 0; + static bool is_first_node; + const char *clk_name_ptr = NULL; + const char *dev_name_ptr = NULL; + + if (clk_ptr->dev) { + dev_name_ptr = dev_name(clk_ptr->dev); + + clk_name_ptr = clk_ptr->core->name; + + if (is_first_node) { + /* Iterate the list to check duplicate entry */ + list_for_each_entry(temp_node_ptr, &head_node, list) { + if (temp_node_ptr->clk_name == clk_name_ptr && + temp_node_ptr->dev_name == dev_name_ptr) { + repeat_count++; + break; + } + } + } + + is_first_node = 1; + + if (!repeat_count && clk_ptr->core->enable_count) { + /*Creating Node*/ + new_node_ptr = kmalloc(sizeof(*new_node_ptr), + GFP_KERNEL); + if (!new_node_ptr) + return; + + /*Assgin the data that is received*/ + new_node_ptr->clk_name = clk_name_ptr; + new_node_ptr->dev_name = dev_name_ptr; + + /*Init the list within the struct*/ + INIT_LIST_HEAD(&new_node_ptr->list); + + /*Add Node to Linked List*/ + list_add_tail(&new_node_ptr->list, &head_node); + } + } +} + +/* Function to remove the clk and device entry */ +static void clk_dev_dentry(struct clk *clk) +{ + struct clk_dev_list *temp_node_ptr = NULL; + struct clk_dev_list *cur_node_ptr = NULL; + + if (clk->dev) { + /* Go through the list and free the memory */ + list_for_each_entry_safe(cur_node_ptr, temp_node_ptr, + &head_node, list) { + if (cur_node_ptr->clk_name == clk->core->name && + cur_node_ptr->dev_name == dev_name(clk->dev)) { + list_del(&cur_node_ptr->list); + kfree(cur_node_ptr); + } + } + } +} +#endif + /*** runtime pm ***/ static int clk_pm_runtime_get(struct clk_core *core) { @@ -1020,6 +1098,9 @@ void clk_disable(struct clk *clk) return; clk_core_disable_lock(clk->core); +#ifdef CONFIG_DEBUG_CLK_CONSUMER + clk_dev_dentry(clk); +#endif } EXPORT_SYMBOL_GPL(clk_disable); @@ -1181,10 +1262,21 @@ EXPORT_SYMBOL_GPL(clk_restore_context); */ int clk_enable(struct clk *clk) { +#ifdef CONFIG_DEBUG_CLK_CONSUMER + int ret = 0; +#endif if (!clk) return 0; +#ifndef CONFIG_DEBUG_CLK_CONSUMER return clk_core_enable_lock(clk->core); +#else + ret = clk_core_enable_lock(clk->core); + if (!ret) + clk_dev_entry(clk); + + return ret; +#endif } EXPORT_SYMBOL_GPL(clk_enable); @@ -2986,6 +3078,29 @@ static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) clk_core_get_scaled_duty_cycle(c, 100000)); } +#ifdef CONFIG_DEBUG_CLK_CONSUMER +static int clk_devices_show(struct seq_file *s, void *data) +{ + struct clk_dev_list *clk_dev_node; + + seq_puts(s, " clock_name devices_name\n"); + seq_puts(s, "-------------------------------------------------------------------------\n"); + + clk_prepare_lock(); + + /*Traversing Linked List and Print its Members*/ + list_for_each_entry(clk_dev_node, &head_node, list) { + seq_printf(s, "%35s %35s\n", clk_dev_node->clk_name, + clk_dev_node->dev_name); + } + + clk_prepare_unlock(); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(clk_devices); +#endif + static void clk_dump_subtree(struct seq_file *s, struct clk_core *c, int level) { struct clk_core *child; @@ -3256,7 +3371,10 @@ static int __init clk_debug_init(void) &clk_summary_fops); debugfs_create_file("clk_orphan_dump", 0444, rootdir, &orphan_list, &clk_dump_fops); - +#ifdef CONFIG_DEBUG_CLK_CONSUMER + debugfs_create_file("clk_devices_name", 0444, rootdir, NULL, + &clk_devices_fops); +#endif mutex_lock(&clk_debug_lock); hlist_for_each_entry(core, &clk_debug_list, debug_node) clk_debug_create_one(core, rootdir); -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] Common clock: To list active consumers of clocks 2022-06-22 16:32 ` [PATCH] " <Vishal Badole> @ 2022-06-22 19:37 ` Randy Dunlap 0 siblings, 0 replies; 17+ messages in thread From: Randy Dunlap @ 2022-06-22 19:37 UTC (permalink / raw) To: Stephen Boyd, mturquette, inux-clk, linux-kernel, badolevishal1116 Cc: chinmoyghosh2001, mintupatel89 On 6/22/22 09:32, <Vishal Badole> wrote: > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index c44247d..549cdda 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -23,6 +23,14 @@ config COMMON_CLK > menu "Common Clock Framework" > depends on COMMON_CLK > > +config DEBUG_CLK_CONSUMER > + bool "Debug feature to list clocks and their active consumers" > + depends on DEBUG_FS && COMMON_CLK > + help > + Clock consumer debug feature supports for clock debugging. Chose y Choose y > + to get debug entry in file system to list clocks and their active > + consumer devices. -- ~Randy ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Common clock: To list active consumers of clocks 2022-06-10 19:40 ` [PATCH 5.4] Common clock: To list active consumers of clocks Stephen Boyd [not found] ` <CAOUcgRfB-r3aERYeLumExgpTVzsDsBuyOWT+nCJ_OfOv1g0L9g@mail.gmail.com> 2022-06-22 16:32 ` [PATCH] " <Vishal Badole> @ 2022-06-22 17:02 ` <Vishal Badole> [not found] ` <20220624010550.582BBC341C7@smtp.kernel.org> 2 siblings, 1 reply; 17+ messages in thread From: <Vishal Badole> @ 2022-06-22 17:02 UTC (permalink / raw) To: Stephen Boyd, mturquette, linux-clk, linux-kernel Cc: chinmoyghosh2001, mintupatel89 From f2e9d78bd0f135206fbfbf2e0178a5782b972939 Mon Sep 17 00:00:00 2001 From: Vishal Badole <badolevishal1116@gmail.com> Date: Tue, 21 Jun 2022 09:55:51 +0530 Subject: [PATCH] Common clock: To list active consumers of clocks This feature lists the name of clocks and their consumer devices. Using this feature user can easily check which device is using a perticular clock. Consumers without dev_id are listed as no_dev_id. Co-developed-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> Co-developed-by: Mintu Patel <mintupatel89@gmail.com> Signed-off-by: Mintu Patel <mintupatel89@gmail.com> Acked-by: Vimal Kumar <vimal.kumar32@gmail.com> Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> --- drivers/clk/clk.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index ed11918..b191009 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3018,6 +3018,63 @@ static int clk_summary_show(struct seq_file *s, void *data) } DEFINE_SHOW_ATTRIBUTE(clk_summary); +static void clk_consumer_show_one(struct seq_file *s, struct clk_core *core, int level) +{ + struct clk *clk_user; + const char *consumer; + + hlist_for_each_entry(clk_user, &core->clks, clks_node) { + if (!clk_user->dev_id) + consumer = "no_dev_id"; + else + consumer = clk_user->dev_id; + + seq_printf(s, "%*s%-*s %30s %5d %7d ", + level * 3 + 1, "", + 30 - level * 3, clk_user->core->name, consumer, + clk_user->core->enable_count, clk_user->core->prepare_count); + + if (clk_user->core->ops->is_enabled) + seq_printf(s, " %8c\n", clk_core_is_enabled(clk_user->core) ? 'Y' : 'N'); + else if (!clk_user->core->ops->enable) + seq_printf(s, " %8c\n", 'Y'); + else + seq_printf(s, " %8c\n", '?'); + } +} + +static void clk_consumer_show_subtree(struct seq_file *s, struct clk_core *c, int level) +{ + struct clk_core *child; + + clk_consumer_show_one(s, c, level); + + hlist_for_each_entry(child, &c->children, child_node) + clk_consumer_show_subtree(s, child, level + 1); +} + +static int clk_consumer_show(struct seq_file *s, void *data) +{ + struct clk_core *c; + struct hlist_head **lists = (struct hlist_head **)s->private; + + seq_puts(s, " enable prepare hardware\n"); + seq_puts(s, " clock consumer count count enable\n"); + seq_puts(s, "-----------------------------------------------------------------------------------------\n"); + clk_prepare_lock(); + + /*Traversing Linked List to print clock consumer*/ + + for (; *lists; lists++) + hlist_for_each_entry(c, *lists, child_node) + clk_consumer_show_subtree(s, c, 0); + + clk_prepare_unlock(); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(clk_consumer); + static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) { int phase; @@ -3437,6 +3494,8 @@ static int __init clk_debug_init(void) &clk_summary_fops); debugfs_create_file("clk_orphan_dump", 0444, rootdir, &orphan_list, &clk_dump_fops); + debugfs_create_file("clk_consumer", 0444, rootdir, &all_lists, + &clk_consumer_fops); mutex_lock(&clk_debug_lock); hlist_for_each_entry(core, &clk_debug_list, debug_node) -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <20220624010550.582BBC341C7@smtp.kernel.org>]
* Re: [PATCH] Common clock: To list active consumers of clocks [not found] ` <20220624010550.582BBC341C7@smtp.kernel.org> @ 2022-06-26 18:25 ` <Vishal Badole> 2022-08-02 22:49 ` Elliott, Robert (Servers) 2022-06-28 5:08 ` [PATCH v3] Common clock: To " Vishal Badole 2022-08-02 18:09 ` Vishal Badole 2 siblings, 1 reply; 17+ messages in thread From: <Vishal Badole> @ 2022-06-26 18:25 UTC (permalink / raw) To: Stephen Boyd, mturquette, inux-clk, linux-kernel Cc: chinmoyghosh2001, mintupatel89, vimal.kumar32 On Thu, Jun 23, 2022 at 06:05:48PM -0700, Stephen Boyd wrote: > Quoting <Vishal Badole> (2022-06-22 10:02:20) > > > > From f2e9d78bd0f135206fbfbf2e0178a5782b972939 Mon Sep 17 00:00:00 2001 > > From: Vishal Badole <badolevishal1116@gmail.com> > > Date: Tue, 21 Jun 2022 09:55:51 +0530 > > Subject: [PATCH] Common clock: To list active consumers of clocks > > That patch is still malformed. Please try again. Also, stop sending it > as a reply-to the previous patch. Thanks! > We have applied and checked the patch on top of the mainline and not able to see that it is malformed. We will share revised patch using git send mail. > > > > This feature lists the name of clocks and their consumer devices. > > Using this feature user can easily check which device is using a > > perticular clock. Consumers without dev_id are listed as no_dev_id. > > > > Co-developed-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > > Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > > Co-developed-by: Mintu Patel <mintupatel89@gmail.com> > > Signed-off-by: Mintu Patel <mintupatel89@gmail.com> > > Acked-by: Vimal Kumar <vimal.kumar32@gmail.com> > > The acked-by tag is largely for maintainer use. Please remove it. See > Documentation/process/5.Posting.rst for more info. > Agreed, We will update this in the revised patch. > > Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> > > --- > > drivers/clk/clk.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 59 insertions(+) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index ed11918..b191009 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -3018,6 +3018,63 @@ static int clk_summary_show(struct seq_file *s, void *data) > > } > > DEFINE_SHOW_ATTRIBUTE(clk_summary); > > > > +static void clk_consumer_show_one(struct seq_file *s, struct clk_core *core, int level) > > +{ > > + struct clk *clk_user; > > + const char *consumer; > > + > > + hlist_for_each_entry(clk_user, &core->clks, clks_node) { > > + if (!clk_user->dev_id) > > + consumer = "no_dev_id"; > > + else > > + consumer = clk_user->dev_id; > > We can just pass NULL if there isn't a dev_id and get a nice "(NULL)" > print instead of "no_dev_id". > Agreed, we will replace "no_dev_id" with "deviceless" in the revised patch. > > + > > + seq_printf(s, "%*s%-*s %30s %5d %7d ", > > + level * 3 + 1, "", > > + 30 - level * 3, clk_user->core->name, consumer, > > + clk_user->core->enable_count, clk_user->core->prepare_count); > > It would be great to not print the core enable count here and instead > have two levels of enable accounting so we can print the per-user count. > Basically, one in the 'struct clk_core' and one in the 'struct clk'. If > that isn't done then this print is going to duplicate the count for > every 'struct clk' and be meaningless. > We will add enable count member to struct clock to represent per user count and will print that one along with clock and consumer name > > + > > + if (clk_user->core->ops->is_enabled) > > + seq_printf(s, " %8c\n", clk_core_is_enabled(clk_user->core) ? 'Y' : 'N'); > > + else if (!clk_user->core->ops->enable) > > + seq_printf(s, " %8c\n", 'Y'); > > + else > > + seq_printf(s, " %8c\n", '?'); > > I don't think we need any of these prints. They're already covered in > the summary. And the summary should be able to print the users. See > regulator_summary_show_subtree() for inspiration. It looks like they > print "deviceless" for the "no_dev_id" case so maybe just use that > instead of NULL print. > We will remove above prints in the revised patch. We are facing indentation issue whle printing consumer in summary as given below enable prepare protect duty hardware per-user clock count count count rateccuracy phase cycle enable consumer count clk_mcasp0_fixed 0 0 0 24576000 0 50000 Y deviceless 0 In this case it will be better to have a separate debugfs entry as clK_consumer to print clock, consumer and per-user count. > > + } > > +} > > + > > +static void clk_consumer_show_subtree(struct seq_file *s, struct clk_core *c, int level) > > +{ > > + struct clk_core *child; > > + > > + clk_consumer_show_one(s, c, level); > > + > > + hlist_for_each_entry(child, &c->children, child_node) > > + clk_consumer_show_subtree(s, child, level + 1); > > +} > > + > > +static int clk_consumer_show(struct seq_file *s, void *data) > > +{ > > + struct clk_core *c; > > + struct hlist_head **lists = (struct hlist_head **)s->private; > > + > > + seq_puts(s, " enable prepare hardware\n"); > > + seq_puts(s, " clock consumer count count enable\n"); > > + seq_puts(s, "-----------------------------------------------------------------------------------------\n"); > > + clk_prepare_lock(); > > + > > + /*Traversing Linked List to print clock consumer*/ > > Please run scripts/checkpatch.pl, as this comment needs space after /* > and before */ > We will update this in revised patch. > > + > > + for (; *lists; lists++) > > + hlist_for_each_entry(c, *lists, child_node) > > + clk_consumer_show_subtree(s, c, 0); > > + > > + clk_prepare_unlock(); > > + > > + return 0; > > +} > > +DEFINE_SHOW_ATTRIBUTE(clk_consumer); > > + > > static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) > > { > > int phase; ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] Common clock: To list active consumers of clocks 2022-06-26 18:25 ` <Vishal Badole> @ 2022-08-02 22:49 ` Elliott, Robert (Servers) 2022-08-08 17:00 ` <Vishal Badole> 0 siblings, 1 reply; 17+ messages in thread From: Elliott, Robert (Servers) @ 2022-08-02 22:49 UTC (permalink / raw) To: '<Vishal Badole>', Stephen Boyd, mturquette, inux-clk, linux-kernel Cc: chinmoyghosh2001, mintupatel89, vimal.kumar32 > -----Original Message----- > From: <Vishal Badole> <badolevishal1116@gmail.com> > Sent: Sunday, June 26, 2022 1:25 PM > To: Stephen Boyd <sboyd@kernel.org>; mturquette@baylibre.com; inux- > clk@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: chinmoyghosh2001@gmail.com; mintupatel89@gmail.com; > vimal.kumar32@gmail.com > Subject: Re: [PATCH] Common clock: To list active consumers of clocks > ... > We will remove above prints in the revised patch. We are facing > indentation issue whle printing consumer in summary > as given below > enable prepare protect duty hardware per-user > clock count count count rateccuracy phase cycle enable consumer count > clk_mcasp0_fixed 0 0 0 24576000 0 50000 Y > deviceless 0 Consider making the kernel output simple, greppable, and parseable (e.g., comma-separated fields, one entry per line, no multi-line column headers) and let a userspace tool do the fancy formatting. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Common clock: To list active consumers of clocks 2022-08-02 22:49 ` Elliott, Robert (Servers) @ 2022-08-08 17:00 ` <Vishal Badole> 2022-08-21 5:07 ` Elliott, Robert (Servers) 0 siblings, 1 reply; 17+ messages in thread From: <Vishal Badole> @ 2022-08-08 17:00 UTC (permalink / raw) To: Elliott, Robert (Servers) Cc: Stephen Boyd, mturquette, inux-clk, linux-kernel, chinmoyghosh2001, mintupatel89, vimal.kumar32 On Tue, Aug 02, 2022 at 10:49:17PM +0000, Elliott, Robert (Servers) wrote: > > > > -----Original Message----- > > From: <Vishal Badole> <badolevishal1116@gmail.com> > > Sent: Sunday, June 26, 2022 1:25 PM > > To: Stephen Boyd <sboyd@kernel.org>; mturquette@baylibre.com; inux- > > clk@vger.kernel.org; linux-kernel@vger.kernel.org > > Cc: chinmoyghosh2001@gmail.com; mintupatel89@gmail.com; > > vimal.kumar32@gmail.com > > Subject: Re: [PATCH] Common clock: To list active consumers of clocks > > > ... > > We will remove above prints in the revised patch. We are facing > > indentation issue whle printing consumer in summary > > as given below > > enable prepare protect duty hardware per-user > > clock count count count rateccuracy phase cycle enable consumer count > > clk_mcasp0_fixed 0 0 0 24576000 0 50000 Y > > deviceless 0 > > > Consider making the kernel output simple, greppable, and parseable (e.g., > comma-separated fields, one entry per line, no multi-line column headers) > and let a userspace tool do the fancy formatting. > > > > Hi Robert, We have raised another patch for the same. Please find the below link for reference: https://www.spinics.net/lists/kernel/msg4459705.html Regards, Vishal ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] Common clock: To list active consumers of clocks 2022-08-08 17:00 ` <Vishal Badole> @ 2022-08-21 5:07 ` Elliott, Robert (Servers) 2022-08-21 17:59 ` <Vishal Badole> 0 siblings, 1 reply; 17+ messages in thread From: Elliott, Robert (Servers) @ 2022-08-21 5:07 UTC (permalink / raw) To: <Vishal Badole> Cc: Stephen Boyd, mturquette, inux-clk, linux-kernel, chinmoyghosh2001, mintupatel89, vimal.kumar32 > -----Original Message----- > From: <Vishal Badole> <badolevishal1116@gmail.com> > Sent: Monday, August 8, 2022 12:00 PM > To: Elliott, Robert (Servers) <elliott@hpe.com> > Cc: Stephen Boyd <sboyd@kernel.org>; mturquette@baylibre.com; inux- > clk@vger.kernel.org; linux-kernel@vger.kernel.org; chinmoyghosh2001@gmail.com; > mintupatel89@gmail.com; vimal.kumar32@gmail.com > Subject: Re: [PATCH] Common clock: To list active consumers of clocks > > On Tue, Aug 02, 2022 at 10:49:17PM +0000, Elliott, Robert (Servers) wrote: > > > > > > > -----Original Message----- > > > From: <Vishal Badole> <badolevishal1116@gmail.com> > > > Sent: Sunday, June 26, 2022 1:25 PM > > > To: Stephen Boyd <sboyd@kernel.org>; mturquette@baylibre.com; inux- > > > clk@vger.kernel.org; linux-kernel@vger.kernel.org > > > Cc: chinmoyghosh2001@gmail.com; mintupatel89@gmail.com; > > > vimal.kumar32@gmail.com > > > Subject: Re: [PATCH] Common clock: To list active consumers of clocks > > > > > ... > > > We will remove above prints in the revised patch. We are facing > > > indentation issue whle printing consumer in summary > > > as given below > > > enable prepare protect > duty hardware per-user > > > clock count count count > rateccuracy phase cycle enable consumer count > > > clk_mcasp0_fixed 0 0 0 > 24576000 0 50000 Y > > > deviceless 0 > > > > Consider making the kernel output simple, greppable, and parseable (e.g., > > comma-separated fields, one entry per line, no multi-line column headers) > > and let a userspace tool do the fancy formatting. > > > Hi Robert, > We have raised another patch for the same. Please find the below link > for reference: > > https://www.spinics.net/lists/kernel/msg4459705.html That output is still not parsable. I suggest making the kernel output more like: clock,enable count,prepare count,protect count,rate,accuracy,phase,duty cycle,hardware enable,consumer,per-user count clk_mcasp0_fixed,0,0,0,24576000,0,0,50000,Y,deviceless,0 clk_mcasp0,0,0,0,24576000,0,0,50000,N,simple-audio-card;cpu,0 and make a userspace program like lsmod, lscpu, lsblk, lspci, or lsusb to print the data with fancy columns or apply other filters. That allows adding or removing column headers, assuming the userspace program doesn't hardcode assumptions about them. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Common clock: To list active consumers of clocks 2022-08-21 5:07 ` Elliott, Robert (Servers) @ 2022-08-21 17:59 ` <Vishal Badole> 0 siblings, 0 replies; 17+ messages in thread From: <Vishal Badole> @ 2022-08-21 17:59 UTC (permalink / raw) To: Elliott, Robert (Servers) Cc: Stephen Boyd, mturquette, linux-clk, linux-kernel, chinmoyghosh2001, mintupatel89, vimal.kumar32 On Sun, Aug 21, 2022 at 05:07:00AM +0000, Elliott, Robert (Servers) wrote: > > > > -----Original Message----- > > From: <Vishal Badole> <badolevishal1116@gmail.com> > > Sent: Monday, August 8, 2022 12:00 PM > > To: Elliott, Robert (Servers) <elliott@hpe.com> > > Cc: Stephen Boyd <sboyd@kernel.org>; mturquette@baylibre.com; inux- > > clk@vger.kernel.org; linux-kernel@vger.kernel.org; chinmoyghosh2001@gmail.com; > > mintupatel89@gmail.com; vimal.kumar32@gmail.com > > Subject: Re: [PATCH] Common clock: To list active consumers of clocks > > > > On Tue, Aug 02, 2022 at 10:49:17PM +0000, Elliott, Robert (Servers) wrote: > > > > > > > > > > -----Original Message----- > > > > From: <Vishal Badole> <badolevishal1116@gmail.com> > > > > Sent: Sunday, June 26, 2022 1:25 PM > > > > To: Stephen Boyd <sboyd@kernel.org>; mturquette@baylibre.com; inux- > > > > clk@vger.kernel.org; linux-kernel@vger.kernel.org > > > > Cc: chinmoyghosh2001@gmail.com; mintupatel89@gmail.com; > > > > vimal.kumar32@gmail.com > > > > Subject: Re: [PATCH] Common clock: To list active consumers of clocks > > > > > > > ... > > > > We will remove above prints in the revised patch. We are facing > > > > indentation issue whle printing consumer in summary > > > > as given below > > > > enable prepare protect > > duty hardware per-user > > > > clock count count count > > rateccuracy phase cycle enable consumer count > > > > clk_mcasp0_fixed 0 0 0 > > 24576000 0 50000 Y > > > > deviceless 0 > > > > > > Consider making the kernel output simple, greppable, and parseable (e.g., > > > comma-separated fields, one entry per line, no multi-line column headers) > > > and let a userspace tool do the fancy formatting. > > > > > Hi Robert, > > We have raised another patch for the same. Please find the below link > > for reference: > > > > https://www.spinics.net/lists/kernel/msg4459705.html > > That output is still not parsable. > > I suggest making the kernel output more like: > clock,enable count,prepare count,protect count,rate,accuracy,phase,duty cycle,hardware enable,consumer,per-user count > clk_mcasp0_fixed,0,0,0,24576000,0,0,50000,Y,deviceless,0 > clk_mcasp0,0,0,0,24576000,0,0,50000,N,simple-audio-card;cpu,0 > > and make a userspace program like lsmod, lscpu, lsblk, lspci, > or lsusb to print the data with fancy columns or apply > other filters. > > That allows adding or removing column headers, assuming the > userspace program doesn't hardcode assumptions about them. > Hi Robert, As per the review given by stephen Boyd, who is one of maintainer of clk.c, suggested to add consumer's name and per user count in clock summary only. We are also getting proper formatted and parsable output on our target board console but when we copy and paste the same in commit message its format is getting changed. Please apply this patch and check on your target. Regards, Vishal ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3] Common clock: To list active consumers of clocks [not found] ` <20220624010550.582BBC341C7@smtp.kernel.org> 2022-06-26 18:25 ` <Vishal Badole> @ 2022-06-28 5:08 ` Vishal Badole 2022-08-02 18:09 ` Vishal Badole 2 siblings, 0 replies; 17+ messages in thread From: Vishal Badole @ 2022-06-28 5:08 UTC (permalink / raw) To: sboyd Cc: mturquette, inux-clk, linux-kernel, chinmoyghosh2001, mintupatel89, vimal.kumar32, Vishal Badole This feature lists the clock consumer's name and per-user enable count in clock summary. Using this feature user can easily check which device has acquired a perticular clock and it is enabled by respective device or not. for example: debian@beaglebone:~$ cat /sys/kernel/debug/clk/clk_summary enable prepare protect duty hardware per-user clock count count count rate accuracy phase cycle enable consumer count ---------------------------------------------------------------------------------------------------------------------------- clk_mcasp0_fixed 0 0 0 24576000 0 0 50000 Y deviceless 0 deviceless 0 clk_mcasp0 0 0 0 24576000 0 0 50000 N simple-audio-card,cpu 0 deviceless 0 Co-developed-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> Co-developed-by: Mintu Patel <mintupatel89@gmail.com> Signed-off-by: Mintu Patel <mintupatel89@gmail.com> Co-developed-by: Vimal Kumar <vimal.kumar32@gmail.com> Signed-off-by: Vimal Kumar <vimal.kumar32@gmail.com> Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> --- drivers/clk/clk.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index ed11918..6c4249e 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -102,6 +102,7 @@ struct clk { unsigned long min_rate; unsigned long max_rate; unsigned int exclusive_count; + unsigned int enable_count; struct hlist_node clks_node; }; @@ -1015,6 +1016,10 @@ void clk_disable(struct clk *clk) return; clk_core_disable_lock(clk->core); + + if (clk->enable_count > 0) + clk->enable_count--; + } EXPORT_SYMBOL_GPL(clk_disable); @@ -1176,10 +1181,16 @@ EXPORT_SYMBOL_GPL(clk_restore_context); */ int clk_enable(struct clk *clk) { + int ret; + if (!clk) return 0; - return clk_core_enable_lock(clk->core); + ret = clk_core_enable_lock(clk->core); + if (!ret) + clk->enable_count++; + + return ret; } EXPORT_SYMBOL_GPL(clk_enable); @@ -2960,28 +2971,41 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, int level) { int phase; + struct clk *clk_user; + int multi_node = 0; - seq_printf(s, "%*s%-*s %7d %8d %8d %11lu %10lu ", + seq_printf(s, "%*s%-*s %-7d %-8d %-8d %-11lu %-10lu ", level * 3 + 1, "", - 30 - level * 3, c->name, + 35 - level * 3, c->name, c->enable_count, c->prepare_count, c->protect_count, clk_core_get_rate_recalc(c), clk_core_get_accuracy_recalc(c)); phase = clk_core_get_phase(c); if (phase >= 0) - seq_printf(s, "%5d", phase); + seq_printf(s, "%-5d", phase); else seq_puts(s, "-----"); - seq_printf(s, " %6d", clk_core_get_scaled_duty_cycle(c, 100000)); + seq_printf(s, " %-6d", clk_core_get_scaled_duty_cycle(c, 100000)); if (c->ops->is_enabled) - seq_printf(s, " %9c\n", clk_core_is_enabled(c) ? 'Y' : 'N'); + seq_printf(s, " %5c ", clk_core_is_enabled(c) ? 'Y' : 'N'); else if (!c->ops->enable) - seq_printf(s, " %9c\n", 'Y'); + seq_printf(s, " %5c ", 'Y'); else - seq_printf(s, " %9c\n", '?'); + seq_printf(s, " %5c ", '?'); + + hlist_for_each_entry(clk_user, &c->clks, clks_node) { + seq_printf(s, "%*s%-*s %-4d\n", + level * 3 + 2 + 105 * multi_node, "", + 30, + clk_user->dev_id ? clk_user->dev_id : "deviceless", + clk_user->enable_count); + + multi_node = 1; + } + } static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c, @@ -3002,9 +3026,9 @@ static int clk_summary_show(struct seq_file *s, void *data) struct clk_core *c; struct hlist_head **lists = (struct hlist_head **)s->private; - seq_puts(s, " enable prepare protect duty hardware\n"); - seq_puts(s, " clock count count count rate accuracy phase cycle enable\n"); - seq_puts(s, "-------------------------------------------------------------------------------------------------------\n"); + seq_puts(s, " enable prepare protect duty hardware per-user\n"); + seq_puts(s, " clock count count count rate accuracy phase cycle enable consumer count\n"); + seq_puts(s, "-------------------------------------------------------------------------------------------------------------------------------------------\n"); clk_prepare_lock(); -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3] Common clock: To list active consumers of clocks [not found] ` <20220624010550.582BBC341C7@smtp.kernel.org> 2022-06-26 18:25 ` <Vishal Badole> 2022-06-28 5:08 ` [PATCH v3] Common clock: To " Vishal Badole @ 2022-08-02 18:09 ` Vishal Badole 2022-08-08 16:46 ` <Vishal Badole> ` (2 more replies) 2 siblings, 3 replies; 17+ messages in thread From: Vishal Badole @ 2022-08-02 18:09 UTC (permalink / raw) To: sboyd Cc: mturquette, linux-clk, linux-kernel, chinmoyghosh2001, vimal.kumar32, Vishal Badole, Mintu Patel This feature lists the clock consumer's name and per-user enable count in clock summary. Using this feature user can easily check which device has acquired a perticular clock and it is enabled by respective device or not. for example: $ cat /sys/kernel/debug/clk/clk_summary enable prepare protect duty hardware per-user clock count count count rate accuracy phase cycle enable consumer count ---------------------------------------------------------------------------------------------------------------------------- clk_mcasp0_fixed 0 0 0 24576000 0 0 50000 Y deviceless 0 deviceless 0 clk_mcasp0 0 0 0 24576000 0 0 50000 N simple-audio-card,cpu 0 deviceless 0 Co-developed-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> Co-developed-by: Mintu Patel <mintupatel89@gmail.com> Signed-off-by: Mintu Patel <mintupatel89@gmail.com> Co-developed-by: Vimal Kumar <vimal.kumar32@gmail.com> Signed-off-by: Vimal Kumar <vimal.kumar32@gmail.com> Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> --- drivers/clk/clk.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index f00d4c1..c96079f 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -102,6 +102,7 @@ struct clk { unsigned long min_rate; unsigned long max_rate; unsigned int exclusive_count; + unsigned int enable_count; struct hlist_node clks_node; }; @@ -1008,6 +1009,10 @@ void clk_disable(struct clk *clk) return; clk_core_disable_lock(clk->core); + + if (clk->enable_count > 0) + clk->enable_count--; + } EXPORT_SYMBOL_GPL(clk_disable); @@ -1169,10 +1174,16 @@ EXPORT_SYMBOL_GPL(clk_restore_context); */ int clk_enable(struct clk *clk) { + int ret; + if (!clk) return 0; - return clk_core_enable_lock(clk->core); + ret = clk_core_enable_lock(clk->core); + if (!ret) + clk->enable_count++; + + return ret; } EXPORT_SYMBOL_GPL(clk_enable); @@ -2953,28 +2964,41 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, int level) { int phase; + struct clk *clk_user; + int multi_node = 0; - seq_printf(s, "%*s%-*s %7d %8d %8d %11lu %10lu ", + seq_printf(s, "%*s%-*s %-7d %-8d %-8d %-11lu %-10lu ", level * 3 + 1, "", - 30 - level * 3, c->name, + 35 - level * 3, c->name, c->enable_count, c->prepare_count, c->protect_count, clk_core_get_rate_recalc(c), clk_core_get_accuracy_recalc(c)); phase = clk_core_get_phase(c); if (phase >= 0) - seq_printf(s, "%5d", phase); + seq_printf(s, "%-5d", phase); else seq_puts(s, "-----"); - seq_printf(s, " %6d", clk_core_get_scaled_duty_cycle(c, 100000)); + seq_printf(s, " %-6d", clk_core_get_scaled_duty_cycle(c, 100000)); if (c->ops->is_enabled) - seq_printf(s, " %9c\n", clk_core_is_enabled(c) ? 'Y' : 'N'); + seq_printf(s, " %5c ", clk_core_is_enabled(c) ? 'Y' : 'N'); else if (!c->ops->enable) - seq_printf(s, " %9c\n", 'Y'); + seq_printf(s, " %5c ", 'Y'); else - seq_printf(s, " %9c\n", '?'); + seq_printf(s, " %5c ", '?'); + + hlist_for_each_entry(clk_user, &c->clks, clks_node) { + seq_printf(s, "%*s%-*s %-4d\n", + level * 3 + 2 + 105 * multi_node, "", + 30, + clk_user->dev_id ? clk_user->dev_id : "deviceless", + clk_user->enable_count); + + multi_node = 1; + } + } static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c, @@ -2995,9 +3019,9 @@ static int clk_summary_show(struct seq_file *s, void *data) struct clk_core *c; struct hlist_head **lists = (struct hlist_head **)s->private; - seq_puts(s, " enable prepare protect duty hardware\n"); - seq_puts(s, " clock count count count rate accuracy phase cycle enable\n"); - seq_puts(s, "-------------------------------------------------------------------------------------------------------\n"); + seq_puts(s, " enable prepare protect duty hardware per-user\n"); + seq_puts(s, " clock count count count rate accuracy phase cycle enable consumer count\n"); + seq_puts(s, "-------------------------------------------------------------------------------------------------------------------------------------------\n"); clk_prepare_lock(); -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Common clock: To list active consumers of clocks 2022-08-02 18:09 ` Vishal Badole @ 2022-08-08 16:46 ` <Vishal Badole> 2022-08-21 18:06 ` <Vishal Badole> 2022-08-22 23:50 ` Stephen Boyd 2 siblings, 0 replies; 17+ messages in thread From: <Vishal Badole> @ 2022-08-08 16:46 UTC (permalink / raw) To: sboyd Cc: mturquette, linux-clk, linux-kernel, chinmoyghosh2001, vimal.kumar32, Mintu Patel On Tue, Aug 02, 2022 at 11:39:47PM +0530, Vishal Badole wrote: > This feature lists the clock consumer's name and per-user enable count > in clock summary. Using this feature user can easily check which device > has acquired a perticular clock and it is enabled by respective device > or not. > for example: > $ cat /sys/kernel/debug/clk/clk_summary > enable prepare protect duty hardware per-user > clock count count count rate accuracy phase cycle enable consumer count > ---------------------------------------------------------------------------------------------------------------------------- > clk_mcasp0_fixed 0 0 0 24576000 0 0 50000 Y deviceless 0 > deviceless 0 > clk_mcasp0 0 0 0 24576000 0 0 50000 N simple-audio-card,cpu 0 > deviceless 0 > > Co-developed-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > Co-developed-by: Mintu Patel <mintupatel89@gmail.com> > Signed-off-by: Mintu Patel <mintupatel89@gmail.com> > Co-developed-by: Vimal Kumar <vimal.kumar32@gmail.com> > Signed-off-by: Vimal Kumar <vimal.kumar32@gmail.com> > Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> > --- > drivers/clk/clk.c | 46 +++++++++++++++++++++++++++++++++++----------- > 1 file changed, 35 insertions(+), 11 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index f00d4c1..c96079f 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -102,6 +102,7 @@ struct clk { > unsigned long min_rate; > unsigned long max_rate; > unsigned int exclusive_count; > + unsigned int enable_count; > struct hlist_node clks_node; > }; > > @@ -1008,6 +1009,10 @@ void clk_disable(struct clk *clk) > return; > > clk_core_disable_lock(clk->core); > + > + if (clk->enable_count > 0) > + clk->enable_count--; > + > } > EXPORT_SYMBOL_GPL(clk_disable); > > @@ -1169,10 +1174,16 @@ EXPORT_SYMBOL_GPL(clk_restore_context); > */ > int clk_enable(struct clk *clk) > { > + int ret; > + > if (!clk) > return 0; > > - return clk_core_enable_lock(clk->core); > + ret = clk_core_enable_lock(clk->core); > + if (!ret) > + clk->enable_count++; > + > + return ret; > } > EXPORT_SYMBOL_GPL(clk_enable); > > @@ -2953,28 +2964,41 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, > int level) > { > int phase; > + struct clk *clk_user; > + int multi_node = 0; > > - seq_printf(s, "%*s%-*s %7d %8d %8d %11lu %10lu ", > + seq_printf(s, "%*s%-*s %-7d %-8d %-8d %-11lu %-10lu ", > level * 3 + 1, "", > - 30 - level * 3, c->name, > + 35 - level * 3, c->name, > c->enable_count, c->prepare_count, c->protect_count, > clk_core_get_rate_recalc(c), > clk_core_get_accuracy_recalc(c)); > > phase = clk_core_get_phase(c); > if (phase >= 0) > - seq_printf(s, "%5d", phase); > + seq_printf(s, "%-5d", phase); > else > seq_puts(s, "-----"); > > - seq_printf(s, " %6d", clk_core_get_scaled_duty_cycle(c, 100000)); > + seq_printf(s, " %-6d", clk_core_get_scaled_duty_cycle(c, 100000)); > > if (c->ops->is_enabled) > - seq_printf(s, " %9c\n", clk_core_is_enabled(c) ? 'Y' : 'N'); > + seq_printf(s, " %5c ", clk_core_is_enabled(c) ? 'Y' : 'N'); > else if (!c->ops->enable) > - seq_printf(s, " %9c\n", 'Y'); > + seq_printf(s, " %5c ", 'Y'); > else > - seq_printf(s, " %9c\n", '?'); > + seq_printf(s, " %5c ", '?'); > + > + hlist_for_each_entry(clk_user, &c->clks, clks_node) { > + seq_printf(s, "%*s%-*s %-4d\n", > + level * 3 + 2 + 105 * multi_node, "", > + 30, > + clk_user->dev_id ? clk_user->dev_id : "deviceless", > + clk_user->enable_count); > + > + multi_node = 1; > + } > + > } > > static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c, > @@ -2995,9 +3019,9 @@ static int clk_summary_show(struct seq_file *s, void *data) > struct clk_core *c; > struct hlist_head **lists = (struct hlist_head **)s->private; > > - seq_puts(s, " enable prepare protect duty hardware\n"); > - seq_puts(s, " clock count count count rate accuracy phase cycle enable\n"); > - seq_puts(s, "-------------------------------------------------------------------------------------------------------\n"); > + seq_puts(s, " enable prepare protect duty hardware per-user\n"); > + seq_puts(s, " clock count count count rate accuracy phase cycle enable consumer count\n"); > + seq_puts(s, "-------------------------------------------------------------------------------------------------------------------------------------------\n"); > > clk_prepare_lock(); > > -- > 2.7.4 > Hi Stephen, Have you got a chance to review the above patch? We have made the changes as per the reviews, please have a look on the patch. Regards, Vishal ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Common clock: To list active consumers of clocks 2022-08-02 18:09 ` Vishal Badole 2022-08-08 16:46 ` <Vishal Badole> @ 2022-08-21 18:06 ` <Vishal Badole> 2022-08-22 23:50 ` Stephen Boyd 2 siblings, 0 replies; 17+ messages in thread From: <Vishal Badole> @ 2022-08-21 18:06 UTC (permalink / raw) To: sboyd Cc: mturquette, linux-clk, linux-kernel, chinmoyghosh2001, vimal.kumar32, Mintu Patel On Tue, Aug 02, 2022 at 11:39:47PM +0530, Vishal Badole wrote: > This feature lists the clock consumer's name and per-user enable count > in clock summary. Using this feature user can easily check which device > has acquired a perticular clock and it is enabled by respective device > or not. > for example: > $ cat /sys/kernel/debug/clk/clk_summary > enable prepare protect duty hardware per-user > clock count count count rate accuracy phase cycle enable consumer count > ---------------------------------------------------------------------------------------------------------------------------- > clk_mcasp0_fixed 0 0 0 24576000 0 0 50000 Y deviceless 0 > deviceless 0 > clk_mcasp0 0 0 0 24576000 0 0 50000 N simple-audio-card,cpu 0 > deviceless 0 > > Co-developed-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > Co-developed-by: Mintu Patel <mintupatel89@gmail.com> > Signed-off-by: Mintu Patel <mintupatel89@gmail.com> > Co-developed-by: Vimal Kumar <vimal.kumar32@gmail.com> > Signed-off-by: Vimal Kumar <vimal.kumar32@gmail.com> > Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> > --- > drivers/clk/clk.c | 46 +++++++++++++++++++++++++++++++++++----------- > 1 file changed, 35 insertions(+), 11 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index f00d4c1..c96079f 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -102,6 +102,7 @@ struct clk { > unsigned long min_rate; > unsigned long max_rate; > unsigned int exclusive_count; > + unsigned int enable_count; > struct hlist_node clks_node; > }; > > @@ -1008,6 +1009,10 @@ void clk_disable(struct clk *clk) > return; > > clk_core_disable_lock(clk->core); > + > + if (clk->enable_count > 0) > + clk->enable_count--; > + > } > EXPORT_SYMBOL_GPL(clk_disable); > > @@ -1169,10 +1174,16 @@ EXPORT_SYMBOL_GPL(clk_restore_context); > */ > int clk_enable(struct clk *clk) > { > + int ret; > + > if (!clk) > return 0; > > - return clk_core_enable_lock(clk->core); > + ret = clk_core_enable_lock(clk->core); > + if (!ret) > + clk->enable_count++; > + > + return ret; > } > EXPORT_SYMBOL_GPL(clk_enable); > > @@ -2953,28 +2964,41 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, > int level) > { > int phase; > + struct clk *clk_user; > + int multi_node = 0; > > - seq_printf(s, "%*s%-*s %7d %8d %8d %11lu %10lu ", > + seq_printf(s, "%*s%-*s %-7d %-8d %-8d %-11lu %-10lu ", > level * 3 + 1, "", > - 30 - level * 3, c->name, > + 35 - level * 3, c->name, > c->enable_count, c->prepare_count, c->protect_count, > clk_core_get_rate_recalc(c), > clk_core_get_accuracy_recalc(c)); > > phase = clk_core_get_phase(c); > if (phase >= 0) > - seq_printf(s, "%5d", phase); > + seq_printf(s, "%-5d", phase); > else > seq_puts(s, "-----"); > > - seq_printf(s, " %6d", clk_core_get_scaled_duty_cycle(c, 100000)); > + seq_printf(s, " %-6d", clk_core_get_scaled_duty_cycle(c, 100000)); > > if (c->ops->is_enabled) > - seq_printf(s, " %9c\n", clk_core_is_enabled(c) ? 'Y' : 'N'); > + seq_printf(s, " %5c ", clk_core_is_enabled(c) ? 'Y' : 'N'); > else if (!c->ops->enable) > - seq_printf(s, " %9c\n", 'Y'); > + seq_printf(s, " %5c ", 'Y'); > else > - seq_printf(s, " %9c\n", '?'); > + seq_printf(s, " %5c ", '?'); > + > + hlist_for_each_entry(clk_user, &c->clks, clks_node) { > + seq_printf(s, "%*s%-*s %-4d\n", > + level * 3 + 2 + 105 * multi_node, "", > + 30, > + clk_user->dev_id ? clk_user->dev_id : "deviceless", > + clk_user->enable_count); > + > + multi_node = 1; > + } > + > } > > static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c, > @@ -2995,9 +3019,9 @@ static int clk_summary_show(struct seq_file *s, void *data) > struct clk_core *c; > struct hlist_head **lists = (struct hlist_head **)s->private; > > - seq_puts(s, " enable prepare protect duty hardware\n"); > - seq_puts(s, " clock count count count rate accuracy phase cycle enable\n"); > - seq_puts(s, "-------------------------------------------------------------------------------------------------------\n"); > + seq_puts(s, " enable prepare protect duty hardware per-user\n"); > + seq_puts(s, " clock count count count rate accuracy phase cycle enable consumer count\n"); > + seq_puts(s, "-------------------------------------------------------------------------------------------------------------------------------------------\n"); > > clk_prepare_lock(); > > -- > 2.7.4 > Hi Stephen, Please review the above patch. Here we have made the changes as per your review points. Note: The example format in commit meassage is getting changed during copy paste but we are getting proper formatted and parsable output on actual target. Regards, Vishal ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Common clock: To list active consumers of clocks 2022-08-02 18:09 ` Vishal Badole 2022-08-08 16:46 ` <Vishal Badole> 2022-08-21 18:06 ` <Vishal Badole> @ 2022-08-22 23:50 ` Stephen Boyd 2022-08-26 17:34 ` <Vishal Badole> 2022-11-27 18:00 ` <Vishal Badole> 2 siblings, 2 replies; 17+ messages in thread From: Stephen Boyd @ 2022-08-22 23:50 UTC (permalink / raw) To: Vishal Badole Cc: mturquette, linux-clk, linux-kernel, chinmoyghosh2001, vimal.kumar32, Vishal Badole, Mintu Patel Quoting Vishal Badole (2022-08-02 11:09:47) > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index f00d4c1..c96079f 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -102,6 +102,7 @@ struct clk { > unsigned long min_rate; > unsigned long max_rate; > unsigned int exclusive_count; > + unsigned int enable_count; > struct hlist_node clks_node; > }; > > @@ -1008,6 +1009,10 @@ void clk_disable(struct clk *clk) > return; > > clk_core_disable_lock(clk->core); > + > + if (clk->enable_count > 0) > + clk->enable_count--; > + > } > EXPORT_SYMBOL_GPL(clk_disable); > > @@ -1169,10 +1174,16 @@ EXPORT_SYMBOL_GPL(clk_restore_context); > */ > int clk_enable(struct clk *clk) > { > + int ret; > + > if (!clk) > return 0; > > - return clk_core_enable_lock(clk->core); > + ret = clk_core_enable_lock(clk->core); > + if (!ret) > + clk->enable_count++; > + > + return ret; > } > EXPORT_SYMBOL_GPL(clk_enable); We'll want the above three hunks to be a different patch so we can discuss the merits of tracking per user enable counts. Do you have a usecase for this or is it "just for fun"? By adding a count we have more code, and we waste more memory to track this stat. I really would rather not bloat just because, so please elaborate on your use case. > > @@ -2953,28 +2964,41 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, > int level) > { > int phase; > + struct clk *clk_user; > + int multi_node = 0; > > - seq_printf(s, "%*s%-*s %7d %8d %8d %11lu %10lu ", > + seq_printf(s, "%*s%-*s %-7d %-8d %-8d %-11lu %-10lu ", > level * 3 + 1, "", > - 30 - level * 3, c->name, > + 35 - level * 3, c->name, > c->enable_count, c->prepare_count, c->protect_count, > clk_core_get_rate_recalc(c), > clk_core_get_accuracy_recalc(c)); > > phase = clk_core_get_phase(c); > if (phase >= 0) > - seq_printf(s, "%5d", phase); > + seq_printf(s, "%-5d", phase); > else > seq_puts(s, "-----"); > > - seq_printf(s, " %6d", clk_core_get_scaled_duty_cycle(c, 100000)); > + seq_printf(s, " %-6d", clk_core_get_scaled_duty_cycle(c, 100000)); > > if (c->ops->is_enabled) > - seq_printf(s, " %9c\n", clk_core_is_enabled(c) ? 'Y' : 'N'); > + seq_printf(s, " %5c ", clk_core_is_enabled(c) ? 'Y' : 'N'); > else if (!c->ops->enable) > - seq_printf(s, " %9c\n", 'Y'); > + seq_printf(s, " %5c ", 'Y'); > else > - seq_printf(s, " %9c\n", '?'); > + seq_printf(s, " %5c ", '?'); > + > + hlist_for_each_entry(clk_user, &c->clks, clks_node) { > + seq_printf(s, "%*s%-*s %-4d\n", > + level * 3 + 2 + 105 * multi_node, "", > + 30, > + clk_user->dev_id ? clk_user->dev_id : "deviceless", > + clk_user->enable_count); > + > + multi_node = 1; This part that prints the dev_id might be useful and can be the first patch in the series. In that same patch, please print the con_id so we know which clk it is for the device. We should also improve of_clk_get() so that the index is visible to the 'struct clk::con_id' somehow. Maybe we can convert the integer index into a string and assign that to con_id in that case as well. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Common clock: To list active consumers of clocks 2022-08-22 23:50 ` Stephen Boyd @ 2022-08-26 17:34 ` <Vishal Badole> 2022-11-27 18:00 ` <Vishal Badole> 1 sibling, 0 replies; 17+ messages in thread From: <Vishal Badole> @ 2022-08-26 17:34 UTC (permalink / raw) To: Stephen Boyd Cc: mturquette, linux-clk, linux-kernel, chinmoyghosh2001, vimal.kumar32, Mintu Patel On Mon, Aug 22, 2022 at 04:50:12PM -0700, Stephen Boyd wrote: > Quoting Vishal Badole (2022-08-02 11:09:47) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index f00d4c1..c96079f 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -102,6 +102,7 @@ struct clk { > > unsigned long min_rate; > > unsigned long max_rate; > > unsigned int exclusive_count; > > + unsigned int enable_count; > > struct hlist_node clks_node; > > }; > > > > @@ -1008,6 +1009,10 @@ void clk_disable(struct clk *clk) > > return; > > > > clk_core_disable_lock(clk->core); > > + > > + if (clk->enable_count > 0) > > + clk->enable_count--; > > + > > } > > EXPORT_SYMBOL_GPL(clk_disable); > > > > @@ -1169,10 +1174,16 @@ EXPORT_SYMBOL_GPL(clk_restore_context); > > */ > > int clk_enable(struct clk *clk) > > { > > + int ret; > > + > > if (!clk) > > return 0; > > > > - return clk_core_enable_lock(clk->core); > > + ret = clk_core_enable_lock(clk->core); > > + if (!ret) > > + clk->enable_count++; > > + > > + return ret; > > } > > EXPORT_SYMBOL_GPL(clk_enable); > > We'll want the above three hunks to be a different patch so we can > discuss the merits of tracking per user enable counts. Agreed, we will create a separate patch for the same. > Do you have a usecase for this or is it "just for fun"? By adding a count we have more > code, and we waste more memory to track this stat. I really would rather > not bloat just because, so please elaborate on your use case. > Use case for per user count: If a consumer acquires the clocks without calling clk_get() or devm_clk_get() and enables without calling clk_enable() or clk_prepare_enable() (by bypassing the common clock framework), then dev_id will not be sufficient to tell about how clk is acquired. Here per user enable count can be used to tell that how clk is acquired and it is enabled by that particular device or not in case also where dev_id is NULL. We referred regualtor framework suggested by you in one of review point where they are also using enable count. > > > > @@ -2953,28 +2964,41 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, > > int level) > > { > > int phase; > > + struct clk *clk_user; > > + int multi_node = 0; > > > > - seq_printf(s, "%*s%-*s %7d %8d %8d %11lu %10lu ", > > + seq_printf(s, "%*s%-*s %-7d %-8d %-8d %-11lu %-10lu ", > > level * 3 + 1, "", > > - 30 - level * 3, c->name, > > + 35 - level * 3, c->name, > > c->enable_count, c->prepare_count, c->protect_count, > > clk_core_get_rate_recalc(c), > > clk_core_get_accuracy_recalc(c)); > > > > phase = clk_core_get_phase(c); > > if (phase >= 0) > > - seq_printf(s, "%5d", phase); > > + seq_printf(s, "%-5d", phase); > > else > > seq_puts(s, "-----"); > > > > - seq_printf(s, " %6d", clk_core_get_scaled_duty_cycle(c, 100000)); > > + seq_printf(s, " %-6d", clk_core_get_scaled_duty_cycle(c, 100000)); > > > > if (c->ops->is_enabled) > > - seq_printf(s, " %9c\n", clk_core_is_enabled(c) ? 'Y' : 'N'); > > + seq_printf(s, " %5c ", clk_core_is_enabled(c) ? 'Y' : 'N'); > > else if (!c->ops->enable) > > - seq_printf(s, " %9c\n", 'Y'); > > + seq_printf(s, " %5c ", 'Y'); > > else > > - seq_printf(s, " %9c\n", '?'); > > + seq_printf(s, " %5c ", '?'); > > + > > + hlist_for_each_entry(clk_user, &c->clks, clks_node) { > > + seq_printf(s, "%*s%-*s %-4d\n", > > + level * 3 + 2 + 105 * multi_node, "", > > + 30, > > + clk_user->dev_id ? clk_user->dev_id : "deviceless", > > + clk_user->enable_count); > > + > > + multi_node = 1; > > This part that prints the dev_id might be useful and can be the first > patch in the series. In that same patch, please print the con_id so we > know which clk it is for the device. We should also improve of_clk_get() > so that the index is visible to the 'struct clk::con_id' somehow. Maybe > we can convert the integer index into a string and assign that to con_id > in that case as well. Agreed, We will create a fresh patch where we will print dev_id and consumer id in clock summary. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Common clock: To list active consumers of clocks 2022-08-22 23:50 ` Stephen Boyd 2022-08-26 17:34 ` <Vishal Badole> @ 2022-11-27 18:00 ` <Vishal Badole> 1 sibling, 0 replies; 17+ messages in thread From: <Vishal Badole> @ 2022-11-27 18:00 UTC (permalink / raw) To: Stephen Boyd Cc: mturquette, linux-clk, linux-kernel, chinmoyghosh2001, vimal.kumar32, Mintu Patel Hi Stephen, As per your suggestions, we have updated and sent another gerrit with message Id <1669569799-8526-1-git-send-email-badolevishal1116@gmail.com> In this new patch we are listing the clock consumers name along with consumer id in clk_summary. example: cat /sys/kernel/debug/clk/clk_summary enable prepare protect duty hardware Connection clock count count count rate accuracy phase cycle enable consumer Id ------------------------------------------------------------------------------------------------------------------------------ clk_mcasp0_fixed 0 0 0 24576000 0 0 50000 Y deviceless of_clk_get_from_provider deviceless no_connection_id clk_mcasp0 0 0 0 24576000 0 0 50000 N simple-audio-card,cpu deviceless no_connection_id no_connection_id Please review the latest patch. New patch details: Message ID: <1669569799-8526-1-git-send-email-badolevishal1116@gmail.com> Subject: [PATCH v5] Common clock: To list active consumers of clocks Regards, Vishal ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-11-27 18:01 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAEXpiVQihEadxsNodarz2-wxSAipfpzEaA8zKpnozszC+weYTQ@mail.gmail.com>
2022-06-10 19:40 ` [PATCH 5.4] Common clock: To list active consumers of clocks Stephen Boyd
[not found] ` <CAOUcgRfB-r3aERYeLumExgpTVzsDsBuyOWT+nCJ_OfOv1g0L9g@mail.gmail.com>
2022-06-15 19:23 ` <Vishal Badole>
2022-06-22 16:32 ` [PATCH] " <Vishal Badole>
2022-06-22 19:37 ` Randy Dunlap
2022-06-22 17:02 ` <Vishal Badole>
[not found] ` <20220624010550.582BBC341C7@smtp.kernel.org>
2022-06-26 18:25 ` <Vishal Badole>
2022-08-02 22:49 ` Elliott, Robert (Servers)
2022-08-08 17:00 ` <Vishal Badole>
2022-08-21 5:07 ` Elliott, Robert (Servers)
2022-08-21 17:59 ` <Vishal Badole>
2022-06-28 5:08 ` [PATCH v3] Common clock: To " Vishal Badole
2022-08-02 18:09 ` Vishal Badole
2022-08-08 16:46 ` <Vishal Badole>
2022-08-21 18:06 ` <Vishal Badole>
2022-08-22 23:50 ` Stephen Boyd
2022-08-26 17:34 ` <Vishal Badole>
2022-11-27 18:00 ` <Vishal Badole>
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.