linux-kernel - RE: [PATCH v2 2/2] clocksource: Add Intel Keem Bay Timer Support

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: <DM6PR11MB4250EB33E23178D211D0411DFBA30@DM6PR11MB4250.namprd11.prod.outlook.com>
Date:   Tue, 19 Jan 2021 02:56:36 +0000
From:   "Ayyathurai, Vijayakannan" <vijayakannan.ayyathurai@...el.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "will@...nel.org" <will@...nel.org>
CC:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "mgross@...ux.intel.com" <mgross@...ux.intel.com>,
        "Wan Mohamad, Wan Ahmad Zainie" 
        <wan.ahmad.zainie.wan.mohamad@...el.com>,
        "Raja Subramanian, Lakshmi Bai" 
        <lakshmi.bai.raja.subramanian@...el.com>,
        "Seow, Chen Yong" <chen.yong.seow@...el.com>
Subject: RE: [PATCH v2 2/2] clocksource: Add Intel Keem Bay Timer Support

Hi Daniel,

> From: Daniel Lezcano <daniel.lezcano@...aro.org>
> > From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@...el.com>
> 
> [ ... ]
> 
> > +static struct timer_of keembay_ce_to = {
> > +	.flags	= TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
> > +	.clkevt = {
> > +		.name			= "keembay_sys_clkevt",
> > +		.features		= CLOCK_EVT_FEAT_PERIODIC |
> > +					  CLOCK_EVT_FEAT_ONESHOT  |
> > +					  CLOCK_EVT_FEAT_DYNIRQ,
> > +		.rating			= TIM_RATING,
> > +		.set_next_event		=
> keembay_timer_set_next_event,
> > +		.set_state_periodic	= keembay_timer_periodic,
> > +		.set_state_shutdown	= keembay_timer_shutdown,
> > +	},
> > +	.of_base = {
> > +		.index = 0,
> > +	},
> > +	.of_irq = {
> > +		.handler = keembay_timer_isr,
> > +		.flags = IRQF_TIMER | IRQF_IRQPOLL,
> 
> Is the IRQPOLL flag really needed here ?
> 

Not really needed. I will remove this redundant Flag in my next version.

> > +static int __init keembay_timer_init(struct device_node *np)
> > +{
> > +	struct keembay_init_data data;
> > +	int ret;
> > +
> > +	data.base = of_iomap(np, 2);
> > +	if (!data.base)
> > +		return -ENXIO;
> > +
> > +	ret = keembay_clocksource_init(np, &data);
> > +	if (ret)
> > +		goto exit;
> > +
> > +	ret = keembay_clockevent_init(np, &data);
> 
> Is this missing ?
> 

Yes. Either case it goes to the exit path. So I thought of avoiding this error handling code.

> 	if (ret)
> 		goto exit;
> 
> 	return 0;
> 
> > +
> > +exit:
> > +	keembay_timer_cleanup(np, &data);
> > +
> > +	return ret;
> > +}
> > +
> > +TIMER_OF_DECLARE(keembay_timer, "intel,keembay-timer",
> keembay_timer_init);
> >
> 

Thanks,
Vijay