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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Jan 2018 17:16:41 +0100
From:   Paul Cercueil <paul@...pouillou.net>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Mark Rutland <mark.rutland@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <marc.zyngier@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Lee Jones <lee.jones@...aro.org>,
        linux-clk <linux-clk@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 8/9] clocksource: Add a new timer-ingenic driver

Hi Rob,

Le jeu. 11 janv. 2018 à 15:53, Rob Herring <robh+dt@...nel.org> a 
écrit :
> On Wed, Jan 10, 2018 at 4:48 PM, Paul Cercueil <paul@...pouillou.net> 
> 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@...pouillou.net>
>>  ---
>>   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