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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0151C433EF for ; Sun, 26 Sep 2021 11:39:28 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id A381B60E9B for ; Sun, 26 Sep 2021 11:39:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A381B60E9B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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:MIME-Version:References:In-Reply-To: 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=i1JlCyosOi6PMbrV/c2xuCNs9xOhXNVnXwUcrCDOa2w=; b=TwzPMMFt5JaVuj 3Y4QcUe4th79rocSlShX3b2BgTAGXsGHaW2nl8tVvEu8gWXbG/lBovwoDxt9b7oYJE8jXRySP+bQq UgtW8DqPe7I+sks1a7P2lmNc86O0ybF3+jGGR+J9eOdsSh6W2OND6LiiuWRe8Zcr65SEtAFilF2Zf AZxzfRQ4kf1osBOKWgBhcS1pj69G1yCF93oHAUSZIvncfvzBbk2M8NDC99E08Rx368OhTrGYLVkFr 9ruj49t886R4fSjtty8Im2Yz00cfVB211znwMhPULeAwHkq6P8Fhwax9oCf5Yy882Hp05V6wAx/Bi Cb9HsFhV5VqHCrchvX+A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mUSTr-000T2f-Mr; Sun, 26 Sep 2021 11:38:03 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mUSTe-000T0b-LS for linux-arm-kernel@lists.infradead.org; Sun, 26 Sep 2021 11:37:52 +0000 Received: from jic23-huawei (cpc108967-cmbg20-2-0-cust86.5-4.cable.virginm.net [81.101.6.87]) (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 C1ED061038; Sun, 26 Sep 2021 11:37:47 +0000 (UTC) Date: Sun, 26 Sep 2021 12:41:37 +0100 From: Jonathan Cameron To: Cai Huoqing Cc: Lars-Peter Clausen , Rob Herring , Shawn Guo , "Sascha Hauer" , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , , , , Subject: Re: [PATCH v6 1/3] iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC Message-ID: <20210926124137.0121a68d@jic23-huawei> In-Reply-To: <20210925020555.129-2-caihuoqing@baidu.com> References: <20210925020555.129-1-caihuoqing@baidu.com> <20210925020555.129-2-caihuoqing@baidu.com> X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.30; 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-20210926_043750_787437_069DA365 X-CRM114-Status: GOOD ( 19.99 ) 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 Sat, 25 Sep 2021 10:05:45 +0800 Cai Huoqing wrote: > The NXP i.MX 8QuadXPlus SOC has a new ADC IP, so add > driver support for this ADC. > > Signed-off-by: Cai Huoqing Hi Cai Huoqing, Having had a 'final' read through of the driver, I am basically happy to merge this after Fabio has had time for another look (plus anyone else who wishes to of course!) There were a few minor things inline though that I'll tidy up whilst applying. If you do a v7 for some other reason please sort these out as well. Thanks, Jonathan ... > +#define IMX8QXP_ADR_ADC_TCTRL(tid) (0xc0 + tid * 4) > +#define IMX8QXP_ADR_ADC_CMDH(cid) (0x100 + cid * 8) > +#define IMX8QXP_ADR_ADC_CMDL(cid) (0x104 + cid * 8) In macros, it is always a good idea to put brackets around any use of parameters so as to avoid potential odd issues due to operator precedence. (0xc0 + (tid) * 4) > +#define IMX8QXP_ADR_ADC_RESFIFO 0x300 > +#define IMX8QXP_ADR_ADC_TST 0xffc ... > + > +struct imx8qxp_adc { > + struct device *dev; > + void __iomem *regs; > + struct clk *clk; > + struct clk *ipg_clk; > + struct regulator *vref; > + struct mutex lock; A lock should have documentation to identify what it's precise scope is. I can add /* Serialise ADC channel reads */ above the lock definition whilst applying if you aren't doing a v7 for other reasons. > + struct completion completion; > +}; ... > + > +static irqreturn_t imx8qxp_adc_isr(int irq, void *dev_id) > +{ > + struct imx8qxp_adc *adc = dev_id; > + Really minor, but the blank line here doesn't help readability much and is inconsistent with the rest of the driver. I might remove this whilst applying if nothing else comes up. > + u32 fifo_count; > + > + fifo_count = FIELD_GET(IMX8QXP_ADC_FCTRL_FCOUNT_MASK, > + readl(adc->regs + IMX8QXP_ADR_ADC_FCTRL)); > + > + if (fifo_count) > + complete(&adc->completion); > + > + return IRQ_HANDLED; > +} > + ... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel