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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 78BE0C77B7A for ; Fri, 2 Jun 2023 14:49:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=5G5ggYvEY+/d6K2ORPbh760MxnwhFgLBSeAUpiHihis=; b=MonNP43Hr2wq3j LdQ7zzwiNO9WvbFENOWihG1L2yIIRwqd883mw/yXMd/sjBq8PcXeIZhx4QrAvUacEk7anjqEebIHQ FV/PoVgLCaejnSIkk6yuDa4PO7KhPUZ8l0NkAbxYcIcdTDGu2bjkX7OFJHqUTrOpz52oFcGhNQ6P/ aGgdeHCWj2sYEYXEOqCv0BHUmNM74elOyYM4faZDWB0aSq7VFU9/lgvIEw0d7H0gz/b8yeCUau+cs e0KM2QZfdFnMQUimV5H/aG3niyv2A4Nay6wR7IILhzOp/qpF7t3VGuSYvO0ne/4b57wK+Sq/A2/bu IEZKP2T2b9KJR0hP3BGQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q565h-0079fG-1r; Fri, 02 Jun 2023 14:49:21 +0000 Received: from mga11.intel.com ([192.55.52.93]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q565e-0079eP-1e; Fri, 02 Jun 2023 14:49:19 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1685717358; x=1717253358; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=YPUlxeS32cApP+ZDIUHR9khrq40A8i+TVQo/LyymAAg=; b=PJSC832DQt4fw6Mg5Js8wcQnoYStLOALwkvEMeYopZzIuHh8iZCjJAk/ +/AAOiv75uHIwgyYQD5Ji7mT/YY0rPeKY6o6nk9QcGXe1OKMF7gXuCIS8 aJWUr10jfO2qBbva4GJQ2j2FdEAEof6e2q/r7gacv72cOrJEyyXP0bBUL WwKUSrfkaWMdf566AuCaaJ6AOOwxPLRI9abUUsxLn9fG+680dfs5A/NCr /matzKUYpPadmsihYTzQb1tepNlSyRKCczx2QVSAdPnxdLxRcXqArm+bN aXOfJee3XJOPcxYWGPDg2zdn3BlVyz/VT2dRp083bqned3y31ryjhufMc A==; X-IronPort-AV: E=McAfee;i="6600,9927,10729"; a="353378803" X-IronPort-AV: E=Sophos;i="6.00,213,1681196400"; d="scan'208";a="353378803" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Jun 2023 07:49:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10729"; a="772905903" X-IronPort-AV: E=Sophos;i="6.00,213,1681196400"; d="scan'208";a="772905903" Received: from smile.fi.intel.com ([10.237.72.54]) by fmsmga008.fm.intel.com with ESMTP; 02 Jun 2023 07:49:07 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.96) (envelope-from ) id 1q565Q-000fQj-0X; Fri, 02 Jun 2023 17:49:04 +0300 Date: Fri, 2 Jun 2023 17:49:03 +0300 From: Andy Shevchenko To: Maksim Kiselev Cc: linux-iio@vger.kernel.org, Jonathan Cameron , Lars-Peter Clausen , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Paul Walmsley , Palmer Dabbelt , Albert Ou , Philipp Zabel , Cristian Ciocaltea , Cosmin Tanislav , Miquel Raynal , ChiaEn Wu , Arnd Bergmann , Ramona Bolboaca , Caleb Connolly , ChiYuan Huang , Ibrahim Tilki , William Breathitt Gray , Leonard =?iso-8859-1?Q?G=F6hrs?= , Haibo Chen , Hugo Villeneuve , Mike Looijmans , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: Re: [PATCH v2 1/3] iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC Message-ID: References: <20230601223104.1243871-1-bigunclemax@gmail.com> <20230601223104.1243871-2-bigunclemax@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230601223104.1243871-2-bigunclemax@gmail.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230602_074918_591962_06A0CD4A X-CRM114-Status: GOOD ( 21.66 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Fri, Jun 02, 2023 at 01:30:39AM +0300, Maksim Kiselev wrote: > From: Maxim Kiselev > > The General Purpose ADC (GPADC) can convert the external signal into > a certain proportion of digital value, to realize the measurement of > analog signal, which can be applied to power detection and key detection. > > Theoretically, this ADC can support up to 16 channels. All SoCs below > contain this GPADC IP. The only difference between them is the number > of available channels: > > T113 - 1 channel > D1 - 2 channels > R329 - 4 channels > T507 - 4 channels ... > +struct sun20i_gpadc_iio { > + struct regmap *regmap; > + struct completion completion; > + struct mutex lock; The locks should be explained (what are they for? what do they protect?). > + int lastch; > +}; ... > +static const struct regmap_config sun20i_gpadc_regmap_config = { > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > + .fast_io = true, I forgot if I asked about regmap lock do you need it? > +}; ... > + if (!wait_for_completion_timeout(&info->completion, > + msecs_to_jiffies(100))) { Dunno if it's better to have this parameter to be defined with self-explanatory name. > + ret = -ETIMEDOUT; > + goto err; > + } ... > +err: err_unlock: > + mutex_unlock(&info->lock); > + > + return ret; ... > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = sun20i_gpadc_adc_read(info, chan, val); > + return ret; return sun...; > + case IIO_CHAN_INFO_SCALE: > + /* value in mv = 1800mV / 4096 raw */ > + *val = 1800; > + *val2 = 12; > + return IIO_VAL_FRACTIONAL_LOG2; > + default: > + return -EINVAL; > + } ... > + if (num_channels > SUN20I_GPADC_MAX_CHANNELS) { > + dev_err(dev, "num of channel children out of range"); > + return -EINVAL; > + } Is it really critical error? ... > + channels = devm_kcalloc(dev, num_channels, > + sizeof(*channels), GFP_KERNEL); At least one more parameter can be located on the previous line. > + if (!channels) > + return -ENOMEM; ... > +err_child_out: err_put_child: The goto labels should be self-explanatory of what to expect when goto. > + fwnode_handle_put(node); > + > + return ret; ... > + ret = devm_request_irq(dev, irq, sun20i_gpadc_irq_handler, > + 0, dev_name(dev), info); > + if (ret < 0) Here... > + return dev_err_probe(dev, ret, > + "failed requesting irq %d\n", irq); ... > + ret = devm_iio_device_register(dev, indio_dev); > + if (ret < 0) ...and here, do the positive returned values even possible? > + return dev_err_probe(dev, ret, > + "could not register the device\n"); ... > + { .compatible = "allwinner,sun20i-d1-gpadc", }, Inner comma is not needed. -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel