Ping.... :)
On 2018/5/8 10:19, Shawn Lin wrote:
> Hi Michael and Stephen
>
> Any chance to take a look at this patch? :)
>
> On 2018/4/8 15:28, Shawn Lin wrote:
>> CLK_DIVIDER_EVEN is used for clock divders that should only
>> use even number in the div field.
>>
>> Two clock divder should consider to use this flag
>> (1) The divder is physically only support even div number to
>> generate 50% duty cycle clock rate.
>> (2) The divder's clock consumer request it to use even div number
>> to generate the most closest requested rate for whatever reason.
>>
>> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
>> request divder to use even number when working at a high throughput
>> speed mode reliably. However, that wasn't guaranteed by clock framework.
>> So the previous tricky is to carefully assign magic clock rate to their
>> parents as well as consumer's clock rate when requesting. That works
>> bad in practice if folks change the parents clock rate or the clock
>> hierarchy randomly. That also work bad if the consumer's clock rate
>> came from the DT, which is changed so fraquent for different boards.
>>
>> To make it's less prone to make mistake and to make it really respect
>> the fact that the divder should use even number to the div field, we
>> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
>> which could guarantee the div field is even number, however, obviously
>> it skips the even numbers which isn't the power of 2, but maybe which is
>> the best div to generate closest clock rate for consumer.
>>
>> Look at the examples here when doing some random test on Rockchip's
>> platforms, by changing the requested clock rate from DT, namely
>> assigning different max-frquency for MMC node,
>>
>> when the mmc host driver requests 80MHz with CLK_DIVIDER_POWER_OF_TWO
>> flag for the clock divder, it shows the final clock rate is 61.44Mhz
>>
>> pll_vpll0??????? 1??????? 1 983039999???? 0?? 0
>> ??? vpll0??????? 4??????? 4 983039999???? 0?? 0
>> ????? clk_emmc_div?????? 1??????? 1 122880000???? 0?? 0
>> ????clk_emmc??? 1??????? 1 122880000???? 0?? 0
>> ????? emmc_sample??? 0??????? 0? 61440000??? 0 155
>> ????? emmc_drv??? 0??????? 0? 61440000??? 0 180
>>
>> With this patch and add CLK_DIVIDER_EVEN flag for clk_emmc_div, we get
>> the final clock rate, 67.7376MHz.
>>
>> pll_vpll1?????????????? 1??????? 1 812851199??? 0?? 0
>> ??? vpll1??????????????? 2??????? 2 812851199???? 0?? 0
>> ????? clk_emmc_div?????? 1??????? 1 135475200???? 0?? 0
>> ????clk_emmc??? 1??????? 1 135475200???? 0?? 0
>> ????? emmc_sample?? 0??????? 0? 67737600???? 0 113
>> ????? emmc_drv??? 0??????? 0? 67737600??? 0 180
>>
>> Apprently 67737600 is better than 61440000 when requesting 80MHz.
>> Of course, we could have more case that worsen the gap between
>> the desired rate and the actual rate if using CLK_DIVIDER_POWER_OF_TWO.
>>
>> Alternatively, clk_div_table could be resorted to handle different kinds
>> of div limilation, and it seems to be applied to irregular div
>> calculation
>> in practice by current clock provider drivers, but even number for div
>> field is a rather common, regular and symmetrical requirement. So it is
>> worth to introduces CLK_DIVIDER_EVEN flag for clock framework to make
>> the best in this process.
>>
>> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com>
>>
>> ---
>>
>> Changes in v3:
>> - Fix a wrong if condition check
>>
>> Changes in v2:
>> - Avoid to use div 1 if (CLK_DIVIDER_EVEN | CLK_DIVIDER_POWER_OF_TWO)
>> ?? suggested by Heiko. And seems actually no other flags should be
>> bothered
>> ?? by this newly added CLK_DIVIDER_EVEN.
>>
>> ? drivers/clk/clk-divider.c??? | 26 ++++++++++++++++++++++++--
>> ? include/linux/clk-provider.h |? 3 +++
>> ? 2 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>> index b6234a5..8f305f1 100644
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -159,8 +159,17 @@ static bool _is_valid_table_div(const struct
>> clk_div_table *table,
>> ? static bool _is_valid_div(const struct clk_div_table *table,
>> unsigned int div,
>> ??????????????? unsigned long flags)
>> ? {
>> -??? if (flags & CLK_DIVIDER_POWER_OF_TWO)
>> -??????? return is_power_of_2(div);
>> +??? bool is_valid;
>> +
>> +??? if (flags & CLK_DIVIDER_POWER_OF_TWO) {
>> +??????? is_valid = is_power_of_2(div);
>> +??????? if (flags & CLK_DIVIDER_EVEN)
>> +??????????? return is_valid && (div != 1);
>> +??????? return is_valid;
>> +??? }
>> +
>> +??? if (flags & CLK_DIVIDER_EVEN)
>> +??????? return !(div % 2);
>> ????? if (table)
>> ????????? return _is_valid_table_div(table, div);
>> ????? return true;
>> @@ -208,6 +217,12 @@ static int _div_round_up(const struct
>> clk_div_table *table,
>> ? {
>> ????? int div = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
>> +??? /*
>> +???? * Check even before power of two to avoid div 1 if combination
>> +???? * happens, which applies to all the following similarities.
>> +???? */
>> +??? if (flags & CLK_DIVIDER_EVEN)
>> +??????? div = !(div % 2) ? div : (div + 1);
>> ????? if (flags & CLK_DIVIDER_POWER_OF_TWO)
>> ????????? div = __roundup_pow_of_two(div);
>> ????? if (table)
>> @@ -226,6 +241,11 @@ static int _div_round_closest(const struct
>> clk_div_table *table,
>> ????? up = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
>> ????? down = parent_rate / rate;
>> +??? if (flags & CLK_DIVIDER_EVEN) {
>> +??????? up = !(up % 2) ? up : (up + 1);
>> +??????? down = !(down % 2) ? down : (down - 1);
>> +??? }
>> +
>> ????? if (flags & CLK_DIVIDER_POWER_OF_TWO) {
>> ????????? up = __roundup_pow_of_two(up);
>> ????????? down = __rounddown_pow_of_two(down);
>> @@ -264,6 +284,8 @@ static int _next_div(const struct clk_div_table
>> *table, int div,
>> ? {
>> ????? div++;
>> +??? if (flags & CLK_DIVIDER_EVEN)
>> +??????? div = !(div % 2) ? div : (div + 1);
>> ????? if (flags & CLK_DIVIDER_POWER_OF_TWO)
>> ????????? return __roundup_pow_of_two(div);
>> ????? if (table)
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 210a890..7c59611 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -388,6 +388,8 @@ struct clk_div_table {
>> ?? * CLK_DIVIDER_MAX_AT_ZERO - For dividers which are like
>> CLK_DIVIDER_ONE_BASED
>> ?? *??? except when the value read from the register is zero, the
>> divisor is
>> ?? *??? 2^width of the field.
>> + * CLK_DIVIDER_EVEN - For the dividers which could only use even
>> number in the
>> + *??? div field.
>> ?? */
>> ? struct clk_divider {
>> ????? struct clk_hw??? hw;
>> @@ -409,6 +411,7 @@ struct clk_divider {
>> ? #define CLK_DIVIDER_ROUND_CLOSEST??? BIT(4)
>> ? #define CLK_DIVIDER_READ_ONLY??????? BIT(5)
>> ? #define CLK_DIVIDER_MAX_AT_ZERO??????? BIT(6)
>> +#define CLK_DIVIDER_EVEN??????? BIT(7)
>> ? extern const struct clk_ops clk_divider_ops;
>> ? extern const struct clk_ops clk_divider_ro_ops;
>>
>
>
>
