From patchwork Fri Jan 4 10:31:36 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Igor Grinberg X-Patchwork-Id: 1932481 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 5702A3FF0F for ; Fri, 4 Jan 2013 10:35:03 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Tr4Z2-0007p7-Fc; Fri, 04 Jan 2013 10:31:48 +0000 Received: from softlayer.compulab.co.il ([50.23.254.55] helo=compulab.co.il) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1Tr4Yz-0007oo-5x for linux-arm-kernel@lists.infradead.org; Fri, 04 Jan 2013 10:31:45 +0000 Received: from [62.90.235.247] (port=52359 helo=zimbra-mta.compulab.co.il) by softlayer.compulab.co.il with esmtp (Exim 4.80) (envelope-from ) id 1Tr4Yv-0001JT-PJ; Fri, 04 Jan 2013 12:31:42 +0200 Received: from localhost (localhost.localdomain [127.0.0.1]) by zimbra-mta.compulab.co.il (Postfix) with ESMTP id EDF3D644198; Fri, 4 Jan 2013 12:31:40 +0200 (IST) X-Virus-Scanned: amavisd-new at compulab.co.il Received: from zimbra-mta.compulab.co.il ([127.0.0.1]) by localhost (zimbra-mta.compulab.co.il [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jlepNBsPnc1L; Fri, 4 Jan 2013 12:31:39 +0200 (IST) Received: from [10.1.1.13] (grinberg-pc.compulab.local [10.1.1.13]) by zimbra-mta.compulab.co.il (Postfix) with ESMTP id 49268644194; Fri, 4 Jan 2013 12:31:39 +0200 (IST) Message-ID: <50E6AF88.8050800@compulab.co.il> Date: Fri, 04 Jan 2013 12:31:36 +0200 From: Igor Grinberg Organization: CompuLab Ltd. User-Agent: Mozilla/5.0 (X11; Linux i686; rv:10.0.11) Gecko/20121218 Thunderbird/10.0.11 MIME-Version: 1.0 To: Igor Grinberg Subject: Re: [PATCH v2] ARM: pxa27x: fix ac97 controller warm reset code References: <1357223976-9097-1-git-send-email-mikedunn@newsguy.com> <50E67BAD.7070702@compulab.co.il> In-Reply-To: <50E67BAD.7070702@compulab.co.il> X-Enigmail-Version: 1.3.5 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - softlayer.compulab.co.il X-AntiAbuse: Original Domain - lists.infradead.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - compulab.co.il X-Get-Message-Sender-Via: softlayer.compulab.co.il: acl_c_relayhosts_text_entry: grinberg@compulab.co.il|compulab.co.il X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130104_053145_412558_4163205A X-CRM114-Status: GOOD ( 22.69 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Marek Vasut , Mike Dunn , Mark Brown , Eric Miao , Robert Jarzmik , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On 01/04/13 08:50, Igor Grinberg wrote: > On 01/03/13 16:39, Mike Dunn wrote: >> Change log: >> v2: Heed Igor's admonishment to define a macro instead of using MFP_CFG_OUT >> directly. Thanks. > > Usually, the change log is not useful after applying the patch, so > it should go below the scissors line ("---"). > >> >> This patch fixes some code that implements a work-around to a hardware bug in >> the ac97 controller on the pxa27x. A bug in the controller's warm reset >> functionality requires that the mfp used by the controller as the AC97_RESET_n >> line be temporarily reconfigured as a generic output gpio (AF0) and manually >> held high for the duration of the warm reset cycle. This is what was done in >> the original code, but it was broken long ago by commit >> fb1bf8cd13bfa7ed0364ab0d82f717fc020d35f6 >> ([ARM] pxa: introduce processor specific pxa27x_assert_ac97reset()) >> which changed the mfp to a GPIO input instead of a high output. Looking at this one more time... fb1bf8cd13 ([ARM] pxa: introduce processor specific pxa27x_assert_ac97reset()) also removed the call to pxa_gpio_mode() function which effectively set AF to GPIO and configured the GPIO to output high. (Later b1d9bf1d ([ARM] pxa: remove pxa_gpio_mode() and files) removed the pxa_gpio_mode() function) See below... >> >> Signed-off-by: Mike Dunn > > Thanks! > > Acked-by: Igor Grinberg > >> --- >> arch/arm/mach-pxa/include/mach/mfp-pxa27x.h | 3 +++ >> arch/arm/mach-pxa/pxa27x.c | 4 ++-- >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/mach-pxa/include/mach/mfp-pxa27x.h b/arch/arm/mach-pxa/include/mach/mfp-pxa27x.h >> index a611ad3..8281e17 100644 >> --- a/arch/arm/mach-pxa/include/mach/mfp-pxa27x.h >> +++ b/arch/arm/mach-pxa/include/mach/mfp-pxa27x.h >> @@ -463,6 +463,9 @@ >> GPIO76_LCD_PCLK, \ >> GPIO77_LCD_BIAS >> >> +/* these enable a work-around for a hw bug in pxa27x during ac97 warm reset */ >> +#define GPIO113_AC97_nRESET_GPIO_HIGH MFP_CFG_OUT(GPIO113, AF0, DRIVE_HIGH) >> +#define GPIO95_AC97_nRESET_GPIO_HIGH MFP_CFG_OUT(GPIO95, AF0, DRIVE_HIGH) The DRIVE_HIGH does not really configures the GPIO to output high, but only sets the MFP_LPM_DRIVE_HIGH bit which in turn is only effective in low power modes. This means, that by doing the above, you just configure the MFP for GPIO output, but do not assign it a value, so it gets driven with some undefined value. This is not safe. Can you please, check if the attached patch below does the job? >> >> extern int keypad_set_wake(unsigned int on); >> #endif /* __ASM_ARCH_MFP_PXA27X_H */ >> diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c >> index 8047ee0..616cb87 100644 >> --- a/arch/arm/mach-pxa/pxa27x.c >> +++ b/arch/arm/mach-pxa/pxa27x.c >> @@ -47,9 +47,9 @@ void pxa27x_clear_otgph(void) >> EXPORT_SYMBOL(pxa27x_clear_otgph); >> >> static unsigned long ac97_reset_config[] = { >> - GPIO113_GPIO, >> + GPIO113_AC97_nRESET_GPIO_HIGH, >> GPIO113_AC97_nRESET, >> - GPIO95_GPIO, >> + GPIO95_AC97_nRESET_GPIO_HIGH, >> GPIO95_AC97_nRESET, >> }; >> > diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c index 8047ee0..bbe15b0 100644 --- a/arch/arm/mach-pxa/pxa27x.c +++ b/arch/arm/mach-pxa/pxa27x.c @@ -62,6 +62,11 @@ void pxa27x_assert_ac97reset(int reset_gpio, int on) if (reset_gpio == 95) pxa2xx_mfp_config(on ? &ac97_reset_config[2] : &ac97_reset_config[3], 1); + + if (on) + gpio_direction_output(reset_gpio, 1); + else + gpio_direction_input(reset_gpio); } EXPORT_SYMBOL_GPL(pxa27x_assert_ac97reset);