From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752154AbdBINw2 (ORCPT ); Thu, 9 Feb 2017 08:52:28 -0500 Received: from smtpout.microchip.com ([198.175.253.82]:43624 "EHLO email.microchip.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751375AbdBINw0 (ORCPT ); Thu, 9 Feb 2017 08:52:26 -0500 Subject: Re: [PATCH 1/1] mtd: spi-nor: remove WARN_ONCE() message in spi_nor_write() To: Brian Norris , Cyrille Pitchen References: <0078578d0f5d2402ac623afabf601d25998f84a9.1481044434.git.cyrille.pitchen@atmel.com> <20161208163109.17429627@bbrezillon> <4ffbad4b-f1a5-a998-fe5b-f73b934435e4@wedev4u.fr> <20170209022347.GO94627@google.com> CC: Boris Brezillon , , , , From: Cyrille Pitchen Message-ID: <4420829e-6d08-111f-685c-65ea7f2d215c@atmel.com> Date: Thu, 9 Feb 2017 14:50:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170209022347.GO94627@google.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: =?Windows-1252?Q?H4sIAAAAAAAAC+NgFrrJKsWRWlGSWpSXmKPExsXCxeXDottTPCfCYEP3?= =?Windows-1252?Q?3dO8FgdeLGSxOHJhLbPF4Yb3zBaXd81hs9jdtIzd4uiee8wWk3e+YXRg?= =?Windows-1252?Q?93iy6SKjx85Zd9k9Ni+p97g5r9Dj8yY5j1dX5rEHsEWxZuYl5VcksGZM?= =?Windows-1252?Q?WNPDWLBNouLY3WksDYyPhbsYuTiEBJYzSvzpnMbexcjBISwQJvF2XVwX?= =?Windows-1252?Q?IyeHiECcxLkluxghah4wSpxc3soGkmAWWMAosW+FPIjNJmAo8fbBUVYQ?= =?Windows-1252?Q?m1fARuL19G3MIDaLgIpE79krjCC2qECExPynq5ggagQlTs58wgJicwoY?= =?Windows-1252?Q?SHx4d5sJYqaBxJFFc1ghbHmJ5q2zweYICahJLGxZAWZLCARKbFn5mAnC?= =?Windows-1252?Q?dpLoOjmRDcK2kzg8/SI7hO0gsX3XFxaYml2fL0P1aktsf7WPFcLWkdh2?= =?Windows-1252?Q?sB+qxlZiz4yJUDPdJR48Wg5l+0rMetgAVRMl0fjsIdMERslZSF6YheTs?= =?Windows-1252?Q?WUjOXsDIvIpR2tnDTzc4TNc1wtnDwEQvNzmjQDc3MTNPLzk/dxMjJJIz?= =?Windows-1252?Q?dzC2dUUcYpTkYFIS5ZUtmBMhxJeUn1KZkVicEV9UmpNafIhRhoNDSYJ3?= =?Windows-1252?Q?XhFQTrAoNT21Ii0zB5hSYNJMHJyHGCU4eJREeN+B1PAWFyTmFmemQ+RP?= =?Windows-1252?Q?MUpKifOuBkkIgCQySvPgei8xikoJ854EWcpTkFqUm1kCEb/FKMbxkInj?= =?Windows-1252?Q?MZMQS15+XqoU0KkMQGDA+IpRnINRSZjXH2QcT2ZeCdyaV0AXMAFdcP30?= =?Windows-1252?Q?LJALShIRUlINjIcX2BWfrj7A16AXUMkgW368cwX/sflv59jW/Ffx5+nv?= =?Windows-1252?Q?ubzgvtuXRUt388e8PLDg9c2fMVUlbj62DO9+r1B5cZSzi3/r6wPRvo/Z?= =?Windows-1252?Q?b3pvPrQosk0gMlbozaGKibKrZomsbmDovCgSN+9AUFvP9ONvnGb9/tOn?= =?Windows-1252?Q?c/TGqx/5XCkbEtuMv/8NVbwbkqPEUpyRaKjFXFScCADHV4P/bAMAAA?= =?Windows-1252?Q?=3D=3D?= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Brian, Le 09/02/2017 à 03:23, Brian Norris a écrit : > On Tue, Dec 13, 2016 at 05:43:52PM +0100, Cyrille Pitchen wrote: >> Le 08/12/2016 à 16:31, Boris Brezillon a écrit : >>> On Tue, 6 Dec 2016 18:14:24 +0100 >>> Cyrille Pitchen wrote: >>> >>>> This patch removes the WARN_ONCE() test in spi_nor_write(). >>>> This macro triggers the display of a warning message almost every time we >>>> use a UBI file-system because a write operation is performed at offset 64, >>>> which is in the middle of the SPI NOR memory page. This is a valid >>>> operation for ubifs. >>>> >>>> Hence this warning is pretty annoying and useless so we just remove it. >>>> >>>> Signed-off-by: Cyrille Pitchen >>>> Suggested-by: Richard Weinberger >>>> Suggested-by: Andras Szemzo >>> >>> Acked-by: Boris Brezillon >>> >> Applied to git://github.com/spi-nor/linux.git > > Do you have any idea on how to handle or communicate this better? I > recall Michal added this because he was adding new write looping that > didn't previously exist; we might create partial-page writes because his > SPI controller was too dumb to make larger ones. > > Anyway, since this is hitting real (and more or less false positive) > use cases, then this patch is probably good. > > Brian > Some people had complained and reported "bugs" about this warning during kernel boot especially when using ubifs. If I remember, someone even suggested that Michal's patch should be reverted but I think except for the WARN_ONCE() his patch is good and should be kept. So I proposed the remove only the WARN_ONCE() line. spi_nor_scan() sets mtd->writesize to 1, so for upper mtd layers such as ubi, it is a valid operation to write less than a page size. Besides, if we changed the mtd->writesize value to nor->page_size for instance, it would have an impact on existing ubi file systems which may then appear as corrupted if I've understood correctly. Maybe Richard can confirm this point? Also, the warning doesn't fix anything: everything works just as before. So IF the driver was buggy before, it is still buggy today. Displaying such an intrusive warning (backtrace and so on) "scares" people (at the first look, we think a kernel oops/crash has occurred) but currently we are not able to propose any solution. In most, if not all, cases this warning is a false positive as for most memories it is valid to write a single page with more than one Page Program commands or to write data starting from the middle of the page. The only hardware limitation at the SPI NOR side is that we can't cross the page boundary. If we would do so, internally the SPI NOR memory would wrap the address instead of incrementing it hence the additional data would be written at the beginning of the same page and not at the beginning of the next one. That why Michal's patch should be kept, IMHO. I hope those additional explanations will help you :) Best regards, Cyrille