* [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property to avoid setting SRWD bit in status register
@ 2023-06-16 8:55 ` Amit Kumar Mahapatra
0 siblings, 0 replies; 12+ messages in thread
From: Amit Kumar Mahapatra @ 2023-06-16 8:55 UTC (permalink / raw)
To: tudor.ambarus, pratyush, miquel.raynal, richard, vigneshr,
robh+dt, krzysztof.kozlowski+dt, conor+dt
Cc: git, michael, linux-mtd, devicetree, linux-kernel, amitrkcian2002,
Amit Kumar Mahapatra
If the WP signal of the flash device is not connected and the software sets
the status register write disable (SRWD) bit in the status register then
thestatus register permanently becomes read-only. To avoid this added a new
boolean DT property "broken-wp". If WP signal is not connected, then this
property should be set in the DT to avoid setting the SRWD during status
register write operation.
Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
.../devicetree/bindings/mtd/jedec,spi-nor.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
index 89959e5c47ba..10a6df752f6f 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
@@ -70,6 +70,21 @@ properties:
be used on such systems, to denote the absence of a reliable reset
mechanism.
+ broken-wp:
+ type: boolean
+ description:
+ The status register write disable (SRWD) bit in status register, combined
+ with the WP signal, provides hardware data protection for the device. When
+ the SRWD bit is set to 1, and the WP signal is either driven LOW or hard
+ strapped to LOW, the status register nonvolatile bits become read-only and
+ the WRITE STATUS REGISTER operation will not execute. The only way to exit
+ this hardware-protected mode is to drive WP HIGH. If the WP signal of the
+ flash device is not connected then status register permanently becomes
+ read-only as the SRWD bit cannot be reset. This boolean flag can be used
+ on systems in which WP signal is not connected, to avoid setting the SRWD
+ bit while writing the status register. If the WP signal is hard strapped
+ to LOW then it is not broken as it can be a valid use case.
+
reset-gpios:
description:
A GPIO line connected to the RESET (active low) signal of the device.
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property to avoid setting SRWD bit in status register
2023-06-16 8:55 ` Amit Kumar Mahapatra
@ 2023-06-16 9:51 ` Michael Walle
-1 siblings, 0 replies; 12+ messages in thread
From: Michael Walle @ 2023-06-16 9:51 UTC (permalink / raw)
To: Amit Kumar Mahapatra
Cc: tudor.ambarus, pratyush, miquel.raynal, richard, vigneshr,
robh+dt, krzysztof.kozlowski+dt, conor+dt, git, linux-mtd,
devicetree, linux-kernel, amitrkcian2002
Am 2023-06-16 10:55, schrieb Amit Kumar Mahapatra:
> If the WP signal of the flash device is not connected and the software
> sets
> the status register write disable (SRWD) bit in the status register
> then
> thestatus register permanently becomes read-only. To avoid this added a
> new
> boolean DT property "broken-wp". If WP signal is not connected, then
> this
> property should be set in the DT to avoid setting the SRWD during
> status
> register write operation.
>
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../devicetree/bindings/mtd/jedec,spi-nor.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> index 89959e5c47ba..10a6df752f6f 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> @@ -70,6 +70,21 @@ properties:
> be used on such systems, to denote the absence of a reliable
> reset
> mechanism.
>
> + broken-wp:
> + type: boolean
> + description:
> + The status register write disable (SRWD) bit in status register,
> combined
> + with the WP signal, provides hardware data protection for the
> device. When
> + the SRWD bit is set to 1, and the WP signal is either driven LOW
> or hard
> + strapped to LOW, the status register nonvolatile bits become
> read-only and
> + the WRITE STATUS REGISTER operation will not execute. The only
> way to exit
> + this hardware-protected mode is to drive WP HIGH. If the WP
> signal of the
> + flash device is not connected then status register permanently
> becomes
> + read-only as the SRWD bit cannot be reset. This boolean flag can
> be used
> + on systems in which WP signal is not connected, to avoid setting
> the SRWD
> + bit while writing the status register.
This is not true for all flashes, for example, Macronix flashes seem to
have
an internal pull-up, so if the pin is unconnected H/W write protection
is
disabled.
So maybe only mention that "if the pin is wrongly tied to GND (that
includes
internal pull-downs) or it is left floating".
Same goes for your comment in the driver code. Sorry for nitpicking, but
I fear some misuse of this property to disable the locking of the status
register.
> If the WP signal is hard strapped
> + to LOW then it is not broken as it can be a valid use case.
> +
I'm not sure it the bindings use negative notion of pins, because that
signal is usually called WP#.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property to avoid setting SRWD bit in status register
@ 2023-06-16 9:51 ` Michael Walle
0 siblings, 0 replies; 12+ messages in thread
From: Michael Walle @ 2023-06-16 9:51 UTC (permalink / raw)
To: Amit Kumar Mahapatra
Cc: tudor.ambarus, pratyush, miquel.raynal, richard, vigneshr,
robh+dt, krzysztof.kozlowski+dt, conor+dt, git, linux-mtd,
devicetree, linux-kernel, amitrkcian2002
Am 2023-06-16 10:55, schrieb Amit Kumar Mahapatra:
> If the WP signal of the flash device is not connected and the software
> sets
> the status register write disable (SRWD) bit in the status register
> then
> thestatus register permanently becomes read-only. To avoid this added a
> new
> boolean DT property "broken-wp". If WP signal is not connected, then
> this
> property should be set in the DT to avoid setting the SRWD during
> status
> register write operation.
>
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../devicetree/bindings/mtd/jedec,spi-nor.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> index 89959e5c47ba..10a6df752f6f 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> @@ -70,6 +70,21 @@ properties:
> be used on such systems, to denote the absence of a reliable
> reset
> mechanism.
>
> + broken-wp:
> + type: boolean
> + description:
> + The status register write disable (SRWD) bit in status register,
> combined
> + with the WP signal, provides hardware data protection for the
> device. When
> + the SRWD bit is set to 1, and the WP signal is either driven LOW
> or hard
> + strapped to LOW, the status register nonvolatile bits become
> read-only and
> + the WRITE STATUS REGISTER operation will not execute. The only
> way to exit
> + this hardware-protected mode is to drive WP HIGH. If the WP
> signal of the
> + flash device is not connected then status register permanently
> becomes
> + read-only as the SRWD bit cannot be reset. This boolean flag can
> be used
> + on systems in which WP signal is not connected, to avoid setting
> the SRWD
> + bit while writing the status register.
This is not true for all flashes, for example, Macronix flashes seem to
have
an internal pull-up, so if the pin is unconnected H/W write protection
is
disabled.
So maybe only mention that "if the pin is wrongly tied to GND (that
includes
internal pull-downs) or it is left floating".
Same goes for your comment in the driver code. Sorry for nitpicking, but
I fear some misuse of this property to disable the locking of the status
register.
> If the WP signal is hard strapped
> + to LOW then it is not broken as it can be a valid use case.
> +
I'm not sure it the bindings use negative notion of pins, because that
signal is usually called WP#.
-michael
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property to avoid setting SRWD bit in status register
2023-06-16 8:55 ` Amit Kumar Mahapatra
@ 2023-06-16 15:25 ` Rob Herring
-1 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2023-06-16 15:25 UTC (permalink / raw)
To: Amit Kumar Mahapatra
Cc: tudor.ambarus, pratyush, miquel.raynal, richard, vigneshr,
krzysztof.kozlowski+dt, conor+dt, git, michael, linux-mtd,
devicetree, linux-kernel, amitrkcian2002
On Fri, Jun 16, 2023 at 02:25:12PM +0530, Amit Kumar Mahapatra wrote:
> If the WP signal of the flash device is not connected and the software sets
> the status register write disable (SRWD) bit in the status register then
> thestatus register permanently becomes read-only. To avoid this added a new
> boolean DT property "broken-wp". If WP signal is not connected, then this
> property should be set in the DT to avoid setting the SRWD during status
> register write operation.
>
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../devicetree/bindings/mtd/jedec,spi-nor.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> index 89959e5c47ba..10a6df752f6f 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> @@ -70,6 +70,21 @@ properties:
> be used on such systems, to denote the absence of a reliable reset
> mechanism.
>
> + broken-wp:
In the tied low case, that's designed behavior rather than broken. The
name should reflect that.
> + type: boolean
> + description:
> + The status register write disable (SRWD) bit in status register, combined
> + with the WP signal, provides hardware data protection for the device. When
> + the SRWD bit is set to 1, and the WP signal is either driven LOW or hard
> + strapped to LOW, the status register nonvolatile bits become read-only and
> + the WRITE STATUS REGISTER operation will not execute. The only way to exit
> + this hardware-protected mode is to drive WP HIGH. If the WP signal of the
> + flash device is not connected then status register permanently becomes
> + read-only as the SRWD bit cannot be reset. This boolean flag can be used
> + on systems in which WP signal is not connected, to avoid setting the SRWD
> + bit while writing the status register. If the WP signal is hard strapped
> + to LOW then it is not broken as it can be a valid use case.
> +
> reset-gpios:
> description:
> A GPIO line connected to the RESET (active low) signal of the device.
> --
> 2.17.1
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property to avoid setting SRWD bit in status register
@ 2023-06-16 15:25 ` Rob Herring
0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2023-06-16 15:25 UTC (permalink / raw)
To: Amit Kumar Mahapatra
Cc: tudor.ambarus, pratyush, miquel.raynal, richard, vigneshr,
krzysztof.kozlowski+dt, conor+dt, git, michael, linux-mtd,
devicetree, linux-kernel, amitrkcian2002
On Fri, Jun 16, 2023 at 02:25:12PM +0530, Amit Kumar Mahapatra wrote:
> If the WP signal of the flash device is not connected and the software sets
> the status register write disable (SRWD) bit in the status register then
> thestatus register permanently becomes read-only. To avoid this added a new
> boolean DT property "broken-wp". If WP signal is not connected, then this
> property should be set in the DT to avoid setting the SRWD during status
> register write operation.
>
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../devicetree/bindings/mtd/jedec,spi-nor.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> index 89959e5c47ba..10a6df752f6f 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> @@ -70,6 +70,21 @@ properties:
> be used on such systems, to denote the absence of a reliable reset
> mechanism.
>
> + broken-wp:
In the tied low case, that's designed behavior rather than broken. The
name should reflect that.
> + type: boolean
> + description:
> + The status register write disable (SRWD) bit in status register, combined
> + with the WP signal, provides hardware data protection for the device. When
> + the SRWD bit is set to 1, and the WP signal is either driven LOW or hard
> + strapped to LOW, the status register nonvolatile bits become read-only and
> + the WRITE STATUS REGISTER operation will not execute. The only way to exit
> + this hardware-protected mode is to drive WP HIGH. If the WP signal of the
> + flash device is not connected then status register permanently becomes
> + read-only as the SRWD bit cannot be reset. This boolean flag can be used
> + on systems in which WP signal is not connected, to avoid setting the SRWD
> + bit while writing the status register. If the WP signal is hard strapped
> + to LOW then it is not broken as it can be a valid use case.
> +
> reset-gpios:
> description:
> A GPIO line connected to the RESET (active low) signal of the device.
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property to avoid setting SRWD bit in status register
2023-06-16 15:25 ` Rob Herring
@ 2023-06-19 11:49 ` Mahapatra, Amit Kumar
-1 siblings, 0 replies; 12+ messages in thread
From: Mahapatra, Amit Kumar @ 2023-06-19 11:49 UTC (permalink / raw)
To: Rob Herring
Cc: tudor.ambarus@linaro.org, pratyush@kernel.org,
miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
git (AMD-Xilinx), michael@walle.cc, linux-mtd@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
amitrkcian2002@gmail.com
Hello Rob,
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Friday, June 16, 2023 8:56 PM
> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@amd.com>
> Cc: tudor.ambarus@linaro.org; pratyush@kernel.org;
> miquel.raynal@bootlin.com; richard@nod.at; vigneshr@ti.com;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; git (AMD-Xilinx)
> <git@amd.com>; michael@walle.cc; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> amitrkcian2002@gmail.com
> Subject: Re: [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property
> to avoid setting SRWD bit in status register
>
> On Fri, Jun 16, 2023 at 02:25:12PM +0530, Amit Kumar Mahapatra wrote:
> > If the WP signal of the flash device is not connected and the software
> > sets the status register write disable (SRWD) bit in the status
> > register then thestatus register permanently becomes read-only. To
> > avoid this added a new boolean DT property "broken-wp". If WP signal
> > is not connected, then this property should be set in the DT to avoid
> > setting the SRWD during status register write operation.
> >
> > Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > .../devicetree/bindings/mtd/jedec,spi-nor.yaml | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > index 89959e5c47ba..10a6df752f6f 100644
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > @@ -70,6 +70,21 @@ properties:
> > be used on such systems, to denote the absence of a reliable reset
> > mechanism.
> >
> > + broken-wp:
>
> In the tied low case, that's designed behavior rather than broken. The name
> should reflect that.
In that case will it be ok to change it to "no-wp". Please let me know if
you have any other suitable name.
Regards,
Amit
>
> > + type: boolean
> > + description:
> > + The status register write disable (SRWD) bit in status register, combined
> > + with the WP signal, provides hardware data protection for the device.
> When
> > + the SRWD bit is set to 1, and the WP signal is either driven LOW or hard
> > + strapped to LOW, the status register nonvolatile bits become read-only
> and
> > + the WRITE STATUS REGISTER operation will not execute. The only way
> to exit
> > + this hardware-protected mode is to drive WP HIGH. If the WP signal of
> the
> > + flash device is not connected then status register permanently becomes
> > + read-only as the SRWD bit cannot be reset. This boolean flag can be
> used
> > + on systems in which WP signal is not connected, to avoid setting the
> SRWD
> > + bit while writing the status register. If the WP signal is hard strapped
> > + to LOW then it is not broken as it can be a valid use case.
> > +
> > reset-gpios:
> > description:
> > A GPIO line connected to the RESET (active low) signal of the device.
> > --
> > 2.17.1
> >
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property to avoid setting SRWD bit in status register
@ 2023-06-19 11:49 ` Mahapatra, Amit Kumar
0 siblings, 0 replies; 12+ messages in thread
From: Mahapatra, Amit Kumar @ 2023-06-19 11:49 UTC (permalink / raw)
To: Rob Herring
Cc: tudor.ambarus@linaro.org, pratyush@kernel.org,
miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
git (AMD-Xilinx), michael@walle.cc, linux-mtd@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
amitrkcian2002@gmail.com
Hello Rob,
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Friday, June 16, 2023 8:56 PM
> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@amd.com>
> Cc: tudor.ambarus@linaro.org; pratyush@kernel.org;
> miquel.raynal@bootlin.com; richard@nod.at; vigneshr@ti.com;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; git (AMD-Xilinx)
> <git@amd.com>; michael@walle.cc; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> amitrkcian2002@gmail.com
> Subject: Re: [PATCH v2 1/2] dt-bindings: mtd: jedec, spi-nor: Add DT property
> to avoid setting SRWD bit in status register
>
> On Fri, Jun 16, 2023 at 02:25:12PM +0530, Amit Kumar Mahapatra wrote:
> > If the WP signal of the flash device is not connected and the software
> > sets the status register write disable (SRWD) bit in the status
> > register then thestatus register permanently becomes read-only. To
> > avoid this added a new boolean DT property "broken-wp". If WP signal
> > is not connected, then this property should be set in the DT to avoid
> > setting the SRWD during status register write operation.
> >
> > Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > .../devicetree/bindings/mtd/jedec,spi-nor.yaml | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > index 89959e5c47ba..10a6df752f6f 100644
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > @@ -70,6 +70,21 @@ properties:
> > be used on such systems, to denote the absence of a reliable reset
> > mechanism.
> >
> > + broken-wp:
>
> In the tied low case, that's designed behavior rather than broken. The name
> should reflect that.
In that case will it be ok to change it to "no-wp". Please let me know if
you have any other suitable name.
Regards,
Amit
>
> > + type: boolean
> > + description:
> > + The status register write disable (SRWD) bit in status register, combined
> > + with the WP signal, provides hardware data protection for the device.
> When
> > + the SRWD bit is set to 1, and the WP signal is either driven LOW or hard
> > + strapped to LOW, the status register nonvolatile bits become read-only
> and
> > + the WRITE STATUS REGISTER operation will not execute. The only way
> to exit
> > + this hardware-protected mode is to drive WP HIGH. If the WP signal of
> the
> > + flash device is not connected then status register permanently becomes
> > + read-only as the SRWD bit cannot be reset. This boolean flag can be
> used
> > + on systems in which WP signal is not connected, to avoid setting the
> SRWD
> > + bit while writing the status register. If the WP signal is hard strapped
> > + to LOW then it is not broken as it can be a valid use case.
> > +
> > reset-gpios:
> > description:
> > A GPIO line connected to the RESET (active low) signal of the device.
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread