Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1754911AbdDLQlC (ORCPT <rfc822;w@1wt.eu>);
        Wed, 12 Apr 2017 12:41:02 -0400
Received: from smtprelay0196.hostedemail.com ([216.40.44.196]:51599 "EHLO
        smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL)
        by vger.kernel.org with ESMTP id S1754895AbdDLQkz (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 12 Apr 2017 12:40:55 -0400
X-Session-Marker: 6A6F6540706572636865732E636F6D
X-Spam-Summary: 
X-HE-Tag: route76_1dd91c94cf019
X-Filterd-Recvd-Size: 3807
Message-ID: <1492015203.28586.1.camel@perches.com>
Subject: Re: [PATCH V14 02/10] ras: acpi/apei: cper: generic error data
 entry v3 per ACPI 6.1
From: Joe Perches <joe@perches.com>
To: Borislav Petkov <bp@alien8.de>, Tyler Baicar <tbaicar@codeaurora.org>
Cc: christoffer.dall@linaro.org, marc.zyngier@arm.com, pbonzini@redhat.com,
        rkrcmar@redhat.com, linux@armlinux.org.uk, catalin.marinas@arm.com,
        will.deacon@arm.com, rjw@rjwysocki.net, lenb@kernel.org,
        matt@codeblueprint.co.uk, robert.moore@intel.com, lv.zheng@intel.com,
        nkaje@codeaurora.org, zjzhang@codeaurora.org, mark.rutland@arm.com,
        james.morse@arm.com, akpm@linux-foundation.org,
        eun.taik.lee@samsung.com, sandeepa.s.prabhu@gmail.com,
        labbott@redhat.com, shijie.huang@arm.com, rruigrok@codeaurora.org,
        paul.gortmaker@windriver.com, tn@semihalf.com, fu.wei@linaro.org,
        rostedt@goodmis.org, bristot@redhat.com,
        linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-acpi@vger.kernel.org, linux-efi@vger.kernel.org,
        devel@acpica.org, Suzuki.Poulose@arm.com, punit.agrawal@arm.com,
        astone@redhat.com, harba@codeaurora.org, hanjun.guo@linaro.org,
        john.garry@huawei.com, shiju.jose@huawei.com, gengdongjiu@huawei.com,
        xiexiuqi@huawei.com
Date: Wed, 12 Apr 2017 09:40:03 -0700
In-Reply-To: <20170412133447.n2yft7v266v6k6gh@pd.tnic>
References: <1490729440-32591-1-git-send-email-tbaicar@codeaurora.org>
         <1490729440-32591-3-git-send-email-tbaicar@codeaurora.org>
         <20170412133447.n2yft7v266v6k6gh@pd.tnic>
Content-Type: text/plain; charset="ISO-8859-1"
X-Mailer: Evolution 3.22.3-0ubuntu0.1 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1843
Lines: 51

On Wed, 2017-04-12 at 15:34 +0200, Borislav Petkov wrote:
> On Tue, Mar 28, 2017 at 01:30:32PM -0600, Tyler Baicar wrote:
> > Currently when a RAS error is reported it is not timestamped.
[]
> > diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
[]
> > +#define acpi_hest_generic_data_error_length(gdata)	\
> > +	(((struct acpi_hest_generic_data *)(gdata))->error_data_length)
> > +#define acpi_hest_generic_data_size(gdata)		\
> > +	((acpi_hest_generic_data_version(gdata) >= 3) ?	\
> > +	sizeof(struct acpi_hest_generic_data_v300) :	\
> > +	sizeof(struct acpi_hest_generic_data))
> > +#define acpi_hest_generic_data_record_size(gdata)	\
> > +	(acpi_hest_generic_data_size(gdata) +		\
> > +	acpi_hest_generic_data_error_length(gdata))
> > +#define acpi_hest_generic_data_next(gdata)		\
> > +	((void *)(gdata) + acpi_hest_generic_data_record_size(gdata))
> 
> This is one unreadable pile of too long names with a clearly redundant
> and too long prefix. Please shorten it all.

Naming is generally author's choice and internal
consistency has value too.

acpi_hest_generic<foo> is already used throughout this codebase
in multiple files and paths.

> > @@ -73,3 +85,13 @@ static inline void ghes_edac_unregister(struct ghes *ghes)
> >  {
> >  }
> >  #endif
> > +
> > +#define acpi_hest_generic_data_version(gdata)			\
> > +	(gdata->revision >> 8)
> > +
> > +static inline void *acpi_hest_generic_data_payload(struct acpi_hest_generic_data *gdata)
> 
> Lemme try to shorten it:
> 
> static inline void *acpi_hest_get_payload(struct acpi_hest_gdata *d)
> {
> 	if (hest_gdata_ver(d) >= 3)
> 		return (void *)(((struct acpi_hest_gdata_v3 *)d) + 1);
> 	else
> 		return d + 1;
> }
> 
> Now this is much more readable IMO. You can actually see what's going
> on. And you still know what the struct names are.

trivial: unnecessary cast to void *