Re: [PATCH v2,1/2] mtd: spi-nor: giga: gd25lq64c: Disable quad mode according to bus width

From: Tudor Ambarus <tudor.ambarus@linaro.org>
To: Michael Walle <michael@walle.cc>
Cc: Hsin-Yi Wang <hsinyi@chromium.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
Date: Wed, 16 Aug 2023 13:34:47 +0100	[thread overview]
Message-ID: <0011a25a-e096-73ac-9800-9d8e35efdc8b@linaro.org> (raw)
In-Reply-To: <80ec748f37f40ae5c3c3c5d1602681b3@walle.cc>

On 8/16/23 13:22, Michael Walle wrote:
> Hi,
> 
>>> like a fundamental problem and that commit 39d1e3340c73 ("mtd: spi-nor:
>>> Fix clearing of QE bit on lock()/unlock()") is broken in that regard.
>>
>> what's wrong with the mentioned commit?
> 
>         } else if (nor->params->quad_enable) {
>                 /*
>                  * If the Status Register 2 Read command (35h) is not
>                  * supported, we should at least be sure we don't
>                  * change the value of the SR2 Quad Enable bit.
>                  *
>                  * We can safely assume that when the Quad Enable method is
>                  * set, the value of the QE bit is one, as a consequence of the
>                  * nor->params->quad_enable() call.
>                  *
>                  * We can safely assume that the Quad Enable bit is present in
>                  * the Status Register 2 at BIT(1). According to the JESD216
>                  * revB standard, BFPT DWORDS[15], bits 22:20, the 16-bit
>                  * Write Status (01h) command is available just for the cases
>                  * in which the QE bit is described in SR2 at BIT(1).
>                  */
>                 sr_cr[1] = SR2_QUAD_EN_BIT1;
>         } else {
>                 sr_cr[1] = 0;
>         }
> 
> "We can safely assume that when the Quad Enable method..". We cannot, if we
> don't have 4 I/O lines. The quad_enable is just the op how to do it, but not
> *if* can do it. It seems to be missing the same check as the
> spi_nor_quad_enable(). But I'm not sure if it's that simple.
> 

I see. Then extending the if condition should do the trick, as
spi_nor_write_16bit_sr_and_check() is called after setup. Something
like:

if (spi_nor_get_protocol_width(nor->read_proto) == 4 &&
    spi_nor_get_protocol_width(nor->write_proto) == 4 &&
    nor->params->quad_enable)

Is this what Hsin-Yi is hitting?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel