Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751824AbdCCMpw (ORCPT ); Fri, 3 Mar 2017 07:45:52 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:34754 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751369AbdCCMpr (ORCPT ); Fri, 3 Mar 2017 07:45:47 -0500 Date: Fri, 3 Mar 2017 13:34:19 +0100 From: Boris Brezillon To: Srinivas Kandagatla Cc: Alban , linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-mtd@lists.infradead.org, Cyrille Pitchen , Richard Weinberger , Marek Vasut , Brian Norris , David Woodhouse , Mark Rutland , Rob Herring , Maxime Ripard , Moritz Fischer Subject: Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API Message-ID: <20170303133419.2383dbce@bbrezillon> In-Reply-To: References: <1488484223-844-1-git-send-email-albeu@free.fr> <1488484223-844-3-git-send-email-albeu@free.fr> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1246 Lines: 33 On Fri, 3 Mar 2017 11:23:16 +0000 Srinivas Kandagatla wrote: > > > + mutex_lock(&mtd_nvmem_list_lock); > > + list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) { > > + if (mtd_nvmem->mtd == mtd) { > > + list_del(&mtd_nvmem->list); > > + found = true; > > + break; > > + } > > + } > > + mutex_unlock(&mtd_nvmem_list_lock); > > + > > + if (found) { > > + if (nvmem_unregister(mtd_nvmem->nvmem)) > > + dev_err(&mtd->dev, > > + "Failed to unregister NVMEM device\n"); > > I will be nice to feedback error to top layer, as it does not make sense > to remove providers if there are active consumers using it. > > del_mtd_device(), unregister_mtd_user() have return values, I see no > reason why notifiers should not return errors. > May be if we should fix the remove() call backs to handle and return errors. It's more complicated than that. What should you do if one of the ->remove() notifier in the middle of the list is returning an error? Some of them have already taken the remove notification into account. Should we call ->add() back on those notifiers? Also, I'm not sure they are all safe against double ->remove() calls, so if we might be in trouble when the removal is retried.