From: Peter De Schrijver <pdeschrijver@nvidia.com>
To: Stephen Boyd <sboyd@kernel.org>
Cc: Aapo Vienamo <avienamo@nvidia.com>,
Prashant Gaikwad <pgaikwad@nvidia.com>,
Michael Turquette <mturquette@baylibre.com>,
Thierry Reding <thierry.reding@gmail.com>,
Jonathan Hunter <jonathanh@nvidia.com>,
<linux-kernel@vger.kernel.org>, <linux-clk@vger.kernel.org>,
<linux-tegra@vger.kernel.org>
Subject: Re: [PATCH v2 2/3] clk: tegra: Add sdmmc mux divider clock
Date: Mon, 9 Jul 2018 11:00:12 +0300 [thread overview]
Message-ID: <20180709080012.GE27696@tbergstrom-lnx.Nvidia.com> (raw)
In-Reply-To: <153090111049.143105.9486166124584607061@swboyd.mtv.corp.google.com>
On Fri, Jul 06, 2018 at 11:18:30AM -0700, Stephen Boyd wrote:
> Quoting Aapo Vienamo (2018-07-04 03:17:34)
> > diff --git a/drivers/clk/tegra/clk-sdmmc-mux.c b/drivers/clk/tegra/clk-sdmmc-mux.c
> > new file mode 100644
> > index 0000000..8e19cb3
> > --- /dev/null
> > +++ b/drivers/clk/tegra/clk-sdmmc-mux.c
> > @@ -0,0 +1,254 @@
> > +/*
> > + * Copyright (c) 2018 NVIDIA CORPORATION. All rights reserved.
> > + *
> > + * based on clk-mux.c
> > +
> > + * Copyright (C) 2011 Sascha Hauer, Pengutronix <s.hauer@pengutronix.de>
> > + * Copyright (C) 2011 Richard Zhao, Linaro <richard.zhao@linaro.org>
> > + * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> Any chance we can get SPDX tags here instead of all the boiler plate?
>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/err.h>
> > +#include <linux/types.h>
> > +
> > +#include "clk.h"
> > +
> > +#define DIV_MASK GENMASK(7, 0)
> > +#define MUX_SHIFT 29
> > +#define MUX_MASK GENMASK(MUX_SHIFT + 2, MUX_SHIFT)
> > +
> > +#define get_max_div(d) DIV_MASK
> > +#define get_div_field(val) ((val) & DIV_MASK)
> > +#define get_mux_field(val) (((val) & MUX_MASK) >> MUX_SHIFT)
> > +
> > +static const char * const mux_sdmmc_parents[] = { "pll_p", "pll_c4_out2",
> > + "pll_c4_out0", "pll_c4_out1",
> > + "clk_m" };
> > +static u8 mux_lj_idx[] = { [0] = 0, [1] = 1, [2] = 2, [3] = 5, [4] = 6 };
> > +static u8 mux_non_lj_idx[] = { [0] = 0, [1] = 3, [2] = 7, [3] = 4, [4] = 6 };
>
> These can be const?
>
> > +
> > +static u8 clk_sdmmc_mux_get_parent(struct clk_hw *hw)
> > +{
> [...]
> > +static int clk_sdmmc_mux_set_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long parent_rate)
> > +{
> > + struct tegra_sdmmc_mux *sdmmc_mux = to_clk_sdmmc_mux(hw);
> > + int div;
> > + unsigned long flags = 0;
> > + u32 val;
> > + u8 src;
> > +
> > + div = div71_get(rate, parent_rate, 8, 1, sdmmc_mux->div_flags);
> > + if (div < 0)
> > + return div;
> > +
> > + if (sdmmc_mux->lock)
> > + spin_lock_irqsave(sdmmc_mux->lock, flags);
> > +
> > + src = clk_sdmmc_mux_get_parent(hw);
> > + if (div)
> > + src = mux_non_lj_idx[src];
> > + else
> > + src = mux_lj_idx[src];
> > +
> > + val = src << MUX_SHIFT;
> > + val |= div;
> > + writel(val, sdmmc_mux->reg);
> > + fence_udelay(2, sdmmc_mux->reg);
> > +
> > + if (sdmmc_mux->lock)
> > + spin_unlock_irqrestore(sdmmc_mux->lock, flags);
>
> This conditional locking will give sparse a headache. O well.
>
> > +
> > + return 0;
> > +}
> > +
> > +static int clk_sdmmc_mux_is_enabled(struct clk_hw *hw)
> > +{
> > + struct tegra_sdmmc_mux *sdmmc_mux = to_clk_sdmmc_mux(hw);
> > + const struct clk_ops *gate_ops = sdmmc_mux->gate_ops;
> > + struct clk_hw *gate_hw = &sdmmc_mux->gate.hw;
> > +
> > + __clk_hw_set_clk(gate_hw, hw);
> > +
> > + return gate_ops->is_enabled(gate_hw);
> > +}
> > +
> > +static int clk_sdmmc_mux_enable(struct clk_hw *hw)
> > +{
> > + struct tegra_sdmmc_mux *sdmmc_mux = to_clk_sdmmc_mux(hw);
> > + const struct clk_ops *gate_ops = sdmmc_mux->gate_ops;
> > + struct clk_hw *gate_hw = &sdmmc_mux->gate.hw;
> > +
> > + __clk_hw_set_clk(gate_hw, hw);
> > +
> > + return gate_ops->enable(gate_hw);
> > +}
> > +
> > +static void clk_sdmmc_mux_disable(struct clk_hw *hw)
> > +{
> > + struct tegra_sdmmc_mux *sdmmc_mux = to_clk_sdmmc_mux(hw);
> > + const struct clk_ops *gate_ops = sdmmc_mux->gate_ops;
> > + struct clk_hw *gate_hw = &sdmmc_mux->gate.hw;
> > +
> > + gate_ops->disable(gate_hw);
> > +}
> > +
> > +const struct clk_ops tegra_clk_sdmmc_mux_ops = {
>
> static?
>
> > + .get_parent = clk_sdmmc_mux_get_parent,
> > + .set_parent = clk_sdmmc_mux_set_parent,
> > + .determine_rate = clk_sdmmc_mux_determine_rate,
> > + .recalc_rate = clk_sdmmc_mux_recalc_rate,
> > + .set_rate = clk_sdmmc_mux_set_rate,
> > + .is_enabled = clk_sdmmc_mux_is_enabled,
> > + .enable = clk_sdmmc_mux_enable,
> > + .disable = clk_sdmmc_mux_disable,
> > +};
> > +
> > diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> > index f14e136..4c3d0f5 100644
> > --- a/drivers/clk/tegra/clk.h
> > +++ b/drivers/clk/tegra/clk.h
> > @@ -19,6 +19,7 @@
> >
> > #include <linux/clk-provider.h>
> > #include <linux/clkdev.h>
> > +#include <linux/delay.h>
>
> Why? Include this in the C file that cares please.
>
The fence_udelay macro uses udelay and is used in clk-sdmmc-mux.c. However it
seems more appropriate to include delay.h here rather than in every single
user of the macro.
Peter.
next prev parent reply other threads:[~2018-07-09 8:00 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-04 10:17 [PATCH v2 1/3] clk: tegra: refactor 7.1 div calculation Aapo Vienamo
2018-07-04 10:17 ` [PATCH v2 2/3] clk: tegra: Add sdmmc mux divider clock Aapo Vienamo
2018-07-06 18:18 ` Stephen Boyd
2018-07-09 8:00 ` Peter De Schrijver [this message]
2018-07-04 10:17 ` [PATCH v2 3/3] clk: tegra: make sdmmc2 and sdmmc4 as sdmmc clocks Aapo Vienamo
2018-07-05 11:43 ` [PATCH v2 1/3] clk: tegra: refactor 7.1 div calculation Peter De Schrijver
2018-07-06 18:21 ` Stephen Boyd
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=20180709080012.GE27696@tbergstrom-lnx.Nvidia.com \
--to=pdeschrijver@nvidia.com \
--cc=avienamo@nvidia.com \
--cc=jonathanh@nvidia.com \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=pgaikwad@nvidia.com \
--cc=sboyd@kernel.org \
--cc=thierry.reding@gmail.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).