Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1299550 > unrolled thread

Re: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

Started byMichael Turquette <mturquette@baylibre.com>
First post2015-12-31 02:40 +0100
Last post2016-01-02 02:30 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs Michael Turquette <mturquette@baylibre.com> - 2015-12-31 02:40 +0100
    Re: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-01-02 02:30 +0100

#1299550 — Re: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

FromMichael Turquette <mturquette@baylibre.com>
Date2015-12-31 02:40 +0100
SubjectRe: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs
Message-ID<qLAnE-68n-7@gated-at.bofh.it>
Hello Yamada-san,

Quoting Masahiro Yamada (2015-12-28 02:20:58)
> diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
> new file mode 100644
> index 0000000..7606f27
> --- /dev/null
> +++ b/drivers/clk/uniphier/Kconfig
> @@ -0,0 +1,35 @@
> +menuconfig CLK_UNIPHIER
> +       bool "Clock drivers for UniPhier SoCs"
> +       depends on ARCH_UNIPHIER

Might want to make the above line:

	depends on ARCH_UNIPHIER || COMPILE_TEST

> +       depends on OF
> +       default y
> +       help
> +         Supports clock drivers for UniPhier SoCs.

Why menuconfig? Can we make this a non-visible config option selected by
the UniPhier platform?

> +
> +if CLK_UNIPHIER

Please drop the if statement here and have the below options 'depends'
on CLK_UNIPHIER.

> +
> +config CLK_UNIPHIER_PH1_SLD3
> +       bool "Clock driver for UniPhier PH1-sLD3 SoC"
> +       default y

Can you make these drivers into loadable modules? If so you might
consider tristate.

> +
> +config CLK_UNIPHIER_PH1_LD4
> +       bool "Clock driver for UniPhier PH1-LD4 SoC"
> +       default y
> +
> +config CLK_UNIPHIER_PH1_PRO4
> +       bool "Clock driver for UniPhier PH1-Pro4 SoC"
> +       default y
> +
> +config CLK_UNIPHIER_PH1_SLD8
> +       bool "Clock driver for UniPhier PH1-sLD8 SoC"
> +       default y
> +
> +config CLK_UNIPHIER_PH1_PRO5
> +       bool "Clock driver for UniPhier PH1-Pro5 SoC"
> +       default y
> +
> +config CLK_UNIPHIER_PROXSTREAM2
> +       bool "Clock driver for UniPhier ProXstream2/PH1-LD6b SoC"
> +       default y
> +
> +endif
> diff --git a/drivers/clk/uniphier/Makefile b/drivers/clk/uniphier/Makefile
> new file mode 100644
> index 0000000..3be1a17
> --- /dev/null
> +++ b/drivers/clk/uniphier/Makefile
> @@ -0,0 +1,10 @@
> +obj-y += clk-uniphier-core.o
> +obj-y += clk-uniphier-peri.o
> +obj-y += clk-uniphier-mio.o
> +
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD3)    += clk-ph1-sld3.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_LD4)     += clk-ph1-ld4.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_PRO4)    += clk-ph1-pro4.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD8)    += clk-ph1-sld8.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_PRO5)    += clk-ph1-pro5.o
> +obj-$(CONFIG_CLK_UNIPHIER_PROXSTREAM2) += clk-proxstream2.o
> diff --git a/drivers/clk/uniphier/clk-ph1-ld4.c b/drivers/clk/uniphier/clk-ph1-ld4.c
> new file mode 100644
> index 0000000..48d342f
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-ph1-ld4.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro@socionext.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "clk-uniphier.h"
> +
> +static struct uniphier_clk_init_data ph1_ld4_clk_idata[] __initdata = {
> +       {
> +               .name = "spll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 65,
> +                       .div = 1,
> +               },
> +       },
> +       {
> +               .name = "upll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 288000,
> +                       .div = 24576,
> +               },
> +       },
> +       {
> +               .name = "a2pll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 24,
> +                       .div = 1,
> +               },
> +       },
> +       {
> +               .name = "uart",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 3,
> +               .data.factor = {
> +                       .parent_name = "a2pll",
> +                       .mult = 1,
> +                       .div = 16,
> +               },
> +       },
> +       {
> +               .name = "i2c",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 4,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 16,
> +               },
> +       },
> +       {
> +               .name = "arm-scu",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 7,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 32,
> +               },
> +       },
> +       {
> +               .name = "stdmac-clken",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = -1,
> +               .data.gate = {
> +                       .parent_name = NULL,
> +                       .reg = 0x2104,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "stdmac",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = 10,
> +               .data.gate = {
> +                       .parent_name = "stdmac-clken",
> +                       .reg = 0x2000,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "ehci",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 18,
> +               .data.factor = {
> +                       .parent_name = "upll",
> +                       .mult = 1,
> +                       .div = 12,
> +               },
> +       },
> +       { /* sentinel */ }
> +};
> +
> +static void __init ph1_ld4_clk_init(struct device_node *np)
> +{
> +       uniphier_clk_init(np, ph1_ld4_clk_idata);
> +}
> +CLK_OF_DECLARE(ph1_ld4_clk, "socionext,ph1-ld4-sysctrl", ph1_ld4_clk_init);

Can you avoid using CLK_OF_DECLARE and instead make this into a
platform_driver? For examples please see drivers/clk/qcom*.c

> diff --git a/drivers/clk/uniphier/clk-ph1-pro4.c b/drivers/clk/uniphier/clk-ph1-pro4.c
> new file mode 100644
> index 0000000..052813a
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-ph1-pro4.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro@socionext.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "clk-uniphier.h"
> +
> +static struct uniphier_clk_init_data ph1_pro4_clk_idata[] __initdata = {
> +       {
> +               .name = "spll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 64,
> +                       .div = 1,
> +               },
> +       },
> +       {
> +               .name = "upll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 288,
> +                       .div = 25,
> +               },
> +       },
> +       {
> +               .name = "a2pll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = "upll",
> +                       .mult = 256,
> +                       .div = 125,
> +               },
> +       },
> +       {
> +               .name = "uart",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 3,
> +               .data.factor = {
> +                       .parent_name = "a2pll",
> +                       .mult = 1,
> +                       .div = 8,
> +               },
> +       },
> +       {
> +               .name = "fi2c",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 4,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 32,
> +               },
> +       },
> +       {
> +               .name = "arm-scu",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 7,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 32,
> +               },
> +       },
> +       {
> +               .name = "stdmac-clken",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = -1,
> +               .data.gate = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .reg = 0x2104,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "stdmac",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = 10,
> +               .data.gate = {
> +                       .parent_name = "stdmac-clken",
> +                       .reg = 0x2000,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "ehci",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 18,
> +               .data.factor = {
> +                       .parent_name = "upll",
> +                       .mult = 1,
> +                       .div = 12,
> +               },
> +       },
> +       { /* sentinel */ }
> +};
> +
> +static void __init ph1_pro4_clk_init(struct device_node *np)
> +{
> +       uniphier_clk_init(np, ph1_pro4_clk_idata);
> +}
> +CLK_OF_DECLARE(ph1_pro4_clk, "socionext,ph1-pro4-sysctrl", ph1_pro4_clk_init);

Ditto.

> diff --git a/drivers/clk/uniphier/clk-ph1-pro5.c b/drivers/clk/uniphier/clk-ph1-pro5.c
> new file mode 100644
> index 0000000..9f782d9
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-ph1-pro5.c
> @@ -0,0 +1,107 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro@socionext.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "clk-uniphier.h"
> +
> +static struct uniphier_clk_init_data ph1_pro5_clk_idata[] __initdata = {
> +       {
> +               .name = "spll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 120,
> +                       .div = 1,
> +               },
> +       },
> +       {
> +               .name = "dapll1",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 128,
> +                       .div = 125,
> +               },
> +       },
> +       {
> +               .name = "dapll2",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = "upll",
> +                       .mult = 144,
> +                       .div = 5,
> +               },
> +       },
> +       {
> +               .name = "uart",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 3,
> +               .data.factor = {
> +                       .parent_name = "dapll2",
> +                       .mult = 1,
> +                       .div = 8,
> +               },
> +       },
> +       {
> +               .name = "fi2c",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 4,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 48,
> +               },
> +       },
> +       {
> +               .name = "arm-scu",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 7,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 48,
> +               },
> +       },
> +       {
> +               .name = "stdmac-clken",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = -1,
> +               .data.gate = {
> +                       .parent_name = NULL,
> +                       .reg = 0x2104,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "stdmac",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = 10,
> +               .data.gate = {
> +                       .parent_name = "stdmac-clken",
> +                       .reg = 0x2000,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       { /* sentinel */ }
> +};
> +
> +static void __init ph1_pro5_clk_init(struct device_node *np)
> +{
> +       uniphier_clk_init(np, ph1_pro5_clk_idata);
> +}
> +CLK_OF_DECLARE(ph1_pro5_clk, "socionext,ph1-pro5-sysctrl", ph1_pro5_clk_init);

Ditto.

> diff --git a/drivers/clk/uniphier/clk-ph1-sld3.c b/drivers/clk/uniphier/clk-ph1-sld3.c
> new file mode 100644
> index 0000000..7249a81
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-ph1-sld3.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro@socionext.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "clk-uniphier.h"
> +
> +static struct uniphier_clk_init_data ph1_sld3_clk_idata[] __initdata = {
> +       {
> +               .name = "spll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 65,
> +                       .div = 1,
> +               },
> +       },
> +       {
> +               .name = "upll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 288000,
> +                       .div = 24576,
> +               },
> +       },
> +       {
> +               .name = "a2pll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 24,
> +                       .div = 1,
> +               },
> +       },
> +       {
> +               .name = "uart",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 3,
> +               .data.factor = {
> +                       .parent_name = "a2pll",
> +                       .mult = 1,
> +                       .div = 16,
> +               },
> +       },
> +       {
> +               .name = "i2c",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 4,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 16,
> +               },
> +       },
> +       {
> +               .name = "arm-scu",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 7,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 32,
> +               },
> +       },
> +       {
> +               .name = "stdmac-clken",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = -1,
> +               .data.gate = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .reg = 0x2104,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "stdmac",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = 10,
> +               .data.gate = {
> +                       .parent_name = "stdmac-clken",
> +                       .reg = 0x2000,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "ehci",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 18,
> +               .data.factor = {
> +                       .parent_name = "upll",
> +                       .mult = 1,
> +                       .div = 12,
> +               },
> +       },
> +       { /* sentinel */ }
> +};
> +
> +static void __init ph1_sld3_clk_init(struct device_node *np)
> +{
> +       uniphier_clk_init(np, ph1_sld3_clk_idata);
> +}
> +CLK_OF_DECLARE(ph1_sld3_clk, "socionext,ph1-sld3-sysctrl", ph1_sld3_clk_init);

Ditto.

> diff --git a/drivers/clk/uniphier/clk-ph1-sld8.c b/drivers/clk/uniphier/clk-ph1-sld8.c
> new file mode 100644
> index 0000000..bfebd38
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-ph1-sld8.c
> @@ -0,0 +1,107 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro@socionext.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "clk-uniphier.h"
> +
> +static struct uniphier_clk_init_data ph1_sld8_clk_idata[] __initdata = {
> +       {
> +               .name = "spll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 64,
> +                       .div = 1,
> +               },
> +       },
> +       {
> +               .name = "upll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 288,
> +                       .div = 25,
> +               },
> +       },
> +       {
> +               .name = "uart",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 3,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 20,
> +               },
> +       },
> +       {
> +               .name = "i2c",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 4,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 16,
> +               },
> +       },
> +       {
> +               .name = "arm-scu",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 7,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 32,
> +               },
> +       },
> +       {
> +               .name = "stdmac-clken",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = -1,
> +               .data.gate = {
> +                       .parent_name = NULL,
> +                       .reg = 0x2104,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "stdmac",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = 10,
> +               .data.gate = {
> +                       .parent_name = "stdmac-clken",
> +                       .reg = 0x2000,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "ehci",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 18,
> +               .data.factor = {
> +                       .parent_name = "upll",
> +                       .mult = 1,
> +                       .div = 12,
> +               },
> +       },
> +       { /* sentinel */ }
> +};
> +
> +static void __init ph1_sld8_clk_init(struct device_node *np)
> +{
> +       uniphier_clk_init(np, ph1_sld8_clk_idata);
> +}
> +CLK_OF_DECLARE(ph1_sld8_clk, "socionext,ph1-sld8-sysctrl", ph1_sld8_clk_init);

Ditto.

> diff --git a/drivers/clk/uniphier/clk-proxstream2.c b/drivers/clk/uniphier/clk-proxstream2.c
> new file mode 100644
> index 0000000..b3ffedc
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-proxstream2.c
> @@ -0,0 +1,88 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro@socionext.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "clk-uniphier.h"
> +
> +static struct uniphier_clk_init_data proxstream2_clk_idata[] __initdata = {
> +       {
> +               .name = "spll",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = -1,
> +               .data.factor = {
> +                       .parent_name = UNIPHIER_CLK_EXT "ref",
> +                       .mult = 96,
> +                       .div = 1,
> +               },
> +       },
> +       {
> +               .name = "uart",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 3,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 27,
> +               },
> +       },
> +       {
> +               .name = "fi2c",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 4,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 48,
> +               },
> +       },
> +       {
> +               .name = "arm-scu",
> +               .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +               .output_index = 7,
> +               .data.factor = {
> +                       .parent_name = "spll",
> +                       .mult = 1,
> +                       .div = 48,
> +               },
> +       },
> +       {
> +               .name = "stdmac-clken",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = -1,
> +               .data.gate = {
> +                       .parent_name = NULL,
> +                       .reg = 0x2104,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       {
> +               .name = "stdmac",
> +               .type = UNIPHIER_CLK_TYPE_GATE,
> +               .output_index = 10,
> +               .data.gate = {
> +                       .parent_name = "stdmac-clken",
> +                       .reg = 0x2000,
> +                       .bit_idx = 10,
> +               },
> +       },
> +       { /* sentinel */ }
> +};
> +
> +static void __init proxstream2_clk_init(struct device_node *np)
> +{
> +       uniphier_clk_init(np, proxstream2_clk_idata);
> +}
> +CLK_OF_DECLARE(proxstream2_clk, "socionext,proxstream2-sysctrl",
> +              proxstream2_clk_init);

Ditto.

> diff --git a/drivers/clk/uniphier/clk-uniphier-core.c b/drivers/clk/uniphier/clk-uniphier-core.c
> new file mode 100644
> index 0000000..8680101
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-uniphier-core.c
> @@ -0,0 +1,151 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro@socionext.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#define pr_fmt(fmt)            "uniphier-clk: " fmt
> +
> +#include <linux/clk-provider.h>
> +#include <linux/log2.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +
> +#include "clk-uniphier.h"
> +
> +static void __init uniphier_clk_update_parent_name(struct device_node *np,
> +                                                  const char **parent_name)
> +{
> +       const char *new_name;
> +       int index;
> +
> +       if (!parent_name || !*parent_name)
> +               return;
> +
> +       if (strncmp(*parent_name, UNIPHIER_CLK_EXT, strlen(UNIPHIER_CLK_EXT)))
> +               return;
> +
> +       index = of_property_match_string(np, "clock-names",
> +                               *parent_name + strlen(UNIPHIER_CLK_EXT));
> +       new_name = of_clk_get_parent_name(np, index);
> +       if (new_name)
> +               *parent_name = new_name;

Where can I find the DT binding description and DTS for this clock
controller? I'm confused why the parent name function is necessary and
I'd like to see the data.

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1300018

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-01-02 02:30 +0100
Message-ID<qMjb4-1Ay-3@gated-at.bofh.it>
In reply to#1299550
Hi Michael,


2015-12-31 10:35 GMT+09:00 Michael Turquette <mturquette@baylibre.com>:
> Hello Yamada-san,
>
> Quoting Masahiro Yamada (2015-12-28 02:20:58)
>> diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
>> new file mode 100644
>> index 0000000..7606f27
>> --- /dev/null
>> +++ b/drivers/clk/uniphier/Kconfig
>> @@ -0,0 +1,35 @@
>> +menuconfig CLK_UNIPHIER
>> +       bool "Clock drivers for UniPhier SoCs"
>> +       depends on ARCH_UNIPHIER
>
> Might want to make the above line:
>
>         depends on ARCH_UNIPHIER || COMPILE_TEST
>
>> +       depends on OF
>> +       default y
>> +       help
>> +         Supports clock drivers for UniPhier SoCs.
>
> Why menuconfig? Can we make this a non-visible config option selected by
> the UniPhier platform?

Yes, I can do that if you like.

Do you prefer to making it flat, like all clocks are shown in the
"Common Clock Framework" menu?

Or, is it better to categorize SoC clocks with "menu" for each SoC family?



>> +
>> +if CLK_UNIPHIER
>
> Please drop the if statement here and have the below options 'depends'
> on CLK_UNIPHIER.

Why is it bad?

I think surrounding all the options with "if CLK_UNIPHIER" ... "endif"
is easier than adding "depends on CLK_UNIPHIER" to each of them.



>> +
>> +config CLK_UNIPHIER_PH1_SLD3
>> +       bool "Clock driver for UniPhier PH1-sLD3 SoC"
>> +       default y
>
> Can you make these drivers into loadable modules? If so you might
> consider tristate.

For some, I can.
For some, I can't (because they must provide an input clock for the timer.)


>> +
>> +static void __init ph1_ld4_clk_init(struct device_node *np)
>> +{
>> +       uniphier_clk_init(np, ph1_ld4_clk_idata);
>> +}
>> +CLK_OF_DECLARE(ph1_ld4_clk, "socionext,ph1-ld4-sysctrl", ph1_ld4_clk_init);
>
> Can you avoid using CLK_OF_DECLARE and instead make this into a
> platform_driver? For examples please see drivers/clk/qcom*.c


For clk-uniphier-peri.c and clk-uniphier-mio.c, yes.
They provide clocks for peripheral devices like USB, MMC, etc., so I
can delay them.


For clk-ph1-*.c, I am afraid I can't.
They provide an input clock for the timer (ARM global timer).
I think they should be initialized very early.


Is platform_driver better than CLK_OF_DECLARE() where it is possible?

I just chose CLK_OF_DECLARE() for consistency
and it seems the majority.



>> +
>> +static void __init uniphier_clk_update_parent_name(struct device_node *np,
>> +                                                  const char **parent_name)
>> +{
>> +       const char *new_name;
>> +       int index;
>> +
>> +       if (!parent_name || !*parent_name)
>> +               return;
>> +
>> +       if (strncmp(*parent_name, UNIPHIER_CLK_EXT, strlen(UNIPHIER_CLK_EXT)))
>> +               return;
>> +
>> +       index = of_property_match_string(np, "clock-names",
>> +                               *parent_name + strlen(UNIPHIER_CLK_EXT));
>> +       new_name = of_clk_get_parent_name(np, index);
>> +       if (new_name)
>> +               *parent_name = new_name;
>
> Where can I find the DT binding description and DTS for this clock
> controller? I'm confused why the parent name function is necessary and
> I'd like to see the data.


clk_register() needs the name strings of parent clocks,
not pointers to parent clocks.
(It looks a bit odd to me, although it allows us to register clocks in
any order.
In stead, we have to think about orphan-walking.)
So, we need to exactly know the names of parent clocks.

Let's assume the clock system that consists of some hardware blocks.


  root_clk: root_clk {
        compatible = "foo-clk";
        reg = <0x10000000 0x1000>;
        #clock-cells = <1>;
  };

  bar_clk {
        compatible = "bar-clk";
        reg = <0x20000000 0x1000>;
        #clock-cells = <1>;
        clock-names = "input"
        clocks = <&root_clk 0>;
   };

  baz_clk {
        compatible = "baz-clk";
        reg = <0x30000000 0x1000>;
        #clock-cells = <1>;
        clock-names = "input"
        clocks = <&root_clk 1>;
   };



The "bar_clk" is a clock provider for other peripherals,
and also a consumer of "root_clk".

The "bar_clk" wants to know the name of index 0 of root_clk
because it is needed when calling clk_register().


Let's say the names of clocks provided root_clk are, "for-bar123", "for-baz456".

The "bar_clk" needs the string "for-bar123", but it does not know.
(Likewise, the "baz_clk" needs the string "for-baz456", but it does not know.)

Of course, I can hard-code "for-bar123" in the driver of "bar_clk",
but I think it is annoying to keep the name synced between the
provider and the consumer.

So, I implemented uniphier_clk_update_parent_name()
to convert the clock-names into the parent clock name.
In this case, this function takes "input" and converts it into "for-bar123".


Is there any good idea to solve this problem?


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web