Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp2364413rwd; Sun, 21 May 2023 19:37:50 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ48uVvADRbI/p7K81Zn/tRxXSVSwUFKhE3HxI5VSR4BLcNVsQZyMDvN/uIpJGoy3TAH+H6P X-Received: by 2002:a17:902:ecc9:b0:1a6:f93a:a135 with SMTP id a9-20020a170902ecc900b001a6f93aa135mr12002460plh.61.1684723070084; Sun, 21 May 2023 19:37:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684723070; cv=none; d=google.com; s=arc-20160816; b=n2dvr06Y2TimuEvdbHR5lMLXPjW4fqshKkjG9V1mZQvoP8YGm5DkEKnZyWmKCUnCOz 4qKQBBja0YILYJ8BQV11a9XYCs2rmzuxiM135bRGsHgCvS8vpUjy4ds5JGyZPxHtzbaQ cazGaP1JFpu//IrsQOFH4vJBNr1qEpMe7RBX2kl/leuFTgJttdzC0vBa2q+PMVMexf/Z xspc7ortY9yBWSwY3Lvl86h6hSQK6hRQjHZYCblvP3BCYS35wt/CRDRrf66/yNmlWqWE /S0Sv/fxWLQUKbBG+xTxcr3w573Tg6NOmGpES+GtdPf/f6toBY3vrSMwgfFdZnobBh9N EfPw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=9hhzQTnaAy411+IhN900JyorZ91IqoCCvwAENIoeFjE=; b=avTrGCukwHu9fES+K2+A9jL/l+mU1ehKcUFWe5I0qNYTP+Z4TVpU2fAxlKbHpM8873 DbNTNOi6jML6HweLEw5v9sRwf0S03QKQxo67P0RcUg98cYrr8KuLccIWiiu8mj5n7oBn R29/kKpPEewnrMIfT57Q8FPEjsaWLn6Ya8F0ES7n65Y3lUZL+iCFRhYdBgkbufARnkvc DUFkkChB6NSC6KcDKfkcurMWaK4mgK2uuvjfOyWgBt3P0YgtsLZqgLdL5jQQmKGRL6uD WerfXhngXZ60C4+N4L2rsHfWf2+00vsk6dnKJXg54i3HR39aPrsFx5CT7q9oXy9yu4ct 2zSA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=McmCYlHh; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id ju11-20020a170903428b00b001aaecc0b701si3744505plb.127.2023.05.21.19.37.37; Sun, 21 May 2023 19:37:50 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=McmCYlHh; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231346AbjEVCQh (ORCPT + 99 others); Sun, 21 May 2023 22:16:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229825AbjEVCQg (ORCPT ); Sun, 21 May 2023 22:16:36 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1BE1FB7; Sun, 21 May 2023 19:16:35 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id AC7EF61946; Mon, 22 May 2023 02:16:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1892BC4339C; Mon, 22 May 2023 02:16:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1684721794; bh=n2uaZ9rV4gw+Dcgfo0DSk4CEyQozD7dCVzjmhdw+dyA=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=McmCYlHhVDzHL5kc9dRRAsAuH8eHQrfIKjNgAFmIHR8b+WsIXqluaqokvmVoA6Urs JVtb2sic/Hw0Eoyx77hO8ZWA6mkz6GAttMvZ98b8AWpbUad4BoBgxnM4pkk5lk8FNF zPoL0UVJsRcBYHvytdIxHWfxHaYqgB9x7IzKEd2mvQ3AAeKsx3NnqZVKB1gIDU9WP6 mddSrh6Fne00SjCxscAZAwtQGkq+TyqXBvAPl3OL32VhytWhp9CrsXxSqew0hGb6n6 74I4xgNt88bx9RdzIcBgN3yf1C4IDDsNXa0Q8Lo9x4V2b0kj02T8fAm4LREMZurWhv 8d6GoeQYA7Gcw== Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-510eb980ce2so6007775a12.2; Sun, 21 May 2023 19:16:33 -0700 (PDT) X-Gm-Message-State: AC+VfDwZwAtvnFsYF4fta7MI4yQEoTT8JxbaiogcapTKS1lWU7sixyD6 yb8IIwpe3vfXTcxk5s9TH8h7ruGpwl2jYbJd4nI= X-Received: by 2002:aa7:c6c8:0:b0:50b:c397:bbac with SMTP id b8-20020aa7c6c8000000b0050bc397bbacmr7184287eds.29.1684721792303; Sun, 21 May 2023 19:16:32 -0700 (PDT) MIME-Version: 1.0 References: <20230518184541.2627-1-jszhang@kernel.org> <20230518184541.2627-5-jszhang@kernel.org> <20230518-driving-secluding-793b3192776e@spud> In-Reply-To: <20230518-driving-secluding-793b3192776e@spud> From: Guo Ren Date: Mon, 22 May 2023 10:16:19 +0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 4/9] dt-binding: riscv: add T-HEAD CPU reset To: Conor Dooley Cc: Jisheng Zhang , Thomas Gleixner , Marc Zyngier , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Palmer Dabbelt , Paul Walmsley , Albert Ou , linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-riscv@lists.infradead.org, Yangtao Li , Wei Fu , Icenowy Zheng Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 19, 2023 at 3:53=E2=80=AFAM Conor Dooley wro= te: > > Hey Jisheng, > > On Fri, May 19, 2023 at 02:45:36AM +0800, Jisheng Zhang wrote: > > The secondary CPUs in T-HEAD SMP capable platforms need some special > > handling. The first one is to write the warm reset entry to entry > > register. The second one is write a SoC specific control value to > > a SoC specific control reg. The last one is to clone some CSRs for > > secondary CPUs to ensure these CSRs' values are the same as the > > main boot CPU. This DT node is mainly used by opensbi firmware. > > > > Signed-off-by: Jisheng Zhang > > --- > > .../bindings/riscv/thead,cpu-reset.yaml | 69 +++++++++++++++++++ > > 1 file changed, 69 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/riscv/thead,cpu-r= eset.yaml > > > > diff --git a/Documentation/devicetree/bindings/riscv/thead,cpu-reset.ya= ml b/Documentation/devicetree/bindings/riscv/thead,cpu-reset.yaml > > new file mode 100644 > > index 000000000000..ba8c87583b6b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/riscv/thead,cpu-reset.yaml > > @@ -0,0 +1,69 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/riscv/thead,cpu-reset.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: T-HEAD cpu reset controller > > + > > +maintainers: > > + - Jisheng Zhang > > + > > +description: | > > + The secondary CPUs in T-HEAD SMP capable platforms need some special > > + handling. The first one is to write the warm reset entry to entry > > + register. The second one is write a SoC specific control value to > > + a SoC specific control reg. The last one is to clone some CSRs for > > + secondary CPUs to ensure these CSRs' values are the same as the > > + main boot CPU. > > Okay.. > > > + > > +properties: > > + $nodename: > > + pattern: "^cpurst" > > Firstly, why the nodename enforcement? We have a compatible, so we > should be okay, no? Yes, it needn't. > > > + > > + compatible: > > + oneOf: > > + - description: CPU reset on T-HEAD TH1520 SoC > > + items: > > + - const: thead,reset-th1520 > > You've only got one thing here, you don't need the oneOf. > Also, s/reset-th1520/th1520-reset/ please - although I do not know if > "reset" is the right word here. Do we know what the IP block is called > in the TRM/T-Head docs? Perhaps Guo Ren does if not. It's called CPU reset controller; every core has reset_ctrl & reset_entry signals; Soc just gathers them into some regs. For th1520, we have 4 reset_entries registers and 1 reset_ctrl register. Fu Wei would give out more details about it. > > > + entry-reg: > > + $ref: /schemas/types.yaml#/definitions/uint64 > > + description: | > > + The entry reg address. > > + > > + entry-cn > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + The entry reg count. > > + > > + control-reg: > > + $ref: /schemas/types.yaml#/definitions/uint32 It should be uint64. > > + description: | > > + The control reg address. > > + > > + control-val: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + The value to be set into the control reg. > > + > > + csr-copy: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + description: | > > + The CSR registers to be cloned during CPU warm reset. > > All of these values set on a per-soc basis, right? Yes > If so, I don't think they should be in here at all since you should be > able to figure out the offsets from the base & the values to write based > on the compatible string alone, no? The driver works with all T-HEAD CPUs, not only for th1520. Some vendors may have their own custom CSRs, so the csr-copy feature is flexible enough to adjust in dts. As far as I can tell, hardware teams typically prefer to focus on the firmware binary rather than setting up the software compiling environment. > > Putting register values into the DT is always "suspect"! It's not register values, it's register offset/ CSR number. > > > +required: > > + - compatible > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + cpurst: cpurst@ffff019050 { > ^^^^^^^^^^^^^^^^^ > This is also "suspect" and implies that "entry reg" should just be a > normal "reg" property. Yes, but we needn't reg, here. It should be: cpurst { Thx for debugging. > > > > + compatible =3D "thead,reset-th1520"; > > + entry-reg =3D <0xff 0xff019050>; > > + entry-cnt =3D <4>; > > + control-reg =3D <0xff 0xff015004>; > > + control-val =3D <0x1c>; > > + csr-copy =3D <0x7f3 0x7c0 0x7c1 0x7c2 0x7c3 0x7c5 0x7cc>; > > + }; > > -- > > 2.40.0 > > --=20 Best Regards Guo Ren