Re: [PATCH v3] clk: Add Si5341/Si5340 driver

From: Mike Looijmans
Date: Fri Jun 28 2019 - 02:42:17 EST


On 27-06-19 23:06, Stephen Boyd wrote:
> Quoting Mike Looijmans (2019-05-17 06:23:52)
>> Adds a driver for the Si5341 and Si5340 chips. The driver does not fully
>> support all features of these chips, but allows the chip to be used
>> without any support from the "clockbuilder pro" software.
>>
>> If the chip is preprogrammed, that is, you bought one with some defaults
>> burned in, or you programmed the NVM in some way, the driver will just
>> take over the current settings and only change them on demand. Otherwise
>> the input must be a fixed XTAL in its most basic configuration (no
>> predividers, no feedback, etc.).
>>
>> The driver supports dynamic changes of multisynth, output dividers and
>> enabling or powering down outputs and multisynths.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@xxxxxxxx>
>> ---
>
> Applied to clk-next + some fixes. I'm not super thrilled about the kHz
> thing but we don't have a solution for it right now so might as well
> come back to it later.

Thanks for the fixes. And I'm not exactly proud of that kHz part either.

While thinking about a solution, I've also had a use case for less than 1Hz
frequency adjustment (a video clock to "follow" another one). These clock
generators allow for ridiculous ranges and accuracy, you can request it to
generate a 200000000.0005 Hz clock.


> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
> index 1a310835b53c..72424eb7e5f8 100644
> --- a/drivers/clk/clk-si5341.c
> +++ b/drivers/clk/clk-si5341.c
> @@ -374,7 +374,7 @@ static unsigned long si5341_clk_recalc_rate(struct clk_hw *hw,
> */
> shift = 0;
> res = m_num;
> - while (res & 0xffff00000000) {
> + while (res & 0xffff00000000ULL) {
> ++shift;
> res >>= 1;
> }
> @@ -921,7 +921,7 @@ static int si5341_write_multiple(struct clk_si5341 *data,
> return 0;
> }
>
> -const struct si5341_reg_default si5341_preamble[] = {
> +static const struct si5341_reg_default si5341_preamble[] = {
> { 0x0B25, 0x00 },
> { 0x0502, 0x01 },
> { 0x0505, 0x03 },
> @@ -994,7 +994,7 @@ static const struct regmap_range si5341_regmap_volatile_range[] = {
> regmap_reg_range(SI5341_SYNTH_N_UPD(4), SI5341_SYNTH_N_UPD(4)),
> };
>
> -const struct regmap_access_table si5341_regmap_volatile = {
> +static const struct regmap_access_table si5341_regmap_volatile = {
> .yes_ranges = si5341_regmap_volatile_range,
> .n_yes_ranges = ARRAY_SIZE(si5341_regmap_volatile_range),
> };
> @@ -1016,7 +1016,6 @@ static const struct regmap_config si5341_regmap_config = {
> .reg_bits = 8,
> .val_bits = 8,
> .cache_type = REGCACHE_RBTREE,
> - .max_register = 0,
> .ranges = si5341_regmap_ranges,
> .num_ranges = ARRAY_SIZE(si5341_regmap_ranges),
> .max_register = SI5341_REGISTER_MAX,
> @@ -1328,7 +1327,7 @@ MODULE_DEVICE_TABLE(i2c, si5341_id);
> static const struct of_device_id clk_si5341_of_match[] = {
> { .compatible = "silabs,si5340" },
> { .compatible = "silabs,si5341" },
> - { },
> + { }
> };
> MODULE_DEVICE_TABLE(of, clk_si5341_of_match);
>
>