* [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted @ 2020-07-08 11:07 ` Sudeep Holla 0 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-08 11:07 UTC (permalink / raw) To: linux-arm-kernel, linux-clk, Stephen Boyd Cc: Sudeep Holla, linux-kernel, Michael Turquette, Dien Pham Instead of relying on the firmware to keep the clock rates sorted, let us sort the list. This is not essential for clock layer but it helps to find the min and max rates easily from the list. Fixes: 5f6c6430e904 ("firmware: arm_scmi: add initial support for clock protocol") Reported-by: Dien Pham <dien.pham.ry@renesas.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/clock.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) Hi Dien-san, If you could review/test these patches, I can queue them ASAP. I am planning to send the PR for ARM SoC later this week, so I need your tested-by. Regards, Sudeep diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index 4c2227662b26..2dd119cdebf6 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -5,6 +5,8 @@ * Copyright (C) 2018 ARM Ltd. */ +#include <linux/sort.h> + #include "common.h" enum scmi_clock_protocol_cmd { @@ -121,6 +123,13 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, return ret; } +static int rate_cmp_func(const void *_r1, const void *_r2) +{ + u64 *r1 = _r1, *r2 = _r2; + + return r1 - r2; +} + static int scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, struct scmi_clock_info *clk) @@ -184,8 +193,10 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, */ } while (num_returned && num_remaining); - if (rate_discrete) + if (rate_discrete) { clk->list.num_rates = tot_rate_cnt; + sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); + } clk->rate_discrete = rate_discrete; -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted @ 2020-07-08 11:07 ` Sudeep Holla 0 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-08 11:07 UTC (permalink / raw) To: linux-arm-kernel, linux-clk, Stephen Boyd Cc: Michael Turquette, Dien Pham, linux-kernel, Sudeep Holla Instead of relying on the firmware to keep the clock rates sorted, let us sort the list. This is not essential for clock layer but it helps to find the min and max rates easily from the list. Fixes: 5f6c6430e904 ("firmware: arm_scmi: add initial support for clock protocol") Reported-by: Dien Pham <dien.pham.ry@renesas.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/clock.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) Hi Dien-san, If you could review/test these patches, I can queue them ASAP. I am planning to send the PR for ARM SoC later this week, so I need your tested-by. Regards, Sudeep diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index 4c2227662b26..2dd119cdebf6 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -5,6 +5,8 @@ * Copyright (C) 2018 ARM Ltd. */ +#include <linux/sort.h> + #include "common.h" enum scmi_clock_protocol_cmd { @@ -121,6 +123,13 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, return ret; } +static int rate_cmp_func(const void *_r1, const void *_r2) +{ + u64 *r1 = _r1, *r2 = _r2; + + return r1 - r2; +} + static int scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, struct scmi_clock_info *clk) @@ -184,8 +193,10 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, */ } while (num_returned && num_remaining); - if (rate_discrete) + if (rate_discrete) { clk->list.num_rates = tot_rate_cnt; + sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); + } clk->rate_discrete = rate_discrete; -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] clk: scmi: Fix min and max rate when registering clocks with discrete rates 2020-07-08 11:07 ` Sudeep Holla @ 2020-07-08 11:07 ` Sudeep Holla -1 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-08 11:07 UTC (permalink / raw) To: linux-arm-kernel, linux-clk, Stephen Boyd Cc: Sudeep Holla, linux-kernel, Michael Turquette, Dien Pham Currently we are not initializing the scmi clock with discrete rates correctly. We fetch the min_rate and max_rate value only for clocks with ranges and ignore the ones with discrete rates. This will lead to wrong initialization of rate range when clock supports discrete rate. Fix this by using the first and the last rate in the sorted list of the discrete clock rates while registering the clock. Fixes: 6d6a1d82eaef7 ("clk: add support for clocks provided by SCMI") Reported-by: Dien Pham <dien.pham.ry@renesas.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/clk/clk-scmi.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) Hi Stephen, If you fine, I can take this via ARM SoC along with the change in firmware driver. But it is fine if you want to merge this independently as it should be fine. Let me know either way. Regards, Sudeep diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c index c491f5de0f3f..ea65b7bf1408 100644 --- a/drivers/clk/clk-scmi.c +++ b/drivers/clk/clk-scmi.c @@ -103,6 +103,8 @@ static const struct clk_ops scmi_clk_ops = { static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) { int ret; + unsigned long min_rate, max_rate; + struct clk_init_data init = { .flags = CLK_GET_RATE_NOCACHE, .num_parents = 0, @@ -112,9 +114,23 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) sclk->hw.init = &init; ret = devm_clk_hw_register(dev, &sclk->hw); - if (!ret) - clk_hw_set_rate_range(&sclk->hw, sclk->info->range.min_rate, - sclk->info->range.max_rate); + if (ret) + return ret; + + if (sclk->info->rate_discrete) { + int num_rates = sclk->info->list.num_rates; + + if (num_rates <= 0) + return -EINVAL; + + min_rate = sclk->info->list.rates[0] + max_rate = sclk->info->list.rates[num_rates - 1]; + } else { + min_rate = sclk->info->range.min_rate; + max_rate = sclk->info->range.max_rate; + } + + clk_hw_set_rate_range(&sclk->hw, min_rate, max_rate); return ret; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] clk: scmi: Fix min and max rate when registering clocks with discrete rates @ 2020-07-08 11:07 ` Sudeep Holla 0 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-08 11:07 UTC (permalink / raw) To: linux-arm-kernel, linux-clk, Stephen Boyd Cc: Michael Turquette, Dien Pham, linux-kernel, Sudeep Holla Currently we are not initializing the scmi clock with discrete rates correctly. We fetch the min_rate and max_rate value only for clocks with ranges and ignore the ones with discrete rates. This will lead to wrong initialization of rate range when clock supports discrete rate. Fix this by using the first and the last rate in the sorted list of the discrete clock rates while registering the clock. Fixes: 6d6a1d82eaef7 ("clk: add support for clocks provided by SCMI") Reported-by: Dien Pham <dien.pham.ry@renesas.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/clk/clk-scmi.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) Hi Stephen, If you fine, I can take this via ARM SoC along with the change in firmware driver. But it is fine if you want to merge this independently as it should be fine. Let me know either way. Regards, Sudeep diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c index c491f5de0f3f..ea65b7bf1408 100644 --- a/drivers/clk/clk-scmi.c +++ b/drivers/clk/clk-scmi.c @@ -103,6 +103,8 @@ static const struct clk_ops scmi_clk_ops = { static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) { int ret; + unsigned long min_rate, max_rate; + struct clk_init_data init = { .flags = CLK_GET_RATE_NOCACHE, .num_parents = 0, @@ -112,9 +114,23 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) sclk->hw.init = &init; ret = devm_clk_hw_register(dev, &sclk->hw); - if (!ret) - clk_hw_set_rate_range(&sclk->hw, sclk->info->range.min_rate, - sclk->info->range.max_rate); + if (ret) + return ret; + + if (sclk->info->rate_discrete) { + int num_rates = sclk->info->list.num_rates; + + if (num_rates <= 0) + return -EINVAL; + + min_rate = sclk->info->list.rates[0] + max_rate = sclk->info->list.rates[num_rates - 1]; + } else { + min_rate = sclk->info->range.min_rate; + max_rate = sclk->info->range.max_rate; + } + + clk_hw_set_rate_range(&sclk->hw, min_rate, max_rate); return ret; } -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] clk: scmi: Fix min and max rate when registering clocks with discrete rates 2020-07-08 11:07 ` Sudeep Holla (?) @ 2020-07-08 18:44 ` kernel test robot -1 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2020-07-08 18:44 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2756 bytes --] Hi Sudeep, I love your patch! Yet something to improve: [auto build test ERROR on clk/clk-next] [also build test ERROR on rockchip/for-next shawnguo/for-next soc/for-next linus/master v5.8-rc4 next-20200707] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Sudeep-Holla/firmware-arm_scmi-Keep-the-discrete-clock-rates-sorted/20200708-190957 base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next config: sparc-allyesconfig (attached as .config) compiler: sparc64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/clk/clk-scmi.c: In function 'scmi_clk_ops_init': >> drivers/clk/clk-scmi.c:126:39: error: expected ';' before 'max_rate' 126 | min_rate = sclk->info->list.rates[0] | ^ | ; 127 | max_rate = sclk->info->list.rates[num_rates - 1]; | ~~~~~~~~ vim +126 drivers/clk/clk-scmi.c 102 103 static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) 104 { 105 int ret; 106 unsigned long min_rate, max_rate; 107 108 struct clk_init_data init = { 109 .flags = CLK_GET_RATE_NOCACHE, 110 .num_parents = 0, 111 .ops = &scmi_clk_ops, 112 .name = sclk->info->name, 113 }; 114 115 sclk->hw.init = &init; 116 ret = devm_clk_hw_register(dev, &sclk->hw); 117 if (ret) 118 return ret; 119 120 if (sclk->info->rate_discrete) { 121 int num_rates = sclk->info->list.num_rates; 122 123 if (num_rates <= 0) 124 return -EINVAL; 125 > 126 min_rate = sclk->info->list.rates[0] 127 max_rate = sclk->info->list.rates[num_rates - 1]; 128 } else { 129 min_rate = sclk->info->range.min_rate; 130 max_rate = sclk->info->range.max_rate; 131 } 132 133 clk_hw_set_rate_range(&sclk->hw, min_rate, max_rate); 134 return ret; 135 } 136 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 66257 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] clk: scmi: Fix min and max rate when registering clocks with discrete rates 2020-07-08 11:07 ` Sudeep Holla @ 2020-07-08 20:24 ` Sudeep Holla -1 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-08 20:24 UTC (permalink / raw) To: linux-arm-kernel, linux-clk, Stephen Boyd Cc: linux-kernel, Michael Turquette, Dien Pham, Sudeep Holla On Wed, Jul 08, 2020 at 12:07:25PM +0100, Sudeep Holla wrote: > Currently we are not initializing the scmi clock with discrete rates > correctly. We fetch the min_rate and max_rate value only for clocks with > ranges and ignore the ones with discrete rates. This will lead to wrong > initialization of rate range when clock supports discrete rate. > > Fix this by using the first and the last rate in the sorted list of the > discrete clock rates while registering the clock. > > Fixes: 6d6a1d82eaef7 ("clk: add support for clocks provided by SCMI") > Reported-by: Dien Pham <dien.pham.ry@renesas.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/clk/clk-scmi.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > Hi Stephen, > > If you fine, I can take this via ARM SoC along with the change in firmware > driver. But it is fine if you want to merge this independently as it should > be fine. Let me know either way. > > Regards, > Sudeep > > diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c > index c491f5de0f3f..ea65b7bf1408 100644 > --- a/drivers/clk/clk-scmi.c > +++ b/drivers/clk/clk-scmi.c > @@ -103,6 +103,8 @@ static const struct clk_ops scmi_clk_ops = { > static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) > { > int ret; > + unsigned long min_rate, max_rate; > + > struct clk_init_data init = { > .flags = CLK_GET_RATE_NOCACHE, > .num_parents = 0, > @@ -112,9 +114,23 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) > > sclk->hw.init = &init; > ret = devm_clk_hw_register(dev, &sclk->hw); > - if (!ret) > - clk_hw_set_rate_range(&sclk->hw, sclk->info->range.min_rate, > - sclk->info->range.max_rate); > + if (ret) > + return ret; > + > + if (sclk->info->rate_discrete) { > + int num_rates = sclk->info->list.num_rates; > + > + if (num_rates <= 0) > + return -EINVAL; > + > + min_rate = sclk->info->list.rates[0] I seem to have sent a version with ; missing above though I fixed but sent the old stale version as I had written a note to you 🙁 -- Regards, Sudeep ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] clk: scmi: Fix min and max rate when registering clocks with discrete rates @ 2020-07-08 20:24 ` Sudeep Holla 0 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-08 20:24 UTC (permalink / raw) To: linux-arm-kernel, linux-clk, Stephen Boyd Cc: Michael Turquette, Sudeep Holla, linux-kernel, Dien Pham On Wed, Jul 08, 2020 at 12:07:25PM +0100, Sudeep Holla wrote: > Currently we are not initializing the scmi clock with discrete rates > correctly. We fetch the min_rate and max_rate value only for clocks with > ranges and ignore the ones with discrete rates. This will lead to wrong > initialization of rate range when clock supports discrete rate. > > Fix this by using the first and the last rate in the sorted list of the > discrete clock rates while registering the clock. > > Fixes: 6d6a1d82eaef7 ("clk: add support for clocks provided by SCMI") > Reported-by: Dien Pham <dien.pham.ry@renesas.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/clk/clk-scmi.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > Hi Stephen, > > If you fine, I can take this via ARM SoC along with the change in firmware > driver. But it is fine if you want to merge this independently as it should > be fine. Let me know either way. > > Regards, > Sudeep > > diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c > index c491f5de0f3f..ea65b7bf1408 100644 > --- a/drivers/clk/clk-scmi.c > +++ b/drivers/clk/clk-scmi.c > @@ -103,6 +103,8 @@ static const struct clk_ops scmi_clk_ops = { > static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) > { > int ret; > + unsigned long min_rate, max_rate; > + > struct clk_init_data init = { > .flags = CLK_GET_RATE_NOCACHE, > .num_parents = 0, > @@ -112,9 +114,23 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk) > > sclk->hw.init = &init; > ret = devm_clk_hw_register(dev, &sclk->hw); > - if (!ret) > - clk_hw_set_rate_range(&sclk->hw, sclk->info->range.min_rate, > - sclk->info->range.max_rate); > + if (ret) > + return ret; > + > + if (sclk->info->rate_discrete) { > + int num_rates = sclk->info->list.num_rates; > + > + if (num_rates <= 0) > + return -EINVAL; > + > + min_rate = sclk->info->list.rates[0] I seem to have sent a version with ; missing above though I fixed but sent the old stale version as I had written a note to you 🙁 -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted 2020-07-08 11:07 ` Sudeep Holla @ 2020-07-09 8:20 ` Dien Pham -1 siblings, 0 replies; 15+ messages in thread From: Dien Pham @ 2020-07-09 8:20 UTC (permalink / raw) To: Sudeep Holla, linux-arm-kernel, linux-clk, Stephen Boyd Cc: linux-kernel, Michael Turquette Hi Sudeep, I share my build warning and some in-line comment below: CC drivers/firmware/arm_scmi/clock.o drivers/firmware/arm_scmi/clock.c: In function 'rate_cmp_func': drivers/firmware/arm_scmi/clock.c:127:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] u64 *r1 = _r1, *r2 = _r2; ^~~ drivers/firmware/arm_scmi/clock.c:127:23: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] u64 *r1 = _r1, *r2 = _r2; ^~~ CC arch/arm64/kernel/vdso.o drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >-----Original Message----- >From: Sudeep Holla <sudeep.holla@arm.com> >Sent: Wednesday, July 8, 2020 6:07 PM >To: linux-arm-kernel@lists.infradead.org; linux-clk@vger.kernel.org; Stephen Boyd <sboyd@kernel.org> >Cc: Sudeep Holla <sudeep.holla@arm.com>; linux-kernel@vger.kernel.org; Michael Turquette <mturquette@baylibre.com>; Dien Pham <dien.pham.ry@renesas.com> >Subject: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted > >Instead of relying on the firmware to keep the clock rates sorted, let us sort the list. This is not essential for clock layer but it helps to find the min and max rates easily from the list. > >Fixes: 5f6c6430e904 ("firmware: arm_scmi: add initial support for clock protocol") >Reported-by: Dien Pham <dien.pham.ry@renesas.com> >Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >--- > drivers/firmware/arm_scmi/clock.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > >Hi Dien-san, > >If you could review/test these patches, I can queue them ASAP. >I am planning to send the PR for ARM SoC later this week, so I need your tested-by. I applied the patch, Although there are some build warnings, but the patch effect is ok. > >Regards, >Sudeep > >diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c >index 4c2227662b26..2dd119cdebf6 100644 >--- a/drivers/firmware/arm_scmi/clock.c >+++ b/drivers/firmware/arm_scmi/clock.c >@@ -5,6 +5,8 @@ > * Copyright (C) 2018 ARM Ltd. > */ > >+#include <linux/sort.h> >+ > #include "common.h" > > enum scmi_clock_protocol_cmd { >@@ -121,6 +123,13 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, > return ret; > } > >+static int rate_cmp_func(const void *_r1, const void *_r2) { >+ u64 *r1 = _r1, *r2 = _r2; It is better to add 'const' as below to avoid warning. const u64 *r1 = _r1, *r2 = _r2; >+ >+ return r1 - r2; r1 and r2 are u64, but returned value is 'int' type. Do you think we should improve this ? e.g. return (int)r1 - r2; >+} >+ > static int > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > struct scmi_clock_info *clk) >@@ -184,8 +193,10 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > */ > } while (num_returned && num_remaining); > >- if (rate_discrete) >+ if (rate_discrete) { > clk->list.num_rates = tot_rate_cnt; >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); About warning of above line, I think it relates to below snip of code: if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); break; } I see that in this case is true, it is not proceeded as error case, If so I think you can update 'rate' for value from 'tot_rate_cnt' to SCMI_MAX_NUM_RATES at here. How do you think ? >+ } > > clk->rate_discrete = rate_discrete; > >-- >2.17.1 Best regard. DIEN Pham ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted @ 2020-07-09 8:20 ` Dien Pham 0 siblings, 0 replies; 15+ messages in thread From: Dien Pham @ 2020-07-09 8:20 UTC (permalink / raw) To: Sudeep Holla, linux-arm-kernel, linux-clk, Stephen Boyd Cc: Michael Turquette, linux-kernel Hi Sudeep, I share my build warning and some in-line comment below: CC drivers/firmware/arm_scmi/clock.o drivers/firmware/arm_scmi/clock.c: In function 'rate_cmp_func': drivers/firmware/arm_scmi/clock.c:127:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] u64 *r1 = _r1, *r2 = _r2; ^~~ drivers/firmware/arm_scmi/clock.c:127:23: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] u64 *r1 = _r1, *r2 = _r2; ^~~ CC arch/arm64/kernel/vdso.o drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >-----Original Message----- >From: Sudeep Holla <sudeep.holla@arm.com> >Sent: Wednesday, July 8, 2020 6:07 PM >To: linux-arm-kernel@lists.infradead.org; linux-clk@vger.kernel.org; Stephen Boyd <sboyd@kernel.org> >Cc: Sudeep Holla <sudeep.holla@arm.com>; linux-kernel@vger.kernel.org; Michael Turquette <mturquette@baylibre.com>; Dien Pham <dien.pham.ry@renesas.com> >Subject: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted > >Instead of relying on the firmware to keep the clock rates sorted, let us sort the list. This is not essential for clock layer but it helps to find the min and max rates easily from the list. > >Fixes: 5f6c6430e904 ("firmware: arm_scmi: add initial support for clock protocol") >Reported-by: Dien Pham <dien.pham.ry@renesas.com> >Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >--- > drivers/firmware/arm_scmi/clock.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > >Hi Dien-san, > >If you could review/test these patches, I can queue them ASAP. >I am planning to send the PR for ARM SoC later this week, so I need your tested-by. I applied the patch, Although there are some build warnings, but the patch effect is ok. > >Regards, >Sudeep > >diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c >index 4c2227662b26..2dd119cdebf6 100644 >--- a/drivers/firmware/arm_scmi/clock.c >+++ b/drivers/firmware/arm_scmi/clock.c >@@ -5,6 +5,8 @@ > * Copyright (C) 2018 ARM Ltd. > */ > >+#include <linux/sort.h> >+ > #include "common.h" > > enum scmi_clock_protocol_cmd { >@@ -121,6 +123,13 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, > return ret; > } > >+static int rate_cmp_func(const void *_r1, const void *_r2) { >+ u64 *r1 = _r1, *r2 = _r2; It is better to add 'const' as below to avoid warning. const u64 *r1 = _r1, *r2 = _r2; >+ >+ return r1 - r2; r1 and r2 are u64, but returned value is 'int' type. Do you think we should improve this ? e.g. return (int)r1 - r2; >+} >+ > static int > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > struct scmi_clock_info *clk) >@@ -184,8 +193,10 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > */ > } while (num_returned && num_remaining); > >- if (rate_discrete) >+ if (rate_discrete) { > clk->list.num_rates = tot_rate_cnt; >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); About warning of above line, I think it relates to below snip of code: if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); break; } I see that in this case is true, it is not proceeded as error case, If so I think you can update 'rate' for value from 'tot_rate_cnt' to SCMI_MAX_NUM_RATES at here. How do you think ? >+ } > > clk->rate_discrete = rate_discrete; > >-- >2.17.1 Best regard. DIEN Pham _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted 2020-07-09 8:20 ` Dien Pham @ 2020-07-09 8:38 ` Sudeep Holla -1 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-09 8:38 UTC (permalink / raw) To: Dien Pham Cc: linux-arm-kernel, linux-clk, Stephen Boyd, linux-kernel, Michael Turquette Hi Dien-san, On Thu, Jul 09, 2020 at 08:20:51AM +0000, Dien Pham wrote: > Hi Sudeep, > > I share my build warning and some in-line comment below: > > CC drivers/firmware/arm_scmi/clock.o > drivers/firmware/arm_scmi/clock.c: In function 'rate_cmp_func': > drivers/firmware/arm_scmi/clock.c:127:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > u64 *r1 = _r1, *r2 = _r2; > ^~~ > drivers/firmware/arm_scmi/clock.c:127:23: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > u64 *r1 = _r1, *r2 = _r2; > ^~~ > CC arch/arm64/kernel/vdso.o > drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': > drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] > sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Sorry for this. I noticed yesterday when I built but strangely I had created patches before I fixed these and sent them instead of fixed version. My mistake. > >-----Original Message----- > >From: Sudeep Holla <sudeep.holla@arm.com> > >Sent: Wednesday, July 8, 2020 6:07 PM > >To: linux-arm-kernel@lists.infradead.org; linux-clk@vger.kernel.org; Stephen Boyd <sboyd@kernel.org> > >Cc: Sudeep Holla <sudeep.holla@arm.com>; linux-kernel@vger.kernel.org; Michael Turquette <mturquette@baylibre.com>; Dien Pham <dien.pham.ry@renesas.com> > >Subject: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted > > > >Instead of relying on the firmware to keep the clock rates sorted, let us sort the list. This is not essential for clock layer but it helps to find the min and max rates easily from the list. > > > >Fixes: 5f6c6430e904 ("firmware: arm_scmi: add initial support for clock protocol") > >Reported-by: Dien Pham <dien.pham.ry@renesas.com> > >Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > >--- > > drivers/firmware/arm_scmi/clock.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > >Hi Dien-san, > > > >If you could review/test these patches, I can queue them ASAP. > >I am planning to send the PR for ARM SoC later this week, so I need your tested-by. > > I applied the patch, > Although there are some build warnings, but the patch effect is ok. > Thanks for testing. > > > >Regards, > >Sudeep > > > >diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c > >index 4c2227662b26..2dd119cdebf6 100644 > >--- a/drivers/firmware/arm_scmi/clock.c > >+++ b/drivers/firmware/arm_scmi/clock.c > >@@ -5,6 +5,8 @@ > > * Copyright (C) 2018 ARM Ltd. > > */ > > > >+#include <linux/sort.h> > >+ > > #include "common.h" > > > > enum scmi_clock_protocol_cmd { > >@@ -121,6 +123,13 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, > > return ret; > > } > > > >+static int rate_cmp_func(const void *_r1, const void *_r2) { > >+ u64 *r1 = _r1, *r2 = _r2; > > It is better to add 'const' as below to avoid warning. > const u64 *r1 = _r1, *r2 = _r2; > Yes, I have this in the correct version which I sent as v2 this morning. > >+ > >+ return r1 - r2; > > r1 and r2 are u64, but returned value is 'int' type. > Do you think we should improve this ? e.g. return (int)r1 - r2; > Not changing to const above must suffice. > >+} > >+ > > static int > > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > struct scmi_clock_info *clk) > >@@ -184,8 +193,10 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > */ > > } while (num_returned && num_remaining); > > > >- if (rate_discrete) > >+ if (rate_discrete) { > > clk->list.num_rates = tot_rate_cnt; > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > About warning of above line, I think it relates to below snip of code: > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > break; > } > I don't understand your comment and relation to above warning. > I see that in this case is true, it is not proceeded as error case, > If so I think you can update 'rate' for value from 'tot_rate_cnt' to SCMI_MAX_NUM_RATES at here. > How do you think ? > -- Regards, Sudeep ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted @ 2020-07-09 8:38 ` Sudeep Holla 0 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-09 8:38 UTC (permalink / raw) To: Dien Pham Cc: Stephen Boyd, Michael Turquette, linux-clk, linux-arm-kernel, linux-kernel Hi Dien-san, On Thu, Jul 09, 2020 at 08:20:51AM +0000, Dien Pham wrote: > Hi Sudeep, > > I share my build warning and some in-line comment below: > > CC drivers/firmware/arm_scmi/clock.o > drivers/firmware/arm_scmi/clock.c: In function 'rate_cmp_func': > drivers/firmware/arm_scmi/clock.c:127:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > u64 *r1 = _r1, *r2 = _r2; > ^~~ > drivers/firmware/arm_scmi/clock.c:127:23: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > u64 *r1 = _r1, *r2 = _r2; > ^~~ > CC arch/arm64/kernel/vdso.o > drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': > drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] > sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Sorry for this. I noticed yesterday when I built but strangely I had created patches before I fixed these and sent them instead of fixed version. My mistake. > >-----Original Message----- > >From: Sudeep Holla <sudeep.holla@arm.com> > >Sent: Wednesday, July 8, 2020 6:07 PM > >To: linux-arm-kernel@lists.infradead.org; linux-clk@vger.kernel.org; Stephen Boyd <sboyd@kernel.org> > >Cc: Sudeep Holla <sudeep.holla@arm.com>; linux-kernel@vger.kernel.org; Michael Turquette <mturquette@baylibre.com>; Dien Pham <dien.pham.ry@renesas.com> > >Subject: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted > > > >Instead of relying on the firmware to keep the clock rates sorted, let us sort the list. This is not essential for clock layer but it helps to find the min and max rates easily from the list. > > > >Fixes: 5f6c6430e904 ("firmware: arm_scmi: add initial support for clock protocol") > >Reported-by: Dien Pham <dien.pham.ry@renesas.com> > >Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > >--- > > drivers/firmware/arm_scmi/clock.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > >Hi Dien-san, > > > >If you could review/test these patches, I can queue them ASAP. > >I am planning to send the PR for ARM SoC later this week, so I need your tested-by. > > I applied the patch, > Although there are some build warnings, but the patch effect is ok. > Thanks for testing. > > > >Regards, > >Sudeep > > > >diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c > >index 4c2227662b26..2dd119cdebf6 100644 > >--- a/drivers/firmware/arm_scmi/clock.c > >+++ b/drivers/firmware/arm_scmi/clock.c > >@@ -5,6 +5,8 @@ > > * Copyright (C) 2018 ARM Ltd. > > */ > > > >+#include <linux/sort.h> > >+ > > #include "common.h" > > > > enum scmi_clock_protocol_cmd { > >@@ -121,6 +123,13 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, > > return ret; > > } > > > >+static int rate_cmp_func(const void *_r1, const void *_r2) { > >+ u64 *r1 = _r1, *r2 = _r2; > > It is better to add 'const' as below to avoid warning. > const u64 *r1 = _r1, *r2 = _r2; > Yes, I have this in the correct version which I sent as v2 this morning. > >+ > >+ return r1 - r2; > > r1 and r2 are u64, but returned value is 'int' type. > Do you think we should improve this ? e.g. return (int)r1 - r2; > Not changing to const above must suffice. > >+} > >+ > > static int > > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > struct scmi_clock_info *clk) > >@@ -184,8 +193,10 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > */ > > } while (num_returned && num_remaining); > > > >- if (rate_discrete) > >+ if (rate_discrete) { > > clk->list.num_rates = tot_rate_cnt; > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > About warning of above line, I think it relates to below snip of code: > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > break; > } > I don't understand your comment and relation to above warning. > I see that in this case is true, it is not proceeded as error case, > If so I think you can update 'rate' for value from 'tot_rate_cnt' to SCMI_MAX_NUM_RATES at here. > How do you think ? > -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted 2020-07-09 8:38 ` Sudeep Holla @ 2020-07-09 8:53 ` Dien Pham -1 siblings, 0 replies; 15+ messages in thread From: Dien Pham @ 2020-07-09 8:53 UTC (permalink / raw) To: Sudeep Holla Cc: linux-arm-kernel, linux-clk, Stephen Boyd, linux-kernel, Michael Turquette Dear Sudeep-san, > > >+} > > >+ > > > static int > > > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > > struct scmi_clock_info *clk) @@ -184,8 +193,10 @@ > > >scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > > */ > > > } while (num_returned && num_remaining); > > > > > >- if (rate_discrete) > > >+ if (rate_discrete) { > > > clk->list.num_rates = tot_rate_cnt; > > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > > > About warning of above line, I think it relates to below snip of code: > > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > > break; > > } > > > > I don't understand your comment and relation to above warning. I'd like to mention about below warning. >drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': >drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] > sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); The warning for line > > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); But, I think that it is affected by 'break' of below line. And for fixing this warning, I think we should fix inside this 'if' block. > > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > > break; > > } Best regard, DIEN Pham ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted @ 2020-07-09 8:53 ` Dien Pham 0 siblings, 0 replies; 15+ messages in thread From: Dien Pham @ 2020-07-09 8:53 UTC (permalink / raw) To: Sudeep Holla Cc: Stephen Boyd, Michael Turquette, linux-clk, linux-arm-kernel, linux-kernel Dear Sudeep-san, > > >+} > > >+ > > > static int > > > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > > struct scmi_clock_info *clk) @@ -184,8 +193,10 @@ > > >scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > > */ > > > } while (num_returned && num_remaining); > > > > > >- if (rate_discrete) > > >+ if (rate_discrete) { > > > clk->list.num_rates = tot_rate_cnt; > > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > > > About warning of above line, I think it relates to below snip of code: > > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > > break; > > } > > > > I don't understand your comment and relation to above warning. I'd like to mention about below warning. >drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': >drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] > sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); The warning for line > > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); But, I think that it is affected by 'break' of below line. And for fixing this warning, I think we should fix inside this 'if' block. > > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > > break; > > } Best regard, DIEN Pham _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted 2020-07-09 8:53 ` Dien Pham @ 2020-07-09 9:47 ` Sudeep Holla -1 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-09 9:47 UTC (permalink / raw) To: Dien Pham Cc: linux-arm-kernel, linux-clk, Stephen Boyd, linux-kernel, Sudeep Holla, Michael Turquette On Thu, Jul 09, 2020 at 08:53:44AM +0000, Dien Pham wrote: > Dear Sudeep-san, > > > > >+} > > > >+ > > > > static int > > > > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > > > struct scmi_clock_info *clk) @@ -184,8 +193,10 @@ > > > >scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > > > */ > > > > } while (num_returned && num_remaining); > > > > > > > >- if (rate_discrete) > > > >+ if (rate_discrete) { > > > > clk->list.num_rates = tot_rate_cnt; > > > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > > > > > About warning of above line, I think it relates to below snip of code: > > > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > > > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > > > break; > > > } > > > > > > > I don't understand your comment and relation to above warning. > > I'd like to mention about below warning. > > >drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': > >drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] Ah ok, sorry I didn't see this one. I am unable to observe this in the default build, I will check with W=1. Thanks for that. > > sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > The warning for line > > > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > But, I think that it is affected by 'break' of below line. And for fixing > this warning, I think we should fix inside this 'if' block. OK will take a look. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted @ 2020-07-09 9:47 ` Sudeep Holla 0 siblings, 0 replies; 15+ messages in thread From: Sudeep Holla @ 2020-07-09 9:47 UTC (permalink / raw) To: Dien Pham Cc: Stephen Boyd, Michael Turquette, linux-kernel, Sudeep Holla, linux-clk, linux-arm-kernel On Thu, Jul 09, 2020 at 08:53:44AM +0000, Dien Pham wrote: > Dear Sudeep-san, > > > > >+} > > > >+ > > > > static int > > > > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > > > struct scmi_clock_info *clk) @@ -184,8 +193,10 @@ > > > >scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > > > */ > > > > } while (num_returned && num_remaining); > > > > > > > >- if (rate_discrete) > > > >+ if (rate_discrete) { > > > > clk->list.num_rates = tot_rate_cnt; > > > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > > > > > About warning of above line, I think it relates to below snip of code: > > > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > > > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > > > break; > > > } > > > > > > > I don't understand your comment and relation to above warning. > > I'd like to mention about below warning. > > >drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': > >drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] Ah ok, sorry I didn't see this one. I am unable to observe this in the default build, I will check with W=1. Thanks for that. > > sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > The warning for line > > > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > But, I think that it is affected by 'break' of below line. And for fixing > this warning, I think we should fix inside this 'if' block. OK will take a look. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-07-09 9:51 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-08 11:07 [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted Sudeep Holla 2020-07-08 11:07 ` Sudeep Holla 2020-07-08 11:07 ` [PATCH 2/2] clk: scmi: Fix min and max rate when registering clocks with discrete rates Sudeep Holla 2020-07-08 11:07 ` Sudeep Holla 2020-07-08 18:44 ` kernel test robot 2020-07-08 20:24 ` Sudeep Holla 2020-07-08 20:24 ` Sudeep Holla 2020-07-09 8:20 ` [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted Dien Pham 2020-07-09 8:20 ` Dien Pham 2020-07-09 8:38 ` Sudeep Holla 2020-07-09 8:38 ` Sudeep Holla 2020-07-09 8:53 ` Dien Pham 2020-07-09 8:53 ` Dien Pham 2020-07-09 9:47 ` Sudeep Holla 2020-07-09 9:47 ` Sudeep Holla
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.