Re: [PATCH v2 2/3] clk: canaan: Add clock driver for Canaan K230

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 08, 2025 at 07:53:08PM +0800, Xukai Wang wrote:
> This patch provides basic support for the K230 clock, which does not
> cover all clocks.
> 
> The clock tree of the K230 SoC consists of OSC24M, PLLs and sysclk.
> 
> Co-developed-by: Troy Mitchell <TroyMitchell988@xxxxxxxxx>
> Signed-off-by: Troy Mitchell <TroyMitchell988@xxxxxxxxx>
> Signed-off-by: Xukai Wang <kingxukai@xxxxxxxxxxxx>
> ---
>  drivers/clk/Kconfig    |    6 +
>  drivers/clk/Makefile   |    1 +
>  drivers/clk/clk-k230.c | 1424 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1431 insertions(+)
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 299bc678ed1b9fcd9110bb8c5937a1bd1ea60e23..f63355ab8adeeee90d29d1a975607f5dc0a58bd6 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -464,6 +464,12 @@ config COMMON_CLK_K210
>  	help
>  	  Support for the Canaan Kendryte K210 RISC-V SoC clocks.
>  
> +config COMMON_CLK_K230
> +	bool "Clock driver for the Canaan Kendryte K230 SoC"
> +	depends on OF && (ARCH_CANAAN || COMPILE_TEST)
> +        help
> +          Support for the Canaan Kendryte K230 RISC-V SoC clocks.
> +
>  config COMMON_CLK_SP7021
>  	tristate "Clock driver for Sunplus SP7021 SoC"
>  	depends on SOC_SP7021 || COMPILE_TEST
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index fb8878a5d7d93da6bec487460cdf63f1f764a431..5df50b1e14c701ed38397bfb257db26e8dd278b8 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_MACH_ASPEED_G6)		+= clk-ast2600.o
>  obj-$(CONFIG_ARCH_HIGHBANK)		+= clk-highbank.o
>  obj-$(CONFIG_CLK_HSDK)			+= clk-hsdk-pll.o
>  obj-$(CONFIG_COMMON_CLK_K210)		+= clk-k210.o
> +obj-$(CONFIG_COMMON_CLK_K230)		+= clk-k230.o
>  obj-$(CONFIG_LMK04832)			+= clk-lmk04832.o
>  obj-$(CONFIG_COMMON_CLK_LAN966X)	+= clk-lan966x.o
>  obj-$(CONFIG_COMMON_CLK_LOCHNAGAR)	+= clk-lochnagar.o
> diff --git a/drivers/clk/clk-k230.c b/drivers/clk/clk-k230.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..412944699a4e57502df4155fd0811d41444d15bc
> --- /dev/null
> +++ b/drivers/clk/clk-k230.c
> @@ -0,0 +1,1424 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Kendryte Canaan K230 Clock Drivers
> + *
> + * Author: Xukai Wang <kingxukai@xxxxxxxxxxxx>
> + * Author: Troy Mitchell <troymitchell988@xxxxxxxxx>
> + */
> +
> +#include <dt-bindings/clock/k230-clk.h>
> +#include <linux/bitfield.h>

Where is it used?

> +#include <linux/clk.h>

Are you sure you use it?

> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>

Not used?

> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>

Where is it used?

> +#include <linux/of_clk.h>
> +#include <linux/of_device.h>

And this?

> +#include <linux/of_platform.h>

And this?

> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +/* PLL control register bits. */
> +#define K230_PLL_BYPASS_ENABLE		BIT(19)
> +#define K230_PLL_GATE_ENABLE		BIT(2)
> +#define K230_PLL_GATE_WRITE_ENABLE	BIT(18)
> +#define K230_PLL_OD_SHIFT		24
> +#define K230_PLL_OD_MASK		0xF
> +#define K230_PLL_R_SHIFT		16
> +#define K230_PLL_R_MASK			0x3F
> +#define K230_PLL_F_SHIFT		0
> +#define K230_PLL_F_MASK			0x1FFFF
> +#define K230_PLL0_OFFSET_BASE		0x00
> +#define K230_PLL1_OFFSET_BASE		0x10
> +#define K230_PLL2_OFFSET_BASE		0x20
> +#define K230_PLL3_OFFSET_BASE		0x30
> +#define K230_PLL_DIV_REG_OFFSET		0x00
> +#define K230_PLL_BYPASS_REG_OFFSET	0x04
> +#define K230_PLL_GATE_REG_OFFSET	0x08
> +#define K230_PLL_LOCK_REG_OFFSET	0x0C
> +
> +/* PLL lock register bits.  */
> +#define K230_PLL_STATUS_MASK            BIT(0)
> +
> +/* K230 CLK registers offset */
> +#define K230_CLK_AUDIO_CLKDIV_OFFSET 0x34
> +#define K230_CLK_PDM_CLKDIV_OFFSET 0x40
> +#define K230_CLK_CODEC_ADC_MCLKDIV_OFFSET 0x38
> +#define K230_CLK_CODEC_DAC_MCLKDIV_OFFSET 0x3c
> +
> +/* K230 CLK MACROS */
> +#define K230_GATE_FORMAT(_reg, _bit, _reverse, _have_gate)                      \
> +	.gate_reg_off = (_reg),                                                 \
> +	.gate_bit_enable = (_bit),                                              \
> +	.gate_bit_reverse = (_reverse),                                         \
> +	.have_gate = (_have_gate)
> +
> +#define K230_RATE_FORMAT(_mul_min, _mul_max, _mul_shift, _mul_mask,             \
> +			_div_min, _div_max, _div_shift, _div_mask,		\
> +			_reg, _bit, _method, _reg_c, _bit_c,			\
> +			_mul_min_c, _mul_max_c, _mul_shift_c, _mul_mask_c,      \
> +			_have_rate, _have_rate_c)                               \
> +	.rate_mul_min = (_mul_min),                                             \
> +	.rate_mul_max = (_mul_max),                                             \
> +	.rate_mul_shift = (_mul_shift),                                         \
> +	.rate_mul_mask = (_mul_mask),                                           \
> +	.rate_mul_min_c = (_mul_min_c),                                         \
> +	.rate_mul_max_c = (_mul_max_c),                                         \
> +	.rate_mul_shift_c = (_mul_shift_c),                                     \
> +	.rate_mul_mask_c = (_mul_mask_c),                                       \
> +	.rate_div_min = (_div_min),                                             \
> +	.rate_div_max = (_div_max),                                             \
> +	.rate_div_shift = (_div_shift),                                         \
> +	.rate_div_mask = (_div_mask),                                           \
> +	.rate_reg_off = (_reg),                                                 \
> +	.rate_reg_off_c = (_reg_c),                                             \
> +	.rate_write_enable_bit = (_bit),                                        \
> +	.rate_write_enable_bit_c = (_bit_c),                                    \
> +	.method = (_method),                                                    \
> +	.have_rate = (_have_rate),                                              \
> +	.have_rate_c = (_have_rate_c)
> +
> +#define K230_MUX_FORMAT(_reg, _shift, _mask, _have_mux)                         \
> +	.mux_reg_off = (_reg),                                                  \
> +	.mux_reg_shift = (_shift),                                              \
> +	.mux_reg_mask = (_mask),                                                \
> +	.have_mux = (_have_mux)
> +
> +#define K230_GATE_FORMAT_ZERO K230_GATE_FORMAT(0, 0, 0, false)
> +#define K230_RATE_FORMAT_ZERO K230_RATE_FORMAT(0, 0, 0, 0, 0, 0,                \
> +						0, 0, 0, 0, 0, 0,		\
> +						0, 0, 0, 0, 0, false, false)
> +#define K230_MUX_FORMAT_ZERO K230_MUX_FORMAT(0, 0, 0, false)
> +
> +struct k230_sysclk;
> +
> +/* K230 PLLs. */
> +enum k230_pll_id {
> +	K230_PLL0, K230_PLL1, K230_PLL2, K230_PLL3, K230_PLL_NUM
> +};
> +
> +struct k230_pll {
> +	enum k230_pll_id id;
> +	struct k230_sysclk *ksc;
> +	void __iomem *div, *bypass, *gate, *lock;
> +	struct clk_hw hw;
> +};
> +
> +#define to_k230_pll(_hw)	container_of(_hw, struct k230_pll, hw)
> +
> +struct k230_pll_cfg {
> +	u32 reg;
> +	enum k230_pll_id pll_id;
> +	const char *name;
> +};
> +
> +static struct k230_pll_cfg k230_pll_cfgs[] = {

Why this cannot be const?

> +	[K230_PLL0] = {
> +		.reg = K230_PLL0_OFFSET_BASE,
> +		.pll_id = K230_PLL0,
> +		.name = "pll0",
> +	},
> +	[K230_PLL1] = {
> +		.reg = K230_PLL1_OFFSET_BASE,
> +		.pll_id = K230_PLL1,
> +		.name = "pll1",
> +	},
> +	[K230_PLL2] = {
> +		.reg = K230_PLL2_OFFSET_BASE,
> +		.pll_id = K230_PLL2,
> +		.name = "pll2",
> +	},
> +	[K230_PLL3] = {
> +		.reg = K230_PLL3_OFFSET_BASE,
> +		.pll_id = K230_PLL3,
> +		.name = "pll3",
> +	},
> +};
> +
> +struct k230_pll_div {
> +	struct k230_sysclk *ksc;
> +	struct clk_hw *hw;
> +};
> +
> +struct k230_pll_div_cfg {
> +	const char *parent_name, *name;
> +	int div;
> +};
> +
> +/* K230 PLL_DIVS */
> +enum k230_pll_div_id {
> +	K230_PLL0_DIV2,
> +	K230_PLL0_DIV3,
> +	K230_PLL0_DIV4,
> +	K230_PLL1_DIV2,
> +	K230_PLL1_DIV3,
> +	K230_PLL1_DIV4,
> +	K230_PLL2_DIV2,
> +	K230_PLL2_DIV3,
> +	K230_PLL2_DIV4,
> +	K230_PLL3_DIV2,
> +	K230_PLL3_DIV3,
> +	K230_PLL3_DIV4,
> +	K230_PLL_DIV_NUM
> +};
> +
> +static struct k230_pll_div_cfg k230_pll_div_cfgs[] = {

And this?

> +	[K230_PLL0_DIV2] = { "pll0", "pll0_div2", 2},
> +	[K230_PLL0_DIV3] = { "pll0", "pll0_div3", 3},
> +	[K230_PLL0_DIV4] = { "pll0", "pll0_div4", 4},
> +	[K230_PLL1_DIV2] = { "pll1", "pll1_div2", 2},
> +	[K230_PLL1_DIV3] = { "pll1", "pll1_div3", 3},
> +	[K230_PLL1_DIV4] = { "pll1", "pll1_div4", 4},
> +	[K230_PLL2_DIV2] = { "pll2", "pll2_div2", 2},
> +	[K230_PLL2_DIV3] = { "pll2", "pll2_div3", 3},
> +	[K230_PLL2_DIV4] = { "pll2", "pll2_div4", 4},
> +	[K230_PLL3_DIV2] = { "pll3", "pll3_div2", 2},
> +	[K230_PLL3_DIV3] = { "pll3", "pll3_div3", 3},
> +	[K230_PLL3_DIV4] = { "pll3", "pll3_div4", 4},
> +};
> +
> +/* K230 CLKS. */
> +struct k230_clk {
> +	int id;
> +	struct k230_sysclk *ksc;
> +	struct clk_hw hw;
> +};
> +
> +#define to_k230_clk(_hw)	container_of(_hw, struct k230_clk, hw)
> +
> +enum k230_clk_div_type {
> +	K230_MUL,
> +	K230_DIV,
> +	K230_MUL_DIV,
> +};
> +
> +enum k230_clk_parent_type {
> +	K230_OSC24M,
> +	K230_PLL,
> +	K230_PLL_DIV,
> +	K230_CLK_COMPOSITE,
> +};
> +
> +#define K230_CLK_MAX_PARENT_NUM 6
> +
> +struct k230_clk_parent {
> +	enum k230_clk_parent_type type;
> +	union {
> +		enum k230_pll_div_id pll_div_id;
> +		enum k230_pll_id pll_id;
> +		int clk_id;
> +	};
> +};
> +
> +struct k230_clk_cfg {
> +	/* attr */
> +	const char *name;
> +	/* 0-read & write; 1-read only */
> +	bool read_only;
> +	int num_parent;
> +	struct k230_clk_parent parent[K230_CLK_MAX_PARENT_NUM];
> +	bool status;
> +	int flags;
> +
> +	/* rate reg */
> +	u32 rate_reg_off;
> +	u32 rate_reg_off_c;
> +	void __iomem *rate_reg;
> +	void __iomem *rate_reg_c;
> +	/* rate info*/
> +	u32 rate_write_enable_bit;
> +	u32 rate_write_enable_bit_c;
> +	enum k230_clk_div_type method;
> +	bool have_rate;
> +	bool have_rate_c;
> +	/* rate mul */
> +	u32 rate_mul_min;
> +	u32 rate_mul_max;
> +	u32 rate_mul_shift;
> +	u32 rate_mul_mask;
> +	/* rate mul-changable */
> +	u32 rate_mul_min_c;
> +	u32 rate_mul_max_c;
> +	u32 rate_mul_shift_c;
> +	u32 rate_mul_mask_c;
> +	/* rate div */
> +	u32 rate_div_min;
> +	u32 rate_div_max;
> +	u32 rate_div_shift;
> +	u32 rate_div_mask;
> +
> +	/* gate reg */
> +	u32 gate_reg_off;
> +	void __iomem *gate_reg;
> +	/* gate info*/
> +	bool have_gate;
> +	u32 gate_bit_enable;
> +	u32 gate_bit_reverse;
> +
> +	/* mux reg */
> +	u32 mux_reg_off;
> +	void __iomem *mux_reg;
> +	/* mux info */
> +	bool have_mux;
> +	u32 mux_reg_shift;
> +	u32 mux_reg_mask;
> +};
> +
> +static struct k230_clk_cfg k230_clk_cfgs[] = {

And this...


BTW, you have mixed order of declarations and definitions of static
structures/data. Usually first come declarations and then definitions.
Not inter-mixed.

> +	[K230_CPU0_SRC] = {
> +		.name = "cpu0_src",
> +		.read_only = false,
> +		.flags = 0,
> +		.status = true,
> +		.num_parent = 1,
> +		.parent[0] = {
> +			.type = K230_PLL_DIV,
> +			.pll_div_id = K230_PLL0_DIV2,
> +		},
> +		K230_RATE_FORMAT(1, 16, 0, 0,
> +				 16, 16, 1, 0xF,
> +				 0x0, 31, K230_MUL, 0, 0,
> +				 0, 0, 0, 0,
> +				 true, false),
> +		K230_GATE_FORMAT(0, 0, 0, true),
> +		K230_MUX_FORMAT_ZERO,
> +	},
> +	[K230_CPU0_ACLK] = {
> +		.name = "cpu0_aclk",
> +		.read_only = false,
> +		.flags = 0,
> +		.status = true,
> +		.num_parent = 1,
> +		.parent[0] = {
> +			.type = K230_CLK_COMPOSITE,
> +			.clk_id = K230_CPU0_SRC,
> +		},
> +		K230_RATE_FORMAT(1, 1, 0, 0,
> +				 1, 8, 7, 0x7,
> +				 0x0, 31, K230_MUL, 0, 0,
> +				 0, 0, 0, 0,
> +				 true, false),
> +		K230_GATE_FORMAT_ZERO,
> +		K230_MUX_FORMAT_ZERO,
> +	},
> +	[K230_CPU0_PLIC] = {
> +		.name = "cpu0_plic",
> +		.read_only = false,
> +		.flags = 0,
> +		.status = true,
> +		.num_parent = 1,
> +		.parent[0] = {
> +			.type = K230_CLK_COMPOSITE,
> +			.clk_id = K230_CPU0_SRC,
> +		},
> +		K230_RATE_FORMAT(1, 1, 0, 0,
> +				 1, 8, 10, 0x7,
> +				 0x0, 31, K230_DIV, 0, 0,
> +				 0, 0, 0, 0,
> +				 true, false),
> +		K230_GATE_FORMAT(0, 9, 0, true),
> +		K230_MUX_FORMAT_ZERO,
> +	},
> +	[K230_CPU0_NOC_DDRCP4] = {
> +		.name = "cpu0_noc_ddrcp4",
> +		.read_only = false,
> +		.flags = 0,
> +		.status = true,
> +		.num_parent = 1,
> +		.parent[0] = {
> +			.type = K230_CLK_COMPOSITE,
> +			.clk_id = K230_CPU0_SRC,
> +		},
> +		K230_RATE_FORMAT_ZERO,
> +		K230_GATE_FORMAT(0x60, 7, 0, true),
> +		K230_MUX_FORMAT_ZERO,
> +	},
> +	[K230_CPU0_PCLK] = {
> +		.name = "cpu0_pclk",
> +		.read_only = false,
> +		.flags = 0,
> +		.status = true,
> +		.num_parent = 1,
> +		.parent[0] = {
> +			.type = K230_PLL_DIV,
> +			.pll_div_id = K230_PLL0_DIV4,
> +		},
> +		K230_RATE_FORMAT(1, 1, 0, 0,
> +				 1, 8, 15, 0x7,
> +				 0x0, 31, K230_DIV, 0, 0,
> +				 0, 0, 0, 0,
> +				 true, false),
> +		K230_GATE_FORMAT(0, 13, 0, true),
> +		K230_MUX_FORMAT_ZERO,
> +	},
> +	[K230_PMU_PCLK] = {
> +		.name = "pmu_pclk",
> +		.read_only = false,
> +		.flags = 0,
> +		.status = true,
> +		.num_parent = 1,
> +		.parent[0] = {
> +			.type = K230_OSC24M,
> +		},
> +		K230_RATE_FORMAT_ZERO,
> +		K230_GATE_FORMAT(0x10, 0, 0, true),
> +		K230_MUX_FORMAT_ZERO,
> +	},
> +	[K230_HS_OSPI_SRC] = {
> +		.name = "hs_ospi_src",
> +		.read_only = false,
> +		.flags = 0,
> +		.status = true,
> +		.num_parent = 2,
> +		.parent[0] = {
> +			.type = K230_PLL_DIV,
> +			.pll_div_id = K230_PLL0_DIV2,
> +		},
> +		.parent[1] = {
> +			.type = K230_PLL_DIV,
> +			.pll_div_id = K230_PLL2_DIV4,
> +		},
> +		K230_RATE_FORMAT_ZERO,
> +		K230_GATE_FORMAT(0x18, 24, 0, true),
> +		K230_MUX_FORMAT(0x20, 18, 0x1, true),
> +	},
> +};
> +
> +#define K230_NUM_CLKS ARRAY_SIZE(k230_clk_cfgs)
> +
> +struct k230_sysclk {
> +	struct platform_device *pdev;
> +	void __iomem			*pll_regs, *regs;
> +	spinlock_t			pll_lock, clk_lock;
> +	struct k230_pll			plls[K230_PLL_NUM];
> +	struct k230_clk			clks[K230_NUM_CLKS];
> +	struct k230_pll_div		dclks[K230_PLL_DIV_NUM];
> +} clksrc;

No, drop clksrc. No singletons. Especially no globla singletons.


...

> +static int k230_clk_find_approximate(struct k230_clk *clk,
> +				     u32 mul_min,
> +				     u32 mul_max,
> +				     u32 div_min,
> +				     u32 div_max,
> +				     enum k230_clk_div_type method,
> +				     unsigned long rate,
> +				     unsigned long parent_rate,
> +				     u32 *div,
> +				     u32 *mul)
> +{
> +	long abs_min;
> +	long abs_current;
> +	long perfect_divide;
> +	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +
> +	u32 codec_clk[9] = {


You do not like const atribute, do you?

> +		2048000,
> +		3072000,
> +		4096000,
> +		6144000,
> +		8192000,
> +		11289600,
> +		12288000,
> +		24576000,
> +		49152000
> +	};
> +
> +	u32 codec_div[9][2] = {
> +		{3125, 16},
> +		{3125, 24},
> +		{3125, 32},
> +		{3125, 48},
> +		{3125, 64},
> +		{15625, 441},
> +		{3125, 96},
> +		{3125, 192},
> +		{3125, 384}
> +	};
> +
> +	u32 pdm_clk[20] = {
> +		128000,
> +		192000,
> +		256000,
> +		384000,
> +		512000,
> +		768000,
> +		1024000,
> +		1411200,
> +		1536000,
> +		2048000,
> +		2822400,
> +		3072000,
> +		4096000,
> +		5644800,
> +		6144000,
> +		8192000,
> +		11289600,
> +		12288000,
> +		24576000,
> +		49152000
> +	};
> +
> +	u32 pdm_div[20][2] = {
> +		{3125, 1},
> +		{6250, 3},
> +		{3125, 2},
> +		{3125, 3},
> +		{3125, 4},
> +		{3125, 6},
> +		{3125, 8},
> +		{125000, 441},
> +		{3125, 12},
> +		{3125, 16},
> +		{62500, 441},
> +		{3125, 24},
> +		{3125, 32},
> +		{31250, 441},
> +		{3125, 48},
> +		{3125, 64},
> +		{15625, 441},
> +		{3125, 96},
> +		{3125, 192},
> +		{3125, 384}
> +	};
> +

...

> +
> +static int k230_clk_init_plls(struct platform_device *pdev)
> +{
> +	struct k230_sysclk *ksc = &clksrc;
> +	int ret = 0;
> +
> +	spin_lock_init(&ksc->pll_lock);
> +
> +	ksc->pll_regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (!ksc->pll_regs) {
> +		dev_err(&pdev->dev, "failed to map registers\n");
> +		ret = PTR_ERR(ksc->pll_regs);

What is the point of ret here?

> +		return ret;
> +	}
> +
> +	ret = k230_register_plls(pdev, ksc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "register plls failed %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = k230_register_pll_divs(pdev, ksc);
> +	if (ret)
> +		dev_err(&pdev->dev, "register pll_divs failed %d\n", ret);
> +
> +	ret = devm_of_clk_add_hw_provider(&pdev->dev, k230_clk_hw_pll_divs_onecell_get, ksc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "add plls provider failed %d\n", ret);
> +		return ret;
> +	}
> +
> +	for (int i = 0; i < K230_PLL_DIV_NUM; i++) {
> +		ret = devm_clk_hw_register_clkdev(&pdev->dev, ksc->dclks[i].hw,
> +						  k230_pll_div_cfgs[i].name, NULL);
> +		if (ret) {
> +			dev_err(&pdev->dev, "clock_lookup create failed %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int k230_clk_init_sysclk(struct platform_device *pdev)
> +{
> +	struct k230_sysclk *ksc = &clksrc;
> +	int ret = 0;
> +
> +	spin_lock_init(&ksc->clk_lock);
> +
> +	ksc->regs = devm_platform_ioremap_resource(pdev, 1);
> +	if (!ksc->regs) {
> +		dev_err(&pdev->dev, "failed to map registers\n");
> +		ret = PTR_ERR(ksc->regs);
> +		return ret;
> +	}
> +
> +	ret = k230_register_clks(pdev, ksc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "register clock provider failed %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = devm_of_clk_add_hw_provider(&pdev->dev, k230_clk_hw_onecell_get, ksc);
> +	if (ret)
> +		dev_err(&pdev->dev, "add clock provider failed %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int k230_clk_probe(struct platform_device *pdev)
> +{
> +	struct k230_sysclk *ksc = &clksrc;
> +	int ret = 0;
> +
> +	ksc->pdev = pdev;
> +
> +	if (!pdev) {
> +		dev_err(&pdev->dev, "platform device pointer is NULL\n");

So how did you probe? Is this even remotely possible?

> +		ret = -EINVAL;
> +		return ret;
> +	}
> +
> +	ret = k230_clk_init_plls(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "init plls failed with %d\n", ret);

syntax is always: return dev_err_probe

> +		return ret;
> +	}
> +
> +	ret = k230_clk_init_sysclk(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "init clks failed with %d\n", ret);

syntax is always: return dev_err_probe

fix it in all other applicable places as well


> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id k230_clk_ids[] = {
> +	{ .compatible = "canaan,k230-clk" },
> +	{ /* Sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, k230_clk_ids);
> +
> +static struct platform_driver k230_clk_driver = {
> +	.driver = {
> +		.name  = "k230_clock_controller",
> +		.of_match_table = k230_clk_ids,
> +	},
> +	.probe = k230_clk_probe,
> +};
> +builtin_platform_driver(k230_clk_driver);
> 
> -- 
> 2.34.1
> 




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux