Hi Simon,
On Wed, Jul 19, 2017 at 11:05 AM, Simon Glass sjg@chromium.org wrote:
On 14 July 2017 at 05:55, Mario Six mario.six@gdsys.cc wrote:
Add a driver for the ICS8N3QV01 Quad-Frequency Programmable VCXO.
Signed-off-by: Mario Six mario.six@gdsys.cc
drivers/clk/Kconfig | 6 ++ drivers/clk/Makefile | 1 + drivers/clk/ics8n3qv01.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+) create mode 100644 drivers/clk/ics8n3qv01.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 44da716b26..f6f3810b64 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -56,4 +56,10 @@ source "drivers/clk/uniphier/Kconfig" source "drivers/clk/exynos/Kconfig" source "drivers/clk/at91/Kconfig"
+config ICS8N3QV01
bool "Enable ICS8N3QV01 VCXO driver"depends on CLKhelpSupport for the ICS8N3QV01 VCXO.What is this? Can you describe it a bit more here?
I'll add some details in v2.
endmenu diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 2746a8016a..d7cc486d23 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -21,3 +21,4 @@ obj-$(CONFIG_CLK_BCM6345) += clk_bcm6345.o obj-$(CONFIG_CLK_BOSTON) += clk_boston.o obj-$(CONFIG_ARCH_ASPEED) += aspeed/ obj-$(CONFIG_STM32F7) += clk_stm32f7.o +obj-$(CONFIG_ICS8N3QV01) += ics8n3qv01.o
Would be good if we could have these in alpha order. If you have time can you do a patch to tidy that up before or after?
Sure, I'll order them.
diff --git a/drivers/clk/ics8n3qv01.c b/drivers/clk/ics8n3qv01.c new file mode 100644 index 0000000000..f5f4b74982 --- /dev/null +++ b/drivers/clk/ics8n3qv01.c @@ -0,0 +1,184 @@ +/*
- (C) Copyright 2017
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- based on the gdsys osd driver, which is
- (C) Copyright 2010
- Dirk Eibach, Guntermann & Drunck GmbH, eibach@gdsys.de
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <dm.h> +#include <clk-uclass.h> +#include <i2c.h>
+const long long ICS8N3QV01_FREF = 114285000; +const long long ICS8N3QV01_FREF_LL = 114285000LL; +const long long ICS8N3QV01_F_DEFAULT_0 = 156250000LL; +const long long ICS8N3QV01_F_DEFAULT_1 = 125000000LL; +const long long ICS8N3QV01_F_DEFAULT_2 = 100000000LL; +const long long ICS8N3QV01_F_DEFAULT_3 = 25175000LL;
+struct ics8n3qv01_priv {
ulong rate;+};
+static uint ics8n3qv01_get_fout_calc(struct udevice *dev, uint index) +{
u64 n, mint, mfrac;u8 reg_a, reg_b, reg_c, reg_d, reg_f;u64 fout_calc;if (index > 3)What is 3? Should this be an enum/#define?
The device is a VCXO (Voltage-Controlled Crystal Oscillator) that supplies four frequencies (indexed 0 to 3), so accessing frequency indexes higher than 3 makes no sense. Maybe a MAX_FREQ_INDEX define would help to clarify the intent here.
return 0;reg_a = dm_i2c_reg_read(dev, 0 + index);Error checking?
I'll refactor the method to return an error code and use a output parameter for the frequency (and add error checking, of course).
reg_b = dm_i2c_reg_read(dev, 4 + index);reg_c = dm_i2c_reg_read(dev, 8 + index);reg_d = dm_i2c_reg_read(dev, 12 + index);reg_f = dm_i2c_reg_read(dev, 20 + index);mint = ((reg_a >> 1) & 0x1f) | (reg_f & 0x20);mfrac = ((reg_a & 0x01) << 17) | (reg_b << 9) | (reg_c << 1)| (reg_d >> 7);n = reg_d & 0x7f;fout_calc = (mint * ICS8N3QV01_FREF_LL+ mfrac * ICS8N3QV01_FREF_LL / 262144LL+ ICS8N3QV01_FREF_LL / 524288LL+ n / 2)/ n* 1000000/ (1000000 - 100);return fout_calc;+}
+static void ics8n3qv01_calc_parameters(uint fout, uint *_mint, uint *_mfrac,
uint *_n)+{
uint n, foutiic, fvcoiic, mint;u64 mfrac;n = (2215000000U + fout / 2) / fout;if ((n & 1) && (n > 5))n -= 1;foutiic = fout - (fout / 10000);fvcoiic = foutiic * n;mint = fvcoiic / 114285000;if ((mint < 17) || (mint > 63))printf("ics8n3qv01_calc_parameters: cannot determine mint\n");return error?
dito.
mfrac = ((u64)fvcoiic % 114285000LL) * 262144LL/ 114285000LL;*_mint = mint;*_mfrac = mfrac;*_n = n;+}
+static ulong ics8n3qv01_set_rate(struct clk *clk, ulong fout) +{
struct ics8n3qv01_priv *priv = dev_get_priv(clk->dev);uint n, mint, mfrac, fout_calc;u64 fout_prog;long long off_ppm;u8 reg0, reg4, reg8, reg12, reg18, reg20;priv->rate = fout;fout_calc = ics8n3qv01_get_fout_calc(clk->dev, 1);off_ppm = (fout_calc - ICS8N3QV01_F_DEFAULT_1) * 1000000/ ICS8N3QV01_F_DEFAULT_1;printf("%s: PLL is off by %lld ppm\n", clk->dev->name, off_ppm);debug()?
That's actually the "status message" we want to see from the clock during startup; if we know that the value is reasonable during problem diagnosis, we can be pretty sure that at least the pixel clock works reliably.
fout_prog = (u64)fout * (u64)fout_calc/ ICS8N3QV01_F_DEFAULT_1;ics8n3qv01_calc_parameters(fout_prog, &mint, &mfrac, &n);reg0 = dm_i2c_reg_read(clk->dev, 0) & 0xc0;reg0 |= (mint & 0x1f) << 1;reg0 |= (mfrac >> 17) & 0x01;dm_i2c_reg_write(clk->dev, 0, reg0);Check error?
See above, will refactor in v2.
reg4 = mfrac >> 9;dm_i2c_reg_write(clk->dev, 4, reg4);reg8 = mfrac >> 1;dm_i2c_reg_write(clk->dev, 8, reg8);reg12 = mfrac << 7;reg12 |= n & 0x7f;dm_i2c_reg_write(clk->dev, 12, reg12);reg18 = dm_i2c_reg_read(clk->dev, 18) & 0x03;reg18 |= 0x20;dm_i2c_reg_write(clk->dev, 18, reg18);reg20 = dm_i2c_reg_read(clk->dev, 20) & 0x1f;reg20 |= mint & (1 << 5);dm_i2c_reg_write(clk->dev, 20, reg20);return 0;+}
+static int ics8n3qv01_request(struct clk *clock) +{
return 0;+}
+static ulong ics8n3qv01_get_rate(struct clk *clk) +{
struct ics8n3qv01_priv *priv = dev_get_priv(clk->dev);return priv->rate;+}
+static int ics8n3qv01_enable(struct clk *clk) +{
return 0;+}
+static int ics8n3qv01_disable(struct clk *clk) +{
return 0;+}
+static const struct clk_ops ics8n3qv01_ops = {
.request = ics8n3qv01_request,.get_rate = ics8n3qv01_get_rate,.set_rate = ics8n3qv01_set_rate,.enable = ics8n3qv01_enable,.disable = ics8n3qv01_disable,+};
+static const struct udevice_id ics8n3qv01_ids[] = {
{ .compatible = "idt,ics8n3qv01" },{ /* sentinel */ }+};
+int ics8n3qv01_probe(struct udevice *dev) +{
struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);struct udevice *dummy;if (dm_i2c_probe(dev->parent, chip->chip_addr, chip->flags, &dummy)) {You should not need to probe here - DM should do this for you.
Ah, OK, the check was in the old ics8n3qv01 driver, so I just adapted it verbatim to DM. I'll remove it in v2.
printf("ics8n3qv01: I2C probe did not succeed.\n");return -1;}return 0;+}
+U_BOOT_DRIVER(ics8n3qv01) = {
.name = "ics8n3qv01",.id = UCLASS_CLK,.ops = &ics8n3qv01_ops,.of_match = ics8n3qv01_ids,.probe = ics8n3qv01_probe,.priv_auto_alloc_size = sizeof(struct ics8n3qv01_priv),+};
2.11.0
Regards, Simon
Best regards,
Mario