diff mbox series

soc: qcom: Rework BCM_TCS_CMD macro

Message ID 20241025084823.475199-1-eugen.hristev@linaro.org (mailing list archive)
State Superseded
Headers show
Series soc: qcom: Rework BCM_TCS_CMD macro | expand

Commit Message

Eugen Hristev Oct. 25, 2024, 8:48 a.m. UTC
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(-)

Comments

Konrad Dybcio Oct. 25, 2024, 9:03 a.m. UTC | #1
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
Eugen Hristev Oct. 25, 2024, 12:06 p.m. UTC | #2
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 ?
Konrad Dybcio Oct. 25, 2024, 5:03 p.m. UTC | #3
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
kernel test robot Oct. 25, 2024, 9:06 p.m. UTC | #4
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
kernel test robot Oct. 26, 2024, 7:47 a.m. UTC | #5
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
Eugen Hristev Oct. 28, 2024, 12:22 p.m. UTC | #6
On 10/25/24 20:03, Konrad Dybcio wrote:
> 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?

That works too. It would move the `__force u32` to a higher level, but 
works. I am sending a v2 with that.

> 
> Konrad
diff mbox series

Patch

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__ */