From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1E677C25B08 for ; Sat, 13 Aug 2022 05:30:31 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 24F0B849B7; Sat, 13 Aug 2022 07:30:29 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="et1osc43"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id AB35084B10; Sat, 13 Aug 2022 07:30:27 +0200 (CEST) Received: from mail-qk1-x72e.google.com (mail-qk1-x72e.google.com [IPv6:2607:f8b0:4864:20::72e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 45AEE84944 for ; Sat, 13 Aug 2022 07:30:22 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=seanga2@gmail.com Received: by mail-qk1-x72e.google.com with SMTP id i24so2219796qkg.13 for ; Fri, 12 Aug 2022 22:30:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject:from:to:cc; bh=U7qbc//2A/JI4B7qDT9YWpm/HyoKavCXtqLu2Rh7vQc=; b=et1osc43dHS7E9SBUN94PQ4VN4QyNNkkMAhLEfzNPMhvsXXWHkL3po+dEw+Iynn6Ul 4RToiNrHT4ckOU5Ia7hnuFVSrMBqe/hCSDzML9XkyBdPXHogrOhT3y39GYZzJ0820edO PxfzL73RL2SegyriK3v+iCv5wdEYymYUzUUTEMrnvrV6ov/byeHqJWyaghAsA3JAKPFo r7HymyozV3poi/8/z2DPhWVzKd1/k/jY65vBiCYeOwq0CiXFF1P6i2d4+/CQJ/lhKBpg sxyB6q/s+9QfW3puYqTB0t+CGq4X2IVWh3PgCzFPD8JNg0/Z8/0hjMOK924RJoqC9ndh X0iw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc; bh=U7qbc//2A/JI4B7qDT9YWpm/HyoKavCXtqLu2Rh7vQc=; b=P3Xzfj1IfMvg/q9vYgLoBtNNOQF/s21v84l7Y8tOGrKqpwddyVET44mkE8ZvEjqCR9 F7lwauKfS2J5W3cq+k/GhTCJqrwg5yK3C7lNEeQlwsdmw+e7YtJ4YHyJsxcDDOp51UPh XaoMSmt98s8cwomkeoxyY3wh0KzP4OXZepWiyo/uz8jCqACCeCxDhhzeR5TktSSPSTJv vTUsZEZH+74fTX8SaI7F/FpepFbYzBfxdTZ22DVwgqXr6ZZ5GvlsoQkIx0QhxCm9CB6E /f9HA2NrVcVDn09p3W6AMucfUg29x6CRfMYjmTApIyithF7tOujL1UFB9aMX0cNGVrEj jFtA== X-Gm-Message-State: ACgBeo3weGyapFs+knNQ81UvR0haaySNKM9ru/oces4PNxN1uIJcgezD OghobFU2bW3DReBEBu1jXNAprBqywrE= X-Google-Smtp-Source: AA6agR72lXMUCU2au5u+3DGd6IQGq4akanEg0tcJWMKvFoDo9QInJ94MQ7OCZmmQCHipemkdcqtPiw== X-Received: by 2002:a05:620a:4050:b0:6b5:d2bf:f570 with SMTP id i16-20020a05620a405000b006b5d2bff570mr5051436qko.421.1660368620676; Fri, 12 Aug 2022 22:30:20 -0700 (PDT) Received: from [192.168.1.201] (pool-173-73-95-180.washdc.fios.verizon.net. [173.73.95.180]) by smtp.gmail.com with ESMTPSA id s16-20020a05620a255000b006b5f06186aesm3146879qko.65.2022.08.12.22.30.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 12 Aug 2022 22:30:20 -0700 (PDT) Subject: Re: [RFC PATCH v1 3/9] clk: renesas: add R906G032 driver To: Ralph Siemsen , u-boot@lists.denx.de Cc: Lukasz Majewski References: <20220809125959.217333-1-ralph.siemsen@linaro.org> <20220809125959.217333-4-ralph.siemsen@linaro.org> From: Sean Anderson Message-ID: <152abc91-7460-1a4d-063a-136b4b5f0d4b@gmail.com> Date: Sat, 13 Aug 2022 01:30:19 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20220809125959.217333-4-ralph.siemsen@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean 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 > --- > - 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 , > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +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