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 X-Spam-Level: X-Spam-Status: No, score=-9.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DAF1DC48BE0 for ; Fri, 11 Jun 2021 23:14:33 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 94F70613DF for ; Fri, 11 Jun 2021 23:14:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 94F70613DF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 0811380488; Sat, 12 Jun 2021 01:14: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="LVAzrRA7"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 4494380797; Sat, 12 Jun 2021 01:14:27 +0200 (CEST) Received: from mail-qk1-x72a.google.com (mail-qk1-x72a.google.com [IPv6:2607:f8b0:4864:20::72a]) (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 BBCA480488 for ; Sat, 12 Jun 2021 01:14:23 +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-x72a.google.com with SMTP id c18so17613216qkc.11 for ; Fri, 11 Jun 2021 16:14:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=lglFZzQW3rQlyXY6r43fO5ho2xrPvCQp2feYBs6r6g0=; b=LVAzrRA7yz3qxIS3k4t4JPDyeEqAM9TyvQDlSL1rBx3Szm4F9Gm7KjH9mgyrLhNc6b +CuVpdGUcNZe02i/y5hoynzOnLzWvF1aEQRvOolVQ8+2Ck8bqHtyU9bIr1VCDnS8Wh5D TxtkqJK/JFHbTSUPwiYzjE6L3P5zrDD/p+XkVhwvTBTAp0ffwWGpqDc8p+1XnpSSZldG ldZ1KoyJ5s4eWplgetQz0Qkm7oCWs5ZjL/duwxmXJyCBHe4LKXKcwviWAhdvhjTyFMbI iwh5VwyCK0yRS7UwIt8zF/Ssu5vYawqAjp5LMKAqZNvjZE1y/slP5+R26YokJsvWWfw2 nfqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=lglFZzQW3rQlyXY6r43fO5ho2xrPvCQp2feYBs6r6g0=; b=Zh26BV2k02zhZCNGjE98PAT9TWmq5HOXHE9zkTvSw5Iii9e4oEEIFWRXn8va1l/ppj 7kzt3Ld+EBGrgKCMn0lycIAUaErv2ofQGp6Np8bNXureiM20KWQVmmB/wZkPXy5aBjH0 MKeO9wI0ax0qHvxdjJ6ki3Ka/gfGq+oda2hzsI4InOm5Qx0wctGiT1gaatf819CKC/mE mcZRoysjobkk5w1nkPoW/UL7IhzOcFfqf9DwQ0GYTZVm6mrYqLCXNtKX5dKUGMah/ig/ dgcD9reguq488oVtASbd+3+XmK7YJ2+kIelIGejx+SS8MY1EV8Kc3xzVgUJytZ9IYk5a lifQ== X-Gm-Message-State: AOAM532MkW2RAKKlJ/PNjMWOfbSfyYS/r+dxnwvIJtv6Gcf9HnWxTIwU j6r/yaZvw2nLcMvt2FmhW9Q= X-Google-Smtp-Source: ABdhPJx6Eo/UeosmbWIacxynYSPQOGGzb5Vp0V11N747cnTXWOCv3VUyq01tLnkMx3rbj8zx/UB4eA== X-Received: by 2002:a05:620a:6b3:: with SMTP id i19mr6387671qkh.55.1623453262428; Fri, 11 Jun 2021 16:14:22 -0700 (PDT) Received: from [192.168.1.201] (pool-74-96-87-9.washdc.fios.verizon.net. [74.96.87.9]) by smtp.googlemail.com with ESMTPSA id x15sm5487302qkh.19.2021.06.11.16.14.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 11 Jun 2021 16:14:22 -0700 (PDT) Subject: Re: [PATCH v3 00/11] clk: k210: Rewrite K210 clock without CCF To: u-boot@lists.denx.de Cc: Damien Le Moal , Leo Liang , Lukasz Majewski , Andreas Dannenberg , Lokesh Vutla , Philipp Tomsich References: <20210611041617.1665833-1-seanga2@gmail.com> From: Sean Anderson Message-ID: <7d593061-2063-0741-dfe7-2aeb8a5c9d2e@gmail.com> Date: Fri, 11 Jun 2021 19:14:20 -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: <20210611041617.1665833-1-seanga2@gmail.com> 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.34 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.2 at phobos.denx.de X-Virus-Status: Clean On 6/11/21 12:16 AM, Sean Anderson wrote: > This is something I've been meaning to do for a while but only just got around > to. The CCF has been quite unwieldy in a few ways: > > * It is very rigid, and there are not easy ways to hook into it without > rewriting many things. See e.g. things like the bypass clock and all the _half > clocks which were created because CCF didn't support the dividers used on the > k210. While preparing this series, I encountered several edge cases which I > had initially overlooked (or which were not supported in the initial release). > These would have been very difficult to fix with CCF, but were much easier to > address because each funcion is implemented in one place. > * There is a lot of magic going on under the curtains because of all the CCF > code which lives in many different files. Some things live in drivers, but > many things live in e.g. clk-uclass.c. So many things live in so many files > and it can be very difficult to get a handle on what exactly happens. > Complicating this is that there is a conflation of struct clk as a handle and > struct clk as a device. In this regard, refcounting is completely broken. IMO > we should just do away with refcounts and only disable clocks when explicitly > asked for. > * It is very dependent on runtime initialization. Typically, everything is > initialized by calling into various register() functions, usually with several > wrappers to avoid specifying all the arguments. This balloons the runtime > memory usage since there are so many devices created. It also makes it hard to > debug, since if you do it the "typical" way it is easy to accidentally assign > a clock to the wrong register. > * It inflates code size by pulling in not just some dead code (e.g. this driver > does not use divider tables but they are in clk-divider anyway) but also > pulling in numerous imx-specific clocks. This could be fixed, but I don't want > to due to the other reasons listed. > > I am very happy to have completely excised it from my driver. IMO there should > be big warning signs on the CCF warning not to use it for new code. This would > hopefully dissuade those like myself who had no idea that CCF was *not* in fact > a good way to write a clock driver. > > Overall there is a total savings of 12k from this series. > text data bss dec hex filename > 292485 32672 12624 337781 52775 before/u-boot > 283125 29856 12624 325605 4f7e5 after/u-boot > > This series depends on > https://patchwork.ozlabs.org/project/uboot/list/?series=238211 > > Changes in v3: > - Always define clk_defaults_stage, even if clk_set_defaults is a dummy > - Fix inverted condition for setting defaults > - Fix val not being set for K210_DIV_POWER > - Add CLK_K210_SET_RATE to defconfig so these changes apply > > Changes in v2: > - Convert stage to enum > - Only force probe clocks pre-reloc > - Rebase on u-boot/master > > Sean Anderson (11): > clk: Allow force setting clock defaults before relocation > clk: k210: Rewrite to remove CCF > clk: k210: Move pll into the rest of the driver > clk: k210: Implement soc_clk_dump > clk: k210: Re-add support for setting rate > clk: k210: Don't set PLL rates if we are already at the correct rate > clk: k210: Remove bypass driver > clk: k210: Move k210 clock out of its own subdirectory > k210: dts: Set PLL1 to the same rate as PLL0 > k210: Don't imply CCF > test: Add K210 PLL tests to sandbox defconfigs > > MAINTAINERS | 4 +- > arch/riscv/dts/k210.dtsi | 2 + > board/sipeed/maix/Kconfig | 2 - > configs/sandbox64_defconfig | 2 + > configs/sandbox_defconfig | 2 + > configs/sandbox_flattree_defconfig | 2 + > configs/sipeed_maix_bitm_defconfig | 2 +- > drivers/clk/Kconfig | 14 +- > drivers/clk/Makefile | 2 +- > drivers/clk/clk-uclass.c | 27 +- > drivers/clk/clk_kendryte.c | 1320 +++++++++++++++++++++++ > drivers/clk/kendryte/Kconfig | 12 - > drivers/clk/kendryte/Makefile | 1 - > drivers/clk/kendryte/bypass.c | 273 ----- > drivers/clk/kendryte/clk.c | 668 ------------ > drivers/clk/kendryte/pll.c | 585 ---------- > drivers/clk/rockchip/clk_rk3308.c | 2 +- > drivers/core/device.c | 2 +- > drivers/net/gmac_rockchip.c | 2 +- > include/clk.h | 30 +- > include/dt-bindings/clock/k210-sysctl.h | 94 +- > include/kendryte/bypass.h | 31 - > include/kendryte/clk.h | 35 - > include/kendryte/pll.h | 34 - > 24 files changed, 1437 insertions(+), 1711 deletions(-) > create mode 100644 drivers/clk/clk_kendryte.c > delete mode 100644 drivers/clk/kendryte/Kconfig > delete mode 100644 drivers/clk/kendryte/Makefile > delete mode 100644 drivers/clk/kendryte/bypass.c > delete mode 100644 drivers/clk/kendryte/clk.c > delete mode 100644 drivers/clk/kendryte/pll.c > delete mode 100644 include/kendryte/bypass.h > delete mode 100644 include/kendryte/clk.h > passing CI: https://dev.azure.com/seanga2/u-boot/_build/results?buildId=70&view=results