Re: [PATCH 08/16] clk: qcom: smd-rpm: drop the rpm_status_id field

Dmitry Baryshkov
2022-12-05T21:38:23Z

On 5 December 2022 20:04:54 GMT+03:00, Alex Elder <elder@linaro.org> wrote:
>On 12/3/22 11:58 AM, Dmitry Baryshkov wrote:
>> The rpm_status_id field is a leftover from the non-SMD clocks. It is of
>> no use for the SMD-RPM clock driver and is always equal to zero. Drop it
>> completely.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>Since the macro is defining static structures it's fine, but it
>could be reassuring to see the rpm_status_id explicitly assigned
>to 0. (Just a comment, no change needed.) It's a good change.

Excuse me, probably I misunderstood your comment. The field is removed completely, so there is no place for it to be assigned to 0.

>
>Reviewed-by: Alex Elder <elder@linaro.org>
>
>> ---
>> drivers/clk/qcom/clk-smd-rpm.c | 20 +++++++-------------
>> 1 file changed, 7 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
>> index e52e0e242294..828cae6769f9 100644
>> --- a/drivers/clk/qcom/clk-smd-rpm.c
>> +++ b/drivers/clk/qcom/clk-smd-rpm.c
>> @@ -25,13 +25,11 @@
>> #define QCOM_RPM_SMD_KEY_STATE 0x54415453
>> #define QCOM_RPM_SCALING_ENABLE_ID 0x2
>> -#define __DEFINE_CLK_SMD_RPM(_platform, _name, _active, type, r_id, stat_id, \
>> - key) \
>> +#define __DEFINE_CLK_SMD_RPM(_platform, _name, _active, type, r_id, key) \
>> static struct clk_smd_rpm _platform##_##_active; \
>> static struct clk_smd_rpm _platform##_##_name = { \
>> .rpm_res_type = (type), \
>> .rpm_clk_id = (r_id), \
>> - .rpm_status_id = (stat_id), \
>> .rpm_key = (key), \
>> .peer = &_platform##_##_active, \
>> .rate = INT_MAX, \
>> @@ -48,7 +46,6 @@
>> static struct clk_smd_rpm _platform##_##_active = { \
>> .rpm_res_type = (type), \
>> .rpm_clk_id = (r_id), \
>> - .rpm_status_id = (stat_id), \
>> .active_only = true, \
>> .rpm_key = (key), \
>> .peer = &_platform##_##_name, \
>> @@ -65,12 +62,11 @@
>> }
>> #define __DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, type, r_id, \
>> - stat_id, r, key) \
>> + r, key) \
>> static struct clk_smd_rpm _platform##_##_active; \
>> static struct clk_smd_rpm _platform##_##_name = { \
>> .rpm_res_type = (type), \
>> .rpm_clk_id = (r_id), \
>> - .rpm_status_id = (stat_id), \
>> .rpm_key = (key), \
>> .branch = true, \
>> .peer = &_platform##_##_active, \
>> @@ -88,7 +84,6 @@
>> static struct clk_smd_rpm _platform##_##_active = { \
>> .rpm_res_type = (type), \
>> .rpm_clk_id = (r_id), \
>> - .rpm_status_id = (stat_id), \
>> .active_only = true, \
>> .rpm_key = (key), \
>> .branch = true, \
>> @@ -107,19 +102,19 @@
>> #define DEFINE_CLK_SMD_RPM(_platform, _name, _active, type, r_id) \
>> __DEFINE_CLK_SMD_RPM(_platform, _name, _active, type, r_id, \
>> - 0, QCOM_RPM_SMD_KEY_RATE)
>> + QCOM_RPM_SMD_KEY_RATE)
>> #define DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, type, r_id, r) \
>> __DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, type, \
>> - r_id, 0, r, QCOM_RPM_SMD_KEY_ENABLE)
>> + r_id, r, QCOM_RPM_SMD_KEY_ENABLE)
>> #define DEFINE_CLK_SMD_RPM_QDSS(_platform, _name, _active, type, r_id) \
>> __DEFINE_CLK_SMD_RPM(_platform, _name, _active, type, r_id, \
>> - 0, QCOM_RPM_SMD_KEY_STATE)
>> + QCOM_RPM_SMD_KEY_STATE)
>> #define DEFINE_CLK_SMD_RPM_XO_BUFFER(_platform, _name, _active, r_id, r) \
>> __DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, \
>> - QCOM_SMD_RPM_CLK_BUF_A, r_id, 0, r, \
>> + QCOM_SMD_RPM_CLK_BUF_A, r_id, r, \
>> QCOM_RPM_KEY_SOFTWARE_ENABLE)
>> #define DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(_platform, _name, _active, \
>> @@ -128,7 +123,7 @@
>> r_id, r); \
>> __DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name##_pin, \
>> _active##_pin, \
>> - QCOM_SMD_RPM_CLK_BUF_A, r_id, 0, r, \
>> + QCOM_SMD_RPM_CLK_BUF_A, r_id, r, \
>> QCOM_RPM_KEY_PIN_CTRL_CLK_BUFFER_ENABLE_KEY)
>> #define to_clk_smd_rpm(_hw) container_of(_hw, struct clk_smd_rpm, hw)
>> @@ -137,7 +132,6 @@ struct clk_smd_rpm {
>> const int rpm_res_type;
>> const int rpm_key;
>> const int rpm_clk_id;
>> - const int rpm_status_id;
>> const bool active_only;
>> bool enabled;
>> bool branch;
>

--
With best wishes
Dmitry