[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <TY1PR01MB176960BEDC23EA64095EF6F2F5080@TY1PR01MB1769.jpnprd01.prod.outlook.com>
Date: Thu, 30 Aug 2018 08:30:59 +0000
From: Phil Edworthy <phil.edworthy@...esas.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Russell King <linux@...linux.org.uk>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Simon Horman <horms@...ge.net.au>,
"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v4 1/2] clk: Add of_clk_get_by_name_optional() function
Hi Andy,
On 29 August 2018 18:11 Andy Shevchenko wrote:
> On Tue, Jul 31, 2018 at 01:10:59PM +0100, Phil Edworthy wrote:
> > Quite a few drivers get an optional clock, e.g. a clock required
> > to access peripheral's registers that is always enabled on some
> > devices.
> >
> > This function behaves the same as of_clk_get_by_name() except that
> > it will return NULL instead of -ENOENT.
>
> > + if (optional && (index == -EINVAL || index == -ENODATA ||
> > + PTR_ERR(clk) == -ENOENT)) {
>
> A nit: I would rather rearrange this to be
>
> if (optional &&
> (... || ... || ...)) {
>
> (disregard 80 characters limit for second line)
Thanks for the review. I'm not particularly happy with the patch as is, it's
pretty messy. I was in rush trying to get things done before holidays - always
a mistake.
Would you mind if I sent a v5 that I feel is somewhat clearer?
Thanks
Phil
Powered by blists - more mailing lists