Re: [PATCH v3 2/4] clk: arizona: Add clock driver for the Arizona devices

Stephen Boyd
2016-05-07T00:55:06Z

I've applied this to clk-next but still have a question, see
below.

On 01/08, Charles Keepax wrote:
> diff --git a/drivers/clk/clk-arizona.c b/drivers/clk/clk-arizona.c
> new file mode 100644
> index 0000000..eaf2877
> --- /dev/null
> +++ b/drivers/clk/clk-arizona.c
> +
> +static int arizona_clk_of_get_pdata(struct arizona *arizona)
> +{
> + const char * const pins[] = { "mclk1", "mclk2" };
> + struct clk *mclk;
> + int i;
> +
> + if (!of_property_read_bool(arizona->dev->of_node, "clocks"))
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(pins); ++i) {
> + mclk = of_clk_get_by_name(arizona->dev->of_node, pins[i]);
> + if (IS_ERR(mclk))
> + return PTR_ERR(mclk);
> +
> + if (clk_get_rate(mclk) == CLK32K_RATE) {
> + arizona->pdata.clk32k_src = ARIZONA_32KZ_MCLK1 + i;
> + arizona->pdata.clk32k_parent = __clk_get_name(mclk);
> + }
> +
> + clk_put(mclk);

Could this be done through assigned parents instead of this rate
checking stuff? Presumably DT could tell us how the clk tree
should be configured.

> + }
> +
> + return 0;
> +}
> +
> +static int arizona_clk_probe(struct platform_device *pdev)
> +{
> + struct arizona *arizona = dev_get_drvdata(pdev->dev.parent);
> + struct arizona_clk *clkdata;
> + int ret;
> +
> + struct clk_init_data clk32k_init = {
> + .name = "arizona-32k",
> + .ops = &arizona_32k_ops,
> + };
> +
> + if (IS_ENABLED(CONFIG_OF) && !dev_get_platdata(arizona->dev)) {
> + ret = arizona_clk_of_get_pdata(arizona);
> + if (ret) {
> + dev_err(arizona->dev, "Failed parsing clock DT: %d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + clkdata = devm_kzalloc(&pdev->dev, sizeof(*clkdata), GFP_KERNEL);
> + if (!clkdata)
> + return -ENOMEM;
> +
> + clkdata->arizona = arizona;
> +
> + switch (arizona->pdata.clk32k_src) {
> + case 0:
> + arizona->pdata.clk32k_src = ARIZONA_32KZ_MCLK2;
> + /* Fall through */
> + case ARIZONA_32KZ_MCLK1:
> + case ARIZONA_32KZ_MCLK2:
> + case ARIZONA_32KZ_NONE:
> + regmap_update_bits(arizona->regmap, ARIZONA_CLOCK_32K_1,
> + ARIZONA_CLK_32K_SRC_MASK,
> + arizona->pdata.clk32k_src - 1);
> + break;
> + default:
> + dev_err(arizona->dev, "Invalid 32kHz clock source: %d\n",
> + arizona->pdata.clk32k_src);
> + return -EINVAL;
> + }
> +
> + if (arizona->pdata.clk32k_parent) {
> + clk32k_init.num_parents = 1;
> + clk32k_init.parent_names = &arizona->pdata.clk32k_parent;
> + } else {
> + clk32k_init.flags |= CLK_IS_ROOT;

This flag is going away so I deleted it.

> + }
> +
> + clkdata->clk32k_hw.init = &clk32k_init;
> + clkdata->clk32k = devm_clk_register(&pdev->dev, &clkdata->clk32k_hw);
> + if (IS_ERR(clkdata->clk32k)) {
> + ret = PTR_ERR(clkdata->clk32k);
> + dev_err(arizona->dev, "Failed to register 32k clock: %d\n",
> + ret);
> + return ret;
> + }

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project