[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <35bc18b2e685e8596b1fdc1a2e6212dc98725cd4.camel@maquefel.me>
Date: Wed, 21 Jun 2023 14:22:37 +0300
From: Nikita Shubin <nikita.shubin@...uefel.me>
To: andy.shevchenko@...il.com
Cc: Alexander Sverdlin <alexander.sverdlin@...il.com>,
Arnd Bergmann <arnd@...db.de>,
Linus Walleij <linus.walleij@...aro.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Michael Peters <mpeters@...eddedts.com>,
Kris Bahnsen <kris@...eddedts.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 09/43] clocksource: ep93xx: Add driver for Cirrus
Logic EP93xx
Hello Andy!
Agreed to almost, still... :
>
> ...
>
> > +static struct ep93xx_tcu *ep93xx_tcu;
>
> Global?!
> Can it be derived from struct clocksource?
>
It's look like a common practice for read_sched_clock, even for most
new drivers. I would like for comment from Daniel or Thomas before
ripping it out.
>
> > + irq = irq_of_parse_and_map(np, 0);
> > + if (irq <= 0) {
> > + pr_err("ERROR: invalid interrupt number\n");
> > + ret = -EINVAL;
>
> Shadowed error in case of negative returned code. Why?
Majority of clocksource drivers shadow it. Same like above.
All other comments applied - thank you!
Powered by blists - more mailing lists