| Message ID | 20241025084823.475199-1-eugen.hristev@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | soc: qcom: Rework BCM_TCS_CMD macro | expand |
On 25.10.2024 10:48 AM, Eugen Hristev wrote: > Reworked BCM_TCS_CMD macro in order to fix warnings from sparse: > > drivers/clk/qcom/clk-rpmh.c:270:28: warning: restricted __le32 degrades to integer > drivers/clk/qcom/clk-rpmh.c:270:28: warning: restricted __le32 degrades to integer > > While at it, used u32_encode_bits which made the code easier to > follow and removed unnecessary shift definitions. > > Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org> > --- > include/soc/qcom/tcs.h | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff --git a/include/soc/qcom/tcs.h b/include/soc/qcom/tcs.h > index 3acca067c72b..130ed2582f37 100644 > --- a/include/soc/qcom/tcs.h > +++ b/include/soc/qcom/tcs.h > @@ -60,22 +60,19 @@ struct tcs_request { > struct tcs_cmd *cmds; > }; > > -#define BCM_TCS_CMD_COMMIT_SHFT 30 > #define BCM_TCS_CMD_COMMIT_MASK 0x40000000 > -#define BCM_TCS_CMD_VALID_SHFT 29 > #define BCM_TCS_CMD_VALID_MASK 0x20000000 > -#define BCM_TCS_CMD_VOTE_X_SHFT 14 > #define BCM_TCS_CMD_VOTE_MASK 0x3fff > -#define BCM_TCS_CMD_VOTE_Y_SHFT 0 > -#define BCM_TCS_CMD_VOTE_Y_MASK 0xfffc000 > +#define BCM_TCS_CMD_VOTE_Y_MASK 0x3fff > +#define BCM_TCS_CMD_VOTE_X_MASK 0xfffc000 > > /* Construct a Bus Clock Manager (BCM) specific TCS command */ > #define BCM_TCS_CMD(commit, valid, vote_x, vote_y) \ > - (((commit) << BCM_TCS_CMD_COMMIT_SHFT) | \ > - ((valid) << BCM_TCS_CMD_VALID_SHFT) | \ > - ((cpu_to_le32(vote_x) & \ > - BCM_TCS_CMD_VOTE_MASK) << BCM_TCS_CMD_VOTE_X_SHFT) | \ > - ((cpu_to_le32(vote_y) & \ > - BCM_TCS_CMD_VOTE_MASK) << BCM_TCS_CMD_VOTE_Y_SHFT)) > + (u32_encode_bits(commit, BCM_TCS_CMD_COMMIT_MASK) | \ > + u32_encode_bits(valid, BCM_TCS_CMD_VALID_MASK) | \ > + u32_encode_bits((__force u32)cpu_to_le32(vote_x), \ > + BCM_TCS_CMD_VOTE_X_MASK) | \ > + u32_encode_bits((__force u32)cpu_to_le32(vote_y), \ > + BCM_TCS_CMD_VOTE_Y_MASK)) FIELD_PREP/GET? Konrad
On 10/25/24 12:03, Konrad Dybcio wrote: > On 25.10.2024 10:48 AM, Eugen Hristev wrote: >> Reworked BCM_TCS_CMD macro in order to fix warnings from sparse: >> >> drivers/clk/qcom/clk-rpmh.c:270:28: warning: restricted __le32 degrades to integer >> drivers/clk/qcom/clk-rpmh.c:270:28: warning: restricted __le32 degrades to integer >> >> While at it, used u32_encode_bits which made the code easier to >> follow and removed unnecessary shift definitions. >> >> Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org> >> --- >> include/soc/qcom/tcs.h | 19 ++++++++----------- >> 1 file changed, 8 insertions(+), 11 deletions(-) >> >> diff --git a/include/soc/qcom/tcs.h b/include/soc/qcom/tcs.h >> index 3acca067c72b..130ed2582f37 100644 >> --- a/include/soc/qcom/tcs.h >> +++ b/include/soc/qcom/tcs.h >> @@ -60,22 +60,19 @@ struct tcs_request { >> struct tcs_cmd *cmds; >> }; >> >> -#define BCM_TCS_CMD_COMMIT_SHFT 30 >> #define BCM_TCS_CMD_COMMIT_MASK 0x40000000 >> -#define BCM_TCS_CMD_VALID_SHFT 29 >> #define BCM_TCS_CMD_VALID_MASK 0x20000000 >> -#define BCM_TCS_CMD_VOTE_X_SHFT 14 >> #define BCM_TCS_CMD_VOTE_MASK 0x3fff >> -#define BCM_TCS_CMD_VOTE_Y_SHFT 0 >> -#define BCM_TCS_CMD_VOTE_Y_MASK 0xfffc000 >> +#define BCM_TCS_CMD_VOTE_Y_MASK 0x3fff >> +#define BCM_TCS_CMD_VOTE_X_MASK 0xfffc000 >> >> /* Construct a Bus Clock Manager (BCM) specific TCS command */ >> #define BCM_TCS_CMD(commit, valid, vote_x, vote_y) \ >> - (((commit) << BCM_TCS_CMD_COMMIT_SHFT) | \ >> - ((valid) << BCM_TCS_CMD_VALID_SHFT) | \ >> - ((cpu_to_le32(vote_x) & \ >> - BCM_TCS_CMD_VOTE_MASK) << BCM_TCS_CMD_VOTE_X_SHFT) | \ >> - ((cpu_to_le32(vote_y) & \ >> - BCM_TCS_CMD_VOTE_MASK) << BCM_TCS_CMD_VOTE_Y_SHFT)) >> + (u32_encode_bits(commit, BCM_TCS_CMD_COMMIT_MASK) | \ >> + u32_encode_bits(valid, BCM_TCS_CMD_VALID_MASK) | \ >> + u32_encode_bits((__force u32)cpu_to_le32(vote_x), \ >> + BCM_TCS_CMD_VOTE_X_MASK) | \ >> + u32_encode_bits((__force u32)cpu_to_le32(vote_y), \ >> + BCM_TCS_CMD_VOTE_Y_MASK)) > > FIELD_PREP/GET? > > Konrad What would be the difference/advantage in using FIELD_PREP/GET instead of u32_encode_bits ?
On 25.10.2024 2:06 PM, Eugen Hristev wrote: > > > On 10/25/24 12:03, Konrad Dybcio wrote: >> On 25.10.2024 10:48 AM, Eugen Hristev wrote: >>> Reworked BCM_TCS_CMD macro in order to fix warnings from sparse: >>> >>> drivers/clk/qcom/clk-rpmh.c:270:28: warning: restricted __le32 degrades to integer >>> drivers/clk/qcom/clk-rpmh.c:270:28: warning: restricted __le32 degrades to integer >>> >>> While at it, used u32_encode_bits which made the code easier to >>> follow and removed unnecessary shift definitions. >>> >>> Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org> >>> --- >>> include/soc/qcom/tcs.h | 19 ++++++++----------- >>> 1 file changed, 8 insertions(+), 11 deletions(-) >>> >>> diff --git a/include/soc/qcom/tcs.h b/include/soc/qcom/tcs.h >>> index 3acca067c72b..130ed2582f37 100644 >>> --- a/include/soc/qcom/tcs.h >>> +++ b/include/soc/qcom/tcs.h >>> @@ -60,22 +60,19 @@ struct tcs_request { >>> struct tcs_cmd *cmds; >>> }; >>> -#define BCM_TCS_CMD_COMMIT_SHFT 30 >>> #define BCM_TCS_CMD_COMMIT_MASK 0x40000000 >>> -#define BCM_TCS_CMD_VALID_SHFT 29 >>> #define BCM_TCS_CMD_VALID_MASK 0x20000000 >>> -#define BCM_TCS_CMD_VOTE_X_SHFT 14 >>> #define BCM_TCS_CMD_VOTE_MASK 0x3fff >>> -#define BCM_TCS_CMD_VOTE_Y_SHFT 0 >>> -#define BCM_TCS_CMD_VOTE_Y_MASK 0xfffc000 >>> +#define BCM_TCS_CMD_VOTE_Y_MASK 0x3fff >>> +#define BCM_TCS_CMD_VOTE_X_MASK 0xfffc000 >>> /* Construct a Bus Clock Manager (BCM) specific TCS command */ >>> #define BCM_TCS_CMD(commit, valid, vote_x, vote_y) \ >>> - (((commit) << BCM_TCS_CMD_COMMIT_SHFT) | \ >>> - ((valid) << BCM_TCS_CMD_VALID_SHFT) | \ >>> - ((cpu_to_le32(vote_x) & \ >>> - BCM_TCS_CMD_VOTE_MASK) << BCM_TCS_CMD_VOTE_X_SHFT) | \ >>> - ((cpu_to_le32(vote_y) & \ >>> - BCM_TCS_CMD_VOTE_MASK) << BCM_TCS_CMD_VOTE_Y_SHFT)) >>> + (u32_encode_bits(commit, BCM_TCS_CMD_COMMIT_MASK) | \ >>> + u32_encode_bits(valid, BCM_TCS_CMD_VALID_MASK) | \ >>> + u32_encode_bits((__force u32)cpu_to_le32(vote_x), \ >>> + BCM_TCS_CMD_VOTE_X_MASK) | \ >>> + u32_encode_bits((__force u32)cpu_to_le32(vote_y), \ >>> + BCM_TCS_CMD_VOTE_Y_MASK)) >> >> FIELD_PREP/GET? >> >> Konrad > > What would be the difference/advantage in using FIELD_PREP/GET instead of u32_encode_bits ? Probably none. I thought it was a function and not another magic macro, as it's lowercase.. Doesn't le32_encode_bits do what you need then? Konrad
Hi Eugen,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.12-rc4 next-20241025]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Eugen-Hristev/soc-qcom-Rework-BCM_TCS_CMD-macro/20241025-165212
base: linus/master
patch link: https://lore.kernel.org/r/20241025084823.475199-1-eugen.hristev%40linaro.org
patch subject: [PATCH] soc: qcom: Rework BCM_TCS_CMD macro
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20241026/202410260419.qGKBvJHZ-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241026/202410260419.qGKBvJHZ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410260419.qGKBvJHZ-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/soc/qcom/rpmh.h:9,
from drivers/clk/qcom/clk-rpmh.c:13:
drivers/clk/qcom/clk-rpmh.c: In function 'clk_rpmh_bcm_send_cmd':
>> include/soc/qcom/tcs.h:71:10: error: implicit declaration of function 'u32_encode_bits' [-Werror=implicit-function-declaration]
71 | (u32_encode_bits(commit, BCM_TCS_CMD_COMMIT_MASK) | \
| ^~~~~~~~~~~~~~~
drivers/clk/qcom/clk-rpmh.c:270:28: note: in expansion of macro 'BCM_TCS_CMD'
270 | cmd.data = BCM_TCS_CMD(1, enable, 0, cmd_state);
| ^~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/u32_encode_bits +71 include/soc/qcom/tcs.h
68
69 /* Construct a Bus Clock Manager (BCM) specific TCS command */
70 #define BCM_TCS_CMD(commit, valid, vote_x, vote_y) \
> 71 (u32_encode_bits(commit, BCM_TCS_CMD_COMMIT_MASK) | \
72 u32_encode_bits(valid, BCM_TCS_CMD_VALID_MASK) | \
73 u32_encode_bits((__force u32)cpu_to_le32(vote_x), \
74 BCM_TCS_CMD_VOTE_X_MASK) | \
75 u32_encode_bits((__force u32)cpu_to_le32(vote_y), \
76 BCM_TCS_CMD_VOTE_Y_MASK))
77
Hi Eugen,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.12-rc4 next-20241025]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Eugen-Hristev/soc-qcom-Rework-BCM_TCS_CMD-macro/20241025-165212
base: linus/master
patch link: https://lore.kernel.org/r/20241025084823.475199-1-eugen.hristev%40linaro.org
patch subject: [PATCH] soc: qcom: Rework BCM_TCS_CMD macro
config: x86_64-buildonly-randconfig-004-20241026 (https://download.01.org/0day-ci/archive/20241026/202410261552.E8TiGYdV-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241026/202410261552.E8TiGYdV-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410261552.E8TiGYdV-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/clk/qcom/clk-rpmh.c:270:14: error: call to undeclared function 'u32_encode_bits'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
270 | cmd.data = BCM_TCS_CMD(1, enable, 0, cmd_state);
| ^
include/soc/qcom/tcs.h:71:3: note: expanded from macro 'BCM_TCS_CMD'
71 | (u32_encode_bits(commit, BCM_TCS_CMD_COMMIT_MASK) | \
| ^
1 error generated.
vim +/u32_encode_bits +270 drivers/clk/qcom/clk-rpmh.c
9c7e47025a6b9a Taniya Das 2018-05-09 250
04053f4d23a41b David Dai 2019-01-24 251 static int clk_rpmh_bcm_send_cmd(struct clk_rpmh *c, bool enable)
04053f4d23a41b David Dai 2019-01-24 252 {
04053f4d23a41b David Dai 2019-01-24 253 struct tcs_cmd cmd = { 0 };
04053f4d23a41b David Dai 2019-01-24 254 u32 cmd_state;
2cf7a4cbcb4e10 Stephen Boyd 2020-03-09 255 int ret = 0;
04053f4d23a41b David Dai 2019-01-24 256
04053f4d23a41b David Dai 2019-01-24 257 mutex_lock(&rpmh_clk_lock);
04053f4d23a41b David Dai 2019-01-24 258 if (enable) {
04053f4d23a41b David Dai 2019-01-24 259 cmd_state = 1;
04053f4d23a41b David Dai 2019-01-24 260 if (c->aggr_state)
04053f4d23a41b David Dai 2019-01-24 261 cmd_state = c->aggr_state;
2cf7a4cbcb4e10 Stephen Boyd 2020-03-09 262 } else {
2cf7a4cbcb4e10 Stephen Boyd 2020-03-09 263 cmd_state = 0;
04053f4d23a41b David Dai 2019-01-24 264 }
04053f4d23a41b David Dai 2019-01-24 265
a4e5af27e6f6a8 Mike Tipton 2024-08-09 266 cmd_state = min(cmd_state, BCM_TCS_CMD_VOTE_MASK);
a4e5af27e6f6a8 Mike Tipton 2024-08-09 267
2cf7a4cbcb4e10 Stephen Boyd 2020-03-09 268 if (c->last_sent_aggr_state != cmd_state) {
04053f4d23a41b David Dai 2019-01-24 269 cmd.addr = c->res_addr;
6311b6521bcc80 Jordan Crouse 2019-08-05 @270 cmd.data = BCM_TCS_CMD(1, enable, 0, cmd_state);
04053f4d23a41b David Dai 2019-01-24 271
29f66b625281a3 Stephen Boyd 2022-05-17 272 /*
29f66b625281a3 Stephen Boyd 2022-05-17 273 * Send only an active only state request. RPMh continues to
29f66b625281a3 Stephen Boyd 2022-05-17 274 * use the active state when we're in sleep/wake state as long
29f66b625281a3 Stephen Boyd 2022-05-17 275 * as the sleep/wake state has never been set.
29f66b625281a3 Stephen Boyd 2022-05-17 276 */
dad4e7fda4bdc1 Mike Tipton 2020-02-14 277 ret = clk_rpmh_send(c, RPMH_ACTIVE_ONLY_STATE, &cmd, enable);
04053f4d23a41b David Dai 2019-01-24 278 if (ret) {
04053f4d23a41b David Dai 2019-01-24 279 dev_err(c->dev, "set active state of %s failed: (%d)\n",
04053f4d23a41b David Dai 2019-01-24 280 c->res_name, ret);
2cf7a4cbcb4e10 Stephen Boyd 2020-03-09 281 } else {
04053f4d23a41b David Dai 2019-01-24 282 c->last_sent_aggr_state = cmd_state;
2cf7a4cbcb4e10 Stephen Boyd 2020-03-09 283 }
2cf7a4cbcb4e10 Stephen Boyd 2020-03-09 284 }
04053f4d23a41b David Dai 2019-01-24 285
04053f4d23a41b David Dai 2019-01-24 286 mutex_unlock(&rpmh_clk_lock);
04053f4d23a41b David Dai 2019-01-24 287
2cf7a4cbcb4e10 Stephen Boyd 2020-03-09 288 return ret;
04053f4d23a41b David Dai 2019-01-24 289 }
04053f4d23a41b David Dai 2019-01-24 290
diff --git a/include/soc/qcom/tcs.h b/include/soc/qcom/tcs.h index 3acca067c72b..130ed2582f37 100644 --- a/include/soc/qcom/tcs.h +++ b/include/soc/qcom/tcs.h @@ -60,22 +60,19 @@ struct tcs_request { struct tcs_cmd *cmds; }; -#define BCM_TCS_CMD_COMMIT_SHFT 30 #define BCM_TCS_CMD_COMMIT_MASK 0x40000000 -#define BCM_TCS_CMD_VALID_SHFT 29 #define BCM_TCS_CMD_VALID_MASK 0x20000000 -#define BCM_TCS_CMD_VOTE_X_SHFT 14 #define BCM_TCS_CMD_VOTE_MASK 0x3fff -#define BCM_TCS_CMD_VOTE_Y_SHFT 0 -#define BCM_TCS_CMD_VOTE_Y_MASK 0xfffc000 +#define BCM_TCS_CMD_VOTE_Y_MASK 0x3fff +#define BCM_TCS_CMD_VOTE_X_MASK 0xfffc000 /* Construct a Bus Clock Manager (BCM) specific TCS command */ #define BCM_TCS_CMD(commit, valid, vote_x, vote_y) \ - (((commit) << BCM_TCS_CMD_COMMIT_SHFT) | \ - ((valid) << BCM_TCS_CMD_VALID_SHFT) | \ - ((cpu_to_le32(vote_x) & \ - BCM_TCS_CMD_VOTE_MASK) << BCM_TCS_CMD_VOTE_X_SHFT) | \ - ((cpu_to_le32(vote_y) & \ - BCM_TCS_CMD_VOTE_MASK) << BCM_TCS_CMD_VOTE_Y_SHFT)) + (u32_encode_bits(commit, BCM_TCS_CMD_COMMIT_MASK) | \ + u32_encode_bits(valid, BCM_TCS_CMD_VALID_MASK) | \ + u32_encode_bits((__force u32)cpu_to_le32(vote_x), \ + BCM_TCS_CMD_VOTE_X_MASK) | \ + u32_encode_bits((__force u32)cpu_to_le32(vote_y), \ + BCM_TCS_CMD_VOTE_Y_MASK)) #endif /* __SOC_QCOM_TCS_H__ */
Reworked BCM_TCS_CMD macro in order to fix warnings from sparse: drivers/clk/qcom/clk-rpmh.c:270:28: warning: restricted __le32 degrades to integer drivers/clk/qcom/clk-rpmh.c:270:28: warning: restricted __le32 degrades to integer While at it, used u32_encode_bits which made the code easier to follow and removed unnecessary shift definitions. Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org> --- include/soc/qcom/tcs.h | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)