From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregory CLEMENT Subject: Re: [PATCH 06/17] gpio: mvebu: add suspend/resume support Date: Fri, 31 Oct 2014 08:52:15 +0100 Message-ID: <54533FAF.7010505@free-electrons.com> References: <1414151970-6626-1-git-send-email-thomas.petazzoni@free-electrons.com> <1414151970-6626-7-git-send-email-thomas.petazzoni@free-electrons.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: Received: from down.free-electrons.com ([37.187.137.238]:40335 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751244AbaJaHwW (ORCPT ); Fri, 31 Oct 2014 03:52:22 -0400 In-Reply-To: Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Linus Walleij Cc: Thomas Petazzoni , Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , "linux-arm-kernel@lists.infradead.org" , Tawfik Bayouk , Nadav Haklai , Lior Amsalem , Ezequiel Garcia , "devicetree@vger.kernel.org" , Alexandre Courbot , "linux-gpio@vger.kernel.org" Hi Linus, > >> + switch (mvchip->soc_variant) { >> + case MVEBU_GPIO_SOC_VARIANT_ORION: >> + mvchip->edge_mask_regs[0] = >> + readl(mvchip->membase + GPIO_EDGE_MASK_OFF); >> + mvchip->level_mask_regs[0] = >> + readl(mvchip->membase + GPIO_LEVEL_MASK_OFF); >> + break; > > You are assigning index [0] twice, why? There must be some > reason, and it should be stated in a comment. If the first read > is necessary for hardware reasons, don't assign it but > discard the result. Maybe I missed something but for me these 2 registers are different: one is the _EDGE_ mask and the other the _LEVEL_ mask. Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: gregory.clement@free-electrons.com (Gregory CLEMENT) Date: Fri, 31 Oct 2014 08:52:15 +0100 Subject: [PATCH 06/17] gpio: mvebu: add suspend/resume support In-Reply-To: References: <1414151970-6626-1-git-send-email-thomas.petazzoni@free-electrons.com> <1414151970-6626-7-git-send-email-thomas.petazzoni@free-electrons.com> Message-ID: <54533FAF.7010505@free-electrons.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Linus, > >> + switch (mvchip->soc_variant) { >> + case MVEBU_GPIO_SOC_VARIANT_ORION: >> + mvchip->edge_mask_regs[0] = >> + readl(mvchip->membase + GPIO_EDGE_MASK_OFF); >> + mvchip->level_mask_regs[0] = >> + readl(mvchip->membase + GPIO_LEVEL_MASK_OFF); >> + break; > > You are assigning index [0] twice, why? There must be some > reason, and it should be stated in a comment. If the first read > is necessary for hardware reasons, don't assign it but > discard the result. Maybe I missed something but for me these 2 registers are different: one is the _EDGE_ mask and the other the _LEVEL_ mask. Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com