On 7.5.2018 03:20, James Kelly wrote:
> Replace existing CCF clock providers with new clock provider that can
> be enhanced to meet our needs.
>
> AXI clock prepare/enable/disable/unprepare is now managed by regmap APIs.
>
> Unregistering of clk instances now handled by devm APIs.
>
> Drop warning that fractional ratios are not supported because it used
> undocumented register fields and it won't be needed anymore.
>
> Signed-off-by: James Kelly <jamespeterkelly@xxxxxxxxx>
> ---
> .../clocking-wizard/clk-xlnx-clock-wizard.c | 211 +++++++--------------
> 1 file changed, 71 insertions(+), 140 deletions(-)
>
> diff --git a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
> index ba9b1dc93d50..1dbeda92fb9a 100644
> --- a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
> +++ b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
> @@ -76,24 +76,6 @@
> #define WZRD_CLKNAME_IN1 "clk_in1"
> #define WZRD_FLAG_MULTIPLY BIT(0)
>
> -#define WZRD_CLK_CFG_REG(n) (0x200 + 4 * (n))
> -
> -#define WZRD_CLKOUT0_FRAC_EN BIT(18)
> -#define WZRD_CLKFBOUT_FRAC_EN BIT(26)
> -
> -#define WZRD_CLKFBOUT_MULT_SHIFT 8
> -#define WZRD_CLKFBOUT_MULT_MASK (0xff << WZRD_CLKFBOUT_MULT_SHIFT)
> -#define WZRD_DIVCLK_DIVIDE_SHIFT 0
> -#define WZRD_DIVCLK_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
> -#define WZRD_CLKOUT_DIVIDE_SHIFT 0
> -#define WZRD_CLKOUT_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
> -
> -enum clk_wzrd_int_clks {
> - wzrd_clk_mul_div,
> - wzrd_clk_mul,
> - wzrd_clk_int_max
> -};
> -
> enum clk_wzrd_clk {
> WZRD_CLK_DIV,
> WZRD_CLK_PLL,
> @@ -149,14 +131,13 @@ struct clk_wzrd_clk_data {
> container_of(_hw, struct clk_wzrd_clk_data, hw)
>
> /**
> - * struct clk_wzrd:
> - * @clk_data: Clock data
> + * struct clk_wzrd - Platform driver data for clocking wizard device
> + *
> + * @clk_data: Clock data accessible to other devices via device tree
> * @nb: Notifier block
> - * @base: Memory base
> * @regmap: Register map for hardware
> * @clk_in1: Handle to input clock 'clk_in1'
> * @axi_clk: Handle to input clock 's_axi_aclk'
> - * @clks_internal: Internal clocks
> * @clkout: Output clocks
> * @speed_grade: Speed grade of the device
> * @suspended: Flag indicating power state of the device
> @@ -167,11 +148,9 @@ struct clk_wzrd_clk_data {
> struct clk_wzrd {
> struct clk_onecell_data clk_data;
> struct notifier_block nb;
> - void __iomem *base;
> struct regmap *regmap;
> struct clk *clk_in1;
> struct clk *axi_clk;
> - struct clk *clks_internal[wzrd_clk_int_max];
> struct clk *clkout[WZRD_NUM_OUTPUTS];
> unsigned int speed_grade;
> bool suspended;
> @@ -179,7 +158,6 @@ struct clk_wzrd {
> struct clk_wzrd_clk_data pll_data;
> struct clk_wzrd_clk_data clkout_data[0];
> };
> -
> #define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb)
>
> /* maximum frequencies for input/output clocks per speed grade */
> @@ -321,7 +299,6 @@ static int __maybe_unused clk_wzrd_suspend(struct device *dev)
> {
> struct clk_wzrd *cw = dev_get_drvdata(dev);
>
> - clk_disable_unprepare(cw->axi_clk);
> cw->suspended = true;
>
> return 0;
> @@ -329,15 +306,8 @@ static int __maybe_unused clk_wzrd_suspend(struct device *dev)
>
> static int __maybe_unused clk_wzrd_resume(struct device *dev)
> {
> - int ret;
> struct clk_wzrd *cw = dev_get_drvdata(dev);
>
> - ret = clk_prepare_enable(cw->axi_clk);
> - if (ret) {
> - dev_err(dev, "unable to enable s_axi_aclk\n");
> - return ret;
> - }
> -
> cw->suspended = false;
>
> return 0;
> @@ -348,17 +318,32 @@ static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops, clk_wzrd_suspend,
>
> static int clk_wzrd_get_device_tree_data(struct device *dev)
> {
> - int ret;
> - unsigned long rate;
> + int num_outputs, ret;
> struct clk_wzrd *cw;
> + struct device_node *node = dev->of_node;
> +
> + num_outputs = of_property_count_strings(node,
> + "clock-output-names");
> + if (num_outputs < 1) {
> + dev_err(dev, "No clock output names in device tree\n");
> + if (num_outputs < 0)
> + return num_outputs;
> + else
> + return -EINVAL;
> + }
> + if (num_outputs > WZRD_NUM_OUTPUTS) {
> + dev_warn(dev, "Too many clock output names in device tree\n");
> + num_outputs = WZRD_NUM_OUTPUTS;
> + }
>
> - cw = devm_kzalloc(dev, sizeof(*cw), GFP_KERNEL);
> + cw = devm_kzalloc(dev, sizeof(*cw) + num_outputs *
> + sizeof(struct clk_wzrd_clk_data), GFP_KERNEL);
> if (!cw)
> return -ENOMEM;
> dev_set_drvdata(dev, cw);
> + cw->clk_data.clk_num = num_outputs;
>
> - ret = of_property_read_u32(dev->of_node, "speed-grade",
> - &cw->speed_grade);
> + ret = of_property_read_u32(node, "speed-grade", &cw->speed_grade);
> if (!ret) {
> if (cw->speed_grade < 1 || cw->speed_grade > 3) {
> dev_warn(dev, "invalid speed grade '%d'\n",
> @@ -380,18 +365,12 @@ static int clk_wzrd_get_device_tree_data(struct device *dev)
> dev_err(dev, "Clock %s not found\n", WZRD_CLKNAME_AXI);
> return PTR_ERR(cw->axi_clk);
> }
> - ret = clk_prepare_enable(cw->axi_clk);
> + ret = clk_set_max_rate(cw->axi_clk, WZRD_ACLK_MAX_FREQ);
> if (ret) {
> - dev_err(dev, "enabling %s failed\n", WZRD_CLKNAME_AXI);
> + dev_err(dev, "Unable to set clock %s to valid range\n",
> + WZRD_CLKNAME_AXI);
> return ret;
> }
> - rate = clk_get_rate(cw->axi_clk);
> - if (rate > WZRD_ACLK_MAX_FREQ) {
> - dev_err(dev, "%s frequency (%lu) too high\n", WZRD_CLKNAME_AXI,
> - rate);
> - clk_disable_unprepare(cw->axi_clk);
> - return -EINVAL;
> - }
>
> return 0;
> }
> @@ -399,12 +378,18 @@ static int clk_wzrd_get_device_tree_data(struct device *dev)
> static int clk_wzrd_regmap_alloc(struct device *dev)
> {
> struct resource *mem;
> + void __iomem *base;
> struct clk_wzrd *cw = dev_get_drvdata(dev);
>
> mem = platform_get_resource(to_platform_device(dev), IORESOURCE_MEM, 0);
> - cw->base = devm_ioremap_resource(dev, mem);
> - if (IS_ERR(cw->base))
> - return PTR_ERR(cw->base);
> + base = devm_ioremap_resource(dev, mem);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + cw->regmap = devm_regmap_init_mmio_clk(dev, WZRD_CLKNAME_AXI,
> + base, &clk_wzrd_regmap_config);
> + if (IS_ERR(cw->regmap))
> + return PTR_ERR(cw->regmap);
>
> return 0;
> }
> @@ -412,115 +397,68 @@ static int clk_wzrd_regmap_alloc(struct device *dev)
> static int clk_wzrd_register_ccf(struct device *dev)
> {
> int i, ret;
> - u32 reg;
> - const char *clk_name;
> + const char *clk_div_name;
> + const char *clk_pll_name;
> +
> struct clk_wzrd *cw = dev_get_drvdata(dev);
>
> - /* we don't support fractional div/mul yet */
> - reg = readl(cw->base + WZRD_CLK_CFG_REG(0)) &
> - WZRD_CLKFBOUT_FRAC_EN;
> - reg |= readl(cw->base + WZRD_CLK_CFG_REG(2)) &
> - WZRD_CLKOUT0_FRAC_EN;
> - if (reg)
> - dev_warn(dev, "fractional div/mul not supported\n");
> -
> - /* register div */
> - reg = (readl(cw->base + WZRD_CLK_CFG_REG(0)) &
> - WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
> - clk_name = kasprintf(GFP_KERNEL, "%s_mul_div", dev_name(dev));
> - if (!clk_name) {
> - ret = -ENOMEM;
> - goto err_disable_clk;
> - }
> - cw->clks_internal[wzrd_clk_mul_div] = clk_register_fixed_factor(
> - dev, clk_name,
> - __clk_get_name(cw->clk_in1),
> - 0, 1, reg);
> - kfree(clk_name);
> - if (IS_ERR(cw->clks_internal[wzrd_clk_mul_div])) {
> - dev_err(dev, "unable to register divider clock\n");
> - ret = PTR_ERR(cw->clks_internal[wzrd_clk_mul_div]);
> - goto err_disable_clk;
> - }
> + clk_div_name = kasprintf(GFP_KERNEL, "%s_div", dev_name(dev));
> + if (!clk_div_name)
> + return -ENOMEM;
> + ret = clk_wzrd_register_clk(dev, clk_div_name, WZRD_CLK_DIV, 0, 0);
> + if (ret)
> + goto err_free_div_name;
>
> - /* register multiplier */
> - reg = (readl(cw->base + WZRD_CLK_CFG_REG(0)) &
> - WZRD_CLKFBOUT_MULT_MASK) >> WZRD_CLKFBOUT_MULT_SHIFT;
> - clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(dev));
> - if (!clk_name) {
> + clk_pll_name = kasprintf(GFP_KERNEL, "%s_pll", dev_name(dev));
> + if (!clk_pll_name) {
> ret = -ENOMEM;
> - goto err_rm_int_clk;
> - }
> - cw->clks_internal[wzrd_clk_mul] = clk_register_fixed_factor(
> - dev, clk_name,
> - __clk_get_name(cw->clks_internal[wzrd_clk_mul_div]),
> - 0, reg, 1);
> - if (IS_ERR(cw->clks_internal[wzrd_clk_mul])) {
> - dev_err(dev, "unable to register fixed-factor clock\n");
> - ret = PTR_ERR(cw->clks_internal[wzrd_clk_mul]);
> - goto err_rm_int_clk;
> + goto err_free_div_name;
> }
> + ret = clk_wzrd_register_clk(dev, clk_pll_name, WZRD_CLK_PLL, 0, 0);
> + if (ret)
> + goto err_free_pll_name;
>
> - /* register div per output */
> - for (i = WZRD_NUM_OUTPUTS - 1; i >= 0 ; i--) {
> + for (i = cw->clk_data.clk_num - 1; i >= 0 ; i--) {
> const char *clkout_name;
>
> if (of_property_read_string_index(dev->of_node,
> "clock-output-names", i,
> &clkout_name)) {
> - dev_err(dev,
> - "clock output name not specified\n");
> + dev_err(dev, "clock output %d name not specified\n", i);
> ret = -EINVAL;
> - goto err_rm_int_clks;
> + goto err_free_pll_name;
> }
> - reg = readl(cw->base + WZRD_CLK_CFG_REG(2) + i * 12);
> - reg &= WZRD_CLKOUT_DIVIDE_MASK;
> - reg >>= WZRD_CLKOUT_DIVIDE_SHIFT;
> - cw->clkout[i] = clk_register_fixed_factor(dev, clkout_name,
> - clk_name, 0, 1, reg);
> - if (IS_ERR(cw->clkout[i])) {
> - int j;
> -
> - for (j = i + 1; j < WZRD_NUM_OUTPUTS; j++)
> - clk_unregister(cw->clkout[j]);
> - dev_err(dev,
> - "unable to register divider clock\n");
> - ret = PTR_ERR(cw->clkout[i]);
> - goto err_rm_int_clks;
> - }
> - }
>
> - kfree(clk_name);
> + ret = clk_wzrd_register_clk(dev, clkout_name, WZRD_CLK_OUT,
> + i, 0);
> + if (ret)
> + goto err_free_pll_name;
> +
> + cw->clkout[i] = cw->clkout_data[i].hw.clk;
> + }
>
> cw->clk_data.clks = cw->clkout;
> - cw->clk_data.clk_num = ARRAY_SIZE(cw->clkout);
> of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
> &cw->clk_data);
>
> if (cw->speed_grade) {
> cw->nb.notifier_call = clk_wzrd_clk_notifier;
>
> - ret = clk_notifier_register(cw->clk_in1,
> - &cw->nb);
> - if (ret)
> - dev_warn(dev,
> - "unable to register clock notifier\n");
> + ret = clk_notifier_register(cw->clk_in1, &cw->nb);
> + if (ret)
> + dev_warn(dev, "Unable to register input clock notifier\n");
>
> ret = clk_notifier_register(cw->axi_clk, &cw->nb);
> - if (ret)
> - dev_warn(dev,
> - "unable to register clock notifier\n");
> + if (ret)
> + dev_warn(dev, "Unable to register AXI clock notifier\n");
> }
>
> - return 0;
> + ret = 0;
>
> -err_rm_int_clks:
> - clk_unregister(cw->clks_internal[1]);
> -err_rm_int_clk:
> - kfree(clk_name);
> - clk_unregister(cw->clks_internal[0]);
> -err_disable_clk:
> - clk_disable_unprepare(cw->axi_clk);
> +err_free_pll_name:
> + kfree(clk_pll_name);
> +err_free_div_name:
> + kfree(clk_div_name);
>
> return ret;
> }
> @@ -547,23 +485,15 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>
> static int clk_wzrd_remove(struct platform_device *pdev)
> {
> - int i;
> struct clk_wzrd *cw = platform_get_drvdata(pdev);
>
> of_clk_del_provider(pdev->dev.of_node);
>
> - for (i = 0; i < WZRD_NUM_OUTPUTS; i++)
> - clk_unregister(cw->clkout[i]);
> - for (i = 0; i < wzrd_clk_int_max; i++)
> - clk_unregister(cw->clks_internal[i]);
> -
> if (cw->speed_grade) {
> clk_notifier_unregister(cw->axi_clk, &cw->nb);
> clk_notifier_unregister(cw->clk_in1, &cw->nb);
> }
>
> - clk_disable_unprepare(cw->axi_clk);
> -
> return 0;
> }
>
> @@ -577,6 +507,7 @@ static struct platform_driver clk_wzrd_driver = {
> .driver = {
> .name = "clk-wizard",
> .of_match_table = clk_wzrd_ids,
> + .owner = THIS_MODULE,
This is not needed and coccinelle check this.
drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c:510:3-8: No need
to set .owner here. The core will do it.
Also there is one more warning.
drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c:391:1-3:
WARNING: PTR_ERR_OR_ZERO can be used
Thanks,
Michal
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
