Re: [PATCH v4 25/37] clk: ingenic: add driver for Ingenic SoC CGU clocks

[James Hogan <james.hogan@imgtec.com>]

[-- Attachment #1: Type: text/plain, Size: 3973 bytes --]

Hi Paul,

On 24/04/15 14:17, Paul Burton wrote:
> +static int register_clock(struct ingenic_cgu *cgu, unsigned idx)
> +{
> +	const struct ingenic_cgu_clk_info *clk_info = &cgu->clock_info[idx];
> +	struct clk_init_data clk_init;
> +	struct ingenic_clk *ingenic_clk = NULL;
> +	struct clk *clk, *parent;
> +	const char *parent_names[4];
> +	unsigned caps, i, num_possible;
> +	int err = -EINVAL;
> +
> +	BUILD_BUG_ON(ARRAY_SIZE(clk_info->parents) > ARRAY_SIZE(parent_names));
> +
> +	if (clk_info->type == CGU_CLK_EXT) {
> +		clk = of_clk_get_by_name(cgu->np, clk_info->name);
> +		if (IS_ERR(clk)) {
> +			pr_err("%s: no external clock '%s' provided\n",
> +			       __func__, clk_info->name);
> +			err = -ENODEV;
> +			goto out;
> +		}
> +		err = clk_register_clkdev(clk, clk_info->name, NULL);
> +		if (err) {
> +			clk_put(clk);
> +			goto out;
> +		}
> +		cgu->clocks.clks[idx] = clk;
> +		return 0;
> +	}
> +
> +	if (!clk_info->type) {
> +		pr_err("%s: no clock type specified for '%s'\n", __func__,
> +		       clk_info->name);
> +		goto out;
> +	}
> +
> +	ingenic_clk = kzalloc(sizeof(*ingenic_clk), GFP_KERNEL);
> +	if (!ingenic_clk) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ingenic_clk->hw.init = &clk_init;
> +	ingenic_clk->cgu = cgu;
> +	ingenic_clk->idx = idx;
> +
> +	clk_init.name = clk_info->name;
> +	clk_init.flags = 0;
> +	clk_init.parent_names = parent_names;
> +
> +	caps = clk_info->type;
> +
> +	if (caps & (CGU_CLK_MUX | CGU_CLK_CUSTOM)) {
> +		clk_init.num_parents = 0;
> +
> +		if (caps & CGU_CLK_MUX)
> +			num_possible = 1 << clk_info->mux.bits;
> +		else
> +			num_possible = ARRAY_SIZE(clk_info->parents);
> +
> +		for (i = 0; i < num_possible; i++) {
> +			if (clk_info->parents[i] == -1)
> +				continue;
> +
> +			parent = cgu->clocks.clks[clk_info->parents[i]];
> +			parent_names[clk_init.num_parents] =
> +				__clk_get_name(parent);
> +			clk_init.num_parents++;
> +		}
> +
> +		BUG_ON(!clk_init.num_parents);
> +		BUG_ON(clk_init.num_parents > ARRAY_SIZE(parent_names));
> +	} else {
> +		BUG_ON(clk_info->parents[0] == -1);
> +		clk_init.num_parents = 1;
> +		parent = cgu->clocks.clks[clk_info->parents[0]];
> +		parent_names[0] = __clk_get_name(parent);
> +	}
> +
> +	if (caps & CGU_CLK_CUSTOM) {
> +		clk_init.ops = clk_info->custom.clk_ops;
> +
> +		caps &= ~CGU_CLK_CUSTOM;
> +
> +		if (caps) {
> +			pr_err("%s: custom clock may not be combined with type 0x%x\n",
> +			       __func__, caps);
> +			goto out;
> +		}
> +	} else if (caps & CGU_CLK_PLL) {
> +		clk_init.ops = &ingenic_pll_ops;
> +
> +		caps &= ~CGU_CLK_PLL;
> +
> +		if (caps) {
> +			pr_err("%s: PLL may not be combined with type 0x%x\n",
> +			       __func__, caps);
> +			goto out;
> +		}
> +	} else {
> +		clk_init.ops = &ingenic_clk_ops;
> +	}
> +
> +	/* nothing to do for gates or fixed dividers */
> +	caps &= ~(CGU_CLK_GATE | CGU_CLK_FIXDIV);
> +
> +	if (caps & CGU_CLK_MUX) {
> +		if (!(caps & CGU_CLK_MUX_GLITCHFREE))
> +			clk_init.flags |= CLK_SET_PARENT_GATE;
> +
> +		caps &= ~(CGU_CLK_MUX | CGU_CLK_MUX_GLITCHFREE);
> +	}
> +
> +	if (caps & CGU_CLK_DIV) {
> +		caps &= ~CGU_CLK_DIV;
> +	} else {
> +		/* pass rate changes to the parent clock */
> +		clk_init.flags |= CLK_SET_RATE_PARENT;
> +	}
> +
> +	if (caps) {
> +		pr_err("%s: unknown clock type 0x%x\n", __func__, caps);
> +		goto out;
> +	}
> +
> +	clk = clk_register(NULL, &ingenic_clk->hw);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: failed to register clock '%s'\n", __func__,
> +		       clk_info->name);
> +		err = PTR_ERR(clk);
> +		goto out;
> +	}
> +
> +	err = clk_register_clkdev(clk, clk_info->name, NULL);
> +	if (err)
> +		goto out;

Again, should this unregister the clock in the event of failure?

> +
> +	cgu->clocks.clks[idx] = clk;
> +out:
> +	if (err)
> +		kfree(ingenic_clk);
> +	return err;
> +}

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]