[PATCH v2 0/4] clk: add BCM63268 timer clock and reset

[PATCH v2 0/4] clk: add BCM63268 timer clock and reset

[Álvaro Fernández Rojas]

Broadcom BCM63268 has a timer clock and reset controller which has the
following layout:
  #define POR_RESET_STATUS            (1 << 31)
  #define HW_RESET_STATUS             (1 << 30)
  #define SW_RESET_STATUS             (1 << 29)
  #define USB_REF_CLKEN               (1 << 18)
  #define UTO_EXTIN_CLKEN             (1 << 17)
  #define UTO_CLK50_SEL               (1 << 16)
  #define FAP2_PLL_CLKEN              (1 << 15)
  #define FAP2_PLL_FREQ_SHIFT         12
  #define FAP1_PLL_CLKEN              (1 << 11)
  #define FAP1_PLL_FREQ_SHIFT         8
  #define WAKEON_DSL                  (1 << 7)
  #define WAKEON_EPHY                 (1 << 6)
  #define DSL_ENERGY_DETECT_ENABLE    (1 << 4)
  #define GPHY_1_ENERGY_DETECT_ENABLE (1 << 3)
  #define EPHY_3_ENERGY_DETECT_ENABLE (1 << 2)
  #define EPHY_2_ENERGY_DETECT_ENABLE (1 << 1)
  #define EPHY_1_ENERGY_DETECT_ENABLE (1 << 0)

v2: add changes suggested by Stephen Boyd.

Álvaro Fernández Rojas (4):
  dt-bindings: clk: add BCM63268 timer clock definitions
  dt-bindings: reset: add BCM63268 timer reset definitions
  dt-bindings: clock: Add BCM63268 timer binding
  clk: bcm: Add BCM63268 timer clock and reset driver

 .../clock/brcm,bcm63268-timer-clocks.yaml     |  40 +++
 drivers/clk/bcm/Kconfig                       |   9 +
 drivers/clk/bcm/Makefile                      |   1 +
 drivers/clk/bcm/clk-bcm63268-timer.c          | 231 ++++++++++++++++++
 include/dt-bindings/clock/bcm63268-clock.h    |  13 +
 include/dt-bindings/reset/bcm63268-reset.h    |   4 +
 6 files changed, 298 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml
 create mode 100644 drivers/clk/bcm/clk-bcm63268-timer.c

-- 
2.20.1

[PATCH v2 1/4] dt-bindings: clk: add BCM63268 timer clock definitions

[Álvaro Fernández Rojas]

Add missing timer clock definitions for BCM63268.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 v2: change commit title, as suggested by Stephen Boyd

 include/dt-bindings/clock/bcm63268-clock.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/dt-bindings/clock/bcm63268-clock.h b/include/dt-bindings/clock/bcm63268-clock.h
index da23e691d359..dea8adc8510e 100644
--- a/include/dt-bindings/clock/bcm63268-clock.h
+++ b/include/dt-bindings/clock/bcm63268-clock.h
@@ -27,4 +27,17 @@
 #define BCM63268_CLK_TBUS	27
 #define BCM63268_CLK_ROBOSW250	31
 
+#define BCM63268_TCLK_EPHY1		0
+#define BCM63268_TCLK_EPHY2		1
+#define BCM63268_TCLK_EPHY3		2
+#define BCM63268_TCLK_GPHY1		3
+#define BCM63268_TCLK_DSL		4
+#define BCM63268_TCLK_WAKEON_EPHY	6
+#define BCM63268_TCLK_WAKEON_DSL	7
+#define BCM63268_TCLK_FAP1		11
+#define BCM63268_TCLK_FAP2		15
+#define BCM63268_TCLK_UTO_50		16
+#define BCM63268_TCLK_UTO_EXTIN		17
+#define BCM63268_TCLK_USB_REF		18
+
 #endif /* __DT_BINDINGS_CLOCK_BCM63268_H */
-- 
2.20.1

[PATCH v2 2/4] dt-bindings: reset: add BCM63268 timer reset definitions

[Álvaro Fernández Rojas]

Add missing timer reset definitions for BCM63268.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 v2: change commit title, as suggested by Stephen Boyd

 include/dt-bindings/reset/bcm63268-reset.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/reset/bcm63268-reset.h b/include/dt-bindings/reset/bcm63268-reset.h
index 6a6403a4c2d5..d87a7882782a 100644
--- a/include/dt-bindings/reset/bcm63268-reset.h
+++ b/include/dt-bindings/reset/bcm63268-reset.h
@@ -23,4 +23,8 @@
 #define BCM63268_RST_PCIE_HARD	17
 #define BCM63268_RST_GPHY	18
 
+#define BCM63268_TRST_SW	29
+#define BCM63268_TRST_HW	30
+#define BCM63268_TRST_POR	31
+
 #endif /* __DT_BINDINGS_RESET_BCM63268_H */
-- 
2.20.1

[PATCH v2 3/4] dt-bindings: clock: Add BCM63268 timer binding

[Álvaro Fernández Rojas]

Document the Broadcom BCM63268 Clock and Reset controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 v2: no changes

 .../clock/brcm,bcm63268-timer-clocks.yaml     | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml

diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml b/Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml
new file mode 100644
index 000000000000..199818b2fb6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/brcm,bcm63268-timer-clocks.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM63268 Timer Clock and Reset Device Tree Bindings
+
+maintainers:
+  - Álvaro Fernández Rojas <noltari@gmail.com>
+
+properties:
+  compatible:
+    const: brcm,bcm63268-timer-clocks
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    timer_clk: clock-controller@100000ac {
+      compatible = "brcm,bcm63268-timer-clocks";
+      reg = <0x100000ac 0x4>;
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+    };
-- 
2.20.1

[PATCH v2 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Álvaro Fernández Rojas]

Add driver for BCM63268 timer clock and reset controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: add changes suggested by Stephen Boyd

 drivers/clk/bcm/Kconfig              |   9 ++
 drivers/clk/bcm/Makefile             |   1 +
 drivers/clk/bcm/clk-bcm63268-timer.c | 231 +++++++++++++++++++++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 drivers/clk/bcm/clk-bcm63268-timer.c

diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
index ec738f74a026..952c3b6ff71a 100644
--- a/drivers/clk/bcm/Kconfig
+++ b/drivers/clk/bcm/Kconfig
@@ -37,6 +37,15 @@ config CLK_BCM_63XX_GATE
 	  Enable common clock framework support for Broadcom BCM63xx DSL SoCs
 	  based on the MIPS architecture
 
+config CLK_BCM63268_TIMER
+	bool "Broadcom BCM63268 timer clock and reset support"
+	depends on BMIPS_GENERIC || COMPILE_TEST
+	default BMIPS_GENERIC
+	select RESET_CONTROLLER
+	help
+	  Enable timer clock and reset support for Broadcom BCM63268 DSL SoCs
+	  based on the MIPS architecture.
+
 config CLK_BCM_KONA
 	bool "Broadcom Kona CCU clock support"
 	depends on ARCH_BCM_MOBILE || COMPILE_TEST
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index edb66b44cb27..d0b6f4b1fb08 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_CLK_BCM_63XX)	+= clk-bcm63xx.o
 obj-$(CONFIG_CLK_BCM_63XX_GATE)	+= clk-bcm63xx-gate.o
+obj-$(CONFIG_CLK_BCM63268_TIMER) += clk-bcm63268-timer.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-kona.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-kona-setup.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm281xx.o
diff --git a/drivers/clk/bcm/clk-bcm63268-timer.c b/drivers/clk/bcm/clk-bcm63268-timer.c
new file mode 100644
index 000000000000..c86394b434ec
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm63268-timer.c
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * BCM63268 Timer Clock and Reset Controller Driver
+ *
+ * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+
+#include <dt-bindings/clock/bcm63268-clock.h>
+
+#define BCM63268_TIMER_RESET_SLEEP_MIN_US	10000
+#define BCM63268_TIMER_RESET_SLEEP_MAX_US	20000
+
+struct bcm63268_tclkrst_hw {
+	void __iomem *regs;
+	spinlock_t lock;
+
+	struct reset_controller_dev rcdev;
+	struct clk_hw_onecell_data data;
+};
+
+struct bcm63268_tclk_table_entry {
+	const char * const name;
+	u8 bit;
+};
+
+static const struct bcm63268_tclk_table_entry bcm63268_timer_clocks[] = {
+	{
+		.name = "ephy1",
+		.bit = BCM63268_TCLK_EPHY1,
+	}, {
+		.name = "ephy2",
+		.bit = BCM63268_TCLK_EPHY2,
+	}, {
+		.name = "ephy3",
+		.bit = BCM63268_TCLK_EPHY3,
+	}, {
+		.name = "gphy1",
+		.bit = BCM63268_TCLK_GPHY1,
+	}, {
+		.name = "dsl",
+		.bit = BCM63268_TCLK_DSL,
+	}, {
+		.name = "wakeon_ephy",
+		.bit = BCM63268_TCLK_WAKEON_EPHY,
+	}, {
+		.name = "wakeon_dsl",
+		.bit = BCM63268_TCLK_WAKEON_DSL,
+	}, {
+		.name = "fap1_pll",
+		.bit = BCM63268_TCLK_FAP1,
+	}, {
+		.name = "fap2_pll",
+		.bit = BCM63268_TCLK_FAP2,
+	}, {
+		.name = "uto_50",
+		.bit = BCM63268_TCLK_UTO_50,
+	}, {
+		.name = "uto_extin",
+		.bit = BCM63268_TCLK_UTO_EXTIN,
+	}, {
+		.name = "usb_ref",
+		.bit = BCM63268_TCLK_USB_REF,
+	}, {
+		/* sentinel */
+	}
+};
+
+static inline struct bcm63268_tclkrst_hw *
+to_bcm63268_timer_reset(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev);
+}
+
+static int bcm63268_timer_reset_update(struct reset_controller_dev *rcdev,
+				unsigned long id, bool assert)
+{
+	struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
+	unsigned long flags;
+	uint32_t val;
+
+	spin_lock_irqsave(&reset->lock, flags);
+	val = __raw_readl(reset->regs);
+	if (assert)
+		val &= ~BIT(id);
+	else
+		val |= BIT(id);
+	__raw_writel(val, reset->regs);
+	spin_unlock_irqrestore(&reset->lock, flags);
+
+	return 0;
+}
+
+static int bcm63268_timer_reset_assert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	return bcm63268_timer_reset_update(rcdev, id, true);
+}
+
+static int bcm63268_timer_reset_deassert(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	return bcm63268_timer_reset_update(rcdev, id, false);
+}
+
+static int bcm63268_timer_reset_reset(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	bcm63268_timer_reset_update(rcdev, id, true);
+	usleep_range(BCM63268_TIMER_RESET_SLEEP_MIN_US,
+		     BCM63268_TIMER_RESET_SLEEP_MAX_US);
+
+	bcm63268_timer_reset_update(rcdev, id, false);
+	/*
+	 * Ensure component is taken out reset state by sleeping also after
+	 * deasserting the reset. Otherwise, the component may not be ready
+	 * for operation.
+	 */
+	usleep_range(BCM63268_TIMER_RESET_SLEEP_MIN_US,
+		     BCM63268_TIMER_RESET_SLEEP_MAX_US);
+
+	return 0;
+}
+
+static int bcm63268_timer_reset_status(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
+
+	return !(__raw_readl(reset->regs) & BIT(id));
+}
+
+static struct reset_control_ops bcm63268_timer_reset_ops = {
+	.assert = bcm63268_timer_reset_assert,
+	.deassert = bcm63268_timer_reset_deassert,
+	.reset = bcm63268_timer_reset_reset,
+	.status = bcm63268_timer_reset_status,
+};
+
+static int bcm63268_tclk_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct bcm63268_tclk_table_entry *entry, *table;
+	struct bcm63268_tclkrst_hw *hw;
+	struct clk_hw *clk;
+	u8 maxbit = 0;
+	int i, ret;
+
+	table = of_device_get_match_data(dev);
+	if (!table)
+		return -EINVAL;
+
+	for (entry = table; entry->name; entry++)
+		maxbit = max(maxbit, entry->bit);
+	maxbit++;
+
+	hw = devm_kzalloc(&pdev->dev, struct_size(hw, data.hws, maxbit),
+			  GFP_KERNEL);
+	if (!hw)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, hw);
+
+	spin_lock_init(&hw->lock);
+
+	hw->data.num = maxbit;
+	for (i = 0; i < maxbit; i++)
+		hw->data.hws[i] = ERR_PTR(-ENODEV);
+
+	hw->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(hw->regs))
+		return PTR_ERR(hw->regs);
+
+	for (entry = table; entry->name; entry++) {
+		clk = clk_hw_register_gate(dev, entry->name, NULL, 0,
+					   hw->regs, entry->bit,
+					   CLK_GATE_BIG_ENDIAN, &hw->lock);
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+			goto out_err;
+		}
+
+		hw->data.hws[entry->bit] = clk;
+	}
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+					  &hw->data);
+	if (ret)
+		return ret;
+
+	hw->rcdev.of_node = dev->of_node;
+	hw->rcdev.ops = &bcm63268_timer_reset_ops;
+
+	ret = devm_reset_controller_register(dev, &hw->rcdev);
+	if (ret)
+		dev_err(dev, "Failed to register reset controller\n");
+
+	return 0;
+
+out_err:
+	for (i = 0; i < hw->data.num; i++) {
+		if (!IS_ERR(hw->data.hws[i]))
+			clk_hw_unregister_gate(hw->data.hws[i]);
+	}
+
+	return ret;
+}
+
+static const struct of_device_id bcm63268_tclk_dt_ids[] = {
+	{
+		.compatible = "brcm,bcm63268-timer-clocks",
+		.data = &bcm63268_timer_clocks,
+	}, {
+		/* sentinel */
+	}
+};
+
+static struct platform_driver bcm63268_tclk = {
+	.probe = bcm63268_tclk_probe,
+	.driver = {
+		.name = "bcm63268-timer-clock",
+		.of_match_table = bcm63268_tclk_dt_ids,
+	},
+};
+builtin_platform_driver(bcm63268_tclk);
-- 
2.20.1

Re: [PATCH v2 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2968 bytes --]

Hi "Álvaro,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on robh/for-next pza/reset/next v5.12-rc3 next-20210319]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/lvaro-Fern-ndez-Rojas/clk-add-BCM63268-timer-clock-and-reset/20210315-202837
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/f163044e385e4769e7b9c8ea16f34575f3db4516
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review lvaro-Fern-ndez-Rojas/clk-add-BCM63268-timer-clock-and-reset/20210315-202837
        git checkout f163044e385e4769e7b9c8ea16f34575f3db4516
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/clk/bcm/clk-bcm63268-timer.c: In function 'bcm63268_timer_reset_update':
>> drivers/clk/bcm/clk-bcm63268-timer.c:89:8: error: implicit declaration of function '__raw_readl'; did you mean '__raw_read_lock'? [-Werror=implicit-function-declaration]
      89 |  val = __raw_readl(reset->regs);
         |        ^~~~~~~~~~~
         |        __raw_read_lock
>> drivers/clk/bcm/clk-bcm63268-timer.c:94:2: error: implicit declaration of function '__raw_writel'; did you mean '__raw_write_lock'? [-Werror=implicit-function-declaration]
      94 |  __raw_writel(val, reset->regs);
         |  ^~~~~~~~~~~~
         |  __raw_write_lock
   cc1: some warnings being treated as errors


vim +89 drivers/clk/bcm/clk-bcm63268-timer.c

    80	
    81	static int bcm63268_timer_reset_update(struct reset_controller_dev *rcdev,
    82					unsigned long id, bool assert)
    83	{
    84		struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
    85		unsigned long flags;
    86		uint32_t val;
    87	
    88		spin_lock_irqsave(&reset->lock, flags);
  > 89		val = __raw_readl(reset->regs);
    90		if (assert)
    91			val &= ~BIT(id);
    92		else
    93			val |= BIT(id);
  > 94		__raw_writel(val, reset->regs);
    95		spin_unlock_irqrestore(&reset->lock, flags);
    96	
    97		return 0;
    98	}
    99	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 77622 bytes --]

[PATCH v3 0/4] clk: add BCM63268 timer clock and reset

[Álvaro Fernández Rojas]

Broadcom BCM63268 has a timer clock and reset controller which has the
following layout:
  #define POR_RESET_STATUS            (1 << 31)
  #define HW_RESET_STATUS             (1 << 30)
  #define SW_RESET_STATUS             (1 << 29)
  #define USB_REF_CLKEN               (1 << 18)
  #define UTO_EXTIN_CLKEN             (1 << 17)
  #define UTO_CLK50_SEL               (1 << 16)
  #define FAP2_PLL_CLKEN              (1 << 15)
  #define FAP2_PLL_FREQ_SHIFT         12
  #define FAP1_PLL_CLKEN              (1 << 11)
  #define FAP1_PLL_FREQ_SHIFT         8
  #define WAKEON_DSL                  (1 << 7)
  #define WAKEON_EPHY                 (1 << 6)
  #define DSL_ENERGY_DETECT_ENABLE    (1 << 4)
  #define GPHY_1_ENERGY_DETECT_ENABLE (1 << 3)
  #define EPHY_3_ENERGY_DETECT_ENABLE (1 << 2)
  #define EPHY_2_ENERGY_DETECT_ENABLE (1 << 1)
  #define EPHY_1_ENERGY_DETECT_ENABLE (1 << 0)

Also excuse me for the delay in the v3, but I totally forgot about this...

v3: add missing <linux/io.h> include to fix build warning
v2: add changes suggested by Stephen Boyd.

Álvaro Fernández Rojas (4):
  dt-bindings: clk: add BCM63268 timer clock definitions
  dt-bindings: reset: add BCM63268 timer reset definitions
  dt-bindings: clock: Add BCM63268 timer binding
  clk: bcm: Add BCM63268 timer clock and reset driver

 .../clock/brcm,bcm63268-timer-clocks.yaml     |  40 +++
 drivers/clk/bcm/Kconfig                       |   9 +
 drivers/clk/bcm/Makefile                      |   1 +
 drivers/clk/bcm/clk-bcm63268-timer.c          | 232 ++++++++++++++++++
 include/dt-bindings/clock/bcm63268-clock.h    |  13 +
 include/dt-bindings/reset/bcm63268-reset.h    |   4 +
 6 files changed, 299 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml
 create mode 100644 drivers/clk/bcm/clk-bcm63268-timer.c

-- 
2.30.2

[PATCH v3 1/4] dt-bindings: clk: add BCM63268 timer clock definitions

[Álvaro Fernández Rojas]

Add missing timer clock definitions for BCM63268.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 v3: no changes
 v2: change commit title, as suggested by Stephen Boyd

 include/dt-bindings/clock/bcm63268-clock.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/dt-bindings/clock/bcm63268-clock.h b/include/dt-bindings/clock/bcm63268-clock.h
index da23e691d359..dea8adc8510e 100644
--- a/include/dt-bindings/clock/bcm63268-clock.h
+++ b/include/dt-bindings/clock/bcm63268-clock.h
@@ -27,4 +27,17 @@
 #define BCM63268_CLK_TBUS	27
 #define BCM63268_CLK_ROBOSW250	31
 
+#define BCM63268_TCLK_EPHY1		0
+#define BCM63268_TCLK_EPHY2		1
+#define BCM63268_TCLK_EPHY3		2
+#define BCM63268_TCLK_GPHY1		3
+#define BCM63268_TCLK_DSL		4
+#define BCM63268_TCLK_WAKEON_EPHY	6
+#define BCM63268_TCLK_WAKEON_DSL	7
+#define BCM63268_TCLK_FAP1		11
+#define BCM63268_TCLK_FAP2		15
+#define BCM63268_TCLK_UTO_50		16
+#define BCM63268_TCLK_UTO_EXTIN		17
+#define BCM63268_TCLK_USB_REF		18
+
 #endif /* __DT_BINDINGS_CLOCK_BCM63268_H */
-- 
2.30.2

[PATCH v3 2/4] dt-bindings: reset: add BCM63268 timer reset definitions

[Álvaro Fernández Rojas]

Add missing timer reset definitions for BCM63268.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 v3: no changes
 v2: change commit title, as suggested by Stephen Boyd

 include/dt-bindings/reset/bcm63268-reset.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/reset/bcm63268-reset.h b/include/dt-bindings/reset/bcm63268-reset.h
index 6a6403a4c2d5..d87a7882782a 100644
--- a/include/dt-bindings/reset/bcm63268-reset.h
+++ b/include/dt-bindings/reset/bcm63268-reset.h
@@ -23,4 +23,8 @@
 #define BCM63268_RST_PCIE_HARD	17
 #define BCM63268_RST_GPHY	18
 
+#define BCM63268_TRST_SW	29
+#define BCM63268_TRST_HW	30
+#define BCM63268_TRST_POR	31
+
 #endif /* __DT_BINDINGS_RESET_BCM63268_H */
-- 
2.30.2

[PATCH v3 3/4] dt-bindings: clock: Add BCM63268 timer binding

[Álvaro Fernández Rojas]

Document the Broadcom BCM63268 Clock and Reset controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 v3: no changes
 v2: no changes

 .../clock/brcm,bcm63268-timer-clocks.yaml     | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml

diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml b/Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml
new file mode 100644
index 000000000000..199818b2fb6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/brcm,bcm63268-timer-clocks.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM63268 Timer Clock and Reset Device Tree Bindings
+
+maintainers:
+  - Álvaro Fernández Rojas <noltari@gmail.com>
+
+properties:
+  compatible:
+    const: brcm,bcm63268-timer-clocks
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    timer_clk: clock-controller@100000ac {
+      compatible = "brcm,bcm63268-timer-clocks";
+      reg = <0x100000ac 0x4>;
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+    };
-- 
2.30.2

[PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Álvaro Fernández Rojas]

Add driver for BCM63268 timer clock and reset controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v3: add missing <linux/io.h> include to fix build warning
 v2: add changes suggested by Stephen Boyd

 drivers/clk/bcm/Kconfig              |   9 ++
 drivers/clk/bcm/Makefile             |   1 +
 drivers/clk/bcm/clk-bcm63268-timer.c | 232 +++++++++++++++++++++++++++
 3 files changed, 242 insertions(+)
 create mode 100644 drivers/clk/bcm/clk-bcm63268-timer.c

diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
index 77266afb1c79..a972d763eb77 100644
--- a/drivers/clk/bcm/Kconfig
+++ b/drivers/clk/bcm/Kconfig
@@ -37,6 +37,15 @@ config CLK_BCM_63XX_GATE
 	  Enable common clock framework support for Broadcom BCM63xx DSL SoCs
 	  based on the MIPS architecture
 
+config CLK_BCM63268_TIMER
+	bool "Broadcom BCM63268 timer clock and reset support"
+	depends on BMIPS_GENERIC || COMPILE_TEST
+	default BMIPS_GENERIC
+	select RESET_CONTROLLER
+	help
+	  Enable timer clock and reset support for Broadcom BCM63268 DSL SoCs
+	  based on the MIPS architecture.
+
 config CLK_BCM_KONA
 	bool "Broadcom Kona CCU clock support"
 	depends on ARCH_BCM_MOBILE || COMPILE_TEST
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index edb66b44cb27..d0b6f4b1fb08 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_CLK_BCM_63XX)	+= clk-bcm63xx.o
 obj-$(CONFIG_CLK_BCM_63XX_GATE)	+= clk-bcm63xx-gate.o
+obj-$(CONFIG_CLK_BCM63268_TIMER) += clk-bcm63268-timer.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-kona.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-kona-setup.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm281xx.o
diff --git a/drivers/clk/bcm/clk-bcm63268-timer.c b/drivers/clk/bcm/clk-bcm63268-timer.c
new file mode 100644
index 000000000000..6a1fdd193cb5
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm63268-timer.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * BCM63268 Timer Clock and Reset Controller Driver
+ *
+ * Copyright (C) 2023 Álvaro Fernández Rojas <noltari@gmail.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+
+#include <dt-bindings/clock/bcm63268-clock.h>
+
+#define BCM63268_TIMER_RESET_SLEEP_MIN_US	10000
+#define BCM63268_TIMER_RESET_SLEEP_MAX_US	20000
+
+struct bcm63268_tclkrst_hw {
+	void __iomem *regs;
+	spinlock_t lock;
+
+	struct reset_controller_dev rcdev;
+	struct clk_hw_onecell_data data;
+};
+
+struct bcm63268_tclk_table_entry {
+	const char * const name;
+	u8 bit;
+};
+
+static const struct bcm63268_tclk_table_entry bcm63268_timer_clocks[] = {
+	{
+		.name = "ephy1",
+		.bit = BCM63268_TCLK_EPHY1,
+	}, {
+		.name = "ephy2",
+		.bit = BCM63268_TCLK_EPHY2,
+	}, {
+		.name = "ephy3",
+		.bit = BCM63268_TCLK_EPHY3,
+	}, {
+		.name = "gphy1",
+		.bit = BCM63268_TCLK_GPHY1,
+	}, {
+		.name = "dsl",
+		.bit = BCM63268_TCLK_DSL,
+	}, {
+		.name = "wakeon_ephy",
+		.bit = BCM63268_TCLK_WAKEON_EPHY,
+	}, {
+		.name = "wakeon_dsl",
+		.bit = BCM63268_TCLK_WAKEON_DSL,
+	}, {
+		.name = "fap1_pll",
+		.bit = BCM63268_TCLK_FAP1,
+	}, {
+		.name = "fap2_pll",
+		.bit = BCM63268_TCLK_FAP2,
+	}, {
+		.name = "uto_50",
+		.bit = BCM63268_TCLK_UTO_50,
+	}, {
+		.name = "uto_extin",
+		.bit = BCM63268_TCLK_UTO_EXTIN,
+	}, {
+		.name = "usb_ref",
+		.bit = BCM63268_TCLK_USB_REF,
+	}, {
+		/* sentinel */
+	}
+};
+
+static inline struct bcm63268_tclkrst_hw *
+to_bcm63268_timer_reset(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev);
+}
+
+static int bcm63268_timer_reset_update(struct reset_controller_dev *rcdev,
+				unsigned long id, bool assert)
+{
+	struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
+	unsigned long flags;
+	uint32_t val;
+
+	spin_lock_irqsave(&reset->lock, flags);
+	val = __raw_readl(reset->regs);
+	if (assert)
+		val &= ~BIT(id);
+	else
+		val |= BIT(id);
+	__raw_writel(val, reset->regs);
+	spin_unlock_irqrestore(&reset->lock, flags);
+
+	return 0;
+}
+
+static int bcm63268_timer_reset_assert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	return bcm63268_timer_reset_update(rcdev, id, true);
+}
+
+static int bcm63268_timer_reset_deassert(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	return bcm63268_timer_reset_update(rcdev, id, false);
+}
+
+static int bcm63268_timer_reset_reset(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	bcm63268_timer_reset_update(rcdev, id, true);
+	usleep_range(BCM63268_TIMER_RESET_SLEEP_MIN_US,
+		     BCM63268_TIMER_RESET_SLEEP_MAX_US);
+
+	bcm63268_timer_reset_update(rcdev, id, false);
+	/*
+	 * Ensure component is taken out reset state by sleeping also after
+	 * deasserting the reset. Otherwise, the component may not be ready
+	 * for operation.
+	 */
+	usleep_range(BCM63268_TIMER_RESET_SLEEP_MIN_US,
+		     BCM63268_TIMER_RESET_SLEEP_MAX_US);
+
+	return 0;
+}
+
+static int bcm63268_timer_reset_status(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
+
+	return !(__raw_readl(reset->regs) & BIT(id));
+}
+
+static struct reset_control_ops bcm63268_timer_reset_ops = {
+	.assert = bcm63268_timer_reset_assert,
+	.deassert = bcm63268_timer_reset_deassert,
+	.reset = bcm63268_timer_reset_reset,
+	.status = bcm63268_timer_reset_status,
+};
+
+static int bcm63268_tclk_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct bcm63268_tclk_table_entry *entry, *table;
+	struct bcm63268_tclkrst_hw *hw;
+	struct clk_hw *clk;
+	u8 maxbit = 0;
+	int i, ret;
+
+	table = of_device_get_match_data(dev);
+	if (!table)
+		return -EINVAL;
+
+	for (entry = table; entry->name; entry++)
+		maxbit = max(maxbit, entry->bit);
+	maxbit++;
+
+	hw = devm_kzalloc(&pdev->dev, struct_size(hw, data.hws, maxbit),
+			  GFP_KERNEL);
+	if (!hw)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, hw);
+
+	spin_lock_init(&hw->lock);
+
+	hw->data.num = maxbit;
+	for (i = 0; i < maxbit; i++)
+		hw->data.hws[i] = ERR_PTR(-ENODEV);
+
+	hw->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(hw->regs))
+		return PTR_ERR(hw->regs);
+
+	for (entry = table; entry->name; entry++) {
+		clk = clk_hw_register_gate(dev, entry->name, NULL, 0,
+					   hw->regs, entry->bit,
+					   CLK_GATE_BIG_ENDIAN, &hw->lock);
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+			goto out_err;
+		}
+
+		hw->data.hws[entry->bit] = clk;
+	}
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+					  &hw->data);
+	if (ret)
+		return ret;
+
+	hw->rcdev.of_node = dev->of_node;
+	hw->rcdev.ops = &bcm63268_timer_reset_ops;
+
+	ret = devm_reset_controller_register(dev, &hw->rcdev);
+	if (ret)
+		dev_err(dev, "Failed to register reset controller\n");
+
+	return 0;
+
+out_err:
+	for (i = 0; i < hw->data.num; i++) {
+		if (!IS_ERR(hw->data.hws[i]))
+			clk_hw_unregister_gate(hw->data.hws[i]);
+	}
+
+	return ret;
+}
+
+static const struct of_device_id bcm63268_tclk_dt_ids[] = {
+	{
+		.compatible = "brcm,bcm63268-timer-clocks",
+		.data = &bcm63268_timer_clocks,
+	}, {
+		/* sentinel */
+	}
+};
+
+static struct platform_driver bcm63268_tclk = {
+	.probe = bcm63268_tclk_probe,
+	.driver = {
+		.name = "bcm63268-timer-clock",
+		.of_match_table = bcm63268_tclk_dt_ids,
+	},
+};
+builtin_platform_driver(bcm63268_tclk);
-- 
2.30.2

Re: [PATCH v3 0/4] clk: add BCM63268 timer clock and reset

[Stephen Boyd]

Quoting Álvaro Fernández Rojas (2023-03-21 13:10:18)
> Broadcom BCM63268 has a timer clock and reset controller which has the
> following layout:
>   #define POR_RESET_STATUS            (1 << 31)
>   #define HW_RESET_STATUS             (1 << 30)
>   #define SW_RESET_STATUS             (1 << 29)
>   #define USB_REF_CLKEN               (1 << 18)
>   #define UTO_EXTIN_CLKEN             (1 << 17)
>   #define UTO_CLK50_SEL               (1 << 16)
>   #define FAP2_PLL_CLKEN              (1 << 15)
>   #define FAP2_PLL_FREQ_SHIFT         12
>   #define FAP1_PLL_CLKEN              (1 << 11)
>   #define FAP1_PLL_FREQ_SHIFT         8
>   #define WAKEON_DSL                  (1 << 7)
>   #define WAKEON_EPHY                 (1 << 6)
>   #define DSL_ENERGY_DETECT_ENABLE    (1 << 4)
>   #define GPHY_1_ENERGY_DETECT_ENABLE (1 << 3)
>   #define EPHY_3_ENERGY_DETECT_ENABLE (1 << 2)
>   #define EPHY_2_ENERGY_DETECT_ENABLE (1 << 1)
>   #define EPHY_1_ENERGY_DETECT_ENABLE (1 << 0)
> 
> Also excuse me for the delay in the v3, but I totally forgot about this...

Please don't send as a reply to a previous round. It makes applying the
patch series more difficult and buries the new series deep down in the
mail thread.

Re: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Stephen Boyd]

Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22)
> diff --git a/drivers/clk/bcm/clk-bcm63268-timer.c b/drivers/clk/bcm/clk-bcm63268-timer.c
> new file mode 100644
> index 000000000000..6a1fdd193cb5
> --- /dev/null
> +++ b/drivers/clk/bcm/clk-bcm63268-timer.c
> @@ -0,0 +1,232 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * BCM63268 Timer Clock and Reset Controller Driver
[...]
> +
> +static inline struct bcm63268_tclkrst_hw *
> +to_bcm63268_timer_reset(struct reset_controller_dev *rcdev)
> +{
> +       return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev);
> +}
> +
> +static int bcm63268_timer_reset_update(struct reset_controller_dev *rcdev,
> +                               unsigned long id, bool assert)
> +{
> +       struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
> +       unsigned long flags;
> +       uint32_t val;
> +
> +       spin_lock_irqsave(&reset->lock, flags);
> +       val = __raw_readl(reset->regs);

Use regular ol readl() here, unless you have some need for no barrires
or byte swapping.

> +       if (assert)
> +               val &= ~BIT(id);
> +       else
> +               val |= BIT(id);
> +       __raw_writel(val, reset->regs);

Same.

> +       spin_unlock_irqrestore(&reset->lock, flags);
> +
> +       return 0;
> +}
> +
[...]
> +
> +static int bcm63268_tclk_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       const struct bcm63268_tclk_table_entry *entry, *table;
> +       struct bcm63268_tclkrst_hw *hw;
> +       struct clk_hw *clk;
> +       u8 maxbit = 0;
> +       int i, ret;
> +
> +       table = of_device_get_match_data(dev);

Use device_get_match_data() instead.

> +       if (!table)
> +               return -EINVAL;
> +
> +       for (entry = table; entry->name; entry++)
> +               maxbit = max(maxbit, entry->bit);
> +       maxbit++;
> +
> +       hw = devm_kzalloc(&pdev->dev, struct_size(hw, data.hws, maxbit),
> +                         GFP_KERNEL);
> +       if (!hw)
> +               return -ENOMEM;
> +
> +       platform_set_drvdata(pdev, hw);
> +
> +       spin_lock_init(&hw->lock);
> +
> +       hw->data.num = maxbit;
> +       for (i = 0; i < maxbit; i++)
> +               hw->data.hws[i] = ERR_PTR(-ENODEV);
> +
> +       hw->regs = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(hw->regs))
> +               return PTR_ERR(hw->regs);
> +
> +       for (entry = table; entry->name; entry++) {
> +               clk = clk_hw_register_gate(dev, entry->name, NULL, 0,

Use devm?

> +                                          hw->regs, entry->bit,
> +                                          CLK_GATE_BIG_ENDIAN, &hw->lock);
> +               if (IS_ERR(clk)) {
> +                       ret = PTR_ERR(clk);
> +                       goto out_err;
> +               }
> +
> +               hw->data.hws[entry->bit] = clk;
> +       }
> +
> +       ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> +                                         &hw->data);
> +       if (ret)
> +               return ret;
> +
> +       hw->rcdev.of_node = dev->of_node;
> +       hw->rcdev.ops = &bcm63268_timer_reset_ops;
> +
> +       ret = devm_reset_controller_register(dev, &hw->rcdev);
> +       if (ret)
> +               dev_err(dev, "Failed to register reset controller\n");
> +
> +       return 0;
> +
> +out_err:
> +       for (i = 0; i < hw->data.num; i++) {
> +               if (!IS_ERR(hw->data.hws[i]))
> +                       clk_hw_unregister_gate(hw->data.hws[i]);

And then drop this?

> +       }
> +
> +       return ret;
> +}
> +
> +static const struct of_device_id bcm63268_tclk_dt_ids[] = {
> +       {
> +               .compatible = "brcm,bcm63268-timer-clocks",
> +               .data = &bcm63268_timer_clocks,

Are you planning on adding more SoCs to this driver? The data can
currently be always assumed to be bcm63268_timer_clocks

> +       }, {
> +               /* sentinel */
> +       }
> +};
> +

Re: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Florian Fainelli]

On 3/21/23 15:57, Stephen Boyd wrote:
> Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22)
>> diff --git a/drivers/clk/bcm/clk-bcm63268-timer.c b/drivers/clk/bcm/clk-bcm63268-timer.c
>> new file mode 100644
>> index 000000000000..6a1fdd193cb5
>> --- /dev/null
>> +++ b/drivers/clk/bcm/clk-bcm63268-timer.c
>> @@ -0,0 +1,232 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * BCM63268 Timer Clock and Reset Controller Driver
> [...]
>> +
>> +static inline struct bcm63268_tclkrst_hw *
>> +to_bcm63268_timer_reset(struct reset_controller_dev *rcdev)
>> +{
>> +       return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev);
>> +}
>> +
>> +static int bcm63268_timer_reset_update(struct reset_controller_dev *rcdev,
>> +                               unsigned long id, bool assert)
>> +{
>> +       struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
>> +       unsigned long flags;
>> +       uint32_t val;
>> +
>> +       spin_lock_irqsave(&reset->lock, flags);
>> +       val = __raw_readl(reset->regs);
> 
> Use regular ol readl() here, unless you have some need for no barrires
> or byte swapping.

These SoCs are big-endian, require native endian register access and 
have no posted writes within their bus logic (UBUS) and require no 
barriers, hence the use of __raw_readl() and __raw_writel() is adequate.
-- 
Florian

Re: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Stephen Boyd]

Quoting Florian Fainelli (2023-03-21 16:00:29)
> On 3/21/23 15:57, Stephen Boyd wrote:
> > Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22)
> >> diff --git a/drivers/clk/bcm/clk-bcm63268-timer.c b/drivers/clk/bcm/clk-bcm63268-timer.c
> >> new file mode 100644
> >> index 000000000000..6a1fdd193cb5
> >> --- /dev/null
> >> +++ b/drivers/clk/bcm/clk-bcm63268-timer.c
> >> @@ -0,0 +1,232 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * BCM63268 Timer Clock and Reset Controller Driver
> > [...]
> >> +
> >> +static inline struct bcm63268_tclkrst_hw *
> >> +to_bcm63268_timer_reset(struct reset_controller_dev *rcdev)
> >> +{
> >> +       return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev);
> >> +}
> >> +
> >> +static int bcm63268_timer_reset_update(struct reset_controller_dev *rcdev,
> >> +                               unsigned long id, bool assert)
> >> +{
> >> +       struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
> >> +       unsigned long flags;
> >> +       uint32_t val;
> >> +
> >> +       spin_lock_irqsave(&reset->lock, flags);
> >> +       val = __raw_readl(reset->regs);
> > 
> > Use regular ol readl() here, unless you have some need for no barrires
> > or byte swapping.
> 
> These SoCs are big-endian, require native endian register access and 
> have no posted writes within their bus logic (UBUS) and require no 
> barriers, hence the use of __raw_readl() and __raw_writel() is adequate.
> 

Use ioread32be() then?

Re: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Florian Fainelli]

On 3/21/23 16:06, Stephen Boyd wrote:
> Quoting Florian Fainelli (2023-03-21 16:00:29)
>> On 3/21/23 15:57, Stephen Boyd wrote:
>>> Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22)
>>>> diff --git a/drivers/clk/bcm/clk-bcm63268-timer.c b/drivers/clk/bcm/clk-bcm63268-timer.c
>>>> new file mode 100644
>>>> index 000000000000..6a1fdd193cb5
>>>> --- /dev/null
>>>> +++ b/drivers/clk/bcm/clk-bcm63268-timer.c
>>>> @@ -0,0 +1,232 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * BCM63268 Timer Clock and Reset Controller Driver
>>> [...]
>>>> +
>>>> +static inline struct bcm63268_tclkrst_hw *
>>>> +to_bcm63268_timer_reset(struct reset_controller_dev *rcdev)
>>>> +{
>>>> +       return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev);
>>>> +}
>>>> +
>>>> +static int bcm63268_timer_reset_update(struct reset_controller_dev *rcdev,
>>>> +                               unsigned long id, bool assert)
>>>> +{
>>>> +       struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev);
>>>> +       unsigned long flags;
>>>> +       uint32_t val;
>>>> +
>>>> +       spin_lock_irqsave(&reset->lock, flags);
>>>> +       val = __raw_readl(reset->regs);
>>>
>>> Use regular ol readl() here, unless you have some need for no barrires
>>> or byte swapping.
>>
>> These SoCs are big-endian, require native endian register access and
>> have no posted writes within their bus logic (UBUS) and require no
>> barriers, hence the use of __raw_readl() and __raw_writel() is adequate.
>>
> 
> Use ioread32be() then?

BCM63xx drivers tend to use __raw_{read,write}l for consistency and to 
make it clear that no barriers, no endian swapping is necessary, I would 
prefer to remain consistent with that convention.
-- 
Florian

Re: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Stephen Boyd]

Quoting Florian Fainelli (2023-03-21 16:09:54)
> On 3/21/23 16:06, Stephen Boyd wrote:
> > Quoting Florian Fainelli (2023-03-21 16:00:29)
> >>
> >> These SoCs are big-endian, require native endian register access and
> >> have no posted writes within their bus logic (UBUS) and require no
> >> barriers, hence the use of __raw_readl() and __raw_writel() is adequate.
> >>
> > 
> > Use ioread32be() then?
> 
> BCM63xx drivers tend to use __raw_{read,write}l for consistency and to 
> make it clear that no barriers, no endian swapping is necessary, I would 
> prefer to remain consistent with that convention.

Ok.

Is the clk device big-endian? Or the CPU is big-endian? SoC being
big-endian sounds like the devices in the SoC are big-endian. I hope we
never plop this device down with a CPU that's litle-endian.

Re: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Álvaro Fernández Rojas]

El mié, 22 mar 2023 a las 0:23, Stephen Boyd (<sboyd@kernel.org>) escribió:
>
> Quoting Florian Fainelli (2023-03-21 16:09:54)
> > On 3/21/23 16:06, Stephen Boyd wrote:
> > > Quoting Florian Fainelli (2023-03-21 16:00:29)
> > >>
> > >> These SoCs are big-endian, require native endian register access and
> > >> have no posted writes within their bus logic (UBUS) and require no
> > >> barriers, hence the use of __raw_readl() and __raw_writel() is adequate.
> > >>
> > >
> > > Use ioread32be() then?
> >
> > BCM63xx drivers tend to use __raw_{read,write}l for consistency and to
> > make it clear that no barriers, no endian swapping is necessary, I would
> > prefer to remain consistent with that convention.
>
> Ok.
>
> Is the clk device big-endian? Or the CPU is big-endian? SoC being
> big-endian sounds like the devices in the SoC are big-endian. I hope we
> never plop this device down with a CPU that's litle-endian.

The SoC is big-endian. I've only worked with MIPS big-endian devices
from Broadcom, so I'm not really sure, but this seems to be very
BCM63268-specific...
Other BCM63xx devices I know have separate clock and reset
controllers, but not this kind of timer, clock and reset controller...

--
Álvaro

Re: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Florian Fainelli]

On 3/21/23 16:23, Stephen Boyd wrote:
> Quoting Florian Fainelli (2023-03-21 16:09:54)
>> On 3/21/23 16:06, Stephen Boyd wrote:
>>> Quoting Florian Fainelli (2023-03-21 16:00:29)
>>>>
>>>> These SoCs are big-endian, require native endian register access and
>>>> have no posted writes within their bus logic (UBUS) and require no
>>>> barriers, hence the use of __raw_readl() and __raw_writel() is adequate.
>>>>
>>>
>>> Use ioread32be() then?
>>
>> BCM63xx drivers tend to use __raw_{read,write}l for consistency and to
>> make it clear that no barriers, no endian swapping is necessary, I would
>> prefer to remain consistent with that convention.
> 
> Ok.
> 
> Is the clk device big-endian? Or the CPU is big-endian? SoC being
> big-endian sounds like the devices in the SoC are big-endian. I hope we
> never plop this device down with a CPU that's litle-endian.

The CPU is big endian and the peripheral and bus to access the 
peripheral are native endian, so also big endian in that case.

The newer SoCs are ARM-based and are little endian, but we already have 
a clock driver for those.
-- 
Florian

Re: [PATCH v3 0/4] clk: add BCM63268 timer clock and reset

[Álvaro Fernández Rojas]

El mar, 21 mar 2023 a las 23:54, Stephen Boyd (<sboyd@kernel.org>) escribió:
>
> Quoting Álvaro Fernández Rojas (2023-03-21 13:10:18)
> > Broadcom BCM63268 has a timer clock and reset controller which has the
> > following layout:
> >   #define POR_RESET_STATUS            (1 << 31)
> >   #define HW_RESET_STATUS             (1 << 30)
> >   #define SW_RESET_STATUS             (1 << 29)
> >   #define USB_REF_CLKEN               (1 << 18)
> >   #define UTO_EXTIN_CLKEN             (1 << 17)
> >   #define UTO_CLK50_SEL               (1 << 16)
> >   #define FAP2_PLL_CLKEN              (1 << 15)
> >   #define FAP2_PLL_FREQ_SHIFT         12
> >   #define FAP1_PLL_CLKEN              (1 << 11)
> >   #define FAP1_PLL_FREQ_SHIFT         8
> >   #define WAKEON_DSL                  (1 << 7)
> >   #define WAKEON_EPHY                 (1 << 6)
> >   #define DSL_ENERGY_DETECT_ENABLE    (1 << 4)
> >   #define GPHY_1_ENERGY_DETECT_ENABLE (1 << 3)
> >   #define EPHY_3_ENERGY_DETECT_ENABLE (1 << 2)
> >   #define EPHY_2_ENERGY_DETECT_ENABLE (1 << 1)
> >   #define EPHY_1_ENERGY_DETECT_ENABLE (1 << 0)
> >
> > Also excuse me for the delay in the v3, but I totally forgot about this...
>
> Please don't send as a reply to a previous round. It makes applying the
> patch series more difficult and buries the new series deep down in the
> mail thread.

Excuse me for that, but other kernel maintainers prefer it this way.

--
Álvaro