[RFC PATCH v1 3/9] clk: renesas: add R906G032 driver

Sean Anderson seanga2 at gmail.com
Sat Aug 13 07:30:19 CEST 2022


On 8/9/22 8:59 AM, Ralph Siemsen wrote:
> Clock driver for the Renesas RZ/N1 SoC family. This is based
> on the Linux kernel drivers/clk/renesas/r9a06g032-clocks.c.
> 
> Notable difference: this version avoids allocating a 'struct clk'
> for each clock source, as this is problematic before relocation.
> Instead, it uses the same approach as existing Renesas RCAR2/3
> clock drivers, using a temporary structure filled on-the-fly.
> 
> Signed-off-by: Ralph Siemsen <ralph.siemsen at linaro.org>
> ---
> - TODO: add support for div_table
> 
>   drivers/clk/renesas/Kconfig            |   6 +
>   drivers/clk/renesas/Makefile           |   1 +
>   drivers/clk/renesas/r9a06g032-clocks.c | 734 +++++++++++++++++++++++++
>   3 files changed, 741 insertions(+)
>   create mode 100644 drivers/clk/renesas/r9a06g032-clocks.c
> 
> diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
> index c53ff3ce01..e2f72fc04f 100644
> --- a/drivers/clk/renesas/Kconfig
> +++ b/drivers/clk/renesas/Kconfig
> @@ -120,3 +120,9 @@ config CLK_R8A779A0
>   	depends on CLK_RCAR_GEN3
>   	help
>   	  Enable this to support the clocks on Renesas R8A779A0 SoC.
> +
> +config CLK_R9A06G032
> +	bool "Renesas R9A06G032 clock driver"
> +	depends on CLK_RENESAS
> +	help
> +	  Enable this to support the clocks on Renesas R9A06G032 SoC.

nit: on the

...

> diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
> index 2cd2c69f68..9981f1a0bc 100644
> --- a/drivers/clk/renesas/Makefile
> +++ b/drivers/clk/renesas/Makefile
> @@ -17,3 +17,4 @@ obj-$(CONFIG_CLK_R8A77980) += r8a77980-cpg-mssr.o
>   obj-$(CONFIG_CLK_R8A77990) += r8a77990-cpg-mssr.o
>   obj-$(CONFIG_CLK_R8A77995) += r8a77995-cpg-mssr.o
>   obj-$(CONFIG_CLK_R8A779A0) += r8a779a0-cpg-mssr.o
> +obj-$(CONFIG_CLK_R9A06G032) += r9a06g032-clocks.o
> diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
> new file mode 100644
> index 0000000000..9c8f51eb96
> --- /dev/null
> +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> @@ -0,0 +1,734 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * R9A06G032 clock driver
> + *
> + * Copyright (C) 2018 Renesas Electronics Europe Limited
> + *
> + * Michel Pollet <michel.pollet at bp.renesas.com>, <buserror at gmail.com>
> + */
> +
> +#include <common.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <linux/bitops.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <asm/io.h>
> +
> +#include <dt-bindings/clock/r9a06g032-sysctrl.h>
> +
> +struct r9a06g032_gate {

Can you add some documentation for each of the fields? Same for r9a06g032_clkdesc.

https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html

> +	u16 gate, reset, ready, midle,
> +		scon, mirack, mistat;

What are the scon/mirack/mistat fields for? You define them for a lot
of clocks, but I don't see them used in the driver.

> +};
> +
> +/* This is used to describe a clock for instantiation */
> +struct r9a06g032_clkdesc {
> +	const char *name;
> +	uint32_t managed: 1;
> +	uint32_t type: 3;

I wonder if we could define the enum here?

> +	uint32_t index: 8;
> +	uint32_t source : 8; /* source index + 1 (0 == none) */
> +	/* these are used to populate the bitsel struct */
> +	union {
> +		struct r9a06g032_gate gate;
> +		/* for dividers */
> +		struct {
> +			unsigned int div_min : 10, div_max : 10, reg: 10;
> +			u16 div_table[4];
> +		};
> +		/* For fixed-factor ones */
> +		struct {
> +			u16 div, mul;
> +		};
> +		/* for dual gate */
> +		struct {
> +			uint16_t group : 1;
> +			u16 sel, g1, r1, g2, r2;
> +		} dual;
> +	};
> +};
> +
> +#define I_GATE(_clk, _rst, _rdy, _midle, _scon, _mirack, _mistat) \
> +	{ .gate = _clk, .reset = _rst, \

If these fields have bitfield inside them, then those bitfields should
be assigned/constructed separately. That is, if .reset is actually a combined
offset/bit, then you need to expose those in the macro. Since you have a lot of these, you might want to do something like

#define BIT_OFFSET	GENMASK(15, 5)
#define BIT_SHIFT	GENMASK(4, 0)

#define PACK_BIT(offset, shift)		(FIELD_PREP(BIT_OFFSET, offset) | FIELD_PREP(BIT_SHIFT, shift))

> +		.ready = _rdy, .midle = _midle, \
> +		.scon = _scon, .mirack = _mirack, .mistat = _mistat }

Please put each assignment on a separate line

> +#define D_GATE(_idx, _n, _src, ...) \
> +	{ .type = K_GATE, .index = R9A06G032_##_idx, \
> +		.source = 1 + R9A06G032_##_src, .name = _n, \
> +		.gate = I_GATE(__VA_ARGS__) }
> +#define D_MODULE(_idx, _n, _src, ...) \
> +	{ .type = K_GATE, .index = R9A06G032_##_idx, \
> +		.source = 1 + R9A06G032_##_src, .name = _n, \
> +		.managed = 1, .gate = I_GATE(__VA_ARGS__) }
> +#define D_ROOT(_idx, _n, _mul, _div) \
> +	{ .type = K_FFC, .index = R9A06G032_##_idx, .name = _n, \
> +		.div = _div, .mul = _mul }
> +#define D_FFC(_idx, _n, _src, _div) \
> +	{ .type = K_FFC, .index = R9A06G032_##_idx, \
> +		.source = 1 + R9A06G032_##_src, .name = _n, \
> +		.div = _div, .mul = 1}
> +#define D_DIV(_idx, _n, _src, _reg, _min, _max, ...) \
> +	{ .type = K_DIV, .index = R9A06G032_##_idx, \
> +		.source = 1 + R9A06G032_##_src, .name = _n, \
> +		.reg = _reg, .div_min = _min, .div_max = _max, \
> +		.div_table = { __VA_ARGS__ } }
> +#define D_UGATE(_idx, _n, _src, _g, _g1, _r1, _g2, _r2) \
> +	{ .type = K_DUALGATE, .index = R9A06G032_##_idx, \
> +		.source = 1 + R9A06G032_##_src, .name = _n, \
> +		.dual = { .group = _g, \
> +			.g1 = _g1, .r1 = _r1, .g2 = _g2, .r2 = _r2 }, }
> +
> +enum { K_GATE = 0, K_FFC, K_DIV, K_BITSEL, K_DUALGATE };

Please put each member on a separate line.

> +
> +/* Internal clock IDs */
> +#define R9A06G032_CLKOUT		0
> +#define R9A06G032_CLKOUT_D10		2
> +#define R9A06G032_CLKOUT_D16		3
> +#define R9A06G032_CLKOUT_D160		4
> +#define R9A06G032_CLKOUT_D1OR2		5
> +#define R9A06G032_CLKOUT_D20		6
> +#define R9A06G032_CLKOUT_D40		7
> +#define R9A06G032_CLKOUT_D5		8
> +#define R9A06G032_CLKOUT_D8		9
> +#define R9A06G032_DIV_ADC		10
> +#define R9A06G032_DIV_I2C		11
> +#define R9A06G032_DIV_NAND		12
> +#define R9A06G032_DIV_P1_PG		13
> +#define R9A06G032_DIV_P2_PG		14
> +#define R9A06G032_DIV_P3_PG		15
> +#define R9A06G032_DIV_P4_PG		16
> +#define R9A06G032_DIV_P5_PG		17
> +#define R9A06G032_DIV_P6_PG		18
> +#define R9A06G032_DIV_QSPI0		19
> +#define R9A06G032_DIV_QSPI1		20
> +#define R9A06G032_DIV_REF_SYNC		21
> +#define R9A06G032_DIV_SDIO0		22
> +#define R9A06G032_DIV_SDIO1		23
> +#define R9A06G032_DIV_SWITCH		24
> +#define R9A06G032_DIV_UART		25
> +#define R9A06G032_DIV_MOTOR		64
> +#define R9A06G032_CLK_DDRPHY_PLLCLK_D4	78
> +#define R9A06G032_CLK_ECAT100_D4	79
> +#define R9A06G032_CLK_HSR100_D2		80
> +#define R9A06G032_CLK_REF_SYNC_D4	81
> +#define R9A06G032_CLK_REF_SYNC_D8	82
> +#define R9A06G032_CLK_SERCOS100_D2	83
> +#define R9A06G032_DIV_CA7		84
> +
> +#define R9A06G032_UART_GROUP_012	154
> +#define R9A06G032_UART_GROUP_34567	155

Can you put these in your dt-bindings header? I think that would make it
much clearer why there are gaps, and would avoid someone accidentally
duplicating a clock id (although I suppose your array initializer below
might complain?)

> +#define R9A06G032_CLOCK_COUNT		(R9A06G032_UART_GROUP_34567 + 1)
> +
> +static const struct r9a06g032_clkdesc r9a06g032_clocks[] = {
> +	D_ROOT(CLKOUT, "clkout", 25, 1),
> +	D_ROOT(CLK_PLL_USB, "clk_pll_usb", 12, 10),
> +	D_FFC(CLKOUT_D10, "clkout_d10", CLKOUT, 10),
> +	D_FFC(CLKOUT_D16, "clkout_d16", CLKOUT, 16),
> +	D_FFC(CLKOUT_D160, "clkout_d160", CLKOUT, 160),
> +	D_DIV(CLKOUT_D1OR2, "clkout_d1or2", CLKOUT, 0, 1, 2),
> +	D_FFC(CLKOUT_D20, "clkout_d20", CLKOUT, 20),
> +	D_FFC(CLKOUT_D40, "clkout_d40", CLKOUT, 40),
> +	D_FFC(CLKOUT_D5, "clkout_d5", CLKOUT, 5),
> +	D_FFC(CLKOUT_D8, "clkout_d8", CLKOUT, 8),
> +	D_DIV(DIV_ADC, "div_adc", CLKOUT, 77, 50, 250),
> +	D_DIV(DIV_I2C, "div_i2c", CLKOUT, 78, 12, 16),
> +	D_DIV(DIV_NAND, "div_nand", CLKOUT, 82, 12, 32),
> +	D_DIV(DIV_P1_PG, "div_p1_pg", CLKOUT, 68, 12, 200),
> +	D_DIV(DIV_P2_PG, "div_p2_pg", CLKOUT, 62, 12, 128),
> +	D_DIV(DIV_P3_PG, "div_p3_pg", CLKOUT, 64, 8, 128),
> +	D_DIV(DIV_P4_PG, "div_p4_pg", CLKOUT, 66, 8, 128),
> +	D_DIV(DIV_P5_PG, "div_p5_pg", CLKOUT, 71, 10, 40),
> +	D_DIV(DIV_P6_PG, "div_p6_pg", CLKOUT, 18, 12, 64),
> +	D_DIV(DIV_QSPI0, "div_qspi0", CLKOUT, 73, 3, 7),
> +	D_DIV(DIV_QSPI1, "div_qspi1", CLKOUT, 25, 3, 7),
> +	D_DIV(DIV_REF_SYNC, "div_ref_sync", CLKOUT, 56, 2, 16, 2, 4, 8, 16),
> +	D_DIV(DIV_SDIO0, "div_sdio0", CLKOUT, 74, 20, 128),
> +	D_DIV(DIV_SDIO1, "div_sdio1", CLKOUT, 75, 20, 128),
> +	D_DIV(DIV_SWITCH, "div_switch", CLKOUT, 37, 5, 40),
> +	D_DIV(DIV_UART, "div_uart", CLKOUT, 79, 12, 128),
> +	D_GATE(CLK_25_PG4, "clk_25_pg4", CLKOUT_D40, 0x749, 0x74a, 0x74b, 0, 0xae3, 0, 0),
> +	D_GATE(CLK_25_PG5, "clk_25_pg5", CLKOUT_D40, 0x74c, 0x74d, 0x74e, 0, 0xae4, 0, 0),
> +	D_GATE(CLK_25_PG6, "clk_25_pg6", CLKOUT_D40, 0x74f, 0x750, 0x751, 0, 0xae5, 0, 0),
> +	D_GATE(CLK_25_PG7, "clk_25_pg7", CLKOUT_D40, 0x752, 0x753, 0x754, 0, 0xae6, 0, 0),
> +	D_GATE(CLK_25_PG8, "clk_25_pg8", CLKOUT_D40, 0x755, 0x756, 0x757, 0, 0xae7, 0, 0),
> +	D_GATE(CLK_ADC, "clk_adc", DIV_ADC, 0x1ea, 0x1eb, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_ECAT100, "clk_ecat100", CLKOUT_D10, 0x405, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_HSR100, "clk_hsr100", CLKOUT_D10, 0x483, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_I2C0, "clk_i2c0", DIV_I2C, 0x1e6, 0x1e7, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_I2C1, "clk_i2c1", DIV_I2C, 0x1e8, 0x1e9, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_MII_REF, "clk_mii_ref", CLKOUT_D40, 0x342, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_NAND, "clk_nand", DIV_NAND, 0x284, 0x285, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_NOUSBP2_PG6, "clk_nousbp2_pg6", DIV_P2_PG, 0x774, 0x775, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_P1_PG2, "clk_p1_pg2", DIV_P1_PG, 0x862, 0x863, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_P1_PG3, "clk_p1_pg3", DIV_P1_PG, 0x864, 0x865, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_P1_PG4, "clk_p1_pg4", DIV_P1_PG, 0x866, 0x867, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_P4_PG3, "clk_p4_pg3", DIV_P4_PG, 0x824, 0x825, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_P4_PG4, "clk_p4_pg4", DIV_P4_PG, 0x826, 0x827, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_P6_PG1, "clk_p6_pg1", DIV_P6_PG, 0x8a0, 0x8a1, 0x8a2, 0, 0xb60, 0, 0),
> +	D_GATE(CLK_P6_PG2, "clk_p6_pg2", DIV_P6_PG, 0x8a3, 0x8a4, 0x8a5, 0, 0xb61, 0, 0),
> +	D_GATE(CLK_P6_PG3, "clk_p6_pg3", DIV_P6_PG, 0x8a6, 0x8a7, 0x8a8, 0, 0xb62, 0, 0),
> +	D_GATE(CLK_P6_PG4, "clk_p6_pg4", DIV_P6_PG, 0x8a9, 0x8aa, 0x8ab, 0, 0xb63, 0, 0),
> +	D_MODULE(CLK_PCI_USB, "clk_pci_usb", CLKOUT_D40, 0xe6, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_QSPI0, "clk_qspi0", DIV_QSPI0, 0x2a4, 0x2a5, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_QSPI1, "clk_qspi1", DIV_QSPI1, 0x484, 0x485, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_RGMII_REF, "clk_rgmii_ref", CLKOUT_D8, 0x340, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_RMII_REF, "clk_rmii_ref", CLKOUT_D20, 0x341, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SDIO0, "clk_sdio0", DIV_SDIO0, 0x64, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SDIO1, "clk_sdio1", DIV_SDIO1, 0x644, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SERCOS100, "clk_sercos100", CLKOUT_D10, 0x425, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SLCD, "clk_slcd", DIV_P1_PG, 0x860, 0x861, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SPI0, "clk_spi0", DIV_P3_PG, 0x7e0, 0x7e1, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SPI1, "clk_spi1", DIV_P3_PG, 0x7e2, 0x7e3, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SPI2, "clk_spi2", DIV_P3_PG, 0x7e4, 0x7e5, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SPI3, "clk_spi3", DIV_P3_PG, 0x7e6, 0x7e7, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SPI4, "clk_spi4", DIV_P4_PG, 0x820, 0x821, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SPI5, "clk_spi5", DIV_P4_PG, 0x822, 0x823, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SWITCH, "clk_switch", DIV_SWITCH, 0x982, 0x983, 0, 0, 0, 0, 0),
> +	D_DIV(DIV_MOTOR, "div_motor", CLKOUT_D5, 84, 2, 8),
> +	D_MODULE(HCLK_ECAT125, "hclk_ecat125", CLKOUT_D8, 0x400, 0x401, 0, 0x402, 0, 0x440, 0x441),
> +	D_MODULE(HCLK_PINCONFIG, "hclk_pinconfig", CLKOUT_D40, 0x740, 0x741, 0x742, 0, 0xae0, 0, 0),
> +	D_MODULE(HCLK_SERCOS, "hclk_sercos", CLKOUT_D10, 0x420, 0x422, 0, 0x421, 0, 0x460, 0x461),
> +	D_MODULE(HCLK_SGPIO2, "hclk_sgpio2", DIV_P5_PG, 0x8c3, 0x8c4, 0x8c5, 0, 0xb41, 0, 0),
> +	D_MODULE(HCLK_SGPIO3, "hclk_sgpio3", DIV_P5_PG, 0x8c6, 0x8c7, 0x8c8, 0, 0xb42, 0, 0),
> +	D_MODULE(HCLK_SGPIO4, "hclk_sgpio4", DIV_P5_PG, 0x8c9, 0x8ca, 0x8cb, 0, 0xb43, 0, 0),
> +	D_MODULE(HCLK_TIMER0, "hclk_timer0", CLKOUT_D40, 0x743, 0x744, 0x745, 0, 0xae1, 0, 0),
> +	D_MODULE(HCLK_TIMER1, "hclk_timer1", CLKOUT_D40, 0x746, 0x747, 0x748, 0, 0xae2, 0, 0),
> +	D_MODULE(HCLK_USBF, "hclk_usbf", CLKOUT_D8, 0xe3, 0, 0, 0xe4, 0, 0x102, 0x103),
> +	D_MODULE(HCLK_USBH, "hclk_usbh", CLKOUT_D8, 0xe0, 0xe1, 0, 0xe2, 0, 0x100, 0x101),
> +	D_MODULE(HCLK_USBPM, "hclk_usbpm", CLKOUT_D8, 0xe5, 0, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_48_PG_F, "clk_48_pg_f", CLK_48, 0x78c, 0x78d, 0, 0x78e, 0, 0xb04, 0xb05),
> +	D_GATE(CLK_48_PG4, "clk_48_pg4", CLK_48, 0x789, 0x78a, 0x78b, 0, 0xb03, 0, 0),
> +	D_FFC(CLK_DDRPHY_PLLCLK_D4, "clk_ddrphy_pllclk_d4", CLK_DDRPHY_PLLCLK, 4),
> +	D_FFC(CLK_ECAT100_D4, "clk_ecat100_d4", CLK_ECAT100, 4),
> +	D_FFC(CLK_HSR100_D2, "clk_hsr100_d2", CLK_HSR100, 2),
> +	D_FFC(CLK_REF_SYNC_D4, "clk_ref_sync_d4", CLK_REF_SYNC, 4),
> +	D_FFC(CLK_REF_SYNC_D8, "clk_ref_sync_d8", CLK_REF_SYNC, 8),
> +	D_FFC(CLK_SERCOS100_D2, "clk_sercos100_d2", CLK_SERCOS100, 2),
> +	D_DIV(DIV_CA7, "div_ca7", CLK_REF_SYNC, 57, 1, 4, 1, 2, 4),
> +	D_MODULE(HCLK_CAN0, "hclk_can0", CLK_48, 0x783, 0x784, 0x785, 0, 0xb01, 0, 0),
> +	D_MODULE(HCLK_CAN1, "hclk_can1", CLK_48, 0x786, 0x787, 0x788, 0, 0xb02, 0, 0),
> +	D_MODULE(HCLK_DELTASIGMA, "hclk_deltasigma", DIV_MOTOR, 0x1ef, 0x1f0, 0x1f1, 0, 0, 0, 0),
> +	D_MODULE(HCLK_PWMPTO, "hclk_pwmpto", DIV_MOTOR, 0x1ec, 0x1ed, 0x1ee, 0, 0, 0, 0),
> +	D_MODULE(HCLK_RSV, "hclk_rsv", CLK_48, 0x780, 0x781, 0x782, 0, 0xb00, 0, 0),
> +	D_MODULE(HCLK_SGPIO0, "hclk_sgpio0", DIV_MOTOR, 0x1e0, 0x1e1, 0x1e2, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SGPIO1, "hclk_sgpio1", DIV_MOTOR, 0x1e3, 0x1e4, 0x1e5, 0, 0, 0, 0),
> +	D_DIV(RTOS_MDC, "rtos_mdc", CLK_REF_SYNC, 100, 80, 640, 80, 160, 320, 640),
> +	D_GATE(CLK_CM3, "clk_cm3", CLK_REF_SYNC_D4, 0xba0, 0xba1, 0, 0xba2, 0, 0xbc0, 0xbc1),
> +	D_GATE(CLK_DDRC, "clk_ddrc", CLK_DDRPHY_PLLCLK_D4, 0x323, 0x324, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_ECAT25, "clk_ecat25", CLK_ECAT100_D4, 0x403, 0x404, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_HSR50, "clk_hsr50", CLK_HSR100_D2, 0x484, 0x485, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_HW_RTOS, "clk_hw_rtos", CLK_REF_SYNC_D4, 0xc60, 0xc61, 0, 0, 0, 0, 0),
> +	D_GATE(CLK_SERCOS50, "clk_sercos50", CLK_SERCOS100_D2, 0x424, 0x423, 0, 0, 0, 0, 0),
> +	D_MODULE(HCLK_ADC, "hclk_adc", CLK_REF_SYNC_D8, 0x1af, 0x1b0, 0x1b1, 0, 0, 0, 0),
> +	D_MODULE(HCLK_CM3, "hclk_cm3", CLK_REF_SYNC_D4, 0xc20, 0xc21, 0xc22, 0, 0, 0, 0),
> +	D_MODULE(HCLK_CRYPTO_EIP150, "hclk_crypto_eip150", CLK_REF_SYNC_D4, 0x123, 0x124, 0x125, 0, 0x142, 0, 0),
> +	D_MODULE(HCLK_CRYPTO_EIP93, "hclk_crypto_eip93", CLK_REF_SYNC_D4, 0x120, 0x121, 0, 0x122, 0, 0x140, 0x141),
> +	D_MODULE(HCLK_DDRC, "hclk_ddrc", CLK_REF_SYNC_D4, 0x320, 0x322, 0, 0x321, 0, 0x3a0, 0x3a1),
> +	D_MODULE(HCLK_DMA0, "hclk_dma0", CLK_REF_SYNC_D4, 0x260, 0x261, 0x262, 0x263, 0x2c0, 0x2c1, 0x2c2),
> +	D_MODULE(HCLK_DMA1, "hclk_dma1", CLK_REF_SYNC_D4, 0x264, 0x265, 0x266, 0x267, 0x2c3, 0x2c4, 0x2c5),
> +	D_MODULE(HCLK_GMAC0, "hclk_gmac0", CLK_REF_SYNC_D4, 0x360, 0x361, 0x362, 0x363, 0x3c0, 0x3c1, 0x3c2),
> +	D_MODULE(HCLK_GMAC1, "hclk_gmac1", CLK_REF_SYNC_D4, 0x380, 0x381, 0x382, 0x383, 0x3e0, 0x3e1, 0x3e2),
> +	D_MODULE(HCLK_GPIO0, "hclk_gpio0", CLK_REF_SYNC_D4, 0x212, 0x213, 0x214, 0, 0, 0, 0),
> +	D_MODULE(HCLK_GPIO1, "hclk_gpio1", CLK_REF_SYNC_D4, 0x215, 0x216, 0x217, 0, 0, 0, 0),
> +	D_MODULE(HCLK_GPIO2, "hclk_gpio2", CLK_REF_SYNC_D4, 0x229, 0x22a, 0x22b, 0, 0, 0, 0),
> +	D_MODULE(HCLK_HSR, "hclk_hsr", CLK_HSR100_D2, 0x480, 0x482, 0, 0x481, 0, 0x4c0, 0x4c1),
> +	D_MODULE(HCLK_I2C0, "hclk_i2c0", CLK_REF_SYNC_D8, 0x1a9, 0x1aa, 0x1ab, 0, 0, 0, 0),
> +	D_MODULE(HCLK_I2C1, "hclk_i2c1", CLK_REF_SYNC_D8, 0x1ac, 0x1ad, 0x1ae, 0, 0, 0, 0),
> +	D_MODULE(HCLK_LCD, "hclk_lcd", CLK_REF_SYNC_D4, 0x7a0, 0x7a1, 0x7a2, 0, 0xb20, 0, 0),
> +	D_MODULE(HCLK_MSEBI_M, "hclk_msebi_m", CLK_REF_SYNC_D4, 0x164, 0x165, 0x166, 0, 0x183, 0, 0),
> +	D_MODULE(HCLK_MSEBI_S, "hclk_msebi_s", CLK_REF_SYNC_D4, 0x160, 0x161, 0x162, 0x163, 0x180, 0x181, 0x182),
> +	D_MODULE(HCLK_NAND, "hclk_nand", CLK_REF_SYNC_D4, 0x280, 0x281, 0x282, 0x283, 0x2e0, 0x2e1, 0x2e2),
> +	D_MODULE(HCLK_PG_I, "hclk_pg_i", CLK_REF_SYNC_D4, 0x7ac, 0x7ad, 0, 0x7ae, 0, 0xb24, 0xb25),
> +	D_MODULE(HCLK_PG19, "hclk_pg19", CLK_REF_SYNC_D4, 0x22c, 0x22d, 0x22e, 0, 0, 0, 0),
> +	D_MODULE(HCLK_PG20, "hclk_pg20", CLK_REF_SYNC_D4, 0x22f, 0x230, 0x231, 0, 0, 0, 0),
> +	D_MODULE(HCLK_PG3, "hclk_pg3", CLK_REF_SYNC_D4, 0x7a6, 0x7a7, 0x7a8, 0, 0xb22, 0, 0),
> +	D_MODULE(HCLK_PG4, "hclk_pg4", CLK_REF_SYNC_D4, 0x7a9, 0x7aa, 0x7ab, 0, 0xb23, 0, 0),
> +	D_MODULE(HCLK_QSPI0, "hclk_qspi0", CLK_REF_SYNC_D4, 0x2a0, 0x2a1, 0x2a2, 0x2a3, 0x300, 0x301, 0x302),
> +	D_MODULE(HCLK_QSPI1, "hclk_qspi1", CLK_REF_SYNC_D4, 0x480, 0x481, 0x482, 0x483, 0x4c0, 0x4c1, 0x4c2),
> +	D_MODULE(HCLK_ROM, "hclk_rom", CLK_REF_SYNC_D4, 0xaa0, 0xaa1, 0xaa2, 0, 0xb80, 0, 0),
> +	D_MODULE(HCLK_RTC, "hclk_rtc", CLK_REF_SYNC_D8, 0xa00, 0, 0, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SDIO0, "hclk_sdio0", CLK_REF_SYNC_D4, 0x60, 0x61, 0x62, 0x63, 0x80, 0x81, 0x82),
> +	D_MODULE(HCLK_SDIO1, "hclk_sdio1", CLK_REF_SYNC_D4, 0x640, 0x641, 0x642, 0x643, 0x660, 0x661, 0x662),
> +	D_MODULE(HCLK_SEMAP, "hclk_semap", CLK_REF_SYNC_D4, 0x7a3, 0x7a4, 0x7a5, 0, 0xb21, 0, 0),
> +	D_MODULE(HCLK_SPI0, "hclk_spi0", CLK_REF_SYNC_D4, 0x200, 0x201, 0x202, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SPI1, "hclk_spi1", CLK_REF_SYNC_D4, 0x203, 0x204, 0x205, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SPI2, "hclk_spi2", CLK_REF_SYNC_D4, 0x206, 0x207, 0x208, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SPI3, "hclk_spi3", CLK_REF_SYNC_D4, 0x209, 0x20a, 0x20b, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SPI4, "hclk_spi4", CLK_REF_SYNC_D4, 0x20c, 0x20d, 0x20e, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SPI5, "hclk_spi5", CLK_REF_SYNC_D4, 0x20f, 0x210, 0x211, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SWITCH, "hclk_switch", CLK_REF_SYNC_D4, 0x980, 0, 0x981, 0, 0, 0, 0),
> +	D_MODULE(HCLK_SWITCH_RG, "hclk_switch_rg", CLK_REF_SYNC_D4, 0xc40, 0xc41, 0xc42, 0, 0, 0, 0),
> +	D_MODULE(HCLK_UART0, "hclk_uart0", CLK_REF_SYNC_D8, 0x1a0, 0x1a1, 0x1a2, 0, 0, 0, 0),
> +	D_MODULE(HCLK_UART1, "hclk_uart1", CLK_REF_SYNC_D8, 0x1a3, 0x1a4, 0x1a5, 0, 0, 0, 0),
> +	D_MODULE(HCLK_UART2, "hclk_uart2", CLK_REF_SYNC_D8, 0x1a6, 0x1a7, 0x1a8, 0, 0, 0, 0),
> +	D_MODULE(HCLK_UART3, "hclk_uart3", CLK_REF_SYNC_D4, 0x218, 0x219, 0x21a, 0, 0, 0, 0),
> +	D_MODULE(HCLK_UART4, "hclk_uart4", CLK_REF_SYNC_D4, 0x21b, 0x21c, 0x21d, 0, 0, 0, 0),
> +	D_MODULE(HCLK_UART5, "hclk_uart5", CLK_REF_SYNC_D4, 0x220, 0x221, 0x222, 0, 0, 0, 0),
> +	D_MODULE(HCLK_UART6, "hclk_uart6", CLK_REF_SYNC_D4, 0x223, 0x224, 0x225, 0, 0, 0, 0),
> +	D_MODULE(HCLK_UART7, "hclk_uart7", CLK_REF_SYNC_D4, 0x226, 0x227, 0x228, 0, 0, 0, 0),
> +	/*
> +	 * These are not hardware clocks, but are needed to handle the special
> +	 * case where we have a 'selector bit' that doesn't just change the
> +	 * parent for a clock, but also the gate it's supposed to use.
> +	 */
> +	{
> +		.index = R9A06G032_UART_GROUP_012,
> +		.name = "uart_group_012",
> +		.type = K_BITSEL,
> +		.source = 1 + R9A06G032_DIV_UART,
> +		/* R9A06G032_SYSCTRL_REG_PWRCTRL_PG0_0 */
> +		.dual.sel = ((0x34 / 4) << 5) | 30,
> +		.dual.group = 0,
> +	},
> +	{
> +		.index = R9A06G032_UART_GROUP_34567,
> +		.name = "uart_group_34567",
> +		.type = K_BITSEL,
> +		.source = 1 + R9A06G032_DIV_P2_PG,
> +		/* R9A06G032_SYSCTRL_REG_PWRCTRL_PG1_PR2 */
> +		.dual.sel = ((0xec / 4) << 5) | 24,
> +		.dual.group = 1,
> +	},
> +	D_UGATE(CLK_UART0, "clk_uart0", UART_GROUP_012, 0, 0x1b2, 0x1b3, 0x1b4, 0x1b5),
> +	D_UGATE(CLK_UART1, "clk_uart1", UART_GROUP_012, 0, 0x1b6, 0x1b7, 0x1b8, 0x1b9),
> +	D_UGATE(CLK_UART2, "clk_uart2", UART_GROUP_012, 0, 0x1ba, 0x1bb, 0x1bc, 0x1bd),
> +	D_UGATE(CLK_UART3, "clk_uart3", UART_GROUP_34567, 1, 0x760, 0x761, 0x762, 0x763),
> +	D_UGATE(CLK_UART4, "clk_uart4", UART_GROUP_34567, 1, 0x764, 0x765, 0x766, 0x767),
> +	D_UGATE(CLK_UART5, "clk_uart5", UART_GROUP_34567, 1, 0x768, 0x769, 0x76a, 0x76b),
> +	D_UGATE(CLK_UART6, "clk_uart6", UART_GROUP_34567, 1, 0x76c, 0x76d, 0x76e, 0x76f),
> +	D_UGATE(CLK_UART7, "clk_uart7", UART_GROUP_34567, 1, 0x770, 0x771, 0x772, 0x773),
> +};
> +
> +struct r9a06g032_priv {
> +	struct regmap		*regmap;
> +	struct clk		mclk;
> +};
> +
> +static const struct r9a06g032_clkdesc *r9a06g032_clk_get(struct clk *clk)
> +{
> +	const unsigned long clkid = clk->id & 0xffff;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(r9a06g032_clocks); i++) {
> +		if (r9a06g032_clocks[i].index == clkid)
> +			return &r9a06g032_clocks[i];
> +	}
> +
> +	return NULL;
> +}
> +
> +static int r9a06g032_clk_get_parent(struct clk *clk, struct clk *parent)
> +{
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +
> +	if (!desc)
> +		return -ENODEV;

ENOENT please

see https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#error-codes

> +
> +	if (desc->source == 0)

if (!desc->source)

although I would reverse the clauses to make the condition clearer

> +		parent->id = ~0;	/* Top-level clock */

Can you use a define for this (instead of referring to ~0 everywhere)

> +	else
> +		parent->id = desc->source - 1;
> +
> +	parent->dev = clk->dev;

I think you need to clk_request here.

> +	return 0;
> +}
> +
> +static ulong r9a06g032_clk_get_parent_rate(struct clk *clk)
> +{
> +	struct clk parent;
> +
> +	if (r9a06g032_clk_get_parent(clk, &parent)) {
> +		debug("Failed to get parent clock for id=%lu\b", clk->id);

dev_dbg please

> +		return 0;

Return -ENOENT here please. You can check for this with IS_ERR.

> +	}
> +
> +	if (parent.id == ~0) {
> +		struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
> +		ulong rate = clk_get_rate(&clocks->mclk);

You need a newline here

> +		return rate;
> +	}
> +
> +	return clk_get_rate(&parent);
> +}
> +
> +/* register/bit pairs are encoded as an uint16_t */
> +static void
> +clk_rdesc_set(struct r9a06g032_priv *clocks,
> +	      u16 one, unsigned int on)
> +{
> +	uint offset = 4 * (one >> 5);
> +	uint mask = 1U << (one & 0x1f);
> +	uint val = ((!!on) << (one & 0x1f));

Please either use bitfields for this, or use FIELD_GET() and friends.

> +	regmap_update_bits(clocks->regmap, offset, mask, val);
> +}
> +
> +static int
> +clk_rdesc_get(struct r9a06g032_priv *clocks,
> +	      uint16_t one)

I think this all fits on one line?

> +{
> +	uint offset = 4 * (one >> 5);
> +	u32 val = 0;
> +
> +	regmap_read(clocks->regmap, offset, &val);
> +
> +	return !!(val & (1U << (one & 0x1f)));
> +}
> +
> +/*
> + * Cheating a little bit here: leverage the existing code to control the
> + * per-clock reset. It should really be handled by a reset controller instead.
> + */
> +void clk_rzn1_reset_state(struct clk *clk, int on)
> +{
> +	struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +	assert(desc);
> +	assert(desc->type == K_GATE);
> +	const struct r9a06g032_gate *g = &desc->gate;
> +	assert(g->reset);

Please order declarations all at the beginning. In this case, you will need to do something like

	struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
	const struct r9a06g032_gate *g;

	assert(desc);
	assert(desc->type == K_GATE);
	g = &desc->gate
	assert(g->reset);

> +	clk_rdesc_set(clocks, g->reset, on);
> +}
> +
> +/*
> + * This implements the R9A06G032 clock gate 'driver'. We cannot use the system's
> + * clock gate framework as the gates on the R9A06G032 have a special enabling
> + * sequence, therefore we use this little proxy.
> + */
> +static int r9a06g032_clk_gate_set(struct clk *clk, int on)
> +{
> +	struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +	assert(desc);
> +	assert(desc->type == K_GATE);
> +	const struct r9a06g032_gate *g = &desc->gate;

ditto

> +	clk_rdesc_set(clocks, g->gate, on);
> +	/* De-assert reset */
> +	if (g->reset)
> +		clk_rdesc_set(clocks, g->reset, 1);
> +
> +	/* Hardware manual recommends 5us delay after enabling clock & reset */
> +	udelay(5);
> +
> +	/* If the peripheral is memory mapped (i.e. an AXI slave), there is an
> +	 * associated SLVRDY bit in the System Controller that needs to be set
> +	 * so that the FlexWAY bus fabric passes on the read/write requests.
> +	 */
> +	if (g->ready || g->midle) {
> +		if (g->ready)
> +			clk_rdesc_set(clocks, g->ready, on);
> +		/* Clear 'Master Idle Request' bit */
> +		if (g->midle)
> +			clk_rdesc_set(clocks, g->midle, !on);
> +	}
> +	/* Note: We don't wait for FlexWAY Socket Connection signal */
> +
> +	return 0;
> +}
> +
> +static int r9a06g032_clk_gate_enable(struct clk *clk)
> +{
> +	return r9a06g032_clk_gate_set(clk, 1);
> +}
> +
> +static int r9a06g032_clk_gate_disable(struct clk *clk)
> +{
> +	return r9a06g032_clk_gate_set(clk, 0);
> +}
> +
> +/*
> + * Fixed factor clock
> + */
> +static ulong r9a06g032_ffc_get_rate(struct clk *clk)
> +{
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +	unsigned long parent_rate = r9a06g032_clk_get_parent_rate(clk);
> +	unsigned long long rate;
> +
> +	if (parent_rate == 0) {
> +		debug("%s: parent_rate is zero\n", __func__);
> +		return 0;
> +	}
> +
> +	rate = (unsigned long long)parent_rate * desc->mul;
> +	rate = DIV_ROUND_UP(rate, desc->div);
> +	return (ulong)rate;
> +}
> +
> +/*
> + * This implements R9A06G032 clock divider 'driver'. This differs from the
> + * standard clk_divider because the set_rate method must also set b[31] to
> + * trigger the hardware rate change. In theory it should also wait for this
> + * bit to clear.
> + */
> +static ulong r9a06g032_div_get_rate(struct clk *clk)
> +{
> +	struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +	unsigned long parent_rate = r9a06g032_clk_get_parent_rate(clk);
> +	u32 div = 0;
> +
> +	if (parent_rate == 0) {
> +		debug("%s: parent_rate is zero\n", __func__);

Didn't you already log this?

> +		return 0;
> +	}
> +
> +	regmap_read(clocks->regmap, 4 * desc->reg, &div);
> +
> +	if (div < desc->div_min)
> +		div = desc->div_min;
> +	else if (div > desc->div_max)
> +		div = desc->div_max;
> +	return DIV_ROUND_UP(parent_rate, div);

DIV_ROUND_CLOSEST?

> +}
> +
> +static ulong r9a06g032_div_set_rate(struct clk *clk, ulong rate)
> +{
> +	struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +	unsigned long parent_rate = r9a06g032_clk_get_parent_rate(clk);
> +
> +	if (parent_rate == 0) {
> +		debug("%s: parent_rate is zero\n", __func__);
> +		return 0;
> +	}
> +
> +	/* + 1 to cope with rates that have the remainder dropped */
> +	u32 div = DIV_ROUND_UP(parent_rate, rate + 1);
> +
> +	/* Clamp to allowable range */
> +	if (div < desc->div_min)
> +		div = desc->div_min;
> +	else if (div > desc->div_max)
> +		div = desc->div_max;
> +
> +	/* TODO: use the .div_table if provided */
> +	if (desc->div_table[0])
> +		pr_err("ERROR: %s: div_table not implemented\n", __func__);

dev_err

But can't you just leave out the div_table member?

> +
> +	pr_devel("%s clkid %lu rate %ld parent %ld div %d\n", __func__, clk->id,
> +		 rate, parent_rate, div);

dev_dbg

> +
> +	/*
> +	 * Need to write the bit 31 with the divider value to
> +	 * latch it. Technically we should wait until it has been
> +	 * cleared too.
> +	 * TODO: Find whether this callback is sleepable, in case
> +	 * the hardware /does/ require some sort of spinloop here.
> +	 */
> +	regmap_write(clocks->regmap, 4 * desc->reg, div | BIT(31));
> +
> +	return 0;
> +}
> +
> +/*
> + * Dual gate. This handles toggling the approprate clock/reset bits,
> + * which depends on the mux setting above.
> + */
> +static int r9a06g032_clk_dualgate_setenable(struct r9a06g032_priv *clocks,
> +					    const struct r9a06g032_clkdesc *desc,
> +					    int enable)
> +{
> +	u8 sel_bit = clk_rdesc_get(clocks, desc->dual.sel);
> +	u16 gate[2] = { desc->dual.g1, desc->dual.g2 };
> +	u16 reset[2] = { desc->dual.r1, desc->dual.r2 };
> +
> +	/* we always turn off the 'other' gate, regardless */
> +	clk_rdesc_set(clocks, gate[!sel_bit], 0);
> +	if (reset[!sel_bit])
> +		clk_rdesc_set(clocks, reset[!sel_bit], 1);
> +
> +	/* set the gate as requested */
> +	clk_rdesc_set(clocks, gate[sel_bit], enable);
> +	if (reset[sel_bit])
> +		clk_rdesc_set(clocks, reset[sel_bit], 1);
> +
> +	return 0;
> +}
> +
> +static int r9a06g032_clk_dualgate_enable(struct clk *clk)
> +{
> +	struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +
> +	return r9a06g032_clk_dualgate_setenable(clocks, desc, 1);
> +}
> +
> +static int r9a06g032_clk_dualgate_disable(struct clk *clk)
> +{
> +	struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +
> +	return r9a06g032_clk_dualgate_setenable(clocks, desc, 0);
> +}
> +
> +static int r9a06g032_clk_dualgate_is_enabled(struct clk *clk)
> +{
> +	struct r9a06g032_priv *clocks = dev_get_priv(clk->dev);
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +	u8 sel_bit = clk_rdesc_get(clocks, desc->dual.sel);
> +	u16 gate[2] = { desc->dual.g1, desc->dual.g2 };
> +
> +	return clk_rdesc_get(clocks, gate[sel_bit]);
> +}
> +
> +/*
> + * Main clock driver
> + */
> +static int r9a06g032_clk_enable(struct clk *clk)
> +{
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +
> +	switch (desc->type) {
> +	case K_GATE:
> +		return r9a06g032_clk_gate_enable(clk);
> +	case K_DUALGATE:
> +		return r9a06g032_clk_dualgate_enable(clk);
> +	default:
> +		printf("ERROR: %s:%d unhandled type=%d\n", __func__, __LINE__, desc->type);

Assert or dev_dbg is better here. This is "impossible" so we try and
avoid increasing image size in these cases.

> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int r9a06g032_clk_disable(struct clk *clk)
> +{
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +
> +	switch (desc->type) {
> +	case K_GATE:
> +		return r9a06g032_clk_gate_disable(clk);
> +	case K_DUALGATE:
> +		return r9a06g032_clk_dualgate_disable(clk);
> +	default:
> +		printf("ERROR: %s:%d unhandled type=%d\n", __func__, __LINE__, desc->type);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static ulong r9a06g032_clk_get_rate(struct clk *clk)
> +{
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +	ulong ret = 0;
> +
> +	assert(desc);
> +
> +	switch (desc->type) {
> +	case K_FFC:
> +		ret = r9a06g032_ffc_get_rate(clk);
> +		break;
> +	case K_GATE:
> +		ret = r9a06g032_clk_get_parent_rate(clk);
> +		break;
> +	case K_DIV:
> +		ret = r9a06g032_div_get_rate(clk);
> +		break;
> +	case K_BITSEL:
> +		/*
> +		 * Look at the mux to determine parent.
> +		 * 0 means it is coming from UART DIV (group 012 or 34567)
> +		 * 1 means it is coming from USB_PLL
> +		 */
> +		if (r9a06g032_clk_dualgate_is_enabled(clk)) {
> +			struct clk clk = { .id = R9A06G032_CLK_PLL_USB };
> +			ret = r9a06g032_clk_get_parent_rate(&clk);
> +		}
> +		ret = r9a06g032_clk_get_parent_rate(clk);
> +		break;
> +	case K_DUALGATE:
> +		ret = r9a06g032_clk_get_parent_rate(clk);
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static ulong r9a06g032_clk_set_rate(struct clk *clk, ulong rate)
> +{
> +	const struct r9a06g032_clkdesc *desc = r9a06g032_clk_get(clk);
> +	ulong ret = 0;
> +
> +	assert(desc);
> +
> +	switch (desc->type) {
> +	case K_DIV:
> +		ret = r9a06g032_div_set_rate(clk, rate);
> +		break;
> +	default:
> +		printf("ERROR: %s:%d not implemented yet\n", __func__, __LINE__);
> +	};
> +
> +	return ret;
> +}
> +
> +static int r9a06g032_clk_of_xlate(struct clk *clk, struct ofnode_phandle_args *args)
> +{
> +	if (args->args_count != 1) {
> +		debug("Invalid args_count: %d\n", args->args_count);
> +		return -EINVAL;
> +	}
> +
> +	clk->id = args->args[0];
> +
> +	return 0;
> +}
> +
> +static const struct clk_ops r9a06g032_clk_ops = {
> +	.enable		= r9a06g032_clk_enable,
> +	.disable	= r9a06g032_clk_disable,
> +	.get_rate	= r9a06g032_clk_get_rate,
> +	.set_rate	= r9a06g032_clk_set_rate,
> +	.of_xlate	= r9a06g032_clk_of_xlate,
> +};
> +
> +static int r9a06g032_clk_probe(struct udevice *dev)
> +{
> +	struct r9a06g032_priv *priv = dev_get_priv(dev);
> +	int err;
> +
> +	priv->regmap = syscon_regmap_lookup_by_phandle(dev, "regmap");
> +	if (!priv->regmap) {

IS_ERR(priv->regmap)

> +		pr_err("unable to find regmap\n");

dev_dbg

> +		return -ENODEV;

return ERR_PTR(priv->regmap)

> +	}
> +
> +	/* Enable S/W reset */
> +	regmap_write(priv->regmap, 0x120, 0x41);
> +
> +	err = clk_get_by_name(dev, "mclk", &priv->mclk);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int r9a06g032_clk_remove(struct udevice *dev)
> +{
> +	return 0;
> +}

Not necessary.

> +
> +static const struct udevice_id r9a06g032_clk_ids[] = {
> +	{ .compatible = "renesas,r9a06g032-sysctrl" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(clk_r9a06g032) = {
> +	.name		= "clk_r9a06g032",
> +	.id		= UCLASS_CLK,
> +	.of_match	= r9a06g032_clk_ids,
> +	.priv_auto	= sizeof(struct r9a06g032_priv),
> +	.ops		= &r9a06g032_clk_ops,
> +	.probe		= &r9a06g032_clk_probe,
> +	.remove		= &r9a06g032_clk_remove,
> +	.flags		= DM_FLAG_PRE_RELOC,
> +};
> 

The overall structure looks good; most of these things you
should be able to iron out fairly easily.

--Sean


More information about the U-Boot mailing list