Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Chen-Yu Tsai]

On Tue, Apr 29, 2014 at 1:14 AM, Boris BREZILLON
<boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote:
> Hi Chen-Yu,
>
> On 28/04/2014 17:59, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Mon, Apr 28, 2014 at 10:58 PM, Boris BREZILLON
>> <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote:
>>> The PRCM (Power/Reset/Clock Management) unit provides several clock
>>> devices:
>>> - AR100 clk: used to clock the Power Management co-processor
>>> - AHB0 clk: used to clock the AHB0 bus
>>> - APB0 clk and gates: used to clk
>>>
>>> Add support for these clks in a separate driver so that they can be probed
>>> as platform devices instead of registered during early init.
>>> We need this to be able to probe PRCM MFD subdevices.
>>>
>>> Signed-off-by: Boris BREZILLON <boris.brezillon@xxxxxxxxxxxxxxxxxx>
>>> ---
>>>  drivers/clk/sunxi/Makefile         |   2 +
>>>  drivers/clk/sunxi/clk-sun6i-prcm.c | 253 +++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 255 insertions(+)
>>>  create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm.c
>>>
>>> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>>> index b5bac91..ef8cdc9 100644
>>> --- a/drivers/clk/sunxi/Makefile
>>> +++ b/drivers/clk/sunxi/Makefile
>>> @@ -3,3 +3,5 @@
>>>  #
>>>
>>>  obj-y += clk-sunxi.o clk-factors.o
>>> +
>>> +obj-$(CONFIG_MFD_SUN6I_PRCM) += clk-sun6i-prcm.o
>>> diff --git a/drivers/clk/sunxi/clk-sun6i-prcm.c b/drivers/clk/sunxi/clk-sun6i-prcm.c
>>> new file mode 100644
>>> index 0000000..bb7b25a
>>> --- /dev/null
>>> +++ b/drivers/clk/sunxi/clk-sun6i-prcm.c
>>> @@ -0,0 +1,253 @@
>>> +/*
>>> + * Copyright (C) 2014 Free Electrons
>>> + *
>>> + * License Terms: GNU General Public License v2
>>> + * Author: Boris BREZILLON <boris.brezillon@xxxxxxxxxxxxxxxxxx>
>>> + *
>>> + * Allwinner PRCM (Power/Reset/Clock Management) driver
>>> + *
>>> + */
>>> +
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define SUN6I_APB0_GATES_MAX_SIZE      32
>>> +#define SUN6I_AR100_MAX_PARENTS                4
>>> +
>>> +static int sun6i_a31_ar100_mux_clk_register(struct platform_device *pdev)
>>> +{
>>> +       const char *parents[SUN6I_AR100_MAX_PARENTS];
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       const char *clk_name = np->name;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       struct clk *clk;
>>> +       int nparents;
>>> +       int i;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>> +       if (IS_ERR(reg))
>>> +               return PTR_ERR(reg);
>>> +
>>> +       nparents = of_clk_get_parent_count(np);
>>> +       if (nparents > SUN6I_AR100_MAX_PARENTS)
>>> +               nparents = SUN6I_AR100_MAX_PARENTS;
>>> +
>>> +       for (i = 0; i < nparents; i++)
>>> +               parents[i] = of_clk_get_parent_name(np, i);
>>> +
>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>> +
>>> +       clk = clk_register_mux(&pdev->dev, clk_name, parents, nparents,
>>> +                              CLK_SET_RATE_NO_REPARENT, reg,
>>> +                              16, 2, 0, NULL);
>>> +       if (IS_ERR(clk))
>>> +               return PTR_ERR(clk);
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> +}
>>> +
>>> +static int sun6i_a31_ar100_clk_register(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       const char *clk_name = np->name;
>>> +       const char *clk_parent;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       struct clk *clk;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>> +       if (IS_ERR(reg))
>>> +               return PTR_ERR(reg);
>>> +
>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>> +       if (!clk_parent)
>>> +               return -EINVAL;
>>> +
>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>> +
>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>> +                                  0, reg, 4, 2, CLK_DIVIDER_POWER_OF_TWO,
>>> +                                  NULL);
>>> +       if (IS_ERR(clk))
>>> +               return PTR_ERR(clk);
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> +}
>>> +
>>> +static int sun6i_a31_ar100_div_clk_register(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       const char *clk_name = np->name;
>>> +       const char *clk_parent;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       struct clk *clk;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>> +       if (IS_ERR(reg))
>>> +               return PTR_ERR(reg);
>>> +
>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>> +       if (!clk_parent)
>>> +               return -EINVAL;
>>> +
>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>> +
>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>> +                                  0, reg, 8, 5, 0, NULL);
>>> +       if (IS_ERR(clk))
>>> +               return PTR_ERR(clk);
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> +}
>> Would it be possible to merge the 3 ar100 clocks into 1 composite clock?
>> They do share the same register, and are related to each other.
>
> Okay, I'll take a look at composite clocks.
>
>> It might be possible to re-use some code from the sunxi clock driver
>> for this, though I'm not sure if that's a good idea, sharing code
>> between modules. Emilio?
>
> Actually, this is what I tried in the first place, but then I realized
> it would require exporting some functions declared as static in clk-sunxi.c.
> Moreover, these functions do not make use of devm functions (because all
> the clks declared in clk-sunxi.c are initiliazed during early), and I'd
> like to uses devm function when clks are declared as platform devices.

Yeah, that's why I was uncertain about it.

> Anyway, I'll wait for Emilio's review ;-).
>
>>
>>> +
>>> +static int sun6i_a31_apb0_clk_register(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       const char *clk_name = np->name;
>>> +       const char *clk_parent;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       struct clk *clk;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>>> +       if (IS_ERR(reg))
>>> +               return PTR_ERR(reg);
>>> +
>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>> +       if (!clk_parent)
>>> +               return -EINVAL;
>>> +
>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>> +
>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>> +                                  0, reg, 0, 2, CLK_DIVIDER_POWER_OF_TWO,
>>> +                                  NULL);
>> I just looked at the sun6i kernel code again.
>>
>>   http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/ccm_i.h;hb=refs/heads/allwinner-sunxi-a31#l376
>>
>> and
>>
>>   http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/sys_clk.c;hb=refs/heads/allwinner-sunxi-a31#l882
>>
>> The divider on the A31 is /2, /2, /4, /8. You might need a table for that.
>
> I saw that in the datasheet, but I just thought it was a mistake (that's
> weird to have 2 times the same divisor).

Oh, you have the datasheet?

We also have muxes with 2 PLL6's. And Allwinner's u-boot p2wi driver also
assumes 12 MHz, instead of 24 MHz, as the source clock rate. Either they
messed up twice, or they had some reason to do it like this. The p2wi
driver in u-boot-sunxi also assumes 12 MHz, and I assume it works for
everyone with A31 hardware.

>> This is different from the A23 manual I used to document most of the PRCM.
>> I apologize for not catching this earlier. I have updated the wiki.
>
> No problem.
>
>>
>>> +       if (IS_ERR(clk))
>>> +               return PTR_ERR(clk);
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> +}
>>> +
>>> +static int sun6i_a31_apb0_gates_clk_register(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       struct clk_onecell_data *clk_data;
>>> +       const char *clk_parent;
>>> +       const char *clk_name;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       int gate_id;
>>> +       int ngates;
>>> +       int i;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>>> +       if (!reg)
>>> +               return PTR_ERR(reg);
>>> +
>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>> +       if (!clk_parent)
>>> +               return -EINVAL;
>>> +
>>> +       ngates = of_property_count_strings(np, "clock-output-names");
>>> +       if (ngates < 0)
>>> +               return ngates;
>>> +
>>> +       if (!ngates || ngates > SUN6I_APB0_GATES_MAX_SIZE)
>>> +               return -EINVAL;
>>> +
>>> +       clk_data = devm_kzalloc(&pdev->dev, sizeof(struct clk_onecell_data),
>>> +                               GFP_KERNEL);
>>> +       if (!clk_data)
>>> +               return -ENOMEM;
>>> +
>>> +       clk_data->clks = devm_kzalloc(&pdev->dev,
>>> +                                     SUN6I_APB0_GATES_MAX_SIZE *
>>> +                                     sizeof(struct clk *),
>>> +                                     GFP_KERNEL);
>>> +       if (!clk_data->clks)
>>> +               return -ENOMEM;
>>> +
>>> +       for (i = 0; i < ngates; i++) {
>>> +               of_property_read_string_index(np, "clock-output-names",
>>> +                                             i, &clk_name);
>>> +
>>> +               gate_id = i;
>>> +               of_property_read_u32_index(np, "clock-indices", i, &gate_id);
>>> +
>>> +               WARN_ON(gate_id >= SUN6I_APB0_GATES_MAX_SIZE);
>>> +               if (gate_id >= SUN6I_APB0_GATES_MAX_SIZE)
>>> +                       continue;
>>> +
>>> +               clk_data->clks[gate_id] = clk_register_gate(&pdev->dev,
>>> +                                                           clk_name,
>>> +                                                           clk_parent, 0,
>>> +                                                           reg, gate_id,
>>> +                                                           0, NULL);
>>> +               WARN_ON(IS_ERR(clk_data->clks[gate_id]));
>>> +       }
>>> +
>>> +       clk_data->clk_num = ngates;
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
>>> +}
>>> +
>>> +const struct of_device_id sun6i_a31_prcm_clk_dt_ids[] = {
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-ar100-mux-clk",
>>> +               .data = sun6i_a31_ar100_mux_clk_register,
>>> +       },
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-ar100-clk",
>>> +               .data = sun6i_a31_ar100_clk_register,
>>> +       },
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-ar100-div-clk",
>>> +               .data = sun6i_a31_ar100_div_clk_register,
>>> +       },
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-apb0-clk",
>>> +               .data = sun6i_a31_apb0_clk_register,
>>> +       },
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-apb0-gates-clk",
>>> +               .data = sun6i_a31_apb0_gates_clk_register,
>>> +       },
>>> +       { /* sentinel */ }
>>> +};
>>> +
>>> +static int sun6i_a31_prcm_clk_probe(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       int (*register_func)(struct platform_device *pdev);
>>> +       const struct of_device_id *match;
>>> +
>>> +       match = of_match_node(sun6i_a31_prcm_clk_dt_ids, np);
>>> +       if (!match)
>>> +               return -EINVAL;
>>> +
>>> +       register_func = match->data;
>>> +       return register_func(pdev);
>>> +}
>>> +
>>> +static struct platform_driver sun6i_a31_prcm_clk_driver = {
>>> +       .driver = {
>>> +               .name = "sun6i-a31-prcm-clk",
>>> +               .owner = THIS_MODULE,
>>> +               .of_match_table = sun6i_a31_prcm_clk_dt_ids,
>>> +       },
>>> +       .probe = sun6i_a31_prcm_clk_probe,
>>> +};
>>> +module_platform_driver(sun6i_a31_prcm_clk_driver);
>>> +
>>> +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon@xxxxxxxxxxxxxxxxxx");
>>> +MODULE_DESCRIPTION("Allwinner Reset Controller Driver");
>>> +MODULE_LICENSE("GPL v2");
>> The rest looks good to me, but Emilio should be able to give you more feedback.
>>
>> Thanks for all the sun6i support you've done!
>
> Thanks for your review.
>
> Best Regards,
>
> Boris
>>
>> Cheers
>> ChenYu
>
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/