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=-12.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_2 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 2F86CC48BE0 for ; Fri, 11 Jun 2021 08:21:37 +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 0B8F16008E for ; Fri, 11 Jun 2021 08:21:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0B8F16008E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de 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 C646782CBE; Fri, 11 Jun 2021 10:21:33 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1623399694; bh=EvmkX/9k98MFawmlJzeQSS0SVLYvef9WLpaYvsQV/Nw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=a/ObNRavD01m6z6taAFWlIMzPZN/GzCqc4YLor3MJ7z8tpN6Gf3Me3mNMiDLZ30Sh esLYd2s7YQUdKiGodTWmXA9VFGSwFhfwwpItdIASmtYznhYYoM8TdP55+NI+mW5SRw h7AsubEW4yl4BeR273UIgtfJ/nndJlNqgsUNdnT0pFcd7zPqUjsi77SfRbCVDBTC/0 A8EBCwK1N0gRc9QmA/bQlyXjsMIZDUgiNnvdc3n8aqQ6gQDNnAebLNkjZI+uBFhFou 0uB83lckk/VZ4xJiF75qrcwooaMOxRcA8u8HJQlZsWAhmVxU8zh5LzzeMLYHTNfagh vJB0hYUBEB4DA== Received: from ktm (85-222-111-42.dynamic.chello.pl [85.222.111.42]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: lukma@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 94E6382AB1; Fri, 11 Jun 2021 10:21:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1623399692; bh=EvmkX/9k98MFawmlJzeQSS0SVLYvef9WLpaYvsQV/Nw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=H8GO0Uz63WfJ6dcOvPiPy1MrL5fmYCjEgvVGf9lohBk6QUBgTaGYF4lrIMFwjySBm gWfy1yuWlHa5mjDhTat1MzA7s8h/EqQUa09V/Xz5EChPTQU1nqWE31r++U1f0E7z40 dA2D+9NfcBkoyCz5gzKzOE/h6ed2FBUMF6uMoJ4Ho397Ge7Ldw38Grsdjat54Po+ig LaIsaXFVUwYEUmF4WPX4JWIg8ApbxE8T7KT9+oKpcxLNw11ljNiLt7epppep708khK RNLYhHy9vpuq57T9FlFxYSn5Un8FTVevYV+0NGoS8ciyZ/m81HsYUslRAOwpzndkac ETkR5Ai8oRCog== Date: Fri, 11 Jun 2021 10:21:24 +0200 From: Lukasz Majewski To: Sean Anderson Cc: u-boot@lists.denx.de, Damien Le Moal , Leo Liang , Andreas Dannenberg , Lokesh Vutla , Philipp Tomsich Subject: Re: [PATCH v3 00/11] clk: k210: Rewrite K210 clock without CCF Message-ID: <20210611102124.4f95c62f@ktm> In-Reply-To: <20210611041617.1665833-1-seanga2@gmail.com> References: <20210611041617.1665833-1-seanga2@gmail.com> Organization: denx.de X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; boundary="Sig_/C_qSYO71gD4.+IWutGZHUXj"; protocol="application/pgp-signature" 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.102.4 at phobos.denx.de X-Virus-Status: Clean --Sig_/C_qSYO71gD4.+IWutGZHUXj Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Fri, 11 Jun 2021 00:16:06 -0400 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: >=20 > * 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. >=20 > 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. You mean for U-Boot or for Linux ? >=20 > 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 >=20 I'm not going to defend CCF to the last breath, I know their issues. However, the goal for CCF was to have: 1. Ported code from Linux (with some code simplification) 2. Get the standard approach to the clock subsystem. I'm just wondering if we (as a community) want to have such diversity - I mean each architecture would have different clock driver (under the device model) As fair as I can see - the K210 already has support for CCF in Linux: drivers/clk/clk-k210.c=20 Reading the above comment - it looks like you couldn't simplyfy this Linux driver to be smaller and fit into U-Boot? Sean, do you think that other archs can benefit from your work? > This series depends on > https://patchwork.ozlabs.org/project/uboot/list/?series=3D238211 >=20 > 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 >=20 > Changes in v2: > - Convert stage to enum > - Only force probe clocks pre-reloc > - Rebase on u-boot/master >=20 > 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 >=20 > 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 >=20 Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de --Sig_/C_qSYO71gD4.+IWutGZHUXj Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEgAyFJ+N6uu6+XupJAR8vZIA0zr0FAmDDHQQACgkQAR8vZIA0 zr3Z5gf+L3zZG9YIAlymOHP3pwuyJcvKKetYSPCBNvbMCaXIOIeh0rINSb9fzjA+ 5iDOYroygqco+AdozGb2xk8NXqwq2DwNMiqS5NqF2OCq/xo3rUL4+j/ZKCTFoN0w PXrmydcVwJU3jYylnwEEOgI3RkzQLmIcCPDKv9OZQiMPTSy/K2J8750vlSkYNfva AdE8GaQvO7lmdq9a9l9DPLiogNonzcWe4wYmjP8C3d3zVaxHFTpLPGGBD2NtZkT4 8iPNKMzD0wvv//s3hdUOyBYTgNQZE0U2zZMuV5g5cnaAwgHsvS8gu/1l48TwaQXR Dg2qS654RQX/g8+hNiY88zXWN7zRPw== =O7uF -----END PGP SIGNATURE----- --Sig_/C_qSYO71gD4.+IWutGZHUXj--