Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
Date:   Wed, 16 Aug 2023 13:51:51 +0200
From:   Michael Walle <michael@walle.cc>
To:     Hsin-Yi Wang <hsinyi@chromium.org>
Cc:     Tudor Ambarus <tudor.ambarus@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Bjorn Andersson <andersson@kernel.org>,
        Pratyush Yadav <pratyush@kernel.org>,
        "Miquel Raynal )" <miquel.raynal@bootlin.com>,
        "Richard Weinberger )" <richard@nod.at>,
        "Vignesh Raghavendra )" <vigneshr@ti.com>,
        Rob Herring <robh+dt@kernel.org>,
        linux-mtd@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org,
        cros-qcom-dts-watchers@chromium.org,
        Andy Gross <agross@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>
Subject: Re: [PATCH v2,1/2] mtd: spi-nor: giga: gd25lq64c: Disable quad mode
 according to bus width
In-Reply-To: <20230816104245.2676965-1-hsinyi@chromium.org>
References: <20230816104245.2676965-1-hsinyi@chromium.org>
Message-ID: <6702bac712daab13698b9bb9ad81d49e@walle.cc>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Am 2023-08-16 12:38, schrieb Hsin-Yi Wang:
> gd25lq64c has Quad Enable Requirement flag parsed as
> BFPT_DWORD15_QER_SR2_BIT1_BUGGY in BFPT, even if spi-{rx/tx}-bus-width
> set as non QUAD, eg. 0, 1, 2... Thus quad_enable will not be NULL and
> quad enable (QE) bit will be set to 1 by default. According to
> datasheet[1], if QE bit is enabled, WP pin will become IO pin and the
> system can't use write protection feature, and it's also not 
> recommended
> to set QE bit to 1[1].
> 
> Add a post_bfpt fixup that reads spi-rx-bus-width to remove quad_enable
> if the width is set to below QUAD mode.
> 
> [1]
> https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20220714/DS-00012-GD25LQ64C-Rev3.4.pdf
> page 13
> 
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>  drivers/mtd/spi-nor/gigadevice.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/gigadevice.c 
> b/drivers/mtd/spi-nor/gigadevice.c
> index d57ddaf1525b3..8ea89e1858f9b 100644
> --- a/drivers/mtd/spi-nor/gigadevice.c
> +++ b/drivers/mtd/spi-nor/gigadevice.c
> @@ -33,6 +33,31 @@ static const struct spi_nor_fixups gd25q256_fixups = 
> {
>  	.post_bfpt = gd25q256_post_bfpt,
>  };
> 
> +static int
> +gd25lq64c_post_bfpt(struct spi_nor *nor,
> +		    const struct sfdp_parameter_header *bfpt_header,
> +		    const struct sfdp_bfpt *bfpt)
> +{
> +	struct device_node *np = spi_nor_get_flash_node(nor);
> +	u32 value;
> +
> +	/*
> +	 * Even if spi-{tx,rx}-bus-width is set to DUAL mode, due to the QER
> +	 * flag parsed from BFPT is BFPT_DWORD15_QER_SR2_BIT1_BUGGY, so the
> +	 * quad_enable will be set and QE bit set to 1.
> +	 */
> +	if (!of_property_read_u32(np, "spi-rx-bus-width", &value)) {
> +		if (value <= 2)
> +			nor->params->quad_enable = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct spi_nor_fixups gd25lq64c_fixups = {
> +	.post_bfpt = gd25lq64c_post_bfpt,

No. Please fix it in the core and not just for this part. To me it seems
like a fundamental problem and that commit 39d1e3340c73 ("mtd: spi-nor:
Fix clearing of QE bit on lock()/unlock()") is broken in that regard.
Tudor?

-michael