by Neil Armstrong

[permalink] [raw]

Subject: Re: [PATCH 3/7] clk: meson: add triple phase clock driver

On 26/04/2018 10:47, Neil Armstrong wrote:
> On 25/04/2018 18:33, Jerome Brunet wrote:
>> Add a driver to control the output of the sample clock generator found
>> in the axg audio clock controller.
>>
>> The goal of this driver is to coherently control the phase provided to
>> the different element using the sample clock generator. This simplify
>> the usage of the sample clock generator a lot, without comprising the
>> ability of the SoC.
>>
>> Signed-off-by: Jerome Brunet <[email protected]>
>> ---
>> drivers/clk/meson/Kconfig | 5 +++
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/clk-triphase.c | 68 ++++++++++++++++++++++++++++++++++++++++
>> drivers/clk/meson/clkc-audio.h | 20 ++++++++++++
>> 4 files changed, 94 insertions(+)
>> create mode 100644 drivers/clk/meson/clk-triphase.c
>> create mode 100644 drivers/clk/meson/clkc-audio.h
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index 87d69573e172..7f7fd6fb3809 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -3,6 +3,11 @@ config COMMON_CLK_AMLOGIC
>> depends on ARCH_MESON || COMPILE_TEST
>> select COMMON_CLK_REGMAP_MESON
>>
>> +config COMMON_CLK_AMLOGIC_AUDIO
>> + bool
>> + depends on ARCH_MESON || COMPILE_TEST
>> + select COMMON_CLK_AMLOGIC
>> +
>> config COMMON_CLK_REGMAP_MESON
>> bool
>> select REGMAP
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index 352fb848c406..64bb917fe1f0 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -4,6 +4,7 @@
>>
>> obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
>> obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-phase.o
>> +obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO) += clk-triphase.o
>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
>> obj-$(CONFIG_COMMON_CLK_AXG) += axg.o
>> diff --git a/drivers/clk/meson/clk-triphase.c b/drivers/clk/meson/clk-triphase.c
>> new file mode 100644
>> index 000000000000..9508c03c73c1
>> --- /dev/null
>> +++ b/drivers/clk/meson/clk-triphase.c
>> @@ -0,0 +1,68 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2018 BayLibre, SAS.
>> + * Author: Jerome Brunet <[email protected]>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include "clkc-audio.h"
>> +
>> +/*
>> + * This is a special clock for the audio controller.
>> + * The phase of mst_sclk clock output can be controlled independently
>> + * for the outside world (ph0), the tdmout (ph1) and tdmin (ph2).
>> + * Controlling these 3 phases as just one makes things simpler and
>> + * give the same clock view to all the element on the i2s bus.
>> + * If necessary, we can still control the phase in the tdm block
>> + * which makes these independent control redundant.
>> + */
>> +static inline struct meson_clk_triphase_data *
>> +meson_clk_triphase_data(struct clk_regmap *clk)
>> +{
>> + return (struct meson_clk_triphase_data *)clk->data;
>> +}
>> +
>> +static void meson_clk_triphase_sync(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk);
>> + unsigned int val;
>> +
>> + /* Get phase 0 and sync it to phase 1 and 2 */
>> + val = meson_parm_read(clk->map, &tph->ph0);
>> + meson_parm_write(clk->map, &tph->ph1, val);
>> + meson_parm_write(clk->map, &tph->ph2, val);
>> +}
>> +
>> +static int meson_clk_triphase_get_phase(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk);
>> + unsigned int val;
>> +
>> + /* Phase are in sync, reading phase 0 is enough */
>> + val = meson_parm_read(clk->map, &tph->ph0);
>> +
>> + return meson_clk_degrees_from_val(val, tph->ph0.width);
>> +}
>> +
>> +static int meson_clk_triphase_set_phase(struct clk_hw *hw, int degrees)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk);
>> + unsigned int val;
>> +
>> + val = meson_clk_degrees_to_val(degrees, tph->ph0.width);
>> + meson_parm_write(clk->map, &tph->ph0, val);
>> + meson_parm_write(clk->map, &tph->ph1, val);
>> + meson_parm_write(clk->map, &tph->ph2, val);
>> +
>> + return 0;
>> +}
>> +
>> +const struct clk_ops meson_clk_triphase_ops = {
>> + .init = meson_clk_triphase_sync,
>> + .get_phase = meson_clk_triphase_get_phase,
>> + .set_phase = meson_clk_triphase_set_phase,
>> +};
>> +EXPORT_SYMBOL_GPL(meson_clk_triphase_ops);
>> diff --git a/drivers/clk/meson/clkc-audio.h b/drivers/clk/meson/clkc-audio.h
>> new file mode 100644
>> index 000000000000..286ff1201258
>> --- /dev/null
>> +++ b/drivers/clk/meson/clkc-audio.h
>> @@ -0,0 +1,20 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>
> // SPDX-License-Identifier: GPL-2.0
>
> Checkpatch should have warned about this !

My bad, I wasn't totally aware there is an exception in headers...

>
>> +/*
>> + * Copyright (c) 2018 BayLibre, SAS.
>> + * Author: Jerome Brunet <[email protected]>
>> + */
>> +
>> +#ifndef __MESON_CLKC_AUDIO_H
>> +#define __MESON_CLKC_AUDIO_H
>> +
>> +#include "clkc.h"
>> +
>> +struct meson_clk_triphase_data {
>> + struct parm ph0;
>> + struct parm ph1;
>> + struct parm ph2;
>> +};
>> +
>> +extern const struct clk_ops meson_clk_triphase_ops;
>> +
>> +#endif /* __MESON_CLKC_AUDIO_H */
>>
>
> Apart that :
>
> Acked-by: Neil Armstrong <[email protected]>
>

2018-04-27 01:15:45

Quoting Neil Armstrong (2018-04-26 01:49:29)
> On 25/04/2018 18:33, Jerome Brunet wrote:
> > +#endif /*__AXG_AUDIO_CLKC_H */
> >
>
> Acked-by: Neil Armstrong <[email protected]>

Please trim replies.