From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CCD7AC00A8F for ; Tue, 24 Oct 2023 08:59:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234020AbjJXI75 (ORCPT ); Tue, 24 Oct 2023 04:59:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41868 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234015AbjJXI7z (ORCPT ); Tue, 24 Oct 2023 04:59:55 -0400 Received: from mail.3ffe.de (0001.3ffe.de [IPv6:2a01:4f8:c0c:9d57::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B7B4712E for ; Tue, 24 Oct 2023 01:59:52 -0700 (PDT) Received: from 3ffe.de (0001.3ffe.de [IPv6:2a01:4f8:c0c:9d57::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.3ffe.de (Postfix) with ESMTPSA id 2992E133; Tue, 24 Oct 2023 10:59:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=walle.cc; s=mail2022082101; t=1698137990; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZqGqaIFtGkF5J02xsc7tMyJ4K90Kq7KYiL/pwEQrR8A=; b=P19fH+nc6zNGHO+8hat2XBGJCy1s0Bc3pRhGCdjL/AjBOS2bGL69m6+p1IO749fclu25CL cDtYBYZLAZvHnJX8S/6iV8aTjboMOiC1lRzzVSdGfrVxDw4+rkgpNe0yDrANYH5FpfPpkA ifUlCGqFJEOm9SW6y/bGZX3/8A2nf8l8JJ9zo8ORlxAnHJx4Om1+yBD0ZQ5AjB6Zdyfi6E I6XHCX8CvhHzS5tu/DNtPa4zK0GKHCvKRJH7aSGRblMmYVxaZ7buV7/ZoPN0DByGplndGq AqZvLZqUXU2YFZcooc+zrkZUCwtGtbXojoxBLago26fh4NrBjIcU6sdqNiJJ7g== MIME-Version: 1.0 Date: Tue, 24 Oct 2023 10:59:50 +0200 From: Michael Walle To: AceLan Kao Cc: Tudor Ambarus , Pratyush Yadav , Miquel Raynal , Richard Weinberger , Vignesh Raghavendra , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, mika.westerberg@linux.intel.com Subject: Re: [PATCH v2] mtd: spi-nor: Improve reporting for software reset failures In-Reply-To: <20231024074332.462741-1-acelan.kao@canonical.com> References: <20231024074332.462741-1-acelan.kao@canonical.com> Message-ID: <4c11d06931f9b4e29c0e8feafc18e763@walle.cc> X-Sender: michael@walle.cc Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [+Mika] > From: "Chia-Lin Kao (AceLan)" > > When the software reset command isn't supported, we now report it as an > informational message(dev_info) instead of a warning(dev_warn). > This adjustment helps avoid unnecessary alarm and confusion regarding > software reset capabilities. > > v2. only lower the priority for the not supported failure > > Signed-off-by: Chia-Lin Kao (AceLan) > --- > drivers/mtd/spi-nor/core.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 1b0c6770c14e..76920dbc568b 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -3252,7 +3252,10 @@ static void spi_nor_soft_reset(struct spi_nor > *nor) > > ret = spi_mem_exec_op(nor->spimem, &op); > if (ret) { > - dev_warn(nor->dev, "Software reset failed: %d\n", ret); > + if (ret == -ENOTSUPP) It bothers me that we use ENOTSUPP here. We should really use EOPNOTSUPP. The core uses EOPNOTSUPP everywhere except for the intel things. Please have a look at changing that to EOPNOTSUPP. See also: https://lore.kernel.org/linux-mtd/85f9c462-c155-dc17-dc97-3254acfa55d2@microchip.com/ > + dev_info(nor->dev, "Software reset enable command doesn't support: > %d\n", ret); I'm not sure this is helpful. It's only the intel SPI controller which does magic things (instead of just issuing our commands). Mika, do you know wether your controller will do a reset on it's own? I presume so, because AFAIR you have some kind of high level controller which also does SFDP parsing and read opcode handling on their own. > + else > + dev_warn(nor->dev, "Software reset failed: %d\n", ret); > return; > } > > @@ -3262,7 +3265,10 @@ static void spi_nor_soft_reset(struct spi_nor > *nor) > > ret = spi_mem_exec_op(nor->spimem, &op); > if (ret) { > - dev_warn(nor->dev, "Software reset failed: %d\n", ret); > + if (ret == -ENOTSUPP) > + dev_info(nor->dev, "Software reset command doesn't support: %d\n", > ret); I'd leave that as is, because how are the chances that the first one is supported and the second command, isn't? When working with the intel controller, we'll return early after the first spi_mem_exec_op(). -michael > + else > + dev_warn(nor->dev, "Software reset failed: %d\n", ret); > return; > }