Received: by 2002:a05:6a10:5bc5:0:0:0:0 with SMTP id os5csp1597884pxb; Thu, 4 Nov 2021 05:20:02 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxI9/pAPRtw0hzdZl4fTpE9tS2AcpCQC5qg/IKf2zwb9Cn6A3BRDi6rm23tne/Aks9kI96R X-Received: by 2002:a17:907:60d6:: with SMTP id hv22mr9633017ejc.492.1636028402431; Thu, 04 Nov 2021 05:20:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1636028402; cv=none; d=google.com; s=arc-20160816; b=V2qpAp9kVmtHGJk553BEMmxsX0MnV1OPfMfBOUHU9aaCuxfgov951iofDUKJi6g65Y bpl8fetjQ6y2TOwp7NSgZiXCDlO0UL4GUPfAu2M9r4xHXgkr6sQBplqo2SuJ4Nv2y+qA lGZI+LJ5hB4by7kJTu8HtaZ89cUhvagwFEgdY4vYkhgyyx0Us7LVKSqHS6Zh8ZhqOaUC r5GsazbQun7kJyXRXmZKYOY9Z+G5KVVHF+TYx4l1Tu7N2QMg4f4LdaONNV/+uW2HoV6a 9wP+Jkagbfmnv1BKS9Y24vb070xlcTVQLNLKyzvMtqMndPBrwXwCqxAr/uB/Uh6f77A5 oTkw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version; bh=BbQxkXh1kRX5txg91IzBReR7Etxrh9MaaIL5VubC4mA=; b=ke8xhX5Btb5EadXvkjWORcXvx+wDk48QPpFtZAiY/gx23fmavm30IHqHQG8sAdEN1f i48D7bux4h7lUi7Eb2OvvgowE6tJ3fCZ5H0UY0123evRuRk3zHwnljPlasweckdswmK6 yNAnA7UnBPmRiPz9qJu6LD+Chs2Fmylw/PA5Bk4Agxi1C2GjmAuijaBPg6klkPqRxOTX ORUExciyMx+0mY/S41pGiC4cMwfMj0WWAy7GmWzyHLKpaPthm+SbRoMypFWmZPh8M2I9 +QLXtWU4lUoElw7g1puRotorvQXPR9DQGhFTPu0MKFWN27nqd2rGwH/64iTQcCNW1FXY VdIg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id gt31si8282098ejc.644.2021.11.04.05.19.34; Thu, 04 Nov 2021 05:20:02 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231486AbhKDMSg (ORCPT + 99 others); Thu, 4 Nov 2021 08:18:36 -0400 Received: from mail-pg1-f176.google.com ([209.85.215.176]:36458 "EHLO mail-pg1-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231312AbhKDMSf (ORCPT ); Thu, 4 Nov 2021 08:18:35 -0400 Received: by mail-pg1-f176.google.com with SMTP id 75so5255429pga.3; Thu, 04 Nov 2021 05:15:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=BbQxkXh1kRX5txg91IzBReR7Etxrh9MaaIL5VubC4mA=; b=HwaDncFOGCkngqsx9mhphaCX9kCINHoPVkIt6zwRsCF/XLC2Stu1pMzkcI+laGuggx WtNWzQUf2HtiwZIfpaYVa9QyDVnnPIR+Ky9MkxgDEmCIp82Kyo36261eV3WsG8iMsCKy 5TCL0XWo9yi+TVJKKbpqstz22CmCChIBximeJSIGh5vJ2FMkGGBINKHcxELqzVxdCTxI zEKb9sIhDT5YogiZZx/SvRUSADQKYBn/5ScV7KhC7wTRifgwlxz9f4vpPGaCMew6adsC ZpyYu7d+Op6oDiH2gPau7GmWP6DWUOtIC48kROCfF0V+ItWTtAmTfdPWBEK3Ime5eJWY J06w== X-Gm-Message-State: AOAM531W3zX6vHHFbMupsQ67P1Kpvc/B/GyqSYBooLA22s889KpCBeeX EG8YRrIrekcSgxqj3hSYml4CvAHE1xoEDZYPKWM= X-Received: by 2002:a05:6a00:1354:b0:494:5227:42c7 with SMTP id k20-20020a056a00135400b00494522742c7mr5310054pfu.53.1636028156990; Thu, 04 Nov 2021 05:15:56 -0700 (PDT) MIME-Version: 1.0 References: <20211102161125.1144023-1-kernel@esmil.dk> <20211102161125.1144023-10-kernel@esmil.dk> In-Reply-To: From: Emil Renner Berthing Date: Thu, 4 Nov 2021 13:15:46 +0100 Message-ID: Subject: Re: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver To: Andy Shevchenko Cc: Yury Norov , linux-riscv , devicetree , linux-clk , "open list:GPIO SUBSYSTEM" , "open list:SERIAL DRIVERS" , Palmer Dabbelt , Paul Walmsley , Rob Herring , Michael Turquette , Stephen Boyd , Thomas Gleixner , Marc Zyngier , Philipp Zabel , Linus Walleij , Greg Kroah-Hartman , Daniel Lezcano , Andy Shevchenko , Jiri Slaby , Maximilian Luz , Sagar Kadam , Drew Fustini , Geert Uytterhoeven , Michael Zhu , Fu Wei , Anup Patel , Atish Patra , Matteo Croce , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2 Nov 2021 at 22:17, Emil Renner Berthing wrote: > On Tue, 2 Nov 2021 at 21:14, Andy Shevchenko wrote: > > On Tue, Nov 2, 2021 at 9:59 PM Emil Renner Berthing wrote: > > > On Tue, 2 Nov 2021 at 20:43, Andy Shevchenko wrote: > > > > On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing wrote: > > > > ... > > > > > > > +/* > > > > > + * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of > > > > > + * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the > > > > > + * same line. > > > > > + * most reset lines have their status inverted so a 0 in the STATUS register > > > > > + * means the line is asserted and a 1 means it's deasserted. a few lines don't > > > > > + * though, so store the expected value of the status registers when all lines > > > > > + * are asserted. > > > > > + */ > > > > > > > > Besides missing capitalization, > > > > > > I'm confused. it was you who wanted all comments to capitalized the same.. > > > > Yes and there are two types of the comments, one-liners and > > multi-line. In multi-line you usually use proper English grammar, > > where capitalization means what it means. For the one-liners just > > choose either small letters or capital letters to start them with. > > That sounds reasonable, it was just that you complained about > inconsistent comments in the pinctrl driver that follows the above. > > > > if it sounds like bitmap, use bitmap. > > > > I have checked DT definitions and it seems you don't even need the > > > > BIT_MASK() macro, > > > > > > > > > +static const u32 jh7100_reset_asserted[4] = { > > > > > + /* STATUS0 register */ > > > > > + BIT_MASK32(JH7100_RST_U74) | > > > > > + BIT_MASK32(JH7100_RST_VP6_DRESET) | > > > > > + BIT_MASK32(JH7100_RST_VP6_BRESET), > > > > > + /* STATUS1 register */ > > > > > + BIT_MASK32(JH7100_RST_HIFI4_DRESET) | > > > > > + BIT_MASK32(JH7100_RST_HIFI4_BRESET), > > > > > + /* STATUS2 register */ > > > > > + BIT_MASK32(JH7100_RST_E24), > > > > > + /* STATUS3 register */ > > > > > + 0, > > > > > +}; > > > > > > > > Yury, do we have any clever (clean) way to initialize a bitmap with > > > > particular bits so that it will be a constant from the beginning? If > > > > no, any suggestion what we can provide to such users? > > > > > > The problem is, that even if we could initialize this without the > > > monstrosity in our last conversation a 64bit bitmap would still > > > produce worse code. As it is now it's simply a 32bit load and mask > > > with index and mask already calculated for the registers. In the > > > status callback the mask can even be folded into the register read > > > mask. With a 64bit bitmap you'd need to calculate new 64bit index and > > > masks, and then conditionally shift the bits into position. > > > > Why? You may use 8 byte IO (writeq() / readq() or their relaxed versions), no? > > > > > If this reflection of the 32bit registers bothers you that much > > > > What bothers me is hidden endianess issues (yeah, here it might be > > theoretical, but consider that somebody will look at your code and use > > it as the best example ever). > > Wouldn't endian issues be a reason to make sure we read 32bit > registers with 32bit reads? Or do you expect a hypothetical big-endian > StarFive SoC to also change the order of the registers? Hi Andy. I'd really like to understand your reasoning here. As far as I can tell reading 2 adjacent 32bit registers with a 64bit read as you're proposing is exactly what would cause endian issues. Eg. on little endian you'd get reg0 | reg1 << 32 whereas on big-endian you'd get reg0 << 32 | reg1. /Emil