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

[PATCH 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)

Álvaro Fernández Rojas (4):
  mips: bmips: add BCM63268 timer clock definitions
  mips: bmips: 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.20.1

[PATCH 1/4] mips: bmips: 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>
---
 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

Re: [PATCH 1/4] mips: bmips: add BCM63268 timer clock definitions

[Rob Herring]

On Thu, 25 Feb 2021 20:41:58 +0100, Álvaro Fernández Rojas wrote:
> Add missing timer clock definitions for BCM63268.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  include/dt-bindings/clock/bcm63268-clock.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/4] mips: bmips: add BCM63268 timer clock definitions

[Stephen Boyd]

Subject should probably be clk related instead of mips prefixed. Or
dt-bindings: clk: ?

[PATCH 2/4] mips: bmips: 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>
---
 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

Re: [PATCH 2/4] mips: bmips: add BCM63268 timer reset definitions

[Rob Herring]

On Thu, Feb 25, 2021 at 08:41:59PM +0100, Álvaro Fernández Rojas wrote:
> Add missing timer reset definitions for BCM63268.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  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

Numbering should be local to the provider, so shouldn't this be 0-2? 
Unless these numbers correspond to something in the h/w (bit positions 
for example).

> +
>  #endif /* __DT_BINDINGS_RESET_BCM63268_H */
> -- 
> 2.20.1
> 

Re: [PATCH 2/4] mips: bmips: add BCM63268 timer reset definitions

[Álvaro Fernández Rojas]

Hi Rob,

El 06/03/2021 a las 22:17, Rob Herring escribió:
> On Thu, Feb 25, 2021 at 08:41:59PM +0100, Álvaro Fernández Rojas wrote:
>> Add missing timer reset definitions for BCM63268.
>>
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> ---
>>   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
> 
> Numbering should be local to the provider, so shouldn't this be 0-2?
> Unless these numbers correspond to something in the h/w (bit positions
> for example).

Numbering corresponds to bit positions in the HW:
     uint32        ClkRstCtl;
#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)

http://datashed.science/misc/bcm/gpl/broadcom-sdk-416L05/shared/opensource/include/bcm963xx/63268_map_part.h

> 
>> +
>>   #endif /* __DT_BINDINGS_RESET_BCM63268_H */
>> -- 
>> 2.20.1
>>

Best regards,
Álvaro.

Re: [PATCH 2/4] mips: bmips: add BCM63268 timer reset definitions

[Rob Herring]

On Sun, Mar 7, 2021 at 3:08 AM Álvaro Fernández Rojas <noltari@gmail.com> wrote:
>
> Hi Rob,
>
> El 06/03/2021 a las 22:17, Rob Herring escribió:
> > On Thu, Feb 25, 2021 at 08:41:59PM +0100, Álvaro Fernández Rojas wrote:
> >> Add missing timer reset definitions for BCM63268.
> >>
> >> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> >> ---
> >>   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
> >
> > Numbering should be local to the provider, so shouldn't this be 0-2?
> > Unless these numbers correspond to something in the h/w (bit positions
> > for example).
>
> Numbering corresponds to bit positions in the HW:

Okay, good.

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 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>
---
 .../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

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

[Rob Herring]

On Thu, 25 Feb 2021 20:42:00 +0100, Álvaro Fernández Rojas wrote:
> Document the Broadcom BCM63268 Clock and Reset controller.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  .../clock/brcm,bcm63268-timer-clocks.yaml     | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 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>
---
 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 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..5609c4ddb50c
--- /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) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/init.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;
+	unsigned long flags;
+};
+
+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;
+	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_t(u8, 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++) {
+		struct clk_hw *clk;
+
+		clk = clk_hw_register_gate(dev, entry->name, NULL,
+					   entry->flags, 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 = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
+				     &hw->data);
+	if (!ret)
+		return 0;
+
+	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");
+
+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 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

[Stephen Boyd]

Quoting Alvaro Fernandez Rojas (2021-02-25 11:42:01)
> diff --git a/drivers/clk/bcm/clk-bcm63268-timer.c b/drivers/clk/bcm/clk-bcm63268-timer.c
> new file mode 100644
> index 000000000000..5609c4ddb50c
> --- /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) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>

Is this used?

> +#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;
> +       unsigned long flags;

This isn't used. Drop it?

> +};
> +
> +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;
> +       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_t(u8, maxbit, entry->bit);

Is max_t() required? Hopefully just max() can be used.

> +       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++) {
> +               struct clk_hw *clk;

Please move this declaration to start of the function instead of local
to this loop.

> +
> +               clk = clk_hw_register_gate(dev, entry->name, NULL,
> +                                          entry->flags, 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 = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,

Please use devm or unregister this on failure.

> +                                    &hw->data);
> +       if (!ret)
> +               return 0;
> +
> +       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");

Why do we only register reset controller on failure to add the clk hw
provider?

> +
> +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]);
> +       }
> +