From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33603) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPXCj-00077e-WE for qemu-devel@nongnu.org; Mon, 17 Mar 2014 09:03:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WPXCZ-0002oe-QL for qemu-devel@nongnu.org; Mon, 17 Mar 2014 09:03:45 -0400 Received: from mail-lb0-f177.google.com ([209.85.217.177]:64646) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPXCZ-0002nX-Je for qemu-devel@nongnu.org; Mon, 17 Mar 2014 09:03:35 -0400 Received: by mail-lb0-f177.google.com with SMTP id z11so3592164lbi.36 for ; Mon, 17 Mar 2014 06:03:34 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1394134385-1727-1-git-send-email-peter.maydell@linaro.org> <1394134385-1727-14-git-send-email-peter.maydell@linaro.org> From: Peter Maydell Date: Mon, 17 Mar 2014 13:03:14 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v4 13/21] target-arm: Use dedicated CPU state fields for ARM946 access bit registers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Rob Herring , Patch Tracking , Michael Matz , Claudio Fontana , Alexander Graf , "qemu-devel@nongnu.org Developers" , Laurent Desnogues , Dirk Mueller , Will Newton , =?UTF-8?B?QWxleCBCZW5uw6ll?= , "kvmarm@lists.cs.columbia.edu" , Christoffer Dall , Richard Henderson On 17 March 2014 05:20, Peter Crosthwaite wrote: > On Fri, Mar 7, 2014 at 5:32 AM, Peter Maydell wrote: >> static const ARMCPRegInfo pmsav5_cp_reginfo[] = { >> { .name = "DATA_AP", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0, >> .access = PL1_RW, .type = ARM_CP_NO_MIGRATE, >> - .fieldoffset = offsetof(CPUARMState, cp15.c5_data), .resetvalue = 0, >> + .fieldoffset = offsetof(CPUARMState, cp15.pmsav5_data_ap), >> + .resetvalue = 0, > > I know its just motion of existing code, but what's the policy on zero > resets? Can we leave them out for brevity? Checkpatch complains when a > global is explictly 0 initialized, so it seems sane that the same rule > applies to individual fields (just checkpatch probably has hard time > figuring this one out). The semantics allow you to leave out .resetvalue if it is 0; I think generally when I've written reginfo definitions I've tended to put in the .resetvalue as an indication of "I know this reginfo needs a reset value and it is zero" as opposed to "I didn't think about reset when I wrote this". We definitely don't want checkpatch to warn about explicit zero initializers in structs like this, otherwise we wouldn't be able to say ".crm = 0, .opc1 = 0, .opc2 = 0", which would be less comprehensible than writing them out explicitly I think. thanks -- PMM