Messages in this thread |  | | Subject | Re: [PATCH] clk, clk-si5341: Support multiple input ports | From | Stephen Boyd <> | Date | Mon, 06 Jan 2020 21:48:37 -0800 |
| |
Quoting Mike Looijmans (2019-12-05 03:57:34) > The Si5341 and Si5340 have multiple input clock options. So far, the driver > only supported the XTAL input, this adds support for the three external > clock inputs as well. > > If the clock chip is't programmed at boot, the driver will default to the
isn't
> XTAL input as before. If there is no "xtal" clock input available, it will > pick the first connected input (e.g. "in0") as the input clock for the PLL. > One can use clock-assigned-parents to select a particular clock as input. > > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c > index 6e780c2a9e6b..f7dba7698083 100644 > --- a/drivers/clk/clk-si5341.c > +++ b/drivers/clk/clk-si5341.c > @@ -4,7 +4,6 @@ > * Copyright (C) 2019 Topic Embedded Products > * Author: Mike Looijmans <mike.looijmans@topic.nl> > */ > - > #include <linux/clk.h> > #include <linux/clk-provider.h> > #include <linux/delay.h>
I think we can do without this hunk.
> @@ -390,7 +410,112 @@ static unsigned long si5341_clk_recalc_rate(struct clk_hw *hw, > return (unsigned long)res; > } > > +static int si5341_clk_get_selected_input(struct clk_si5341 *data) > +{ > + int err; > + u32 val; > + > + err = regmap_read(data->regmap, SI5341_IN_SEL, &val); > + if (err < 0) > + return err; > + > + return (val & SI5341_IN_SEL_MASK) >> SI5341_IN_SEL_SHIFT; > +} > + > +static unsigned char si5341_clk_get_parent(struct clk_hw *hw)
Please use u8 for now.
> +{ > + struct clk_si5341 *data = to_clk_si5341(hw); > + int res = si5341_clk_get_selected_input(data); > + > + if (res < 0) > + return 0; /* Apparently we cannot report errors */
For now this is the case. I'll rekick the series to convert this API to a function that returns clk_hw pointers.
> + > + return res; > +} > + [...] > @@ -985,7 +1110,8 @@ static const struct regmap_range si5341_regmap_volatile_range[] = { > regmap_reg_range(0x000C, 0x0012), /* Status */ > regmap_reg_range(0x001C, 0x001E), /* reset, finc/fdec */ > regmap_reg_range(0x00E2, 0x00FE), /* NVM, interrupts, device ready */ > - /* Update bits for synth config */ > + /* Update bits for P divider and synth config */ > + regmap_reg_range(SI5341_PX_UPD, SI5341_PX_UPD), > regmap_reg_range(SI5341_SYNTH_N_UPD(0), SI5341_SYNTH_N_UPD(0)), > regmap_reg_range(SI5341_SYNTH_N_UPD(1), SI5341_SYNTH_N_UPD(1)), > regmap_reg_range(SI5341_SYNTH_N_UPD(2), SI5341_SYNTH_N_UPD(2)), > @@ -1122,6 +1248,7 @@ static int si5341_initialize_pll(struct clk_si5341 *data) > struct device_node *np = data->i2c_client->dev.of_node; > u32 m_num = 0; > u32 m_den = 0; > + int sel; > > if (of_property_read_u32(np, "silabs,pll-m-num", &m_num)) { > dev_err(&data->i2c_client->dev, > @@ -1135,7 +1262,11 @@ static int si5341_initialize_pll(struct clk_si5341 *data) > if (!m_num || !m_den) { > dev_err(&data->i2c_client->dev, > "PLL configuration invalid, assume 14GHz\n"); > - m_den = clk_get_rate(data->pxtal) / 10; > + sel = si5341_clk_get_selected_input(data); > + if (sel < 0) > + return sel; > + > + m_den = clk_get_rate(data->input_clk[sel]) / 10; > m_num = 1400000000; > } > > @@ -1143,11 +1274,52 @@ static int si5341_initialize_pll(struct clk_si5341 *data) > SI5341_PLL_M_NUM, m_num, m_den); > } > > +static int si5341_clk_select_active_input(struct clk_si5341 *data) > +{ > + int res; > + int err; > + int i; > + > + res = si5341_clk_get_selected_input(data); > + if (res < 0) > + return res; > + > + /* If the current register setting is invalid, pick the first input */ > + if (!data->input_clk[res]) { > + dev_dbg(&data->i2c_client->dev, > + "Input %d not connected, rerouting\n", res); > + res = -ENODEV; > + for (i = 0; i < SI5341_NUM_INPUTS; ++i) { > + if (data->input_clk[i]) { > + res = i; > + break; > + } > + } > + if (res < 0) {
What if res is == SI5341_NUM_INPUTS?
> + dev_err(&data->i2c_client->dev, > + "No clock input available\n"); > + return res; > + } > + } > + > + /* Make sure the selected clock is also enabled and routed */ > + err = si5341_clk_reparent(data, res); > + if (err < 0) > + return err; > + > + err = clk_prepare_enable(data->input_clk[res]);
Is it possible to do this setup and configuration stuff when the clk is prepared by something? Maybe I've asked this before but I'd prefer that this driver is a clk provider and not a clk consumer. If some default setup needs to be done, preferably do that via direct register writes or by calling the clk_ops functions directly instead of going through the framework, preferably before registering the clks to the framework.
> + if (err < 0) > + return err; > + > + return res; > +} > +
|  |