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

[Rob Herring]

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).

Rob