2022-01-17 10:42:59

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH 0/2] Realtek Otto timer/counter support

Realtek's family of networking SoCs (RTL838x, RTL839x, RTL930x, RTL931x)
have a set of timers. These patches seek to provide a devicetree binding
for them, and a first implementation of the driver.

Most of these SoCs can use the MIPS clocksource and clockevent timers,
but the CEVT_R4K timer on the RTL930x series was left disconnected. This
series therefore requires these timers to be able to function.

Except for the RTL838x series, these SoCs also support some form of SMP,
but this is not yet implemented as the platform's IRQ driver does not
support this yet. The driver also implements a fixed divider for the
timer's internal clock, since this is good enough at this point.

I've tested this driver on RTL8380, RTL8393, and RTL9302 SoCs, running
in single-CPU mode. When replacing the CEVT_R4K timer, these platforms
then function as expected.

There's probably still other things that I missed, but I currently have
the following questions:
- If the internal clock is added later, should this be added to the
devicetree as a self-referenced clock? It's not possible to use this
clock outside of the timer, so I guess it would mainly be to allow the
DT code to build a complete overview of the clock hierarchy.
- When adding SMP support, what would be the best way to configure the
CPU affinity? And what does FEAT_DYNIRQ imply exactly?

Best,
Sander

Sander Vanheule (2):
dt-bindings: timer: Add realtek,otto-tc binding
clocksource/drivers: Add Realtek Otto timer driver

.../bindings/timer/realtek,otto-tc.yaml | 64 ++++++
MAINTAINERS | 6 +
drivers/clocksource/Kconfig | 8 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-realtek-otto.c | 216 ++++++++++++++++++
5 files changed, 295 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
create mode 100644 drivers/clocksource/timer-realtek-otto.c

--
2.34.1


2022-01-17 11:06:43

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: timer: Add realtek,otto-tc binding

New binding for the timer/counter blocks found on the Realtek Otto MIPS
platform.

Signed-off-by: Sander Vanheule <[email protected]>
---
.../bindings/timer/realtek,otto-tc.yaml | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml

diff --git a/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml b/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
new file mode 100644
index 000000000000..12971b9ecdf5
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/realtek,otto-tc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek Otto platform timer/counter bindings
+
+description:
+ "Up-counting 28-bit timer that can operate in oneshot or repeating mode,
+ providing an interrupt at roll-over.
+
+ The timer is driven by a divided clock, derived from the bus clock. The clock
+ divisor is configurable from 2 to 65535. Divisor values of 0 and 1 disable
+ the timer clock. The timer can also be enabled or disabled independently from
+ the clock (divisor) selection.
+
+ The number of timers supported by the different SoC families is:
+ - RTL8380: 5 timers
+ - RTL8390: 5 timers
+ - RTL9300: 6 timers
+ - RTL9310: 7 timers"
+
+maintainers:
+ - Sander Vanheule <[email protected]>
+
+properties:
+ compatible:
+ const: realtek,otto-tc
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Parent clock from peripheral bus
+
+ clock-names:
+ items:
+ - const: bus
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ timer0: timer@3100 {
+ compatible = "realtek,otto-tc";
+ reg = <0x3100 0x10>;
+
+ interrupts = <29 4>;
+
+ clocks = <&lx_clk>;
+ clock-names = "bus";
+ };
--
2.34.1

2022-01-17 11:06:43

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH 2/2] clocksource/drivers: Add Realtek Otto timer driver

Driver to provide initial support for the timer/counter blocks found on
Realtek Otto MIPS SoCs.

This implementation hard enables the derived clock with a divisor of 2,
to obtain the highest possible timer resolution. A real derived and
reconfigurable timer clock can be implemented later, should the need
arise for longer timeouts.

Since this platform has other clocksources (CSRC_R4K or MIPS GIC), the
timers are only added as clockevent devices.

Signed-off-by: Sander Vanheule <[email protected]>
---
MAINTAINERS | 6 +
drivers/clocksource/Kconfig | 8 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-realtek-otto.c | 216 +++++++++++++++++++++++
4 files changed, 231 insertions(+)
create mode 100644 drivers/clocksource/timer-realtek-otto.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6d9dafd09de0..ad7c9b10fcd8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16342,6 +16342,12 @@ S: Maintained
F: include/sound/rt*.h
F: sound/soc/codecs/rt*

+REALTEK OTTO TIMER
+M: Sander Vanheule <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
+F: drivers/clocksource/timer-realtek-otto.c
+
REALTEK OTTO WATCHDOG
M: Sander Vanheule <[email protected]>
L: [email protected]
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index cfb8ea0df3b1..03557568182d 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -84,6 +84,14 @@ config IXP4XX_TIMER
help
Enables support for the Intel XScale IXP4xx SoC timer.

+config REALTEK_OTTO_TIMER
+ bool "Realtek Otto timer driver"
+ depends on MACH_REALTEK_RTL || COMPILE_TEST
+ select TIMER_OF
+ help
+ Enables support for the timer/counter found on Realtek's networking
+ SoC series RTL838x, RTL839x, RTL930x, RTL931x.
+
config ROCKCHIP_TIMER
bool "Rockchip timer driver" if COMPILE_TEST
depends on ARM || ARM64
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index fa5f624eadb6..43c792e4a574 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_DW_APB_TIMER) += dw_apb_timer.o
obj-$(CONFIG_DW_APB_TIMER_OF) += dw_apb_timer_of.o
obj-$(CONFIG_FTTMR010_TIMER) += timer-fttmr010.o
obj-$(CONFIG_IXP4XX_TIMER) += timer-ixp4xx.o
+obj-$(CONFIG_REALTEK_OTTO_TIMER) += timer-realtek-otto.o
obj-$(CONFIG_ROCKCHIP_TIMER) += timer-rockchip.o
obj-$(CONFIG_CLKSRC_NOMADIK_MTU) += nomadik-mtu.o
obj-$(CONFIG_CLKSRC_DBX500_PRCMU) += clksrc-dbx500-prcmu.o
diff --git a/drivers/clocksource/timer-realtek-otto.c b/drivers/clocksource/timer-realtek-otto.c
new file mode 100644
index 000000000000..2c07be042614
--- /dev/null
+++ b/drivers/clocksource/timer-realtek-otto.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Realtek Otto platform timer/counter
+ *
+ * Copyright
+ * Sander Vanheule - 2022
+ *
+ * Up-counting 28-bit timer that can operate in oneshot or repeat mode,
+ * providing interrupts at roll-over. The maximum value is written to the DATA
+ * register, while the current timer value can be read from the CNT register.
+ *
+ * A divided clock is used to drive the timer, derived from the bus clock. The
+ * clock divisor is configurable from 2 to 65535. Divisor values of 0 and 1
+ * disable the timer clock. The timer can also be enabled independently from
+ * the clock selection with the ENABLE flag.
+ *
+ * The SoC's interrupt controller can configure the CPU affinity of the timer
+ * interrupts to any subset of the available CPUs.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+
+#include "timer-of.h"
+
+struct otto_tc_ctrl {
+ struct timer_of to;
+ spinlock_t lock;
+};
+
+#define OTTO_TC_REG_OFFSET(to, offset) (timer_of_base(to) + offset)
+
+#define OTTO_TC_REG_DATA(to) OTTO_TC_REG_OFFSET(to, 0x0)
+#define OTTO_TC_REG_CNT(to) OTTO_TC_REG_OFFSET(to, 0x4)
+#define OTTO_TC_WIDTH 28
+#define OTTO_TC_MAX_PERIOD BIT(OTTO_TC_WIDTH)
+
+#define OTTO_TC_REG_CTL(to) OTTO_TC_REG_OFFSET(to, 0x8)
+#define OTTO_TC_CTL_ENABLE BIT(28)
+#define OTTO_TC_CTL_MODE BIT(24)
+#define OTTO_TC_MODE_ONESHOT FIELD_PREP(OTTO_TC_CTL_MODE, 0)
+#define OTTO_TC_MODE_REPEAT FIELD_PREP(OTTO_TC_CTL_MODE, 1)
+#define OTTO_TC_CTL_DIVISOR GENMASK(15, 0)
+
+#define OTTO_TC_MIN_DIVISOR 2
+#define OTTO_TC_MAX_DIVISOR OTTO_TC_CTL_DIVISOR
+
+#define OTTO_TC_REG_ICTL(to) OTTO_TC_REG_OFFSET(to, 0xC)
+#define OTTO_TC_ICTL_ENABLE BIT(20)
+#define OTTO_TC_ICTL_STATUS BIT(16)
+
+static inline void otto_tc_irq_enable_clear(struct timer_of *to)
+{
+ writel(OTTO_TC_ICTL_ENABLE | OTTO_TC_ICTL_STATUS, OTTO_TC_REG_ICTL(to));
+}
+
+static inline struct otto_tc_ctrl *otto_tc_timer_to_ctrl(struct timer_of *to)
+{
+ return container_of(to, struct otto_tc_ctrl, to);
+}
+
+static irqreturn_t otto_tc_handler(int irq, void *dev_id)
+{
+ struct clock_event_device *ced = dev_id;
+ struct timer_of *to = to_timer_of(ced);
+
+ otto_tc_irq_enable_clear(to);
+ ced->event_handler(ced);
+
+ return IRQ_HANDLED;
+}
+
+static void otto_tc_set_divisor(struct otto_tc_ctrl *ctrl, u16 divisor)
+{
+ struct timer_of *to = &ctrl->to;
+ unsigned long flags;
+ u32 val;
+
+ spin_lock_irqsave(&ctrl->lock, flags);
+ val = readl(OTTO_TC_REG_CTL(to)) & ~OTTO_TC_CTL_DIVISOR;
+ val |= FIELD_PREP(OTTO_TC_CTL_DIVISOR, divisor);
+ writel(val, OTTO_TC_REG_CTL(to));
+ spin_unlock_irqrestore(&ctrl->lock, flags);
+}
+
+static void otto_tc_set_timeout(struct clock_event_device *ced, u32 value)
+{
+ struct timer_of *to = to_timer_of(ced);
+ struct otto_tc_ctrl *ctrl = otto_tc_timer_to_ctrl(to);
+ unsigned long flags;
+
+ spin_lock_irqsave(&ctrl->lock, flags);
+ writel(value, OTTO_TC_REG_DATA(to));
+ spin_unlock_irqrestore(&ctrl->lock, flags);
+}
+
+/*
+ * Timers can only be (re)started from the disabled state. The clkevt state
+ * machine is expected perform the necessary disables (shutdown) before moving
+ * a timer into an enabled state through .set_oneshot() or .set_periodic().
+ */
+static void otto_tc_set_mode(struct clock_event_device *ced, bool started, u32 mode)
+{
+ struct timer_of *to = to_timer_of(ced);
+ struct otto_tc_ctrl *ctrl = otto_tc_timer_to_ctrl(to);
+ unsigned long flags;
+ u32 val;
+
+ spin_lock_irqsave(&ctrl->lock, flags);
+ val = readl(OTTO_TC_REG_CTL(to));
+ val = (val & ~OTTO_TC_CTL_MODE) | mode;
+ if (started)
+ val |= OTTO_TC_CTL_ENABLE;
+ else
+ val &= ~OTTO_TC_CTL_ENABLE;
+ writel(val, OTTO_TC_REG_CTL(to));
+ spin_unlock_irqrestore(&ctrl->lock, flags);
+}
+
+static int otto_tc_set_next_event(unsigned long evt, struct clock_event_device *ced)
+{
+ otto_tc_set_timeout(ced, evt);
+
+ return 0;
+}
+
+static int otto_tc_set_periodic(struct clock_event_device *ced)
+{
+ struct timer_of *to = to_timer_of(ced);
+
+ otto_tc_set_timeout(ced, timer_of_period(to));
+ otto_tc_set_mode(ced, true, OTTO_TC_MODE_REPEAT);
+
+ return 0;
+}
+
+static int otto_tc_set_oneshot(struct clock_event_device *ced)
+{
+ otto_tc_set_mode(ced, true, OTTO_TC_MODE_ONESHOT);
+
+ return 0;
+}
+
+static int otto_tc_set_oneshot_stopped(struct clock_event_device *ced)
+{
+ otto_tc_set_mode(ced, false, OTTO_TC_MODE_ONESHOT);
+
+ return 0;
+}
+
+static int otto_tc_set_shutdown(struct clock_event_device *ced)
+{
+ otto_tc_set_mode(ced, false, 0);
+
+ return 0;
+}
+
+static void __init otto_tc_init_clkevt(struct timer_of *to)
+{
+ struct clock_event_device *ced = &to->clkevt;
+
+ ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC |
+ CLOCK_EVT_FEAT_DYNIRQ;
+ ced->set_next_event = otto_tc_set_next_event;
+ ced->set_state_periodic = otto_tc_set_periodic;
+ ced->set_state_oneshot = otto_tc_set_oneshot;
+ ced->set_state_oneshot_stopped = otto_tc_set_oneshot_stopped;
+ ced->set_state_shutdown = otto_tc_set_shutdown;
+ ced->cpumask = cpumask_of(0);
+ ced->rating = 300;
+
+ clockevents_config_and_register(ced, timer_of_rate(to), 1, OTTO_TC_MAX_PERIOD);
+}
+
+static int __init otto_tc_init(struct device_node *node)
+{
+ struct otto_tc_ctrl *ctrl;
+ int err;
+
+ ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
+ if (!ctrl)
+ return -ENOMEM;
+
+ spin_lock_init(&ctrl->lock);
+
+ ctrl->to.flags = TIMER_OF_BASE | TIMER_OF_IRQ | TIMER_OF_CLOCK;
+ ctrl->to.of_clk.name = "bus";
+ ctrl->to.of_irq.flags = IRQF_TIMER;
+ ctrl->to.of_irq.handler = otto_tc_handler;
+
+ err = timer_of_init(node, &ctrl->to);
+ if (err)
+ goto err_out;
+
+ /* Reset timer state */
+ writel(0, OTTO_TC_REG_CTL(&ctrl->to));
+ writel(0, OTTO_TC_REG_DATA(&ctrl->to));
+
+ /* TODO Replace by a real derived clock */
+ otto_tc_set_divisor(ctrl, OTTO_TC_MIN_DIVISOR);
+ ctrl->to.of_clk.rate /= OTTO_TC_MIN_DIVISOR;
+ ctrl->to.of_clk.period /= OTTO_TC_MIN_DIVISOR;
+
+ otto_tc_irq_enable_clear(&ctrl->to);
+ otto_tc_init_clkevt(&ctrl->to);
+
+ return 0;
+
+err_out:
+ kfree(ctrl);
+
+ return err;
+}
+TIMER_OF_DECLARE(otto_tc, "realtek,otto-tc", otto_tc_init);
--
2.34.1

2022-01-17 11:07:51

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] clocksource/drivers: Add Realtek Otto timer driver

Hi Sander,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20220116]
[cannot apply to tip/timers/core linux/master linus/master daniel-lezcano/clockevents/next v5.16 v5.16-rc8 v5.16-rc7 v5.16]
[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/Sander-Vanheule/Realtek-Otto-timer-counter-support/20220117-054003
base: 70e6f1b39929bf6755a9c55b79fe720f7c8b9436
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20220117/[email protected]/config)
compiler: sparc64-linux-gcc (GCC) 11.2.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/1c346209c6655c06ab28df22f821ffa06a792a14
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sander-Vanheule/Realtek-Otto-timer-counter-support/20220117-054003
git checkout 1c346209c6655c06ab28df22f821ffa06a792a14
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sparc SHELL=/bin/bash drivers/clocksource/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/clocksource/timer-realtek-otto.c: In function 'otto_tc_init':
drivers/clocksource/timer-realtek-otto.c:182:16: error: implicit declaration of function 'kzalloc'; did you mean 'd_alloc'? [-Werror=implicit-function-declaration]
182 | ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
| ^~~~~~~
| d_alloc
>> drivers/clocksource/timer-realtek-otto.c:182:14: warning: assignment to 'struct otto_tc_ctrl *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
182 | ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
| ^
drivers/clocksource/timer-realtek-otto.c:212:9: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration]
212 | kfree(ctrl);
| ^~~~~
cc1: some warnings being treated as errors


vim +182 drivers/clocksource/timer-realtek-otto.c

176
177 static int __init otto_tc_init(struct device_node *node)
178 {
179 struct otto_tc_ctrl *ctrl;
180 int err;
181
> 182 ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2022-01-17 15:59:34

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] clocksource/drivers: Add Realtek Otto timer driver

Hi Sander,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20220116]
[cannot apply to tip/timers/core linux/master linus/master daniel-lezcano/clockevents/next v5.16 v5.16-rc8 v5.16-rc7 v5.16]
[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/Sander-Vanheule/Realtek-Otto-timer-counter-support/20220117-054003
base: 70e6f1b39929bf6755a9c55b79fe720f7c8b9436
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20220117/[email protected]/config)
compiler: sparc64-linux-gcc (GCC) 11.2.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/1c346209c6655c06ab28df22f821ffa06a792a14
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sander-Vanheule/Realtek-Otto-timer-counter-support/20220117-054003
git checkout 1c346209c6655c06ab28df22f821ffa06a792a14
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sparc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/clocksource/timer-realtek-otto.c: In function 'otto_tc_init':
>> drivers/clocksource/timer-realtek-otto.c:182:16: error: implicit declaration of function 'kzalloc'; did you mean 'd_alloc'? [-Werror=implicit-function-declaration]
182 | ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
| ^~~~~~~
| d_alloc
drivers/clocksource/timer-realtek-otto.c:182:14: warning: assignment to 'struct otto_tc_ctrl *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
182 | ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
| ^
>> drivers/clocksource/timer-realtek-otto.c:212:9: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration]
212 | kfree(ctrl);
| ^~~~~
cc1: some warnings being treated as errors


vim +182 drivers/clocksource/timer-realtek-otto.c

176
177 static int __init otto_tc_init(struct device_node *node)
178 {
179 struct otto_tc_ctrl *ctrl;
180 int err;
181
> 182 ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
183 if (!ctrl)
184 return -ENOMEM;
185
186 spin_lock_init(&ctrl->lock);
187
188 ctrl->to.flags = TIMER_OF_BASE | TIMER_OF_IRQ | TIMER_OF_CLOCK;
189 ctrl->to.of_clk.name = "bus";
190 ctrl->to.of_irq.flags = IRQF_TIMER;
191 ctrl->to.of_irq.handler = otto_tc_handler;
192
193 err = timer_of_init(node, &ctrl->to);
194 if (err)
195 goto err_out;
196
197 /* Reset timer state */
198 writel(0, OTTO_TC_REG_CTL(&ctrl->to));
199 writel(0, OTTO_TC_REG_DATA(&ctrl->to));
200
201 /* TODO Replace by a real derived clock */
202 otto_tc_set_divisor(ctrl, OTTO_TC_MIN_DIVISOR);
203 ctrl->to.of_clk.rate /= OTTO_TC_MIN_DIVISOR;
204 ctrl->to.of_clk.period /= OTTO_TC_MIN_DIVISOR;
205
206 otto_tc_irq_enable_clear(&ctrl->to);
207 otto_tc_init_clkevt(&ctrl->to);
208
209 return 0;
210
211 err_out:
> 212 kfree(ctrl);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2022-01-17 16:59:33

by Sander Vanheule

[permalink] [raw]
Subject: Re: [PATCH 2/2] clocksource/drivers: Add Realtek Otto timer driver

On Mon, 2022-01-17 at 09:28 +0800, kernel test robot wrote:
>    drivers/clocksource/timer-realtek-otto.c: In function 'otto_tc_init':
>    drivers/clocksource/timer-realtek-otto.c:182:16: error: implicit
> declaration of function 'kzalloc'; did you mean 'd_alloc'? [-Werror=implicit-
> function-declaration]
>      182 |         ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
>          |                ^~~~~~~
>          |                d_alloc
> > > drivers/clocksource/timer-realtek-otto.c:182:14: warning: assignment to
> > > 'struct otto_tc_ctrl *' from 'int' makes pointer from integer without a
> > > cast [-Wint-conversion]
>      182 |         ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
>          |              ^
>    drivers/clocksource/timer-realtek-otto.c:212:9: error: implicit declaration
> of function 'kfree' [-Werror=implicit-function-declaration]
>      212 |         kfree(ctrl);
>          |         ^~~~~
>    cc1: some warnings being treated as errors
>
>
> vim +182 drivers/clocksource/timer-realtek-otto.c
>
>    176  
>    177  static int __init otto_tc_init(struct device_node *node)
>    178  {
>    179          struct otto_tc_ctrl *ctrl;
>    180          int err;
>    181  
>  > 182          ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);

I'll include linux/slab.h in v2.

Best,
Sander

2022-02-09 08:46:49

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: timer: Add realtek,otto-tc binding

On Sun, Jan 16, 2022 at 10:39:24PM +0100, Sander Vanheule wrote:
> New binding for the timer/counter blocks found on the Realtek Otto MIPS
> platform.
>
> Signed-off-by: Sander Vanheule <[email protected]>
> ---
> .../bindings/timer/realtek,otto-tc.yaml | 64 +++++++++++++++++++
> 1 file changed, 64 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
>
> diff --git a/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml b/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
> new file mode 100644
> index 000000000000..12971b9ecdf5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/realtek,otto-tc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek Otto platform timer/counter bindings
> +
> +description:

'|' at the end if you want to keep formatting.

> + "Up-counting 28-bit timer that can operate in oneshot or repeating mode,

And drop the quotes.

> + providing an interrupt at roll-over.
> +
> + The timer is driven by a divided clock, derived from the bus clock. The clock
> + divisor is configurable from 2 to 65535. Divisor values of 0 and 1 disable
> + the timer clock. The timer can also be enabled or disabled independently from
> + the clock (divisor) selection.
> +
> + The number of timers supported by the different SoC families is:
> + - RTL8380: 5 timers
> + - RTL8390: 5 timers
> + - RTL9300: 6 timers
> + - RTL9310: 7 timers"
> +
> +maintainers:
> + - Sander Vanheule <[email protected]>
> +
> +properties:
> + compatible:
> + const: realtek,otto-tc

4 SoCs with differences in the block, you need 4 SoC specific
compatibles. With a fallback if appropriate.

> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: Parent clock from peripheral bus
> +
> + clock-names:
> + items:
> + - const: bus
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> +
> +examples:
> + - |
> + timer0: timer@3100 {

Drop unused labels.

> + compatible = "realtek,otto-tc";
> + reg = <0x3100 0x10>;
> +
> + interrupts = <29 4>;
> +
> + clocks = <&lx_clk>;
> + clock-names = "bus";
> + };
> --
> 2.34.1
>
>

2022-02-09 12:30:26

by Sander Vanheule

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: timer: Add realtek,otto-tc binding

Hi Rob,

On Tue, 2022-02-08 at 20:49 -0600, Rob Herring wrote:
> On Sun, Jan 16, 2022 at 10:39:24PM +0100, Sander Vanheule wrote:
> > New binding for the timer/counter blocks found on the Realtek Otto MIPS
> > platform.
> >
> > Signed-off-by: Sander Vanheule <[email protected]>
> > ---
> >  .../bindings/timer/realtek,otto-tc.yaml       | 64 +++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-
> > tc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
> > b/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
> > new file mode 100644
> > index 000000000000..12971b9ecdf5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/realtek,otto-tc.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/timer/realtek,otto-tc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Realtek Otto platform timer/counter bindings
> > +
> > +description:
>
> '|' at the end if you want to keep formatting.
>
> > +  "Up-counting 28-bit timer that can operate in oneshot or repeating mode,
>
> And drop the quotes.
>
> > +  providing an interrupt at roll-over.
> > +
> > +  The timer is driven by a divided clock, derived from the bus clock. The
> > clock
> > +  divisor is configurable from 2 to 65535. Divisor values of 0 and 1
> > disable
> > +  the timer clock. The timer can also be enabled or disabled independently
> > from
> > +  the clock (divisor) selection.
> > +
> > +  The number of timers supported by the different SoC families is:
> > +  - RTL8380: 5 timers
> > +  - RTL8390: 5 timers
> > +  - RTL9300: 6 timers
> > +  - RTL9310: 7 timers"
> > +
> > +maintainers:
> > +  - Sander Vanheule <[email protected]>
> > +
> > +properties:
> > +  compatible:
> > +    const: realtek,otto-tc
>
> 4 SoCs with differences in the block, you need 4 SoC specific
> compatibles. With a fallback if appropriate.
>
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: Parent clock from peripheral bus
> > +
> > +  clock-names:
> > +    items:
> > +      - const: bus
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +
> > +examples:
> > +  - |
> > +    timer0: timer@3100 {
>
> Drop unused labels.

I forgot to remove this, as originally I had...

>
> > +      compatible = "realtek,otto-tc";
> > +      reg = <0x3100 0x10>;
> > +
> > +      interrupts = <29 4>;
> > +
> > +      clocks = <&lx_clk>;
> > +      clock-names = "bus";

clocks = <&lx_clk>, <&timer0>;
clock-names = "bus", "timer";

to have a self-reference for the (private) derived clock. Not sure if it makes
sense to do it like this though, or if only the bus clock would be sufficient.

I'll also fix your other remarks in v2.

Best,
Sander


> > +    };
> > --
> > 2.34.1
> >
> >