From: Stephen Boyd <sboyd@codeaurora.org> To: James Liao <jamesjj.liao@mediatek.com> Cc: Erin Lo <erin.lo@mediatek.com>, Matthias Brugger <matthias.bgg@gmail.com>, Mike Turquette <mturquette@baylibre.com>, Rob Herring <robh@kernel.org>, Arnd Bergmann <arnd@arndb.de>, Sascha Hauer <kernel@pengutronix.de>, Daniel Kurtz <djkurtz@chromium.org>, Philipp Zabel <p.zabel@pengutronix.de>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-clk@vger.kernel.org, srv_heupstream@mediatek.com, ms.chen@mediatek.com, robert.chou@mediatek.com, Shunli Wang <shunli.wang@mediatek.com> Subject: Re: [PATCH v14 1/4] clk: mediatek: Add MT2701 clock support Date: Mon, 31 Oct 2016 11:06:47 -0700 [thread overview] Message-ID: <20161031180647.GR16026@codeaurora.org> (raw) In-Reply-To: <1477896558.5376.13.camel@mtksdaap41> On 10/31, James Liao wrote: > On Thu, 2016-10-27 at 18:17 -0700, Stephen Boyd wrote: > > On 10/21, Erin Lo wrote: > > > @@ -244,3 +256,31 @@ void mtk_clk_register_composites(const struct mtk_composite *mcs, > > > clk_data->clks[mc->id] = clk; > > > } > > > } > > > + > > > +void mtk_clk_register_dividers(const struct mtk_clk_divider *mcds, > > > + int num, void __iomem *base, spinlock_t *lock, > > > + struct clk_onecell_data *clk_data) > > > +{ > > > + struct clk *clk; > > > + int i; > > > + > > > + for (i = 0; i < num; i++) { > > > + const struct mtk_clk_divider *mcd = &mcds[i]; > > > + > > > + if (clk_data && !IS_ERR_OR_NULL(clk_data->clks[mcd->id])) > > > > NULL is a valid clk. IS_ERR_OR_NULL is usually wrong. > > Why NULL is a valid clk? Perhaps at some point we'll want to return a NULL pointer to clk_get() callers so that they can handle things like optional clocks easily without having any storage requirements. I don't know if we'll ever do that, but that's just a possibility. > > clk_data is designed for multiple initialization from different clock > types, such as infra_clk_data in clk-mt2701.c. So it will ignore valid > clocks to avoid duplicated clock registration. Here I assume a clock > pointer with error code or NULL to be an invalid (not initialized) > clock. > Ok. Would it be possible to initialize the array with all error pointers? That would make things less error prone, but it probably doesn't matter at all anyway because this is done during registration time. IS_ERR_OR_NULL makes me take a second look each time, because it's usually wrong. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
WARNING: multiple messages have this Message-ID (diff)
From: sboyd@codeaurora.org (Stephen Boyd) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v14 1/4] clk: mediatek: Add MT2701 clock support Date: Mon, 31 Oct 2016 11:06:47 -0700 [thread overview] Message-ID: <20161031180647.GR16026@codeaurora.org> (raw) In-Reply-To: <1477896558.5376.13.camel@mtksdaap41> On 10/31, James Liao wrote: > On Thu, 2016-10-27 at 18:17 -0700, Stephen Boyd wrote: > > On 10/21, Erin Lo wrote: > > > @@ -244,3 +256,31 @@ void mtk_clk_register_composites(const struct mtk_composite *mcs, > > > clk_data->clks[mc->id] = clk; > > > } > > > } > > > + > > > +void mtk_clk_register_dividers(const struct mtk_clk_divider *mcds, > > > + int num, void __iomem *base, spinlock_t *lock, > > > + struct clk_onecell_data *clk_data) > > > +{ > > > + struct clk *clk; > > > + int i; > > > + > > > + for (i = 0; i < num; i++) { > > > + const struct mtk_clk_divider *mcd = &mcds[i]; > > > + > > > + if (clk_data && !IS_ERR_OR_NULL(clk_data->clks[mcd->id])) > > > > NULL is a valid clk. IS_ERR_OR_NULL is usually wrong. > > Why NULL is a valid clk? Perhaps at some point we'll want to return a NULL pointer to clk_get() callers so that they can handle things like optional clocks easily without having any storage requirements. I don't know if we'll ever do that, but that's just a possibility. > > clk_data is designed for multiple initialization from different clock > types, such as infra_clk_data in clk-mt2701.c. So it will ignore valid > clocks to avoid duplicated clock registration. Here I assume a clock > pointer with error code or NULL to be an invalid (not initialized) > clock. > Ok. Would it be possible to initialize the array with all error pointers? That would make things less error prone, but it probably doesn't matter at all anyway because this is done during registration time. IS_ERR_OR_NULL makes me take a second look each time, because it's usually wrong. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
next prev parent reply other threads:[~2016-10-31 18:06 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-10-21 3:32 [PATCH v14 0/4] Add clock support for Mediatek MT2701 Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-21 3:32 ` [PATCH v14 1/4] clk: mediatek: Add MT2701 clock support Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-28 1:17 ` Stephen Boyd 2016-10-28 1:17 ` Stephen Boyd 2016-10-28 1:17 ` Stephen Boyd 2016-10-31 6:49 ` James Liao 2016-10-31 6:49 ` James Liao 2016-10-31 6:49 ` James Liao 2016-10-31 18:06 ` Stephen Boyd [this message] 2016-10-31 18:06 ` Stephen Boyd 2016-11-01 9:23 ` James Liao 2016-11-01 9:23 ` James Liao 2016-11-01 9:23 ` James Liao 2016-10-21 3:32 ` [PATCH v14 2/4] reset: mediatek: Add MT2701 reset driver Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-21 3:32 ` [PATCH v14 3/4] arm: dts: mt2701: Add clock controller device nodes Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-21 3:32 ` [PATCH v14 4/4] arm: dts: mt2701: Use real clock for UARTs Erin Lo 2016-10-21 3:32 ` Erin Lo 2016-10-21 3:32 ` Erin Lo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20161031180647.GR16026@codeaurora.org \ --to=sboyd@codeaurora.org \ --cc=arnd@arndb.de \ --cc=devicetree@vger.kernel.org \ --cc=djkurtz@chromium.org \ --cc=erin.lo@mediatek.com \ --cc=jamesjj.liao@mediatek.com \ --cc=kernel@pengutronix.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=matthias.bgg@gmail.com \ --cc=ms.chen@mediatek.com \ --cc=mturquette@baylibre.com \ --cc=p.zabel@pengutronix.de \ --cc=robert.chou@mediatek.com \ --cc=robh@kernel.org \ --cc=shunli.wang@mediatek.com \ --cc=srv_heupstream@mediatek.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.