[V6,00/12] add clock driver for Spreadtrum platforms

From: Chunyan Zhang <zhang.chunyan@linaro.org>

This series adds Spreadtrum clock support together with its binding
documentation and devicetree data.

This patchset also added a few common clock mactos into drivers/clk/clk_common.h,
which are generally useful for all vendors' clock driver, sunxi-ng, zte, sprd
(added in this patchse) use them (or part of them) at present, once this patchset
is merged, I can help to remove the duplicated code which is under the vendors'
respective directories.

Thanks,
Chunyan

Changes from V5: (https://lkml.org/lkml/2017/11/20/21)
* Rebased the whole patch-set to 4.15-rc1;
* Fixed kbuild-test warnings;
* Switched to use devm_clk_hw_register() instead of clk_hw_register();
* Removed useless debug information from sc9860-clk.c.
  
Changes from V4: (https://lkml.org/lkml/2017/11/10/30)
* Added acked-by of Rob Herring;
* Removed spin lock from Spreadtrum's gate, mux, div drivers, since we have
  switched to use regmap.

Changes from V3: (https://lkml.org/lkml/2017/11/2/61)
* Addressed comments from Julien Thierry:
  - Clean the if branch of sprd_mux_helper_get_parent()
  - Have the Gate clock macros and ops for both mode (i.e. sc_gate and gate) separate;
  - Have the Mux clock macros with/without table separate, and same changes
    for the composite clock.
* Switched the function name from _endisable to _toggle;
* Fixed Kbuild test error:
  - Added exporting sprd_clk_regmap_init() which would be used in other module(s);
* Change the function sprd_clk_set_regmap() to the static one, and removed the
  declear from the include file;
* Addressed comments from Rob:
  - Separate the dt-binding include file from the driver patch;
  - Documented more for the property "clocks"
* Changed the syscon device names;
* Changed the name of 'sprd_mux_internal' to 'sprd_mux_ssel'
  

Changes from V2: (http://lkml.iu.edu/hypermail/linux/kernel/1707.1/01504.html)
* Switch to use regmap to access registers;
* Splited all clocks into 16 separated nodes, for each belongs to a single address area; 
* Rearranged the order of clock declaration in sc9860-clk.c, sorted them upon the address area;
* Added syscon device tree nodes which will be quoted by the node of clocks which are in
  the same address area with the syscon device;
* Revised the binding documentation according to the dt modification. 

Changes from V1: (https://lkml.org/lkml/2017/6/17/356)
* Address Stephen's comments:
  - Switch to use platform device driver instead of the DT probing mechanism.
  - Move the common clock macro out from vendor directory, but need to remove those
    overlap code from other vendors (such as sunxi-ng) once this get merged.
  - Add support to be built as a module.
  - Add 'sprd_' prefix for all spin locks used in these drivers.
  - Mark input parameter of sprd_x with const.
  - Remove unreasonable dependencies to CONFIG_64BIT.
  - Add readl() after writing the same register.
  - Remove CLK_IS_BASIC which is no longer used.
  - Remove unnecessery CLK_IGNORE_UNUSED when defining a clock.
  - Change to expose all clock index.
  - Use clk_ instead of ccu.
  - Add Kconfig for sprd clocks.
  - Move the fixed clocks out from the soc node.
  - Switch to use 64-bit math in pll driver instead of 32-bit math.
* Revise binding documentation according to dt modification.
* Rename sc9860.c to sc9860-clk.c


Chunyan Zhang (12):
  drivers: move clock common macros out from vendor directories
  clk: sprd: Add common infrastructure
  clk: sprd: add gate clock support
  clk: sprd: add mux clock support
  clk: sprd: add divider clock support
  clk: sprd: add composite clock support
  clk: sprd: add adjustable pll support
  dt-bindings: Add Spreadtrum clock binding documentation
  clk: sprd: Add dt-bindings include file for SC9860
  clk: sprd: add clocks support for SC9860
  arm64: dts: add syscon for whale2 platform
  arm64: dts: add clocks for SC9860

 Documentation/devicetree/bindings/clock/sprd.txt |   63 +
 arch/arm64/boot/dts/sprd/sc9860.dtsi             |  115 ++
 arch/arm64/boot/dts/sprd/whale2.dtsi             |   48 +-
 drivers/clk/Kconfig                              |    1 +
 drivers/clk/Makefile                             |    1 +
 drivers/clk/clk_common.h                         |   60 +
 drivers/clk/sprd/Kconfig                         |   14 +
 drivers/clk/sprd/Makefile                        |   11 +
 drivers/clk/sprd/common.c                        |   99 ++
 drivers/clk/sprd/common.h                        |   52 +
 drivers/clk/sprd/composite.c                     |   62 +
 drivers/clk/sprd/composite.h                     |   53 +
 drivers/clk/sprd/div.c                           |   92 +
 drivers/clk/sprd/div.h                           |   77 +
 drivers/clk/sprd/gate.c                          |  113 ++
 drivers/clk/sprd/gate.h                          |   61 +
 drivers/clk/sprd/mux.c                           |   78 +
 drivers/clk/sprd/mux.h                           |   76 +
 drivers/clk/sprd/pll.c                           |  268 +++
 drivers/clk/sprd/pll.h                           |  110 ++
 drivers/clk/sprd/sc9860-clk.c                    | 1981 ++++++++++++++++++++++
 include/dt-bindings/clock/sprd,sc9860-clk.h      |  408 +++++
 22 files changed, 3841 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/sprd.txt
 create mode 100644 drivers/clk/clk_common.h
 create mode 100644 drivers/clk/sprd/Kconfig
 create mode 100644 drivers/clk/sprd/Makefile
 create mode 100644 drivers/clk/sprd/common.c
 create mode 100644 drivers/clk/sprd/common.h
 create mode 100644 drivers/clk/sprd/composite.c
 create mode 100644 drivers/clk/sprd/composite.h
 create mode 100644 drivers/clk/sprd/div.c
 create mode 100644 drivers/clk/sprd/div.h
 create mode 100644 drivers/clk/sprd/gate.c
 create mode 100644 drivers/clk/sprd/gate.h
 create mode 100644 drivers/clk/sprd/mux.c
 create mode 100644 drivers/clk/sprd/mux.h
 create mode 100644 drivers/clk/sprd/pll.c
 create mode 100644 drivers/clk/sprd/pll.h
 create mode 100644 drivers/clk/sprd/sc9860-clk.c
 create mode 100644 include/dt-bindings/clock/sprd,sc9860-clk.h

On 11/27, Chunyan Zhang wrote:
> These macros are used by more than one SoC vendor platforms, avoid to
> have many copies of these code, this patch moves them to the common
> clock directory which every clock drivers can access to.
> 
> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> ---
>  drivers/clk/clk_common.h | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 drivers/clk/clk_common.h
> 
> diff --git a/drivers/clk/clk_common.h b/drivers/clk/clk_common.h
> new file mode 100644
> index 0000000..21e93d2
> --- /dev/null
> +++ b/drivers/clk/clk_common.h
> @@ -0,0 +1,60 @@
> +/*
> + * drivers/clk/clk_common.h

We don't need this in the file too. Please remove this line.

> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#ifndef _CLK_COMMON_H_
> +#define _CLK_COMMON_H_
> +
> +#include <linux/clk-provider.h>

Maybe these macros should just go into clk-provider.h?

> +
> +#define CLK_HW_INIT(_name, _parent, _ops, _flags)		\
> +	(&(struct clk_init_data) {				\
> +		.flags		= _flags,			\
> +		.name		= _name,			\
> +		.parent_names	= (const char *[]) { _parent },	\
> +		.num_parents	= 1,				\
> +		.ops		= _ops,				\
> +	})

Hopefully we don't extend the init structure anymore to have
something else. I guess we'll do something if that happens.

> +
> +#define CLK_HW_INIT_PARENTS(_name, _parents, _ops, _flags)	\
> +	(&(struct clk_init_data) {				\
> +		.flags		= _flags,			\
> +		.name		= _name,			\
> +		.parent_names	= _parents,			\
> +		.num_parents	= ARRAY_SIZE(_parents),		\
> +		.ops		= _ops,				\
> +	})
> +
> +#define CLK_HW_INIT_NO_PARENT(_name, _ops, _flags)     \
> +	(&(struct clk_init_data) {                      \
> +		.flags          = _flags,               \
> +		.name           = _name,                \
> +		.parent_names   = NULL,                 \
> +		.num_parents    = 0,                    \
> +		.ops            = _ops,                 \
> +	})
> +
> +#define CLK_FIXED_FACTOR(_struct, _name, _parent,			\
> +			_div, _mult, _flags)				\
> +	struct clk_fixed_factor _struct = {				\
> +		.div		= _div,					\
> +		.mult		= _mult,				\
> +		.hw.init	= CLK_HW_INIT(_name,			\
> +					      _parent,			\
> +					      &clk_fixed_factor_ops,	\
> +					      _flags),			\
> +	}
> +
> +#define CLK_FIXED_RATE(_struct, _name, _flags,				\
> +		       _fixed_rate, _fixed_accuracy)			\
> +	struct clk_fixed_rate _struct = {				\
> +		.fixed_rate	= _fixed_rate,				\
> +		.fixed_accuracy	= _fixed_accuracy,			\
> +		.hw.init	= CLK_HW_INIT_NO_PARENT(_name,		\
> +					  &clk_fixed_rate_ops,		\
> +							_flags),	\
> +	}

Maybe don't add this one? Usually fixed rate clks come from DT.

On 11/27, Chunyan Zhang wrote:
> +
> +	sprd_clk_set_regmap(desc, regmap);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(sprd_clk_regmap_init);
> +
> +int sprd_clk_probe(struct device *dev, struct clk_hw_onecell_data *clkhw)
> +{
> +	int i, ret = 0;

ret shouldn't need to be initialized here.

> +	struct clk_hw *hw;
> +
> +	for (i = 0; i < clkhw->num; i++) {
> +
> +		hw = clkhw->hws[i];
> +
> +		if (!hw)
> +			continue;
> +
> +		ret = devm_clk_hw_register(dev, hw);
> +		if (ret) {
> +			dev_err(dev, "Couldn't register clock %d - %s\n",
> +				i, hw->init->name);
> +			return ret;
> +		}
> +	}
> +
> +	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
> +				     clkhw);

You can use devm_ now for this.

> +	if (ret)
> +		dev_err(dev, "Failed to add clock provider.\n");

Please remove the full stop on error messages.

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(sprd_clk_probe);
> +
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/sprd/common.h b/drivers/clk/sprd/common.h
> new file mode 100644
> index 0000000..8cd774e
> --- /dev/null
> +++ b/drivers/clk/sprd/common.h
> @@ -0,0 +1,52 @@
> +/*
> + * Spreadtrum clock infrastructure
> + *
> + * Copyright (C) 2017 Spreadtrum, Inc.
> + * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#ifndef _SPRD_CLK_COMMON_H_
> +#define _SPRD_CLK_COMMON_H_
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +
> +#include "../clk_common.h"
> +
> +struct device_node;
> +
> +struct sprd_clk_common {
> +	struct regmap	*regmap;
> +	u32		reg;
> +	struct clk_hw	hw;
> +};
> +
> +struct sprd_clk_desc {
> +	struct sprd_clk_common		**clk_clks;
> +	unsigned long			num_clk_clks;
> +	struct clk_hw_onecell_data      *hw_clks;
> +};
> +
> +#define sprd_regmap_read(map, reg, val)				\
> +({								\
> +	(map) ? regmap_read((map), (reg), (val)) : (-EINVAL);	\

Do we sometimes not have a map? This seems overly cautious.

> +})
> +
> +#define sprd_regmap_write(map, reg, val)			\
> +({								\

On 7 December 2017 at 14:47, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/27, Chunyan Zhang wrote:
>> These macros are used by more than one SoC vendor platforms, avoid to
>> have many copies of these code, this patch moves them to the common
>> clock directory which every clock drivers can access to.
>>
>> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>> ---
>>  drivers/clk/clk_common.h | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 60 insertions(+)
>>  create mode 100644 drivers/clk/clk_common.h
>>
>> diff --git a/drivers/clk/clk_common.h b/drivers/clk/clk_common.h
>> new file mode 100644
>> index 0000000..21e93d2
>> --- /dev/null
>> +++ b/drivers/clk/clk_common.h
>> @@ -0,0 +1,60 @@
>> +/*
>> + * drivers/clk/clk_common.h
>
> We don't need this in the file too. Please remove this line.
>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0
>> + */
>> +
>> +#ifndef _CLK_COMMON_H_
>> +#define _CLK_COMMON_H_
>> +
>> +#include <linux/clk-provider.h>
>
> Maybe these macros should just go into clk-provider.h?

Ok.  And I will also remove the same macros from sunxi-ng/ccu_common.h
and zte/clk.h.

>
>> +
>> +#define CLK_HW_INIT(_name, _parent, _ops, _flags)            \
>> +     (&(struct clk_init_data) {                              \
>> +             .flags          = _flags,                       \
>> +             .name           = _name,                        \
>> +             .parent_names   = (const char *[]) { _parent }, \
>> +             .num_parents    = 1,                            \
>> +             .ops            = _ops,                         \
>> +     })
>
> Hopefully we don't extend the init structure anymore to have
> something else. I guess we'll do something if that happens.
>
>> +
>> +#define CLK_HW_INIT_PARENTS(_name, _parents, _ops, _flags)   \
>> +     (&(struct clk_init_data) {                              \
>> +             .flags          = _flags,                       \
>> +             .name           = _name,                        \
>> +             .parent_names   = _parents,                     \
>> +             .num_parents    = ARRAY_SIZE(_parents),         \
>> +             .ops            = _ops,                         \
>> +     })
>> +
>> +#define CLK_HW_INIT_NO_PARENT(_name, _ops, _flags)     \
>> +     (&(struct clk_init_data) {                      \
>> +             .flags          = _flags,               \
>> +             .name           = _name,                \
>> +             .parent_names   = NULL,                 \
>> +             .num_parents    = 0,                    \
>> +             .ops            = _ops,                 \
>> +     })
>> +
>> +#define CLK_FIXED_FACTOR(_struct, _name, _parent,                    \
>> +                     _div, _mult, _flags)                            \
>> +     struct clk_fixed_factor _struct = {                             \
>> +             .div            = _div,                                 \
>> +             .mult           = _mult,                                \
>> +             .hw.init        = CLK_HW_INIT(_name,                    \
>> +                                           _parent,                  \
>> +                                           &clk_fixed_factor_ops,    \
>> +                                           _flags),                  \
>> +     }
>> +
>> +#define CLK_FIXED_RATE(_struct, _name, _flags,                               \
>> +                    _fixed_rate, _fixed_accuracy)                    \
>> +     struct clk_fixed_rate _struct = {                               \
>> +             .fixed_rate     = _fixed_rate,                          \
>> +             .fixed_accuracy = _fixed_accuracy,                      \
>> +             .hw.init        = CLK_HW_INIT_NO_PARENT(_name,          \
>> +                                       &clk_fixed_rate_ops,          \
>> +                                                     _flags),        \
>> +     }
>
> Maybe don't add this one? Usually fixed rate clks come from DT.

Ok, will also move the fixed rate clks from our driver to DT.

Thanks,
Chunyan

>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html