2021-02-25 19:54:22

by Álvaro Fernández Rojas

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

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


2021-02-25 19:54:51

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH 3/4] dt-bindings: clock: Add BCM63268 timer binding

Document the Broadcom BCM63268 Clock and Reset controller.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
.../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 <[email protected]>
+
+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

2021-02-25 19:55:37

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH 2/4] mips: bmips: add BCM63268 timer reset definitions

Add missing timer reset definitions for BCM63268.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
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

2021-02-25 19:56:34

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH 1/4] mips: bmips: add BCM63268 timer clock definitions

Add missing timer clock definitions for BCM63268.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
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

2021-02-25 19:56:50

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH 4/4] clk: bcm: Add BCM63268 timer clock and reset driver

Add driver for BCM63268 timer clock and reset controller.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
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 <[email protected]>
+ */
+
+#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

2021-03-06 21:13:28

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> include/dt-bindings/clock/bcm63268-clock.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>

Acked-by: Rob Herring <[email protected]>

2021-03-06 21:19:22

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> 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
>

2021-03-06 21:24:33

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> .../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 <[email protected]>

2021-03-07 10:27:22

by Álvaro Fernández Rojas

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

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 <[email protected]>
>> ---
>> 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.

2021-03-09 19:23:54

by Rob Herring (Arm)

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

On Sun, Mar 7, 2021 at 3:08 AM Álvaro Fernández Rojas <[email protected]> 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 <[email protected]>
> >> ---
> >> 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 <[email protected]>

2021-03-13 22:56:04

by Stephen Boyd

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

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 <[email protected]>
> + */
> +
> +#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]);
> + }
> +

2021-03-13 23:00:11

by Stephen Boyd

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

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