* [PATCH v2] clk: renesas: r9a06g032: Add clock domain support
@ 2019-05-21 12:35 Gareth Williams
2019-05-22 12:01 ` Geert Uytterhoeven
0 siblings, 1 reply; 4+ messages in thread
From: Gareth Williams @ 2019-05-21 12:35 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Phil Edworthy
Cc: Gareth Williams, linux-renesas-soc, linux-clk, linux-kernel
There are several clocks on the r9ag032 which are currently not enabled
in their drivers that can be delegated to clock domain system for power
management. Therefore add support for clock domain functionality to the
r9a06g032 clock driver.
Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
v2:
- Rebased onto kernel/git/geert/renesas-drivers.git
---
drivers/clk/renesas/r9a06g032-clocks.c | 243 ++++++++++++++++++++++++---------
1 file changed, 176 insertions(+), 67 deletions(-)
diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
index 97c7247..2e580dd 100644
--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c
@@ -16,6 +16,8 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_domain.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <dt-bindings/clock/r9a06g032-sysctrl.h>
@@ -28,6 +30,7 @@ struct r9a06g032_gate {
/* This is used to describe a clock for instantiation */
struct r9a06g032_clkdesc {
const char *name;
+ bool managed;
uint32_t type: 3;
uint32_t index: 8;
uint32_t source : 8; /* source index + 1 (0 == none) */
@@ -60,7 +63,11 @@ struct r9a06g032_clkdesc {
#define D_GATE(_idx, _n, _src, ...) \
{ .type = K_GATE, .index = R9A06G032_##_idx, \
.source = 1 + R9A06G032_##_src, .name = _n, \
- .gate = I_GATE(__VA_ARGS__), }
+ .managed = 0, .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 }
@@ -170,7 +177,7 @@ static const struct r9a06g032_clkdesc r9a06g032_clocks[] __initconst = {
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_GATE(CLK_PCI_USB, "clk_pci_usb", CLKOUT_D40, 0xe6, 0, 0, 0, 0, 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),
@@ -187,17 +194,17 @@ static const struct r9a06g032_clkdesc r9a06g032_clocks[] __initconst = {
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_GATE(HCLK_ECAT125, "hclk_ecat125", CLKOUT_D8, 0x400, 0x401, 0, 0x402, 0, 0x440, 0x441),
- D_GATE(HCLK_PINCONFIG, "hclk_pinconfig", CLKOUT_D40, 0x740, 0x741, 0x742, 0, 0xae0, 0, 0),
- D_GATE(HCLK_SERCOS, "hclk_sercos", CLKOUT_D10, 0x420, 0x422, 0, 0x421, 0, 0x460, 0x461),
- D_GATE(HCLK_SGPIO2, "hclk_sgpio2", DIV_P5_PG, 0x8c3, 0x8c4, 0x8c5, 0, 0xb41, 0, 0),
- D_GATE(HCLK_SGPIO3, "hclk_sgpio3", DIV_P5_PG, 0x8c6, 0x8c7, 0x8c8, 0, 0xb42, 0, 0),
- D_GATE(HCLK_SGPIO4, "hclk_sgpio4", DIV_P5_PG, 0x8c9, 0x8ca, 0x8cb, 0, 0xb43, 0, 0),
- D_GATE(HCLK_TIMER0, "hclk_timer0", CLKOUT_D40, 0x743, 0x744, 0x745, 0, 0xae1, 0, 0),
- D_GATE(HCLK_TIMER1, "hclk_timer1", CLKOUT_D40, 0x746, 0x747, 0x748, 0, 0xae2, 0, 0),
- D_GATE(HCLK_USBF, "hclk_usbf", CLKOUT_D8, 0xe3, 0, 0, 0xe4, 0, 0x102, 0x103),
- D_GATE(HCLK_USBH, "hclk_usbh", CLKOUT_D8, 0xe0, 0xe1, 0, 0xe2, 0, 0x100, 0x101),
- D_GATE(HCLK_USBPM, "hclk_usbpm", CLKOUT_D8, 0xe5, 0, 0, 0, 0, 0, 0),
+ 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),
@@ -207,13 +214,13 @@ static const struct r9a06g032_clkdesc r9a06g032_clocks[] __initconst = {
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_GATE(HCLK_CAN0, "hclk_can0", CLK_48, 0x783, 0x784, 0x785, 0, 0xb01, 0, 0),
- D_GATE(HCLK_CAN1, "hclk_can1", CLK_48, 0x786, 0x787, 0x788, 0, 0xb02, 0, 0),
- D_GATE(HCLK_DELTASIGMA, "hclk_deltasigma", DIV_MOTOR, 0x1ef, 0x1f0, 0x1f1, 0, 0, 0, 0),
- D_GATE(HCLK_PWMPTO, "hclk_pwmpto", DIV_MOTOR, 0x1ec, 0x1ed, 0x1ee, 0, 0, 0, 0),
- D_GATE(HCLK_RSV, "hclk_rsv", CLK_48, 0x780, 0x781, 0x782, 0, 0xb00, 0, 0),
- D_GATE(HCLK_SGPIO0, "hclk_sgpio0", DIV_MOTOR, 0x1e0, 0x1e1, 0x1e2, 0, 0, 0, 0),
- D_GATE(HCLK_SGPIO1, "hclk_sgpio1", DIV_MOTOR, 0x1e3, 0x1e4, 0x1e5, 0, 0, 0, 0),
+ 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),
@@ -221,53 +228,53 @@ static const struct r9a06g032_clkdesc r9a06g032_clocks[] __initconst = {
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_GATE(HCLK_ADC, "hclk_adc", CLK_REF_SYNC_D8, 0x1af, 0x1b0, 0x1b1, 0, 0, 0, 0),
- D_GATE(HCLK_CM3, "hclk_cm3", CLK_REF_SYNC_D4, 0xc20, 0xc21, 0xc22, 0, 0, 0, 0),
- D_GATE(HCLK_CRYPTO_EIP150, "hclk_crypto_eip150", CLK_REF_SYNC_D4, 0x123, 0x124, 0x125, 0, 0x142, 0, 0),
- D_GATE(HCLK_CRYPTO_EIP93, "hclk_crypto_eip93", CLK_REF_SYNC_D4, 0x120, 0x121, 0, 0x122, 0, 0x140, 0x141),
- D_GATE(HCLK_DDRC, "hclk_ddrc", CLK_REF_SYNC_D4, 0x320, 0x322, 0, 0x321, 0, 0x3a0, 0x3a1),
- D_GATE(HCLK_DMA0, "hclk_dma0", CLK_REF_SYNC_D4, 0x260, 0x261, 0x262, 0x263, 0x2c0, 0x2c1, 0x2c2),
- D_GATE(HCLK_DMA1, "hclk_dma1", CLK_REF_SYNC_D4, 0x264, 0x265, 0x266, 0x267, 0x2c3, 0x2c4, 0x2c5),
- D_GATE(HCLK_GMAC0, "hclk_gmac0", CLK_REF_SYNC_D4, 0x360, 0x361, 0x362, 0x363, 0x3c0, 0x3c1, 0x3c2),
- D_GATE(HCLK_GMAC1, "hclk_gmac1", CLK_REF_SYNC_D4, 0x380, 0x381, 0x382, 0x383, 0x3e0, 0x3e1, 0x3e2),
- D_GATE(HCLK_GPIO0, "hclk_gpio0", CLK_REF_SYNC_D4, 0x212, 0x213, 0x214, 0, 0, 0, 0),
- D_GATE(HCLK_GPIO1, "hclk_gpio1", CLK_REF_SYNC_D4, 0x215, 0x216, 0x217, 0, 0, 0, 0),
- D_GATE(HCLK_GPIO2, "hclk_gpio2", CLK_REF_SYNC_D4, 0x229, 0x22a, 0x22b, 0, 0, 0, 0),
- D_GATE(HCLK_HSR, "hclk_hsr", CLK_HSR100_D2, 0x480, 0x482, 0, 0x481, 0, 0x4c0, 0x4c1),
- D_GATE(HCLK_I2C0, "hclk_i2c0", CLK_REF_SYNC_D8, 0x1a9, 0x1aa, 0x1ab, 0, 0, 0, 0),
- D_GATE(HCLK_I2C1, "hclk_i2c1", CLK_REF_SYNC_D8, 0x1ac, 0x1ad, 0x1ae, 0, 0, 0, 0),
- D_GATE(HCLK_LCD, "hclk_lcd", CLK_REF_SYNC_D4, 0x7a0, 0x7a1, 0x7a2, 0, 0xb20, 0, 0),
- D_GATE(HCLK_MSEBI_M, "hclk_msebi_m", CLK_REF_SYNC_D4, 0x164, 0x165, 0x166, 0, 0x183, 0, 0),
- D_GATE(HCLK_MSEBI_S, "hclk_msebi_s", CLK_REF_SYNC_D4, 0x160, 0x161, 0x162, 0x163, 0x180, 0x181, 0x182),
- D_GATE(HCLK_NAND, "hclk_nand", CLK_REF_SYNC_D4, 0x280, 0x281, 0x282, 0x283, 0x2e0, 0x2e1, 0x2e2),
- D_GATE(HCLK_PG_I, "hclk_pg_i", CLK_REF_SYNC_D4, 0x7ac, 0x7ad, 0, 0x7ae, 0, 0xb24, 0xb25),
- D_GATE(HCLK_PG19, "hclk_pg19", CLK_REF_SYNC_D4, 0x22c, 0x22d, 0x22e, 0, 0, 0, 0),
- D_GATE(HCLK_PG20, "hclk_pg20", CLK_REF_SYNC_D4, 0x22f, 0x230, 0x231, 0, 0, 0, 0),
- D_GATE(HCLK_PG3, "hclk_pg3", CLK_REF_SYNC_D4, 0x7a6, 0x7a7, 0x7a8, 0, 0xb22, 0, 0),
- D_GATE(HCLK_PG4, "hclk_pg4", CLK_REF_SYNC_D4, 0x7a9, 0x7aa, 0x7ab, 0, 0xb23, 0, 0),
- D_GATE(HCLK_QSPI0, "hclk_qspi0", CLK_REF_SYNC_D4, 0x2a0, 0x2a1, 0x2a2, 0x2a3, 0x300, 0x301, 0x302),
- D_GATE(HCLK_QSPI1, "hclk_qspi1", CLK_REF_SYNC_D4, 0x480, 0x481, 0x482, 0x483, 0x4c0, 0x4c1, 0x4c2),
- D_GATE(HCLK_ROM, "hclk_rom", CLK_REF_SYNC_D4, 0xaa0, 0xaa1, 0xaa2, 0, 0xb80, 0, 0),
- D_GATE(HCLK_RTC, "hclk_rtc", CLK_REF_SYNC_D8, 0xa00, 0, 0, 0, 0, 0, 0),
- D_GATE(HCLK_SDIO0, "hclk_sdio0", CLK_REF_SYNC_D4, 0x60, 0x61, 0x62, 0x63, 0x80, 0x81, 0x82),
- D_GATE(HCLK_SDIO1, "hclk_sdio1", CLK_REF_SYNC_D4, 0x640, 0x641, 0x642, 0x643, 0x660, 0x661, 0x662),
- D_GATE(HCLK_SEMAP, "hclk_semap", CLK_REF_SYNC_D4, 0x7a3, 0x7a4, 0x7a5, 0, 0xb21, 0, 0),
- D_GATE(HCLK_SPI0, "hclk_spi0", CLK_REF_SYNC_D4, 0x200, 0x201, 0x202, 0, 0, 0, 0),
- D_GATE(HCLK_SPI1, "hclk_spi1", CLK_REF_SYNC_D4, 0x203, 0x204, 0x205, 0, 0, 0, 0),
- D_GATE(HCLK_SPI2, "hclk_spi2", CLK_REF_SYNC_D4, 0x206, 0x207, 0x208, 0, 0, 0, 0),
- D_GATE(HCLK_SPI3, "hclk_spi3", CLK_REF_SYNC_D4, 0x209, 0x20a, 0x20b, 0, 0, 0, 0),
- D_GATE(HCLK_SPI4, "hclk_spi4", CLK_REF_SYNC_D4, 0x20c, 0x20d, 0x20e, 0, 0, 0, 0),
- D_GATE(HCLK_SPI5, "hclk_spi5", CLK_REF_SYNC_D4, 0x20f, 0x210, 0x211, 0, 0, 0, 0),
- D_GATE(HCLK_SWITCH, "hclk_switch", CLK_REF_SYNC_D4, 0x980, 0, 0x981, 0, 0, 0, 0),
- D_GATE(HCLK_SWITCH_RG, "hclk_switch_rg", CLK_REF_SYNC_D4, 0xc40, 0xc41, 0xc42, 0, 0, 0, 0),
- D_GATE(HCLK_UART0, "hclk_uart0", CLK_REF_SYNC_D8, 0x1a0, 0x1a1, 0x1a2, 0, 0, 0, 0),
- D_GATE(HCLK_UART1, "hclk_uart1", CLK_REF_SYNC_D8, 0x1a3, 0x1a4, 0x1a5, 0, 0, 0, 0),
- D_GATE(HCLK_UART2, "hclk_uart2", CLK_REF_SYNC_D8, 0x1a6, 0x1a7, 0x1a8, 0, 0, 0, 0),
- D_GATE(HCLK_UART3, "hclk_uart3", CLK_REF_SYNC_D4, 0x218, 0x219, 0x21a, 0, 0, 0, 0),
- D_GATE(HCLK_UART4, "hclk_uart4", CLK_REF_SYNC_D4, 0x21b, 0x21c, 0x21d, 0, 0, 0, 0),
- D_GATE(HCLK_UART5, "hclk_uart5", CLK_REF_SYNC_D4, 0x220, 0x221, 0x222, 0, 0, 0, 0),
- D_GATE(HCLK_UART6, "hclk_uart6", CLK_REF_SYNC_D4, 0x223, 0x224, 0x225, 0, 0, 0, 0),
- D_GATE(HCLK_UART7, "hclk_uart7", CLK_REF_SYNC_D4, 0x226, 0x227, 0x228, 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
@@ -344,6 +351,104 @@ struct r9a06g032_clk_gate {
#define to_r9a06g032_gate(_hw) container_of(_hw, struct r9a06g032_clk_gate, hw)
+struct r9a06g032_clk_domain {
+ struct generic_pm_domain genpd;
+ struct device_node *np;
+};
+
+static struct r9a06g032_clk_domain *r9a06g032_clk_domain;
+
+static int create_add_module_clock(struct of_phandle_args *clkspec,
+ struct device *dev)
+{
+ struct clk *clk;
+ int error = 0;
+
+ clk = of_clk_get_from_provider(clkspec);
+
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ error = pm_clk_create(dev);
+ if (error) {
+ dev_err(dev, "pm_clk_create failed %d\n", error);
+ clk_put(clk);
+ return error;
+ }
+
+ error = pm_clk_add_clk(dev, clk);
+ if (error) {
+ dev_err(dev, "pm_clk_add_clk %pC failed %d\n", clk, error);
+ pm_clk_destroy(dev);
+ clk_put(clk);
+ }
+
+ return error;
+}
+
+int __init r9a06g032_attach_dev(struct generic_pm_domain *unused,
+ struct device *dev)
+{
+ struct r9a06g032_clk_domain *pd = r9a06g032_clk_domain;
+ struct device_node *np = dev->of_node;
+ struct of_phandle_args clkspec;
+ int i = 0;
+ int error;
+
+ if (!pd) {
+ dev_dbg(dev, "RZN1 clock domain not yet available\n");
+ return -EPROBE_DEFER;
+ }
+
+ while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
+ &clkspec)) {
+ int index = clkspec.args[0];
+
+ if (index < R9A06G032_CLOCK_COUNT &&
+ r9a06g032_clocks[index].managed) {
+ error = create_add_module_clock(&clkspec, dev);
+
+ if (error)
+ return error;
+
+ of_node_put(clkspec.np);
+ }
+ i++;
+ }
+
+ return 0;
+}
+
+void r9a06g032_detach_dev(struct generic_pm_domain *unused, struct device *dev)
+{
+ if (!pm_clk_no_clocks(dev))
+ pm_clk_destroy(dev);
+}
+
+static int __init r9a06g032_add_clk_domain(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ struct generic_pm_domain *genpd;
+ struct r9a06g032_clk_domain *pd;
+
+ pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+ if (!pd)
+ return -ENOMEM;
+
+ pd->np = np;
+
+ genpd = &pd->genpd;
+ genpd->name = np->name;
+ genpd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
+ genpd->attach_dev = r9a06g032_attach_dev;
+ genpd->detach_dev = r9a06g032_detach_dev;
+ pm_genpd_init(genpd, &pm_domain_always_on_gov, false);
+ r9a06g032_clk_domain = pd;
+
+ of_genpd_add_provider_simple(np, genpd);
+ return 0;
+}
+
static void
r9a06g032_clk_gate_set(struct r9a06g032_priv *clocks,
struct r9a06g032_gate *g, int on)
@@ -870,6 +975,10 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
if (error)
return error;
+ error = r9a06g032_add_clk_domain(dev);
+ if (error)
+ return error;
+
return devm_add_action_or_reset(dev,
r9a06g032_clocks_del_clk_provider, np);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] clk: renesas: r9a06g032: Add clock domain support
2019-05-21 12:35 [PATCH v2] clk: renesas: r9a06g032: Add clock domain support Gareth Williams
@ 2019-05-22 12:01 ` Geert Uytterhoeven
2019-05-24 12:28 ` Gareth Williams
0 siblings, 1 reply; 4+ messages in thread
From: Geert Uytterhoeven @ 2019-05-22 12:01 UTC (permalink / raw)
To: Gareth Williams
Cc: Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
Phil Edworthy, Linux-Renesas, linux-clk,
Linux Kernel Mailing List
Hi Gareth,
On Tue, May 21, 2019 at 2:35 PM Gareth Williams
<gareth.williams.jx@renesas.com> wrote:
> There are several clocks on the r9ag032 which are currently not enabled
> in their drivers that can be delegated to clock domain system for power
> management. Therefore add support for clock domain functionality to the
> r9a06g032 clock driver.
>
> Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> ---
> v2:
> - Rebased onto kernel/git/geert/renesas-drivers.git
Thanks for the update!
> drivers/clk/renesas/r9a06g032-clocks.c | 243 ++++++++++++++++++++++++---------
> 1 file changed, 176 insertions(+), 67 deletions(-)
Please also update
Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt,
to describe #power-domain-cells (must be 0), and to update the provider
and consumer examples.
> --- a/drivers/clk/renesas/r9a06g032-clocks.c
> +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> @@ -28,6 +30,7 @@ struct r9a06g032_gate {
> /* This is used to describe a clock for instantiation */
> struct r9a06g032_clkdesc {
> const char *name;
> + bool managed;
This flag can be integrated into the bit field below, avoiding a size
increase.
> uint32_t type: 3;
> uint32_t index: 8;
> uint32_t source : 8; /* source index + 1 (0 == none) */
> @@ -60,7 +63,11 @@ struct r9a06g032_clkdesc {
> #define D_GATE(_idx, _n, _src, ...) \
> { .type = K_GATE, .index = R9A06G032_##_idx, \
> .source = 1 + R9A06G032_##_src, .name = _n, \
> - .gate = I_GATE(__VA_ARGS__), }
> + .managed = 0, .gate = I_GATE(__VA_ARGS__) }
No need to initialize static variable fields to zero.
> +#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 }
> @@ -170,7 +177,7 @@ static const struct r9a06g032_clkdesc r9a06g032_clocks[] __initconst = {
> 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_GATE(CLK_PCI_USB, "clk_pci_usb", CLKOUT_D40, 0xe6, 0, 0, 0, 0, 0, 0),
> + D_MODULE(CLK_PCI_USB, "clk_pci_usb", CLKOUT_D40, 0xe6, 0, 0, 0, 0, 0, 0),
Is this correct? All other module clocks are HCLK_*?
You do have separate module clocks for USB host/function/power management
below?
> 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),
> @@ -187,17 +194,17 @@ static const struct r9a06g032_clkdesc r9a06g032_clocks[] __initconst = {
> 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_GATE(HCLK_ECAT125, "hclk_ecat125", CLKOUT_D8, 0x400, 0x401, 0, 0x402, 0, 0x440, 0x441),
> - D_GATE(HCLK_PINCONFIG, "hclk_pinconfig", CLKOUT_D40, 0x740, 0x741, 0x742, 0, 0xae0, 0, 0),
> - D_GATE(HCLK_SERCOS, "hclk_sercos", CLKOUT_D10, 0x420, 0x422, 0, 0x421, 0, 0x460, 0x461),
> - D_GATE(HCLK_SGPIO2, "hclk_sgpio2", DIV_P5_PG, 0x8c3, 0x8c4, 0x8c5, 0, 0xb41, 0, 0),
> - D_GATE(HCLK_SGPIO3, "hclk_sgpio3", DIV_P5_PG, 0x8c6, 0x8c7, 0x8c8, 0, 0xb42, 0, 0),
> - D_GATE(HCLK_SGPIO4, "hclk_sgpio4", DIV_P5_PG, 0x8c9, 0x8ca, 0x8cb, 0, 0xb43, 0, 0),
> - D_GATE(HCLK_TIMER0, "hclk_timer0", CLKOUT_D40, 0x743, 0x744, 0x745, 0, 0xae1, 0, 0),
> - D_GATE(HCLK_TIMER1, "hclk_timer1", CLKOUT_D40, 0x746, 0x747, 0x748, 0, 0xae2, 0, 0),
> - D_GATE(HCLK_USBF, "hclk_usbf", CLKOUT_D8, 0xe3, 0, 0, 0xe4, 0, 0x102, 0x103),
> - D_GATE(HCLK_USBH, "hclk_usbh", CLKOUT_D8, 0xe0, 0xe1, 0, 0xe2, 0, 0x100, 0x101),
> - D_GATE(HCLK_USBPM, "hclk_usbpm", CLKOUT_D8, 0xe5, 0, 0, 0, 0, 0, 0),
... here.
> + 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),
> @@ -344,6 +351,104 @@ struct r9a06g032_clk_gate {
>
> #define to_r9a06g032_gate(_hw) container_of(_hw, struct r9a06g032_clk_gate, hw)
>
> +struct r9a06g032_clk_domain {
> + struct generic_pm_domain genpd;
> + struct device_node *np;
np is unused (but see below), and unneeded, as nowadays you can use
generic_pm_domain.dev.of_node instead. Hence you can just use struct
generic_pm_domain directly, instead of wrapping it.
> +};
> +
> +static struct r9a06g032_clk_domain *r9a06g032_clk_domain;
I don't think this singleton is needed, as there's no interaction with a
second PM Domain driver (renesas-cpg-mssr interacts with rcar-sysc).
> +static int create_add_module_clock(struct of_phandle_args *clkspec,
> + struct device *dev)
> +{
> + struct clk *clk;
> + int error = 0;
> +
> + clk = of_clk_get_from_provider(clkspec);
> +
Please drop this blank line.
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + error = pm_clk_create(dev);
> + if (error) {
> + dev_err(dev, "pm_clk_create failed %d\n", error);
This can only happen when running out of memory, so no need to print a
message.
> + clk_put(clk);
> + return error;
> + }
> +
> + error = pm_clk_add_clk(dev, clk);
> + if (error) {
> + dev_err(dev, "pm_clk_add_clk %pC failed %d\n", clk, error);
Likewise, I think.
> + pm_clk_destroy(dev);
> + clk_put(clk);
> + }
> +
> + return error;
> +}
> +
> +int __init r9a06g032_attach_dev(struct generic_pm_domain *unused,
Missing static.
Please drop the __init, as devices can be attached anytime
(no section mismatch warnings?).
> + struct device *dev)
> +{
> + struct r9a06g032_clk_domain *pd = r9a06g032_clk_domain;
You can just use the passed generic_pm_domain instead...
> + struct device_node *np = dev->of_node;
> + struct of_phandle_args clkspec;
> + int i = 0;
> + int error;
> +
> + if (!pd) {
... and this check is no longer needed.
> + dev_dbg(dev, "RZN1 clock domain not yet available\n");
> + return -EPROBE_DEFER;
> + }
> +
> + while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
> + &clkspec)) {
As an entry in a clocks property may refer to a different type of clock
(e.g. an external crystal), you need to add a check here, to make sure
the clkspec refers to a clock handled by this driver:
if (clkspec->np != pd->genpd.dev.of_node)
continue
> + int index = clkspec.args[0];
> +
> + if (index < R9A06G032_CLOCK_COUNT &&
> + r9a06g032_clocks[index].managed) {
> + error = create_add_module_clock(&clkspec, dev);
> +
> + if (error)
> + return error;
> +
> + of_node_put(clkspec.np);
Please move this line up, before the error check, to avoid a reference leak.
> + }
> + i++;
> + }
> +
> + return 0;
> +}
> +
> +void r9a06g032_detach_dev(struct generic_pm_domain *unused, struct device *dev)
Missing static.
> +{
> + if (!pm_clk_no_clocks(dev))
> + pm_clk_destroy(dev);
> +}
> +
> +static int __init r9a06g032_add_clk_domain(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct generic_pm_domain *genpd;
> + struct r9a06g032_clk_domain *pd;
Just struct generic_pm_domain *pd, and s/genpd/pd/ below.
> +
> + pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> + if (!pd)
> + return -ENOMEM;
> +
> + pd->np = np;
Not needed.
> +
> + genpd = &pd->genpd;
Not needed.
> + genpd->name = np->name;
> + genpd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
> + genpd->attach_dev = r9a06g032_attach_dev;
> + genpd->detach_dev = r9a06g032_detach_dev;
> + pm_genpd_init(genpd, &pm_domain_always_on_gov, false);
> + r9a06g032_clk_domain = pd;
Not needed.
> +
> + of_genpd_add_provider_simple(np, genpd);
> + return 0;
> +}
> +
> static void
> r9a06g032_clk_gate_set(struct r9a06g032_priv *clocks,
> struct r9a06g032_gate *g, int on)
> @@ -870,6 +975,10 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
> if (error)
> return error;
>
> + error = r9a06g032_add_clk_domain(dev);
I would do this after the call to devm_add_action_or_reset() below.
> + if (error)
> + return error;
> +
> return devm_add_action_or_reset(dev,
> r9a06g032_clocks_del_clk_provider, np);
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH v2] clk: renesas: r9a06g032: Add clock domain support
2019-05-22 12:01 ` Geert Uytterhoeven
@ 2019-05-24 12:28 ` Gareth Williams
2019-05-24 12:39 ` Geert Uytterhoeven
0 siblings, 1 reply; 4+ messages in thread
From: Gareth Williams @ 2019-05-24 12:28 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
Phil Edworthy, Linux-Renesas, linux-clk,
Linux Kernel Mailing List
Hi Geert,
Thanks for the feedback. I added some additional information about the
logic I used for the USB clocks inline below and will send V3 shortly.
On Tue, May 22, 2019 at 1:02 PM Gareth Williams
<geert@linux-m68k.org> wrote:
> Hi Gareth,
>
> On Tue, May 21, 2019 at 2:35 PM Gareth Williams
> <gareth.williams.jx@renesas.com> wrote:
> > There are several clocks on the r9ag032 which are currently not
> > enabled in their drivers that can be delegated to clock domain system
> > for power management. Therefore add support for clock domain
> > functionality to the
> > r9a06g032 clock driver.
> >
> > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> > ---
> > v2:
> > - Rebased onto kernel/git/geert/renesas-drivers.git
>
> Thanks for the update!
>
> > drivers/clk/renesas/r9a06g032-clocks.c | 243
> > ++++++++++++++++++++++++---------
> > 1 file changed, 176 insertions(+), 67 deletions(-)
>
> Please also update
> Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt,
> to describe #power-domain-cells (must be 0), and to update the provider
> and consumer examples.
>
> > --- a/drivers/clk/renesas/r9a06g032-clocks.c
> > +++ b/drivers/clk/renesas/r9a06g032-clocks.c
>
> > @@ -28,6 +30,7 @@ struct r9a06g032_gate {
> > /* This is used to describe a clock for instantiation */ struct
> > r9a06g032_clkdesc {
> > const char *name;
> > + bool managed;
>
> This flag can be integrated into the bit field below, avoiding a size increase.
>
> > uint32_t type: 3;
> > uint32_t index: 8;
> > uint32_t source : 8; /* source index + 1 (0 == none) */ @@
> > -60,7 +63,11 @@ struct r9a06g032_clkdesc { #define D_GATE(_idx, _n,
> > _src, ...) \
> > { .type = K_GATE, .index = R9A06G032_##_idx, \
> > .source = 1 + R9A06G032_##_src, .name = _n, \
> > - .gate = I_GATE(__VA_ARGS__), }
> > + .managed = 0, .gate = I_GATE(__VA_ARGS__) }
>
> No need to initialize static variable fields to zero.
>
> > +#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 } @@ -170,7 +177,7 @@ static
> > const struct r9a06g032_clkdesc r9a06g032_clocks[] __initconst = {
> > 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_GATE(CLK_PCI_USB, "clk_pci_usb", CLKOUT_D40, 0xe6, 0, 0, 0, 0, 0,
> 0),
> > + D_MODULE(CLK_PCI_USB, "clk_pci_usb", CLKOUT_D40, 0xe6, 0, 0,
> > + 0, 0, 0, 0),
>
> Is this correct? All other module clocks are HCLK_*?
> You do have separate module clocks for USB host/function/power
> management below?
The naming is different here to keep consistency with the manuals,
however the USB PCI clock was added as a D_MODULE because it
does not need to be enabled at all times. CLK_PCI_USB only needs
to be enabled when the USB host is active, based on Figure 10.13
(Host Controller Power-Down Flow) in the RZN1 System Manual.
>
> > 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), @@ -187,17 +194,17 @@ static const struct r9a06g032_clkdesc
> r9a06g032_clocks[] __initconst = {
> > 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_GATE(HCLK_ECAT125, "hclk_ecat125", CLKOUT_D8, 0x400, 0x401, 0,
> 0x402, 0, 0x440, 0x441),
> > - D_GATE(HCLK_PINCONFIG, "hclk_pinconfig", CLKOUT_D40, 0x740,
> 0x741, 0x742, 0, 0xae0, 0, 0),
> > - D_GATE(HCLK_SERCOS, "hclk_sercos", CLKOUT_D10, 0x420, 0x422, 0,
> 0x421, 0, 0x460, 0x461),
> > - D_GATE(HCLK_SGPIO2, "hclk_sgpio2", DIV_P5_PG, 0x8c3, 0x8c4,
> 0x8c5, 0, 0xb41, 0, 0),
> > - D_GATE(HCLK_SGPIO3, "hclk_sgpio3", DIV_P5_PG, 0x8c6, 0x8c7,
> 0x8c8, 0, 0xb42, 0, 0),
> > - D_GATE(HCLK_SGPIO4, "hclk_sgpio4", DIV_P5_PG, 0x8c9, 0x8ca,
> 0x8cb, 0, 0xb43, 0, 0),
> > - D_GATE(HCLK_TIMER0, "hclk_timer0", CLKOUT_D40, 0x743, 0x744,
> 0x745, 0, 0xae1, 0, 0),
> > - D_GATE(HCLK_TIMER1, "hclk_timer1", CLKOUT_D40, 0x746, 0x747,
> 0x748, 0, 0xae2, 0, 0),
> > - D_GATE(HCLK_USBF, "hclk_usbf", CLKOUT_D8, 0xe3, 0, 0, 0xe4, 0,
> 0x102, 0x103),
> > - D_GATE(HCLK_USBH, "hclk_usbh", CLKOUT_D8, 0xe0, 0xe1, 0, 0xe2, 0,
> 0x100, 0x101),
> > - D_GATE(HCLK_USBPM, "hclk_usbpm", CLKOUT_D8, 0xe5, 0, 0, 0, 0, 0,
> 0),
>
> ... here.
HCLK_USBPM needs to be enabled at all times because it is used for VBUS detection
and wakeup for both the USB host and function. Therefore D_GATE was used.
USB_HCLKF is a bus clock used by the USB function, although it can technically be
turned off whilst waiting for VBUS detection (system manual:
"10.6.6.2 VBUS Detection"), our USB function driver does not currently implement
PM. I will update this to a module in the next version as I see no issue doing
so conceptually or through testing.
>
> > + 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),
>
> > @@ -344,6 +351,104 @@ struct r9a06g032_clk_gate {
> >
> > #define to_r9a06g032_gate(_hw) container_of(_hw, struct
> > r9a06g032_clk_gate, hw)
> >
> > +struct r9a06g032_clk_domain {
> > + struct generic_pm_domain genpd;
> > + struct device_node *np;
>
> np is unused (but see below), and unneeded, as nowadays you can use
> generic_pm_domain.dev.of_node instead. Hence you can just use struct
> generic_pm_domain directly, instead of wrapping it.
>
> > +};
> > +
> > +static struct r9a06g032_clk_domain *r9a06g032_clk_domain;
>
> I don't think this singleton is needed, as there's no interaction with a second
> PM Domain driver (renesas-cpg-mssr interacts with rcar-sysc).
>
> > +static int create_add_module_clock(struct of_phandle_args *clkspec,
> > + struct device *dev) {
> > + struct clk *clk;
> > + int error = 0;
> > +
> > + clk = of_clk_get_from_provider(clkspec);
> > +
>
> Please drop this blank line.
>
> > + if (IS_ERR(clk))
> > + return PTR_ERR(clk);
> > +
> > + error = pm_clk_create(dev);
> > + if (error) {
> > + dev_err(dev, "pm_clk_create failed %d\n", error);
>
> This can only happen when running out of memory, so no need to print a
> message.
>
> > + clk_put(clk);
> > + return error;
> > + }
> > +
> > + error = pm_clk_add_clk(dev, clk);
> > + if (error) {
> > + dev_err(dev, "pm_clk_add_clk %pC failed %d\n", clk,
> > + error);
>
> Likewise, I think.
>
> > + pm_clk_destroy(dev);
> > + clk_put(clk);
> > + }
> > +
> > + return error;
> > +}
> > +
> > +int __init r9a06g032_attach_dev(struct generic_pm_domain *unused,
>
> Missing static.
> Please drop the __init, as devices can be attached anytime (no section
> mismatch warnings?).
Because the clock array used __initconst, I only got a section mismatch warning
without __init in the attach function. I will remove both so it compiles cleanly
without expecting devices to be attached at one point.
>
> > + struct device *dev) {
> > + struct r9a06g032_clk_domain *pd = r9a06g032_clk_domain;
>
> You can just use the passed generic_pm_domain instead...
>
> > + struct device_node *np = dev->of_node;
> > + struct of_phandle_args clkspec;
> > + int i = 0;
> > + int error;
> > +
> > + if (!pd) {
>
> ... and this check is no longer needed.
>
> > + dev_dbg(dev, "RZN1 clock domain not yet available\n");
> > + return -EPROBE_DEFER;
>
>
> > + }
> > +
> > + while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
> > + &clkspec)) {
>
> As an entry in a clocks property may refer to a different type of clock (e.g. an
> external crystal), you need to add a check here, to make sure the clkspec
> refers to a clock handled by this driver:
>
> if (clkspec->np != pd->genpd.dev.of_node)
> continue
>
> > + int index = clkspec.args[0];
> > +
> > + if (index < R9A06G032_CLOCK_COUNT &&
> > + r9a06g032_clocks[index].managed) {
> > + error = create_add_module_clock(&clkspec,
> > + dev);
> > +
> > + if (error)
> > + return error;
> > +
> > + of_node_put(clkspec.np);
>
> Please move this line up, before the error check, to avoid a reference leak.
>
> > + }
> > + i++;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +void r9a06g032_detach_dev(struct generic_pm_domain *unused, struct
> > +device *dev)
>
> Missing static.
>
> > +{
> > + if (!pm_clk_no_clocks(dev))
> > + pm_clk_destroy(dev);
> > +}
> > +
> > +static int __init r9a06g032_add_clk_domain(struct device *dev) {
> > + struct device_node *np = dev->of_node;
> > + struct generic_pm_domain *genpd;
> > + struct r9a06g032_clk_domain *pd;
>
> Just struct generic_pm_domain *pd, and s/genpd/pd/ below.
>
> > +
> > + pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> > + if (!pd)
> > + return -ENOMEM;
> > +
> > + pd->np = np;
>
> Not needed.
>
> > +
> > + genpd = &pd->genpd;
>
> Not needed.
>
> > + genpd->name = np->name;
> > + genpd->flags = GENPD_FLAG_PM_CLK |
> GENPD_FLAG_ACTIVE_WAKEUP;
> > + genpd->attach_dev = r9a06g032_attach_dev;
> > + genpd->detach_dev = r9a06g032_detach_dev;
> > + pm_genpd_init(genpd, &pm_domain_always_on_gov, false);
> > + r9a06g032_clk_domain = pd;
>
> Not needed.
>
> > +
> > + of_genpd_add_provider_simple(np, genpd);
> > + return 0;
> > +}
> > +
> > static void
> > r9a06g032_clk_gate_set(struct r9a06g032_priv *clocks,
> > struct r9a06g032_gate *g, int on) @@ -870,6
> > +975,10 @@ static int __init r9a06g032_clocks_probe(struct
> platform_device *pdev)
> > if (error)
> > return error;
> >
> > + error = r9a06g032_add_clk_domain(dev);
>
> I would do this after the call to devm_add_action_or_reset() below.
>
> > + if (error)
> > + return error;
> > +
> > return devm_add_action_or_reset(dev,
> >
> > r9a06g032_clocks_del_clk_provider, np); }
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Kind Regards,
Gareth Williams
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] clk: renesas: r9a06g032: Add clock domain support
2019-05-24 12:28 ` Gareth Williams
@ 2019-05-24 12:39 ` Geert Uytterhoeven
0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2019-05-24 12:39 UTC (permalink / raw)
To: Gareth Williams
Cc: Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
Phil Edworthy, Linux-Renesas, linux-clk,
Linux Kernel Mailing List
Hi Gareth,
On Fri, May 24, 2019 at 2:28 PM Gareth Williams
<gareth.williams.jx@renesas.com> wrote:
> On Tue, May 22, 2019 at 1:02 PM Gareth Williams
> <geert@linux-m68k.org> wrote:
> > On Tue, May 21, 2019 at 2:35 PM Gareth Williams
> > <gareth.williams.jx@renesas.com> wrote:
> > > There are several clocks on the r9ag032 which are currently not
> > > enabled in their drivers that can be delegated to clock domain system
> > > for power management. Therefore add support for clock domain
> > > functionality to the
> > > r9a06g032 clock driver.
> > >
> > > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> > > ---
> > > v2:
> > > - Rebased onto kernel/git/geert/renesas-drivers.git
> >
> > Thanks for the update!
> >
> > > drivers/clk/renesas/r9a06g032-clocks.c | 243
> > > ++++++++++++++++++++++++---------
> > > 1 file changed, 176 insertions(+), 67 deletions(-)
> >
> > Please also update
> > Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt,
> > to describe #power-domain-cells (must be 0), and to update the provider
> > and consumer examples.
> >
> > > --- a/drivers/clk/renesas/r9a06g032-clocks.c
> > > +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> > > +int __init r9a06g032_attach_dev(struct generic_pm_domain *unused,
> >
> > Missing static.
> > Please drop the __init, as devices can be attached anytime (no section
> > mismatch warnings?).
> Because the clock array used __initconst, I only got a section mismatch warning
> without __init in the attach function. I will remove both so it compiles cleanly
> without expecting devices to be attached at one point.
Oh right, r9a06g032_attach_dev() uses r9a06g032_clocks[].
So the __initconst must be indeed dropped from the latter, unless you find some
way to store the managed flag elsewhere.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-24 12:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-21 12:35 [PATCH v2] clk: renesas: r9a06g032: Add clock domain support Gareth Williams
2019-05-22 12:01 ` Geert Uytterhoeven
2019-05-24 12:28 ` Gareth Williams
2019-05-24 12:39 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).