[{"id":1776415,"web_url":"http://patchwork.ozlabs.org/comment/1776415/","msgid":"<20170927170811.57830c1f@bbrezillon>","list_archive_url":null,"date":"2017-09-27T15:08:11","subject":"Re: [PATCH v3 4/8] mtd: nand: atmel: Avoid ECC errors when leaving\n\tbackup mode","submitter":{"id":63120,"url":"http://patchwork.ozlabs.org/api/people/63120/","name":"Boris Brezillon","email":"boris.brezillon@free-electrons.com"},"content":"On Wed, 27 Sep 2017 10:35:51 +0200\nRomain Izard wrote:\n\n> During backup mode, the contents of all registers will be cleared as the\n> SoC will be completely powered down. For a product that boots on NAND\n> Flash memory, the bootloader will obviously use the related controller\n> to read the Flash and correct any detected error in the memory, before\n> handling back control to the kernel's resuming entry point.\n> \n> But it does not clean the NAND controller registers after use and on its\n> side the kernel driver expects the error locator to be powered down and\n> in a clean state. Add a resume hook for the PMECC error locator, and\n> reset its registers.\n> \n> Signed-off-by: Romain Izard \n> ---\n> Change in v3:\n> * keep the PMECC disabled when not in use, and use atmel_pmecc_resume to\n> reset the controller after the bootloader has left it enabled.\n> \n> drivers/mtd/nand/atmel/nand-controller.c | 3 +++\n> drivers/mtd/nand/atmel/pmecc.c | 22 ++++++++++++++--------\n> drivers/mtd/nand/atmel/pmecc.h | 1 +\n> 3 files changed, 18 insertions(+), 8 deletions(-)\n> \n> diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c\n> index f25eca79f4e5..86c2199380c2 100644\n> --- a/drivers/mtd/nand/atmel/nand-controller.c\n> +++ b/drivers/mtd/nand/atmel/nand-controller.c\n> @@ -2530,6 +2530,9 @@ static __maybe_unused int atmel_nand_controller_resume(struct device *dev)\n> \tstruct atmel_nand_controller *nc = dev_get_drvdata(dev);\n> \tstruct atmel_nand *nand;\n> \n> +\tif (nand->pmecc)\n> +\t\tatmel_pmecc_resume(nand->pmecc);\n> +\n\nnand is used uninitialized here, and atmel_pmecc_resume() should be\npassed a atmel_pmecc object not a atmel_pmecc_user.\n\n\tif (nc->pmecc)\n\t\tatmel_pmecc_resume(nc->pmecc);\n\n> \tlist_for_each_entry(nand, &nc->chips, node) {\n> \t\tint i;\n> \n> diff --git a/drivers/mtd/nand/atmel/pmecc.c b/drivers/mtd/nand/atmel/pmecc.c\n> index 146af8218314..ff09c0f25dd4 100644\n> --- a/drivers/mtd/nand/atmel/pmecc.c\n> +++ b/drivers/mtd/nand/atmel/pmecc.c\n> @@ -765,6 +765,12 @@ void atmel_pmecc_get_generated_eccbytes(struct atmel_pmecc_user *user,\n> }\n> EXPORT_SYMBOL_GPL(atmel_pmecc_get_generated_eccbytes);\n> \n> +void atmel_pmecc_reset(struct atmel_pmecc *pmecc)\n> +{\n> +\twritel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);\n> +\twritel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);\n> +}\n\nIt's not used outside of this file, so it should have a static\nspecifier. Anyway, I wonder why you don't expose atmel_pmecc_reset()\ndirectly instead of creating this atmel_pmecc_resume() wrapper.\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-pwm\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pwm-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2Lmd3bmYz9tXs\n\tfor ;\n\tThu, 28 Sep 2017 01:08:17 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752813AbdI0PIQ (ORCPT );\n\tWed, 27 Sep 2017 11:08:16 -0400","from mail.free-electrons.com ([62.4.15.54]:40087 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752445AbdI0PIO (ORCPT\n\t); Wed, 27 Sep 2017 11:08:14 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 5C475208CD; Wed, 27 Sep 2017 17:08:11 +0200 (CEST)","from bbrezillon (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr\n\t[90.63.216.87])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id C6A8120884;\n\tWed, 27 Sep 2017 17:08:10 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT\n\tshortcircuit=ham autolearn=disabled version=3.4.0","Date":"Wed, 27 Sep 2017 17:08:11 +0200","From":"Boris Brezillon ","To":"Romain Izard ","Cc":"Michael Turquette ,\n\tStephen Boyd , Lee Jones ,\n\tWenyou Yang , Josh Wu ,\n\tRichard Weinberger ,\n\tDavid Woodhouse ,\n\tBrian Norris ,\n\tMarek Vasut ,\n\tCyrille Pitchen ,\n\tThierry Reding ,\n\tRichard Genoud ,\n\tGreg Kroah-Hartman ,\n\tJiri Slaby , Alan Stern ,\n\tLudovic Desroches ,\n\tNicolas Ferre ,\n\tAlexandre Belloni ,\n\tlinux-pwm@vger.kernel.org, linux-usb@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,\n\tlinux-serial@vger.kernel.org, linux-clk@vger.kernel.org","Subject":"Re: [PATCH v3 4/8] mtd: nand: atmel: Avoid ECC errors when leaving\n\tbackup mode","Message-ID":"<20170927170811.57830c1f@bbrezillon>","In-Reply-To":"<20170927083555.16580-5-romain.izard.pro@gmail.com>","References":"<20170927083555.16580-1-romain.izard.pro@gmail.com>\n\t<20170927083555.16580-5-romain.izard.pro@gmail.com>","X-Mailer":"Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Sender":"linux-pwm-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pwm@vger.kernel.org"}},{"id":1776426,"web_url":"http://patchwork.ozlabs.org/comment/1776426/","msgid":"","list_archive_url":null,"date":"2017-09-27T15:32:10","subject":"Re: [PATCH v3 4/8] mtd: nand: atmel: Avoid ECC errors when leaving\n\tbackup mode","submitter":{"id":8236,"url":"http://patchwork.ozlabs.org/api/people/8236/","name":"Romain Izard","email":"romain.izard.pro@gmail.com"},"content":"2017-09-27 17:08 GMT+02:00 Boris Brezillon :\n> On Wed, 27 Sep 2017 10:35:51 +0200\n> Romain Izard wrote:\n>\n>> During backup mode, the contents of all registers will be cleared as the\n>> SoC will be completely powered down. For a product that boots on NAND\n>> Flash memory, the bootloader will obviously use the related controller\n>> to read the Flash and correct any detected error in the memory, before\n>> handling back control to the kernel's resuming entry point.\n>>\n>> But it does not clean the NAND controller registers after use and on its\n>> side the kernel driver expects the error locator to be powered down and\n>> in a clean state. Add a resume hook for the PMECC error locator, and\n>> reset its registers.\n>>\n>> Signed-off-by: Romain Izard \n>> ---\n>> Change in v3:\n>> * keep the PMECC disabled when not in use, and use atmel_pmecc_resume to\n>> reset the controller after the bootloader has left it enabled.\n>>\n>> drivers/mtd/nand/atmel/nand-controller.c | 3 +++\n>> drivers/mtd/nand/atmel/pmecc.c | 22 ++++++++++++++--------\n>> drivers/mtd/nand/atmel/pmecc.h | 1 +\n>> 3 files changed, 18 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c\n>> index f25eca79f4e5..86c2199380c2 100644\n>> --- a/drivers/mtd/nand/atmel/nand-controller.c\n>> +++ b/drivers/mtd/nand/atmel/nand-controller.c\n>> @@ -2530,6 +2530,9 @@ static __maybe_unused int atmel_nand_controller_resume(struct device *dev)\n>> struct atmel_nand_controller *nc = dev_get_drvdata(dev);\n>> struct atmel_nand *nand;\n>>\n>> + if (nand->pmecc)\n>> + atmel_pmecc_resume(nand->pmecc);\n>> +\n>\n> nand is used uninitialized here, and atmel_pmecc_resume() should be\n> passed a atmel_pmecc object not a atmel_pmecc_user.\n>\n> if (nc->pmecc)\n> atmel_pmecc_resume(nc->pmecc);\n>\nAnd yet I thought I correctly tested this code...\n\n:(\n\n>> list_for_each_entry(nand, &nc->chips, node) {\n>> int i;\n>>\n>> diff --git a/drivers/mtd/nand/atmel/pmecc.c b/drivers/mtd/nand/atmel/pmecc.c\n>> index 146af8218314..ff09c0f25dd4 100644\n>> --- a/drivers/mtd/nand/atmel/pmecc.c\n>> +++ b/drivers/mtd/nand/atmel/pmecc.c\n>> @@ -765,6 +765,12 @@ void atmel_pmecc_get_generated_eccbytes(struct atmel_pmecc_user *user,\n>> }\n>> EXPORT_SYMBOL_GPL(atmel_pmecc_get_generated_eccbytes);\n>>\n>> +void atmel_pmecc_reset(struct atmel_pmecc *pmecc)\n>> +{\n>> + writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);\n>> + writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);\n>> +}\n>\n> It's not used outside of this file, so it should have a static\n> specifier. Anyway, I wonder why you don't expose atmel_pmecc_reset()\n> directly instead of creating this atmel_pmecc_resume() wrapper.\n\nI will fix this...","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pwm-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tsecure) header.d=mobile-devices.fr header.i=@mobile-devices.fr\n\theader.b=\"PqMvXj7D\"; \n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"IHEf9Po7\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2MJf64N9z9t3m\n\tfor ;\n\tThu, 28 Sep 2017 01:32:34 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752484AbdI0Pcd (ORCPT );\n\tWed, 27 Sep 2017 11:32:33 -0400","from mail-qk0-f179.google.com ([209.85.220.179]:52323 \"EHLO\n\tmail-qk0-f179.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752030AbdI0Pcb (ORCPT\n\t); Wed, 27 Sep 2017 11:32:31 -0400","by mail-qk0-f179.google.com with SMTP id o77so13669700qke.9\n\tfor ; Wed, 27 Sep 2017 08:32:31 -0700 (PDT)","by 10.200.48.120 with HTTP; Wed, 27 Sep 2017 08:32:10 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=mobile-devices.fr; s=google;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=Sm17Y/k078HA50+jIyUFtsFrVDAi0q8yANlgCdc982w=;\n\tb=PqMvXj7D3h+padP5OR4S94QUx6uvg/RzI8sY/k8HH2jXoa6aWveL2ixkCGfYbSNlHU\n\t2z83lb3puDGjT/RUeIOrz5rJ/+vHkSg1UxhPhNVN99Y0bkzu6hBXVj1+pc+S4UQCKrLP\n\t10T/YR112ZlRLg2vlU22XgoGtKQqKIBhSKPfo=","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=Sm17Y/k078HA50+jIyUFtsFrVDAi0q8yANlgCdc982w=;\n\tb=IHEf9Po7aFkuiVfmft5oV1twEhDvz5kS1OSN3cYKO8mbUnq21juO0/R4nQbPVef+4W\n\tomgbvvpeHKTxOz97TcMc+kWQnDcBV1DJHdD4XwwpP2/cR5MRXiTaz3fFjp4vwsaiTV3L\n\tvcYCyBzdsPPpyJkrR8kvmmD1R8RCM5HNC4tM09o5i4G5ZSAGlKB75zsGMSiKfZXOzAWw\n\tzKeCgk39m7QonfRr0EQ01zYck76P/QTcbfT/o55yxqrHZHNxfqeshRDZ1y6P/5UTFMU3\n\tSF1G0KE2V7VK5dbfxY+ic4apnx0Pjsq22sFEryzJ/AMqtUwdWZsmzhpRDVvkdWlDo5H1\n\tCRhg=="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=Sm17Y/k078HA50+jIyUFtsFrVDAi0q8yANlgCdc982w=;\n\tb=UhNO1va0B8BR8zp7vUPkIHMuhYSul2xIE+BdDa0uRcEtBewm8EM7qum9RmsGZFVSPm\n\tY3kBNEd5cSS5fYl1wfkpbp0pjHokX84x8rTvkLBpVm5y+4nZBB+U41UfOYdHAfGUTqT7\n\trwbUPX5E1MIP2CTYu3N3yp+4nMui67PefQxZ+hO38qfs2vGAQIKcxKmTma5eYPiK6VRK\n\toD49gVYM2Da1CMUit1EZPgJxF7u+72BJUcOkN9XwErhLnAytvtiSVd1N63pIb/zZgNac\n\tNLCLa6Fh0PqLq7ggL8qJmAnPGU++Xo4T6DZNG2VOfwobK0PWB7w7TsQNctUOe6sKomgM\n\tkXAg==","X-Gm-Message-State":"AHPjjUgSaVjoeopw78/tcMGNQUjT4rNMt0oMoBN77szpwkamTiDIJ4Kc\n\tY26ZIK+YzQGh8kV8Ey3BXIAr6cgRj4g89+hYQgopZw==","X-Google-Smtp-Source":"AOwi7QCU750XPNP+UztucKF0g0HTw/NbXD4TdDdONOCDkAUwN+9uZjC6Sfg1u5o67Or/b4mxssYnoONMTKy0dowA6gU=","X-Received":"by 10.55.209.16 with SMTP id s16mr2835038qki.46.1506526350963;\n\tWed, 27 Sep 2017 08:32:30 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170927170811.57830c1f@bbrezillon>","References":"<20170927083555.16580-1-romain.izard.pro@gmail.com>\n\t<20170927083555.16580-5-romain.izard.pro@gmail.com>\n\t<20170927170811.57830c1f@bbrezillon>","From":"Romain Izard ","Date":"Wed, 27 Sep 2017 17:32:10 +0200","X-Google-Sender-Auth":"pReTrqq9fvUFtxS8Rke_Vhngehg","Message-ID":"","Subject":"Re: [PATCH v3 4/8] mtd: nand: atmel: Avoid ECC errors when leaving\n\tbackup mode","To":"Boris Brezillon ","Cc":"Michael Turquette ,\n\tStephen Boyd , Lee Jones ,\n\tWenyou Yang , Josh Wu ,\n\tRichard Weinberger ,\n\tDavid Woodhouse ,\n\tBrian Norris ,\n\tMarek Vasut ,\n\tCyrille Pitchen ,\n\tThierry Reding ,\n\tRichard Genoud ,\n\tGreg Kroah-Hartman ,\n\tJiri Slaby , Alan Stern ,\n\tLudovic Desroches ,\n\tNicolas Ferre ,\n\tAlexandre Belloni ,\n\tlinux-pwm@vger.kernel.org, linux-usb@vger.kernel.org,\n\tLKML ,\n\tlinux-mtd ,\n\tlinux-serial@vger.kernel.org, linux-clk@vger.kernel.org","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-pwm-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pwm@vger.kernel.org"}}]