From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mg.richtek.com (mg.richtek.com [220.130.44.152]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 54AD91848; Wed, 3 Jan 2024 01:05:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=richtek.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=richtek.com X-MailGates: (SIP:2,PASS,NONE)(compute_score:DELIVER,40,3) Received: from 192.168.10.47 by mg.richtek.com with MailGates ESMTPS Server V6.0(636810:0:AUTH_RELAY) (envelope-from ) (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256/256); Wed, 03 Jan 2024 09:04:52 +0800 (CST) Received: from ex3.rt.l (192.168.10.46) by ex4.rt.l (192.168.10.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1258.27; Wed, 3 Jan 2024 09:04:52 +0800 Received: from linuxcarl2.richtek.com (192.168.10.154) by ex3.rt.l (192.168.10.45) with Microsoft SMTP Server id 15.2.1258.27 via Frontend Transport; Wed, 3 Jan 2024 09:04:52 +0800 Date: Wed, 3 Jan 2024 09:04:52 +0800 From: ChiYuan Huang To: Jonathan Cameron CC: Krzysztof Kozlowski , Conor Dooley , Lars-Peter Clausen , Rob Herring , Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , , , Subject: Re: [PATCH v2 2/2] iio: adc: rtq6056: Add support for the whole RTQ6056 family Message-ID: <20240103010452.GA6655@linuxcarl2.richtek.com> References: <74db15583a9a68701dbff5a1a967c0d987d6dfb6.1703762557.git.cy_huang@richtek.com> <20231230120347.0816bd09@jic23-huawei> <20240102083042.GA13611@linuxcarl2.richtek.com> <20240102193642.0d6d2007@jic23-huawei> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20240102193642.0d6d2007@jic23-huawei> User-Agent: Mutt/1.5.21 (2010-09-15) On Tue, Jan 02, 2024 at 07:36:42PM +0000, Jonathan Cameron wrote: > On Tue, 2 Jan 2024 16:30:42 +0800 > ChiYuan Huang wrote: > > > Hi, Johathan: > > > > Most comments are good and will be fixed in next revision. > > > > Still one comment I cannot make sure. > > > > Please see the comment that's below yours. > > > Hi ChiYuan, > > It's good practice to crop away all the parts where the discussion is finished. > Makes it easier for people to find the bit you want discussion to continue on! > > I've done so in this reply. > > ... > > > > + > > > > enum { > > > > RTQ6056_CH_VSHUNT = 0, > > > > RTQ6056_CH_VBUS, > > > > @@ -50,16 +60,29 @@ enum { > > > > enum { > > > > F_OPMODE = 0, > > > > F_VSHUNTCT, > > > > + F_SADC = F_VSHUNTCT, > > > > > > If the devices have different register fields, better to have different enums > > > for them as well as that should result in less confusing code. > > > > > Actually, this is all the same register, just the control naming difference. > > If not to define the new eum, I can remain to use the same field to handle rtq6059 part. > > If the bits in the register control the same thing across both parts then > add a comment alongside the enum to make that clear. > > Given the naming that seems very unlikely. PGA and AVG would eman > very different things to me for starters (oversampling vs a programmble > gain amplifier on the front end) > I'm also thinking how to write this difference like as comments or a seperate enum. But if to define a new enum, many function about the regfield controls must be seperated for 6056 and 6059. > > > > > > > F_VBUSCT, > > > > + F_BADC = F_VBUSCT, > > > > F_AVG, > > > > + F_PGA = F_AVG, > > > > F_RESET, > > > > F_MAX_FIELDS > > > > }; What if to keep the original coding, just to rename the different part like as below F_SADC -> F_RTQ6059_SDAC F_BADC -> F_RTQ6059_BADC F_PGA -> F_RTQ6059_PGA At least, the nameing already shows the difference between 6056 and 6059. Only these three parts are different, others are the same like as F_OPMODE, F_RESET.