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=-15.8 required=3.0 tests=BAYES_00,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 795CEC4338F for ; Wed, 28 Jul 2021 10:41:20 +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 AA6A360C3F for ; Wed, 28 Jul 2021 10:41:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org AA6A360C3F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sifive.com 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 D3200829C6; Wed, 28 Jul 2021 12:41:17 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=sifive.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=sifive.com header.i=@sifive.com header.b="Sp7OyYiV"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8962182BD7; Wed, 28 Jul 2021 12:41:15 +0200 (CEST) Received: from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com [IPv6:2a00:1450:4864:20::12a]) (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 3421C82956 for ; Wed, 28 Jul 2021 12:41:12 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=sifive.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=zong.li@sifive.com Received: by mail-lf1-x12a.google.com with SMTP id u3so3009850lff.9 for ; Wed, 28 Jul 2021 03:41:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=6BESXLj11WpFrUlKpKRPlaPi7QTSVRiEJsaw6nqNxFg=; b=Sp7OyYiVv2KkP4INb7m1th8YMioHaXfXtalPc9CXutS0ognFoVCoAjdCrz5gR+zcbU QMan1fBjTX0KEDbFD5IASo2jYveWUj/52HtdkCypTsuGTiAUIY46dp2gSKl8UJEJdyW3 FtaPuVic6NUivJODDBF0vrksSsEWl49gCWfnN6so0WOojel+CYGAcV7uNuZLYXC5X4Zf RFcKQ5IN6wsRyAfMuMNZQJp1yuH9GOCi7FZ8uWvXL6b5YloVcSs/wVJSJck5JsZrGBF0 rSq1dU4ggP4j84S2dmW7iK8EVOdXwbiFKEIgShblI2odehyLjTSNOqlOouAJ1P7KWRDr ymFQ== 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; bh=6BESXLj11WpFrUlKpKRPlaPi7QTSVRiEJsaw6nqNxFg=; b=XBBREELun6PaKHuLllEZ/csAxXafUb42roXDeKZTKlPMH0EuU8ghlhA/k5ntf7KP5b dZvCaYwEDFBk62FExsbGHe9zb3dNMzRUqPIcl8/2j6lFwkPnNx4U9QfOf5JXx6CRIUzJ E9TcPu/VsElgsItW5pStxseVd3cQNmh67XUeEofDFyx6R2gxiYqQG/HOtpv4S7KE9o+s djxBoCoK5DnfDLBGRX/g2+2X9v5hfqhHd50k6VAsvsPqTa3ubQcGBvt39wz8fjGYIqkG XSbYe5w1RHb0zuaNOMQq1ZX+BQLXJ+kd7zotUW/nocXgQ7QA26BT2q4QlPLKv7hNP8rO i79Q== X-Gm-Message-State: AOAM531wUx5uO1YcG/zP86IsI4oSJ2TlKpOz2lMxaQ93FA0HIpzr33ha gsYNhO8SVPSK5sdBoAtuUJXRane4gZxSwurtc8YCoQ== X-Google-Smtp-Source: ABdhPJyQ8OviTN8Rpbi8qQN2uRbWGgFjFOprJA5uvbEEa6QYrFCKK5d5gOVH//f0+7RxQ4PHJzojdvcj4lAwLun8XG0= X-Received: by 2002:ac2:521b:: with SMTP id a27mr20420103lfl.560.1627468871410; Wed, 28 Jul 2021 03:41:11 -0700 (PDT) MIME-Version: 1.0 References: <20210727085452.25708-1-zong.li@sifive.com> In-Reply-To: From: Zong Li Date: Wed, 28 Jul 2021 18:40:59 +0800 Message-ID: Subject: Re: [PATCH 1/2] cache: add sifive composable cache driver To: Sean Anderson Cc: rick@andestech.com, Leo Liang , Bin Meng , sjg@chromium.org, Green Wan , Paul Walmsley , u-boot@lists.denx.de Content-Type: text/plain; charset="UTF-8" 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 Wed, Jul 28, 2021 at 12:23 PM Sean Anderson wrote: > > On 7/27/21 4:54 AM, Zong Li wrote: > > This driver is currently responsible for enabling all ccache ways. > > Can you expand on this a little? Perhaps describe the hardware a little. For example, > you could describe what a way/bank is, and that they can't be disabled by the hardware. Certainly, let me give more details there. > > > > > Signed-off-by: Zong Li > > --- > > drivers/cache/Kconfig | 7 +++ > > drivers/cache/Makefile | 1 + > > drivers/cache/cache-sifive-ccache.c | 69 +++++++++++++++++++++++++++++ > > 3 files changed, 77 insertions(+) > > create mode 100644 drivers/cache/cache-sifive-ccache.c > > > > diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig > > index 1e452ad6d9..b903e3e935 100644 > > --- a/drivers/cache/Kconfig > > +++ b/drivers/cache/Kconfig > > @@ -39,4 +39,11 @@ config NCORE_CACHE > > controller. The driver initializes cache directories and coherent > > agent interfaces. > > > > +config SIFIVE_CACHE_CCACHE > > Just SIFIVE_CCACHE (or SIFIVE_CACHE) please. The idea is that the configuration name needs to be able to distinguish multiple cache devices. For example, if there are other two devices related to cache, we could give the following three configurations: - SIFIVE_CACHE_CCACHE - SIFIVE_CACHE_XXX - SIFIVE_CACHE_YYY SIFIVE_CCACHE is also ok to me, then we would get the following configuration names in the future. - SIFIVE_CCACHE - SIFIVE_XXX - SIFIVE_YYY > > > + bool "SiFive composable cache" > > + select CACHE > > + help > > + This driver is for SiFive Composable L2/L3 cache. It enables cache > > + ways of composable cache. > > + > > endmenu > > diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile > > index fed50be3f9..92c6c5a83f 100644 > > --- a/drivers/cache/Makefile > > +++ b/drivers/cache/Makefile > > @@ -4,3 +4,4 @@ obj-$(CONFIG_SANDBOX) += sandbox_cache.o > > obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o > > obj-$(CONFIG_NCORE_CACHE) += cache-ncore.o > > obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o > > +obj-$(CONFIG_SIFIVE_CACHE_CCACHE) += cache-sifive-ccache.o > > diff --git a/drivers/cache/cache-sifive-ccache.c b/drivers/cache/cache-sifive-ccache.c > > new file mode 100644 > > index 0000000000..9ea064912f > > --- /dev/null > > +++ b/drivers/cache/cache-sifive-ccache.c > > @@ -0,0 +1,69 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2021 SiFive > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define SIFIVE_CCACHE_CONFIG 0x000 > > +#define SIFIVE_CCACHE_ENABLE 0x008 > > WAY_ENABLE? Changes in the next version. > > > + > > +#define SIFIVE_CCACHE_NUM_WAY_MASK GENMASK(15, 8) > > +#define SIFIVE_CCACHE_NUM_WAY_SHIFT 8 > > + > > +struct sifive_ccache { > > + void __iomem *base; > > +}; > > + > > +static int sifive_ccache_enable_all_ways(struct udevice *dev) > > +{ > > + struct sifive_ccache *priv = dev_get_priv(dev); > > + u32 config; > > + u32 ways; > > + > > + config = readl(priv->base + SIFIVE_CCACHE_CONFIG); > > + ways = (config & SIFIVE_CCACHE_NUM_WAY_MASK) >> SIFIVE_CCACHE_NUM_WAY_SHIFT; > > ways = FIELD_GET(SIFIVE_CCACHE_NUM_WAY_MASK, config); > > and perhaps this should be named SIFIVE_CCACHE_CONFIG_WAYS to better match the datasheet? > Yes, change it in the next version. > > + > > + writel(ways - 1, priv->base + SIFIVE_CCACHE_ENABLE); > > + > > + return 0; > > +} > > + > > +static int sifive_ccache_enable(struct udevice *dev) > > +{ > > + return sifive_ccache_enable_all_ways(dev); > > Any reason to have this in a separate function? > sifive_ccache_enable isn't clear enough to me, we couldn't be straightforward to know what to enable. > > +} > > + > > +static const struct cache_ops sifive_ccache_ops = { > > + .enable = sifive_ccache_enable, > > Please implement get_info as well. It should effectively just be > Add get_info in the next version. Thanks. > get_info() > { > struct sifive_ccache *priv = dev_get_priv(dev); > > info->base = priv->base; > return 0; > } > > > +}; > > + > > +static int sifive_ccache_probe(struct udevice *dev) > > +{ > > + struct sifive_ccache *priv = dev_get_priv(dev); > > + > > + priv->base = dev_read_addr_ptr(dev); > > + if (!priv->base) > > + return -ENODEV; > > Please return -EINVAL instead [1]. > Thanks. Modify it in the next version. > --Sean > > [1] https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#error-codes > > > + > > + return 0; > > +} > > + > > +static const struct udevice_id sifive_ccache_ids[] = { > > + { .compatible = "sifive,fu540-c000-ccache" }, > > + { .compatible = "sifive,fu740-c000-ccache" }, > > + {} > > +}; > > + > > +U_BOOT_DRIVER(sifive_ccache) = { > > + .name = "sifive_ccache", > > + .id = UCLASS_CACHE, > > + .of_match = sifive_ccache_ids, > > + .probe = sifive_ccache_probe, > > + .priv_auto = sizeof(struct sifive_ccache), > > + .ops = &sifive_ccache_ops, > > +}; > > >