Re: [PATCH v3 8/9] clocksource: Add a new timer-ingenic driver

From: Paul Cercueil
Date: Thu Jan 11 2018 - 11:16:57 EST


Hi Rob,

Le jeu. 11 janv. 2018 à 15:53, Rob Herring <robh+dt@xxxxxxxxxx> a écrit :
On Wed, Jan 10, 2018 at 4:48 PM, Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
This driver will use the TCU (Timer Counter Unit) present on the Ingenic
JZ47xx SoCs to provide the kernel with a clocksource and timers.

Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
---
drivers/clocksource/Kconfig | 8 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-ingenic.c | 258 ++++++++++++++++++++++++++++++++++++
3 files changed, 267 insertions(+)
create mode 100644 drivers/clocksource/timer-ingenic.c

v2: Use SPDX identifier for the license
v3: - Move documentation to its own patch
- Search the devicetree for PWM clients, and use all the TCU
channels that won't be used for PWM

[...]

+static int __init ingenic_tcu_init(struct device_node *np)
+{
+ unsigned long available_channels = GENMASK(NUM_CHANNELS - 1, 0);
+ struct device_node *node;
+ struct ingenic_tcu *tcu;
+ unsigned int i, channel;
+ int err;
+ u32 val;
+
+ for_each_node_with_property(node, "pwms") {
+ err = of_property_read_u32_index(node, "pwms", 1, &val);

This is the right idea, but a bit fragile. Perhaps its good enough for
your platform, but it would fail if you have another PWM provider like
the gpio-pwm binding or the cell size is not 1 (BTW, I thought the PWM
binding defined 3 cells typically).

Index 1 is always the channel number of the PWM, it works fine with 3-cells
PWM bindings, that's what I tested it with.

Ok, would it be enough to check that "the PWM clients we detect are connected
to the PWM driver whose parent is also our parent" (since both drivers are in
the same syscon/simple-mfd node)?

Thanks,
-Paul