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=-11.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=unavailable 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 8758CC433DF for ; Sun, 18 Oct 2020 10:24:32 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 047FF20760 for ; Sun, 18 Oct 2020 10:24:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ri3Fvk/W"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="TnUnk8Pa" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 047FF20760 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=05CEa9Lz5AZH62THqGoiNWgZ715kt/Kh+f0cbVAoghs=; b=ri3Fvk/WVTwp5zntZfLBX4/0j gdrmm7HZaHkgPBZ50+Ws8Xuo+XhxaLrzTnsOJq5hI1+1RilkrCfe+5afs9rUeRf9xXDJofOGPh3tE jpyCzPypfzTuNhkhKabHWlc3FYCJD5DWfuUuCJdSQZqi6QnwJ/cCmAFRrJWxoqHrEYn8+HihNtRo6 CHEigkVT06oZJc9U8hVjUw9WGgAMdbyWMX3MQmM/eV4yMPYa2cpDMZPWcJp5bo76MuGR+3EUQJSAS pjgEGg+EjTTRlf3d9oUnqfCJ1bcDnfYMsl4du+it078uC4s1BBdvfIscuSHrWfuji6cI5H3l39xrf QbUcDTceg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kU5q6-0001kY-U9; Sun, 18 Oct 2020 10:22:58 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kU5q4-0001iy-Ey for linux-arm-kernel@lists.infradead.org; Sun, 18 Oct 2020 10:22:57 +0000 Received: from archlinux (cpc149474-cmbg20-2-0-cust94.5-4.cable.virginm.net [82.4.196.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D21FF20760; Sun, 18 Oct 2020 10:22:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1603016575; bh=8EUCCLIus+bj3TBpR6tLKPOgQLXNBSMW8Xg9mEpfHRw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=TnUnk8PaVvi3DWuoo5FEVDYu1vYf6streZxPObIhrGv9pxLZZD+8TPS99zdYCpzJT DmQRDf3jM4humUWUc8QDdmmh4JNSfyAJYdCMr7EayImQZFAv3txV9Na0tnShv1JYrq qvo/q7WUYVs2wSKp52Sz2gPX72ivWhjx5hM04Se4= Date: Sun, 18 Oct 2020 11:22:49 +0100 From: Jonathan Cameron To: Billy Tsai Subject: Re: [PATCH 1/3] iio: adc: aspeed: Orgnaize and add the define of adc Message-ID: <20201018112249.44dd3bde@archlinux> In-Reply-To: <20201013103245.16723-2-billy_tsai@aspeedtech.com> References: <20201013103245.16723-1-billy_tsai@aspeedtech.com> <20201013103245.16723-2-billy_tsai@aspeedtech.com> X-Mailer: Claws Mail 3.17.7 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201018_062256_775475_7229DE25 X-CRM114-Status: GOOD ( 23.79 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, lars@metafoo.de, p.zabel@pengutronix.de, linux-aspeed@lists.ozlabs.org, BMC-SW@aspeedtech.com, andrew@aj.id.au, linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org, robh+dt@kernel.org, joel@jms.id.au, pmeerw@pmeerw.net, knaack.h@gmx.de, alexandru.ardelean@analog.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 13 Oct 2020 18:32:43 +0800 Billy Tsai wrote: > This patch organizes the define of adc to multiple partitions > and adds the new bit field define for ast2600 driver. Should be 2 patch patches. If you need to do a reorg, do it first, then add new bits in a second patch. That way we are reviewing one non functional change, and one that actually does something. As a general rule, I'd also prefer to just see the additional defines added as and when they are used (in the patch that first uses them). > > Signed-off-by: Billy Tsai > --- > drivers/iio/adc/aspeed_adc.c | 42 ++++++++++++++++++++++++++++++++---- > 1 file changed, 38 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c > index 1e5375235cfe..ae400c4d6d40 100644 > --- a/drivers/iio/adc/aspeed_adc.c > +++ b/drivers/iio/adc/aspeed_adc.c > @@ -21,23 +21,57 @@ > #include > #include > > +/********************************************************** > + * ADC feature define > + *********************************************************/ I'm generally of the view that block comments like this normally imply the defines are not sufficiently self identifying. It should be possible to know what sort of thing they are at the point of use without having to go find a comment in the header. So if you think these are needed, perhaps reconsider the naming of of the defines? Personally I just don't seem them as necessary. Like all comments, they tend to 'rot' over time so if they aren't adding information, better not to have them! > #define ASPEED_RESOLUTION_BITS 10 > #define ASPEED_CLOCKS_PER_SAMPLE 12 > > +/********************************************************** > + * ADC HW register offset define > + *********************************************************/ > #define ASPEED_REG_ENGINE_CONTROL 0x00 > #define ASPEED_REG_INTERRUPT_CONTROL 0x04 > #define ASPEED_REG_VGA_DETECT_CONTROL 0x08 > #define ASPEED_REG_CLOCK_CONTROL 0x0C > +#define ASPEED_REG_COMPENSATION_TRIM 0xC4 > #define ASPEED_REG_MAX 0xC0 > > +/********************************************************** > + * ADC register Bit field > + *********************************************************/ > +/*ENGINE_CONTROL */ Inconsistent spacing after /* > +/* [0] */ > +#define ASPEED_ENGINE_ENABLE BIT(0) > +/* [3:1] */ If the docs are needed, then better to use FIELD_PREP and GENMASK as that is going to be self documenting at the actual point of the defines. > #define ASPEED_OPERATION_MODE_POWER_DOWN (0x0 << 1) > #define ASPEED_OPERATION_MODE_STANDBY (0x1 << 1) > #define ASPEED_OPERATION_MODE_NORMAL (0x7 << 1) > - > -#define ASPEED_ENGINE_ENABLE BIT(0) > - > +/* [4] */ > +#define ASPEED_CTRL_COMPENSATION BIT(4) > +/* [5] */ > +#define ASPEED_AUTOPENSATING BIT(5) > +/* [7:6] */ > +#define ASPEED_REF_VOLTAGE_2500mV (0 << 6) > +#define ASPEED_REF_VOLTAGE_1200mV (1 << 6) > +#define ASPEED_REF_VOLTAGE_EXT_HIGH (2 << 6) > +#define ASPEED_REF_VOLTAGE_EXT_LOW (3 << 6) > +#define ASPEED_BATTERY_SENSING_VOL_DIVIDE_2_3 (0 << 6) > +#define ASPEED_BATTERY_SENSING_VOL_DIVIDE_1_3 (1 << 6) > +/* [8] */ > #define ASPEED_ADC_CTRL_INIT_RDY BIT(8) > - > +/* [12] */ > +#define ASPEED_ADC_CH7_VOLTAGE_NORMAL (0 << 12) > +#define ASPEED_ADC_CH7_VOLTAGE_BATTERY (1 << 12) > +/* [13] */ > +#define ASPEED_ADC_EN_BATTERY_SENSING BIT(13) > +/* [31:16] */ > +#define ASPEED_ADC_CTRL_CH_EN(n) (1 << (16 + n)) > +#define ASPEED_ADC_CTRL_CH_EN_ALL GENMASK(31, 16) > + > +/********************************************************** > + * Software setting > + *********************************************************/ > #define ASPEED_ADC_INIT_POLLING_TIME 500 > #define ASPEED_ADC_INIT_TIMEOUT 500000 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel