linux-kernel - Re: [PATCH v1 02/10] clockevents/drivers: add MPS2 Timer 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]

Message-ID: <56700BFF.90001@arm.com>
Date:	Tue, 15 Dec 2015 12:47:59 +0000
From:	Vladimir Murzin <vladimir.murzin@....com>
To:	Daniel Lezcano <daniel.lezcano@...aro.org>, arnd@...db.de,
	linux@....linux.org.uk, gregkh@...uxfoundation.org,
	tglx@...utronix.de, u.kleine-koenig@...gutronix.de,
	afaerber@...e.de, mcoquelin.stm32@...il.com
CC:	Mark.Rutland@....com, Pawel.Moll@....com,
	ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
	jslaby@...e.cz, robh+dt@...nel.org, devicetree@...r.kernel.org,
	linux-serial@...r.kernel.org, linux-api@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 02/10] clockevents/drivers: add MPS2 Timer driver

On 14/12/15 13:36, Daniel Lezcano wrote:
> On 12/02/2015 10:33 AM, Vladimir Murzin wrote:
>> MPS2 platform has simple 32 bits general purpose countdown timers.
>>
>> The driver uses the first detected timer as a clocksource and the rest
>> of the timers as a clockevent
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@....com>
>> ---
> 
> [ ... ]
> 
>> +static void clockevent_mps2_writel(u32 val, struct clock_event_device
>> *c, u32 offset)
>> +{
>> +    writel(val, to_mps2_clkevt(c)->reg + offset);
>> +}
> 
> Is it possible to use writel_relaxed here ?

I think it is possible. I'll update that part.

> 
> [ ... ]
> 
>> +static int mps2_timer_set_periodic(struct clock_event_device *ce)
>> +{
>> +    u32 clock_count_per_tick = to_mps2_clkevt(ce)->clock_count_per_tick;
>> +
>> +    clockevent_mps2_writel(clock_count_per_tick, ce, TIMER_RELOAD);
>> +    clockevent_mps2_writel(clock_count_per_tick, ce, TIMER_VALUE);
>> +    clockevent_mps2_writel(TIMER_CTRL_IE | TIMER_CTRL_ENABLE, ce,
>> TIMER_CTRL);
>> +
>> +    return 0;
>> +}
>> +
>> +static irqreturn_t mps2_timer_interrupt(int irq, void *dev_id)
>> +{
>> +    struct clockevent_mps2 *ce = dev_id;
>> +    u32 status = readl(ce->reg + TIMER_INT);
>> +
>> +    if (!status) {
>> +        pr_warn("spuirous interrupt\n");
> 
> typo: 'spurious'
> 
> [ ... ]
> 
>> +    ce = kzalloc(sizeof(struct clockevent_mps2), GFP_KERNEL);
>> +    if (!ce) {
>> +        ret = -ENOMEM;
>> +        pr_err("failed to allocate clockevent: %d\n", ret);
> 
> There is already a stack trace in the kernel when an allocation fails.
> 
> With the above fixed:
> 
> Acked-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> 
> 

All above fixed locally.

Thanks
Vladimir


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/