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=-16.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED 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 DEA2BC4338F for ; Mon, 26 Jul 2021 14:07:04 +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 2566160F42 for ; Mon, 26 Jul 2021 14:07:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2566160F42 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1ED1283358; Mon, 26 Jul 2021 16:07:02 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="BkaP5A/g"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 015F283377; Mon, 26 Jul 2021 16:06:49 +0200 (CEST) Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) (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 E119583362 for ; Mon, 26 Jul 2021 16:06:39 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-wr1-x436.google.com with SMTP id r2so11291168wrl.1 for ; Mon, 26 Jul 2021 07:06:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=wW2wdGIZHfz57NvARizjqg6v6DCAqJVpP6FKePYezvM=; b=BkaP5A/gap/X4p/lf9w8IfmsyaPq6HZMZhWfTL6mdLxaEJxUmKqVzOyDhvSjknLXcf wF5ds5gCiGtxKzxnahP98iW9t7zl8MP6e4QVFNL9cAOcmqJGyv0TICD3ilSFY1TgSV4n iL5PMpzV3CR/3VyYShosuzb6fimyT37ivqRDk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=wW2wdGIZHfz57NvARizjqg6v6DCAqJVpP6FKePYezvM=; b=p7wZ1ldt7QuHxi7IjgifVtCR++da7q3mAGpGjAUDIV8/lC7OMIj3ceZ+yQFKqMh7CR h1flEb2oF1ZYkDkdKrPe0hbBI/XjWGmWARbHWxivcjtX1R+6QyKkAx7O56AA74FKqcVr jIaYb2PzssueoB5+z4tDpsUMF+PnsKBIAFS3UdCGTVyWtr4BmQcVXvorLdjJ/k+t8Pty Hn8xJQ7+hPa9IL/A3xZfj5mdJZp0JoopB70LrnDdHFEjEAgVv3FE0z78UMm0aIRX3X6U 25TIMQqOluLv9rKxL0OOa8Gxa0KHmm2a1w3cyHbDqt94YIy/becq9l3MNH069DI8GKcU +HSw== X-Gm-Message-State: AOAM530y7QQqgV1P+bg6rmID2UCPvHXjrAoDx5CzuG0dQiFAaaf3X+hE tTm8VDcTqvrzdvJtB5P7L4NyM3XfJkjCaKkCabUd4Q== X-Google-Smtp-Source: ABdhPJx8GgAL73oK39T5hFhLJVH2838jUvvIClNkM5Kbzda5Vc4SYtIVIMS/Jg4khviA8h1WE3f/WV3//O1XOHLesPA= X-Received: by 2002:adf:e409:: with SMTP id g9mr20531983wrm.66.1627308399212; Mon, 26 Jul 2021 07:06:39 -0700 (PDT) MIME-Version: 1.0 References: <20210713185900.59692-1-paweljarosz3691@gmail.com> <20210713185900.59692-3-paweljarosz3691@gmail.com> <5b9de505-a0cf-1ba5-6fad-270a9f353620@gmail.com> In-Reply-To: <5b9de505-a0cf-1ba5-6fad-270a9f353620@gmail.com> From: Simon Glass Date: Mon, 26 Jul 2021 08:06:26 -0600 Message-ID: Subject: Re: [PATCH v6 2/4] rockchip: rk3066: add clock driver for rk3066 soc To: =?UTF-8?Q?Pawe=C5=82_Jarosz?= Cc: Kever Yang , Philipp Tomsich , Lukasz Majewski , U-Boot Mailing List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Hi Pawe=C5=82, On Sun, 25 Jul 2021 at 08:15, Pawe=C5=82 Jarosz = wrote: > > Hi Simon, > > > sorry for late response i was offline a bit > > W dniu 13.07.2021 o 22:17, Simon Glass pisze: > > Hi Pawe=C5=82, > > > > On Tue, 13 Jul 2021 at 12:59, Pawe=C5=82 Jarosz wrote: > >> Add clock driver for rk3066 platform. > >> > >> Signed-off-by: Pawe=C5=82 Jarosz > >> Acked-by: Philipp Tomsich > >> --- > >> > >> Changes since v1: > >> - updated to shifted masks > >> - moved clk init to tpl > >> > >> Changes since v2: > >> - none > >> > >> Changes since v3: > >> - none > >> > >> Changes since v4: > >> - updated to current codebase > >> - fixed compilation errors > >> > >> Changes since v5: > >> - various style changes > >> - added clk_enable/clk_disable support for nand and mmc clocks > >> - updated maintainer email > >> - renamed uint32_t to u32 > >> - used #if IS_ENABLED macro instead #ifdef > >> > >> > >> > >> .../include/asm/arch-rockchip/cru_rk3066.h | 203 +++++ > >> drivers/clk/rockchip/Makefile | 1 + > >> drivers/clk/rockchip/clk_rk3066.c | 704 +++++++++++++++++= + > >> 3 files changed, 908 insertions(+) > >> create mode 100644 arch/arm/include/asm/arch-rockchip/cru_rk3066.h > >> create mode 100644 drivers/clk/rockchip/clk_rk3066.c > >> > > [..] > > > >> + > >> +static int rk3066_clk_of_to_plat(struct udevice *dev) > >> +{ > >> +#if !CONFIG_IS_ENABLED(OF_PLATDATA) > >> + struct rk3066_clk_priv *priv =3D dev_get_priv(dev); > >> + > >> + priv->cru =3D dev_read_addr_ptr(dev); > >> +#endif > >> + > >> + return 0; > >> +} > >> + > >> +static int rk3066_clk_probe(struct udevice *dev) > >> +{ > >> + struct rk3066_clk_priv *priv =3D dev_get_priv(dev); > >> + > >> + priv->grf =3D syscon_get_first_range(ROCKCHIP_SYSCON_GRF); > >> + if (IS_ERR(priv->grf)) > >> + return PTR_ERR(priv->grf); > >> + > >> +#if IS_ENABLED(CONFIG_TPL_BUILD) > > Do you need that? The line below should take care of it. > > > Yep. Later rkclk_init and rkclk_configure_cpu should be only executed in = TPL. But CONFIG_IS_ENABLED(OF_PLATDATA) should be enough to check that, right, since it is only true in TPL? You the TPL check seems to be redundant. > > > >> +#if CONFIG_IS_ENABLED(OF_PLATDATA) > >> + struct rk3066_clk_plat *plat =3D dev_get_plat(dev); > >> + > >> + priv->cru =3D map_sysmem(plat->dtd.reg[0], plat->dtd.reg[1]); > >> +#endif > >> + > >> + rkclk_init(priv->cru, priv->grf); > >> + > >> + /* Init CPU frequency */ > >> + rkclk_configure_cpu(priv->cru, priv->grf, APLL_SAFE_HZ); > >> +#endif > >> + > >> + return 0; > >> +} > >> + > >> +static int rk3066_clk_bind(struct udevice *dev) > >> +{ > >> + int ret; > >> + struct udevice *sys_child; > >> + struct sysreset_reg *priv; > >> + > >> + /* The reset driver does not have a device node, so bind it he= re */ > >> + ret =3D device_bind_driver(dev, "rockchip_sysreset", "sysreset= ", > >> + &sys_child); > >> + if (ret) { > >> + debug("Warning: No sysreset driver: ret=3D%d\n", ret); > >> + } else { > >> + priv =3D malloc(sizeof(struct sysreset_reg)); > >> + priv->glb_srst_fst_value =3D offsetof(struct rk3066_cr= u, > >> + cru_glb_srst_fst_v= alue); > >> + priv->glb_srst_snd_value =3D offsetof(struct rk3066_cr= u, > >> + cru_glb_srst_snd_v= alue); > >> + dev_set_priv(sys_child, priv); > >> + } > >> + > >> +#if CONFIG_IS_ENABLED(RESET_ROCKCHIP) > > Can you use if() instead of #if ? > > > Yes, but what is the difference? Sorry ... I don't know what to ask googl= e to get the answer. > > Is it in this case doing the same thing and just looking better? We are avoiding the preprocessor these days in favour of compile-time checks, since it increases build coverage and makes the code easier to read (there are many other things here). Also patman should warn you about this? Regards, Simon