* [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates
@ 2020-08-28 13:39 Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 1/8] dt-bindings: add documentation of xilinx clocking wizard Shubhrajyoti Datta
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta
In the thread [1] Greg suggested that we move the driver
to the clk from the staging.
Add patches to address the concerns regarding the fractional and
set rate support in the TODO.
The patch set does the following
- Trivial fixes for kernel doc.
- Move the driver to the clk folder
- Add capability to set rate.
- Add fractional support.
- Add support for configurable outputs.
- Make the output names unique so that multiple instances
do not crib.
Changes in the v3:
Added the cover-letter.
Add patches for rate setting and fractional support
Add patches for warning.
Remove the driver from staging as suggested
v4:
Reorder the patches.
Merge the CLK_IS_BASIC patch.
Add the yaml form of binding document
v5:
Fix a mismerge
v6:
Fix the yaml warning
use poll timedout
[1] https://spinics.net/lists/linux-driver-devel/msg117326.html
Shubhrajyoti Datta (8):
dt-bindings: add documentation of xilinx clocking wizard
clk: clock-wizard: Add the clockwizard to clk directory
clk: clock-wizard: Fix kernel-doc warning
clk: clock-wizard: Add support for dynamic reconfiguration
clk: clock-wizard: Add support for fractional support
clk: clock-wizard: Remove the hardcoding of the clock outputs
clk: clock-wizard: Update the fixed factor divisors
staging: clocking-wizard: Delete the driver from the staging
.../bindings/clock/xlnx,clocking-wizard.yaml | 71 +++
drivers/clk/Kconfig | 9 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-xlnx-clock-wizard.c | 657 +++++++++++++++++++++
drivers/staging/Kconfig | 2 -
drivers/staging/Makefile | 1 -
drivers/staging/clocking-wizard/Kconfig | 10 -
drivers/staging/clocking-wizard/Makefile | 2 -
drivers/staging/clocking-wizard/TODO | 12 -
.../clocking-wizard/clk-xlnx-clock-wizard.c | 333 -----------
drivers/staging/clocking-wizard/dt-binding.txt | 30 -
11 files changed, 738 insertions(+), 390 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
delete mode 100644 drivers/staging/clocking-wizard/Kconfig
delete mode 100644 drivers/staging/clocking-wizard/Makefile
delete mode 100644 drivers/staging/clocking-wizard/TODO
delete mode 100644 drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt
--
2.1.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v6 1/8] dt-bindings: add documentation of xilinx clocking wizard
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
@ 2020-08-28 13:39 ` Shubhrajyoti Datta
2020-08-28 21:56 ` Rob Herring
2020-08-28 13:39 ` [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk directory Shubhrajyoti Datta
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta
Add the devicetree binding for the xilinx clocking wizard.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v6:
Fix a yaml warning
.../bindings/clock/xlnx,clocking-wizard.yaml | 71 ++++++++++++++++++++++
1 file changed, 71 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
new file mode 100644
index 0000000..ca63593
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx clocking wizard
+
+maintainers:
+ - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
+
+description: |
+ The clocking wizard is a soft ip clocking block of Xilinx versal. It
+ reads required input clock frequencies from the devicetree and acts as clock
+ clock output.
+
+select: false
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - xlnx,clocking-wizard
+ - xlnx,clocking-wizard-6.0
+
+ "#clock-cells":
+ const: 1
+
+ clocks:
+ description: List of clock specifiers which are external input
+ clocks to the given clock controller.
+ items:
+ - description: clock input
+ - description: axi clock
+
+ clock-names:
+ items:
+ - const: clk_in1
+ - const: s_axi_aclk
+
+ speed-grade:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - enum: [1, 2, 3]
+ description:
+ Speed grade of the device.
+ maxItems: 1
+
+required:
+ - compatible
+ - "#clock-cells"
+ - clocks
+ - clock-names
+ - speed-grade
+
+additionalProperties: false
+
+examples:
+ - |
+ clock-generator@40040000 {
+ #clock-cells = <1>;
+ reg = <0x40040000 0x1000>;
+ compatible = "xlnx,clk-wizard-1.0";
+ speed-grade = <1>;
+ clock-names = "clk_in1", "s_axi_aclk";
+ clocks = <&clkc 15>, <&clkc 15>;
+ clock-output-names = "clk_out1", "clk_out2",
+ "clk_out3", "clk_out4", "clk_out5",
+ "clk_out6", "clk_out7";
+ };
+...
--
2.1.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk directory
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 1/8] dt-bindings: add documentation of xilinx clocking wizard Shubhrajyoti Datta
@ 2020-08-28 13:39 ` Shubhrajyoti Datta
2020-09-21 21:19 ` Stephen Boyd
2020-08-28 13:39 ` [PATCH v6 3/8] clk: clock-wizard: Fix kernel-doc warning Shubhrajyoti Datta
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta
Add clocking wizard driver to clk.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
drivers/clk/Kconfig | 9 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-xlnx-clock-wizard.c | 338 ++++++++++++++++++++++++++++++++++++
3 files changed, 348 insertions(+)
create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 4026fac..a0e29dd 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -359,6 +359,15 @@ config COMMON_CLK_FIXED_MMIO
help
Support for Memory Mapped IO Fixed clocks
+config COMMON_CLK_XLNX_CLKWZRD
+ tristate "Xilinx Clocking Wizard"
+ depends on COMMON_CLK && OF
+ help
+ Support for the Xilinx Clocking Wizard IP core clock generator.
+ Adds support for clocking wizard and compatible.
+ This driver supports the Xilinx clocking wizard programmable clock
+ synthesizer. The number of output is configurable in the design.
+
source "drivers/clk/actions/Kconfig"
source "drivers/clk/analogbits/Kconfig"
source "drivers/clk/baikal-t1/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index da8fcf1..1ad6414 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_COMMON_CLK_VC5) += clk-versaclock5.o
obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
obj-$(CONFIG_COMMON_CLK_XGENE) += clk-xgene.o
+obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD) += clk-xlnx-clock-wizard.o
# please keep this section sorted lexicographically by directory path name
obj-y += actions/
diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
new file mode 100644
index 0000000..b31524a
--- /dev/null
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx 'Clocking Wizard' driver
+ *
+ * Copyright (C) 2013 - 2020 Xilinx
+ *
+ * Sören Brinkmann <soren.brinkmann@xilinx.com>
+ * Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/module.h>
+#include <linux/err.h>
+
+#define WZRD_NUM_OUTPUTS 7
+#define WZRD_ACLK_MAX_FREQ 250000000UL
+
+#define WZRD_CLK_CFG_REG(n) (0x200 + 4 * (n))
+
+#define WZRD_CLKOUT0_FRAC_EN BIT(18)
+#define WZRD_CLKFBOUT_FRAC_EN BIT(26)
+
+#define WZRD_CLKFBOUT_MULT_SHIFT 8
+#define WZRD_CLKFBOUT_MULT_MASK (0xff << WZRD_CLKFBOUT_MULT_SHIFT)
+#define WZRD_DIVCLK_DIVIDE_SHIFT 0
+#define WZRD_DIVCLK_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
+#define WZRD_CLKOUT_DIVIDE_SHIFT 0
+#define WZRD_CLKOUT_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
+
+enum clk_wzrd_int_clks {
+ wzrd_clk_mul,
+ wzrd_clk_mul_div,
+ wzrd_clk_int_max
+};
+
+/**
+ * struct clk_wzrd:
+ * @clk_data: Clock data
+ * @nb: Notifier block
+ * @base: Memory base
+ * @clk_in1: Handle to input clock 'clk_in1'
+ * @axi_clk: Handle to input clock 's_axi_aclk'
+ * @clks_internal: Internal clocks
+ * @clkout: Output clocks
+ * @speed_grade: Speed grade of the device
+ * @suspended: Flag indicating power state of the device
+ */
+struct clk_wzrd {
+ struct clk_onecell_data clk_data;
+ struct notifier_block nb;
+ void __iomem *base;
+ struct clk *clk_in1;
+ struct clk *axi_clk;
+ struct clk *clks_internal[wzrd_clk_int_max];
+ struct clk *clkout[WZRD_NUM_OUTPUTS];
+ unsigned int speed_grade;
+ bool suspended;
+};
+
+#define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb)
+
+/* maximum frequencies for input/output clocks per speed grade */
+static const unsigned long clk_wzrd_max_freq[] = {
+ 800000000UL,
+ 933000000UL,
+ 1066000000UL
+};
+
+static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
+ void *data)
+{
+ unsigned long max;
+ struct clk_notifier_data *ndata = data;
+ struct clk_wzrd *clk_wzrd = to_clk_wzrd(nb);
+
+ if (clk_wzrd->suspended)
+ return NOTIFY_OK;
+
+ if (ndata->clk == clk_wzrd->clk_in1)
+ max = clk_wzrd_max_freq[clk_wzrd->speed_grade - 1];
+ else if (ndata->clk == clk_wzrd->axi_clk)
+ max = WZRD_ACLK_MAX_FREQ;
+ else
+ return NOTIFY_DONE; /* should never happen */
+
+ switch (event) {
+ case PRE_RATE_CHANGE:
+ if (ndata->new_rate > max)
+ return NOTIFY_BAD;
+ return NOTIFY_OK;
+ case POST_RATE_CHANGE:
+ case ABORT_RATE_CHANGE:
+ default:
+ return NOTIFY_DONE;
+ }
+}
+
+static int __maybe_unused clk_wzrd_suspend(struct device *dev)
+{
+ struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(clk_wzrd->axi_clk);
+ clk_wzrd->suspended = true;
+
+ return 0;
+}
+
+static int __maybe_unused clk_wzrd_resume(struct device *dev)
+{
+ int ret;
+ struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
+
+ ret = clk_prepare_enable(clk_wzrd->axi_clk);
+ if (ret) {
+ dev_err(dev, "unable to enable s_axi_aclk\n");
+ return ret;
+ }
+
+ clk_wzrd->suspended = false;
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops, clk_wzrd_suspend,
+ clk_wzrd_resume);
+
+static int clk_wzrd_probe(struct platform_device *pdev)
+{
+ int i, ret;
+ u32 reg;
+ unsigned long rate;
+ const char *clk_name;
+ struct clk_wzrd *clk_wzrd;
+ struct resource *mem;
+ struct device_node *np = pdev->dev.of_node;
+
+ clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
+ if (!clk_wzrd)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, clk_wzrd);
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ clk_wzrd->base = devm_ioremap_resource(&pdev->dev, mem);
+ if (IS_ERR(clk_wzrd->base))
+ return PTR_ERR(clk_wzrd->base);
+
+ ret = of_property_read_u32(np, "speed-grade", &clk_wzrd->speed_grade);
+ if (!ret) {
+ if (clk_wzrd->speed_grade < 1 || clk_wzrd->speed_grade > 3) {
+ dev_warn(&pdev->dev, "invalid speed grade '%d'\n",
+ clk_wzrd->speed_grade);
+ clk_wzrd->speed_grade = 0;
+ }
+ }
+
+ clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
+ if (IS_ERR(clk_wzrd->clk_in1)) {
+ if (clk_wzrd->clk_in1 != ERR_PTR(-EPROBE_DEFER))
+ dev_err(&pdev->dev, "clk_in1 not found\n");
+ return PTR_ERR(clk_wzrd->clk_in1);
+ }
+
+ clk_wzrd->axi_clk = devm_clk_get(&pdev->dev, "s_axi_aclk");
+ if (IS_ERR(clk_wzrd->axi_clk)) {
+ if (clk_wzrd->axi_clk != ERR_PTR(-EPROBE_DEFER))
+ dev_err(&pdev->dev, "s_axi_aclk not found\n");
+ return PTR_ERR(clk_wzrd->axi_clk);
+ }
+ ret = clk_prepare_enable(clk_wzrd->axi_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "enabling s_axi_aclk failed\n");
+ return ret;
+ }
+ rate = clk_get_rate(clk_wzrd->axi_clk);
+ if (rate > WZRD_ACLK_MAX_FREQ) {
+ dev_err(&pdev->dev, "s_axi_aclk frequency (%lu) too high\n",
+ rate);
+ ret = -EINVAL;
+ goto err_disable_clk;
+ }
+
+ /* we don't support fractional div/mul yet */
+ reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
+ WZRD_CLKFBOUT_FRAC_EN;
+ reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) &
+ WZRD_CLKOUT0_FRAC_EN;
+ if (reg)
+ dev_warn(&pdev->dev, "fractional div/mul not supported\n");
+
+ /* register multiplier */
+ reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
+ WZRD_CLKFBOUT_MULT_MASK) >> WZRD_CLKFBOUT_MULT_SHIFT;
+ clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
+ if (!clk_name) {
+ ret = -ENOMEM;
+ goto err_disable_clk;
+ }
+ clk_wzrd->clks_internal[wzrd_clk_mul] = clk_register_fixed_factor
+ (&pdev->dev, clk_name,
+ __clk_get_name(clk_wzrd->clk_in1),
+ 0, reg, 1);
+ kfree(clk_name);
+ if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul])) {
+ dev_err(&pdev->dev, "unable to register fixed-factor clock\n");
+ ret = PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul]);
+ goto err_disable_clk;
+ }
+
+ /* register div */
+ reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
+ WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
+ clk_name = kasprintf(GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
+ if (!clk_name) {
+ ret = -ENOMEM;
+ goto err_rm_int_clk;
+ }
+
+ clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_fixed_factor
+ (&pdev->dev, clk_name,
+ __clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
+ 0, 1, reg);
+ if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div])) {
+ dev_err(&pdev->dev, "unable to register divider clock\n");
+ ret = PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div]);
+ goto err_rm_int_clk;
+ }
+
+ /* register div per output */
+ for (i = WZRD_NUM_OUTPUTS - 1; i >= 0 ; i--) {
+ const char *clkout_name;
+
+ if (of_property_read_string_index(np, "clock-output-names", i,
+ &clkout_name)) {
+ dev_err(&pdev->dev,
+ "clock output name not specified\n");
+ ret = -EINVAL;
+ goto err_rm_int_clks;
+ }
+ reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2) + i * 12);
+ reg &= WZRD_CLKOUT_DIVIDE_MASK;
+ reg >>= WZRD_CLKOUT_DIVIDE_SHIFT;
+ clk_wzrd->clkout[i] = clk_register_fixed_factor
+ (&pdev->dev, clkout_name, clk_name, 0, 1, reg);
+ if (IS_ERR(clk_wzrd->clkout[i])) {
+ int j;
+
+ for (j = i + 1; j < WZRD_NUM_OUTPUTS; j++)
+ clk_unregister(clk_wzrd->clkout[j]);
+ dev_err(&pdev->dev,
+ "unable to register divider clock\n");
+ ret = PTR_ERR(clk_wzrd->clkout[i]);
+ goto err_rm_int_clks;
+ }
+ }
+
+ kfree(clk_name);
+
+ clk_wzrd->clk_data.clks = clk_wzrd->clkout;
+ clk_wzrd->clk_data.clk_num = ARRAY_SIZE(clk_wzrd->clkout);
+ of_clk_add_provider(np, of_clk_src_onecell_get, &clk_wzrd->clk_data);
+
+ if (clk_wzrd->speed_grade) {
+ clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
+
+ ret = clk_notifier_register(clk_wzrd->clk_in1,
+ &clk_wzrd->nb);
+ if (ret)
+ dev_warn(&pdev->dev,
+ "unable to register clock notifier\n");
+
+ ret = clk_notifier_register(clk_wzrd->axi_clk, &clk_wzrd->nb);
+ if (ret)
+ dev_warn(&pdev->dev,
+ "unable to register clock notifier\n");
+ }
+
+ return 0;
+
+err_rm_int_clks:
+ clk_unregister(clk_wzrd->clks_internal[1]);
+err_rm_int_clk:
+ kfree(clk_name);
+ clk_unregister(clk_wzrd->clks_internal[0]);
+err_disable_clk:
+ clk_disable_unprepare(clk_wzrd->axi_clk);
+
+ return ret;
+}
+
+static int clk_wzrd_remove(struct platform_device *pdev)
+{
+ int i;
+ struct clk_wzrd *clk_wzrd = platform_get_drvdata(pdev);
+
+ of_clk_del_provider(pdev->dev.of_node);
+
+ for (i = 0; i < WZRD_NUM_OUTPUTS; i++)
+ clk_unregister(clk_wzrd->clkout[i]);
+ for (i = 0; i < wzrd_clk_int_max; i++)
+ clk_unregister(clk_wzrd->clks_internal[i]);
+
+ if (clk_wzrd->speed_grade) {
+ clk_notifier_unregister(clk_wzrd->axi_clk, &clk_wzrd->nb);
+ clk_notifier_unregister(clk_wzrd->clk_in1, &clk_wzrd->nb);
+ }
+
+ clk_disable_unprepare(clk_wzrd->axi_clk);
+
+ return 0;
+}
+
+static const struct of_device_id clk_wzrd_ids[] = {
+ { .compatible = "xlnx,clocking-wizard" },
+ { .compatible = "xlnx,clocking-wizard-6.0" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, clk_wzrd_ids);
+
+static struct platform_driver clk_wzrd_driver = {
+ .driver = {
+ .name = "clk-wizard",
+ .of_match_table = clk_wzrd_ids,
+ .pm = &clk_wzrd_dev_pm_ops,
+ },
+ .probe = clk_wzrd_probe,
+ .remove = clk_wzrd_remove,
+};
+module_platform_driver(clk_wzrd_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Soeren Brinkmann <soren.brinkmann@xilinx.com");
+MODULE_DESCRIPTION("Driver for the Xilinx Clocking Wizard IP core");
--
2.1.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 3/8] clk: clock-wizard: Fix kernel-doc warning
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 1/8] dt-bindings: add documentation of xilinx clocking wizard Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk directory Shubhrajyoti Datta
@ 2020-08-28 13:39 ` Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 4/8] clk: clock-wizard: Add support for dynamic reconfiguration Shubhrajyoti Datta
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta
Update description for the clocking wizard structure
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
drivers/clk/clk-xlnx-clock-wizard.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index b31524a..d6577c8 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -40,7 +40,8 @@ enum clk_wzrd_int_clks {
};
/**
- * struct clk_wzrd:
+ * struct clk_wzrd - Clock wizard private data structure
+ *
* @clk_data: Clock data
* @nb: Notifier block
* @base: Memory base
--
2.1.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 4/8] clk: clock-wizard: Add support for dynamic reconfiguration
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
` (2 preceding siblings ...)
2020-08-28 13:39 ` [PATCH v6 3/8] clk: clock-wizard: Fix kernel-doc warning Shubhrajyoti Datta
@ 2020-08-28 13:39 ` Shubhrajyoti Datta
2020-09-21 21:26 ` Stephen Boyd
2020-08-28 13:39 ` [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support Shubhrajyoti Datta
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta, Chirag Parekh
The patch adds support for dynamic reconfiguration of clock output rate.
Output clocks are registered as dividers and set rate callback function
is used for dynamic reconfiguration.
Based on the initial work from Chirag.
Signed-off-by: Chirag Parekh <chirag.parekh@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v6:
Remove the typecast.
use min for capping frequency.
use polled timeout
drivers/clk/clk-xlnx-clock-wizard.c | 185 ++++++++++++++++++++++++++++++++++--
1 file changed, 179 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index d6577c8..8dfcec8 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -17,6 +17,7 @@
#include <linux/of.h>
#include <linux/module.h>
#include <linux/err.h>
+#include <linux/iopoll.h>
#define WZRD_NUM_OUTPUTS 7
#define WZRD_ACLK_MAX_FREQ 250000000UL
@@ -31,8 +32,23 @@
#define WZRD_DIVCLK_DIVIDE_SHIFT 0
#define WZRD_DIVCLK_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
#define WZRD_CLKOUT_DIVIDE_SHIFT 0
+#define WZRD_CLKOUT_DIVIDE_WIDTH 8
#define WZRD_CLKOUT_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
+#define WZRD_DR_MAX_INT_DIV_VALUE 255
+#define WZRD_DR_NUM_RETRIES 10000
+#define WZRD_DR_STATUS_REG_OFFSET 0x04
+#define WZRD_DR_LOCK_BIT_MASK 0x00000001
+#define WZRD_DR_INIT_REG_OFFSET 0x25C
+#define WZRD_DR_DIV_TO_PHASE_OFFSET 4
+#define WZRD_DR_BEGIN_DYNA_RECONF 0x03
+
+/* Get the mask from width */
+#define div_mask(width) ((1 << (width)) - 1)
+
+/* Extract divider instance from clock hardware instance */
+#define to_clk_wzrd_divider(_hw) container_of(_hw, struct clk_wzrd_divider, hw)
+
enum clk_wzrd_int_clks {
wzrd_clk_mul,
wzrd_clk_mul_div,
@@ -64,6 +80,29 @@ struct clk_wzrd {
bool suspended;
};
+/**
+ * struct clk_wzrd_divider - clock divider specific to clk_wzrd
+ *
+ * @hw: handle between common and hardware-specific interfaces
+ * @base: base address of register containing the divider
+ * @offset: offset address of register containing the divider
+ * @shift: shift to the divider bit field
+ * @width: width of the divider bit field
+ * @flags: clk_wzrd divider flags
+ * @table: array of value/divider pairs, last entry should have div = 0
+ * @lock: register lock
+ */
+struct clk_wzrd_divider {
+ struct clk_hw hw;
+ void __iomem *base;
+ u16 offset;
+ u8 shift;
+ u8 width;
+ u8 flags;
+ const struct clk_div_table *table;
+ spinlock_t *lock; /* divider lock */
+};
+
#define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb)
/* maximum frequencies for input/output clocks per speed grade */
@@ -73,6 +112,136 @@ static const unsigned long clk_wzrd_max_freq[] = {
1066000000UL
};
+/* spin lock variable for clk_wzrd */
+static DEFINE_SPINLOCK(clkwzrd_lock);
+
+static unsigned long clk_wzrd_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
+ void __iomem *div_addr = divider->base + divider->offset;
+ unsigned int val;
+
+ val = readl(div_addr) >> divider->shift;
+ val &= div_mask(divider->width);
+
+ return divider_recalc_rate(hw, parent_rate, val, divider->table,
+ divider->flags, divider->width);
+}
+
+static int clk_wzrd_dynamic_reconfig(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int err = 0;
+ u32 value;
+ struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
+ void __iomem *div_addr = divider->base + divider->offset;
+
+ value = DIV_ROUND_CLOSEST(parent_rate, rate);
+
+ /* Cap the value to max */
+ min(value, (u32)WZRD_DR_MAX_INT_DIV_VALUE);
+
+ /* Set divisor and clear phase offset */
+ writel(value, div_addr);
+ writel(0x00, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET);
+
+ /* Check status register */
+ err = readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
+ value & WZRD_DR_LOCK_BIT_MASK,
+ WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
+ if (err)
+ return err;
+
+ /* Initiate reconfiguration */
+ writel(WZRD_DR_BEGIN_DYNA_RECONF,
+ divider->base + WZRD_DR_INIT_REG_OFFSET);
+
+ /* Check status register */
+ err = readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
+ value & WZRD_DR_LOCK_BIT_MASK,
+ WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
+
+ return err;
+}
+
+static long clk_wzrd_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ u8 div;
+
+ /*
+ * since we don't change parent rate we just round rate to closest
+ * achievable
+ */
+ div = DIV_ROUND_CLOSEST(*prate, rate);
+
+ return (*prate / div);
+}
+
+static const struct clk_ops clk_wzrd_clk_divider_ops = {
+ .round_rate = clk_wzrd_round_rate,
+ .set_rate = clk_wzrd_dynamic_reconfig,
+ .recalc_rate = clk_wzrd_recalc_rate,
+};
+
+static struct clk *clk_wzrd_register_divider(struct device *dev,
+ const char *name,
+ const char *parent_name,
+ unsigned long flags,
+ void __iomem *base, u16 offset,
+ u8 shift, u8 width,
+ u8 clk_divider_flags,
+ const struct clk_div_table *table,
+ spinlock_t *lock)
+{
+ struct clk_wzrd_divider *div;
+ struct clk_hw *hw;
+ struct clk_init_data init;
+ int ret;
+
+ if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
+ if (width + shift > 16) {
+ pr_warn("divider value exceeds LOWORD field\n");
+ return ERR_PTR(-EINVAL);
+ }
+ }
+
+ /* allocate the divider */
+ div = kzalloc(sizeof(*div), GFP_KERNEL);
+ if (!div)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
+ init.ops = &clk_divider_ro_ops;
+ else
+ init.ops = &clk_wzrd_clk_divider_ops;
+ init.flags = flags;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = (parent_name ? 1 : 0);
+
+ /* struct clk_divider assignments */
+ div->base = base;
+ div->offset = offset;
+ div->shift = shift;
+ div->width = width;
+ div->flags = clk_divider_flags;
+ div->lock = lock;
+ div->hw.init = &init;
+ div->table = table;
+
+ /* register the clock */
+ hw = &div->hw;
+ ret = clk_hw_register(dev, hw);
+ if (ret) {
+ kfree(div);
+ hw = ERR_PTR(ret);
+ }
+
+ return hw->clk;
+}
+
static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
void *data)
{
@@ -225,7 +394,8 @@ static int clk_wzrd_probe(struct platform_device *pdev)
clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_fixed_factor
(&pdev->dev, clk_name,
__clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
- 0, 1, reg);
+ flags, ctrl_reg, 0, 8, CLK_DIVIDER_ONE_BASED |
+ CLK_DIVIDER_ALLOW_ZERO, &clkwzrd_lock);
if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div])) {
dev_err(&pdev->dev, "unable to register divider clock\n");
ret = PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div]);
@@ -243,11 +413,14 @@ static int clk_wzrd_probe(struct platform_device *pdev)
ret = -EINVAL;
goto err_rm_int_clks;
}
- reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2) + i * 12);
- reg &= WZRD_CLKOUT_DIVIDE_MASK;
- reg >>= WZRD_CLKOUT_DIVIDE_SHIFT;
- clk_wzrd->clkout[i] = clk_register_fixed_factor
- (&pdev->dev, clkout_name, clk_name, 0, 1, reg);
+ clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev,
+ clkout_name,
+ clk_name, 0,
+ clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
+ WZRD_CLKOUT_DIVIDE_SHIFT,
+ WZRD_CLKOUT_DIVIDE_WIDTH,
+ CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
+ NULL, &clkwzrd_lock);
if (IS_ERR(clk_wzrd->clkout[i])) {
int j;
--
2.1.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
` (3 preceding siblings ...)
2020-08-28 13:39 ` [PATCH v6 4/8] clk: clock-wizard: Add support for dynamic reconfiguration Shubhrajyoti Datta
@ 2020-08-28 13:39 ` Shubhrajyoti Datta
2020-09-21 21:17 ` Stephen Boyd
2020-09-22 9:17 ` Dan Carpenter
2020-08-28 13:39 ` [PATCH v6 6/8] clk: clock-wizard: Remove the hardcoding of the clock outputs Shubhrajyoti Datta
` (2 subsequent siblings)
7 siblings, 2 replies; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta
Currently the set rate granularity is to integral divisors.
Add support for the fractional divisors.
Only the first output0 is fractional in the hardware.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v6:
remove unnecessary typecast
remove unnecessary locks
use polled timeout
drivers/clk/clk-xlnx-clock-wizard.c | 166 +++++++++++++++++++++++++++++++++---
1 file changed, 153 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index 8dfcec8..1af59a4 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -29,20 +29,25 @@
#define WZRD_CLKFBOUT_MULT_SHIFT 8
#define WZRD_CLKFBOUT_MULT_MASK (0xff << WZRD_CLKFBOUT_MULT_SHIFT)
+#define WZRD_CLKFBOUT_FRAC_SHIFT 16
+#define WZRD_CLKFBOUT_FRAC_MASK (0x3ff << WZRD_CLKFBOUT_FRAC_SHIFT)
#define WZRD_DIVCLK_DIVIDE_SHIFT 0
#define WZRD_DIVCLK_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
#define WZRD_CLKOUT_DIVIDE_SHIFT 0
#define WZRD_CLKOUT_DIVIDE_WIDTH 8
#define WZRD_CLKOUT_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
+#define WZRD_CLKOUT_FRAC_SHIFT 8
+#define WZRD_CLKOUT_FRAC_MASK 0x3ff
#define WZRD_DR_MAX_INT_DIV_VALUE 255
-#define WZRD_DR_NUM_RETRIES 10000
#define WZRD_DR_STATUS_REG_OFFSET 0x04
#define WZRD_DR_LOCK_BIT_MASK 0x00000001
#define WZRD_DR_INIT_REG_OFFSET 0x25C
#define WZRD_DR_DIV_TO_PHASE_OFFSET 4
#define WZRD_DR_BEGIN_DYNA_RECONF 0x03
+#define WZRD_USEC_POLL 10
+#define WZRD_TIMEOUT_POLL 1000
/* Get the mask from width */
#define div_mask(width) ((1 << (width)) - 1)
@@ -52,6 +57,7 @@
enum clk_wzrd_int_clks {
wzrd_clk_mul,
wzrd_clk_mul_div,
+ wzrd_clk_mul_frac,
wzrd_clk_int_max
};
@@ -185,6 +191,134 @@ static const struct clk_ops clk_wzrd_clk_divider_ops = {
.recalc_rate = clk_wzrd_recalc_rate,
};
+static unsigned long clk_wzrd_recalc_ratef(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ unsigned int val;
+ u32 div, frac;
+ struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
+ void __iomem *div_addr = divider->base + divider->offset;
+
+ val = readl(div_addr);
+ div = val & div_mask(divider->width);
+ frac = (val >> WZRD_CLKOUT_FRAC_SHIFT) & WZRD_CLKOUT_FRAC_MASK;
+
+ return ((parent_rate * 1000) / ((div * 1000) + frac));
+}
+
+static int clk_wzrd_dynamic_reconfig_f(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int err;
+ u32 value, pre;
+ unsigned long rate_div, f, clockout0_div;
+ struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
+ void __iomem *div_addr = divider->base + divider->offset;
+
+ rate_div = ((parent_rate * 1000) / rate);
+ clockout0_div = rate_div / 1000;
+
+ pre = DIV_ROUND_CLOSEST((parent_rate * 1000), rate);
+ f = (u32)(pre - (clockout0_div * 1000));
+ f = f & WZRD_CLKOUT_FRAC_MASK;
+
+ value = ((f << WZRD_CLKOUT_DIVIDE_WIDTH) | (clockout0_div &
+ WZRD_CLKOUT_DIVIDE_MASK));
+
+ /* Set divisor and clear phase offset */
+ writel(value, div_addr);
+ writel(0x0, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET);
+
+ /* Check status register */
+ err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
+ value & WZRD_DR_LOCK_BIT_MASK,
+ WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
+ if (err)
+ return err;
+
+ /* Initiate reconfiguration */
+ writel(WZRD_DR_BEGIN_DYNA_RECONF,
+ divider->base + WZRD_DR_INIT_REG_OFFSET);
+
+ /* Check status register */
+ err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
+ value & WZRD_DR_LOCK_BIT_MASK,
+ WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
+
+ return err;
+}
+
+static long clk_wzrd_round_rate_f(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ return rate;
+}
+
+static const struct clk_ops clk_wzrd_clk_divider_ops_f = {
+ .round_rate = clk_wzrd_round_rate_f,
+ .set_rate = clk_wzrd_dynamic_reconfig_f,
+ .recalc_rate = clk_wzrd_recalc_ratef,
+};
+
+static struct clk *clk_wzrd_register_divf(struct device *dev,
+ const char *name,
+ const char *parent_name,
+ unsigned long flags,
+ void __iomem *base, u16 offset,
+ u8 shift, u8 width,
+ u8 clk_divider_flags,
+ const struct clk_div_table *table,
+ spinlock_t *lock)
+{
+ struct clk_wzrd_divider *div;
+ struct clk_hw *hw;
+ struct clk_init_data init;
+ int ret;
+
+ if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
+ if (width + shift > 16) {
+ pr_warn("divider value exceeds LOWORD field\n");
+ return ERR_PTR(-EINVAL);
+ }
+ }
+
+ /* allocate the divider */
+ div = kzalloc(sizeof(*div), GFP_KERNEL);
+ if (!div)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+
+ if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
+ init.ops = &clk_divider_ro_ops;
+ else
+ init.ops = &clk_wzrd_clk_divider_ops_f;
+
+ init.flags = flags;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = (parent_name ? 1 : 0);
+
+ /* struct clk_divider assignments */
+ div->base = base;
+ div->offset = offset;
+ div->shift = shift;
+ div->width = width;
+ div->flags = clk_divider_flags;
+ div->lock = lock;
+ div->hw.init = &init;
+ div->table = table;
+
+ /* register the clock */
+ hw = &div->hw;
+ ret = clk_hw_register(dev, hw);
+ if (ret) {
+ kfree(div);
+ return ERR_PTR(ret);
+ }
+
+ return hw->clk;
+}
+
static struct clk *clk_wzrd_register_divider(struct device *dev,
const char *name,
const char *parent_name,
@@ -303,7 +437,7 @@ static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops, clk_wzrd_suspend,
static int clk_wzrd_probe(struct platform_device *pdev)
{
int i, ret;
- u32 reg;
+ u32 reg, reg_f, mult;
unsigned long rate;
const char *clk_name;
struct clk_wzrd *clk_wzrd;
@@ -355,17 +489,13 @@ static int clk_wzrd_probe(struct platform_device *pdev)
goto err_disable_clk;
}
- /* we don't support fractional div/mul yet */
- reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
- WZRD_CLKFBOUT_FRAC_EN;
- reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) &
- WZRD_CLKOUT0_FRAC_EN;
- if (reg)
- dev_warn(&pdev->dev, "fractional div/mul not supported\n");
-
/* register multiplier */
reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
WZRD_CLKFBOUT_MULT_MASK) >> WZRD_CLKFBOUT_MULT_SHIFT;
+ reg_f = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
+ WZRD_CLKFBOUT_FRAC_MASK) >> WZRD_CLKFBOUT_FRAC_SHIFT;
+
+ mult = ((reg * 1000) + reg_f);
clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
if (!clk_name) {
ret = -ENOMEM;
@@ -374,7 +504,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
clk_wzrd->clks_internal[wzrd_clk_mul] = clk_register_fixed_factor
(&pdev->dev, clk_name,
__clk_get_name(clk_wzrd->clk_in1),
- 0, reg, 1);
+ 0, mult, 1000);
kfree(clk_name);
if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul])) {
dev_err(&pdev->dev, "unable to register fixed-factor clock\n");
@@ -413,8 +543,18 @@ static int clk_wzrd_probe(struct platform_device *pdev)
ret = -EINVAL;
goto err_rm_int_clks;
}
- clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev,
- clkout_name,
+ if (!i)
+ clk_wzrd->clkout[i] = clk_wzrd_register_divf
+ (&pdev->dev, clkout_name,
+ clk_name, 0,
+ clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
+ WZRD_CLKOUT_DIVIDE_SHIFT,
+ WZRD_CLKOUT_DIVIDE_WIDTH,
+ CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
+ NULL, &clkwzrd_lock);
+ else
+ clk_wzrd->clkout[i] = clk_wzrd_register_divider
+ (&pdev->dev, clkout_name,
clk_name, 0,
clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
WZRD_CLKOUT_DIVIDE_SHIFT,
--
2.1.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 6/8] clk: clock-wizard: Remove the hardcoding of the clock outputs
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
` (4 preceding siblings ...)
2020-08-28 13:39 ` [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support Shubhrajyoti Datta
@ 2020-08-28 13:39 ` Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 7/8] clk: clock-wizard: Update the fixed factor divisors Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 8/8] staging: clocking-wizard: Delete the driver from the staging Shubhrajyoti Datta
7 siblings, 0 replies; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta
The number of output clocks are configurable in the hardware.
Currently the driver registers the maximum number of outputs.
Fix the same by registering only the outputs that are there.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v4:
Assign output in this patch
drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index 1af59a4..ded4cdd 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -442,6 +442,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
const char *clk_name;
struct clk_wzrd *clk_wzrd;
struct resource *mem;
+ int outputs;
struct device_node *np = pdev->dev.of_node;
clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
@@ -512,6 +513,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
goto err_disable_clk;
}
+ outputs = of_property_count_strings(np, "clock-output-names");
/* register div */
reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
@@ -533,7 +535,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
}
/* register div per output */
- for (i = WZRD_NUM_OUTPUTS - 1; i >= 0 ; i--) {
+ for (i = outputs - 1; i >= 0 ; i--) {
const char *clkout_name;
if (of_property_read_string_index(np, "clock-output-names", i,
@@ -564,7 +566,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
if (IS_ERR(clk_wzrd->clkout[i])) {
int j;
- for (j = i + 1; j < WZRD_NUM_OUTPUTS; j++)
+ for (j = i + 1; j < outputs; j++)
clk_unregister(clk_wzrd->clkout[j]);
dev_err(&pdev->dev,
"unable to register divider clock\n");
--
2.1.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 7/8] clk: clock-wizard: Update the fixed factor divisors
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
` (5 preceding siblings ...)
2020-08-28 13:39 ` [PATCH v6 6/8] clk: clock-wizard: Remove the hardcoding of the clock outputs Shubhrajyoti Datta
@ 2020-08-28 13:39 ` Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 8/8] staging: clocking-wizard: Delete the driver from the staging Shubhrajyoti Datta
7 siblings, 0 replies; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta
Update the fixed factor clock registration to register the divisors.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
drivers/clk/clk-xlnx-clock-wizard.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index ded4cdd..fd69eb0 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -440,9 +440,11 @@ static int clk_wzrd_probe(struct platform_device *pdev)
u32 reg, reg_f, mult;
unsigned long rate;
const char *clk_name;
+ void __iomem *ctrl_reg;
struct clk_wzrd *clk_wzrd;
struct resource *mem;
int outputs;
+ unsigned long flags = 0;
struct device_node *np = pdev->dev.of_node;
clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
@@ -514,16 +516,17 @@ static int clk_wzrd_probe(struct platform_device *pdev)
}
outputs = of_property_count_strings(np, "clock-output-names");
- /* register div */
- reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
- WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
+ if (outputs == 1)
+ flags = CLK_SET_RATE_PARENT;
clk_name = kasprintf(GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
if (!clk_name) {
ret = -ENOMEM;
goto err_rm_int_clk;
}
- clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_fixed_factor
+ ctrl_reg = clk_wzrd->base + WZRD_CLK_CFG_REG(0);
+ /* register div */
+ clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_divider
(&pdev->dev, clk_name,
__clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
flags, ctrl_reg, 0, 8, CLK_DIVIDER_ONE_BASED |
@@ -548,7 +551,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
if (!i)
clk_wzrd->clkout[i] = clk_wzrd_register_divf
(&pdev->dev, clkout_name,
- clk_name, 0,
+ clk_name, flags,
clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
WZRD_CLKOUT_DIVIDE_SHIFT,
WZRD_CLKOUT_DIVIDE_WIDTH,
--
2.1.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 8/8] staging: clocking-wizard: Delete the driver from the staging
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
` (6 preceding siblings ...)
2020-08-28 13:39 ` [PATCH v6 7/8] clk: clock-wizard: Update the fixed factor divisors Shubhrajyoti Datta
@ 2020-08-28 13:39 ` Shubhrajyoti Datta
7 siblings, 0 replies; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-08-28 13:39 UTC (permalink / raw)
To: linux-clk
Cc: devicetree, linux-kernel, devel, sboyd, robh+dt, gregkh,
mturquette, Shubhrajyoti Datta
Delete the driver from the staging as it is in drivers/clk.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/staging/Kconfig | 2 -
drivers/staging/Makefile | 1 -
drivers/staging/clocking-wizard/Kconfig | 10 -
drivers/staging/clocking-wizard/Makefile | 2 -
drivers/staging/clocking-wizard/TODO | 12 -
.../clocking-wizard/clk-xlnx-clock-wizard.c | 333 ---------------------
drivers/staging/clocking-wizard/dt-binding.txt | 30 --
7 files changed, 390 deletions(-)
delete mode 100644 drivers/staging/clocking-wizard/Kconfig
delete mode 100644 drivers/staging/clocking-wizard/Makefile
delete mode 100644 drivers/staging/clocking-wizard/TODO
delete mode 100644 drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index e6c831c..bae49c6 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -76,8 +76,6 @@ source "drivers/staging/gs_fpgaboot/Kconfig"
source "drivers/staging/unisys/Kconfig"
-source "drivers/staging/clocking-wizard/Kconfig"
-
source "drivers/staging/fbtft/Kconfig"
source "drivers/staging/fsl-dpaa2/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index a3b1fd0..f5a3e57 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -28,7 +28,6 @@ obj-$(CONFIG_FIREWIRE_SERIAL) += fwserial/
obj-$(CONFIG_GOLDFISH) += goldfish/
obj-$(CONFIG_GS_FPGABOOT) += gs_fpgaboot/
obj-$(CONFIG_UNISYSSPAR) += unisys/
-obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD) += clocking-wizard/
obj-$(CONFIG_FB_TFT) += fbtft/
obj-$(CONFIG_FSL_DPAA2) += fsl-dpaa2/
obj-$(CONFIG_MOST) += most/
diff --git a/drivers/staging/clocking-wizard/Kconfig b/drivers/staging/clocking-wizard/Kconfig
deleted file mode 100644
index 69cf514..0000000
--- a/drivers/staging/clocking-wizard/Kconfig
+++ /dev/null
@@ -1,10 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-#
-# Xilinx Clocking Wizard Driver
-#
-
-config COMMON_CLK_XLNX_CLKWZRD
- tristate "Xilinx Clocking Wizard"
- depends on COMMON_CLK && OF && IOMEM
- help
- Support for the Xilinx Clocking Wizard IP core clock generator.
diff --git a/drivers/staging/clocking-wizard/Makefile b/drivers/staging/clocking-wizard/Makefile
deleted file mode 100644
index b1f9152..0000000
--- a/drivers/staging/clocking-wizard/Makefile
+++ /dev/null
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD) += clk-xlnx-clock-wizard.o
diff --git a/drivers/staging/clocking-wizard/TODO b/drivers/staging/clocking-wizard/TODO
deleted file mode 100644
index ebe99db..0000000
--- a/drivers/staging/clocking-wizard/TODO
+++ /dev/null
@@ -1,12 +0,0 @@
-TODO:
- - support for fractional multiplier
- - support for fractional divider (output 0 only)
- - support for set_rate() operations (may benefit from Stephen Boyd's
- refactoring of the clk primitives: https://lkml.org/lkml/2014/9/5/766)
- - review arithmetic
- - overflow after multiplication?
- - maximize accuracy before divisions
-
-Patches to:
- Greg Kroah-Hartman <gregkh@linuxfoundation.org>
- Sören Brinkmann <soren.brinkmann@xilinx.com>
diff --git a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
deleted file mode 100644
index e52a64b..0000000
--- a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
+++ /dev/null
@@ -1,333 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Xilinx 'Clocking Wizard' driver
- *
- * Copyright (C) 2013 - 2014 Xilinx
- *
- * Sören Brinkmann <soren.brinkmann@xilinx.com>
- */
-
-#include <linux/platform_device.h>
-#include <linux/clk.h>
-#include <linux/clk-provider.h>
-#include <linux/slab.h>
-#include <linux/io.h>
-#include <linux/of.h>
-#include <linux/module.h>
-#include <linux/err.h>
-
-#define WZRD_NUM_OUTPUTS 7
-#define WZRD_ACLK_MAX_FREQ 250000000UL
-
-#define WZRD_CLK_CFG_REG(n) (0x200 + 4 * (n))
-
-#define WZRD_CLKOUT0_FRAC_EN BIT(18)
-#define WZRD_CLKFBOUT_FRAC_EN BIT(26)
-
-#define WZRD_CLKFBOUT_MULT_SHIFT 8
-#define WZRD_CLKFBOUT_MULT_MASK (0xff << WZRD_CLKFBOUT_MULT_SHIFT)
-#define WZRD_DIVCLK_DIVIDE_SHIFT 0
-#define WZRD_DIVCLK_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
-#define WZRD_CLKOUT_DIVIDE_SHIFT 0
-#define WZRD_CLKOUT_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
-
-enum clk_wzrd_int_clks {
- wzrd_clk_mul,
- wzrd_clk_mul_div,
- wzrd_clk_int_max
-};
-
-/**
- * struct clk_wzrd:
- * @clk_data: Clock data
- * @nb: Notifier block
- * @base: Memory base
- * @clk_in1: Handle to input clock 'clk_in1'
- * @axi_clk: Handle to input clock 's_axi_aclk'
- * @clks_internal: Internal clocks
- * @clkout: Output clocks
- * @speed_grade: Speed grade of the device
- * @suspended: Flag indicating power state of the device
- */
-struct clk_wzrd {
- struct clk_onecell_data clk_data;
- struct notifier_block nb;
- void __iomem *base;
- struct clk *clk_in1;
- struct clk *axi_clk;
- struct clk *clks_internal[wzrd_clk_int_max];
- struct clk *clkout[WZRD_NUM_OUTPUTS];
- unsigned int speed_grade;
- bool suspended;
-};
-
-#define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb)
-
-/* maximum frequencies for input/output clocks per speed grade */
-static const unsigned long clk_wzrd_max_freq[] = {
- 800000000UL,
- 933000000UL,
- 1066000000UL
-};
-
-static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
- void *data)
-{
- unsigned long max;
- struct clk_notifier_data *ndata = data;
- struct clk_wzrd *clk_wzrd = to_clk_wzrd(nb);
-
- if (clk_wzrd->suspended)
- return NOTIFY_OK;
-
- if (ndata->clk == clk_wzrd->clk_in1)
- max = clk_wzrd_max_freq[clk_wzrd->speed_grade - 1];
- else if (ndata->clk == clk_wzrd->axi_clk)
- max = WZRD_ACLK_MAX_FREQ;
- else
- return NOTIFY_DONE; /* should never happen */
-
- switch (event) {
- case PRE_RATE_CHANGE:
- if (ndata->new_rate > max)
- return NOTIFY_BAD;
- return NOTIFY_OK;
- case POST_RATE_CHANGE:
- case ABORT_RATE_CHANGE:
- default:
- return NOTIFY_DONE;
- }
-}
-
-static int __maybe_unused clk_wzrd_suspend(struct device *dev)
-{
- struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
-
- clk_disable_unprepare(clk_wzrd->axi_clk);
- clk_wzrd->suspended = true;
-
- return 0;
-}
-
-static int __maybe_unused clk_wzrd_resume(struct device *dev)
-{
- int ret;
- struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
-
- ret = clk_prepare_enable(clk_wzrd->axi_clk);
- if (ret) {
- dev_err(dev, "unable to enable s_axi_aclk\n");
- return ret;
- }
-
- clk_wzrd->suspended = false;
-
- return 0;
-}
-
-static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops, clk_wzrd_suspend,
- clk_wzrd_resume);
-
-static int clk_wzrd_probe(struct platform_device *pdev)
-{
- int i, ret;
- u32 reg;
- unsigned long rate;
- const char *clk_name;
- struct clk_wzrd *clk_wzrd;
- struct device_node *np = pdev->dev.of_node;
-
- clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
- if (!clk_wzrd)
- return -ENOMEM;
- platform_set_drvdata(pdev, clk_wzrd);
-
- clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(clk_wzrd->base))
- return PTR_ERR(clk_wzrd->base);
-
- ret = of_property_read_u32(np, "speed-grade", &clk_wzrd->speed_grade);
- if (!ret) {
- if (clk_wzrd->speed_grade < 1 || clk_wzrd->speed_grade > 3) {
- dev_warn(&pdev->dev, "invalid speed grade '%d'\n",
- clk_wzrd->speed_grade);
- clk_wzrd->speed_grade = 0;
- }
- }
-
- clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
- if (IS_ERR(clk_wzrd->clk_in1)) {
- if (clk_wzrd->clk_in1 != ERR_PTR(-EPROBE_DEFER))
- dev_err(&pdev->dev, "clk_in1 not found\n");
- return PTR_ERR(clk_wzrd->clk_in1);
- }
-
- clk_wzrd->axi_clk = devm_clk_get(&pdev->dev, "s_axi_aclk");
- if (IS_ERR(clk_wzrd->axi_clk)) {
- if (clk_wzrd->axi_clk != ERR_PTR(-EPROBE_DEFER))
- dev_err(&pdev->dev, "s_axi_aclk not found\n");
- return PTR_ERR(clk_wzrd->axi_clk);
- }
- ret = clk_prepare_enable(clk_wzrd->axi_clk);
- if (ret) {
- dev_err(&pdev->dev, "enabling s_axi_aclk failed\n");
- return ret;
- }
- rate = clk_get_rate(clk_wzrd->axi_clk);
- if (rate > WZRD_ACLK_MAX_FREQ) {
- dev_err(&pdev->dev, "s_axi_aclk frequency (%lu) too high\n",
- rate);
- ret = -EINVAL;
- goto err_disable_clk;
- }
-
- /* we don't support fractional div/mul yet */
- reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
- WZRD_CLKFBOUT_FRAC_EN;
- reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) &
- WZRD_CLKOUT0_FRAC_EN;
- if (reg)
- dev_warn(&pdev->dev, "fractional div/mul not supported\n");
-
- /* register multiplier */
- reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
- WZRD_CLKFBOUT_MULT_MASK) >> WZRD_CLKFBOUT_MULT_SHIFT;
- clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
- if (!clk_name) {
- ret = -ENOMEM;
- goto err_disable_clk;
- }
- clk_wzrd->clks_internal[wzrd_clk_mul] = clk_register_fixed_factor
- (&pdev->dev, clk_name,
- __clk_get_name(clk_wzrd->clk_in1),
- 0, reg, 1);
- kfree(clk_name);
- if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul])) {
- dev_err(&pdev->dev, "unable to register fixed-factor clock\n");
- ret = PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul]);
- goto err_disable_clk;
- }
-
- /* register div */
- reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
- WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
- clk_name = kasprintf(GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
- if (!clk_name) {
- ret = -ENOMEM;
- goto err_rm_int_clk;
- }
-
- clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_fixed_factor
- (&pdev->dev, clk_name,
- __clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
- 0, 1, reg);
- if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div])) {
- dev_err(&pdev->dev, "unable to register divider clock\n");
- ret = PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div]);
- goto err_rm_int_clk;
- }
-
- /* register div per output */
- for (i = WZRD_NUM_OUTPUTS - 1; i >= 0 ; i--) {
- const char *clkout_name;
-
- if (of_property_read_string_index(np, "clock-output-names", i,
- &clkout_name)) {
- dev_err(&pdev->dev,
- "clock output name not specified\n");
- ret = -EINVAL;
- goto err_rm_int_clks;
- }
- reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2) + i * 12);
- reg &= WZRD_CLKOUT_DIVIDE_MASK;
- reg >>= WZRD_CLKOUT_DIVIDE_SHIFT;
- clk_wzrd->clkout[i] = clk_register_fixed_factor
- (&pdev->dev, clkout_name, clk_name, 0, 1, reg);
- if (IS_ERR(clk_wzrd->clkout[i])) {
- int j;
-
- for (j = i + 1; j < WZRD_NUM_OUTPUTS; j++)
- clk_unregister(clk_wzrd->clkout[j]);
- dev_err(&pdev->dev,
- "unable to register divider clock\n");
- ret = PTR_ERR(clk_wzrd->clkout[i]);
- goto err_rm_int_clks;
- }
- }
-
- kfree(clk_name);
-
- clk_wzrd->clk_data.clks = clk_wzrd->clkout;
- clk_wzrd->clk_data.clk_num = ARRAY_SIZE(clk_wzrd->clkout);
- of_clk_add_provider(np, of_clk_src_onecell_get, &clk_wzrd->clk_data);
-
- if (clk_wzrd->speed_grade) {
- clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
-
- ret = clk_notifier_register(clk_wzrd->clk_in1,
- &clk_wzrd->nb);
- if (ret)
- dev_warn(&pdev->dev,
- "unable to register clock notifier\n");
-
- ret = clk_notifier_register(clk_wzrd->axi_clk, &clk_wzrd->nb);
- if (ret)
- dev_warn(&pdev->dev,
- "unable to register clock notifier\n");
- }
-
- return 0;
-
-err_rm_int_clks:
- clk_unregister(clk_wzrd->clks_internal[1]);
-err_rm_int_clk:
- kfree(clk_name);
- clk_unregister(clk_wzrd->clks_internal[0]);
-err_disable_clk:
- clk_disable_unprepare(clk_wzrd->axi_clk);
-
- return ret;
-}
-
-static int clk_wzrd_remove(struct platform_device *pdev)
-{
- int i;
- struct clk_wzrd *clk_wzrd = platform_get_drvdata(pdev);
-
- of_clk_del_provider(pdev->dev.of_node);
-
- for (i = 0; i < WZRD_NUM_OUTPUTS; i++)
- clk_unregister(clk_wzrd->clkout[i]);
- for (i = 0; i < wzrd_clk_int_max; i++)
- clk_unregister(clk_wzrd->clks_internal[i]);
-
- if (clk_wzrd->speed_grade) {
- clk_notifier_unregister(clk_wzrd->axi_clk, &clk_wzrd->nb);
- clk_notifier_unregister(clk_wzrd->clk_in1, &clk_wzrd->nb);
- }
-
- clk_disable_unprepare(clk_wzrd->axi_clk);
-
- return 0;
-}
-
-static const struct of_device_id clk_wzrd_ids[] = {
- { .compatible = "xlnx,clocking-wizard" },
- { },
-};
-MODULE_DEVICE_TABLE(of, clk_wzrd_ids);
-
-static struct platform_driver clk_wzrd_driver = {
- .driver = {
- .name = "clk-wizard",
- .of_match_table = clk_wzrd_ids,
- .pm = &clk_wzrd_dev_pm_ops,
- },
- .probe = clk_wzrd_probe,
- .remove = clk_wzrd_remove,
-};
-module_platform_driver(clk_wzrd_driver);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Soeren Brinkmann <soren.brinkmann@xilinx.com");
-MODULE_DESCRIPTION("Driver for the Xilinx Clocking Wizard IP core");
diff --git a/drivers/staging/clocking-wizard/dt-binding.txt b/drivers/staging/clocking-wizard/dt-binding.txt
deleted file mode 100644
index efb67ff..0000000
--- a/drivers/staging/clocking-wizard/dt-binding.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Binding for Xilinx Clocking Wizard IP Core
-
-This binding uses the common clock binding[1]. Details about the devices can be
-found in the product guide[2].
-
-[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
-[2] Clocking Wizard Product Guide
-https://www.xilinx.com/support/documentation/ip_documentation/clk_wiz/v5_1/pg065-clk-wiz.pdf
-
-Required properties:
- - compatible: Must be 'xlnx,clocking-wizard'
- - reg: Base and size of the cores register space
- - clocks: Handle to input clock
- - clock-names: Tuple containing 'clk_in1' and 's_axi_aclk'
- - clock-output-names: Names for the output clocks
-
-Optional properties:
- - speed-grade: Speed grade of the device (valid values are 1..3)
-
-Example:
- clock-generator@40040000 {
- reg = <0x40040000 0x1000>;
- compatible = "xlnx,clocking-wizard";
- speed-grade = <1>;
- clock-names = "clk_in1", "s_axi_aclk";
- clocks = <&clkc 15>, <&clkc 15>;
- clock-output-names = "clk_out0", "clk_out1", "clk_out2",
- "clk_out3", "clk_out4", "clk_out5",
- "clk_out6", "clk_out7";
- };
--
2.1.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v6 1/8] dt-bindings: add documentation of xilinx clocking wizard
2020-08-28 13:39 ` [PATCH v6 1/8] dt-bindings: add documentation of xilinx clocking wizard Shubhrajyoti Datta
@ 2020-08-28 21:56 ` Rob Herring
0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2020-08-28 21:56 UTC (permalink / raw)
To: Shubhrajyoti Datta
Cc: linux-clk, devicetree, linux-kernel, devel, sboyd, gregkh, mturquette
On Fri, Aug 28, 2020 at 07:09:49PM +0530, Shubhrajyoti Datta wrote:
> Add the devicetree binding for the xilinx clocking wizard.
>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v6:
> Fix a yaml warning
>
> .../bindings/clock/xlnx,clocking-wizard.yaml | 71 ++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> new file mode 100644
> index 0000000..ca63593
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx clocking wizard
> +
> +maintainers:
> + - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> +
> +description: |
> + The clocking wizard is a soft ip clocking block of Xilinx versal. It
> + reads required input clock frequencies from the devicetree and acts as clock
> + clock output.
> +
> +select: false
> +
> +properties:
> + compatible:
> + items:
> + - enum:
You can drop 'items' since there is only 1.
> + - xlnx,clocking-wizard
> + - xlnx,clocking-wizard-6.0
> +
> + "#clock-cells":
> + const: 1
> +
> + clocks:
> + description: List of clock specifiers which are external input
> + clocks to the given clock controller.
Drop this. That's every 'clocks' property.
> + items:
> + - description: clock input
> + - description: axi clock
> +
> + clock-names:
> + items:
> + - const: clk_in1
> + - const: s_axi_aclk
> +
> + speed-grade:
Needs a vendor prefix.
> + allOf:
You can remove the 'allOf'.
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - enum: [1, 2, 3]
> + description:
> + Speed grade of the device.
> + maxItems: 1
Drop this. A 'uint32' is always 1 item.
> +
> +required:
> + - compatible
> + - "#clock-cells"
> + - clocks
> + - clock-names
> + - speed-grade
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + clock-generator@40040000 {
> + #clock-cells = <1>;
> + reg = <0x40040000 0x1000>;
> + compatible = "xlnx,clk-wizard-1.0";
Doesn't match the schema.
> + speed-grade = <1>;
> + clock-names = "clk_in1", "s_axi_aclk";
> + clocks = <&clkc 15>, <&clkc 15>;
> + clock-output-names = "clk_out1", "clk_out2",
> + "clk_out3", "clk_out4", "clk_out5",
> + "clk_out6", "clk_out7";
> + };
> +...
> --
> 2.1.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support
2020-08-28 13:39 ` [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support Shubhrajyoti Datta
@ 2020-09-21 21:17 ` Stephen Boyd
2020-09-24 6:23 ` Shubhrajyoti Datta
2020-09-22 9:17 ` Dan Carpenter
1 sibling, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2020-09-21 21:17 UTC (permalink / raw)
To: Shubhrajyoti Datta, linux-clk
Cc: devicetree, linux-kernel, devel, robh+dt, gregkh, mturquette,
Shubhrajyoti Datta
Quoting Shubhrajyoti Datta (2020-08-28 06:39:53)
> Currently the set rate granularity is to integral divisors.
> Add support for the fractional divisors.
> Only the first output0 is fractional in the hardware.
>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Getting closer.
> diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> index 8dfcec8..1af59a4 100644
> --- a/drivers/clk/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> @@ -185,6 +191,134 @@ static const struct clk_ops clk_wzrd_clk_divider_ops = {
> .recalc_rate = clk_wzrd_recalc_rate,
> };
>
> +static unsigned long clk_wzrd_recalc_ratef(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + unsigned int val;
> + u32 div, frac;
> + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
> + void __iomem *div_addr = divider->base + divider->offset;
> +
> + val = readl(div_addr);
> + div = val & div_mask(divider->width);
> + frac = (val >> WZRD_CLKOUT_FRAC_SHIFT) & WZRD_CLKOUT_FRAC_MASK;
> +
> + return ((parent_rate * 1000) / ((div * 1000) + frac));
Please remove extra parenthesis. And is this mult_frac()?
> +}
> +
> +static int clk_wzrd_dynamic_reconfig_f(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + int err;
> + u32 value, pre;
> + unsigned long rate_div, f, clockout0_div;
> + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
> + void __iomem *div_addr = divider->base + divider->offset;
> +
> + rate_div = ((parent_rate * 1000) / rate);
> + clockout0_div = rate_div / 1000;
> +
> + pre = DIV_ROUND_CLOSEST((parent_rate * 1000), rate);
> + f = (u32)(pre - (clockout0_div * 1000));
> + f = f & WZRD_CLKOUT_FRAC_MASK;
> +
> + value = ((f << WZRD_CLKOUT_DIVIDE_WIDTH) | (clockout0_div &
> + WZRD_CLKOUT_DIVIDE_MASK));
Please split this to multiple lines.
> +
> + /* Set divisor and clear phase offset */
> + writel(value, div_addr);
> + writel(0x0, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET);
> +
> + /* Check status register */
> + err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
> + value & WZRD_DR_LOCK_BIT_MASK,
> + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
> + if (err)
> + return err;
> +
> + /* Initiate reconfiguration */
> + writel(WZRD_DR_BEGIN_DYNA_RECONF,
> + divider->base + WZRD_DR_INIT_REG_OFFSET);
> +
> + /* Check status register */
> + err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
> + value & WZRD_DR_LOCK_BIT_MASK,
> + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
> +
> + return err;
Just return readl_poll_timeout() please.
> +}
> +
> +static long clk_wzrd_round_rate_f(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + return rate;
Can every rate be supported? This function is supposed to tell the clk
framework what rate will be achieved if we call clk_set_rate() with
'rate' passed to this function. Almost always returning 'rate' is not
the case.
>
> +
> +static const struct clk_ops clk_wzrd_clk_divider_ops_f = {
> + .round_rate = clk_wzrd_round_rate_f,
> + .set_rate = clk_wzrd_dynamic_reconfig_f,
> + .recalc_rate = clk_wzrd_recalc_ratef,
> +};
> +
> +static struct clk *clk_wzrd_register_divf(struct device *dev,
> + const char *name,
> + const char *parent_name,
> + unsigned long flags,
> + void __iomem *base, u16 offset,
> + u8 shift, u8 width,
> + u8 clk_divider_flags,
> + const struct clk_div_table *table,
> + spinlock_t *lock)
> +{
> + struct clk_wzrd_divider *div;
> + struct clk_hw *hw;
> + struct clk_init_data init;
> + int ret;
> +
> + if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
Is this used? It's a rockchip specific flag mostly so probably not?
> + if (width + shift > 16) {
> + pr_warn("divider value exceeds LOWORD field\n");
> + return ERR_PTR(-EINVAL);
> + }
> + }
> +
> + /* allocate the divider */
Please remove useless comments like this.
> + div = kzalloc(sizeof(*div), GFP_KERNEL);
> + if (!div)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> +
> + if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
Is this flag used?
> + init.ops = &clk_divider_ro_ops;
> + else
> + init.ops = &clk_wzrd_clk_divider_ops_f;
> +
> + init.flags = flags;
> + init.parent_names = (parent_name ? &parent_name : NULL);
> + init.num_parents = (parent_name ? 1 : 0);
Do you have cases where there isn't a parent? Hopefully not, so this
can be simplified.
>
> + /* struct clk_divider assignments */
Drop this comment?
> + div->base = base;
> + div->offset = offset;
> + div->shift = shift;
> + div->width = width;
> + div->flags = clk_divider_flags;
> + div->lock = lock;
> + div->hw.init = &init;
> + div->table = table;
> +
> + /* register the clock */
Drop this comment?
> + hw = &div->hw;
> + ret = clk_hw_register(dev, hw);
Any reason we can't use devm_clk_hw_register() here?
> + if (ret) {
> + kfree(div);
> + return ERR_PTR(ret);
> + }
> +
> + return hw->clk;
> +}
> +
> static struct clk *clk_wzrd_register_divider(struct device *dev,
> const char *name,
> const char *parent_name,
> @@ -355,17 +489,13 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> goto err_disable_clk;
> }
>
> - /* we don't support fractional div/mul yet */
> - reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
> - WZRD_CLKFBOUT_FRAC_EN;
> - reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) &
> - WZRD_CLKOUT0_FRAC_EN;
> - if (reg)
> - dev_warn(&pdev->dev, "fractional div/mul not supported\n");
> -
> /* register multiplier */
> reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
> WZRD_CLKFBOUT_MULT_MASK) >> WZRD_CLKFBOUT_MULT_SHIFT;
> + reg_f = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
> + WZRD_CLKFBOUT_FRAC_MASK) >> WZRD_CLKFBOUT_FRAC_SHIFT;
Use two lines please. Read into variable on one line, shift on another.
> +
> + mult = ((reg * 1000) + reg_f);
Please remove extra parenthesis.
> clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
> if (!clk_name) {
> ret = -ENOMEM;
> @@ -413,8 +543,18 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> ret = -EINVAL;
> goto err_rm_int_clks;
> }
> - clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev,
> - clkout_name,
> + if (!i)
> + clk_wzrd->clkout[i] = clk_wzrd_register_divf
> + (&pdev->dev, clkout_name,
> + clk_name, 0,
> + clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
> + WZRD_CLKOUT_DIVIDE_SHIFT,
> + WZRD_CLKOUT_DIVIDE_WIDTH,
> + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
> + NULL, &clkwzrd_lock);
> + else
> + clk_wzrd->clkout[i] = clk_wzrd_register_divider
> + (&pdev->dev, clkout_name,
> clk_name, 0,
> clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
> WZRD_CLKOUT_DIVIDE_SHIFT,
>
I wonder if a do-while loop with flags set to ONE_BASED and ALLOW_ZERO
could work and then flags gets overwritten to be just DIVIDE_SHIFT? Then
we don't have to duplicate the registration line.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk directory
2020-08-28 13:39 ` [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk directory Shubhrajyoti Datta
@ 2020-09-21 21:19 ` Stephen Boyd
2020-09-21 21:21 ` Stephen Boyd
0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2020-09-21 21:19 UTC (permalink / raw)
To: Shubhrajyoti Datta, linux-clk
Cc: devicetree, linux-kernel, devel, robh+dt, gregkh, mturquette,
Shubhrajyoti Datta
Quoting Shubhrajyoti Datta (2020-08-28 06:39:50)
> Add clocking wizard driver to clk.
>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
Can this be combined with patch #6? And then use git format-patch -C -M
when generating patches? I'm trying to see if anything changed from the
file in the staging tree or if this is just a copy from staging to clk
directory. Would also be useful if that was stated in the commit text.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk directory
2020-09-21 21:19 ` Stephen Boyd
@ 2020-09-21 21:21 ` Stephen Boyd
2020-11-04 4:07 ` Shubhrajyoti Datta
0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2020-09-21 21:21 UTC (permalink / raw)
To: Shubhrajyoti Datta, linux-clk
Cc: devicetree, linux-kernel, devel, robh+dt, gregkh, mturquette,
Shubhrajyoti Datta
Quoting Stephen Boyd (2020-09-21 14:19:59)
> Quoting Shubhrajyoti Datta (2020-08-28 06:39:50)
> > Add clocking wizard driver to clk.
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > ---
>
> Can this be combined with patch #6?
Sorry, I meant patch #8.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/8] clk: clock-wizard: Add support for dynamic reconfiguration
2020-08-28 13:39 ` [PATCH v6 4/8] clk: clock-wizard: Add support for dynamic reconfiguration Shubhrajyoti Datta
@ 2020-09-21 21:26 ` Stephen Boyd
0 siblings, 0 replies; 17+ messages in thread
From: Stephen Boyd @ 2020-09-21 21:26 UTC (permalink / raw)
To: Shubhrajyoti Datta, linux-clk
Cc: devicetree, linux-kernel, devel, robh+dt, gregkh, mturquette,
Shubhrajyoti Datta, Chirag Parekh
Quoting Shubhrajyoti Datta (2020-08-28 06:39:52)
> The patch adds support for dynamic reconfiguration of clock output rate.
> Output clocks are registered as dividers and set rate callback function
> is used for dynamic reconfiguration.
>
> Based on the initial work from Chirag.
>
> Signed-off-by: Chirag Parekh <chirag.parekh@xilinx.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
A lot of the same comments apply here.
> v6:
> Remove the typecast.
> use min for capping frequency.
> use polled timeout
>
> drivers/clk/clk-xlnx-clock-wizard.c | 185 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 179 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> index d6577c8..8dfcec8 100644
> --- a/drivers/clk/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> @@ -31,8 +32,23 @@
> #define WZRD_DIVCLK_DIVIDE_SHIFT 0
> #define WZRD_DIVCLK_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
> #define WZRD_CLKOUT_DIVIDE_SHIFT 0
> +#define WZRD_CLKOUT_DIVIDE_WIDTH 8
> #define WZRD_CLKOUT_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
>
> +#define WZRD_DR_MAX_INT_DIV_VALUE 255
> +#define WZRD_DR_NUM_RETRIES 10000
> +#define WZRD_DR_STATUS_REG_OFFSET 0x04
> +#define WZRD_DR_LOCK_BIT_MASK 0x00000001
> +#define WZRD_DR_INIT_REG_OFFSET 0x25C
> +#define WZRD_DR_DIV_TO_PHASE_OFFSET 4
> +#define WZRD_DR_BEGIN_DYNA_RECONF 0x03
> +
> +/* Get the mask from width */
> +#define div_mask(width) ((1 << (width)) - 1)
> +
> +/* Extract divider instance from clock hardware instance */
> +#define to_clk_wzrd_divider(_hw) container_of(_hw, struct clk_wzrd_divider, hw)
> +
> enum clk_wzrd_int_clks {
> wzrd_clk_mul,
> wzrd_clk_mul_div,
> @@ -73,6 +112,136 @@ static const unsigned long clk_wzrd_max_freq[] = {
> 1066000000UL
> };
>
> +/* spin lock variable for clk_wzrd */
> +static DEFINE_SPINLOCK(clkwzrd_lock);
What is it protecting?
> +
> +static unsigned long clk_wzrd_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
> + void __iomem *div_addr = divider->base + divider->offset;
> + unsigned int val;
> +
> + val = readl(div_addr) >> divider->shift;
> + val &= div_mask(divider->width);
> +
> + return divider_recalc_rate(hw, parent_rate, val, divider->table,
> + divider->flags, divider->width);
> +}
> +
> +static int clk_wzrd_dynamic_reconfig(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + int err = 0;
> + u32 value;
> + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
> + void __iomem *div_addr = divider->base + divider->offset;
Shouldn't the lock be held here?
> +
> + value = DIV_ROUND_CLOSEST(parent_rate, rate);
> +
> + /* Cap the value to max */
> + min(value, (u32)WZRD_DR_MAX_INT_DIV_VALUE);
> +
> + /* Set divisor and clear phase offset */
> + writel(value, div_addr);
> + writel(0x00, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET);
> +
> + /* Check status register */
> + err = readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
> + value & WZRD_DR_LOCK_BIT_MASK,
> + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
> + if (err)
> + return err;
> +
> + /* Initiate reconfiguration */
> + writel(WZRD_DR_BEGIN_DYNA_RECONF,
> + divider->base + WZRD_DR_INIT_REG_OFFSET);
> +
> + /* Check status register */
> + err = readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
> + value & WZRD_DR_LOCK_BIT_MASK,
> + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
> +
> + return err;
return readl_poll_timeout().
> +}
> +
> +static long clk_wzrd_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + u8 div;
> +
> + /*
> + * since we don't change parent rate we just round rate to closest
> + * achievable
> + */
> + div = DIV_ROUND_CLOSEST(*prate, rate);
> +
> + return (*prate / div);
Drop useless parens please.
> +}
> +
> +static const struct clk_ops clk_wzrd_clk_divider_ops = {
> + .round_rate = clk_wzrd_round_rate,
> + .set_rate = clk_wzrd_dynamic_reconfig,
> + .recalc_rate = clk_wzrd_recalc_rate,
> +};
> +
> +static struct clk *clk_wzrd_register_divider(struct device *dev,
> + const char *name,
> + const char *parent_name,
> + unsigned long flags,
> + void __iomem *base, u16 offset,
> + u8 shift, u8 width,
> + u8 clk_divider_flags,
> + const struct clk_div_table *table,
> + spinlock_t *lock)
> +{
> + struct clk_wzrd_divider *div;
> + struct clk_hw *hw;
> + struct clk_init_data init;
> + int ret;
> +
> + if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
Is it used?
> + if (width + shift > 16) {
> + pr_warn("divider value exceeds LOWORD field\n");
> + return ERR_PTR(-EINVAL);
> + }
> + }
> +
> + /* allocate the divider */
> + div = kzalloc(sizeof(*div), GFP_KERNEL);
> + if (!div)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
Is this used?
> + init.ops = &clk_divider_ro_ops;
> + else
> + init.ops = &clk_wzrd_clk_divider_ops;
> + init.flags = flags;
> + init.parent_names = (parent_name ? &parent_name : NULL);
> + init.num_parents = (parent_name ? 1 : 0);
Doesn't it always have a parent?
> +
> + /* struct clk_divider assignments */
Drop useless comments please.
> + div->base = base;
> + div->offset = offset;
> + div->shift = shift;
> + div->width = width;
> + div->flags = clk_divider_flags;
> + div->lock = lock;
> + div->hw.init = &init;
> + div->table = table;
> +
> + /* register the clock */
> + hw = &div->hw;
> + ret = clk_hw_register(dev, hw);
devm_clk_hw_register()?
> + if (ret) {
> + kfree(div);
Why not a devm_kzalloc() and then drop this?
> + hw = ERR_PTR(ret);
> + }
> +
> + return hw->clk;
> +}
> +
> static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
> void *data)
> {
> @@ -243,11 +413,14 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> ret = -EINVAL;
> goto err_rm_int_clks;
> }
> - reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2) + i * 12);
> - reg &= WZRD_CLKOUT_DIVIDE_MASK;
> - reg >>= WZRD_CLKOUT_DIVIDE_SHIFT;
> - clk_wzrd->clkout[i] = clk_register_fixed_factor
> - (&pdev->dev, clkout_name, clk_name, 0, 1, reg);
> + clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev,
> + clkout_name,
> + clk_name, 0,
> + clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
> + WZRD_CLKOUT_DIVIDE_SHIFT,
> + WZRD_CLKOUT_DIVIDE_WIDTH,
> + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
> + NULL, &clkwzrd_lock);
Also wonder if we could have a clk_wzrd_register_divider() API that
knows most of these things and just takes a number indicating which clk
it is? Then the caller isn't a bunch of lines of code that has to be
mentally carried to the callee.
> if (IS_ERR(clk_wzrd->clkout[i])) {
> int j;
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support
2020-08-28 13:39 ` [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support Shubhrajyoti Datta
2020-09-21 21:17 ` Stephen Boyd
@ 2020-09-22 9:17 ` Dan Carpenter
1 sibling, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2020-09-22 9:17 UTC (permalink / raw)
To: Shubhrajyoti Datta
Cc: linux-clk, devel, devicetree, sboyd, gregkh, mturquette,
linux-kernel, robh+dt
On Fri, Aug 28, 2020 at 07:09:53PM +0530, Shubhrajyoti Datta wrote:
> +
> + /* Check status register */
> + err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
> + value & WZRD_DR_LOCK_BIT_MASK,
> + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
Checkpatch will catch the missing space in "err= readl_".
regards,
dan carpenter
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support
2020-09-21 21:17 ` Stephen Boyd
@ 2020-09-24 6:23 ` Shubhrajyoti Datta
0 siblings, 0 replies; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-09-24 6:23 UTC (permalink / raw)
To: Stephen Boyd, linux-clk
Cc: devicetree, linux-kernel, devel, robh+dt, gregkh, mturquette
Hi ,
Thanks for the review.
> -----Original Message-----
> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Tuesday, September 22, 2020 2:48 AM
> To: Shubhrajyoti Datta <shubhraj@xilinx.com>; linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@driverdev.osuosl.org; robh+dt@kernel.org;
> gregkh@linuxfoundation.org; mturquette@baylibre.com; Shubhrajyoti
> Datta <shubhraj@xilinx.com>
> Subject: Re: [PATCH v6 5/8] clk: clock-wizard: Add support for fractional
> support
>
> Quoting Shubhrajyoti Datta (2020-08-28 06:39:53)
> > Currently the set rate granularity is to integral divisors.
> > Add support for the fractional divisors.
> > Only the first output0 is fractional in the hardware.
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>
> Getting closer.
>
> > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c
> > b/drivers/clk/clk-xlnx-clock-wizard.c
> > index 8dfcec8..1af59a4 100644
> > --- a/drivers/clk/clk-xlnx-clock-wizard.c
> > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> > @@ -185,6 +191,134 @@ static const struct clk_ops
> clk_wzrd_clk_divider_ops = {
> > .recalc_rate = clk_wzrd_recalc_rate, };
> >
> > +static unsigned long clk_wzrd_recalc_ratef(struct clk_hw *hw,
> > + unsigned long parent_rate)
> > +{
> > + unsigned int val;
> > + u32 div, frac;
> > + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
> > + void __iomem *div_addr = divider->base + divider->offset;
> > +
> > + val = readl(div_addr);
> > + div = val & div_mask(divider->width);
> > + frac = (val >> WZRD_CLKOUT_FRAC_SHIFT) &
> > + WZRD_CLKOUT_FRAC_MASK;
> > +
> > + return ((parent_rate * 1000) / ((div * 1000) + frac));
>
> Please remove extra parenthesis. And is this mult_frac()?
>
Will fix
> > +}
> > +
> > +static int clk_wzrd_dynamic_reconfig_f(struct clk_hw *hw, unsigned long
> rate,
> > + unsigned long parent_rate) {
> > + int err;
> > + u32 value, pre;
> > + unsigned long rate_div, f, clockout0_div;
> > + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
> > + void __iomem *div_addr = divider->base + divider->offset;
> > +
> > + rate_div = ((parent_rate * 1000) / rate);
> > + clockout0_div = rate_div / 1000;
> > +
> > + pre = DIV_ROUND_CLOSEST((parent_rate * 1000), rate);
> > + f = (u32)(pre - (clockout0_div * 1000));
> > + f = f & WZRD_CLKOUT_FRAC_MASK;
> > +
> > + value = ((f << WZRD_CLKOUT_DIVIDE_WIDTH) | (clockout0_div &
> > + WZRD_CLKOUT_DIVIDE_MASK));
>
> Please split this to multiple lines.
Will fix
>
> > +
> > + /* Set divisor and clear phase offset */
> > + writel(value, div_addr);
> > + writel(0x0, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET);
> > +
> > + /* Check status register */
> > + err= readl_poll_timeout(divider->base +
> WZRD_DR_STATUS_REG_OFFSET, value,
> > + value & WZRD_DR_LOCK_BIT_MASK,
> > + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
> > + if (err)
> > + return err;
> > +
> > + /* Initiate reconfiguration */
> > + writel(WZRD_DR_BEGIN_DYNA_RECONF,
> > + divider->base + WZRD_DR_INIT_REG_OFFSET);
> > +
> > + /* Check status register */
> > + err= readl_poll_timeout(divider->base +
> WZRD_DR_STATUS_REG_OFFSET, value,
> > + value & WZRD_DR_LOCK_BIT_MASK,
> > + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
> > +
> > + return err;
>
> Just return readl_poll_timeout() please.
Will fix
>
> > +}
> > +
> > +static long clk_wzrd_round_rate_f(struct clk_hw *hw, unsigned long
> rate,
> > + unsigned long *prate) {
> > + return rate;
>
> Can every rate be supported? This function is supposed to tell the clk
> framework what rate will be achieved if we call clk_set_rate() with 'rate'
> passed to this function. Almost always returning 'rate' is not the case.
>
We can support rate upto 3 decimal places to prevent truncation here we are
Returning rate.
> >
> > +
> > +static const struct clk_ops clk_wzrd_clk_divider_ops_f = {
> > + .round_rate = clk_wzrd_round_rate_f,
> > + .set_rate = clk_wzrd_dynamic_reconfig_f,
> > + .recalc_rate = clk_wzrd_recalc_ratef, };
> > +
> > +static struct clk *clk_wzrd_register_divf(struct device *dev,
> > + const char *name,
> > + const char *parent_name,
> > + unsigned long flags,
> > + void __iomem *base, u16 offset,
> > + u8 shift, u8 width,
> > + u8 clk_divider_flags,
> > + const struct clk_div_table *table,
> > + spinlock_t *lock) {
> > + struct clk_wzrd_divider *div;
> > + struct clk_hw *hw;
> > + struct clk_init_data init;
> > + int ret;
> > +
> > + if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
>
> Is this used? It's a rockchip specific flag mostly so probably not?
>
> > + if (width + shift > 16) {
> > + pr_warn("divider value exceeds LOWORD field\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > + }
> > +
> > + /* allocate the divider */
>
> Please remove useless comments like this.
Will fix
>
> > + div = kzalloc(sizeof(*div), GFP_KERNEL);
> > + if (!div)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + init.name = name;
> > +
> > + if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
>
> Is this flag used?
Will fix
>
> > + init.ops = &clk_divider_ro_ops;
> > + else
> > + init.ops = &clk_wzrd_clk_divider_ops_f;
> > +
> > + init.flags = flags;
> > + init.parent_names = (parent_name ? &parent_name : NULL);
> > + init.num_parents = (parent_name ? 1 : 0);
>
> Do you have cases where there isn't a parent? Hopefully not, so this can be
> simplified.
>
Will fix
> >
> > + /* struct clk_divider assignments */
>
> Drop this comment?
Will fix
>
> > + div->base = base;
> > + div->offset = offset;
> > + div->shift = shift;
> > + div->width = width;
> > + div->flags = clk_divider_flags;
> > + div->lock = lock;
> > + div->hw.init = &init;
> > + div->table = table;
> > +
> > + /* register the clock */
>
> Drop this comment?
Will fix
>
> > + hw = &div->hw;
> > + ret = clk_hw_register(dev, hw);
>
> Any reason we can't use devm_clk_hw_register() here?
>
Will do
> > + if (ret) {
> > + kfree(div);
> > + return ERR_PTR(ret);
> > + }
> > +
> > + return hw->clk;
> > +}
> > +
> > static struct clk *clk_wzrd_register_divider(struct device *dev,
> > const char *name,
> > const char *parent_name,
> > @@ -355,17 +489,13 @@ static int clk_wzrd_probe(struct
> platform_device *pdev)
> > goto err_disable_clk;
> > }
> >
> > - /* we don't support fractional div/mul yet */
> > - reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
> > - WZRD_CLKFBOUT_FRAC_EN;
> > - reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) &
> > - WZRD_CLKOUT0_FRAC_EN;
> > - if (reg)
> > - dev_warn(&pdev->dev, "fractional div/mul not supported\n");
> > -
> > /* register multiplier */
> > reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
> > WZRD_CLKFBOUT_MULT_MASK) >>
> > WZRD_CLKFBOUT_MULT_SHIFT;
> > + reg_f = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
> > + WZRD_CLKFBOUT_FRAC_MASK) >>
> > + WZRD_CLKFBOUT_FRAC_SHIFT;
>
> Use two lines please. Read into variable on one line, shift on another.
>
Will fix
> > +
> > + mult = ((reg * 1000) + reg_f);
>
> Please remove extra parenthesis.
>
> > clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev-
> >dev));
> > if (!clk_name) {
> > ret = -ENOMEM;
> > @@ -413,8 +543,18 @@ static int clk_wzrd_probe(struct platform_device
> *pdev)
> > ret = -EINVAL;
> > goto err_rm_int_clks;
> > }
> > - clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev,
> > - clkout_name,
> > + if (!i)
> > + clk_wzrd->clkout[i] = clk_wzrd_register_divf
> > + (&pdev->dev, clkout_name,
> > + clk_name, 0,
> > + clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
> > + WZRD_CLKOUT_DIVIDE_SHIFT,
> > + WZRD_CLKOUT_DIVIDE_WIDTH,
> > + CLK_DIVIDER_ONE_BASED |
> CLK_DIVIDER_ALLOW_ZERO,
> > + NULL, &clkwzrd_lock);
> > + else
> > + clk_wzrd->clkout[i] = clk_wzrd_register_divider
> > + (&pdev->dev, clkout_name,
> > clk_name, 0,
> > clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
> > WZRD_CLKOUT_DIVIDE_SHIFT,
> >
>
> I wonder if a do-while loop with flags set to ONE_BASED and ALLOW_ZERO
> could work and then flags gets overwritten to be just DIVIDE_SHIFT? Then
> we don't have to duplicate the registration line.
I did not understand this comment in one case I am registering for the fractional operations
In another we are using the integral operations
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk directory
2020-09-21 21:21 ` Stephen Boyd
@ 2020-11-04 4:07 ` Shubhrajyoti Datta
0 siblings, 0 replies; 17+ messages in thread
From: Shubhrajyoti Datta @ 2020-11-04 4:07 UTC (permalink / raw)
To: Stephen Boyd, linux-clk
Cc: devicetree, linux-kernel, devel, robh+dt, gregkh, mturquette
Hi Stephen,
Thanks for the review.
> -----Original Message-----
> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Tuesday, September 22, 2020 2:52 AM
> To: Shubhrajyoti Datta <shubhraj@xilinx.com>; linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@driverdev.osuosl.org; robh+dt@kernel.org;
> gregkh@linuxfoundation.org; mturquette@baylibre.com; Shubhrajyoti
> Datta <shubhraj@xilinx.com>
> Subject: Re: [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk
> directory
>
> Quoting Stephen Boyd (2020-09-21 14:19:59)
> > Quoting Shubhrajyoti Datta (2020-08-28 06:39:50)
> > > Add clocking wizard driver to clk.
> > >
> > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > > ---
> >
> > Can this be combined with patch #6?
>
> Sorry, I meant patch #8.
Did not get the comment.
Do you want to split the makefile and the driver parts of the patch.
Or the whole of the patch to be merged.
Or do you want the staging patch merged with here?
[PATCH v6 8/8] staging: clocking-wizard: Delete the driver from the staging
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-11-04 4:07 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-28 13:39 [PATCH v6 0/8] clk: clk-wizard: clock-wizard: Driver updates Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 1/8] dt-bindings: add documentation of xilinx clocking wizard Shubhrajyoti Datta
2020-08-28 21:56 ` Rob Herring
2020-08-28 13:39 ` [PATCH v6 2/8] clk: clock-wizard: Add the clockwizard to clk directory Shubhrajyoti Datta
2020-09-21 21:19 ` Stephen Boyd
2020-09-21 21:21 ` Stephen Boyd
2020-11-04 4:07 ` Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 3/8] clk: clock-wizard: Fix kernel-doc warning Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 4/8] clk: clock-wizard: Add support for dynamic reconfiguration Shubhrajyoti Datta
2020-09-21 21:26 ` Stephen Boyd
2020-08-28 13:39 ` [PATCH v6 5/8] clk: clock-wizard: Add support for fractional support Shubhrajyoti Datta
2020-09-21 21:17 ` Stephen Boyd
2020-09-24 6:23 ` Shubhrajyoti Datta
2020-09-22 9:17 ` Dan Carpenter
2020-08-28 13:39 ` [PATCH v6 6/8] clk: clock-wizard: Remove the hardcoding of the clock outputs Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 7/8] clk: clock-wizard: Update the fixed factor divisors Shubhrajyoti Datta
2020-08-28 13:39 ` [PATCH v6 8/8] staging: clocking-wizard: Delete the driver from the staging Shubhrajyoti Datta
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).