mbox series

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

Message ID 20230815154412.713846-1-hsinyi@chromium.org
Headers show
Series Add a property to override the quad mode | expand

Message

Hsin-Yi Wang Aug. 15, 2023, 3:31 p.m. UTC 
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(+)

Comments

Michael Walle Aug. 15, 2023, 3:59 p.m. UTC | #1 
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
Hsin-Yi Wang Aug. 15, 2023, 5:21 p.m. UTC | #2 
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
>
Krzysztof Kozlowski Aug. 15, 2023, 7:19 p.m. UTC | #3 
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
Hsin-Yi Wang Aug. 16, 2023, 10:48 a.m. UTC | #4 
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
>