Re: [PATCH 05/19] clk: meson: add regmap clocks

[Jerome Brunet]

On Thu, 2018-02-08 at 15:33 +0800, Yixun Lan wrote:
> > +/**
> > + * struct clk_regmap_mux_data - regmap backed multiplexer clock specific 
> > data
> > + *
> > + * @hw:  handle between common and hardware-specific interfaces
> > + * @offset:  offset of theregister controlling multiplexer
> > + * @table:   array of parent indexed register values
> > + * @shift:   shift to multiplexer bit field
> > + * @width:   width of mutliplexer bit field
> 
>   ~~ this is wrong, please update to keep it sync with the
> struct definition

Indeed, it is.
This is just a copy/paste of the mux documentation provided in
include/linux/clk-provider.h ... which is also inaccurate.

Thanks for pointing it out

> 
> > + * @flags:   hardware-specific flags
> > + *
> > + * Flags:
> > + * Same as clk_divider except CLK_MUX_HIWORD_MASK which is ignored
> > + */
> > +struct clk_regmap_mux_data {
> > + unsigned intoffset;
> > + u32 *table;
> > + u32 mask;
> 
> ~ here
> > + u8  shift;
> > + u8  flags;
> > +};
> > +

Re: [PATCH 05/19] clk: meson: add regmap clocks

[Jerome Brunet]

On Thu, 2018-02-08 at 15:33 +0800, Yixun Lan wrote:
> > +/**
> > + * struct clk_regmap_mux_data - regmap backed multiplexer clock specific 
> > data
> > + *
> > + * @hw:  handle between common and hardware-specific interfaces
> > + * @offset:  offset of theregister controlling multiplexer
> > + * @table:   array of parent indexed register values
> > + * @shift:   shift to multiplexer bit field
> > + * @width:   width of mutliplexer bit field
> 
>   ~~ this is wrong, please update to keep it sync with the
> struct definition

Indeed, it is.
This is just a copy/paste of the mux documentation provided in
include/linux/clk-provider.h ... which is also inaccurate.

Thanks for pointing it out

> 
> > + * @flags:   hardware-specific flags
> > + *
> > + * Flags:
> > + * Same as clk_divider except CLK_MUX_HIWORD_MASK which is ignored
> > + */
> > +struct clk_regmap_mux_data {
> > + unsigned intoffset;
> > + u32 *table;
> > + u32 mask;
> 
> ~ here
> > + u8  shift;
> > + u8  flags;
> > +};
> > +

Re: [PATCH 05/19] clk: meson: add regmap clocks

[Yixun Lan]

HI Jerome:

On 02/01/18 02:09, Jerome Brunet wrote:
> Meson clock controllers needs to move the classical iomem registers to
> regmap. This is triggered because the HHI controllers found on the GXBB
> and GXL host more than just clocks. To properly handle this, we would
> like to migrate HHI to syscon. Also GXBB AO clock controller already use
> regmap, AXG AO and Audio clock controllers will as well.
> 
> The purpose of this change is to provide a common structure to these
> meson controllers (and possibly others) for regmap based clocks.
> 
> This change provides the basic gate, mux and divider, based on the
> helpers provided by the related generic clocks
> 
> Signed-off-by: Jerome Brunet 
> ---
>  drivers/clk/meson/Kconfig  |   4 +
>  drivers/clk/meson/Makefile |   1 +
>  drivers/clk/meson/clk-regmap.c | 166 
> +
>  drivers/clk/meson/clk-regmap.h | 111 +++
>  4 files changed, 282 insertions(+)
>  create mode 100644 drivers/clk/meson/clk-regmap.c
>  create mode 100644 drivers/clk/meson/clk-regmap.h
> 
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 7694302c70a4..e97e85077da1 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -3,6 +3,10 @@ config COMMON_CLK_AMLOGIC
>   depends on OF
>   depends on ARCH_MESON || COMPILE_TEST
>  
> +config COMMON_CLK_REGMAP_MESON
> + bool
> + select REGMAP
> +
>  config COMMON_CLK_MESON8B
>   bool
>   depends on COMMON_CLK_AMLOGIC
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 3c03ce583798..11a5058a 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-cpu.o 
> clk-mpll.o clk-audio-div
>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o 
> gxbb-aoclk-regmap.o gxbb-aoclk-32k.o
>  obj-$(CONFIG_COMMON_CLK_AXG)  += axg.o
> +obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)+= clk-regmap.o
> diff --git a/drivers/clk/meson/clk-regmap.c b/drivers/clk/meson/clk-regmap.c
> new file mode 100644
> index ..3645fdb62343
> --- /dev/null
> +++ b/drivers/clk/meson/clk-regmap.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 BayLibre, SAS.
> +// Author: Jerome Brunet 
> +
> +#include "clk-regmap.h"
> +
> +static int clk_regmap_gate_endisable(struct clk_hw *hw, int enable)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_gate_data *gate = clk_get_regmap_gate_data(clk);
> + int set = gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;
> +
> + set ^= enable;
> +
> + return regmap_update_bits(clk->map, gate->offset, BIT(gate->bit_idx),
> +   set ? BIT(gate->bit_idx) : 0);
> +}
> +
> +static int clk_regmap_gate_enable(struct clk_hw *hw)
> +{
> + return clk_regmap_gate_endisable(hw, 1);
> +}
> +
> +static void clk_regmap_gate_disable(struct clk_hw *hw)
> +{
> + clk_regmap_gate_endisable(hw, 0);
> +}
> +
> +static int clk_regmap_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_gate_data *gate = clk_get_regmap_gate_data(clk);
> + unsigned int val;
> +
> + regmap_read(clk->map, gate->offset, );
> + if (gate->flags & CLK_GATE_SET_TO_DISABLE)
> + val ^= BIT(gate->bit_idx);
> +
> + val &= BIT(gate->bit_idx);
> +
> + return val ? 1 : 0;
> +}
> +
> +const struct clk_ops clk_regmap_gate_ops = {
> + .enable = clk_regmap_gate_enable,
> + .disable = clk_regmap_gate_disable,
> + .is_enabled = clk_regmap_gate_is_enabled,
> +};
> +EXPORT_SYMBOL_GPL(clk_regmap_gate_ops);
> +
> +static unsigned long clk_regmap_div_recalc_rate(struct clk_hw *hw,
> + unsigned long prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(clk->map, div->offset, );
> + if (ret)
> + /* Gives a hint that something is wrong */
> + return 0;
> +
> + val >>= div->shift;
> + val &= clk_div_mask(div->width);
> + return divider_recalc_rate(hw, prate, val, div->table, div->flags,
> +div->width);
> +}
> +
> +static long clk_regmap_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +   unsigned long *prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
> + unsigned int val;
> + int ret;
> +
> + /* if read only, just return current value */
> + if (div->flags & CLK_DIVIDER_READ_ONLY) {
> + ret = regmap_read(clk->map, div->offset, 

Re: [PATCH 05/19] clk: meson: add regmap clocks

[Yixun Lan]

HI Jerome:

On 02/01/18 02:09, Jerome Brunet wrote:
> Meson clock controllers needs to move the classical iomem registers to
> regmap. This is triggered because the HHI controllers found on the GXBB
> and GXL host more than just clocks. To properly handle this, we would
> like to migrate HHI to syscon. Also GXBB AO clock controller already use
> regmap, AXG AO and Audio clock controllers will as well.
> 
> The purpose of this change is to provide a common structure to these
> meson controllers (and possibly others) for regmap based clocks.
> 
> This change provides the basic gate, mux and divider, based on the
> helpers provided by the related generic clocks
> 
> Signed-off-by: Jerome Brunet 
> ---
>  drivers/clk/meson/Kconfig  |   4 +
>  drivers/clk/meson/Makefile |   1 +
>  drivers/clk/meson/clk-regmap.c | 166 
> +
>  drivers/clk/meson/clk-regmap.h | 111 +++
>  4 files changed, 282 insertions(+)
>  create mode 100644 drivers/clk/meson/clk-regmap.c
>  create mode 100644 drivers/clk/meson/clk-regmap.h
> 
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 7694302c70a4..e97e85077da1 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -3,6 +3,10 @@ config COMMON_CLK_AMLOGIC
>   depends on OF
>   depends on ARCH_MESON || COMPILE_TEST
>  
> +config COMMON_CLK_REGMAP_MESON
> + bool
> + select REGMAP
> +
>  config COMMON_CLK_MESON8B
>   bool
>   depends on COMMON_CLK_AMLOGIC
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 3c03ce583798..11a5058a 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-cpu.o 
> clk-mpll.o clk-audio-div
>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o 
> gxbb-aoclk-regmap.o gxbb-aoclk-32k.o
>  obj-$(CONFIG_COMMON_CLK_AXG)  += axg.o
> +obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)+= clk-regmap.o
> diff --git a/drivers/clk/meson/clk-regmap.c b/drivers/clk/meson/clk-regmap.c
> new file mode 100644
> index ..3645fdb62343
> --- /dev/null
> +++ b/drivers/clk/meson/clk-regmap.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 BayLibre, SAS.
> +// Author: Jerome Brunet 
> +
> +#include "clk-regmap.h"
> +
> +static int clk_regmap_gate_endisable(struct clk_hw *hw, int enable)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_gate_data *gate = clk_get_regmap_gate_data(clk);
> + int set = gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;
> +
> + set ^= enable;
> +
> + return regmap_update_bits(clk->map, gate->offset, BIT(gate->bit_idx),
> +   set ? BIT(gate->bit_idx) : 0);
> +}
> +
> +static int clk_regmap_gate_enable(struct clk_hw *hw)
> +{
> + return clk_regmap_gate_endisable(hw, 1);
> +}
> +
> +static void clk_regmap_gate_disable(struct clk_hw *hw)
> +{
> + clk_regmap_gate_endisable(hw, 0);
> +}
> +
> +static int clk_regmap_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_gate_data *gate = clk_get_regmap_gate_data(clk);
> + unsigned int val;
> +
> + regmap_read(clk->map, gate->offset, );
> + if (gate->flags & CLK_GATE_SET_TO_DISABLE)
> + val ^= BIT(gate->bit_idx);
> +
> + val &= BIT(gate->bit_idx);
> +
> + return val ? 1 : 0;
> +}
> +
> +const struct clk_ops clk_regmap_gate_ops = {
> + .enable = clk_regmap_gate_enable,
> + .disable = clk_regmap_gate_disable,
> + .is_enabled = clk_regmap_gate_is_enabled,
> +};
> +EXPORT_SYMBOL_GPL(clk_regmap_gate_ops);
> +
> +static unsigned long clk_regmap_div_recalc_rate(struct clk_hw *hw,
> + unsigned long prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(clk->map, div->offset, );
> + if (ret)
> + /* Gives a hint that something is wrong */
> + return 0;
> +
> + val >>= div->shift;
> + val &= clk_div_mask(div->width);
> + return divider_recalc_rate(hw, prate, val, div->table, div->flags,
> +div->width);
> +}
> +
> +static long clk_regmap_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +   unsigned long *prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
> + unsigned int val;
> + int ret;
> +
> + /* if read only, just return current value */
> + if (div->flags & CLK_DIVIDER_READ_ONLY) {
> + ret = regmap_read(clk->map, div->offset, );
> + if (ret)
> +