diff mbox series

[2/2] clk: meson-g12a: Add AO Clock controller driver

Message ID 1533894868-85815-3-git-send-email-jian.hu@amlogic.com (mailing list archive)
State Changes Requested
Headers show
Series clk: meson-g12a: Add AO clock controller driver | expand

Commit Message

Jian Hu Aug. 10, 2018, 9:54 a.m. UTC 
Add a Clock driver for the ALways-On part
of the Amlogic Meson-G12A SoC.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 drivers/clk/meson/Makefile     |   2 +-
 drivers/clk/meson/g12a-aoclk.c | 170 +++++++++++++++++++++++++++++++++++++++++
 drivers/clk/meson/g12a-aoclk.h |  36 +++++++++
 3 files changed, 207 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/meson/g12a-aoclk.c
 create mode 100644 drivers/clk/meson/g12a-aoclk.h

Comments

Jerome Brunet Aug. 14, 2018, 12:40 p.m. UTC | #1 
On Fri, 2018-08-10 at 17:54 +0800, Jian Hu wrote:
> Add a Clock driver for the ALways-On part
> of the Amlogic Meson-G12A SoC.
> 
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
>  drivers/clk/meson/Makefile     |   2 +-
>  drivers/clk/meson/g12a-aoclk.c | 170 +++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/meson/g12a-aoclk.h |  36 +++++++++
>  3 files changed, 207 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/meson/g12a-aoclk.c
>  create mode 100644 drivers/clk/meson/g12a-aoclk.h
> 
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 2b1a562..d5c2dcd 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -9,5 +9,5 @@ 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 axg-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_AXG_AUDIO)	+= axg-audio.o
> -obj-$(CONFIG_COMMON_CLK_G12A)	 += g12a.o
> +obj-$(CONFIG_COMMON_CLK_G12A)	 += g12a.o g12a-aoclk.c
>  obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)	+= clk-regmap.o
> diff --git a/drivers/clk/meson/g12a-aoclk.c b/drivers/clk/meson/g12a-aoclk.c
> new file mode 100644
> index 0000000..a5cd95c
> --- /dev/null
> +++ b/drivers/clk/meson/g12a-aoclk.c
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Amlogic Meson-G12A Clock Controller Driver
> + *
> + * Copyright (c) 2016 Baylibre SAS.
> + * Author: Michael Turquette <mturquette@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Jian Hu <jian.hu@amlogic.com>
> + */
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/init.h>
> +#include "clkc.h"
> +#include "g12a-aoclk.h"
> +
> +#define G12A_AO_GATE0(_name, _bit)					\
> +static struct clk_regmap _name##_ao = {					\
> +	.data = &(struct clk_regmap_gate_data) {			\
> +		.offset = (AO_CLK_GATE0),			\
> +		.bit_idx = (_bit),					\
> +	},								\
> +	.hw.init = &(struct clk_init_data) {				\
> +		.name = #_name "_ao",					\
> +		.ops = &clk_regmap_gate_ops,				\
> +		.parent_names = (const char *[]){ "clk81" },		\
> +		.num_parents = 1,					\
> +	},								\
> +}
> +
> +G12A_AO_GATE0(ahb_bus,		0);
> +G12A_AO_GATE0(remote,		1);
> +G12A_AO_GATE0(i2c_master,	2);
> +G12A_AO_GATE0(i2c_slave,	3);
> +G12A_AO_GATE0(uart1,		4);
> +G12A_AO_GATE0(prod_i2c,		5);
> +G12A_AO_GATE0(uart2,		6);
> +G12A_AO_GATE0(ir_blaster,	7);
> +G12A_AO_GATE0(saradc,		8);
> +
> +static struct clk_regmap ao_clk81 = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = AO_RTI_PWR_CNTL_REG0,
> +		.mask = 0x1,
> +		.shift = 8,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "ao_clk81",
> +		.ops = &clk_regmap_mux_ro_ops,
> +		.parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},

I think it is time we stop taking clock input from nowhere.
With the addition of the axg audio clock controller, there is now an example of
how do so.

This clock controller apparently has 3 inputs:
* xtal
* ao_xtal
* clk_81

I'd like to see that appear this DT bindings and the probe function

Same goes for the EE controller which should only take the xtal.

> +		.num_parents = 2,
> +	},
> +};
> +
> +static struct clk_regmap g12a_saradc_mux = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = AO_SAR_CLK,
> +		.mask = 0x3,
> +		.shift = 9,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "g12a_saradc_mux",
> +		.ops = &clk_regmap_mux_ops,
> +		.parent_names = (const char *[]){ "xtal", "ao_clk81" },
> +		.num_parents = 2,
> +	},
> +};
> +
> +static struct clk_regmap g12a_saradc_div = {
> +	.data = &(struct clk_regmap_div_data) {
> +		.offset = AO_SAR_CLK,
> +		.shift = 0,
> +		.width = 8,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "g12a_saradc_div",
> +		.ops = &clk_regmap_divider_ops,
> +		.parent_names = (const char *[]){ "g12a_saradc_mux" },
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_regmap g12a_saradc_gate = {
> +	.data = &(struct clk_regmap_gate_data) {
> +		.offset = AO_SAR_CLK,
> +		.bit_idx = 8,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "g12a_saradc_gate",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_names = (const char *[]){ "g12a_saradc_div" },
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static unsigned int g12a_aoclk_reset[] = {
> +	[RESET_AO_REMOTE] =	16,
> +	[RESET_AO_UART1] =	17,
> +	[RESET_AO_I2C_MASTER] = 18,
> +	[RESET_AO_I2C_SLAVE] =	19,
> +	[RESET_AO_SARADC] =	20,
> +	[RESET_AO_UART2] =	22,
> +	[RESET_AO_IR_BLASTER] = 23,
> +};
> +
> +static struct clk_regmap *g12a_aoclk_regmap[] = {
> +	[CLKID_AO_AHB_BUS]	= &ahb_bus_ao,
> +	[CLKID_AO_REMOTE]	= &remote_ao,
> +	[CLKID_AO_I2C_MASTER]	= &i2c_master_ao,
> +	[CLKID_AO_I2C_SLAVE]	= &i2c_slave_ao,
> +	[CLKID_AO_UART1]	= &uart1_ao,
> +	[CLKID_AO_PROD_I2C]	= &prod_i2c_ao,
> +	[CLKID_AO_UART2]	= &uart2_ao,
> +	[CLKID_AO_IR_BLASTER]	= &ir_blaster_ao,
> +	[CLKID_AO_SAR_ADC]	= &saradc_ao,
> +	[CLKID_AO_CLK81]	= &ao_clk81,
> +	[CLKID_AO_SAR_ADC_SEL]	= &g12a_saradc_mux,
> +	[CLKID_AO_SAR_ADC_DIV]	= &g12a_saradc_div,
> +	[CLKID_AO_SAR_ADC_CLK]	= &g12a_saradc_gate,

Please be consistent while naming the clock, either prefix all with g12a, or
none.

> +};
> +
> +static struct clk_hw_onecell_data g12a_aoclk_onecell_data = {
> +	.hws = {
> +		[CLKID_AO_AHB_BUS]	= &ahb_bus_ao.hw,
> +		[CLKID_AO_REMOTE]	= &remote_ao.hw,
> +		[CLKID_AO_I2C_MASTER]	= &i2c_master_ao.hw,
> +		[CLKID_AO_I2C_SLAVE]	= &i2c_slave_ao.hw,
> +		[CLKID_AO_UART1]	= &uart1_ao.hw,
> +		[CLKID_AO_PROD_I2C]	= &prod_i2c_ao.hw,
> +		[CLKID_AO_UART2]	= &uart2_ao.hw,
> +		[CLKID_AO_IR_BLASTER]	= &ir_blaster_ao.hw,
> +		[CLKID_AO_SAR_ADC]	= &saradc_ao.hw,
> +		[CLKID_AO_CLK81]	= &ao_clk81.hw,
> +		[CLKID_AO_SAR_ADC_SEL]	= &g12a_saradc_mux.hw,
> +		[CLKID_AO_SAR_ADC_DIV]	= &g12a_saradc_div.hw,
> +		[CLKID_AO_SAR_ADC_CLK]	= &g12a_saradc_gate.hw,
> +	},
> +	.num = NR_CLKS,
> +};
> +
> +static struct meson_aoclk_data g12a_aoclkc_data = {
> +	.reset_reg	= AO_RTI_GEN_CNTL_REG0,
> +	.num_reset	= ARRAY_SIZE(g12a_aoclk_reset),
> +	.reset		= g12a_aoclk_reset,
> +	.num_clks	= ARRAY_SIZE(g12a_aoclk_regmap),
> +	.clks		= g12a_aoclk_regmap,
> +	.hw_data	= &g12a_aoclk_onecell_data,
> +};
> +
> +static const struct of_device_id g12a_aoclkc_match_table[] = {
> +	{
> +		.compatible	= "amlogic,g12a-aoclkc",
> +		.data		= &g12a_aoclkc_data,
> +	},
> +	{ }
> +};
> +
> +static struct platform_driver g12a_aoclkc_driver = {
> +	.probe		= meson_aoclkc_probe,
> +	.driver		= {
> +		.name	= "g12a-aoclkc",
> +		.of_match_table = g12a_aoclkc_match_table,
> +	},
> +};
> +
> +builtin_platform_driver(g12a_aoclkc_driver);
> diff --git a/drivers/clk/meson/g12a-aoclk.h b/drivers/clk/meson/g12a-aoclk.h
> new file mode 100644
> index 0000000..007183e
> --- /dev/null
> +++ b/drivers/clk/meson/g12a-aoclk.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2017 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Jian Hu <Jian.Hu@amlogic.com>
> + */
> +
> +#ifndef __G12A_AOCLKC_H
> +#define __G12A_AOCLKC_H
> +
> +//#include "meson-aoclk.h"

??? Do you need this or not ?

> +
> +#define NR_CLKS				14
> +
> +/* AO Configuration Clock registers offsets
> + * Register offsets from the data sheet must be multiplied by 4.
> + */
> +#define AO_RTI_PWR_CNTL_REG0		0x10
> +#define AO_RTI_GEN_CNTL_REG0		0x40
> +#define AO_OSCIN_CNTL			0x58
> +#define AO_CRT_CLK_CNTL1		0x68
> +#define AO_SAR_CLK			0x90
> +#define AO_RTC_ALT_CLK_CNTL0		0x94
> +#define AO_RTC_ALT_CLK_CNTL1		0x98
> +
> +/*
> + *AO CLK81 gate clocks
> + */
> +#define AO_CLK_GATE0			0x4c
> +
> +#include <dt-bindings/clock/g12a-aoclkc.h>
> +#include <dt-bindings/reset/g12a-aoclkc.h>
> +
> +#endif /* __G12A_AOCLKC_H */
Jian Hu Aug. 24, 2018, 1:34 p.m. UTC | #2 
Hi: Jerome

Please see my commits.

On 2018/8/14 20:40, Jerome Brunet wrote:
> On Fri, 2018-08-10 at 17:54 +0800, Jian Hu wrote:
>> Add a Clock driver for the ALways-On part
>> of the Amlogic Meson-G12A SoC.
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>>   drivers/clk/meson/Makefile     |   2 +-
>>   drivers/clk/meson/g12a-aoclk.c | 170 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/clk/meson/g12a-aoclk.h |  36 +++++++++
>>   3 files changed, 207 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/clk/meson/g12a-aoclk.c
>>   create mode 100644 drivers/clk/meson/g12a-aoclk.h
>>
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index 2b1a562..d5c2dcd 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -9,5 +9,5 @@ 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 axg-aoclk.o
>>   obj-$(CONFIG_COMMON_CLK_AXG_AUDIO)	+= axg-audio.o
>> -obj-$(CONFIG_COMMON_CLK_G12A)	 += g12a.o
>> +obj-$(CONFIG_COMMON_CLK_G12A)	 += g12a.o g12a-aoclk.c
>>   obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)	+= clk-regmap.o
>> diff --git a/drivers/clk/meson/g12a-aoclk.c b/drivers/clk/meson/g12a-aoclk.c
>> new file mode 100644
>> index 0000000..a5cd95c
>> --- /dev/null
>> +++ b/drivers/clk/meson/g12a-aoclk.c
>> @@ -0,0 +1,170 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Amlogic Meson-G12A Clock Controller Driver
>> + *
>> + * Copyright (c) 2016 Baylibre SAS.
>> + * Author: Michael Turquette <mturquette@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Jian Hu <jian.hu@amlogic.com>
>> + */
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/init.h>
>> +#include "clkc.h"
>> +#include "g12a-aoclk.h"
>> +
>> +#define G12A_AO_GATE0(_name, _bit)					\
>> +static struct clk_regmap _name##_ao = {					\
>> +	.data = &(struct clk_regmap_gate_data) {			\
>> +		.offset = (AO_CLK_GATE0),			\
>> +		.bit_idx = (_bit),					\
>> +	},								\
>> +	.hw.init = &(struct clk_init_data) {				\
>> +		.name = #_name "_ao",					\
>> +		.ops = &clk_regmap_gate_ops,				\
>> +		.parent_names = (const char *[]){ "clk81" },		\
>> +		.num_parents = 1,					\
>> +	},								\
>> +}
>> +
>> +G12A_AO_GATE0(ahb_bus,		0);
>> +G12A_AO_GATE0(remote,		1);
>> +G12A_AO_GATE0(i2c_master,	2);
>> +G12A_AO_GATE0(i2c_slave,	3);
>> +G12A_AO_GATE0(uart1,		4);
>> +G12A_AO_GATE0(prod_i2c,		5);
>> +G12A_AO_GATE0(uart2,		6);
>> +G12A_AO_GATE0(ir_blaster,	7);
>> +G12A_AO_GATE0(saradc,		8);
>> +
>> +static struct clk_regmap ao_clk81 = {
>> +	.data = &(struct clk_regmap_mux_data) {
>> +		.offset = AO_RTI_PWR_CNTL_REG0,
>> +		.mask = 0x1,
>> +		.shift = 8,
>> +	},
>> +	.hw.init = &(struct clk_init_data){
>> +		.name = "ao_clk81",
>> +		.ops = &clk_regmap_mux_ro_ops,
>> +		.parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},
> 
> I think it is time we stop taking clock input from nowhere.
> With the addition of the axg audio clock controller, there is now an example of
> how do so.
> 
> This clock controller apparently has 3 inputs:
> * xtal
> * ao_xtal
> * clk_81
> 
> I'd like to see that appear this DT bindings and the probe function
> 
> Same goes for the EE controller which should only take the xtal.
> 

I am confued about aoclk81's parent clocks.

I can not get the example of axg audio clock driver, Could you provide 
the link? Had it merged into clk-meson.git?

my local latest commit is:

commit cd2b3132bd7e84dc6a739fef73e5b5c3f2b3a0bb
Author: Jerome Brunet <jbrunet@baylibre.com>
Date:   Wed Aug 1 16:00:53 2018 +0200

     clk: meson: clk-pll: drop hard-coded rates from pll tables

     Putting hard-coded rates inside the parameter tables assumes that
     the parent is known and will never change. That's a big assumption
     we should not make.

     We have everything we need to recalculate the output rate using
     the parent rate and the rest of the parameters. Let's do so and
     drop the rates from the tables.

     Acked-by: Neil Armstrong <narmstrong@baylibre.com>
     Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

>> +		.num_parents = 2,
>> +	},
>> +};
>> +
>> +static struct clk_regmap g12a_saradc_mux = {
>> +	.data = &(struct clk_regmap_mux_data) {
>> +		.offset = AO_SAR_CLK,
>> +		.mask = 0x3,
>> +		.shift = 9,
>> +	},
>> +	.hw.init = &(struct clk_init_data){
>> +		.name = "g12a_saradc_mux",
>> +		.ops = &clk_regmap_mux_ops,
>> +		.parent_names = (const char *[]){ "xtal", "ao_clk81" },
>> +		.num_parents = 2,
>> +	},
>> +};
>> +
>> +static struct clk_regmap g12a_saradc_div = {
>> +	.data = &(struct clk_regmap_div_data) {
>> +		.offset = AO_SAR_CLK,
>> +		.shift = 0,
>> +		.width = 8,
>> +	},
>> +	.hw.init = &(struct clk_init_data){
>> +		.name = "g12a_saradc_div",
>> +		.ops = &clk_regmap_divider_ops,
>> +		.parent_names = (const char *[]){ "g12a_saradc_mux" },
>> +		.num_parents = 1,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +	},
>> +};
>> +
>> +static struct clk_regmap g12a_saradc_gate = {
>> +	.data = &(struct clk_regmap_gate_data) {
>> +		.offset = AO_SAR_CLK,
>> +		.bit_idx = 8,
>> +	},
>> +	.hw.init = &(struct clk_init_data){
>> +		.name = "g12a_saradc_gate",
>> +		.ops = &clk_regmap_gate_ops,
>> +		.parent_names = (const char *[]){ "g12a_saradc_div" },
>> +		.num_parents = 1,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +	},
>> +};
>> +
>> +static unsigned int g12a_aoclk_reset[] = {
>> +	[RESET_AO_REMOTE] =	16,
>> +	[RESET_AO_UART1] =	17,
>> +	[RESET_AO_I2C_MASTER] = 18,
>> +	[RESET_AO_I2C_SLAVE] =	19,
>> +	[RESET_AO_SARADC] =	20,
>> +	[RESET_AO_UART2] =	22,
>> +	[RESET_AO_IR_BLASTER] = 23,
>> +};
>> +
>> +static struct clk_regmap *g12a_aoclk_regmap[] = {
>> +	[CLKID_AO_AHB_BUS]	= &ahb_bus_ao,
>> +	[CLKID_AO_REMOTE]	= &remote_ao,
>> +	[CLKID_AO_I2C_MASTER]	= &i2c_master_ao,
>> +	[CLKID_AO_I2C_SLAVE]	= &i2c_slave_ao,
>> +	[CLKID_AO_UART1]	= &uart1_ao,
>> +	[CLKID_AO_PROD_I2C]	= &prod_i2c_ao,
>> +	[CLKID_AO_UART2]	= &uart2_ao,
>> +	[CLKID_AO_IR_BLASTER]	= &ir_blaster_ao,
>> +	[CLKID_AO_SAR_ADC]	= &saradc_ao,
>> +	[CLKID_AO_CLK81]	= &ao_clk81,
>> +	[CLKID_AO_SAR_ADC_SEL]	= &g12a_saradc_mux,
>> +	[CLKID_AO_SAR_ADC_DIV]	= &g12a_saradc_div,
>> +	[CLKID_AO_SAR_ADC_CLK]	= &g12a_saradc_gate,
> 
> Please be consistent while naming the clock, either prefix all with g12a, or
> none.
> 
OK, I will add g12a prefix to be consistent with other clocks
>> +};
>> +
>> +static struct clk_hw_onecell_data g12a_aoclk_onecell_data = {
>> +	.hws = {
>> +		[CLKID_AO_AHB_BUS]	= &ahb_bus_ao.hw,
>> +		[CLKID_AO_REMOTE]	= &remote_ao.hw,
>> +		[CLKID_AO_I2C_MASTER]	= &i2c_master_ao.hw,
>> +		[CLKID_AO_I2C_SLAVE]	= &i2c_slave_ao.hw,
>> +		[CLKID_AO_UART1]	= &uart1_ao.hw,
>> +		[CLKID_AO_PROD_I2C]	= &prod_i2c_ao.hw,
>> +		[CLKID_AO_UART2]	= &uart2_ao.hw,
>> +		[CLKID_AO_IR_BLASTER]	= &ir_blaster_ao.hw,
>> +		[CLKID_AO_SAR_ADC]	= &saradc_ao.hw,
>> +		[CLKID_AO_CLK81]	= &ao_clk81.hw,
>> +		[CLKID_AO_SAR_ADC_SEL]	= &g12a_saradc_mux.hw,
>> +		[CLKID_AO_SAR_ADC_DIV]	= &g12a_saradc_div.hw,
>> +		[CLKID_AO_SAR_ADC_CLK]	= &g12a_saradc_gate.hw,
>> +	},
>> +	.num = NR_CLKS,
>> +};
>> +
>> +static struct meson_aoclk_data g12a_aoclkc_data = {
>> +	.reset_reg	= AO_RTI_GEN_CNTL_REG0,
>> +	.num_reset	= ARRAY_SIZE(g12a_aoclk_reset),
>> +	.reset		= g12a_aoclk_reset,
>> +	.num_clks	= ARRAY_SIZE(g12a_aoclk_regmap),
>> +	.clks		= g12a_aoclk_regmap,
>> +	.hw_data	= &g12a_aoclk_onecell_data,
>> +};
>> +
>> +static const struct of_device_id g12a_aoclkc_match_table[] = {
>> +	{
>> +		.compatible	= "amlogic,g12a-aoclkc",
>> +		.data		= &g12a_aoclkc_data,
>> +	},
>> +	{ }
>> +};
>> +
>> +static struct platform_driver g12a_aoclkc_driver = {
>> +	.probe		= meson_aoclkc_probe,
>> +	.driver		= {
>> +		.name	= "g12a-aoclkc",
>> +		.of_match_table = g12a_aoclkc_match_table,
>> +	},
>> +};
>> +
>> +builtin_platform_driver(g12a_aoclkc_driver);
>> diff --git a/drivers/clk/meson/g12a-aoclk.h b/drivers/clk/meson/g12a-aoclk.h
>> new file mode 100644
>> index 0000000..007183e
>> --- /dev/null
>> +++ b/drivers/clk/meson/g12a-aoclk.h
>> @@ -0,0 +1,36 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2017 BayLibre, SAS
>> + * Author: Neil Armstrong <narmstrong@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Jian Hu <Jian.Hu@amlogic.com>
>> + */
>> +
>> +#ifndef __G12A_AOCLKC_H
>> +#define __G12A_AOCLKC_H
>> +
>> +//#include "meson-aoclk.h"
> 
> ??? Do you need this or not ?
> It is useless,I will delete it, Thanks for review.
>> +
>> +#define NR_CLKS				14
>> +
>> +/* AO Configuration Clock registers offsets
>> + * Register offsets from the data sheet must be multiplied by 4.
>> + */
>> +#define AO_RTI_PWR_CNTL_REG0		0x10
>> +#define AO_RTI_GEN_CNTL_REG0		0x40
>> +#define AO_OSCIN_CNTL			0x58
>> +#define AO_CRT_CLK_CNTL1		0x68
>> +#define AO_SAR_CLK			0x90
>> +#define AO_RTC_ALT_CLK_CNTL0		0x94
>> +#define AO_RTC_ALT_CLK_CNTL1		0x98
>> +
>> +/*
>> + *AO CLK81 gate clocks
>> + */
>> +#define AO_CLK_GATE0			0x4c
>> +
>> +#include <dt-bindings/clock/g12a-aoclkc.h>
>> +#include <dt-bindings/reset/g12a-aoclkc.h>
>> +
>> +#endif /* __G12A_AOCLKC_H */
> 
> 
> .
>
Jerome Brunet Aug. 27, 2018, 1:07 p.m. UTC | #3 
On Fri, 2018-08-24 at 21:34 +0800, Jian Hu wrote:
> > 
> 
> I am confued about aoclk81's parent clocks.
> 
> I can not get the example of axg audio clock driver, Could you provide 
> the link? Had it merged into clk-meson.git?

Yes and mainline as well : drivers/clk/meson/axg-audio.c

Basically this driver is creating bypass input clocks (audio_pclk, mst_in[0-9],
etc...) .

This allows to collect input clocks from DT (like any consumer should) will
keeping constant in the controller clock tree.

From what I've seen of your controller drivers, the EE controller should have
one input, the AO should have 3.
Jian Hu Aug. 28, 2018, 3:35 a.m. UTC | #4 
Hi: Jerome

On 2018/8/27 21:07, Jerome Brunet wrote:
> On Fri, 2018-08-24 at 21:34 +0800, Jian Hu wrote:
>>>
>>
>> I am confued about aoclk81's parent clocks.
>>
>> I can not get the example of axg audio clock driver, Could you provide
>> the link? Had it merged into clk-meson.git?
> 
> Yes and mainline as well : drivers/clk/meson/axg-audio.c
> 
> Basically this driver is creating bypass input clocks (audio_pclk, mst_in[0-9],
> etc...) .
> 
> This allows to collect input clocks from DT (like any consumer should) will
> keeping constant in the controller clock tree.
> 
>>From what I've seen of your controller drivers, the EE controller should have
> one input, the AO should have 3.
> 
> 
> 
> .
> 
I still can not get the example meaning in axg audio driver.

In 26 page of A113D_Datasheet V0.7 20170725-Baylibre.pdf,We can see the 
aoclk81 has two parents. clk81 and ao_slow_clk. I can not get 3 parents.

                          clk81|\
                        -------| \        aoclk81
src0 |\                       |  |-------------------
-----| \     ao_slow_clk      |  |
      |  |---------------------| /
-----| /                      |/
  src1|/


src0 is from xtal, if can generate 32k clock, but it is never used.
src1 is from gpio clock, It is never used. If necessary， the ao_slow_clk 
maybe described.

So why aoclk81 has 3 parents?
Jerome Brunet Sept. 14, 2018, 8:33 a.m. UTC | #5 
On Tue, 2018-08-28 at 11:35 +0800, Jian Hu wrote:
> Hi: Jerome
> 
> On 2018/8/27 21:07, Jerome Brunet wrote:
> > On Fri, 2018-08-24 at 21:34 +0800, Jian Hu wrote:
> > > > 
> > > 
> > > I am confued about aoclk81's parent clocks.
> > > 
> > > I can not get the example of axg audio clock driver, Could you provide
> > > the link? Had it merged into clk-meson.git?
> > 
> > Yes and mainline as well : drivers/clk/meson/axg-audio.c
> > 
> > Basically this driver is creating bypass input clocks (audio_pclk, mst_in[0-9],
> > etc...) .
> > 
> > This allows to collect input clocks from DT (like any consumer should) will
> > keeping constant in the controller clock tree.
> > 
> > > From what I've seen of your controller drivers, the EE controller should have
> > 
> > one input, the AO should have 3.
> > 
> > 
> > 
> > .
> > 
> 
> I still can not get the example meaning in axg audio driver.

here is one specific example:


https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/meson/axg-audio.c?h=v4.19-rc1#n785


> 
> In 26 page of A113D_Datasheet V0.7 20170725-Baylibre.pdf,We can see the 
> aoclk81 has two parents. clk81 and ao_slow_clk. I can not get 3 parents.
> 
>                           clk81|\
>                         -------| \        aoclk81
> src0 |\                       |  |-------------------
> -----| \     ao_slow_clk      |  |
>       |  |---------------------| /
> -----| /                      |/
>   src1|/
> 
> 
> src0 is from xtal, if can generate 32k clock, but it is never used.
> src1 is from gpio clock, It is never used. If necessary， the ao_slow_clk 
> maybe described.
> 
> So why aoclk81 has 3 parents?

I never said that. I said your clock controller driver should have 3 input
clocks. These clock are the one used by the controller but not produced by it:
for the AO controller xtal, ao_xtal, and ee controller clk81.
diff mbox series

Patch

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 2b1a562..d5c2dcd 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -9,5 +9,5 @@  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 axg-aoclk.o
 obj-$(CONFIG_COMMON_CLK_AXG_AUDIO)	+= axg-audio.o
-obj-$(CONFIG_COMMON_CLK_G12A)	 += g12a.o
+obj-$(CONFIG_COMMON_CLK_G12A)	 += g12a.o g12a-aoclk.c
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)	+= clk-regmap.o
diff --git a/drivers/clk/meson/g12a-aoclk.c b/drivers/clk/meson/g12a-aoclk.c
new file mode 100644
index 0000000..a5cd95c
--- /dev/null
+++ b/drivers/clk/meson/g12a-aoclk.c
@@ -0,0 +1,170 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson-G12A Clock Controller Driver
+ *
+ * Copyright (c) 2016 Baylibre SAS.
+ * Author: Michael Turquette <mturquette@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Jian Hu <jian.hu@amlogic.com>
+ */
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/mfd/syscon.h>
+#include <linux/init.h>
+#include "clkc.h"
+#include "g12a-aoclk.h"
+
+#define G12A_AO_GATE0(_name, _bit)					\
+static struct clk_regmap _name##_ao = {					\
+	.data = &(struct clk_regmap_gate_data) {			\
+		.offset = (AO_CLK_GATE0),			\
+		.bit_idx = (_bit),					\
+	},								\
+	.hw.init = &(struct clk_init_data) {				\
+		.name = #_name "_ao",					\
+		.ops = &clk_regmap_gate_ops,				\
+		.parent_names = (const char *[]){ "clk81" },		\
+		.num_parents = 1,					\
+	},								\
+}
+
+G12A_AO_GATE0(ahb_bus,		0);
+G12A_AO_GATE0(remote,		1);
+G12A_AO_GATE0(i2c_master,	2);
+G12A_AO_GATE0(i2c_slave,	3);
+G12A_AO_GATE0(uart1,		4);
+G12A_AO_GATE0(prod_i2c,		5);
+G12A_AO_GATE0(uart2,		6);
+G12A_AO_GATE0(ir_blaster,	7);
+G12A_AO_GATE0(saradc,		8);
+
+static struct clk_regmap ao_clk81 = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = AO_RTI_PWR_CNTL_REG0,
+		.mask = 0x1,
+		.shift = 8,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "ao_clk81",
+		.ops = &clk_regmap_mux_ro_ops,
+		.parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},
+		.num_parents = 2,
+	},
+};
+
+static struct clk_regmap g12a_saradc_mux = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = AO_SAR_CLK,
+		.mask = 0x3,
+		.shift = 9,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "g12a_saradc_mux",
+		.ops = &clk_regmap_mux_ops,
+		.parent_names = (const char *[]){ "xtal", "ao_clk81" },
+		.num_parents = 2,
+	},
+};
+
+static struct clk_regmap g12a_saradc_div = {
+	.data = &(struct clk_regmap_div_data) {
+		.offset = AO_SAR_CLK,
+		.shift = 0,
+		.width = 8,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "g12a_saradc_div",
+		.ops = &clk_regmap_divider_ops,
+		.parent_names = (const char *[]){ "g12a_saradc_mux" },
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap g12a_saradc_gate = {
+	.data = &(struct clk_regmap_gate_data) {
+		.offset = AO_SAR_CLK,
+		.bit_idx = 8,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "g12a_saradc_gate",
+		.ops = &clk_regmap_gate_ops,
+		.parent_names = (const char *[]){ "g12a_saradc_div" },
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static unsigned int g12a_aoclk_reset[] = {
+	[RESET_AO_REMOTE] =	16,
+	[RESET_AO_UART1] =	17,
+	[RESET_AO_I2C_MASTER] = 18,
+	[RESET_AO_I2C_SLAVE] =	19,
+	[RESET_AO_SARADC] =	20,
+	[RESET_AO_UART2] =	22,
+	[RESET_AO_IR_BLASTER] = 23,
+};
+
+static struct clk_regmap *g12a_aoclk_regmap[] = {
+	[CLKID_AO_AHB_BUS]	= &ahb_bus_ao,
+	[CLKID_AO_REMOTE]	= &remote_ao,
+	[CLKID_AO_I2C_MASTER]	= &i2c_master_ao,
+	[CLKID_AO_I2C_SLAVE]	= &i2c_slave_ao,
+	[CLKID_AO_UART1]	= &uart1_ao,
+	[CLKID_AO_PROD_I2C]	= &prod_i2c_ao,
+	[CLKID_AO_UART2]	= &uart2_ao,
+	[CLKID_AO_IR_BLASTER]	= &ir_blaster_ao,
+	[CLKID_AO_SAR_ADC]	= &saradc_ao,
+	[CLKID_AO_CLK81]	= &ao_clk81,
+	[CLKID_AO_SAR_ADC_SEL]	= &g12a_saradc_mux,
+	[CLKID_AO_SAR_ADC_DIV]	= &g12a_saradc_div,
+	[CLKID_AO_SAR_ADC_CLK]	= &g12a_saradc_gate,
+};
+
+static struct clk_hw_onecell_data g12a_aoclk_onecell_data = {
+	.hws = {
+		[CLKID_AO_AHB_BUS]	= &ahb_bus_ao.hw,
+		[CLKID_AO_REMOTE]	= &remote_ao.hw,
+		[CLKID_AO_I2C_MASTER]	= &i2c_master_ao.hw,
+		[CLKID_AO_I2C_SLAVE]	= &i2c_slave_ao.hw,
+		[CLKID_AO_UART1]	= &uart1_ao.hw,
+		[CLKID_AO_PROD_I2C]	= &prod_i2c_ao.hw,
+		[CLKID_AO_UART2]	= &uart2_ao.hw,
+		[CLKID_AO_IR_BLASTER]	= &ir_blaster_ao.hw,
+		[CLKID_AO_SAR_ADC]	= &saradc_ao.hw,
+		[CLKID_AO_CLK81]	= &ao_clk81.hw,
+		[CLKID_AO_SAR_ADC_SEL]	= &g12a_saradc_mux.hw,
+		[CLKID_AO_SAR_ADC_DIV]	= &g12a_saradc_div.hw,
+		[CLKID_AO_SAR_ADC_CLK]	= &g12a_saradc_gate.hw,
+	},
+	.num = NR_CLKS,
+};
+
+static struct meson_aoclk_data g12a_aoclkc_data = {
+	.reset_reg	= AO_RTI_GEN_CNTL_REG0,
+	.num_reset	= ARRAY_SIZE(g12a_aoclk_reset),
+	.reset		= g12a_aoclk_reset,
+	.num_clks	= ARRAY_SIZE(g12a_aoclk_regmap),
+	.clks		= g12a_aoclk_regmap,
+	.hw_data	= &g12a_aoclk_onecell_data,
+};
+
+static const struct of_device_id g12a_aoclkc_match_table[] = {
+	{
+		.compatible	= "amlogic,g12a-aoclkc",
+		.data		= &g12a_aoclkc_data,
+	},
+	{ }
+};
+
+static struct platform_driver g12a_aoclkc_driver = {
+	.probe		= meson_aoclkc_probe,
+	.driver		= {
+		.name	= "g12a-aoclkc",
+		.of_match_table = g12a_aoclkc_match_table,
+	},
+};
+
+builtin_platform_driver(g12a_aoclkc_driver);
diff --git a/drivers/clk/meson/g12a-aoclk.h b/drivers/clk/meson/g12a-aoclk.h
new file mode 100644
index 0000000..007183e
--- /dev/null
+++ b/drivers/clk/meson/g12a-aoclk.h
@@ -0,0 +1,36 @@ 
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2017 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Jian Hu <Jian.Hu@amlogic.com>
+ */
+
+#ifndef __G12A_AOCLKC_H
+#define __G12A_AOCLKC_H
+
+//#include "meson-aoclk.h"
+
+#define NR_CLKS				14
+
+/* AO Configuration Clock registers offsets
+ * Register offsets from the data sheet must be multiplied by 4.
+ */
+#define AO_RTI_PWR_CNTL_REG0		0x10
+#define AO_RTI_GEN_CNTL_REG0		0x40
+#define AO_OSCIN_CNTL			0x58
+#define AO_CRT_CLK_CNTL1		0x68
+#define AO_SAR_CLK			0x90
+#define AO_RTC_ALT_CLK_CNTL0		0x94
+#define AO_RTC_ALT_CLK_CNTL1		0x98
+
+/*
+ *AO CLK81 gate clocks
+ */
+#define AO_CLK_GATE0			0x4c
+
+#include <dt-bindings/clock/g12a-aoclkc.h>
+#include <dt-bindings/reset/g12a-aoclkc.h>
+
+#endif /* __G12A_AOCLKC_H */