From: zhang.lyra@gmail.com (Chunyan Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V3 05/11] clk: sprd: add mux clock support
Date: Fri, 3 Nov 2017 20:12:35 +0800 [thread overview]
Message-ID: <CAAfSe-tUF1YiDf_y4-GXBKTjiVCkL-y627mUjss6CZ6jr0CPTg@mail.gmail.com> (raw)
In-Reply-To: <532aed43-03bf-ea26-bdc2-d7deb1093f53@arm.com>
Hi Julien,
On 3 November 2017 at 02:11, Julien Thierry <julien.thierry@arm.com> wrote:
> Hi,
>
>
> On 02/11/17 06:56, Chunyan Zhang wrote:
>>
>> This patch adds clock multiplexor support for Spreadtrum platforms,
>> the mux clocks also can be found in sprd composite clocks, so
>> provides two helpers that can be reused later on.
>>
>> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>> ---
>> drivers/clk/sprd/Makefile | 1 +
>> drivers/clk/sprd/mux.c | 89
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/clk/sprd/mux.h | 65 ++++++++++++++++++++++++++++++++++
>> 3 files changed, 155 insertions(+)
>> create mode 100644 drivers/clk/sprd/mux.c
>> create mode 100644 drivers/clk/sprd/mux.h
>>
>> diff --git a/drivers/clk/sprd/Makefile b/drivers/clk/sprd/Makefile
>> index 8cd5592..cee36b5 100644
>> --- a/drivers/clk/sprd/Makefile
>> +++ b/drivers/clk/sprd/Makefile
>> @@ -2,3 +2,4 @@ obj-$(CONFIG_SPRD_COMMON_CLK) += clk-sprd.o
>> clk-sprd-y += common.o
>> clk-sprd-y += gate.o
>> +clk-sprd-y += mux.o
>> diff --git a/drivers/clk/sprd/mux.c b/drivers/clk/sprd/mux.c
>> new file mode 100644
>> index 0000000..5a344e0
>> --- /dev/null
>> +++ b/drivers/clk/sprd/mux.c
>> @@ -0,0 +1,89 @@
>> +/*
>> + * Spreadtrum multiplexer clock driver
>> + *
>> + * Copyright (C) 2017 Spreadtrum, Inc.
>> + * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "mux.h"
>> +
>> +DEFINE_SPINLOCK(sprd_mux_lock);
>> +EXPORT_SYMBOL_GPL(sprd_mux_lock);
>> +
>> +u8 sprd_mux_helper_get_parent(const struct sprd_clk_common *common,
>> + const struct sprd_mux_internal *mux)
>> +{
>> + unsigned int reg;
>> + u8 parent;
>> + int num_parents;
>> + int i;
>> +
>> + sprd_regmap_read(common->regmap, common->reg, ®);
>> + parent = reg >> mux->shift;
>> + parent &= (1 << mux->width) - 1;
>> +
>> + if (mux->table) {
>> + num_parents = clk_hw_get_num_parents(&common->hw);
>> +
>> + for (i = 0; i < num_parents; i++)
>> + if (parent == mux->table[i] ||
>> + (i < (num_parents - 1) && parent >
>> mux->table[i] &&
>> + parent < mux->table[i + 1]))
>> + return i;
>> + if (i == num_parents)
>> + return i - 1;
>
>
> The if branch is not necessary since you only get there when the loop has
> finished, so the condition is always true. And the loop can be simplified
> to:
>
> for (i = 0; i < num_parents - 1; i++)
> if (parent >= mux->table[i] && parent < mux->table[i + 1])
> return i;
>
> return num_parents;
>
>
>
>> + }
>> +
>> + return parent;
>> +}
>> +EXPORT_SYMBOL_GPL(sprd_mux_helper_get_parent);
>> +
>> +static u8 sprd_mux_get_parent(struct clk_hw *hw)
>> +{
>> + struct sprd_mux *cm = hw_to_sprd_mux(hw);
>> +
>> + return sprd_mux_helper_get_parent(&cm->common, &cm->mux);
>> +}
>> +
>> +int sprd_mux_helper_set_parent(const struct sprd_clk_common *common,
>> + const struct sprd_mux_internal *mux,
>> + u8 index)
>> +{
>> + unsigned long flags = 0;
>> + unsigned int reg;
>> +
>> + if (mux->table)
>> + index = mux->table[index];
>> +
>> + spin_lock_irqsave(common->lock, flags);
>> +
>> + sprd_regmap_read(common->regmap, common->reg, ®);
>> + reg &= ~GENMASK(mux->width + mux->shift - 1, mux->shift);
>> + sprd_regmap_write(common->regmap, common->reg,
>> + reg | (index << mux->shift));
>> +
>> + spin_unlock_irqrestore(common->lock, flags);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(sprd_mux_helper_set_parent);
>> +
>> +static int sprd_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> + struct sprd_mux *cm = hw_to_sprd_mux(hw);
>> +
>> + return sprd_mux_helper_set_parent(&cm->common, &cm->mux, index);
>> +}
>> +
>> +const struct clk_ops sprd_mux_ops = {
>> + .get_parent = sprd_mux_get_parent,
>> + .set_parent = sprd_mux_set_parent,
>> + .determine_rate = __clk_mux_determine_rate,
>> +};
>> +EXPORT_SYMBOL_GPL(sprd_mux_ops);
>
>
> Same as with the other patch, I'd recommend have one set of ops for direct
> mux and another one for a mux using a table to map the parents. Keeping
> functions for both modes separate.
>
I might prefer to keep them together, since separating them will
increase many lines of code for maintaining :)
And will export double of these functions, not only for mux driver but
also for composite driver.
And I think the name like "sprd_mux_helper_set_parent_table" is a little long.
Thanks,
Chunyan
> Cheers,
>
>
>> diff --git a/drivers/clk/sprd/mux.h b/drivers/clk/sprd/mux.h
>> new file mode 100644
>> index 0000000..148ca8c
>> --- /dev/null
>> +++ b/drivers/clk/sprd/mux.h
>> @@ -0,0 +1,65 @@
>> +/*
>> + * Spreadtrum multiplexer clock driver
>> + *
>> + * Copyright (C) 2017 Spreadtrum, Inc.
>> + * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0
>> + */
>> +
>> +#ifndef _SPRD_MUX_H_
>> +#define _SPRD_MUX_H_
>> +
>> +#include "common.h"
>> +
>> +struct sprd_mux_internal {
>> + u8 shift;
>> + u8 width;
>> + const u8 *table;
>> +};
>> +
>> +struct sprd_mux {
>> + struct sprd_mux_internal mux;
>> + struct sprd_clk_common common;
>> +};
>> +
>> +#define _SPRD_MUX_CLK(_shift, _width, _table) \
>> + { \
>> + .shift = _shift, \
>> + .width = _width, \
>> + .table = _table, \
>> + }
>> +
>> +#define SPRD_MUX_CLK(_struct, _name, _parents, _table, \
>> + _reg, _shift, _width, \
>> + _flags) \
>> + struct sprd_mux _struct = { \
>> + .mux = _SPRD_MUX_CLK(_shift, _width, _table), \
>> + .common = { \
>> + .regmap = NULL, \
>> + .reg = _reg, \
>> + .lock = &sprd_mux_lock, \
>> + .hw.init = CLK_HW_INIT_PARENTS(_name, \
>> + _parents, \
>> + &sprd_mux_ops, \
>> + _flags), \
>> + } \
>> + }
>> +
>> +static inline struct sprd_mux *hw_to_sprd_mux(const struct clk_hw *hw)
>> +{
>> + struct sprd_clk_common *common = hw_to_sprd_clk_common(hw);
>> +
>> + return container_of(common, struct sprd_mux, common);
>> +}
>> +
>> +extern const struct clk_ops sprd_mux_ops;
>> +extern spinlock_t sprd_mux_lock;
>> +
>> +u8 sprd_mux_helper_get_parent(const struct sprd_clk_common *common,
>> + const struct sprd_mux_internal *mux);
>> +int sprd_mux_helper_set_parent(const struct sprd_clk_common *common,
>> + const struct sprd_mux_internal *mux,
>> + u8 index);
>> +
>> +#endif /* _SPRD_MUX_H_ */
>>
>
> --
> Julien Thierry
next prev parent reply other threads:[~2017-11-03 12:12 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-02 6:56 [PATCH V3 00/11] add clock driver for Spreadtrum platforms Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 01/11] drivers: move clock common macros out from vendor directories Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 02/11] dt-bindings: Add Spreadtrum clock binding documentation Chunyan Zhang
2017-11-06 17:15 ` Rob Herring
2017-11-07 7:01 ` Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 03/11] clk: sprd: Add common infrastructure Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 04/11] clk: sprd: add gate clock support Chunyan Zhang
2017-11-02 17:45 ` Julien Thierry
2017-11-03 12:12 ` Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 05/11] clk: sprd: add mux " Chunyan Zhang
2017-11-02 18:11 ` Julien Thierry
2017-11-02 18:22 ` Julien Thierry
2017-11-03 12:12 ` Chunyan Zhang
2017-11-03 12:12 ` Chunyan Zhang [this message]
2017-11-02 6:56 ` [PATCH V3 06/11] clk: sprd: add divider " Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 07/11] clk: sprd: add composite " Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 08/11] clk: sprd: add adjustable pll support Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 09/11] clk: sprd: add clocks support for SC9860 Chunyan Zhang
2017-11-05 7:59 ` kbuild test robot
2017-11-06 21:43 ` Rob Herring
2017-11-09 8:18 ` Chunyan Zhang
2017-11-10 21:10 ` Rob Herring
2017-11-02 6:56 ` [PATCH V3 10/11] arm64: dts: add syscon for whale2 platform Chunyan Zhang
2017-11-02 6:56 ` [PATCH V3 11/11] arm64: dts: add clocks for SC9860 Chunyan Zhang
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=CAAfSe-tUF1YiDf_y4-GXBKTjiVCkL-y627mUjss6CZ6jr0CPTg@mail.gmail.com \
--to=zhang.lyra@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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).