| Message ID | 20220213173310.152230-2-marex@denx.de (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] dt-bindings: clk: rs9: Add Renesas 9-series I2C PCIe clock generator | expand |
Quoting Marek Vasut (2022-02-13 09:33:10) > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 6a98291350b64..3ec27842ec779 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -68,6 +68,7 @@ obj-$(CONFIG_COMMON_CLK_STM32MP157) += clk-stm32mp1.o > obj-$(CONFIG_COMMON_CLK_TPS68470) += clk-tps68470.o > obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o > obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o > +obj-$(CONFIG_COMMON_CLK_RS9_PCIE) += clk-renesas-pcie.o Is there a reason it doesn't go into drivers/clk/renesas? > obj-$(CONFIG_COMMON_CLK_VC5) += clk-versaclock5.o > obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o > obj-$(CONFIG_COMMON_CLK_XGENE) += clk-xgene.o > diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c > new file mode 100644 > index 0000000000000..0ad132cbe41b8 > --- /dev/null > +++ b/drivers/clk/clk-renesas-pcie.c > @@ -0,0 +1,336 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Driver for Renesas 9-series PCIe clock generator driver > + * > + * The following series can be supported: > + * - 9FGV/9DBV/9DMV/9FGL/9DML/9QXL/9SQ > + * Currently supported: > + * - 9FGV0241 > + * > + * Copyright (C) 2022 Marek Vasut <marex@denx.de> > + */ > + > +#include <linux/clk.h> Drop this include please. > +#include <linux/clk-provider.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/interrupt.h> Is this used? If not please remove. > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_platform.h> Is this used? If not please remove. > +#include <linux/rational.h> Is this used? If not please remove. > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +#define RS9_REG_OE 0x0 > +#define RS9_REG_OE_DIF_OE(n) BIT((n) + 1) > +#define RS9_REG_SS 0x1 > +#define RS9_REG_SS_AMP_0V6 0x0 > +#define RS9_REG_SS_AMP_0V7 0x1 > +#define RS9_REG_SS_AMP_0V8 0x2 > +#define RS9_REG_SS_AMP_0V9 0x3 > +#define RS9_REG_SS_AMP_MASK 0x3 > +#define RS9_REG_SS_SSC_100 0 > +#define RS9_REG_SS_SSC_M025 (1 << 3) > +#define RS9_REG_SS_SSC_M050 (3 << 3) > +#define RS9_REG_SS_SSC_MASK (3 << 3) > +#define RS9_REG_SS_SSC_LOCK BIT(5) > +#define RS9_REG_SR 0x2 > +#define RS9_REG_SR_2V0_DIF(n) 0 > +#define RS9_REG_SR_3V0_DIF(n) BIT((n) + 1) > +#define RS9_REG_SR_DIF_MASK(n) BIT((n) + 1) > +#define RS9_REG_REF 0x3 > +#define RS9_REG_REF_OE BIT(4) > +#define RS9_REG_REF_OD BIT(5) > +#define RS9_REG_REF_SR_SLOWEST 0 > +#define RS9_REG_REF_SR_SLOW (1 << 6) > +#define RS9_REG_REF_SR_FAST (2 << 6) > +#define RS9_REG_REF_SR_FASTER (3 << 6) > +#define RS9_REG_VID 0x5 > +#define RS9_REG_DID 0x6 > +#define RS9_REG_BCP 0x7 > + > +/* Supported Renesas 9-series models. */ > +enum rs9_model { > + RENESAS_9FGV0241, > +}; > + > +/* Structure to describe features of a particular 9-series model */ > +struct rs9_chip_info { > + const enum rs9_model model; > + unsigned int num_clks; > +}; > + > +struct rs9_driver_data { > + struct i2c_client *client; > + struct regmap *regmap; > + const struct rs9_chip_info *chip_info; > + struct clk *pin_xin; > + struct clk_hw *clk_dif[2]; > + u8 pll_amplitude; > + u8 pll_ssc; > + u8 clk_dif_sr; > +}; > + > +/* > + * Renesas 9-series i2c regmap > + */ > +static const struct regmap_range rs9_readable_ranges[] = { > + regmap_reg_range(RS9_REG_OE, RS9_REG_REF), > + regmap_reg_range(RS9_REG_VID, RS9_REG_BCP), > +}; > + > +static const struct regmap_access_table rs9_readable_table = { > + .yes_ranges = rs9_readable_ranges, > + .n_yes_ranges = ARRAY_SIZE(rs9_readable_ranges), > +}; > + > +static const struct regmap_range rs9_writeable_ranges[] = { > + regmap_reg_range(RS9_REG_OE, RS9_REG_REF), > + regmap_reg_range(RS9_REG_BCP, RS9_REG_BCP), > +}; > + > +static const struct regmap_access_table rs9_writeable_table = { > + .yes_ranges = rs9_writeable_ranges, > + .n_yes_ranges = ARRAY_SIZE(rs9_writeable_ranges), > +}; > + > +static const struct regmap_config rs9_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .cache_type = REGCACHE_RBTREE, > + .max_register = 0x8, That's a huge cache! > + .rd_table = &rs9_readable_table, > + .wr_table = &rs9_writeable_table, > +}; > + > +static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx) > +{ > + struct i2c_client *client = rs9->client; > + unsigned char *name = "DIF0"; > + struct device_node *np; > + int ret; > + u32 sr; > + > + /* Set defaults */ > + rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx); > + rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx); > + > + name[3] += idx; > + np = of_get_child_by_name(client->dev.of_node, name); > + if (!np) > + return 0; > + > + /* Output clock slew rate */ > + ret = of_property_read_u32(np, "renesas,slew-rate", &sr); Put of_node_put(np) here please > + if (!ret) { > + if (sr == 2000000) { /* 2V/ns */ > + rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx); > + rs9->clk_dif_sr |= RS9_REG_SR_2V0_DIF(idx); > + } else if (sr == 3000000) { /* 3V/ns (default) */ > + rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx); > + rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx); > + } else > + ret = dev_err_probe(&client->dev, -EINVAL, > + "Invalid renesas,slew-rate value\n"); > + } > + > + of_node_put(np); > + > + return ret; > +} > + > +static int rs9_get_common_config(struct rs9_driver_data *rs9) > +{ > + struct i2c_client *client = rs9->client; > + struct device_node *np = client->dev.of_node; > + unsigned int amp, ssc; > + int ret; > + > + /* Set defaults */ > + rs9->pll_amplitude = RS9_REG_SS_AMP_0V7; > + rs9->pll_ssc = RS9_REG_SS_SSC_100; > + > + /* Output clock amplitude */ > + ret = of_property_read_u32(np, "renesas,out-amplitude", &); > + if (!ret) { > + if (amp == 600000) /* 0.6V */ > + rs9->pll_amplitude = RS9_REG_SS_AMP_0V6; > + else if (amp == 700000) /* 0.7V (default) */ > + rs9->pll_amplitude = RS9_REG_SS_AMP_0V7; > + else if (amp == 800000) /* 0.8V */ > + rs9->pll_amplitude = RS9_REG_SS_AMP_0V8; > + else if (amp == 900000) /* 0.9V */ > + rs9->pll_amplitude = RS9_REG_SS_AMP_0V9; > + else > + return dev_err_probe(&client->dev, -EINVAL, > + "Invalid renesas,out-amplitude value\n"); > + } > + > + /* Output clock spread spectrum */ > + ret = of_property_read_u32(np, "renesas,out-spread-spectrum", &ssc); > + if (!ret) { > + if (ssc == 100000) /* 100% ... no spread (default) */ > + rs9->pll_ssc = RS9_REG_SS_SSC_100; > + else if (ssc == 99750) /* -0.25% ... down spread */ > + rs9->pll_ssc = RS9_REG_SS_SSC_M025; > + else if (ssc == 99500) /* -0.50% ... down spread */ > + rs9->pll_ssc = RS9_REG_SS_SSC_M050; > + else > + return dev_err_probe(&client->dev, -EINVAL, > + "Invalid renesas,out-spread-spectrum value\n"); > + } > + > + return 0; > +} > + > +static void rs9_update_config(struct rs9_driver_data *rs9) > +{ > + int i; > + > + /* If amplitude is non-default, update it. */ > + if (rs9->pll_amplitude != RS9_REG_SS_AMP_0V7) { > + regmap_update_bits(rs9->regmap, RS9_REG_SS, RS9_REG_SS_AMP_MASK, > + rs9->pll_amplitude); > + } > + > + /* If SSC is non-default, update it. */ > + if (rs9->pll_ssc != RS9_REG_SS_SSC_100) { > + regmap_update_bits(rs9->regmap, RS9_REG_SS, RS9_REG_SS_SSC_MASK, > + rs9->pll_ssc); > + } > + > + for (i = 0; i < rs9->chip_info->num_clks; i++) { > + if (rs9->clk_dif_sr & RS9_REG_SR_3V0_DIF(i)) > + continue; > + > + regmap_update_bits(rs9->regmap, RS9_REG_SR, RS9_REG_SR_3V0_DIF(i), > + rs9->clk_dif_sr & RS9_REG_SR_3V0_DIF(i)); > + } > +} > + > +static struct clk_hw * > +rs9_of_clk_get(struct of_phandle_args *clkspec, void *data) > +{ > + struct rs9_driver_data *rs9 = data; > + unsigned int idx = clkspec->args[0]; > + > + return rs9->clk_dif[idx]; > +} > + > +static const struct of_device_id clk_rs9_of_match[]; Why the forward declare? > + > +static int rs9_probe(struct i2c_client *client, const struct i2c_device_id *id) > +{ > + struct device_node *np = client->dev.of_node; > + unsigned char *name = "DIF0"; > + struct rs9_driver_data *rs9; > + const char *parent_clk; > + struct clk_hw *hw; > + int i, ret; > + > + rs9 = devm_kzalloc(&client->dev, sizeof(*rs9), GFP_KERNEL); > + if (!rs9) > + return -ENOMEM; > + > + i2c_set_clientdata(client, rs9); > + rs9->client = client; > + rs9->chip_info = of_device_get_match_data(&client->dev); Check for NULL? Use device_get_match_data()? Or does that not work for i2c devices? > + > + /* Fetch common configuration from DT (if specified) */ > + ret = rs9_get_common_config(rs9); > + if (ret) > + return ret; > + > + /* Fetch DIFx output configuration from DT (if specified) */ > + for (i = 0; i < rs9->chip_info->num_clks; i++) { > + ret = rs9_get_output_config(rs9, i); > + if (ret) > + return ret; > + } > + > + /* Mandatory XTal */ Oh it's mandatory here but not in the binding? > + parent_clk = of_clk_get_parent_name(np, 0); Use clk_parent_data please. > + if (!parent_clk) > + return dev_err_probe(&client->dev, -EINVAL, > + "Missing XTal input clock\n"); > + > + rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config); > + if (IS_ERR(rs9->regmap)) > + return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap), > + "Failed to allocate register map\n"); > + > + /* Register clock */ > + for (i = 0; i < rs9->chip_info->num_clks; i++) { > + name[3]++; > + hw = clk_hw_register_fixed_factor(&client->dev, name, > + parent_clk, 0, 4, 1); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + rs9->clk_dif[i] = hw; > + } > + > + ret = devm_of_clk_add_hw_provider(&client->dev, rs9_of_clk_get, rs9); > + if (!ret) > + rs9_update_config(rs9); > + > + return ret; > +}
On 2/18/22 00:45, Stephen Boyd wrote: > Quoting Marek Vasut (2022-02-13 09:33:10) >> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile >> index 6a98291350b64..3ec27842ec779 100644 >> --- a/drivers/clk/Makefile >> +++ b/drivers/clk/Makefile >> @@ -68,6 +68,7 @@ obj-$(CONFIG_COMMON_CLK_STM32MP157) += clk-stm32mp1.o >> obj-$(CONFIG_COMMON_CLK_TPS68470) += clk-tps68470.o >> obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o >> obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o >> +obj-$(CONFIG_COMMON_CLK_RS9_PCIE) += clk-renesas-pcie.o > > Is there a reason it doesn't go into drivers/clk/renesas? The drivers/clk/renesas/ is for renesas SoC (R-Car/RZ/...), this chip is different group (it's probably even IDT PLL IP). [...] >> +#include <linux/mod_devicetable.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_platform.h> > > Is this used? If not please remove. This one is for of_device_get_match_data() [...] >> +static struct clk_hw * >> +rs9_of_clk_get(struct of_phandle_args *clkspec, void *data) >> +{ >> + struct rs9_driver_data *rs9 = data; >> + unsigned int idx = clkspec->args[0]; >> + >> + return rs9->clk_dif[idx]; >> +} >> + >> +static const struct of_device_id clk_rs9_of_match[]; > > Why the forward declare? Remnant from development, dropped. >> +static int rs9_probe(struct i2c_client *client, const struct i2c_device_id *id) >> +{ >> + struct device_node *np = client->dev.of_node; >> + unsigned char *name = "DIF0"; >> + struct rs9_driver_data *rs9; >> + const char *parent_clk; >> + struct clk_hw *hw; >> + int i, ret; >> + >> + rs9 = devm_kzalloc(&client->dev, sizeof(*rs9), GFP_KERNEL); >> + if (!rs9) >> + return -ENOMEM; >> + >> + i2c_set_clientdata(client, rs9); >> + rs9->client = client; >> + rs9->chip_info = of_device_get_match_data(&client->dev); > > Check for NULL? Use device_get_match_data()? Or does that not work for > i2c devices? > >> + >> + /* Fetch common configuration from DT (if specified) */ >> + ret = rs9_get_common_config(rs9); >> + if (ret) >> + return ret; >> + >> + /* Fetch DIFx output configuration from DT (if specified) */ >> + for (i = 0; i < rs9->chip_info->num_clks; i++) { >> + ret = rs9_get_output_config(rs9, i); >> + if (ret) >> + return ret; >> + } >> + >> + /* Mandatory XTal */ > > Oh it's mandatory here but not in the binding? > >> + parent_clk = of_clk_get_parent_name(np, 0); > > Use clk_parent_data please. This one line I don't understand -- can you expand on what you expect me to do here ? >> + if (!parent_clk) >> + return dev_err_probe(&client->dev, -EINVAL, >> + "Missing XTal input clock\n"); >> + >> + rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config); >> + if (IS_ERR(rs9->regmap)) >> + return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap), >> + "Failed to allocate register map\n"); >> + >> + /* Register clock */ >> + for (i = 0; i < rs9->chip_info->num_clks; i++) { >> + name[3]++; >> + hw = clk_hw_register_fixed_factor(&client->dev, name, >> + parent_clk, 0, 4, 1); >> + if (IS_ERR(hw)) >> + return PTR_ERR(hw); >> + >> + rs9->clk_dif[i] = hw; >> + } >> + >> + ret = devm_of_clk_add_hw_provider(&client->dev, rs9_of_clk_get, rs9); >> + if (!ret) >> + rs9_update_config(rs9); >> + >> + return ret; >> +} [...]
Quoting Marek Vasut (2022-02-17 17:26:22) > On 2/18/22 00:45, Stephen Boyd wrote: > > Quoting Marek Vasut (2022-02-13 09:33:10) > >> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > >> index 6a98291350b64..3ec27842ec779 100644 > >> --- a/drivers/clk/Makefile > >> +++ b/drivers/clk/Makefile > >> @@ -68,6 +68,7 @@ obj-$(CONFIG_COMMON_CLK_STM32MP157) += clk-stm32mp1.o > >> obj-$(CONFIG_COMMON_CLK_TPS68470) += clk-tps68470.o > >> obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o > >> obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o > >> +obj-$(CONFIG_COMMON_CLK_RS9_PCIE) += clk-renesas-pcie.o > > > > Is there a reason it doesn't go into drivers/clk/renesas? > > The drivers/clk/renesas/ is for renesas SoC (R-Car/RZ/...), > this chip is different group (it's probably even IDT PLL IP). Ah ok so it's not a renesas SoC but a renesas IP? > > [...] > > >> +#include <linux/mod_devicetable.h> > >> +#include <linux/module.h> > >> +#include <linux/of.h> > >> +#include <linux/of_platform.h> > > > > Is this used? If not please remove. > > This one is for of_device_get_match_data() > So it's going away? > > >> +static int rs9_probe(struct i2c_client *client, const struct i2c_device_id *id) > >> +{ > >> + struct device_node *np = client->dev.of_node; > >> + unsigned char *name = "DIF0"; > >> + struct rs9_driver_data *rs9; > >> + const char *parent_clk; > >> + struct clk_hw *hw; > >> + int i, ret; > >> + > >> + rs9 = devm_kzalloc(&client->dev, sizeof(*rs9), GFP_KERNEL); > >> + if (!rs9) > >> + return -ENOMEM; > >> + > >> + i2c_set_clientdata(client, rs9); > >> + rs9->client = client; > >> + rs9->chip_info = of_device_get_match_data(&client->dev); > > > > Check for NULL? Use device_get_match_data()? Or does that not work for > > i2c devices? ?? > > > >> + > >> + /* Fetch common configuration from DT (if specified) */ > >> + ret = rs9_get_common_config(rs9); > >> + if (ret) > >> + return ret; > >> + > >> + /* Fetch DIFx output configuration from DT (if specified) */ > >> + for (i = 0; i < rs9->chip_info->num_clks; i++) { > >> + ret = rs9_get_output_config(rs9, i); > >> + if (ret) > >> + return ret; > >> + } > >> + > >> + /* Mandatory XTal */ > > > > Oh it's mandatory here but not in the binding? > > > >> + parent_clk = of_clk_get_parent_name(np, 0); > > > > Use clk_parent_data please. > > This one line I don't understand -- can you expand on what you expect me > to do here ? Use 'struct clk_parent_data' and set .index to 0 so that when registering the clk you don't need to get the parent clk name. > > >> + if (!parent_clk) > >> + return dev_err_probe(&client->dev, -EINVAL, > >> + "Missing XTal input clock\n"); > >> + > >> + rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config); > >> + if (IS_ERR(rs9->regmap)) > >> + return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap), > >> + "Failed to allocate register map\n"); > >> + > >> + /* Register clock */ > >> + for (i = 0; i < rs9->chip_info->num_clks; i++) { > >> + name[3]++; > >> + hw = clk_hw_register_fixed_factor(&client->dev, name, > >> + parent_clk, 0, 4, 1); To do that it looks like maybe we'll need to export __clk_hw_register_fixed_factor() and introduces some sort of clk_hw_register_fixed_factor_parent_data() API. > >> + if (IS_ERR(hw)) > >> + return PTR_ERR(hw); > >> + > >> + rs9->clk_dif[i] = hw;
On 2/18/22 23:15, Stephen Boyd wrote: Hi, >>>> @@ -68,6 +68,7 @@ obj-$(CONFIG_COMMON_CLK_STM32MP157) += clk-stm32mp1.o >>>> obj-$(CONFIG_COMMON_CLK_TPS68470) += clk-tps68470.o >>>> obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o >>>> obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o >>>> +obj-$(CONFIG_COMMON_CLK_RS9_PCIE) += clk-renesas-pcie.o >>> >>> Is there a reason it doesn't go into drivers/clk/renesas? >> >> The drivers/clk/renesas/ is for renesas SoC (R-Car/RZ/...), >> this chip is different group (it's probably even IDT PLL IP). > > Ah ok so it's not a renesas SoC but a renesas IP? I suspect it is IDT IP, since the datasheet looks similar to what versaclock datasheets used to look like, except the register layout is much simpler and the registers are completely different. So it _might_ be some mutation of the IDT PLL, and it is now owned by renesas. The renesas SoC clock IP is something entirely different from the IP used here. >>>> +#include <linux/mod_devicetable.h> >>>> +#include <linux/module.h> >>>> +#include <linux/of.h> >>>> +#include <linux/of_platform.h> >>> >>> Is this used? If not please remove. >> >> This one is for of_device_get_match_data() >> > > So it's going away? Yes [...] >>> Use clk_parent_data please. >> >> This one line I don't understand -- can you expand on what you expect me >> to do here ? > > Use 'struct clk_parent_data' and set .index to 0 so that when > registering the clk you don't need to get the parent clk name. > >> >>>> + if (!parent_clk) >>>> + return dev_err_probe(&client->dev, -EINVAL, >>>> + "Missing XTal input clock\n"); >>>> + >>>> + rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config); >>>> + if (IS_ERR(rs9->regmap)) >>>> + return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap), >>>> + "Failed to allocate register map\n"); >>>> + >>>> + /* Register clock */ >>>> + for (i = 0; i < rs9->chip_info->num_clks; i++) { >>>> + name[3]++; >>>> + hw = clk_hw_register_fixed_factor(&client->dev, name, >>>> + parent_clk, 0, 4, 1); > > To do that it looks like maybe we'll need to export > __clk_hw_register_fixed_factor() and introduces some sort of > clk_hw_register_fixed_factor_parent_data() API. Setting parent_clk to NULL should be enough. [...]
Quoting Marek Vasut (2022-02-18 17:11:04) > On 2/18/22 23:15, Stephen Boyd wrote: > >> > >>>> + if (!parent_clk) > >>>> + return dev_err_probe(&client->dev, -EINVAL, > >>>> + "Missing XTal input clock\n"); > >>>> + > >>>> + rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config); > >>>> + if (IS_ERR(rs9->regmap)) > >>>> + return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap), > >>>> + "Failed to allocate register map\n"); > >>>> + > >>>> + /* Register clock */ > >>>> + for (i = 0; i < rs9->chip_info->num_clks; i++) { > >>>> + name[3]++; > >>>> + hw = clk_hw_register_fixed_factor(&client->dev, name, > >>>> + parent_clk, 0, 4, 1); > > > > To do that it looks like maybe we'll need to export > > __clk_hw_register_fixed_factor() and introduces some sort of > > clk_hw_register_fixed_factor_parent_data() API. > > Setting parent_clk to NULL should be enough. > Perfect, but also weird. I worry that's a bug that snuck in. Probably a good idea to not rely on that.
On 2/19/22 04:05, Stephen Boyd wrote: > Quoting Marek Vasut (2022-02-18 17:11:04) >> On 2/18/22 23:15, Stephen Boyd wrote: >>>> >>>>>> + if (!parent_clk) >>>>>> + return dev_err_probe(&client->dev, -EINVAL, >>>>>> + "Missing XTal input clock\n"); >>>>>> + >>>>>> + rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config); >>>>>> + if (IS_ERR(rs9->regmap)) >>>>>> + return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap), >>>>>> + "Failed to allocate register map\n"); >>>>>> + >>>>>> + /* Register clock */ >>>>>> + for (i = 0; i < rs9->chip_info->num_clks; i++) { >>>>>> + name[3]++; >>>>>> + hw = clk_hw_register_fixed_factor(&client->dev, name, >>>>>> + parent_clk, 0, 4, 1); >>> >>> To do that it looks like maybe we'll need to export >>> __clk_hw_register_fixed_factor() and introduces some sort of >>> clk_hw_register_fixed_factor_parent_data() API. >> >> Setting parent_clk to NULL should be enough. >> > > Perfect, but also weird. I worry that's a bug that snuck in. Probably a > good idea to not rely on that. No, I was wrong, the index=0 is right, and it is already fixed in V2.
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 3cdf33470a750..05fc6fb2fddd5 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -340,6 +340,15 @@ config COMMON_CLK_OXNAS help Support for the OXNAS SoC Family clocks. +config COMMON_CLK_RS9_PCIE + tristate "Clock driver for Renesas 9-series PCIe clock generators" + depends on I2C + depends on OF + select REGMAP_I2C + help + This driver supports the Renesas 9-series PCIe clock generator + models 9FGV/9DBV/9DMV/9FGL/9DML/9QXL/9SQ. + config COMMON_CLK_VC5 tristate "Clock driver for IDT VersaClock 5,6 devices" depends on I2C diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 6a98291350b64..3ec27842ec779 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -68,6 +68,7 @@ obj-$(CONFIG_COMMON_CLK_STM32MP157) += clk-stm32mp1.o obj-$(CONFIG_COMMON_CLK_TPS68470) += clk-tps68470.o obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o +obj-$(CONFIG_COMMON_CLK_RS9_PCIE) += clk-renesas-pcie.o obj-$(CONFIG_COMMON_CLK_VC5) += clk-versaclock5.o obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o obj-$(CONFIG_COMMON_CLK_XGENE) += clk-xgene.o diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c new file mode 100644 index 0000000000000..0ad132cbe41b8 --- /dev/null +++ b/drivers/clk/clk-renesas-pcie.c @@ -0,0 +1,336 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Driver for Renesas 9-series PCIe clock generator driver + * + * The following series can be supported: + * - 9FGV/9DBV/9DMV/9FGL/9DML/9QXL/9SQ + * Currently supported: + * - 9FGV0241 + * + * Copyright (C) 2022 Marek Vasut <marex@denx.de> + */ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_platform.h> +#include <linux/rational.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#define RS9_REG_OE 0x0 +#define RS9_REG_OE_DIF_OE(n) BIT((n) + 1) +#define RS9_REG_SS 0x1 +#define RS9_REG_SS_AMP_0V6 0x0 +#define RS9_REG_SS_AMP_0V7 0x1 +#define RS9_REG_SS_AMP_0V8 0x2 +#define RS9_REG_SS_AMP_0V9 0x3 +#define RS9_REG_SS_AMP_MASK 0x3 +#define RS9_REG_SS_SSC_100 0 +#define RS9_REG_SS_SSC_M025 (1 << 3) +#define RS9_REG_SS_SSC_M050 (3 << 3) +#define RS9_REG_SS_SSC_MASK (3 << 3) +#define RS9_REG_SS_SSC_LOCK BIT(5) +#define RS9_REG_SR 0x2 +#define RS9_REG_SR_2V0_DIF(n) 0 +#define RS9_REG_SR_3V0_DIF(n) BIT((n) + 1) +#define RS9_REG_SR_DIF_MASK(n) BIT((n) + 1) +#define RS9_REG_REF 0x3 +#define RS9_REG_REF_OE BIT(4) +#define RS9_REG_REF_OD BIT(5) +#define RS9_REG_REF_SR_SLOWEST 0 +#define RS9_REG_REF_SR_SLOW (1 << 6) +#define RS9_REG_REF_SR_FAST (2 << 6) +#define RS9_REG_REF_SR_FASTER (3 << 6) +#define RS9_REG_VID 0x5 +#define RS9_REG_DID 0x6 +#define RS9_REG_BCP 0x7 + +/* Supported Renesas 9-series models. */ +enum rs9_model { + RENESAS_9FGV0241, +}; + +/* Structure to describe features of a particular 9-series model */ +struct rs9_chip_info { + const enum rs9_model model; + unsigned int num_clks; +}; + +struct rs9_driver_data { + struct i2c_client *client; + struct regmap *regmap; + const struct rs9_chip_info *chip_info; + struct clk *pin_xin; + struct clk_hw *clk_dif[2]; + u8 pll_amplitude; + u8 pll_ssc; + u8 clk_dif_sr; +}; + +/* + * Renesas 9-series i2c regmap + */ +static const struct regmap_range rs9_readable_ranges[] = { + regmap_reg_range(RS9_REG_OE, RS9_REG_REF), + regmap_reg_range(RS9_REG_VID, RS9_REG_BCP), +}; + +static const struct regmap_access_table rs9_readable_table = { + .yes_ranges = rs9_readable_ranges, + .n_yes_ranges = ARRAY_SIZE(rs9_readable_ranges), +}; + +static const struct regmap_range rs9_writeable_ranges[] = { + regmap_reg_range(RS9_REG_OE, RS9_REG_REF), + regmap_reg_range(RS9_REG_BCP, RS9_REG_BCP), +}; + +static const struct regmap_access_table rs9_writeable_table = { + .yes_ranges = rs9_writeable_ranges, + .n_yes_ranges = ARRAY_SIZE(rs9_writeable_ranges), +}; + +static const struct regmap_config rs9_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .cache_type = REGCACHE_RBTREE, + .max_register = 0x8, + .rd_table = &rs9_readable_table, + .wr_table = &rs9_writeable_table, +}; + +static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx) +{ + struct i2c_client *client = rs9->client; + unsigned char *name = "DIF0"; + struct device_node *np; + int ret; + u32 sr; + + /* Set defaults */ + rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx); + rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx); + + name[3] += idx; + np = of_get_child_by_name(client->dev.of_node, name); + if (!np) + return 0; + + /* Output clock slew rate */ + ret = of_property_read_u32(np, "renesas,slew-rate", &sr); + if (!ret) { + if (sr == 2000000) { /* 2V/ns */ + rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx); + rs9->clk_dif_sr |= RS9_REG_SR_2V0_DIF(idx); + } else if (sr == 3000000) { /* 3V/ns (default) */ + rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx); + rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx); + } else + ret = dev_err_probe(&client->dev, -EINVAL, + "Invalid renesas,slew-rate value\n"); + } + + of_node_put(np); + + return ret; +} + +static int rs9_get_common_config(struct rs9_driver_data *rs9) +{ + struct i2c_client *client = rs9->client; + struct device_node *np = client->dev.of_node; + unsigned int amp, ssc; + int ret; + + /* Set defaults */ + rs9->pll_amplitude = RS9_REG_SS_AMP_0V7; + rs9->pll_ssc = RS9_REG_SS_SSC_100; + + /* Output clock amplitude */ + ret = of_property_read_u32(np, "renesas,out-amplitude", &); + if (!ret) { + if (amp == 600000) /* 0.6V */ + rs9->pll_amplitude = RS9_REG_SS_AMP_0V6; + else if (amp == 700000) /* 0.7V (default) */ + rs9->pll_amplitude = RS9_REG_SS_AMP_0V7; + else if (amp == 800000) /* 0.8V */ + rs9->pll_amplitude = RS9_REG_SS_AMP_0V8; + else if (amp == 900000) /* 0.9V */ + rs9->pll_amplitude = RS9_REG_SS_AMP_0V9; + else + return dev_err_probe(&client->dev, -EINVAL, + "Invalid renesas,out-amplitude value\n"); + } + + /* Output clock spread spectrum */ + ret = of_property_read_u32(np, "renesas,out-spread-spectrum", &ssc); + if (!ret) { + if (ssc == 100000) /* 100% ... no spread (default) */ + rs9->pll_ssc = RS9_REG_SS_SSC_100; + else if (ssc == 99750) /* -0.25% ... down spread */ + rs9->pll_ssc = RS9_REG_SS_SSC_M025; + else if (ssc == 99500) /* -0.50% ... down spread */ + rs9->pll_ssc = RS9_REG_SS_SSC_M050; + else + return dev_err_probe(&client->dev, -EINVAL, + "Invalid renesas,out-spread-spectrum value\n"); + } + + return 0; +} + +static void rs9_update_config(struct rs9_driver_data *rs9) +{ + int i; + + /* If amplitude is non-default, update it. */ + if (rs9->pll_amplitude != RS9_REG_SS_AMP_0V7) { + regmap_update_bits(rs9->regmap, RS9_REG_SS, RS9_REG_SS_AMP_MASK, + rs9->pll_amplitude); + } + + /* If SSC is non-default, update it. */ + if (rs9->pll_ssc != RS9_REG_SS_SSC_100) { + regmap_update_bits(rs9->regmap, RS9_REG_SS, RS9_REG_SS_SSC_MASK, + rs9->pll_ssc); + } + + for (i = 0; i < rs9->chip_info->num_clks; i++) { + if (rs9->clk_dif_sr & RS9_REG_SR_3V0_DIF(i)) + continue; + + regmap_update_bits(rs9->regmap, RS9_REG_SR, RS9_REG_SR_3V0_DIF(i), + rs9->clk_dif_sr & RS9_REG_SR_3V0_DIF(i)); + } +} + +static struct clk_hw * +rs9_of_clk_get(struct of_phandle_args *clkspec, void *data) +{ + struct rs9_driver_data *rs9 = data; + unsigned int idx = clkspec->args[0]; + + return rs9->clk_dif[idx]; +} + +static const struct of_device_id clk_rs9_of_match[]; + +static int rs9_probe(struct i2c_client *client, const struct i2c_device_id *id) +{ + struct device_node *np = client->dev.of_node; + unsigned char *name = "DIF0"; + struct rs9_driver_data *rs9; + const char *parent_clk; + struct clk_hw *hw; + int i, ret; + + rs9 = devm_kzalloc(&client->dev, sizeof(*rs9), GFP_KERNEL); + if (!rs9) + return -ENOMEM; + + i2c_set_clientdata(client, rs9); + rs9->client = client; + rs9->chip_info = of_device_get_match_data(&client->dev); + + /* Fetch common configuration from DT (if specified) */ + ret = rs9_get_common_config(rs9); + if (ret) + return ret; + + /* Fetch DIFx output configuration from DT (if specified) */ + for (i = 0; i < rs9->chip_info->num_clks; i++) { + ret = rs9_get_output_config(rs9, i); + if (ret) + return ret; + } + + /* Mandatory XTal */ + parent_clk = of_clk_get_parent_name(np, 0); + if (!parent_clk) + return dev_err_probe(&client->dev, -EINVAL, + "Missing XTal input clock\n"); + + rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config); + if (IS_ERR(rs9->regmap)) + return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap), + "Failed to allocate register map\n"); + + /* Register clock */ + for (i = 0; i < rs9->chip_info->num_clks; i++) { + name[3]++; + hw = clk_hw_register_fixed_factor(&client->dev, name, + parent_clk, 0, 4, 1); + if (IS_ERR(hw)) + return PTR_ERR(hw); + + rs9->clk_dif[i] = hw; + } + + ret = devm_of_clk_add_hw_provider(&client->dev, rs9_of_clk_get, rs9); + if (!ret) + rs9_update_config(rs9); + + return ret; +} + +static int __maybe_unused rs9_suspend(struct device *dev) +{ + struct rs9_driver_data *rs9 = dev_get_drvdata(dev); + + regcache_cache_only(rs9->regmap, true); + regcache_mark_dirty(rs9->regmap); + + return 0; +} + +static int __maybe_unused rs9_resume(struct device *dev) +{ + struct rs9_driver_data *rs9 = dev_get_drvdata(dev); + int ret; + + regcache_cache_only(rs9->regmap, false); + ret = regcache_sync(rs9->regmap); + if (ret) + dev_err(dev, "Failed to restore register map: %d\n", ret); + return ret; +} + +static const struct rs9_chip_info renesas_9fgv0241_info = { + .model = RENESAS_9FGV0241, + .num_clks = 2, +}; + +static const struct i2c_device_id rs9_id[] = { + { "9fgv0241", .driver_data = RENESAS_9FGV0241 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, rs9_id); + +static const struct of_device_id clk_rs9_of_match[] = { + { .compatible = "renesas,9fgv0241", .data = &renesas_9fgv0241_info }, + { }, +}; +MODULE_DEVICE_TABLE(of, clk_rs9_of_match); + +static SIMPLE_DEV_PM_OPS(rs9_pm_ops, rs9_suspend, rs9_resume); + +static struct i2c_driver rs9_driver = { + .driver = { + .name = "clk-renesas-pcie-9series", + .pm = &rs9_pm_ops, + .of_match_table = clk_rs9_of_match, + }, + .probe = rs9_probe, + .id_table = rs9_id, +}; +module_i2c_driver(rs9_driver); + +MODULE_AUTHOR("Marek Vasut <marex@denx.de>"); +MODULE_DESCRIPTION("Renesas 9-series PCIe clock generator driver"); +MODULE_LICENSE("GPL");
Add driver for Renesas 9-series PCIe clock generators. This driver is designed to support 9FGV/9DBV/9DMV/9FGL/9DML/9QXL/9SQ series I2C PCIe clock generators, currently the only tested and supported chip is 9FGV0241. The driver is capable of configuring per-chip spread spectrum mode and output amplitude, as well as per-output slew rate. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Michael Turquette <mturquette@baylibre.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Stephen Boyd <sboyd@kernel.org> Cc: devicetree@vger.kernel.org To: linux-clk@vger.kernel.org --- drivers/clk/Kconfig | 9 + drivers/clk/Makefile | 1 + drivers/clk/clk-renesas-pcie.c | 336 +++++++++++++++++++++++++++++++++ 3 files changed, 346 insertions(+) create mode 100644 drivers/clk/clk-renesas-pcie.c