* [PATCH 0/2] Add APSS clock controller support for IPQ6018
@ 2020-02-27 9:55 Sivaprakash Murugesan
2020-02-27 9:55 ` [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
2020-02-27 9:55 ` [PATCH 2/2] clk: qcom: Add " Sivaprakash Murugesan
0 siblings, 2 replies; 15+ messages in thread
From: Sivaprakash Murugesan @ 2020-02-27 9:55 UTC (permalink / raw)
To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel
Cc: sivaprak
The APSS clock controller in ipq6018 based devices supports cpu with
frequencies above 800Mhz.
This patch series adds the support for the same.
Sivaprakash Murugesan (2):
clk: qcom: Add DT bindings for ipq6018 apss clock controller
clk: qcom: Add ipq6018 apss clock controller
.../devicetree/bindings/clock/qcom,apsscc.yaml | 58 ++++++
drivers/clk/qcom/Kconfig | 8 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/apss-ipq6018.c | 210 +++++++++++++++++++++
include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 +++
5 files changed, 303 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
create mode 100644 drivers/clk/qcom/apss-ipq6018.c
create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-02-27 9:55 [PATCH 0/2] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
@ 2020-02-27 9:55 ` Sivaprakash Murugesan
2020-02-27 10:38 ` Sibi Sankar
` (2 more replies)
2020-02-27 9:55 ` [PATCH 2/2] clk: qcom: Add " Sivaprakash Murugesan
1 sibling, 3 replies; 15+ messages in thread
From: Sivaprakash Murugesan @ 2020-02-27 9:55 UTC (permalink / raw)
To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel
Cc: sivaprak
add dt-binding for ipq6018 apss clock controller
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
.../devicetree/bindings/clock/qcom,apsscc.yaml | 58 ++++++++++++++++++++++
include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 ++++++++++
2 files changed, 84 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
diff --git a/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
new file mode 100644
index 0000000..7433721
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/qcom,apsscc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IPQ6018 APSS Clock Controller Binding
+
+maintainers:
+ - Stephen Boyd <sboyd@kernel.org>
+
+description: |
+ Qualcomm IPQ6018 APSS clock control module which supports the clocks with
+ frequencies above 800Mhz.
+
+properties:
+ compatible :
+ const: qcom,apss-ipq6018
+
+ clocks:
+ description: clocks required for this controller.
+ maxItems: 4
+
+ clock-names:
+ description: clock output names of required clocks.
+ maxItems: 4
+
+ '#clock-cells':
+ const: 1
+
+ '#reset-cells':
+ const: 1
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+ - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
+ apss_clk: qcom,apss_clk@b111000 {
+ compatible = "qcom,apss-ipq6018";
+ clocks = <&xo>, <&gcc GPLL0>,
+ <&gcc GPLL2>, <&gcc GPLL4>;
+ clock-names = "xo", "gpll0",
+ "gpll2", "gpll4";
+ reg = <0xb11100c 0x5ff4>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
+...
diff --git a/include/dt-bindings/clock/qcom,apss-ipq6018.h b/include/dt-bindings/clock/qcom,apss-ipq6018.h
new file mode 100644
index 0000000..ed9d7d8
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,apss-ipq6018.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
+#define _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
+
+#define APSS_PLL_EARLY 0
+#define APSS_PLL 1
+#define APCS_ALIAS0_CLK_SRC 2
+#define APCS_ALIAS0_CORE_CLK 3
+
+#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] clk: qcom: Add ipq6018 apss clock controller
2020-02-27 9:55 [PATCH 0/2] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
2020-02-27 9:55 ` [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
@ 2020-02-27 9:55 ` Sivaprakash Murugesan
2020-02-27 17:40 ` kbuild test robot
` (2 more replies)
1 sibling, 3 replies; 15+ messages in thread
From: Sivaprakash Murugesan @ 2020-02-27 9:55 UTC (permalink / raw)
To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel
Cc: sivaprak
This patch adds the support for apss clock controller found on ipq6018
devices.
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
drivers/clk/qcom/Kconfig | 8 ++
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/apss-ipq6018.c | 210 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 219 insertions(+)
create mode 100644 drivers/clk/qcom/apss-ipq6018.c
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 15cdcdc..37e4ce2 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -89,6 +89,14 @@ config APQ_MMCC_8084
Say Y if you want to support multimedia devices such as display,
graphics, video encode/decode, camera, etc.
+config IPQ_APSS_6018
+ tristate "IPQ6018 APSS Clock Controller"
+ select IPQ_GCC_6018
+ help
+ Support for APSS clock controller on ipq6018 devices. The
+ APSS clock controller supports frequencies higher than 800Mhz.
+ Say Y if you want to support higher frequencies on ipq6018 devices.
+
config IPQ_GCC_4019
tristate "IPQ4019 Global Clock Controller"
help
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 656a87e..2b61715 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -19,6 +19,7 @@ clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
# Keep alphabetically sorted by config
obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
obj-$(CONFIG_APQ_MMCC_8084) += mmcc-apq8084.o
+obj-$(CONFIG_IPQ_APSS_6018) += apss-ipq6018.o
obj-$(CONFIG_IPQ_GCC_4019) += gcc-ipq4019.o
obj-$(CONFIG_IPQ_GCC_6018) += gcc-ipq6018.o
obj-$(CONFIG_IPQ_GCC_806X) += gcc-ipq806x.o
diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
new file mode 100644
index 0000000..04b8962
--- /dev/null
+++ b/drivers/clk/qcom/apss-ipq6018.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+
+#include <linux/reset-controller.h>
+#include <dt-bindings/clock/qcom,apss-ipq6018.h>
+
+#include "common.h"
+#include "clk-regmap.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+#include "clk-alpha-pll.h"
+#include "clk-regmap-divider.h"
+#include "clk-regmap-mux.h"
+#include "reset.h"
+
+#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
+
+enum {
+ P_XO,
+ P_GPLL0,
+ P_GPLL2,
+ P_GPLL4,
+ P_APSS_PLL_EARLY,
+ P_APSS_PLL
+};
+
+const u8 apss_pll_offsets[] = {
+ [PLL_OFF_L_VAL] = 0x08,
+ [PLL_OFF_ALPHA_VAL] = 0x10,
+ [PLL_OFF_USER_CTL] = 0x18,
+ [PLL_OFF_CONFIG_CTL] = 0x20,
+ [PLL_OFF_CONFIG_CTL_U] = 0x24,
+ [PLL_OFF_STATUS] = 0x28,
+ [PLL_OFF_TEST_CTL] = 0x30,
+ [PLL_OFF_TEST_CTL_U] = 0x34,
+};
+
+static struct clk_alpha_pll apss_pll_early = {
+ .offset = 0x4ff4,
+ .regs = apss_pll_offsets,
+ .flags = SUPPORTS_DYNAMIC_UPDATE,
+ .clkr = {
+ .enable_reg = 0x4ff4,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "apss_pll_early",
+ .parent_data = &(const struct clk_parent_data){
+ .fw_name = "xo",
+ },
+ .num_parents = 1,
+ .ops = &clk_alpha_pll_huayra_ops,
+ },
+ },
+};
+
+static struct clk_alpha_pll_postdiv apss_pll = {
+ .offset = 0x4ff4,
+ .regs = apss_pll_offsets,
+ .width = 2,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "apss_pll",
+ .parent_hws = (const struct clk_hw *[]){
+ &apss_pll_early.clkr.hw },
+ .num_parents = 1,
+ .ops = &clk_alpha_pll_postdiv_ro_ops,
+ },
+};
+
+static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
+ { .fw_name = "xo" },
+ { .fw_name = "gpll0"},
+ { .fw_name = "gpll2"},
+ { .fw_name = "gpll4"},
+ { .hw = &apss_pll.clkr.hw },
+ { .hw = &apss_pll_early.clkr.hw },
+};
+
+static const struct parent_map parents_apcs_alias0_clk_src_map[] = {
+ { P_XO, 0 },
+ { P_GPLL0, 4 },
+ { P_GPLL2, 2 },
+ { P_GPLL4, 1 },
+ { P_APSS_PLL, 3 },
+ { P_APSS_PLL_EARLY, 5 },
+};
+
+static const struct freq_tbl ftbl_apcs_alias0_clk_src[] = {
+ F(24000000, P_XO, 1, 0, 0),
+ F(864000000, P_APSS_PLL_EARLY, 1, 0, 0),
+ F(1056000000, P_APSS_PLL_EARLY, 1, 0, 0),
+ F(1200000000, P_APSS_PLL_EARLY, 1, 0, 0),
+ F(1320000000, P_APSS_PLL_EARLY, 1, 0, 0),
+ F(1440000000, P_APSS_PLL_EARLY, 1, 0, 0),
+ F(1608000000, P_APSS_PLL_EARLY, 1, 0, 0),
+ F(1800000000, P_APSS_PLL_EARLY, 1, 0, 0),
+ { }
+};
+
+static struct clk_rcg2 apcs_alias0_clk_src = {
+ .cmd_rcgr = 0x0044,
+ .freq_tbl = ftbl_apcs_alias0_clk_src,
+ .hid_width = 5,
+ .parent_map = parents_apcs_alias0_clk_src_map,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "apcs_alias0_clk_src",
+ .parent_data = parents_apcs_alias0_clk_src,
+ .num_parents = 6,
+ .ops = &clk_rcg2_ops,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_branch apcs_alias0_core_clk = {
+ .halt_reg = 0x004c,
+ .clkr = {
+ .enable_reg = 0x004c,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "apcs_alias0_core_clk",
+ .parent_hws = (const struct clk_hw *[]){
+ &apcs_alias0_clk_src.clkr.hw },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_regmap *apss_ipq6018_clks[] = {
+ [APSS_PLL_EARLY] = &apss_pll_early.clkr,
+ [APSS_PLL] = &apss_pll.clkr,
+ [APCS_ALIAS0_CLK_SRC] = &apcs_alias0_clk_src.clkr,
+ [APCS_ALIAS0_CORE_CLK] = &apcs_alias0_core_clk.clkr,
+};
+
+static const struct alpha_pll_config apss_pll_config = {
+ .l = 0x37,
+ .config_ctl_val = 0x04141200,
+ .config_ctl_hi_val = 0x0,
+ .early_output_mask = BIT(3),
+ .main_output_mask = BIT(0),
+};
+
+static const struct of_device_id apss_ipq6018_match_table[] = {
+ { .compatible = "qcom,apss-ipq6018" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, apss_ipq6018_match_table);
+
+static const struct regmap_config apss_ipq6018_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x5ffc,
+ .fast_io = true,
+};
+
+static const struct qcom_cc_desc apss_ipq6018_desc = {
+ .config = &apss_ipq6018_regmap_config,
+ .clks = apss_ipq6018_clks,
+ .num_clks = ARRAY_SIZE(apss_ipq6018_clks),
+};
+
+static int apss_ipq6018_probe(struct platform_device *pdev)
+{
+ struct regmap *regmap;
+
+ regmap = qcom_cc_map(pdev, &apss_ipq6018_desc);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ clk_alpha_pll_configure(&apss_pll_early, regmap, &apss_pll_config);
+
+ return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
+}
+
+static struct platform_driver apss_ipq6018_driver = {
+ .probe = apss_ipq6018_probe,
+ .driver = {
+ .name = "qcom,apss-ipq6018",
+ .of_match_table = apss_ipq6018_match_table,
+ },
+};
+
+static int __init apss_ipq6018_init(void)
+{
+ return platform_driver_register(&apss_ipq6018_driver);
+}
+core_initcall(apss_ipq6018_init);
+
+static void __exit apss_ipq6018_exit(void)
+{
+ platform_driver_unregister(&apss_ipq6018_driver);
+}
+module_exit(apss_ipq6018_exit);
+
+MODULE_DESCRIPTION("QCA APSS IPQ6018 Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-02-27 9:55 ` [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
@ 2020-02-27 10:38 ` Sibi Sankar
2020-02-27 10:55 ` Sivaprakash Murugesan
2020-04-05 8:19 ` Sivaprakash Murugesan
2020-02-27 17:14 ` Rob Herring
2020-03-09 21:13 ` Rob Herring
2 siblings, 2 replies; 15+ messages in thread
From: Sibi Sankar @ 2020-02-27 10:38 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-msm-owner
Hey Sivaprakash,
On 2020-02-27 15:25, Sivaprakash Murugesan wrote:
> add dt-binding for ipq6018 apss clock controller
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> .../devicetree/bindings/clock/qcom,apsscc.yaml | 58
> ++++++++++++++++++++++
> include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 ++++++++++
> 2 files changed, 84 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
> create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
> b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
> new file mode 100644
> index 0000000..7433721
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0-only
Dual license
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/clock/qcom,apsscc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm IPQ6018 APSS Clock Controller Binding
> +
> +maintainers:
> + - Stephen Boyd <sboyd@kernel.org>
> +
> +description: |
> + Qualcomm IPQ6018 APSS clock control module which supports the clocks
> with
> + frequencies above 800Mhz.
> +
> +properties:
> + compatible :
> + const: qcom,apss-ipq6018
Please use qcom,<chip>-<device>
instead.
> +
> + clocks:
> + description: clocks required for this controller.
> + maxItems: 4
> +
> + clock-names:
> + description: clock output names of required clocks.
> + maxItems: 4
> +
> + '#clock-cells':
> + const: 1
> +
> + '#reset-cells':
> + const: 1
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - '#clock-cells'
> + - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
> + apss_clk: qcom,apss_clk@b111000 {
> + compatible = "qcom,apss-ipq6018";
> + clocks = <&xo>, <&gcc GPLL0>,
> + <&gcc GPLL2>, <&gcc GPLL4>;
> + clock-names = "xo", "gpll0",
> + "gpll2", "gpll4";
> + reg = <0xb11100c 0x5ff4>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> +...
> diff --git a/include/dt-bindings/clock/qcom,apss-ipq6018.h
> b/include/dt-bindings/clock/qcom,apss-ipq6018.h
> new file mode 100644
> index 0000000..ed9d7d8
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,apss-ipq6018.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> + *
> + * Permission to use, copy, modify, and/or distribute this software
> for any
> + * purpose with or without fee is hereby granted, provided that the
> above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE
> FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
> DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING
> OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
^^ is not needed just the SPDX
license identifier is enough.
> + */
> +
> +#ifndef _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
> +#define _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
> +
> +#define APSS_PLL_EARLY 0
> +#define APSS_PLL 1
> +#define APCS_ALIAS0_CLK_SRC 2
> +#define APCS_ALIAS0_CORE_CLK 3
> +
> +#endif
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-02-27 10:38 ` Sibi Sankar
@ 2020-02-27 10:55 ` Sivaprakash Murugesan
2020-04-05 8:19 ` Sivaprakash Murugesan
1 sibling, 0 replies; 15+ messages in thread
From: Sivaprakash Murugesan @ 2020-02-27 10:55 UTC (permalink / raw)
To: Sibi Sankar
Cc: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-msm-owner
Hi Sibi,
Thanks for the review.
On 2/27/2020 4:08 PM, Sibi Sankar wrote:
> Hey Sivaprakash,
>
> On 2020-02-27 15:25, Sivaprakash Murugesan wrote:
>> add dt-binding for ipq6018 apss clock controller
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>> .../devicetree/bindings/clock/qcom,apsscc.yaml | 58
>> ++++++++++++++++++++++
>> include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 ++++++++++
>> 2 files changed, 84 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> new file mode 100644
>> index 0000000..7433721
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>
> Dual license
missed it. will add in next series.
>
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/bindings/clock/qcom,apsscc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm IPQ6018 APSS Clock Controller Binding
>> +
>> +maintainers:
>> + - Stephen Boyd <sboyd@kernel.org>
>> +
>> +description: |
>> + Qualcomm IPQ6018 APSS clock control module which supports the
>> clocks with
>> + frequencies above 800Mhz.
>> +
>> +properties:
>> + compatible :
>> + const: qcom,apss-ipq6018
>
> Please use qcom,<chip>-<device>
> instead.
ok.
>
>> +
>> + clocks:
>> + description: clocks required for this controller.
>> + maxItems: 4
>> +
>> + clock-names:
>> + description: clock output names of required clocks.
>> + maxItems: 4
>> +
>> + '#clock-cells':
>> + const: 1
>> +
>> + '#reset-cells':
>> + const: 1
>> +
>> + reg:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - '#clock-cells'
>> + - '#reset-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
>> + apss_clk: qcom,apss_clk@b111000 {
>> + compatible = "qcom,apss-ipq6018";
>> + clocks = <&xo>, <&gcc GPLL0>,
>> + <&gcc GPLL2>, <&gcc GPLL4>;
>> + clock-names = "xo", "gpll0",
>> + "gpll2", "gpll4";
>> + reg = <0xb11100c 0x5ff4>;
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + };
>> +...
>> diff --git a/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> b/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> new file mode 100644
>> index 0000000..ed9d7d8
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software
>> for any
>> + * purpose with or without fee is hereby granted, provided that the
>> above
>> + * copyright notice and this permission notice appear in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
>> WARRANTIES
>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
>> LIABLE FOR
>> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
>> DAMAGES
>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER
>> IN AN
>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING
>> OUT OF
>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>
> ^^ is not needed just the SPDX
> license identifier is enough.
>
ok.
Thanks,
Siva
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-02-27 9:55 ` [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
2020-02-27 10:38 ` Sibi Sankar
@ 2020-02-27 17:14 ` Rob Herring
2020-03-04 5:24 ` Sivaprakash Murugesan
2020-03-09 21:13 ` Rob Herring
2 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2020-02-27 17:14 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel, sivaprak
On Thu, 27 Feb 2020 15:25:17 +0530, Sivaprakash Murugesan wrote:
> add dt-binding for ipq6018 apss clock controller
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> .../devicetree/bindings/clock/qcom,apsscc.yaml | 58 ++++++++++++++++++++++
> include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 ++++++++++
> 2 files changed, 84 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
> create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
Documentation/devicetree/bindings/clock/qcom,apsscc.example.dts:17:10: fatal error: dt-bindings/clock/qcom,gcc-ipq6018.h: No such file or directory
#include <dt-bindings/clock/qcom,gcc-ipq6018.h>
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
scripts/Makefile.lib:300: recipe for target 'Documentation/devicetree/bindings/clock/qcom,apsscc.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/clock/qcom,apsscc.example.dt.yaml] Error 1
Makefile:1263: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1245691
Please check and re-submit.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] clk: qcom: Add ipq6018 apss clock controller
2020-02-27 9:55 ` [PATCH 2/2] clk: qcom: Add " Sivaprakash Murugesan
@ 2020-02-27 17:40 ` kbuild test robot
2020-02-27 17:40 ` [RFC PATCH] clk: qcom: apss_pll_offsets[] can be static kbuild test robot
2020-03-09 20:24 ` [PATCH 2/2] clk: qcom: Add ipq6018 apss clock controller Stephen Boyd
2 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2020-02-27 17:40 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: kbuild-all, agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel
Hi Sivaprakash,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on v5.6-rc3 next-20200227]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Sivaprakash-Murugesan/Add-APSS-clock-controller-support-for-IPQ6018/20200227-185847
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-173-ge0787745-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/clk/qcom/apss-ipq6018.c:39:10: sparse: sparse: symbol 'apss_pll_offsets' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH] clk: qcom: apss_pll_offsets[] can be static
2020-02-27 9:55 ` [PATCH 2/2] clk: qcom: Add " Sivaprakash Murugesan
2020-02-27 17:40 ` kbuild test robot
@ 2020-02-27 17:40 ` kbuild test robot
2020-03-09 20:24 ` [PATCH 2/2] clk: qcom: Add ipq6018 apss clock controller Stephen Boyd
2 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2020-02-27 17:40 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: kbuild-all, agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel
Fixes: bfce07143090 ("clk: qcom: Add ipq6018 apss clock controller")
Signed-off-by: kbuild test robot <lkp@intel.com>
---
apss-ipq6018.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
index 04b8962dbc8e0..663f0949ab109 100644
--- a/drivers/clk/qcom/apss-ipq6018.c
+++ b/drivers/clk/qcom/apss-ipq6018.c
@@ -36,7 +36,7 @@ enum {
P_APSS_PLL
};
-const u8 apss_pll_offsets[] = {
+static const u8 apss_pll_offsets[] = {
[PLL_OFF_L_VAL] = 0x08,
[PLL_OFF_ALPHA_VAL] = 0x10,
[PLL_OFF_USER_CTL] = 0x18,
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-02-27 17:14 ` Rob Herring
@ 2020-03-04 5:24 ` Sivaprakash Murugesan
2020-03-09 21:07 ` Rob Herring
0 siblings, 1 reply; 15+ messages in thread
From: Sivaprakash Murugesan @ 2020-03-04 5:24 UTC (permalink / raw)
To: Rob Herring
Cc: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel
Hi Rob,
I ran make dt_binding_check and dtbs_check both on mainline(5.6-rc4) and
linux-next both are successful.
The file qcom,gcc-ipq6018.h is merged in 5.6, not sure what is going wrong.
Could you please help?
Thanks,
Siva
On 2/27/2020 10:44 PM, Rob Herring wrote:
> On Thu, 27 Feb 2020 15:25:17 +0530, Sivaprakash Murugesan wrote:
>> add dt-binding for ipq6018 apss clock controller
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>> .../devicetree/bindings/clock/qcom,apsscc.yaml | 58 ++++++++++++++++++++++
>> include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 ++++++++++
>> 2 files changed, 84 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
>>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
> Documentation/devicetree/bindings/clock/qcom,apsscc.example.dts:17:10: fatal error: dt-bindings/clock/qcom,gcc-ipq6018.h: No such file or directory
> #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> scripts/Makefile.lib:300: recipe for target 'Documentation/devicetree/bindings/clock/qcom,apsscc.example.dt.yaml' failed
> make[1]: *** [Documentation/devicetree/bindings/clock/qcom,apsscc.example.dt.yaml] Error 1
> Makefile:1263: recipe for target 'dt_binding_check' failed
> make: *** [dt_binding_check] Error 2
>
> See https://patchwork.ozlabs.org/patch/1245691
> Please check and re-submit.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] clk: qcom: Add ipq6018 apss clock controller
2020-02-27 9:55 ` [PATCH 2/2] clk: qcom: Add " Sivaprakash Murugesan
2020-02-27 17:40 ` kbuild test robot
2020-02-27 17:40 ` [RFC PATCH] clk: qcom: apss_pll_offsets[] can be static kbuild test robot
@ 2020-03-09 20:24 ` Stephen Boyd
2020-04-05 8:26 ` Sivaprakash Murugesan
2 siblings, 1 reply; 15+ messages in thread
From: Stephen Boyd @ 2020-03-09 20:24 UTC (permalink / raw)
To: Sivaprakash Murugesan, agross, bjorn.andersson, devicetree,
linux-arm-msm, linux-clk, linux-kernel, mturquette, robh+dt
Cc: sivaprak
Quoting Sivaprakash Murugesan (2020-02-27 01:55:18)
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 15cdcdc..37e4ce2 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -89,6 +89,14 @@ config APQ_MMCC_8084
> Say Y if you want to support multimedia devices such as display,
> graphics, video encode/decode, camera, etc.
>
> +config IPQ_APSS_6018
> + tristate "IPQ6018 APSS Clock Controller"
> + select IPQ_GCC_6018
> + help
> + Support for APSS clock controller on ipq6018 devices. The
> + APSS clock controller supports frequencies higher than 800Mhz.
supports CPU frequencies? It's not clear what APSS is to a lot of people
out there.
> + Say Y if you want to support higher frequencies on ipq6018 devices.
support CPU frequency scaling on ipq6018?
> +
> config IPQ_GCC_4019
> tristate "IPQ4019 Global Clock Controller"
> help
> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> new file mode 100644
> index 0000000..04b8962
> --- /dev/null
> +++ b/drivers/clk/qcom/apss-ipq6018.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
Are these two includes needed at all?
> +#include <linux/clk-provider.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/reset-controller.h>
> +#include <dt-bindings/clock/qcom,apss-ipq6018.h>
> +
> +#include "common.h"
> +#include "clk-regmap.h"
> +#include "clk-pll.h"
> +#include "clk-rcg.h"
> +#include "clk-branch.h"
> +#include "clk-alpha-pll.h"
> +#include "clk-regmap-divider.h"
> +#include "clk-regmap-mux.h"
> +#include "reset.h"
> +
> +#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
This can be removed. It's common in clk-rcg.h now
> +
> +static struct clk_branch apcs_alias0_core_clk = {
> + .halt_reg = 0x004c,
> + .clkr = {
> + .enable_reg = 0x004c,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "apcs_alias0_core_clk",
> + .parent_hws = (const struct clk_hw *[]){
> + &apcs_alias0_clk_src.clkr.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
Please add a comment about why CLK_IS_CRITICAL is here. Presumably in
the case that a cpufreq driver doesn't probe and claim this clk?
> + .ops = &clk_branch2_ops,
> + },
> + },
> +};
> +
> +
[...]
> +
> +static int __init apss_ipq6018_init(void)
> +{
> + return platform_driver_register(&apss_ipq6018_driver);
> +}
> +core_initcall(apss_ipq6018_init);
> +
> +static void __exit apss_ipq6018_exit(void)
> +{
> + platform_driver_unregister(&apss_ipq6018_driver);
> +}
> +module_exit(apss_ipq6018_exit);
Any reason this can't just be module_platform_driver()?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-03-04 5:24 ` Sivaprakash Murugesan
@ 2020-03-09 21:07 ` Rob Herring
0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2020-03-09 21:07 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: agross, bjorn.andersson, mturquette, sboyd, linux-arm-msm,
linux-clk, devicetree, linux-kernel
On Wed, Mar 04, 2020 at 10:54:56AM +0530, Sivaprakash Murugesan wrote:
> Hi Rob,
>
> I ran make dt_binding_check and dtbs_check both on mainline(5.6-rc4) and
> linux-next both are successful.
>
> The file qcom,gcc-ipq6018.h is merged in 5.6, not sure what is going wrong.
>
> Could you please help?
Sorry, my fault there. The checks have to be warning free and that
didn't happen til 5.6-rc5, so I was still based on 5.5.
Rob
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-02-27 9:55 ` [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
2020-02-27 10:38 ` Sibi Sankar
2020-02-27 17:14 ` Rob Herring
@ 2020-03-09 21:13 ` Rob Herring
2020-04-05 8:30 ` Sivaprakash Murugesan
2 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2020-03-09 21:13 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: agross, bjorn.andersson, mturquette, sboyd, linux-arm-msm,
linux-clk, devicetree, linux-kernel
On Thu, Feb 27, 2020 at 03:25:17PM +0530, Sivaprakash Murugesan wrote:
> add dt-binding for ipq6018 apss clock controller
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> .../devicetree/bindings/clock/qcom,apsscc.yaml | 58 ++++++++++++++++++++++
> include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 ++++++++++
> 2 files changed, 84 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
> create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
> new file mode 100644
> index 0000000..7433721
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0-only
Dual license new bindings please:
(GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/clock/qcom,apsscc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm IPQ6018 APSS Clock Controller Binding
> +
> +maintainers:
> + - Stephen Boyd <sboyd@kernel.org>
I'd expect this to be a QCom person, not who is applying patches.
> +
> +description: |
You can drop '|'.
> + Qualcomm IPQ6018 APSS clock control module which supports the clocks with
> + frequencies above 800Mhz.
> +
> +properties:
> + compatible :
> + const: qcom,apss-ipq6018
Normal ordering is: qcom,ipq6018-apss
> +
> + clocks:
> + description: clocks required for this controller.
> + maxItems: 4
Need to define what each clock is.
> +
> + clock-names:
> + description: clock output names of required clocks.
> + maxItems: 4
> +
> + '#clock-cells':
> + const: 1
> +
> + '#reset-cells':
> + const: 1
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - '#clock-cells'
> + - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
> + apss_clk: qcom,apss_clk@b111000 {
I thought I'd finally seen the last of these Qcom node names...
clock-controller@...
> + compatible = "qcom,apss-ipq6018";
> + clocks = <&xo>, <&gcc GPLL0>,
> + <&gcc GPLL2>, <&gcc GPLL4>;
> + clock-names = "xo", "gpll0",
> + "gpll2", "gpll4";
> + reg = <0xb11100c 0x5ff4>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> +...
> diff --git a/include/dt-bindings/clock/qcom,apss-ipq6018.h b/include/dt-bindings/clock/qcom,apss-ipq6018.h
> new file mode 100644
> index 0000000..ed9d7d8
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,apss-ipq6018.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
I'm pretty sure your employer would like an additional license here.
> +/*
> + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#ifndef _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
> +#define _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
> +
> +#define APSS_PLL_EARLY 0
> +#define APSS_PLL 1
> +#define APCS_ALIAS0_CLK_SRC 2
> +#define APCS_ALIAS0_CORE_CLK 3
> +
> +#endif
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-02-27 10:38 ` Sibi Sankar
2020-02-27 10:55 ` Sivaprakash Murugesan
@ 2020-04-05 8:19 ` Sivaprakash Murugesan
1 sibling, 0 replies; 15+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-05 8:19 UTC (permalink / raw)
To: Sibi Sankar
Cc: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-msm-owner
Hi Sibi,
Thanks for the review.
On 2/27/2020 4:08 PM, Sibi Sankar wrote:
> Hey Sivaprakash,
>
> On 2020-02-27 15:25, Sivaprakash Murugesan wrote:
>> add dt-binding for ipq6018 apss clock controller
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>> .../devicetree/bindings/clock/qcom,apsscc.yaml | 58
>> ++++++++++++++++++++++
>> include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 ++++++++++
>> 2 files changed, 84 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> new file mode 100644
>> index 0000000..7433721
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>
> Dual license
ok.
>
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/bindings/clock/qcom,apsscc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm IPQ6018 APSS Clock Controller Binding
>> +
>> +maintainers:
>> + - Stephen Boyd <sboyd@kernel.org>
>> +
>> +description: |
>> + Qualcomm IPQ6018 APSS clock control module which supports the
>> clocks with
>> + frequencies above 800Mhz.
>> +
>> +properties:
>> + compatible :
>> + const: qcom,apss-ipq6018
>
> Please use qcom,<chip>-<device>
> instead.
>
ok.
>> +
>> + clocks:
>> + description: clocks required for this controller.
>> + maxItems: 4
>> +
>> + clock-names:
>> + description: clock output names of required clocks.
>> + maxItems: 4
>> +
>> + '#clock-cells':
>> + const: 1
>> +
>> + '#reset-cells':
>> + const: 1
>> +
>> + reg:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - '#clock-cells'
>> + - '#reset-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
>> + apss_clk: qcom,apss_clk@b111000 {
>> + compatible = "qcom,apss-ipq6018";
>> + clocks = <&xo>, <&gcc GPLL0>,
>> + <&gcc GPLL2>, <&gcc GPLL4>;
>> + clock-names = "xo", "gpll0",
>> + "gpll2", "gpll4";
>> + reg = <0xb11100c 0x5ff4>;
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + };
>> +...
>> diff --git a/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> b/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> new file mode 100644
>> index 0000000..ed9d7d8
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software
>> for any
>> + * purpose with or without fee is hereby granted, provided that the
>> above
>> + * copyright notice and this permission notice appear in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
>> WARRANTIES
>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
>> LIABLE FOR
>> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
>> DAMAGES
>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER
>> IN AN
>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING
>> OUT OF
>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>
> ^^ is not needed just the SPDX
> license identifier is enough.
ok.
>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
>> +#define _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
>> +
>> +#define APSS_PLL_EARLY 0
>> +#define APSS_PLL 1
>> +#define APCS_ALIAS0_CLK_SRC 2
>> +#define APCS_ALIAS0_CORE_CLK 3
>> +
>> +#endif
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] clk: qcom: Add ipq6018 apss clock controller
2020-03-09 20:24 ` [PATCH 2/2] clk: qcom: Add ipq6018 apss clock controller Stephen Boyd
@ 2020-04-05 8:26 ` Sivaprakash Murugesan
0 siblings, 0 replies; 15+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-05 8:26 UTC (permalink / raw)
To: Stephen Boyd, agross, bjorn.andersson, devicetree, linux-arm-msm,
linux-clk, linux-kernel, mturquette, robh+dt
Hi Stephen,
Thanks for the review.
On 3/10/2020 1:54 AM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-02-27 01:55:18)
>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
>> index 15cdcdc..37e4ce2 100644
>> --- a/drivers/clk/qcom/Kconfig
>> +++ b/drivers/clk/qcom/Kconfig
>> @@ -89,6 +89,14 @@ config APQ_MMCC_8084
>> Say Y if you want to support multimedia devices such as display,
>> graphics, video encode/decode, camera, etc.
>>
>> +config IPQ_APSS_6018
>> + tristate "IPQ6018 APSS Clock Controller"
>> + select IPQ_GCC_6018
>> + help
>> + Support for APSS clock controller on ipq6018 devices. The
>> + APSS clock controller supports frequencies higher than 800Mhz.
> supports CPU frequencies? It's not clear what APSS is to a lot of people
> out there.
ok. Will try to elaborate.
>
>> + Say Y if you want to support higher frequencies on ipq6018 devices.
> support CPU frequency scaling on ipq6018?
ok.
>
>> +
>> config IPQ_GCC_4019
>> tristate "IPQ4019 Global Clock Controller"
>> help
>> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
>> new file mode 100644
>> index 0000000..04b8962
>> --- /dev/null
>> +++ b/drivers/clk/qcom/apss-ipq6018.c
>> @@ -0,0 +1,210 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/err.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
> Are these two includes needed at all?
No they're not. will remove it.
>> +#include <linux/clk-provider.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <linux/reset-controller.h>
>> +#include <dt-bindings/clock/qcom,apss-ipq6018.h>
>> +
>> +#include "common.h"
>> +#include "clk-regmap.h"
>> +#include "clk-pll.h"
>> +#include "clk-rcg.h"
>> +#include "clk-branch.h"
>> +#include "clk-alpha-pll.h"
>> +#include "clk-regmap-divider.h"
>> +#include "clk-regmap-mux.h"
>> +#include "reset.h"
>> +
>> +#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
> This can be removed. It's common in clk-rcg.h now
ok.
>
>> +
>> +static struct clk_branch apcs_alias0_core_clk = {
>> + .halt_reg = 0x004c,
>> + .clkr = {
>> + .enable_reg = 0x004c,
>> + .enable_mask = BIT(0),
>> + .hw.init = &(struct clk_init_data){
>> + .name = "apcs_alias0_core_clk",
>> + .parent_hws = (const struct clk_hw *[]){
>> + &apcs_alias0_clk_src.clkr.hw },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> Please add a comment about why CLK_IS_CRITICAL is here. Presumably in
> the case that a cpufreq driver doesn't probe and claim this clk?
ok.
>> + .ops = &clk_branch2_ops,
>> + },
>> + },
>> +};
>> +
>> +
> [...]
>> +
>> +static int __init apss_ipq6018_init(void)
>> +{
>> + return platform_driver_register(&apss_ipq6018_driver);
>> +}
>> +core_initcall(apss_ipq6018_init);
>> +
>> +static void __exit apss_ipq6018_exit(void)
>> +{
>> + platform_driver_unregister(&apss_ipq6018_driver);
>> +}
>> +module_exit(apss_ipq6018_exit);
> Any reason this can't just be module_platform_driver()?
yeah it can be. will fix it.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller
2020-03-09 21:13 ` Rob Herring
@ 2020-04-05 8:30 ` Sivaprakash Murugesan
0 siblings, 0 replies; 15+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-05 8:30 UTC (permalink / raw)
To: Rob Herring
Cc: agross, bjorn.andersson, mturquette, sboyd, linux-arm-msm,
linux-clk, devicetree, linux-kernel
Hi Rob,
Thanks for the review.
On 3/10/2020 2:43 AM, Rob Herring wrote:
> On Thu, Feb 27, 2020 at 03:25:17PM +0530, Sivaprakash Murugesan wrote:
>> add dt-binding for ipq6018 apss clock controller
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>> .../devicetree/bindings/clock/qcom,apsscc.yaml | 58 ++++++++++++++++++++++
>> include/dt-bindings/clock/qcom,apss-ipq6018.h | 26 ++++++++++
>> 2 files changed, 84 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> new file mode 100644
>> index 0000000..7433721
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
> Dual license new bindings please:
>
> (GPL-2.0-only OR BSD-2-Clause)
ok.
>
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/bindings/clock/qcom,apsscc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm IPQ6018 APSS Clock Controller Binding
>> +
>> +maintainers:
>> + - Stephen Boyd <sboyd@kernel.org>
> I'd expect this to be a QCom person, not who is applying patches.
>
>> +
>> +description: |
> You can drop '|'.
ok.
>> + Qualcomm IPQ6018 APSS clock control module which supports the clocks with
>> + frequencies above 800Mhz.
>> +
>> +properties:
>> + compatible :
>> + const: qcom,apss-ipq6018
> Normal ordering is: qcom,ipq6018-apss
ok.
>> +
>> + clocks:
>> + description: clocks required for this controller.
>> + maxItems: 4
> Need to define what each clock is.
ok.
>> +
>> + clock-names:
>> + description: clock output names of required clocks.
>> + maxItems: 4
>> +
>> + '#clock-cells':
>> + const: 1
>> +
>> + '#reset-cells':
>> + const: 1
>> +
>> + reg:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - '#clock-cells'
>> + - '#reset-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
>> + apss_clk: qcom,apss_clk@b111000 {
> I thought I'd finally seen the last of these Qcom node names...
>
> clock-controller@...
ok.
>> + compatible = "qcom,apss-ipq6018";
>> + clocks = <&xo>, <&gcc GPLL0>,
>> + <&gcc GPLL2>, <&gcc GPLL4>;
>> + clock-names = "xo", "gpll0",
>> + "gpll2", "gpll4";
>> + reg = <0xb11100c 0x5ff4>;
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + };
>> +...
>> diff --git a/include/dt-bindings/clock/qcom,apss-ipq6018.h b/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> new file mode 100644
>> index 0000000..ed9d7d8
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
> I'm pretty sure your employer would like an additional license here.
my bad. will correct it.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-04-05 8:30 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 9:55 [PATCH 0/2] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
2020-02-27 9:55 ` [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
2020-02-27 10:38 ` Sibi Sankar
2020-02-27 10:55 ` Sivaprakash Murugesan
2020-04-05 8:19 ` Sivaprakash Murugesan
2020-02-27 17:14 ` Rob Herring
2020-03-04 5:24 ` Sivaprakash Murugesan
2020-03-09 21:07 ` Rob Herring
2020-03-09 21:13 ` Rob Herring
2020-04-05 8:30 ` Sivaprakash Murugesan
2020-02-27 9:55 ` [PATCH 2/2] clk: qcom: Add " Sivaprakash Murugesan
2020-02-27 17:40 ` kbuild test robot
2020-02-27 17:40 ` [RFC PATCH] clk: qcom: apss_pll_offsets[] can be static kbuild test robot
2020-03-09 20:24 ` [PATCH 2/2] clk: qcom: Add ipq6018 apss clock controller Stephen Boyd
2020-04-05 8:26 ` Sivaprakash Murugesan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).