Re: [PATCH 11/16] clk: qcom: smd-rpm: simplify XO_BUFFER clocks definitions

Dmitry Baryshkov
2022-12-06T23:30:50Z

On 05/12/2022 19:05, Alex Elder wrote:
> On 12/3/22 11:58 AM, Dmitry Baryshkov wrote:
>> Remove the duplication between the names of the normal and active-only
>> XO_BUFFER and XO_BUFFER_PINCTRL clocks by using preprocessor logic to
>> add _a suffix.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> This is nice.  See two comments below.
>
> Reviewed-by: Alex Elder <elder@linaro.org>
>
>> ---
>>   drivers/clk/qcom/clk-smd-rpm.c | 54 +++++++++++++++++-----------------
>>   1 file changed, 27 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c
>> b/drivers/clk/qcom/clk-smd-rpm.c
>> index cb47d69889fb..9f33dbd60e96 100644
>> --- a/drivers/clk/qcom/clk-smd-rpm.c
>> +++ b/drivers/clk/qcom/clk-smd-rpm.c
>> @@ -112,17 +112,17 @@
>>           __DEFINE_CLK_SMD_RPM(_platform, _name, _active, type, r_id,   \
>>           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,          \
>> +#define DEFINE_CLK_SMD_RPM_XO_BUFFER(_platform, _name, r_id,
>> r)              \
>> +        __DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _name##_a,      \
>>           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,          \
>> +#define DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(_platform,
>> _name,              \
>>                            r_id, r)                  \
>
> Can the above line be merged with its predecessor?
>
> (I now have looked at later patches, and I see you add a new argument
> that makes this original alignment still make sense.  If that's why
> you didn't here, you've done the right thing.)

Let's fix it in this patch, to remove possible questions.

>
>> -        DEFINE_CLK_SMD_RPM_XO_BUFFER(_platform, _name,
>> _active,          \
>> +        DEFINE_CLK_SMD_RPM_XO_BUFFER(_platform, _name,              \
>>                            r_id, r);                  \
>
> Same comment here.
>

--
With best wishes
Dmitry