Re: [PATCH v1 09/14] clk: msm: Add support for MSM8960's globalclock controller (GCC)

From: Mark Rutland
Date: Thu Aug 08 2013 - 13:00:52 EST


Hi Stephen,

On Thu, Jul 25, 2013 at 01:43:37AM +0100, Stephen Boyd wrote:
> Fill in the data and wire up the global clock controller to the
> MSM clock driver. This should allow most non-multimedia device
> drivers to control their clocks on 8960 based platforms.
>
> Cc: devicetree@xxxxxxxxxxxxxxx
> Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
> ---
> .../devicetree/bindings/clock/qcom,gcc.txt | 55 +++++++
> drivers/clk/msm/Kconfig | 10 ++
> drivers/clk/msm/Makefile | 2 +
> drivers/clk/msm/core.c | 3 +
> drivers/clk/msm/gcc-8960.c | 174 +++++++++++++++++++++
> drivers/clk/msm/internal.h | 2 +
> 6 files changed, 246 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc.txt
> create mode 100644 drivers/clk/msm/gcc-8960.c
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc.txt b/Documentation/devicetree/bindings/clock/qcom,gcc.txt
> new file mode 100644
> index 0000000..2311e1a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc.txt
> @@ -0,0 +1,55 @@
> +MSM Global Clock Controller Binding
> +-----------------------------------
> +
> +Required properties :
> +- compatible : shall contain at least "qcom,gcc" and only one of the
> + following:
> +
> + "qcom,gcc-8660"
> + "qcom,gcc-8960"
> +
> +- reg : shall contain base register location and length
> +- clocks : shall contain clocks supplied by the clock controller
> +
> +Example:
> + clock-controller@900000 {
> + compatible = "qcom,gcc-8960", "qcom,gcc";
> + reg = <0x900000 0x4000>;
> +
> + clocks {
> + pxo: pxo {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <27000000>;
> + };
> +
> + pll8: pll8 {
> + #clock-cells = <0>;
> + compatible = "qcom,pll";
> + clocks = <&pxo>;
> + };
> +
> + vpll8: vpll8 {
> + #clock-cells = <0>;
> + compatible = "qcom,pll-vote";
> + clocks = <&pll8>;
> + };
> +
> + gsbi5_uart_rcg: gsbi5_uart_rcg {
> + #clock-cells = <0>;
> + compatible = "qcom,p2-mn16-clock";
> + clocks = <&pxo>, <&vpll8>;
> + };
> +
> + gsbi5_uart_clk: gsbi5_uart_cxc {
> + #clock-cells = <0>;
> + compatible = "qcom,cxc-clock";
> + clocks = <&gsbi5_uart_rcg>;
> + };
> +
> + gsbi5_uart_ahb: gsbi5_uart_ahb {
> + #clock-cells = <0>;
> + compatible = "qcom,cxc-hg-clock";
> + };
> + };
> + };

I'm slightly confused by this. How is each of the clocks described in
the clocks node related to a portion of the register set?

If the set of clocks is fixed, surely the gcc node gives you enough
information alone, and the whole block can be modelled as a single
provider of multiple clock outputs, or it's not fixed, and some linkage
needs to be defined?

The code seems to imply the former, unless only a subset of clocks may
be present? In that case, the set of clocks which might be present
should be described in the binding.

Thanks,
Mark.

> diff --git a/drivers/clk/msm/Kconfig b/drivers/clk/msm/Kconfig
> index bf7e3d2..3eaffb6 100644
> --- a/drivers/clk/msm/Kconfig
> +++ b/drivers/clk/msm/Kconfig
> @@ -2,3 +2,13 @@ menuconfig COMMON_CLK_MSM
> tristate "Support for Qualcomm's MSM designs"
> depends on OF
>
> +if COMMON_CLK_MSM
> +
> +config MSM_GCC_8960
> + bool "MSM8960 Global Clock Controller"
> + help
> + Support for the global clock controller on msm8960 devices.
> + Say Y if you want to use peripheral devices such as UART, SPI,
> + i2c, USB, SD/eMMC, SATA, PCIe, etc.
> +
> +endif
> diff --git a/drivers/clk/msm/Makefile b/drivers/clk/msm/Makefile
> index 9cfd0d7..c785943 100644
> --- a/drivers/clk/msm/Makefile
> +++ b/drivers/clk/msm/Makefile
> @@ -6,3 +6,5 @@ clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-rcg2.o
> clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-branch.o
>
> clk-msm-$(CONFIG_COMMON_CLK_MSM) += core.o
> +
> +clk-msm-$(CONFIG_MSM_GCC_8960) += gcc-8960.o
> diff --git a/drivers/clk/msm/core.c b/drivers/clk/msm/core.c
> index b1904c0..b8e702b 100644
> --- a/drivers/clk/msm/core.c
> +++ b/drivers/clk/msm/core.c
> @@ -173,6 +173,9 @@ typedef struct clk *
> struct cc_data *cc);
>
> static const struct of_device_id msm_cc_match_table[] = {
> +#ifdef CONFIG_MSM_GCC_8960
> + { .compatible = "qcom,gcc-8960", .data = &msm_gcc_8960_matches },
> +#endif
> { }
> };
> MODULE_DEVICE_TABLE(of, msm_cc_match_table);
> diff --git a/drivers/clk/msm/gcc-8960.c b/drivers/clk/msm/gcc-8960.c
> new file mode 100644
> index 0000000..b57d2dd
> --- /dev/null
> +++ b/drivers/clk/msm/gcc-8960.c
> @@ -0,0 +1,174 @@
> +/*
> + * Copyright (c) 2013, The Linux Foundation. All rights reserved.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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 "internal.h"
> +#include "clk-pll.h"
> +#include "clk-rcg.h"
> +#include "clk-branch.h"
> +
> +static struct pll_desc pll8_desc = {
> + .l_reg = 0x3144,
> + .m_reg = 0x3148,
> + .n_reg = 0x314c,
> + .config_reg = 0x3154,
> + .mode_reg = 0x3140,
> + .status_reg = 0x3158,
> + .status_bit = 16,
> +};
> +
> +static struct pll_vote_desc pll8_vote_desc = {
> + .vote_reg = 0x34c0,
> + .vote_bit = 8,
> +};
> +
> +#define PXO 0
> +#define PLL8 1
> +
> +static u8 gcc_pxo_pll8_map[] = {
> + [PXO] = 0,
> + [PLL8] = 3,
> +};
> +
> +static struct freq_tbl clk_tbl_gsbi_uart[] = {
> + { 1843200, PLL8, 2, 6, 625 },
> + { 3686400, PLL8, 2, 12, 625 },
> + { 7372800, PLL8, 2, 24, 625 },
> + { 14745600, PLL8, 2, 48, 625 },
> + { 16000000, PLL8, 4, 1, 6 },
> + { 24000000, PLL8, 4, 1, 4 },
> + { 32000000, PLL8, 4, 1, 3 },
> + { 40000000, PLL8, 1, 5, 48 },
> + { 46400000, PLL8, 1, 29, 240 },
> + { 48000000, PLL8, 4, 1, 2 },
> + { 51200000, PLL8, 1, 2, 15 },
> + { 56000000, PLL8, 1, 7, 48 },
> + { 58982400, PLL8, 1, 96, 625 },
> + { 64000000, PLL8, 2, 1, 3 },
> + { }
> +};
> +
> +static struct rcg_desc gsbi5_uart_rcg = {
> + .ctl_reg = 0x2a54,
> + .ns_reg = 0x2a54,
> + .md_reg = 0x2a50,
> + .ctl_bit = 11,
> + .mnctr_en_bit = 8,
> + .mnctr_reset_bit = 7,
> + .mnctr_mode_shift = 5,
> + .pre_div_shift = 3,
> + .src_sel_shift = 0,
> + .n_val_shift = 16,
> + .m_val_shift = 16,
> + .parent_map = gcc_pxo_pll8_map,
> + .freq_tbl = clk_tbl_gsbi_uart,
> +};
> +
> +static struct branch_desc gsbi5_uart_cxc = {
> + .ctl_reg = 0x2a54,
> + .halt_reg = 0x2fd0,
> + .ctl_bit = 9,
> + .halt_bit = 22,
> + .halt_check = BRANCH_HALT,
> +};
> +
> +static struct rcg_desc gsbi6_uart_rcg = {
> + .ctl_reg = 0x2a74,
> + .ns_reg = 0x2a74,
> + .md_reg = 0x2a70,
> + .ctl_bit = 11,
> + .mnctr_en_bit = 8,
> + .mnctr_reset_bit = 7,
> + .mnctr_mode_shift = 5,
> + .pre_div_shift = 3,
> + .src_sel_shift = 0,
> + .n_val_shift = 16,
> + .m_val_shift = 16,
> + .parent_map = gcc_pxo_pll8_map,
> + .freq_tbl = clk_tbl_gsbi_uart,
> +};
> +
> +static struct branch_desc gsbi6_uart_cxc = {
> + .ctl_reg = 0x2a74,
> + .halt_reg = 0x2fd0,
> + .ctl_bit = 9,
> + .halt_bit = 18,
> + .halt_check = BRANCH_HALT,
> +};
> +
> +static struct branch_desc gsbi5_uart_ahb = {
> + .ctl_reg = 0x2a40,
> + .halt_reg = 0x2fd0,
> + .hwcg_reg = 0x2a40,
> + .ctl_bit = 4,
> + .hwcg_bit = 6,
> + .halt_bit = 23,
> + .halt_check = BRANCH_HALT,
> +};
> +
> +static struct freq_tbl clk_tbl_gsbi_qup[] = {
> + { 1100000, PXO, 1, 2, 49 },
> + { 5400000, PXO, 1, 1, 5 },
> + { 10800000, PXO, 1, 2, 5 },
> + { 15060000, PLL8, 1, 2, 51 },
> + { 24000000, PLL8, 4, 1, 4 },
> + { 25600000, PLL8, 1, 1, 15 },
> + { 27000000, PXO, 1, 0, 0 },
> + { 48000000, PLL8, 4, 1, 2 },
> + { 51200000, PLL8, 1, 2, 15 },
> + { }
> +};
> +
> +static struct rcg_desc gsbi5_qup_rcg = {
> + .ctl_reg = 0x2a4c,
> + .ns_reg = 0x2a4c,
> + .md_reg = 0x2a48,
> + .ctl_bit = 11,
> + .mnctr_en_bit = 8,
> + .mnctr_reset_bit = 7,
> + .mnctr_mode_shift = 5,
> + .pre_div_shift = 3,
> + .src_sel_shift = 0,
> + .n_val_shift = 16,
> + .m_val_shift = 16,
> + .parent_map = gcc_pxo_pll8_map,
> + .freq_tbl = clk_tbl_gsbi_qup,
> +};
> +
> +static struct branch_desc gsbi5_qup_cxc = {
> + .ctl_reg = 0x2a4c,
> + .halt_reg = 0x2fd0,
> + .ctl_bit = 9,
> + .halt_bit = 20,
> + .halt_check = BRANCH_HALT,
> +};
> +
> +static struct of_clk_match msm_gcc_clk_match[] = {
> + { .name = "cxo" },
> + { .name = "pxo" },
> + { .name = "pll8", .driver_data = &pll8_desc },
> + { .name = "vpll8", .driver_data = &pll8_vote_desc },
> + { .name = "gsbi5_uart_rcg", .driver_data = &gsbi5_uart_rcg },
> + { .name = "gsbi5_uart_cxc", .driver_data = &gsbi5_uart_cxc },
> + { .name = "gsbi6_uart_rcg", .driver_data = &gsbi6_uart_rcg },
> + { .name = "gsbi6_uart_cxc", .driver_data = &gsbi6_uart_cxc },
> + { .name = "gsbi5_uart_ahb", .driver_data = &gsbi5_uart_ahb },
> + { .name = "gsbi5_qup_rcg", .driver_data = &gsbi5_qup_rcg },
> + { .name = "gsbi5_qup_cxc", .driver_data = &gsbi5_qup_cxc },
> +};
> +
> +const struct msm_clk_match msm_gcc_8960_matches = {
> + .matches = msm_gcc_clk_match,
> + .size = ARRAY_SIZE(msm_gcc_clk_match)
> +};
> diff --git a/drivers/clk/msm/internal.h b/drivers/clk/msm/internal.h
> index 177bd3b..b0ffda7 100644
> --- a/drivers/clk/msm/internal.h
> +++ b/drivers/clk/msm/internal.h
> @@ -21,4 +21,6 @@ struct msm_clk_match {
> size_t size;
> };
>
> +extern const struct msm_clk_match msm_gcc_8960_matches;
> +
> #endif
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
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/