clk: add BCM63268 timer clock and reset
Subsystems (3)
v2 cover 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)
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
Signature · 1 line
2.20.1
PATCH v2 1/4 dt-bindings: clk: add BCM63268 timer clock definitions
Add missing timer clock definitions for BCM63268.
---
v2: change commit title, as suggested by Stephen Boyd
include/dt-bindings/clock/bcm63268-clock.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
include/dt-bindings/clock/bcm63268-clock.h +13 -0
@@ -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 */
Signature · 1 line
2.20.1
PATCH v2 2/4 dt-bindings: reset: add BCM63268 timer reset definitions
In reply to Álvaro Fernández RojasAdd missing timer reset definitions for BCM63268.
---
v2: change commit title, as suggested by Stephen Boyd
include/dt-bindings/reset/bcm63268-reset.h | 4 ++++ 1 file changed, 4 insertions(+)
include/dt-bindings/reset/bcm63268-reset.h +4 -0
@@ -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 */
Signature · 1 line
2.20.1
PATCH v2 3/4 dt-bindings: clock: Add BCM63268 timer binding
In reply to Álvaro Fernández RojasDocument the Broadcom BCM63268 Clock and Reset controller.
---
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
Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml new file +40 -0
@@ -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>; + };
Signature · 1 line
2.20.1
PATCH v2 4/4 clk: bcm: Add BCM63268 timer clock and reset driver
In reply to Álvaro Fernández RojasAdd driver for BCM63268 timer clock and reset controller.
---
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
drivers/clk/bcm/Kconfig +9 -0
@@ -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
drivers/clk/bcm/Makefile +1 -0
@@ -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
drivers/clk/bcm/clk-bcm63268-timer.c new file +231 -0
231 lines not on this page · raw message
Signature · 1 line
2.20.1
kernel test robot
build botRe: [PATCH v2 4/4] clk: bcm: Add BCM63268 timer clock and reset driver
Thank you for the patch! Yet something to improve:
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
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_lockcc1: 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
v3 cover clk: add BCM63268 timer clock and reset
In reply to Álvaro Fernández RojasBroadcom 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
Signature · 1 line
2.30.2
PATCH v3 1/4 dt-bindings: clk: add BCM63268 timer clock definitions
Add missing timer clock definitions for BCM63268.
---
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(+)
include/dt-bindings/clock/bcm63268-clock.h +13 -0
@@ -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 */
Signature · 1 line
2.30.2
PATCH v3 2/4 dt-bindings: reset: add BCM63268 timer reset definitions
In reply to Álvaro Fernández RojasAdd missing timer reset definitions for BCM63268.
---
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(+)
include/dt-bindings/reset/bcm63268-reset.h +4 -0
@@ -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 */
Signature · 1 line
2.30.2
PATCH v3 3/4 dt-bindings: clock: Add BCM63268 timer binding
In reply to Álvaro Fernández RojasDocument the Broadcom BCM63268 Clock and Reset controller.
---
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
Documentation/devicetree/bindings/clock/brcm,bcm63268-timer-clocks.yaml new file +40 -0
@@ -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>; + };
Signature · 1 line
2.30.2
PATCH v3 4/4 clk: bcm: Add BCM63268 timer clock and reset driver
In reply to Álvaro Fernández RojasAdd driver for BCM63268 timer clock and reset controller.
---
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
drivers/clk/bcm/Kconfig +9 -0
@@ -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
drivers/clk/bcm/Makefile +1 -0
@@ -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
drivers/clk/bcm/clk-bcm63268-timer.c new file +232 -0
232 lines not on this page · raw message
Signature · 1 line
2.30.2
Stephen Boyd
maintainerRe: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver
Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22)
Quoted 9 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +3
-0@@ -0,0 +1,232 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* BCM63268 Timer Clock and Reset Controller Driver
[...]
Quoted 6 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +6
-0+ +static inline struct bcm63268_tclkrst_hw * +to_bcm63268_timer_reset (struct reset_controller_dev *rcdev) +{ + return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev); +}
drivers/clk/bcm/clk-bcm63268-timer.c line 90
Quoted 10 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +10
-0+ +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.
drivers/clk/bcm/clk-bcm63268-timer.c line 95
Quoted 5 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +5
-0+if (assert) + val &= ~BIT (id); +else + val |=BIT (id); +__raw_writel (val, reset->regs);
Same.
Quoted 5 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +5
-0+spin_unlock_irqrestore (&reset->lock, flags); + +return 0 ; +} +
[...]
drivers/clk/bcm/clk-bcm63268-timer.c line 156
Quoted 11 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +11
-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.
Quoted 16 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +16
-0+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); +
drivers/clk/bcm/clk-bcm63268-timer.c line 182
Quoted 10 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +10
-0+ 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?
Quoted 18 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +18
-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; +
drivers/clk/bcm/clk-bcm63268-timer.c line 210
Quoted 10 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +10
-0+ 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?
drivers/clk/bcm/clk-bcm63268-timer.c line 219
Quoted 9 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +9
-0+ } + +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
Quoted 5 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +5
-0+ }, { +/* sentinel */ + } +}; +
Re: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver
Quoted 22 lines · Stephen Boyd
Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22)
Quoted 9 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +3
-0@@ -0,0 +1,232 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* BCM63268 Timer Clock and Reset Controller Driver [...]
Quoted 11 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +11
-0+ +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);
Quoted 8 lines
drivers/clk/bcm/clk-bcm63268-timer.c line 90
Quoted 5 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +5
-0+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.
Signature · 1 line
Florian
Stephen Boyd
maintainerRe: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver
Quoting Florian Fainelli (2023-03-21 16:00:29)
Quoted 32 lines · Stephen Boyd
Quoted 22 lines
Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22)
Quoted 9 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +3
-0@@ -0,0 +1,232 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* BCM63268 Timer Clock and Reset Controller Driver [...]
Quoted 11 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +11
-0+ +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);Quoted 8 lines
drivers/clk/bcm/clk-bcm63268-timer.c line 90
Quoted 5 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +5
-0+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.
Quoted 4 lines
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
Quoted 33 lines · Stephen Boyd
Quoting Florian Fainelli (2023-03-21 16:00:29)
Quoted 32 lines · Stephen Boyd
Quoted 30 lines
Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22)
Quoted 9 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +3
-0@@ -0,0 +1,232 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* BCM63268 Timer Clock and Reset Controller Driver [...]
Quoted 6 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +6
-0+ +static inline struct bcm63268_tclkrst_hw * +to_bcm63268_timer_reset (struct reset_controller_dev *rcdev) +{ + return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev); +}drivers/clk/bcm/clk-bcm63268-timer.c line 90
Quoted 10 lines
drivers/clk/bcm/clk-bcm63268-timer.c new file +10
-0+ +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.
Quoted 6 lines
Quoted 4 lines
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.
Signature · 1 line
Florian
Stephen Boyd
maintainerRe: [PATCH v3 4/4] clk: bcm: Add BCM63268 timer clock and reset driver
Quoting Florian Fainelli (2023-03-21 16:09:54)
Quoted 8 lines · Stephen Boyd
Quoted 7 lines
Quoting Florian Fainelli (2023-03-21 16:00:29)
Quoted 5 lines
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.
Quoted 5 lines
Quoted 1 line
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
Quoted 16 lines · Stephen Boyd
Quoting Florian Fainelli (2023-03-21 16:09:54)
Quoted 8 lines · Stephen Boyd
Quoted 7 lines
Quoting Florian Fainelli (2023-03-21 16:00:29)
Quoted 5 lines
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.Quoted 5 lines
Quoted 1 line
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.
Quoted 5 lines
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
In reply to Stephen BoydQuoted 15 lines · Stephen Boyd
Quoting Florian Fainelli (2023-03-21 16:09:54)
Quoted 8 lines · Stephen Boyd
Quoted 7 lines
Quoting Florian Fainelli (2023-03-21 16:00:29)
Quoted 5 lines
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.Quoted 5 lines
Quoted 1 line
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.
Quoted 5 lines
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.
Signature · 1 line
Florian
Stephen Boyd
maintainerQuoting Álvaro Fernández Rojas (2023-03-21 13:10:18)
Quoted 15 lines
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)
Quoted 6 lines
#define
GPHY_1_ENERGY_DETECT_ENABLE(1 << 3)
#defineEPHY_3_ENERGY_DETECT_ENABLE(1 << 2)
#defineEPHY_2_ENERGY_DETECT_ENABLE(1 << 1)
#defineEPHY_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.
Quoted 22 lines · Stephen Boyd
Quoting Álvaro Fernández Rojas (2023-03-21 13:10:18)
Quoted 20 lines
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)
#defineEPHY_3_ENERGY_DETECT_ENABLE(1 << 2)
#defineEPHY_2_ENERGY_DETECT_ENABLE(1 << 1)
#defineEPHY_1_ENERGY_DETECT_ENABLE(1 << 0)
Quoted 5 lines
Quoted 1 line
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