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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0F39AC27C53 for ; Fri, 7 Jun 2024 09:23:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=SOHaT4e2vHWVkH4j21OJCnJTaIbKJtbl9zkHHYtSork=; b=4KPz/EwNE7owEW 81LCqbwExr/xFM8HWKFKG/PALvceDA1EqWcowqowpZbaXkhv07tHuEBNtdV4Kru0f7AuOL/pX+kgs W+JqF5t8QDV7be2aOsXg8RfwlT8ENPLcTYfGfEbgwKr00Wp7sD9C9miYHCb0ehAEg5KHrI08uBuMI X2x2IF++h2eLiP9raUaeiy7in64R+62Ke01zX2ggngmS65NcdIpR4a4NR1oPsLzMcbq6+5auVlNLE x1cwAN5YcgBH+ZyDNGplTIzsh1wjr0+OP4QQYhOj4llE4Ucf57ONhDOF7HlmdBsB36qaLXBna5c8F /sYh2vn2sfR+UWqaWtlw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sFVoO-0000000DIK0-4Bdq; Fri, 07 Jun 2024 09:23:05 +0000 Received: from mail-ed1-x534.google.com ([2a00:1450:4864:20::534]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sFVoK-0000000DIHy-2Y4w for linux-mtd@lists.infradead.org; Fri, 07 Jun 2024 09:23:02 +0000 Received: by mail-ed1-x534.google.com with SMTP id 4fb4d7f45d1cf-57a68b0a229so2137665a12.3 for ; Fri, 07 Jun 2024 02:22:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1717752178; x=1718356978; darn=lists.infradead.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=XYf6bGHbHxgNM+h3wZ+n4+ATjohhMtxeUnq9ZlPjUBQ=; b=RN3CCicmstELKWdwjIWNarQK52yR5P/rr+9FpA0be5jaumQY2TsvMtPPva1cTrUzjW EPD76/7s2M+iBpjJCii3xy5P2E3WKxLQLp8We3Pq5ECrYf9MJtac0zhbEanqqN9/H3vI p5lOaWgR8xPy2P/RTeUAfvQaFudTTEwL0EWZiV6Ynlnm9Ijqrb9VQhydkoDA4ntJiLcR KRyzQ7ji/6k/9UUBTnXyrVLI1Zy8Ij3EkuV4XUnr8JMwtzZhei5IxRhO/+Up94SNbdD7 40IuR6hbAdk7Gen8W5BlVMAHzUqkBqgBCSR376VW1gkUBQ8tVE/CVQ6AWZg8Cy/CTZkZ uvdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717752178; x=1718356978; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=XYf6bGHbHxgNM+h3wZ+n4+ATjohhMtxeUnq9ZlPjUBQ=; b=siI1nIMQIOOIWoDe/eABjzORggbMrPJtg7J+cvl0L9+DY4DTRXzb3CyrIZQkcZpxyr TMIHKioNPINqYrpesBpHcjnVNm1yjxEk+uFvz5OIt8jwHus4wo96LSNnCvR5WZCqoixX kZRZZfe+KYkje6TfhlDJGgJMU3L1PCOo7G+w4nMkelFlN8TRBgJLjAfM9X2xjUcrT5UH X3VTDR5obrVMX1UbduqATh3Bz6AlruMlnP9jwxgscbJ2u3HQ4h2xlsBTSr99e8rGBUPj yN6b2PANf1922NyxYL9VUWZfCpXI4/znAgajjDZ5y1Yrw8HUOEICHOJYRC+LSsV3/wkf lCiA== X-Forwarded-Encrypted: i=1; AJvYcCVUYyKq9xCaNKsM3OAtwmUh8wAfRCzN+a62NwlOO2aFs1C9NV7sd8cz6i/OyY2XFam8Ehues3awuBMr+OmkLjkaCsFIFUcbTp9IyPOfzg== X-Gm-Message-State: AOJu0YycbiVcBewgoStvlZNxfwCl+jtDre+AWdtZdmmTzcZ5v2VyQsEH 4Q0wwZfGF2KvjGn4ZSmLqVM5w+INoj+RzEer3pY0bYuJtkRBpsugxK22jqrQY5Q= X-Google-Smtp-Source: AGHT+IG2lEF0vcwlSPmvTy/nHy1p/PsYovaJS1vMKm64l/eji2pqdm/IS5rE0XGcO2f/qHPX3HCDWg== X-Received: by 2002:a50:9b11:0:b0:57c:605c:e23f with SMTP id 4fb4d7f45d1cf-57c605ce464mr179336a12.33.1717752178488; Fri, 07 Jun 2024 02:22:58 -0700 (PDT) Received: from [192.168.2.107] ([79.115.63.17]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-57aae229712sm2456652a12.81.2024.06.07.02.22.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 07 Jun 2024 02:22:57 -0700 (PDT) Message-ID: <1d683b33-16db-4d81-92cb-d98e35b87cba@linaro.org> Date: Fri, 7 Jun 2024 10:22:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/2] mtd: spi-nor: core: add flag for doing optional SFDP To: Esben Haabendal , Michael Walle Cc: Pratyush Yadav , Miquel Raynal , Richard Weinberger , Vignesh Raghavendra , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Rasmus Villemoes References: <20240603-macronix-mx25l3205d-fixups-v2-0-ff98da26835c@geanix.com> <20240603-macronix-mx25l3205d-fixups-v2-1-ff98da26835c@geanix.com> <48719b0f-1a7f-47f9-948a-c981a0a29b41@linaro.org> <874ja6aszh.fsf@geanix.com> Content-Language: en-US From: Tudor Ambarus In-Reply-To: <874ja6aszh.fsf@geanix.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240607_022300_827931_E3446E86 X-CRM114-Status: GOOD ( 31.36 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org On 6/6/24 18:20, Esben Haabendal wrote: > "Michael Walle" writes: > >> On Thu Jun 6, 2024 at 4:52 PM CEST, Tudor Ambarus wrote: >>> On 6/6/24 14:59, Michael Walle wrote: >>>> On Thu Jun 6, 2024 at 3:31 PM CEST, Tudor Ambarus wrote: >>>>> On 6/3/24 14:09, Esben Haabendal wrote: >>>>>> A dedicated flag for triggering call to >>>>>> spi_nor_sfdp_init_params_deprecated() allows enabling optional SFDP read >>>>>> and parse, with fallback to legacy flash parameters, without having dual, >>>>>> quad or octal parameters set in the legacy flash parameters. >>>>>> >>>>>> With this, spi-nor flash parts without SFDP that is replaced with a >>>>>> different flash NOR flash part that does have SFDP, but shares the same >>>>>> manufacturer and device ID is easily handled. >>>>>> >>>>>> Signed-off-by: Esben Haabendal >>>>>> --- >>>>>> drivers/mtd/spi-nor/core.c | 3 ++- >>>>>> drivers/mtd/spi-nor/core.h | 1 + >>>>>> 2 files changed, 3 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c >>>>>> index 3e1f1913536b..1c4d66fc993b 100644 >>>>>> --- a/drivers/mtd/spi-nor/core.c >>>>>> +++ b/drivers/mtd/spi-nor/core.c >>>>>> @@ -2933,7 +2933,8 @@ static void spi_nor_init_params_deprecated(struct spi_nor *nor) >>>>>> >>>>>> spi_nor_manufacturer_init_params(nor); >>>>>> >>>>>> - if (nor->info->no_sfdp_flags & (SPI_NOR_DUAL_READ | >>>>>> + if (nor->info->no_sfdp_flags & (SPI_NOR_TRY_SFDP | >>>>> >>>>> I don't like that we update deprecated methods. The solution though is >>>>> elegant. >>>> >>>> I actually had the same concern. But currently there is no >>>> non-deprecated way to handle this case, right? >>>> >>>> Right now we have the following cases: >>>> (1) pure SFDP parsing >>>> (2) non-SFDP flashes with static configuration only >>>> (3) legacy implementation, where the magic flags decide whether we >>>> use SFDP >>>> >>>> Which case is eventually used depends on the ID of the flash - >>>> assuming there will only be IDs which either fall into (1) *or* (2). >>>> That assumption is clearly wrong :) >>>> >>>> I'd propose a new case in spi_nor_init_params() >>>> (4) try SFDP with a fallback to the static flags from the >>>> flash_info db. >>>> >>> >>> that's not that bad, but I would avoid doing it if it's not common. You >>> also have to update the core a bit, you can't use no_sfdp_flags & >>> TRY_SFDP, it's misleading. Does it worth it? >> >> IMHO no_sfdp_flags is the correct place (maybe TRY_SFDP is wrong, >> maybe SFDP_FALLBACK?) > > TRY_SFDP might not be the best choice. But SFDP_FALLBACK sounds to me > like it is fallback _to_ SFDP, so rather counter-intuitive. > >> because the flash is first treated like in >> case (2). Then SFDP is tried based on that flag. > > It is first treated like in case (2), and then tried for case (1), > falling back to the result from case (2) if/when case (1) fails. > >> Is it worth it? I >> don't know, Esben is doing the development here ;) So up to him. > > I am not sure exactly how it should look like, but I do like the idea > proposed above, case (4). It is easier to describe and understand than > the current legacy implementation. > >>> I won't oppose too much, but to me it feels that we're trying to keep >>> alive a dead man. >> >> Maybe, but we'd have a readily solution if we face a similar >> problem in the future. I'm really not sure, how many flashes there >> are, but I think these magic bits (which tells the legacy >> implementation to try SFDP) will mask quite a few of these. >> I.e. in an ideal world where we could finally drop case (3) and >> you'd need to split the flashes between case (1) or (2), I think >> there will be quite some in (4). > > I like this. Judging by the way Macronix is handling this particular > chip, I strongly assume that there are several other examples of this > for other Macronix parts. Of-course, as long as the original part using > the particular flash id supported SFDP, and all later flashes using the > same id also does, none of this is needed. > okay, let's implement 4/ ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/