* [PATCH v9 0/2] Add clkcfg driver for Microchip PolarFire SoC
@ 2021-12-16 14:00 conor.dooley
2021-12-16 14:00 ` [PATCH v9 1/2] dt-bindings: clk: microchip: Add Microchip PolarFire host binding conor.dooley
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: conor.dooley @ 2021-12-16 14:00 UTC (permalink / raw)
To: mturquette, sboyd, linux-clk, robh+dt, devicetree
Cc: krzysztof.kozlowski, geert, david.abdurachmanov, palmer,
daire.mcnamara, cyril.jean, conor.dooley
From: Conor Dooley <conor.dooley@microchip.com>
This patchset adds support for the Microchip PolarFire clkcfg
hardware block.
Changes since v8:
* Use devm_clk_hw_unregister directly
* Get parent clk in probe rather than register function
Changes since v7:
* Dropped clock-output-names again (oops)
Changes since v6:
* Dropped clock-output-names *as a required property*
* Dropped if(lock) check on spinlocks, added spinlocks to all
read-modify-write register access
* Removed kfree()s on non-dynamically allocated variables
* Use devm_clk_get to get the reference clock
* Account for reserved clock when calculating the size of num_clks
Changes since v5:
* Dropped clock-output-names property
Major changes since v4:
* Adjusted license for microchip,mpfs-clock.h to match microchip,mpfs.yaml
* Corrected the number of clocks to 33 from 32
Major changes since v3:
* Patch reformatted so microchip,mpfs-clock.h is part of device-tree patch
Major changes since v2:
* In mpfs_cfg_clk_set_rate, return immediately if divider_get_val
returns <0
* rebased to v5.12-rc1
Major changes since v1:
* Dependency on SOC_MICROCHIP_POLARFIRE
* All references to PFSOC/pfsoc changed to MPFS/mpfs
* Cleaned error handling in _probe
* Re-ordered code to place structs et al at top
Daire McNamara (2):
dt-bindings: clk: microchip: Add Microchip PolarFire host binding
clk: microchip: Add driver for Microchip PolarFire SoC
.../bindings/clock/microchip,mpfs.yaml | 58 +++
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 2 +-
drivers/clk/microchip/Kconfig | 7 +
drivers/clk/microchip/Makefile | 6 +-
drivers/clk/microchip/clk-mpfs.c | 439 ++++++++++++++++++
.../dt-bindings/clock/microchip,mpfs-clock.h | 45 ++
7 files changed, 555 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
create mode 100644 drivers/clk/microchip/Kconfig
create mode 100644 drivers/clk/microchip/clk-mpfs.c
create mode 100644 include/dt-bindings/clock/microchip,mpfs-clock.h
--
2.33.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v9 1/2] dt-bindings: clk: microchip: Add Microchip PolarFire host binding
2021-12-16 14:00 [PATCH v9 0/2] Add clkcfg driver for Microchip PolarFire SoC conor.dooley
@ 2021-12-16 14:00 ` conor.dooley
2022-01-25 0:26 ` Stephen Boyd
2021-12-16 14:00 ` [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC conor.dooley
2022-01-10 10:24 ` [PATCH v9 0/2] Add clkcfg " Conor.Dooley
2 siblings, 1 reply; 8+ messages in thread
From: conor.dooley @ 2021-12-16 14:00 UTC (permalink / raw)
To: mturquette, sboyd, linux-clk, robh+dt, devicetree
Cc: krzysztof.kozlowski, geert, david.abdurachmanov, palmer,
daire.mcnamara, cyril.jean, conor.dooley, Rob Herring
From: Daire McNamara <daire.mcnamara@microchip.com>
Add device tree bindings for the Microchip PolarFire system
clock controller
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/clock/microchip,mpfs.yaml | 58 +++++++++++++++++++
.../dt-bindings/clock/microchip,mpfs-clock.h | 45 ++++++++++++++
2 files changed, 103 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
create mode 100644 include/dt-bindings/clock/microchip,mpfs-clock.h
diff --git a/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml b/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
new file mode 100644
index 000000000000..0c15afa2214c
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/microchip,mpfs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PolarFire Clock Control Module Binding
+
+maintainers:
+ - Daire McNamara <daire.mcnamara@microchip.com>
+
+description: |
+ Microchip PolarFire clock control (CLKCFG) is an integrated clock controller,
+ which gates and enables all peripheral clocks.
+
+ This device tree binding describes 33 gate clocks. Clocks are referenced by
+ user nodes by the CLKCFG node phandle and the clock index in the group, from
+ 0 to 32.
+
+properties:
+ compatible:
+ const: microchip,mpfs-clkcfg
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+ description: |
+ The clock consumer should specify the desired clock by having the clock
+ ID in its "clocks" phandle cell. See include/dt-bindings/clock/microchip,mpfs-clock.h
+ for the full list of PolarFire clock IDs.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+ # Clock Config node:
+ - |
+ #include <dt-bindings/clock/microchip,mpfs-clock.h>
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ clkcfg: clock-controller@20002000 {
+ compatible = "microchip,mpfs-clkcfg";
+ reg = <0x0 0x20002000 0x0 0x1000>;
+ clocks = <&ref>;
+ #clock-cells = <1>;
+ };
+ };
diff --git a/include/dt-bindings/clock/microchip,mpfs-clock.h b/include/dt-bindings/clock/microchip,mpfs-clock.h
new file mode 100644
index 000000000000..73f2a9324857
--- /dev/null
+++ b/include/dt-bindings/clock/microchip,mpfs-clock.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Daire McNamara,<daire.mcnamara@microchip.com>
+ * Copyright (C) 2020 Microchip Technology Inc. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_MICROCHIP_MPFS_H_
+#define _DT_BINDINGS_CLK_MICROCHIP_MPFS_H_
+
+#define CLK_CPU 0
+#define CLK_AXI 1
+#define CLK_AHB 2
+
+#define CLK_ENVM 3
+#define CLK_MAC0 4
+#define CLK_MAC1 5
+#define CLK_MMC 6
+#define CLK_TIMER 7
+#define CLK_MMUART0 8
+#define CLK_MMUART1 9
+#define CLK_MMUART2 10
+#define CLK_MMUART3 11
+#define CLK_MMUART4 12
+#define CLK_SPI0 13
+#define CLK_SPI1 14
+#define CLK_I2C0 15
+#define CLK_I2C1 16
+#define CLK_CAN0 17
+#define CLK_CAN1 18
+#define CLK_USB 19
+#define CLK_RESERVED 20
+#define CLK_RTC 21
+#define CLK_QSPI 22
+#define CLK_GPIO0 23
+#define CLK_GPIO1 24
+#define CLK_GPIO2 25
+#define CLK_DDRC 26
+#define CLK_FIC0 27
+#define CLK_FIC1 28
+#define CLK_FIC2 29
+#define CLK_FIC3 30
+#define CLK_ATHENA 31
+#define CLK_CFM 32
+
+#endif /* _DT_BINDINGS_CLK_MICROCHIP_MPFS_H_ */
--
2.33.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC
2021-12-16 14:00 [PATCH v9 0/2] Add clkcfg driver for Microchip PolarFire SoC conor.dooley
2021-12-16 14:00 ` [PATCH v9 1/2] dt-bindings: clk: microchip: Add Microchip PolarFire host binding conor.dooley
@ 2021-12-16 14:00 ` conor.dooley
2022-01-25 0:48 ` Stephen Boyd
2022-01-10 10:24 ` [PATCH v9 0/2] Add clkcfg " Conor.Dooley
2 siblings, 1 reply; 8+ messages in thread
From: conor.dooley @ 2021-12-16 14:00 UTC (permalink / raw)
To: mturquette, sboyd, linux-clk, robh+dt, devicetree
Cc: krzysztof.kozlowski, geert, david.abdurachmanov, palmer,
daire.mcnamara, cyril.jean, conor.dooley, Padmarao Bengari
From: Daire McNamara <daire.mcnamara@microchip.com>
Add support for clock configuration on Microchip PolarFire SoC
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Co-developed-by: Padmarao Bengari <padmarao.begari@microchip.com>
Signed-off-by: Padmarao Bengari <padmarao.begari@microchip.com>
Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
Co-developed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 2 +-
drivers/clk/microchip/Kconfig | 7 +
drivers/clk/microchip/Makefile | 6 +-
drivers/clk/microchip/clk-mpfs.c | 439 +++++++++++++++++++++++++++++++
5 files changed, 452 insertions(+), 3 deletions(-)
create mode 100644 drivers/clk/microchip/Kconfig
create mode 100644 drivers/clk/microchip/clk-mpfs.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 1b992a554ff8..fe6c757e44fb 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -409,6 +409,7 @@ source "drivers/clk/keystone/Kconfig"
source "drivers/clk/mediatek/Kconfig"
source "drivers/clk/meson/Kconfig"
source "drivers/clk/mstar/Kconfig"
+source "drivers/clk/microchip/Kconfig"
source "drivers/clk/mvebu/Kconfig"
source "drivers/clk/pistachio/Kconfig"
source "drivers/clk/qcom/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d8565ef01b34..d4e43d0431a3 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -90,7 +90,7 @@ obj-$(CONFIG_ARCH_KEYSTONE) += keystone/
obj-$(CONFIG_MACH_LOONGSON32) += loongson1/
obj-y += mediatek/
obj-$(CONFIG_ARCH_MESON) += meson/
-obj-$(CONFIG_MACH_PIC32) += microchip/
+obj-y += microchip/
ifeq ($(CONFIG_COMMON_CLK), y)
obj-$(CONFIG_ARCH_MMP) += mmp/
endif
diff --git a/drivers/clk/microchip/Kconfig b/drivers/clk/microchip/Kconfig
new file mode 100644
index 000000000000..f5edc7b3c07c
--- /dev/null
+++ b/drivers/clk/microchip/Kconfig
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config MCHP_CLK_MPFS
+ bool "Clk driver for PolarFire SoC"
+ depends on (RISCV && SOC_MICROCHIP_POLARFIRE) || COMPILE_TEST
+ help
+ Supports Clock Configuration for PolarFire SoC
diff --git a/drivers/clk/microchip/Makefile b/drivers/clk/microchip/Makefile
index f34b247e870f..0dce0b12eac4 100644
--- a/drivers/clk/microchip/Makefile
+++ b/drivers/clk/microchip/Makefile
@@ -1,3 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_COMMON_CLK_PIC32) += clk-core.o
-obj-$(CONFIG_PIC32MZDA) += clk-pic32mzda.o
+
+obj-$(CONFIG_COMMON_CLK_PIC32) += clk-core.o
+obj-$(CONFIG_PIC32MZDA) += clk-pic32mzda.o
+obj-$(CONFIG_MCHP_CLK_MPFS) += clk-mpfs.o
diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
new file mode 100644
index 000000000000..f9bcfa864b67
--- /dev/null
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -0,0 +1,439 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Daire McNamara,<daire.mcnamara@microchip.com>
+ * Copyright (C) 2020 Microchip Technology Inc. All rights reserved.
+ */
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <dt-bindings/clock/microchip,mpfs-clock.h>
+
+/* address offset of control registers */
+#define REG_CLOCK_CONFIG_CR 0x08u
+#define REG_SUBBLK_CLOCK_CR 0x84u
+#define REG_SUBBLK_RESET_CR 0x88u
+
+struct mpfs_clock_data {
+ void __iomem *base;
+ struct clk_hw_onecell_data hw_data;
+};
+
+struct mpfs_cfg_clock {
+ const char *name;
+ const struct clk_div_table *table;
+ struct clk_hw *parent;
+ unsigned long flags;
+ unsigned int id;
+ u8 shift;
+ u8 width;
+};
+
+struct mpfs_cfg_hw_clock {
+ struct mpfs_cfg_clock cfg;
+ void __iomem *sys_base;
+ /* lock is used to prevent multiple writes */
+ spinlock_t *lock;
+ struct clk_hw hw;
+ struct clk_init_data init;
+};
+
+#define to_mpfs_cfg_clk(_hw) container_of(_hw, struct mpfs_cfg_hw_clock, hw)
+
+struct mpfs_periph_clock {
+ const char *name;
+ struct clk_hw *parent;
+ unsigned long flags;
+ unsigned int id;
+ u8 shift;
+};
+
+struct mpfs_periph_hw_clock {
+ struct mpfs_periph_clock periph;
+ void __iomem *sys_base;
+ /* lock is used to prevent multiple writes */
+ spinlock_t *lock;
+ struct clk_hw hw;
+};
+
+#define to_mpfs_periph_clk(_hw) container_of(_hw, struct mpfs_periph_hw_clock, hw)
+
+/*
+ * mpfs_clk_lock prevents anything else from writing to the
+ * mpfs clk block while a software locked register is being written.
+ */
+static DEFINE_SPINLOCK(mpfs_clk_lock);
+
+static const struct clk_div_table mpfs_div_cpu_axi_table[] = {
+ { 0, 1 }, { 1, 2 }, { 2, 4 }, { 3, 8 },
+ { 0, 0 }
+};
+
+static const struct clk_div_table mpfs_div_ahb_table[] = {
+ { 1, 2 }, { 2, 4}, { 3, 8 },
+ { 0, 0 }
+};
+
+static unsigned long mpfs_cfg_clk_recalc_rate(struct clk_hw *hw, unsigned long prate)
+{
+ struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
+ struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
+ void __iomem *base_addr = cfg_hw->sys_base;
+ u32 val;
+
+ val = readl_relaxed(base_addr + REG_CLOCK_CONFIG_CR) >> cfg->shift;
+ val &= clk_div_mask(cfg->width);
+
+ return prate / (1u << val);
+}
+
+static long mpfs_cfg_clk_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *prate)
+{
+ struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
+ struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
+
+ return divider_round_rate(hw, rate, prate, cfg->table, cfg->width, cfg->flags);
+}
+
+static int mpfs_cfg_clk_set_rate(struct clk_hw *hw, unsigned long rate, unsigned long prate)
+{
+ struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
+ struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
+ void __iomem *base_addr = cfg_hw->sys_base;
+ unsigned long flags = 0;
+ u32 val;
+ int divider_setting;
+
+ divider_setting = divider_get_val(rate, prate, cfg->table, cfg->width, cfg_hw->cfg.flags);
+
+ if (divider_setting < 0)
+ return divider_setting;
+
+ spin_lock_irqsave(cfg_hw->lock, flags);
+
+ val = readl_relaxed(base_addr + REG_CLOCK_CONFIG_CR);
+ val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
+ val |= divider_setting << cfg->shift;
+ writel_relaxed(val, base_addr + REG_CLOCK_CONFIG_CR);
+
+ spin_unlock_irqrestore(cfg_hw->lock, flags);
+
+ return 0;
+}
+
+static const struct clk_ops mpfs_clk_cfg_ops = {
+ .recalc_rate = mpfs_cfg_clk_recalc_rate,
+ .round_rate = mpfs_cfg_clk_round_rate,
+ .set_rate = mpfs_cfg_clk_set_rate,
+};
+
+#define CLK_CFG(_id, _name, _shift, _width, _table, _flags) { \
+ .cfg.id = _id, \
+ .cfg.name = _name, \
+ .cfg.shift = _shift, \
+ .cfg.width = _width, \
+ .cfg.table = _table, \
+ }
+
+static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
+ CLK_CFG(CLK_CPU, "clk_cpu", 0, 2, mpfs_div_cpu_axi_table, 0),
+ CLK_CFG(CLK_AXI, "clk_axi", 2, 2, mpfs_div_cpu_axi_table, 0),
+ CLK_CFG(CLK_AHB, "clk_ahb", 4, 2, mpfs_div_ahb_table, 0),
+};
+
+static struct clk_hw *mpfs_clk_register_cfg(struct device *dev,
+ struct mpfs_cfg_hw_clock *cfg_hw,
+ void __iomem *sys_base)
+{
+ struct clk_hw *hw;
+ int err;
+
+ cfg_hw->sys_base = sys_base;
+ cfg_hw->lock = &mpfs_clk_lock;
+
+ hw = &cfg_hw->hw;
+ err = devm_clk_hw_register(dev, hw);
+ if (err)
+ return ERR_PTR(err);
+
+ return hw;
+}
+
+static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
+ unsigned int num_clks, struct mpfs_clock_data *data,
+ struct clk *clk_parent)
+{
+ struct clk_hw *hw;
+ void __iomem *sys_base = data->base;
+ unsigned int i, id;
+
+ for (i = 0; i < num_clks; i++) {
+ struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
+
+ cfg_hw->cfg.parent = __clk_get_hw(clk_parent);
+ cfg_hw->hw.init = CLK_HW_INIT_HW(cfg_hw->cfg.name, cfg_hw->cfg.parent,
+ &mpfs_clk_cfg_ops, cfg_hw->cfg.flags);
+ hw = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
+ if (IS_ERR(hw)) {
+ dev_err(dev, "failed to register clock %s\n", cfg_hw->cfg.name);
+ goto err_clk;
+ }
+
+ id = cfg_hws[i].cfg.id;
+ data->hw_data.hws[id] = hw;
+ }
+
+ return 0;
+
+err_clk:
+ while (i--)
+ devm_clk_hw_unregister(dev, data->hw_data.hws[cfg_hws[i].cfg.id]);
+
+ return PTR_ERR(hw);
+}
+
+static int mpfs_periph_clk_enable(struct clk_hw *hw)
+{
+ struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
+ struct mpfs_periph_clock *periph = &periph_hw->periph;
+ void __iomem *base_addr = periph_hw->sys_base;
+ u32 reg, val;
+ unsigned long flags = 0;
+
+ spin_lock_irqsave(periph_hw->lock, flags);
+
+ reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
+ val = reg & ~(1u << periph->shift);
+ writel_relaxed(val, base_addr + REG_SUBBLK_RESET_CR);
+
+ reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
+ val = reg | (1u << periph->shift);
+ writel_relaxed(val, base_addr + REG_SUBBLK_CLOCK_CR);
+
+ spin_unlock_irqrestore(periph_hw->lock, flags);
+
+ return 0;
+}
+
+static void mpfs_periph_clk_disable(struct clk_hw *hw)
+{
+ struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
+ struct mpfs_periph_clock *periph = &periph_hw->periph;
+ void __iomem *base_addr = periph_hw->sys_base;
+ u32 reg, val;
+ unsigned long flags = 0;
+
+ spin_lock_irqsave(periph_hw->lock, flags);
+
+ reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
+ val = reg | (1u << periph->shift);
+ writel_relaxed(val, base_addr + REG_SUBBLK_RESET_CR);
+
+ reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
+ val = reg & ~(1u << periph->shift);
+ writel_relaxed(val, base_addr + REG_SUBBLK_CLOCK_CR);
+
+ spin_unlock_irqrestore(periph_hw->lock, flags);
+}
+
+static int mpfs_periph_clk_is_enabled(struct clk_hw *hw)
+{
+ struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
+ struct mpfs_periph_clock *periph = &periph_hw->periph;
+ void __iomem *base_addr = periph_hw->sys_base;
+ u32 reg;
+
+ reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
+ if ((reg & (1u << periph->shift)) == 0u) {
+ reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
+ if (reg & (1u << periph->shift))
+ return 1;
+ }
+
+ return 0;
+}
+
+static unsigned long mpfs_periph_clk_recalc_rate(struct clk_hw *hw, unsigned long prate)
+{
+ return prate;
+}
+
+static const struct clk_ops mpfs_periph_clk_ops = {
+ .enable = mpfs_periph_clk_enable,
+ .disable = mpfs_periph_clk_disable,
+ .is_enabled = mpfs_periph_clk_is_enabled,
+ .recalc_rate = mpfs_periph_clk_recalc_rate,
+};
+
+#define CLK_PERIPH(_id, _name, _parent, _shift, _flags) { \
+ .periph.id = _id, \
+ .periph.name = _name, \
+ .periph.shift = _shift, \
+ .periph.flags = _flags, \
+ .periph.parent = _parent, \
+}
+
+#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT].hw)
+
+static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
+ CLK_PERIPH(CLK_ENVM, "clk_periph_envm", PARENT_CLK(AHB), 0, CLK_IS_CRITICAL),
+ CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", PARENT_CLK(AHB), 1, 0),
+ CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", PARENT_CLK(AHB), 2, 0),
+ CLK_PERIPH(CLK_MMC, "clk_periph_mmc", PARENT_CLK(AHB), 3, 0),
+ CLK_PERIPH(CLK_TIMER, "clk_periph_timer", PARENT_CLK(AHB), 4, 0),
+ CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", PARENT_CLK(AHB), 5, CLK_IS_CRITICAL),
+ CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", PARENT_CLK(AHB), 6, 0),
+ CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", PARENT_CLK(AHB), 7, 0),
+ CLK_PERIPH(CLK_MMUART3, "clk_periph_mmuart3", PARENT_CLK(AHB), 8, 0),
+ CLK_PERIPH(CLK_MMUART4, "clk_periph_mmuart4", PARENT_CLK(AHB), 9, 0),
+ CLK_PERIPH(CLK_SPI0, "clk_periph_spi0", PARENT_CLK(AHB), 10, 0),
+ CLK_PERIPH(CLK_SPI1, "clk_periph_spi1", PARENT_CLK(AHB), 11, 0),
+ CLK_PERIPH(CLK_I2C0, "clk_periph_i2c0", PARENT_CLK(AHB), 12, 0),
+ CLK_PERIPH(CLK_I2C1, "clk_periph_i2c1", PARENT_CLK(AHB), 13, 0),
+ CLK_PERIPH(CLK_CAN0, "clk_periph_can0", PARENT_CLK(AHB), 14, 0),
+ CLK_PERIPH(CLK_CAN1, "clk_periph_can1", PARENT_CLK(AHB), 15, 0),
+ CLK_PERIPH(CLK_USB, "clk_periph_usb", PARENT_CLK(AHB), 16, 0),
+ CLK_PERIPH(CLK_RTC, "clk_periph_rtc", PARENT_CLK(AHB), 18, 0),
+ CLK_PERIPH(CLK_QSPI, "clk_periph_qspi", PARENT_CLK(AHB), 19, 0),
+ CLK_PERIPH(CLK_GPIO0, "clk_periph_gpio0", PARENT_CLK(AHB), 20, 0),
+ CLK_PERIPH(CLK_GPIO1, "clk_periph_gpio1", PARENT_CLK(AHB), 21, 0),
+ CLK_PERIPH(CLK_GPIO2, "clk_periph_gpio2", PARENT_CLK(AHB), 22, 0),
+ CLK_PERIPH(CLK_DDRC, "clk_periph_ddrc", PARENT_CLK(AHB), 23, CLK_IS_CRITICAL),
+ CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", PARENT_CLK(AHB), 24, CLK_IS_CRITICAL),
+ CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", PARENT_CLK(AHB), 25, CLK_IS_CRITICAL),
+ CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", PARENT_CLK(AHB), 26, CLK_IS_CRITICAL),
+ CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", PARENT_CLK(AHB), 27, CLK_IS_CRITICAL),
+ CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", PARENT_CLK(AHB), 28, 0),
+ CLK_PERIPH(CLK_CFM, "clk_periph_cfm", PARENT_CLK(AHB), 29, 0),
+};
+
+static struct clk_hw *mpfs_clk_register_periph(struct device *dev,
+ struct mpfs_periph_hw_clock *periph_hw,
+ void __iomem *sys_base)
+{
+ struct clk_hw *hw;
+ int err;
+
+ periph_hw->sys_base = sys_base;
+ periph_hw->lock = &mpfs_clk_lock;
+ hw = &periph_hw->hw;
+ err = devm_clk_hw_register(dev, hw);
+ if (err)
+ return ERR_PTR(err);
+
+ return hw;
+}
+
+static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_clock *periph_hws,
+ int num_clks, struct mpfs_clock_data *data)
+{
+ struct clk_hw *hw;
+ void __iomem *sys_base = data->base;
+ unsigned int i, id;
+
+ for (i = 0; i < num_clks; i++) {
+ struct mpfs_periph_hw_clock *periph_hw = &periph_hws[i];
+
+ periph_hw->hw.init = CLK_HW_INIT_HW(periph_hw->periph.name,
+ periph_hw->periph.parent,
+ &mpfs_periph_clk_ops,
+ periph_hw->periph.flags);
+ hw = mpfs_clk_register_periph(dev, periph_hw, sys_base);
+ if (IS_ERR(hw)) {
+ dev_err(dev, "failed to register clock %s\n", periph_hw->periph.name);
+ goto err_clk;
+ }
+
+ id = periph_hws[i].periph.id;
+ data->hw_data.hws[id] = hw;
+ }
+
+ return 0;
+
+err_clk:
+ while (i--)
+ devm_clk_hw_unregister(dev, data->hw_data.hws[periph_hws[i].periph.id]);
+
+ return PTR_ERR(hw);
+}
+
+static int mpfs_clk_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mpfs_clock_data *clk_data;
+ struct clk *clk_parent;
+ struct resource *res;
+ unsigned int num_clks;
+ int ret;
+
+ //CLK_RESERVED is not part of cfg_clks nor periph_clks, so add 1
+ num_clks = ARRAY_SIZE(mpfs_cfg_clks) + ARRAY_SIZE(mpfs_periph_clks) + 1;
+
+ clk_data = devm_kzalloc(dev, struct_size(clk_data, hw_data.hws, num_clks), GFP_KERNEL);
+ if (!clk_data)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ clk_data->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(clk_data->base))
+ return PTR_ERR(clk_data->base);
+
+ clk_data->hw_data.num = num_clks;
+
+ clk_parent = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk_parent))
+ return PTR_ERR(clk_parent);
+
+ ret = mpfs_clk_register_cfgs(dev, mpfs_cfg_clks, ARRAY_SIZE(mpfs_cfg_clks), clk_data,
+ clk_parent);
+ if (ret)
+ goto err_clk;
+
+ ret = mpfs_clk_register_periphs(dev, mpfs_periph_clks, ARRAY_SIZE(mpfs_periph_clks),
+ clk_data);
+ if (ret)
+ goto err_clk;
+
+ ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, &clk_data->hw_data);
+ if (ret)
+ goto err_clk;
+
+ dev_info(dev, "registered MPFS core clocks\n");
+ return ret;
+
+err_clk:
+ dev_err(dev, "failed to register MPFS core clocks\n");
+ return ret;
+}
+
+static const struct of_device_id mpfs_clk_of_match_table[] = {
+ { .compatible = "microchip,mpfs-clkcfg", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, mpfs_clk_match_table);
+
+static struct platform_driver mpfs_clk_driver = {
+ .probe = mpfs_clk_probe,
+ .driver = {
+ .name = "microchip-mpfs-clkcfg",
+ .of_match_table = mpfs_clk_of_match_table,
+ },
+};
+
+static int __init clk_mpfs_init(void)
+{
+ return platform_driver_register(&mpfs_clk_driver);
+}
+core_initcall(clk_mpfs_init);
+
+static void __exit clk_mpfs_exit(void)
+{
+ platform_driver_unregister(&mpfs_clk_driver);
+}
+module_exit(clk_mpfs_exit);
+
+MODULE_DESCRIPTION("Microchip PolarFire SoC Clock Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:clk-mpfs");
--
2.33.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v9 0/2] Add clkcfg driver for Microchip PolarFire SoC
2021-12-16 14:00 [PATCH v9 0/2] Add clkcfg driver for Microchip PolarFire SoC conor.dooley
2021-12-16 14:00 ` [PATCH v9 1/2] dt-bindings: clk: microchip: Add Microchip PolarFire host binding conor.dooley
2021-12-16 14:00 ` [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC conor.dooley
@ 2022-01-10 10:24 ` Conor.Dooley
2 siblings, 0 replies; 8+ messages in thread
From: Conor.Dooley @ 2022-01-10 10:24 UTC (permalink / raw)
To: mturquette, sboyd, linux-clk, robh+dt, devicetree
Cc: krzysztof.kozlowski, geert, david.abdurachmanov, palmer,
Daire.McNamara, Cyril.Jean
Hi Stephen/Michael,
I've implemented the feedback received & have RB tags from Geert & Rob.
Do you think this could be included on -next?
Thanks,
Conor.
On 16/12/2021 14:00, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> This patchset adds support for the Microchip PolarFire clkcfg
> hardware block.
>
> Changes since v8:
> * Use devm_clk_hw_unregister directly
> * Get parent clk in probe rather than register function
>
> Changes since v7:
> * Dropped clock-output-names again (oops)
>
> Changes since v6:
> * Dropped clock-output-names *as a required property*
> * Dropped if(lock) check on spinlocks, added spinlocks to all
> read-modify-write register access
> * Removed kfree()s on non-dynamically allocated variables
> * Use devm_clk_get to get the reference clock
> * Account for reserved clock when calculating the size of num_clks
>
> Changes since v5:
> * Dropped clock-output-names property
>
> Major changes since v4:
> * Adjusted license for microchip,mpfs-clock.h to match microchip,mpfs.yaml
> * Corrected the number of clocks to 33 from 32
>
> Major changes since v3:
> * Patch reformatted so microchip,mpfs-clock.h is part of device-tree patch
>
> Major changes since v2:
> * In mpfs_cfg_clk_set_rate, return immediately if divider_get_val
> returns <0
> * rebased to v5.12-rc1
>
> Major changes since v1:
> * Dependency on SOC_MICROCHIP_POLARFIRE
> * All references to PFSOC/pfsoc changed to MPFS/mpfs
> * Cleaned error handling in _probe
> * Re-ordered code to place structs et al at top
>
> Daire McNamara (2):
> dt-bindings: clk: microchip: Add Microchip PolarFire host binding
> clk: microchip: Add driver for Microchip PolarFire SoC
>
> .../bindings/clock/microchip,mpfs.yaml | 58 +++
> drivers/clk/Kconfig | 1 +
> drivers/clk/Makefile | 2 +-
> drivers/clk/microchip/Kconfig | 7 +
> drivers/clk/microchip/Makefile | 6 +-
> drivers/clk/microchip/clk-mpfs.c | 439 ++++++++++++++++++
> .../dt-bindings/clock/microchip,mpfs-clock.h | 45 ++
> 7 files changed, 555 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
> create mode 100644 drivers/clk/microchip/Kconfig
> create mode 100644 drivers/clk/microchip/clk-mpfs.c
> create mode 100644 include/dt-bindings/clock/microchip,mpfs-clock.h
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v9 1/2] dt-bindings: clk: microchip: Add Microchip PolarFire host binding
2021-12-16 14:00 ` [PATCH v9 1/2] dt-bindings: clk: microchip: Add Microchip PolarFire host binding conor.dooley
@ 2022-01-25 0:26 ` Stephen Boyd
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2022-01-25 0:26 UTC (permalink / raw)
To: conor.dooley, devicetree, linux-clk, mturquette, robh+dt
Cc: krzysztof.kozlowski, geert, david.abdurachmanov, palmer,
daire.mcnamara, cyril.jean, conor.dooley, Rob Herring
Quoting conor.dooley@microchip.com (2021-12-16 06:00:21)
> From: Daire McNamara <daire.mcnamara@microchip.com>
>
> Add device tree bindings for the Microchip PolarFire system
> clock controller
>
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
Applied to clk-next
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC
2021-12-16 14:00 ` [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC conor.dooley
@ 2022-01-25 0:48 ` Stephen Boyd
2022-01-25 13:40 ` conor.dooley
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Boyd @ 2022-01-25 0:48 UTC (permalink / raw)
To: conor.dooley, devicetree, linux-clk, mturquette, robh+dt
Cc: krzysztof.kozlowski, geert, david.abdurachmanov, palmer,
daire.mcnamara, cyril.jean, conor.dooley, Padmarao Bengari
Quoting conor.dooley@microchip.com (2021-12-16 06:00:22)
> diff --git a/drivers/clk/microchip/Makefile b/drivers/clk/microchip/Makefile
> index f34b247e870f..0dce0b12eac4 100644
> --- a/drivers/clk/microchip/Makefile
> +++ b/drivers/clk/microchip/Makefile
> @@ -1,3 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_COMMON_CLK_PIC32) += clk-core.o
Can you move this config to the microchip/Kconfig file?
> -obj-$(CONFIG_PIC32MZDA) += clk-pic32mzda.o
> +
> +obj-$(CONFIG_COMMON_CLK_PIC32) += clk-core.o
> +obj-$(CONFIG_PIC32MZDA) += clk-pic32mzda.o
Why did these two lines change? Please undo it.
> +obj-$(CONFIG_MCHP_CLK_MPFS) += clk-mpfs.o
> diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
> new file mode 100644
> index 000000000000..f9bcfa864b67
> --- /dev/null
> +++ b/drivers/clk/microchip/clk-mpfs.c
> @@ -0,0 +1,439 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Daire McNamara,<daire.mcnamara@microchip.com>
> + * Copyright (C) 2020 Microchip Technology Inc. All rights reserved.
> + */
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>
Drop this include.
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <dt-bindings/clock/microchip,mpfs-clock.h>
> +
> +/* address offset of control registers */
> +#define REG_CLOCK_CONFIG_CR 0x08u
> +#define REG_SUBBLK_CLOCK_CR 0x84u
> +#define REG_SUBBLK_RESET_CR 0x88u
> +
> +struct mpfs_clock_data {
> + void __iomem *base;
> + struct clk_hw_onecell_data hw_data;
> +};
> +
> +struct mpfs_cfg_clock {
> + const char *name;
Why is this needed?
> + const struct clk_div_table *table;
> + struct clk_hw *parent;
Why is this needed?
> + unsigned long flags;
> + unsigned int id;
> + u8 shift;
> + u8 width;
> +};
> +
> +struct mpfs_cfg_hw_clock {
> + struct mpfs_cfg_clock cfg;
> + void __iomem *sys_base;
> + /* lock is used to prevent multiple writes */
> + spinlock_t *lock;
> + struct clk_hw hw;
> + struct clk_init_data init;
> +};
> +
> +#define to_mpfs_cfg_clk(_hw) container_of(_hw, struct mpfs_cfg_hw_clock, hw)
> +
> +struct mpfs_periph_clock {
> + const char *name;
> + struct clk_hw *parent;
Why are these two needed?
> + unsigned long flags;
> + unsigned int id;
> + u8 shift;
> +};
> +
> +struct mpfs_periph_hw_clock {
> + struct mpfs_periph_clock periph;
> + void __iomem *sys_base;
> + /* lock is used to prevent multiple writes */
> + spinlock_t *lock;
> + struct clk_hw hw;
> +};
> +
> +#define to_mpfs_periph_clk(_hw) container_of(_hw, struct mpfs_periph_hw_clock, hw)
> +
> +/*
> + * mpfs_clk_lock prevents anything else from writing to the
> + * mpfs clk block while a software locked register is being written.
> + */
> +static DEFINE_SPINLOCK(mpfs_clk_lock);
> +
> +static const struct clk_div_table mpfs_div_cpu_axi_table[] = {
> + { 0, 1 }, { 1, 2 }, { 2, 4 }, { 3, 8 },
> + { 0, 0 }
> +};
> +
> +static const struct clk_div_table mpfs_div_ahb_table[] = {
> + { 1, 2 }, { 2, 4}, { 3, 8 },
> + { 0, 0 }
> +};
> +
> +static unsigned long mpfs_cfg_clk_recalc_rate(struct clk_hw *hw, unsigned long prate)
> +{
> + struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
> + struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
> + void __iomem *base_addr = cfg_hw->sys_base;
> + u32 val;
> +
> + val = readl_relaxed(base_addr + REG_CLOCK_CONFIG_CR) >> cfg->shift;
> + val &= clk_div_mask(cfg->width);
> +
> + return prate / (1u << val);
> +}
> +
> +static long mpfs_cfg_clk_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *prate)
> +{
> + struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
> + struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
> +
> + return divider_round_rate(hw, rate, prate, cfg->table, cfg->width, cfg->flags);
> +}
> +
> +static int mpfs_cfg_clk_set_rate(struct clk_hw *hw, unsigned long rate, unsigned long prate)
> +{
> + struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
> + struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
> + void __iomem *base_addr = cfg_hw->sys_base;
> + unsigned long flags = 0;
> + u32 val;
> + int divider_setting;
> +
> + divider_setting = divider_get_val(rate, prate, cfg->table, cfg->width, cfg_hw->cfg.flags);
I think these are clk_hw flags and not divider flags being passed as the
last argument? If so, that's wrong.
> +
> + if (divider_setting < 0)
> + return divider_setting;
> +
> + spin_lock_irqsave(cfg_hw->lock, flags);
> +
> + val = readl_relaxed(base_addr + REG_CLOCK_CONFIG_CR);
> + val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
> + val |= divider_setting << cfg->shift;
> + writel_relaxed(val, base_addr + REG_CLOCK_CONFIG_CR);
> +
> + spin_unlock_irqrestore(cfg_hw->lock, flags);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops mpfs_clk_cfg_ops = {
> + .recalc_rate = mpfs_cfg_clk_recalc_rate,
> + .round_rate = mpfs_cfg_clk_round_rate,
> + .set_rate = mpfs_cfg_clk_set_rate,
> +};
> +
> +#define CLK_CFG(_id, _name, _shift, _width, _table, _flags) { \
> + .cfg.id = _id, \
> + .cfg.name = _name, \
> + .cfg.shift = _shift, \
> + .cfg.width = _width, \
> + .cfg.table = _table, \
> + }
> +
> +static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
> + CLK_CFG(CLK_CPU, "clk_cpu", 0, 2, mpfs_div_cpu_axi_table, 0),
> + CLK_CFG(CLK_AXI, "clk_axi", 2, 2, mpfs_div_cpu_axi_table, 0),
> + CLK_CFG(CLK_AHB, "clk_ahb", 4, 2, mpfs_div_ahb_table, 0),
> +};
> +
> +static struct clk_hw *mpfs_clk_register_cfg(struct device *dev,
> + struct mpfs_cfg_hw_clock *cfg_hw,
> + void __iomem *sys_base)
> +{
> + struct clk_hw *hw;
> + int err;
> +
> + cfg_hw->sys_base = sys_base;
> + cfg_hw->lock = &mpfs_clk_lock;
Instead of having a struct member why not use the lock directly as it's
all local to this file?
> +
> + hw = &cfg_hw->hw;
> + err = devm_clk_hw_register(dev, hw);
> + if (err)
> + return ERR_PTR(err);
> +
> + return hw;
> +}
> +
> +static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
> + unsigned int num_clks, struct mpfs_clock_data *data,
> + struct clk *clk_parent)
> +{
> + struct clk_hw *hw;
> + void __iomem *sys_base = data->base;
> + unsigned int i, id;
> +
> + for (i = 0; i < num_clks; i++) {
> + struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
> +
> + cfg_hw->cfg.parent = __clk_get_hw(clk_parent);
> + cfg_hw->hw.init = CLK_HW_INIT_HW(cfg_hw->cfg.name, cfg_hw->cfg.parent,
> + &mpfs_clk_cfg_ops, cfg_hw->cfg.flags);
> + hw = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
> + if (IS_ERR(hw)) {
> + dev_err(dev, "failed to register clock %s\n", cfg_hw->cfg.name);
> + goto err_clk;
> + }
> +
> + id = cfg_hws[i].cfg.id;
> + data->hw_data.hws[id] = hw;
> + }
> +
> + return 0;
> +
> +err_clk:
> + while (i--)
> + devm_clk_hw_unregister(dev, data->hw_data.hws[cfg_hws[i].cfg.id]);
The same comments below apply here.
> +
> + return PTR_ERR(hw);
> +}
> +
> +static int mpfs_periph_clk_enable(struct clk_hw *hw)
> +{
> + struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
> + struct mpfs_periph_clock *periph = &periph_hw->periph;
> + void __iomem *base_addr = periph_hw->sys_base;
> + u32 reg, val;
> + unsigned long flags = 0;
> +
> + spin_lock_irqsave(periph_hw->lock, flags);
> +
> + reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
> + val = reg & ~(1u << periph->shift);
> + writel_relaxed(val, base_addr + REG_SUBBLK_RESET_CR);
> +
> + reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
> + val = reg | (1u << periph->shift);
> + writel_relaxed(val, base_addr + REG_SUBBLK_CLOCK_CR);
> +
> + spin_unlock_irqrestore(periph_hw->lock, flags);
> +
> + return 0;
> +}
> +
> +static void mpfs_periph_clk_disable(struct clk_hw *hw)
> +{
> + struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
> + struct mpfs_periph_clock *periph = &periph_hw->periph;
> + void __iomem *base_addr = periph_hw->sys_base;
> + u32 reg, val;
> + unsigned long flags = 0;
Don't initialize flags for spin_lock_irqsave().
> +
> + spin_lock_irqsave(periph_hw->lock, flags);
> +
> + reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
> + val = reg | (1u << periph->shift);
> + writel_relaxed(val, base_addr + REG_SUBBLK_RESET_CR);
> +
> + reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
> + val = reg & ~(1u << periph->shift);
> + writel_relaxed(val, base_addr + REG_SUBBLK_CLOCK_CR);
> +
> + spin_unlock_irqrestore(periph_hw->lock, flags);
> +}
> +
> +static int mpfs_periph_clk_is_enabled(struct clk_hw *hw)
> +{
> + struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
> + struct mpfs_periph_clock *periph = &periph_hw->periph;
> + void __iomem *base_addr = periph_hw->sys_base;
> + u32 reg;
> +
> + reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
> + if ((reg & (1u << periph->shift)) == 0u) {
> + reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
> + if (reg & (1u << periph->shift))
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static unsigned long mpfs_periph_clk_recalc_rate(struct clk_hw *hw, unsigned long prate)
> +{
> + return prate;
If it only returns the parent rate then this function can be omitted as
it's the default.
> +}
> +
> +static const struct clk_ops mpfs_periph_clk_ops = {
> + .enable = mpfs_periph_clk_enable,
> + .disable = mpfs_periph_clk_disable,
> + .is_enabled = mpfs_periph_clk_is_enabled,
> + .recalc_rate = mpfs_periph_clk_recalc_rate,
> +};
> +
> +#define CLK_PERIPH(_id, _name, _parent, _shift, _flags) { \
> + .periph.id = _id, \
Why the double tab?
> + .periph.name = _name, \
> + .periph.shift = _shift, \
> + .periph.flags = _flags, \
> + .periph.parent = _parent, \
> +}
> +
> +#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT].hw)
> +
> +static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
> + CLK_PERIPH(CLK_ENVM, "clk_periph_envm", PARENT_CLK(AHB), 0, CLK_IS_CRITICAL),
All CLK_IS_CRITICAL needs a comment why it's critical.
> + CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", PARENT_CLK(AHB), 1, 0),
> + CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", PARENT_CLK(AHB), 2, 0),
> + CLK_PERIPH(CLK_MMC, "clk_periph_mmc", PARENT_CLK(AHB), 3, 0),
> + CLK_PERIPH(CLK_TIMER, "clk_periph_timer", PARENT_CLK(AHB), 4, 0),
> + CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", PARENT_CLK(AHB), 5, CLK_IS_CRITICAL),
> + CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", PARENT_CLK(AHB), 6, 0),
> + CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", PARENT_CLK(AHB), 7, 0),
> + CLK_PERIPH(CLK_MMUART3, "clk_periph_mmuart3", PARENT_CLK(AHB), 8, 0),
> + CLK_PERIPH(CLK_MMUART4, "clk_periph_mmuart4", PARENT_CLK(AHB), 9, 0),
> + CLK_PERIPH(CLK_SPI0, "clk_periph_spi0", PARENT_CLK(AHB), 10, 0),
> + CLK_PERIPH(CLK_SPI1, "clk_periph_spi1", PARENT_CLK(AHB), 11, 0),
> + CLK_PERIPH(CLK_I2C0, "clk_periph_i2c0", PARENT_CLK(AHB), 12, 0),
> + CLK_PERIPH(CLK_I2C1, "clk_periph_i2c1", PARENT_CLK(AHB), 13, 0),
> + CLK_PERIPH(CLK_CAN0, "clk_periph_can0", PARENT_CLK(AHB), 14, 0),
> + CLK_PERIPH(CLK_CAN1, "clk_periph_can1", PARENT_CLK(AHB), 15, 0),
> + CLK_PERIPH(CLK_USB, "clk_periph_usb", PARENT_CLK(AHB), 16, 0),
> + CLK_PERIPH(CLK_RTC, "clk_periph_rtc", PARENT_CLK(AHB), 18, 0),
> + CLK_PERIPH(CLK_QSPI, "clk_periph_qspi", PARENT_CLK(AHB), 19, 0),
> + CLK_PERIPH(CLK_GPIO0, "clk_periph_gpio0", PARENT_CLK(AHB), 20, 0),
> + CLK_PERIPH(CLK_GPIO1, "clk_periph_gpio1", PARENT_CLK(AHB), 21, 0),
> + CLK_PERIPH(CLK_GPIO2, "clk_periph_gpio2", PARENT_CLK(AHB), 22, 0),
> + CLK_PERIPH(CLK_DDRC, "clk_periph_ddrc", PARENT_CLK(AHB), 23, CLK_IS_CRITICAL),
> + CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", PARENT_CLK(AHB), 24, CLK_IS_CRITICAL),
> + CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", PARENT_CLK(AHB), 25, CLK_IS_CRITICAL),
> + CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", PARENT_CLK(AHB), 26, CLK_IS_CRITICAL),
> + CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", PARENT_CLK(AHB), 27, CLK_IS_CRITICAL),
> + CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", PARENT_CLK(AHB), 28, 0),
> + CLK_PERIPH(CLK_CFM, "clk_periph_cfm", PARENT_CLK(AHB), 29, 0),
> +};
> +
> +static struct clk_hw *mpfs_clk_register_periph(struct device *dev,
Just return an int.
> + struct mpfs_periph_hw_clock *periph_hw,
> + void __iomem *sys_base)
> +{
> + struct clk_hw *hw;
> + int err;
> +
> + periph_hw->sys_base = sys_base;
> + periph_hw->lock = &mpfs_clk_lock;
> + hw = &periph_hw->hw;
> + err = devm_clk_hw_register(dev, hw);
> + if (err)
> + return ERR_PTR(err);
> +
> + return hw;
return devm_clk_hw_register(dev, &periph_hw->hw);
> +}
> +
> +static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_clock *periph_hws,
> + int num_clks, struct mpfs_clock_data *data)
> +{
> + struct clk_hw *hw;
> + void __iomem *sys_base = data->base;
> + unsigned int i, id;
> +
> + for (i = 0; i < num_clks; i++) {
> + struct mpfs_periph_hw_clock *periph_hw = &periph_hws[i];
> +
> + periph_hw->hw.init = CLK_HW_INIT_HW(periph_hw->periph.name,
> + periph_hw->periph.parent,
> + &mpfs_periph_clk_ops,
> + periph_hw->periph.flags);
> + hw = mpfs_clk_register_periph(dev, periph_hw, sys_base);
> + if (IS_ERR(hw)) {
I'd prefer this check for an int instead.
if (ret) {
dev_err_probe(dev, ....)
return ret;
> + dev_err(dev, "failed to register clock %s\n", periph_hw->periph.name);
> + goto err_clk;
> + }
> +
> + id = periph_hws[i].periph.id;
> + data->hw_data.hws[id] = hw;
And then this can use &periph_hw->hw directly.
> + }
> +
> + return 0;
> +
> +err_clk:
> + while (i--)
> + devm_clk_hw_unregister(dev, data->hw_data.hws[periph_hws[i].periph.id]);
There's no need for this. Let the clk_hws get unregistered when driver
probe fails.
> +
> + return PTR_ERR(hw);
> +}
> +
> +static int mpfs_clk_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mpfs_clock_data *clk_data;
> + struct clk *clk_parent;
> + struct resource *res;
> + unsigned int num_clks;
> + int ret;
> +
> + //CLK_RESERVED is not part of cfg_clks nor periph_clks, so add 1
Use /* these style of comments */
> + num_clks = ARRAY_SIZE(mpfs_cfg_clks) + ARRAY_SIZE(mpfs_periph_clks) + 1;
> +
> + clk_data = devm_kzalloc(dev, struct_size(clk_data, hw_data.hws, num_clks), GFP_KERNEL);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + clk_data->base = devm_ioremap_resource(dev, res);
devm_platform_ioremap_resource()
> + if (IS_ERR(clk_data->base))
> + return PTR_ERR(clk_data->base);
> +
> + clk_data->hw_data.num = num_clks;
> +
> + clk_parent = devm_clk_get(dev, NULL);
Use clk_parent_data instead please.
> + if (IS_ERR(clk_parent))
> + return PTR_ERR(clk_parent);
> +
> + ret = mpfs_clk_register_cfgs(dev, mpfs_cfg_clks, ARRAY_SIZE(mpfs_cfg_clks), clk_data,
> + clk_parent);
> + if (ret)
> + goto err_clk;
> +
> + ret = mpfs_clk_register_periphs(dev, mpfs_periph_clks, ARRAY_SIZE(mpfs_periph_clks),
> + clk_data);
> + if (ret)
> + goto err_clk;
> +
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, &clk_data->hw_data);
> + if (ret)
> + goto err_clk;
> +
> + dev_info(dev, "registered MPFS core clocks\n");
Make dev_dbg() or remove. We have driver core debug prints that
duplicate this.
> + return ret;
> +
> +err_clk:
> + dev_err(dev, "failed to register MPFS core clocks\n");
> + return ret;
Drop this goto zone please. The driver failing to probe message should
be pushed down into the respective functions instead of being generic
for the entire driver. Then the goto can be replaced with a simple
'return ret'.
> +}
> +
> +static const struct of_device_id mpfs_clk_of_match_table[] = {
> + { .compatible = "microchip,mpfs-clkcfg", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, mpfs_clk_match_table);
> +
> +static struct platform_driver mpfs_clk_driver = {
> + .probe = mpfs_clk_probe,
> + .driver = {
> + .name = "microchip-mpfs-clkcfg",
> + .of_match_table = mpfs_clk_of_match_table,
> + },
> +};
> +
> +static int __init clk_mpfs_init(void)
> +{
> + return platform_driver_register(&mpfs_clk_driver);
> +}
> +core_initcall(clk_mpfs_init);
> +
> +static void __exit clk_mpfs_exit(void)
> +{
> + platform_driver_unregister(&mpfs_clk_driver);
> +}
> +module_exit(clk_mpfs_exit);
> +
> +MODULE_DESCRIPTION("Microchip PolarFire SoC Clock Driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:clk-mpfs");
An alias isn't required, please remove.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC
2022-01-25 0:48 ` Stephen Boyd
@ 2022-01-25 13:40 ` conor.dooley
2022-01-25 20:23 ` Stephen Boyd
0 siblings, 1 reply; 8+ messages in thread
From: conor.dooley @ 2022-01-25 13:40 UTC (permalink / raw)
To: sboyd
Cc: conor.dooley, cyril.jean, daire.mcnamara, david.abdurachmanov,
devicetree, geert, krzysztof.kozlowski, linux-clk, mturquette,
padmarao.begari, palmer, robh+dt
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Quoting conor.dooley@microchip.com (2021-12-16 06:00:22)
> > diff --git a/drivers/clk/microchip/Makefile b/drivers/clk/microchip/Makefile
> > index f34b247e870f..0dce0b12eac4 100644
> > --- a/drivers/clk/microchip/Makefile
> > +++ b/drivers/clk/microchip/Makefile
Snipping the rest, will/have addressed them.
> > +static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
> > + unsigned int num_clks, struct mpfs_clock_data *data,
> > + struct clk *clk_parent)
> > +{
> > + struct clk_hw *hw;
> > + void __iomem *sys_base = data->base;
> > + unsigned int i, id;
> > +
> > + for (i = 0; i < num_clks; i++) {
> > + struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
> > +
> > + cfg_hw->cfg.parent = __clk_get_hw(clk_parent);
> > + cfg_hw->hw.init = CLK_HW_INIT_HW(cfg_hw->cfg.name, cfg_hw->cfg.parent,
> > + &mpfs_clk_cfg_ops, cfg_hw->cfg.flags);
> > + hw = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
> > + if (IS_ERR(hw)) {
> > + dev_err(dev, "failed to register clock %s\n", cfg_hw->cfg.name);
> > + goto err_clk;
> > + }
> > +
> > + id = cfg_hws[i].cfg.id;
> > + data->hw_data.hws[id] = hw;
> > + }
> > +
> > + return 0;
> > +
> > +err_clk:
> > + while (i--)
> > + devm_clk_hw_unregister(dev, data->hw_data.hws[cfg_hws[i].cfg.id]);
>
> > + clk_parent = devm_clk_get(dev, NULL);
>
> Use clk_parent_data instead please.
>
> > + if (IS_ERR(clk_parent))
> > + return PTR_ERR(clk_parent);
Please correct me if I am misinterpreting:
I had the devm_clk_get() in there to pickup the refclk from the device
tree as a result of previous feedback. I have replaced this with the
following, which I have found in several other drivers - does it achieve
the same thing?
If it does, all of the args to CLK_HW_INIT_PARENTS_DATA are now set at
compile time & I will take CLK_HW_INIT_PARENTS_DATA back out of this
function.
static struct clk_parent_data mpfs_cfg_parent[] = {
{ .index = 0 },
};
static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
unsigned int num_clks, struct mpfs_clock_data *data)
{
void __iomem *sys_base = data->base;
unsigned int i, id;
int ret;
for (i = 0; i < num_clks; i++) {
struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
cfg_hw->hw.init = CLK_HW_INIT_PARENTS_DATA(cfg_hw->cfg.name, mpfs_cfg_parent,
&mpfs_clk_cfg_ops, cfg_hw->cfg.flags);
ret = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
if (ret) {
dev_err_probe(dev, ret, "failed to register clock %s\n",
cfg_hw->cfg.name);
return ret;
}
id = cfg_hws[i].cfg.id;
data->hw_data.hws[id] = &cfg_hw->hw;
}
return 0;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC
2022-01-25 13:40 ` conor.dooley
@ 2022-01-25 20:23 ` Stephen Boyd
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2022-01-25 20:23 UTC (permalink / raw)
To: conor.dooley
Cc: conor.dooley, cyril.jean, daire.mcnamara, david.abdurachmanov,
devicetree, geert, krzysztof.kozlowski, linux-clk, mturquette,
padmarao.begari, palmer, robh+dt
Quoting conor.dooley@microchip.com (2022-01-25 05:40:11)
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Quoting conor.dooley@microchip.com (2021-12-16 06:00:22)
> > > diff --git a/drivers/clk/microchip/Makefile b/drivers/clk/microchip/Makefile
> > > index f34b247e870f..0dce0b12eac4 100644
> > > --- a/drivers/clk/microchip/Makefile
> > > +++ b/drivers/clk/microchip/Makefile
>
> Snipping the rest, will/have addressed them.
>
> > > +static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
> > > + unsigned int num_clks, struct mpfs_clock_data *data,
> > > + struct clk *clk_parent)
> > > +{
> > > + struct clk_hw *hw;
> > > + void __iomem *sys_base = data->base;
> > > + unsigned int i, id;
> > > +
> > > + for (i = 0; i < num_clks; i++) {
> > > + struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
> > > +
> > > + cfg_hw->cfg.parent = __clk_get_hw(clk_parent);
> > > + cfg_hw->hw.init = CLK_HW_INIT_HW(cfg_hw->cfg.name, cfg_hw->cfg.parent,
> > > + &mpfs_clk_cfg_ops, cfg_hw->cfg.flags);
> > > + hw = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
> > > + if (IS_ERR(hw)) {
> > > + dev_err(dev, "failed to register clock %s\n", cfg_hw->cfg.name);
> > > + goto err_clk;
> > > + }
> > > +
> > > + id = cfg_hws[i].cfg.id;
> > > + data->hw_data.hws[id] = hw;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +err_clk:
> > > + while (i--)
> > > + devm_clk_hw_unregister(dev, data->hw_data.hws[cfg_hws[i].cfg.id]);
> >
> > > + clk_parent = devm_clk_get(dev, NULL);
> >
> > Use clk_parent_data instead please.
> >
> > > + if (IS_ERR(clk_parent))
> > > + return PTR_ERR(clk_parent);
>
>
> Please correct me if I am misinterpreting:
> I had the devm_clk_get() in there to pickup the refclk from the device
> tree as a result of previous feedback. I have replaced this with the
> following, which I have found in several other drivers - does it achieve
> the same thing?
> If it does, all of the args to CLK_HW_INIT_PARENTS_DATA are now set at
> compile time & I will take CLK_HW_INIT_PARENTS_DATA back out of this
> function.
>
> static struct clk_parent_data mpfs_cfg_parent[] = {
> { .index = 0 },
> };
Yes this should be sufficient. Make it const though.
>
> static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
> unsigned int num_clks, struct mpfs_clock_data *data)
> {
> void __iomem *sys_base = data->base;
> unsigned int i, id;
> int ret;
>
> for (i = 0; i < num_clks; i++) {
> struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
>
> cfg_hw->hw.init = CLK_HW_INIT_PARENTS_DATA(cfg_hw->cfg.name, mpfs_cfg_parent,
> &mpfs_clk_cfg_ops, cfg_hw->cfg.flags);
>
> ret = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
> if (ret) {
> dev_err_probe(dev, ret, "failed to register clock %s\n",
> cfg_hw->cfg.name);
> return ret;
> }
>
> id = cfg_hws[i].cfg.id;
> data->hw_data.hws[id] = &cfg_hw->hw;
> }
>
> return 0;
> }
Looks good. Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-25 20:23 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 14:00 [PATCH v9 0/2] Add clkcfg driver for Microchip PolarFire SoC conor.dooley
2021-12-16 14:00 ` [PATCH v9 1/2] dt-bindings: clk: microchip: Add Microchip PolarFire host binding conor.dooley
2022-01-25 0:26 ` Stephen Boyd
2021-12-16 14:00 ` [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC conor.dooley
2022-01-25 0:48 ` Stephen Boyd
2022-01-25 13:40 ` conor.dooley
2022-01-25 20:23 ` Stephen Boyd
2022-01-10 10:24 ` [PATCH v9 0/2] Add clkcfg " Conor.Dooley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.