[PATCH 0/4] Add a property to override the quad mode

[PATCH 0/4] Add a property to override the quad mode

[Hsin-Yi Wang]

On gigadevice gd25lq64c, the quad mode is enabled after BFPT is parsed.
According to datasheet[1], Quad enable (QE) bit needs to be set to 0 to
use write protection (WP) pin. It also recommends setting default value of
QE to 0 to avoid a potential short issue.

Add a disable-quad-mode property in devicetree that platform can use it to
override the quad mode status parsed from BFPT to use write protection.

[1]
https://www.elm-tech.com/ja/products/spi-flash-memory/gd25lq64/gd25lq64.pdf
page 13

Hsin-Yi Wang (4):
  dt-bindings: mtd: jedec,spi-nor: Add disable-quad-mode property
  mtd: spi-nor: sfdp: read disable-quad-mode property
  arm64: dts: mediatek: mt8183: disable quad mode for spi nor
  arm64: dts: qcom: sc7180: disable quad mode for spi nor

 Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 7 +++++++
 arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi           | 1 +
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi             | 1 +
 drivers/mtd/spi-nor/core.c                               | 5 +++++
 drivers/mtd/spi-nor/core.h                               | 1 +
 drivers/mtd/spi-nor/debugfs.c                            | 1 +
 6 files changed, 16 insertions(+)

-- 
2.41.0.694.ge786442a9b-goog

[PATCH 1/4] dt-bindings: mtd: jedec,spi-nor: Add disable-quad-mode property

[Hsin-Yi Wang]

Some flash devices, eg. gd25lq64c, enable quad mode by default after
spi_nor_parse_bfpt(). However, the systems using these flash devices may
required the quad mode to be turned off for using write protection or to
avoid a potential short issue[1].

Add a disable-quad-mode property in devicetree that system can use it to
override the quad mode status parsed from BFPT.

[1]
https://www.elm-tech.com/ja/products/spi-flash-memory/gd25lq64/gd25lq64.pdf
page 13

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
index 58f0cea160ef5..4cf1da1108500 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
@@ -72,6 +72,13 @@ properties:
       be used on such systems, to denote the absence of a reliable reset
       mechanism.
 
+  disable-quad-mode:
+    type: boolean
+    description:
+      Some flash devices enables QE bit after BFPT is parsed. However, some system
+      may required quad mode to be disabled to use write protection. This boolean
+      flag is to override the quad enable status parsed from BFPT.
+
   no-wp:
     type: boolean
     description:
-- 
2.41.0.694.ge786442a9b-goog

[PATCH 2/4] mtd: spi-nor: sfdp: read disable-quad-mode property

[Hsin-Yi Wang]

Some flash devices, eg. gd25lq64c, enable quad mode by default after
spi_nor_parse_bfpt(). However, the systems using these flash devices may
required the quad mode to be turned off to use write protection or to
avoid a potential short issue[1].

Add a disable-quad-mode property in devicetree that system can use it to
override the quad mode status parsed from BFPT.

[1]https://www.elm-tech.com/ja/products/spi-flash-memory/gd25lq64/gd25lq64.pdf
page 13

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/mtd/spi-nor/core.c    | 5 +++++
 drivers/mtd/spi-nor/core.h    | 1 +
 drivers/mtd/spi-nor/debugfs.c | 1 +
 3 files changed, 7 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 614960c7d22cc..dcf4ff46c37ae 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2847,6 +2847,11 @@ static void spi_nor_init_flags(struct spi_nor *nor)
 	if (of_property_read_bool(np, "no-wp"))
 		nor->flags |= SNOR_F_NO_WP;
 
+	if (of_property_read_bool(np, "disable-quad-mode")) {
+		nor->flags |= SNOR_F_DISABLE_QUAD;
+		nor->params->quad_enable = NULL;
+	}
+
 	if (flags & SPI_NOR_SWP_IS_VOLATILE)
 		nor->flags |= SNOR_F_SWP_IS_VOLATILE;
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 9217379b9cfef..b06bd97668f3a 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -133,6 +133,7 @@ enum spi_nor_option_flags {
 	SNOR_F_RWW		= BIT(14),
 	SNOR_F_ECC		= BIT(15),
 	SNOR_F_NO_WP		= BIT(16),
+	SNOR_F_DISABLE_QUAD	= BIT(17),
 };
 
 struct spi_nor_read_command {
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index 6e163cb5b478c..c17451ae0931a 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -28,6 +28,7 @@ static const char *const snor_f_names[] = {
 	SNOR_F_NAME(RWW),
 	SNOR_F_NAME(ECC),
 	SNOR_F_NAME(NO_WP),
+	SNOR_F_NAME(DISABLE_QUAD),
 };
 #undef SNOR_F_NAME
 
-- 
2.41.0.694.ge786442a9b-goog

[PATCH 3/4] arm64: dts: mediatek: mt8183: disable quad mode for spi nor

[Hsin-Yi Wang]

Some of the SKUs are using gigadevice gd25lq64c flash chip. The chip
default enables quad mode, which results in the write protect pin set to
IO pin. In mt8183 kukui, we won't use quad enable for all SKUs, so apply
the property to disable spi nor's quad mode.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index 6ce16a265e053..8e4761e2b8ff4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -877,6 +877,7 @@ w25q64dw: flash@0 {
 		compatible = "winbond,w25q64dw", "jedec,spi-nor";
 		reg = <0>;
 		spi-max-frequency = <25000000>;
+		disable-quad-mode;
 	};
 };
 
-- 
2.41.0.694.ge786442a9b-goog

Re: [PATCH 3/4] arm64: dts: mediatek: mt8183: disable quad mode for spi nor

[Eugen Hristev]

On 8/15/23 18:31, Hsin-Yi Wang wrote:
> Some of the SKUs are using gigadevice gd25lq64c flash chip. The chip
> default enables quad mode, which results in the write protect pin set to
> IO pin. In mt8183 kukui, we won't use quad enable for all SKUs, so apply
> the property to disable spi nor's quad mode.

Hi Hsin-Yi,

To me this property, and the way you 'apply' it, makes me think that you 
are using the devicetree as a configuration and not a description of the 
hardware itself.
I think the driver should decide whether to use quad or not depending on 
the situation or the pinout of the device (as in your case quad mode 
overlaps with the WP pin)

Eugen

> 
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>   arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index 6ce16a265e053..8e4761e2b8ff4 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> @@ -877,6 +877,7 @@ w25q64dw: flash@0 {
>   		compatible = "winbond,w25q64dw", "jedec,spi-nor";
>   		reg = <0>;
>   		spi-max-frequency = <25000000>;
> +		disable-quad-mode;
>   	};
>   };
>   

[PATCH 4/4] arm64: dts: qcom: sc7180: disable quad mode for spi nor

[Hsin-Yi Wang]

Some of the SKUs are using gigadevice gd25lq64c flash chip. The chip
default enables quad mode, which results in the write protect pin set to
IO pin. In trogdor platforms, we won't use quad enable for all SKUs, so
apply the property to disable spi nor's quad mode.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 5a33e16a8b677..0806ce8e86bea 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -436,6 +436,7 @@ flash@0 {
 		spi-max-frequency = <37500000>;
 		spi-tx-bus-width = <2>;
 		spi-rx-bus-width = <2>;
+		disable-quad-mode;
 	};
 };
 
-- 
2.41.0.694.ge786442a9b-goog

Re: [PATCH 4/4] arm64: dts: qcom: sc7180: disable quad mode for spi nor

[Doug Anderson]

Hi,


On Tue, Aug 15, 2023 at 8:45 AM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> Some of the SKUs are using gigadevice gd25lq64c flash chip. The chip
> default enables quad mode, which results in the write protect pin set to
> IO pin. In trogdor platforms, we won't use quad enable for all SKUs, so
> apply the property to disable spi nor's quad mode.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index 5a33e16a8b677..0806ce8e86bea 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -436,6 +436,7 @@ flash@0 {
>                 spi-max-frequency = <37500000>;
>                 spi-tx-bus-width = <2>;
>                 spi-rx-bus-width = <2>;
> +               disable-quad-mode;

This seems unnecessary. Unless "tx-bus-width" or "rx-bus-width" is 4
then Quad SPI isn't enabled. You don't need an explicit property since
this can just be inferred from the tx and rx bus width.

-Doug

Re: [PATCH 0/4] Add a property to override the quad mode

[Michael Walle]

Hi, 

>On gigadevice gd25lq64c, the quad mode is enabled after BFPT is parsed.
>According to datasheet[1], Quad enable (QE) bit needs to be set to 0 to
>use write protection (WP) pin. It also recommends setting default value of
>QE to 0 to avoid a potential short issue.

So you are using either dual or single io mode. Why can't you use the device tree property spi-{tx,rx}-bus-width? 

>Add a disable-quad-mode property in devicetree that platform can use it to
>override the quad mode status parsed from BFPT to use write protection.
>
>[1]
>https://www.elm-tech.com/ja/products/spi-flash-memory/gd25lq64/gd25lq64.pdf

should be a link on the vendor Homepage if possible. 

-michael

Re: [PATCH 0/4] Add a property to override the quad mode

[Hsin-Yi Wang]

On Tue, Aug 15, 2023 at 11:59 PM Michael Walle <michael@walle.cc> wrote:
>
> Hi,
>
> >On gigadevice gd25lq64c, the quad mode is enabled after BFPT is parsed.
> >According to datasheet[1], Quad enable (QE) bit needs to be set to 0 to
> >use write protection (WP) pin. It also recommends setting default value of
> >QE to 0 to avoid a potential short issue.
>
> So you are using either dual or single io mode. Why can't you use the device tree property spi-{tx,rx}-bus-width?

I tried setting spi-tx-bus-width and spi-rx-bus-width to either 0 or 1
and WP still doesn't work.
For this chip, quad_enable will be set to spi_nor_sr2_bit1_quad_enable
(QER flag is BFPT_DWORD15_QER_SR2_BIT1_BUGGY)[1]

spi_nor_write_sr_and_check() calls
spi_nor_write_16bit_sr_and_check()[2] and the function sets QE bit if
quad_enable is not NULL.

[1] https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/sfdp.c#L575
[2] https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/core.c#L879

Setting spi-{tx,rx}-bus-width still falls to this function and cases.

>
> >Add a disable-quad-mode property in devicetree that platform can use it to
> >override the quad mode status parsed from BFPT to use write protection.
> >
> >[1]
> >https://www.elm-tech.com/ja/products/spi-flash-memory/gd25lq64/gd25lq64.pdf
>
> should be a link on the vendor Homepage if possible.

Right, https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20220714/DS-00012-GD25LQ64C-Rev3.4.pdf
is a more proper datasheet source. The QE description is also in page
13.

>
> -michael
>

Re: [PATCH 0/4] Add a property to override the quad mode

[Krzysztof Kozlowski]

On 15/08/2023 19:21, Hsin-Yi Wang wrote:
> On Tue, Aug 15, 2023 at 11:59 PM Michael Walle <michael@walle.cc> wrote:
>>
>> Hi,
>>
>>> On gigadevice gd25lq64c, the quad mode is enabled after BFPT is parsed.
>>> According to datasheet[1], Quad enable (QE) bit needs to be set to 0 to
>>> use write protection (WP) pin. It also recommends setting default value of
>>> QE to 0 to avoid a potential short issue.
>>
>> So you are using either dual or single io mode. Why can't you use the device tree property spi-{tx,rx}-bus-width?
> 
> I tried setting spi-tx-bus-width and spi-rx-bus-width to either 0 or 1
> and WP still doesn't work.
> For this chip, quad_enable will be set to spi_nor_sr2_bit1_quad_enable
> (QER flag is BFPT_DWORD15_QER_SR2_BIT1_BUGGY)[1]
> 
> spi_nor_write_sr_and_check() calls
> spi_nor_write_16bit_sr_and_check()[2] and the function sets QE bit if
> quad_enable is not NULL.
> 
> [1] https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/sfdp.c#L575
> [2] https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/core.c#L879
> 
> Setting spi-{tx,rx}-bus-width still falls to this function and cases.

with tx/rx bus width = 2, how quad mode is still possible? IOW, why do
you need new property? You wrote here about driver, but I ask about
bindings.

Best regards,
Krzysztof

Re: [PATCH 0/4] Add a property to override the quad mode

[Hsin-Yi Wang]

On Wed, Aug 16, 2023 at 3:19 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 15/08/2023 19:21, Hsin-Yi Wang wrote:
> > On Tue, Aug 15, 2023 at 11:59 PM Michael Walle <michael@walle.cc> wrote:
> >>
> >> Hi,
> >>
> >>> On gigadevice gd25lq64c, the quad mode is enabled after BFPT is parsed.
> >>> According to datasheet[1], Quad enable (QE) bit needs to be set to 0 to
> >>> use write protection (WP) pin. It also recommends setting default value of
> >>> QE to 0 to avoid a potential short issue.
> >>
> >> So you are using either dual or single io mode. Why can't you use the device tree property spi-{tx,rx}-bus-width?
> >
> > I tried setting spi-tx-bus-width and spi-rx-bus-width to either 0 or 1
> > and WP still doesn't work.
> > For this chip, quad_enable will be set to spi_nor_sr2_bit1_quad_enable
> > (QER flag is BFPT_DWORD15_QER_SR2_BIT1_BUGGY)[1]
> >
> > spi_nor_write_sr_and_check() calls
> > spi_nor_write_16bit_sr_and_check()[2] and the function sets QE bit if
> > quad_enable is not NULL.
> >
> > [1] https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/sfdp.c#L575
> > [2] https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/core.c#L879
> >
> > Setting spi-{tx,rx}-bus-width still falls to this function and cases.
>
> with tx/rx bus width = 2, how quad mode is still possible? IOW, why do
> you need new property? You wrote here about driver, but I ask about
> bindings.
>
This may be a bug in the driver that setting spi-{tx,rx}-bus-width
still enables the QE bit. I proposed another method in the chip's
fixup to deal with this issue instead of creating a new binding
property.
v2: https://lore.kernel.org/lkml/20230816104245.2676965-1-hsinyi@chromium.org/


> Best regards,
> Krzysztof
>